Merge "Separate right for foreign user js redirects"

diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34
index 99ede17..4b28012 100644 (file)
--- 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 3bfc8f8..107c546 100644 (file)
--- 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 5a3dae3..a04b29c 100644 (file)
--- 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 339c34c..1bb848f 100644 (file)
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -465,6 +465,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 fb265d3..eeca124 100644 (file)
--- a/languages/i18n/en.json
+++ b/languages/i18n/en.json
@@ -1275,6 +1275,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)",
@@ -1405,6 +1406,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",
@@ -4237,6 +4239,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 a348031..76d969d 100644 (file)
--- a/languages/i18n/qqq.json
+++ b/languages/i18n/qqq.json
@@ -1483,7 +1483,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}}",
@@ -1614,6 +1615,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}}",
@@ -4446,6 +4448,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 5b015b3..8a98217 100644 (file)
--- a/tests/phpunit/includes/Permissions/PermissionManagerTest.php
+++ b/tests/phpunit/includes/Permissions/PermissionManagerTest.php
@@ -3,21 +3,25 @@
 namespace MediaWiki\Tests\Permissions;
 
 use Action;
+use ContentHandler;
 use FauxRequest;
-use MediaWiki\Session\SessionId;
-use MediaWiki\Session\TestUtils;
-use MediaWikiLangTestCase;
-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\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;
+use RequestContext;
+use stdClass;
+use Title;
+use User;
 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
@@ -1683,4 +1745,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;
+       }
+
 }