From 44cbdf67ad834427496c4ed5663a56de6ab60a67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Fri, 22 Jul 2022 10:56:33 +0200 Subject: [PATCH 01/10] Expose group's displayname --- lib/Access.php | 41 +++++++++++++---------------------------- lib/Group_LDAP.php | 17 ++++++++++++++++- lib/Group_Proxy.php | 3 +++ lib/User/IUserTools.php | 2 +- 4 files changed, 33 insertions(+), 30 deletions(-) diff --git a/lib/Access.php b/lib/Access.php index a4366723c..17bd063d0 100644 --- a/lib/Access.php +++ b/lib/Access.php @@ -478,7 +478,7 @@ public function username2dn($name) { * @return string|false with the name to use in ownCloud, false on DN outside of search DN * @throws \OC\ServerNotAvailableException */ - public function dn2groupname($fdn, $ldapName = null) { + public function dn2groupname($fdn) { //To avoid bypassing the base DN settings under certain circumstances //with the group support, check whether the provided DN matches one of //the given Bases @@ -486,7 +486,7 @@ public function dn2groupname($fdn, $ldapName = null) { return false; } - return $this->dn2ocname($fdn, $ldapName, false); + return $this->dn2ocname($fdn, false); } /** @@ -536,7 +536,7 @@ public function groupsMatchFilter($groupDNs) { * @return string|false with with the name to use in ownCloud * @throws \OC\ServerNotAvailableException */ - public function dn2username($fdn, $ldapName = null) { + public function dn2username($fdn) { //To avoid bypassing the base DN settings under certain circumstances //with the group support, check whether the provided DN matches one of //the given Bases @@ -544,7 +544,7 @@ public function dn2username($fdn, $ldapName = null) { return false; } - return $this->dn2ocname($fdn, $ldapName, true); + return $this->dn2ocname($fdn, true); } /** @@ -557,13 +557,15 @@ public function dn2username($fdn, $ldapName = null) { * @throws \BadMethodCallException * @throws \OC\ServerNotAvailableException */ - public function dn2ocname($fdn, $ldapDisplayName = null, $isUser = true) { + public function dn2ocname($fdn, $isUser = true) { if ($isUser) { $mapper = $this->getUserMapper(); $displayNameAttribute = $this->connection->ldapUserDisplayName; + $nameAttribute = (string)$this->connection->ldapExpertUsernameAttr; } else { $mapper = $this->getGroupMapper(); $displayNameAttribute = $this->connection->ldapGroupDisplayName; + $nameAttribute = (string)$this->connection->ldapExpertGroupnameAttr; } //let's try to retrieve the ownCloud name from the mappings table @@ -589,30 +591,13 @@ public function dn2ocname($fdn, $ldapDisplayName = null, $isUser = true) { return false; } - if ($ldapDisplayName === null) { - $ldapDisplayName = $this->readAttribute($fdn, $displayNameAttribute); - if (!isset($ldapDisplayName[0]) && empty($ldapDisplayName[0])) { - \OC::$server->getLogger()->error( - "No or empty name for $fdn.", - ['app' => 'user_ldap'] - ); - return false; - } - $ldapDisplayName = $ldapDisplayName[0]; - } - - if ($isUser) { - $usernameAttribute = (string)$this->connection->ldapExpertUsernameAttr; - if ($usernameAttribute !== '') { - $username = $this->readAttribute($fdn, $usernameAttribute); - $username = $username[0]; - } else { - $username = $uuid; - } - $intName = $this->sanitizeUsername($username); + if ($nameAttribute !== '') { + $name = $this->readAttribute($fdn, $nameAttribute); + $name = $name[0]; } else { - $intName = $ldapDisplayName; + $name = $uuid; } + $intName = $this->sanitizeUsername($name); //a new user/group! Add it only if it doesn't conflict with other backend's users or existing groups //disabling Cache is required to avoid that the new user is cached as not-existing in fooExists check @@ -724,7 +709,7 @@ private function ldap2ownCloudNames($ldapObjects, $isUsers) { $nameByLDAP = $ldapObject[$nameAttribute][0]; } - $ocName = $this->dn2ocname($ldapObject['dn'][0], $nameByLDAP, $isUsers); + $ocName = $this->dn2ocname($ldapObject['dn'][0], $isUsers); if ($ocName) { $ownCloudNames[$ldapObject['dn'][0]] = $ocName; } diff --git a/lib/Group_LDAP.php b/lib/Group_LDAP.php index 383022849..bd586db73 100644 --- a/lib/Group_LDAP.php +++ b/lib/Group_LDAP.php @@ -1001,6 +1001,21 @@ public function groupExists($gid) { return true; } + public function getGroupDetails($gid) { + $dn = $this->access->groupname2dn($gid); + if ($dn === false) { + // FIXME: It seems local groups also end up going through here... + return null; + } + + $attr = $this->access->getConnection()->ldapGroupDisplayName; + $displayname = $this->access->readAttribute($dn, $attr); + return [ + 'gid' => $gid, + 'displayName' => $displayname[0], + ]; + } + /** * Check if backend implements actions * @param int $actions bitwise-or'ed actions @@ -1010,7 +1025,7 @@ public function groupExists($gid) { * compared with OC_USER_BACKEND_CREATE_USER etc. */ public function implementsActions($actions) { - return (bool)(\OC\Group\Backend::COUNT_USERS & $actions); + return (bool)((\OC\Group\Backend::COUNT_USERS | \OC\Group\Backend::GROUP_DETAILS) & $actions); } /** diff --git a/lib/Group_Proxy.php b/lib/Group_Proxy.php index 58eb6b0cb..bb2c72f9b 100644 --- a/lib/Group_Proxy.php +++ b/lib/Group_Proxy.php @@ -207,6 +207,9 @@ public function groupExists($gid) { return $this->handleRequest($gid, 'groupExists', [$gid]); } + public function getGroupDetails($gid) { + return $this->handleRequest($gid, 'getGroupDetails', [$gid]); + } /** * Check if backend implements actions * @param int $actions bitwise-or'ed actions diff --git a/lib/User/IUserTools.php b/lib/User/IUserTools.php index d85c615e3..752d24128 100644 --- a/lib/User/IUserTools.php +++ b/lib/User/IUserTools.php @@ -33,7 +33,7 @@ public function getConnection(); public function readAttribute($dn, $attr, $filter = 'objectClass=*'); - public function dn2username($dn, $ldapname = null); + public function dn2username($dn); /** * returns the LDAP DN for the given internal ownCloud name of the user From 9dddf58a19ed11a05d420dfcf634d5c266b70f6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Fri, 22 Jul 2022 15:13:06 +0200 Subject: [PATCH 02/10] Allow configuration of internal groupname attr --- js/wizard/wizardTabExpert.js | 13 +++++++++++++ lib/Access.php | 3 --- lib/Configuration.php | 4 ++++ lib/Connection.php | 1 + templates/settings.php | 8 ++++++++ 5 files changed, 26 insertions(+), 3 deletions(-) diff --git a/js/wizard/wizardTabExpert.js b/js/wizard/wizardTabExpert.js index 7cfd49ba0..d241b9a28 100644 --- a/js/wizard/wizardTabExpert.js +++ b/js/wizard/wizardTabExpert.js @@ -28,6 +28,10 @@ OCA = OCA || {}; $element: $('#ldap_expert_username_attr'), setMethod: 'setUsernameAttribute' }, + ldap_expert_groupname_attr: { + $element: $('#ldap_expert_groupname_attr'), + setMethod: 'setGroupnameAttribute' + }, ldap_expert_uuid_user_attr: { $element: $('#ldap_expert_uuid_user_attr'), setMethod: 'setUserUUIDAttribute' @@ -73,6 +77,15 @@ OCA = OCA || {}; this.setElementValue(this.managedItems.ldap_expert_username_attr.$element, attribute); }, + /** + * sets the attribute to be used to create an ownCloud ID (username) + * + * @param {string} attribute + */ + setGroupnameAttribute: function(attribute) { + this.setElementValue(this.managedItems.ldap_expert_groupname_attr.$element, attribute); + }, + /** * sets the attribute that provides an unique identifier per LDAP user * entry diff --git a/lib/Access.php b/lib/Access.php index 17bd063d0..5461038a2 100644 --- a/lib/Access.php +++ b/lib/Access.php @@ -474,7 +474,6 @@ public function username2dn($name) { * returns the internal ownCloud name for the given LDAP DN of the group, false on DN outside of search DN or failure * * @param string $fdn the dn of the group object - * @param string $ldapName optional, the display name of the object * @return string|false with the name to use in ownCloud, false on DN outside of search DN * @throws \OC\ServerNotAvailableException */ @@ -532,7 +531,6 @@ public function groupsMatchFilter($groupDNs) { * returns the internal ownCloud name for the given LDAP DN of the user, false on DN outside of search DN or failure * * @param string $fdn the dn of the user object - * @param string $ldapName optional, the display name of the object * @return string|false with with the name to use in ownCloud * @throws \OC\ServerNotAvailableException */ @@ -551,7 +549,6 @@ public function dn2username($fdn) { * returns an internal ownCloud name for the given LDAP DN, false on DN outside of search DN * * @param string $fdn the dn of the user object - * @param string $ldapDisplayName optional, the display name of the object * @param bool $isUser optional, whether it is a user object (otherwise group assumed) * @return string|false with with the name to use in ownCloud * @throws \BadMethodCallException diff --git a/lib/Configuration.php b/lib/Configuration.php index 306c95131..ed235612e 100644 --- a/lib/Configuration.php +++ b/lib/Configuration.php @@ -80,6 +80,7 @@ * @property string $hasMemberOfFilterSupport, * @property string $useMemberOfToDetectMembership, * @property string $ldapExpertUsernameAttr, + * @property string $ldapExpertGroupnameAttr, * @property string $ldapExpertUUIDUserAttr, * @property string $ldapExpertUUIDGroupAttr, * @property string $lastJpegPhotoLookup, @@ -148,6 +149,7 @@ class Configuration { 'hasMemberOfFilterSupport' => false, 'useMemberOfToDetectMembership' => true, 'ldapExpertUsernameAttr' => null, + 'ldapExpertGroupnameAttr' => null, 'ldapExpertUUIDUserAttr' => null, 'ldapExpertUUIDGroupAttr' => null, 'lastJpegPhotoLookup' => null, @@ -546,6 +548,7 @@ public function getDefaults() { 'ldap_attributes_for_user_search' => '', 'ldap_attributes_for_group_search' => '', 'ldap_expert_username_attr' => '', + 'ldap_expert_groupname_attr' => '', 'ldap_expert_uuid_user_attr' => '', 'ldap_expert_uuid_group_attr' => '', 'has_memberof_filter_support' => 0, @@ -605,6 +608,7 @@ public function getConfigTranslationArray() { 'ldap_attributes_for_user_search' => 'ldapAttributesForUserSearch', 'ldap_attributes_for_group_search' => 'ldapAttributesForGroupSearch', 'ldap_expert_username_attr' => 'ldapExpertUsernameAttr', + 'ldap_expert_groupname_attr' => 'ldapExpertGroupnameAttr', 'ldap_expert_uuid_user_attr' => 'ldapExpertUUIDUserAttr', 'ldap_expert_uuid_group_attr' => 'ldapExpertUUIDGroupAttr', 'has_memberof_filter_support' => 'hasMemberOfFilterSupport', diff --git a/lib/Connection.php b/lib/Connection.php index d20032bd0..ccb8a7e4a 100644 --- a/lib/Connection.php +++ b/lib/Connection.php @@ -61,6 +61,7 @@ * @property string $ldapQuotaAttribute * @property string $ldapEmailAttribute * @property string $ldapExpertUsernameAttr + * @property string $ldapExpertGroupnameAttr * @property string $homeFolderNamingRule * @property array $ldapAttributesForUserSearch * @property string $ldapUuidUserAttribute diff --git a/templates/settings.php b/templates/settings.php index 90e74807e..7563f9488 100644 --- a/templates/settings.php +++ b/templates/settings.php @@ -290,6 +290,14 @@ +
+

t('Internal Groupname')); ?>

