Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

BlockUser: Restore blocking autoblocked IP addresses

Browse source 
This seems to be a regression from 166ed5c.
DatabaseBlock::newFromTarget may return an autoblock for an IP
unless there is an existing manual block. If so, ignore it
because it is allowed to have an IP address both manually
blocked and autoblocked (see ipb_address_unique index).

Also add an integration test case.

Bug: T287798
Change-Id: I7c9a66ba0ffe759f43f4d0821c30fb94649d3dea
This commit is contained in:
Matěj Suchánek 2021-08-01 11:05:36 +02:00
parent b9fe32197f
commit 1902efbcc7
2 changed files with 61 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
includes/block/BlockUser.php
View file

@ -555,6 +555,18 @@ class BlockUser {
// xxx: there is an identical call at the beginning of ::placeBlock
$priorBlock = DatabaseBlock::newFromTarget( $this->target, null, /*fromPrimary=*/true );
// T287798: we are blocking an IP that is currently autoblocked
// we can ignore the block because ipb_address_unique allows the IP address
// be both manually blocked and autoblocked
// this will work as long as DatabaseBlock::newLoad prefers manual IP blocks
// over autoblocks
if ( $priorBlock !== null
&& $priorBlock->getType() === AbstractBlock::TYPE_AUTO
&& $this->targetType === AbstractBlock::TYPE_IP
) {
$priorBlock = null;
}
$isReblock = false;
if ( $priorBlock !== null ) {
// Reblock only if the caller wants so

49
tests/phpunit/integration/includes/block/BlockUserTest.php
View file

@ -211,4 +211,53 @@ class BlockUserTest extends MediaWikiIntegrationTestCase {
$this->assertSame( $priorBlock->getId(), $hookPriorBlock->getId() );
}
/**
* @covers MediaWiki\Block\BlockUser::placeBlockInternal
*/
public function testIPBlockAllowedAutoblockPreserved() {
$blockStatus = $this->blockUserFactory->newBlockUser(
$this->user,
$this->mockAnonUltimateAuthority(),
'infinity',
'test block with autoblocking',
[ 'isAutoblocking' => true ]
)->placeBlockUnsafe();
$this->assertTrue( $blockStatus->isOK() );
$block = $blockStatus->getValue();
$target = '1.2.3.4';
$autoBlockId = $block->doAutoblock( $target );
$this->assertNotFalse( $autoBlockId );
$hookPriorBlock = false;
$this->setTemporaryHook(
'BlockIpComplete',
static function ( $block, $legacyUser, $priorBlock )
use ( &$hookPriorBlock )
{
$hookPriorBlock = $priorBlock;
}
);
$IPBlockStatus = $this->blockUserFactory->newBlockUser(
$target,
$this->mockAnonUltimateAuthority(),
'infinity',
'test IP block'
)->placeBlockUnsafe();
$this->assertTrue( $IPBlockStatus->isOK() );
$IPBlock = $IPBlockStatus->getValue();
$this->assertInstanceOf( DatabaseBlock::class, $IPBlock );
$this->assertNull( $hookPriorBlock );
$blockIds = array_map(
static function ( DatabaseBlock $block ) {
return $block->getId();
},
DatabaseBlock::newListFromTarget( $target, null, /*fromPrimary=*/true )
);
$this->assertContains( $autoBlockId, $blockIds );
$this->assertContains( $IPBlock->getId(), $blockIds );
}
}