Similar to how Drupal 7 provides an 'administrator' role by default (with all permissions), we can and should do the same for the Admin Role module.

CommentFileSizeAuthor
#1 642130-adminrole-install-D6.patch670 bytesDave Reid
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Attached is the simle adminrole.install file that I'm using on my production sites that removes a step that I had to perform manually.

Dave Reid’s picture

Status: Active » Needs review
Bevan’s picture

I didn't apply or test functionality yet (I trust you have), but here are some notes;

We need a @file header.

We should add hook_uninstall() if we are adding a hook_install().

+  $rid = db_result(db_query("SELECT rid FROM {role} WHERE name = 'administrator'"));

I'm not sure this is a good idea, since it might be a security vulnerability (for existing sites). Maybe we should also check that the role has certain permissions like "administer site configuration".

Maybe we should also check for roles like 'admin', and possibly others? E.g. WHERE name IN ('admin', 'administrator').

At the end we need adminrole module to do it's thing – set all permissions for that role.

This review is powered by Dreditor.

Dave Reid’s picture

Status: Needs review » Fixed

Yeah I'm seeing a point that we shouldn't do anything if there is a role name that starts with 'admin' since they might have already started using proper permissions. Committed a modified version of the patch to CVS.

Bevan’s picture

Please upload the patch here and link to the CVS commit message/page; one of these; http://drupal.org/project/cvs/128621

Thanks!

Dave Reid’s picture

Sorry, I already deleted the patch files I had made.

CVS commit for 6.x: http://drupal.org/cvs?commit=293940
CVS commit for 5.x: http://drupal.org/cvs?commit=293942

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.