Fix protection rights usage

It has long been recognized that using the 'protect' right to control
the ability to edit sysop-protected pages is troublesome. r31247 fixed
this by adding an 'editprotected' right, but for some reason in r32164
this was changed to bypass protection completely instead of fixing the
bug identified in r31462.

This patch goes back to do it the right way: editprotected no longer
bypasses all protection, and it is used instead of 'protect' for
controlling access to sysop-protected pages. For good measure, the same
is done with autoconfirmed protection (semiprotection): a new
editsemiprotected right is created instead of overloading the
existing autoconfirmed right.

This also fixes bug 27152 by making editprotected no longer special.

Bug: 13137
Bug: 27152
Change-Id: I6bf650a3fbdab8589ae6945c8c916eafd949e41c

diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index 9be9a9b..45a6f01 100644 (file)
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -32,6 +32,12 @@ production.
   'editmywatchlist', 'viewmyprivateinfo', 'editmyprivateinfo', and
   'editmyoptions' restrict actions that were formerly allowed by default. They
   have been added to the default for $wgGroupPermissions['*'].
+* The 'editprotected' right no longer allows bypassing of all page protection
+  restrictions. Any group using it for this purpose will now need to have all
+  the individual rights listed in $wgRestrictionTypes for the same effect.
+* The 'protect' and 'autoconfirmed' rights are no longer used for the default
+  page protection levels. The rights 'editprotected' and 'editsemiprotected'
+  are now used for this purpose instead.
 
 === New features in 1.22 ===
 * (bug 44525) mediawiki.jqueryMsg can now parse (whitelisted) HTML elements and attributes.
@@ -125,6 +131,10 @@ production.
   watchlists.
 * (bug 40518) mw.toolbar: Implemented mw.toolbar.addButtons for adding multiple
  button objects in one call.
+* Rights used for the default protection levels ('sysop' and 'autoconfirmed')
+  are now used just for that purpose, instead of overloading other rights. This
+  allows easy granting of the ability to edit sysop-protected pages without
+  also granting the ability to protect and unprotect.
 * (bug 48256) Make brackets in section edit links accessible to CSS.
   They are now wrapped in <span class="mw-editsection-bracket" />.
 

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index f30a8b6..10f2885 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -3899,17 +3899,18 @@ $wgGroupPermissions['user']['sendemail'] = true;
 
 // Implicit group for accounts that pass $wgAutoConfirmAge
 $wgGroupPermissions['autoconfirmed']['autoconfirmed'] = true;
+$wgGroupPermissions['autoconfirmed']['editsemiprotected'] = true;
 
 // Users with bot privilege can have their edits hidden
 // from various log pages by default
 $wgGroupPermissions['bot']['bot'] = true;
 $wgGroupPermissions['bot']['autoconfirmed'] = true;
+$wgGroupPermissions['bot']['editsemiprotected'] = true;
 $wgGroupPermissions['bot']['nominornewtalk'] = true;
 $wgGroupPermissions['bot']['autopatrol'] = true;
 $wgGroupPermissions['bot']['suppressredirect'] = true;
 $wgGroupPermissions['bot']['apihighlimits'] = true;
 $wgGroupPermissions['bot']['writeapi'] = true;
-#$wgGroupPermissions['bot']['editprotected'] = true; // can edit all protected pages without cascade protection enabled
 
 // Most extra permission abilities go to this group
 $wgGroupPermissions['sysop']['block'] = true;
@@ -3930,6 +3931,7 @@ $wgGroupPermissions['sysop']['move-rootuserpages'] = true;
 $wgGroupPermissions['sysop']['patrol'] = true;
 $wgGroupPermissions['sysop']['autopatrol'] = true;
 $wgGroupPermissions['sysop']['protect'] = true;
+$wgGroupPermissions['sysop']['editprotected'] = true;
 $wgGroupPermissions['sysop']['proxyunbannable'] = true;
 $wgGroupPermissions['sysop']['rollback'] = true;
 $wgGroupPermissions['sysop']['upload'] = true;
@@ -3937,6 +3939,7 @@ $wgGroupPermissions['sysop']['reupload'] = true;
 $wgGroupPermissions['sysop']['reupload-shared'] = true;
 $wgGroupPermissions['sysop']['unwatchedpages'] = true;
 $wgGroupPermissions['sysop']['autoconfirmed'] = true;
+$wgGroupPermissions['sysop']['editsemiprotected'] = true;
 $wgGroupPermissions['sysop']['ipblock-exempt'] = true;
 $wgGroupPermissions['sysop']['blockemail'] = true;
 $wgGroupPermissions['sysop']['markbotedits'] = true;
