Merge "Only return filters visible to user in search"

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 );
+	}
+
 }