From: jenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Date: Wed, 22 May 2019 09:48:08 +0000 (+0000)
Subject: Merge "Fix logic in NamespaceInfo::getRestrictionLevels"
X-Git-Tag: 1.34.0-rc.0~1640
X-Git-Url: https://git.cyclocoop.org/%7B%24admin_url%7Dmembres/cotisations/rappels.php?a=commitdiff_plain;h=0bb36bf5aaa72e3af0d7e47177895184f315fbb6;hp=fd8602984efc1d86dc987491a1a96f737a681091;p=lhc%2Fweb%2Fwiklou.git

Merge "Fix logic in NamespaceInfo::getRestrictionLevels"
---

diff --git a/includes/title/NamespaceInfo.php b/includes/title/NamespaceInfo.php
index 7cfadc0144..cdb8f2554f 100644
--- a/includes/title/NamespaceInfo.php
+++ b/includes/title/NamespaceInfo.php
@@ -524,9 +524,15 @@ class NamespaceInfo {
 			return $levels;
 		}
 
-		// First, get the list of groups that can edit this namespace.
-		$namespaceGroups = [];
-		$combine = 'array_merge';
+		// $wgNamespaceProtection can require one or more rights to edit the namespace, which
+		// may be satisfied by membership in multiple groups each giving a subset of those rights.
+		// A restriction level is redundant if, for any one of the namespace rights, all groups
+		// giving that right also give the restriction level's right. Or, conversely, a
+		// restriction level is not redundant if, for every namespace right, there's at least one
+		// group giving that right without the restriction level's right.
+		//
+		// First, for each right, get a list of groups with that right.
+		$namespaceRightGroups = [];
 		foreach ( (array)$this->options->get( 'NamespaceProtection' )[$index] as $right ) {
 			if ( $right == 'sysop' ) {
 				$right = 'editprotected'; // BC
@@ -535,15 +541,11 @@ class NamespaceInfo {
 				$right = 'editsemiprotected'; // BC
 			}
 			if ( $right != '' ) {
-				$namespaceGroups = call_user_func( $combine, $namespaceGroups,
-					User::getGroupsWithPermission( $right ) );
-				$combine = 'array_intersect';
+				$namespaceRightGroups[$right] = User::getGroupsWithPermission( $right );
 			}
 		}
 
-		// Now, keep only those restriction levels where there is at least one
-		// group that can edit the namespace but would be blocked by the
-		// restriction.
+		// Now, go through the protection levels one by one.
 		$usableLevels = [ '' ];
 		foreach ( $this->options->get( 'RestrictionLevels' ) as $level ) {
 			$right = $level;
@@ -553,9 +555,19 @@ class NamespaceInfo {
 			if ( $right == 'autoconfirmed' ) {
 				$right = 'editsemiprotected'; // BC
 			}
-			if ( $right != '' && ( !$user || $user->isAllowed( $right ) ) &&
-				array_diff( $namespaceGroups, User::getGroupsWithPermission( $right ) )
+
+			if ( $right != '' &&
+				!isset( $namespaceRightGroups[$right] ) &&
+				( !$user || $user->isAllowed( $right ) )
 			) {
+				// Do any of the namespace rights imply the restriction right? (see explanation above)
+				foreach ( $namespaceRightGroups as $groups ) {
+					if ( !array_diff( $groups, User::getGroupsWithPermission( $right ) ) ) {
+						// Yes, this one does.
+						continue 2;
+					}
+				}
+				// No, keep the restriction level
 				$usableLevels[] = $level;
 			}
 		}
diff --git a/tests/phpunit/includes/title/NamespaceInfoTest.php b/tests/phpunit/includes/title/NamespaceInfoTest.php
index 4e376e8051..b1262a33a9 100644
--- a/tests/phpunit/includes/title/NamespaceInfoTest.php
+++ b/tests/phpunit/includes/title/NamespaceInfoTest.php
@@ -1300,11 +1300,7 @@ class NamespaceInfoTest extends MediaWikiTestCase {
 			'No namespace restriction' => [ [ '', 'autoconfirmed', 'sysop' ], NS_TALK ],
 			'Restricted to autoconfirmed' => [ [ '', 'sysop' ], NS_MAIN ],
 			'Restricted to sysop' => [ [ '' ], NS_USER ],
-			// @todo Bug -- 'sysop' protection should be allowed in this case. Someone who's
-			// autoconfirmed and also privileged can edit this namespace, and would be blocked by
-			// the sysop protection.
-			'Restricted to someone in two groups' => [ [ '' ], 101 ],
-
+			'Restricted to someone in two groups' => [ [ '', 'sysop' ], 101 ],
 			'No special permissions' => [ [ '' ], NS_TALK, $this->getMockUser() ],
 			'autoconfirmed' => [
 				[ '', 'autoconfirmed' ],