Another system config: allow_user_to_change_email_address

Murena-SAS · Oct 6, 2023 · 4b2672e · 4b2672e
1 parent a719b43
commit 4b2672e
Show file tree

Hide file tree

Showing 12 changed files with 103 additions and 10 deletions.
diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php
@@ -610,14 +610,17 @@ public function getEditableFieldsForUser(string $userId): DataResponse {
 			$targetUser = $currentLoggedInUser;
 		}
 
-		// Editing self (display, email)
+		// Editing self (display)
 		if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) {
 			if (
 				$targetUser->getBackend() instanceof ISetDisplayNameBackend
 				|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)
 			) {
 				$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
 			}
+		}
+		// Editing self (email)
+		if ($this->config->getSystemValue('allow_user_to_change_email_address', true) !== false) {
 			$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
 		}
 
@@ -753,7 +756,7 @@ public function editUser(string $userId, string $key, string $value): DataRespon
 
 		$permittedFields = [];
 		if ($targetUser->getUID() === $currentLoggedInUser->getUID()) {
-			// Editing self (display, email)
+			// Editing self (display)
 			if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) {
 				if (
 					$targetUser->getBackend() instanceof ISetDisplayNameBackend
@@ -762,8 +765,12 @@ public function editUser(string $userId, string $key, string $value): DataRespon
 					$permittedFields[] = self::USER_FIELD_DISPLAYNAME;
 					$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
 				}
+			}
+			// Editing self (email)
+			if ($this->config->getSystemValue('allow_user_to_change_email_address', true) !== false) {
 				$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
 			}
+
 
 			$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME . self::SCOPE_SUFFIX;
 			$permittedFields[] = IAccountManager::PROPERTY_EMAIL . self::SCOPE_SUFFIX;

diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
@@ -2127,6 +2127,12 @@ public function testEditUserSelfEditChangeLanguage() {
 				['allow_user_to_change_display_name', true, true],
 				['force_language', false, false],
 			]);
+		$this->config->expects($this->any())
+			->method('getSystemValue')
+			->willReturnMap([
+				['allow_user_to_change_email_address', true, true],
+				['force_language', false, false],
+			]);
 
 		$loggedInUser = $this->createMock(IUser::class);
 		$loggedInUser
@@ -2184,7 +2190,13 @@ public function testEditUserSelfEditChangeLanguageButForced($forced) {
 				['allow_user_to_change_display_name', true, true],
 				['force_language', false, $forced],
 			]);
-
+		$this->config->expects($this->any())
+			->method('getSystemValue')
+			->willReturnMap([
+				['allow_user_to_change_email_address', true, true],
+				['force_language', false, $forced],
+			]);
+
 		$loggedInUser = $this->createMock(IUser::class);
 		$loggedInUser
 			->expects($this->any())
@@ -4171,6 +4183,12 @@ public function testGetEditableFields(bool $allowedToChangeDisplayName, string $
 				$this->equalTo('allow_user_to_change_display_name'),
 				$this->anything()
 			)->willReturn($allowedToChangeDisplayName);
+		$this->config
+			->method('getSystemValue')
+			->with(
+				$this->equalTo('allow_user_to_change_email_address'),
+				$this->anything()
+			)->willReturn($allowedToChangeDisplayName);
 
 		$user = $this->createMock(IUser::class);
 		$this->userSession->method('getUser')

diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php
@@ -419,9 +419,14 @@ public function setUserSettings(?string $avatarScope = null,
 			IAccountManager::PROPERTY_FEDIVERSE => ['value' => $fediverse, 'scope' => $fediverseScope],
 		];
 		$allowUserToChangeDisplayName = $this->config->getSystemValueBool('allow_user_to_change_display_name', true);
