Skip to content

Commit c5a8fae

Browse files
susnuxsusnux
authored
Merge pull request #51899 from nextcloud/backport/51745/stable29
[stable29] fix(settings): Handle email change restriction separately from display name change restriction
2 parents ba4dc2a + a6f3d15 commit c5a8fae

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
@@ -708,14 +708,16 @@ public function getEditableFieldsForUser(string $userId): DataResponse {
708708
$targetUser = $currentLoggedInUser;
709709
}
710710

711-
// Editing self (display, email)
712-
if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) {
713-
if (
714-
$targetUser->getBackend() instanceof ISetDisplayNameBackend
715-
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)
716-
) {
717-
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
718-
}
711+
$allowDisplayNameChange = $this->config->getSystemValue('allow_user_to_change_display_name', true);
712+
if ($allowDisplayNameChange === true && (
713+
$targetUser->getBackend() instanceof ISetDisplayNameBackend
714+
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)
715+
)) {
716+
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
717+
}
718+
719+
// Fallback to display name value to avoid changing behavior with the new option.
720+
if ($this->config->getSystemValue('allow_user_to_change_email', true)) {
719721
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
720722
}
721723

@@ -862,15 +864,16 @@ public function editUser(string $userId, string $key, string $value): DataRespon
862864

863865
$permittedFields = [];
864866
if ($targetUser->getUID() === $currentLoggedInUser->getUID()) {
865-
// Editing self (display, email)
866-
if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) {
867-
if (
868-
$targetUser->getBackend() instanceof ISetDisplayNameBackend
869-
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)
870-
) {
871-
$permittedFields[] = self::USER_FIELD_DISPLAYNAME;
872-
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
873-
}
867+
$allowDisplayNameChange = $this->config->getSystemValue('allow_user_to_change_display_name', true);
868+
if ($allowDisplayNameChange !== false && (
869+
$targetUser->getBackend() instanceof ISetDisplayNameBackend
870+
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)
871+
)) {
872+
$permittedFields[] = self::USER_FIELD_DISPLAYNAME;
873+
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
874+
}
875+
876+
if ($this->config->getSystemValue('allow_user_to_change_email', true)) {
874877
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
875878
}
876879

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

Lines changed: 82 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
use OCP\UserInterface;
7575
use PHPUnit\Framework\MockObject\MockObject;
7676
use Psr\Log\LoggerInterface;
77+
use RuntimeException;
7778
use Test\TestCase;
7879

