Improved: now supports logging in as alternate user plus comma separated values, IP ranges and wildcards

EDIT: The version I posted had a bug I didn't spot which, though it matched the IPs fine, failed to log in!

Please use the version in the next comment, not the one directly below...

Here's the working version...

EDIT: Please use the version in the next comment, as it's loads better!

Hello again...

In addition to the above, my client needed the ability to have auto IP logged-in users be able to log in as another user. So, based on the great work in #698758: Need a way to switch users or log out (thanks guys!) I have integrated their approach into my improved version that can handle IP ranges etc.

This new version (attached) adds a menu item 'Login as a different user' plus a nice link on the 'auto-login complete' message that, when clicked, clears the session and goes to the login screen.

It uses hook_form_alter to change the login block and page to add a 'Log in automatically' link at the top for users that have already PASSED the IP login check. I've tried also to avoid the overheads of checking too much, but this needs further testing.

The main issues with this version are: a) the code now needs a gentle re-factor & tidy up to hit Drupal coding standards; b) the Drupal-standard 'logout' links clash with the 'Login as a different user' - anyone know how to override the normal 'logout' behaviour/menu item?; and c) needs Drupal community testing/fixing!

It's a little unpolished but seems to work. The attached code represents a much changed/improved version of this module (so much has changed that a patch is pointless!) - please do let me know how you get on...

----

Note that this version is supported by me because it is live on a busy site requiring these features. Any bug fixes or enhancements will be posted to this thread and once this attached version has been tested properly I'll contact the module maintainer to get this - or a version that is similar - released.

EDIT: There's another, better version a bit lower down, use that one instead.

I think it's a caching issue as it works fine with Drupal's caching disabled but on 'normal' the login as a different user error happens. I'll take a look.

And I was thinking about adding some admin options and your suggestions are good ideas, so I'll try to add some of them too.

I'm a bit busy for the next week but will try to get an improved/fixed version done soon.

FYI I'm working updating this code right now to use hook_boot and the caching issue outlined in #5... Once it's tested I'll post back here.

More soon!

Ok, a fixed/improved version is attached.

