Posted by mtcs on September 7, 2011 at 8:47am
2 followers
Jump to:
| Project: | IP Login |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | postponed |
Issue Summary
As far as I understand this module if multiple users are matched by ip_login_check() there is no way to specify which uid should be selected.
I can imagine an ideal solution to introduce weights to IP matching rules and a possibility to reorder users on admin/settings/ip_login.
But we can also use a simple rule: the more complex a pattern is the higher the weight. Specifying the pattern complexity could be as simple as ordering by pattern length:
$partial_matches = db_query("SELECT uid, ip_match FROM {ip_login_user} WHERE ip_match LIKE ('%s') ORDER by LENGTH(ip_match) DESC", '%' . $addr[0] . '.%');
Comments
#1
I've wondered about overlaps before and thought, and then wondered if it's likely, and if it matters. I mean, how many people will have a IP Login-enabled site and also two users with an overlapping IP range?
You're right though on both counts - a weight would be a good idea, as would the rule of thumb that says bigger rules (and therefore less specific matching) are lower priority than short ones. It's a bit hacky though...
I think this is related to #1252994: ip_match should be a multiple value field, possible changes to schema for better performance in that a small schema change could be on the cards. But I also think this is a rare corner case and would rather keep things simple...
Downgrading for now, will think about it more when I have time - if you have any other ideas, please let us know. Thanks mtcs.
#2
I've committed a similar change -- mine was ORDER BY ASC, not DESC since shorter IP ranges should be prioritised over long ones. It's a start but will need something more concrete eventually...
#3
Thanks for considering this.
Having overlapping subnets is indeed reality at large organizations where nested subnets are very common (e.g. department / division / county, etc. ...). IP addresses from a local subnet (e.g. department) have other permissions than those from a broader subnet (e.g. division) or from a different local subnet (other departments). In these situations IPs from a local subnet will also match the IP rule describing a broader address range so rule weights are needed.
I've used DESC because it assumes that a more specific range mask is a longer string (more specific means less IP addresses in range, e.g. 192.* vs. 192.168.* - I want the second mask to fire first).
It's clearly a hack I know.
Introducing weights on the admin interface would be way better. It's not a quick work though.
If you consider using ip2long() (see my other patch: #1271976: Subnet matching), it could also be possible to calculate a better weight automatically.
Since the subnet patch will not work with this simple calculated weight - and I need both of them -, I will continue work on this issue.
#4
I'll look at your subnet matching patch later this week, thanks.
I think there are 3 broad - but related - issues the module has presently:
I don't want to do any changes to the schema or code unless we can deal with 1 and 2 cleanly, and ideally prepare for IPv6 stuff which will only get more pressing as time goes on. I note the inet_ntop() function (and its counterpart inet_pton()) look very handy indeed...
#5
Postponing major changes like this to the forthcoming 2.1 branch, want to get 2.0 out the way and bug-free first.