[stable30] fix: validate account properties as a repair step

lib/composer/composer/autoload_classmap.php

@@ -1815,10 +1815,11 @@
     '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',

lib/composer/composer/autoload_static.php

@@ -1848,10 +1848,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',

lib/private/Repair.php

@@ -37,10 +37,10 @@
 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;
@@ -191,6 +191,7 @@ public static function getRepairSteps(): array {
 			\OCP\Server::get(RepairLogoDimension::class),
 			\OCP\Server::get(RemoveLegacyDatadirFile::class),
 			\OCP\Server::get(AddCleanupDeletedUsersBackgroundJob::class),
+			\OCP\Server::get(SanitizeAccountProperties::class),
 		];
 	}
 
@@ -205,7 +206,6 @@ public static function getExpensiveRepairSteps() {
 			new OldGroupMembershipShares(\OC::$server->getDatabaseConnection(), \OC::$server->getGroupManager()),
 			new RemoveBrokenProperties(\OC::$server->getDatabaseConnection()),
 			new RepairMimeTypes(\OC::$server->getConfig(), \OC::$server->getDatabaseConnection()),
-			\OC::$server->get(ValidatePhoneNumber::class),
 			\OC::$server->get(DeleteSchedulingObjects::class),
 		];
 	}

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.');
+	}
+}

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');
+		}
+	}
+}

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]);
+	}
+}

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);
+	}
+}