Skip to content
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.

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

fix(ldap): store last known user groups #40443

Merged
merged 6 commits into from
Oct 12, 2023
Merged
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
4 changes: 4 additions & 0 deletions apps/user_ldap/lib/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,10 @@ public function getFromCache($key) {
return json_decode(base64_decode($this->cache->get($key) ?? ''), true);
}

public function getConfigPrefix(): string {
return $this->configPrefix;
}

/**
* @param string $key
* @param mixed $value
Expand Down
64 changes: 60 additions & 4 deletions apps/user_ldap/lib/Group_LDAP.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,17 @@

use Exception;
use OC\ServerNotAvailableException;
use OCA\User_LDAP\User\OfflineUser;
use OCP\Cache\CappedMemoryCache;
use OCP\GroupInterface;
use OCP\Group\Backend\ABackend;
use OCP\Group\Backend\IDeleteGroupBackend;
use OCP\Group\Backend\IGetDisplayNameBackend;
use OCP\IConfig;
use OCP\IUserManager;
use OCP\Server;
use Psr\Log\LoggerInterface;
use function json_decode;

class Group_LDAP extends ABackend implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend, IDeleteGroupBackend {
protected bool $enabled = false;
Expand All @@ -70,8 +75,15 @@ class Group_LDAP extends ABackend implements GroupInterface, IGroupLDAP, IGetDis
* @var string $ldapGroupMemberAssocAttr contains the LDAP setting (in lower case) with the same name
*/
protected string $ldapGroupMemberAssocAttr;

public function __construct(Access $access, GroupPluginManager $groupPluginManager) {
private IConfig $config;
private IUserManager $ncUserManager;

public function __construct(
Access $access,
GroupPluginManager $groupPluginManager,
IConfig $config,
IUserManager $ncUserManager
) {
$this->access = $access;
$filter = $this->access->connection->ldapGroupFilter;
$gAssoc = $this->access->connection->ldapGroupMemberAssocAttr;
Expand All @@ -83,8 +95,10 @@ public function __construct(Access $access, GroupPluginManager $groupPluginManag
$this->cachedGroupsByMember = new CappedMemoryCache();
$this->cachedNestedGroups = new CappedMemoryCache();
$this->groupPluginManager = $groupPluginManager;
$this->logger = \OCP\Server::get(LoggerInterface::class);
$this->logger = Server::get(LoggerInterface::class);
$this->ldapGroupMemberAssocAttr = strtolower((string)$gAssoc);
$this->config = $config;
$this->ncUserManager = $ncUserManager;
}

/**
Expand Down Expand Up @@ -439,6 +453,7 @@ public function getGroupGidNumber(string $dn) {
public function getUserGidNumber(string $dn) {
$gidNumber = false;
if ($this->access->connection->hasGidNumber) {
// FIXME: when $dn does not exist on LDAP anymore, this will be set wrongly to false :/
$gidNumber = $this->getEntryGidNumber($dn, $this->access->connection->ldapGidNumber);
if ($gidNumber === false) {
$this->access->connection->hasGidNumber = false;
Expand Down Expand Up @@ -653,6 +668,25 @@ public function getUserPrimaryGroup(string $dn) {
return false;
}

private function isUserOnLDAP(string $uid): bool {
// forces a user exists check - but does not help if a positive result is cached, while group info is not
$ncUser = $this->ncUserManager->get($uid);
if ($ncUser === null) {
return false;
}
$backend = $ncUser->getBackend();
Fixed Show fixed Hide fixed
if ($backend instanceof User_Proxy) {
// ignoring cache as safeguard (and we are behind the group cache check anyway)
return $backend->userExistsOnLDAP($uid, true);
}
return false;
}

protected function getCachedGroupsForUserId(string $uid): array {
$groupStr = $this->config->getUserValue($uid, 'user_ldap', 'cached-group-memberships-' . $this->access->connection->getConfigPrefix(), '[]');
return json_decode($groupStr) ?? [];
}

/**
* This function fetches all groups a user belongs to. It does not check
* if the user exists at all.
Expand All @@ -664,15 +698,25 @@ public function getUserPrimaryGroup(string $dn) {
* @throws Exception
* @throws ServerNotAvailableException
*/
public function getUserGroups($uid) {
public function getUserGroups($uid): array {
if (!$this->enabled) {
return [];
}
$ncUid = $uid;

$cacheKey = 'getUserGroups' . $uid;
$userGroups = $this->access->connection->getFromCache($cacheKey);
if (!is_null($userGroups)) {
blizzz marked this conversation as resolved.
Show resolved Hide resolved
return $userGroups;
}

$user = $this->access->userManager->get($uid);
if ($user instanceof OfflineUser) {
// We load known group memberships from configuration for remnants,
// because LDAP server does not contain them anymore
return $this->getCachedGroupsForUserId($uid);
}

$userDN = $this->access->username2dn($uid);
if (!$userDN) {
$this->access->connection->writeToCache($cacheKey, []);
Expand Down Expand Up @@ -784,9 +828,21 @@ public function getUserGroups($uid) {
$groups[] = $gidGroupName;
}

if (empty($groups) && !$this->isUserOnLDAP($ncUid)) {
// Groups are enabled, but you user has none? Potentially suspicious:
// it could be that the user was deleted from LDAP, but we are not
// aware of it yet.
$groups = $this->getCachedGroupsForUserId($ncUid);
$this->access->connection->writeToCache($cacheKey, $groups);
return $groups;
}

$groups = array_unique($groups, SORT_LOCALE_STRING);
$this->access->connection->writeToCache($cacheKey, $groups);

$groupStr = \json_encode($groups);
$this->config->setUserValue($ncUid, 'user_ldap', 'cached-group-memberships-' . $this->access->connection->getConfigPrefix(), $groupStr);

return $groups;
}

Expand Down
12 changes: 10 additions & 2 deletions apps/user_ldap/lib/Group_Proxy.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,31 @@
use OCP\Group\Backend\IGroupDetailsBackend;
use OCP\Group\Backend\INamedBackend;
use OCP\GroupInterface;
use OCP\IConfig;
use OCP\IUserManager;

class Group_Proxy extends Proxy implements \OCP\GroupInterface, IGroupLDAP, IGetDisplayNameBackend, INamedBackend, IDeleteGroupBackend, IBatchMethodsBackend {
private $backends = [];
private ?Group_LDAP $refBackend = null;
private Helper $helper;
private GroupPluginManager $groupPluginManager;
private bool $isSetUp = false;
private IConfig $config;
private IUserManager $ncUserManager;

public function __construct(
Helper $helper,
ILDAPWrapper $ldap,
AccessFactory $accessFactory,
GroupPluginManager $groupPluginManager
GroupPluginManager $groupPluginManager,
IConfig $config,
IUserManager $ncUserManager,
) {
parent::__construct($ldap, $accessFactory);
$this->helper = $helper;
$this->groupPluginManager = $groupPluginManager;
$this->config = $config;
$this->ncUserManager = $ncUserManager;
}

protected function setup(): void {
Expand All @@ -62,7 +70,7 @@ protected function setup(): void {
$serverConfigPrefixes = $this->helper->getServerConfigurationPrefixes(true);
foreach ($serverConfigPrefixes as $configPrefix) {
$this->backends[$configPrefix] =
new Group_LDAP($this->getAccess($configPrefix), $this->groupPluginManager);
new Group_LDAP($this->getAccess($configPrefix), $this->groupPluginManager, $this->config, $this->ncUserManager);
if (is_null($this->refBackend)) {
$this->refBackend = &$this->backends[$configPrefix];
}
Expand Down
Loading
Loading