Skip to content

Conversation

@icewind1991
Copy link
Member

When using ldap, there are setups where neither the uid nor the loginname are the "correct" username to pass to external storages.

This adds an option to use any ldap property as the username to pass to the external storage, you can configure it using

occ files_external:config <mount id> login_ldap_attr <attribute name>

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Jan 4, 2021
@icewind1991 icewind1991 added this to the Nextcloud 21 milestone Jan 4, 2021
@icewind1991 icewind1991 force-pushed the external-storage-login-ldap branch from 5c8013e to 58f3f5c Compare January 4, 2021 18:53
@rullzer rullzer mentioned this pull request Jan 6, 2021
5 tasks
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Fine by me
🐘

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Fine with me too 👍

@MorrisJobke MorrisJobke merged commit 38e534e into master Jan 7, 2021
@MorrisJobke MorrisJobke deleted the external-storage-login-ldap branch January 7, 2021 12:33
Comment on lines +122 to +137
$access = $ldap->getLDAPAccess($user->getUID());
$connection = $access->getConnection();
$key = "external_login::" . $user->getUID() . "::" . $property;
$cached = $connection->getFromCache($key);

if ($cached !== null) {
return $cached;
}

$value = $access->readAttribute($access->username2dn($user->getUID()), $property);
if (count($value) > 0) {
$value = current($value);
} else {
return null;
}
$connection->writeToCache($key, $value);
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer it would use \OCP\ILDAPProvider (via its factory) instead of using private user_ldap classes. https://github.com/nextcloud/server/tree/master/lib/public/LDAP

@blizzz
Copy link
Member

blizzz commented Jan 7, 2021

Either way. It should not have been merged after feature freeze → Reverting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants