Merge pull request #51745 from nextcloud/fix/settings/email-change-restriction

fix(settings): Handle email change restriction separately from display name change restriction
This commit is contained in:
John Molakvoæ 2025-04-03 08:19:37 +02:00 committed by GitHub
commit fff96486cd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 164 additions and 33 deletions

View file

@ -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', true)) {
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
}
@ -907,15 +909,16 @@ 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;
}
if ($this->config->getSystemValue('allow_user_to_change_email', true)) {
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
}

View file

@ -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')

View file

@ -114,6 +114,7 @@ class PersonalInfo implements ISettings {
$accountParameters = [
'avatarChangeSupported' => $user->canChangeAvatar(),
'displayNameChangeSupported' => $user->canChangeDisplayName(),
'emailChangeSupported' => $user->canChangeEmail(),
'federationEnabled' => $federationEnabled,
'lookupServerUploadEnabled' => $lookupServerUploadEnabled,
];

View file

@ -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,
}

View file

@ -0,0 +1,33 @@
<?php
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OC\Core\Migrations;
use OCP\IConfig;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
/**
* Add `allow_user_to_change_email` system config
*/
class Version32000Date20250402182800 extends SimpleMigrationStep {
public function __construct(
private IConfig $config,
) {
}
public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) {
$allowDisplayName = $this->config->getSystemValue('allow_user_to_change_display_name', null);
$allowEmail = $this->config->getSystemValue('allow_user_to_change_email', null);
// if displayname was set, but not the email setting, then set the email setting to the same as the email setting
if ($allowDisplayName !== null && $allowEmail === null) {
$this->config->setSystemValue('allow_user_to_change_email', $allowDisplayName === true);
}
}
}

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View file

@ -1453,6 +1453,7 @@ return array(
'OC\\Core\\Migrations\\Version31000Date20240101084401' => $baseDir . '/core/Migrations/Version31000Date20240101084401.php',
'OC\\Core\\Migrations\\Version31000Date20240814184402' => $baseDir . '/core/Migrations/Version31000Date20240814184402.php',
'OC\\Core\\Migrations\\Version31000Date20250213102442' => $baseDir . '/core/Migrations/Version31000Date20250213102442.php',
'OC\\Core\\Migrations\\Version32000Date20250402182800' => $baseDir . '/core/Migrations/Version32000Date20250402182800.php',
'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',
'OC\\Core\\ResponseDefinitions' => $baseDir . '/core/ResponseDefinitions.php',
'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php',

View file

@ -1502,6 +1502,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Core\\Migrations\\Version31000Date20240101084401' => __DIR__ . '/../../..' . '/core/Migrations/Version31000Date20240101084401.php',
'OC\\Core\\Migrations\\Version31000Date20240814184402' => __DIR__ . '/../../..' . '/core/Migrations/Version31000Date20240814184402.php',
'OC\\Core\\Migrations\\Version31000Date20250213102442' => __DIR__ . '/../../..' . '/core/Migrations/Version31000Date20250213102442.php',
'OC\\Core\\Migrations\\Version32000Date20250402182800' => __DIR__ . '/../../..' . '/core/Migrations/Version32000Date20250402182800.php',
'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',
'OC\\Core\\ResponseDefinitions' => __DIR__ . '/../../..' . '/core/ResponseDefinitions.php',
'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php',

View file

@ -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();
}

View file

@ -447,6 +447,10 @@ class User implements IUser {
return $this->backend->implementsActions(Backend::SET_DISPLAYNAME);
}
public function canChangeEmail(): bool {
return $this->config->getSystemValueBool('allow_user_to_change_email', true);
}
/**
* check if the user is enabled
*

View file

@ -149,6 +149,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
*