uc_roles_user_submit() assumes that the $edit argument passed to it contains an array of roles for the user being edited. This is not the case if the logged in user does not have "administer permissions" (in which case checkbox list roles are presented to edit). In this case, when user_save is called after the submit hook is invoked, core will remove all roles which don't appear in $edit['roles']. You end up with a user stripped of all roles (except the expiring one just granted to her by uc_roles).
The attached patch corrects this by checking if $edit['roles'] is set and, if not, it creates the array.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | uc_roles_avoid_roles_deletion_1.patch | 552 bytes | dwightaspinwall |
| uc_roles_avoid_roles_deletion.patch | 1.04 KB | dwightaspinwall |
Comments
Comment #1
ralva83638 commentedI also just noticed this, for me I was trying to setup a user role that could be a store admin and add time or remove time from purchased roles by editing the user page. Without giving that role the permission to "administrator permissions" and with the role assign module installed and setup the admin can see the roles in the list, he can add a role expiration which does not add the role, and he/she cannot add or remove time in the uber cart roles block. I can see why all this is happening this way and I believe some better integration with role assign may be needed to fix this rather than just this patch, which is also very necessary, or maybe just a uc role assign module?
Comment #2
dwightaspinwall commentedI don't think the bug has to do with the role assign module (which module are you talking about anyway?). The reason "he can add a role expiration which does not add the role" is because of the specific bug I describe in the original issue.
Comment #3
dwightaspinwall commentedbump - can we get this committed?
Comment #4
longwaveIs $account->roles available to use instead of re-reading the roles from the database?
Comment #5
dwightaspinwall commentedGood catch. No need to hit the db, as the roles are in $account. New patch attached - much simpler.
Comment #6
tr commentedTagging.
Comment #7
freelockWorks here, patch seems appropriate.
Comment #8
longwaveCommitted to 6.x, needs porting to 7.x.
Comment #9
Island Usurper commentedI think 7.x-3.x already has a better fix. In uc_roles_user_presave(), it checks that $edit['roles'] is set before deleting the roles that weren't selected. I think the way that it's been fixed, if you try to remove a role, then the expiration data from uc_roles won't get deleted.
Comment #10
longwaveLooks like there is nothing to do here now.