Add missing permission check to canSeeLogDetailsForFilter

`canSeeLogDetails` should also be checked when a filter is protected, as
it is the base right for being able to see abuselog entries. With this
in mind, check that immediately at the beginning of the method, instead
of repeating calls. Also merge the conditionals, and return early when a
permission check fails.

Move a test up so that it comes immediately after its data provider, and
add test cases for a few combinations of rights.

Change-Id: Ic3cf58f43803bef8bf2d65566434baff145b3fd5
This commit is contained in:
Daimona Eaytoy 2024-07-02 22:43:09 +02:00
parent 71b1c0d0e6
commit 6ac574dada
2 changed files with 49 additions and 37 deletions

View file

@ -4,6 +4,7 @@ namespace MediaWiki\Extension\AbuseFilter;
use MediaWiki\Config\ServiceOptions;
use MediaWiki\Extension\AbuseFilter\Filter\AbstractFilter;
use MediaWiki\Extension\AbuseFilter\Filter\Flags;
use MediaWiki\Permissions\Authority;
/**
@ -168,27 +169,21 @@ class AbuseFilterPermissionManager {
* @return bool
*/
public function canSeeLogDetailsForFilter( Authority $performer, int $privacyLevel ): bool {
if ( $privacyLevel ) {
$res = true;
if ( FilterUtils::isHidden( $privacyLevel ) ) {
if (
!$this->canSeeLogDetails( $performer ) ||
!$this->canViewPrivateFiltersLogs( $performer )
) {
$res = false;
}
}
if ( FilterUtils::isProtected( $privacyLevel ) ) {
if ( !$this->canViewProtectedVariables( $performer ) ) {
$res = false;
}
}
return $res;
if ( !$this->canSeeLogDetails( $performer ) ) {
return false;
}
return $this->canSeeLogDetails( $performer );
if ( $privacyLevel === Flags::FILTER_PUBLIC ) {
return true;
}
if ( FilterUtils::isHidden( $privacyLevel ) && !$this->canViewPrivateFiltersLogs( $performer ) ) {
return false;
}
if ( FilterUtils::isProtected( $privacyLevel ) && !$this->canViewProtectedVariables( $performer ) ) {
return false;
}
return true;
}
/**

View file

@ -174,22 +174,53 @@ class AbuseFilterPermissionManagerTest extends MediaWikiUnitTestCase {
$details = [ 0 => 'abusefilter-log-detail' ];
$private = [ 1 => 'abusefilter-log-private' ];
$protected = [ 2 => 'abusefilter-access-protected-vars' ];
yield 'filter hidden, not privileged' => [ Flags::FILTER_HIDDEN, [], false ];
yield 'filter hidden, details only' => [ Flags::FILTER_HIDDEN, $details, false ];
yield 'filter hidden, private logs only' => [ Flags::FILTER_HIDDEN, $private, false ];
yield 'filter hidden, details and private logs' => [ Flags::FILTER_HIDDEN, $details + $private, true ];
yield 'filter protected, not privileged' => [ Flags::FILTER_USES_PROTECTED_VARS, [], false ];
yield 'filter protected, privileged' => [ Flags::FILTER_USES_PROTECTED_VARS, $protected, true ];
yield 'filter protected, details only' => [ Flags::FILTER_USES_PROTECTED_VARS, $details, false ];
yield 'filter protected, protected logs only' => [ Flags::FILTER_USES_PROTECTED_VARS, $protected, false ];
yield 'filter protected, privileged' => [ Flags::FILTER_USES_PROTECTED_VARS, $details + $protected, true ];
$hiddenProtected = Flags::FILTER_HIDDEN | Flags::FILTER_USES_PROTECTED_VARS;
yield 'filter hidden and protected, not privileged' => [ $hiddenProtected, [], false ];
yield 'filter hidden and protected, details only' => [ $hiddenProtected, $details, false ];
yield 'filter hidden and protected, private only' => [ $hiddenProtected, $private, false ];
yield 'filter hidden and protected, protected only' => [ $hiddenProtected, $protected, false ];
yield 'filter hidden and protected, details and private only' => [
Flags::FILTER_HIDDEN | Flags::FILTER_USES_PROTECTED_VARS, $details + $private, false
$hiddenProtected, $details + $private, false
];
yield 'filter hidden and protected, protected only' => [
Flags::FILTER_HIDDEN | Flags::FILTER_USES_PROTECTED_VARS, $protected, false
yield 'filter hidden and protected, details and protected only' => [
$hiddenProtected, $details + $protected, false
];
yield 'filter hidden and protected, private and protected only' => [
$hiddenProtected, $private + $protected, false
];
yield 'filter hidden and protected, privileged' => [
$hiddenProtected, $details + $private + $protected, true
];
yield 'filter visible, not privileged' => [ Flags::FILTER_PUBLIC, [], false ];
yield 'filter visible, privileged' => [ Flags::FILTER_PUBLIC, $details, true ];
}
/**
* @param int $privacyLevel
* @param array $rights
* @param bool $expected
* @dataProvider provideCanSeeLogDetailsForFilter
*/
public function testCanSeeLogDetailsForFilter( int $privacyLevel, array $rights, bool $expected ) {
$performer = $this->mockRegisteredAuthorityWithPermissions( $rights );
$this->assertSame(
$expected,
$this->getPermMan()->canSeeLogDetailsForFilter( $performer, $privacyLevel )
);
}
public function provideCanViewProtectedVariables(): Generator {
$block = $this->createMock( DatabaseBlock::class );
$block->method( 'isSiteWide' )->willReturn( true );
@ -261,20 +292,6 @@ class AbuseFilterPermissionManagerTest extends MediaWikiUnitTestCase {
);
}
/**
* @param int $privacyLevel
* @param array $rights
* @param bool $expected
* @dataProvider provideCanSeeLogDetailsForFilter
*/
public function testCanSeeLogDetailsForFilter( int $privacyLevel, array $rights, bool $expected ) {
$performer = $this->mockRegisteredAuthorityWithPermissions( $rights );
$this->assertSame(
$expected,
$this->getPermMan()->canSeeLogDetailsForFilter( $performer, $privacyLevel )
);
}
public static function provideSimpleCases(): array {
return [
'not allowed' => [ false ],