diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index 2db8d547dd..a7a34ec932 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -180,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 && mr != nil { + if mr, err := i.removeMemberFromGroupEntry(group, e.DN); 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 diff --git a/services/graph/pkg/identity/ldap_education_class.go b/services/graph/pkg/identity/ldap_education_class.go index 4fc3082bae..93ba9b1376 100644 --- a/services/graph/pkg/identity/ldap_education_class.go +++ b/services/graph/pkg/identity/ldap_education_class.go @@ -2,32 +2,262 @@ package identity import ( "context" + "fmt" "net/url" + "github.com/go-ldap/ldap/v3" libregraph "github.com/owncloud/libre-graph-api-go" + oldap "github.com/owncloud/ocis/v2/ocis-pkg/ldap" + "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0/errorcode" ) +type educationClassAttributeMap struct { + externalID string + classification string +} + +func newEducationClassAttributeMap() educationClassAttributeMap { + return educationClassAttributeMap{ + externalID: "ocEducationExternalId", + classification: "ocEducationClassType", + } +} + // GetEducationClasses implements the EducationBackend interface for the LDAP backend. func (i *LDAP) GetEducationClasses(ctx context.Context, queryParam url.Values) ([]*libregraph.EducationClass, error) { - return nil, errNotImplemented + logger := i.logger.SubloggerWithRequestID(ctx) + logger.Debug().Str("backend", "ldap").Msg("GetEducationClasses") + + search := queryParam.Get("search") + if search == "" { + search = queryParam.Get("$search") + } + + var classFilter string + if search != "" { + search = ldap.EscapeFilter(search) + classFilter = fmt.Sprintf( + "(|(%s=%s)(%s=%s*)(%s=%s*))", + i.educationConfig.classAttributeMap.externalID, search, + i.groupAttributeMap.name, search, + i.groupAttributeMap.id, search, + ) + } + classFilter = fmt.Sprintf("(&%s(objectClass=%s)%s)", i.groupFilter, i.educationConfig.classObjectClass, classFilter) + + classAttrs := i.getEducationClassAttrTypes(false) + + searchRequest := ldap.NewSearchRequest( + i.groupBaseDN, i.groupScope, ldap.NeverDerefAliases, 0, 0, false, + classFilter, + classAttrs, + nil, + ) + logger.Debug().Str("backend", "ldap"). + Str("base", searchRequest.BaseDN). + Str("filter", searchRequest.Filter). + Int("scope", searchRequest.Scope). + Int("sizelimit", searchRequest.SizeLimit). + Interface("attributes", searchRequest.Attributes). + Msg("GetEducationClasses") + res, err := i.conn.Search(searchRequest) + if err != nil { + return nil, errorcode.New(errorcode.ItemNotFound, err.Error()) + } + + classes := make([]*libregraph.EducationClass, 0, len(res.Entries)) + + var c *libregraph.EducationClass + for _, e := range res.Entries { + if c = i.createEducationClassModelFromLDAP(e); c == nil { + continue + } + classes = append(classes, c) + } + return classes, nil } // CreateEducationClass implements the EducationBackend interface for the LDAP backend. +// An EducationClass is mapped to an LDAP entry of the "groupOfNames" structural ObjectClass. +// With a few additional Attributes added on top via the "ocEducationClass" auxiallary ObjectClass. func (i *LDAP) CreateEducationClass(ctx context.Context, class libregraph.EducationClass) (*libregraph.EducationClass, error) { - return nil, errNotImplemented + logger := i.logger.SubloggerWithRequestID(ctx) + logger.Debug().Str("backend", "ldap").Msg("create educationClass") + if !i.writeEnabled { + return nil, errorcode.New(errorcode.NotAllowed, "server is configured read-only") + } + ar, err := i.educationClassToAddRequest(class) + if err != nil { + return nil, err + } + + if err := i.conn.Add(ar); err != nil { + return nil, err + } + + // Read back group from LDAP to get the generated UUID + e, err := i.getEducationClassByDN(ar.DN) + if err != nil { + return nil, err + } + return i.createEducationClassModelFromLDAP(e), nil } // GetEducationClass implements the EducationBackend interface for the LDAP backend. -func (i *LDAP) GetEducationClass(ctx context.Context, namedOrID string, queryParam url.Values) (*libregraph.EducationClass, error) { - return nil, errNotImplemented +func (i *LDAP) GetEducationClass(ctx context.Context, id string, queryParam url.Values) (*libregraph.EducationClass, error) { + logger := i.logger.SubloggerWithRequestID(ctx) + logger.Debug().Str("backend", "ldap").Msg("GetEducationClass") + e, err := i.getEducationClassByID(id, false) + if err != nil { + return nil, err + } + var class *libregraph.EducationClass + if class = i.createEducationClassModelFromLDAP(e); class == nil { + return nil, errorcode.New(errorcode.ItemNotFound, "not found") + } + return class, nil } // DeleteEducationClass implements the EducationBackend interface for the LDAP backend. -func (i *LDAP) DeleteEducationClass(ctx context.Context, nameOrID string) error { - return errNotImplemented +func (i *LDAP) DeleteEducationClass(ctx context.Context, id string) error { + logger := i.logger.SubloggerWithRequestID(ctx) + logger.Debug().Str("backend", "ldap").Msg("DeleteEducationClass") + if !i.writeEnabled { + return ErrReadOnly + } + e, err := i.getEducationClassByID(id, false) + if err != nil { + return err + } + + dr := ldap.DelRequest{DN: e.DN} + if err = i.conn.Del(&dr); err != nil { + return err + } + + // TODO update any users that are member of this school + return nil } // GetEducationClassMembers implements the EducationBackend interface for the LDAP backend. -func (i *LDAP) GetEducationClassMembers(ctx context.Context, nameOrID string) ([]*libregraph.EducationUser, error) { - return nil, errNotImplemented +func (i *LDAP) GetEducationClassMembers(ctx context.Context, id string) ([]*libregraph.EducationUser, error) { + logger := i.logger.SubloggerWithRequestID(ctx) + logger.Debug().Str("backend", "ldap").Msg("GetEducationClassMembers") + e, err := i.getEducationClassByID(id, true) + if err != nil { + return nil, err + } + + memberEntries, err := i.expandLDAPGroupMembers(ctx, e) + result := make([]*libregraph.EducationUser, 0, len(memberEntries)) + if err != nil { + return nil, err + } + for _, member := range memberEntries { + if u := i.createEducationUserModelFromLDAP(member); u != nil { + result = append(result, u) + } + } + + return result, nil +} + +func (i *LDAP) educationClassToAddRequest(class libregraph.EducationClass) (*ldap.AddRequest, error) { + plainGroup := i.educationClassToGroup(class) + ldapAttrs, err := i.groupToLDAPAttrValues(*plainGroup) + if err != nil { + return nil, err + } + ldapAttrs, err = i.educationClassToLDAPAttrValues(class, ldapAttrs) + if err != nil { + return nil, err + } + + ar := ldap.NewAddRequest(i.getEducationClassLDAPDN(class), nil) + + for attrType, values := range ldapAttrs { + ar.Attribute(attrType, values) + } + return ar, nil +} + +func (i *LDAP) educationClassToGroup(class libregraph.EducationClass) *libregraph.Group { + group := libregraph.NewGroup() + group.SetDisplayName(class.DisplayName) + + return group +} + +func (i *LDAP) educationClassToLDAPAttrValues(class libregraph.EducationClass, attrs ldapAttributeValues) (ldapAttributeValues, error) { + if externalID, ok := class.GetExternalIdOk(); ok { + attrs[i.educationConfig.classAttributeMap.externalID] = []string{*externalID} + } + if classification, ok := class.GetClassificationOk(); ok { + attrs[i.educationConfig.classAttributeMap.classification] = []string{*classification} + } + attrs["objectClass"] = append(attrs["objectClass"], i.educationConfig.classObjectClass) + return attrs, nil +} + +func (i *LDAP) getEducationClassAttrTypes(requestMembers bool) []string { + attrs := []string{ + i.groupAttributeMap.name, + i.groupAttributeMap.id, + i.educationConfig.classAttributeMap.classification, + i.educationConfig.classAttributeMap.externalID, + } + if requestMembers { + attrs = append(attrs, i.groupAttributeMap.member) + } + return attrs +} + +func (i *LDAP) getEducationClassByDN(dn string) (*ldap.Entry, error) { + filter := fmt.Sprintf("(objectClass=%s)", i.educationConfig.classObjectClass) + + if i.groupFilter != "" { + filter = fmt.Sprintf("(&%s(%s))", filter, i.groupFilter) + } + + return i.getEntryByDN(dn, i.getEducationClassAttrTypes(false), filter) +} + +func (i *LDAP) createEducationClassModelFromLDAP(e *ldap.Entry) *libregraph.EducationClass { + group := i.createGroupModelFromLDAP(e) + return i.groupToEducationClass(*group, e) +} + +func (i *LDAP) groupToEducationClass(group libregraph.Group, e *ldap.Entry) *libregraph.EducationClass { + class := libregraph.NewEducationClass(group.GetDisplayName(), "") + class.SetId(group.GetId()) + + if e != nil { + // Set the education User specific Attributes from the supplied LDAP Entry + if externalID := e.GetEqualFoldAttributeValue(i.educationConfig.classAttributeMap.externalID); externalID != "" { + class.SetExternalId(externalID) + } + if classification := e.GetEqualFoldAttributeValue(i.educationConfig.classAttributeMap.classification); classification != "" { + class.SetClassification(classification) + } + } + + return class +} + +func (i *LDAP) getEducationClassLDAPDN(class libregraph.EducationClass) string { + return fmt.Sprintf("ocEducationExternalId=%s,%s", oldap.EscapeDNAttributeValue(class.GetExternalId()), i.groupBaseDN) +} + +// getEducationClassByID looks up a class by id (and be ID or externalID) +func (i *LDAP) getEducationClassByID(id string, requestMembers bool) (*ldap.Entry, error) { + id = ldap.EscapeFilter(id) + filter := fmt.Sprintf("(|(%s=%s)(%s=%s))", + i.groupAttributeMap.id, id, + i.educationConfig.classAttributeMap.externalID, id) + return i.getEducationClassByFilter(filter, requestMembers) +} + +func (i *LDAP) getEducationClassByFilter(filter string, requestMembers bool) (*ldap.Entry, error) { + filter = fmt.Sprintf("(&%s(objectClass=%s)%s)", i.groupFilter, i.educationConfig.classObjectClass, filter) + return i.searchLDAPEntryByFilter(i.groupBaseDN, i.getEducationClassAttrTypes(requestMembers), filter) } diff --git a/services/graph/pkg/identity/ldap_education_class_test.go b/services/graph/pkg/identity/ldap_education_class_test.go new file mode 100644 index 0000000000..f727130215 --- /dev/null +++ b/services/graph/pkg/identity/ldap_education_class_test.go @@ -0,0 +1,302 @@ +package identity + +import ( + "context" + "errors" + "net/url" + "testing" + + "github.com/go-ldap/ldap/v3" + libregraph "github.com/owncloud/libre-graph-api-go" + "github.com/owncloud/ocis/v2/services/graph/mocks" + "github.com/test-go/testify/assert" + "github.com/test-go/testify/mock" +) + +var classEntry = ldap.NewEntry("ocEducationExternalId=Math0123", + map[string][]string{ + "cn": {"Math"}, + "ocEducationExternalId": {"Math0123"}, + "ocEducationClassType": {"course"}, + "entryUUID": {"abcd-defg"}, + }) +var classEntryWithMember = ldap.NewEntry("ocEducationExternalId=Math0123", + map[string][]string{ + "cn": {"Math"}, + "ocEducationExternalId": {"Math0123"}, + "ocEducationClassType": {"course"}, + "entryUUID": {"abcd-defg"}, + "member": {"uid=user"}, + }) + +func TestCreateEducationClass(t *testing.T) { + lm := &mocks.Client{} + lm.On("Add", mock.Anything). + Return(nil) + + lm.On("Search", mock.Anything). + Return( + &ldap.SearchResult{ + Entries: []*ldap.Entry{classEntry}, + }, + nil) + + b, err := getMockedBackend(lm, eduConfig, &logger) + assert.Nil(t, err) + assert.NotEqual(t, "", b.educationConfig.classObjectClass) + class := libregraph.NewEducationClass("Math", "course") + class.SetExternalId("Math0123") + class.SetId("abcd-defg") + res_class, err := b.CreateEducationClass(context.Background(), *class) + lm.AssertNumberOfCalls(t, "Add", 1) + lm.AssertNumberOfCalls(t, "Search", 1) + assert.Nil(t, err) + assert.NotNil(t, res_class) + assert.Equal(t, res_class.GetDisplayName(), class.GetDisplayName()) + assert.Equal(t, res_class.GetId(), class.GetId()) + assert.Equal(t, res_class.GetExternalId(), class.GetExternalId()) + assert.Equal(t, res_class.GetClassification(), class.GetClassification()) +} + +func TestGetEducationClasses(t *testing.T) { + lm := &mocks.Client{} + lm.On("Search", mock.Anything).Return(nil, ldap.NewError(ldap.LDAPResultOperationsError, errors.New("mock"))) + b, _ := getMockedBackend(lm, lconfig, &logger) + _, err := b.GetEducationClasses(context.Background(), url.Values{}) + if err == nil || err.Error() != "itemNotFound" { + t.Errorf("Expected 'itemNotFound' got '%s'", err.Error()) + } + + lm = &mocks.Client{} + lm.On("Search", mock.Anything).Return(&ldap.SearchResult{}, nil) + b, _ = getMockedBackend(lm, lconfig, &logger) + g, err := b.GetEducationClasses(context.Background(), url.Values{}) + if err != nil { + t.Errorf("Expected success, got '%s'", err.Error()) + } else if g == nil || len(g) != 0 { + t.Errorf("Expected zero length user slice") + } + + lm = &mocks.Client{} + lm.On("Search", mock.Anything).Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{classEntry}, + }, nil) + b, _ = getMockedBackend(lm, lconfig, &logger) + g, err = b.GetEducationClasses(context.Background(), url.Values{}) + if err != nil { + t.Errorf("Expected GetEducationClasses to succeed. Got %s", err.Error()) + } else if *g[0].Id != classEntry.GetEqualFoldAttributeValue(b.groupAttributeMap.id) { + t.Errorf("Expected GetEducationClasses to return a valid group") + } +} + +func TestGetEducationClass(t *testing.T) { + tests := []struct { + name string + id string + filter string + expectedItemNotFound bool + }{ + { + name: "Test search class using id", + id: "abcd-defg", + filter: "(&(objectClass=ocEducationClass)(|(entryUUID=abcd-defg)(ocEducationExternalId=abcd-defg)))", + expectedItemNotFound: false, + }, + { + name: "Test search class using unknown Id", + id: "xxxx-xxxx", + filter: "(&(objectClass=ocEducationClass)(|(entryUUID=xxxx-xxxx)(ocEducationExternalId=xxxx-xxxx)))", + expectedItemNotFound: true, + }, + { + name: "Test search class using external ID", + id: "Math0123", + filter: "(&(objectClass=ocEducationClass)(|(entryUUID=Math0123)(ocEducationExternalId=Math0123)))", + expectedItemNotFound: false, + }, + { + name: "Test search school using unknown externalID", + id: "Unknown3210", + filter: "(&(objectClass=ocEducationClass)(|(entryUUID=Unknown3210)(ocEducationExternalId=Unknown3210)))", + expectedItemNotFound: true, + }, + } + + for _, tt := range tests { + lm := &mocks.Client{} + sr := &ldap.SearchRequest{ + BaseDN: "ou=groups,dc=test", + Scope: 2, + SizeLimit: 1, + Filter: tt.filter, + Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId"}, + Controls: []ldap.Control(nil), + } + if tt.expectedItemNotFound { + lm.On("Search", sr).Return(&ldap.SearchResult{Entries: []*ldap.Entry{}}, nil) + } else { + lm.On("Search", sr).Return(&ldap.SearchResult{Entries: []*ldap.Entry{classEntry}}, nil) + } + + b, err := getMockedBackend(lm, eduConfig, &logger) + assert.Nil(t, err) + + class, err := b.GetEducationClass(context.Background(), tt.id, nil) + lm.AssertNumberOfCalls(t, "Search", 1) + + if tt.expectedItemNotFound { + assert.NotNil(t, err) + assert.Equal(t, "itemNotFound", err.Error()) + } else { + assert.Nil(t, err) + assert.Equal(t, "Math", class.GetDisplayName()) + assert.Equal(t, "abcd-defg", class.GetId()) + assert.Equal(t, "Math0123", class.GetExternalId()) + } + } +} + +func TestDeleteEducationClass(t *testing.T) { + tests := []struct { + name string + id string + filter string + expectedItemNotFound bool + }{ + { + name: "Test search class using id", + id: "abcd-defg", + filter: "(&(objectClass=ocEducationClass)(|(entryUUID=abcd-defg)(ocEducationExternalId=abcd-defg)))", + expectedItemNotFound: false, + }, + { + name: "Test search class using unknown Id", + id: "xxxx-xxxx", + filter: "(&(objectClass=ocEducationClass)(|(entryUUID=xxxx-xxxx)(ocEducationExternalId=xxxx-xxxx)))", + expectedItemNotFound: true, + }, + { + name: "Test search class using external ID", + id: "Math0123", + filter: "(&(objectClass=ocEducationClass)(|(entryUUID=Math0123)(ocEducationExternalId=Math0123)))", + expectedItemNotFound: false, + }, + { + name: "Test search school using unknown externalID", + id: "Unknown3210", + filter: "(&(objectClass=ocEducationClass)(|(entryUUID=Unknown3210)(ocEducationExternalId=Unknown3210)))", + expectedItemNotFound: true, + }, + } + + for _, tt := range tests { + lm := &mocks.Client{} + sr := &ldap.SearchRequest{ + BaseDN: "ou=groups,dc=test", + Scope: 2, + SizeLimit: 1, + Filter: tt.filter, + Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId"}, + Controls: []ldap.Control(nil), + } + if tt.expectedItemNotFound { + lm.On("Search", sr).Return(&ldap.SearchResult{Entries: []*ldap.Entry{}}, nil) + } else { + lm.On("Search", sr).Return(&ldap.SearchResult{Entries: []*ldap.Entry{classEntry}}, nil) + } + dr := &ldap.DelRequest{ + DN: "ocEducationExternalId=Math0123", + } + lm.On("Del", dr).Return(nil) + + b, err := getMockedBackend(lm, eduConfig, &logger) + assert.Nil(t, err) + + err = b.DeleteEducationClass(context.Background(), tt.id) + lm.AssertNumberOfCalls(t, "Search", 1) + + if tt.expectedItemNotFound { + lm.AssertNumberOfCalls(t, "Del", 0) + assert.NotNil(t, err) + assert.Equal(t, "itemNotFound", err.Error()) + } else { + assert.Nil(t, err) + } + } +} + +func TestGetEducationClassMembers(t *testing.T) { + tests := []struct { + name string + id string + filter string + expectedItemNotFound bool + }{ + { + name: "Test search class using id", + id: "abcd-defg", + filter: "(&(objectClass=ocEducationClass)(|(entryUUID=abcd-defg)(ocEducationExternalId=abcd-defg)))", + expectedItemNotFound: false, + }, + { + name: "Test search class using unknown Id", + id: "xxxx-xxxx", + filter: "(&(objectClass=ocEducationClass)(|(entryUUID=xxxx-xxxx)(ocEducationExternalId=xxxx-xxxx)))", + expectedItemNotFound: true, + }, + { + name: "Test search class using external ID", + id: "Math0123", + filter: "(&(objectClass=ocEducationClass)(|(entryUUID=Math0123)(ocEducationExternalId=Math0123)))", + expectedItemNotFound: false, + }, + { + name: "Test search school using unknown externalID", + id: "Unknown3210", + filter: "(&(objectClass=ocEducationClass)(|(entryUUID=Unknown3210)(ocEducationExternalId=Unknown3210)))", + expectedItemNotFound: true, + }, + } + + for _, tt := range tests { + lm := &mocks.Client{} + user_sr := &ldap.SearchRequest{ + BaseDN: "uid=user", + Scope: 0, + SizeLimit: 1, + Filter: "(objectClass=inetOrgPerson)", + Attributes: []string{"displayname", "entryUUID", "mail", "uid", "sn", "givenname"}, + Controls: []ldap.Control(nil), + } + lm.On("Search", user_sr).Return(&ldap.SearchResult{Entries: []*ldap.Entry{userEntry}}, nil) + sr := &ldap.SearchRequest{ + BaseDN: "ou=groups,dc=test", + Scope: 2, + SizeLimit: 1, + Filter: tt.filter, + Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "member"}, + Controls: []ldap.Control(nil), + } + if tt.expectedItemNotFound { + lm.On("Search", sr).Return(&ldap.SearchResult{Entries: []*ldap.Entry{}}, nil) + } else { + lm.On("Search", sr).Return(&ldap.SearchResult{Entries: []*ldap.Entry{classEntryWithMember}}, nil) + } + + b, err := getMockedBackend(lm, eduConfig, &logger) + assert.Nil(t, err) + + users, err := b.GetEducationClassMembers(context.Background(), tt.id) + + if tt.expectedItemNotFound { + lm.AssertNumberOfCalls(t, "Search", 1) + assert.NotNil(t, err) + assert.Equal(t, "itemNotFound", err.Error()) + } else { + lm.AssertNumberOfCalls(t, "Search", 2) + assert.Nil(t, err) + assert.Equal(t, len(users), 1) + } + } +} diff --git a/services/graph/pkg/identity/ldap_education_school.go b/services/graph/pkg/identity/ldap_education_school.go index a50f543fb6..77e1252ab6 100644 --- a/services/graph/pkg/identity/ldap_education_school.go +++ b/services/graph/pkg/identity/ldap_education_school.go @@ -26,6 +26,9 @@ type educationConfig struct { userObjectClass string userAttributeMap educationUserAttributeMap + + classObjectClass string + classAttributeMap educationClassAttributeMap } type schoolAttributeMap struct { @@ -52,6 +55,9 @@ func defaultEducationConfig() educationConfig { userObjectClass: "ocEducationUser", userAttributeMap: newEducationUserAttributeMap(), + + classObjectClass: "ocEducationClass", + classAttributeMap: newEducationClassAttributeMap(), } } diff --git a/services/graph/pkg/identity/ldap_group.go b/services/graph/pkg/identity/ldap_group.go index ebb428a8ab..49a0d1e639 100644 --- a/services/graph/pkg/identity/ldap_group.go +++ b/services/graph/pkg/identity/ldap_group.go @@ -22,6 +22,7 @@ type groupAttributeMap struct { memberSyntax string } +// GetGroup implements the Backend Interface for the LDAP Backend func (i *LDAP) GetGroup(ctx context.Context, nameOrID string, queryParam url.Values) (*libregraph.Group, error) { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("GetGroup") @@ -53,6 +54,7 @@ func (i *LDAP) GetGroup(ctx context.Context, nameOrID string, queryParam url.Val return g, nil } +// GetGroups implements the Backend Interface for the LDAP Backend func (i *LDAP) GetGroups(ctx context.Context, queryParam url.Values) ([]*libregraph.Group, error) { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("GetGroups") @@ -166,37 +168,12 @@ func (i *LDAP) CreateGroup(ctx context.Context, group libregraph.Group) (*libreg if !i.writeEnabled { return nil, errorcode.New(errorcode.NotAllowed, "server is configured read-only") } - ar := ldap.AddRequest{ - DN: fmt.Sprintf("cn=%s,%s", oldap.EscapeDNAttributeValue(*group.DisplayName), i.groupBaseDN), - Attributes: []ldap.Attribute{ - { - Type: i.groupAttributeMap.name, - Vals: []string{*group.DisplayName}, - }, - // This is a crutch to allow groups without members for LDAP Server's which - // that apply strict Schema checking. The RFCs define "member/uniqueMember" - // as required attribute for groupOfNames/groupOfUniqueNames. So we - // add an empty string (which is a valid DN) as the initial member. - // It will be replace once real members are added. - // We might wanna use the newer, but not so broadly used "groupOfMembers" - // objectclass (RFC2307bis-02) where "member" is optional. - { - Type: i.groupAttributeMap.member, - Vals: []string{""}, - }, - }, + ar, err := i.groupToAddRequest(group) + if err != nil { + return nil, err } - // TODO make group objectclass configurable to support e.g. posixGroup, groupOfUniqueNames, groupOfMembers?} - objectClasses := []string{"groupOfNames", "top"} - - if !i.useServerUUID { - ar.Attribute("owncloudUUID", []string{uuid.Must(uuid.NewV4()).String()}) - objectClasses = append(objectClasses, "owncloud") - } - ar.Attribute("objectClass", objectClasses) - - if err := i.conn.Add(&ar); err != nil { + if err := i.conn.Add(ar); err != nil { return nil, err } @@ -260,7 +237,7 @@ func (i *LDAP) AddMembersToGroup(ctx context.Context, groupID string, memberIDs currentSet[nCurrentMember] = struct{}{} } - var newMemberDNs []string + var newMemberDN []string for _, memberID := range memberIDs { me, err := i.getLDAPUserByID(memberID) if err != nil { @@ -272,14 +249,14 @@ func (i *LDAP) AddMembersToGroup(ctx context.Context, groupID string, memberIDs return err } if _, present := currentSet[nDN]; !present { - newMemberDNs = append(newMemberDNs, me.DN) + newMemberDN = append(newMemberDN, me.DN) } else { logger.Debug().Str("memberDN", me.DN).Msg("Member already present in group. Skipping") } } - if len(newMemberDNs) > 0 { - mr.Add(i.groupAttributeMap.member, newMemberDNs) + if len(newMemberDN) > 0 { + mr.Add(i.groupAttributeMap.member, newMemberDN) if err := i.conn.Modify(&mr); err != nil { return err @@ -304,12 +281,50 @@ 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 && mr != nil { + if mr, err := i.removeMemberFromGroupEntry(ge, me.DN); err == nil { return i.conn.Modify(mr) } return nil } +func (i *LDAP) groupToAddRequest(group libregraph.Group) (*ldap.AddRequest, error) { + ar := ldap.NewAddRequest(i.getGroupLDAPDN(group), nil) + + attrMap, err := i.groupToLDAPAttrValues(group) + if err != nil { + return nil, err + } + for attrType, values := range attrMap { + ar.Attribute(attrType, values) + } + return ar, nil +} + +func (i *LDAP) getGroupLDAPDN(group libregraph.Group) string { + return fmt.Sprintf("cn=%s,%s", oldap.EscapeDNAttributeValue(group.GetDisplayName()), i.groupBaseDN) +} + +func (i *LDAP) groupToLDAPAttrValues(group libregraph.Group) (map[string][]string, error) { + attrs := map[string][]string{ + i.groupAttributeMap.name: {group.GetDisplayName()}, + "objectClass": {"groupOfNames", "top"}, + // This is a crutch to allow groups without members for LDAP servers + // that apply strict Schema checking. The RFCs define "member/uniqueMember" + // as required attribute for groupOfNames/groupOfUniqueNames. So we + // add an empty string (which is a valid DN) as the initial member. + // It will be replaced once real members are added. + // We might wanna use the newer, but not so broadly used "groupOfMembers" + // objectclass (RFC2307bis-02) where "member" is optional. + i.groupAttributeMap.member: {""}, + } + + if !i.useServerUUID { + attrs["owncloudUUID"] = []string{uuid.Must(uuid.NewV4()).String()} + attrs["objectClass"] = append(attrs["objectClass"], "owncloud") + } + return attrs, nil +} + func (i *LDAP) expandLDAPGroupMembers(ctx context.Context, e *ldap.Entry) ([]*ldap.Entry, error) { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("expandLDAPGroupMembers") @@ -428,7 +443,7 @@ func (i *LDAP) removeMemberFromGroupEntry(group *ldap.Entry, memberDN string) (* 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, nil + return nil, ErrNotFound } mr := ldap.ModifyRequest{DN: group.DN}