Fix LDAP race conditions
* getFromCache is wrapped in isCached * inbetween the two calls the cache entry hits it's TTL * getFromCache returns null * this fix only checkes if the returned value is null and return only non-null values
This commit is contained in:
parent
85d809c0d3
commit
62a59854f0
6 changed files with 32 additions and 48 deletions
|
@ -72,8 +72,9 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
|
|||
return false;
|
||||
}
|
||||
$cacheKey = 'inGroup'.$uid.':'.$gid;
|
||||
if($this->access->connection->isCached($cacheKey)) {
|
||||
return $this->access->connection->getFromCache($cacheKey);
|
||||
$inGroup = $this->access->connection->getFromCache($cacheKey);
|
||||
if(!is_null($inGroup)) {
|
||||
return (bool)$inGroup;
|
||||
}
|
||||
|
||||
$userDN = $this->access->username2dn($uid);
|
||||
|
@ -84,8 +85,8 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
|
|||
}
|
||||
|
||||
$cacheKeyMembers = 'inGroup-members:'.$gid;
|
||||
if($this->access->connection->isCached($cacheKeyMembers)) {
|
||||
$members = $this->access->connection->getFromCache($cacheKeyMembers);
|
||||
$members = $this->access->connection->getFromCache($cacheKeyMembers);
|
||||
if(!is_null($members)) {
|
||||
$this->cachedGroupMembers[$gid] = $members;
|
||||
$isInGroup = in_array($userDN, $members);
|
||||
$this->access->connection->writeToCache($cacheKey, $isInGroup);
|
||||
|
@ -204,8 +205,9 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
|
|||
}
|
||||
// used extensively in cron job, caching makes sense for nested groups
|
||||
$cacheKey = '_groupMembers'.$dnGroup;
|
||||
if($this->access->connection->isCached($cacheKey)) {
|
||||
return $this->access->connection->getFromCache($cacheKey);
|
||||
$groupMembers = $this->access->connection->getFromCache($cacheKey);
|
||||
if(!is_null($groupMembers)) {
|
||||
return $groupMembers;
|
||||
}
|
||||
$seen[$dnGroup] = 1;
|
||||
$members = $this->access->readAttribute($dnGroup, $this->access->connection->ldapGroupMemberAssocAttr,
|
||||
|
@ -267,11 +269,9 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
|
|||
*/
|
||||
public function primaryGroupID2Name($gid, $dn) {
|
||||
$cacheKey = 'primaryGroupIDtoName';
|
||||
if($this->access->connection->isCached($cacheKey)) {
|
||||
$groupNames = $this->access->connection->getFromCache($cacheKey);
|
||||
if(isset($groupNames[$gid])) {
|
||||
return $groupNames[$gid];
|
||||
}
|
||||
$groupNames = $this->access->connection->getFromCache($cacheKey);
|
||||
if(!is_null($groupNames) && isset($groupNames[$gid])) {
|
||||
return $groupNames[$gid];
|
||||
}
|
||||
|
||||
$domainObjectSid = $this->access->getSID($dn);
|
||||
|
@ -440,8 +440,9 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
|
|||
return array();
|
||||
}
|
||||
$cacheKey = 'getUserGroups'.$uid;
|
||||
if($this->access->connection->isCached($cacheKey)) {
|
||||
return $this->access->connection->getFromCache($cacheKey);
|
||||
$userGroups = $this->access->connection->getFromCache($cacheKey);
|
||||
if(!is_null($userGroups)) {
|
||||
return $userGroups;
|
||||
}
|
||||
$userDN = $this->access->username2dn($uid);
|
||||
if(!$userDN) {
|
||||
|
@ -861,8 +862,9 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
|
|||
* @return bool
|
||||
*/
|
||||
public function groupExists($gid) {
|
||||
if($this->access->connection->isCached('groupExists'.$gid)) {
|
||||
return $this->access->connection->getFromCache('groupExists'.$gid);
|
||||
$groupExists = $this->access->connection->getFromCache('groupExists'.$gid);
|
||||
if(!is_null($groupExists)) {
|
||||
return (bool)$groupExists;
|
||||
}
|
||||
|
||||
//getting dn, if false the group does not exist. If dn, it may be mapped
|
||||
|
|
|
@ -364,8 +364,9 @@ class Access extends LDAPUtility implements user\IUserTools {
|
|||
$validGroupDNs = [];
|
||||
foreach($groupDNs as $dn) {
|
||||
$cacheKey = 'groupsMatchFilter-'.$dn;
|
||||
if($this->connection->isCached($cacheKey)) {
|
||||
if($this->connection->getFromCache($cacheKey)) {
|
||||
$groupMatchFilter = $this->connection->getFromCache($cacheKey);
|
||||
if(!is_null($groupMatchFilter)) {
|
||||
if($groupMatchFilter) {
|
||||
$validGroupDNs[] = $dn;
|
||||
}
|
||||
continue;
|
||||
|
@ -1505,8 +1506,9 @@ class Access extends LDAPUtility implements user\IUserTools {
|
|||
public function getSID($dn) {
|
||||
$domainDN = $this->getDomainDNFromDN($dn);
|
||||
$cacheKey = 'getSID-'.$domainDN;
|
||||
if($this->connection->isCached($cacheKey)) {
|
||||
return $this->connection->getFromCache($cacheKey);
|
||||
$sid = $this->connection->getFromCache($cacheKey);
|
||||
if(!is_null($sid)) {
|
||||
return $sid;
|
||||
}
|
||||
|
||||
$objectSid = $this->readAttribute($domainDN, 'objectsid');
|
||||
|
|
|
@ -213,30 +213,11 @@ class Connection extends LDAPUtility {
|
|||
if(is_null($this->cache) || !$this->configuration->ldapCacheTTL) {
|
||||
return null;
|
||||
}
|
||||
if(!$this->isCached($key)) {
|
||||
return null;
|
||||
|
||||
}
|
||||
$key = $this->getCacheKey($key);
|
||||
|
||||
return json_decode(base64_decode($this->cache->get($key)), true);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $key
|
||||
* @return bool
|
||||
*/
|
||||
public function isCached($key) {
|
||||
if(!$this->configured) {
|
||||
$this->readConfiguration();
|
||||
}
|
||||
if(is_null($this->cache) || !$this->configuration->ldapCacheTTL) {
|
||||
return false;
|
||||
}
|
||||
$key = $this->getCacheKey($key);
|
||||
return $this->cache->hasKey($key);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $key
|
||||
* @param mixed $value
|
||||
|
|
|
@ -297,8 +297,9 @@ class User {
|
|||
|
||||
public function getMemberOfGroups() {
|
||||
$cacheKey = 'getMemberOf'.$this->getUsername();
|
||||
if($this->connection->isCached($cacheKey)) {
|
||||
return $this->connection->getFromCache($cacheKey);
|
||||
$memberOfGroups = $this->connection->getFromCache($cacheKey);
|
||||
if(!is_null($memberOfGroups)) {
|
||||
return $memberOfGroups;
|
||||
}
|
||||
$groupDNs = $this->access->readAttribute($this->getDN(), 'memberOf');
|
||||
$this->connection->writeToCache($cacheKey, $groupDNs);
|
||||
|
|
|
@ -294,10 +294,6 @@ class Test_Group_Ldap extends \Test\TestCase {
|
|||
$uid = 'someUser';
|
||||
$gid = 'someGroup';
|
||||
$cacheKey = 'inGroup'.$uid.':'.$gid;
|
||||
$access->connection->expects($this->once())
|
||||
->method('isCached')
|
||||
->with($cacheKey)
|
||||
->will($this->returnValue(true));
|
||||
|
||||
$access->connection->expects($this->once())
|
||||
->method('getFromCache')
|
||||
|
|
|
@ -250,8 +250,9 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
|
|||
* @throws \Exception when connection could not be established
|
||||
*/
|
||||
public function userExists($uid) {
|
||||
if($this->access->connection->isCached('userExists'.$uid)) {
|
||||
return $this->access->connection->getFromCache('userExists'.$uid);
|
||||
$userExists = $this->access->connection->getFromCache('userExists'.$uid);
|
||||
if(!is_null($userExists)) {
|
||||
return (bool)$userExists;
|
||||
}
|
||||
//getting dn, if false the user does not exist. If dn, he may be mapped only, requires more checking.
|
||||
$user = $this->access->userManager->get($uid);
|
||||
|
@ -321,8 +322,9 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
|
|||
}
|
||||
|
||||
$cacheKey = 'getHome'.$uid;
|
||||
if($this->access->connection->isCached($cacheKey)) {
|
||||
return $this->access->connection->getFromCache($cacheKey);
|
||||
$path = $this->access->connection->getFromCache($cacheKey);
|
||||
if(!is_null($path)) {
|
||||
return $path;
|
||||
}
|
||||
|
||||
$user = $this->access->userManager->get($uid);
|
||||
|
|
Loading…
Reference in a new issue