This is a mild refactor of the code to tidy up some definitions in introduced and use hook_boot instead of hook_init (See #509028: doesn't log in until after second page load) to avoid caching and (hopefully) improve performance. It also appears in my first batch of testing that it's stopped the caching problem noted in #5. I also ran it through Coder module and fixed a few things.

The 'log in as another user' has been fixed, with its menu item being ditched in favour of hook_user method, so I've managed to get the standard Drupal logout mechanism calling my code which logs out and sets session flag to avoid automatically logging back in. It seems to work much better than before, though there's a chance this module could stop other modules' hook_user($op == logout) from being called... Probably depends on module weights and may not even be an issue - or at least a rare corner case.

I'm now going to add some admin settings based on suggestions by generalelektrix in #5. I have also contacted the module's maintainer, davidwhthomas, to get his input as I hadn't yet done so (my bad, just very busy) - hopefully he/I/we can begin the process of getting these improvements folded into a new DEV release.

Please test and let me know it goes... Any suggestions or issues post here please!!!

Thought I'd post these here...

Settings I'm suggest adding to the module's admin screen

1. [x] (checkbox) "Allow users who are logged in by IP to log out and choose another user." -- this would ideally disable the logout menu option, or failing that override the 'log in as another user' feature to force users to log back in immediately, essentially preventing logging out.
2. Log in by IP automatically links & text:
   • [x] Show IP Login link in Navigation block
     
     Text to use for link [ text box, currently "Log in automatically" ]
   • [x] Show IP Login link on User Login block
     
     Text to use for link [ text box, currently "Log in automatically" ]
   • [x] Show IP Login link on User Login page
     
     Text to use for link [ text box, currently "Log in automatically" ]
     
     Explanatory help text underneath link [ text box, currently "Your computer's IP address has been matched and validated." ]
3. Destination (Drupal path) of after login [ text box, normally "user" ]

Other To Dos on my list:

• (DONE in my next version) #818022: replace $_SERVER['REMOTE_ADDR'] with ip_address() to handle proxy servers better
• Check usage of $GLOBALS['conf']['cache'] in ip_login_attempt_login() - not sure it's needed, or if it even does what I think! must test...
• Possibly integrate with Content Profile as source of IP addresses for users?

If anyone has any comments or other simple admin screen settings, please post them.

Did you flush the caches and rebuild the menu first? I changed some menu entries so that might explain why you're seeing links for /ip_login_as_different_user (which is no longer in the code) instead of just using the existing /logout page... Let me know - that page you mention should not exist.

All your points regarding admin options are taken on board, including a weight option for "Log in automatically" link on the login page/block. Thinking about it, I'll probably just make the system hide text/links if the admin field is left empty to save messing around with a load of extraneous check boxes.

More soon,

Jim

Ok, yet another version - and this one is good! I have:

• Implemented hook_perm and added two permissions: "administer ip login" self explanatory; "can log in as another user" which allows users who are logged in by IP to log out and choose another user. Users who don't have this permission are simply logged straight back in again, which is what the current release version of this module does. I'm not able to hide 'logout' via hook_menu_alter, so will perhaps later inject some CSS to hide the 'Log out' menu link... Not sure, seems brittle - ideas?
• Fixed the improper (D5) use of url() I cut/pasted from ipAuthenticator (oops!)
• Implemented #818022: replace $_SERVER['REMOTE_ADDR'] with ip_address() to handle proxies properly
• Tried to ensure security where possible using filter_xss_admin() and adding an admin screen warning if the chosen profile field's visibililty isn't "Hidden".
• Tried to ensure the interface is completely translatable.
• Updated admin settings:
  • Moved admin menu settings code to ip_login.admin.inc file for performance and clarity.
  • Implemented hook_help
  • Added options for link text and visibility for Navigation menu item, and login page and block.
  • Added option for different destination after IP login to override the user's original requested page
  • Added weight options for auto-login links so they can be placed anywhere within the login page/block's form

I haven't yet added the option to log in automatically to the menu as I'm still not sure the best way to do this for now.

Just added a failsafe so that people can still override the auto login if running from their own computer (i.e. their IP is 127.0.0.1)... Caught myself out and couldn't break out of a test account without hacking the code!! At least the permission checks work! ;-)

I guess the code should also check the server's IP too, and allow requests from that machine to log in, though that's slightly less important because servers will be 127.0.0.1 to themselves anyway.

So the _ip_login_can_use_alternate_account should have this extra line before the return line:
if (ip_address() == '127.0.0.1') return TRUE;

Oh and PLEASE REMEMBER to clear your caches and rebuild the menus to avoid weird errors...

@msamavi:

#17 - This module is not client side or browser dependent, there can be no issue with it under Chrome. I use Chrome under Linux and have seen no browser-related issues at all.

#18 - Works great for me - if a request comes in from an IP-match user wanting, say, http://www.example.com/somepage they get logged in and sent back to http://www.example.com/somepage?ip_login_no_cache=b8fe4285c0c0de2b03af76... (or some other md5 hash). Are you sure the field in the admin screen is completely blank? All the code is doing is getting the original request and sending it though Drupal's url() function. It works as far as I can see, and I can't see a way for it not to...

There are permissions to be set to allow users to log in as another user... Please check them and use v5 if possible as it's generally better. Full instructions on the top of the admin screen.

As for the contradictory messages, I think it's just that the security on logging out needs a little tweak since I made some last minute changes. By default users, except Administrators and those coming from 127.0.0.1 CANNOT log out without being logged back in - this is what the current LIVE version of the module does. Once you've set the permissions for the roles that can log out and log in as another user, these issues will go away.

I'm working on v6 now, mainly tweaks and bug fixes - more in a few days.

Hi generalelektrix, no that's not normal - it's a bug I literally just fixed the minute before I saw your message!

v6 attached, should fix a couple of weird issues like the ones you mentioned, plus tidy things up a little and use Drupal's t() function more appropriately.

That should be an admin screen option? In the 'Login page link' and 'Login block link' sections at www.example.com/admin/settings/iplogin...

I've checked the code and all occurrences of "Log in automatically" are wrapped in t() and therefore - apart from the admin screen settings above - should be translatable. Maybe also try flushing the caches again if you're still not seeing it.

There are a couple of minor issues with v6 so v7 will be along sometime this week. If you have any suggestions for other improvements, let me know.

Thanks generalelektrix, but after a bit of research, that approach is not recommended; from http://api.drupal.org/api/function/t/6 :

However tempting it is, custom data from user input or other non-code sources should not be passed through t(). Doing so leads to the following problems and errors:

The t() system doesn't support updates to existing strings. When user data is updated, the next time it's passed through t() a new record is created instead of an update. The database bloats over time and any existing translations are orphaned with each update.
The t() system assumes any data it receives is in English. User data may be in another language, producing translation errors.
The "Built-in interface" text group in the locale system is used to produce translations for storage in .po files. When non-code strings are passed through t(), they are added to this text group, which is rendered inaccurate since it is a mix of actual interface strings and various user input strings of uncertain origin.

This means strings users enter through the interface should not be translated... As is the case in Drupal core (e.g. http://api.drupal.org/api/function/system_site_information_settings/6). So the current approach is correct, and users must manually override these strings in the admin interface because the defaults are translatable from English automatically - IF the correct .pot file exists (e.g. fr.pot). Does that make sense? I've never done translations...

However, if you felt like contributing to help this module it would be amazing if you fancied making an actual translation of its interface to French using Drupal's inbuilt methods - See http://drupal.org/translators and http://drupal.org/node/220341... It's a big ask, I know, so no pressure whatsoever, but that would really improve things for other French speakers. If you're interested, shout, and I'll post v7 which includes some grammatical improvements.

Hi generalelektrix, thanks for all that... I'm just tweaking the module and will post v7 shortly.

Having thought and looked about more, I think your original solution in #27 is the right one. I mean, it's not the 'correct' way to do it, but then that means Drupal's translation methods don't support user entered strings without your method, so I will move the t() outside the variable_get() method.

I'll also look into getting that .pot file you posted into the module - thanks so much for that! More later.

And on your last point: 10.*.*.* is already supported - wildcards and ranges can be in any quadrant of the IP address except the first. Can you confirm it works please (does at home when I use 127.*.*.*)

v7 attached!... Thanks for all the .po work! A only a few strings have changed since the last version, just for clarity and grammar, and I've also wrapped the help text in t(), too. I also fixed those translation issues you raised in #29.

I've spoken to David (the maintainer) via email and he said he was happy to see the module evolve and thanked me for the work I've done. He had reservations about the security of allowing wildcards and ranges in IPs, and suggested using Regular Expressions instead. I said many use-cases for this module (intranets or large institutional clients) need an IP range/wildcard and most web admins do not know regexp and so that would represent a serious barrier to use for many people. I suggested that RegExp should be an option, not a replacement for more human-readable IP matching. As a result of that I also suggested (then added) the code to warn users about having the IP address profile field set to anything other than 'hidden', plus I re-checked my IP matching code and cannot see a way to corrupt it really since any strings that don't contain numbers will be thrown out.

I also offered to become a co-maintainer of the module since I have a paying client who needs this module and am keen and getting pretty experienced at Drupal coding... No response to that or any of the points above as yet. I'm sure he's busy and will get back to us when he has a moment - I know he's got his eye on the issues list so hopefully he'll weigh in shortly... Hello David?!

Anyway, I feel this version of the module is a huge improvement in many ways from the 'official' version - I really hope it can become a 6.x DEV even a v2 alpha/beta release at some point soon. I'm also happy to help updated the module page's documentation if needed.

v8, with changes you requested in #35 and the French translation, thank you... I think the module is pretty much done - hidden bugs aside - isn't it?

Hi David, (#38)

Great that you're thinking about committing this soon - I'm really glad to have been able to contribute.

hook_boot solved the caching issue nicely, yes, but as you say it meant I had to manually boostrap Drupal to get access to all the relevant functions. The reason I added the full bootstrap was because of the call to url() in ip_login_login - I tried lower levels of bootstrap but it didn't work. I realise it's not the ideal choice - indeed, I did start by trying to include_once the right files, but each needed ever more dependencies so in the end I just boostrapped the whole lot as it felt less hacky, if perhaps not as efficient. If anyone has better suggestions or ideas, I'm all ears...

The module is definitely longer... However, the code that does the hard work of IP checking is very small (smaller than before, tidier definitely). After the 'core' there's a function for logging in as another user and some hooks for overriding the login blocks/page and providing better help. After that, the rest of the code lives in the .admin.inc which is only called for the admin page. I certainly don't think it's at all bloated as it sticks to the core purpose of the module but fixes a couple of omissions from the original while adding administrative flexibility so people don't need to override basic theme methods. The 'general user' is, given the types of users with known, fixed IPs: intranet users, home developers, institutional users/clients etc. I feel the new features help all categories of user - indeed, everything I added except a few admin screen options was needed by my client and others on this issue tracker. Is there anything specific you feel doesn't belong?

@Derry (#39): I'd just install the current version of IP login (as it sounds like you already have), then extract the improved v8 zip file over the top of it... Or just install v8 directly - in either case the ip_login folder with the files inside should live in /sites/all/modules. In other words, using a test/dev version of a module is exactly the same as using a release version as far as installation is concerned. Good luck.

@msamavi - Interesting, but that's a pretty big feature request that takes the module in a new direction. Please open a new 'feature request' on the IP Login issue tracker so it can be discussed separately and perhaps integrated if the community thinks it's a goer.

@davidbessler: "Is there a way to have it actually automatically log in if the ip address is matched instead of bringing me to the login screen where I can click on the "automatically log me in" link? Am I doing something wrong?"

It's working as designed, and behaves _exactly_ like the old module EXCEPT: if you've logged out a flag is set in the session to allow you choose another account rather than be logged in automatically... This only happens for users with the correct permission AND/OR visitors from 127.0.0.1 to avoid being locked out of the admin section of your own site (as happened to me)! When I try it on my machine (using an incognito window on Chrome) I'm automatically logged directly as expected.

I would: clear your caches to be sure then open the site in a clean session (using incognito mode or equivalent for Firefox etc) and it'll work fine - UNTIL you log out, in which case the 127.0.0.1 test will set a flag so you get the option to choose another account rather than auto-logging in.

It's a deliberate safety feature to avoid users with 'administer' permissions or home developers being locked out of the site. Your temporary solution is simply bypassing these checks I added and logging in directly.

Please confirm...

Hi David, could you explain exactly the process, your settings and what isn't working for you? Are you still using your 'hack' from #44? I'd remove it if so as it could short-circuit the process... Have you set up the ip_login permissions correctly? Have you disabled Block Caching (perhaps the cause of the missing option)?

Version 8 is working fine for me and, I believe, generalelektrix so without being able to reproduce your problem(s) I currently don't understand what's going on. I will certainly try to look into this as soon as you reply with more steps/details though.

The 'log in as another user' feature is required by me and several others and I think v2 above is the last updated version that has multiple IPs/ranges/wildcards but not that feature, however it's buggy and had a problem with caching.

Let me know, keen to get to the bottom of this.

@David: actually about 10 minutes after posting the above my client said one of their users is having an issue similar to what you describe. Please do post any/all information you have to help me track this little bugger down! Thanks.

A little update: My client's issue does not appear related to IP login at this stage, but we're still investigating that one and will keep you posted.

I've now tested and re-tested the various code paths around auto login, logout and registration etc on my DEV machine but am yet to see this issue. As soon as I get more information I'll certainly be on this like a fat kid on a tasty tasty cake...

---------------

@David/#52 - your idea is a good one, but it's a new feature - I saw nothing in the original code that could do what you're asking. Anyway, it's a nice idea and could be implemented in one (or probably both) of these ways:

1. A timestamp in the session that, once expired, will auto-login on the next page request.
2. Additionally/alternatively some client-side JavaScript/jQuery that puts a little I'm thinking a little countdown timer in the login block/page that then sends the to the user/login_by_ip page when it gets to 0. It might be annoying for users to be pulled away from what their doing though, so perhaps that's either an admin option or just a timer telling the user that they will be auto-logged in as per (1) above?

There would need to be a new admin screen field to configure the timeout variable too... Has anyone got any further thoughts on this?

OK, there are too many issues in this thread now and I don't have time to look at any of them for at least the next week due to other commitments.

So I propose:

1. Version 8 is moved into the 6.x-2.x-dev branch and continues along side the current version 1 until it's proven and can go to beta, rc or 2.0 full release. This allows other people to submit patches again, too.
2. The 3 related related issues (auto-login timer, specific pages login occurs, possible intermittent bug) raised in this issue ticket are split out into separate ones, then this issue is closed so the issue tracker works properly again for the new branch.
3. I think it would be good if were made co-maintainer of this module since I've largely rewritten it, have a paying client needing my rewritten verison, have 3 years Drupal and 12 years web development experience, am keen and am offering my help. Plus I'm on Drupal.org and developing most days, too.

What do you think David? Anyone?

Hi vilepickle... That line is a belt-and-braces- approach to ensure a session is completely destroyed before logging an account in and redirecting them. ipAuthtenticator (where I believe that snippet of code got copied from) had it, and it makes sense in a paranoid way. The code is:

// Destroy the current session, if we have one
if (session_name() <> '') session_destroy();

And there are two solutions:

1. Remove/Comment out the line - which will remove this extra safety but probably cause little or no side effects in the real world.
2. Make sure the IF statement actually properly detects the case where the session is not initialised, and therefore doesn't try to destroy it unless it's actually there.

So you can certainly remove that line in the mean time if you don't want to have the logged errors. I'd rather we did 2) though. I'm going to roll version 9 in the couple of next weeks. It will have a couple of very minor tweaks and some extra diagnostics in place.

The nebulous bug mentioned earlier in this thread (#49) has not materialised for my client at all, nor for me during my testing, and since no one else has complained or provided any information and the processes used by this version are mostly the same as the original module, I'm now of the opinion no such issues exists. But then having a proper issue tracker would be NICE so others could get involved!

It would be really great if David Thomas (davidwhthomas, the module maintainer) could let us all know his plans and answer the points in #56 soon. Hello, David? There's at least 5 (probably more) people using this version of the module, when can we move this to the 6.2.x branch so we can all contribute quickly and clearly please?

Hi All, just a quickie to say I've just finished a bunch of hectic work and am about to move house... I will try to get v9 up and answer any recent questions later this week. Sorry for the delay, Jim

Hi guys.. Am moving house for the next couple of days but afterwards the workload is much lighter - will get on this ASAP. Jim

@All: Feel like I'm perpetually putting this off, really sorry guys. I'm still somewhat 'homeless' and my broadband in my temporary office is, frankly, shit. If anyone wants to debug in the mean time, go for it! A comparison of the differences between v6, v7 and v8 might show the cause of issue... Otherwise I hope to be more settled and ready on this for next week. Thanks for your patience.

@TDobrowolski: This module doesn't really work like that. It associates an IP address (or range etc) with a user account. Since the account must already exist, it will already have roles assigned. It's beyond the remit of this module to assign another role, though there are modules that could do this. (Perhaps this module could use Rules integration or something? Hmmm...) In any case, fine control over roles based on IP isn't what this module does, sorry.

Best,

Jim

Hi Guys, I've finally moved house and got broadband... I'm plugged back into the matrix!

@phrancescot: Great, glad it's working for you. I've still never had a reported problem from my client either and haven't seen an issue with v8 myself...

And I've got the co-maintainer say-so from David who maintains the module presently - http://drupal.org/node/943480 - so it's definitely on my list of things to do to very soon to ask for a CVS account and submit this either a new 1.x version or a new 2.x.dev branch...

FYI I've now applied for a Drupal CVS account here: http://drupal.org/node/999254

Note the latest v9 is now attached here: http://drupal.org/node/999254#comment-3832358 - only minor tweaks and typos really, plus implementation of hook_uninstall().

Hi Justin,

#78: thanks for the input - I'll amend on my next pass.

#79: I've had an uber-busy few months and really want to get back to this... Should have chance soon...

My dilemma is basically revolving around how to move forward; do I go for a 2.x branch which includes the work in this issue or removes the dependency on the core Profile module, thus easing a D7 version, or do I just commit v9 from here as 6.x-DEV knowing that branch will be abandoned later this year...

It's not that I can't do both, it's that I have less time than I want. #1087822: IP Login incompatible with SecurePages Hijack Prevention module is a concern too. If you have a preference shout now! I haven't had the headspace to think too much about it.

In either case, I should hopefully be free to commit something late this month/early next.

Thanks for the catch Justin! Latest version of DEV attached.

FYI the roadmap is roughly:

1. Remove reliance on Profile module,
2. Submit 6.2.x-DEV branch,
3. Get feedback, fix, tweak
4. Submit 7.2x-DEV branch,
5. Wait a bit, fix bugs, go for proper 2.0 release

I hope to do 1) and 2) before the end of the month.

v11 including proper hook_user calls which fixes #1128382: Does not work with with Rules? attached...

I am pleased to announce a new 6.x-2.x-DEV branch has been committed! Merry xmas!

This committed version is v11 from the above post, but obviously now integrates properly with Drupal's issues and release systems.

As noted, the plan is now to:

• Remove reliance on core Profile module, replacing with its own small user to IP range table.
• Release 6.x-2.0-betas, fixing bugs and testing upgrade from 1.x branch
• Start on a D7 version once D6 version goes to official 2.0 - or someone helps out with some patches!!!

Ha! thanks phrancescot... Hope to make some more improvements soon.