Skip to content

Commit

Permalink
Also check implementsAction method
Browse files Browse the repository at this point in the history
Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
nickvergessen committed Apr 27, 2021
1 parent f67a10e commit 03b467b
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 5 deletions.
10 changes: 7 additions & 3 deletions apps/provisioning_api/lib/Controller/UsersController.php
Expand Up @@ -50,6 +50,7 @@
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;
Expand Down Expand Up @@ -568,7 +569,8 @@ public function getEditableFields(?string $userId = null): DataResponse {

// 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;
Expand Down Expand Up @@ -607,7 +609,8 @@ public function editUser(string $userId, string $key, string $value): DataRespon
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;
}
Expand Down Expand Up @@ -649,7 +652,8 @@ public function editUser(string $userId, string $key, string $value): DataRespon
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;
}
Expand Down
87 changes: 85 additions & 2 deletions apps/provisioning_api/tests/Controller/UsersControllerTest.php
Expand Up @@ -66,7 +66,6 @@
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;
Expand Down Expand Up @@ -1491,6 +1490,12 @@ public function testEditUserRegularUserSelfEditChangeEmailValid() {
->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());
}

Expand Down Expand Up @@ -1524,6 +1529,12 @@ public function testEditUserRegularUserSelfEditChangeEmailInvalid() {
->method('getUID')
->willReturn('UID');

$backend = $this->createMock(UserInterface::class);
$targetUser
->expects($this->any())
->method('getBackend')
->willReturn($backend);

$this->api->editUser('UserToEdit', 'email', 'demo.org');
}

Expand Down Expand Up @@ -1557,6 +1568,12 @@ public function testEditUserRegularUserSelfEditChangeProperty($propertyName, $ol
->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)
Expand Down Expand Up @@ -1601,6 +1618,12 @@ public function testEditUserRegularUserSelfEditChangePropertyScope($propertyName
->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)
Expand Down Expand Up @@ -1645,6 +1668,12 @@ public function testEditUserRegularUserSelfEditChangePassword() {
->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());
}

Expand Down Expand Up @@ -1678,6 +1707,12 @@ public function testEditUserRegularUserSelfEditChangeQuota() {
->method('getUID')
->willReturn('UID');

$backend = $this->createMock(UserInterface::class);
$targetUser
->expects($this->any())
->method('getBackend')
->willReturn($backend);

$this->api->editUser('UserToEdit', 'quota', 'NewQuota');
}

Expand Down Expand Up @@ -1710,6 +1745,12 @@ public function testEditUserAdminUserSelfEditChangeValidQuota() {
->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());
}

Expand Down Expand Up @@ -1745,6 +1786,12 @@ public function testEditUserAdminUserSelfEditChangeInvalidQuota() {
->method('getUID')
->willReturn('UID');

$backend = $this->createMock(UserInterface::class);
$targetUser
->expects($this->any())
->method('getBackend')
->willReturn($backend);

$this->api->editUser('UserToEdit', 'quota', 'ABC');
}

Expand Down Expand Up @@ -1784,6 +1831,12 @@ public function testEditUserAdminUserEditChangeValidQuota() {
->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());
}

Expand Down Expand Up @@ -1826,6 +1879,12 @@ public function testEditUserSelfEditChangeLanguage() {
->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());
}

Expand Down Expand Up @@ -1876,6 +1935,12 @@ public function testEditUserSelfEditChangeLanguageButForced($forced) {
->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());
}

Expand Down Expand Up @@ -1917,6 +1982,12 @@ public function testEditUserAdminEditChangeLanguage() {
->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());
}

Expand Down Expand Up @@ -1963,6 +2034,12 @@ public function testEditUserAdminEditChangeLanguageInvalidLanguage() {
->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());
}

Expand Down Expand Up @@ -2002,6 +2079,12 @@ public function testEditUserSubadminUserAccessible() {
->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());
}

Expand Down Expand Up @@ -3737,7 +3820,7 @@ public function dataGetEditableFields() {
IAccountManager::PROPERTY_WEBSITE,
IAccountManager::PROPERTY_TWITTER,
]],
[true, IGetDisplayNameBackend::class, [
[true, UserInterface::class, [
IAccountManager::PROPERTY_EMAIL,
IAccountManager::PROPERTY_PHONE,
IAccountManager::PROPERTY_ADDRESS,
Expand Down

0 comments on commit 03b467b

Please sign in to comment.