Only return filters visible to user in search

Search is restricted to users with the right to view private variables
but not necessarily the right to view protected variables. Users who
don't have the right to view protected variables shouldn't be able to
search against protected variables, as this might leak the PII.

- Filter out filters using protected variables in search results
  if the user doesn't have the right to view protected variables

Bug: T367390
Change-Id: I7412112c9cc676f29d706b116b779bc17183a952

includes/Special/SpecialAbuseFilter.php

@@ -80,6 +80,7 @@ class SpecialAbuseFilter extends AbuseFilterSpecialPage {
 		],
 		AbuseFilterViewList::class => [
 			'LinkBatchFactory',
+			'ConnectionProvider',
 			AbuseFilterPermissionManager::SERVICE_NAME,
 			FilterProfiler::SERVICE_NAME,
 			SpecsFormatter::SERVICE_NAME,

includes/View/AbuseFilterViewList.php

@@ -16,6 +16,7 @@ use MediaWiki\HTMLForm\HTMLForm;
 use MediaWiki\Linker\LinkRenderer;
 use OOUI;
 use StringUtils;
+use Wikimedia\Rdbms\IConnectionProvider;
 
 /**
  * The default view used in Special:AbuseFilter
@@ -25,6 +26,9 @@ class AbuseFilterViewList extends AbuseFilterView {
 	/** @var LinkBatchFactory */
 	private $linkBatchFactory;
 
+	/** @var IConnectionProvider */
+	private $dbProvider;
+
 	/** @var FilterProfiler */
 	private $filterProfiler;
 
@@ -36,6 +40,7 @@ class AbuseFilterViewList extends AbuseFilterView {
 
 	/**
 	 * @param LinkBatchFactory $linkBatchFactory
+	 * @param IConnectionProvider $dbProvider
 	 * @param AbuseFilterPermissionManager $afPermManager
 	 * @param FilterProfiler $filterProfiler
 	 * @param SpecsFormatter $specsFormatter
@@ -47,6 +52,7 @@ class AbuseFilterViewList extends AbuseFilterView {
 	 */
 	public function __construct(
 		LinkBatchFactory $linkBatchFactory,
+		IConnectionProvider $dbProvider,
 		AbuseFilterPermissionManager $afPermManager,
 		FilterProfiler $filterProfiler,
 		SpecsFormatter $specsFormatter,
@@ -58,6 +64,7 @@ class AbuseFilterViewList extends AbuseFilterView {
 	) {
 		parent::__construct( $afPermManager, $context, $linkRenderer, $basePageName, $params );
 		$this->linkBatchFactory = $linkBatchFactory;
+		$this->dbProvider = $dbProvider;
 		$this->filterProfiler = $filterProfiler;
 		$this->specsFormatter = $specsFormatter;
 		$this->specsFormatter->setMessageLocalizer( $context );
@@ -155,6 +162,11 @@ 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;

tests/phpunit/integration/Special/SpecialAbuseFilterTest.php

@@ -3,6 +3,12 @@
 namespace MediaWiki\Extension\AbuseFilter\Tests\Integration\Special;
 
 use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
+use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
+use MediaWiki\Extension\AbuseFilter\Filter\Filter;
+use MediaWiki\Extension\AbuseFilter\Filter\Flags;
+use MediaWiki\Extension\AbuseFilter\Filter\LastEditInfo;
+use MediaWiki\Extension\AbuseFilter\Filter\MutableFilter;
+use MediaWiki\Extension\AbuseFilter\Filter\Specs;
 use MediaWiki\Extension\AbuseFilter\Special\SpecialAbuseFilter;
 use MediaWiki\Extension\AbuseFilter\View\AbuseFilterViewDiff;
 use MediaWiki\Extension\AbuseFilter\View\AbuseFilterViewEdit;
@@ -14,7 +20,11 @@ use MediaWiki\Extension\AbuseFilter\View\AbuseFilterViewRevert;
 use MediaWiki\Extension\AbuseFilter\View\AbuseFilterViewTestBatch;
 use MediaWiki\Extension\AbuseFilter\View\AbuseFilterViewTools;
 use MediaWiki\MediaWikiServices;
+use MediaWiki\Permissions\SimpleAuthority;
+use MediaWiki\Request\FauxRequest;
+use MediaWiki\Tests\Unit\Permissions\MockAuthorityTrait;
 use SpecialPageTestBase;
+use Wikimedia\TestingAccessWrapper;
 
 /**
  * @covers \MediaWiki\Extension\AbuseFilter\Special\SpecialAbuseFilter
@@ -29,8 +39,10 @@ use SpecialPageTestBase;
  * @covers \MediaWiki\Extension\AbuseFilter\View\AbuseFilterViewRevert
  * @covers \MediaWiki\Extension\AbuseFilter\View\AbuseFilterViewTestBatch
  * @covers \MediaWiki\Extension\AbuseFilter\View\AbuseFilterViewTools
+ * @group Database
  */
 class SpecialAbuseFilterTest extends SpecialPageTestBase {
+	use MockAuthorityTrait;
 
 	/**
 	 * @dataProvider provideInstantiateView
@@ -70,4 +82,131 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
 		return $sp;
 	}
 
+	/**
+	 * Adapted from FilterStoreTest->getFilterFromSpecs()
+	 *
+	 * @param array $filterSpecs
+	 * @param array $actions
+	 * @return Filter
+	 */
+	private function getFilterFromSpecs( array $filterSpecs, array $actions = [] ): Filter {
+		return new Filter(
+			new Specs(
+				$filterSpecs['rules'],
+				$filterSpecs['comments'],
+				$filterSpecs['name'],
+				array_keys( $filterSpecs['actions'] ),
+				$filterSpecs['group']
+			),
+			new Flags(
+				$filterSpecs['enabled'],
+				$filterSpecs['deleted'],
+				$filterSpecs['hidden'],
+				$filterSpecs['global']
+			),
+			$actions,
+			new LastEditInfo(
+				$filterSpecs['user'],
+				$filterSpecs['user_text'],
+				$filterSpecs['timestamp']
+			),
+			$filterSpecs['id'],
+			$filterSpecs['hit_count'],
+			$filterSpecs['throttled']
+		);
+	}
+
+	/**
+	 * Adapted from FilterStoreTest->createFilter()
+	 *
+	 * @param array $row
+	 */
+	private function createFilter( array $row ): void {
+		$row['timestamp'] = $this->getDb()->timestamp( $row['timestamp'] );
+		$filter = $this->getFilterFromSpecs( $row );
+		$oldFilter = MutableFilter::newDefault();
+		// Use some black magic to bypass checks
+		/** @var FilterStore $filterStore */
+		$filterStore = TestingAccessWrapper::newFromObject( AbuseFilterServices::getFilterStore() );
+		$row = $filterStore->filterToDatabaseRow( $filter, $oldFilter );
+		$row['af_actor'] = $this->getServiceContainer()->getActorNormalization()->acquireActorId(
+			$this->getTestUser()->getUserIdentity(),
+			$this->getDb()
+		);
+		$this->getDb()->newInsertQueryBuilder()
+			->insertInto( 'abuse_filter' )
+			->row( $row )
+			->caller( __METHOD__ )
+			->execute();
+	}
+
+	public function testProtectedVarsFilterVisibility() {
+		// Add filter to query for
+		$filter = [
+			'id' => '1',
+			'rules' => 'user_unnamed_ip = 1.2.3.4',
+			'name' => 'Filter with protected variables',
+			'hidden' => Flags::FILTER_USES_PROTECTED_VARS,
+			'user' => 0,
+			'user_text' => 'FilterTester',
+			'timestamp' => '20190826000000',
+			'enabled' => 1,
+			'comments' => '',
+			'hit_count' => 0,
+			'throttled' => 0,
+			'deleted' => 0,
+			'actions' => [],
+			'global' => 0,
+			'group' => 'default'
+		];
+		$this->createFilter( $filter );
+
+		// Create the user to query for filters
+		$user = $this->getTestSysop()->getUser();
+
+		// Create an authority who can see private filters but not protected variables
+		$authorityCannotViewProtectedVar = new SimpleAuthority(
+			$user,
+			[ 'abusefilter-log-private', 'abusefilter-view-private' ]
+		);
+
+		// Create an authority who can see private and protected variables
+		$authorityCanViewProtectedVar = new SimpleAuthority(
+			$user,
+			[ 'abusefilter-access-protected-vars', 'abusefilter-log-private', 'abusefilter-view-private' ]
+		);
+
+		// 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( [
+			'sort' => 'af_id',
+			'limit' => 50,
+			'asc' => '',
+			'desc' => 1,
+			'deletedfilters' => 'hide',
+			'querypattern' => 'user_unnamed_ip',
+			'searchoption' => 'LIKE',
+			'rulescope' => 'all',
+			'furtheroptions' => []
+		] );
+
+		// Assert that the user who cannot see protected variables sees no filters
+		[ $html, ] = $this->executeSpecialPage(
+			'',
+			$requestWithProtectedVar,
+			null,
+			$authorityCannotViewProtectedVar
+		);
+		$this->assertStringContainsString( 'table_pager_empty', $html );
+
+		// Assert that the user who can see protected variables sees the filter from the db
+		[ $html, ] = $this->executeSpecialPage(
+			'',
+			$requestWithProtectedVar,
+			null,
+			$authorityCanViewProtectedVar
+		);
+		$this->assertStringContainsString( '1.2.3.4', $html );
+	}
+
 }