Only return display name as editable when the user backend allows it

Signed-off-by: Joas Schilling <coding@schilljs.com>

3 files changed, 67 insertions, 11 deletions

diff --git a/apps/provisioning_api/appinfo/routes.php b/apps/provisioning_api/appinfo/routes.php
index 912dd82e853..6982a8ba288 100644
--- a/apps/provisioning_api/appinfo/routes.php
+++ b/apps/provisioning_api/appinfo/routes.php
@@ -53,6 +53,7 @@ return [
 		['root' => '/cloud', 'name' => 'Users#getUser', 'url' => '/users/{userId}', 'verb' => 'GET'],
 		['root' => '/cloud', 'name' => 'Users#getCurrentUser', 'url' => '/user', 'verb' => 'GET'],
 		['root' => '/cloud', 'name' => 'Users#getEditableFields', 'url' => '/user/fields', 'verb' => 'GET'],
+		['root' => '/cloud', 'name' => 'Users#getEditableFields', 'url' => '/user/fields/{userId}', 'verb' => 'GET'],
 		['root' => '/cloud', 'name' => 'Users#editUser', 'url' => '/users/{userId}', 'verb' => 'PUT'],
 		['root' => '/cloud', 'name' => 'Users#wipeUserDevices', 'url' => '/users/{userId}/wipe', 'verb' => 'POST'],
 		['root' => '/cloud', 'name' => 'Users#deleteUser', 'url' => '/users/{userId}', 'verb' => 'DELETE'],
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';
diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
index 4754c5a468d..d4c2faba98b 100644
--- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php
+++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
@@ -66,6 +66,8 @@ 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;
 use Psr\Log\LoggerInterface;
@@ -1446,6 +1448,10 @@ class UsersControllerTest extends TestCase {
 			->willReturn($targetUser);
 		$targetUser
 			->expects($this->once())
+			->method('getBackend')
+			->willReturn($this->createMock(ISetDisplayNameBackend::class));
+		$targetUser
+			->expects($this->once())
 			->method('setDisplayName')
 			->with('NewDisplayName');
 		$targetUser
@@ -3717,20 +3723,27 @@ class UsersControllerTest extends TestCase {
 
 	public function dataGetEditableFields() {
 		return [
-			[false, [
+			[false, ISetDisplayNameBackend::class, [
 				IAccountManager::PROPERTY_PHONE,
 				IAccountManager::PROPERTY_ADDRESS,
 				IAccountManager::PROPERTY_WEBSITE,
 				IAccountManager::PROPERTY_TWITTER,
 			]],
-			[ true, [
+			[true, ISetDisplayNameBackend::class, [
 				IAccountManager::PROPERTY_DISPLAYNAME,
 				IAccountManager::PROPERTY_EMAIL,
 				IAccountManager::PROPERTY_PHONE,
 				IAccountManager::PROPERTY_ADDRESS,
 				IAccountManager::PROPERTY_WEBSITE,
 				IAccountManager::PROPERTY_TWITTER,
-			]]
+			]],
+			[true, IGetDisplayNameBackend::class, [
+				IAccountManager::PROPERTY_EMAIL,
+				IAccountManager::PROPERTY_PHONE,
+				IAccountManager::PROPERTY_ADDRESS,
+				IAccountManager::PROPERTY_WEBSITE,
+				IAccountManager::PROPERTY_TWITTER,
+			]],
 		];
 	}
 
@@ -3738,9 +3751,10 @@ class UsersControllerTest extends TestCase {
 	 * @dataProvider dataGetEditableFields
 	 *
 	 * @param bool $allowedToChangeDisplayName
+	 * @param string $userBackend
 	 * @param array $expected
 	 */
-	public function testGetEditableFields(bool $allowedToChangeDisplayName, array $expected) {
+	public function testGetEditableFields(bool $allowedToChangeDisplayName, string $userBackend, array $expected) {
 		$this->config
 			->method('getSystemValue')
 			->with(
@@ -3748,8 +3762,19 @@ class UsersControllerTest extends TestCase {
 				$this->anything()
 			)->willReturn($allowedToChangeDisplayName);
 
+		$user = $this->createMock(IUser::class);
+		$this->userSession->method('getUser')
+			->willReturn($user);
+
+		$backend = $this->createMock($userBackend);
+
+		$user->method('getUID')
+			->willReturn('userId');
+		$user->method('getBackend')
+			->willReturn($backend);
+
 		$expectedResp = new DataResponse($expected);
-		$this->assertEquals($expectedResp, $this->api->getEditableFields());
+		$this->assertEquals($expectedResp, $this->api->getEditableFields('userId'));
 	}
 
 	private function mockAccount($targetUser, $accountProperties) {