From a5dbd74017e7e0633c43c570993785886bc4456d Mon Sep 17 00:00:00 2001
From: STran <stran@wikimedia.org>
Date: Thu, 5 Dec 2024 02:10:21 -0800
Subject: [PATCH] Allow all users to see protected filters on
 Special:AbuseFilter

Why:
T367390 hid protected filters from users who didn't have the right to see them
because users with access to private filters also had access to search against
filters and were inadvertantly allowed to search against protected filters regardless
of whether or not they had the right to see them.

Now, in order to be consistent with how AbuseFilter displays private filters, reveal
protected filters to all users but continue to restrict access to their details.

What:
- Only hide protected filters when a user runs a search and doesn't have permission
  to access them.

Bug: T381470
Change-Id: I2acb7e066885f6da18b29876c21c5d7d199b9886
---
 includes/View/AbuseFilterViewList.php             | 15 ++++++++++-----
 .../Special/SpecialAbuseFilterTest.php            | 14 +++++++++++++-
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/includes/View/AbuseFilterViewList.php b/includes/View/AbuseFilterViewList.php
index bd2f3a098..cb899eacd 100644
--- a/includes/View/AbuseFilterViewList.php
+++ b/includes/View/AbuseFilterViewList.php
@@ -162,11 +162,6 @@ class AbuseFilterViewList extends AbuseFilterView {
 			$conds['af_global'] = 1;
 		}
 
-		if ( !$this->afPermManager->canViewProtectedVariables( $performer ) ) {
-			$dbr = $this->dbProvider->getReplicaDatabase();
-			$conds[] = $dbr->bitAnd( 'af_hidden', Flags::FILTER_USES_PROTECTED_VARS ) . ' = 0';
-		}
-
 		if ( $searchmode !== null ) {
 			// Check the search pattern. Filtering the results is done in AbuseFilterPager
 			$error = null;
@@ -190,6 +185,16 @@ class AbuseFilterViewList extends AbuseFilterView {
 				$conds = [ 'af_deleted' => 0 ];
 				$searchmode = $querypattern = null;
 			}
+
+			// Viewers with the right to view private filters have access to the search
+			// function, which can query against protected filters and potentially expose PII.
+			// Remove protected filters from the query if the user doesn't have the right to search
+			// against them. This allows protected filters to be visible in the general list of
+			// filters at all other times.
+			if ( !$this->afPermManager->canViewProtectedVariables( $performer ) ) {
+				$dbr = $this->dbProvider->getReplicaDatabase();
+				$conds[] = $dbr->bitAnd( 'af_hidden', Flags::FILTER_USES_PROTECTED_VARS ) . ' = 0';
+			}
 		}
 
 		$this->showList(
diff --git a/tests/phpunit/integration/Special/SpecialAbuseFilterTest.php b/tests/phpunit/integration/Special/SpecialAbuseFilterTest.php
index d87b6f49f..2d36e587d 100644
--- a/tests/phpunit/integration/Special/SpecialAbuseFilterTest.php
+++ b/tests/phpunit/integration/Special/SpecialAbuseFilterTest.php
@@ -386,6 +386,18 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
 	}
 
 	public function testViewListProtectedVarsFilterVisibility() {
+		// Ensure that even if the user cannot view the details of a protected filter
+		// they can still see the filter in the filter list
+		[ $html, ] = $this->executeSpecialPage(
+			'',
+			new FauxRequest(),
+			null,
+			$this->authorityCannotUseProtectedVar
+		);
+		$this->assertStringContainsString( 'abusefilter-protected', $html );
+	}
+
+	public function testViewListWithSearchQueryProtectedVarsFilterVisibility() {
 		// Stub out a page with query results for a filter that uses protected variables
 		// &sort=af_id&limit=50&asc=&desc=1&deletedfilters=hide&querypattern=user_unnamed_ip&searchoption=LIKE
 		$requestWithProtectedVar = new FauxRequest( [
@@ -400,7 +412,7 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
 			'furtheroptions' => []
 		] );
 
-		// Assert that the user who cannot see protected variables sees no filters
+		// Assert that the user who cannot see protected variables sees no filters when searching
 		[ $html, ] = $this->executeSpecialPage(
 			'',
 			$requestWithProtectedVar,