Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace OCA\Provisioning_API\Controller;

use OC\Group\DisplayNameCache as GroupDisplayNameCache;
use OC\Group\Manager as GroupManager;
use OC\User\Backend;
use OC\User\NoUserException;
Expand Down Expand Up @@ -36,6 +37,7 @@

/**
* @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions
* @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions
* @psalm-import-type Provisioning_APIUserDetailsQuota from ResponseDefinitions
*/
abstract class AUserDataOCSController extends OCSController {
Expand All @@ -62,6 +64,7 @@ public function __construct(
protected ISubAdmin $subAdminManager,
protected IFactory $l10nFactory,
protected IRootFolder $rootFolder,
private GroupDisplayNameCache $groupDisplayNameCache,
) {
parent::__construct($appName, $request);
}
Expand Down Expand Up @@ -105,6 +108,7 @@ protected function getUserData(string $userId, bool $includeScopes = false): ?ar
$userAccount = $this->accountManager->getAccount($targetUserObject);
$groups = $this->groupManager->getUserGroups($targetUserObject);
$gids = [];
$gidsDisplayName = [];
foreach ($groups as $group) {
$gids[] = $group->getGID();
}
Expand Down
3 changes: 3 additions & 0 deletions apps/provisioning_api/lib/Controller/GroupsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace OCA\Provisioning_API\Controller;

use OC\Group\DisplayNameCache as GroupDisplayNameCache;
use OCA\Provisioning_API\ResponseDefinitions;
use OCA\Settings\Settings\Admin\Sharing;
use OCA\Settings\Settings\Admin\Users;
Expand Down Expand Up @@ -52,6 +53,7 @@ public function __construct(
IFactory $l10nFactory,
IRootFolder $rootFolder,
private LoggerInterface $logger,
protected GroupDisplayNameCache $groupDisplayNameCache,
) {
parent::__construct($appName,
$request,
Expand All @@ -63,6 +65,7 @@ public function __construct(
$subAdminManager,
$l10nFactory,
$rootFolder,
$this->groupDisplayNameCache,
);
}

Expand Down
26 changes: 24 additions & 2 deletions apps/provisioning_api/lib/Controller/UsersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

use InvalidArgumentException;
use OC\Authentication\Token\RemoteWipe;
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
use OC\Group\Group;
use OC\KnownUser\KnownUserService;
use OC\User\Backend;
Expand Down Expand Up @@ -83,6 +84,7 @@
private IPhoneNumberUtil $phoneNumberUtil,
private IAppManager $appManager,
private IAppConfig $appConfig,
protected GroupDisplayNameCache $groupDisplayNameCache,
) {
parent::__construct(
$appName,
Expand All @@ -95,6 +97,7 @@
$subAdminManager,
$l10nFactory,
$rootFolder,
$groupDisplayNameCache,
);

$this->l10n = $l10nFactory->get($appName);
Expand Down Expand Up @@ -148,11 +151,11 @@
* @param string $search Text to search for
* @param int|null $limit Limit the amount of groups returned
* @param int $offset Offset for searching for groups
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>}, array{}>
* @return DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>, groups: array<string, Provisioning_APIUserDetailsGroupDisplayname}, array{}>
*
* 200: Users details returned
*/
#[NoAdminRequired]

Check failure on line 158 in apps/provisioning_api/lib/Controller/UsersController.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidDocblock

apps/provisioning_api/lib/Controller/UsersController.php:158:2: InvalidDocblock: Invalid string DataResponse<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>, groups: array<string, Provisioning_APIUserDetailsGroupDisplayname}, array{}> * 200: Users details returned in docblock for OCA\Provisioning_API\Controller\UsersController::getUsersDetails (see https://psalm.dev/008)
public function getUsersDetails(string $search = '', ?int $limit = null, int $offset = 0): DataResponse {
$currentUser = $this->userSession->getUser();
$users = [];
Expand Down Expand Up @@ -200,10 +203,29 @@
}

return new DataResponse([
'users' => $usersDetails
'users' => $usersDetails,
'groups' => $this->findGroupsWithDisplayname($usersDetails),
]);
}

private function findGroupsWithDisplayname(array $userDetails): array {
$groupIds = [];

foreach ($userDetails as $userDetail) {
if (isset($userDetail['groups'])) {
array_push($groupIds, ...array_values($userDetail['groups']));
}
}

$groupIds = array_unique($groupIds);
sort($groupIds);

return array_map(function ($groupId) {
$displayname = $this->groupDisplayNameCache->getDisplayName($groupId) ?? $groupId;
return ['id' => $groupId, 'name' => $displayname];
}, $groupIds);
}

