From: Brad Jorsch Date: Fri, 12 Jul 2013 15:06:41 +0000 (-0400) Subject: Add User::isEveryoneAllowed function X-Git-Tag: 1.31.0-rc.0~19203 X-Git-Url: https://git.cyclocoop.org/%28%28?a=commitdiff_plain;h=76623e75dac07e1def6ae085958a7893a25badad;p=lhc%2Fweb%2Fwiklou.git Add User::isEveryoneAllowed function User::groupHasPermission is used for various purposes, from checking whether it makes sense to show a "hide logged-in users" on Special:NewPages to showing different error messages in some places when 'user' or 'autoconfirmed' is allowed the action to avoiding unstubbing $wgUser to check $wgUser->isAllowed( 'read' ) in the common case where 'read' permission is granted to everyone. For the OAuth work, we need to be able to catch that last type of use without interfering with the others. This change introduces User::isEveryoneAllowed() to be used for that type of check, which both makes sure the right granted to '*' isn't revoked from any group and calls a hook to allow extensions to indicate that they might remove the right. Change-Id: Idfee1b4d0613aaf52e143164acd6022459415c49 --- diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22 index ce10301fe7..cde5b2611c 100644 --- a/RELEASE-NOTES-1.22 +++ b/RELEASE-NOTES-1.22 @@ -143,6 +143,9 @@ production. * Change tag lists (shown on recent changes, watchlist, user contributions, history pages, diff pages) now include a link to Special:Tags to distinguish them from edit summaries. +* Added a new method and hook, User::isEveryoneAllowed() and + UserIsEveryoneAllowed, for use in situations where a "does everyone have this + right?" check is used to avoid more expensive checks. === Bug fixes in 1.22 === * Disable Special:PasswordReset when $wgEnableEmail is false. Previously one diff --git a/docs/hooks.txt b/docs/hooks.txt index ae2a5dcfa7..fb9a528e90 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -2579,6 +2579,10 @@ $title: Title of the page in question $ip: User's IP address &$blocked: Whether the user is blocked, to be modified by the hook +'UserIsEveryoneAllowed': Check if all users are allowed some user right; return +false if a UserGetRights hook might remove the named right. +$right: The user right being checked + 'UserLoadAfterLoadFromSession': Called to authenticate users on external or environmental means; occurs after session is loaded. $user: user object being loaded diff --git a/includes/AjaxDispatcher.php b/includes/AjaxDispatcher.php index e22fe20af9..bddbeb3497 100644 --- a/includes/AjaxDispatcher.php +++ b/includes/AjaxDispatcher.php @@ -113,13 +113,11 @@ class AjaxDispatcher { 'Bad Request', "unknown function " . (string) $this->func_name ); - } elseif ( !in_array( 'read', User::getGroupPermissions( array( '*' ) ), true ) - && !$wgUser->isAllowed( 'read' ) ) - { + } elseif ( !User::isEveryoneAllowed( 'read' ) && !$wgUser->isAllowed( 'read' ) ) { wfHttpError( 403, 'Forbidden', - 'You must log in to view pages.' ); + 'You are not allowed to view pages.' ); } else { wfDebug( __METHOD__ . ' dispatching ' . $this->func_name . "\n" ); diff --git a/includes/SpecialPage.php b/includes/SpecialPage.php index ac838a5f65..ad9618f735 100644 --- a/includes/SpecialPage.php +++ b/includes/SpecialPage.php @@ -553,8 +553,8 @@ class SpecialPage { * pages? */ public function isRestricted() { - // DWIM: If all anons can do something, then it is not restricted - return $this->mRestriction != '' && !User::groupHasPermission( '*', $this->mRestriction ); + // DWIM: If everyone can do something, then it is not restricted + return $this->mRestriction != '' && !User::isEveryoneAllowed( $this->mRestriction ); } /** diff --git a/includes/Title.php b/includes/Title.php index 072bf44bdd..12a08ee688 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -2100,33 +2100,9 @@ class Title { */ private function checkReadPermissions( $action, $user, $errors, $doExpensiveQueries, $short ) { global $wgWhitelistRead, $wgWhitelistReadRegexp, $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 ( !User::groupHasPermission( '*', 'read' ) ) { - # Not a public wiki, so no shortcut - $useShortcut = false; - } elseif ( !empty( $wgRevokePermissions ) ) { - /** - * Iterate through each group with permissions being revoked (key not included since we don't care - * what the group name is), then check if the read permission is being revoked. If it is, then - * we don't use the shortcut below since the user might not be able to read, even though anon - * reading is allowed. - */ - foreach ( $wgRevokePermissions as $perms ) { - if ( !empty( $perms['read'] ) ) { - # We might be removing the read right from the user, so no shortcut - $useShortcut = false; - break; - } - } - } - } $whitelisted = false; - if ( $useShortcut ) { + if ( User::isEveryoneAllowed( 'read' ) ) { # Shortcut for public wikis, allows skipping quite a bit of code $whitelisted = true; } elseif ( $user->isAllowed( 'read' ) ) { diff --git a/includes/User.php b/includes/User.php index 46450e66a4..fa489b36f7 100644 --- a/includes/User.php +++ b/includes/User.php @@ -3974,6 +3974,10 @@ class User { /** * Check, if the given group has the given permission * + * If you're wanting to check whether all users have a permission, use + * User::isEveryoneAllowed() instead. That properly checks if it's revoked + * from anyone. + * * @since 1.21 * @param string $group Group to check * @param string $role Role to check @@ -3985,6 +3989,44 @@ class User { && !( isset( $wgRevokePermissions[$group][$role] ) && $wgRevokePermissions[$group][$role] ); } + /** + * Check if all users have the given permission + * + * @since 1.22 + * @param string $right Right to check + * @return bool + */ + public static function isEveryoneAllowed( $right ) { + global $wgGroupPermissions, $wgRevokePermissions; + static $cache = array(); + + if ( isset( $cache[$right] ) ) { + return $cache[$right]; + } + + if ( !isset( $wgGroupPermissions['*'][$right] ) || !$wgGroupPermissions['*'][$right] ) { + $cache[$right] = false; + return false; + } + + // If it's revoked anywhere, then everyone doesn't have it + foreach ( $wgRevokePermissions as $rights ) { + if ( isset( $rights[$right] ) && $rights[$right] ) { + $cache[$right] = false; + return false; + } + } + + // Allow extensions (e.g. OAuth) to say false + if ( !wfRunHooks( 'UserIsEveryoneAllowed', array( $right ) ) ) { + $cache[$right] = false; + return false; + } + + $cache[$right] = true; + return true; + } + /** * Get the localized descriptive name for a group, if it exists * diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php index 5ddb3abfc6..49a0b3c284 100644 --- a/includes/api/ApiMain.php +++ b/includes/api/ApiMain.php @@ -769,7 +769,7 @@ class ApiMain extends ApiBase { */ protected function checkExecutePermissions( $module ) { $user = $this->getUser(); - if ( $module->isReadMode() && !User::groupHasPermission( '*', 'read' ) && + if ( $module->isReadMode() && !User::isEveryoneAllowed( 'read' ) && !$user->isAllowed( 'read' ) ) { $this->dieUsageMsg( 'readrequired' );