wikimedia/mediawiki-extensions-AbuseFilter
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/AbuseFilter.git synced 2024-11-11 16:28:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add test traits for uploads and account creation

Browse source 
Ideally, this might live in MediaWikiIntegrationTestCase. For the
createaccount one, AuthManager should also provide a method to log the
creation, because currently we are forced to copypaste that code here.

 - Add the missing tests for 'upload' in RCVariableGenerator, and adjust
the existing ones (delete file afterwards, more tablesUsed, use the
right extension).

 - Exclude from the coverage report a couple of lines which should
theoretically be unreachable. Escalate logging to WARN level, where it's
more likely to be spotted.

 - Remove an unused method (RCVariableGenerator::newFromID). This denies
   the need to maintain and cover it. We also don't want this generator
   to act as a factory.

Overall, this change brings the coverage for RCVariableGenerator to 100%

Bug: T201193
Change-Id: I425c3d9f6800f74eb6e4eda483b90cfb3bbbcb51
This commit is contained in:
Daimona Eaytoy 2020-09-16 11:15:38 +02:00
parent 2e13d58c74
commit 6c8a29698b
6 changed files with 167 additions and 98 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
extension.json
View file

@ -189,7 +189,9 @@
},
"TestAutoloadClasses": {
"AbuseFilterConsequencesTest": "tests/phpunit/AbuseFilterConsequencesTest.php",
"AbuseFilterParserTestCase": "tests/phpunit/unit/AbuseFilterParserTestCase.php"
"AbuseFilterParserTestCase": "tests/phpunit/unit/AbuseFilterParserTestCase.php",
"AbuseFilterUploadTestTrait": "tests/phpunit/AbuseFilterUploadTestTrait.php",
"AbuseFilterCreateAccountTestTrait": "tests/phpunit/AbuseFilterCreateAccountTestTrait.php"
},
"ResourceModules": {
"ext.abuseFilter": {

31
includes/VariableGenerator/RCVariableGenerator.php
View file

@ -40,29 +40,6 @@ class RCVariableGenerator extends VariableGenerator {
$this->contextUser = $contextUser;
}
/**
* Get an instance for a given rc_id.
*
* @todo FIXME this method doesn't appear to have any uses
*
* @param int $id
* @param AbuseFilterVariableHolder $vars
* @param User $contextUser
* @return self|null
*/
public static function newFromId(
int $id,
AbuseFilterVariableHolder $vars,
User $contextUser
) : ?self {
$rc = RecentChange::newFromId( $id );
if ( !$rc ) {
return null;
}
return new self( $vars, $rc, $contextUser );
}
/**
* @return AbuseFilterVariableHolder|null
*/
@ -89,8 +66,10 @@ class RCVariableGenerator extends VariableGenerator {
$this->addEditVarsForRow();
} else {
// @todo Ensure this cannot happen, and throw if it does
// @codeCoverageIgnoreStart
wfLogWarning( 'Cannot understand the given recentchanges row!' );
return null;
// @codeCoverageIgnoreEnd
}
$this->addGenericVars();
@ -178,10 +157,12 @@ class RCVariableGenerator extends VariableGenerator {
$title, [ 'time' => $time, 'private' => $this->contextUser ]
);
if ( !$file ) {
// FixMe This shouldn't happen!
// @fixme Ensure this cannot happen!
// @codeCoverageIgnoreStart
$logger = LoggerFactory::getInstance( 'AbuseFilter' );
$logger->debug( "Cannot find file from RC row with title $title" );
$logger->warning( "Cannot find file from RC row with title $title" );
return $this;
// @codeCoverageIgnoreEnd
}
// This is the same as AbuseFilterHooks::filterUpload, but from a different source

62
tests/phpunit/AbuseFilterConsequencesTest.php
View file

@ -45,6 +45,9 @@ use PHPUnit\Framework\MockObject\MockObject;
* @covers AbuseFilterParser
*/
class AbuseFilterConsequencesTest extends MediaWikiTestCase {
use AbuseFilterCreateAccountTestTrait;
use AbuseFilterUploadTestTrait;
/**
* @var User The user performing actions
*/
@ -74,7 +77,9 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
'logging',
'change_tag',
'user',
'text'
'text',
'image',
'oldimage',
];
// phpcs:disable Generic.Files.LineLength
@ -370,8 +375,7 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
protected function tearDown() : void {
// Paranoia: ensure no fake timestamp leftover
MWTimestamp::setFakeTime( false );
// Clear any upload
$_FILES = [];
$this->clearUploads();
parent::tearDown();
}
@ -518,35 +522,6 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
return $status;
}
/**
* Upload a file. This is based on ApiUploadTestCase::fakeUploadFile
* @param string $fileName
* @param string $pageText
* @param string $summary
* @return Status
*/
private function doUpload( string $fileName, string $pageText, string $summary ) : Status {
$imgGen = new RandomImageGenerator();
// Use SVG, since the ImageGenerator doesn't need anything special to create it
$format = 'svg';
$mime = 'image/svg+xml';
$filePath = $imgGen->writeImages( 1, $format, $this->getNewTempDirectory() )[0];
clearstatcache();
$_FILES[ 'wpUploadFile' ] = [
'name' => $fileName,
'type' => $mime,
'tmp_name' => $filePath,
'error' => UPLOAD_ERR_OK,
'size' => filesize( $filePath ),
];
$request = new FauxRequest( [
'wpDestFile' => $fileName
] );
$ub = UploadBase::createFromRequest( $request );
$ub->verifyUpload();
return $ub->performUpload( $summary, $pageText, false, $this->user );
}
/**
* Executes an action to filter
*
@ -596,22 +571,11 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
);
break;
case 'createaccount':
$user = User::newFromName( $params['username'] );
// A creatable username must exist to be passed to $logEntry->setPerformer(),
// so create the account.
$user->addToDatabase();
$provider = new AbuseFilterPreAuthenticationProvider();
$status = $provider->testForAccountCreation( $user, $user, [] );
$logEntry = new ManualLogEntry( 'newusers', 'create' );
$logEntry->setPerformer( $user );
$logEntry->setTarget( $user->getUserPage() );
$logid = $logEntry->insert();
$logEntry->publish( $logid );
$status = $this->createAccount( $params['username'] );
break;
case 'upload':
$status = $this->doUpload(
[ $status, $this->clearPath ] = $this->doUpload(
$this->user,
$params['target'],
$params['newText'] ?? 'AbuseFilter test upload',
$params['summary'] ?? 'Test'
@ -653,7 +617,7 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
}
$logType = $actionParams['action'] === 'createaccount' ? 'newusers' : $actionParams['action'];
$logAction = $logType === 'newusers' ? 'create' : $logType;
$logAction = $logType === 'newusers' ? 'create2' : $logType;
$title = Title::newFromText( $actionParams['target'] );
$id = $this->db->selectField(
'logging',
@ -1019,7 +983,7 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
[ 5 ],
[
'action' => 'upload',
'target' => 'MyFile.jpg',
'target' => 'MyFile.svg',
],
[ 'tag' => [ 5 ] ]
],
@ -1027,7 +991,7 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
[ 22 ],
[
'action' => 'upload',
'target' => 'MyFile.jpg',
'target' => 'MyFile.svg',
'newText' => 'Block me please!',
'summary' => 'Asking to be blocked'
],

43
tests/phpunit/AbuseFilterCreateAccountTestTrait.php Normal file
View file

@ -0,0 +1,43 @@
<?php
/**
* This trait can be used to create accounts in integration tests.
* NOTE: The implementing classes MUST extend MediaWikiIntegrationTestCase
* @todo This might be moved to MediaWikiIntegrationTestCase
*/
trait AbuseFilterCreateAccountTestTrait {
/**
* @param string $accountName
* @param User|null $creator Defaults to the newly created user
* @param bool $autocreate
* @return StatusValue
*/
protected function createAccount(
string $accountName,
User $creator = null,
bool $autocreate = false
) : StatusValue {
$user = User::newFromName( $accountName );
// A creatable username must exist to be passed to $logEntry->setPerformer(),
// so create the account.
$user->addToDatabase();
$creator = $creator ?? $user;
$provider = new AbuseFilterPreAuthenticationProvider();
$status = $provider->testForAccountCreation( $user, $creator, [] );
// FIXME This is a bit hacky, but AuthManager doesn't expose any methods for logging
$subType = $autocreate ? 'autocreate' : 'create2';
$logEntry = new \ManualLogEntry( 'newusers', $subType );
$logEntry->setPerformer( $creator );
$logEntry->setTarget( Title::makeTitle( NS_USER, $accountName ) );
$logEntry->setComment( 'Fooobarcomment' );
$logEntry->setParameters( [
'4::userid' => $user->getId(),
] );
$logid = $logEntry->insert();
$logEntry->publish( $logid );
return $status;
}
}

67
tests/phpunit/AbuseFilterUploadTestTrait.php Normal file
View file

@ -0,0 +1,67 @@
<?php
use MediaWiki\MediaWikiServices;
/**
* This trait can be used to perform uploads in integration tests.
* NOTE: The implementing classes MUST extend MediaWikiIntegrationTestCase
* The tearDown method must clear $_FILES and the local file
* @todo This might be moved to MediaWikiIntegrationTestCase
*
* @method string getNewTempDirectory()
* @method setMwGlobals($pairs)
*/
trait AbuseFilterUploadTestTrait {
/**
* @var string|null The path represented by this variable will be cleared in tearDown
*/
protected $clearPath;
/**
* Clear any temporary uploads, should be called from tearDown
*/
protected function clearUploads() : void {
$_FILES = [];
if ( $this->clearPath ) {
$backend = MediaWikiServices::getInstance()->getRepoGroup()->getLocalRepo()->getBackend();
$backend->delete( [ 'src' => $this->clearPath ], [ 'force' => 1 ] );
$this->clearPath = null;
}
}
/**
* This is based on ApiUploadTestCase::fakeUploadFile
*
* @param User $user
* @param string $fileName
* @param string $pageText
* @param string $summary
* @return array [ Status, file path ]
*/
protected function doUpload( User $user, string $fileName, string $pageText, string $summary ) : array {
global $wgFileExtensions;
$this->setMwGlobals( [ 'wgFileExtensions' => array_merge( $wgFileExtensions, [ 'svg' ] ) ] );
$imgGen = new RandomImageGenerator();
// Use SVG, since the ImageGenerator doesn't need anything special to create it
$format = 'svg';
$mime = 'image/svg+xml';
$filePath = $imgGen->writeImages( 1, $format, $this->getNewTempDirectory() )[0];
clearstatcache();
$_FILES[ 'wpUploadFile' ] = [
'name' => $fileName,
'type' => $mime,
'tmp_name' => $filePath,
'error' => UPLOAD_ERR_OK,
'size' => filesize( $filePath ),
];
$request = new FauxRequest( [
'wpDestFile' => $fileName
] );
/** @var UploadFromFile $ub */
$ub = UploadBase::createFromRequest( $request );
$ub->verifyUpload();
$status = $ub->performUpload( $summary, $pageText, false, $user );
return [ $status, $ub->getLocalFile()->getPath() ];
}
}

58
tests/phpunit/AbuseFilterVariableGeneratorDBTest.php
View file

@ -12,12 +12,17 @@ use MediaWiki\MediaWikiServices;
* @group Database
*/
class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
use AbuseFilterCreateAccountTestTrait;
use AbuseFilterUploadTestTrait;
protected $tablesUsed = [
'page',
'text',
'page_restrictions',
'user',
'recentchanges',
'image',
'oldimage',
];
/**
@ -25,6 +30,7 @@ class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
*/
protected function tearDown() : void {
MWTimestamp::setFakeTime( false );
$this->clearUploads();
parent::tearDown();
}
@ -259,7 +265,8 @@ class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
$expectedValues = [
'user_name' => $user->getName(),
'action' => $action,
'summary' => $summary
'summary' => $summary,
'timestamp' => $timestamp
];
switch ( $type ) {
@ -269,13 +276,11 @@ class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
case 'edit':
$newText = 'Some new text for testing RC vars.';
$this->editPage( $title->getText(), $newText, $summary, $title->getNamespace(), $user );
$expectedValues += [
'page_id' => $page->getId(),
'page_namespace' => $title->getNamespace(),
'page_title' => $title->getText(),
'page_prefixedtitle' => $title->getPrefixedText(),
'timestamp' => $timestamp
'page_prefixedtitle' => $title->getPrefixedText()
];
break;
case 'move':
@ -293,8 +298,7 @@ class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
'moved_to_id' => $newID,
'moved_to_namespace' => $newTitle->getNamespace(),
'moved_to_title' => $newTitle->getText(),
'moved_to_prefixedtitle' => $newTitle->getPrefixedText(),
'timestamp' => $timestamp
'moved_to_prefixedtitle' => $newTitle->getPrefixedText()
];
break;
case 'delete':
@ -303,24 +307,12 @@ class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
'page_id' => $page->getId(),
'page_namespace' => $title->getNamespace(),
'page_title' => $title->getText(),
'page_prefixedtitle' => $title->getPrefixedText(),
'timestamp' => $timestamp
'page_prefixedtitle' => $title->getPrefixedText()
];
break;
case 'newusers':
// FIXME AuthManager doesn't expose the method for doing this. And mocking AuthManager
// and everything here seems overkill.
$accountName = 'AbuseFilter dummy user';
$subType = $action === 'createaccount' ? 'create2' : 'autocreate';
$logEntry = new \ManualLogEntry( 'newusers', $subType );
$logEntry->setPerformer( $user );
$logEntry->setTarget( Title::makeTitle( NS_USER, $accountName ) );
$logEntry->setComment( 'Fooobarcomment' );
$logEntry->setParameters( [
'4::userid' => $user->getId(),
] );
$logid = $logEntry->insert();
$logEntry->publish( $logid );
$this->createAccount( $accountName, $user, $action === 'autocreateaccount' );
$expectedValues = [
'action' => $action,
@ -330,9 +322,29 @@ class AbuseFilterVariableGeneratorDBTest extends MediaWikiIntegrationTestCase {
];
break;
case 'upload':
// TODO Same problem as AuthManager, but worse because here we'd really have to
// upload a file (addUploadVars immediately tries reading the file)
$this->markTestIncomplete( 'Implement "upload" case!' );
$fileName = 'My File.svg';
$destTitle = Title::makeTitle( NS_FILE, $fileName );
$page = WikiPage::factory( $destTitle );
[ $status, $this->clearPath ] = $this->doUpload( $user, $fileName, 'Some text', $summary );
if ( !$status->isGood() ) {
throw new LogicException( "Cannot upload file:\n$status" );
}
// Since the SVG is randomly generated, we need to read some properties live
$file = MediaWikiServices::getInstance()->getRepoGroup()->getLocalRepo()->newFile( $destTitle );
$expectedValues += [
'page_id' => $page->getId(),
'page_namespace' => $destTitle->getNamespace(),
'page_title' => $destTitle->getText(),
'page_prefixedtitle' => $destTitle->getPrefixedText(),
'file_sha1' => \Wikimedia\base_convert( $file->getSha1(), 36, 16, 40 ),
'file_size' => $file->getSize(),
'file_mime' => 'image/svg+xml',
'file_mediatype' => 'DRAWING',
'file_width' => $file->getWidth(),
'file_height' => $file->getHeight(),
'file_bits_per_channel' => $file->getBitDepth(),
];
break;
default:
throw new LogicException( "Type $type not recognized!" );