Merge pull request #27719 from nextcloud/backport/27474/stable22

apps/provisioning_api/appinfo/routes.php

@@ -54,6 +54,7 @@ return [
 		['root' => '/cloud', 'name' => 'Users#getEditableFields', 'url' => '/user/fields', 'verb' => 'GET'],
 		['root' => '/cloud', 'name' => 'Users#getEditableFieldsForUser', 'url' => '/user/fields/{userId}', 'verb' => 'GET'],
 		['root' => '/cloud', 'name' => 'Users#editUser', 'url' => '/users/{userId}', 'verb' => 'PUT'],
+		['root' => '/cloud', 'name' => 'Users#editUserMultiValue', 'url' => '/users/{userId}/{collectionName}', 'verb' => 'PUT', 'requirements' => ['collectionName' => '^(?!enable$|disable$)[a-zA-Z0-9_]*$']],
 		['root' => '/cloud', 'name' => 'Users#wipeUserDevices', 'url' => '/users/{userId}/wipe', 'verb' => 'POST'],
 		['root' => '/cloud', 'name' => 'Users#deleteUser', 'url' => '/users/{userId}', 'verb' => 'DELETE'],
 		['root' => '/cloud', 'name' => 'Users#enableUser', 'url' => '/users/{userId}/enable', 'verb' => 'PUT'],

apps/provisioning_api/lib/Controller/AUserData.php

@@ -150,6 +150,20 @@ abstract class AUserData extends OCSController {
 			if ($includeScopes) {
 				$data[IAccountManager::PROPERTY_EMAIL . self::SCOPE_SUFFIX] = $userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getScope();
 			}
+
+			$additionalEmails = $additionalEmailScopes = [];
+			$emailCollection = $userAccount->getPropertyCollection(IAccountManager::COLLECTION_EMAIL);
+			foreach ($emailCollection->getProperties() as $property) {
+				$additionalEmails[] = $property->getValue();
+				if ($includeScopes) {
+					$additionalEmailScopes[] = $property->getScope();
+				}
+			}
+			$data[IAccountManager::COLLECTION_EMAIL] = $additionalEmails;
+			if ($includeScopes) {
+				$data[IAccountManager::COLLECTION_EMAIL . self::SCOPE_SUFFIX] = $additionalEmailScopes;
+			}
+
 			$data[IAccountManager::PROPERTY_DISPLAYNAME] = $targetUserObject->getDisplayName();
 			if ($includeScopes) {
 				$data[IAccountManager::PROPERTY_DISPLAYNAME . self::SCOPE_SUFFIX] = $userAccount->getProperty(IAccountManager::PROPERTY_DISPLAYNAME)->getScope();

apps/provisioning_api/lib/Controller/UsersController.php

@@ -52,7 +52,8 @@ use OC\KnownUser\KnownUserService;
 use OC\User\Backend;
 use OCA\Settings\Mailer\NewUserMailHelper;
 use OCP\Accounts\IAccountManager;
-use OCP\App\IAppManager;
+use OCP\Accounts\IAccountProperty;
+use OCP\Accounts\PropertyDoesNotExistException;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCS\OCSException;
@@ -75,8 +76,6 @@ use Psr\Log\LoggerInterface;
 
 class UsersController extends AUserData {
 
-	/** @var IAppManager */
-	private $appManager;
 	/** @var IURLGenerator */
 	protected $urlGenerator;
 	/** @var LoggerInterface */
@@ -98,7 +97,6 @@ class UsersController extends AUserData {
 								IRequest $request,
 								IUserManager $userManager,
 								IConfig $config,
-								IAppManager $appManager,
 								IGroupManager $groupManager,
 								IUserSession $userSession,
 								IAccountManager $accountManager,
@@ -119,7 +117,6 @@ class UsersController extends AUserData {
 							$accountManager,
 							$l10nFactory);
 
-		$this->appManager = $appManager;
 		$this->urlGenerator = $urlGenerator;
 		$this->logger = $logger;
 		$this->l10nFactory = $l10nFactory;
@@ -592,6 +589,7 @@ class UsersController extends AUserData {
 			$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
 		}
 
+		$permittedFields[] = IAccountManager::COLLECTION_EMAIL;
 		$permittedFields[] = IAccountManager::PROPERTY_PHONE;
 		$permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
 		$permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
@@ -600,6 +598,92 @@ class UsersController extends AUserData {
 		return new DataResponse($permittedFields);
 	}
 
+	/**
+	 * @NoAdminRequired
+	 * @NoSubAdminRequired
+	 * @PasswordConfirmationRequired
+	 *
+	 * @throws OCSException
+	 */
+	public function editUserMultiValue(
+		string $userId,
+		string $collectionName,
+		string $key,
+		string $value
+	): DataResponse {
+		$currentLoggedInUser = $this->userSession->getUser();
+		if ($currentLoggedInUser === null) {
+			throw new OCSException('', OCSController::RESPOND_UNAUTHORISED);
+		}
+
+		$targetUser = $this->userManager->get($userId);
+		if ($targetUser === null) {
+			throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
+		}
+
+		$permittedFields = [];
+		if ($targetUser->getUID() === $currentLoggedInUser->getUID()) {
+			// Editing self (display, email)
+			$permittedFields[] = IAccountManager::COLLECTION_EMAIL;
+			$permittedFields[] = IAccountManager::COLLECTION_EMAIL . self::SCOPE_SUFFIX;
+		} else {
+			// Check if admin / subadmin
+			$subAdminManager = $this->groupManager->getSubAdmin();
+			if ($this->groupManager->isAdmin($currentLoggedInUser->getUID())
+				|| $subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) {
+				// They have permissions over the user
+
+				$permittedFields[] = IAccountManager::COLLECTION_EMAIL;
+			} else {
+				// No rights
+				throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
+			}
+		}
+
+		// Check if permitted to edit this field
+		if (!in_array($collectionName, $permittedFields)) {
+			throw new OCSException('', 103);
+		}
+
+		switch ($collectionName) {
+			case IAccountManager::COLLECTION_EMAIL:
+				$userAccount = $this->accountManager->getAccount($targetUser);
+				$mailCollection = $userAccount->getPropertyCollection(IAccountManager::COLLECTION_EMAIL);
+				$mailCollection->removePropertyByValue($key);
+				if ($value !== '') {
+					$mailCollection->addPropertyWithDefaults($value);
+				}
+				$this->accountManager->updateAccount($userAccount);
+				break;
+
+			case IAccountManager::COLLECTION_EMAIL . self::SCOPE_SUFFIX:
+				$userAccount = $this->accountManager->getAccount($targetUser);
+				$mailCollection = $userAccount->getPropertyCollection(IAccountManager::COLLECTION_EMAIL);
+				$targetProperty = null;
+				foreach ($mailCollection->getProperties() as $property) {
+					if ($property->getValue() === $key) {
+						$targetProperty = $property;
+						break;
+					}
+				}
+				if ($targetProperty instanceof IAccountProperty) {
+					try {
+						$targetProperty->setScope($value);
+						$this->accountManager->updateAccount($userAccount);
+					} catch (\InvalidArgumentException $e) {
+						throw new OCSException('', 102);
+					}
+				} else {
+					throw new OCSException('', 102);
+				}
+				break;
+
+			default:
+				throw new OCSException('', 103);
+		}
+		return new DataResponse();
+	}
+
 	/**
 	 * @NoAdminRequired
 	 * @NoSubAdminRequired
@@ -636,6 +720,8 @@ class UsersController extends AUserData {
 			$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME . self::SCOPE_SUFFIX;
 			$permittedFields[] = IAccountManager::PROPERTY_EMAIL . self::SCOPE_SUFFIX;
 
+			$permittedFields[] = IAccountManager::COLLECTION_EMAIL;
+
 			$permittedFields[] = 'password';
 			if ($this->config->getSystemValue('force_language', false) === false ||
 				$this->groupManager->isAdmin($currentLoggedInUser->getUID())) {
@@ -674,6 +760,7 @@ class UsersController extends AUserData {
 					$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
 				}
 				$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
+				$permittedFields[] = IAccountManager::COLLECTION_EMAIL;
 				$permittedFields[] = 'password';
 				$permittedFields[] = 'language';
 				$permittedFields[] = 'locale';
@@ -746,24 +833,42 @@ class UsersController extends AUserData {
 					throw new OCSException('', 102);
 				}
 				break;
+			case IAccountManager::COLLECTION_EMAIL:
+				if (filter_var($value, FILTER_VALIDATE_EMAIL) && $value !== $targetUser->getEMailAddress()) {
+					$userAccount = $this->accountManager->getAccount($targetUser);
+					$mailCollection = $userAccount->getPropertyCollection(IAccountManager::COLLECTION_EMAIL);
+					foreach ($mailCollection->getProperties() as $property) {
+						if ($property->getValue() === $value) {
+							break;
+						}
+					}
+					$mailCollection->addPropertyWithDefaults($value);
+					$this->accountManager->updateAccount($userAccount);
+				} else {
+					throw new OCSException('', 102);
+				}
+				break;
 			case IAccountManager::PROPERTY_PHONE:
 			case IAccountManager::PROPERTY_ADDRESS:
 			case IAccountManager::PROPERTY_WEBSITE:
 			case IAccountManager::PROPERTY_TWITTER:
 				$userAccount = $this->accountManager->getAccount($targetUser);
-				$userProperty = $userAccount->getProperty($key);
-				if ($userProperty->getValue() !== $value) {
-					try {
-						$userProperty->setValue($value);
-						$this->accountManager->updateAccount($userAccount);
-
-						if ($userProperty->getName() === IAccountManager::PROPERTY_PHONE) {
-							$this->knownUserService->deleteByContactUserId($targetUser->getUID());
+				try {
+					$userProperty = $userAccount->getProperty($key);
+					if ($userProperty->getValue() !== $value) {
+						try {
+							$userProperty->setValue($value);
+							if ($userProperty->getName() === IAccountManager::PROPERTY_PHONE) {
+								$this->knownUserService->deleteByContactUserId($targetUser->getUID());
+							}
+						} catch (\InvalidArgumentException $e) {
+							throw new OCSException('Invalid ' . $e->getMessage(), 102);
 						}
-					} catch (\InvalidArgumentException $e) {
-						throw new OCSException('Invalid ' . $e->getMessage(), 102);
 					}
+				} catch (PropertyDoesNotExistException $e) {
+					$userAccount->setProperty($key, $value, IAccountManager::SCOPE_PRIVATE, IAccountManager::NOT_VERIFIED);
 				}
+				$this->accountManager->updateAccount($userAccount);
 				break;
 			case IAccountManager::PROPERTY_DISPLAYNAME . self::SCOPE_SUFFIX:
 			case IAccountManager::PROPERTY_EMAIL . self::SCOPE_SUFFIX:

apps/provisioning_api/tests/Controller/UsersControllerTest.php

@@ -50,7 +50,6 @@ use OCA\Settings\Mailer\NewUserMailHelper;
 use OCP\Accounts\IAccount;
 use OCP\Accounts\IAccountManager;
 use OCP\Accounts\IAccountProperty;
-use OCP\App\IAppManager;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
@@ -77,8 +76,6 @@ class UsersControllerTest extends TestCase {
 	protected $userManager;
 	/** @var IConfig|MockObject */
 	protected $config;
-	/** @var IAppManager|MockObject */
-	protected $appManager;
 	/** @var Manager|MockObject */
 	protected $groupManager;
 	/** @var IUserSession|MockObject */
@@ -111,7 +108,6 @@ class UsersControllerTest extends TestCase {
 
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->config = $this->createMock(IConfig::class);
-		$this->appManager = $this->createMock(IAppManager::class);
 		$this->groupManager = $this->createMock(Manager::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
@@ -131,7 +127,6 @@ class UsersControllerTest extends TestCase {
 				$this->request,
 				$this->userManager,
 				$this->config,
-				$this->appManager,
 				$this->groupManager,
 				$this->userSession,
 				$this->accountManager,
@@ -395,7 +390,6 @@ class UsersControllerTest extends TestCase {
 				$this->request,
 				$this->userManager,
 				$this->config,
-				$this->appManager,
 				$this->groupManager,
 				$this->userSession,
 				$this->accountManager,
@@ -1071,7 +1065,8 @@ class UsersControllerTest extends TestCase {
 			'backendCapabilities' => [
 				'setDisplayName' => true,
 				'setPassword' => true,
-			]
+			],
+			'additional_mail' => [],
 		];
 		$this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID']));
 	}
@@ -1198,7 +1193,8 @@ class UsersControllerTest extends TestCase {
 			'backendCapabilities' => [
 				'setDisplayName' => true,
 				'setPassword' => true,
-			]
+			],
+			'additional_mail' => [],
 		];
 		$this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID']));
 	}
@@ -1363,7 +1359,8 @@ class UsersControllerTest extends TestCase {
 			'backendCapabilities' => [
 				'setDisplayName' => false,
 				'setPassword' => false,
-			]
+			],
+			'additional_mail' => [],
 		];
 		$this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID']));
 	}
@@ -3437,7 +3434,6 @@ class UsersControllerTest extends TestCase {
 				$this->request,
 				$this->userManager,
 				$this->config,
-				$this->appManager,
 				$this->groupManager,
 				$this->userSession,
 				$this->accountManager,
@@ -3510,7 +3506,6 @@ class UsersControllerTest extends TestCase {
 				$this->request,
 				$this->userManager,
 				$this->config,
-				$this->appManager,
 				$this->groupManager,
 				$this->userSession,
 				$this->accountManager,
@@ -3848,6 +3843,7 @@ class UsersControllerTest extends TestCase {
 	public function dataGetEditableFields() {
 		return [
 			[false, ISetDisplayNameBackend::class, [
+				IAccountManager::COLLECTION_EMAIL,
 				IAccountManager::PROPERTY_PHONE,
 				IAccountManager::PROPERTY_ADDRESS,
 				IAccountManager::PROPERTY_WEBSITE,
@@ -3856,6 +3852,7 @@ class UsersControllerTest extends TestCase {
 			[true, ISetDisplayNameBackend::class, [
 				IAccountManager::PROPERTY_DISPLAYNAME,
 				IAccountManager::PROPERTY_EMAIL,
+				IAccountManager::COLLECTION_EMAIL,
 				IAccountManager::PROPERTY_PHONE,
 				IAccountManager::PROPERTY_ADDRESS,
 				IAccountManager::PROPERTY_WEBSITE,
@@ -3863,6 +3860,7 @@ class UsersControllerTest extends TestCase {
 			]],
 			[true, UserInterface::class, [
 				IAccountManager::PROPERTY_EMAIL,
+				IAccountManager::COLLECTION_EMAIL,
 				IAccountManager::PROPERTY_PHONE,
 				IAccountManager::PROPERTY_ADDRESS,
 				IAccountManager::PROPERTY_WEBSITE,

build/integration/features/bootstrap/Provisioning.php

@@ -168,14 +168,19 @@ trait Provisioning {
 		$response = $client->get($fullUrl, $options);
 		foreach ($settings->getRows() as $setting) {
 			$value = json_decode(json_encode(simplexml_load_string($response->getBody())->data->{$setting[0]}), 1);
-			if (isset($value[0])) {
+			if (isset($value['element']) && in_array($setting[0], ['additional_mail', 'additional_mailScope'], true)) {
+				$expectedValues = explode(';', $setting[1]);
+				foreach ($expectedValues as $expected) {
+					Assert::assertTrue(in_array($expected, $value['element'], true));
+				}
+			} elseif (isset($value[0])) {
 				Assert::assertEquals($setting[1], $value[0], "", 0.0, 10, true);
 			} else {
 				Assert::assertEquals('', $setting[1]);
 			}
 		}
 	}
-	
+
 	/**
 	 * @Then /^group "([^"]*)" has$/
 	 *
@@ -194,7 +199,7 @@ trait Provisioning {
 		$options['headers'] = [
 			'OCS-APIREQUEST' => 'true',
 		];
-		
+
 		$response = $client->get($fullUrl, $options);
 		$groupDetails = simplexml_load_string($response->getBody())->data[0]->groups[0]->element;
 		foreach ($settings->getRows() as $setting) {
@@ -206,7 +211,7 @@ trait Provisioning {
 			}
 		}
 	}
-	
+
 
 	/**
 	 * @Then /^user "([^"]*)" has editable fields$/
@@ -967,4 +972,38 @@ trait Provisioning {
 		}
 		$this->usingServer($previousServer);
 	}
+
+	/**
+	 * @Then /^user "([^"]*)" has not$/
+	 */
+	public function userHasNotSetting($user, \Behat\Gherkin\Node\TableNode $settings) {
+		$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/cloud/users/$user";
+		$client = new Client();
+		$options = [];
+		if ($this->currentUser === 'admin') {
+			$options['auth'] = $this->adminUser;
+		} else {
+			$options['auth'] = [$this->currentUser, $this->regularUser];
+		}
+		$options['headers'] = [
+			'OCS-APIREQUEST' => 'true',
+		];
+
+		$response = $client->get($fullUrl, $options);
+		foreach ($settings->getRows() as $setting) {
+			$value = json_decode(json_encode(simplexml_load_string($response->getBody())->data->{$setting[0]}), 1);
+			if (isset($value[0])) {
+				if (in_array($setting[0], ['additional_mail', 'additional_mailScope'], true)) {
+					$expectedValues = explode(';', $setting[1]);
+					foreach ($expectedValues as $expected) {
+						Assert::assertFalse(in_array($expected, $value, true));
+					}
+				} else {
+					Assert::assertNotEquals($setting[1], $value[0], "", 0.0, 10, true);
+				}
+			} else {
+				Assert::assertNotEquals('', $setting[1]);
+			}
+		}
+	}
 }

build/integration/features/provisioning-v1.feature

@@ -62,6 +62,7 @@ Feature: provisioning
     Then user "brand-new-user" has editable fields
       | displayname |
       | email |
+      | additional_mail |
       | phone |
       | address |
       | website |
@@ -70,6 +71,7 @@ Feature: provisioning
     Then user "brand-new-user" has editable fields
       | displayname |
       | email |
+      | additional_mail |
       | phone |
       | address |
       | website |
@@ -77,6 +79,7 @@ Feature: provisioning
     Then user "self" has editable fields
       | displayname |
       | email |
+      | additional_mail |
       | phone |
       | address |
       | website |
@@ -100,6 +103,16 @@ Feature: provisioning
 			| value | no-reply@nextcloud.com |
 		And the OCS status code should be "100"
 		And the HTTP status code should be "200"
+    And sending "PUT" to "/cloud/users/brand-new-user" with
+      | key | additional_mail |
+      | value | no.reply@nextcloud.com |
+    And the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And sending "PUT" to "/cloud/users/brand-new-user" with
+      | key | additional_mail |
+      | value | noreply@nextcloud.com |
+    And the OCS status code should be "100"
+    And the HTTP status code should be "200"
 		And sending "PUT" to "/cloud/users/brand-new-user" with
 			| key | phone |
 			| value | +49 711 / 25 24 28-90 |
@@ -124,6 +137,7 @@ Feature: provisioning
 			| id | brand-new-user |
 			| displayname | Brand New User |
 			| email | no-reply@nextcloud.com |
+      | additional_mail | no.reply@nextcloud.com;noreply@nextcloud.com |
 			| phone | +4971125242890 |
 			| address | Foo Bar Town |
 			| website | https://nextcloud.com |
@@ -177,6 +191,33 @@ Feature: provisioning
 			| displaynameScope | v2-federated |
 			| avatarScope | v2-local |
 
+  Scenario: Edit a user account multivalue property scopes
+    Given user "brand-new-user" exists
+    And As an "brand-new-user"
+    When sending "PUT" to "/cloud/users/brand-new-user" with
+      | key | additional_mail |
+      | value | no.reply@nextcloud.com |
+    And the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And sending "PUT" to "/cloud/users/brand-new-user" with
+      | key | additional_mail |
+      | value | noreply@nextcloud.com |
+    And the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    When sending "PUT" to "/cloud/users/brand-new-user/additional_mailScope" with
+      | key | no.reply@nextcloud.com |
+      | value | v2-federated |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    When sending "PUT" to "/cloud/users/brand-new-user/additional_mailScope" with
+      | key | noreply@nextcloud.com |
+      | value | v2-published |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    Then user "brand-new-user" has
+      | id | brand-new-user |
+      | additional_mailScope | v2-federated;v2-published |
+
 	Scenario: Edit a user account properties scopes with invalid or unsupported value
 		Given user "brand-new-user" exists
     And As an "brand-new-user"
@@ -196,6 +237,43 @@ Feature: provisioning
 		Then the OCS status code should be "102"
 		And the HTTP status code should be "200"
 
+  Scenario: Edit a user account multi-value property scopes with invalid or unsupported value
+    Given user "brand-new-user" exists
+    And As an "brand-new-user"
+    When sending "PUT" to "/cloud/users/brand-new-user" with
+      | key | additional_mail |
+      | value | no.reply@nextcloud.com |
+    And the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    When sending "PUT" to "/cloud/users/brand-new-user/additional_mailScope" with
+      | key | no.reply@nextcloud.com |
+      | value | invalid |
+    Then the OCS status code should be "102"
+    And the HTTP status code should be "200"
+
+  Scenario: Delete a user account multi-value property value
+    Given user "brand-new-user" exists
+    And As an "brand-new-user"
+    When sending "PUT" to "/cloud/users/brand-new-user" with
+      | key | additional_mail |
+      | value | no.reply@nextcloud.com |
+    And the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And sending "PUT" to "/cloud/users/brand-new-user" with
+      | key | additional_mail |
+      | value | noreply@nextcloud.com |
+    And the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    When sending "PUT" to "/cloud/users/brand-new-user/additional_mail" with
+      | key | no.reply@nextcloud.com |
+      | value | |
+    And the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    Then user "brand-new-user" has
+      | additional_mail | noreply@nextcloud.com |
+    Then user "brand-new-user" has not
+      | additional_mail | no.reply@nextcloud.com |
+
 	Scenario: An admin cannot edit user account property scopes
     Given As an "admin"
 		And user "brand-new-user" exists
@@ -233,7 +311,7 @@ Feature: provisioning
 		And group "new-group" exists
 		And group "new-group" has
 			| displayname | new-group |
-			
+
 	Scenario: Create a group with custom display name
 		Given As an "admin"
 		And group "new-group" does not exist

lib/base.php

@@ -68,6 +68,7 @@ use OCP\Share;
 use OC\Encryption\HookManager;
 use OC\Files\Filesystem;
 use OC\Share20\Hooks;
+use OCP\User\Events\UserChangedEvent;
 
 require_once 'public/Constants.php';
 
@@ -843,8 +844,9 @@ class OC {
 	}
 
 	private static function registerAccountHooks() {
-		$hookHandler = \OC::$server->get(\OC\Accounts\Hooks::class);
-		\OCP\Util::connectHook('OC_User', 'changeUser', $hookHandler, 'changeUserHook');
+		/** @var IEventDispatcher $dispatcher */
+		$dispatcher = \OC::$server->get(IEventDispatcher::class);
+		$dispatcher->addServiceListener(UserChangedEvent::class, \OC\Accounts\Hooks::class);
 	}
 
 	private static function registerAppRestrictionsHooks() {

lib/private/Accounts/Account.php

@@ -33,6 +33,7 @@ use OCP\Accounts\IAccountProperty;
 use OCP\Accounts\IAccountPropertyCollection;
 use OCP\Accounts\PropertyDoesNotExistException;
 use OCP\IUser;
+use RuntimeException;
 
 class Account implements IAccount {
 	use TAccountsHelper;
@@ -116,13 +117,16 @@ class Account implements IAccount {
 		return $this;
 	}
 
-	public function getPropertyCollection(string $propertyCollection): IAccountPropertyCollection {
-		if (!array_key_exists($propertyCollection, $this->properties)) {
-			throw new PropertyDoesNotExistException($propertyCollection);
+	public function getPropertyCollection(string $propertyCollectionName): IAccountPropertyCollection {
+		if (!$this->isCollection($propertyCollectionName)) {
+			throw new PropertyDoesNotExistException($propertyCollectionName);
 		}
-		if (!$this->properties[$propertyCollection] instanceof IAccountPropertyCollection) {
-			throw new \RuntimeException('Requested collection is not an IAccountPropertyCollection');
+		if (!array_key_exists($propertyCollectionName, $this->properties)) {
+			$this->properties[$propertyCollectionName] = new AccountPropertyCollection($propertyCollectionName);
 		}
-		return $this->properties[$propertyCollection];
+		if (!$this->properties[$propertyCollectionName] instanceof IAccountPropertyCollection) {
+			throw new RuntimeException('Requested collection is not an IAccountPropertyCollection');
+		}
+		return $this->properties[$propertyCollectionName];
 	}
 }

lib/private/Accounts/AccountManager.php

@@ -32,6 +32,7 @@
  */
 namespace OC\Accounts;
 
+use InvalidArgumentException;
 use libphonenumber\NumberParseException;
 use libphonenumber\PhoneNumber;
 use libphonenumber\PhoneNumberFormat;
@@ -39,6 +40,9 @@ use libphonenumber\PhoneNumberUtil;
 use OCA\Settings\BackgroundJobs\VerifyUserData;
 use OCP\Accounts\IAccount;
 use OCP\Accounts\IAccountManager;
+use OCP\Accounts\IAccountProperty;
+use OCP\Accounts\IAccountPropertyCollection;
+use OCP\Accounts\PropertyDoesNotExistException;
 use OCP\BackgroundJob\IJobList;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IConfig;
@@ -48,7 +52,9 @@ use Psr\Log\LoggerInterface;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\EventDispatcher\GenericEvent;
 use function array_flip;
+use function iterator_to_array;
 use function json_decode;
+use function json_encode;
 use function json_last_error;
 
 /**
@@ -98,7 +104,7 @@ class AccountManager implements IAccountManager {
 	/**
 	 * @param string $input
 	 * @return string Provided phone number in E.164 format when it was a valid number
-	 * @throws \InvalidArgumentException When the phone number was invalid or no default region is set and the number doesn't start with a country code
+	 * @throws InvalidArgumentException When the phone number was invalid or no default region is set and the number doesn't start with a country code
 	 */
 	protected function parsePhoneNumber(string $input): string {
 		$defaultRegion = $this->config->getSystemValueString('default_phone_region', '');
@@ -106,7 +112,7 @@ class AccountManager implements IAccountManager {
 		if ($defaultRegion === '') {
 			// When no default region is set, only +49… numbers are valid
 			if (strpos($input, '+') !== 0) {
-				throw new \InvalidArgumentException(self::PROPERTY_PHONE);
+				throw new InvalidArgumentException(self::PROPERTY_PHONE);
 			}
 
 			$defaultRegion = 'EN';
@@ -121,140 +127,107 @@ class AccountManager implements IAccountManager {
 		} catch (NumberParseException $e) {
 		}
 
-		throw new \InvalidArgumentException(self::PROPERTY_PHONE);
+		throw new InvalidArgumentException(self::PROPERTY_PHONE);
 	}
 
 	/**
 	 *
 	 * @param string $input
 	 * @return string
-	 * @throws \InvalidArgumentException When the website did not have http(s) as protocol or the host name was empty
+	 * @throws InvalidArgumentException When the website did not have http(s) as protocol or the host name was empty
 	 */
 	protected function parseWebsite(string $input): string {
 		$parts = parse_url($input);
 		if (!isset($parts['scheme']) || ($parts['scheme'] !== 'https' && $parts['scheme'] !== 'http')) {
-			throw new \InvalidArgumentException(self::PROPERTY_WEBSITE);
+			throw new InvalidArgumentException(self::PROPERTY_WEBSITE);
 		}
 
 		if (!isset($parts['host']) || $parts['host'] === '') {
-			throw new \InvalidArgumentException(self::PROPERTY_WEBSITE);
+			throw new InvalidArgumentException(self::PROPERTY_WEBSITE);
 		}
 
 		return $input;
 	}
 
-	protected function sanitizeLength(array &$propertyData, bool $throwOnData = false): void {
-		if (isset($propertyData['value']) && strlen($propertyData['value']) > 2048) {
-			if ($throwOnData) {
-				throw new \InvalidArgumentException();
-			} else {
-				$propertyData['value'] = '';
-			}
-		}
-	}
-
-	protected function testValueLengths(array &$data, bool $throwOnData = false): void {
-		try {
-			foreach ($data as $propertyName => &$propertyData) {
-				if ($this->isCollection($propertyName)) {
-					$this->testValueLengths($propertyData, $throwOnData);
-				} else {
-					$this->sanitizeLength($propertyData, $throwOnData);
-				}
-			}
-		} catch (\InvalidArgumentException $e) {
-			throw new \InvalidArgumentException($propertyName);
-		}
-	}
-
-	protected function testPropertyScopes(array &$data, array $allowedScopes, bool $throwOnData = false, string $parentPropertyName = null): void {
-		foreach ($data as $propertyNameOrIndex => &$propertyData) {
-			if ($this->isCollection($propertyNameOrIndex)) {
-				$this->testPropertyScopes($propertyData, $allowedScopes, $throwOnData);
-			} elseif (isset($propertyData['scope'])) {
-				$effectivePropertyName = $parentPropertyName ?? $propertyNameOrIndex;
-
-				if ($throwOnData && !in_array($propertyData['scope'], $allowedScopes, true)) {
-					throw new \InvalidArgumentException('scope');
-				}
-
-				if (
-					$propertyData['scope'] === self::SCOPE_PRIVATE
-					&& ($effectivePropertyName === self::PROPERTY_DISPLAYNAME || $effectivePropertyName === self::PROPERTY_EMAIL)
-				) {
-					if ($throwOnData) {
-						// v2-private is not available for these fields
-						throw new \InvalidArgumentException('scope');
-					} else {
-						// default to local
-						$data[$propertyNameOrIndex]['scope'] = self::SCOPE_LOCAL;
-					}
-				} else {
-					// migrate scope values to the new format
-					// invalid scopes are mapped to a default value
-					$data[$propertyNameOrIndex]['scope'] = AccountProperty::mapScopeToV2($propertyData['scope']);
-				}
-			}
-		}
-	}
-
 	/**
-	 * update user record
-	 *
-	 * @param IUser $user
-	 * @param array $data
-	 * @param bool $throwOnData Set to true if you can inform the user about invalid data
-	 * @return array The potentially modified data (e.g. phone numbers are converted to E.164 format)
-	 * @throws \InvalidArgumentException Message is the property that was invalid
+	 * @param IAccountProperty[] $properties
 	 */
-	public function updateUser(IUser $user, array $data, bool $throwOnData = false): array {
-		$userData = $this->getUser($user);
+	protected function testValueLengths(array $properties, bool $throwOnData = false): void {
+		foreach ($properties as $property) {
+			if (strlen($property->getValue()) > 2048) {
+				if ($throwOnData) {
+					throw new InvalidArgumentException();
+				} else {
+					$property->setValue('');
+				}
+			}
+		}
+	}
+
+	protected function testPropertyScope(IAccountProperty $property, array $allowedScopes, bool $throwOnData): void {
+		if ($throwOnData && !in_array($property->getScope(), $allowedScopes, true)) {
+			throw new InvalidArgumentException('scope');
+		}
+
+		if (
+			$property->getScope() === self::SCOPE_PRIVATE
+			&& in_array($property->getName(), [self::PROPERTY_DISPLAYNAME, self::PROPERTY_EMAIL])
+		) {
+			if ($throwOnData) {
+				// v2-private is not available for these fields
+				throw new InvalidArgumentException('scope');
+			} else {
+				// default to local
+				$property->setScope(self::SCOPE_LOCAL);
+			}
+		} else {
+			// migrate scope values to the new format
+			// invalid scopes are mapped to a default value
+			$property->setScope(AccountProperty::mapScopeToV2($property->getScope()));
+		}
+	}
+
+	protected function sanitizePhoneNumberValue(IAccountProperty $property, bool $throwOnData = false) {
+		if ($property->getName() !== self::PROPERTY_PHONE) {
+			if ($throwOnData) {
+				throw new InvalidArgumentException(sprintf('sanitizePhoneNumberValue can only sanitize phone numbers, %s given', $property->getName()));
+			}
+			return;
+		}
+		if ($property->getValue() === '') {
+			return;
+		}
+		try {
+			$property->setValue($this->parsePhoneNumber($property->getValue()));
+		} catch (InvalidArgumentException $e) {
+			if ($throwOnData) {
+				throw $e;
+			}
+			$property->setValue('');
+		}
+	}
+
+	protected function sanitizeWebsite(IAccountProperty $property, bool $throwOnData = false) {
+		if ($property->getName() !== self::PROPERTY_WEBSITE) {
+			if ($throwOnData) {
+				throw new InvalidArgumentException(sprintf('sanitizeWebsite can only sanitize web domains, %s given', $property->getName()));
+			}
+		}
+		try {
+			$property->setValue($this->parseWebsite($property->getValue()));
+		} catch (InvalidArgumentException $e) {
+			if ($throwOnData) {
+				throw $e;
+			}
+			$property->setValue('');
+		}
+	}
+
+	protected function updateUser(IUser $user, array $data, bool $throwOnData = false): array {
+		$oldUserData = $this->getUser($user, false);
 		$updated = true;
 
-		if (isset($data[self::PROPERTY_PHONE]) && $data[self::PROPERTY_PHONE]['value'] !== '') {
-			// Sanitize null value.
-			$data[self::PROPERTY_PHONE]['value'] = $data[self::PROPERTY_PHONE]['value'] ?? '';
-
-			try {
-				$data[self::PROPERTY_PHONE]['value'] = $this->parsePhoneNumber($data[self::PROPERTY_PHONE]['value']);
-			} catch (\InvalidArgumentException $e) {
-				if ($throwOnData) {
-					throw $e;
-				}
-				$data[self::PROPERTY_PHONE]['value'] = '';
-			}
-		}
-
-		$this->testValueLengths($data);
-
-		if (isset($data[self::PROPERTY_WEBSITE]) && $data[self::PROPERTY_WEBSITE]['value'] !== '') {
-			try {
-				$data[self::PROPERTY_WEBSITE]['value'] = $this->parseWebsite($data[self::PROPERTY_WEBSITE]['value']);
-			} catch (\InvalidArgumentException $e) {
-				if ($throwOnData) {
-					throw $e;
-				}
-				$data[self::PROPERTY_WEBSITE]['value'] = '';
-			}
-		}
-
-		$allowedScopes = [
-			self::SCOPE_PRIVATE,
-			self::SCOPE_LOCAL,
-			self::SCOPE_FEDERATED,
-			self::SCOPE_PUBLISHED,
-			self::VISIBILITY_PRIVATE,
-			self::VISIBILITY_CONTACTS_ONLY,
-			self::VISIBILITY_PUBLIC,
-		];
-
-		$this->testPropertyScopes($data, $allowedScopes, $throwOnData);
-
-		if (empty($userData)) {
-			$this->insertNewUser($user, $data);
-		} elseif ($userData !== $data) {
-			$data = $this->checkEmailVerification($userData, $data, $user);
-			$data = $this->updateVerifyStatus($userData, $data);
+		if ($oldUserData !== $data) {
 			$this->updateExistingUser($user, $data);
 		} else {
 			// nothing needs to be done if new and old data set are the same
@@ -301,17 +274,15 @@ class AccountManager implements IAccountManager {
 
 	/**
 	 * get stored data from a given user
-	 *
-	 * @deprecated use getAccount instead to make sure migrated properties work correctly
 	 */
-	public function getUser(IUser $user, bool $insertIfNotExists = true): array {
+	protected function getUser(IUser $user, bool $insertIfNotExists = true): array {
 		$uid = $user->getUID();
 		$query = $this->connection->getQueryBuilder();
 		$query->select('data')
 			->from($this->table)
 			->where($query->expr()->eq('uid', $query->createParameter('uid')))
 			->setParameter('uid', $uid);
-		$result = $query->execute();
+		$result = $query->executeQuery();
 		$accountData = $result->fetchAll();
 		$result->closeCursor();
 
@@ -323,10 +294,8 @@ class AccountManager implements IAccountManager {
 			return $userData;
 		}
 
-		$userDataArray = json_decode($accountData[0]['data'], true);
-		$jsonError = json_last_error();
-		if ($userDataArray === null || $userDataArray === [] || $jsonError !== JSON_ERROR_NONE) {
-			$this->logger->critical("User data of $uid contained invalid JSON (error $jsonError), hence falling back to a default user record");
+		$userDataArray = $this->importFromJson($accountData[0]['data'], $uid);
+		if ($userDataArray === null || $userDataArray === []) {
 			return $this->buildDefaultUserRecord($user);
 		}
 
@@ -344,7 +313,7 @@ class AccountManager implements IAccountManager {
 		$matches = [];
 		foreach ($chunks as $chunk) {
 			$query->setParameter('values', $chunk, IQueryBuilder::PARAM_STR_ARRAY);
-			$result = $query->execute();
+			$result = $query->executeQuery();
 
 			while ($row = $result->fetch()) {
 				$matches[$row['uid']] = $row['value'];
@@ -369,100 +338,75 @@ class AccountManager implements IAccountManager {
 	/**
 	 * check if we need to ask the server for email verification, if yes we create a cronjob
 	 *
-	 * @param $oldData
-	 * @param $newData
-	 * @param IUser $user
-	 * @return array
 	 */
-	protected function checkEmailVerification($oldData, $newData, IUser $user): array {
-		if ($oldData[self::PROPERTY_EMAIL]['value'] !== $newData[self::PROPERTY_EMAIL]['value']) {
+	protected function checkEmailVerification(IAccount $updatedAccount, array $oldData): void {
+		try {
+			$property = $updatedAccount->getProperty(self::PROPERTY_EMAIL);
+		} catch (PropertyDoesNotExistException $e) {
+			return;
+		}
+		$oldMail = isset($oldData[self::PROPERTY_EMAIL]) ? $oldData[self::PROPERTY_EMAIL]['value']['value'] : '';
+		if ($oldMail !== $property->getValue()) {
 			$this->jobList->add(VerifyUserData::class,
 				[
 					'verificationCode' => '',
-					'data' => $newData[self::PROPERTY_EMAIL]['value'],
+					'data' => $property->getValue(),
 					'type' => self::PROPERTY_EMAIL,
-					'uid' => $user->getUID(),
+					'uid' => $updatedAccount->getUser()->getUID(),
 					'try' => 0,
 					'lastRun' => time()
 				]
 			);
-			$newData[self::PROPERTY_EMAIL]['verified'] = self::VERIFICATION_IN_PROGRESS;
-		}
 
-		return $newData;
+
+
+
+			$property->setVerified(self::VERIFICATION_IN_PROGRESS);
+		}
 	}
 
 	/**
 	 * make sure that all expected data are set
 	 *
-	 * @param array $userData
-	 * @return array
 	 */
-	protected function addMissingDefaultValues(array $userData) {
-		foreach ($userData as $key => $value) {
-			if (!isset($userData[$key]['verified'])) {
-				$userData[$key]['verified'] = self::NOT_VERIFIED;
+	protected function addMissingDefaultValues(array $userData): array {
+		foreach ($userData as $i => $value) {
+			if (!isset($value['verified'])) {
+				$userData[$i]['verified'] = self::NOT_VERIFIED;
 			}
 		}
 
 		return $userData;
 	}
 
-	/**
-	 * reset verification status if personal data changed
-	 *
-	 * @param array $oldData
-	 * @param array $newData
-	 * @return array
-	 */
-	protected function updateVerifyStatus(array $oldData, array $newData): array {
+	protected function updateVerificationStatus(IAccount $updatedAccount, array $oldData): void {
+		static $propertiesVerifiableByLookupServer = [
+			self::PROPERTY_TWITTER,
+			self::PROPERTY_WEBSITE,
+			self::PROPERTY_EMAIL,
+		];
 
-		// which account was already verified successfully?
-		$twitterVerified = isset($oldData[self::PROPERTY_TWITTER]['verified']) && $oldData[self::PROPERTY_TWITTER]['verified'] === self::VERIFIED;
-		$websiteVerified = isset($oldData[self::PROPERTY_WEBSITE]['verified']) && $oldData[self::PROPERTY_WEBSITE]['verified'] === self::VERIFIED;
-		$emailVerified = isset($oldData[self::PROPERTY_EMAIL]['verified']) && $oldData[self::PROPERTY_EMAIL]['verified'] === self::VERIFIED;
-
-		// keep old verification status if we don't have a new one
-		if (!isset($newData[self::PROPERTY_TWITTER]['verified'])) {
-			// keep old verification status if value didn't changed and an old value exists
-			$keepOldStatus = $newData[self::PROPERTY_TWITTER]['value'] === $oldData[self::PROPERTY_TWITTER]['value'] && isset($oldData[self::PROPERTY_TWITTER]['verified']);
-			$newData[self::PROPERTY_TWITTER]['verified'] = $keepOldStatus ? $oldData[self::PROPERTY_TWITTER]['verified'] : self::NOT_VERIFIED;
+		foreach ($propertiesVerifiableByLookupServer as $propertyName) {
+			try {
+				$property = $updatedAccount->getProperty($propertyName);
+			} catch (PropertyDoesNotExistException $e) {
+				continue;
+			}
+			$wasVerified = isset($oldData[$propertyName])
+				&& isset($oldData[$propertyName]['verified'])
+				&& $oldData[$propertyName]['verified'] === self::VERIFIED;
+			if ((!isset($oldData[$propertyName])
+					|| !isset($oldData[$propertyName]['value'])
+					|| $property->getValue() !== $oldData[$propertyName]['value'])
+				&& ($property->getVerified() !== self::NOT_VERIFIED
+					|| $wasVerified)
+				) {
+				$property->setVerified(self::NOT_VERIFIED);
+			}
 		}
-
-		if (!isset($newData[self::PROPERTY_WEBSITE]['verified'])) {
-			// keep old verification status if value didn't changed and an old value exists
-			$keepOldStatus = $newData[self::PROPERTY_WEBSITE]['value'] === $oldData[self::PROPERTY_WEBSITE]['value'] && isset($oldData[self::PROPERTY_WEBSITE]['verified']);
-			$newData[self::PROPERTY_WEBSITE]['verified'] = $keepOldStatus ? $oldData[self::PROPERTY_WEBSITE]['verified'] : self::NOT_VERIFIED;
-		}
-
-		if (!isset($newData[self::PROPERTY_EMAIL]['verified'])) {
-			// keep old verification status if value didn't changed and an old value exists
-			$keepOldStatus = $newData[self::PROPERTY_EMAIL]['value'] === $oldData[self::PROPERTY_EMAIL]['value'] && isset($oldData[self::PROPERTY_EMAIL]['verified']);
-			$newData[self::PROPERTY_EMAIL]['verified'] = $keepOldStatus ? $oldData[self::PROPERTY_EMAIL]['verified'] : self::VERIFICATION_IN_PROGRESS;
-		}
-
-		// reset verification status if a value from a previously verified data was changed
-		if ($twitterVerified &&
-			$oldData[self::PROPERTY_TWITTER]['value'] !== $newData[self::PROPERTY_TWITTER]['value']
-		) {
-			$newData[self::PROPERTY_TWITTER]['verified'] = self::NOT_VERIFIED;
-		}
-
-		if ($websiteVerified &&
-			$oldData[self::PROPERTY_WEBSITE]['value'] !== $newData[self::PROPERTY_WEBSITE]['value']
-		) {
-			$newData[self::PROPERTY_WEBSITE]['verified'] = self::NOT_VERIFIED;
-		}
-
-		if ($emailVerified &&
-			$oldData[self::PROPERTY_EMAIL]['value'] !== $newData[self::PROPERTY_EMAIL]['value']
-		) {
-			$newData[self::PROPERTY_EMAIL]['verified'] = self::NOT_VERIFIED;
-		}
-
-		return $newData;
 	}
 
+
 	/**
 	 * add new user to accounts table
 	 *
@@ -471,7 +415,7 @@ class AccountManager implements IAccountManager {
 	 */
 	protected function insertNewUser(IUser $user, array $data): void {
 		$uid = $user->getUID();
-		$jsonEncodedData = json_encode($data);
+		$jsonEncodedData = $this->prepareJson($data);
 		$query = $this->connection->getQueryBuilder();
 		$query->insert($this->table)
 			->values(
@@ -480,12 +424,55 @@ class AccountManager implements IAccountManager {
 					'data' => $query->createNamedParameter($jsonEncodedData),
 				]
 			)
-			->execute();
+			->executeStatement();
 
 		$this->deleteUserData($user);
 		$this->writeUserData($user, $data);
 	}
 
+	protected function prepareJson(array $data): string {
+		$preparedData = [];
+		foreach ($data as $dataRow) {
+			$propertyName = $dataRow['name'];
+			unset($dataRow['name']);
+			if (!$this->isCollection($propertyName)) {
+				$preparedData[$propertyName] = $dataRow;
+				continue;
+			}
+			if (!isset($preparedData[$propertyName])) {
+				$preparedData[$propertyName] = [];
+			}
+			$preparedData[$propertyName][] = $dataRow;
+		}
+		return json_encode($preparedData);
+	}
+
+	protected function importFromJson(string $json, string $userId): ?array {
+		$result = [];
+		$jsonArray = json_decode($json, true);
+		$jsonError = json_last_error();
+		if ($jsonError !== JSON_ERROR_NONE) {
+			$this->logger->critical(
+				'User data of {uid} contained invalid JSON (error {json_error}), hence falling back to a default user record',
+				[
+					'uid' => $userId,
+					'json_error' => $jsonError
+				]
+			);
+			return null;
+		}
+		foreach ($jsonArray as $propertyName => $row) {
+			if (!$this->isCollection($propertyName)) {
+				$result[] = array_merge($row, ['name' => $propertyName]);
+				continue;
+			}
+			foreach ($row as $singleRow) {
+				$result[] = array_merge($singleRow, ['name' => $propertyName]);
+			}
+		}
+		return $result;
+	}
+
 	/**
 	 * update existing user in accounts table
 	 *
@@ -494,12 +481,12 @@ class AccountManager implements IAccountManager {
 	 */
 	protected function updateExistingUser(IUser $user, array $data): void {
 		$uid = $user->getUID();
-		$jsonEncodedData = json_encode($data);
+		$jsonEncodedData = $this->prepareJson($data);
 		$query = $this->connection->getQueryBuilder();
 		$query->update($this->table)
 			->set('data', $query->createNamedParameter($jsonEncodedData))
 			->where($query->expr()->eq('uid', $query->createNamedParameter($uid)))
-			->execute();
+			->executeStatement();
 
 		$this->deleteUserData($user);
 		$this->writeUserData($user, $data);
@@ -518,19 +505,16 @@ class AccountManager implements IAccountManager {
 		$this->writeUserDataProperties($query, $data);
 	}
 
-	protected function writeUserDataProperties(IQueryBuilder $query, array $data, string $parentPropertyName = null): void {
-		foreach ($data as $propertyName => $property) {
-			if ($this->isCollection($propertyName)) {
-				$this->writeUserDataProperties($query, $property, $propertyName);
-				continue;
-			}
-			if (($parentPropertyName ?? $propertyName) === self::PROPERTY_AVATAR) {
+	protected function writeUserDataProperties(IQueryBuilder $query, array $data): void {
+		foreach ($data as $property) {
+			if ($property['name'] === self::PROPERTY_AVATAR) {
 				continue;
 			}
 
-			$query->setParameter('name', $parentPropertyName ?? $propertyName)
+
+			$query->setParameter('name', $property['name'])
 				->setParameter('value', $property['value'] ?? '');
-			$query->execute();
+			$query->executeStatement();
 		}
 	}
 
@@ -542,53 +526,80 @@ class AccountManager implements IAccountManager {
 	 */
 	protected function buildDefaultUserRecord(IUser $user) {
 		return [
-			self::PROPERTY_DISPLAYNAME =>
-				[
-					'value' => $user->getDisplayName(),
-					'scope' => self::SCOPE_FEDERATED,
-					'verified' => self::NOT_VERIFIED,
-				],
-			self::PROPERTY_ADDRESS =>
-				[
-					'value' => '',
-					'scope' => self::SCOPE_LOCAL,
-					'verified' => self::NOT_VERIFIED,
-				],
-			self::PROPERTY_WEBSITE =>
-				[
-					'value' => '',
-					'scope' => self::SCOPE_LOCAL,
-					'verified' => self::NOT_VERIFIED,
-				],
-			self::PROPERTY_EMAIL =>
-				[
-					'value' => $user->getEMailAddress(),
-					'scope' => self::SCOPE_FEDERATED,
-					'verified' => self::NOT_VERIFIED,
-				],
-			self::PROPERTY_AVATAR =>
-				[
-					'scope' => self::SCOPE_FEDERATED
-				],
-			self::PROPERTY_PHONE =>
-				[
-					'value' => '',
-					'scope' => self::SCOPE_LOCAL,
-					'verified' => self::NOT_VERIFIED,
-				],
-			self::PROPERTY_TWITTER =>
-				[
-					'value' => '',
-					'scope' => self::SCOPE_LOCAL,
-					'verified' => self::NOT_VERIFIED,
-				],
+
+			[
+				'name' => self::PROPERTY_DISPLAYNAME,
+				'value' => $user->getDisplayName(),
+				'scope' => self::SCOPE_FEDERATED,
+				'verified' => self::NOT_VERIFIED,
+			],
+
+			[
+				'name' => self::PROPERTY_ADDRESS,
+				'value' => '',
+				'scope' => self::SCOPE_LOCAL,
+				'verified' => self::NOT_VERIFIED,
+			],
+
+			[
+				'name' => self::PROPERTY_WEBSITE,
+				'value' => '',
+				'scope' => self::SCOPE_LOCAL,
+				'verified' => self::NOT_VERIFIED,
+			],
+
+			[
+				'name' => self::PROPERTY_EMAIL,
+				'value' => $user->getEMailAddress(),
+				'scope' => self::SCOPE_FEDERATED,
+				'verified' => self::NOT_VERIFIED,
+			],
+
+			[
+				'name' => self::PROPERTY_AVATAR,
+				'scope' => self::SCOPE_FEDERATED
+			],
+
+			[
+				'name' => self::PROPERTY_PHONE,
+				'value' => '',
+				'scope' => self::SCOPE_LOCAL,
+				'verified' => self::NOT_VERIFIED,
+			],
+
+			[
+				'name' => self::PROPERTY_TWITTER,
+				'value' => '',
+				'scope' => self::SCOPE_LOCAL,
+				'verified' => self::NOT_VERIFIED,
+			],
+
 		];
 	}
 
+	private function arrayDataToCollection(IAccount $account, array $data): IAccountPropertyCollection {
+		$collection = $account->getPropertyCollection($data['name']);
+
+		$p = new AccountProperty(
+			$data['name'],
+			$data['value'] ?? '',
+			$data['scope'] ?? self::SCOPE_LOCAL,
+			$data['verified'] ?? self::NOT_VERIFIED,
+			''
+		);
+		$collection->addProperty($p);
+
+		return $collection;
+	}
+
 	private function parseAccountData(IUser $user, $data): Account {
 		$account = new Account($user);
-		foreach ($data as $property => $accountData) {
-			$account->setProperty($property, $accountData['value'] ?? '', $accountData['scope'] ?? self::SCOPE_LOCAL, $accountData['verified'] ?? self::NOT_VERIFIED);
+		foreach ($data as $accountData) {
+			if ($this->isCollection($accountData['name'])) {
+				$account->setPropertyCollection($this->arrayDataToCollection($account, $accountData));
+			} else {
+				$account->setProperty($accountData['name'], $accountData['value'] ?? '', $accountData['scope'] ?? self::SCOPE_LOCAL, $accountData['verified'] ?? self::NOT_VERIFIED);
+			}
 		}
 		return $account;
 	}
@@ -598,10 +609,42 @@ class AccountManager implements IAccountManager {
 	}
 
 	public function updateAccount(IAccount $account): void {
-		$data = [];
+		$this->testValueLengths(iterator_to_array($account->getAllProperties()), true);
+		try {
+			$property = $account->getProperty(self::PROPERTY_PHONE);
+			$this->sanitizePhoneNumberValue($property);
+		} catch (PropertyDoesNotExistException $e) {
+			//  valid case, nothing to do
+		}
 
-		foreach ($account->getProperties() as $property) {
-			$data[$property->getName()] = [
+		try {
+			$property = $account->getProperty(self::PROPERTY_WEBSITE);
+			$this->sanitizeWebsite($property);
+		} catch (PropertyDoesNotExistException $e) {
+			//  valid case, nothing to do
+		}
+
+		static $allowedScopes = [
+			self::SCOPE_PRIVATE,
+			self::SCOPE_LOCAL,
+			self::SCOPE_FEDERATED,
+			self::SCOPE_PUBLISHED,
+			self::VISIBILITY_PRIVATE,
+			self::VISIBILITY_CONTACTS_ONLY,
+			self::VISIBILITY_PUBLIC,
+		];
+		foreach ($account->getAllProperties() as $property) {
+			$this->testPropertyScope($property, $allowedScopes, true);
+		}
+
+		$oldData = $this->getUser($account->getUser(), false);
+		$this->updateVerificationStatus($account, $oldData);
+		$this->checkEmailVerification($account, $oldData);
+
+		$data = [];
+		foreach ($account->getAllProperties() as $property) {
+			$data[] = [
+				'name' => $property->getName(),
 				'value' => $property->getValue(),
 				'scope' => $property->getScope(),
 				'verified' => $property->getVerified(),

lib/private/Accounts/AccountPropertyCollection.php

@@ -27,6 +27,7 @@ declare(strict_types=1);
 namespace OC\Accounts;
 
 use InvalidArgumentException;
+use OCP\Accounts\IAccountManager;
 use OCP\Accounts\IAccountProperty;
 use OCP\Accounts\IAccountPropertyCollection;
 
@@ -63,6 +64,18 @@ class AccountPropertyCollection implements IAccountPropertyCollection {
 		return $this;
 	}
 
+	public function addPropertyWithDefaults(string $value): IAccountPropertyCollection {
+		$property = new AccountProperty(
+			$this->collectionName,
+			$value,
+			IAccountManager::SCOPE_LOCAL,
+			IAccountManager::NOT_VERIFIED,
+			''
+		);
+		$this->addProperty($property);
+		return $this;
+	}
+
 	public function removeProperty(IAccountProperty $property): IAccountPropertyCollection {
 		$ref = array_search($property, $this->properties, true);
 		if ($ref !== false) {

lib/private/Accounts/Hooks.php

@@ -25,66 +25,55 @@
 namespace OC\Accounts;
 
 use OCP\Accounts\IAccountManager;
+use OCP\Accounts\PropertyDoesNotExistException;
+use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventListener;
 use OCP\IUser;
+use OCP\User\Events\UserChangedEvent;
 use Psr\Log\LoggerInterface;
 
-class Hooks {
+class Hooks implements IEventListener {
 
-	/** @var AccountManager|null */
+	/** @var IAccountManager */
 	private $accountManager;
-
 	/** @var LoggerInterface */
 	private $logger;
 
-	public function __construct(LoggerInterface $logger) {
+	public function __construct(LoggerInterface $logger, IAccountManager $accountManager) {
 		$this->logger = $logger;
+		$this->accountManager = $accountManager;
 	}
 
 	/**
 	 * update accounts table if email address or display name was changed from outside
-	 *
-	 * @param array $params
 	 */
-	public function changeUserHook($params) {
-		$accountManager = $this->getAccountManager();
+	public function changeUserHook(IUser $user, string $feature, $newValue): void {
+		$account = $this->accountManager->getAccount($user);
 
-		/** @var IUser $user */
-		$user = isset($params['user']) ? $params['user'] : null;
-		$feature = isset($params['feature']) ? $params['feature'] : null;
-		$newValue = isset($params['value']) ? $params['value'] : null;
-
-		if (is_null($user) || is_null($feature) || is_null($newValue)) {
-			$this->logger->warning('Missing expected parameters in change user hook');
+		try {
+			switch ($feature) {
+				case 'eMailAddress':
+					$property = $account->getProperty(IAccountManager::PROPERTY_EMAIL);
+					break;
+				case 'displayName':
+					$property = $account->getProperty(IAccountManager::PROPERTY_DISPLAYNAME);
+					break;
+			}
+		} catch (PropertyDoesNotExistException $e) {
+			$this->logger->debug($e->getMessage(), ['exception' => $e]);
 			return;
 		}
 
-		$accountData = $accountManager->getUser($user);
-
-		switch ($feature) {
-			case 'eMailAddress':
-				if ($accountData[IAccountManager::PROPERTY_EMAIL]['value'] !== $newValue) {
-					$accountData[IAccountManager::PROPERTY_EMAIL]['value'] = $newValue;
-					$accountManager->updateUser($user, $accountData);
-				}
-				break;
-			case 'displayName':
-				if ($accountData[IAccountManager::PROPERTY_DISPLAYNAME]['value'] !== $newValue) {
-					$accountData[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $newValue;
-					$accountManager->updateUser($user, $accountData);
-				}
-				break;
+		if (isset($property) && $property->getValue() !== (string)$newValue) {
+			$property->setValue($newValue);
+			$this->accountManager->updateAccount($account);
 		}
 	}
 
-	/**
-	 * return instance of accountManager
-	 *
-	 * @return AccountManager
-	 */
-	protected function getAccountManager(): AccountManager {
-		if ($this->accountManager === null) {
-			$this->accountManager = \OC::$server->query(AccountManager::class);
+	public function handle(Event $event): void {
+		if (!$event instanceof UserChangedEvent) {
+			return;
 		}
-		return $this->accountManager;
+		$this->changeUserHook($event->getUser(), $event->getFeature(), $event->getValue());
 	}
 }

lib/private/Avatar/AvatarManager.php

@@ -39,6 +39,7 @@ use OC\KnownUser\KnownUserService;
 use OC\User\Manager;
 use OC\User\NoUserException;
 use OCP\Accounts\IAccountManager;
+use OCP\Accounts\PropertyDoesNotExistException;
 use OCP\Files\IAppData;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
@@ -126,9 +127,13 @@ class AvatarManager implements IAvatarManager {
 			$folder = $this->appData->newFolder($userId);
 		}
 
-		$account = $this->accountManager->getAccount($user);
-		$avatarProperties = $account->getProperty(IAccountManager::PROPERTY_AVATAR);
-		$avatarScope = $avatarProperties->getScope();
+		try {
+			$account = $this->accountManager->getAccount($user);
+			$avatarProperties = $account->getProperty(IAccountManager::PROPERTY_AVATAR);
+			$avatarScope = $avatarProperties->getScope();
+		} catch (PropertyDoesNotExistException $e) {
+			$avatarScope = '';
+		}
 
 		if (
 			// v2-private scope hides the avatar from public access and from unknown users

lib/public/Accounts/IAccount.php

@@ -94,9 +94,10 @@ interface IAccount extends \JsonSerializable {
 	/**
 	 * Returns the requestes propery collection (multi-value properties)
 	 *
+	 * @throws PropertyDoesNotExistException against invalid collection name
 	 * @since 22.0.0
 	 */
-	public function getPropertyCollection(string $propertyCollection): IAccountPropertyCollection;
+	public function getPropertyCollection(string $propertyCollectionName): IAccountPropertyCollection;
 
 	/**
 	 * Get all properties that match the provided filters for scope and verification status

lib/public/Accounts/IAccountPropertyCollection.php

@@ -68,6 +68,14 @@ interface IAccountPropertyCollection extends JsonSerializable {
 	 */
 	public function addProperty(IAccountProperty $property): IAccountPropertyCollection;
 
+	/**
+	 * adds a property to this collection with only specifying the value
+	 *
+	 * @throws InvalidArgumentException
+	 * @since 22.0.0
+	 */
+	public function addPropertyWithDefaults(string $value): IAccountPropertyCollection;
+
 	/**
 	 * removes a property of this collection
 	 *

tests/lib/Accounts/AccountManagerTest.php

@@ -108,65 +108,191 @@ class AccountManagerTest extends TestCase {
 			[
 				'user' => $this->makeUser('j.doe', 'Jane Doe', 'jane.doe@acme.com'),
 				'data' => [
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Jane Doe', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_EMAIL => ['value' => 'jane.doe@acme.com', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_ADDRESS => ['value' => 'some street', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://acme.com', 'scope' => IAccountManager::SCOPE_PRIVATE],
+					[
+						'name' => IAccountManager::PROPERTY_DISPLAYNAME,
+						'value' => 'Jane Doe',
+						'scope' => IAccountManager::SCOPE_PUBLISHED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_EMAIL,
+						'value' => 'jane.doe@acme.com',
+						'scope' => IAccountManager::SCOPE_LOCAL
+					],
+					[
+						'name' => IAccountManager::PROPERTY_TWITTER,
+						'value' => '@sometwitter',
+						'scope' => IAccountManager::SCOPE_PUBLISHED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_PHONE,
+						'value' => '+491601231212',
+						'scope' => IAccountManager::SCOPE_FEDERATED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_ADDRESS,
+						'value' => 'some street',
+						'scope' => IAccountManager::SCOPE_LOCAL
+					],
+					[
+						'name' => IAccountManager::PROPERTY_WEBSITE,
+						'value' => 'https://acme.com',
+						'scope' => IAccountManager::SCOPE_PRIVATE
+					],
 				],
 			],
 			[
 				'user' => $this->makeUser('a.allison', 'Alice Allison', 'a.allison@example.org'),
 				'data' => [
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Alice Allison', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_EMAIL => ['value' => 'a.allison@example.org', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_TWITTER => ['value' => '@a_alice', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491602312121', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_ADDRESS => ['value' => 'Dundee Road 45', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::SCOPE_LOCAL],
+					[
+						'name' => IAccountManager::PROPERTY_DISPLAYNAME,
+						'value' => 'Alice Allison',
+						'scope' => IAccountManager::SCOPE_LOCAL
+					],
+					[
+						'name' => IAccountManager::PROPERTY_EMAIL,
+						'value' => 'a.allison@example.org',
+						'scope' => IAccountManager::SCOPE_LOCAL
+					],
+					[
+						'name' => IAccountManager::PROPERTY_TWITTER,
+						'value' => '@a_alice',
+						'scope' => IAccountManager::SCOPE_FEDERATED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_PHONE,
+						'value' => '+491602312121',
+						'scope' => IAccountManager::SCOPE_LOCAL
+					],
+					[
+						'name' => IAccountManager::PROPERTY_ADDRESS,
+						'value' => 'Dundee Road 45',
+						'scope' => IAccountManager::SCOPE_LOCAL
+					],
+					[
+						'name' => IAccountManager::PROPERTY_WEBSITE,
+						'value' => 'https://example.org',
+						'scope' => IAccountManager::SCOPE_LOCAL
+					],
 				],
 			],
 			[
 				'user' => $this->makeUser('b32c5a5b-1084-4380-8856-e5223b16de9f', 'Armel Oliseh', 'oliseh@example.com'),
 				'data' => [
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Armel Oliseh', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_EMAIL => ['value' => 'oliseh@example.com', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_TWITTER => ['value' => '', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491603121212', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_ADDRESS => ['value' => 'Sunflower Blvd. 77', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.com', 'scope' => IAccountManager::SCOPE_PUBLISHED],
+					[
+						'name' => IAccountManager::PROPERTY_DISPLAYNAME,
+						'value' => 'Armel Oliseh',
+						'scope' => IAccountManager::SCOPE_PUBLISHED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_EMAIL,
+						'value' => 'oliseh@example.com',
+						'scope' => IAccountManager::SCOPE_PUBLISHED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_TWITTER,
+						'value' => '',
+						'scope' => IAccountManager::SCOPE_LOCAL
+					],
+					[
+						'name' => IAccountManager::PROPERTY_PHONE,
+						'value' => '+491603121212',
+						'scope' => IAccountManager::SCOPE_PUBLISHED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_ADDRESS,
+						'value' => 'Sunflower Blvd. 77',
+						'scope' => IAccountManager::SCOPE_PUBLISHED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_WEBSITE,
+						'value' => 'https://example.com',
+						'scope' => IAccountManager::SCOPE_PUBLISHED
+					],
 				],
 			],
 			[
 				'user' => $this->makeUser('31b5316a-9b57-4b17-970a-315a4cbe73eb', 'K. Cheng', 'cheng@emca.com'),
 				'data' => [
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'K. Cheng', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_EMAIL => ['value' => 'cheng@emca.com', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_TWITTER => ['value' => '', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+71601212123', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_ADDRESS => ['value' => 'Pinapple Street 22', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://emca.com', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::COLLECTION_EMAIL => [
-						['value' => 'k.cheng@emca.com', 'scope' => IAccountManager::SCOPE_LOCAL],
-						['value' => 'kai.cheng@emca.com', 'scope' => IAccountManager::SCOPE_LOCAL],
+					[
+						'name' => IAccountManager::PROPERTY_DISPLAYNAME,
+						'value' => 'K. Cheng',
+						'scope' => IAccountManager::SCOPE_FEDERATED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_EMAIL,
+						'value' => 'cheng@emca.com',
+						'scope' => IAccountManager::SCOPE_FEDERATED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_TWITTER,
+						'value' => '', '
+						scope' => IAccountManager::SCOPE_LOCAL
+					],
+					[
+						'name' => IAccountManager::PROPERTY_PHONE,
+						'value' => '+71601212123',
+						'scope' => IAccountManager::SCOPE_LOCAL
+					],
+					[
+						'name' => IAccountManager::PROPERTY_ADDRESS,
+						'value' => 'Pinapple Street 22',
+						'scope' => IAccountManager::SCOPE_LOCAL
+					],
+					[
+						'name' => IAccountManager::PROPERTY_WEBSITE,
+						'value' => 'https://emca.com',
+						'scope' => IAccountManager::SCOPE_FEDERATED
+					],
+					[
+						'name' => IAccountManager::COLLECTION_EMAIL,
+						'value' => 'k.cheng@emca.com',
+						'scope' => IAccountManager::SCOPE_LOCAL
+					],
+					[
+						'name' => IAccountManager::COLLECTION_EMAIL,
+						'value' => 'kai.cheng@emca.com',
+						'scope' => IAccountManager::SCOPE_LOCAL
 					],
 				],
 			],
 			[
 				'user' => $this->makeUser('goodpal@elpmaxe.org', 'Goodpal, Kim', 'goodpal@elpmaxe.org'),
 				'data' => [
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Goodpal, Kim', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_EMAIL => ['value' => 'goodpal@elpmaxe.org', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_TWITTER => ['value' => '', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+71602121231', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_ADDRESS => ['value' => 'Octopus Ave 17', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://elpmaxe.org', 'scope' => IAccountManager::SCOPE_PUBLISHED],
+					[
+						'name' => IAccountManager::PROPERTY_DISPLAYNAME,
+						'value' => 'Goodpal, Kim',
+						'scope' => IAccountManager::SCOPE_PUBLISHED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_EMAIL,
+						'value' => 'goodpal@elpmaxe.org',
+						'scope' => IAccountManager::SCOPE_PUBLISHED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_TWITTER,
+						'value' => '',
+						'scope' => IAccountManager::SCOPE_LOCAL
+					],
+					[
+						'name' => IAccountManager::PROPERTY_PHONE,
+						'value' => '+71602121231',
+						'scope' => IAccountManager::SCOPE_FEDERATED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_ADDRESS,
+						'value' => 'Octopus Ave 17',
+						'scope' => IAccountManager::SCOPE_FEDERATED
+					],
+					[
+						'name' => IAccountManager::PROPERTY_WEBSITE,
+						'value' => 'https://elpmaxe.org',
+						'scope' => IAccountManager::SCOPE_PUBLISHED
+					],
 				],
 			],
 		];
 		foreach ($users as $userInfo) {
-			$this->accountManager->updateUser($userInfo['user'], $userInfo['data'], false);
+			$this->invokePrivate($this->accountManager, 'updateUser', [$userInfo['user'], $userInfo['data'], false]);
 		}
 	}
 
@@ -198,7 +324,7 @@ class AccountManagerTest extends TestCase {
 	 * @param bool $updateExisting
 	 */
 	public function testUpdateUser($newData, $oldData, $insertNew, $updateExisting) {
-		$accountManager = $this->getInstance(['getUser', 'insertNewUser', 'updateExistingUser', 'updateVerifyStatus', 'checkEmailVerification']);
+		$accountManager = $this->getInstance(['getUser', 'insertNewUser', 'updateExistingUser']);
 		/** @var IUser $user */
 		$user = $this->createMock(IUser::class);
 
@@ -206,10 +332,6 @@ class AccountManagerTest extends TestCase {
 		$accountManager->expects($this->once())->method('getUser')->with($user)->willReturn($oldData);
 
 		if ($updateExisting) {
-			$accountManager->expects($this->once())->method('checkEmailVerification')
-				->with($oldData, $newData, $user)->willReturn($newData);
-			$accountManager->expects($this->once())->method('updateVerifyStatus')
-				->with($oldData, $newData)->willReturn($newData);
 			$accountManager->expects($this->once())->method('updateExistingUser')
 				->with($user, $newData);
 			$accountManager->expects($this->never())->method('insertNewUser');
@@ -222,8 +344,6 @@ class AccountManagerTest extends TestCase {
 
 		if (!$insertNew && !$updateExisting) {
 			$accountManager->expects($this->never())->method('updateExistingUser');
-			$accountManager->expects($this->never())->method('checkEmailVerification');
-			$accountManager->expects($this->never())->method('updateVerifyStatus');
 			$accountManager->expects($this->never())->method('insertNewUser');
 			$this->eventDispatcher->expects($this->never())->method('dispatch');
 		} else {
@@ -239,231 +359,26 @@ class AccountManagerTest extends TestCase {
 				);
 		}
 
-		$accountManager->updateUser($user, $newData);
+		$this->invokePrivate($accountManager, 'updateUser', [$user, $newData]);
 	}
 
 	public function dataTrueFalse() {
 		return [
+			#$newData | $oldData | $insertNew | $updateExisting
 			[['myProperty' => ['value' => 'newData']], ['myProperty' => ['value' => 'oldData']], false, true],
-			[['myProperty' => ['value' => 'newData']], [], true, false],
 			[['myProperty' => ['value' => 'oldData']], ['myProperty' => ['value' => 'oldData']], false, false]
 		];
 	}
 
-	public function updateUserSetScopeProvider() {
-		return [
-			// regular scope switching
-			[
-				[
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_AVATAR => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_ADDRESS => ['value' => 'some street', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::SCOPE_PRIVATE],
-				],
-				[
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PRIVATE],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_ADDRESS => ['value' => 'some street', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-				],
-				[
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PRIVATE],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_ADDRESS => ['value' => 'some street', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-				],
-			],
-			// legacy scope mapping, the given visibility values get converted to scopes
-			[
-				[
-					IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::SCOPE_PRIVATE],
-				],
-				[
-					IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::VISIBILITY_PUBLIC],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::VISIBILITY_CONTACTS_ONLY],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::VISIBILITY_PRIVATE],
-				],
-				[
-					IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::SCOPE_LOCAL],
-				],
-			],
-			// invalid or unsupported scope values get converted to SCOPE_LOCAL
-			[
-				[
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_FEDERATED],
-				],
-				[
-					// SCOPE_PRIVATE is not allowed for display name and email
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_PRIVATE],
-					IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_PRIVATE],
-					IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_LOCAL],
-				],
-				[
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_LOCAL],
-				],
-				false, false,
-			],
-			// illegal scope values
-			[
-				[
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_FEDERATED],
-					IAccountManager::PROPERTY_ADDRESS => ['value' => 'some street', 'scope' => IAccountManager::SCOPE_LOCAL],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::SCOPE_PRIVATE],
-				],
-				[
-					IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => ''],
-					IAccountManager::PROPERTY_ADDRESS => ['value' => 'some street', 'scope' => 'v2-invalid'],
-					IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => 'invalid'],
-				],
-				[],
-				true, true
-			],
-			// invalid or unsupported scope values throw an exception when passing $throwOnData=true
-			[
-				[IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_PUBLISHED]],
-				[IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_PRIVATE]],
-				null,
-				// throw exception
-				true, true,
-			],
-			[
-				[IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_PUBLISHED]],
-				[IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_PRIVATE]],
-				null,
-				// throw exception
-				true, true,
-			],
-			[
-				[IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED]],
-				[IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => 'invalid']],
-				null,
-				// throw exception
-				true, true,
-			],
-		];
-	}
-
-	/**
-	 * @dataProvider updateUserSetScopeProvider
-	 */
-	public function testUpdateUserSetScope($oldData, $newData, $savedData, $throwOnData = true, $expectedThrow = false) {
-		$accountManager = $this->getInstance(['getUser', 'insertNewUser', 'updateExistingUser', 'updateVerifyStatus', 'checkEmailVerification']);
-		/** @var IUser $user */
-		$user = $this->createMock(IUser::class);
-
-		$accountManager->expects($this->once())->method('getUser')->with($user)->willReturn($oldData);
-
-		if ($expectedThrow) {
-			$accountManager->expects($this->never())->method('updateExistingUser');
-			$this->expectException(\InvalidArgumentException::class);
-			$this->expectExceptionMessage('scope');
-		} else {
-			$accountManager->expects($this->once())->method('checkEmailVerification')
-				->with($oldData, $savedData, $user)->willReturn($savedData);
-			$accountManager->expects($this->once())->method('updateVerifyStatus')
-				->with($oldData, $savedData)->willReturn($savedData);
-			$accountManager->expects($this->once())->method('updateExistingUser')
-				->with($user, $savedData);
-			$accountManager->expects($this->never())->method('insertNewUser');
-		}
-
-		$accountManager->updateUser($user, $newData, $throwOnData);
-	}
-
-	/**
-	 * @dataProvider dataTestGetUser
-	 *
-	 * @param string $setUser
-	 * @param array $setData
-	 * @param IUser $askUser
-	 * @param array $expectedData
-	 * @param bool $userAlreadyExists
-	 */
-	public function testGetUser($setUser, $setData, $askUser, $expectedData, $userAlreadyExists) {
-		$accountManager = $this->getInstance(['buildDefaultUserRecord', 'insertNewUser', 'addMissingDefaultValues']);
-		if (!$userAlreadyExists) {
-			$accountManager->expects($this->once())->method('buildDefaultUserRecord')
-				->with($askUser)->willReturn($expectedData);
-			$accountManager->expects($this->once())->method('insertNewUser')
-				->with($askUser, $expectedData);
-		}
-
-		if (empty($expectedData)) {
-			$accountManager->expects($this->never())->method('addMissingDefaultValues');
-		} else {
-			$accountManager->expects($this->once())->method('addMissingDefaultValues')->with($expectedData)
-				->willReturn($expectedData);
-		}
-
-		$this->addDummyValuesToTable($setUser, $setData);
-		$this->assertEquals($expectedData,
-			$accountManager->getUser($askUser)
-		);
-	}
-
-	public function dataTestGetUser() {
-		$user1 = $this->getMockBuilder(IUser::class)->getMock();
-		$user1->expects($this->any())->method('getUID')->willReturn('user1');
-		$user2 = $this->getMockBuilder(IUser::class)->getMock();
-		$user2->expects($this->any())->method('getUID')->willReturn('user2');
-		return [
-			['user1', ['key' => 'value'], $user1, ['key' => 'value'], true],
-			['user1', ['key' => 'value'], $user2, [], false],
-		];
-	}
-
-	public function testUpdateExistingUser() {
-		$user = $this->getMockBuilder(IUser::class)->getMock();
-		$user->expects($this->atLeastOnce())->method('getUID')->willReturn('uid');
-		$oldData = ['key' => ['value' => 'value']];
-		$newData = ['newKey' => ['value' => 'newValue']];
-
-		$this->addDummyValuesToTable('uid', $oldData);
-		$this->invokePrivate($this->accountManager, 'updateExistingUser', [$user, $newData]);
-		$newDataFromTable = $this->getDataFromTable('uid');
-		$this->assertEquals($newData, $newDataFromTable);
-	}
-
-	public function testInsertNewUser() {
-		$user = $this->getMockBuilder(IUser::class)->getMock();
-		$uid = 'uid';
-		$data = ['key' => ['value' => 'value']];
-
-		$user->expects($this->atLeastOnce())->method('getUID')->willReturn($uid);
-		$this->assertNull($this->getDataFromTable($uid));
-		$this->invokePrivate($this->accountManager, 'insertNewUser', [$user, $data]);
-
-		$dataFromDb = $this->getDataFromTable($uid);
-		$this->assertEquals($data, $dataFromDb);
-	}
-
 	public function testAddMissingDefaultValues() {
 		$input = [
-			'key1' => ['value' => 'value1', 'verified' => '0'],
-			'key2' => ['value' => 'value1'],
+			['value' => 'value1', 'verified' => '0', 'name' => 'key1'],
+			['value' => 'value1', 'name' => 'key2'],
 		];
 
 		$expected = [
-			'key1' => ['value' => 'value1', 'verified' => '0'],
-			'key2' => ['value' => 'value1', 'verified' => '0'],
+			['value' => 'value1', 'verified' => '0', 'name' => 'key1'],
+			['value' => 'value1', 'name' => 'key2', 'verified' => '0'],
 		];
 
 		$result = $this->invokePrivate($this->accountManager, 'addMissingDefaultValues', [$input]);
@@ -483,46 +398,30 @@ class AccountManagerTest extends TestCase {
 			->execute();
 	}
 
-	private function getDataFromTable($uid) {
-		$query = $this->connection->getQueryBuilder();
-		$query->select('data')->from($this->table)
-			->where($query->expr()->eq('uid', $query->createParameter('uid')))
-			->setParameter('uid', $uid);
-		$query->execute();
-
-		$qResult = $query->execute();
-		$result = $qResult->fetchAll();
-		$qResult->closeCursor();
-
-		if (!empty($result)) {
-			return json_decode($result[0]['data'], true);
-		}
-	}
-
 	public function testGetAccount() {
 		$accountManager = $this->getInstance(['getUser']);
 		/** @var IUser $user */
 		$user = $this->createMock(IUser::class);
 
 		$data = [
-			IAccountManager::PROPERTY_TWITTER =>
-				[
-					'value' => '@twitterhandle',
-					'scope' => IAccountManager::SCOPE_LOCAL,
-					'verified' => IAccountManager::NOT_VERIFIED,
-				],
-			IAccountManager::PROPERTY_EMAIL =>
-				[
-					'value' => 'test@example.com',
-					'scope' => IAccountManager::SCOPE_PUBLISHED,
-					'verified' => IAccountManager::VERIFICATION_IN_PROGRESS,
-				],
-			IAccountManager::PROPERTY_WEBSITE =>
-				[
-					'value' => 'https://example.com',
-					'scope' => IAccountManager::SCOPE_FEDERATED,
-					'verified' => IAccountManager::VERIFIED,
-				],
+			[
+				'value' => '@twitterhandle',
+				'scope' => IAccountManager::SCOPE_LOCAL,
+				'verified' => IAccountManager::NOT_VERIFIED,
+				'name' => IAccountManager::PROPERTY_TWITTER,
+			],
+			[
+				'value' => 'test@example.com',
+				'scope' => IAccountManager::SCOPE_PUBLISHED,
+				'verified' => IAccountManager::VERIFICATION_IN_PROGRESS,
+				'name' => IAccountManager::PROPERTY_EMAIL,
+			],
+			[
+				'value' => 'https://example.com',
+				'scope' => IAccountManager::SCOPE_FEDERATED,
+				'verified' => IAccountManager::VERIFIED,
+				'name' => IAccountManager::PROPERTY_WEBSITE,
+			],
 		];
 		$expected = new Account($user);
 		$expected->setProperty(IAccountManager::PROPERTY_TWITTER, '@twitterhandle', IAccountManager::SCOPE_LOCAL, IAccountManager::NOT_VERIFIED);

tests/lib/Accounts/HooksTest.php

@@ -23,7 +23,9 @@ namespace Test\Accounts;
 
 use OC\Accounts\AccountManager;
 use OC\Accounts\Hooks;
+use OCP\Accounts\IAccount;
 use OCP\Accounts\IAccountManager;
+use OCP\Accounts\IAccountProperty;
 use OCP\IUser;
 use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
@@ -43,7 +45,7 @@ class HooksTest extends TestCase {
 	/** @var AccountManager|MockObject */
 	private $accountManager;
 
-	/** @var Hooks|MockObject */
+	/** @var Hooks */
 	private $hooks;
 
 	protected function setUp(): void {
@@ -53,12 +55,7 @@ class HooksTest extends TestCase {
 		$this->accountManager = $this->getMockBuilder(AccountManager::class)
 			->disableOriginalConstructor()->getMock();
 
-		$this->hooks = $this->getMockBuilder(Hooks::class)
-			->setConstructorArgs([$this->logger])
-			->setMethods(['getAccountManager'])
-			->getMock();
-
-		$this->hooks->method('getAccountManager')->willReturn($this->accountManager);
+		$this->hooks = new Hooks($this->logger, $this->accountManager);
 	}
 
 	/**
@@ -72,48 +69,57 @@ class HooksTest extends TestCase {
 	 */
 	public function testChangeUserHook($params, $data, $setEmail, $setDisplayName, $error) {
 		if ($error) {
-			$this->accountManager->expects($this->never())->method('getUser');
-			$this->accountManager->expects($this->never())->method('updateUser');
+			$this->accountManager->expects($this->never())->method('updateAccount');
 		} else {
-			$this->accountManager->expects($this->once())->method('getUser')->willReturn($data);
-			$newData = $data;
+			$account = $this->createMock(IAccount::class);
+			$this->accountManager->expects($this->atLeastOnce())->method('getAccount')->willReturn($account);
 			if ($setEmail) {
-				$newData[IAccountManager::PROPERTY_EMAIL]['value'] = $params['value'];
-				$this->accountManager->expects($this->once())->method('updateUser')
-					->with($params['user'], $newData);
+				$property = $this->createMock(IAccountProperty::class);
+				$property->expects($this->atLeastOnce())
+					->method('getValue')
+					->willReturn($data[IAccountManager::PROPERTY_EMAIL]['value']);
+				$property->expects($this->atLeastOnce())
+					->method('setValue')
+					->with($params['value']);
+
+				$account->expects($this->atLeastOnce())
+					->method('getProperty')
+					->with(IAccountManager::PROPERTY_EMAIL)
+					->willReturn($property);
+
+				$this->accountManager->expects($this->once())
+					->method('updateAccount')
+					->with($account);
 			} elseif ($setDisplayName) {
-				$newData[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $params['value'];
-				$this->accountManager->expects($this->once())->method('updateUser')
-					->with($params['user'], $newData);
+				$property = $this->createMock(IAccountProperty::class);
+				$property->expects($this->atLeastOnce())
+					->method('getValue')
+					->willReturn($data[IAccountManager::PROPERTY_DISPLAYNAME]['value']);
+				$property->expects($this->atLeastOnce())
+					->method('setValue')
+					->with($params['value']);
+
+				$account->expects($this->atLeastOnce())
+					->method('getProperty')
+					->with(IAccountManager::PROPERTY_DISPLAYNAME)
+					->willReturn($property);
+
+				$this->accountManager->expects($this->once())
+					->method('updateAccount')
+					->with($account);
 			} else {
-				$this->accountManager->expects($this->never())->method('updateUser');
+				$this->accountManager->expects($this->never())->method('updateAccount');
 			}
 		}
 
-		$this->hooks->changeUserHook($params);
+		$this->hooks->changeUserHook($params['user'], $params['feature'], $params['value']);
 	}
 
 	public function dataTestChangeUserHook() {
 		$user = $this->createMock(IUser::class);
 		return [
 			[
-				['feature' => '', 'value' => ''],
-				[
-					IAccountManager::PROPERTY_EMAIL => ['value' => ''],
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => '']
-				],
-				false, false, true
-			],
-			[
-				['user' => $user, 'value' => ''],
-				[
-					IAccountManager::PROPERTY_EMAIL => ['value' => ''],
-					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => '']
-				],
-				false, false, true
-			],
-			[
-				['user' => $user, 'feature' => ''],
+				['user' => $user, 'feature' => '', 'value' => ''],
 				[
 					IAccountManager::PROPERTY_EMAIL => ['value' => ''],
 					IAccountManager::PROPERTY_DISPLAYNAME => ['value' => '']
@@ -146,10 +152,4 @@ class HooksTest extends TestCase {
 			],
 		];
 	}
-
-	public function testGetAccountManager() {
-		$hooks = new Hooks($this->logger);
-		$result = $this->invokePrivate($hooks, 'getAccountManager');
-		$this->assertInstanceOf(AccountManager::class, $result);
-	}
 }