Posted by adub on June 12, 2009 at 10:00am
| Project: | Password policy |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Any thoughts on making the module (or at least password expiration) role-based? I am looking at setting up user/roles for services and it's not so good if those passwords also expire.
Comments
#1
This would be a great feature indeed. In my case, I'd like to force the policy (specifically the expiration feature) on moderators and administrators but not on normal users.
#2
+1 to that. Especially useful on ecommerce sites.
I'd be willing to sponsor work for it if someone could give me details.
thanks,
Ken
#3
+1. I'd like to be able to enforce a password change on all admin users whilst leaving other users (roles) unaffected. I would also like to able to specify a password strength policy for admin users only, leaving other rolls without a policy, or possibly a different policy.
#4
Where would you want role configuration to be added and how would the settings default - default expire with exemption or default exclude with inclusion.
#5
Hi @deekayen, thanks for getting back to me on this.
I think that you should be able to define multiple policies, as you can now, but that there should be an interface for assigning policies to rolls. Possibly similar to how IMCE works when assigning IMCE profiles to user roles.
I'm not quite sure I understand "and how would the settings default - default expire with exemption or default exclude with inclusion."
If password policy profiles were grouped as profiles, and explicitly assigned on a role by role basis, I don't think there would be a need for any defaults. If you don't know it already, please look at the IMCE configuration page to see what I mean regarding profiles and role assignment.
#6
+1 to that
#7
You can already define multiple policies and they list at admin/settings/password_policy/list. The problem is that I think you can only use one of them at a time as a global rule. I'm not sure why that is.
Is it preferable to have a page where the roles are all listed one the same screen with a dropdown option of the policy to apply to them or as a form alteration to each role configuration page (admin/user/roles/edit/16).
#8
I know that you can already define multiple policies, but to be honest I never understood the point unless you can either apply different policies to different roles, or use something like rules to determine if they should be applied or not.
I would say keep the password policy settings in the password policy configuration area. I don't know of any other modules that form alter the core role editing screen, and it may be counter-intuitive to users of passowrd policy to find half of their settings on that page. I think that a simple admin page within the admin/settings/password_policy section listing all the roles with a dropdown allowing you to assign a specific policy to each role would suffice.
#9
I have been having a discussion with a few people and I personally think having multiple policies is the way to go, its quite interesting how complicated it can be to achieve depending on hows its implemented.
Some of the points raised are that if multiple policies were available to be assigned on a per role basis and users can have multiple roles how do you resolve which policy should apply? We originally thought about using weighting on the policies and the highest weight would then be applied in that instance, do you's think that is the best solution though?
Also what happens when a user changes role to one with a different policy? Should their account be flagged for next login to make sure that they update their password to comply with the new policy?
Also what about auditing, should we audit when people change roles and this affects their password policy?
Currently I am just working on implementing the ability to skip password expiration based on a users role, would this be of interest to anyone if I role a patch when its completed and tested?
#10
@markevans: A patch that allows password expiration to be skipped for certain roles doesn't quite get us out of our current situation, which is that we are unable to enforce both and password strength and password change on our admin users without affecting other users. But is a step in the right direction.
Regarding the assignment of policies to roles, I think that weighting would do the trick for determining which policy gets applied. The most similar example of this I can think of is the better_formats module - which allows one input format to be applied to each role, and uses weighting to determine which one gets applied.
#11
Has anyone started work on adding this feature? I've been contacted by a client who only needs to apply password policy to certain roles, and as far as I can see there's two ways I can implement this:
1. Add a simple permission for "enforce password policy" - but this won't be of use to anyone to wants to apply a policy to all "authenticated user" roles and then ignore it (or use a different policy) for a further subset of roles, as all "authenticated user" permissions apply to any logged in user.
2. Allow multiple policies to be active at once, using the better_formats method of weighting to determine which one is applied to a particular role.
Regarding what happens when changing a user's role, the only way I can see around this is to force the user to change their password at next login, if the role change would also mean a different policy is enforced - otherwise it's possible for users to have passwords that do not follow policy, which would defeat the purpose of the module in the first place. As role assignment is a fairly rare operation on most sites, I can't think of any cases where this would really be a problem, though I guess forcing a password change could be a further option in the module if needed?
#12
@longwave
I have done a very quick change for our internal use which I mentioned earlier in the thread, this just allows certain roles to bypass the password expiry aspect based on a users role. Its not been tested fully yet but if you would like I can create a patch?
I do plan on coming back to this to allow the multiple policies but so far I haven't had the chance to get this coded, if you have the time to get started on this great, I am happy to help if you need it.
I like the idea of getting the user to change their password, having discussed it with a colleague here we were thinking of only forcing the change of password if the current one doesn't validate against the new policy, what do you think?
#13
I ageee.
#14
I was thinking that we can't validate the existing password against policy at role change time, as we don't actually know the plaintext password, so we would have to force them to change it to run the tests. But instead we should be able to invalidate the user's session, forcing them to log in again, and then capture and test the password they use to log in, and then only force them to change it if any of the tests fail?
#15
@longwave: Good point, that sounds sensible approach.
#16
@longwave: Yes very good point, I think your solution sounds sensible
#17
Not sure what the existing behaviour is in this case, but this would be internally consistent if users are already logged out when a policy is edited (i.e. made more stringent).
#19
A first attempt at a patch for this functionality is attached. This applies to the 6.x-1.x-dev branch and allows you to select multiple active policies with a weight and a set of roles for each one. The lightest weight policy that matches any of the user's roles will be applied (policies do not cascade or inherit from each other). You will need to run update.php as a new table and field have been added.
The hook_cron() function is rather complex and is difficult to test. If someone can review and/or test this on an existing site setup, that would be useful - I've tested briefly with some sample data here and it appears to work but I would like to ensure all cases are covered correctly.
The patch does not do anything special when an existing user changes roles. Expiration and warning settings will apply immediately but they will not be forced to change their password if it does not match the new policy. Still not sure of the best way of approaching this, any ideas are welcome.
#20
Great!
I should have some time this week to test this against a clean install and also our existing installs and will report any feedback here.
#21
Also see http://drupal.org/project/force_password_change which surfaced recently.
#22
@longwave
I just had a go at testing this tonight against a clean install, seems to work great.
I tested the following things.
Create 2 user roles, user + moderator
Applied a different policy to each with different requirements for password complexity
Set moderator to have the lowest rating and then logged in as a user set with both user + moderator roles, when changing password the moderator settings applied.
Then logged out and then set the role to have the user policy to have the lowest weighting and then logged back in as the user and went to change the password and the user role restrictions applied.
I haven't yet tested the cron hook, will try and do that this weekend.
So far all is working brilliantly
#23
@longwave
I have now had some time to run some quick tests for cron and haven't noticed anything strange. I would like to do some more tests which I will try and get done this weekend.
What are peoples thoughts about getting this patch committed?
#24
Despite my lack of commentary, I'm excited about this patch and afreeman should be reviewing it shortly, so we can get it committed.
#25
I've run some tests and everything looks good except for a couple of minor issues with Coder:
#
Line 48: Use update_sql() instead of db_query() in hook_update_N()
db_query("INSERT INTO {password_policy_role} (pid, rid) VALUES (%d, %d)", $row->pid, DRUPAL_AUTHENTICATED_RID);
#
severity: normalLine 49: Use update_sql() instead of db_query() in hook_update_N()
db_query("INSERT INTO {password_policy_role} (pid, rid) VALUES (%d, %d)", $row->pid, DRUPAL_ANONYMOUS_RID);
#26
Updated patch attached that uses update_sql() as requested, and that also directly includes the new schemas rather than calling the hook function to retrieve them.
#27
I'm getting a WSoD when I try to add policies after applying this patch. I've tried clearing cache and uninstall/reinstall and have been unable to get this to clear. I'm not ruling out problems with my local site config but it would be nice to have someone else review this to see if it's just me.
I also noticed a minor issue with the roles list: the anonymous users role is one of the available choices. Since anon users by definition don't have passwords you should probably not include it.
#28
I can't reproduce that WSoD here, if you can look in your error log to find out what caused it that would be helpful.
On the user registration form, users can choose their own password (if the option is selected to do so in user settings), and the anonymous user password policy will be applied in this case. Perhaps anonymous users should be treated as if they were authenticated users for the purpose of this module, but I guess theoretically someone might want strong passwords at registration time then a different set of rules when changing passwords later on? For sites that apply roles at signup it might also not be best to assume authenticated user policy should be used for new users.
#29
I committed #26. If the WSoD is true, open it as a new issue.
#30
Automatically closed -- issue fixed for 2 weeks with no activity.