Anomie has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/71536


Change subject: Fix protection rights usage
......................................................................

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
---
M RELEASE-NOTES-1.22
M includes/DefaultSettings.php
M includes/ProtectionForm.php
M includes/Title.php
M includes/User.php
M includes/WikiPage.php
M languages/messages/MessagesEn.php
M languages/messages/MessagesQqq.php
M maintenance/dictionary/mediawiki.dic
M maintenance/language/messages.inc
M tests/phpunit/includes/TitlePermissionTest.php
11 files changed, 109 insertions(+), 30 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/36/71536/1

diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index fb8896a..72d7137 100644
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -31,6 +31,12 @@
 * New rights 'editmyusercss', 'editmyuserjs', 'viewmywatchlist',
   and 'editmywatchlist' 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.
@@ -119,6 +125,10 @@
   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 fixes in 1.22 ===
 * Disable Special:PasswordReset when $wgEnableEmail is false. Previously one
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 31659f3..8578b7c 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -3896,17 +3896,18 @@
 
 // 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;
@@ -3927,6 +3928,7 @@
 $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;
@@ -3934,6 +3936,7 @@
 $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;
@@ -4032,7 +4035,8 @@
  * 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' );
 
@@ -4047,7 +4051,8 @@
  * "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..a14f1a0 100644
--- a/includes/ProtectionForm.php
+++ b/includes/ProtectionForm.php
@@ -135,8 +135,13 @@
                        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 ) ) {
@@ -562,8 +567,13 @@
                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..9ae7424 100644
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -1925,18 +1925,21 @@
         */
        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 @@
                        # 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 @@
                        $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'] ) )
@@ -3548,7 +3561,13 @@
                        }
                } 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 685fe96..4c07b68 100644
--- a/includes/User.php
+++ b/includes/User.php
@@ -127,6 +127,7 @@
                'editmyusercss',
                'editmyuserjs',
                'editmywatchlist',
+               'editsemiprotected',
                'editusercssjs', #deprecated
                'editusercss',
                'edituserjs',
diff --git a/includes/WikiPage.php b/includes/WikiPage.php
index 398a424..4aab674 100644
--- a/includes/WikiPage.php
+++ b/includes/WikiPage.php
@@ -2348,8 +2348,11 @@
                        $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 b29c204..52e3aff 100644
--- a/languages/messages/MessagesEn.php
+++ b/languages/messages/MessagesEn.php
@@ -2089,7 +2089,8 @@
 '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 7f4d1ee..5e8f3d7 100644
--- a/languages/messages/MessagesQqq.php
+++ b/languages/messages/MessagesQqq.php
@@ -2910,6 +2910,7 @@
 {{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 f73dfc1..9468640 100644
--- a/maintenance/dictionary/mediawiki.dic
+++ b/maintenance/dictionary/mediawiki.dic
@@ -1301,6 +1301,7 @@
 editsection
 editsectionhint
 editsectiononrightclick
+editsemiprotected
 editsonly
 editthispage
 edittime
diff --git a/maintenance/language/messages.inc 
b/maintenance/language/messages.inc
index 900a350..0048657 100644
--- a/maintenance/language/messages.inc
+++ b/maintenance/language/messages.inc
@@ -1217,6 +1217,7 @@
                '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
--- a/tests/phpunit/includes/TitlePermissionTest.php
+++ b/tests/phpunit/includes/TitlePermissionTest.php
@@ -506,39 +506,59 @@
 
                $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 @@
                $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 @@
 
                $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,

-- 
To view, visit https://gerrit.wikimedia.org/r/71536
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6bf650a3fbdab8589ae6945c8c916eafd949e41c
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie <bjor...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to