Administrator role for Drupal core so modules can set sensible defaults

Marking as duplicate of http://drupal.org/node/64861

Route to core through contributed modules recommended by Greggles in #drupal IRC chat:

• http://drupal.org/project/adminrole - looks fantastic
• http://drupal.org/project/role_weights

ben-agaric: That's an issue of "super admin" in my opinion. For a regular admin role: what needs to be coded? Just an empty role that modules could give reasonable defaults in would be huge, as far as Drupal usability out of the box.
[2:10pm] greggles: ben-agaric: there is a module that does this already
[2:10pm] ben-agaric: greggles: I'd love to know what it is, but my point is it has to be in core
[2:11pm] greggles: ben-agaric: "has to be" ? why?
[2:11pm] ben-agaric: I make a module. I can set reasonable default permissions for anonymous users
[2:11pm] ben-agaric: and for authenticated users
[2:11pm] ben-agaric: but even if there are obvious defaults for a vanilla admin user, a module can't set those
[2:11pm] Bdragon|zzz: You know, if hook_perm accepted an array of arrays, where the defaults for anon / auth / admin were returned....
[2:11pm] greggles: ben-agaric: as a start how about making your module rely on role weights and using that
[2:11pm] ben-agaric: the user has to go in and set these permissions-- sometimes before they can use the module
[2:12pm] ben-agaric: role weights?
[2:12pm] greggles: ben-agaric: it's another module
[2:12pm] greggles: ben-agaric: and "admin role" is the module I was talking about
[2:12pm] • Bdragon|zzz shrugs and goes back to working....
[2:12pm] Bdragon|zzz is now known as Bdragon|work.
[2:12pm] greggles: ben-agaric: generally speaking, changes like this happen via a contrib which gets really popular
[2:12pm] chx: admin role should not entail much more than changing the exception in teh beggining of user_access to isset($user->roles[DRUPAL_ADMIN-ROLE])
[2:13pm] chx: all you need to do is handle an upgrade somehow which is not gonna be asy
[2:13pm] chx: maybe role 0.
[2:13pm] chx: or increase the rid by 1 in Drupal 7 and use rid 1
[2:13pm] ben-agaric: greggles: so what are the two modules that could be sort of a progressive enhancement, if present other modules can set intelligent default role permissions for more than anon and authenticated?
[2:14pm] greggles: ben-agaric: role-weights is the module that provides features like this
[2:15pm] greggles: ben-agaric: if expanded and if other modules provided enhanced function when its installed then that would be a path
[2:15pm] greggles: ben-agaric: or a similar idea is the admin-role module

The issue here is sensible defaults. This is a key component of usability. Until I get around to making a patch for this, I'm going to keep adding examples of why this is needed.

Today's example, from the Panels 2 Beta upgrade note:

Don't forget to give yourself the additional permissions this module creates if you're not logged on as user/1!

Heck, this is a usability issue and could be put in Drupal 6 even.

AND a security issue. Matthew Saunders creates an admin role as a matter of course and recommends everyone does so:

Log out of [User 1] and only ever use it if there is an emergency--your theme becomes so broken you can't get into any part of the site--or for site upgrades.

After that I use UID2 for my various configuration and adminstration functions.

Note that this request is different from a longstanding feature request, Default Admin Role, which duplicates the superusers powers (essentially, makes user number 1 no more special than any other user).

The code part of this request is trivial-- adding an empty role, but one that core and contrib modules can assume is there. Which is why I'm spending all the energy on the argument ;-)

coreb wrote on the 'create superuser role' post:

I prefer to treat uid=1 on a drupal site the same as root account on a unix machine. Only use it when you have to.

Create a personal account for yourself for everyday interaction on the site, and create roles with more options enabled in access control (for administrators or moderators). Use the uid=1 account only when you need to do really important things.

Which is exactly what having a core admin role encourages-- not having an admin role, having only anonymous and authenticated, encourages using UID 1 for everything.

... and I'm not the first (or only) to suggest this!

Charlie Lowe, 2003 September, in a thread about creating a super-admin role but retaining a power edge for user 1:

as a matter of compromise to help with convenience, I suggest just adding to the basic Drupal install an admin role with admin permissions on the modules that are loaded as default on initial installation. No coding necessary; just add it to database.mysql. That'll make it easier for those that want an admin role, but still respect the security concerns of those who want a root user.

