Closed (fixed)
Project:
Administer Users by Role
Version:
7.x-2.0-beta1
Component:
Code
Priority:
Critical
Category:
Bug report
Assigned:
Reporter:
Created:
20 Nov 2014 at 11:46 UTC
Updated:
16 Nov 2016 at 08:19 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
adamps commentedComment #2
adamps commentedComment #3
adamps commentedComment #4
ar-jan commentedI just want to say it's good to see you're working on this!
Comment #5
adamps commentedThere are so many issues to fix that it's simply not practical to have a separate patch for each one. Here is a summary of the changes in this patch and why. Yes, I am aware I haven't uploaded the patch yet - I want to upload the description first so that people don't start using it without knowing what they are getting.
1) Permissions (administerusersbyrole_permission)
2) Menus (hook_menu_alter).
3) Calculation of access (_administerusersbyrole_can_*)
4) Views - just minor tidy up
Comment #6
adamps commentedOK, here is my first version of the patch. Comments, success/bug reports and questions welcome. Those of you that have indicate on other issues that you would like to have a working version of the module - I need your help now to test, review, criticise and so on.
This is my first try at being a module maintainer so please forgive my mistakes in following the correct process.
Using the patch
Updated instructions to use this module:
Key points:
Comment #8
adamps commentedComment #9
adamps commentedGiven that the changes to make it secure have led to some changes in the permissions, and there isn't a clean upgrade, I have decided it's best to make a version 2 of the module. Please check out the dev release that should be available shortly.
Comment #10
adamps commentedFixes now available in latest release
Comment #11
ciss commentedI know this comes awfully late, but I'm somewhat surprised to see that using role IDs was considered a solution. This creates huge problems with exported roles that are hard to work around.
From what I've seen in the diffs the actual fix would have been to not rely on user_roles() (which uses a query that is tagged with "translatabe") but instead query the role names directly. Role name changes could have easily been tracked in hook_user_role_presave().
Any chance we could go back to using names? It's the next best thing we have to machine names in D7. I'd be happy to open an issue and start working on it.
Comment #12
adamps commented@ciss This is a meta-issue so not the best place for specific issue debating. I suggest the best place for you to look is the more recent issue #2414853: Errors after export and re-import via features . Please read that and add a comment there if you wish.
Bear in mind a) this problem goes away with D8 and b) we have a stable release now (yes you are awfully late!) and renaming the permissions would be a complexity with potential bugs for all users. It might be most realistic for you to create a patch which you apply locally and share it here for anyone else who also wants it. I doubt there will be a lot of new releases to D7. Also it might be easier if your comments could stick to the facts without sharing your personal thoughts on what is "huge" and "easily" done.
Comment #13
ciss commented@AdamPS I'm sorry if I sounded too harsh and offended you. I commented on this issue because it provided the best context (as the changes were introduced in a commit that references this issue).
Since the other issue references the D8 branch I'll post the patch to a new issue (even if it will have no chance of getting committed). It might take a few weeks since we've decided to use a rather dirty workaround for now, but hopefully I'll be able to put my money where my mouth is.
Comment #14
adamps commentedNo problem, no offense taken - I should have put a smiley. It's definitely valuable to have the patch available for anyone who wants it. Please link your new issue from the existing one.
Comment #15
ciss commentedFYI we've worked around the issue by using role_export. And even without role_export the problem of exporting roles consistently would probably still be outside of this module's scope (not to mention there are other alternatives like the available feature hooks, and Features itself already does some permission string rewriting for taxonomies).
Whoever reads this: there won't be any patch coming from us.