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 ce10301..cde5b26 100644 (file)
--- 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 ae2a5dc..fb9a528 100644 (file)
--- 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 e22fe20..bddbeb3 100644 (file)
--- 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 ac838a5..ad9618f 100644 (file)
--- 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 072bf44..12a08ee 100644 (file)
--- 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 46450e6..fa489b3 100644 (file)
--- 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 5ddb3ab..49a0b3c 100644 (file)
--- 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' );