Drupal 7 version (and updates to 6.x-2.x branch)

Hi mrfree, the Drupal 7 version is in the pipeline, but presently IP Login relies on the D6 core Profile module which is not present in D7.

Hence, ideally this module would have it's own small database table that holds user IP matches, rather than needing a manually-created profile field. This will improve the D6 version and pave the way to proper D7 version.

So I'm changing this request to a task with the rough outline of the work to be done to that others can provide patches or ideas.

The work to do is:

1. Add a new hook_schema and other database code to generate the user-IP table
2. Add hook_user stuff to allow IPs to be entered for various users by users with the 'administer ip login' permission
3. Change existing lookups to use this table
4. Provide an upgrade path for 6.x-1.x and early 6.x-2.x-DEV users that automatically moves the contents of the Profile IP field into the new table
5. Change the admin screen user interface to remove the reference and other checks for Profile fields
6. Add a tab to the admin screen to allow listing of all users with IP logins set
7. TEST!
8. Begin Drupal 7 port

Once 1, 2, 3 and 5 (plus ideally 6) above are done a port to D7 can happen - that should be relatively straight forward. Any and all patches for the above against 6.x-2.x-DEV are warmly welcomed!

Just a note, the 'Profile2' module is the successor to the core profile module in D7 and provides private fields for users, such as IP address. It may be an optional replacement for the D7 version.

http://drupal.org/project/profile2

Hi David, thanks for your note...

I saw that, but it's still a dependency on another module for no good reason... The two sites I've used IP login on used Content Profile instead of the crappy D6 Profile module for everything except an IP Login field - I.e. IP Login forced the enabing and use of Profile in sites that did not otherwise need it. And semantically (and perhaps pedantically!), this is not 'Profile' data really - it's account/access information.

On D7, between the Entity, core Fields (that work with user accounts) and the Profile2 module there are 2/3 possible paths to related data - all needing different code.

So I thought now would be the perfect time to drop the dependency on Profile or similar modules and not have to worry about relationships with external code except for the maintenance of a simple database table (schema probably: int "uid", varchar "ip_match"). This all lives in the .install file and from there it's just a few hooks to implement the rest of this, which should largely be the same in D6, D7 and (probably) D8 - database API and hook interface changes aside.

Of course there's the option of using both code paths, but that's clearly more maintenance and bloat and goes against the KISS principle... Ultimately I believe this module is an excellent choice for what it does - the best in fact - but needs one last tidy up job to be fully self-contained, compact and forward compatible with minimal extra code - hence the 2.x branch.

What do you think? Anyone?

Hi Jim,

I'm not too fussed about using profile2.

It would be nice if the D7 core user fields could be used instead, as users are 'fieldable' in core and that provides the UI for adding, editing and storing the IP details.

The problem there is that, by default, the user fields are accessible to the user and there are probably cases where the IP address should be locked down to admins only for setting/editing.

So, perhaps an approach using core user fields with some code to set the add/edit access control would be an option?

Otherwise, using a separate dedicated db table is a reasonable approach also.

Just trying to think of the simplest solution using existing functionality so as to avoid reinventing the wheel if possible.

Cheers muchly,

David

Jim, David,
I'm using ip_login since last summer on D6. I now have a D7-system which needs this functionality, too. Client has a deadline - as they mostly do :-/
My use case has only 1 automatic / generic user for all users within a domain.

I don't suppose Jim will be ready porting in that short notice, so my plan of approach is:
1- create an ip-loginfield using drupal core;
2- deny the user acces to his account (as you discusssed above) e.g. with http://drupal.org/project/user_settings_access;
3- port module to D7, stripping/reducing all management to the fixed field $user->field_ip_login
I'll let you know about the results.

Hi Johnv,

Good plan. Your updated code can be retrofitted to the D6 version to make the D7 version.

FYI I've added a .install file on my PC and am now integrating this new database table into the hooks. I'll upload a new 6.x-2.x-DEV release later this week. Hence I've done 1) and I'm doing 2) and 3) above now.