7980
class UsersControllerTest extends TestCase {
@@ -1679,6 +1680,8 @@ public function testEditUserRegularUserSelfEditChangeEmailValid() {
16791680
->method('getBackend')
16801681
->willReturn($backend);
16811682

1683+
$this->config->method('getSystemValue')->willReturnCallback(fn (string $key, mixed $default) => $default);
1684+
16821685
$this->assertEquals([], $this->api->editUser('UserToEdit', 'email', 'demo@nextcloud.com')->getData());
16831686
}
16841687

@@ -1873,6 +1876,8 @@ public function testEditUserRegularUserSelfEditChangeEmailInvalid() {
18731876
->method('getBackend')
18741877
->willReturn($backend);
18751878

1879+
$this->config->method('getSystemValue')->willReturnCallback(fn (string $key, mixed $default) => $default);
1880+
18761881
$this->api->editUser('UserToEdit', 'email', 'demo.org');
18771882
}
18781883

@@ -4244,7 +4249,8 @@ public function testResendWelcomeMessageFailed() {
42444249

42454250
public function dataGetEditableFields() {
42464251
return [
4247-
[false, ISetDisplayNameBackend::class, [
4252+
[false, true, ISetDisplayNameBackend::class, [
4253+
IAccountManager::PROPERTY_EMAIL,
42484254
IAccountManager::COLLECTION_EMAIL,
42494255
IAccountManager::PROPERTY_PHONE,
42504256
IAccountManager::PROPERTY_ADDRESS,
@@ -4257,8 +4263,49 @@ public function dataGetEditableFields() {
42574263
IAccountManager::PROPERTY_BIOGRAPHY,
42584264
IAccountManager::PROPERTY_PROFILE_ENABLED,
42594265
]],
4260-
[true, ISetDisplayNameBackend::class, [
4266+
[true, false, ISetDisplayNameBackend::class, [
42614267
IAccountManager::PROPERTY_DISPLAYNAME,
4268+
IAccountManager::COLLECTION_EMAIL,
4269+
IAccountManager::PROPERTY_PHONE,
4270+
IAccountManager::PROPERTY_ADDRESS,
4271+
IAccountManager::PROPERTY_WEBSITE,
4272+
IAccountManager::PROPERTY_TWITTER,
4273+
IAccountManager::PROPERTY_FEDIVERSE,
4274+
IAccountManager::PROPERTY_ORGANISATION,
4275+
IAccountManager::PROPERTY_ROLE,
4276+
IAccountManager::PROPERTY_HEADLINE,
4277+
IAccountManager::PROPERTY_BIOGRAPHY,
4278+
IAccountManager::PROPERTY_PROFILE_ENABLED,
4279+
]],
4280+
[true, true, ISetDisplayNameBackend::class, [
4281+
IAccountManager::PROPERTY_DISPLAYNAME,
4282+
IAccountManager::PROPERTY_EMAIL,
4283+
IAccountManager::COLLECTION_EMAIL,
4284+
IAccountManager::PROPERTY_PHONE,
4285+
IAccountManager::PROPERTY_ADDRESS,
4286+
IAccountManager::PROPERTY_WEBSITE,
4287+
IAccountManager::PROPERTY_TWITTER,
4288+
IAccountManager::PROPERTY_FEDIVERSE,
4289+
IAccountManager::PROPERTY_ORGANISATION,
4290+
IAccountManager::PROPERTY_ROLE,
4291+
IAccountManager::PROPERTY_HEADLINE,
4292+
IAccountManager::PROPERTY_BIOGRAPHY,
4293+
IAccountManager::PROPERTY_PROFILE_ENABLED,
4294+
]],
4295+
[false, false, ISetDisplayNameBackend::class, [
4296+
IAccountManager::COLLECTION_EMAIL,
4297+
IAccountManager::PROPERTY_PHONE,
4298+
IAccountManager::PROPERTY_ADDRESS,
4299+
IAccountManager::PROPERTY_WEBSITE,
4300+
IAccountManager::PROPERTY_TWITTER,
4301+
IAccountManager::PROPERTY_FEDIVERSE,
4302+
IAccountManager::PROPERTY_ORGANISATION,
4303+
IAccountManager::PROPERTY_ROLE,
4304+
IAccountManager::PROPERTY_HEADLINE,
4305+
IAccountManager::PROPERTY_BIOGRAPHY,
4306+
IAccountManager::PROPERTY_PROFILE_ENABLED,
4307+
]],
4308+
[false, true, UserInterface::class, [
42624309
IAccountManager::PROPERTY_EMAIL,
42634310
IAccountManager::COLLECTION_EMAIL,
42644311
IAccountManager::PROPERTY_PHONE,
@@ -4272,7 +4319,20 @@ public function dataGetEditableFields() {
42724319
IAccountManager::PROPERTY_BIOGRAPHY,
42734320
IAccountManager::PROPERTY_PROFILE_ENABLED,
42744321
]],
4275-
[true, UserInterface::class, [
4322+
[true, false, UserInterface::class, [
4323+
IAccountManager::COLLECTION_EMAIL,
4324+
IAccountManager::PROPERTY_PHONE,
4325+
IAccountManager::PROPERTY_ADDRESS,
4326+
IAccountManager::PROPERTY_WEBSITE,
4327+
IAccountManager::PROPERTY_TWITTER,
4328+
IAccountManager::PROPERTY_FEDIVERSE,
4329+
IAccountManager::PROPERTY_ORGANISATION,
4330+
IAccountManager::PROPERTY_ROLE,
4331+
IAccountManager::PROPERTY_HEADLINE,
4332+
IAccountManager::PROPERTY_BIOGRAPHY,
4333+
IAccountManager::PROPERTY_PROFILE_ENABLED,
4334+
]],
4335+
[true, true, UserInterface::class, [
42764336
IAccountManager::PROPERTY_EMAIL,
42774337
IAccountManager::COLLECTION_EMAIL,
42784338
IAccountManager::PROPERTY_PHONE,
@@ -4286,6 +4346,19 @@ public function dataGetEditableFields() {
42864346
IAccountManager::PROPERTY_BIOGRAPHY,
42874347
IAccountManager::PROPERTY_PROFILE_ENABLED,
42884348
]],
4349+
[false, false, UserInterface::class, [
4350+
IAccountManager::COLLECTION_EMAIL,
4351+
IAccountManager::PROPERTY_PHONE,
4352+
IAccountManager::PROPERTY_ADDRESS,
4353+
IAccountManager::PROPERTY_WEBSITE,
4354+
IAccountManager::PROPERTY_TWITTER,
4355+
IAccountManager::PROPERTY_FEDIVERSE,
4356+
IAccountManager::PROPERTY_ORGANISATION,
4357+
IAccountManager::PROPERTY_ROLE,
4358+
IAccountManager::PROPERTY_HEADLINE,
4359+
IAccountManager::PROPERTY_BIOGRAPHY,
4360+
IAccountManager::PROPERTY_PROFILE_ENABLED,
4361+
]],
42894362
];
42904363
}
42914364

@@ -4296,13 +4369,12 @@ public function dataGetEditableFields() {
42964369
* @param string $userBackend
42974370
* @param array $expected
42984371
*/
4299-
public function testGetEditableFields(bool $allowedToChangeDisplayName, string $userBackend, array $expected) {
4300-
$this->config
4301-
->method('getSystemValue')
4302-
->with(
4303-
$this->equalTo('allow_user_to_change_display_name'),
4304-
$this->anything()
4305-
)->willReturn($allowedToChangeDisplayName);
4372+
public function testGetEditableFields(bool $allowedToChangeDisplayName, bool $allowedToChangeEmail, string $userBackend, array $expected): void {
4373+
$this->config->method('getSystemValue')->willReturnCallback(fn (string $key, mixed $default) => match ($key) {
4374+
'allow_user_to_change_display_name' => $allowedToChangeDisplayName,
4375+
'allow_user_to_change_email' => $allowedToChangeEmail,
4376+
default => throw new RuntimeException('Unexpected system config key: ' . $key),
4377+
});
43064378

43074379
$user = $this->createMock(IUser::class);
43084380
$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
@@ -172,6 +172,7 @@ public function getForm(): TemplateResponse {
172172
$accountParameters = [
173173
'avatarChangeSupported' => $user->canChangeAvatar(),
174174
'displayNameChangeSupported' => $user->canChangeDisplayName(),
175+
'emailChangeSupported' => $user->canChangeEmail(),
175176
'federationEnabled' => $federationEnabled,
176177
'lookupServerUploadEnabled' => $lookupServerUploadEnabled,
177178
];

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
:scope.sync="primaryEmail.scope"
3232
@add-additional="onAddAdditionalEmail" />
3333

34-
<template v-if="displayNameChangeSupported">
34+
<template v-if="emailChangeSupported">
3535
<Email :input-id="inputId"
3636
:primary="true"
3737
:scope.sync="primaryEmail.scope"
@@ -74,7 +74,7 @@ import { validateEmail } from '../../../utils/validate.js'
7474
import { handleError } from '../../../utils/handlers.js'
7575
7676
const { emailMap: { additionalEmails, primaryEmail, notificationEmail } } = loadState('settings', 'personalInfoParameters', {})
77-
const { displayNameChangeSupported } = loadState('settings', 'accountParameters', {})
77+
const { emailChangeSupported } = loadState('settings', 'accountParameters', {})
7878
7979
export default {
8080
name: 'EmailSection',
@@ -88,7 +88,7 @@ export default {
8888
return {
8989
accountProperty: ACCOUNT_PROPERTY_READABLE_ENUM.EMAIL,
9090
additionalEmails: additionalEmails.map(properties => ({ ...properties, key: this.generateUniqueKey() })),
91-
displayNameChangeSupported,
91+
emailChangeSupported,
9292
primaryEmail: { ...primaryEmail, readable: NAME_READABLE_ENUM[primaryEmail.name] },
9393
notificationEmail,
9494
}

‎build/psalm-baseline.xml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,6 +1087,11 @@
10871087
<code><![CDATA[null]]></code>
10881088
</NullArgument>
10891089
</file>
1090+
<file src="apps/settings/lib/Settings/Personal/PersonalInfo.php">
1091+
<UndefinedInterfaceMethod>
1092+
<code><![CDATA[canChangeEmail]]></code>
1093+
</UndefinedInterfaceMethod>
1094+
</file>
10901095
<file src="apps/sharebymail/lib/ShareByMailProvider.php">
10911096
<InvalidArgument>
10921097
<code><![CDATA[$share->getId()]]></code>
@@ -2896,6 +2901,11 @@
28962901
<code><![CDATA[false]]></code>
28972902
</FalsableReturnStatement>
28982903
</file>
2904+
<file src="lib/private/User/LazyUser.php">
2905+
<UndefinedInterfaceMethod>
2906+
<code><![CDATA[canChangeEmail]]></code>
2907+
</UndefinedInterfaceMethod>
2908+
</file>
28992909
<file src="lib/private/User/Manager.php">
29002910
<ImplementedReturnTypeMismatch>
29012911
<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
@@ -1284,6 +1284,7 @@
12841284
'OC\\Core\\Migrations\\Version29000Date20240124132202' => $baseDir . '/core/Migrations/Version29000Date20240124132202.php',
12851285
'OC\\Core\\Migrations\\Version29000Date20240131122720' => $baseDir . '/core/Migrations/Version29000Date20240131122720.php',
12861286
'OC\\Core\\Migrations\\Version30000Date20240814180800' => $baseDir . '/core/Migrations/Version30000Date20240814180800.php',
1287+
'OC\\Core\\Migrations\\Version32000Date20250402182800' => $baseDir . '/core/Migrations/Version32000Date20250402182800.php',
12871288
'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',
12881289
'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php',
12891290
'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1317,6 +1317,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
13171317
'OC\\Core\\Migrations\\Version29000Date20240124132202' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20240124132202.php',
13181318
'OC\\Core\\Migrations\\Version29000Date20240131122720' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20240131122720.php',
13191319
'OC\\Core\\Migrations\\Version30000Date20240814180800' => __DIR__ . '/../../..' . '/core/Migrations/Version30000Date20240814180800.php',
1320+
'OC\\Core\\Migrations\\Version32000Date20250402182800' => __DIR__ . '/../../..' . '/core/Migrations/Version32000Date20250402182800.php',
13201321
'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',
13211322
'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php',
13221323
'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php',

0 commit comments

Comments
 (0)