diff options
| -rw-r--r-- | lib/composer/composer/autoload_classmap.php | 3 | ||||
| -rw-r--r-- | lib/composer/composer/autoload_static.php | 3 | ||||
| -rw-r--r-- | lib/private/Repair.php | 6 | ||||
| -rw-r--r-- | lib/private/Repair/NC21/ValidatePhoneNumber.php | 70 | ||||
| -rw-r--r-- | lib/private/Repair/NC29/SanitizeAccountProperties.php | 30 | ||||
| -rw-r--r-- | lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php | 75 | ||||
| -rw-r--r-- | tests/lib/Repair/NC29/SanitizeAccountPropertiesJobTest.php | 116 | ||||
| -rw-r--r-- | tests/lib/Repair/NC29/SanitizeAccountPropertiesTest.php | 43 |
8 files changed, 271 insertions, 75 deletions
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 5c36d30554b..b6095032c4c 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1882,10 +1882,11 @@ return array( 'OC\\Repair\\NC20\\EncryptionMigration' => $baseDir . '/lib/private/Repair/NC20/EncryptionMigration.php', 'OC\\Repair\\NC20\\ShippedDashboardEnable' => $baseDir . '/lib/private/Repair/NC20/ShippedDashboardEnable.php', 'OC\\Repair\\NC21\\AddCheckForUserCertificatesJob' => $baseDir . '/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php', - 'OC\\Repair\\NC21\\ValidatePhoneNumber' => $baseDir . '/lib/private/Repair/NC21/ValidatePhoneNumber.php', 'OC\\Repair\\NC22\\LookupServerSendCheck' => $baseDir . '/lib/private/Repair/NC22/LookupServerSendCheck.php', 'OC\\Repair\\NC24\\AddTokenCleanupJob' => $baseDir . '/lib/private/Repair/NC24/AddTokenCleanupJob.php', 'OC\\Repair\\NC25\\AddMissingSecretJob' => $baseDir . '/lib/private/Repair/NC25/AddMissingSecretJob.php', + 'OC\\Repair\\NC29\\SanitizeAccountProperties' => $baseDir . '/lib/private/Repair/NC29/SanitizeAccountProperties.php', + 'OC\\Repair\\NC29\\SanitizeAccountPropertiesJob' => $baseDir . '/lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php', 'OC\\Repair\\NC30\\RemoveLegacyDatadirFile' => $baseDir . '/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php', 'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\CleanPreviews' => $baseDir . '/lib/private/Repair/Owncloud/CleanPreviews.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index d366d58472f..66df44dfb06 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1931,10 +1931,11 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Repair\\NC20\\EncryptionMigration' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/EncryptionMigration.php', 'OC\\Repair\\NC20\\ShippedDashboardEnable' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/ShippedDashboardEnable.php', 'OC\\Repair\\NC21\\AddCheckForUserCertificatesJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php', - 'OC\\Repair\\NC21\\ValidatePhoneNumber' => __DIR__ . '/../../..' . '/lib/private/Repair/NC21/ValidatePhoneNumber.php', 'OC\\Repair\\NC22\\LookupServerSendCheck' => __DIR__ . '/../../..' . '/lib/private/Repair/NC22/LookupServerSendCheck.php', 'OC\\Repair\\NC24\\AddTokenCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC24/AddTokenCleanupJob.php', 'OC\\Repair\\NC25\\AddMissingSecretJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC25/AddMissingSecretJob.php', + 'OC\\Repair\\NC29\\SanitizeAccountProperties' => __DIR__ . '/../../..' . '/lib/private/Repair/NC29/SanitizeAccountProperties.php', + 'OC\\Repair\\NC29\\SanitizeAccountPropertiesJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php', 'OC\\Repair\\NC30\\RemoveLegacyDatadirFile' => __DIR__ . '/../../..' . '/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php', 'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\CleanPreviews' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/CleanPreviews.php', diff --git a/lib/private/Repair.php b/lib/private/Repair.php index cc2add4a8cf..c5069bff48e 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -37,10 +37,10 @@ use OC\Repair\NC20\EncryptionLegacyCipher; use OC\Repair\NC20\EncryptionMigration; use OC\Repair\NC20\ShippedDashboardEnable; use OC\Repair\NC21\AddCheckForUserCertificatesJob; -use OC\Repair\NC21\ValidatePhoneNumber; use OC\Repair\NC22\LookupServerSendCheck; use OC\Repair\NC24\AddTokenCleanupJob; use OC\Repair\NC25\AddMissingSecretJob; +use OC\Repair\NC29\SanitizeAccountProperties; use OC\Repair\NC30\RemoveLegacyDatadirFile; use OC\Repair\OldGroupMembershipShares; use OC\Repair\Owncloud\CleanPreviews; @@ -194,6 +194,7 @@ class Repair implements IOutput { \OCP\Server::get(RepairLogoDimension::class), \OCP\Server::get(RemoveLegacyDatadirFile::class), \OCP\Server::get(AddCleanupDeletedUsersBackgroundJob::class), + \OCP\Server::get(SanitizeAccountProperties::class), ]; } @@ -212,8 +213,7 @@ class Repair implements IOutput { \OCP\Server::get(IAppConfig::class), \OCP\Server::get(IDBConnection::class) ), - \OC::$server->get(ValidatePhoneNumber::class), - \OC::$server->get(DeleteSchedulingObjects::class), + \OCP\Server::get(DeleteSchedulingObjects::class), ]; } diff --git a/lib/private/Repair/NC21/ValidatePhoneNumber.php b/lib/private/Repair/NC21/ValidatePhoneNumber.php deleted file mode 100644 index 3a6ace37bd2..00000000000 --- a/lib/private/Repair/NC21/ValidatePhoneNumber.php +++ /dev/null @@ -1,70 +0,0 @@ -<?php - -declare(strict_types=1); - -/** - * SPDX-FileCopyrightText: 2020 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: AGPL-3.0-or-later - */ -namespace OC\Repair\NC21; - -use OCP\Accounts\IAccountManager; -use OCP\IConfig; -use OCP\IUser; -use OCP\IUserManager; -use OCP\Migration\IOutput; -use OCP\Migration\IRepairStep; - -class ValidatePhoneNumber implements IRepairStep { - /** @var IConfig */ - protected $config; - /** @var IUserManager */ - protected $userManager; - /** @var IAccountManager */ - private $accountManager; - - public function __construct(IUserManager $userManager, - IAccountManager $accountManager, - IConfig $config) { - $this->config = $config; - $this->userManager = $userManager; - $this->accountManager = $accountManager; - } - - public function getName(): string { - return 'Validate the phone number and store it in a known format for search'; - } - - public function run(IOutput $output): void { - if ($this->config->getSystemValueString('default_phone_region', '') === '') { - $output->warning('Can not validate phone numbers without `default_phone_region` being set in the config file'); - return; - } - - $numUpdated = 0; - $numRemoved = 0; - - $this->userManager->callForSeenUsers(function (IUser $user) use (&$numUpdated, &$numRemoved) { - $account = $this->accountManager->getAccount($user); - $property = $account->getProperty(IAccountManager::PROPERTY_PHONE); - - if ($property->getValue() !== '') { - $this->accountManager->updateAccount($account); - $updatedAccount = $this->accountManager->getAccount($user); - $updatedProperty = $updatedAccount->getProperty(IAccountManager::PROPERTY_PHONE); - - if ($property->getValue() !== $updatedProperty->getValue()) { - if ($updatedProperty->getValue() === '') { - $numRemoved++; - } else { - $numUpdated++; - } - } - } - }); - - if ($numRemoved > 0 || $numUpdated > 0) { - $output->info('Updated ' . $numUpdated . ' entries and cleaned ' . $numRemoved . ' invalid phone numbers'); - } - } -} diff --git a/lib/private/Repair/NC29/SanitizeAccountProperties.php b/lib/private/Repair/NC29/SanitizeAccountProperties.php new file mode 100644 index 00000000000..412570ba71d --- /dev/null +++ b/lib/private/Repair/NC29/SanitizeAccountProperties.php @@ -0,0 +1,30 @@ +<?php + +declare(strict_types=1); + +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +namespace OC\Repair\NC29; + +use OCP\BackgroundJob\IJobList; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; + +class SanitizeAccountProperties implements IRepairStep { + + public function __construct( + private IJobList $jobList, + ) { + } + + public function getName(): string { + return 'Validate account properties and store phone numbers in a known format for search'; + } + + public function run(IOutput $output): void { + $this->jobList->add(SanitizeAccountPropertiesJob::class, null); + $output->info('Queued background to validate account properties.'); + } +} diff --git a/lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php b/lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php new file mode 100644 index 00000000000..55ec445e9da --- /dev/null +++ b/lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php @@ -0,0 +1,75 @@ +<?php + +declare(strict_types=1); + +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +namespace OC\Repair\NC29; + +use InvalidArgumentException; +use OCP\Accounts\IAccountManager; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\QueuedJob; +use OCP\IUser; +use OCP\IUserManager; +use Psr\Log\LoggerInterface; + +class SanitizeAccountPropertiesJob extends QueuedJob { + + private const PROPERTIES_TO_CHECK = [ + IAccountManager::PROPERTY_PHONE, + IAccountManager::PROPERTY_WEBSITE, + IAccountManager::PROPERTY_TWITTER, + IAccountManager::PROPERTY_FEDIVERSE, + ]; + + public function __construct( + ITimeFactory $timeFactory, + private IUserManager $userManager, + private IAccountManager $accountManager, + private LoggerInterface $logger, + ) { + parent::__construct($timeFactory); + $this->setAllowParallelRuns(false); + } + + protected function run(mixed $argument): void { + $numRemoved = 0; + + $this->userManager->callForSeenUsers(function (IUser $user) use (&$numRemoved) { + $account = $this->accountManager->getAccount($user); + $properties = array_keys($account->jsonSerialize()); + + // Check if there are some properties we can sanitize - reduces number of db queries + if (empty(array_intersect($properties, self::PROPERTIES_TO_CHECK))) { + return; + } + + // Limit the loop to the properties we check to ensure there are no infinite loops + // we add one additional loop (+ 1) as we need 1 loop for checking + 1 for update. + $iteration = count(self::PROPERTIES_TO_CHECK) + 1; + while ($iteration-- > 0) { + try { + $this->accountManager->updateAccount($account); + return; + } catch (InvalidArgumentException $e) { + if (in_array($e->getMessage(), IAccountManager::ALLOWED_PROPERTIES)) { + $numRemoved++; + $property = $account->getProperty($e->getMessage()); + $account->setProperty($property->getName(), '', $property->getScope(), IAccountManager::NOT_VERIFIED); + } else { + $this->logger->error('Error while sanitizing account property', ['exception' => $e, 'user' => $user->getUID()]); + return; + } + } + } + $this->logger->error('Iteration limit exceeded while cleaning account properties', ['user' => $user->getUID()]); + }); + + if ($numRemoved > 0) { + $this->logger->info('Cleaned ' . $numRemoved . ' invalid account property entries'); + } + } +} diff --git a/tests/lib/Repair/NC29/SanitizeAccountPropertiesJobTest.php b/tests/lib/Repair/NC29/SanitizeAccountPropertiesJobTest.php new file mode 100644 index 00000000000..e0f4eb3cbc1 --- /dev/null +++ b/tests/lib/Repair/NC29/SanitizeAccountPropertiesJobTest.php @@ -0,0 +1,116 @@ +<?php + +declare(strict_types=1); + +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +namespace OC\Repair\NC29; + +use InvalidArgumentException; +use OCP\Accounts\IAccount; +use OCP\Accounts\IAccountManager; +use OCP\Accounts\IAccountProperty; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IUser; +use OCP\IUserManager; +use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; +use Test\TestCase; + +class SanitizeAccountPropertiesJobTest extends TestCase { + + private IUserManager&MockObject $userManager; + private IAccountManager&MockObject $accountManager; + private LoggerInterface&MockObject $logger; + + private SanitizeAccountPropertiesJob $job; + + protected function setUp(): void { + $this->userManager = $this->createMock(IUserManager::class); + $this->accountManager = $this->createMock(IAccountManager::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->job = new SanitizeAccountPropertiesJob( + $this->createMock(ITimeFactory::class), + $this->userManager, + $this->accountManager, + $this->logger, + ); + } + + public function testParallel() { + self::assertFalse($this->job->getAllowParallelRuns()); + } + + public function testRun(): void { + $users = [ + $this->createMock(IUser::class), + $this->createMock(IUser::class), + $this->createMock(IUser::class), + ]; + $this->userManager + ->expects(self::once()) + ->method('callForSeenUsers') + ->willReturnCallback(fn ($fn) => array_map($fn, $users)); + + $property = $this->createMock(IAccountProperty::class); + $property->expects(self::once())->method('getName')->willReturn(IAccountManager::PROPERTY_PHONE); + $property->expects(self::once())->method('getScope')->willReturn(IAccountManager::SCOPE_LOCAL); + + $account1 = $this->createMock(IAccount::class); + $account1->expects(self::once()) + ->method('getProperty') + ->with(IAccountManager::PROPERTY_PHONE) + ->willReturn($property); + $account1->expects(self::once()) + ->method('setProperty') + ->with(IAccountManager::PROPERTY_PHONE, '', IAccountManager::SCOPE_LOCAL, IAccountManager::NOT_VERIFIED); + $account1->expects(self::once()) + ->method('jsonSerialize') + ->willReturn([ + IAccountManager::PROPERTY_DISPLAYNAME => [], + IAccountManager::PROPERTY_PHONE => [], + ]); + + $account2 = $this->createMock(IAccount::class); + $account2->expects(self::never()) + ->method('getProperty'); + $account2->expects(self::once()) + ->method('jsonSerialize') + ->willReturn([ + IAccountManager::PROPERTY_DISPLAYNAME => [], + IAccountManager::PROPERTY_PHONE => [], + ]); + + $account3 = $this->createMock(IAccount::class); + $account3->expects(self::never()) + ->method('getProperty'); + $account3->expects(self::once()) + ->method('jsonSerialize') + ->willReturn([ + IAccountManager::PROPERTY_DISPLAYNAME => [], + ]); + + $this->accountManager + ->expects(self::exactly(3)) + ->method('getAccount') + ->willReturnMap([ + [$users[0], $account1], + [$users[1], $account2], + [$users[2], $account3], + ]); + $valid = false; + $this->accountManager->expects(self::exactly(3)) + ->method('updateAccount') + ->willReturnCallback(function (IAccount $account) use (&$account1, &$valid) { + if (!$valid && $account === $account1) { + $valid = true; + throw new InvalidArgumentException(IAccountManager::PROPERTY_PHONE); + } + }); + + self::invokePrivate($this->job, 'run', [null]); + } +} diff --git a/tests/lib/Repair/NC29/SanitizeAccountPropertiesTest.php b/tests/lib/Repair/NC29/SanitizeAccountPropertiesTest.php new file mode 100644 index 00000000000..d0d33eb2817 --- /dev/null +++ b/tests/lib/Repair/NC29/SanitizeAccountPropertiesTest.php @@ -0,0 +1,43 @@ +<?php + +declare(strict_types=1); + +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +namespace OC\Repair\NC29; + +use OCP\BackgroundJob\IJobList; +use OCP\Migration\IOutput; +use PHPUnit\Framework\MockObject\MockObject; +use Test\TestCase; + +class SanitizeAccountPropertiesTest extends TestCase { + + private IJobList&MockObject $jobList; + private SanitizeAccountProperties $repairStep; + + protected function setUp(): void { + $this->jobList = $this->createMock(IJobList::class); + + $this->repairStep = new SanitizeAccountProperties($this->jobList); + } + + public function testGetName(): void { + self::assertStringContainsString('Validate account properties', $this->repairStep->getName()); + } + + public function testRun(): void { + $this->jobList->expects(self::once()) + ->method('add') + ->with(SanitizeAccountPropertiesJob::class, null); + + $output = $this->createMock(IOutput::class); + $output->expects(self::once()) + ->method('info') + ->with(self::matchesRegularExpression('/queued background/i')); + + $this->repairStep->run($output); + } +} |