Charlie
http://cyberdash.com/

(That thread overall is more relevant to the issue linked to above, and a more recent duplicate of it that's looked more closely at how to code replacing UID 1 checks with RID 1 checks).

For why we should have a user-controllable admin role (can set permissions however they like, just as they can for anonymous and authenticated users) see for instance this recent Drupal.org use case Create admin role with block access and without PHP access.

Usability. Security. Simplicity. No coding needed, just a default role that any users could set up, but with an ID that modules can rely on to set defaults. (Still think it could go in 6, but I'll tactically concede ;-)

I agree, there are more than enough good reasons for this! Too late for D6 maybe, but this should not be neglected in D7.

Benjamin stated several times that this is a trivial thing to implement. I disagree.

Adding another default role to core must presume to change every site owner's definition of rid3. What is that rid right now, and what's it being used for? To presume that is doesn't exist would be folly. To assume that you can take ownership of it for your own need would be tragix, to say the least.

But, it can, and should be done. Here's how. Leave rid 1 and rid2 alone. We've all written so much code that assumes rid 1 & 2 are anon and auth users, respectively, that nobody wants to go back in time and change all that logic.

Instead, we'll add an rid3 called 'administrator'. If rid3 already exists, we move it to ridX+1 and store it there. Care most be taken to comb the popular contrib modules that deal with roles and rid's to make sure nothing gets missed before this is committed to core.

The idea of a third default role within core is not new, and I remember it coming up during my Drupalcon Boston talk about "Making Modules More Admin-Friendly". I actually even proposed a hook_configuration() system to help out the process of installing, enabling and configurating modules.

I'd be willing to write a D7 patch for this, but before I do, I need to make sure that we cannot achieve Meta Roles for core, cause that would be so much sweeter and better than just making a third role. Only when the Meta Roles proposal is won't-fixed will I begin working on this particular issue's patch. Sound good? We don't need any duplication of efforts.

This issue is closely linked to #248598: Some permissions should never be for anonymous. Because the other issue has a broader scope and aims to build security foundations that could be use to build a default "administrator role", I'm tempted to mark this one as duplicate, or at least to postpone it.

Ok, I'll bite. Let's postpone this thread until we need to revisit it again.

How about setting anonymous user to 0, administrator to 1, and authenticated user to 2. This would map with the same thing we do for user IDs for those things, and not screw with any of the existing role IDs. There are defined constants DRUPAL_AUTHENTICATED_RID and DRUPAL_ANONYMOUS_RID. If any code is not using them, file a (critical) bug. No reason to postpone this issue, imo, unless I'm completely on crack (which is always possible).

Also I strongly disagree that this issue should have any bearing whatsoever on #248598: Some permissions should never be for anonymous. These two issues are addressing two completely different things.

I'll roll a patch for the suggestion in #13 right away. I like that idea, a lot.

I just came on over here from the other issue (#248598: Some permissions should never be for anonymous). I agree that these are definitely not duplicates, but I think Damien is right that they are related. They are kind of opposite sides of the same coin:

• Here the goal is to let modules require, or at least recommend, that certain permissions be (initially) assigned to an administrative role.
• At the other issue, the goal was to let modules require, or at least recommend, that certain permissions NOT be assigned to NON-administrative roles.

The same sort of information is needed for both issues; the only question is what you do with it when you have it.

Hardcoding a single DRUPAL_ADMINISTRATIVE_RID would definitely be useful to a large number of sites, but in the long run, I think this extra hardcoding might be a bit of a step backwards. The ideas we were starting to develop at #248598: Some permissions should never be for anonymous would give you all this for free without any hardcoding needed, and in a way that would work for any site. In particular, we learned as a result of the discussion there that it's a design flaw to ask modules to know anything about how roles are being used; we tried it that way initially, but it was quickly pointed out to us that on some sites (intranets), anonymous users might actually be the administrators. Alternatively, some sites might have multiple "administrator-level" roles ("manager", "webmaster", etc). Hardcoding DRUPAL_ADMINISTRATIVE_RID and then asking modules to assign permissions to this role can never address these kinds of possibilities.

So the question I would have is the following: Is it worth it (and is it realistic) to try to do this the "100% right way" within the D7 development cycle, or is it better to go with the somewhat-flawed-but-still-useful approach of a hardcoded DRUPAL_ADMINISTRATIVE_RID?

Senpai's quite right (#10), the matter of not messing with existing roles makes this non-trivial. His solution is good but Webchick's #13 is perfectly elegant. Thank you Senpai for acting where I talk.

@David_Rothstein: a "hardcoded" administrative role is not a flaw but in fact the point: sensible defaults to let core and contrib modules speed up the setup for 80 percent of implementations, for a usability and a security benefit. The ultimate definition of permissions is still up to the person setting up the site, just like now. Or more likely the defaults could be mostly accepted, and lesser and greater administrative-type roles also created as necessary. (It would be interesting to see the administrative role as a "base" role for roles in an administration category, the same as authenticated user is a base for all other roles, but that's already confusing enough and couldn't be considered without some significant change to the UI, a visual flag of what permissions are inherited.)

I am very excited by the prospect of a default administrative role making it into D7.

^{benjamin, Agaric Design Collective}

Hardcoding something that's only useful for 80% of sites is by definition at least a little bit of a flaw, unless the other 20% have an easy way to override it... then it becomes a brilliant usability improvement instead ;)

So in that spirit: I'm curious about how "hardcoded" this administrative role would actually need to be. You've said above that it should be possible to delete this role (which sounds good), but you've also said you want modules to be able to assume it's there and interact with it by setting "sensible defaults." How exactly do you envision this? Are you thinking that a module should assign permissions directly to this role, e.g. via db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", DRUPAL_ADMINISTRATIVE_RID, 'administer mymodule'), or were you imagining a more elaborate mechanism?

If modules are being encouraged to insert permissions directly, then I'd worry that's a major security flaw... unless Drupal hardcodes that role extremely tightly and treats it as a very special case, with bells, whistles, etc. (Which is certainly not the end of the world, but it does then mean Drupal is less flexible and the 20% minority will get annoyed.)

Or maybe I'm just missing the point entirely?

David, this will pave the way for something like a hook_configuration() proposal to allow contrib modules the ability to easily install themselves with pre-configured role permissions.

Wow, hook_configuration() sounds like a really great idea.

For the issue at hand, though, it's still necessary to think about how hardcoded the administrative role should be, for the following reason. Suppose the only thing that's hardcoded is a fixed role ID for modules to use, as proposed above. The problem I see with that is this: Let's say I'm setting up my site and I go to add some roles. I have a whole list of roles I want to add, but "Administrator" isn't one of the ones I need. So I decide to rename the "Administrator" role provided by Drupal to "People I Don't Trust Very Much" and start putting people I don't trust in that role. Seems like a perfectly reasonable thing to do; after all, Drupal gives me complete freedom to decide how I want to use a role. However, once I start enabling new modules, some of those modules are then going to be trying to assign their administrative permissions to the "People I Don't Trust Very Much" role! If the permission assignments are done automatically, this could be catastrophic. If done in a managed way via hook_configuration(), then it's less catastrophic, maybe, but still annoying; every time I install a new module, Drupal will recommend that I give all the module's administrative permissions to these untrusted users.

So that's why I think more might need to be hardcoded then just the role ID, to prevent the above scenario. (How much, exactly, I'm not sure...)

Thinking about this more, what if we took a slightly different approach... maybe we could borrow some small ideas from #248598: Some permissions should never be for anonymous without needing the whole thing:

Rather than having "administrator" be a role that is treated specially, how about making it a "tag" (for lack of a better word) that can be applied to existing roles? The site owner ultimately gets to decide which roles the tag is applied to. There could easily then be a simple API for modules to assign permissions to these roles; e.g., calling user_assign_administrative_permissions(array('permission1', 'permission2')) would assign the listed permissions to all the roles on the site that have been tagged as "administrative". And if hook_configuration() happens, that could use the same API just as easily.

This would provide much more flexibility. Sites that have no use for this feature could simply not tag any of their roles. And sites with unusual setups, e.g. intranets where anonymous users are the administrators, could tag the anonymous user role as administrative, and then it would work fine for them too.

I haven't thought through all the implications of this, but does that seem like it might be a better approach?

Ok, here's the patch I promised in #15. It moves the rid for anonymouse userz from 1 to 0, adds an rid for' administrator' as 1, and leaves rid2 as the authenticated user.

Patch Testing Instructions
To test this patch, drop your existing database, apply the patch, and then perform a new install. Navigate to /admin/user/roles. Observe the new role called 'administrator, then go to admin/user/permissions and try out some settings. Any permissions you change should stick.

Also notice that there are no users yet assigned to this administrator role, which means that uid1 *must* explicitly assign users to this role in order to open up any semblance of security risk. Cool.

Two Notes:
1. If this approach is somewhat acceptable, I will be providing Simpletests for all of this. No, the simpletest doesn't pass right now, so please don't even think of applying this to core yet.

2. There needs to be an upgrade hook for system.install. That process needs some debate from you guys as to how best to support all database types.

Attachment	Size
new_administrator_role.patch	8.96 KB

This looks pretty good to me (although I don't think this is the patch to get 'node' back into user interface text sadly).

If we had an administrator role, I'd quite like to take 'create pages' away from authenticated users and give it to admin users on install - would differentiate the two content types a bit more than they currently are.

The help text needs work, and I'll volunteer to give it a try. I share catch's concern about node, plus, for better or worse, this goes against the trend began in Drupal 6 (that some disagree with) where we separated out some of the HTML (like li, etc) from translatable text).

Keith Smith: I would *love* some help on the help text. I rewrote it to try and explain the new role, then realized it was eye-glazingly long. Help a brother out?

@Catch: What are you referring to by "get 'node' back into user interface text"?

@All: Here are the roles that an 'administrator' would have by default (i.e., not uid1, but any other user who was granted this new role by the site's creator):

* administer blocks
* access comments (Normally given to authenticated users as well)
* administer comments
* post comments (Normally given to authenticated users as well)
* post comments without approval (Normally given to authenticated users as well)
* administer filters
* administer menu
* access content (Normally given to authenticated users as well)
* administer content types
* administer nodes
* access administration pages
* administer site configuration
* select different theme
* administer taxonomy
* access user profiles
* administer permissions
* administer users

Let's discuss the merits or pitfalls of these default perms for the Administrator role.

(I'm changing the status back to CNR to get more eyes on this. Keith, submit your patch to change the text at anytime. This issue won't get committed to core until we work on the text a bit, I'm sure, but we need some testers as well.)

Senpai: Your help text mentions the word 'node', there's been a couple of year process removing 'node' from the UI (which I'd like to see reversed), it should probably say 'content' or 'posts' or something instead for now.

I'd personally be in favour of the administrator role having all permissions by default, to encourage people to use an account other than user/1 for day-to-day administration.

a good idea. i'd even go a step further and create a an editor role that has administer nodes and maybe a couple more.

@Moshe: I'd thought about that too, but the trixy thing is this; rid3 is probably already used up on an existing site. I really didn't wanna delete the existing rid3 in favor of creating a default 'editor' role, even though that is one of the roles that most people create on every new site that gets installed.

That's also part of why the hook_update() is being so difficult right now. You can see in the existing patch that
system.install performs a

which takes the three newly installed roles and change them from 1,2,3 to 0,1,2 then resets the table's auto_increment counter so that the next admin-created role gets a 3, thereby avoiding a hole a position 3.

During a site update, however, rid 3 is most likely in use, and thus, this same approach doesn't werk so goot as a hook_update(). I need ideas, people.

I don't think there is a problem with having a "hole" for the role id 3. That's exactly what you would get if you created a few new roles then deleted the one with role id 3. Auto-incrementing just guarantees that every new entry in a table has a unique value for the specified column, not that all values in that column are consecutive.

@flobruit: Ahh, but the system.install *can* easily avoid a hole at position 3, whereas an update hook must respect an already existing rid3. That's the dilemma. We can install a series of rids on a clean slate, but during a hook_update, how do I successfully implement a database agnostic way of creating rid0, rid1, and rid2 without messing up the existing rid3?

Maybe move rid3 out of the way temporarily?

Why bother with an upgrade for this feature? Seems to me that it will do more harm than good. Let those sites be.

I agree with Moshe. Leave this for new sites only - sites upgrading to D7 should have their roles fixed up by the time they get there. And also having an editor role in there would be great.

Having said that, if it's possible, I'd rather see this in the default install profile than system.install - so we can the minimum install profile won't get them if it goes in, and other install profiles can define their own roles and permissions for particular use cases.

Oh wait. You're talking about making this another reserved role, that's guaranteed to be there for modules etc., in that case we probably need an upgrade path since a lot of code is going to depend on these constants. hmm.

Yeah, I was thinking that I needed to make an upgrade path from D6 for this new feature, or we'll end up with a lot of conversations that go like this:

"Ok, navigate to admin/user/permissions, and uncheck the 'administer users' box for your Administrator role. That'll fix you right up."

"Huh, I don't have an 'administrator' role. What now?"

Here's my thoughts while driving to work this morning. During the upgrade, run a db_query to get the contents of rid3, store it in an array, perform the UPDATE that installs rid0, 1 and 2, reset the AUTO_INCREMENT value to 3, then INSERT the contents of the array, thereby preserving the original rid3.

This does not help with the proposed creation of an rid for 'editor'. Hmm, needs more thought.

Why not just increment every non-core rid +1 (or 2 if editor gets added)? The only place rid should ever be checked is in code, and we don't need to provide backwards compatibility for code, just upgrade instructions.

Bdragon mentioned access control lists via IRC. That's something I hadn't even thought of. It needs some checking into, to be sure.

Sorry, but, an editor role is a) not in the scope of this issue, and b) IMHO, not very Drupalish. If you need one, add it to your install profile. If we would follow this path, we could also turn *all* roles into meta-roles. What would we gain from that? Lots of code in modules that need to check for even more special roles. Just have a look at all the modules that need to deal with DRUPAL_ANONYMOUS_RID and DRUPAL_AUTHENTICATED_RID already and you (hopefully) get the point, why this would suck.

Incrementing rids will be a mess for existing custom code. I'd rather avoid changing rids and potentially wrecking custom access control code.

A better solution is how the filter module does it. It uses a constant called FILTER_FORMAT_DEFAULT which points to a dynamic format as determined by filter_resolve_format($format).

Maybe something like this might work:

In this way, authenticated users stay at rid = 2, just like they always were. Webchick suspects that AUTO_INCREMENT might be a MySQL only function. I wonder if that means this statement breaks on non-MySQL databases. Does db_query() take things like that into account?

##Patch number 2

This one satisfies Catch's comment #26 by removing the word 'node' from the help text, and changes some of the help text that I'd previously missed. It addresses Keith.Smith's comment #24 by separating the HTML markup from the t() strings. Also included is an upgrade path from D6 that should, to the best of my knowledge, work across all databases.

Attachment	Size
new_administrator_role_0.patch	15.73 KB

The text is almost there. I'll have to read it again. I notice one inconsistency in capitalizing Role vs. role in user-facing text. Usually these days, when we refer to something that occurs exactly the same way in the interface we just italicize it, so that may be an option.

The last patch in this issue was no longer applying to the latest dev version, so I attempted to recreate it (attached), with two small changes. The first change was that there was already a function system_update_7010() so I made the new one system_update_7011(). The second change relates to the problem I had trying to run the update. The original patch had:

However, that gave me an error so I tried

which I also see within system.install, however it spits out an error ( Undefined index: db_type in drupal7\modules\system\system.install on line 3087. ) and aborts the update.

If I manually apply the aborted updates then the changes work as expected and are a much needed improvement to the Drupal default roles. Can someone help me with the system.install error so we can get this patch back on track?

Attachment	Size
new_administrator_role_1.patch	0 bytes

Sorry, I forgot to run the last patch through dos2unix. The attached patch has been run through it...

Attachment	Size
new_administrator_role_1.patch	15.49 KB

Alex, that's almost certainly been changed with the new database layer. I'm not sure what you'd need to do with the new db layer though for it to play nice.

Just to provide another example of why something like this is important, the website of a local politician was recently "cracked." The reason: they left it open for users to register and then granted all permissions to the authenticated role. If they had an admin role on the site, they probably wouldn't have given all the permission to the Authenticated role. So, I agree with the arguments that this is a usability and security enhancement.

Setting back to needs work given the pdo changes.

A quick IRC convo with Bdragon to refresh my memory on why ACLists was important to this patch, which I referenced in #36. The jist of it is this,
it was about setting defaults easily. Adding defaults to hook_perm so that an official "admin" role would get the right permissions added automatically when installing a contrib module.

I've written this down here so that when i come back to this patch, I'll be able to check this angle out. Oh, and yeah, like AlexUA said, I'm gonna need help with the PDO database layer changes. I've read the books, but AFAIK, this isn't straight PDO is it. It's Drupal PDO! ;-)