Skip to content

search disabled users by display name or email #997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hweihwang wants to merge 1 commit into
base: master
Choose a base branch
from
Open

search disabled users by display name or email #997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
"bamarni/composer-bin-plugin": true
}
},
"autoload-dev": {
"psr-4": {
"OCP\\": "vendor/nextcloud/ocp/OCP"
}
},
"extra": {
"bamarni-bin": {
"bin-links": true,
Expand Down
96 changes: 80 additions & 16 deletions lib/GroupBackend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

namespace OCA\User_SAML;

use OC\User\LazyUser;
Copy link
Member

@blizzz blizzz Dec 4, 2025

Choose a reason for hiding this comment

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

This is problematic, it is not part of the API. It is also bad that the internal backend makes use of it, while there is no chance for external ones.

Would it be sufficient to implement IProvideEnabledStateBackend instead? Then the user management abstraction would create the LazyUser objects.

use OCP\DB\Exception;
use OCP\Group\Backend\ABackend;
use OCP\Group\Backend\IAddToGroupBackend;
Expand All @@ -16,19 +17,24 @@
use OCP\Group\Backend\IGetDisplayNameBackend;
use OCP\Group\Backend\INamedBackend;
use OCP\Group\Backend\IRemoveFromGroupBackend;
use OCP\Group\Backend\ISearchableGroupBackend;
use OCP\Group\Backend\ISetDisplayNameBackend;
use OCP\IDBConnection;
use OCP\IUserManager;
use OCP\Server;
use PDO;
use Psr\Log\LoggerInterface;

class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBackend, ICreateGroupBackend, IDeleteGroupBackend, IGetDisplayNameBackend, IRemoveFromGroupBackend, ISetDisplayNameBackend, INamedBackend {
class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBackend, ICreateGroupBackend, IDeleteGroupBackend, IGetDisplayNameBackend, IRemoveFromGroupBackend, ISetDisplayNameBackend, INamedBackend, ISearchableGroupBackend {

/** @var array */
private $groupCache = [];

public const TABLE_GROUPS = 'user_saml_groups';
public const TABLE_MEMBERS = 'user_saml_group_members';

private ?IUserManager $userManager = null;

public function __construct(
protected IDBConnection $dbc,
protected LoggerInterface $logger,
Expand Down Expand Up @@ -152,16 +158,45 @@ public function groupExistsWithDifferentGid(string $samlGid): ?string {
* @return array<int,string> User ids
*/
public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0): array {
return array_values(array_map(
static fn ($user) => $user->getUID(),
$this->searchInGroup($gid, $search, $limit, $offset)
));
}

/**
* @return array<string,\OCP\IUser>
*/
public function searchInGroup(string $gid, string $search = '', int $limit = -1, int $offset = 0): array {
$query = $this->dbc->getQueryBuilder();
$query->select('uid')
->from(self::TABLE_MEMBERS)
->where($query->expr()->eq('gid', $query->createNamedParameter($gid)))
->orderBy('uid', 'ASC');
$query->select('m.uid', 'dn.value AS displayname')
->from(self::TABLE_MEMBERS, 'm')
->leftJoin('m', 'accounts_data', 'dn',
$query->expr()->andX(
$query->expr()->eq('dn.uid', 'm.uid'),
$query->expr()->eq('dn.name', $query->createNamedParameter('displayname'))
)
)
->leftJoin('m', 'accounts_data', 'em',
$query->expr()->andX(
$query->expr()->eq('em.uid', 'm.uid'),
$query->expr()->eq('em.name', $query->createNamedParameter('email'))
)
)
->where($query->expr()->eq('m.gid', $query->createNamedParameter($gid)))
->orderBy('m.uid', 'ASC');

if ($search !== '') {
$query->andWhere($query->expr()->like('uid', $query->createNamedParameter(
'%' . $this->dbc->escapeLikeParameter($search) . '%'
)));
$searchParam1 = $query->createNamedParameter('%' . $this->dbc->escapeLikeParameter($search) . '%');
$searchParam2 = $query->createNamedParameter('%' . $this->dbc->escapeLikeParameter($search) . '%');
$searchParam3 = $query->createNamedParameter('%' . $this->dbc->escapeLikeParameter($search) . '%');
$query->andWhere(
$query->expr()->orX(
$query->expr()->ilike('m.uid', $searchParam1),
$query->expr()->ilike('dn.value', $searchParam2),
$query->expr()->ilike('em.value', $searchParam3)
)
);
}

if ($limit !== -1) {
Expand All @@ -172,10 +207,10 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0): arra
}

$result = $query->executeQuery();

$users = [];
$userManager = $this->getUserManager();
while ($row = $result->fetch()) {
$users[] = $row['uid'];
$users[$row['uid']] = new LazyUser($row['uid'], $userManager, $row['displayname'] ?? null);
}
$result->closeCursor();

Expand Down Expand Up @@ -238,14 +273,36 @@ public function removeFromGroup(string $uid, string $gid): bool {

public function countUsersInGroup(string $gid, string $search = ''): int {
$query = $this->dbc->getQueryBuilder();
$query->select($query->func()->count('*', 'num_users'))
->from(self::TABLE_MEMBERS)
->where($query->expr()->eq('gid', $query->createNamedParameter($gid)));
$query->select(
$query->createFunction('COUNT(DISTINCT ' . $query->getColumnName('uid', 'm') . ')')
)
->from(self::TABLE_MEMBERS, 'm')
->where($query->expr()->eq('m.gid', $query->createNamedParameter($gid)));

if ($search !== '') {
$query->andWhere($query->expr()->like('uid', $query->createNamedParameter(
'%' . $this->dbc->escapeLikeParameter($search) . '%'
)));
$query->leftJoin('m', 'accounts_data', 'dn',
$query->expr()->andX(
$query->expr()->eq('dn.uid', 'm.uid'),
$query->expr()->eq('dn.name', $query->createNamedParameter('displayname'))
)
);
$query->leftJoin('m', 'accounts_data', 'em',
$query->expr()->andX(
$query->expr()->eq('em.uid', 'm.uid'),
$query->expr()->eq('em.name', $query->createNamedParameter('email'))
)
);

$searchParam1 = $query->createNamedParameter('%' . $this->dbc->escapeLikeParameter($search) . '%');
$searchParam2 = $query->createNamedParameter('%' . $this->dbc->escapeLikeParameter($search) . '%');
$searchParam3 = $query->createNamedParameter('%' . $this->dbc->escapeLikeParameter($search) . '%');
$query->andWhere(
$query->expr()->orX(
$query->expr()->ilike('m.uid', $searchParam1),
$query->expr()->ilike('dn.value', $searchParam2),
$query->expr()->ilike('em.value', $searchParam3)
)
);
}

$result = $query->executeQuery();
Expand Down Expand Up @@ -321,4 +378,11 @@ public function setDisplayName(string $gid, string $displayName): bool {

return $isUpdated;
}

private function getUserManager(): IUserManager {
if ($this->userManager === null) {
$this->userManager = Server::get(IUserManager::class);
}
return $this->userManager;
}
}
6 changes: 6 additions & 0 deletions tests/stub.phpstub
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,9 @@ class OC_User {
public static function useBackend($userBackend): void;
public static function handleApacheAuth(): void;
}

namespace OC\User {
class LazyUser implements \OCP\IUser {
public function __construct(string $uid, \OCP\IUserManager $userManager, ?string $displayName = null, ?\OCP\UserInterface $backend = null) {}
}
}
52 changes: 50 additions & 2 deletions tests/unit/GroupBackendTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,22 @@ class GroupBackendTest extends TestCase {

/** @var GroupBackend */
private $groupBackend;
/** @var IDBConnection */
private $connection;
private $users = [
[
'uid' => 'user_saml_integration_test_uid1',
'displayname' => 'SAML Integration User One',
'email' => 'saml-integration-one@example.test',
'groups' => [
'user_saml_integration_test_gid1',
'SAML_user_saml_integration_test_gid2'
]
],
[
'uid' => 'user_saml_integration_test_uid2',
'displayname' => 'SAML Integration User Two',
'email' => 'saml-integration-two@example.test',
'groups' => [
'user_saml_integration_test_gid1'
]
Expand Down Expand Up @@ -60,20 +66,24 @@ class GroupBackendTest extends TestCase {

public function setUp(): void {
parent::setUp();
$this->groupBackend = new GroupBackend(\OCP\Server::get(IDBConnection::class), $this->createMock(LoggerInterface::class));
$this->connection = \OCP\Server::get(IDBConnection::class);
$this->resetAccountData();
$this->groupBackend = new GroupBackend($this->connection, $this->createMock(LoggerInterface::class));
foreach ($this->groups as $group) {
$this->groupBackend->createGroup($group['gid'], $group['saml_gid']);
}
foreach ($this->users as $user) {
foreach ($user['groups'] as $group) {
$this->groupBackend->addToGroup($user['uid'], $group);
}
$this->setAccountData($user['uid'], 'displayname', $user['displayname']);
$this->setAccountData($user['uid'], 'email', $user['email']);
}
}

public function tearDown(): void {
parent::tearDown();
$this->groupBackend = new GroupBackend(\OCP\Server::get(IDBConnection::class), $this->createMock(LoggerInterface::class));
$this->groupBackend = new GroupBackend($this->connection, $this->createMock(LoggerInterface::class));
foreach ($this->users as $user) {
foreach ($user['groups'] as $group) {
$this->groupBackend->removeFromGroup($user['uid'], $group);
Expand All @@ -82,6 +92,7 @@ public function tearDown(): void {
foreach ($this->groups as $group) {
$this->groupBackend->deleteGroup($group['gid']);
}
$this->resetAccountData();
}

public function testInGroup() {
Expand Down Expand Up @@ -130,4 +141,41 @@ public function testUsersInGroups() {
}
}
}

public function testSearchInGroupMatchesDisplayNameAndEmail(): void {
$groupId = $this->groups[0]['gid'];

$byDisplayName = $this->groupBackend->searchInGroup($groupId, $this->users[0]['displayname']);
$this->assertArrayHasKey($this->users[0]['uid'], $byDisplayName, 'Display name search should return the matching user');

$byEmail = $this->groupBackend->searchInGroup($groupId, $this->users[1]['email']);
$this->assertArrayHasKey($this->users[1]['uid'], $byEmail, 'Email search should return the matching user');

$byUid = $this->groupBackend->searchInGroup($groupId, $this->users[0]['uid']);
$this->assertArrayHasKey($this->users[0]['uid'], $byUid, 'UID search should still work');
}

private function resetAccountData(): void {
foreach ($this->users as $user) {
$qb = $this->connection->getQueryBuilder();
$qb->delete('accounts_data')
->where($qb->expr()->eq('uid', $qb->createNamedParameter($user['uid'])))
->executeStatement();
}
}

private function setAccountData(string $uid, string $name, string $value): void {
$qb = $this->connection->getQueryBuilder();
$qb->delete('accounts_data')
->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid)))
->andWhere($qb->expr()->eq('name', $qb->createNamedParameter($name)))
->executeStatement();

$qb = $this->connection->getQueryBuilder();
$qb->insert('accounts_data')
->setValue('uid', $qb->createNamedParameter($uid))
->setValue('name', $qb->createNamedParameter($name))
->setValue('value', $qb->createNamedParameter($value))
->executeStatement();
}
}
Loading