Skip to content

LDAP: always take the display name from DB, update in the background #25192

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

Closed
blizzz wants to merge 2 commits into from
Closed

LDAP: always take the display name from DB, update in the background #25192

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc00e4a
do not serve LDAP displayname from memcache or LDAP but DB
blizzz Jan 18, 2021
baa43ef
removes dead update code
blizzz Jan 18, 2021
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
Next Next commit
do not serve LDAP displayname from memcache or LDAP but DB 
- saves potentionally frequent readAttribute requests
- updated now like email in the background

Signed-off-by: Arthur Schiwon <[email protected]>
  • Loading branch information
@blizzz
blizzz committed Jan 18, 2021
commit cc00e4a97a2267ca6fbb070df2631cb9e2dcc80b
8 changes: 3 additions & 5 deletions apps/user_ldap/lib/Access.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ private function ldap2NextcloudNames($ldapObjects, $isUsers) {
}
$sndName = isset($ldapObject[$sndAttribute][0])
? $ldapObject[$sndAttribute][0] : '';
$this->cacheUserDisplayName($ncName, $nameByLDAP, $sndName);
$this->storeUserDisplayName($ncName, $nameByLDAP, $sndName);
} elseif ($nameByLDAP !== null) {
$this->cacheGroupDisplayName($ncName, $nameByLDAP);
}
Expand Down Expand Up @@ -745,14 +745,12 @@ public function cacheGroupExists(string $gid): void {
* @param string $displayName2 the second display name
* @throws \Exception
*/
public function cacheUserDisplayName($ocName, $displayName, $displayName2 = '') {
public function storeUserDisplayName($ocName, $displayName, $displayName2 = '') {
$user = $this->userManager->get($ocName);
if ($user === null) {
return;
}
$displayName = $user->composeAndStoreDisplayName($displayName, $displayName2);
$cacheKeyTrunk = 'getDisplayName';
$this->connection->writeToCache($cacheKeyTrunk . $ocName, $displayName);
$user->composeAndStoreDisplayName($displayName, $displayName2);
}

public function cacheGroupDisplayName(string $ncName, string $displayName): void {
Expand Down
9 changes: 4 additions & 5 deletions apps/user_ldap/lib/User/User.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,6 @@ public function processAttributes($ldapEntry) {
}
if ($displayName !== '') {
$this->composeAndStoreDisplayName($displayName, $displayName2);
$this->access->cacheUserDisplayName(
$this->getUsername(),
$displayName,
$displayName2
);
}
unset($attr);

Expand Down Expand Up @@ -404,6 +399,10 @@ public function composeAndStoreDisplayName($displayName, $displayName2 = '') {
return $displayName;
}

public function getDisplayName(): ?string {
return $this->config->getUserValue($this->uid, 'user_ldap', 'displayName', null);
}

/**
* Stores the LDAP Username in the Database
* @param string $userName
Expand Down
66 changes: 18 additions & 48 deletions apps/user_ldap/lib/User_LDAP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@

namespace OCA\User_LDAP;

use Exception;
use OC\ServerNotAvailableException;
use OC\User\Backend;
use OC\User\NoUserException;
Expand Down Expand Up @@ -79,7 +80,7 @@ public function __construct(Access $access, IConfig $ocConfig, INotificationMana
*
* @param string $uid the Nextcloud user name
* @return boolean either the user can or cannot
* @throws \Exception
* @throws Exception
*/
public function canChangeAvatar($uid) {
if ($this->userPluginManager->implementsActions(Backend::PROVIDE_AVATAR)) {
Expand All @@ -106,7 +107,7 @@ public function canChangeAvatar($uid) {
*
* @param string $loginName
* @return string|false
* @throws \Exception
* @throws Exception
*/
public function loginName2UserName($loginName) {
$cacheKey = 'loginName2UserName-' . $loginName;
Expand All @@ -133,7 +134,7 @@ public function loginName2UserName($loginName) {
return false;
}
}

/**
* returns the username for the given LDAP DN, if available
*
Expand Down Expand Up @@ -216,7 +217,7 @@ public function setPassword($uid, $password) {
$user = $this->access->userManager->get($uid);

if (!$user instanceof User) {
throw new \Exception('LDAP setPassword: Could not get user object for uid ' . $uid .
throw new Exception('LDAP setPassword: Could not get user object for uid ' . $uid .
'. Maybe the LDAP entry has no set display name attribute?');
}
if ($user->getUsername() !== false && $this->access->setPassword($user->getDN(), $password)) {
Expand Down Expand Up @@ -287,7 +288,7 @@ public function getUsers($search = '', $limit = 10, $offset = 0) {
* @param string|\OCA\User_LDAP\User\User $user either the Nextcloud user
* name or an instance of that user
* @return bool
* @throws \Exception
* @throws Exception
* @throws \OC\ServerNotAvailableException
*/
public function userExistsOnLDAP($user) {
Expand Down Expand Up @@ -324,7 +325,7 @@ public function userExistsOnLDAP($user) {
return true;
} catch (ServerNotAvailableException $e) {
throw $e;
} catch (\Exception $e) {
} catch (Exception $e) {
$this->access->connection->writeToCache($cacheKey, false);
return false;
}
Expand All @@ -342,7 +343,7 @@ public function userExistsOnLDAP($user) {
* check if a user exists
* @param string $uid the username
* @return boolean
* @throws \Exception when connection could not be established
* @throws Exception when connection could not be established
*/
public function userExists($uid) {
$userExists = $this->access->connection->getFromCache('userExists'.$uid);
Expand Down Expand Up @@ -399,7 +400,7 @@ public function deleteUser($uid) {
* @param string $uid the username
* @return bool|string
* @throws NoUserException
* @throws \Exception
* @throws Exception
*/
public function getHome($uid) {
// user Exists check required as it is not done in user proxy!
Expand Down Expand Up @@ -431,8 +432,10 @@ public function getHome($uid) {

/**
* get display name of the user
*
* @param string $uid user ID of the user
* @return string|false display name
* @throws Exception
*/
public function getDisplayName($uid) {
if ($this->userPluginManager->implementsActions(Backend::GET_DISPLAYNAME)) {
Expand All @@ -443,44 +446,11 @@ public function getDisplayName($uid) {
return false;
}

$cacheKey = 'getDisplayName'.$uid;
if (!is_null($displayName = $this->access->connection->getFromCache($cacheKey))) {
return $displayName;
}

//Check whether the display name is configured to have a 2nd feature
$additionalAttribute = $this->access->connection->ldapUserDisplayName2;
$displayName2 = '';
if ($additionalAttribute !== '') {
$displayName2 = $this->access->readAttribute(
$this->access->username2dn($uid),
$additionalAttribute);
}

$displayName = $this->access->readAttribute(
$this->access->username2dn($uid),
$this->access->connection->ldapUserDisplayName);

if ($displayName && (count($displayName) > 0)) {
$displayName = $displayName[0];

if (is_array($displayName2)) {
$displayName2 = count($displayName2) > 0 ? $displayName2[0] : '';
}

$user = $this->access->userManager->get($uid);
if ($user instanceof User) {
$displayName = $user->composeAndStoreDisplayName($displayName, $displayName2);
$this->access->connection->writeToCache($cacheKey, $displayName);
}
if ($user instanceof OfflineUser) {
/** @var OfflineUser $user*/
$displayName = $user->getDisplayName();
}
return $displayName;
$user = $this->access->userManager->get($uid);
if (null === $user) {
return false;
}

return null;
return $user->getDisplayName();
}

/**
Expand All @@ -492,7 +462,7 @@ public function getDisplayName($uid) {
public function setDisplayName($uid, $displayName) {
if ($this->userPluginManager->implementsActions(Backend::SET_DISPLAYNAME)) {
$this->userPluginManager->setDisplayName($uid, $displayName);
$this->access->cacheUserDisplayName($uid, $displayName);
$this->access->storeUserDisplayName($uid, $displayName);
return $displayName;
}
return false;
Expand Down Expand Up @@ -574,7 +544,7 @@ public function countUsers() {
public function getBackendName() {
return 'LDAP';
}

/**
* Return access for LDAP interaction.
* @param string $uid
Expand All @@ -583,7 +553,7 @@ public function getBackendName() {
public function getLDAPAccess($uid) {
return $this->access;
}

/**
* Return LDAP connection resource from a cloned connection.
* The cloned connection needs to be closed manually.
Expand Down