From eb2dbfdef6c14dbb8756531df3618d394f3adc47 Mon Sep 17 00:00:00 2001
From: Aryeh Gregor <ayg@aryeh.name>
Date: Mon, 6 May 2019 10:27:02 +0300
Subject: [PATCH] Fix logic in NamespaceInfo::getRestrictionLevels

When $wgNamespaceProtection specifies multiple rights for a
namespace, the code was assuming all of those rights had to
come from one group. But MediaWiki allows for the rights to
come from multiple groups that each give a subset of the rights. Allow for that case.

Bug: T222598
Change-Id: I1e9aca6e521260f783bd881e7d095d62bc605dc6
---
 includes/title/NamespaceInfo.php              | 34 +++++++++++++------
 .../includes/title/NamespaceInfoTest.php      |  6 +---
 2 files changed, 24 insertions(+), 16 deletions(-)

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 556c640bd6..5275dcf0c5 100644
--- a/tests/phpunit/includes/title/NamespaceInfoTest.php
+++ b/tests/phpunit/includes/title/NamespaceInfoTest.php
@@ -1295,11 +1295,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' ],
-- 
2.20.1