From 35a0ee2e3e419350bf968a71c6ccb8ef5f44710b Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Tue, 19 Mar 2024 21:43:47 +0100
Subject: [PATCH] fix(ProvisioningApi): only return verified additional mails
 per user

It would not per se be bad to return all of them, however the meta data
about the verified state is missing. Since the information may go out to
connected clients, those may have wrong trust the returned email
addresses.

Email verification still works with this change.

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 .../lib/Controller/AUserData.php              |  3 ++
 apps/testing/appinfo/routes.php               |  5 +++
 .../MailVerificationTestController.php        | 35 +++++++++++++++++++
 .../features/bootstrap/Provisioning.php       | 24 +++++++++++++
 .../features/provisioning-v1.feature          |  4 +++
 5 files changed, 71 insertions(+)
 create mode 100644 apps/testing/lib/Controller/MailVerificationTestController.php

diff --git a/apps/provisioning_api/lib/Controller/AUserData.php b/apps/provisioning_api/lib/Controller/AUserData.php
index d7db48dc33f..ac65c63061f 100644
--- a/apps/provisioning_api/lib/Controller/AUserData.php
+++ b/apps/provisioning_api/lib/Controller/AUserData.php
@@ -149,6 +149,9 @@ abstract class AUserData extends OCSController {
 			$additionalEmails = $additionalEmailScopes = [];
 			$emailCollection = $userAccount->getPropertyCollection(IAccountManager::COLLECTION_EMAIL);
 			foreach ($emailCollection->getProperties() as $property) {
+				if ($property->getLocallyVerified() !== IAccountManager::VERIFIED) {
+					continue;
+				}
 				$additionalEmails[] = $property->getValue();
 				if ($includeScopes) {
 					$additionalEmailScopes[] = $property->getScope();
diff --git a/apps/testing/appinfo/routes.php b/apps/testing/appinfo/routes.php
index 862f63ef4c2..1c89b474838 100644
--- a/apps/testing/appinfo/routes.php
+++ b/apps/testing/appinfo/routes.php
@@ -63,5 +63,10 @@ return [
 				'type' => null
 			]
 		],
+		[
+			'name' => 'MailVerificationTest',
+			'url' => '/api/v1/mailverification',
+			'verb' => 'POST',
+		]
 	],
 ];
diff --git a/apps/testing/lib/Controller/MailVerificationTestController.php b/apps/testing/lib/Controller/MailVerificationTestController.php
new file mode 100644
index 00000000000..95590117fb8
--- /dev/null
+++ b/apps/testing/lib/Controller/MailVerificationTestController.php
@@ -0,0 +1,35 @@
+<?php
+
+namespace OCA\Testing\Controller;
+
+use InvalidArgumentException;
+use OCP\Accounts\IAccountManager;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
+use OCP\AppFramework\Http\DataResponse;
+use OCP\AppFramework\OCSController;
+use OCP\IRequest;
+use OCP\IUser;
+use OCP\IUserManager;
+
+class MailVerificationTestController extends OCSController {
+	public function __construct(
+		$appName,
+		IRequest $request,
+		protected IAccountManager $accountManager,
+		protected IUserManager $userManager,
+	) {
+		parent::__construct($appName, $request);
+	}
+
+	public function verify(string $userId, string $email): DataResponse {
+		$user = $this->userManager->get($userId);
+		$userAccount = $this->accountManager->getAccount($user);
+		$emailProperty = $userAccount->getPropertyCollection(IAccountManager::COLLECTION_EMAIL)
+			->getPropertyByValue($email);
+		if ($emailProperty === null) {
+			throw new InvalidArgumentException('Email not available in account.');
+		}
+		$emailProperty->setLocallyVerified(IAccountManager::VERIFIED);
+		return new DataResponse();
+	}
+}
diff --git a/build/integration/features/bootstrap/Provisioning.php b/build/integration/features/bootstrap/Provisioning.php
index 2fb1c807cc5..b3e3019a4c8 100644
--- a/build/integration/features/bootstrap/Provisioning.php
+++ b/build/integration/features/bootstrap/Provisioning.php
@@ -980,4 +980,28 @@ trait Provisioning {
 			}
 		}
 	}
+
+	/**
+	 * @Then user :user verifies email :email
+	 */
+	public function userVerifiesEmail(string $userId, string $email): void {
+		$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/testing/api/v1/mailverification";
+		$client = new Client();
+		$options = [];
+		if ($this->currentUser === 'admin') {
+			$options['auth'] = $this->adminUser;
+		}
+
+		$options['form_params'] = [
+			'userid' => $userId,
+			'email' => $email,
+		];
+
+		$options['headers'] = [
+			'OCS-APIREQUEST' => 'true',
+		];
+
+		$this->response = $client->post($fullUrl, $options);
+	}
 }
+
diff --git a/build/integration/features/provisioning-v1.feature b/build/integration/features/provisioning-v1.feature
index 2238c820939..35b64bb8c43 100644
--- a/build/integration/features/provisioning-v1.feature
+++ b/build/integration/features/provisioning-v1.feature
@@ -129,11 +129,13 @@ Feature: provisioning
       | value | no.reply@nextcloud.com |
     And the OCS status code should be "100"
     And the HTTP status code should be "200"
+    And user "brand-new-user" verifies email "no.reply@nextcloud.com"
     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 user "brand-new-user" verifies email "noreply@nextcloud.com"
 		And sending "PUT" to "/cloud/users/brand-new-user" with
 			| key | phone |
 			| value | +49 711 / 25 24 28-90 |
@@ -302,11 +304,13 @@ Feature: provisioning
       | value | no.reply6@nextcloud.com |
     And the OCS status code should be "100"
     And the HTTP status code should be "200"
+    And user "brand-new-user" verifies email "no.reply6@nextcloud.com"
     And sending "PUT" to "/cloud/users/brand-new-user" with
       | key | additional_mail |
       | value | noreply7@nextcloud.com |
     And the OCS status code should be "100"
     And the HTTP status code should be "200"
+    And user "brand-new-user" verifies email "no.reply7@nextcloud.com"
     When sending "PUT" to "/cloud/users/brand-new-user/additional_mail" with
       | key | no.reply6@nextcloud.com |
       | value | |