Your work on the rest of it will be very helpful (provided you don't strip everything out!!)

Thanks,

Jim

Please find 2 patches attached for the road to D7:
- ip_login_d6_prepare_d7.patch is a clean patch for D6 with several changes, which are explained below. Please review this patch, so my following/future patch will only contain porting issues.
[EDIT] The last-minute change to function _ip_login_profile_ip_get($partial_matches) is not correct. Change the ddeclaration to function _ip_login_profile_ip_get(&$partial_matches)

- ip_login_d7 is a patch of today's status of my current d7 port. Do not try to apply it - It is only there to satisfy your curiosity :-)

The D6 patch contains the following changes (I left some todo's, so the patch was more readable):
- corrected typo's
- info file: add explicit dependency for 'user', to be able to use 'user_access' later without using profile

- install file:
Done: hook_uninstall is never invoked, renamed to ip_login_uninstall
Done: set variable upon install(): default admin settings are not active, you MUST save, before that, screen does not show actual status.
Todo: move ip_login_uninstall/install to ip_login.install file.

- profile_ip functionality
Done: 2 new functions for encapsulating profile_ip functionality, so I can create other plugin easily
_ip_login_profile_ip_get()
_ip_login_profile_ip_check()

- miscellaneous
function ip_login_login(): mimic function user_authenticate_login more precisely (this is renamed to user_finalize_login() in D7)

- Todo: use prefix '_' for all internal functions (It is easier to see if you implement a hook / IMO it's common practice / But leave it if you don't like it.)

Attached you'll find a complete D7-port in zip and patch format.
Next to straight porting, and the above mentioned issues, it has the following features:
- In D6 user_logout was overridden. This is not possible anymore. the logout-routine, now only does logout. Data is stored in $_COOKIE instead of $_SESSION to hand over data from one session to the next. (Perhaps you find a better way) ITMT handling $_SESSION and $_COOKIE is now encapsulated in new function _ip_login_variable_get(), to have the least possible differences between D6 and D6 branches (after backporting :-) )

- Profile field is encapsulated in new functions_ip_login_profile_ip_get()/_check().
- Profile field is fixed to $user->field_ip_login. It is also not hidden. This is still on Jim's road.
- Messages are not always shown correctly (missing on auto-login, double at admin), but functionality works fine.
- See sources for all comments and todo's.

Hi Johnv, thanks for all your support and coding work!

I've been away on my stag weekend so I'm a little behind (not to mention mentally 'challenged') right now, but I'll have a proper look at all this very soon.

In the mean time if you found time to carry on working on the D7 version to make it functionally complete, that'd be amazing. My D6 work is progressing (install file made, including most/all the changes you allude to; hook_user stuff done; admin screen changes started) but that's all simple - what needs a bit more thought is the upgrade path for D6 1.x users...

But that's for another day - more soon,

Jim

Please find attached an updated patch. it has the following features:
- corrected hook_permission, which gave WSOD;
- info-file has new 'configure' line, which gives more infor on admin/modules page;
- a referral to the devel module, which knows how to switch users in D7: see devel.module/function devel_switch_user()

About hiding the ip_login field: I have tested several modules to hide the field, and used module userprotect to hide the profile/account-page completely. The module is suited however for disabling single fields.
I don't think hiding the field should be a task of ip_login.

Thanks Johnv...

I will create a new 7.x-2.x-DEV (no point having a 1.x, best to keep the names in sync I reckon) next week based on your patch. I'm getting married this weekend so will action this when everything settles down again.

As for the field_ip_login: well this is precisely why I reckon it shouldn't be a core field at all... Just a separate table. This is the path I'm headed down on the 6.x-2.x because it simplifies both the code and operation... So my priority is to drop all 'field-like' hangovers (Profile, Fields, Views etc) and just get on with secure, consistent and easy to use ip_login_user table approach instead.

IF people need 'field-like' properties on this then we can easily expose it to Views or indeed via core Entities/Fields in D7... For now though I just want it to work and not need any tricky setups - it should Just Work.

Thanks again.

Hi Jim, congratulations!
I suspect you won't be back online soon :-)
Attached you'll find the latest D7-version: removed a WSOD on url() ; messages are better; some performance changes; better separation login vs. logout.

Oops, new try

Sorry, I'm kind of new to this, I've tried putting it together without any luck, any idea as to when it will be available to download as a D7 module?

I am sloppy: the last patch was reversed.
Let's just give you the complete code in attached zip-file (remove the .txt suffix before unzipping)
Remember: you have to create a text field 'field_ip_login' in admin/config/people/accounts/fields.

Hi Tomas, I'm back and want to commit Johnv's latest soon as 7.x-2.x-DEV. If you could test it and report back here that would be great.

Once I have your feedback I'll commit this as the starting point for 7 and try to get 6.x-2.x-DEV up to date with my latest changes.

Then there will be a phase of synchronisation between the branches to minimise the differences.

Then we'll be rolling quickly towards 2.x-betas.

Let me know.

* Thanks for all the hard work Johnv! *

Hi Jim, welcome back.
My code is not ready to commit directly. It contains lots of constructions to compare D6 and D7 without using diff's.
I'll be happy to strip to code is you wish, but I though you'd get lost in the changes..
Take a (brief) look a the code and my comments in #7, to decide if you want to apply my complete code to D7.

OK John... Will do the proper comparison in #7 as suggested when I get a mo.

First off, thanks for getting this ready, both of you, but I can't get the configuration screen to load, I get "The website encountered an unexpected error. Please try again later." It could possibly be the site I'm working on, so I will try it on a fresh install as well, I've just been really busy as of late. Sorry for the delay, I got thrown off the site I was looking to use this on and onto something else and I forgot to check for responses.

Edit:
So I tried it on a clean site, and the same issue popped up, with the error:

PDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'p_test20.field_data_field_ip_login' doesn't exist: SELECT DISTINCT field_ip_login_value FROM {field_data_field_ip_login}; Array ( ) in _ip_login_profile_ip_check() (line 684 of [path to drupal]/sites/all/modules/ip_login/ip_login.module).

Tomas, did you create the field first?
- go to admin/config/people/accounts/fields;
- create the Text field 'field_ip_login' with description e.g. 'Automatic login from IP-address';
- get your IP-address , using e.g. http://whatismyipaddress.com/
- go to admin/people, choose a user, set your IP-address
- test again.

As described above, in the D7-version, the Profile dependency is changed by this hard-coded field.
ATM, I use user_readonly to protect this field from being changed by the user. I tested user_protect, but didn't like it.

Oops, I think I named it wrong, works now, sorry, I feel like an ass, and it works perfectly

I just committed a major update to 6.x-2.x-DEV... Apologies for the delay, it's been a busy summer!

This version:

• No longer uses the Profile module for storage, having it's own ip_login_user table that relates users to ip matches
• Has all admin page references to Profile have been removed, as has the dependency in the ip_login.info file
• Integrates with the user account page via hook_user, allowing saving and validation of IP ranges
• Implements a proper upgrade path from earlier versions and imports the contents of the profile ip field into the new table
• Shows a list of IP Login enabled users to the admin screen

So, on this ticket we have got the following left:

1. Testing... PLEASE TEST!!! Works for me, but needs bug hunters and constructive criticism!
2. Integrate johnv's good work on the Drupal 7 port, combining D6 & D7 code bases where possible
3. Push to 6.x-2.0 and 7.x-2.0 as soon as we're all happy

On the last point, thanks again johnv! I haven't done anything with your patches yet because I wanted to get this critical path done first, then we can tidy the code, add D7 stuff and maybe later - if needed - allow the new ip_login_user table to be exposed via field or views APIs.

So, John, I will start working through your code and patches next... If you wanted to update your version with the new code, that'd be great.

I've just committed a newer version which fixes a small user account page validation issue, and incorporates John V's "ip_login_d6_prepare_d7.patch" from #7...

Due to the need to hide IP fields from prying eyes, I'm now certain that having a separate table for IP data is the best approach. If the contents of this table need exposing then the views/entity/field APIs can do that. I don't see why they would be needed for now. Hence, unless someone can convince me I'm wrong, I will not be accepting patches that use the core's field systems that then require extra workarounds to hide the field again. It's pointless, and the better approach is already implemented now in 6.x-2.x-dev.

OK, so some good changes have happened... Johnv, could you examine this new version shout if you like/hate it?: http://drupalcode.org/project/ip_login.git/commit/5ec8bb0

The new field UI is nice, Jim!

I've tested that first:
- The fact that the IP-field is now explicitly hidden from the user, might effect the installed base where users are allowed to maintain the own IP-addresses. You might consider a new permission 'administer own IP address'.
- The table ip_login_user generates the following message in phpmyadmin: "The indexes PRIMARY and uid seem to be equal and one of them could possibly be removed." This is fixed in the included patch by dropping the index.
- If you add and then remove an IP-address from e.g. /user/1/edit, this empty line is shown at admin/settings/ip_login. You can either filter this line or remove it from the table.
- If the user is not allowed to logout and switch user, then what should be the module's behaviour when the user lands on the website for the first time (this session)? Now he can still choose another user as e.g. yesterday.

Please find attached a new prepare_d7.patch file. It is still a D6-version and it contains the following:
- ran through coder module, resulting in minor coding changes, as well as some t() changes.
- localized $_SESSION[IP_CHECKED] in ip_login_boot(), resulting in better readable code. (Man! - is that tricky!)
- changed admin page to 'admin/settings/ip_login', for more consistency in module name.
- moved db_query from admin.inc to separate function in .module - now all db-operations are in 1 file, and D6-D7-differences in admin.inc will be kept to a minimum.
- moved message 'This account does not have permission...' from ip_login_as_different_user() to user_login(). This way, ip_login_as_different_user() only has logout functionality, resembling user_logout().
- above separation paved the way for renaming this function to ip_login_user_logout(). Now it already has the proper name for the D7 hook :-)
- reshuffled IP_UID_MATCH in/around ip_login_as_different_user() - it took me a while to understand the recursion..
Just pick what you like from the changes. Perhaps we are from different coding schools.. I'll wait for your next D6-patch. Then I'll make a new D7-version with minimal version-difference.