+		$allowUserToChangeEmailAddress = $this->config->getSystemValueBool('allow_user_to_change_email_address', true);
 		foreach ($updatable as $property => $data) {
 			if ($allowUserToChangeDisplayName === false
-				&& in_array($property, [IAccountManager::PROPERTY_DISPLAYNAME, IAccountManager::PROPERTY_EMAIL], true)) {
+				&& in_array($property, [IAccountManager::PROPERTY_DISPLAYNAME], true)) {
+				continue;
+			}
+			if ($allowUserToChangeEmailAddress === false
+				&& in_array($property, [IAccountManager::PROPERTY_EMAIL], true)) {
 				continue;
 			}
 			$property = $userAccount->getProperty($property);
@@ -494,7 +499,7 @@ protected function saveUserSettings(IAccount $userAccount): void {
 		if ($oldEmailAddress !== strtolower($userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getValue())) {
 			// this is the only permission a backend provides and is also used
 			// for the permission of setting a email address
-			if (!$userAccount->getUser()->canChangeDisplayName()) {
+			if (!$userAccount->getUser()->canChangeEmailAddress()) {
 				throw new ForbiddenException($this->l10n->t('Unable to change email address'));
 			}
 			$userAccount->getUser()->setSystemEMailAddress($userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getValue());

diff --git a/apps/settings/lib/Settings/Personal/PersonalInfo.php b/apps/settings/lib/Settings/Personal/PersonalInfo.php
@@ -172,6 +172,7 @@ public function getForm(): TemplateResponse {
 		$accountParameters = [
 			'avatarChangeSupported' => $user->canChangeAvatar(),
 			'displayNameChangeSupported' => $user->canChangeDisplayName(),
+			'emailAddressChangeSupported' => $user->canChangeEmailAddress(),
 			'federationEnabled' => $federationEnabled,
 			'lookupServerUploadEnabled' => $lookupServerUploadEnabled,
 		];

diff --git a/apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue b/apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue
@@ -31,7 +31,7 @@
 			:scope.sync="primaryEmail.scope"
 			@add-additional="onAddAdditionalEmail" />
 
-		<template v-if="displayNameChangeSupported">
+		<template v-if="emailAddressChangeSupported">
 			<Email :primary="true"
 				:scope.sync="primaryEmail.scope"
 				:email.sync="primaryEmail.value"
@@ -73,7 +73,7 @@ import { validateEmail } from '../../../utils/validate.js'
 import { handleError } from '../../../utils/handlers.js'
 
 const { emailMap: { additionalEmails, primaryEmail, notificationEmail } } = loadState('settings', 'personalInfoParameters', {})
-const { displayNameChangeSupported } = loadState('settings', 'accountParameters', {})
+const { emailAddressChangeSupported } = loadState('settings', 'accountParameters', {})
 
 export default {
 	name: 'EmailSection',
@@ -87,7 +87,7 @@ export default {
 		return {
 			accountProperty: ACCOUNT_PROPERTY_READABLE_ENUM.EMAIL,
 			additionalEmails: additionalEmails.map(properties => ({ ...properties, key: this.generateUniqueKey() })),
-			displayNameChangeSupported,
+			emailAddressChangeSupported,
 			primaryEmail: { ...primaryEmail, readable: NAME_READABLE_ENUM[primaryEmail.name] },
 			savePrimaryEmailScope,
 			notificationEmail,

diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php
@@ -392,6 +392,11 @@ public function testSetUserSettingsWhenUserDisplayNameChangeNotAllowed() {
 			->method('getSystemValueBool')
 			->with('allow_user_to_change_display_name')
 			->willReturn(false);
+
+		 $this->config->expects($this->once())
+			->method('getSystemValueBool')
+			->with('allow_user_to_change_email_address')
+			->willReturn(false);
 
 		$this->appManager->expects($this->any())
 			->method('isEnabledForUser')
@@ -648,6 +653,7 @@ public function testSaveUserSettings($data,
 		$user->method('getDisplayName')->willReturn($oldDisplayName);
 		$user->method('getSystemEMailAddress')->willReturn($oldEmailAddress);
 		$user->method('canChangeDisplayName')->willReturn(true);
+		$user->method('canChangeEmailAddress')->willReturn(true);
 
 		if (strtolower($data[IAccountManager::PROPERTY_EMAIL]['value']) === strtolower($oldEmailAddress ?? '')) {
 			$user->expects($this->never())->method('setSystemEMailAddress');
@@ -790,7 +796,7 @@ public function testSaveUserSettingsException(
 			});
 
 		if ($data[IAccountManager::PROPERTY_EMAIL]['value'] !== $oldEmailAddress) {
-			$user->method('canChangeDisplayName')
+			$user->method('canChangeEmailAddress')
 				->willReturn($canChangeEmail);
 		}
 

diff --git a/config/config.sample.php b/config/config.sample.php
@@ -240,6 +240,12 @@
  */
 'allow_user_to_change_display_name' => true,
 
+/**
+ * ``true`` allows users to change their email address (on their Personal
+ * pages), and ``false`` prevents them from changing their email address.
+ */
+'allow_user_to_change_email_address' => true,
+
 /**
  * The directory where the skeleton files are located. These files will be
  * copied to the data directory of new users. Leave empty to not copy any

diff --git a/lib/private/User/Backend.php b/lib/private/User/Backend.php
@@ -47,14 +47,15 @@ abstract class Backend implements UserInterface {
 	public const SET_DISPLAYNAME = 1048576;		// 1 << 20
 	public const PROVIDE_AVATAR = 16777216;		// 1 << 24
 	public const COUNT_USERS = 268435456;	// 1 << 28
-
+	public const SET_EMAILADDRESS = 4294967296;		// 1 << 32
 	protected $possibleActions = [
 		self::CREATE_USER => 'createUser',
 		self::SET_PASSWORD => 'setPassword',
 		self::CHECK_PASSWORD => 'checkPassword',
 		self::GET_HOME => 'getHome',
 		self::GET_DISPLAYNAME => 'getDisplayName',
 		self::SET_DISPLAYNAME => 'setDisplayName',
+		self::SET_EMAILADDRESS => 'setEMailAddress',
 		self::PROVIDE_AVATAR => 'canChangeAvatar',
 		self::COUNT_USERS => 'countUsers',
 	];

diff --git a/lib/private/User/LazyUser.php b/lib/private/User/LazyUser.php
@@ -112,6 +112,10 @@ public function canChangeDisplayName() {
 		return $this->getUser()->canChangeDisplayName();
 	}
 
+	public function canChangeEmailAddress() {
+		return $this->getUser()->canChangeEmailAddress();
+	}
+
 	public function isEnabled() {
 		return $this->getUser()->isEnabled();
 	}

diff --git a/lib/private/User/User.php b/lib/private/User/User.php
@@ -426,6 +426,18 @@ public function canChangeDisplayName() {
 		return $this->backend->implementsActions(Backend::SET_DISPLAYNAME);
 	}
 
+	/**
+	 * check if the backend supports changing display names
+	 *
+	 * @return bool
+	 */
+	public function canChangeEmailAddress() {
+		if (!$this->config->getSystemValueBool('allow_user_to_change_email_address', true)) {
+			return false;
+		}
+		return $this->backend->implementsActions(Backend::SET_EMAILADDRESS);
+	}
+
 	/**
 	 * check if the user is enabled
 	 *

diff --git a/lib/public/IUser.php b/lib/public/IUser.php
@@ -143,6 +143,14 @@ public function canChangePassword();
 	 */
 	public function canChangeDisplayName();
 
+	/**
+	 * check if the backend supports changing email addresses
+	 *
+	 * @return bool
+	 * @since 8.0.0
+	 */
+	public function canChangeEmailAddress();
+
 	/**
 	 * check if the user is enabled
 	 *

diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php
@@ -373,6 +373,31 @@ public function testCanChangeDisplayName() {
 		$this->assertTrue($user->canChangeDisplayName());
 	}
 
+	public function testCanChangeEmailAddress() {
+		/**
+		 * @var Backend | MockObject $backend
+		 */
+		$backend = $this->createMock(\Test\Util\User\Dummy::class);
+
+		$backend->expects($this->any())
+			->method('implementsActions')
+			->willReturnCallback(function ($actions) {
+				if ($actions === \OC\User\Backend::SET_DISPLAYNAME) {
+					return true;
+				} else {
+					return false;
+				}
+			});
+
+		$config = $this->createMock(IConfig::class);
+		$config->method('getSystemValueBool')
+			->with('allow_user_to_change_email_address')
+			->willReturn(true);
+
+		$user = new User('foo', $backend, $this->dispatcher, null, $config);
+		$this->assertTrue($user->canChangeEmailAddress());
+	}
+
 	public function testCanChangeDisplayNameNotSupported() {
 		/**
 		 * @var Backend | MockObject $backend