- Patch #300993 by sun: extended the roles and permissions API so we can revoke permissions from a role etc.

6 files changed, 105 insertions, 30 deletions

diff --git a/modules/simpletest/drupal_web_test_case.php b/modules/simpletest/drupal_web_test_case.php
index 548f5a45b..c49d3ad9a 100644
--- a/modules/simpletest/drupal_web_test_case.php
+++ b/modules/simpletest/drupal_web_test_case.php
@@ -901,7 +901,7 @@ class DrupalWebTestCase extends DrupalTestCase {
     $role = new stdClass();
     $role->name = $name;
     user_role_save($role);
-    user_role_set_permissions($role->name, $permissions);
+    user_role_grant_permissions($role->rid, $permissions);
 
     $this->assertTrue(isset($role->rid), t('Created role of name: @name, id: @rid', array('@name' => $name, '@rid' => (isset($role->rid) ? $role->rid : t('-n/a-')))), t('Role'));
     if ($role && !empty($role->rid)) {
diff --git a/modules/user/user.admin.inc b/modules/user/user.admin.inc
index 8b944d324..6f6bf5857 100644
--- a/modules/user/user.admin.inc
+++ b/modules/user/user.admin.inc
@@ -626,7 +626,7 @@ function user_admin_permissions($form, $form_state, $rid = NULL) {
       $form['permission'][] = array(
         '#markup' => $info['name'],
         '#id' => $module,
-        );
+      );
       foreach ($permissions as $perm => $perm_item) {
         $options[$perm] = '';
         $form['permission'][$perm] = array(
@@ -663,8 +663,7 @@ function user_admin_permissions($form, $form_state, $rid = NULL) {
  */
 function user_admin_permissions_submit($form, &$form_state) {
   foreach ($form_state['values']['role_names'] as $rid => $name) {
-    $permissions = array_filter($form_state['values'][$rid]);
-    user_role_set_permissions($rid, $permissions);
+    user_role_change_permissions($rid, $form_state['values'][$rid]);
   }
 
   drupal_set_message(t('The changes have been saved.'));
diff --git a/modules/user/user.module b/modules/user/user.module
index fabed7b80..2c2c8896d 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -2301,39 +2301,69 @@ function user_role_delete($role) {
     ->condition('rid', $role->rid)
     ->execute();
 
+  module_invoke_all('user_role_delete', $role);
+
   // Clear the user access cache.
   drupal_static_reset('user_access');
   drupal_static_reset('user_role_permissions');
-
-  module_invoke_all('user_role_delete', $role);
 }
 
 /**
- * Assign permissions to a user role.
+ * Change permissions for a user role.
  *
- * @param $role
- *   A string with the role name, or an integer with the role ID.
+ * This function may be used to grant and revoke multiple permissions at once.
+ * For example, when a form exposes checkboxes to configure permissions for a
+ * role, the submitted values may be directly passed on in a form submit
+ * handler.
+ *
+ * @param $rid
+ *   The ID of a user role to alter.
  * @param $permissions
- *   An array of permissions strings.
- * @param $merge
- *   A boolean indicating whether to add permissions or to merge
- *   with all existing permissions.
+ *   An array of permissions, where the key holds the permission name and the
+ *   value is an integer or boolean that determines whether to grant or revoke
+ *   the permission:
+ *   @code
+ *     array(
+ *       'administer nodes' => 0,
+ *       'access user profiles' => 1,
+ *     )
+ *   @endcode
+ *   Existing permissions are not changed, unless specified in $permissions.
+ *
+ * @see user_role_grant_permissions()
+ * @see user_role_revoke_permissions()
  */
-function user_role_set_permissions($role, array $permissions = array(), $merge = FALSE) {
-  $role = user_role_load($role);
-  if (!$merge) {
-    // Delete existing permissions for the role.
-    db_delete('role_permission')
-      ->condition('rid', $role->rid)
-      ->execute();
+function user_role_change_permissions($rid, array $permissions = array()) {
+  // Grant new permissions for the role.
+  $grant = array_filter($permissions);
+  if (!empty($grant)) {
+    user_role_grant_permissions($rid, array_keys($grant));
+  }
+  // Revoke permissions for the role.
+  $revoke = array_diff_assoc($permissions, $grant);
+  if (!empty($revoke)) {
+    user_role_revoke_permissions($rid, array_keys($revoke));
   }
+}
 
-  // Assign the new permissions for the role.
-  foreach ($permissions as $permission_string) {
+/**
+ * Grant permissions to a user role.
+ *
+ * @param $rid
+ *   The ID of a user role to alter.
+ * @param $permissions
+ *   A list of permission names to grant.
+ *
+ * @see user_role_change_permissions()
+ * @see user_role_revoke_permissions()
+ */
+function user_role_grant_permissions($rid, array $permissions = array()) {
+  // Grant new permissions for the role.
+  foreach ($permissions as $name) {
     db_merge('role_permission')
       ->key(array(
-        'rid' => $role->rid,
-        'permission' => $permission_string,
+        'rid' => $rid,
+        'permission' => $name,
       ))
       ->execute();
   }
@@ -2341,8 +2371,29 @@ function user_role_set_permissions($role, array $permissions = array(), $merge =
   // Clear the user access cache.
   drupal_static_reset('user_access');
   drupal_static_reset('user_role_permissions');
+}
 
-  return TRUE;
+/**
+ * Revoke permissions from a user role.
+ *
+ * @param $rid
+ *   The ID of a user role to alter.
+ * @param $permissions
+ *   A list of permission names to revoke.
+ *
+ * @see user_role_change_permissions()
+ * @see user_role_grant_permissions()
+ */
+function user_role_revoke_permissions($rid, array $permissions = array()) {
+  // Revoke permissions for the role.
+  db_delete('role_permission')
+    ->condition('rid', $rid)
+    ->condition('permission', $permissions, 'IN')
+    ->execute();
+
+  // Clear the user access cache.
+  drupal_static_reset('user_access');
+  drupal_static_reset('user_role_permissions');
 }
 
 /**
diff --git a/modules/user/user.test b/modules/user/user.test
index fff085b63..14b7c407c 100644
--- a/modules/user/user.test
+++ b/modules/user/user.test
@@ -928,6 +928,31 @@ class UserPermissionsTestCase extends DrupalWebTestCase {
     $this->drupalPost('admin/config/modules', $edit, t('Save configuration'));
     $this->assertTrue(user_access('administer news feeds', $this->admin_user), t('The permission was automatically assigned to the administrator role'));
   }
+
+  /**
+   * Verify proper permission changes by user_role_change_permissions().
+   */
+  function testUserRoleChangePermissions() {
+    $rid = $this->rid;
+    $account = $this->admin_user;
+
+    // Verify current permissions.
+    $this->assertFalse(user_access('administer nodes', $account), t('User does not have "administer nodes" permission.'));
+    $this->assertTrue(user_access('access user profiles', $account), t('User has "access user profiles" permission.'));
+    $this->assertTrue(user_access('administer site configuration', $account), t('User has "administer site configuration" permission.'));
+
+    // Change permissions.
+    $permissions = array(
+      'administer nodes' => 1,
+      'access user profiles' => 0,
+    );
+    user_role_change_permissions($rid, $permissions);
+
+    // Verify proper permission changes.
+    $this->assertTrue(user_access('administer nodes', $account), t('User now has "administer nodes" permission.'));
+    $this->assertFalse(user_access('access user profiles', $account), t('User no longer has "access user profiles" permission.'));
+    $this->assertTrue(user_access('administer site configuration', $account), t('User still has "administer site configuration" permission.'));
+  }
 }
 
 class UserAdminTestCase extends DrupalWebTestCase {
diff --git a/profiles/default/default.install b/profiles/default/default.install
index 60e5d012e..ff08e62ee 100644
--- a/profiles/default/default.install
+++ b/profiles/default/default.install
@@ -184,15 +184,15 @@ function default_install() {
   db_insert('taxonomy_vocabulary_node_type')->fields(array('vid' => $vid, 'type' => 'article'))->execute();
 
   // Enable default permissions for system roles.
-  user_role_set_permissions(DRUPAL_ANONYMOUS_RID, array('access content'));
-  user_role_set_permissions(DRUPAL_AUTHENTICATED_RID, array('access content', 'access comments', 'post comments', 'post comments without approval'));
+  user_role_grant_permissions(DRUPAL_ANONYMOUS_RID, array('access content'));
+  user_role_grant_permissions(DRUPAL_AUTHENTICATED_RID, array('access content', 'access comments', 'post comments', 'post comments without approval'));
 
 
   // Create a default role for site administrators, with all available permissions assigned.
   $admin_role = new stdClass();
   $admin_role->name = 'administrator';
   user_role_save($admin_role);
-  user_role_set_permissions($admin_role->name, array_keys(module_invoke_all('permission')));
+  user_role_grant_permissions($admin_role->rid, array_keys(module_invoke_all('permission')));
   // Set this as the administrator role.
   variable_set('user_admin_role', $admin_role->rid);
 
diff --git a/profiles/expert/expert.install b/profiles/expert/expert.install
index 4693b84b4..0e47ff934 100644
--- a/profiles/expert/expert.install
+++ b/profiles/expert/expert.install
@@ -68,8 +68,8 @@ function expert_install() {
   $query->execute();  
 
   // Enable default permissions for system roles.
-  user_role_set_permissions(DRUPAL_ANONYMOUS_RID, array('access content'));
-  user_role_set_permissions(DRUPAL_AUTHENTICATED_RID, array('access content', 'access comments', 'post comments', 'post comments without approval'));
+  user_role_grant_permissions(DRUPAL_ANONYMOUS_RID, array('access content'));
+  user_role_grant_permissions(DRUPAL_AUTHENTICATED_RID, array('access content', 'access comments', 'post comments', 'post comments without approval'));
 }