PermissionManager: Differentiate between cascading protection of file content and file pages

This patch reworks RestrictionStore::getCascadeProtectionSourcesInternal to return a third and fourth array: * One for cascading restrictions originating from templatelinks * Another for those originating from imagelinks They are used in PermissionManager::checkCascadingSourcesRestrictions to differentiate cascading protection of file content and file page, but could also be used in the future by action=info and other callers. Bug: T24521 Bug: T62109 Bug: T140010 Change-Id: Ia5863f418538106f4fd657c672298ff6ac835805 (cherry picked from commit 7a4952ef2c5d593fae9419bad39f3e9894f42adf)
2025-01-19 03:06:14 +00:00 · 2025-01-19 03:06:14 +00:00 · 7071887383
commit 7071887383
parent 06b9aa64b2
6 changed files with 247 additions and 56 deletions
--- a/includes/Permissions/PermissionManager.php
+++ b/includes/Permissions/PermissionManager.php
@ -1130,14 +1130,32 @@ class PermissionManager {
 	): void {
 		// TODO: remove & rework upon further use of LinkTarget
 		$title = Title::newFromLinkTarget( $page );
+
 		if ( $rigor !== self::RIGOR_QUICK && !$title->isUserConfigPage() ) {
-			[ $cascadingSources, $restrictions ] = $this->restrictionStore->getCascadeProtectionSources( $title );
+			[ $sources, $restrictions, $tlSources, $ilSources ] = $this->restrictionStore
+				->getCascadeProtectionSources( $title );
+
+			// If the file Wikitext isn't transcluded then we
+			// don't care about edit cascade restrictions for edit action
+			if ( $action === 'edit' && $page->getNamespace() === NS_FILE && !$tlSources ) {
+				return;
+			}
+
+			// For the purposes of cascading protection, edit restrictions should apply to uploads or moves
+			// Thus remap upload and move to edit
+			// Unless the file content itself is not transcluded
+			if ( $ilSources && ( $action === 'upload' || $action === 'move' ) ) {
+				$restrictedAction = 'edit';
+			} else {
+				$restrictedAction = $action;
+			}
+
 			// Cascading protection depends on more than this page...
 			// Several cascading protected pages may include this page...
 			// Check each cascading level
 			// This is only for protection restrictions, not for all actions
-			if ( isset( $restrictions[$action] ) ) {
-				foreach ( $restrictions[$action] as $right ) {
+			if ( isset( $restrictions[$restrictedAction] ) ) {
+				foreach ( $restrictions[$restrictedAction] as $right ) {
 					// Backwards compatibility, rewrite sysop -> editprotected
 					if ( $right === 'sysop' ) {
 						$right = 'editprotected';
@ -1148,10 +1166,10 @@ class PermissionManager {
 					}
 					if ( $right != '' && !$this->userHasAllRights( $user, 'protect', $right ) ) {
 						$wikiPages = '';
-						foreach ( $cascadingSources as $pageIdentity ) {
+						foreach ( $sources as $pageIdentity ) {
 							$wikiPages .= '* [[:' . $this->titleFormatter->getPrefixedText( $pageIdentity ) . "]]\n";
 						}
-						$status->fatal( 'cascadeprotected', count( $cascadingSources ), $wikiPages, $action );
+						$status->fatal( 'cascadeprotected', count( $sources ), $wikiPages, $action );
 					}
 				}
 			}
--- a/includes/Permissions/RestrictionStore.php
+++ b/includes/Permissions/RestrictionStore.php
@ -520,9 +520,13 @@ class RestrictionStore {
 	 * Cascading protection: Get the source of any cascading restrictions on this page.
 	 *
 	 * @param PageIdentity $page Must be local
-	 * @return array[] Two elements: First is an array of PageIdentity objects of the pages from
-	 *   which cascading restrictions have come, which may be empty. Second is an array like that
-	 *   returned by getAllRestrictions().
+	 * @return array[] Four elements: First is an array of PageIdentity objects combining the
+	 *   third and fourth elements of this array, which may be empty.
+	 *   Second is an array like that returned by getAllRestrictions().
+	 *   Third is an array of PageIdentity objects of the pages from
+	 *   which cascading restrictions have come, orginating via templatelinks, which may be empty.
+	 *   Fourth is an array of PageIdentity objects of the pages from
+	 *   which cascading restrictions have come, orginating via imagelinks, which may be empty.
 	 */
 	public function getCascadeProtectionSources( PageIdentity $page ): array {
 		$page->assertWiki( PageIdentity::LOCAL );
@ -542,7 +546,7 @@ class RestrictionStore {
 		PageIdentity $page, bool $shortCircuit = false
 	) {
 		if ( !$page->canExist() ) {
-			return $shortCircuit ? false : [ [], [] ];
+			return $shortCircuit ? false : [ [], [], [], [] ];
 		}

 		$cacheEntry = &$this->cache[CacheKeyHelper::getKeyForPage( $page )];
@ -554,38 +558,56 @@ class RestrictionStore {
 		}

 		$dbr = $this->loadBalancer->getConnection( DB_REPLICA );
-		$queryBuilder = $dbr->newSelectQueryBuilder();
-		$queryBuilder->select( [ 'pr_expiry' ] )
+
+		$baseQuery = $dbr->newSelectQueryBuilder()
+			->select( $shortCircuit ? [ 'pr_expiry' ] : [
+				'pr_expiry',
+				'pr_page',
+				'page_namespace',
+				'page_title',
+				'pr_type',
+				'pr_level'
+			] )
 			->from( 'page_restrictions' )
 			->where( [ 'pr_cascade' => 1 ] );

-		if ( $page->getNamespace() === NS_FILE ) {
-			// Files transclusion may receive cascading protection in the future
-			// see https://phabricator.wikimedia.org/T241453
-			$queryBuilder->join( 'imagelinks', null, 'il_from=pr_page' );
-			$queryBuilder->andWhere( [ 'il_to' => $page->getDBkey() ] );
-		} else {
-			$queryBuilder->join( 'templatelinks', null, 'tl_from=pr_page' );
-			$queryBuilder->andWhere(
-				$this->linksMigration->getLinksConditions(
-					'templatelinks',
-					TitleValue::newFromPage( $page )
-				)
-			);
-		}
-
 		if ( !$shortCircuit ) {
-			$queryBuilder->fields( [ 'pr_page', 'page_namespace', 'page_title', 'pr_type', 'pr_level' ] );
-			$queryBuilder->join( 'page', null, 'page_id=pr_page' );
+			$baseQuery->join( 'page', null, 'page_id=pr_page' );
 		}

-		$res = $queryBuilder->caller( __METHOD__ )->fetchResultSet();
+		$imageQuery = clone $baseQuery;
+		$imageQuery->join( 'imagelinks', null, 'il_from=pr_page' )
+			->fields( [
+				'type' => $dbr->addQuotes( 'il' ),
+			] )
+			->andWhere( [ 'il_to' => $page->getDBkey() ] );

-		$sources = [];
+		$templateQuery = clone $baseQuery;
+		$templateQuery->join( 'templatelinks', null, 'tl_from=pr_page' )
+			->fields( [
+				'type' => $dbr->addQuotes( 'tl' ),
+			] )
+			->andWhere(
+				$this->linksMigration->getLinksConditions( 'templatelinks', TitleValue::newFromPage( $page ) )
+			);
+
+		if ( $page->getNamespace() === NS_FILE ) {
+			$unionQuery = $dbr->newUnionQueryBuilder()
+				->add( $imageQuery )
+				->add( $templateQuery )
+				->all();
+
+			$res = $unionQuery->caller( __METHOD__ )->fetchResultSet();
+		} else {
+			$res = $templateQuery->caller( __METHOD__ )->fetchResultSet();
+		}
+
+		$tlSources = [];
+		$ilSources = [];
 		$pageRestrictions = [];
 		$now = wfTimestampNow();
-
 		foreach ( $res as $row ) {
+
 			$expiry = $dbr->decodeExpiry( $row->pr_expiry );
 			if ( $expiry > $now ) {
 				if ( $shortCircuit ) {
@ -593,8 +615,14 @@ class RestrictionStore {
 					return true;
 				}

-				$sources[$row->pr_page] = new PageIdentityValue( $row->pr_page,
-						$row->page_namespace, $row->page_title, PageIdentity::LOCAL );
+				if ( $row->type === 'il' ) {
+					$ilSources[$row->pr_page] = new PageIdentityValue( $row->pr_page,
+					$row->page_namespace, $row->page_title, PageIdentity::LOCAL );
+				} elseif ( $row->type === 'tl' ) {
+					$tlSources[$row->pr_page] = new PageIdentityValue( $row->pr_page,
+					$row->page_namespace, $row->page_title, PageIdentity::LOCAL );
+				}
+
 				// Add groups needed for each restriction type if its not already there
 				// Make sure this restriction type still exists

@ -608,14 +636,16 @@ class RestrictionStore {
 			}
 		}

+		$sources = array_replace( $tlSources, $ilSources );
+
 		$cacheEntry['has_cascading'] = (bool)$sources;
+		$cacheEntry['cascade_sources'] = [ $sources, $pageRestrictions, $tlSources, $ilSources ];

 		if ( $shortCircuit ) {
 			return false;
 		}

-		$cacheEntry['cascade_sources'] = [ $sources, $pageRestrictions ];
-		return [ $sources, $pageRestrictions ];
+		return [ $sources, $pageRestrictions, $tlSources, $ilSources ];
 	}

 	/**
--- a/includes/editpage/IntroMessageBuilder.php
+++ b/includes/editpage/IntroMessageBuilder.php
@ -637,18 +637,20 @@ class IntroMessageBuilder {
 		}
 		if ( $this->restrictionStore->isCascadeProtected( $page ) ) {
 			# Is this page under cascading protection from some source pages?
-			$cascadeSources = $this->restrictionStore->getCascadeProtectionSources( $page )[0];
-			$htmlList = '';
-			# Explain, and list the titles responsible
-			foreach ( $cascadeSources as $source ) {
-				$htmlList .= Html::rawElement( 'li', [], $this->linkRenderer->makeLink( $source ) );
+			$tlCascadeSources = $this->restrictionStore->getCascadeProtectionSources( $page )[2];
+			if ( $tlCascadeSources ) {
+				$htmlList = '';
+				# Explain, and list the titles responsible
+				foreach ( $tlCascadeSources as $source ) {
+					$htmlList .= Html::rawElement( 'li', [], $this->linkRenderer->makeLink( $source ) );
+				}
+				$messages->addWithKey(
+					'cascadeprotectedwarning',
+					$localizer->msg( 'cascadeprotectedwarning', count( $tlCascadeSources ) )->parse() .
+						( $htmlList ? Html::rawElement( 'ul', [], $htmlList ) : '' ),
+					Html::warningBox( '$1', 'mw-cascadeprotectedwarning' )
+				);
 			}
-			$messages->addWithKey(
-				'cascadeprotectedwarning',
-				$localizer->msg( 'cascadeprotectedwarning', count( $cascadeSources ) )->parse() .
-					( $htmlList ? Html::rawElement( 'ul', [], $htmlList ) : '' ),
-				Html::warningBox( '$1', 'mw-cascadeprotectedwarning' )
-			);
 		}
 		if ( !$page->exists() && $this->restrictionStore->getRestrictions( $page, 'create' ) ) {
 			$messages->addWithKey(
--- a/tests/phpunit/includes/Permissions/PermissionManagerTest.php
+++ b/tests/phpunit/includes/Permissions/PermissionManagerTest.php
@ -254,9 +254,15 @@ class PermissionManagerTest extends MediaWikiLangTestCase {
 				[
 					Title::makeTitle( NS_MAIN, "Bogus" ),
 					Title::makeTitle( NS_MAIN, "UnBogus" )
-				], [
+				],
+				[
 					"bogus" => [ 'bogus', "sysop", "protect", "" ],
-				]
+				],
+				[
+					Title::makeTitle( NS_MAIN, "Bogus" ),
+					Title::makeTitle( NS_MAIN, "UnBogus" )
+				],
+				[]
 			],
 		] ];

@ -275,6 +281,51 @@ class PermissionManagerTest extends MediaWikiLangTestCase {
 		);
 	}

+	public function testCascadingSourcesRestrictionsForFile() {
+		$this->setTitle( NS_FILE, 'Test.jpg' );
+		$this->overrideUserPermissions( $this->user, [ 'edit', 'move', 'upload', 'movefile', 'createpage' ] );
+
+		$rs = $this->getServiceContainer()->getRestrictionStore();
+		$wrapper = TestingAccessWrapper::newFromObject( $rs );
+		$wrapper->cache = [ CacheKeyHelper::getKeyForPage( $this->title ) => [
+				'cascade_sources' => [
+					[
+						Title::makeTitle( NS_MAIN, 'FileTemplate' ),
+						Title::makeTitle( NS_MAIN, 'FileUser' )
+					],
+					[
+						'edit' => [ 'sysop' ],
+					],
+					[
+						Title::makeTitle( NS_MAIN, 'FileTemplate' )
+					],
+					[
+						Title::makeTitle( NS_MAIN, 'FileUser' )
+					]
+				],
+			] ];
+
+		$permissionManager = $this->getServiceContainer()->getPermissionManager();
+
+		$this->assertFalse( $permissionManager->userCan( 'upload', $this->user, $this->title ) );
+		$this->assertEquals( [
+			[ 'cascadeprotected', 2, "* [[:FileTemplate]]\n* [[:FileUser]]\n", 'upload' ] ],
+			$permissionManager->getPermissionErrors( 'upload', $this->user, $this->title )
+		);
+
+		$this->assertFalse( $permissionManager->userCan( 'move', $this->user, $this->title ) );
+		$this->assertEquals( [
+			[ 'cascadeprotected', 2, "* [[:FileTemplate]]\n* [[:FileUser]]\n", 'move' ] ],
+			$permissionManager->getPermissionErrors( 'move', $this->user, $this->title )
+		);
+
+		$this->assertFalse( $permissionManager->userCan( 'edit', $this->user, $this->title ) );
+		$this->assertEquals( [
+			[ 'cascadeprotected', 2, "* [[:FileTemplate]]\n* [[:FileUser]]\n", 'edit' ] ],
+			$permissionManager->getPermissionErrors( 'edit', $this->user, $this->title )
+		);
+	}
+
 	/**
 	 * @dataProvider provideActionPermissions
 	 */
--- a/tests/phpunit/integration/includes/Permissions/RestrictionStoreTest.php
+++ b/tests/phpunit/integration/includes/Permissions/RestrictionStoreTest.php
@ -42,6 +42,10 @@ class RestrictionStoreTest extends MediaWikiIntegrationTestCase {
 	private static $testPageRestrictionSource;
 	/** @var array */
 	private static $testPageRestrictionCascade;
+	/** @var array */
+	private static $testFileRestrictionSource;
+	/** @var array */
+	private static $testFileTarget;

 	protected function setUp(): void {
 		parent::setUp();
@ -65,6 +69,12 @@ class RestrictionStoreTest extends MediaWikiIntegrationTestCase {
 			$this->insertPage( 'RestrictionStoreTest_1', '{{RestrictionStoreTestB}}' );

 		$this->updateRestrictions( self::$testPageRestrictionSource['title'], [ 'edit' => 'sysop' ] );
+
+		self::$testFileTarget = $this->insertPage( 'File:RestrictionStoreTest.jpg', 'test file' );
+		self::$testFileRestrictionSource =
+			$this->insertPage( 'RestrictionStoreTest_File', '[[File:RestrictionStoreTest.jpg]]' );
+
+		$this->updateRestrictions( self::$testFileRestrictionSource['title'], [ 'edit' => 'sysop' ], 1 );
 	}

 	private function newRestrictionStore( array $options = [] ) {
@ -100,22 +110,49 @@ class RestrictionStoreTest extends MediaWikiIntegrationTestCase {
 		$page = self::$testPageRestrictionCascade['title'];
 		$pageSource = self::$testPageRestrictionSource['title'];

-		[ $sources, $restrictions ] = $this->newRestrictionStore()
+		[ $sources, $restrictions, $tlSources, $ilSources ] = $this->newRestrictionStore()
 			->getCascadeProtectionSources( $page );
 		$this->assertCount( 1, $sources );
+		$this->assertCount( 1, $tlSources );
+		$this->assertCount( 0, $ilSources );
 		$this->assertTrue( $pageSource->isSamePageAs( $sources[$pageSource->getId()] ) );
 		$this->assertArrayEquals( [ 'edit' => [ 'sysop' ] ], $restrictions );

-		[ $sources, $restrictions ] = $this->newRestrictionStore()
+		[ $sources, $restrictions, $tlSources, $ilSources ] = $this->newRestrictionStore()
 			->getCascadeProtectionSources( $pageSource );
 		$this->assertCount( 0, $sources );
+		$this->assertCount( 0, $tlSources );
+		$this->assertCount( 0, $ilSources );
 		$this->assertCount( 0, $restrictions );
 	}

 	public function testGetCascadeProtectionSourcesSpecialPage() {
-		[ $sources, $restrictions ] = $this->newRestrictionStore()
+		[ $sources, $restrictions, $tlSources, $ilSources ] = $this->newRestrictionStore()
 			->getCascadeProtectionSources( SpecialPage::getTitleFor( 'Whatlinkshere' ) );
 		$this->assertCount( 0, $sources );
+		$this->assertCount( 0, $tlSources );
+		$this->assertCount( 0, $ilSources );
+		$this->assertCount( 0, $restrictions );
+	}
+
+	public function testGetCascadeProtectionSourcesFile() {
+		$page = self::$testFileTarget['title'];
+		$pageSource = self::$testFileRestrictionSource['title'];
+
+		[ $sources, $restrictions, $tlSources, $ilSources ] = $this->newRestrictionStore()
+			->getCascadeProtectionSources( $page );
+
+		$this->assertCount( 1, $sources );
+		$this->assertTrue( $pageSource->isSamePageAs( $sources[$pageSource->getId()] ) );
+		$this->assertArrayEquals( [ 'edit' => [ 'sysop' ] ], $restrictions );
+		$this->assertCount( 1, $ilSources );
+		$this->assertCount( 0, $tlSources );
+
+		[ $sources, $restrictions, $tlSources, $ilSources ] = $this->newRestrictionStore()
+			->getCascadeProtectionSources( $pageSource );
+		$this->assertCount( 0, $sources );
+		$this->assertCount( 0, $tlSources );
+		$this->assertCount( 0, $ilSources );
 		$this->assertCount( 0, $restrictions );
 	}

--- a/tests/phpunit/unit/includes/Permissions/RestrictionStoreTest.php
+++ b/tests/phpunit/unit/includes/Permissions/RestrictionStoreTest.php
@ -30,6 +30,7 @@ use Wikimedia\Rdbms\FakeResultWrapper;
 use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\ILoadBalancer;
 use Wikimedia\Rdbms\SelectQueryBuilder;
+use Wikimedia\Rdbms\UnionQueryBuilder;

 /**
 * @covers \MediaWiki\Permissions\RestrictionStore
@ -66,7 +67,7 @@ class RestrictionStoreTest extends MediaWikiUnitTestCase {
 		foreach ( $expectedCalls as $index => $calls ) {
 			$dbs[$index] = $this->createNoOpMock(
 				IDatabase::class,
-				array_merge( array_keys( $calls ), [ 'newSelectQueryBuilder', 'newDeleteQueryBuilder' ] )
+				array_merge( array_keys( $calls ), [ 'newSelectQueryBuilder', 'newDeleteQueryBuilder', 'newUnionQueryBuilder' ] )
 			);
 			foreach ( $calls as $method => $callback ) {
 				$count = 1;
@ -81,6 +82,11 @@ class RestrictionStoreTest extends MediaWikiUnitTestCase {
 				->willReturnCallback( static function () use ( $dbs, $index ) {
 					return new SelectQueryBuilder( $dbs[$index] );
 				} );
+			$dbs[$index]
+				->method( 'newUnionQueryBuilder' )
+				->willReturnCallback( static function () use ( $dbs, $index ) {
+					return new UnionQueryBuilder( $dbs[$index] );
+				} );
 			$dbs[$index]
 				->method( 'newDeleteQueryBuilder' )
 				->willReturnCallback( static function () use ( $dbs, $index ) {
@ -964,15 +970,61 @@ class RestrictionStoreTest extends MediaWikiUnitTestCase {
 			static function () {
 				return new FakeResultWrapper( [
 					(object)[ 'pr_page' => 1, 'page_namespace' => NS_MAIN, 'page_title' => 'test',
-						'pr_expiry' => 'infinity', 'pr_type' => 'edit', 'pr_level' => 'Sysop' ]
+						'pr_expiry' => 'infinity', 'pr_type' => 'edit', 'pr_level' => 'Sysop',
+						'type' => 'tl' ]
 				] );
-			}
+			},
+			'addQuotes' => [
+				static function () {
+					return 'noop';
+				},
+				2
+			]
 		] ] ] );

 		$page = PageIdentityValue::localIdentity( 1, NS_MAIN, 'X' );
-		[ $sources, $restrictions ] = $obj->getCascadeProtectionSources( $page );
+		[ $sources, $restrictions, $tlSources, $ilSources ] = $obj->getCascadeProtectionSources( $page );
 		$this->assertCount( 1, $sources );
 		$this->assertArrayHasKey( 'edit', $restrictions );
+		$this->assertCount( 0, $ilSources );
+		$this->assertCount( 1, $tlSources );
+	}
+
+	public function testGetCascadeProtectionSourcesFile() {
+		$obj = $this->newRestrictionStore( [ 'db' => [ DB_REPLICA => [
+			'addQuotes' => [
+				static function () {
+					return 'noop';
+				},
+				2
+			],
+			'selectSQLText' => [
+				static function () {
+					return 'noop';
+				},
+				2
+			],
+			'unionQueries' => static function () {
+				return 'noop';
+			},
+			'query' => static function () {
+				return new FakeResultWrapper( [
+					(object)[ 'pr_page' => 1, 'page_namespace' => NS_MAIN, 'page_title' => 'test1',
+						'pr_expiry' => 'infinity', 'pr_type' => 'edit', 'pr_level' => 'Sysop',
+						'type' => 'il' ],
+					(object)[ 'pr_page' => 2, 'page_namespace' => NS_MAIN, 'page_title' => 'test2',
+						'pr_expiry' => 'infinity', 'pr_type' => 'edit', 'pr_level' => 'Sysop',
+						'type' => 'tl' ]
+				] );
+			},
+		] ] ] );
+
+		$page = PageIdentityValue::localIdentity( 1, NS_FILE, 'Image.jpg' );
+		[ $sources, $restrictions, $tlSources, $ilSources ] = $obj->getCascadeProtectionSources( $page );
+		$this->assertCount( 2, $sources );
+		$this->assertCount( 1, $ilSources );
+		$this->assertCount( 1, $tlSources );
+		$this->assertArrayHasKey( 'edit', $restrictions );
 	}

 	public function testGetCascadeProtectionSourcesSpecialPage() {
@ -984,9 +1036,10 @@ class RestrictionStoreTest extends MediaWikiUnitTestCase {
 		] ] ] ] );

 		$page = $this->makeMockTitle( 'Whatlinkshere', [ 'namespace' => NS_SPECIAL ] );
-		[ $sources, $restrictions ] = $obj->getCascadeProtectionSources( $page );
+		[ $sources, $restrictions, $ilSources ] = $obj->getCascadeProtectionSources( $page );
 		$this->assertCount( 0, $sources );
 		$this->assertCount( 0, $restrictions );
+		$this->assertCount( 0, $ilSources );
 	}

 	public function testShouldNotFetchProtectionSettingsIfActionCannotBeRestricted(): void {