fix(provisioning_api): Don't allow to configure the same additional email multiple times

Signed-off-by: Joas Schilling <coding@schilljs.com>

apps/provisioning_api/lib/Controller/UsersController.php

@@ -942,11 +942,11 @@ class UsersController extends AUserData {
 				if (filter_var($value, FILTER_VALIDATE_EMAIL) && $value !== $targetUser->getSystemEMailAddress()) {
 					$userAccount = $this->accountManager->getAccount($targetUser);
 					$mailCollection = $userAccount->getPropertyCollection(IAccountManager::COLLECTION_EMAIL);
-					foreach ($mailCollection->getProperties() as $property) {
-						if ($property->getValue() === $value) {
-							break;
-						}
+
+					if ($mailCollection->getPropertyByValue($value)) {
+						throw new OCSException('', 102);
 					}
+
 					$mailCollection->addPropertyWithDefaults($value);
 					$this->accountManager->updateAccount($userAccount);
 				} else {

apps/provisioning_api/tests/Controller/UsersControllerTest.php

@@ -52,7 +52,9 @@ use OCA\Settings\Mailer\NewUserMailHelper;
 use OCP\Accounts\IAccount;
 use OCP\Accounts\IAccountManager;
 use OCP\Accounts\IAccountProperty;
+use OCP\Accounts\IAccountPropertyCollection;
 use OCP\AppFramework\Http\DataResponse;
+use OCP\AppFramework\OCS\OCSException;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IConfig;
 use OCP\IGroup;
@@ -1544,7 +1546,162 @@ class UsersControllerTest extends TestCase {
 		$this->assertEquals([], $this->api->editUser('UserToEdit', 'email', 'demo@nextcloud.com')->getData());
 	}
 
+	public function testEditUserRegularUserSelfEditAddAdditionalEmailValid(): void {
+		$loggedInUser = $this->getMockBuilder(IUser::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$loggedInUser
+			->expects($this->any())
+			->method('getUID')
+			->willReturn('UID');
+		$targetUser = $this->getMockBuilder(IUser::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$this->userSession
+			->expects($this->once())
+			->method('getUser')
+			->willReturn($loggedInUser);
+		$this->userManager
+			->expects($this->once())
+			->method('get')
+			->with('UserToEdit')
+			->willReturn($targetUser);
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->willReturn('UID');
+
+		$backend = $this->createMock(UserInterface::class);
+		$targetUser
+			->expects($this->any())
+			->method('getBackend')
+			->willReturn($backend);
+
+		$userAccount = $this->createMock(IAccount::class);
+
+		$this->accountManager
+			->expects($this->once())
+			->method('getAccount')
+			->with($targetUser)
+			->willReturn($userAccount);
+		$this->accountManager
+			->expects($this->once())
+			->method('updateAccount')
+			->with($userAccount);
+
+		$this->assertEquals([], $this->api->editUser('UserToEdit', 'additional_mail', 'demo1@nextcloud.com')->getData());
+	}
+
+	public function testEditUserRegularUserSelfEditAddAdditionalEmailMainAddress(): void {
+		$loggedInUser = $this->getMockBuilder(IUser::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$loggedInUser
+			->expects($this->any())
+			->method('getUID')
+			->willReturn('UID');
+		$targetUser = $this->getMockBuilder(IUser::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$this->userSession
+			->expects($this->once())
+			->method('getUser')
+			->willReturn($loggedInUser);
+		$this->userManager
+			->expects($this->once())
+			->method('get')
+			->with('UserToEdit')
+			->willReturn($targetUser);
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->willReturn('UID');
+
+		$backend = $this->createMock(UserInterface::class);
+		$targetUser
+			->expects($this->any())
+			->method('getBackend')
+			->willReturn($backend);
+		$targetUser
+			->expects($this->any())
+			->method('getSystemEMailAddress')
+			->willReturn('demo@nextcloud.com');
+
+		$userAccount = $this->createMock(IAccount::class);
 
+		$this->accountManager
+			->expects($this->never())
+			->method('getAccount')
+			->with($targetUser)
+			->willReturn($userAccount);
+		$this->accountManager
+			->expects($this->never())
+			->method('updateAccount')
+			->with($userAccount);
+
+		$this->expectException(OCSException::class);
+		$this->expectExceptionCode(102);
+		$this->api->editUser('UserToEdit', 'additional_mail', 'demo@nextcloud.com')->getData();
+	}
+
+	public function testEditUserRegularUserSelfEditAddAdditionalEmailDuplicate(): void {
+		$loggedInUser = $this->getMockBuilder(IUser::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$loggedInUser
+			->expects($this->any())
+			->method('getUID')
+			->willReturn('UID');
+		$targetUser = $this->getMockBuilder(IUser::class)
+			->disableOriginalConstructor()
+			->getMock();
+		$this->userSession
+			->expects($this->once())
+			->method('getUser')
+			->willReturn($loggedInUser);
+		$this->userManager
+			->expects($this->once())
+			->method('get')
+			->with('UserToEdit')
+			->willReturn($targetUser);
+		$targetUser
+			->expects($this->any())
+			->method('getUID')
+			->willReturn('UID');
+
+		$backend = $this->createMock(UserInterface::class);
+		$targetUser
+			->expects($this->any())
+			->method('getBackend')
+			->willReturn($backend);
+
+		$property = $this->createMock(IAccountProperty::class);
+		$property->method('getValue')
+			->willReturn('demo1@nextcloud.com');
+		$collection = $this->createMock(IAccountPropertyCollection::class);
+		$collection->method('getPropertyByValue')
+			->with('demo1@nextcloud.com')
+			->willReturn($property);
+
+		$userAccount = $this->createMock(IAccount::class);
+		$userAccount->method('getPropertyCollection')
+			->with(IAccountManager::COLLECTION_EMAIL)
+			->willReturn($collection);
+
+		$this->accountManager
+			->expects($this->once())
+			->method('getAccount')
+			->with($targetUser)
+			->willReturn($userAccount);
+		$this->accountManager
+			->expects($this->never())
+			->method('updateAccount')
+			->with($userAccount);
+
+		$this->expectException(OCSException::class);
+		$this->expectExceptionCode(102);
+		$this->api->editUser('UserToEdit', 'additional_mail', 'demo1@nextcloud.com')->getData();
+	}
 
 	public function testEditUserRegularUserSelfEditChangeEmailInvalid() {
 		$this->expectException(\OCP\AppFramework\OCS\OCSException::class);

build/integration/features/provisioning-v1.feature

@@ -199,6 +199,28 @@ Feature: provisioning
 			| value | private |
 		Then 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 | email |
+			| value | no-reply@nextcloud.com |
+		And the OCS status code should be "100"
+		And the HTTP status code should be "200"
+		# Duplicating primary address
+		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 "102"
+		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"
+		# Duplicating another additional address
+		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 "102"
+		And the HTTP status code should be "200"
 		Then user "brand-new-user" has
 			| id | brand-new-user |
 			| phoneScope | v2-private |