Skip to content

Commit 8020429

Browse files
provokateurinsusnuxartonge
committed
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>
1 parent 57f485d commit 8020429

File tree

7 files changed

+126
-30
lines changed

7 files changed

+126
-30
lines changed

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

Lines changed: 21 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', $allowDisplayNameChange)) {
719721
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
720722
}
721723

@@ -862,15 +864,17 @@ 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+
// Fallback to display name value to avoid changing behavior with the new option.
877+
if ($this->config->getSystemValue('allow_user_to_change_email', $allowDisplayNameChange)) {
874878
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
875879
}
876880

‎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>

‎lib/private/User/LazyUser.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,10 @@ public function canChangeDisplayName() {
115115
return $this->getUser()->canChangeDisplayName();
116116
}
117117

118+
public function canChangeEmail(): bool {
119+
return $this->getUser()->canChangeEmail();
120+
}
121+
118122
public function isEnabled() {
119123
return $this->getUser()->isEnabled();
120124
}

‎lib/private/User/User.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,11 @@ public function canChangeDisplayName() {
440440
return $this->backend->implementsActions(Backend::SET_DISPLAYNAME);
441441
}
442442

443+
public function canChangeEmail(): bool {
444+
// Fallback to display name value to avoid changing behavior with the new option.
445+
return $this->config->getSystemValueBool('allow_user_to_change_email', $this->config->getSystemValueBool('allow_user_to_change_display_name', true));
446+
}
447+
443448
/**
444449
* check if the user is enabled
445450
*

0 commit comments

Comments
 (0)