Hartelijke groet, John

Thanks again John, great stuff!...

Ok, I've:

• Manually applied almost all of the patch as-is -- thanks!
• Kept the function for the user table on the admin screen in admin.inc because it's only ever used there - no point loading extra code on non-admin requests, I reckon... I've also tweaked your approach slightly to avoid pass-by-reference and renamed it _ip_login_users_display_table() for better clarity
• RE: 'administer own IP address' perm... Hmmm... I reckon it's rather unlikely a 'normal' (non-admin) user can do this - so much chance for a broken site! So I won't complicate things until there's a proven need to.
• Fixed the issue where IP range removed for user when saving, but the record wasn't deleted.
• "localized $_SESSION[IP_CHECKED] in ip_login_boot(), resulting in better readable code. (Man! - is that tricky!)" - yes! that code evolved over several iterations and feature requests, thanks for the tidy up... I've committed what you've done but not yet fully tested it, so we may have gained bugs. (not casting aspersions on your code here John, merely pointing out it's no-where near as tested as the rest of the stuff!)
• The patch broke the ability to re-login for those with permission so I had to move the 'This account does not have permission...' message back, plus add another IF to hook_boot to get the old functionality back.
• Understood re other changes, thanks.
• I'm sure our coding styles differ but then much of this isn't my code... We coders occasionally get the 'wow, this really needs re-factoring' feeling, but rarely have the time to do it! Updating 6.x-2.x/7.x-2.x is the perfect time! I'm grateful either way.

SOOOOooooo..... It's all in: http://drupalcode.org/project/ip_login.git/commit/0ed6271 - we're getting dangerously close to 6.x-2.0-beta... next commit will be!

Thanks, Jim, that's fast!
here's one more UX patch for the admin page. It generates a link to the user-edit page.

two more coder issues.

OK, thanks.

We're in nit-pick/minor tweak territory now, so I've made the changes from these two patches and committed 6.x-2.x-beta1! See: http://drupalcode.org/project/ip_login.git/commit/d7260ce

Now the code is tidy, I don't want any more patches not related directly to getting a D7 version at this stage - keen to get a 7.x-2.x-alpha1 branch out very soon!

Release page here: http://drupal.org/node/1226544

Hi Jim, I can't finish the D7-version on such short notice. I'll be on holidays for the next 2 weeks.

I just add my work sofar, in case someone feels like continuing it. This is the status:
- user maintenance is OK
- on the admin page, theme_table() is not showing yet
- user_login is wrong: it takes the incorrect user due to a wrong query
- user_logout gives WSOD due to session_destroy, etc. Check the comment or my previous D7-version, which uses cookies, not sessions.

I've added some stuff in comparation to D6:
- field is shown on user_register-form, too (when creating a user instead of changing it)
- field is deleted from table when user is deleted
- some changes that avoid log messages. They are marked with '// D6'

Well, this is my final attempt for now. It needs testing, and ther are some @todo's left.
login/logout is still not working fine: Since I'm using hook_user_logout, $_SESSION[LOGIN_AS_DIFFERENT_USER] is not passed on nicely to the next session. In my nicely working version from #13 I used $_COOKIE instead.

Thanks for all this, John... I'm off on my belated honeymoon anyway next week, so I'll try to get a D7 alpha up before then but it might be easier to park this until mid-August.

i downloaded the zip file from #15 and it works for me. I haven't applied any of the other patches submitted afterwards and I have no problems.

the only thing i would have liked to see is permissions for limiting who can set their ip based on role. I ended up using the Field Permissions module to do this though.

@laroccadahouse: Thanks for testing... Please note (per comment #23) that the proper D7 version will NOT use the profile/field system, hence not need another module to hide fields again. The D6-2-beta1 does this already.

More work on this module will happen after the 16th of August -- I'm off on my honeymoon!

August 18. Jim should be back although, possibly, not recovered. Looking forward to a D7 alpha version for testing.

I'll be working on #32 the next few days, too. This would then be the next rc.

Thanks all, I'm back but on a massive catch-up mission (the joys of being self-employed/freelance!) so will await John's version before I break out D7 and push to Git.

Well, this is a working version. It has the following issue:
- on the admin page/include, theme_table() is not showing yet; I'd be nice if someone can amend that.
I've added some stuff in comparation to D6:
- field is shown on user_register-form, too (when creating a user instead of changing it)
- some changes that avoid log messages. They are marked with '// D6'

It already contains the 3 patches from August 18, 2011.

Excuse me for not providing a patch, but a zip-file, (with a changed suffix).
happy testing!

please use attached version, not #41.

Thanks John!!! It's committed...

Well actually a version that further minimises the differences between the D6 and D7 version, plus the stuff that happened over on #1263234: Cookies set on all pages even outside listed login pages.

Admin stuff is tested, login stuff partly tested but looks good so far... Please test all!!

This issue will be closed as soon as we've got the 7x DEV up and no-one screams it's broken!

A 7.x-2.x-DEV release has been created!

Give Drupal's packaging bots a little while to sort it out and please test away!

Closing this issue since it's done... All bugs and issues for the 7.x-2.x branch need their own issues added to the tracker.

Thanks again to all who helped, especially JohnV.