+

t('The internal groupname is used to uniquely identify the group. It has the same restrictions as the internal username, in particular, the group name must be immutable and unique. By default, the UUID will be used. This internal groupname won\'t likely by visible because a displayname attribute is intended to be used to show the group.')); ?>

+
+ + +
+

t('Override UUID detection')); ?>

t('By default, the UUID attribute is automatically detected. The UUID attribute is used to doubtlessly identify LDAP users and groups. Also, the internal username will be created based on the UUID, if not specified otherwise above. You can override the setting and pass an attribute of your choice. You must make sure that the attribute of your choice can be fetched for both users and groups and it is unique. Leave it empty for default behavior. Changes will have effect only on newly mapped (added) LDAP users and groups.')); ?>

From bbd48bd8118c9362b489ecacc7c03715e1df1a25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Mon, 25 Jul 2022 10:17:00 +0200 Subject: [PATCH 03/10] Cache details and include migration --- appinfo/Migrations/Version20220725070804.php | 37 ++++++++++++++++++++ appinfo/info.xml | 2 +- lib/Group_LDAP.php | 11 +++++- 3 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 appinfo/Migrations/Version20220725070804.php diff --git a/appinfo/Migrations/Version20220725070804.php b/appinfo/Migrations/Version20220725070804.php new file mode 100644 index 000000000..1481572ea --- /dev/null +++ b/appinfo/Migrations/Version20220725070804.php @@ -0,0 +1,37 @@ +config = $config; + $this->helper = $helper; + } + /** + * @param IOutput $out + */ + public function run(IOutput $out) { + $prefixes = $this->helper->getServerConfigurationPrefixes(); + foreach ($prefixes as $prefix) { + $groupnameValue = $this->config->getAppValue('user_ldap', "{$prefix}ldap_expert_groupname_attr", null); + if ($groupnameValue === null) { + $groupDisplaynameValue = $this->config->getAppValue('user_ldap', "{$prefix}ldap_group_display_name", null); + if ($groupDisplaynameValue !== null) { + $this->config->setAppValue('user_ldap', "{$prefix}ldap_expert_groupname_attr", $groupDisplaynameValue); + } + } + } + } +} diff --git a/appinfo/info.xml b/appinfo/info.xml index b89b10292..8fb8a2fa5 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -14,7 +14,7 @@ More information is available in the [LDAP User and Group Backend documentation] AGPL Jörn Friedrich Dreyer, Tom Needham, Juan Pablo Villafañez Ramos, Dominik Schmidt and Arthur Schiwon - 0.16.1 + 0.17.0 diff --git a/lib/Group_LDAP.php b/lib/Group_LDAP.php index bd586db73..1ad6542ca 100644 --- a/lib/Group_LDAP.php +++ b/lib/Group_LDAP.php @@ -1002,6 +1002,12 @@ public function groupExists($gid) { } public function getGroupDetails($gid) { + $cacheKey = "groupDetails-$gid"; + $details = $this->access->getConnection()->getFromCache($cacheKey); + if ($details !== null) { + return $details; + } + $dn = $this->access->groupname2dn($gid); if ($dn === false) { // FIXME: It seems local groups also end up going through here... @@ -1010,10 +1016,13 @@ public function getGroupDetails($gid) { $attr = $this->access->getConnection()->ldapGroupDisplayName; $displayname = $this->access->readAttribute($dn, $attr); - return [ + + $details = [ 'gid' => $gid, 'displayName' => $displayname[0], ]; + $this->access->getConnection()->writeToCache($cacheKey, $details); + return $details; } /** From 71a19e72ccca9dd8e8c27366eaa4d70e286db260 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Mon, 25 Jul 2022 10:52:17 +0200 Subject: [PATCH 04/10] Remove dead code --- lib/Access.php | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/lib/Access.php b/lib/Access.php index 5461038a2..b3be1f4a5 100644 --- a/lib/Access.php +++ b/lib/Access.php @@ -557,11 +557,9 @@ public function dn2username($fdn) { public function dn2ocname($fdn, $isUser = true) { if ($isUser) { $mapper = $this->getUserMapper(); - $displayNameAttribute = $this->connection->ldapUserDisplayName; $nameAttribute = (string)$this->connection->ldapExpertUsernameAttr; } else { $mapper = $this->getGroupMapper(); - $displayNameAttribute = $this->connection->ldapGroupDisplayName; $nameAttribute = (string)$this->connection->ldapExpertGroupnameAttr; } @@ -691,21 +689,9 @@ public function ownCloudGroupNames($ldapGroups) { * @throws \OC\ServerNotAvailableException */ private function ldap2ownCloudNames($ldapObjects, $isUsers) { - if ($isUsers) { - $nameAttribute = $this->connection->ldapUserDisplayName; - $sndAttribute = $this->connection->ldapUserDisplayName2; - } else { - $nameAttribute = $this->connection->ldapGroupDisplayName; - } $ownCloudNames = []; foreach ($ldapObjects as $ldapObject) { - $nameByLDAP = null; - if (isset($ldapObject[$nameAttribute][0])) { - // might be set, but not necessarily. if so, we use it. - $nameByLDAP = $ldapObject[$nameAttribute][0]; - } - $ocName = $this->dn2ocname($ldapObject['dn'][0], $isUsers); if ($ocName) { $ownCloudNames[$ldapObject['dn'][0]] = $ocName; From 9a7bb374e1ef3a0467cf0b9543e1c02f8862b5cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 26 Jul 2022 16:59:09 +0200 Subject: [PATCH 05/10] Fix comment --- js/wizard/wizardTabExpert.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/wizard/wizardTabExpert.js b/js/wizard/wizardTabExpert.js index d241b9a28..ab7fe717c 100644 --- a/js/wizard/wizardTabExpert.js +++ b/js/wizard/wizardTabExpert.js @@ -78,7 +78,7 @@ OCA = OCA || {}; }, /** - * sets the attribute to be used to create an ownCloud ID (username) + * sets the attribute to be used to create an ownCloud ID (groupname) * * @param {string} attribute */ From a3fb7cb9c8f9dac34989fc66c738dfc18354467c Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Tue, 2 Aug 2022 17:31:53 +0545 Subject: [PATCH 06/10] Define ldapExpertGroupnameAttr in acceptance test CI --- tests/acceptance/setConfig.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/acceptance/setConfig.sh b/tests/acceptance/setConfig.sh index 562561de1..ce7bf2566 100755 --- a/tests/acceptance/setConfig.sh +++ b/tests/acceptance/setConfig.sh @@ -7,6 +7,7 @@ configID="LDAPTestId" ./occ ldap:set-config "$configID" ldapBaseGroups "dc=owncloud,dc=com" ./occ ldap:set-config "$configID" ldapBaseUsers "dc=owncloud,dc=com" ./occ ldap:set-config "$configID" ldapEmailAttribute "mail" +./occ ldap:set-config "$configID" ldapExpertGroupnameAttr "cn" ./occ ldap:set-config "$configID" ldapExpertUUIDUserAttr "uid" ./occ ldap:set-config "$configID" ldapGroupDisplayName "cn" ./occ ldap:set-config "$configID" ldapGroupFilter "(&(|(objectclass=posixGroup)))" From a724b9b0e6b390707ab4f932d003b545202db855 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 2 Aug 2022 17:36:50 +0200 Subject: [PATCH 07/10] Sanitaze only the username, not the groupname --- lib/Access.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/Access.php b/lib/Access.php index b3be1f4a5..273616d9b 100644 --- a/lib/Access.php +++ b/lib/Access.php @@ -592,7 +592,11 @@ public function dn2ocname($fdn, $isUser = true) { } else { $name = $uuid; } - $intName = $this->sanitizeUsername($name); + + $intName = $name; + if ($isUser) { + $intName = $this->sanitizeUsername($name); + } //a new user/group! Add it only if it doesn't conflict with other backend's users or existing groups //disabling Cache is required to avoid that the new user is cached as not-existing in fooExists check From 4ad04c36c9b931f4996fa6b8e8897d8a9cb3beea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Fri, 5 Aug 2022 12:22:16 +0200 Subject: [PATCH 08/10] Ignore group details if displayname isn't found --- lib/Group_LDAP.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/Group_LDAP.php b/lib/Group_LDAP.php index 1ad6542ca..8afcc669d 100644 --- a/lib/Group_LDAP.php +++ b/lib/Group_LDAP.php @@ -1010,12 +1010,15 @@ public function getGroupDetails($gid) { $dn = $this->access->groupname2dn($gid); if ($dn === false) { - // FIXME: It seems local groups also end up going through here... return null; } $attr = $this->access->getConnection()->ldapGroupDisplayName; $displayname = $this->access->readAttribute($dn, $attr); + if (!\is_array($displayname)) { + // displayname attr not found + return null; + } $details = [ 'gid' => $gid, From b618814db340c09bd1c82288e0699313877d0dd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Fri, 26 Aug 2022 10:08:56 +0200 Subject: [PATCH 09/10] Rise minimum OC version to 10.11 --- appinfo/info.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appinfo/info.xml b/appinfo/info.xml index 8fb8a2fa5..968879564 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -25,7 +25,7 @@ More information is available in the [LDAP User and Group Backend documentation] https://raw.githubusercontent.com/owncloud/screenshots/master/user_ldap/ownCloud-app-ldap-user-management.jpg ldap - + User_LDAP From f804a030f9ed10784b6ef57213ba640c36ffb79d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Fri, 4 Nov 2022 09:31:59 +0100 Subject: [PATCH 10/10] Include expectation for the getGroupDetails method --- lib/Group_LDAP.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/Group_LDAP.php b/lib/Group_LDAP.php index 8afcc669d..1f4905350 100644 --- a/lib/Group_LDAP.php +++ b/lib/Group_LDAP.php @@ -1001,6 +1001,16 @@ public function groupExists($gid) { return true; } + /** + * Get the details of the target group. The details consist (as of now) + * of the gid and the displayname of the group. More data might be added + * accordingly to the interface. + * The method returns null on error (such as missing displayname, or + * missing group) + * @param string $gid the gid of the group we want to get the details of + * @return array|null an array containing the gid and the displayname such as + * ['gid' => 'abcdef', 'displayname' => 'my group'] + */ public function getGroupDetails($gid) { $cacheKey = "groupDetails-$gid"; $details = $this->access->getConnection()->getFromCache($cacheKey);