First steps for bug 14801: add backend support for per-namespace permissions to core...
authorBryan Tong Minh <btongminh@users.mediawiki.org>
Sat, 16 Jul 2011 16:09:00 +0000 (16:09 +0000)
committerBryan Tong Minh <btongminh@users.mediawiki.org>
Sat, 16 Jul 2011 16:09:00 +0000 (16:09 +0000)
* User::getRights(), User::isAllowed() and User::getGroupPermissions now optionally accept a namespace parameter. If not set, it will check whether the user has the right for all namespaces.
* Anything that uses Title::getUserPermissionsErrorsInternal() automatically supports per-namespace permissions. This includes Title::getUserPermissionsErrors and Title::(quick)UserCan.
* Fix tests that set User::mRights

The next step would be to change all User::isAllowed() to Title::quickUserCan or pass the namespace to User::isAllowed().

RELEASE-NOTES-1.19
includes/DefaultSettings.php
includes/Title.php
includes/User.php
tests/phpunit/includes/ArticleTablesTest.php
tests/phpunit/includes/TitlePermissionTest.php
tests/phpunit/includes/UserTest.php

index 1f6b398..71d7da1 100644 (file)
@@ -29,6 +29,7 @@ production.
   hooks have been removed.
 * New hook "Collation::factory" to allow extensions to create custom
   category collations.
+* $wgGroupPermissions now supports per namespace permissions.
 
 === New features in 1.19 ===
 * BREAKING CHANGE: action=watch / action=unwatch now requires a token.
index 2b1e148..0d0f28e 100644 (file)
@@ -3304,6 +3304,10 @@ $wgEmailConfirmToEdit = false;
  * unable to perform certain essential tasks or access new functionality
  * when new permissions are introduced and default grants established.
  *
+ * If set to an array instead of a boolean, it is assumed that the array is in
+ * NS => bool form in order to support per-namespace permissions. Note that 
+ * this feature does not fully work for all permission types.
+ *
  * Functionality to make pages inaccessible has not been extensively tested
  * for security. Use at your own risk!
  *
