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