Merge pull request #26815 from nextcloud/backport/26461/stable21
[stable21] Only return display name as editable when the user backend allows it
This commit is contained in:
commit
5415fff855
|
@ -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'],
|
||||
|
|
|
@ -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;
|
||||
|
@ -70,6 +71,7 @@ use OCP\L10N\IFactory;
|
|||
use OCP\Security\ISecureRandom;
|
||||
use OCP\Security\Events\GenerateSecurePasswordEvent;
|
||||
use OCP\EventDispatcher\IEventDispatcher;
|
||||
use OCP\User\Backend\ISetDisplayNameBackend;
|
||||
|
||||
class UsersController extends AUserData {
|
||||
|
||||
|
@ -525,13 +527,39 @@ 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('', \OCP\API::RESPOND_NOT_FOUND);
|
||||
}
|
||||
|
||||
$permittedFields = [];
|
||||
|
||||
if ($userId !== $currentLoggedInUser->getUID()) {
|
||||
$targetUser = $this->userManager->get($userId);
|
||||
if (!$targetUser instanceof IUser) {
|
||||
throw new OCSException('', \OCP\API::RESPOND_NOT_FOUND);
|
||||
}
|
||||
|
||||
$subAdminManager = $this->groupManager->getSubAdmin();
|
||||
if (!$this->groupManager->isAdmin($currentLoggedInUser->getUID())
|
||||
&& !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) {
|
||||
throw new OCSException('', \OCP\API::RESPOND_NOT_FOUND);
|
||||
}
|
||||
} else {
|
||||
$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;
|
||||
}
|
||||
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
|
||||
}
|
||||
|
||||
|
@ -568,8 +596,11 @@ 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
|
||||
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)) {
|
||||
$permittedFields[] = 'display';
|
||||
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
|
||||
}
|
||||
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
|
||||
}
|
||||
|
||||
|
@ -608,8 +639,11 @@ 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
|
||||
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)) {
|
||||
$permittedFields[] = 'display';
|
||||
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
|
||||
}
|
||||
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
|
||||
$permittedFields[] = 'password';
|
||||
$permittedFields[] = 'language';
|
||||
|
|
|
@ -67,6 +67,7 @@ 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 Test\TestCase;
|
||||
|
@ -1443,6 +1444,10 @@ 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')
|
||||
|
@ -1484,6 +1489,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());
|
||||
}
|
||||
|
||||
|
@ -1517,6 +1528,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');
|
||||
}
|
||||
|
||||
|
@ -1550,6 +1567,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)
|
||||
|
@ -1594,6 +1617,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)
|
||||
|
@ -1638,6 +1667,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());
|
||||
}
|
||||
|
||||
|
@ -1671,6 +1706,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');
|
||||
}
|
||||
|
||||
|
@ -1703,6 +1744,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());
|
||||
}
|
||||
|
||||
|
@ -1738,6 +1785,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');
|
||||
}
|
||||
|
||||
|
@ -1777,6 +1830,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());
|
||||
}
|
||||
|
||||
|
@ -1819,6 +1878,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());
|
||||
}
|
||||
|
||||
|
@ -1869,6 +1934,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());
|
||||
}
|
||||
|
||||
|
@ -1910,6 +1981,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());
|
||||
}
|
||||
|
||||
|
@ -1956,6 +2033,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());
|
||||
}
|
||||
|
||||
|
@ -1995,6 +2078,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());
|
||||
}
|
||||
|
||||
|
@ -3716,20 +3805,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, UserInterface::class, [
|
||||
IAccountManager::PROPERTY_EMAIL,
|
||||
IAccountManager::PROPERTY_PHONE,
|
||||
IAccountManager::PROPERTY_ADDRESS,
|
||||
IAccountManager::PROPERTY_WEBSITE,
|
||||
IAccountManager::PROPERTY_TWITTER,
|
||||
]],
|
||||
];
|
||||
}
|
||||
|
||||
|
@ -3737,9 +3833,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(
|
||||
|
@ -3747,8 +3844,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) {
|
||||
|
|
Loading…
Reference in New Issue