From: Kosta Harlan <kharlan@wikimedia.org>
Date: Wed, 3 Oct 2018 16:47:14 +0000 (-0400)
Subject: SECURITY: Fix permissions check for patrol action
X-Git-Tag: 1.34.0-rc.0~3911^2
X-Git-Url: http://git.cyclocoop.org/data/Fool?a=commitdiff_plain;h=890ffc619dd8fea0526cf76e794835f322d39d0c;p=lhc%2Fweb%2Fwiklou.git

SECURITY: Fix permissions check for patrol action

Return existing errors instead of empty array in checkUserConfigPermissions().
Returning an empty array wiped out previously-found errors.

Also add test coverage for patrol action.

Bug: T206130
Change-Id: I2df0551c5837adc578b27082ab6ba2ac95d937f8
---

diff --git a/includes/Title.php b/includes/Title.php
index 5b0c3bc2dd..de551b4d23 100644
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -2447,7 +2447,7 @@ class Title implements LinkTarget {
 		# XXX: this might be better using restrictions
 
 		if ( $action === 'patrol' ) {
-			return [];
+			return $errors;
 		}
 
 		if ( preg_match( '/^' . preg_quote( $user->getName(), '/' ) . '\//', $this->mTextform ) ) {
diff --git a/tests/phpunit/includes/TitlePermissionTest.php b/tests/phpunit/includes/TitlePermissionTest.php
index dd84b7eb01..f53b0596c1 100644
--- a/tests/phpunit/includes/TitlePermissionTest.php
+++ b/tests/phpunit/includes/TitlePermissionTest.php
@@ -454,6 +454,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 	 * @covers Title::checkUserConfigPermissions
 	 */
 	public function testJsConfigEditPermissions() {
+		$prefix = MediaWikiServices::getInstance()->getContentLanguage()->
+			getFormattedNsText( NS_PROJECT );
 		$this->setUser( $this->userName );
 
 		$this->setTitle( NS_USER, $this->userName . '/test.js' );
@@ -466,7 +468,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 
 			[ [ 'badaccess-group0' ], [ 'mycustomjsprotected', 'bogus' ] ],
 			[ [ 'badaccess-group0' ], [ 'mycustomjsprotected', 'bogus' ] ],
-			[ [ 'badaccess-group0' ] ]
+			[ [ 'badaccess-group0' ] ],
+			[ [ 'badaccess-groups', "[[$prefix:Administrators|Administrators]]", 1 ] ]
 		);
 	}
 
@@ -476,6 +479,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 	 * @covers Title::checkUserConfigPermissions
 	 */
 	public function testJsonConfigEditPermissions() {
+		$prefix = MediaWikiServices::getInstance()->getContentLanguage()->
+			getFormattedNsText( NS_PROJECT );
 		$this->setUser( $this->userName );
 
 		$this->setTitle( NS_USER, $this->userName . '/test.json' );
@@ -488,7 +493,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 
 			[ [ 'badaccess-group0' ], [ 'mycustomjsonprotected', 'bogus' ] ],
 			[ [ 'badaccess-group0' ] ],
-			[ [ 'badaccess-group0' ], [ 'mycustomjsonprotected', 'bogus' ] ]
+			[ [ 'badaccess-group0' ], [ 'mycustomjsonprotected', 'bogus' ] ],
+			[ [ 'badaccess-groups', "[[$prefix:Administrators|Administrators]]", 1 ] ]
 		);
 	}
 
@@ -498,6 +504,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 	 * @covers Title::checkUserConfigPermissions
 	 */
 	public function testCssConfigEditPermissions() {
+		$prefix = MediaWikiServices::getInstance()->getContentLanguage()->
+			getFormattedNsText( NS_PROJECT );
 		$this->setUser( $this->userName );
 
 		$this->setTitle( NS_USER, $this->userName . '/test.css' );
@@ -510,7 +518,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 
 			[ [ 'badaccess-group0' ] ],
 			[ [ 'badaccess-group0' ], [ 'mycustomcssprotected', 'bogus' ] ],
-			[ [ 'badaccess-group0' ], [ 'mycustomcssprotected', 'bogus' ] ]
+			[ [ 'badaccess-group0' ], [ 'mycustomcssprotected', 'bogus' ] ],
+			[ [ 'badaccess-groups', "[[$prefix:Administrators|Administrators]]", 1 ] ]
 		);
 	}
 
@@ -520,6 +529,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 	 * @covers Title::checkUserConfigPermissions
 	 */
 	public function testOtherJsConfigEditPermissions() {
+		$prefix = MediaWikiServices::getInstance()->getContentLanguage()->
+			getFormattedNsText( NS_PROJECT );
 		$this->setUser( $this->userName );
 
 		$this->setTitle( NS_USER, $this->altUserName . '/test.js' );
@@ -532,7 +543,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 
 			[ [ 'badaccess-group0' ], [ 'customjsprotected', 'bogus' ] ],
 			[ [ 'badaccess-group0' ], [ 'customjsprotected', 'bogus' ] ],
-			[ [ 'badaccess-group0' ] ]
+			[ [ 'badaccess-group0' ] ],
+			[ [ 'badaccess-groups', "[[$prefix:Administrators|Administrators]]", 1 ] ]
 		);
 	}
 
