From f67a10e8d096ba859540e678857e2611e3b2cfd0 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 8 Apr 2021 13:28:13 +0200 Subject: Only return display name as editable when the user backend allows it Signed-off-by: Joas Schilling --- .../lib/Controller/UsersController.php | 42 ++++++++++++++++++---- 1 file changed, 36 insertions(+), 6 deletions(-) (limited to 'apps/provisioning_api/lib/Controller') diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 5961a3cca05..dad9fdfe3fa 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -70,6 +70,7 @@ use OCP\L10N\IFactory; use OCP\Security\ISecureRandom; use OCP\Security\Events\GenerateSecurePasswordEvent; use OCP\EventDispatcher\IEventDispatcher; +use OCP\User\Backend\ISetDisplayNameBackend; use Psr\Log\LoggerInterface; class UsersController extends AUserData { @@ -538,13 +539,38 @@ class UsersController extends AUserData { /** * @NoAdminRequired * @NoSubAdminRequired + * + * @return DataResponse + * @throws OCSException */ - public function getEditableFields(): DataResponse { + public function getEditableFields(?string $userId = null): DataResponse { + $currentLoggedInUser = $this->userSession->getUser(); + if (!$currentLoggedInUser instanceof IUser) { + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); + } + $permittedFields = []; + if ($userId !== $currentLoggedInUser->getUID()) { + $targetUser = $this->userManager->get($userId); + if (!$targetUser instanceof IUser) { + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); + } + + $subAdminManager = $this->groupManager->getSubAdmin(); + if (!$this->groupManager->isAdmin($currentLoggedInUser->getUID()) + && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) { + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); + } + } else { + $targetUser = $currentLoggedInUser; + } + // Editing self (display, email) if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) { - $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; + if ($targetUser->getBackend() instanceof ISetDisplayNameBackend) { + $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; + } $permittedFields[] = IAccountManager::PROPERTY_EMAIL; } @@ -581,8 +607,10 @@ class UsersController extends AUserData { if ($targetUser->getUID() === $currentLoggedInUser->getUID()) { // Editing self (display, email) if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) { - $permittedFields[] = 'display'; - $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; + if ($targetUser->getBackend() instanceof ISetDisplayNameBackend) { + $permittedFields[] = 'display'; + $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; + } $permittedFields[] = IAccountManager::PROPERTY_EMAIL; } @@ -621,8 +649,10 @@ class UsersController extends AUserData { if ($this->groupManager->isAdmin($currentLoggedInUser->getUID()) || $subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) { // They have permissions over the user - $permittedFields[] = 'display'; - $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; + if ($targetUser->getBackend() instanceof ISetDisplayNameBackend) { + $permittedFields[] = 'display'; + $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; + } $permittedFields[] = IAccountManager::PROPERTY_EMAIL; $permittedFields[] = 'password'; $permittedFields[] = 'language'; -- cgit v1.2.3 From 03b467bd237a2b7849092c3991a76ed1205bd583 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 26 Apr 2021 14:34:03 +0200 Subject: Also check implementsAction method Signed-off-by: Joas Schilling --- .../lib/Controller/UsersController.php | 10 ++- .../tests/Controller/UsersControllerTest.php | 87 +++++++++++++++++++++- 2 files changed, 92 insertions(+), 5 deletions(-) (limited to 'apps/provisioning_api/lib/Controller') diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index dad9fdfe3fa..115b955354b 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -50,6 +50,7 @@ use OC\Accounts\AccountManager; use OC\Authentication\Token\RemoteWipe; use OC\HintException; use OC\KnownUser\KnownUserService; +use OC\User\Backend; use OCA\Settings\Mailer\NewUserMailHelper; use OCP\Accounts\IAccountManager; use OCP\App\IAppManager; @@ -568,7 +569,8 @@ class UsersController extends AUserData { // Editing self (display, email) if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) { - if ($targetUser->getBackend() instanceof ISetDisplayNameBackend) { + if ($targetUser->getBackend() instanceof ISetDisplayNameBackend + || $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)) { $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; } $permittedFields[] = IAccountManager::PROPERTY_EMAIL; @@ -607,7 +609,8 @@ class UsersController extends AUserData { 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) { + if ($targetUser->getBackend() instanceof ISetDisplayNameBackend + || $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)) { $permittedFields[] = 'display'; $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; } @@ -649,7 +652,8 @@ class UsersController extends AUserData { if ($this->groupManager->isAdmin($currentLoggedInUser->getUID()) || $subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) { // They have permissions over the user - if ($targetUser->getBackend() instanceof ISetDisplayNameBackend) { + if ($targetUser->getBackend() instanceof ISetDisplayNameBackend + || $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)) { $permittedFields[] = 'display'; $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; } diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index d4c2faba98b..1afe9be4319 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -66,7 +66,6 @@ use OCP\L10N\IFactory; use OCP\Mail\IEMailTemplate; use OCP\Security\Events\GenerateSecurePasswordEvent; use OCP\Security\ISecureRandom; -use OCP\User\Backend\IGetDisplayNameBackend; use OCP\User\Backend\ISetDisplayNameBackend; use OCP\UserInterface; use PHPUnit\Framework\MockObject\MockObject; @@ -1491,6 +1490,12 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); + $backend = $this->createMock(UserInterface::class); + $targetUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'email', 'demo@nextcloud.com')->getData()); } @@ -1524,6 +1529,12 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); + $backend = $this->createMock(UserInterface::class); + $targetUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->api->editUser('UserToEdit', 'email', 'demo.org'); } @@ -1557,6 +1568,12 @@ class UsersControllerTest extends TestCase { ->with('UserToEdit') ->willReturn($loggedInUser); + $backend = $this->createMock(UserInterface::class); + $loggedInUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->accountManager->expects($this->once()) ->method('getUser') ->with($loggedInUser) @@ -1601,6 +1618,12 @@ class UsersControllerTest extends TestCase { ->with('UserToEdit') ->willReturn($loggedInUser); + $backend = $this->createMock(UserInterface::class); + $loggedInUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->accountManager->expects($this->once()) ->method('getUser') ->with($loggedInUser) @@ -1645,6 +1668,12 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); + $backend = $this->createMock(UserInterface::class); + $targetUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'password', 'NewPassword')->getData()); } @@ -1678,6 +1707,12 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); + $backend = $this->createMock(UserInterface::class); + $targetUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->api->editUser('UserToEdit', 'quota', 'NewQuota'); } @@ -1710,6 +1745,12 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); + $backend = $this->createMock(UserInterface::class); + $targetUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'quota', '3042824')->getData()); } @@ -1745,6 +1786,12 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); + $backend = $this->createMock(UserInterface::class); + $targetUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->api->editUser('UserToEdit', 'quota', 'ABC'); } @@ -1784,6 +1831,12 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); + $backend = $this->createMock(UserInterface::class); + $targetUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'quota', '3042824')->getData()); } @@ -1826,6 +1879,12 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UserToEdit'); + $backend = $this->createMock(UserInterface::class); + $targetUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'language', 'de')->getData()); } @@ -1876,6 +1935,12 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UserToEdit'); + $backend = $this->createMock(UserInterface::class); + $targetUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'language', 'de')->getData()); } @@ -1917,6 +1982,12 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UserToEdit'); + $backend = $this->createMock(UserInterface::class); + $targetUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'language', 'de')->getData()); } @@ -1963,6 +2034,12 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UserToEdit'); + $backend = $this->createMock(UserInterface::class); + $targetUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'language', 'ru')->getData()); } @@ -2002,6 +2079,12 @@ class UsersControllerTest extends TestCase { ->method('getUID') ->willReturn('UID'); + $backend = $this->createMock(UserInterface::class); + $targetUser + ->expects($this->any()) + ->method('getBackend') + ->willReturn($backend); + $this->assertEquals([], $this->api->editUser('UserToEdit', 'quota', '3042824')->getData()); } @@ -3737,7 +3820,7 @@ class UsersControllerTest extends TestCase { IAccountManager::PROPERTY_WEBSITE, IAccountManager::PROPERTY_TWITTER, ]], - [true, IGetDisplayNameBackend::class, [ + [true, UserInterface::class, [ IAccountManager::PROPERTY_EMAIL, IAccountManager::PROPERTY_PHONE, IAccountManager::PROPERTY_ADDRESS, -- cgit v1.2.3