Because of the way this module uses hook_user_presave to restrict which roles can be assigned, it will conflict with other contrib modules that programatically assign roles to users.

For example, the ldap module will assign roles based on ldap groups when a user logs in.

Would it be feasible to validate the assignable roles in the form validation rather than by using hook_user_presave?

Comments

drasgardian created an issue. See original summary.

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented
Category: Bug report » Feature request

No, I think this would be less secure.

Does that mean RoleAssign is incompatible with ldap?

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

I agree that forcing this in the presave hook is tricky.

roleassign_restrict_access() already checks for only applying to users that do have administer users and assign roles permissions. So arguably, it's already "less secure" if you have code that saves users without a current user, e.g. via drush or other scripts.

This is kind of a validation and that shouldn't be done as part of saving IMHO, just like core doesn't force entity validation on save.

I've hit this in combination with behat tests, when you first log in as a user that is allowed to grant some roles, and then as an admin, then you're still logged in with the old user by the time the second user is being created, and then roleassign jumps in and removes the admin role again. This actually currently only happens with the dev-master of DrupalDriver, because that new feature to log you in in behat isn't in the last release yet.