From c9908da103e48129c0d99540266517e6787a5e6a Mon Sep 17 00:00:00 2001 From: Martin Urbanec Date: Wed, 23 Aug 2023 14:06:00 +0200 Subject: [PATCH] IP Masking: Expire temporary accounts in 1 year Why: Temporary accounts (introduced as part of IP Masking) are supposed to expire 1 year after their registration. Automatic account expiration can be done via a maintenance script, which would be periodically executed via cron / systemd. Make it possible for extensions to provide their own logic for generating a list of temporary accounts to invalidate. This is used in CentralAuth to base registration timestamp on the global registration timestamp. The default behavior is "temporary accounts do not expire", given the feature requires a maintenance script to run periodically, which will not be the case on third party instances. What: * Add `expireAfterDays` to $wgAutoCreateTempUser, controlling how many days temporary accounts have. * Add UserSelectQueryBuilder::whereRegisteredTimestamp(), filtering accounts based on user_registration. * Add ExpireTemporaryAccounts maintenance script, which is @stable to extend. Bug: T344695 Change-Id: If17bf84ee6620c8eb784b7d835682ad5e7afdfcc --- autoload.php | 1 + docs/config-schema.yaml | 4 + includes/MainConfigSchema.php | 6 +- includes/config-schema.php | 1 + includes/user/TempUser/RealTempUserConfig.php | 9 + includes/user/TempUser/TempUserConfig.php | 10 + includes/user/TempUser/TempUserCreator.php | 4 + includes/user/UserSelectQueryBuilder.php | 22 +++ maintenance/expireTemporaryAccounts.php | 176 ++++++++++++++++++ .../Permissions/PermissionManagerTest.php | 1 + .../includes/api/query/ApiQueryInfoTest.php | 1 + .../api/query/ApiQuerySiteinfoTest.php | 1 + .../includes/block/DatabaseBlockTest.php | 1 + .../includes/user/UserGroupManagerTest.php | 1 + tests/phpunit/includes/user/UserTest.php | 1 + .../includes/block/BlockUserTest.php | 1 + .../user/TempUser/RealTempUserConfigTest.php | 1 + .../user/TempUser/TempUserCreatorTest.php | 2 + .../includes/user/UserFactoryTest.php | 2 + tests/phpunit/mocks/DummyServicesTrait.php | 1 + 20 files changed, 245 insertions(+), 1 deletion(-) create mode 100644 maintenance/expireTemporaryAccounts.php diff --git a/autoload.php b/autoload.php index dd99f81f26e..5fbb9a10b39 100644 --- a/autoload.php +++ b/autoload.php @@ -443,6 +443,7 @@ $wgAutoloadLocalClasses = [ 'ExecutableFinder' => __DIR__ . '/includes/utils/ExecutableFinder.php', 'Exif' => __DIR__ . '/includes/media/Exif.php', 'ExifBitmapHandler' => __DIR__ . '/includes/media/ExifBitmapHandler.php', + 'ExpireTemporaryAccounts' => __DIR__ . '/maintenance/expireTemporaryAccounts.php', 'ExplodeIterator' => __DIR__ . '/includes/libs/ExplodeIterator.php', 'ExportProgressFilter' => __DIR__ . '/includes/export/ExportProgressFilter.php', 'ExportSites' => __DIR__ . '/maintenance/exportSites.php', diff --git a/docs/config-schema.yaml b/docs/config-schema.yaml index 3aaafee3544..51ee29ba102 100644 --- a/docs/config-schema.yaml +++ b/docs/config-schema.yaml @@ -4855,6 +4855,7 @@ config-schema: reservedPattern: { type: [string, 'null'], default: null } serialProvider: { type: object, default: { type: local } } serialMapping: { type: object, default: { type: plain-numeric } } + expireAfterDays: { type: [integer, 'null'], default: null } type: object description: |- Configuration for automatic creation of temporary accounts on page save. @@ -4901,6 +4902,9 @@ config-schema: be zero-based array indexes. - uppercase: (bool) With "filtered-radix", whether to use uppercase letters, default false. + - expireAfterDays: (int|null, default null) If set, how many days should the temporary + accounts expire? Require expireTemporaryAccounts.php to be periodically executed in + order to work. @since 1.39 default: null AutoblockExpiry: diff --git a/includes/MainConfigSchema.php b/includes/MainConfigSchema.php index a2f18995a5f..b9fba071a7b 100644 --- a/includes/MainConfigSchema.php +++ b/includes/MainConfigSchema.php @@ -7669,6 +7669,9 @@ class MainConfigSchema { * be zero-based array indexes. * - uppercase: (bool) With "filtered-radix", whether to use uppercase * letters, default false. + * - expireAfterDays: (int|null, default null) If set, how many days should the temporary + * accounts expire? Require expireTemporaryAccounts.php to be periodically executed in + * order to work. * * @since 1.39 */ @@ -7680,7 +7683,8 @@ class MainConfigSchema { 'matchPattern' => [ 'type' => 'string', 'default' => '*$1' ], 'reservedPattern' => [ 'type' => 'string|null', 'default' => null ], 'serialProvider' => [ 'type' => 'object', 'default' => [ 'type' => 'local' ] ], - 'serialMapping' => [ 'type' => 'object', 'default' => [ 'type' => 'plain-numeric' ] ] + 'serialMapping' => [ 'type' => 'object', 'default' => [ 'type' => 'plain-numeric' ] ], + 'expireAfterDays' => [ 'type' => 'int|null', 'default' => null ], ], 'type' => 'object', ]; diff --git a/includes/config-schema.php b/includes/config-schema.php index 1f9a4406fd2..eea07b1fbdb 100644 --- a/includes/config-schema.php +++ b/includes/config-schema.php @@ -1160,6 +1160,7 @@ return [ 'serialMapping' => [ 'type' => 'plain-numeric', ], + 'expireAfterDays' => null, ], 'AutoblockExpiry' => 86400, 'BlockAllowsUTEdit' => true, diff --git a/includes/user/TempUser/RealTempUserConfig.php b/includes/user/TempUser/RealTempUserConfig.php index d27ad6504ef..d0a0143d992 100644 --- a/includes/user/TempUser/RealTempUserConfig.php +++ b/includes/user/TempUser/RealTempUserConfig.php @@ -32,6 +32,9 @@ class RealTempUserConfig implements TempUserConfig { /** @var Pattern|null */ private $reservedPattern; + /** @var int|null */ + private $expireAfterDays; + /** * @param array $config See the documentation of $wgAutoCreateTempUser. * - enabled: bool @@ -41,6 +44,7 @@ class RealTempUserConfig implements TempUserConfig { * - reservedPattern: string, optional * - serialProvider: array * - serialMapping: array + * - expireAfterDays: int, optional */ public function __construct( $config ) { if ( $config['enabled'] ?? false ) { @@ -54,6 +58,7 @@ class RealTempUserConfig implements TempUserConfig { } $this->serialProviderConfig = $config['serialProvider']; $this->serialMappingConfig = $config['serialMapping']; + $this->expireAfterDays = $config['expireAfterDays']; } if ( isset( $config['reservedPattern'] ) ) { $this->reservedPattern = new Pattern( 'reservedPattern', $config['reservedPattern'] ); @@ -104,6 +109,10 @@ class RealTempUserConfig implements TempUserConfig { } } + public function getExpireAfterDays(): ?int { + return $this->expireAfterDays; + } + /** * @internal For TempUserCreator only * @return Pattern diff --git a/includes/user/TempUser/TempUserConfig.php b/includes/user/TempUser/TempUserConfig.php index 064a48ae2fe..bfa117ae5ef 100644 --- a/includes/user/TempUser/TempUserConfig.php +++ b/includes/user/TempUser/TempUserConfig.php @@ -74,4 +74,14 @@ interface TempUserConfig { * @return Pattern */ public function getMatchPattern(): Pattern; + + /** + * After how many days do temporary users expire? + * + * @note expireTemporaryAccounts.php maintenance script needs to be periodically executed for + * temp account expiry to work. + * @since 1.42 + * @return int|null Null if temp accounts should never expire + */ + public function getExpireAfterDays(): ?int; } diff --git a/includes/user/TempUser/TempUserCreator.php b/includes/user/TempUser/TempUserCreator.php index 093eb9ab8ce..98ebfee23a2 100644 --- a/includes/user/TempUser/TempUserCreator.php +++ b/includes/user/TempUser/TempUserCreator.php @@ -154,6 +154,10 @@ class TempUserCreator implements TempUserConfig { return $this->config->getMatchPattern(); } + public function getExpireAfterDays(): ?int { + return $this->config->getExpireAfterDays(); + } + /** * Acquire a new username and return it. Permanently reserve the ID in * the database. diff --git a/includes/user/UserSelectQueryBuilder.php b/includes/user/UserSelectQueryBuilder.php index 19a733e426a..a9679a00ef9 100644 --- a/includes/user/UserSelectQueryBuilder.php +++ b/includes/user/UserSelectQueryBuilder.php @@ -35,6 +35,8 @@ class UserSelectQueryBuilder extends SelectQueryBuilder { private $actorStore; private TempUserConfig $tempUserConfig; + private bool $userJoined = false; + /** * @internal * @param IReadableDatabase $db @@ -132,6 +134,26 @@ class UserSelectQueryBuilder extends SelectQueryBuilder { return $this->whereUserNamePrefix( $prefix ); } + /** + * Find registered users who registered + * + * @param string $timestamp + * @param bool $direction Direction flag (if true, user_registration must be before $timestamp) + * @since 1.42 + * @return UserSelectQueryBuilder + */ + public function whereRegisteredTimestamp( string $timestamp, bool $direction ): self { + if ( !$this->userJoined ) { + $this->join( 'user', null, [ "actor_user=user_id" ] ); + $this->userJoined = true; + } + + $this->conds( 'user_registration ' . + ( $direction ? '< ' : '> ' ) . + $this->db->addQuotes( $this->db->timestamp( $timestamp ) ) ); + return $this; + } + /** * Order results by name in $direction * diff --git a/maintenance/expireTemporaryAccounts.php b/maintenance/expireTemporaryAccounts.php new file mode 100644 index 00000000000..60fc95eae8b --- /dev/null +++ b/maintenance/expireTemporaryAccounts.php @@ -0,0 +1,176 @@ +addDescription( 'Expire temporary accounts that exist for more than N days' ); + $this->addOption( 'frequency', 'How frequently the script runs [days]', true, true ); + $this->addOption( 'verbose', 'Verbose logging output' ); + } + + /** + * Construct services the script needs to use + * + * @stable to override + */ + protected function initServices(): void { + $services = MediaWikiServices::getInstance(); + + $this->userIdentityLookup = $services->getUserIdentityLookup(); + $this->userFactory = $services->getUserFactory(); + $this->authManager = $services->getAuthManager(); + $this->tempUserConfig = $services->getTempUserConfig(); + } + + /** + * If --verbose is passed, log to output + * + * @param string $log + * @return void + */ + protected function verboseLog( string $log ) { + if ( $this->hasOption( 'verbose' ) ) { + $this->output( $log ); + } + } + + /** + * Return a SelectQueryBuilder that returns temp accounts to invalidate + * + * This method should return temporary accounts that registered before $registeredBeforeUnix. + * To avoid returning an ever-growing set of accounts, the method should skip users that were + * supposedly invalidated by a previous script run (script runs each $frequencyDays days). + * + * If you override this method, you probably also want to override + * queryBuilderToUserIdentities(). + * + * @stable to override + * @param int $registeredBeforeUnix Cutoff Unix timestamp + * @param int $frequencyDays Script runs each $frequencyDays days + * @return SelectQueryBuilder + */ + protected function getTempAccountsToExpireQueryBuilder( + int $registeredBeforeUnix, + int $frequencyDays + ): SelectQueryBuilder { + return $this->userIdentityLookup->newSelectQueryBuilder() + ->temp() + ->whereRegisteredTimestamp( wfTimestamp( + TS_MW, + $registeredBeforeUnix + ), true ) + ->whereRegisteredTimestamp( wfTimestamp( + TS_MW, + $registeredBeforeUnix - ExpirationAwareness::TTL_DAY * $frequencyDays + ), false ); + } + + /** + * Convert a SelectQueryBuilder into a list of user identities + * + * Default implementation expects $queryBuilder is an instance of UserSelectQueryBuilder. If + * you override getTempAccountsToExpireQueryBuilder() to work with a different query builder, + * this method should be overriden to properly convert the query builder into user identities. + * + * @throws LogicException if $queryBuilder is not UserSelectQueryBuilder + * @stable to override + * @param SelectQueryBuilder $queryBuilder + * @return Iterator + */ + protected function queryBuilderToUserIdentities( SelectQueryBuilder $queryBuilder ): Iterator { + if ( $queryBuilder instanceof UserSelectQueryBuilder ) { + return $queryBuilder->fetchUserIdentities(); + } + + throw new LogicException( + '$queryBuilder is not UserSelectQueryBuilder. Did you forget to override ' . + __METHOD__ . '?' + ); + } + + /** + * Expire a temporary account + * + * Default implementation calls AuthManager::revokeAccessForUser and + * SessionManager::invalidateSessionsForUser. + * + * @stable to override + * @param UserIdentity $tempAccountUserIdentity + */ + protected function expireTemporaryAccount( UserIdentity $tempAccountUserIdentity ): void { + $this->authManager->revokeAccessForUser( $tempAccountUserIdentity->getName() ); + SessionManager::singleton()->invalidateSessionsForUser( + $this->userFactory->newFromUserIdentity( $tempAccountUserIdentity ) + ); + } + + /** + * @inheritDoc + */ + public function execute() { + $this->initServices(); + + if ( !$this->tempUserConfig->isEnabled() ) { + $this->output( 'Temporary accounts are disabled' . PHP_EOL ); + return; + } + + $frequencyDays = (int)$this->getOption( 'frequency' ); + $expiryAfterDays = $this->tempUserConfig->getExpireAfterDays(); + if ( !$expiryAfterDays ) { + $this->output( 'Temporary account expiry is not enabled' . PHP_EOL ); + return; + } + $registeredBeforeUnix = (int)wfTimestamp( TS_UNIX ) - ExpirationAwareness::TTL_DAY * $expiryAfterDays; + + $tempAccounts = $this->queryBuilderToUserIdentities( $this->getTempAccountsToExpireQueryBuilder( + $registeredBeforeUnix, + $frequencyDays + )->caller( __METHOD__ ) ); + + $revokedUsers = 0; + foreach ( $tempAccounts as $tempAccountUserIdentity ) { + $this->expireTemporaryAccount( $tempAccountUserIdentity ); + + $this->verboseLog( + 'Revoking access for ' . $tempAccountUserIdentity->getName() . PHP_EOL + ); + $revokedUsers++; + } + + $this->output( "Revoked access for $revokedUsers temporary users." . PHP_EOL ); + } +} + +$maintClass = ExpireTemporaryAccounts::class; +require_once RUN_MAINTENANCE_IF_MAIN; diff --git a/tests/phpunit/includes/Permissions/PermissionManagerTest.php b/tests/phpunit/includes/Permissions/PermissionManagerTest.php index a8026ec46aa..506094d4afd 100644 --- a/tests/phpunit/includes/Permissions/PermissionManagerTest.php +++ b/tests/phpunit/includes/Permissions/PermissionManagerTest.php @@ -427,6 +427,7 @@ class PermissionManagerTest extends MediaWikiLangTestCase { $this->overrideConfigValues( [ MainConfigNames::AutoCreateTempUser => [ 'enabled' => true, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'serialProvider' => [ 'type' => 'local' ], 'serialMapping' => [ 'type' => 'plain-numeric' ], diff --git a/tests/phpunit/includes/api/query/ApiQueryInfoTest.php b/tests/phpunit/includes/api/query/ApiQueryInfoTest.php index f8cfe4dbedb..fb2caa5a5bc 100644 --- a/tests/phpunit/includes/api/query/ApiQueryInfoTest.php +++ b/tests/phpunit/includes/api/query/ApiQueryInfoTest.php @@ -137,6 +137,7 @@ class ApiQueryInfoTest extends ApiTestCase { $this->setGroupPermissions( '*', 'createaccount', true ); $this->overrideConfigValue( MainConfigNames::AutoCreateTempUser, [ 'enabled' => true, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'genPattern' => 'Unregistered $1', 'serialProvider' => [], diff --git a/tests/phpunit/includes/api/query/ApiQuerySiteinfoTest.php b/tests/phpunit/includes/api/query/ApiQuerySiteinfoTest.php index 3a7bfb6c094..22957abfd2b 100644 --- a/tests/phpunit/includes/api/query/ApiQuerySiteinfoTest.php +++ b/tests/phpunit/includes/api/query/ApiQuerySiteinfoTest.php @@ -377,6 +377,7 @@ class ApiQuerySiteinfoTest extends ApiTestCase { $config = [ 'enabled' => true, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'genPattern' => 'Unregistered $1', 'reservedPattern' => null, diff --git a/tests/phpunit/includes/block/DatabaseBlockTest.php b/tests/phpunit/includes/block/DatabaseBlockTest.php index 0dec481434f..e690fa687d3 100644 --- a/tests/phpunit/includes/block/DatabaseBlockTest.php +++ b/tests/phpunit/includes/block/DatabaseBlockTest.php @@ -134,6 +134,7 @@ class DatabaseBlockTest extends MediaWikiLangTestCase { MainConfigNames::AutoCreateTempUser, [ 'enabled' => true, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'genPattern' => '*Unregistered $1', 'matchPattern' => '*$1', diff --git a/tests/phpunit/includes/user/UserGroupManagerTest.php b/tests/phpunit/includes/user/UserGroupManagerTest.php index 36ef6e2eab9..d01ea4d5f8e 100644 --- a/tests/phpunit/includes/user/UserGroupManagerTest.php +++ b/tests/phpunit/includes/user/UserGroupManagerTest.php @@ -101,6 +101,7 @@ class UserGroupManagerTest extends MediaWikiIntegrationTestCase { new TestLogger(), new RealTempUserConfig( [ 'enabled' => true, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'serialProvider' => [ 'type' => 'local' ], 'serialMapping' => [ 'type' => 'plain-numeric' ], diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php index 5bc4017dc6a..35f060cb01d 100644 --- a/tests/phpunit/includes/user/UserTest.php +++ b/tests/phpunit/includes/user/UserTest.php @@ -1734,6 +1734,7 @@ class UserTest extends MediaWikiIntegrationTestCase { MainConfigNames::AutoCreateTempUser, [ 'enabled' => true, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'genPattern' => '*Unregistered $1', 'matchPattern' => '*$1', diff --git a/tests/phpunit/integration/includes/block/BlockUserTest.php b/tests/phpunit/integration/includes/block/BlockUserTest.php index a57a7a0bd8a..b95dffc94b8 100644 --- a/tests/phpunit/integration/includes/block/BlockUserTest.php +++ b/tests/phpunit/integration/includes/block/BlockUserTest.php @@ -82,6 +82,7 @@ class BlockUserTest extends MediaWikiIntegrationTestCase { MainConfigNames::AutoCreateTempUser, [ 'enabled' => true, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'genPattern' => '*Unregistered $1', 'matchPattern' => '*$1', diff --git a/tests/phpunit/integration/includes/user/TempUser/RealTempUserConfigTest.php b/tests/phpunit/integration/includes/user/TempUser/RealTempUserConfigTest.php index ae7e92b5d26..27a8650362b 100644 --- a/tests/phpunit/integration/includes/user/TempUser/RealTempUserConfigTest.php +++ b/tests/phpunit/integration/includes/user/TempUser/RealTempUserConfigTest.php @@ -14,6 +14,7 @@ class RealTempUserConfigTest extends \MediaWikiIntegrationTestCase { /** This is meant to be the default config from MainConfigSchema */ private const DEFAULTS = [ 'enabled' => false, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'genPattern' => '*Unregistered $1', 'matchPattern' => '*$1', diff --git a/tests/phpunit/integration/includes/user/TempUser/TempUserCreatorTest.php b/tests/phpunit/integration/includes/user/TempUser/TempUserCreatorTest.php index 9210fcea9d0..c870f7d366c 100644 --- a/tests/phpunit/integration/includes/user/TempUser/TempUserCreatorTest.php +++ b/tests/phpunit/integration/includes/user/TempUser/TempUserCreatorTest.php @@ -24,6 +24,7 @@ class TempUserCreatorTest extends \MediaWikiIntegrationTestCase { /** This is meant to be the default config from MainConfigSchema */ private const DEFAULTS = [ 'enabled' => false, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'genPattern' => '*Unregistered $1', 'matchPattern' => '*$1', @@ -39,6 +40,7 @@ class TempUserCreatorTest extends \MediaWikiIntegrationTestCase { MainConfigNames::AutoCreateTempUser, [ 'enabled' => true, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'genPattern' => '*Unregistered $1', 'matchPattern' => '*$1', diff --git a/tests/phpunit/integration/includes/user/UserFactoryTest.php b/tests/phpunit/integration/includes/user/UserFactoryTest.php index b360f5374eb..0e3f04e2b91 100644 --- a/tests/phpunit/integration/includes/user/UserFactoryTest.php +++ b/tests/phpunit/integration/includes/user/UserFactoryTest.php @@ -195,6 +195,7 @@ class UserFactoryTest extends MediaWikiIntegrationTestCase { MainConfigNames::AutoCreateTempUser, [ 'enabled' => true, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'genPattern' => '*Unregistered $1', 'matchPattern' => '*$1', @@ -214,6 +215,7 @@ class UserFactoryTest extends MediaWikiIntegrationTestCase { MainConfigNames::AutoCreateTempUser, [ 'enabled' => true, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'genPattern' => '*Unregistered $1', 'matchPattern' => '*$1', diff --git a/tests/phpunit/mocks/DummyServicesTrait.php b/tests/phpunit/mocks/DummyServicesTrait.php index 26aa94ed4df..cd6baa0c670 100644 --- a/tests/phpunit/mocks/DummyServicesTrait.php +++ b/tests/phpunit/mocks/DummyServicesTrait.php @@ -514,6 +514,7 @@ trait DummyServicesTrait { $options['hookContainer'] ?? $this->createHookContainer(), new RealTempUserConfig( [ 'enabled' => true, + 'expireAfterDays' => null, 'actions' => [ 'edit' ], 'serialProvider' => [ 'type' => 'local' ], 'serialMapping' => [ 'type' => 'plain-numeric' ],