index 6646a75..d3d6b66 100644 (file)
@@ -1239,34 +1239,33 @@ class Title {
         * @return Array list of errors
         */
        private function checkQuickPermissions( $action, $user, $errors, $doExpensiveQueries, $short ) {
+               $ns = $this->getNamespace();
+               
                if ( $action == 'create' ) {
-                       if ( ( $this->isTalkPage() && !$user->isAllowed( 'createtalk' ) ) ||
-                                ( !$this->isTalkPage() && !$user->isAllowed( 'createpage' ) ) ) {
+                       if ( ( $this->isTalkPage() && !$user->isAllowed( 'createtalk', $ns ) ) ||
+                                ( !$this->isTalkPage() && !$user->isAllowed( 'createpage', $ns ) ) ) {
                                $errors[] = $user->isAnon() ? array( 'nocreatetext' ) : array( 'nocreate-loggedin' );
                        }
                } elseif ( $action == 'move' ) {
-                       if ( !$user->isAllowed( 'move-rootuserpages' )
-                                       && $this->mNamespace == NS_USER && !$this->isSubpage() ) {
+                       if ( !$user->isAllowed( 'move-rootuserpages', $ns )
+                                       && $ns == NS_USER && !$this->isSubpage() ) {
                                // Show user page-specific message only if the user can move other pages
                                $errors[] = array( 'cant-move-user-page' );
                        }
 
                        // Check if user is allowed to move files if it's a file
-                       if ( $this->mNamespace == NS_FILE && !$user->isAllowed( 'movefile' ) ) {
+                       if ( $ns == NS_FILE && !$user->isAllowed( 'movefile', $ns ) ) {
                                $errors[] = array( 'movenotallowedfile' );
                        }
 
-                       if ( !$user->isAllowed( 'move' ) ) {
+                       if ( !$user->isAllowed( 'move', $ns) ) {
                                // 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 = in_array( 'move', User::getGroupPermissions( 
+                                       array( 'user' ), $ns ), true );
+                               $autoconfirmedCanMove = in_array( 'move', User::getGroupPermissions( 
+                                       array( 'autoconfirmed' ), $ns ), true );
+
                                if ( $user->isAnon() && ( $userCanMove || $autoconfirmedCanMove ) ) {
                                        // custom message if logged-in users without any special rights can move
                                        $errors[] = array( 'movenologintext' );
@@ -1275,20 +1274,20 @@ class Title {
                                }
                        }
                } elseif ( $action == 'move-target' ) {
-                       if ( !$user->isAllowed( 'move' ) ) {
+                       if ( !$user->isAllowed( 'move', $ns ) ) {
                                // User can't move anything
                                $errors[] = array( 'movenotallowed' );
-                       } elseif ( !$user->isAllowed( 'move-rootuserpages' )
-                                       && $this->mNamespace == NS_USER && !$this->isSubpage() ) {
+                       } elseif ( !$user->isAllowed( 'move-rootuserpages', $ns )
+                                       && $ns == NS_USER && !$this->isSubpage() ) {
                                // Show user page-specific message only if the user can move other pages
                                $errors[] = array( 'cant-move-to-user-page' );
                        }
-               } elseif ( !$user->isAllowed( $action ) ) {
+               } elseif ( !$user->isAllowed( $action, $ns ) ) {
                        // We avoid expensive display logic for quickUserCan's and such
                        $groups = false;
                        if ( !$short ) {
                                $groups = array_map( array( 'User', 'makeGroupLinkWiki' ),
-                                       User::getGroupsWithPermission( $action ) );
+                                       User::getGroupsWithPermission( $action, $ns ) );
                        }
 
                        if ( $groups ) {
@@ -1440,9 +1439,9 @@ class Title {
                        if ( $right == 'sysop' ) {
                                $right = 'protect';
                        }
-                       if ( $right != '' && !$user->isAllowed( $right ) ) {
+                       if ( $right != '' && !$user->isAllowed( $right, $this->mNamespace ) ) {
                                // Users with 'editprotected' permission can edit protected pages
-                               if ( $action == 'edit' && $user->isAllowed( 'editprotected' ) ) {
+                               if ( $action == 'edit' && $user->isAllowed( 'editprotected', $this->mNamespace ) ) {
                                        // Users with 'editprotected' permission cannot edit protected pages
                                        // with cascading option turned on.
                                        if ( $this->mCascadeRestriction ) {
@@ -1483,7 +1482,7 @@ class Title {
                        if ( isset( $restrictions[$action] ) ) {
                                foreach ( $restrictions[$action] as $right ) {
                                        $right = ( $right == 'sysop' ) ? 'protect' : $right;
-                                       if ( $right != '' && !$user->isAllowed( $right ) ) {
+                                       if ( $right != '' && !$user->isAllowed( $right, $this->mNamespace ) ) {
                                                $pages = '';
                                                foreach ( $cascadingSources as $page )
                                                        $pages .= '* [[:' . $page->getPrefixedText() . "]]\n";
@@ -1519,7 +1518,9 @@ class Title {
                                if( $title_protection['pt_create_perm'] == 'sysop' ) {
                                        $title_protection['pt_create_perm'] = 'protect'; // B/C
                                }
-                               if( $title_protection['pt_create_perm'] == '' || !$user->isAllowed( $title_protection['pt_create_perm'] ) ) {
+                               if( $title_protection['pt_create_perm'] == '' || 
+                                               !$user->isAllowed( $title_protection['pt_create_perm'], 
+                                               $this->mNamespace ) ) {
                                        $errors[] = array( 'titleprotected', User::whoIs( $title_protection['pt_user'] ), $title_protection['pt_reason'] );
                                }
                        }
index d93efa8..ccbd792 100644 (file)
@@ -2240,16 +2240,29 @@ class User {
 
        /**
         * Get the permissions this user has.
+        * @param $ns int If numeric, get permissions for this namespace
         * @return Array of String permission names
         */
-       function getRights() {
+       function getRights( $ns = null ) {
+               $key = is_null( $ns ) ? '*' : intval( $ns );
+               
                if ( is_null( $this->mRights ) ) {
-                       $this->mRights = self::getGroupPermissions( $this->getEffectiveGroups() );
-                       wfRunHooks( 'UserGetRights', array( $this, &$this->mRights ) );
+                       $this->mRights = array();
+               }
+               
+               if ( !isset( $this->mRights[$key] ) ) {
+                       $this->mRights[$key] = self::getGroupPermissions( $this->getEffectiveGroups(), $ns );
+                       wfRunHooks( 'UserGetRights', array( $this, &$this->mRights[$key], $ns ) );
                        // Force reindexation of rights when a hook has unset one of them
-                       $this->mRights = array_values( $this->mRights );
+                       $this->mRights[$key] = array_values( $this->mRights[$key] );
+               }
+               if ( is_null( $ns ) ) {
+                       return $this->mRights[$key];
+               } else {
+                       // Merge non namespace specific rights
+                       return array_merge( $this->mRights[$key], $this->getRights() );
                }
-               return $this->mRights;
+               
        }
 
        /**
@@ -2351,7 +2364,7 @@ class User {
                }
                $this->loadGroups();
                $this->mGroups[] = $group;
-               $this->mRights = User::getGroupPermissions( $this->getEffectiveGroups( true ) );
+               $this->mRights = null;
 
                $this->invalidateCache();
        }
@@ -2381,7 +2394,7 @@ class User {
                }
                $this->loadGroups();
                $this->mGroups = array_diff( $this->mGroups, array( $group ) );
-               $this->mRights = User::getGroupPermissions( $this->getEffectiveGroups( true ) );
+               $this->mRights = null;
 
                $this->invalidateCache();
        }
@@ -2434,9 +2447,10 @@ class User {
        /**
         * Internal mechanics of testing a permission
         * @param $action String
+        * @paramn $ns int Namespace optional
         * @return bool
         */
-       public function isAllowed( $action = '' ) {
+       public function isAllowed( $action = '', $ns = null ) {
                if ( $action === '' ) {
                        return true; // In the spirit of DWIM
                }
@@ -2448,7 +2462,7 @@ class User {
                }
                # Use strict parameter to avoid matching numeric 0 accidentally inserted
                # by misconfiguration: 0 == 'foo'
-               return in_array( $action, $this->getRights(), true );
+               return in_array( $action, $this->getRights( $ns ), true );
        }
 
        /**
@@ -3397,26 +3411,51 @@ class User {
         * @param $groups Array of Strings List of internal group names
         * @return Array of Strings List of permission key names for given groups combined
         */
-       static function getGroupPermissions( $groups ) {
+       static function getGroupPermissions( $groups, $ns = null ) {
                global $wgGroupPermissions, $wgRevokePermissions;
                $rights = array();
-               // grant every granted permission first
+               
+               // Grant every granted permission first
                foreach( $groups as $group ) {
                        if( isset( $wgGroupPermissions[$group] ) ) {
-                               $rights = array_merge( $rights,
-                                       // array_filter removes empty items
-                                       array_keys( array_filter( $wgGroupPermissions[$group] ) ) );
+                               $rights = array_merge( $rights, self::extractRights(
+                                       $wgGroupPermissions[$group], $ns ) );
                        }
                }
-               // now revoke the revoked permissions
+               
+               // Revoke the revoked permissions
                foreach( $groups as $group ) {
                        if( isset( $wgRevokePermissions[$group] ) ) {
-                               $rights = array_diff( $rights,
-                                       array_keys( array_filter( $wgRevokePermissions[$group] ) ) );
+                               $rights = array_diff( $rights, self::extractRights( 
+                                       $wgRevokePermissions[$group], $ns ) );
                        }
                }
                return array_unique( $rights );
        }
+       
+       /**
+        * Helper for User::getGroupPermissions
+        * @param array $list
+        * @param int $ns
+        * @return array
+        */
+       private static function extractRights( $list, $ns ) {
+               $rights = array();
+               foreach( $list as $right => $value ) {
+                       if ( is_array( $value ) ) {
+                               # This is a list of namespaces where the permission applies
+                               if ( !is_null( $ns ) && !empty( $value[$ns] ) ) {
+                                       $rights[] = $right;
+                               }
+                       } else {
+                               # This is a boolean indicating that the permission applies
+                               if ( $value ) {
+                                       $rights[] = $right;
+                               }
+                       }
+               }
+               return $rights; 
+       }
 
        /**
         * Get all the groups who have a given permission
@@ -3424,11 +3463,11 @@ class User {
         * @param $role String Role to check
         * @return Array of Strings List of internal group names with the given permission
         */
-       static function getGroupsWithPermission( $role ) {
+       static function getGroupsWithPermission( $role, $ns = null ) {
                global $wgGroupPermissions;
                $allowedGroups = array();
                foreach ( $wgGroupPermissions as $group => $rights ) {
-                       if ( isset( $rights[$role] ) && $rights[$role] ) {
+                       if ( in_array( $role, self::getGroupPermissions( array( $group ), $ns ), true ) ) {
                                $allowedGroups[] = $group;
                        }
                }
index 01776c9..ef8e255 100644 (file)
@@ -11,7 +11,7 @@ class ArticleTablesTest extends MediaWikiLangTestCase {
                $title = Title::newFromText("Bug 14404");
                $article = new Article( $title );
                $wgUser = new User();
-               $wgUser->mRights = array( 'createpage', 'edit', 'purge' );
+               $wgUser->mRights['*'] = array( 'createpage', 'edit', 'purge' );
                $wgLanguageCode = 'es';
                $wgContLang = Language::factory( 'es' );
                
index 1b17968..e9e83dd 100644 (file)
@@ -56,11 +56,17 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
        }
 
        function setUserPerm( $perm ) {
-               if ( is_array( $perm ) ) {
-                       $this->user->mRights = $perm;
-               } else {
-                       $this->user->mRights = array( $perm );
+               // Setting member variables is evil!!!
+
+               if ( !is_array( $perm ) ) {
+                       $perm = array( $perm );
+               }
+               for ($i = 0; $i < 100; $i++) {
+                       $this->user->mRights[$i] = $perm;
                }
+               
+               // Hack, hack hack ...
+               $this->user->mRights['*'] = $perm;
        }
 
        function setTitle( $ns, $title = "Main_Page" ) {
index df91aca..6fa730e 100644 (file)
@@ -1,7 +1,11 @@
 <?php
 
+define( 'NS_UNITTEST', 5600 );
+define( 'NS_UNITTEST_TALK', 5601 );
+
 class UserTest extends MediaWikiTestCase {
        protected $savedGroupPermissions, $savedRevokedPermissions;
+       protected $user;
        
        public function setUp() {
                parent::setUp();
@@ -10,24 +14,40 @@ class UserTest extends MediaWikiTestCase {
                $this->savedRevokedPermissions = $GLOBALS['wgRevokePermissions'];
                
                $this->setUpPermissionGlobals();
+               $this->setUpUser();
        }
        private function setUpPermissionGlobals() {
                global $wgGroupPermissions, $wgRevokePermissions;
                
+               # Data for regular $wgGroupPermissions test
                $wgGroupPermissions['unittesters'] = array(
+                       'test' => true,
                        'runtest' => true,
                        'writetest' => false,
                        'nukeworld' => false,
                );
                $wgGroupPermissions['testwriters'] = array(
+                       'test' => true,
                        'writetest' => true,
                        'modifytest' => true,
                );
-               
+               # Data for regular $wgRevokePermissions test
                $wgRevokePermissions['formertesters'] = array(
                        'runtest' => true,
                );
+               
+               # Data for namespace based $wgGroupPermissions test
+               $wgGroupPermissions['unittesters']['writedocumentation'] = array(
+                       NS_MAIN => false, NS_UNITTEST => true,
+               );
+               $wgGroupPermissions['testwriters']['writedocumentation'] = true;
+               
+       }
+       private function setUpUser() {
+               $this->user = new User;
+               $this->user->addGroup( 'unittesters' );
        }
+       
        public function tearDown() {
                parent::tearDown();
                
@@ -55,4 +75,90 @@ class UserTest extends MediaWikiTestCase {
                $this->assertNotContains( 'modifytest', $rights );
                $this->assertNotContains( 'nukeworld', $rights );               
        }
+       
+       public function testNamespaceGroupPermissions() {
+               $rights = User::getGroupPermissions( array( 'unittesters' ) );
+               $this->assertNotContains( 'writedocumentation', $rights );
+               
+               $rights = User::getGroupPermissions( array( 'unittesters' ) , NS_MAIN );
+               $this->assertNotContains( 'writedocumentation', $rights );
+               $this->assertNotContains( 'modifytest', $rights );
+               
+               $rights = User::getGroupPermissions( array( 'unittesters' ), NS_HELP );
+               $this->assertNotContains( 'writedocumentation', $rights );
+               $this->assertNotContains( 'modifytest', $rights );
+               
+               $rights = User::getGroupPermissions( array( 'unittesters' ), NS_UNITTEST );
+               $this->assertContains( 'writedocumentation', $rights );
+               
+               $rights = User::getGroupPermissions( 
+                       array( 'unittesters', 'testwriters' ), NS_MAIN );
+               $this->assertContains( 'writedocumentation', $rights );
+       }
+       
+       public function testUserPermissions() {
+               $rights = $this->user->getRights();
+               $this->assertContains( 'runtest', $rights );
+               $this->assertNotContains( 'writetest', $rights );
+               $this->assertNotContains( 'modifytest', $rights );
+               $this->assertNotContains( 'nukeworld', $rights );
+               $this->assertNotContains( 'writedocumentation', $rights );      
+               
+               $rights = $this->user->getRights( NS_MAIN );
+               $this->assertNotContains( 'writedocumentation', $rights );
+               $this->assertNotContains( 'modifytest', $rights );
+               
+               $rights = $this->user->getRights( NS_HELP );
+               $this->assertNotContains( 'writedocumentation', $rights );
+               $this->assertNotContains( 'modifytest', $rights );
+               
+               $rights = $this->user->getRights( NS_UNITTEST );
+               $this->assertContains( 'writedocumentation', $rights );
+       }
+       
+       /**
+        * @dataProvider provideGetGroupsWithPermission
+        */
+       public function testGetGroupsWithPermission( $expected, $right, $ns ) {
+               $result = User::getGroupsWithPermission( $right, $ns );
+               sort( $result );
+               sort( $expected );
+               
+               $this->assertEquals( $expected, $result, "Groups with permission $right" . 
+                       ( is_null( $ns ) ? '' : "in namespace $ns" ) );
+       }
+       public function provideGetGroupsWithPermission() {
+               return array(
+                       array( 
+                               array( 'unittesters', 'testwriters' ),
+                               'test',
+                               null
+                       ),              
+                       array( 
+                               array( 'unittesters' ),
+                               'runtest',
+                               null
+                       ),
+                       array( 
+                               array( 'testwriters' ),
+                               'writetest',
+                               null
+                       ),
+                       array( 
+                               array( 'testwriters' ),
+                               'modifytest',
+                               null
+                       ),
+                       array( 
+                               array( 'testwriters' ),
+                               'writedocumentation',
+                               NS_MAIN
+                       ),
+                       array( 
+                               array( 'unittesters', 'testwriters' ),
+                               'writedocumentation',
+                               NS_UNITTEST
+                       ),                              
+               );
+       }
 }
\ No newline at end of file