Skip to content

Commit 2f22c0d

Browse files
AltahrimAltahrim
authored
Merge pull request #51900 from nextcloud/backport/51745/stable30
[stable30] fix(settings): Handle email change restriction separately from display name change restriction
2 parents 9bd991b + 185e97b commit 2f22c0d

File tree

13 files changed

+163
-34
lines changed

13 files changed

+163
-34
lines changed

‎apps/provisioning_api/lib/Controller/UsersController.php‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -744,14 +744,16 @@ public function getEditableFieldsForUser(string $userId): DataResponse {
744744
$targetUser = $currentLoggedInUser;
745745
}
746746

747-
// Editing self (display, email)
748-
if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) {
749-
if (
750-
$targetUser->getBackend() instanceof ISetDisplayNameBackend
751-
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)
752-
) {
753-
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
754-
}
747+
$allowDisplayNameChange = $this->config->getSystemValue('allow_user_to_change_display_name', true);
748+
if ($allowDisplayNameChange === true && (
749+
$targetUser->getBackend() instanceof ISetDisplayNameBackend
750+
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)
751+
)) {
752+
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
753+
}
754+
755+
// Fallback to display name value to avoid changing behavior with the new option.
756+
if ($this->config->getSystemValue('allow_user_to_change_email', true)) {
755757
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
756758
}
757759

@@ -902,15 +904,16 @@ public function editUser(string $userId, string $key, string $value): DataRespon
902904

903905
$permittedFields = [];
904906
if ($targetUser->getUID() === $currentLoggedInUser->getUID()) {
905-
// Editing self (display, email)
906-
if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) {
907-
if (
908-
$targetUser->getBackend() instanceof ISetDisplayNameBackend
909-
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)
910-
) {
911-
$permittedFields[] = self::USER_FIELD_DISPLAYNAME;
912-
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
913-
}
907+
$allowDisplayNameChange = $this->config->getSystemValue('allow_user_to_change_display_name', true);
908+
if ($allowDisplayNameChange !== false && (
909+
$targetUser->getBackend() instanceof ISetDisplayNameBackend
910+
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)
911+
)) {
912+
$permittedFields[] = self::USER_FIELD_DISPLAYNAME;
913+
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
914+
}
915+
916+
if ($this->config->getSystemValue('allow_user_to_change_email', true)) {
914917
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
915918
}
916919

‎apps/provisioning_api/tests/Controller/UsersControllerTest.php‎

Lines changed: 82 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
use OCP\UserInterface;
4242
use PHPUnit\Framework\MockObject\MockObject;
4343
use Psr\Log\LoggerInterface;
44+
use RuntimeException;
4445
use Test\TestCase;
4546

