Avoid direct access to $wgGroupPermissions
authorumherirrender <umherirrender_de.wp@web.de>
Thu, 4 Oct 2012 16:17:46 +0000 (18:17 +0200)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 9 Oct 2012 06:41:23 +0000 (06:41 +0000)
Created a new method User::groupHasPermission and check also
$wgRevokePermissions for the given right

Change-Id: I41edb091fa35c8c68b6f95cc5fd208ea99418cdb

includes/OutputPage.php
includes/ProtectionForm.php
includes/SpecialPage.php
includes/Title.php
includes/User.php
includes/actions/RawAction.php
includes/specials/SpecialNewpages.php

index 02f22d9..4fa33ac 100644 (file)
@@ -2071,8 +2071,6 @@ class OutputPage extends ContextSource {
         * @param $action String: action that was denied or null if unknown
         */
        public function showPermissionsErrorPage( $errors, $action = null ) {
-               global $wgGroupPermissions;
-
                // For some action (read, edit, create and upload), display a "login to do this action"
                // error if all of the following conditions are met:
                // 1. the user is not logged in
@@ -2081,8 +2079,8 @@ class OutputPage extends ContextSource {
                if ( in_array( $action, array( 'read', 'edit', 'createpage', 'createtalk', 'upload' ) )
                        && $this->getUser()->isAnon() && count( $errors ) == 1 && isset( $errors[0][0] )
                        && ( $errors[0][0] == 'badaccess-groups' || $errors[0][0] == 'badaccess-group0' )
-                       && ( ( isset( $wgGroupPermissions['user'][$action] ) && $wgGroupPermissions['user'][$action] )
-                       || ( isset( $wgGroupPermissions['autoconfirmed'][$action] ) && $wgGroupPermissions['autoconfirmed'][$action] ) )
+                       && ( User::groupHasPermission( 'user', $action )
+                       || User::groupHasPermission( 'autoconfirmed', $action ) )
                ) {
                        $displayReturnto = null;
 
index ce0e36b..beb20ea 100644 (file)
@@ -286,12 +286,10 @@ class ProtectionForm {
 
                # They shouldn't be able to do this anyway, but just to make sure, ensure that cascading restrictions aren't being applied
                #  to a semi-protected page.
-               global $wgGroupPermissions;
-
                $edit_restriction = isset( $this->mRestrictions['edit'] ) ? $this->mRestrictions['edit'] : '';
                $this->mCascade = $wgRequest->getBool( 'mwProtect-cascade' );
                if ($this->mCascade && ($edit_restriction != 'protect') &&
-                       !(isset($wgGroupPermissions[$edit_restriction]['protect']) && $wgGroupPermissions[$edit_restriction]['protect'] ) )
+                       !User::groupHasPermission( $edit_restriction, 'protect' ) )
                        $this->mCascade = false;
 
                $status = $this->mArticle->doUpdateRestrictions( $this->mRestrictions, $expiry, $this->mCascade, $reasonstr, $wgUser );
@@ -600,11 +598,11 @@ class ProtectionForm {
        }
 
        function buildCleanupScript() {
-               global $wgRestrictionLevels, $wgGroupPermissions, $wgOut;
+               global $wgRestrictionLevels, $wgOut;
 
                $cascadeableLevels = array();
                foreach( $wgRestrictionLevels as $key ) {
-                       if ( ( isset( $wgGroupPermissions[$key]['protect'] ) && $wgGroupPermissions[$key]['protect'] )
+                       if ( User::groupHasPermission( $key, 'protect' )
                                || $key == 'protect'
                        ) {
                                $cascadeableLevels[] = $key;
index 5b5d0a4..09683a2 100644 (file)
@@ -519,9 +519,8 @@ class SpecialPage {
         *   pages?
         */
        public function isRestricted() {
-               global $wgGroupPermissions;
                // DWIM: If all anons can do something, then it is not restricted
-               return $this->mRestriction != '' && empty( $wgGroupPermissions['*'][$this->mRestriction] );
+               return $this->mRestriction != '' && User::groupHasPermission( '*', $this->mRestriction );
        }
 
        /**
index 3573198..b22a8a1 100644 (file)
@@ -1723,15 +1723,8 @@ class Title {
 
                        if ( !$user->isAllowed( 'move' ) ) {
                                // User can't move anything
-                               global $wgGroupPermissions;
-                               $userCanMove = false;
-                               if ( isset( $wgGroupPermissions['user']['move'] ) ) {
-                                       $userCanMove = $wgGroupPermissions['user']['move'];
-                               }
-                               $autoconfirmedCanMove = false;
-                               if ( isset( $wgGroupPermissions['autoconfirmed']['move'] ) ) {
-                                       $autoconfirmedCanMove = $wgGroupPermissions['autoconfirmed']['move'];
-                               }
+                               $userCanMove = User::groupHasPermission( 'user', 'move' );
+                               $autoconfirmedCanMove = User::groupHasPermission( 'autoconfirmed', 'move' );
                                if ( $user->isAnon() && ( $userCanMove || $autoconfirmedCanMove ) ) {
                                        // custom message if logged-in users without any special rights can move
                                        $errors[] = array( 'movenologintext' );
@@ -2072,13 +2065,13 @@ class Title {
         * @return Array list of errors
         */
        private function checkReadPermissions( $action, $user, $errors, $doExpensiveQueries, $short ) {
-               global $wgWhitelistRead, $wgGroupPermissions, $wgRevokePermissions;
+               global $wgWhitelistRead, $wgRevokePermissions;
                static $useShortcut = null;
 
                # Initialize the $useShortcut boolean, to determine if we can skip quite a bit of code below
                if ( is_null( $useShortcut ) ) {
                        $useShortcut = true;
-                       if ( empty( $wgGroupPermissions['*']['read'] ) ) {
+                       if ( !User::groupHasPermission( '*', 'read' ) ) {
                                # Not a public wiki, so no shortcut
                                $useShortcut = false;
                        } elseif ( !empty( $wgRevokePermissions ) ) {
index 4ab90ed..4332e2b 100644 (file)
@@ -3640,14 +3640,27 @@ class User {
        public static function getGroupsWithPermission( $role ) {
                global $wgGroupPermissions;
                $allowedGroups = array();
-               foreach ( $wgGroupPermissions as $group => $rights ) {
-                       if ( isset( $rights[$role] ) && $rights[$role] ) {
+               foreach ( array_keys( $wgGroupPermissions ) as $group ) {
+                       if ( self::groupHasPermission( $group, $role ) ) {
                                $allowedGroups[] = $group;
                        }
                }
                return $allowedGroups;
        }
 
+       /**
+        * Check, if the given group has the given permission
+        *
+        * @param $group String Group to check
+        * @param $role String Role to check
+        * @return bool
+        */
+       public static function groupHasPermission( $group, $role ) {
+               global $wgGroupPermissions, $wgRevokePermissions;
+               return isset( $wgGroupPermissions[$group][$role] ) && $wgGroupPermissions[$group][$role]
+                       && !( isset( $wgRevokePermissions[$group][$role] ) && $wgRevokePermissions[$group][$role] );
+       }
+
        /**
         * Get the localized descriptive name for a group, if it exists
         *
index 174ca3f..8079493 100644 (file)
@@ -46,7 +46,7 @@ class RawAction extends FormlessAction {
        }
 
        function onView() {
-               global $wgGroupPermissions, $wgSquidMaxage, $wgForcedRawSMaxage, $wgJsMimeType;
+               global $wgSquidMaxage, $wgForcedRawSMaxage, $wgJsMimeType;
 
                $this->getOutput()->disable();
                $request = $this->getRequest();
@@ -91,7 +91,7 @@ class RawAction extends FormlessAction {
                $response->header( 'Content-type: ' . $contentType . '; charset=UTF-8' );
                # Output may contain user-specific data;
                # vary generated content for open sessions on private wikis
-               $privateCache = !$wgGroupPermissions['*']['read'] && ( $smaxage == 0 || session_id() != '' );
+               $privateCache = !User::groupHasPermission( '*', 'read' ) && ( $smaxage == 0 || session_id() != '' );
                # allow the client to cache this for 24 hours
                $mode = $privateCache ? 'private' : 'public';
                $response->header( 'Cache-Control: ' . $mode . ', s-maxage=' . $smaxage . ', max-age=' . $maxage );
index 8e15d55..34bc7a4 100644 (file)
@@ -164,8 +164,6 @@ class SpecialNewpages extends IncludableSpecialPage {
        }
 
        protected function filterLinks() {
-               global $wgGroupPermissions;
-
                // show/hide links
                $showhide = array( $this->msg( 'show' )->escaped(), $this->msg( 'hide' )->escaped() );
 
@@ -181,8 +179,7 @@ class SpecialNewpages extends IncludableSpecialPage {
                }
 
                // Disable some if needed
-               # @todo FIXME: Throws E_NOTICEs if not set; and doesn't obey hooks etc.
-               if ( $wgGroupPermissions['*']['createpage'] !== true ) {
+               if ( !User::groupHasPermission( '*', 'createpage' ) ) {
                        unset( $filters['hideliu'] );
                }
                if ( !$this->getUser()->useNPPatrol() ) {
@@ -488,7 +485,7 @@ class NewPagesPager extends ReverseChronologicalPager {
        }
 
        function getQueryInfo() {
-               global $wgEnableNewpagesUserFilter, $wgGroupPermissions;
+               global $wgEnableNewpagesUserFilter;
                $conds = array();
                $conds['rc_new'] = 1;
 
@@ -510,7 +507,7 @@ class NewPagesPager extends ReverseChronologicalPager {
                        $conds['rc_user_text'] = $user->getText();
                        $rcIndexes = 'rc_user_text';
                # If anons cannot make new pages, don't "exclude logged in users"!
-               } elseif( $wgGroupPermissions['*']['createpage'] && $this->opts->getValue( 'hideliu' ) ) {
+               } elseif( User::groupHasPermission( '*', 'createpage' ) && $this->opts->getValue( 'hideliu' ) ) {
                        $conds['rc_user'] = 0;
                }
                # If this user cannot see patrolled edits or they are off, don't do dumb queries!