From 399e05b256b58d5d806f7057f43be8808586d3cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sw=C3=A4rd?= Date: Thu, 9 Feb 2023 11:38:50 +0100 Subject: [PATCH] Remove code duplication for ldap entry membership. --- services/graph/pkg/identity/backend.go | 3 ++ services/graph/pkg/identity/cs3.go | 6 +++ services/graph/pkg/identity/ldap.go | 40 +++++++++++++++++- .../pkg/identity/ldap_education_class.go | 41 +------------------ services/graph/pkg/identity/ldap_group.go | 41 +------------------ services/graph/pkg/identity/mocks/backend.go | 25 +++++++++++ 6 files changed, 75 insertions(+), 81 deletions(-) diff --git a/services/graph/pkg/identity/backend.go b/services/graph/pkg/identity/backend.go index 4d2679773f..2a2ccddb4a 100644 --- a/services/graph/pkg/identity/backend.go +++ b/services/graph/pkg/identity/backend.go @@ -6,6 +6,7 @@ import ( "github.com/CiscoM31/godata" cs3 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + "github.com/go-ldap/ldap/v3" libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0/errorcode" ) @@ -40,6 +41,8 @@ type Backend interface { AddMembersToGroup(ctx context.Context, groupID string, memberID []string) error // RemoveMemberFromGroup removes a single member (by ID) from a group RemoveMemberFromGroup(ctx context.Context, groupID string, memberID string) error + // RemoveEntryByDNAndAttributeFromEntry creates a request to remove a single member entry by attribute and DN from an ldap entry + RemoveEntryByDNAndAttributeFromEntry(entry *ldap.Entry, dn string, attribute string) (*ldap.ModifyRequest, error) } // EducationBackend defines the Interface for an EducationBackend implementation diff --git a/services/graph/pkg/identity/cs3.go b/services/graph/pkg/identity/cs3.go index d6eab4029d..e963b2fba2 100644 --- a/services/graph/pkg/identity/cs3.go +++ b/services/graph/pkg/identity/cs3.go @@ -9,6 +9,7 @@ import ( cs3user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" + "github.com/go-ldap/ldap/v3" libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/v2/ocis-pkg/log" @@ -207,6 +208,11 @@ func (i *CS3) RemoveMemberFromGroup(ctx context.Context, groupID string, memberI return errorcode.New(errorcode.NotSupported, "not implemented") } +// RemoveEntryByDNAndAttributeFromEntry implements the Backend Interface. It's currently not supported for the CS3 backend +func (i *CS3) RemoveEntryByDNAndAttributeFromEntry(entry *ldap.Entry, dn string, attribute string) (*ldap.ModifyRequest, error) { + return nil, errorcode.New(errorcode.NotSupported, "not implemented") +} + func createGroupModelFromCS3(g *cs3group.Group) *libregraph.Group { if g.Id == nil { g.Id = &cs3group.GroupId{} diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index f7b7c11742..8913e6299e 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -8,6 +8,7 @@ import ( "github.com/CiscoM31/godata" "github.com/go-ldap/ldap/v3" "github.com/gofrs/uuid" + "github.com/libregraph/idm/pkg/ldapdn" libregraph "github.com/owncloud/libre-graph-api-go" oldap "github.com/owncloud/ocis/v2/ocis-pkg/ldap" "github.com/owncloud/ocis/v2/ocis-pkg/log" @@ -179,7 +180,7 @@ func (i *LDAP) DeleteUser(ctx context.Context, nameOrID string) error { for _, group := range groupEntries { logger.Debug().Str("group", group.DN).Str("user", e.DN).Msg("Cleaning up group membership") - if mr, err := i.removeMemberFromGroupEntry(group, e.DN); err == nil { + if mr, err := i.RemoveEntryByDNAndAttributeFromEntry(group, e.DN, i.groupAttributeMap.member); err == nil { if err = i.conn.Modify(mr); err != nil { // Errors when deleting the memberships are only logged as warnings but not returned // to the user as we already successfully deleted the users itself @@ -608,3 +609,40 @@ func stringToScope(scope string) (int, error) { } return s, nil } + +// RemoveEntryByDNAndAttributeFromEntry creates a request to remove a single member entry by attribute and DN from an ldap entry +func (i *LDAP) RemoveEntryByDNAndAttributeFromEntry(entry *ldap.Entry, dn string, attribute string) (*ldap.ModifyRequest, error) { + nOldDN, err := ldapdn.ParseNormalize(dn) + if err != nil { + return nil, err + } + entries := entry.GetEqualFoldAttributeValues(attribute) + found := false + for _, entry := range entries { + if entry == "" { + continue + } + if nEntry, err := ldapdn.ParseNormalize(entry); err != nil { + // We couldn't parse the entry value as a DN. Let's keep it + // as it is but log a warning + i.logger.Warn().Str("entryDN", entry).Err(err).Msg("Couldn't parse DN") + continue + } else { + if nEntry == nOldDN { + found = true + } + } + } + if !found { + i.logger.Debug().Str("backend", "ldap").Str("entry", entry.DN).Str("target", dn). + Msg("The target is not an entry in the attribute list") + return nil, ErrNotFound + } + + mr := ldap.ModifyRequest{DN: entry.DN} + if len(entries) == 1 { + mr.Add(attribute, []string{""}) + } + mr.Delete(attribute, []string{dn}) + return &mr, nil +} diff --git a/services/graph/pkg/identity/ldap_education_class.go b/services/graph/pkg/identity/ldap_education_class.go index cb55178713..8e4bad7f79 100644 --- a/services/graph/pkg/identity/ldap_education_class.go +++ b/services/graph/pkg/identity/ldap_education_class.go @@ -467,48 +467,9 @@ func (i *LDAP) RemoveTeacherFromEducationClass(ctx context.Context, classID stri return err } - if mr, err := i.removeTeacherFromClassEntry(class, teacher.DN); err == nil { + if mr, err := i.RemoveEntryByDNAndAttributeFromEntry(class, teacher.DN, i.educationConfig.classAttributeMap.teachers); err == nil { return i.conn.Modify(mr) } return nil } - -// removeTeacherFromClassEntry creates an LDAP Modify request (not sending it) -// that would update the supplied entry to remove the specified teacher from the -// class -func (i *LDAP) removeTeacherFromClassEntry(class *ldap.Entry, teacherDN string) (*ldap.ModifyRequest, error) { - nOldTeacherDN, err := ldapdn.ParseNormalize(teacherDN) - if err != nil { - return nil, err - } - teachers := class.GetEqualFoldAttributeValues(i.educationConfig.classAttributeMap.teachers) - found := false - for _, teacher := range teachers { - if teacher == "" { - continue - } - if nTeacher, err := ldapdn.ParseNormalize(teacher); err != nil { - // We couldn't parse the teacher value as a DN. Let's keep it - // as it is but log a warning - i.logger.Warn().Str("teacherDN", teacher).Err(err).Msg("Couldn't parse DN") - continue - } else { - if nTeacher == nOldTeacherDN { - found = true - } - } - } - if !found { - i.logger.Debug().Str("backend", "ldap").Str("groupdn", class.DN).Str("teacher", teacherDN). - Msg("The target is not a teacher of the class") - return nil, ErrNotFound - } - - mr := ldap.ModifyRequest{DN: class.DN} - if len(teachers) == 1 { - mr.Add(i.educationConfig.classAttributeMap.teachers, []string{""}) - } - mr.Delete(i.educationConfig.classAttributeMap.teachers, []string{teacherDN}) - return &mr, nil -} diff --git a/services/graph/pkg/identity/ldap_group.go b/services/graph/pkg/identity/ldap_group.go index 70027ed1f6..b572ddd93d 100644 --- a/services/graph/pkg/identity/ldap_group.go +++ b/services/graph/pkg/identity/ldap_group.go @@ -279,7 +279,7 @@ func (i *LDAP) RemoveMemberFromGroup(ctx context.Context, groupID string, member } logger.Debug().Str("backend", "ldap").Str("groupdn", ge.DN).Str("member", me.DN).Msg("remove member") - if mr, err := i.removeMemberFromGroupEntry(ge, me.DN); err == nil { + if mr, err := i.RemoveEntryByDNAndAttributeFromEntry(ge, me.DN, i.groupAttributeMap.member); err == nil { return i.conn.Modify(mr) } return nil @@ -413,45 +413,6 @@ func (i *LDAP) getLDAPGroupsByFilter(filter string, requestMembers, single bool) return res.Entries, nil } -// removeMemberFromGroupEntry creates an LDAP Modify request (not sending it) -// that would update the supplied entry to remove the specified member from the -// group -func (i *LDAP) removeMemberFromGroupEntry(group *ldap.Entry, memberDN string) (*ldap.ModifyRequest, error) { - nOldMemberDN, err := ldapdn.ParseNormalize(memberDN) - if err != nil { - return nil, err - } - members := group.GetEqualFoldAttributeValues(i.groupAttributeMap.member) - found := false - for _, member := range members { - if member == "" { - continue - } - if nMember, err := ldapdn.ParseNormalize(member); err != nil { - // We couldn't parse the member value as a DN. Let's keep it - // as it is but log a warning - i.logger.Warn().Str("memberDN", member).Err(err).Msg("Couldn't parse DN") - continue - } else { - if nMember == nOldMemberDN { - found = true - } - } - } - if !found { - i.logger.Debug().Str("backend", "ldap").Str("groupdn", group.DN).Str("member", memberDN). - Msg("The target is not a member of the group") - return nil, ErrNotFound - } - - mr := ldap.ModifyRequest{DN: group.DN} - if len(members) == 1 { - mr.Add(i.groupAttributeMap.member, []string{""}) - } - mr.Delete(i.groupAttributeMap.member, []string{memberDN}) - return &mr, nil -} - func (i *LDAP) getGroupByDN(dn string) (*ldap.Entry, error) { attrs := []string{ i.groupAttributeMap.id, diff --git a/services/graph/pkg/identity/mocks/backend.go b/services/graph/pkg/identity/mocks/backend.go index 9fcdcd8c19..997b5a04fe 100644 --- a/services/graph/pkg/identity/mocks/backend.go +++ b/services/graph/pkg/identity/mocks/backend.go @@ -7,6 +7,8 @@ import ( godata "github.com/CiscoM31/godata" + ldap "github.com/go-ldap/ldap/v3" + libregraph "github.com/owncloud/libre-graph-api-go" mock "github.com/stretchr/testify/mock" @@ -222,6 +224,29 @@ func (_m *Backend) GetUsers(ctx context.Context, oreq *godata.GoDataRequest) ([] return r0, r1 } +// RemoveEntryByDNAndAttributeFromEntry provides a mock function with given fields: entry, dn, attribute +func (_m *Backend) RemoveEntryByDNAndAttributeFromEntry(entry *ldap.Entry, dn string, attribute string) (*ldap.ModifyRequest, error) { + ret := _m.Called(entry, dn, attribute) + + var r0 *ldap.ModifyRequest + if rf, ok := ret.Get(0).(func(*ldap.Entry, string, string) *ldap.ModifyRequest); ok { + r0 = rf(entry, dn, attribute) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*ldap.ModifyRequest) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*ldap.Entry, string, string) error); ok { + r1 = rf(entry, dn, attribute) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // RemoveMemberFromGroup provides a mock function with given fields: ctx, groupID, memberID func (_m *Backend) RemoveMemberFromGroup(ctx context.Context, groupID string, memberID string) error { ret := _m.Called(ctx, groupID, memberID)