diff --git a/includes/Api/QueryAbuseLog.php b/includes/Api/QueryAbuseLog.php index 6f384a1d7..925d514fc 100644 --- a/includes/Api/QueryAbuseLog.php +++ b/includes/Api/QueryAbuseLog.php @@ -370,14 +370,16 @@ class QueryAbuseLog extends ApiQueryBase { } if ( $shouldLog ) { - // phan check - user_name should always exist but guarantee it does and just in case + // user_name or accountname should always exist -- just in case // if it doesn't, unset the protected variables since they shouldn't be accessed if // the access isn't logged - if ( isset( $entry['details']['user_name'] ) ) { + if ( isset( $entry['details']['user_name'] ) || + isset( $entry['details']['accountname'] ) + ) { $logger = $this->abuseLoggerFactory->getProtectedVarsAccessLogger(); $logger->logViewProtectedVariableValue( $performer->getUser(), - $entry['details']['user_name'] + $entry['details']['user_name'] ?? $entry['details']['accountname'] ); } else { foreach ( $usedProtectedVars as $protectedVariable ) { diff --git a/includes/Special/SpecialAbuseLog.php b/includes/Special/SpecialAbuseLog.php index 3b4acb316..508f51798 100644 --- a/includes/Special/SpecialAbuseLog.php +++ b/includes/Special/SpecialAbuseLog.php @@ -839,7 +839,7 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage { $logger = $this->abuseLoggerFactory->getProtectedVarsAccessLogger(); $logger->logViewProtectedVariableValue( $userAuthority->getUser(), - $varsArray['user_name'] + $varsArray['user_name'] ?? $varsArray['accountname'] ); } diff --git a/includes/View/AbuseFilterViewExamine.php b/includes/View/AbuseFilterViewExamine.php index 300e6c9d1..8d1b5c62f 100644 --- a/includes/View/AbuseFilterViewExamine.php +++ b/includes/View/AbuseFilterViewExamine.php @@ -341,7 +341,7 @@ class AbuseFilterViewExamine extends AbuseFilterView { $logger = $this->abuseLoggerFactory->getProtectedVarsAccessLogger(); $logger->logViewProtectedVariableValue( $userAuthority->getUser(), - $varsArray['user_name'] + $varsArray['user_name'] ?? $varsArray['accountname'] ); } diff --git a/tests/phpunit/integration/Api/QueryAbuseLogTest.php b/tests/phpunit/integration/Api/QueryAbuseLogTest.php index 5598d4352..ec6d33bd2 100644 --- a/tests/phpunit/integration/Api/QueryAbuseLogTest.php +++ b/tests/phpunit/integration/Api/QueryAbuseLogTest.php @@ -32,7 +32,7 @@ class QueryAbuseLogTest extends ApiTestCase { $this->addToAssertionCount( 1 ); } - public function testProtectedVariableValueAcces() { + public function testProtectedVariableValueAccess() { // Add filter to query for $filter = [ 'id' => '1', @@ -54,14 +54,24 @@ class QueryAbuseLogTest extends ApiTestCase { $this->createFilter( $filter ); // Insert a hit on the filter - AbuseFilterServices::getAbuseLoggerFactory()->newLogger( + $abuseFilterLoggerFactory = AbuseFilterServices::getAbuseLoggerFactory(); + $abuseFilterLoggerFactory->newLogger( $this->getExistingTestPage()->getTitle(), $this->getTestUser()->getUser(), VariableHolder::newFromArray( [ 'action' => 'edit', 'user_unnamed_ip' => '1.2.3.4', 'user_name' => 'User1', - ] ) + ] ) + )->addLogEntries( [ 1 => [ 'warn' ] ] ); + $abuseFilterLoggerFactory->newLogger( + $this->getExistingTestPage()->getTitle(), + $this->getTestUser()->getUser(), + VariableHolder::newFromArray( [ + 'action' => 'autocreateaccount', + 'user_unnamed_ip' => '1.2.3.4', + 'accountname' => 'User1', + ] ) )->addLogEntries( [ 1 => [ 'warn' ] ] ); // Update afl_ip to a known value that can be used when it's reconstructed in the variable holder @@ -95,10 +105,13 @@ class QueryAbuseLogTest extends ApiTestCase { $result = $this->doApiRequest( [ 'action' => 'query', 'list' => 'abuselog', - 'aflprop' => 'details' + 'aflprop' => 'details', + 'afldir' => 'older', ], null, null, $authorityCanViewProtectedVar ); $result = $result[0]['query']['abuselog']; - $this->assertSame( '', $result[0]['details']['user_unnamed_ip'] ); + foreach ( $result as $row ) { + $this->assertSame( '', $row['details']['user_unnamed_ip'], 'IP is redacted' ); + } // Enable the preference for the user $userOptions = new StaticUserOptionsLookup( @@ -119,12 +132,20 @@ class QueryAbuseLogTest extends ApiTestCase { $result = $this->doApiRequest( [ 'action' => 'query', 'list' => 'abuselog', - 'aflprop' => 'details' + 'aflprop' => 'details', + 'afldir' => 'older', ], null, null, $authorityCanViewProtectedVar ); $result = $result[0]['query']['abuselog']; - $this->assertSame( '1.2.3.4', $result[0]['details']['user_unnamed_ip'] ); - - $this->resetServices(); + foreach ( $result as $row ) { + $this->assertSame( '1.2.3.4', $row['details']['user_unnamed_ip'] ); + if ( isset( $row['details']['accountname'] ) ) { + $this->assertSame( 'User1', $row['details']['accountname'] ); + $this->assertArrayNotHasKey( 'user_name', $row['details'] ); + } else { + $this->assertSame( 'User1', $row['details']['user_name'] ); + $this->assertArrayNotHasKey( 'accountname', $row['details'] ); + } + } } /**