diff options
author | provokateurin <kate@provokateurin.de> | 2025-03-27 09:31:01 +0100 |
---|---|---|
committer | Ferdinand Thiessen <opensource@fthiessen.de> | 2025-04-02 19:06:25 +0200 |
commit | eb98e99530aaf430cd8759b1ade94e2686a06920 (patch) | |
tree | 6248badc969677e160b4df98eded376618abf460 | |
parent | 18d4888ca9189f3f2344c49a6affbd63ff43bb95 (diff) | |
download | nextcloud-server-eb98e99530aaf430cd8759b1ade94e2686a06920.tar.gz nextcloud-server-eb98e99530aaf430cd8759b1ade94e2686a06920.zip |
fix(settings): Handle email change restriction separately from display name change restriction
Co-authored-by: provokateurin <kate@provokateurin.de>
Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de>
Co-authored-by: Louis <louis@chmn.me>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
-rw-r--r-- | apps/provisioning_api/lib/Controller/UsersController.php | 38 | ||||
-rw-r--r-- | apps/provisioning_api/tests/Controller/UsersControllerTest.php | 97 | ||||
-rw-r--r-- | apps/settings/lib/Settings/Personal/PersonalInfo.php | 1 | ||||
-rw-r--r-- | apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue | 6 | ||||
-rw-r--r-- | lib/private/User/LazyUser.php | 4 | ||||
-rw-r--r-- | lib/private/User/User.php | 5 | ||||
-rw-r--r-- | lib/public/IUser.php | 7 |
7 files changed, 128 insertions, 30 deletions
diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 6b22a010a8c..4b3db45f518 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -748,14 +748,16 @@ class UsersController extends AUserDataOCSController { $targetUser = $currentLoggedInUser; } - // Editing self (display, email) - 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; - } + $allowDisplayNameChange = $this->config->getSystemValue('allow_user_to_change_display_name', true); + if ($allowDisplayNameChange === true && ( + $targetUser->getBackend() instanceof ISetDisplayNameBackend + || $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME) + )) { + $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; + } + + // Fallback to display name value to avoid changing behavior with the new option. + if ($this->config->getSystemValue('allow_user_to_change_email', $allowDisplayNameChange)) { $permittedFields[] = IAccountManager::PROPERTY_EMAIL; } @@ -907,15 +909,17 @@ class UsersController extends AUserDataOCSController { $permittedFields = []; if ($targetUser->getUID() === $currentLoggedInUser->getUID()) { - // Editing self (display, email) - if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) { - if ( - $targetUser->getBackend() instanceof ISetDisplayNameBackend - || $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME) - ) { - $permittedFields[] = self::USER_FIELD_DISPLAYNAME; - $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; - } + $allowDisplayNameChange = $this->config->getSystemValue('allow_user_to_change_display_name', true); + if ($allowDisplayNameChange !== false && ( + $targetUser->getBackend() instanceof ISetDisplayNameBackend + || $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME) + )) { + $permittedFields[] = self::USER_FIELD_DISPLAYNAME; + $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; + } + + // Fallback to display name value to avoid changing behavior with the new option. + if ($this->config->getSystemValue('allow_user_to_change_email', $allowDisplayNameChange)) { $permittedFields[] = IAccountManager::PROPERTY_EMAIL; } diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 439ee0be250..7d4f99356b3 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -42,6 +42,7 @@ use OCP\User\Backend\ISetDisplayNameBackend; use OCP\UserInterface; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; +use RuntimeException; use Test\TestCase; class UsersControllerTest extends TestCase { @@ -1668,6 +1669,8 @@ class UsersControllerTest extends TestCase { ->method('getBackend') ->willReturn($backend); + $this->config->method('getSystemValue')->willReturnCallback(fn (string $key, mixed $default) => $default); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'email', 'demo@nextcloud.com')->getData()); } @@ -1862,6 +1865,8 @@ class UsersControllerTest extends TestCase { ->method('getBackend') ->willReturn($backend); + $this->config->method('getSystemValue')->willReturnCallback(fn (string $key, mixed $default) => $default); + $this->api->editUser('UserToEdit', 'email', 'demo.org'); } @@ -4260,7 +4265,8 @@ class UsersControllerTest extends TestCase { public function dataGetEditableFields() { return [ - [false, ISetDisplayNameBackend::class, [ + [false, true, ISetDisplayNameBackend::class, [ + IAccountManager::PROPERTY_EMAIL, IAccountManager::COLLECTION_EMAIL, IAccountManager::PROPERTY_PHONE, IAccountManager::PROPERTY_ADDRESS, @@ -4274,7 +4280,22 @@ class UsersControllerTest extends TestCase { IAccountManager::PROPERTY_PROFILE_ENABLED, IAccountManager::PROPERTY_PRONOUNS, ]], - [true, ISetDisplayNameBackend::class, [ + [true, false, ISetDisplayNameBackend::class, [ + IAccountManager::PROPERTY_DISPLAYNAME, + IAccountManager::COLLECTION_EMAIL, + IAccountManager::PROPERTY_PHONE, + IAccountManager::PROPERTY_ADDRESS, + IAccountManager::PROPERTY_WEBSITE, + IAccountManager::PROPERTY_TWITTER, + IAccountManager::PROPERTY_FEDIVERSE, + IAccountManager::PROPERTY_ORGANISATION, + IAccountManager::PROPERTY_ROLE, + IAccountManager::PROPERTY_HEADLINE, + IAccountManager::PROPERTY_BIOGRAPHY, + IAccountManager::PROPERTY_PROFILE_ENABLED, + IAccountManager::PROPERTY_PRONOUNS, + ]], + [true, true, ISetDisplayNameBackend::class, [ IAccountManager::PROPERTY_DISPLAYNAME, IAccountManager::PROPERTY_EMAIL, IAccountManager::COLLECTION_EMAIL, @@ -4290,7 +4311,21 @@ class UsersControllerTest extends TestCase { IAccountManager::PROPERTY_PROFILE_ENABLED, IAccountManager::PROPERTY_PRONOUNS, ]], - [true, UserInterface::class, [ + [false, false, ISetDisplayNameBackend::class, [ + IAccountManager::COLLECTION_EMAIL, + IAccountManager::PROPERTY_PHONE, + IAccountManager::PROPERTY_ADDRESS, + IAccountManager::PROPERTY_WEBSITE, + IAccountManager::PROPERTY_TWITTER, + IAccountManager::PROPERTY_FEDIVERSE, + IAccountManager::PROPERTY_ORGANISATION, + IAccountManager::PROPERTY_ROLE, + IAccountManager::PROPERTY_HEADLINE, + IAccountManager::PROPERTY_BIOGRAPHY, + IAccountManager::PROPERTY_PROFILE_ENABLED, + IAccountManager::PROPERTY_PRONOUNS, + ]], + [false, true, UserInterface::class, [ IAccountManager::PROPERTY_EMAIL, IAccountManager::COLLECTION_EMAIL, IAccountManager::PROPERTY_PHONE, @@ -4305,6 +4340,49 @@ class UsersControllerTest extends TestCase { IAccountManager::PROPERTY_PROFILE_ENABLED, IAccountManager::PROPERTY_PRONOUNS, ]], + [true, false, UserInterface::class, [ + IAccountManager::COLLECTION_EMAIL, + IAccountManager::PROPERTY_PHONE, + IAccountManager::PROPERTY_ADDRESS, + IAccountManager::PROPERTY_WEBSITE, + IAccountManager::PROPERTY_TWITTER, + IAccountManager::PROPERTY_FEDIVERSE, + IAccountManager::PROPERTY_ORGANISATION, + IAccountManager::PROPERTY_ROLE, + IAccountManager::PROPERTY_HEADLINE, + IAccountManager::PROPERTY_BIOGRAPHY, + IAccountManager::PROPERTY_PROFILE_ENABLED, + IAccountManager::PROPERTY_PRONOUNS, + ]], + [true, true, UserInterface::class, [ + IAccountManager::PROPERTY_EMAIL, + IAccountManager::COLLECTION_EMAIL, + IAccountManager::PROPERTY_PHONE, + IAccountManager::PROPERTY_ADDRESS, + IAccountManager::PROPERTY_WEBSITE, + IAccountManager::PROPERTY_TWITTER, + IAccountManager::PROPERTY_FEDIVERSE, + IAccountManager::PROPERTY_ORGANISATION, + IAccountManager::PROPERTY_ROLE, + IAccountManager::PROPERTY_HEADLINE, + IAccountManager::PROPERTY_BIOGRAPHY, + IAccountManager::PROPERTY_PROFILE_ENABLED, + IAccountManager::PROPERTY_PRONOUNS, + ]], + [false, false, UserInterface::class, [ + IAccountManager::COLLECTION_EMAIL, + IAccountManager::PROPERTY_PHONE, + IAccountManager::PROPERTY_ADDRESS, + IAccountManager::PROPERTY_WEBSITE, + IAccountManager::PROPERTY_TWITTER, + IAccountManager::PROPERTY_FEDIVERSE, + IAccountManager::PROPERTY_ORGANISATION, + IAccountManager::PROPERTY_ROLE, + IAccountManager::PROPERTY_HEADLINE, + IAccountManager::PROPERTY_BIOGRAPHY, + IAccountManager::PROPERTY_PROFILE_ENABLED, + IAccountManager::PROPERTY_PRONOUNS, + ]], ]; } @@ -4315,13 +4393,12 @@ class UsersControllerTest extends TestCase { * @param string $userBackend * @param array $expected */ - public function testGetEditableFields(bool $allowedToChangeDisplayName, string $userBackend, array $expected): void { - $this->config - ->method('getSystemValue') - ->with( - $this->equalTo('allow_user_to_change_display_name'), - $this->anything() - )->willReturn($allowedToChangeDisplayName); + public function testGetEditableFields(bool $allowedToChangeDisplayName, bool $allowedToChangeEmail, string $userBackend, array $expected): void { + $this->config->method('getSystemValue')->willReturnCallback(fn (string $key, mixed $default) => match ($key) { + 'allow_user_to_change_display_name' => $allowedToChangeDisplayName, + 'allow_user_to_change_email' => $allowedToChangeEmail, + default => throw new RuntimeException('Unexpected system config key: ' . $key), + }); $user = $this->createMock(IUser::class); $this->userSession->method('getUser') diff --git a/apps/settings/lib/Settings/Personal/PersonalInfo.php b/apps/settings/lib/Settings/Personal/PersonalInfo.php index 9860ec5c9c7..1032f97361b 100644 --- a/apps/settings/lib/Settings/Personal/PersonalInfo.php +++ b/apps/settings/lib/Settings/Personal/PersonalInfo.php @@ -114,6 +114,7 @@ class PersonalInfo implements ISettings { $accountParameters = [ 'avatarChangeSupported' => $user->canChangeAvatar(), 'displayNameChangeSupported' => $user->canChangeDisplayName(), + 'emailChangeSupported' => $user->canChangeEmail(), 'federationEnabled' => $federationEnabled, 'lookupServerUploadEnabled' => $lookupServerUploadEnabled, ]; diff --git a/apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue b/apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue index 8fd17922724..f9674a3163b 100644 --- a/apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue +++ b/apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue @@ -13,7 +13,7 @@ :scope.sync="primaryEmail.scope" @add-additional="onAddAdditionalEmail" /> - <template v-if="displayNameChangeSupported"> + <template v-if="emailChangeSupported"> <Email :input-id="inputId" :primary="true" :scope.sync="primaryEmail.scope" @@ -56,7 +56,7 @@ import { validateEmail } from '../../../utils/validate.js' import { handleError } from '../../../utils/handlers.ts' const { emailMap: { additionalEmails, primaryEmail, notificationEmail } } = loadState('settings', 'personalInfoParameters', {}) -const { displayNameChangeSupported } = loadState('settings', 'accountParameters', {}) +const { emailChangeSupported } = loadState('settings', 'accountParameters', {}) export default { name: 'EmailSection', @@ -70,7 +70,7 @@ export default { return { accountProperty: ACCOUNT_PROPERTY_READABLE_ENUM.EMAIL, additionalEmails: additionalEmails.map(properties => ({ ...properties, key: this.generateUniqueKey() })), - displayNameChangeSupported, + emailChangeSupported, primaryEmail: { ...primaryEmail, readable: NAME_READABLE_ENUM[primaryEmail.name] }, notificationEmail, } diff --git a/lib/private/User/LazyUser.php b/lib/private/User/LazyUser.php index 9eb5f9afb25..715265f6a39 100644 --- a/lib/private/User/LazyUser.php +++ b/lib/private/User/LazyUser.php @@ -112,6 +112,10 @@ class LazyUser implements IUser { return $this->getUser()->canChangeDisplayName(); } + public function canChangeEmail(): bool { + return $this->getUser()->canChangeEmail(); + } + public function isEnabled() { return $this->getUser()->isEnabled(); } diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 3d5c13d4431..f04977314e2 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -447,6 +447,11 @@ class User implements IUser { return $this->backend->implementsActions(Backend::SET_DISPLAYNAME); } + public function canChangeEmail(): bool { + // Fallback to display name value to avoid changing behavior with the new option. + return $this->config->getSystemValueBool('allow_user_to_change_email', $this->config->getSystemValueBool('allow_user_to_change_display_name', true)); + } + /** * check if the user is enabled * diff --git a/lib/public/IUser.php b/lib/public/IUser.php index b4808ec045a..227e4f902c7 100644 --- a/lib/public/IUser.php +++ b/lib/public/IUser.php @@ -150,6 +150,13 @@ interface IUser { public function canChangeDisplayName(); /** + * Check if the backend supports changing email + * + * @since 32.0.0 + */ + public function canChangeEmail(): bool; + + /** * check if the user is enabled * * @return bool |