@@ -542,6 +554,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 	 * @covers Title::checkUserConfigPermissions
 	 */
 	public function testOtherJsonConfigEditPermissions() {
+		$prefix = MediaWikiServices::getInstance()->getContentLanguage()->
+			getFormattedNsText( NS_PROJECT );
 		$this->setUser( $this->userName );
 
 		$this->setTitle( NS_USER, $this->altUserName . '/test.json' );
@@ -554,7 +568,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 
 			[ [ 'badaccess-group0' ], [ 'customjsonprotected', 'bogus' ] ],
 			[ [ 'badaccess-group0' ] ],
-			[ [ 'badaccess-group0' ], [ 'customjsonprotected', 'bogus' ] ]
+			[ [ 'badaccess-group0' ], [ 'customjsonprotected', 'bogus' ] ],
+			[ [ 'badaccess-groups', "[[$prefix:Administrators|Administrators]]", 1 ] ]
 		);
 	}
 
@@ -564,6 +579,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 	 * @covers Title::checkUserConfigPermissions
 	 */
 	public function testOtherCssConfigEditPermissions() {
+		$prefix = MediaWikiServices::getInstance()->getContentLanguage()->
+			getFormattedNsText( NS_PROJECT );
 		$this->setUser( $this->userName );
 
 		$this->setTitle( NS_USER, $this->altUserName . '/test.css' );
@@ -576,7 +593,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 
 			[ [ 'badaccess-group0' ] ],
 			[ [ 'badaccess-group0' ], [ 'customcssprotected', 'bogus' ] ],
-			[ [ 'badaccess-group0' ], [ 'customcssprotected', 'bogus' ] ]
+			[ [ 'badaccess-group0' ], [ 'customcssprotected', 'bogus' ] ],
+			[ [ 'badaccess-groups', "[[$prefix:Administrators|Administrators]]", 1 ] ]
 		);
 	}
 
@@ -586,6 +604,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 	 * @covers Title::checkUserConfigPermissions
 	 */
 	public function testOtherNonConfigEditPermissions() {
+		$prefix = MediaWikiServices::getInstance()->getContentLanguage()->
+			getFormattedNsText( NS_PROJECT );
 		$this->setUser( $this->userName );
 
 		$this->setTitle( NS_USER, $this->altUserName . '/tempo' );
@@ -598,7 +618,31 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 
 			[ [ 'badaccess-group0' ] ],
 			[ [ 'badaccess-group0' ] ],
-			[ [ 'badaccess-group0' ] ]
+			[ [ 'badaccess-group0' ] ],
+			[ [ 'badaccess-groups', "[[$prefix:Administrators|Administrators]]", 1 ] ]
+		);
+	}
+
+	/**
+	 * @todo This should use data providers like the other methods here.
+	 * @covers Title::checkUserConfigPermissions
+	 */
+	public function testPatrolActionConfigEditPermissions() {
+		$prefix = MediaWikiServices::getInstance()->getContentLanguage()->
+			getFormattedNsText( NS_PROJECT );
+		$this->setUser( 'anon' );
+		$this->setTitle( NS_USER, 'ToPatrolOrNotToPatrol' );
+		$this->runConfigEditPermissions(
+			[ [ 'badaccess-group0' ] ],
+
+			[ [ 'badaccess-group0' ] ],
+			[ [ 'badaccess-group0' ] ],
+			[ [ 'badaccess-group0' ] ],
+
+			[ [ 'badaccess-group0' ] ],
+			[ [ 'badaccess-group0' ] ],
+			[ [ 'badaccess-group0' ] ],
+			[ [ 'badaccess-groups', "[[$prefix:Administrators|Administrators]]", 1 ] ]
 		);
 	}
 
@@ -609,12 +653,17 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
 		$resultMyJs,
 		$resultUserCss,
 		$resultUserJson,
-		$resultUserJs
+		$resultUserJs,
+		$resultPatrol
 	) {
 		$this->setUserPerm( '' );
 		$result = $this->title->getUserPermissionsErrors( 'bogus', $this->user );
 		$this->assertEquals( $resultNone, $result );
 
+		$this->setUserPerm( '' );
+		$result = $this->title->getUserPermissionsErrors( 'patrol', $this->user );
+		$this->assertEquals( $resultPatrol, $result );
+
 		$this->setUserPerm( 'editmyusercss' );
 		$result = $this->title->getUserPermissionsErrors( 'bogus', $this->user );
 		$this->assertEquals( $resultMyCss, $result );