/**
* Get the list of disabled users and their details
*
Expand Down
5 changes: 5 additions & 0 deletions apps/provisioning_api/lib/ResponseDefinitions.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
*
* @psalm-type Provisioning_APIUserDetailsScope = 'v2-private'|'v2-local'|'v2-federated'|'v2-published'
*
* @psalm-type Provisioning_APIUserDetailsGroupDisplayname = array{
* id: string,
* name: string,
* }
*
* @psalm-type Provisioning_APIUserDetails = array{
* additional_mail: list<string>,
* additional_mailScope?: list<Provisioning_APIUserDetailsScope>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace OCA\Provisioning_API\Tests\Controller;

use OC\Group\DisplayNameCache as GroupDisplayNameCache;
use OC\Group\Manager;
use OC\User\NoUserException;
use OCA\Provisioning_API\Controller\GroupsController;
Expand Down Expand Up @@ -37,6 +38,7 @@ class GroupsControllerTest extends \Test\TestCase {
protected IFactory&MockObject $l10nFactory;
protected LoggerInterface&MockObject $logger;
protected GroupsController&MockObject $api;
private GroupDisplayNameCache&MockObject $groupDisplayNameCache;

private IRootFolder $rootFolder;

Expand All @@ -53,6 +55,7 @@ protected function setUp(): void {
$this->l10nFactory = $this->createMock(IFactory::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->rootFolder = $this->createMock(IRootFolder::class);
$this->groupDisplayNameCache = $this->createMock(GroupDisplayNameCache::class);

$this->groupManager
->method('getSubAdmin')
Expand All @@ -70,7 +73,8 @@ protected function setUp(): void {
$this->subAdminManager,
$this->l10nFactory,
$this->rootFolder,
$this->logger
$this->logger,
$this->groupDisplayNameCache,
])
->onlyMethods(['fillStorageInfo'])
->getMock();
Expand Down
106 changes: 103 additions & 3 deletions apps/provisioning_api/tests/Controller/UsersControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use Exception;
use OC\Authentication\Token\RemoteWipe;
use OC\Group\DisplayNameCache as GroupDisplayNameCache;
use OC\Group\Manager;
use OC\KnownUser\KnownUserService;
use OC\PhoneNumberUtil;
Expand Down Expand Up @@ -70,6 +71,7 @@ class UsersControllerTest extends TestCase {
private IPhoneNumberUtil $phoneNumberUtil;
private IAppManager $appManager;
private IAppConfig&MockObject $appConfig;
private GroupDisplayNameCache&MockObject $groupDisplayNameCache;

protected function setUp(): void {
parent::setUp();
Expand All @@ -93,6 +95,7 @@ protected function setUp(): void {
$this->appManager = $this->createMock(IAppManager::class);
$this->appConfig = $this->createMock(IAppConfig::class);
$this->rootFolder = $this->createMock(IRootFolder::class);
$this->groupDisplayNameCache = $this->createMock(GroupDisplayNameCache::class);

$l10n = $this->createMock(IL10N::class);
$l10n->method('t')->willReturnCallback(fn (string $txt, array $replacement = []) => sprintf($txt, ...$replacement));
Expand Down Expand Up @@ -120,6 +123,7 @@ protected function setUp(): void {
$this->phoneNumberUtil,
$this->appManager,
$this->appConfig,
$this->groupDisplayNameCache,
])
->onlyMethods(['fillStorageInfo'])
->getMock();
Expand Down Expand Up @@ -216,6 +220,87 @@ public function testGetUsersAsSubAdmin(): void {
$this->assertEquals($expected, $this->api->getUsers('MyCustomSearch')->getData());
}

public function testGetUsersDetailsReturnsEmptyGroupsList(): void {
$loggedInUser = $this->getMockBuilder(IUser::class)
->disableOriginalConstructor()
->getMock();
$loggedInUser
->expects($this->once())
->method('getUID')
->willReturn('admin');

$this->userSession
->expects($this->once())
->method('getUser')
->willReturn($loggedInUser);

$this->groupManager
->expects($this->once())
->method('getSubAdmin')
->willReturn($this->subAdminManager);
$this->groupManager
->expects($this->once())
->method('isAdmin')
->with('admin')
->willReturn(true);
$this->groupManager
->expects($this->once())
->method('isDelegatedAdmin')
->with('admin')
->willReturn(false);

$this->userManager
->expects($this->once())
->method('search')
->with('MyCustomSearch', 3, 0)
->willReturn(['UID' => []]);

$api = $this->getMockBuilder(UsersController::class)
->setConstructorArgs([
'provisioning_api',
$this->request,
$this->userManager,
$this->config,
$this->groupManager,
$this->userSession,
$this->accountManager,
$this->subAdminManager,
$this->l10nFactory,
$this->rootFolder,
$this->urlGenerator,
$this->logger,
$this->newUserMailHelper,
$this->secureRandom,
$this->remoteWipe,
$this->knownUserService,
$this->eventDispatcher,
$this->phoneNumberUtil,
$this->appManager,
$this->appConfig,
$this->groupDisplayNameCache,
])
->onlyMethods(['getUserData'])
->getMock();

$api->expects($this->once())
->method('getUserData')
->with('UID')
->willReturn([
'id' => 'UID',
'groups' => [],
]);

$this->assertEquals([
'users' => [
'UID' => [
'id' => 'UID',
'groups' => [],
],
],
'groups' => [],
], $api->getUsersDetails('MyCustomSearch', 3)->getData());
}

private function createUserMock(string $uid, bool $enabled): MockObject&IUser {
$mockUser = $this->getMockBuilder(IUser::class)
->disableOriginalConstructor()
Expand Down Expand Up @@ -506,6 +591,7 @@ public function testAddUserSuccessfulWithDisplayName(): void {
$this->phoneNumberUtil,
$this->appManager,
$this->appConfig,
$this->groupDisplayNameCache,
])
->onlyMethods(['editUser'])
->getMock();
Expand Down Expand Up @@ -1120,18 +1206,23 @@ public function testGetUserDataAsAdmin(): void {
->expects($this->once())
->method('getSubAdminsGroups')
->willReturn([$group3]);
$group0->expects($this->once())
$group0->expects($this->exactly(3))
->method('getGID')
->willReturn('group0');
$group1->expects($this->once())
$group1->expects($this->exactly(3))
->method('getGID')
->willReturn('group1');
$group2->expects($this->once())
$group2->expects($this->exactly(3))
->method('getGID')
->willReturn('group2');
$group3->expects($this->once())
->method('getGID')
->willReturn('group3');
$this->groupDisplayNameCache
->method('getDisplayName')
->willReturnCallback(function (string $gid): string {
return ucfirst($gid);
});

$this->mockAccount($targetUser, [
IAccountManager::PROPERTY_ADDRESS => ['value' => 'address'],
Expand Down Expand Up @@ -1233,6 +1324,11 @@ public function testGetUserDataAsAdmin(): void {
'notify_email' => null,
'manager' => '',
'pronouns' => 'they/them',
'groupsWithDisplayname' => [
['id' => 'group0', 'name' => 'Group0'],
['id' => 'group1', 'name' => 'Group1'],
['id' => 'group2', 'name' => 'Group2'],
],
];
$this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID']));
}
Expand Down Expand Up @@ -1381,6 +1477,7 @@ public function testGetUserDataAsSubAdminAndUserIsAccessible(): void {
'notify_email' => null,
'manager' => '',
'pronouns' => 'they/them',
'groupsWithDisplayname' => [],
];
$this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID']));
}
Expand Down Expand Up @@ -1565,6 +1662,7 @@ public function testGetUserDataAsSubAdminSelfLookup(): void {
'notify_email' => null,
'manager' => '',
'pronouns' => 'they/them',
'groupsWithDisplayname' => [],
];
$this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID']));
}
Expand Down Expand Up @@ -4101,6 +4199,7 @@ public function testGetCurrentUserLoggedIn(): void {
$this->phoneNumberUtil,
$this->appManager,
$this->appConfig,
$this->groupDisplayNameCache,
])
->onlyMethods(['getUserData'])
->getMock();
Expand Down Expand Up @@ -4194,6 +4293,7 @@ public function testGetUser(): void {
$this->phoneNumberUtil,
$this->appManager,
$this->appConfig,
$this->groupDisplayNameCache,
])
->onlyMethods(['getUserData'])
->getMock();
Expand Down
2 changes: 1 addition & 1 deletion apps/settings/src/components/Users/userFormUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export function userToFormData(user, allGroups, quotaOptions, serverLanguages) {
displayName: user.displayname ?? '',
password: '',
email: user.email ?? '',
groups,
groups: user.groupsWithDisplayname,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still needed?

subadminGroups,
quota,
language: resolveLanguage(user, serverLanguages),
Expand Down
Loading
Loading