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/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index da3548375ab..c135360b8f1 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -39,7 +39,6 @@ use OCP\L10N\IFactory; use OCP\Mail\IEMailTemplate; use OCP\Security\Events\GenerateSecurePasswordEvent; use OCP\Security\ISecureRandom; -use OCP\User\Backend\ISetDisplayNameBackend; use OCP\UserInterface; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -1635,6 +1634,10 @@ class UsersControllerTest extends TestCase { $targetUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() ->getMock(); + $targetUser + ->expects($this->once()) + ->method('canChangeDisplayName') + ->willReturn(true); $this->userSession ->expects($this->once()) ->method('getUser') @@ -1644,10 +1647,6 @@ class UsersControllerTest extends TestCase { ->method('get') ->with('UserToEdit') ->willReturn($targetUser); - $targetUser - ->expects($this->once()) - ->method('getBackend') - ->willReturn($this->createMock(ISetDisplayNameBackend::class)); $targetUser ->expects($this->once()) ->method('setDisplayName') @@ -1672,6 +1671,14 @@ class UsersControllerTest extends TestCase { $targetUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() ->getMock(); + $targetUser + ->expects($this->atLeastOnce()) + ->method('canEditProperty') + ->willReturnCallback( + fn (string $property): bool => match($property) { + IAccountManager::PROPERTY_EMAIL => true, + default => false, + }); $this->userSession ->expects($this->once()) ->method('getUser') @@ -1690,12 +1697,6 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); - $backend = $this->createMock(UserInterface::class); - $targetUser - ->expects($this->any()) - ->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()); @@ -1726,12 +1727,6 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); - $backend = $this->createMock(UserInterface::class); - $targetUser - ->expects($this->any()) - ->method('getBackend') - ->willReturn($backend); - $userAccount = $this->createMock(IAccount::class); $this->accountManager @@ -1772,11 +1767,6 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); - $backend = $this->createMock(UserInterface::class); - $targetUser - ->expects($this->any()) - ->method('getBackend') - ->willReturn($backend); $targetUser ->expects($this->any()) ->method('getSystemEMailAddress') @@ -1824,12 +1814,6 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); - $backend = $this->createMock(UserInterface::class); - $targetUser - ->expects($this->any()) - ->method('getBackend') - ->willReturn($backend); - $property = $this->createMock(IAccountProperty::class); $property->method('getValue') ->willReturn('demo1@nextcloud.com'); @@ -1886,11 +1870,14 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); - $backend = $this->createMock(UserInterface::class); $targetUser - ->expects($this->any()) - ->method('getBackend') - ->willReturn($backend); + ->expects($this->atLeastOnce()) + ->method('canEditProperty') + ->willReturnCallback( + fn (string $property): bool => match($property) { + IAccountManager::PROPERTY_EMAIL => true, + default => false, + }); $this->config->method('getSystemValue')->willReturnCallback(fn (string $key, mixed $default) => $default); @@ -1923,6 +1910,14 @@ class UsersControllerTest extends TestCase { ->expects($this->any()) ->method('getUID') ->willReturn('UID'); + $loggedInUser + ->expects($this->atLeastOnce()) + ->method('canEditProperty') + ->willReturnCallback( + fn (string $property): bool => match($property) { + $propertyName => true, + default => false, + }); $this->userSession ->expects($this->once()) ->method('getUser') @@ -4290,137 +4285,79 @@ class UsersControllerTest extends TestCase { public static function dataGetEditableFields(): array { return [ - [false, true, ISetDisplayNameBackend::class, [ + [false, true, [ + IAccountManager::PROPERTY_ADDRESS, + IAccountManager::PROPERTY_BIOGRAPHY, + IAccountManager::PROPERTY_BIRTHDATE, IAccountManager::PROPERTY_EMAIL, - IAccountManager::COLLECTION_EMAIL, - IAccountManager::PROPERTY_PHONE, - IAccountManager::PROPERTY_ADDRESS, - IAccountManager::PROPERTY_WEBSITE, - IAccountManager::PROPERTY_TWITTER, - IAccountManager::PROPERTY_BLUESKY, IAccountManager::PROPERTY_FEDIVERSE, - IAccountManager::PROPERTY_ORGANISATION, - IAccountManager::PROPERTY_ROLE, IAccountManager::PROPERTY_HEADLINE, - IAccountManager::PROPERTY_BIOGRAPHY, + IAccountManager::PROPERTY_ORGANISATION, + IAccountManager::PROPERTY_PHONE, IAccountManager::PROPERTY_PROFILE_ENABLED, IAccountManager::PROPERTY_PRONOUNS, + IAccountManager::PROPERTY_ROLE, + IAccountManager::PROPERTY_TWITTER, + IAccountManager::PROPERTY_BLUESKY, + IAccountManager::PROPERTY_WEBSITE, + IAccountManager::COLLECTION_EMAIL, ]], - [true, false, ISetDisplayNameBackend::class, [ + [true, false, [ + IAccountManager::PROPERTY_ADDRESS, + IAccountManager::PROPERTY_BIOGRAPHY, + IAccountManager::PROPERTY_BIRTHDATE, IAccountManager::PROPERTY_DISPLAYNAME, - IAccountManager::COLLECTION_EMAIL, - IAccountManager::PROPERTY_PHONE, - IAccountManager::PROPERTY_ADDRESS, - IAccountManager::PROPERTY_WEBSITE, - IAccountManager::PROPERTY_TWITTER, - IAccountManager::PROPERTY_BLUESKY, IAccountManager::PROPERTY_FEDIVERSE, - IAccountManager::PROPERTY_ORGANISATION, - IAccountManager::PROPERTY_ROLE, IAccountManager::PROPERTY_HEADLINE, - IAccountManager::PROPERTY_BIOGRAPHY, + IAccountManager::PROPERTY_ORGANISATION, + IAccountManager::PROPERTY_PHONE, IAccountManager::PROPERTY_PROFILE_ENABLED, IAccountManager::PROPERTY_PRONOUNS, + IAccountManager::PROPERTY_ROLE, + IAccountManager::PROPERTY_TWITTER, + IAccountManager::PROPERTY_BLUESKY, + IAccountManager::PROPERTY_WEBSITE, + IAccountManager::COLLECTION_EMAIL, ]], - [true, true, ISetDisplayNameBackend::class, [ + [true, true, [ + IAccountManager::PROPERTY_ADDRESS, + IAccountManager::PROPERTY_BIOGRAPHY, + IAccountManager::PROPERTY_BIRTHDATE, IAccountManager::PROPERTY_DISPLAYNAME, IAccountManager::PROPERTY_EMAIL, - IAccountManager::COLLECTION_EMAIL, - IAccountManager::PROPERTY_PHONE, - IAccountManager::PROPERTY_ADDRESS, - IAccountManager::PROPERTY_WEBSITE, - IAccountManager::PROPERTY_TWITTER, - IAccountManager::PROPERTY_BLUESKY, IAccountManager::PROPERTY_FEDIVERSE, - IAccountManager::PROPERTY_ORGANISATION, - IAccountManager::PROPERTY_ROLE, IAccountManager::PROPERTY_HEADLINE, - IAccountManager::PROPERTY_BIOGRAPHY, + IAccountManager::PROPERTY_ORGANISATION, + IAccountManager::PROPERTY_PHONE, IAccountManager::PROPERTY_PROFILE_ENABLED, IAccountManager::PROPERTY_PRONOUNS, + IAccountManager::PROPERTY_ROLE, + IAccountManager::PROPERTY_TWITTER, + IAccountManager::PROPERTY_BLUESKY, + IAccountManager::PROPERTY_WEBSITE, + IAccountManager::COLLECTION_EMAIL, ]], - [false, false, ISetDisplayNameBackend::class, [ - IAccountManager::COLLECTION_EMAIL, - IAccountManager::PROPERTY_PHONE, + [false, false, [ IAccountManager::PROPERTY_ADDRESS, - IAccountManager::PROPERTY_WEBSITE, - IAccountManager::PROPERTY_TWITTER, - IAccountManager::PROPERTY_BLUESKY, - IAccountManager::PROPERTY_FEDIVERSE, - IAccountManager::PROPERTY_ORGANISATION, - IAccountManager::PROPERTY_ROLE, - IAccountManager::PROPERTY_HEADLINE, IAccountManager::PROPERTY_BIOGRAPHY, + IAccountManager::PROPERTY_BIRTHDATE, + IAccountManager::PROPERTY_FEDIVERSE, + IAccountManager::PROPERTY_HEADLINE, + IAccountManager::PROPERTY_ORGANISATION, + IAccountManager::PROPERTY_PHONE, IAccountManager::PROPERTY_PROFILE_ENABLED, IAccountManager::PROPERTY_PRONOUNS, - ]], - [false, true, UserInterface::class, [ - IAccountManager::PROPERTY_EMAIL, - IAccountManager::COLLECTION_EMAIL, - IAccountManager::PROPERTY_PHONE, - IAccountManager::PROPERTY_ADDRESS, - IAccountManager::PROPERTY_WEBSITE, + IAccountManager::PROPERTY_ROLE, IAccountManager::PROPERTY_TWITTER, IAccountManager::PROPERTY_BLUESKY, - IAccountManager::PROPERTY_FEDIVERSE, - IAccountManager::PROPERTY_ORGANISATION, - IAccountManager::PROPERTY_ROLE, - IAccountManager::PROPERTY_HEADLINE, - IAccountManager::PROPERTY_BIOGRAPHY, - 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_BLUESKY, - 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_BLUESKY, - 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_BLUESKY, - IAccountManager::PROPERTY_FEDIVERSE, - IAccountManager::PROPERTY_ORGANISATION, - IAccountManager::PROPERTY_ROLE, - IAccountManager::PROPERTY_HEADLINE, - IAccountManager::PROPERTY_BIOGRAPHY, - IAccountManager::PROPERTY_PROFILE_ENABLED, - IAccountManager::PROPERTY_PRONOUNS, ]], ]; } #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataGetEditableFields')] - public function testGetEditableFields(bool $allowedToChangeDisplayName, bool $allowedToChangeEmail, string $userBackend, array $expected): void { + public function testGetEditableFields(bool $allowedToChangeDisplayName, bool $allowedToChangeEmail, 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, @@ -4431,12 +4368,16 @@ class UsersControllerTest extends TestCase { $this->userSession->method('getUser') ->willReturn($user); - $backend = $this->createMock($userBackend); - $user->method('getUID') ->willReturn('userId'); - $user->method('getBackend') - ->willReturn($backend); + $user->method('canEditProperty') + ->willReturnCallback( + fn (string $property): bool => match($property) { + IAccountManager::PROPERTY_DISPLAYNAME => $allowedToChangeDisplayName, + IAccountManager::PROPERTY_EMAIL => $allowedToChangeEmail, + default => true, + } + ); $expectedResp = new DataResponse($expected); $this->assertEquals($expectedResp, $this->api->getEditableFields('userId')); 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/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php index 708aeb037fd..e7d4571059f 100644 --- a/apps/user_ldap/lib/User_LDAP.php +++ b/apps/user_ldap/lib/User_LDAP.php @@ -14,15 +14,17 @@ use OCA\User_LDAP\Exceptions\NotOnLDAP; use OCA\User_LDAP\User\DeletedUsersIndex; use OCA\User_LDAP\User\OfflineUser; use OCA\User_LDAP\User\User; +use OCP\Accounts\IAccountManager; use OCP\IUserBackend; use OCP\Notification\IManager as INotificationManager; use OCP\User\Backend\ICountMappedUsersBackend; use OCP\User\Backend\ILimitAwareCountUsersBackend; +use OCP\User\Backend\IPropertyPermissionBackend; use OCP\User\Backend\IProvideEnabledStateBackend; use OCP\UserInterface; use Psr\Log\LoggerInterface; -class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, IUserLDAP, ILimitAwareCountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend { +class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, IUserLDAP, ILimitAwareCountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend, IPropertyPermissionBackend { public function __construct( Access $access, protected INotificationManager $notificationManager, @@ -643,4 +645,23 @@ class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, I public function getDisabledUserList(?int $limit = null, int $offset = 0, string $search = ''): array { throw new \Exception('This is implemented directly in User_Proxy'); } + + public function canEditProperty(string $uid, string $property): bool { + return match($property) { + // Display name is always set by LDAP + IAccountManager::PROPERTY_DISPLAYNAME => false, + IAccountManager::PROPERTY_EMAIL => ((string)$this->access->connection->ldapEmailAttribute !== ''), + IAccountManager::PROPERTY_PHONE => ((string)$this->access->connection->ldapAttributePhone !== ''), + IAccountManager::PROPERTY_WEBSITE => ((string)$this->access->connection->ldapAttributeWebsite !== ''), + IAccountManager::PROPERTY_ADDRESS => ((string)$this->access->connection->ldapAttributeAddress !== ''), + IAccountManager::PROPERTY_FEDIVERSE => ((string)$this->access->connection->ldapAttributeFediverse !== ''), + IAccountManager::PROPERTY_ORGANISATION => ((string)$this->access->connection->ldapAttributeOrganisation !== ''), + IAccountManager::PROPERTY_ROLE => ((string)$this->access->connection->ldapAttributeRole !== ''), + IAccountManager::PROPERTY_HEADLINE => ((string)$this->access->connection->ldapAttributeHeadline !== ''), + IAccountManager::PROPERTY_BIOGRAPHY => ((string)$this->access->connection->ldapAttributeBiography !== ''), + IAccountManager::PROPERTY_BIRTHDATE => ((string)$this->access->connection->ldapAttributeBirthDate !== ''), + IAccountManager::PROPERTY_PRONOUNS => ((string)$this->access->connection->ldapAttributePronouns !== ''), + default => true, + }; + } } diff --git a/apps/user_ldap/lib/User_Proxy.php b/apps/user_ldap/lib/User_Proxy.php index a4a44362ae4..6a01ab34830 100644 --- a/apps/user_ldap/lib/User_Proxy.php +++ b/apps/user_ldap/lib/User_Proxy.php @@ -15,6 +15,7 @@ use OCP\Notification\IManager as INotificationManager; use OCP\User\Backend\ICountMappedUsersBackend; use OCP\User\Backend\IGetDisplayNameBackend; use OCP\User\Backend\ILimitAwareCountUsersBackend; +use OCP\User\Backend\IPropertyPermissionBackend; use OCP\User\Backend\IProvideEnabledStateBackend; use OCP\UserInterface; use Psr\Log\LoggerInterface; @@ -22,7 +23,7 @@ use Psr\Log\LoggerInterface; /** * @template-extends Proxy */ -class User_Proxy extends Proxy implements IUserBackend, UserInterface, IUserLDAP, ILimitAwareCountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend, IGetDisplayNameBackend { +class User_Proxy extends Proxy implements IUserBackend, UserInterface, IUserLDAP, ILimitAwareCountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend, IGetDisplayNameBackend, IPropertyPermissionBackend { public function __construct( private Helper $helper, ILDAPWrapper $ldap, @@ -432,4 +433,8 @@ class User_Proxy extends Proxy implements IUserBackend, UserInterface, IUserLDAP ) ); } + + public function canEditProperty(string $uid, string $property): bool { + return $this->handleRequest($uid, 'canEditProperty', [$uid, $property]); + } } diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 5216a3ef432..a13a21f71c4 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2135,9 +2135,6 @@ - - - @@ -2148,8 +2145,6 @@ - - @@ -2498,8 +2493,6 @@ - - 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 @@ +