4647
class UsersControllerTest extends TestCase {
@@ -1639,6 +1640,8 @@ public function testEditUserRegularUserSelfEditChangeEmailValid() {
16391640
->method('getBackend')
16401641
->willReturn($backend);
16411642

1643+
$this->config->method('getSystemValue')->willReturnCallback(fn (string $key, mixed $default) => $default);
1644+
16421645
$this->assertEquals([], $this->api->editUser('UserToEdit', 'email', 'demo@nextcloud.com')->getData());
16431646
}
16441647

@@ -1833,6 +1836,8 @@ public function testEditUserRegularUserSelfEditChangeEmailInvalid() {
18331836
->method('getBackend')
18341837
->willReturn($backend);
18351838

1839+
$this->config->method('getSystemValue')->willReturnCallback(fn (string $key, mixed $default) => $default);
1840+
18361841
$this->api->editUser('UserToEdit', 'email', 'demo.org');
18371842
}
18381843

@@ -4224,7 +4229,8 @@ public function testResendWelcomeMessageFailed() {
42244229

42254230
public function dataGetEditableFields() {
42264231
return [
4227-
[false, ISetDisplayNameBackend::class, [
4232+
[false, true, ISetDisplayNameBackend::class, [
4233+
IAccountManager::PROPERTY_EMAIL,
42284234
IAccountManager::COLLECTION_EMAIL,
42294235
IAccountManager::PROPERTY_PHONE,
42304236
IAccountManager::PROPERTY_ADDRESS,
@@ -4237,8 +4243,49 @@ public function dataGetEditableFields() {
42374243
IAccountManager::PROPERTY_BIOGRAPHY,
42384244
IAccountManager::PROPERTY_PROFILE_ENABLED,
42394245
]],
4240-
[true, ISetDisplayNameBackend::class, [
4246+
[true, false, ISetDisplayNameBackend::class, [
42414247
IAccountManager::PROPERTY_DISPLAYNAME,
4248+
IAccountManager::COLLECTION_EMAIL,
4249+
IAccountManager::PROPERTY_PHONE,
4250+
IAccountManager::PROPERTY_ADDRESS,
4251+
IAccountManager::PROPERTY_WEBSITE,
4252+
IAccountManager::PROPERTY_TWITTER,
4253+
IAccountManager::PROPERTY_FEDIVERSE,
4254+
IAccountManager::PROPERTY_ORGANISATION,
4255+
IAccountManager::PROPERTY_ROLE,
4256+
IAccountManager::PROPERTY_HEADLINE,
4257+
IAccountManager::PROPERTY_BIOGRAPHY,
4258+
IAccountManager::PROPERTY_PROFILE_ENABLED,
4259+
]],
4260+
[true, true, ISetDisplayNameBackend::class, [
4261+
IAccountManager::PROPERTY_DISPLAYNAME,
4262+
IAccountManager::PROPERTY_EMAIL,
4263+
IAccountManager::COLLECTION_EMAIL,
4264+
IAccountManager::PROPERTY_PHONE,
4265+
IAccountManager::PROPERTY_ADDRESS,
4266+
IAccountManager::PROPERTY_WEBSITE,
4267+
IAccountManager::PROPERTY_TWITTER,
4268+
IAccountManager::PROPERTY_FEDIVERSE,
4269+
IAccountManager::PROPERTY_ORGANISATION,
4270+
IAccountManager::PROPERTY_ROLE,
4271+
IAccountManager::PROPERTY_HEADLINE,
4272+
IAccountManager::PROPERTY_BIOGRAPHY,
4273+
IAccountManager::PROPERTY_PROFILE_ENABLED,
4274+
]],
4275+
[false, false, ISetDisplayNameBackend::class, [
4276+
IAccountManager::COLLECTION_EMAIL,
4277+
IAccountManager::PROPERTY_PHONE,
4278+
IAccountManager::PROPERTY_ADDRESS,
4279+
IAccountManager::PROPERTY_WEBSITE,
4280+
IAccountManager::PROPERTY_TWITTER,
4281+
IAccountManager::PROPERTY_FEDIVERSE,
4282+
IAccountManager::PROPERTY_ORGANISATION,
4283+
IAccountManager::PROPERTY_ROLE,
4284+
IAccountManager::PROPERTY_HEADLINE,
4285+
IAccountManager::PROPERTY_BIOGRAPHY,
4286+
IAccountManager::PROPERTY_PROFILE_ENABLED,
4287+
]],
4288+
[false, true, UserInterface::class, [
42424289
IAccountManager::PROPERTY_EMAIL,
42434290
IAccountManager::COLLECTION_EMAIL,
42444291
IAccountManager::PROPERTY_PHONE,
@@ -4252,7 +4299,20 @@ public function dataGetEditableFields() {
42524299
IAccountManager::PROPERTY_BIOGRAPHY,
42534300
IAccountManager::PROPERTY_PROFILE_ENABLED,
42544301
]],
4255-
[true, UserInterface::class, [
4302+
[true, false, UserInterface::class, [
4303+
IAccountManager::COLLECTION_EMAIL,
4304+
IAccountManager::PROPERTY_PHONE,
4305+
IAccountManager::PROPERTY_ADDRESS,
4306+
IAccountManager::PROPERTY_WEBSITE,
4307+
IAccountManager::PROPERTY_TWITTER,
4308+
IAccountManager::PROPERTY_FEDIVERSE,
4309+
IAccountManager::PROPERTY_ORGANISATION,
4310+
IAccountManager::PROPERTY_ROLE,
4311+
IAccountManager::PROPERTY_HEADLINE,
4312+
IAccountManager::PROPERTY_BIOGRAPHY,
4313+
IAccountManager::PROPERTY_PROFILE_ENABLED,
4314+
]],
4315+
[true, true, UserInterface::class, [
42564316
IAccountManager::PROPERTY_EMAIL,
42574317
IAccountManager::COLLECTION_EMAIL,
42584318
IAccountManager::PROPERTY_PHONE,
@@ -4266,6 +4326,19 @@ public function dataGetEditableFields() {
42664326
IAccountManager::PROPERTY_BIOGRAPHY,
42674327
IAccountManager::PROPERTY_PROFILE_ENABLED,
42684328
]],
4329+
[false, false, UserInterface::class, [
4330+
IAccountManager::COLLECTION_EMAIL,
4331+
IAccountManager::PROPERTY_PHONE,
4332+
IAccountManager::PROPERTY_ADDRESS,
4333+
IAccountManager::PROPERTY_WEBSITE,
4334+
IAccountManager::PROPERTY_TWITTER,
4335+
IAccountManager::PROPERTY_FEDIVERSE,
4336+
IAccountManager::PROPERTY_ORGANISATION,
4337+
IAccountManager::PROPERTY_ROLE,
4338+
IAccountManager::PROPERTY_HEADLINE,
4339+
IAccountManager::PROPERTY_BIOGRAPHY,
4340+
IAccountManager::PROPERTY_PROFILE_ENABLED,
4341+
]],
42694342
];
42704343
}
42714344

@@ -4276,13 +4349,12 @@ public function dataGetEditableFields() {
42764349
* @param string $userBackend
42774350
* @param array $expected
42784351
*/
4279-
public function testGetEditableFields(bool $allowedToChangeDisplayName, string $userBackend, array $expected) {
4280-
$this->config
4281-
->method('getSystemValue')
4282-
->with(
4283-
$this->equalTo('allow_user_to_change_display_name'),
4284-
$this->anything()
4285-
)->willReturn($allowedToChangeDisplayName);
4352+
public function testGetEditableFields(bool $allowedToChangeDisplayName, bool $allowedToChangeEmail, string $userBackend, array $expected): void {
4353+
$this->config->method('getSystemValue')->willReturnCallback(fn (string $key, mixed $default) => match ($key) {
4354+
'allow_user_to_change_display_name' => $allowedToChangeDisplayName,
4355+
'allow_user_to_change_email' => $allowedToChangeEmail,
4356+
default => throw new RuntimeException('Unexpected system config key: ' . $key),
4357+
});
42864358

42874359
$user = $this->createMock(IUser::class);
42884360
$this->userSession->method('getUser')

‎apps/settings/lib/Settings/Personal/PersonalInfo.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ public function getForm(): TemplateResponse {
148148
$accountParameters = [
149149
'avatarChangeSupported' => $user->canChangeAvatar(),
150150
'displayNameChangeSupported' => $user->canChangeDisplayName(),
151+
'emailChangeSupported' => $user->canChangeEmail(),
151152
'federationEnabled' => $federationEnabled,
152153
'lookupServerUploadEnabled' => $lookupServerUploadEnabled,
153154
];

‎apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
:scope.sync="primaryEmail.scope"
1414
@add-additional="onAddAdditionalEmail" />
1515

16-
<template v-if="displayNameChangeSupported">
16+
<template v-if="emailChangeSupported">
1717
<Email :input-id="inputId"
1818
:primary="true"
1919
:scope.sync="primaryEmail.scope"
@@ -56,7 +56,7 @@ import { validateEmail } from '../../../utils/validate.js'
5656
import { handleError } from '../../../utils/handlers.ts'
5757
5858
const { emailMap: { additionalEmails, primaryEmail, notificationEmail } } = loadState('settings', 'personalInfoParameters', {})
59-
const { displayNameChangeSupported } = loadState('settings', 'accountParameters', {})
59+
const { emailChangeSupported } = loadState('settings', 'accountParameters', {})
6060
6161
export default {
6262
name: 'EmailSection',
@@ -70,7 +70,7 @@ export default {
7070
return {
7171
accountProperty: ACCOUNT_PROPERTY_READABLE_ENUM.EMAIL,
7272
additionalEmails: additionalEmails.map(properties => ({ ...properties, key: this.generateUniqueKey() })),
73-
displayNameChangeSupported,
73+
emailChangeSupported,
7474
primaryEmail: { ...primaryEmail, readable: NAME_READABLE_ENUM[primaryEmail.name] },
7575
notificationEmail,
7676
}

‎build/psalm-baseline.xml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,6 +1068,11 @@
10681068
<code><![CDATA[null]]></code>
10691069
</NullArgument>
10701070
</file>
1071+
<file src="apps/settings/lib/Settings/Personal/PersonalInfo.php">
1072+
<UndefinedInterfaceMethod>
1073+
<code><![CDATA[canChangeEmail]]></code>
1074+
</UndefinedInterfaceMethod>
1075+
</file>
10711076
<file src="apps/sharebymail/lib/ShareByMailProvider.php">
10721077
<InvalidArgument>
10731078
<code><![CDATA[$share->getId()]]></code>
@@ -2774,6 +2779,11 @@
27742779
<code><![CDATA[false]]></code>
27752780
</FalsableReturnStatement>
27762781
</file>
2782+
<file src="lib/private/User/LazyUser.php">
2783+
<UndefinedInterfaceMethod>
2784+
<code><![CDATA[canChangeEmail]]></code>
2785+
</UndefinedInterfaceMethod>
2786+
</file>
27772787
<file src="lib/private/User/Manager.php">
27782788
<ImplementedReturnTypeMismatch>
27792789
<code><![CDATA[IUser|false]]></code>

‎core/Migrations/Version32000Date20250402182800.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
namespace OC\Core\Migrations;
8+
9+
use OCP\IConfig;
10+
use OCP\Migration\IOutput;
11+
use OCP\Migration\SimpleMigrationStep;
12+
13+
/**
14+
* Add `allow_user_to_change_email` system config
15+
*/
16+
class Version32000Date20250402182800 extends SimpleMigrationStep {
17+
18+
public function __construct(
19+
private IConfig $config,
20+
) {
21+
}
22+
23+
public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) {
24+
$allowDisplayName = $this->config->getSystemValue('allow_user_to_change_display_name', null);
25+
$allowEmail = $this->config->getSystemValue('allow_user_to_change_email', null);
26+
27+
// if displayname was set, but not the email setting, then set the email setting to the same as the email setting
28+
if ($allowDisplayName !== null && $allowEmail === null) {
29+
$this->config->setSystemValue('allow_user_to_change_email', $allowDisplayName === true);
30+
}
31+
}
32+
33+
}

‎dist/settings-vue-settings-personal-info.js‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/settings-vue-settings-personal-info.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,6 +1377,7 @@
13771377
'OC\\Core\\Migrations\\Version30000Date20240814180800' => $baseDir . '/core/Migrations/Version30000Date20240814180800.php',
13781378
'OC\\Core\\Migrations\\Version30000Date20240815080800' => $baseDir . '/core/Migrations/Version30000Date20240815080800.php',
13791379
'OC\\Core\\Migrations\\Version30000Date20240906095113' => $baseDir . '/core/Migrations/Version30000Date20240906095113.php',
1380+
'OC\\Core\\Migrations\\Version32000Date20250402182800' => $baseDir . '/core/Migrations/Version32000Date20250402182800.php',
13801381
'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',
13811382
'OC\\Core\\ResponseDefinitions' => $baseDir . '/core/ResponseDefinitions.php',
13821383
'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1410,6 +1410,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
14101410
'OC\\Core\\Migrations\\Version30000Date20240814180800' => __DIR__ . '/../../..' . '/core/Migrations/Version30000Date20240814180800.php',
14111411
'OC\\Core\\Migrations\\Version30000Date20240815080800' => __DIR__ . '/../../..' . '/core/Migrations/Version30000Date20240815080800.php',
14121412
'OC\\Core\\Migrations\\Version30000Date20240906095113' => __DIR__ . '/../../..' . '/core/Migrations/Version30000Date20240906095113.php',
1413+
'OC\\Core\\Migrations\\Version32000Date20250402182800' => __DIR__ . '/../../..' . '/core/Migrations/Version32000Date20250402182800.php',
14131414
'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',
14141415
'OC\\Core\\ResponseDefinitions' => __DIR__ . '/../../..' . '/core/ResponseDefinitions.php',
14151416
'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php',

0 commit comments

Comments
 (0)