From de011ee6687651fe1bc10023d93df0e0afeb7f39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 29 Jan 2026 18:01:31 +0100 Subject: [PATCH] feat: Allow user backends to manage property permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not yet reflected in the UI apart from displayname/email/avatar Signed-off-by: Côme Chilliet --- .../lib/Controller/UsersController.php | 90 +++++-------------- .../lib/Controller/UsersController.php | 4 +- .../tests/Controller/UsersControllerTest.php | 15 ++-- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/User/LazyUser.php | 10 ++- lib/private/User/User.php | 67 +++++++------- lib/public/IUser.php | 18 ++-- .../Backend/IPropertyPermissionBackend.php | 29 ++++++ 9 files changed, 122 insertions(+), 113 deletions(-) create mode 100644 lib/public/User/Backend/IPropertyPermissionBackend.php diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 12ada1c0213..ef88985fdaf 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -770,33 +770,20 @@ class UsersController extends AUserDataOCSController { $targetUser = $currentLoggedInUser; } - $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; + foreach (IAccountManager::ALLOWED_PROPERTIES as $property) { + if ($property === IAccountManager::PROPERTY_AVATAR) { + continue; + } + if (!$targetUser->canEditProperty($property)) { + continue; + } + $permittedFields[] = $property; } - // 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; + if ($targetUser->canEditProperty(IAccountManager::COLLECTION_EMAIL)) { + $permittedFields[] = IAccountManager::COLLECTION_EMAIL; } - $permittedFields[] = IAccountManager::COLLECTION_EMAIL; - $permittedFields[] = IAccountManager::PROPERTY_PHONE; - $permittedFields[] = IAccountManager::PROPERTY_ADDRESS; - $permittedFields[] = IAccountManager::PROPERTY_WEBSITE; - $permittedFields[] = IAccountManager::PROPERTY_TWITTER; - $permittedFields[] = IAccountManager::PROPERTY_BLUESKY; - $permittedFields[] = IAccountManager::PROPERTY_FEDIVERSE; - $permittedFields[] = IAccountManager::PROPERTY_ORGANISATION; - $permittedFields[] = IAccountManager::PROPERTY_ROLE; - $permittedFields[] = IAccountManager::PROPERTY_HEADLINE; - $permittedFields[] = IAccountManager::PROPERTY_BIOGRAPHY; - $permittedFields[] = IAccountManager::PROPERTY_PROFILE_ENABLED; - $permittedFields[] = IAccountManager::PROPERTY_PRONOUNS; - return new DataResponse($permittedFields); } @@ -841,7 +828,9 @@ class UsersController extends AUserDataOCSController { $permittedFields = []; if ($targetUser->getUID() === $currentLoggedInUser->getUID()) { // Editing self (display, email) - $permittedFields[] = IAccountManager::COLLECTION_EMAIL; + if ($targetUser->canEditProperty(IAccountManager::COLLECTION_EMAIL)) { + $permittedFields[] = IAccountManager::COLLECTION_EMAIL; + } $permittedFields[] = IAccountManager::COLLECTION_EMAIL . self::SCOPE_SUFFIX; } else { // Check if admin / subadmin @@ -933,23 +922,10 @@ class UsersController extends AUserDataOCSController { $permittedFields = []; if ($targetUser->getUID() === $currentLoggedInUser->getUID()) { - $allowDisplayNameChange = $this->config->getSystemValue('allow_user_to_change_display_name', true); - if ($allowDisplayNameChange !== false && ( - $targetUser->getBackend() instanceof ISetDisplayNameBackend - || $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME) - )) { + if ($targetUser->canChangeDisplayName()) { $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; - } - - $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_EMAIL . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::COLLECTION_EMAIL; $permittedFields[] = self::USER_FIELD_PASSWORD; @@ -972,34 +948,16 @@ class UsersController extends AUserDataOCSController { $permittedFields[] = self::USER_FIELD_FIRST_DAY_OF_WEEK; } - $permittedFields[] = IAccountManager::PROPERTY_PHONE; - $permittedFields[] = IAccountManager::PROPERTY_ADDRESS; - $permittedFields[] = IAccountManager::PROPERTY_WEBSITE; - $permittedFields[] = IAccountManager::PROPERTY_TWITTER; - $permittedFields[] = IAccountManager::PROPERTY_BLUESKY; - $permittedFields[] = IAccountManager::PROPERTY_FEDIVERSE; - $permittedFields[] = IAccountManager::PROPERTY_ORGANISATION; - $permittedFields[] = IAccountManager::PROPERTY_ROLE; - $permittedFields[] = IAccountManager::PROPERTY_HEADLINE; - $permittedFields[] = IAccountManager::PROPERTY_BIOGRAPHY; - $permittedFields[] = IAccountManager::PROPERTY_PROFILE_ENABLED; - $permittedFields[] = IAccountManager::PROPERTY_BIRTHDATE; - $permittedFields[] = IAccountManager::PROPERTY_PRONOUNS; - - $permittedFields[] = IAccountManager::PROPERTY_PHONE . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_ADDRESS . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_WEBSITE . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_TWITTER . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_BLUESKY . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_FEDIVERSE . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_ORGANISATION . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_ROLE . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_HEADLINE . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_BIOGRAPHY . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_PROFILE_ENABLED . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_BIRTHDATE . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_AVATAR . self::SCOPE_SUFFIX; - $permittedFields[] = IAccountManager::PROPERTY_PRONOUNS . self::SCOPE_SUFFIX; + foreach (IAccountManager::ALLOWED_PROPERTIES as $property) { + $permittedFields[] = $property . self::SCOPE_SUFFIX; + if ($property === IAccountManager::PROPERTY_AVATAR) { + continue; + } + if (!$targetUser->canEditProperty($property)) { + continue; + } + $permittedFields[] = $property; + } // If admin they can edit their own quota and manager $isAdmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID()); diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index d08572f926f..43eda1eab1a 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -422,10 +422,8 @@ class UsersController extends Controller { IAccountManager::PROPERTY_BIRTHDATE => ['value' => $birthdate, 'scope' => $birthdateScope], IAccountManager::PROPERTY_PRONOUNS => ['value' => $pronouns, 'scope' => $pronounsScope], ]; - $allowUserToChangeDisplayName = $this->config->getSystemValueBool('allow_user_to_change_display_name', true); foreach ($updatable as $property => $data) { - if ($allowUserToChangeDisplayName === false - && in_array($property, [IAccountManager::PROPERTY_DISPLAYNAME, IAccountManager::PROPERTY_EMAIL], true)) { + if (!$user->canEditProperty($property)) { continue; } $property = $userAccount->getProperty($property); diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index f8e25279df1..ce8de5498d9 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -337,6 +337,16 @@ class UsersControllerTest extends \Test\TestCase { $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('johndoe'); + $user->expects($this->atLeastOnce()) + ->method('canEditProperty') + ->willReturnCallback( + fn (string $property): bool => match($property) { + IAccountManager::PROPERTY_EMAIL, + IAccountManager::PROPERTY_DISPLAYNAME => false, + default => true, + } + ); + $this->userSession->method('getUser')->willReturn($user); /** @var MockObject|IAccount $userAccount */ @@ -378,11 +388,6 @@ class UsersControllerTest extends \Test\TestCase { $emailProperty->expects($this->never()) ->method('setScope'); - $this->config->expects($this->once()) - ->method('getSystemValueBool') - ->with('allow_user_to_change_display_name') - ->willReturn(false); - $this->appManager->expects($this->any()) ->method('isEnabledForUser') ->with('federatedfilesharing') diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 0cec2593039..7c1e273f0c4 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1013,6 +1013,7 @@ return array( 'OCP\\User\\Backend\\ILimitAwareCountUsersBackend' => $baseDir . '/lib/public/User/Backend/ILimitAwareCountUsersBackend.php', 'OCP\\User\\Backend\\IPasswordConfirmationBackend' => $baseDir . '/lib/public/User/Backend/IPasswordConfirmationBackend.php', 'OCP\\User\\Backend\\IPasswordHashBackend' => $baseDir . '/lib/public/User/Backend/IPasswordHashBackend.php', + 'OCP\\User\\Backend\\IPropertyPermissionBackend' => $baseDir . '/lib/public/User/Backend/IPropertyPermissionBackend.php', 'OCP\\User\\Backend\\IProvideAvatarBackend' => $baseDir . '/lib/public/User/Backend/IProvideAvatarBackend.php', 'OCP\\User\\Backend\\IProvideEnabledStateBackend' => $baseDir . '/lib/public/User/Backend/IProvideEnabledStateBackend.php', 'OCP\\User\\Backend\\ISearchKnownUsersBackend' => $baseDir . '/lib/public/User/Backend/ISearchKnownUsersBackend.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index c07b6d127cc..775f8fce9ac 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1054,6 +1054,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\User\\Backend\\ILimitAwareCountUsersBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/ILimitAwareCountUsersBackend.php', 'OCP\\User\\Backend\\IPasswordConfirmationBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IPasswordConfirmationBackend.php', 'OCP\\User\\Backend\\IPasswordHashBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IPasswordHashBackend.php', + 'OCP\\User\\Backend\\IPropertyPermissionBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IPropertyPermissionBackend.php', 'OCP\\User\\Backend\\IProvideAvatarBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IProvideAvatarBackend.php', 'OCP\\User\\Backend\\IProvideEnabledStateBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IProvideEnabledStateBackend.php', 'OCP\\User\\Backend\\ISearchKnownUsersBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/ISearchKnownUsersBackend.php', diff --git a/lib/private/User/LazyUser.php b/lib/private/User/LazyUser.php index 501169019d4..093a1824026 100644 --- a/lib/private/User/LazyUser.php +++ b/lib/private/User/LazyUser.php @@ -100,15 +100,15 @@ class LazyUser implements IUser { return $this->getUser()->getBackend(); } - public function canChangeAvatar() { + public function canChangeAvatar(): bool { return $this->getUser()->canChangeAvatar(); } - public function canChangePassword() { + public function canChangePassword(): bool { return $this->getUser()->canChangePassword(); } - public function canChangeDisplayName() { + public function canChangeDisplayName(): bool { return $this->getUser()->canChangeDisplayName(); } @@ -116,6 +116,10 @@ class LazyUser implements IUser { return $this->getUser()->canChangeEmail(); } + public function canEditProperty(string $property): bool { + return $this->getUser()->canEditProperty($property); + } + public function isEnabled() { return $this->getUser()->isEnabled(); } diff --git a/lib/private/User/User.php b/lib/private/User/User.php index 1de44193cbc..c5719daf67e 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -27,6 +27,7 @@ use OCP\IUserBackend; use OCP\Notification\IManager as INotificationManager; use OCP\User\Backend\IGetHomeBackend; use OCP\User\Backend\IPasswordHashBackend; +use OCP\User\Backend\IPropertyPermissionBackend; use OCP\User\Backend\IProvideAvatarBackend; use OCP\User\Backend\IProvideEnabledStateBackend; use OCP\User\Backend\ISetDisplayNameBackend; @@ -413,44 +414,50 @@ class User implements IUser { return $this->backend; } - /** - * Check if the backend allows the user to change their avatar on Personal page - * - * @return bool - */ - public function canChangeAvatar() { - if ($this->backend instanceof IProvideAvatarBackend || $this->backend->implementsActions(Backend::PROVIDE_AVATAR)) { - /** @var IProvideAvatarBackend $backend */ - $backend = $this->backend; - return $backend->canChangeAvatar($this->uid); - } - return true; + public function canChangeAvatar(): bool { + return $this->canEditProperty(IAccountManager::PROPERTY_AVATAR); } - /** - * check if the backend supports changing passwords - * - * @return bool - */ - public function canChangePassword() { + public function canChangePassword(): bool { return $this->backend->implementsActions(Backend::SET_PASSWORD); } - /** - * check if the backend supports changing display names - * - * @return bool - */ - public function canChangeDisplayName() { - if (!$this->config->getSystemValueBool('allow_user_to_change_display_name', true)) { - return false; - } - return $this->backend->implementsActions(Backend::SET_DISPLAYNAME); + public function canChangeDisplayName(): bool { + return $this->canEditProperty(IAccountManager::PROPERTY_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)); + return $this->canEditProperty(IAccountManager::PROPERTY_EMAIL); + } + + /** + * @param IAccountManager::PROPERTY_*|IAccountManager::COLLECTION_* $property + */ + public function canEditProperty(string $property): bool { + if ($this->backend instanceof IPropertyPermissionBackend) { + $permission = $this->backend->canEditProperty($this->uid, $property); + if (!$permission) { + return false; + } + } + switch ($property) { + case IAccountManager::PROPERTY_DISPLAYNAME: + if (!$this->config->getSystemValueBool('allow_user_to_change_display_name', true)) { + return false; + } + return $this->backend->implementsActions(Backend::SET_DISPLAYNAME); + case IAccountManager::PROPERTY_AVATAR: + if ($this->backend instanceof IProvideAvatarBackend || $this->backend->implementsActions(Backend::PROVIDE_AVATAR)) { + /** @var IProvideAvatarBackend $backend */ + $backend = $this->backend; + return $backend->canChangeAvatar($this->uid); + } + return true; + case IAccountManager::PROPERTY_EMAIL: + return $this->config->getSystemValueBool('allow_user_to_change_email', $this->config->getSystemValueBool('allow_user_to_change_display_name', true)); + default: + return true; + } } /** diff --git a/lib/public/IUser.php b/lib/public/IUser.php index 945e7e1602a..399be67789b 100644 --- a/lib/public/IUser.php +++ b/lib/public/IUser.php @@ -8,12 +8,15 @@ namespace OCP; use InvalidArgumentException; +use OCP\Accounts\IAccountManager; +use OCP\AppFramework\Attribute\Consumable; /** * Interface IUser * * @since 8.0.0 */ +#[Consumable(since: '8.0.0')] interface IUser { /** * @since 32.0.0 @@ -133,26 +136,23 @@ interface IUser { /** * check if the backend allows the user to change their avatar on Personal page * - * @return bool * @since 8.0.0 */ - public function canChangeAvatar(); + public function canChangeAvatar(): bool; /** * check if the backend supports changing passwords * - * @return bool * @since 8.0.0 */ - public function canChangePassword(); + public function canChangePassword(): bool; /** * check if the backend supports changing display names * - * @return bool * @since 8.0.0 */ - public function canChangeDisplayName(); + public function canChangeDisplayName(): bool; /** * Check if the backend supports changing email @@ -161,6 +161,12 @@ interface IUser { */ public function canChangeEmail(): bool; + /** + * @param IAccountManager::PROPERTY_*|IAccountManager::COLLECTION_* $property + * @since 34.0.0 + */ + public function canEditProperty(string $property): bool; + /** * check if the user is enabled * diff --git a/lib/public/User/Backend/IPropertyPermissionBackend.php b/lib/public/User/Backend/IPropertyPermissionBackend.php new file mode 100644 index 00000000000..8c76144a3ce --- /dev/null +++ b/lib/public/User/Backend/IPropertyPermissionBackend.php @@ -0,0 +1,29 @@ +