diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34 index 60d5d96a7e2..3deeaab6d63 100644 --- a/RELEASE-NOTES-1.34 +++ b/RELEASE-NOTES-1.34 @@ -36,6 +36,9 @@ For notes on 1.33.x and older releases, see HISTORY. * $wgEnableSpecialMute (T218265) - This configuration controls whether Special:Mute is available and whether to include a link to it on emails originating from Special:Email. +* editmyuserjsredirect user right – users without this right now cannot edit JS + redirects in their userspace unless the target of the redirect is also in + their userspace. By default, this right is given to everyone. ==== Changed configuration ==== * $wgUseCdn, $wgCdnServers, $wgCdnServersNoPurge, and $wgCdnMaxAge – These four diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 3bfc8f8241e..107c5467247 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -5173,6 +5173,7 @@ $wgGroupPermissions['user']['minoredit'] = true; $wgGroupPermissions['user']['editmyusercss'] = true; $wgGroupPermissions['user']['editmyuserjson'] = true; $wgGroupPermissions['user']['editmyuserjs'] = true; +$wgGroupPermissions['user']['editmyuserjsredirect'] = true; $wgGroupPermissions['user']['purge'] = true; $wgGroupPermissions['user']['sendemail'] = true; $wgGroupPermissions['user']['applychangetags'] = true; diff --git a/includes/Permissions/PermissionManager.php b/includes/Permissions/PermissionManager.php index 98a5b176088..483c1718f7f 100644 --- a/includes/Permissions/PermissionManager.php +++ b/includes/Permissions/PermissionManager.php @@ -23,6 +23,8 @@ use Action; use Exception; use Hooks; use MediaWiki\Linker\LinkTarget; +use MediaWiki\Revision\RevisionLookup; +use MediaWiki\Revision\RevisionRecord; use MediaWiki\Session\SessionManager; use MediaWiki\Special\SpecialPageFactory; use MediaWiki\User\UserIdentity; @@ -55,6 +57,9 @@ class PermissionManager { /** @var SpecialPageFactory */ private $specialPageFactory; + /** @var RevisionLookup */ + private $revisionLookup; + /** @var string[] List of pages names anonymous user may see */ private $whitelistRead; @@ -130,6 +135,7 @@ class PermissionManager { 'editmyusercss', 'editmyuserjson', 'editmyuserjs', + 'editmyuserjsredirect', 'editmywatchlist', 'editsemiprotected', 'editsitecss', @@ -184,6 +190,7 @@ class PermissionManager { /** * @param SpecialPageFactory $specialPageFactory + * @param RevisionLookup $revisionLookup * @param string[] $whitelistRead * @param string[] $whitelistReadRegexp * @param bool $emailConfirmToEdit @@ -195,6 +202,7 @@ class PermissionManager { */ public function __construct( SpecialPageFactory $specialPageFactory, + RevisionLookup $revisionLookup, $whitelistRead, $whitelistReadRegexp, $emailConfirmToEdit, @@ -205,6 +213,7 @@ class PermissionManager { NamespaceInfo $nsInfo ) { $this->specialPageFactory = $specialPageFactory; + $this->revisionLookup = $revisionLookup; $this->whitelistRead = $whitelistRead; $this->whitelistReadRegexp = $whitelistReadRegexp; $this->emailConfirmToEdit = $emailConfirmToEdit; @@ -1134,6 +1143,20 @@ class PermissionManager { && !$user->isAllowedAny( 'editmyuserjs', 'edituserjs' ) ) { $errors[] = [ 'mycustomjsprotected', $action ]; + } elseif ( + $page->isUserJsConfigPage() + && !$user->isAllowedAny( 'edituserjs', 'editmyuserjsredirect' ) + ) { + // T207750 - do not allow users to edit a redirect if they couldn't edit the target + $rev = $this->revisionLookup->getRevisionByTitle( $page ); + $content = $rev ? $rev->getContent( 'main', RevisionRecord::RAW ) : null; + $target = $content ? $content->getUltimateRedirectTarget() : null; + if ( $target && ( + !$target->inNamespace( NS_USER ) + || !preg_match( '/^' . preg_quote( $user->getName(), '/' ) . '\//', $target->getText() ) + ) ) { + $errors[] = [ 'mycustomjsredirectprotected', $action ]; + } } } else { // Users need editmyuser* to edit their own CSS/JSON/JS subpages, except for diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 7d2b3cb14f2..8f9044e5bac 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -466,6 +466,7 @@ return [ $config = $services->getMainConfig(); return new PermissionManager( $services->getSpecialPageFactory(), + $services->getRevisionLookup(), $config->get( 'WhitelistRead' ), $config->get( 'WhitelistReadRegexp' ), $config->get( 'EmailConfirmToEdit' ), diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 0f401213bcd..272b97d8088 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -1273,6 +1273,7 @@ "right-editmyusercss": "Edit your own user CSS files", "right-editmyuserjson": "Edit your own user JSON files", "right-editmyuserjs": "Edit your own user JavaScript files", + "right-editmyuserjsredirect": "Edit your own user JavaScript files that are redirects", "right-viewmywatchlist": "View your own watchlist", "right-editmywatchlist": "Edit your own watchlist. Note some actions will still add pages even without this right.", "right-viewmyprivateinfo": "View your own private data (e.g. email address, real name)", @@ -1403,6 +1404,7 @@ "action-editmyusercss": "edit your own user CSS files", "action-editmyuserjson": "edit your own user JSON files", "action-editmyuserjs": "edit your own user JavaScript files", + "action-editmyuserjsredirect": "edit your own user JavaScript files that are redirects", "action-viewsuppressed": "view revisions hidden from any user", "action-hideuser": "block a username, hiding it from the public", "action-ipblock-exempt": "bypass IP blocks, auto-blocks and range blocks", @@ -4236,6 +4238,7 @@ "passwordpolicies-policy-passwordnotinlargeblacklist": "Password cannot be in the list of 100,000 most commonly used passwords.", "passwordpolicies-policyflag-forcechange": "must change on login", "passwordpolicies-policyflag-suggestchangeonlogin": "suggest change on login", + "mycustomjsredirectprotected": "You do not have permission to edit this JavaScript page because it is a redirect and it does not point inside your userspace.", "easydeflate-invaliddeflate": "Content provided is not properly deflated", "unprotected-js": "For security reasons JavaScript cannot be loaded from unprotected pages. Please only create javascript in the MediaWiki: namespace or as a User subpage", "userlogout-continue": "Do you want to log out?" diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index 965f1633b1e..3032a550eb9 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -1481,7 +1481,8 @@ "right-editsitejs": "{{doc-right|editsitejs}}", "right-editmyusercss": "{{doc-right|editmyusercss}}\nSee also:\n* {{msg-mw|Right-editusercss}}", "right-editmyuserjson": "{{doc-right|editmyuserjson}}\nSee also:\n* {{msg-mw|Right-edituserjson}}", - "right-editmyuserjs": "{{doc-right|editmyuserjs}}\nSee also:\n* {{msg-mw|Right-edituserjs}}", + "right-editmyuserjs": "{{doc-right|editmyuserjs}}\nSee also:\n* {{msg-mw|Right-edituserjs}}\n* {{msg-mw|Right-editmyuserjsredirect}}", + "right-editmyuserjsredirect": "{{doc-right|editmyuserjsredirect}}\nSame as {{msg-mw|Right-editmyuserjs}} except if page is a redirect.\n\nSee also:\n* {{msg-mw|Right-edituserjs}}", "right-viewmywatchlist": "{{doc-right|viewmywatchlist}}", "right-editmywatchlist": "{{doc-right|editmywatchlist}}", "right-viewmyprivateinfo": "{{doc-right|viewmyprivateinfo}}", @@ -1612,6 +1613,7 @@ "action-editmyusercss": "{{doc-action|editmyusercss}}", "action-editmyuserjson": "{{doc-action|editmyuserjson}}", "action-editmyuserjs": "{{doc-action|editmyuserjs}}", + "action-editmyuserjsredirect": "{{doc-action|editmyuserjsredirect}}", "action-viewsuppressed": "{{doc-action|viewsuppressed}}", "action-hideuser": "{{doc-action|hideuser}}", "action-ipblock-exempt": "{{doc-action|ipblock-exempt}}", @@ -4445,6 +4447,7 @@ "passwordpolicies-policy-passwordnotinlargeblacklist": "Password policy that enforces that a password is not in a list of 100,000 number of \"popular\" passwords.", "passwordpolicies-policyflag-forcechange": "Password policy flag that enforces changing invalid passwords on login.", "passwordpolicies-policyflag-suggestchangeonlogin": "Password policy flag that suggests changing invalid passwords on login.", + "mycustomjsredirectprotected": "Error message shown when user tries to edit their own JS page that is a foreign redirect without the 'mycustomjsredirectprotected' right. See also {{mw-msg|mycustomjsprotected}}.", "easydeflate-invaliddeflate": "Error message if the content passed to easydeflate was not deflated (compressed) properly", "unprotected-js": "Error message shown when trying to load javascript via action=raw that is not protected", "userlogout-continue": "Shown if user attempted to log out without a token specified. Probably the user clicked on an old link that hasn't been updated to use the new system. $1 - url that user should click on in order to log out." diff --git a/tests/phpunit/includes/Permissions/PermissionManagerTest.php b/tests/phpunit/includes/Permissions/PermissionManagerTest.php index 03b35b53316..61dcac4ac24 100644 --- a/tests/phpunit/includes/Permissions/PermissionManagerTest.php +++ b/tests/phpunit/includes/Permissions/PermissionManagerTest.php @@ -3,7 +3,18 @@ namespace MediaWiki\Tests\Permissions; use Action; +use ContentHandler; use FauxRequest; +use MediaWiki\Block\DatabaseBlock; +use MediaWiki\Block\Restriction\NamespaceRestriction; +use MediaWiki\Block\Restriction\PageRestriction; +use MediaWiki\Block\SystemBlock; +use MediaWiki\Linker\LinkTarget; +use MediaWiki\MediaWikiServices; +use MediaWiki\Permissions\PermissionManager; +use MediaWiki\Revision\MutableRevisionRecord; +use MediaWiki\Revision\RevisionLookup; +use Wikimedia\ScopedCallback; use MediaWiki\Session\SessionId; use MediaWiki\Session\TestUtils; use MediaWikiLangTestCase; @@ -11,13 +22,6 @@ use RequestContext; use stdClass; use Title; use User; -use MediaWiki\Block\DatabaseBlock; -use MediaWiki\Block\Restriction\NamespaceRestriction; -use MediaWiki\Block\Restriction\PageRestriction; -use MediaWiki\Block\SystemBlock; -use MediaWiki\MediaWikiServices; -use MediaWiki\Permissions\PermissionManager; -use Wikimedia\ScopedCallback; use Wikimedia\TestingAccessWrapper; /** @@ -698,6 +702,64 @@ class PermissionManagerTest extends MediaWikiLangTestCase { ); } + public function testJsConfigRedirectEditPermissions() { + $revision = null; + $user = $this->getTestUser()->getUser(); + $otherUser = $this->getTestUser( 'sysop' )->getUser(); + $localJsTitle = Title::newFromText( 'User:' . $user->getName() . '/foo.js' ); + $otherLocalJsTitle = Title::newFromText( 'User:' . $user->getName() . '/foo2.js' ); + $nonlocalJsTitle = Title::newFromText( 'User:' . $otherUser->getName() . '/foo.js' ); + + $services = MediaWikiServices::getInstance(); + $revisionLookup = $this->getMockBuilder( RevisionLookup::class ) + ->setMethods( [ 'getRevisionByTitle' ] ) + ->getMockForAbstractClass(); + $revisionLookup->method( 'getRevisionByTitle' ) + ->willReturnCallback( function ( LinkTarget $page ) use ( + $services, &$revision, $localJsTitle + ) { + if ( $localJsTitle->equals( Title::newFromLinkTarget( $page ) ) ) { + return $revision; + } else { + return $services->getRevisionLookup()->getRevisionByTitle( $page ); + } + } ); + $permissionManager = new PermissionManager( + $services->getSpecialPageFactory(), + $revisionLookup, + [], + [], + false, + false, + [], + [], + [], + MediaWikiServices::getInstance()->getNamespaceInfo() + ); + $this->setService( 'PermissionManager', $permissionManager ); + + $permissionManager->overrideUserRightsForTesting( $user, [ 'edit', 'editmyuserjs' ] ); + + $revision = $this->getJavascriptRevision( $localJsTitle, $user, '/* script */' ); + $errors = $permissionManager->getPermissionErrors( 'edit', $user, $localJsTitle ); + $this->assertSame( [], $errors ); + + $revision = $this->getJavascriptRedirectRevision( $localJsTitle, $otherLocalJsTitle, $user ); + $errors = $permissionManager->getPermissionErrors( 'edit', $user, $localJsTitle ); + $this->assertSame( [], $errors ); + + $revision = $this->getJavascriptRedirectRevision( $localJsTitle, $nonlocalJsTitle, $user ); + $errors = $permissionManager->getPermissionErrors( 'edit', $user, $localJsTitle ); + $this->assertSame( [ [ 'mycustomjsredirectprotected', 'edit' ] ], $errors ); + + $permissionManager->overrideUserRightsForTesting( $user, + [ 'edit', 'editmyuserjs', 'editmyuserjsredirect' ] ); + + $revision = $this->getJavascriptRedirectRevision( $localJsTitle, $nonlocalJsTitle, $user ); + $errors = $permissionManager->getPermissionErrors( 'edit', $user, $localJsTitle ); + $this->assertSame( [], $errors ); + } + /** * @todo This test method should be split up into separate test methods and * data providers @@ -1684,4 +1746,35 @@ class PermissionManagerTest extends MediaWikiLangTestCase { $this->assertFalse( $permissionManager->userHasRight( $this->user, 'move' ) ); } + /** + * Create a RevisionRecord with a single Javascript main slot. + * @param Title $title + * @param User $user + * @param string $text + * @return MutableRevisionRecord + */ + private function getJavascriptRevision( Title $title, User $user, $text ) { + $content = ContentHandler::makeContent( $text, $title, CONTENT_MODEL_JAVASCRIPT ); + $revision = new MutableRevisionRecord( $title ); + $revision->setContent( 'main', $content ); + return $revision; + } + + /** + * Create a RevisionRecord with a single Javascript redirect main slot. + * @param Title $title + * @param Title $redirectTargetTitle + * @param User $user + * @return MutableRevisionRecord + */ + private function getJavascriptRedirectRevision( + Title $title, Title $redirectTargetTitle, User $user + ) { + $content = ContentHandler::getForModelID( CONTENT_MODEL_JAVASCRIPT ) + ->makeRedirectContent( $redirectTargetTitle ); + $revision = new MutableRevisionRecord( $title ); + $revision->setContent( 'main', $content ); + return $revision; + } + }