From c0181f81445149232cf4d3d2fb55f79292ff0044 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 15 Aug 2023 12:03:13 +0200 Subject: [PATCH] graph: honor the OCIS_LDAP_GROUP_SCHEMA_MEMBER setting Fixes: #7032 --- .../fix-graph-ldap-member-setting.md | 6 ++++ services/graph/pkg/config/config.go | 1 + .../pkg/config/defaults/defaultconfig.go | 1 + services/graph/pkg/identity/ldap.go | 36 +++++++++---------- .../identity/ldap_education_school_test.go | 13 +++---- services/graph/pkg/identity/ldap_group.go | 7 ++-- services/graph/pkg/identity/ldap_test.go | 13 +++---- 7 files changed, 43 insertions(+), 34 deletions(-) create mode 100644 changelog/unreleased/fix-graph-ldap-member-setting.md diff --git a/changelog/unreleased/fix-graph-ldap-member-setting.md b/changelog/unreleased/fix-graph-ldap-member-setting.md new file mode 100644 index 0000000000..9e80560f75 --- /dev/null +++ b/changelog/unreleased/fix-graph-ldap-member-setting.md @@ -0,0 +1,6 @@ +Bugfix: graph service did not honor the OCIS_LDAP_GROUP_SCHEMA_MEMBER setting + +We fixed issue when using a custom LDAP attribute for group members. The graph service +did not honor the OCIS_LDAP_GROUP_SCHEMA_MEMBER environment variable + +https://github.com/owncloud/ocis/issues/7032 diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go index f1603b2928..47ea6c4bb3 100644 --- a/services/graph/pkg/config/config.go +++ b/services/graph/pkg/config/config.go @@ -76,6 +76,7 @@ type LDAP struct { GroupFilter string `yaml:"group_filter" env:"OCIS_LDAP_GROUP_FILTER;LDAP_GROUP_FILTER;GRAPH_LDAP_GROUP_FILTER" desc:"LDAP filter to add to the default filters for group searches." deprecationVersion:"3.0" removalVersion:"4.0.0" deprecationInfo:"LDAP_GROUP_FILTER changing name for consistency" deprecationReplacement:"OCIS_LDAP_GROUP_FILTER"` GroupObjectClass string `yaml:"group_objectclass" env:"OCIS_LDAP_GROUP_OBJECTCLASS;LDAP_GROUP_OBJECTCLASS;GRAPH_LDAP_GROUP_OBJECTCLASS" desc:"The object class to use for groups in the default group search filter ('groupOfNames')." deprecationVersion:"3.0" removalVersion:"4.0.0" deprecationInfo:"LDAP_GROUP_OBJECTCLASS changing name for consistency" deprecationReplacement:"OCIS_LDAP_GROUP_OBJECTCLASS"` GroupNameAttribute string `yaml:"group_name_attribute" env:"OCIS_LDAP_GROUP_SCHEMA_GROUPNAME;LDAP_GROUP_SCHEMA_GROUPNAME;GRAPH_LDAP_GROUP_NAME_ATTRIBUTE" desc:"LDAP Attribute to use for the name of groups." deprecationVersion:"3.0" removalVersion:"4.0.0" deprecationInfo:"LDAP_GROUP_SCHEMA_GROUPNAME changing name for consistency" deprecationReplacement:"OCIS_LDAP_GROUP_SCHEMA_GROUPNAME"` + GroupMemberAttribute string `yaml:"group_member_attribute" env:"OCIS_LDAP_GROUP_SCHEMA_MEMBER;LDAP_GROUP_SCHEMA_MEMBER;GRAPH_LDAP_GROUP_MEMBER_ATTRIBUTE" desc:"LDAP Attribute that is used for group members." deprecationVersion:"3.0" removalVersion:"4.0.0" deprecationInfo:"LDAP_GROUP_SCHEMA_MEMBER changing name for consistency" deprecationReplacement:"OCIS_LDAP_GROUP_SCHEMA_MEMBER"` GroupIDAttribute string `yaml:"group_id_attribute" env:"OCIS_LDAP_GROUP_SCHEMA_ID;LDAP_GROUP_SCHEMA_ID;GRAPH_LDAP_GROUP_ID_ATTRIBUTE" desc:"LDAP Attribute to use as the unique id for groups. This should be a stable globally unique ID like a UUID." deprecationVersion:"3.0" removalVersion:"4.0.0" deprecationInfo:"LDAP_GROUP_SCHEMA_ID changing name for consistency" deprecationReplacement:"OCIS_LDAP_GROUP_SCHEMA_ID"` GroupIDIsOctetString bool `yaml:"group_id_is_octet_string" env:"OCIS_LDAP_GROUP_SCHEMA_ID_IS_OCTETSTRING;GRAPH_LDAP_GROUP_SCHEMA_ID_IS_OCTETSTRING" desc:"Set this to true if the defined 'ID' attribute for groups is of the 'OCTETSTRING' syntax. This is required when using the 'objectGUID' attribute of Active Directory for the group ID's."` diff --git a/services/graph/pkg/config/defaults/defaultconfig.go b/services/graph/pkg/config/defaults/defaultconfig.go index 10871d42b4..b12410a869 100644 --- a/services/graph/pkg/config/defaults/defaultconfig.go +++ b/services/graph/pkg/config/defaults/defaultconfig.go @@ -89,6 +89,7 @@ func DefaultConfig() *config.Config { GroupFilter: "", GroupObjectClass: "groupOfNames", GroupNameAttribute: "cn", + GroupMemberAttribute: "member", GroupIDAttribute: "owncloudUUID", EducationResourcesEnabled: false, }, diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index 7189d45b02..b3625f713b 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -20,10 +20,8 @@ import ( ) const ( - _givenNameAttribute = "givenname" - _surNameAttribute = "sn" - _ldapGroupOfNamesAttribute = "(objectClass=groupOfNames)" - _ldapGroupMemberAttribute = "member" + _givenNameAttribute = "givenname" + _surNameAttribute = "sn" ) // DisableUserMechanismType is used instead of directly using the string values from the configuration. @@ -121,10 +119,9 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD return nil, errors.New("invalid group attribute mappings") } gam := groupAttributeMap{ - name: config.GroupNameAttribute, - id: config.GroupIDAttribute, - member: _ldapGroupMemberAttribute, - memberSyntax: "dn", + name: config.GroupNameAttribute, + id: config.GroupIDAttribute, + member: config.GroupMemberAttribute, } var userScope, groupScope int @@ -1040,15 +1037,16 @@ func (i *LDAP) CreateLDAPGroupByDN(dn string) error { return i.conn.Add(ar) } -func (i *LDAP) disableUser(logger log.Logger, userDN string) (err error) { - group, err := i.getEntryByDN(i.localUserDisableGroupDN, []string{_ldapGroupMemberAttribute}, _ldapGroupOfNamesAttribute) +func (i *LDAP) addUserToDisableGroup(logger log.Logger, userDN string) (err error) { + groupFilter := fmt.Sprintf("(objectClass=%s)", i.groupObjectClass) + group, err := i.getEntryByDN(i.localUserDisableGroupDN, []string{i.groupAttributeMap.member}, groupFilter) if err != nil { return err } mr := ldap.ModifyRequest{DN: group.DN} - mr.Add(_ldapGroupMemberAttribute, []string{userDN}) + mr.Add(i.groupAttributeMap.member, []string{userDN}) err = i.conn.Modify(&mr) var lerr *ldap.Error @@ -1063,15 +1061,16 @@ func (i *LDAP) disableUser(logger log.Logger, userDN string) (err error) { return err } -func (i *LDAP) enableUser(logger log.Logger, userDN string) (err error) { - group, err := i.getEntryByDN(i.localUserDisableGroupDN, []string{_ldapGroupMemberAttribute}, _ldapGroupOfNamesAttribute) +func (i *LDAP) removeUserFromDisableGroup(logger log.Logger, userDN string) (err error) { + groupFilter := fmt.Sprintf("(objectClass=%s)", i.groupObjectClass) + group, err := i.getEntryByDN(i.localUserDisableGroupDN, []string{i.groupAttributeMap.member}, groupFilter) if err != nil { return err } mr := ldap.ModifyRequest{DN: group.DN} - mr.Delete(_ldapGroupMemberAttribute, []string{userDN}) + mr.Delete(i.groupAttributeMap.member, []string{userDN}) err = i.conn.Modify(&mr) var lerr *ldap.Error @@ -1097,7 +1096,8 @@ func (i *LDAP) userEnabledByAttribute(user *ldap.Entry) bool { } func (i *LDAP) usersEnabledStateFromGroup(users []string) (usersEnabledState map[string]bool, err error) { - group, err := i.getEntryByDN(i.localUserDisableGroupDN, []string{_ldapGroupMemberAttribute}, _ldapGroupOfNamesAttribute) + groupFilter := fmt.Sprintf("(objectClass=%s)", i.groupObjectClass) + group, err := i.getEntryByDN(i.localUserDisableGroupDN, []string{i.groupAttributeMap.member}, groupFilter) if err != nil { return nil, err @@ -1108,7 +1108,7 @@ func (i *LDAP) usersEnabledStateFromGroup(users []string) (usersEnabledState map usersEnabledState[user] = true } - for _, memberDN := range group.GetEqualFoldAttributeValues(_ldapGroupMemberAttribute) { + for _, memberDN := range group.GetEqualFoldAttributeValues(i.groupAttributeMap.member) { usersEnabledState[memberDN] = false } @@ -1174,9 +1174,9 @@ func (i *LDAP) updateAccountEnabledState(logger log.Logger, accountEnabled bool, updateNeeded = true case DisableMechanismGroup: if accountEnabled { - err = i.enableUser(logger, e.DN) + err = i.removeUserFromDisableGroup(logger, e.DN) } else { - err = i.disableUser(logger, e.DN) + err = i.addUserToDisableGroup(logger, e.DN) } updateNeeded = false } diff --git a/services/graph/pkg/identity/ldap_education_school_test.go b/services/graph/pkg/identity/ldap_education_school_test.go index 6f4f5824ea..d0e06353b7 100644 --- a/services/graph/pkg/identity/ldap_education_school_test.go +++ b/services/graph/pkg/identity/ldap_education_school_test.go @@ -26,12 +26,13 @@ var eduConfig = config.LDAP{ DisableUserMechanism: "attribute", UserTypeAttribute: "userTypeAttribute", - GroupBaseDN: "ou=groups,dc=test", - GroupObjectClass: "groupOfNames", - GroupSearchScope: "sub", - GroupFilter: "", - GroupNameAttribute: "cn", - GroupIDAttribute: "entryUUID", + GroupBaseDN: "ou=groups,dc=test", + GroupObjectClass: "groupOfNames", + GroupSearchScope: "sub", + GroupFilter: "", + GroupNameAttribute: "cn", + GroupMemberAttribute: "member", + GroupIDAttribute: "entryUUID", WriteEnabled: true, EducationResourcesEnabled: true, diff --git a/services/graph/pkg/identity/ldap_group.go b/services/graph/pkg/identity/ldap_group.go index f9749bbbce..b0cf28c361 100644 --- a/services/graph/pkg/identity/ldap_group.go +++ b/services/graph/pkg/identity/ldap_group.go @@ -17,10 +17,9 @@ import ( ) type groupAttributeMap struct { - name string - id string - member string - memberSyntax string + name string + id string + member string } // GetGroup implements the Backend Interface for the LDAP Backend diff --git a/services/graph/pkg/identity/ldap_test.go b/services/graph/pkg/identity/ldap_test.go index f3a18560ab..fcd5a551ae 100644 --- a/services/graph/pkg/identity/ldap_test.go +++ b/services/graph/pkg/identity/ldap_test.go @@ -40,12 +40,13 @@ var lconfig = config.LDAP{ LdapDisabledUsersGroupDN: disableUsersGroup, DisableUserMechanism: "attribute", - GroupBaseDN: "ou=groups,dc=test", - GroupObjectClass: "groupOfNames", - GroupSearchScope: "sub", - GroupFilter: "", - GroupNameAttribute: "cn", - GroupIDAttribute: "entryUUID", + GroupBaseDN: "ou=groups,dc=test", + GroupObjectClass: "groupOfNames", + GroupSearchScope: "sub", + GroupFilter: "", + GroupNameAttribute: "cn", + GroupMemberAttribute: "member", + GroupIDAttribute: "entryUUID", WriteEnabled: true, }