Module is working great. Love it.
On my installation, if I cycle through enabling/disabling the module the 'Anonymous user' and 'Authenticated user' are both removed from the 'role' table. This then means that no 'Anonymous user' can access content on the site, even if a new 'Anonymous user' is created through the GUI. Drupal expects an rid of 1, and a new 'Anonymous user' created with the GUI gets a random rid. And I do believe it expects 'Authenticated user' to have an rid of 2.
Perhaps the module needs to check for the existence of roles 1 & 2. Create them if needed. Change their rids if needed.
[code found here: http://api.drupal.org/api/function/system_install/6]
// Sanity check to ensure the anonymous and authenticated role IDs are the
// same as the drupal defined constants. In certain situations, this will
// not be true
if ($rid_anonymous != DRUPAL_ANONYMOUS_RID) {
db_query("UPDATE {role} SET rid = %d WHERE rid = %d", DRUPAL_ANONYMOUS_RID, $rid_anonymous);
}
if ($rid_authenticated != DRUPAL_AUTHENTICATED_RID) {
db_query("UPDATE {role} SET rid = %d WHERE rid = %d", DRUPAL_AUTHENTICATED_RID, $rid_authenticated);
}
Sorry, I'm not quite up to the task.
cos
Comments
Comment #1
cos commentedHmm. I guess that when it is disabled, the roles are blown up. So it wouldn't help if someone just disabled the module and never re-enabled it. Is there 'run this code upon disable' hook? That would work...
Comment #2
agentrickardI don't understand.
Comment #3
cos commentedOn _my_ installation:
Yeah, I figured you would never have experienced this. Probably _your_ installation works just mighty fine. That's why it's a 'bug'.
The way I solved my particular problem was by googling and finding the install script with the sanity check for anonymous user. For me it is no longer a 'problem' because I simply go in and tell mysql to add the users and change the rids.
But it took me hours to figure out. So to prevent another user from experiencing this, if there was a way to do a sanity check for the existence of the users with the correct rid when the module is disabled -- and fix it right then, why it could save someone else hours.
Comment #4
agentrickardI don't dispute it's a bug, I just needed a clearer explanation.
The solution is to alter secure_permissions_rebuild(), to ensure that it doesn't run when the module is disabled. This is a weakness in the D6 API which should not exist in the (native) D7 version.
Comment #5
agentrickardWe should probably just put in a check to ensure that these two roles never get deleted.
Comment #6
agentrickardOK, this is a big problem, but it should be easily fixable. Looking at the code, it was never intended for the module to be disabled, which is just plain silly.
Comment #7
agentrickardOne of the sanity checks is in the wrong place.
Comment #8
cos commentedWhoa!
Now I don't know what is going on.
It appears when I turn off other password modules, not just the secure permissions module, my rid 1&2 and all other roles get erased.
I'll check this out more...
Comment #9
cos commentedNo, that isn't happening. But I checked the secure permissions again.
Comment #10
agentrickard#1 is expected behavior, though something we should probably remove.
#2 should not happen if you apply the patch. Did you?
Comment #11
agentrickardOK, dug into this some more, and there are issues with knowing when permissions are set in code. We can't rely on the role IDs, unfortunately, so we try to ensure that roles are never deleted when the data module is disabled.
This patch should correct the issue. Please apply and test. In this case, we totally disable loading from code if the data module is missing, which may not be necessary.
When reporting back, please note that you applied the patch.
See also #843120: Secure Permissions does not know that permissions have been exported, which makes export easier.
Comment #12
agentrickardRemoves a stray debug and fixes a logic error.
Comment #13
cos commentedWorks at my house.
Comment #14
agentrickardComment #15
agentrickardCommitted.
Comment #16
agentrickardNeeds a port to 7.x.
Comment #17
agentrickardAnd the patch.
Comment #18
agentrickardUpdated to HEAD.
Comment #19
agentrickardFixed a bug. Removed the info string.
Comment #20
agentrickardFixed.