From 3372bcc7fcba026b9d2ac58812863e1e74c8ffdf Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 17 May 2019 16:19:23 +0200 Subject: [PATCH 1/2] fixes possible override of uniqueMember by autodetection * uniqueMember was the default so we did not know whether this setting is desired or the initial value * autodetection of the user-group association attribute runs only when it was not set (as far as we knew) * the default is now empty * thus LDAPProvider might return this value as well (in exceptional cases) * if a group base is given (edge case), use this instead of general base * resolves #12682 Signed-off-by: Arthur Schiwon --- .../js/wizard/wizardDetectorUserGroupAssociation.js | 2 +- apps/user_ldap/lib/Configuration.php | 2 +- apps/user_ldap/lib/LDAPProvider.php | 2 +- apps/user_ldap/lib/Wizard.php | 6 +++--- lib/public/LDAP/ILDAPProvider.php | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/user_ldap/js/wizard/wizardDetectorUserGroupAssociation.js b/apps/user_ldap/js/wizard/wizardDetectorUserGroupAssociation.js index 953a0b909a626..a20bdd9ac9bf0 100644 --- a/apps/user_ldap/js/wizard/wizardDetectorUserGroupAssociation.js +++ b/apps/user_ldap/js/wizard/wizardDetectorUserGroupAssociation.js @@ -27,7 +27,7 @@ OCA = OCA || {}; run: function(model, configID) { // TODO: might be better with configuration marker as uniqueMember // is a valid value (although probably less common then member and memberUid). - if(model.configuration.ldap_group_member_assoc_attribute && model.configuration.ldap_group_member_assoc_attribute !== 'uniqueMember') { + if(model.configuration.ldap_group_member_assoc_attribute && model.configuration.ldap_group_member_assoc_attribute !== '') { // a value is already set. Don't overwrite and don't ask LDAP // without reason. return false; diff --git a/apps/user_ldap/lib/Configuration.php b/apps/user_ldap/lib/Configuration.php index ee77702a090fd..a6da6d6518fff 100644 --- a/apps/user_ldap/lib/Configuration.php +++ b/apps/user_ldap/lib/Configuration.php @@ -456,7 +456,7 @@ public function getDefaults() { 'ldap_quota_def' => '', 'ldap_quota_attr' => '', 'ldap_email_attr' => '', - 'ldap_group_member_assoc_attribute' => 'uniqueMember', + 'ldap_group_member_assoc_attribute' => '', 'ldap_cache_ttl' => 600, 'ldap_uuid_user_attribute' => 'auto', 'ldap_uuid_group_attribute' => 'auto', diff --git a/apps/user_ldap/lib/LDAPProvider.php b/apps/user_ldap/lib/LDAPProvider.php index 94793980b396f..4121bdd9d2edd 100644 --- a/apps/user_ldap/lib/LDAPProvider.php +++ b/apps/user_ldap/lib/LDAPProvider.php @@ -279,7 +279,7 @@ public function getLDAPEmailField($uid) { /** * Get the LDAP type of association between users and groups * @param string $gid group id - * @return string the configuration, one of: 'memberUid', 'uniqueMember', 'member', 'gidNumber' + * @return string the configuration, one of: 'memberUid', 'uniqueMember', 'member', 'gidNumber', '' * @throws \Exception if group id was not found in LDAP */ public function getLDAPGroupMemberAssoc($gid) { diff --git a/apps/user_ldap/lib/Wizard.php b/apps/user_ldap/lib/Wizard.php index 26b5a8cab71ca..0ffb74a36a3cf 100644 --- a/apps/user_ldap/lib/Wizard.php +++ b/apps/user_ldap/lib/Wizard.php @@ -794,7 +794,7 @@ private function checkHost() { * @throws \Exception */ private function detectGroupMemberAssoc() { - $possibleAttrs = array('uniqueMember', 'memberUid', 'member', 'gidNumber'); + $possibleAttrs = ['uniqueMember', 'memberUid', 'member', 'gidNumber']; $filter = $this->configuration->ldapGroupFilter; if(empty($filter)) { return false; @@ -803,7 +803,7 @@ private function detectGroupMemberAssoc() { if(!$cr) { throw new \Exception('Could not connect to LDAP'); } - $base = $this->configuration->ldapBase[0]; + $base = $this->configuration->ldapBaseGroups[0] ?: $this->configuration->ldapBase[0]; $rr = $this->ldap->search($cr, $base, $filter, $possibleAttrs, 0, 1000); if(!$this->ldap->isResource($rr)) { return false; @@ -812,7 +812,7 @@ private function detectGroupMemberAssoc() { while(is_resource($er)) { $this->ldap->getDN($cr, $er); $attrs = $this->ldap->getAttributes($cr, $er); - $result = array(); + $result = []; $possibleAttrsCount = count($possibleAttrs); for($i = 0; $i < $possibleAttrsCount; $i++) { if(isset($attrs[$possibleAttrs[$i]])) { diff --git a/lib/public/LDAP/ILDAPProvider.php b/lib/public/LDAP/ILDAPProvider.php index 9e13fb7e796de..586481d837e1c 100644 --- a/lib/public/LDAP/ILDAPProvider.php +++ b/lib/public/LDAP/ILDAPProvider.php @@ -151,7 +151,7 @@ public function getLDAPEmailField($uid); /** * Get the LDAP attribute name for the type of association betweeen users and groups * @param string $gid group id - * @return string the configuration, one of: 'memberUid', 'uniqueMember', 'member', 'gidNumber' + * @return string the configuration, one of: 'memberUid', 'uniqueMember', 'member', 'gidNumber', '' * @throws \Exception if group id was not found in LDAP * @since 13.0.0 */ From 54299f4e66085b9ed6da914167e324bf69bbb9b8 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 22 May 2019 12:48:12 +0200 Subject: [PATCH 2/2] set the ldapGroupMemberAssocAttr for group tests Signed-off-by: Arthur Schiwon --- .../ldap_features/ldap-openldap.feature | 15 +++++++++------ .../ldap_features/openldap-numerical-id.feature | 1 + 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/build/integration/ldap_features/ldap-openldap.feature b/build/integration/ldap_features/ldap-openldap.feature index 6c5ed8b462baa..570cf287a2eb4 100644 --- a/build/integration/ldap_features/ldap-openldap.feature +++ b/build/integration/ldap_features/ldap-openldap.feature @@ -42,8 +42,9 @@ Feature: LDAP Scenario: Test group filter with one specific group Given modify LDAP configuration - | ldapGroupFilter | cn=RedGroup | - | ldapBaseGroups | ou=Groups,ou=Ordinary,dc=nextcloud,dc=ci | + | ldapGroupFilter | cn=RedGroup | + | ldapGroupMemberAssocAttr | member | + | ldapBaseGroups | ou=Groups,ou=Ordinary,dc=nextcloud,dc=ci | And As an "admin" And sending "GET" to "/cloud/groups" Then the OCS status code should be "200" @@ -55,8 +56,9 @@ Feature: LDAP Scenario: Test group filter with two specific groups Given modify LDAP configuration - | ldapGroupFilter | (\|(cn=RedGroup)(cn=GreenGroup)) | - | ldapBaseGroups | ou=Groups,ou=Ordinary,dc=nextcloud,dc=ci | + | ldapGroupFilter | (\|(cn=RedGroup)(cn=GreenGroup)) | + | ldapGroupMemberAssocAttr | member | + | ldapBaseGroups | ou=Groups,ou=Ordinary,dc=nextcloud,dc=ci | And As an "admin" And sending "GET" to "/cloud/groups" Then the OCS status code should be "200" @@ -68,8 +70,9 @@ Feature: LDAP Scenario: Test group filter ruling out a group from a different base Given modify LDAP configuration - | ldapGroupFilter | (objectClass=groupOfNames) | - | ldapBaseGroups | ou=Groups,ou=Ordinary,dc=nextcloud,dc=ci | + | ldapGroupFilter | (objectClass=groupOfNames) | + | ldapGroupMemberAssocAttr | member | + | ldapBaseGroups | ou=Groups,ou=Ordinary,dc=nextcloud,dc=ci | And As an "admin" And sending "GET" to "/cloud/groups" Then the OCS status code should be "200" diff --git a/build/integration/ldap_features/openldap-numerical-id.feature b/build/integration/ldap_features/openldap-numerical-id.feature index 4112df0ae1a6e..4ea63823295fc 100644 --- a/build/integration/ldap_features/openldap-numerical-id.feature +++ b/build/integration/ldap_features/openldap-numerical-id.feature @@ -35,6 +35,7 @@ Scenario: Test LDAP group retrieval with numeric group ids and nesting Given modify LDAP configuration | ldapBaseGroups | ou=NumericGroups,dc=nextcloud,dc=ci | | ldapGroupFilter | (objectclass=groupOfNames) | + | ldapGroupMemberAssocAttr | member | | ldapNestedGroups | 1 | | useMemberOfToDetectMembership | 1 | And As an "admin"