- Patch #248598 by David_Rothstein, pwolanin, aspilicious: improved warning about dangerous permissions.

8 files changed, 70 insertions, 19 deletions

diff --git a/modules/filter/filter.module b/modules/filter/filter.module
index 126ce1123..bec8d947a 100644
--- a/modules/filter/filter.module
+++ b/modules/filter/filter.module
@@ -290,7 +290,7 @@ function filter_admin_format_title($format) {
 function filter_permission() {
   $perms['administer filters'] = array(
     'title' => t('Administer text formats and filters'),
-    'description' => drupal_placeholder(array('text' => t('Warning: Give to trusted roles only; this permission has security implications.'))),
+    'restrict access' => TRUE,
   );
 
   // Generate permissions for each text format. Warn the administrator that any
diff --git a/modules/node/node.module b/modules/node/node.module
index f91e63e92..58c0b9a30 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -1410,11 +1410,11 @@ function node_permission() {
   $perms = array(
     'administer content types' => array(
       'title' => t('Administer content types'),
-      'description' => drupal_placeholder(array('text' => t('Warning: Give to trusted roles only; this permission has security implications.'))),
+      'restrict access' => TRUE,
     ),
     'administer nodes' => array(
       'title' => t('Administer content'),
-      'description' => drupal_placeholder(array('text' => t('Warning: Give to trusted roles only; this permission has security implications.'))),
+      'restrict access' => TRUE,
     ),
     'access content' => array(
       'title' => t('View published content'),
@@ -1424,7 +1424,8 @@ function node_permission() {
     ),
     'bypass node access' => array(
       'title' => t('Bypass content access control'),
-      'description' => t('View, edit and delete all content regardless of permission restrictions. %warning', array('%warning' => t('Warning: Give to trusted roles only; this permission has security implications.'))),
+      'description' => t('View, edit and delete all content regardless of permission restrictions.'),
+      'restrict access' => TRUE,
     ),
     'view revisions' => array(
       'title' => t('View content revisions'),
diff --git a/modules/php/php.module b/modules/php/php.module
index d5a082f42..4b1ac5b6d 100644
--- a/modules/php/php.module
+++ b/modules/php/php.module
@@ -31,7 +31,7 @@ function php_permission() {
   return array(
     'use PHP for settings' => array(
       'title' => t('Use PHP for settings'),
-      'description' => drupal_placeholder(array('text' => t('Warning: Give to trusted roles only; this permission has security implications.'))),
+      'restrict access' => TRUE,
     ),
   );
 }
diff --git a/modules/simpletest/simpletest.module b/modules/simpletest/simpletest.module
index 15cb2353a..56d7fa8ba 100644
--- a/modules/simpletest/simpletest.module
+++ b/modules/simpletest/simpletest.module
@@ -68,7 +68,7 @@ function simpletest_permission() {
   return array(
     'administer unit tests' => array(
       'title' => t('Administer tests'),
-      'description' => drupal_placeholder(array('text' => t('Warning: Give to trusted roles only; this permission has security implications.'))),
+      'restrict access' => TRUE,
     ),
   );
 }
diff --git a/modules/system/system.api.php b/modules/system/system.api.php
index bd42f2292..c43bcf05e 100644
--- a/modules/system/system.api.php
+++ b/modules/system/system.api.php
@@ -1012,13 +1012,22 @@ function hook_system_info_alter(&$info, $file, $type) {
  * For a detailed usage example, see page_example.module.
  *
  * @return
- *   An array of which permission names are the keys and their corresponding
- *   values are descriptions of each permission.
- *   The permission names (keys of the array) must not be wrapped with
- *   the t() function, since the string extractor takes care of
- *   extracting permission names defined in the perm hook for
- *   translation. The permission descriptions (values of the array)
- *   should be wrapped in the t() function so they can be translated.
+ *   An array whose keys are permission names and whose corresponding values
+ *   are arrays containing the following key-value pairs:
+ *   - title: The human-readable name of the permission, to be shown on the
+ *     permission administration page. This should be wrapped in the t()
+ *     function so it can be translated.
+ *   - description: (optional) A description of what the permission does. This
+ *     should be wrapped in the t() function so it can be translated.
+ *   - restrict access: (optional) A boolean which can be set to TRUE to
+ *     indicate that site administrators should restrict access to this
+ *     permission to trusted users. This should be used for permissions that
+ *     have inherent security risks across a variety of potential use cases
+ *     (for example, the "administer filters" and "bypass node access"
+ *     permissions provided by Drupal core). When set to TRUE, a standard
+ *     warning message defined in user_admin_permissions() will be associated
+ *     with the permission and displayed with it on the permission
+ *     administration page. Defaults to FALSE.
  */
 function hook_permission() {
   return array(
diff --git a/modules/system/system.module b/modules/system/system.module
index f4b31ce1c..2154d0573 100644
--- a/modules/system/system.module
+++ b/modules/system/system.module
@@ -207,7 +207,7 @@ function system_permission() {
     ),
     'administer site configuration' => array(
       'title' => t('Administer site configuration'),
-      'description' => drupal_placeholder(array('text' => t('Warning: Give to trusted roles only; this permission has security implications.'))),
+      'restrict access' => TRUE,
     ),
     'administer themes' => array(
       'title' => t('Administer themes'),
diff --git a/modules/user/user.admin.inc b/modules/user/user.admin.inc
index 09e3c554e..23fc14c74 100644
--- a/modules/user/user.admin.inc
+++ b/modules/user/user.admin.inc
@@ -663,7 +663,7 @@ function user_admin_permissions($form, $form_state, $rid = NULL) {
   // Render role/permission overview:
   $options = array();
   $module_info = system_get_info('module');
-  $hide_descriptions = !system_admin_compact_mode();
+  $hide_descriptions = system_admin_compact_mode();
 
   // Get a list of all the modules implementing a hook_permission() and sort by
   // display name.
@@ -680,11 +680,17 @@ function user_admin_permissions($form, $form_state, $rid = NULL) {
         '#id' => $module,
       );
       foreach ($permissions as $perm => $perm_item) {
+        // Fill in default values for the permission.
+        $perm_item += array(
+          'description' => '',
+          'restrict access' => FALSE,
+          'warning' => !empty($perm_item['restrict access']) ? t('Warning: Give to trusted roles only; this permission has security implications.') : '',
+        );
         $options[$perm] = '';
         $form['permission'][$perm] = array(
           '#type' => 'item',
           '#markup' => $perm_item['title'],
-          '#description' => $hide_descriptions && isset($perm_item['description']) ? $perm_item['description'] : NULL,
+          '#description' => theme('user_permission_description', array('permission_item' => $perm_item, 'hide' => $hide_descriptions)),
         );
         foreach ($role_names as $rid => $name) {
           // Builds arrays for checked boxes for each role
@@ -764,6 +770,37 @@ function theme_user_admin_permissions($variables) {
 }
 
 /**
+ * Theme an individual permission description.
+ *
+ * @param $variables
+ *   An associative array containing:
+ *   - permission_item: An associative array representing the permission whose
+ *     description is being themed. Useful keys include:
+ *     - description: The text of the permission description.
+ *     - warning: A security-related warning message about the permission (if
+ *       there is one).
+ *   - hide: A boolean indicating whether or not the permission description was
+ *     requested to be hidden rather than shown.
+ *
+ * @ingroup themeable
+ */
+function theme_user_permission_description($variables) {
+  if (!$variables['hide']) {
+    $description = array();
+    $permission_item = $variables['permission_item'];
+    if (!empty($permission_item['description'])) {
+      $description[] = $permission_item['description'];
+    }
+    if (!empty($permission_item['warning'])) {
+      $description[] = '<em class="permission-warning">' . $permission_item['warning'] . '</em>';
+    }
+    if (!empty($description)) {
+      return implode(' ', $description);
+    }
+  }
+}
+
+/**
  * Menu callback: administer roles.
  *
  * @ingroup forms
diff --git a/modules/user/user.module b/modules/user/user.module
index 2ec522113..ac92802cf 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -110,6 +110,10 @@ function user_theme() {
       'render element' => 'form',
       'file' => 'user.admin.inc',
     ),
+    'user_permission_description' => array(
+      'variables' => array('permission_item' => NULL, 'hide' => NULL),
+      'file' => 'user.admin.inc',
+    ),
     'user_signature' => array(
       'variables' => array('signature' => NULL),
     ),
@@ -735,11 +739,11 @@ function user_permission() {
   return array(
     'administer permissions' =>  array(
       'title' => t('Administer permissions'),
-      'description' => drupal_placeholder(array('text' => t('Warning: Give to trusted roles only; this permission has security implications.'))),
+      'restrict access' => TRUE,
     ),
     'administer users' => array(
       'title' => t('Administer users'),
-      'description' => drupal_placeholder(array('text' => t('Warning: Give to trusted roles only; this permission has security implications.'))),
+      'restrict access' => TRUE,
     ),
     'access user profiles' => array(
       'title' => t('View user profiles'),
@@ -753,7 +757,7 @@ function user_permission() {
     ),
     'select account cancellation method' => array(
       'title' => t('Select method for cancelling own account'),
-      'description' => drupal_placeholder(array('text' => t('Warning: Give to trusted roles only; this permission has security implications.'))),
+      'restrict access' => TRUE,
     ),
   );
 }