From 22dd67ea3c9438d1e253d11d0a993a69e168a568 Mon Sep 17 00:00:00 2001 From: umherirrender Date: Thu, 4 Oct 2012 18:17:46 +0200 Subject: [PATCH] Avoid direct access to $wgGroupPermissions Created a new method User::groupHasPermission and check also $wgRevokePermissions for the given right Change-Id: I41edb091fa35c8c68b6f95cc5fd208ea99418cdb --- includes/OutputPage.php | 6 ++---- includes/ProtectionForm.php | 8 +++----- includes/SpecialPage.php | 3 +-- includes/Title.php | 15 ++++----------- includes/User.php | 17 +++++++++++++++-- includes/actions/RawAction.php | 4 ++-- includes/specials/SpecialNewpages.php | 9 +++------ 7 files changed, 30 insertions(+), 32 deletions(-) diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 02f22d994d..4fa33ac49a 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -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; diff --git a/includes/ProtectionForm.php b/includes/ProtectionForm.php index ce0e36b044..beb20eabc7 100644 --- a/includes/ProtectionForm.php +++ b/includes/ProtectionForm.php @@ -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; diff --git a/includes/SpecialPage.php b/includes/SpecialPage.php index 5b5d0a4eaa..09683a2439 100644 --- a/includes/SpecialPage.php +++ b/includes/SpecialPage.php @@ -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 ); } /** diff --git a/includes/Title.php b/includes/Title.php index 3573198b2c..b22a8a1c0d 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -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 ) ) { diff --git a/includes/User.php b/includes/User.php index 4ab90ed399..4332e2bddf 100644 --- a/includes/User.php +++ b/includes/User.php @@ -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 * diff --git a/includes/actions/RawAction.php b/includes/actions/RawAction.php index 174ca3f86c..8079493264 100644 --- a/includes/actions/RawAction.php +++ b/includes/actions/RawAction.php @@ -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 ); diff --git a/includes/specials/SpecialNewpages.php b/includes/specials/SpecialNewpages.php index 8e15d554fe..34bc7a4eb7 100644 --- a/includes/specials/SpecialNewpages.php +++ b/includes/specials/SpecialNewpages.php @@ -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! -- 2.20.1