mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #26461 from nextcloud/bugfix/noid/only-return-editable-displayname-field-when-allowed-by-backend 

Only return display name as editable when the user backend allows it
This commit is contained in:
Morris Jobke 2021-04-28 20:27:40 +02:00 committed by GitHub
parent d93b001e27 03b467bd23
commit b971f69e26
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 154 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
apps/provisioning_api/appinfo/routes.php
View File

@ -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'],

46
apps/provisioning_api/lib/Controller/UsersController.php
View File

@ -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;
use Psr\Log\LoggerInterface;
class UsersController extends AUserData {
@ -538,13 +540,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('', 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
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)) {
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
}
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
}
@ -581,8 +609,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) {
$permittedFields[] = 'display';
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
if ($targetUser->getBackend() instanceof ISetDisplayNameBackend
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)) {
$permittedFields[] = 'display';
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
}
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
}
@ -621,8 +652,11 @@ 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
|| $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)) {
$permittedFields[] = 'display';
$permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME;
}
$permittedFields[] = IAccountManager::PROPERTY_EMAIL;
$permittedFields[] = 'password';
$permittedFields[] = 'language';

118
apps/provisioning_api/tests/Controller/UsersControllerTest.php
View File

@ -66,6 +66,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 Psr\Log\LoggerInterface;
@ -1444,6 +1445,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')
@ -1485,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());
}
@ -1518,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');
}
@ -1551,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)
@ -1595,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)
@ -1639,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());
}
@ -1672,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');
}
@ -1704,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());
}
@ -1739,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');
}
@ -1778,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());
}
@ -1820,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());
}
@ -1870,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());
}
@ -1911,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());
}
@ -1957,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());
}
@ -1996,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());
}
@ -3717,20 +3806,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,
]],
];
}
@ -3738,9 +3834,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 +3845,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) {