Index: modules/system/system.install =================================================================== RCS file: /cvs/drupal/drupal/modules/system/system.install,v retrieving revision 1.243 diff -u -p -r1.243 system.install --- modules/system/system.install 17 Mar 2008 16:53:58 -0000 1.243 +++ modules/system/system.install 19 Mar 2008 02:35:09 -0000 @@ -366,11 +366,18 @@ function system_install() { // This sets uid 1 (superuser). We skip uid 2 but that's not a big problem. db_query("UPDATE {users} SET uid = 1 WHERE name = '%s'", 'placeholder-for-uid-1'); + // Built-in roles. db_query("INSERT INTO {role} (name) VALUES ('%s')", 'anonymous user'); db_query("INSERT INTO {role} (name) VALUES ('%s')", 'authenticated user'); - db_query("INSERT INTO {permission} (rid, perm, tid) VALUES (%d, '%s', %d)", 1, 'access content', 0); - db_query("INSERT INTO {permission} (rid, perm, tid) VALUES (%d, '%s', %d)", 2, 'access comments, access content, post comments, post comments without approval', 0); + // Anonymous role permissions. + db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", 1, 'access content'); + + // Authenticated role permissions. + db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", 2, 'access comments'); + db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", 2, 'access content'); + db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", 2, 'post comments'); + db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", 2, 'post comments without approval'); db_query("INSERT INTO {variable} (name, value) VALUES ('%s', '%s')", 'theme_default', 's:7:"garland";'); db_query("UPDATE {system} SET status = %d WHERE type = '%s' AND name = '%s'", 1, 'theme', 'garland'); @@ -2659,8 +2666,38 @@ function system_update_7000() { } /** - * @} End of "defgroup updates-6.x-to-7.x" - * The next series of updates should start at 8000. + * Convert to new method of storing permissions. */ +function system_update_7001() { + $ret = array(); + db_create_table($ret, 'role_permission', array( + 'fields' => array( + 'rid' => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE), + // @TODO: Is this the correct max length for a role name? + 'permission' => array('type' => 'varchar', 'length' => 255, 'not null' => TRUE, 'default' => ''), + ), + 'primary key' => array('rid', 'permission'), + 'indexes' => array( + 'permission' => array('permission'), + ), + )); + // Copy the permissions from the old {permission} table to the new {role_permission} table. + $result = db_query("SELECT rid, perm FROM {permission} ORDER BY rid"); + while ($role = db_fetch_object($result)) { + $permissions = explode(', ', $role->perm); + foreach ($permissions as $permission) { + db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", $role->rid, $permission); + } + } + + db_drop_table($ret, 'permission'); + + return $ret; +} + +/** + * @} End of "defgroup updates-6.x-to-7.x" + * The next series of updates should start at 8000. + */ Index: modules/user/user.admin.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/user/user.admin.inc,v retrieving revision 1.19 diff -u -p -r1.19 user.admin.inc --- modules/user/user.admin.inc 20 Feb 2008 13:46:43 -0000 1.19 +++ modules/user/user.admin.inc 19 Mar 2008 02:35:09 -0000 @@ -494,18 +494,15 @@ function user_admin_settings() { */ function user_admin_perm($form_state, $rid = NULL) { if (is_numeric($rid)) { - $result = db_query('SELECT r.rid, p.perm FROM {role} r LEFT JOIN {permission} p ON r.rid = p.rid WHERE r.rid = %d', $rid); + $result = db_query('SELECT r.rid, p.permission FROM {role} r LEFT JOIN {role_permission} p ON r.rid = p.rid WHERE r.rid = %d', $rid); } else { - $result = db_query('SELECT r.rid, p.perm FROM {role} r LEFT JOIN {permission} p ON r.rid = p.rid ORDER BY name'); + $result = db_query('SELECT r.rid, p.permission FROM {role} r LEFT JOIN {role_permission} p ON r.rid = p.rid ORDER BY name'); } - // Compile role array: - // Add a comma at the end so when searching for a permission, we can - // always search for "$perm," to make sure we do not confuse - // permissions that are substrings of each other. - while ($role = db_fetch_object($result)) { - $role_permissions[$role->rid] = $role->perm .','; + // Compile role array. + while ($perm = db_fetch_object($result)) { + $role_permissions[$perm->rid][$perm->permission] = 1; } // Retrieve role names for columns. @@ -537,7 +534,7 @@ function user_admin_perm($form_state, $r ); foreach ($role_names as $rid => $name) { // Builds arrays for checked boxes for each role - if (strpos($role_permissions[$rid], $perm .',') !== FALSE) { + if (isset($role_permissions[$rid][$perm])) { $status[$rid][] = $perm; } } @@ -545,6 +542,8 @@ function user_admin_perm($form_state, $r } } + $form['checkboxes']['#tree'] = TRUE; + // Have to build checkboxes here after checkbox arrays are built foreach ($role_names as $rid => $name) { $form['checkboxes'][$rid] = array('#type' => 'checkboxes', '#options' => $options, '#default_value' => isset($status[$rid]) ? $status[$rid] : array()); @@ -556,22 +555,46 @@ function user_admin_perm($form_state, $r } function user_admin_perm_submit($form, &$form_state) { - // Save permissions: - $result = db_query('SELECT * FROM {role}'); - while ($role = db_fetch_object($result)) { - if (isset($form_state['values'][$role->rid])) { - // Delete, so if we clear every checkbox we reset that role; - // otherwise permissions are active and denied everywhere. - db_query('DELETE FROM {permission} WHERE rid = %d', $role->rid); - $form_state['values'][$role->rid] = array_filter($form_state['values'][$role->rid]); - if (count($form_state['values'][$role->rid])) { - db_query("INSERT INTO {permission} (rid, perm) VALUES (%d, '%s')", $role->rid, implode(', ', array_keys($form_state['values'][$role->rid]))); + /** + * @TODO: This way allows for showing a subset of the available permissions, + * and only affecting those with changes. This may or may not actually be desired. + * If it is decided that this isn't needed behavior, we can get by with just + * wiping everything by rid. + */ + + foreach ($form_state['values']['checkboxes'] as $rid => $value) { + $checked = array_filter($value); + $permissions = array_keys($form['checkboxes'][$rid]['#options']); + foreach ($permissions as $permission) { + // Delete first, so we can easily "unset" permissions when unchecking. + db_query("DELETE FROM {role_permission} WHERE rid = %d AND permission = '%s'", $rid, $permission); + if (isset($checked[$permission])) { + db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", $rid, $permission); } } } + /** + * @TODO: And here's the other way. + * This assumes that all checkboxes are being shown to the admin. + * (The old way had this assumption.) + */ +/* + foreach ($form_state['values']['checkboxes'] as $rid => $value) { + $checked = array_keys(array_filter($value)); + // Delete existing permissions for the role. This handles "unchecking" checkboxes. + db_query('DELETE FROM {role_permission} WHERE rid = %d', $rid); + foreach ($checked as $permission) { + db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", $rid, $permission); + } + } +*/ + drupal_set_message(t('The changes have been saved.')); + // Clear cached permissions. + cache_clear_all('role_perm:', 'cache', TRUE); + // Clear the cached pages cache_clear_all(); } @@ -697,7 +720,7 @@ function user_admin_role_submit($form, & } else if ($form_state['values']['op'] == t('Delete role')) { db_query('DELETE FROM {role} WHERE rid = %d', $form_state['values']['rid']); - db_query('DELETE FROM {permission} WHERE rid = %d', $form_state['values']['rid']); + db_query('DELETE FROM {role_permission} WHERE rid = %d', $form_state['values']['rid']); // Update the users who have this role set: db_query('DELETE FROM {users_roles} WHERE rid = %d', $form_state['values']['rid']); Index: modules/user/user.install =================================================================== RCS file: /cvs/drupal/drupal/modules/user/user.install,v retrieving revision 1.7 diff -u -p -r1.7 user.install --- modules/user/user.install 15 Mar 2008 12:31:29 -0000 1.7 +++ modules/user/user.install 19 Mar 2008 02:35:09 -0000 @@ -74,38 +74,26 @@ function user_schema() { 'primary key' => array('aid'), ); - $schema['permission'] = array( - 'description' => t('Stores permissions for users.'), + $schema['role_permission'] = array( + 'description' => t('Stores the permissions assigned to user roles.'), 'fields' => array( - 'pid' => array( - 'type' => 'serial', - 'not null' => TRUE, - 'description' => t('Primary Key: Unique permission ID.'), - ), 'rid' => array( 'type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, - 'default' => 0, - 'description' => t('The {role}.rid to which the permissions are assigned.'), - ), - 'perm' => array( - 'type' => 'text', - 'not null' => FALSE, - 'size' => 'big', - 'description' => t('List of permissions being assigned.'), + 'description' => t('Foreign Key: Role id.'), ), - 'tid' => array( - 'type' => 'int', - 'unsigned' => TRUE, + 'permission' => array( + 'type' => 'varchar', + 'length' => 255, /**@ @todo: Is this the correct max length for a role name? */ 'not null' => TRUE, - 'default' => 0, - 'description' => t('Originally intended for taxonomy-based permissions, but never used.'), + 'default' => '', + 'description' => t('Permission name.'), ), ), - 'primary key' => array('pid'), + 'primary key' => array('rid', 'permission'), 'indexes' => array( - 'rid' => array('rid'), + 'permission' => array('permission'), ), ); Index: modules/user/user.module =================================================================== RCS file: /cvs/drupal/drupal/modules/user/user.module,v retrieving revision 1.896 diff -u -p -r1.896 user.module --- modules/user/user.module 20 Feb 2008 13:46:43 -0000 1.896 +++ modules/user/user.module 19 Mar 2008 02:35:09 -0000 @@ -464,13 +464,27 @@ function user_access($string, $account = // To reduce the number of SQL queries, we cache the user's permissions // in a static variable. if (!isset($perm[$account->uid])) { - $result = db_query("SELECT p.perm FROM {role} r INNER JOIN {permission} p ON p.rid = r.rid WHERE r.rid IN (". db_placeholders($account->roles) .")", array_keys($account->roles)); + $roles = array_keys($account->roles); + sort($roles); + $cache = cache_get('role_perm:'. implode(',', $roles)); + if (!$cache) { + // If we don't have a cached copy of the permissions array for this + // combination of roles, generate it. + // @todo: Is it actually worth putting DISTINCT here? Multiple roles granting a permission + // is perfectly OK code-wise, it's just a balance of restultset size vs the amount of work the db has to do + // vs the amount of time we spend setting a variable to true more than once... + $result = db_query('SELECT DISTINCT permission from {role_permission} WHERE rid IN ('. db_placeholders($roles) .')', $roles); + $cache = array(); + while ($row = db_fetch_object($result)) { + $cache[$row->permission] = 1; + } - $perms = array(); - while ($row = db_fetch_object($result)) { - $perms += array_flip(explode(', ', $row->perm)); + cache_set('role_perm:'. implode(',', $roles), $cache); + } + else { + $cache = $cache->data; } - $perm[$account->uid] = $perms; + $perm[$account->uid] = $cache; } return isset($perm[$account->uid][$string]); @@ -1634,7 +1648,7 @@ function user_roles($membersonly = FALSE ); if (!empty($permission)) { - $result = db_query("SELECT r.* FROM {role} r INNER JOIN {permission} p ON r.rid = p.rid WHERE p.perm LIKE '%%%s%%' ORDER BY r.name", $permission); + $result = db_query("SELECT r.* FROM {role} r INNER JOIN {role_permission} p ON r.rid = p.rid WHERE p.permission = '%s' ORDER BY r.name", $permission); } else { $result = db_query('SELECT * FROM {role} ORDER BY name'); @@ -1892,16 +1906,16 @@ function user_filters() { foreach (module_list() as $module) { if ($permissions = module_invoke($module, 'perm')) { asort($permissions); - foreach ($permissions as $permission) { - $options[t('@module module', array('@module' => $module))][$permission] = t($permission); + foreach ($permissions as $permission => $description) { + $options[t('@module module', array('@module' => $module))][$permission] = t($description); } } } ksort($options); $filters['permission'] = array( 'title' => t('permission'), - 'join' => 'LEFT JOIN {permission} p ON ur.rid = p.rid', - 'where' => " ((p.perm IS NOT NULL AND p.perm LIKE '%%%s%%') OR u.uid = 1) ", + 'join' => 'LEFT JOIN {role_permission} p ON ur.rid = p.rid', + 'where' => " ((p.permission IS NOT NULL AND p.permission = '%s') OR u.uid = 1) ", 'options' => $options, );