@@ -4035,7 +4038,8 @@ $wgRestrictionTypes = array( 'create', 'edit', 'move', 'upload' );
  * dictates the order on the protection form's lists.
  *
  *   - '' will be ignored (i.e. unprotected)
- *   - 'sysop' is quietly rewritten to 'protect' for backwards compatibility
+ *   - 'autoconfirmed' is quietly rewritten to 'editsemiprotected' for backwards compatibility
+ *   - 'sysop' is quietly rewritten to 'editprotected' for backwards compatibility
  */
 $wgRestrictionLevels = array( '', 'autoconfirmed', 'sysop' );
 
@@ -4050,7 +4054,8 @@ $wgRestrictionLevels = array( '', 'autoconfirmed', 'sysop' );
  * "protect" them by transcluding them on protected pages they are
  * allowed to edit.
  *
- * 'sysop' is quietly rewritten to 'protect' for backwards compatibility.
+ * 'autoconfirmed' is quietly rewritten to 'editsemiprotected' for backwards compatibility.
+ * 'sysop' is quietly rewritten to 'editprotected' for backwards compatibility.
  */
 $wgCascadingRestrictionLevels = array( 'sysop' );
 

diff --git a/includes/ProtectionForm.php b/includes/ProtectionForm.php
index 2f6b65d..4d41d9e 100644 (file)
--- a/includes/ProtectionForm.php
+++ b/includes/ProtectionForm.php
@@ -135,8 +135,13 @@ class ProtectionForm {
                        if ( isset( $val ) && in_array( $val, $wgRestrictionLevels ) ) {
                                // Prevent users from setting levels that they cannot later unset
                                if ( $val == 'sysop' ) {
-                                       // Special case, rewrite sysop to either protect and editprotected
-                                       if ( !$wgUser->isAllowedAny( 'protect', 'editprotected' ) ) {
+                                       // Special case, rewrite sysop to editprotected
+                                       if ( !$wgUser->isAllowed( 'editprotected' ) ) {
+                                               continue;
+                                       }
+                               } elseif ( $val == 'autoconfirmed' ) {
+                                       // Special case, rewrite autoconfirmed to editsemiprotected
+                                       if ( !$wgUser->isAllowed( 'editsemiprotected' ) ) {
                                                continue;
                                        }
                                } elseif ( !$wgUser->isAllowed( $val ) ) {
@@ -297,14 +302,7 @@ 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.
-               $edit_restriction = isset( $this->mRestrictions['edit'] ) ? $this->mRestrictions['edit'] : '';
                $this->mCascade = $wgRequest->getBool( 'mwProtect-cascade' );
-               if ( $this->mCascade && ( $edit_restriction != 'protect' ) &&
-                       !User::groupHasPermission( $edit_restriction, 'protect' ) ) {
-                       $this->mCascade = false;
-               }
 
                $status = $this->mArticle->doUpdateRestrictions( $this->mRestrictions, $expiry, $this->mCascade, $reasonstr, $wgUser );
 
@@ -562,8 +560,13 @@ class ProtectionForm {
                foreach ( $wgRestrictionLevels as $key ) {
                        //don't let them choose levels above their own (aka so they can still unprotect and edit the page). but only when the form isn't disabled
                        if ( $key == 'sysop' ) {
-                               //special case, rewrite sysop to protect and editprotected
-                               if ( !$wgUser->isAllowedAny( 'protect', 'editprotected' ) && !$this->disabled ) {
+                               //special case, rewrite sysop to editprotected
+                               if ( !$wgUser->isAllowed( 'editprotected' ) && !$this->disabled ) {
+                                       continue;
+                               }
+                       } elseif ( $key == 'autoconfirmed' ) {
+                               //special case, rewrite autoconfirmed to editsemiprotected
+                               if ( !$wgUser->isAllowed( 'editsemiprotected' ) && !$this->disabled ) {
                                        continue;
                                }
                        } else {

diff --git a/includes/Title.php b/includes/Title.php
index 56c2ed4..072bf44 100644 (file)
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -1925,18 +1925,21 @@ class Title {
         */
        private function checkPageRestrictions( $action, $user, $errors, $doExpensiveQueries, $short ) {
                foreach ( $this->getRestrictions( $action ) as $right ) {
-                       // Backwards compatibility, rewrite sysop -> protect
+                       // Backwards compatibility, rewrite sysop -> editprotected
                        if ( $right == 'sysop' ) {
-                               $right = 'protect';
+                               $right = 'editprotected';
                        }
-                       if ( $right != '' && !$user->isAllowed( $right ) ) {
-                               // Users with 'editprotected' permission can edit protected pages
-                               // without cascading option turned on.
-                               if ( $action != 'edit' || !$user->isAllowed( 'editprotected' )
-                                       || $this->mCascadeRestriction )
-                               {
-                                       $errors[] = array( 'protectedpagetext', $right );
-                               }
+                       // Backwards compatibility, rewrite autoconfirmed -> editsemiprotected
+                       if ( $right == 'autoconfirmed' ) {
+                               $right = 'editsemiprotected';
+                       }
+                       if ( $right == '' ) {
+                               continue;
+                       }
+                       if ( !$user->isAllowed( $right ) ) {
+                               $errors[] = array( 'protectedpagetext', $right );
+                       } elseif ( $this->mCascadeRestriction && !$user->isAllowed( 'protect' ) ) {
+                               $errors[] = array( 'protectedpagetext', 'protect' );
                        }
                }
 
@@ -1968,8 +1971,15 @@ class Title {
                        # This is only for protection restrictions, not for all actions
                        if ( isset( $restrictions[$action] ) ) {
                                foreach ( $restrictions[$action] as $right ) {
-                                       $right = ( $right == 'sysop' ) ? 'protect' : $right;
-                                       if ( $right != '' && !$user->isAllowed( $right ) ) {
+                                       // Backwards compatibility, rewrite sysop -> editprotected
+                                       if ( $right == 'sysop' ) {
+                                               $right = 'editprotected';
+                                       }
+                                       // Backwards compatibility, rewrite autoconfirmed -> editsemiprotected
+                                       if ( $right == 'autoconfirmed' ) {
+                                               $right = 'editsemiprotected';
+                                       }
+                                       if ( $right != '' && !$user->isAllowedAll( 'protect', $right ) ) {
                                                $pages = '';
                                                foreach ( $cascadingSources as $page ) {
                                                        $pages .= '* [[:' . $page->getPrefixedText() . "]]\n";
@@ -2006,7 +2016,10 @@ class Title {
                        $title_protection = $this->getTitleProtection();
                        if ( $title_protection ) {
                                if ( $title_protection['pt_create_perm'] == 'sysop' ) {
-                                       $title_protection['pt_create_perm'] = 'protect'; // B/C
+                                       $title_protection['pt_create_perm'] = 'editprotected'; // B/C
+                               }
+                               if ( $title_protection['pt_create_perm'] == 'autoconfirmed' ) {
+                                       $title_protection['pt_create_perm'] = 'editsemiprotected'; // B/C
                                }
                                if ( $title_protection['pt_create_perm'] == '' ||
                                        !$user->isAllowed( $title_protection['pt_create_perm'] ) )
@@ -2376,7 +2389,9 @@ class Title {
                        $restrictions = $this->getRestrictions( $action );
                        if ( count( $restrictions ) > 0 ) {
                                foreach ( $restrictions as $restriction ) {
-                                       if ( strtolower( $restriction ) != 'autoconfirmed' ) {
+                                       if ( strtolower( $restriction ) != 'editsemiprotected' &&
+                                               strtolower( $restriction ) != 'autoconfirmed' // BC
+                                       ) {
                                                return false;
                                        }
                                }
@@ -3548,7 +3563,13 @@ class Title {
                        }
                } else {
                        $tp = $nt->getTitleProtection();
-                       $right = ( $tp['pt_create_perm'] == 'sysop' ) ? 'protect' : $tp['pt_create_perm'];
+                       $right = $tp['pt_create_perm'];
+                       if ( $right == 'sysop' ) {
+                               $right = 'editprotected'; // B/C
+                       }
+                       if ( $right == 'autoconfirmed' ) {
+                               $right = 'editsemiprotected'; // B/C
+                       }
                        if ( $tp and !$wgUser->isAllowed( $right ) ) {
                                $errors[] = array( 'cantmove-titleprotected' );
                        }

diff --git a/includes/User.php b/includes/User.php
index 3ee32c3..c1ca666 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -129,6 +129,7 @@ class User {
                'editmyusercss',
                'editmyuserjs',
                'editmywatchlist',
+               'editsemiprotected',
                'editusercssjs', #deprecated
                'editusercss',
                'edituserjs',

diff --git a/includes/WikiPage.php b/includes/WikiPage.php
index 398a424..4aab674 100644 (file)
--- a/includes/WikiPage.php
+++ b/includes/WikiPage.php
@@ -2348,8 +2348,11 @@ class WikiPage implements Page, IDBAccessObject {
                        $editrestriction = isset( $limit['edit'] ) ? array( $limit['edit'] ) : $this->mTitle->getRestrictions( 'edit' );
 
                        $cascadingRestrictionLevels = $wgCascadingRestrictionLevels;
-                       if ( in_array( 'sysop', $cascadingRestrictionLevels ) ) {
-                               $cascadingRestrictionLevels[] = 'protect'; // backwards compatibility
+                       foreach ( array_keys( $cascadingRestrictionLevels, 'sysop' ) as $key ) {
+                               $cascadingRestrictionLevels[$key] = 'editprotected'; // backwards compatibility
+                       }
+                       foreach ( array_keys( $cascadingRestrictionLevels, 'autoconfirmed' ) as $key ) {
+                               $cascadingRestrictionLevels[$key] = 'editsemiprotected'; // backwards compatibility
                        }
 
                        // The schema allows multiple restrictions

diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php
index 0fabd92..adeb95d 100644 (file)
--- a/languages/messages/MessagesEn.php
+++ b/languages/messages/MessagesEn.php
@@ -2089,7 +2089,8 @@ Your email address is not revealed when other users contact you.',
 'right-proxyunbannable'       => 'Bypass automatic blocks of proxies',
 'right-unblockself'           => 'Unblock themselves',
 'right-protect'               => 'Change protection levels and edit protected pages',
-'right-editprotected'         => 'Edit protected pages (without cascading protection)',
+'right-editprotected'         => 'Edit pages protected as "{{int:protect-level-sysop}}"',
+'right-editsemiprotected'     => 'Edit pages protected as "{{int:protect-level-autoconfirmed}}"',
 'right-editinterface'         => 'Edit the user interface',
 'right-editusercssjs'         => "Edit other users' CSS and JavaScript files",
 'right-editusercss'           => "Edit other users' CSS files",

diff --git a/languages/messages/MessagesQqq.php b/languages/messages/MessagesQqq.php
index f2b29ee..616ddcd 100644 (file)
--- a/languages/messages/MessagesQqq.php
+++ b/languages/messages/MessagesQqq.php
@@ -2924,6 +2924,7 @@ This user automatically bypasses IP blocks, auto-blocks and range blocks - so I
 {{doc-singularthey}}',
 'right-protect' => '{{doc-right|protect}}',
 'right-editprotected' => '{{doc-right|editprotected}}',
+'right-editsemiprotected' => '{{doc-right|editsemiprotected}}',
 'right-editinterface' => '{{doc-right|editinterface}}',
 'right-editusercssjs' => '{{doc-right|editusercssjs}}',
 'right-editusercss' => '{{doc-right|editusercss}}

diff --git a/maintenance/dictionary/mediawiki.dic b/maintenance/dictionary/mediawiki.dic
index 656dc33..b86d2c5 100644 (file)
--- a/maintenance/dictionary/mediawiki.dic
+++ b/maintenance/dictionary/mediawiki.dic
@@ -1303,6 +1303,7 @@ edits
 editsection
 editsectionhint
 editsectiononrightclick
+editsemiprotected
 editsonly
 editthispage
 edittime

diff --git a/maintenance/language/messages.inc b/maintenance/language/messages.inc
index 00462d5..2cb94cf 100644 (file)
--- a/maintenance/language/messages.inc
+++ b/maintenance/language/messages.inc
@@ -1218,6 +1218,7 @@ $wgMessageStructure = array(
                'right-unblockself',
                'right-protect',
                'right-editprotected',
+               'right-editsemiprotected',
                'right-editinterface',
                'right-editusercssjs',
                'right-editusercss',

diff --git a/tests/phpunit/includes/TitlePermissionTest.php b/tests/phpunit/includes/TitlePermissionTest.php
index 6ae995e..9144d0c 100644 (file)
--- a/tests/phpunit/includes/TitlePermissionTest.php
+++ b/tests/phpunit/includes/TitlePermissionTest.php
@@ -506,39 +506,59 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 
                $this->assertEquals( array( array( 'badaccess-group0' ),
                                array( 'protectedpagetext', 'bogus' ),
-                               array( 'protectedpagetext', 'protect' ),
+                               array( 'protectedpagetext', 'editprotected' ),
                                array( 'protectedpagetext', 'protect' ) ),
                        $this->title->getUserPermissionsErrors( 'bogus',
                                $this->user ) );
                $this->assertEquals( array( array( 'protectedpagetext', 'bogus' ),
-                               array( 'protectedpagetext', 'protect' ),
+                               array( 'protectedpagetext', 'editprotected' ),
                                array( 'protectedpagetext', 'protect' ) ),
                        $this->title->getUserPermissionsErrors( 'edit',
                                $this->user ) );
                $this->setUserPerm( "" );
                $this->assertEquals( array( array( 'badaccess-group0' ),
                                array( 'protectedpagetext', 'bogus' ),
-                               array( 'protectedpagetext', 'protect' ),
+                               array( 'protectedpagetext', 'editprotected' ),
                                array( 'protectedpagetext', 'protect' ) ),
                        $this->title->getUserPermissionsErrors( 'bogus',
                                $this->user ) );
                $this->assertEquals( array( array( 'badaccess-groups', "*, [[$prefix:Users|Users]]", 2 ),
                                array( 'protectedpagetext', 'bogus' ),
-                               array( 'protectedpagetext', 'protect' ),
+                               array( 'protectedpagetext', 'editprotected' ),
                                array( 'protectedpagetext', 'protect' ) ),
                        $this->title->getUserPermissionsErrors( 'edit',
                                $this->user ) );
                $this->setUserPerm( array( "edit", "editprotected" ) );
                $this->assertEquals( array( array( 'badaccess-group0' ),
                                array( 'protectedpagetext', 'bogus' ),
-                               array( 'protectedpagetext', 'protect' ),
                                array( 'protectedpagetext', 'protect' ) ),
                        $this->title->getUserPermissionsErrors( 'bogus',
                                $this->user ) );
-               $this->assertEquals( array(),
+               $this->assertEquals( array(
+                               array( 'protectedpagetext', 'bogus' ),
+                               array( 'protectedpagetext', 'protect' ) ),
                        $this->title->getUserPermissionsErrors( 'edit',
                                $this->user ) );
+
                $this->title->mCascadeRestriction = true;
+               $this->setUserPerm( "edit" );
+               $this->assertEquals( false,
+                       $this->title->quickUserCan( 'bogus', $this->user ) );
+               $this->assertEquals( false,
+                       $this->title->quickUserCan( 'edit', $this->user ) );
+               $this->assertEquals( array( array( 'badaccess-group0' ),
+                               array( 'protectedpagetext', 'bogus' ),
+                               array( 'protectedpagetext', 'editprotected' ),
+                               array( 'protectedpagetext', 'protect' ) ),
+                       $this->title->getUserPermissionsErrors( 'bogus',
+                               $this->user ) );
+               $this->assertEquals( array( array( 'protectedpagetext', 'bogus' ),
+                               array( 'protectedpagetext', 'editprotected' ),
+                               array( 'protectedpagetext', 'protect' ) ),
+                       $this->title->getUserPermissionsErrors( 'edit',
+                               $this->user ) );
+
+               $this->setUserPerm( array( "edit", "editprotected" ) );
                $this->assertEquals( false,
                        $this->title->quickUserCan( 'bogus', $this->user ) );
                $this->assertEquals( false,
@@ -566,6 +586,7 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
                $this->assertEquals( false,
                        $this->title->userCan( 'bogus', $this->user ) );
                $this->assertEquals( array( array( "cascadeprotected", 2, "* [[:Bogus]]\n* [[:UnBogus]]\n" ),
+                               array( "cascadeprotected", 2, "* [[:Bogus]]\n* [[:UnBogus]]\n" ),
                                array( "cascadeprotected", 2, "* [[:Bogus]]\n* [[:UnBogus]]\n" ) ),
                        $this->title->getUserPermissionsErrors( 'bogus', $this->user ) );
 
@@ -591,6 +612,12 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 
                $this->title->mTitleProtection['pt_create_perm'] = 'sysop';
                $this->setUserPerm( array( 'createpage', 'protect' ) );
+               $this->assertEquals( array( array( 'titleprotected', 'Useruser', 'test' ) ),
+                       $this->title->getUserPermissionsErrors( 'create', $this->user ) );
+               $this->assertEquals( false,
+                       $this->title->userCan( 'create', $this->user ) );
+
+               $this->setUserPerm( array( 'createpage', 'editprotected' ) );
                $this->assertEquals( array(),
                        $this->title->getUserPermissionsErrors( 'create', $this->user ) );
                $this->assertEquals( true,