From 8e1a65fc292c2f6892edf332ef1c6ca96cc419bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sw=C3=A4rd?= Date: Mon, 6 Feb 2023 12:16:04 +0100 Subject: [PATCH 1/6] graph: Add support for listing/adding/removing teachers to a class --- services/graph/pkg/identity/backend.go | 7 + services/graph/pkg/identity/err_education.go | 15 ++ .../pkg/identity/ldap_education_class.go | 175 ++++++++++++++++++ .../pkg/identity/mocks/education_backend.go | 51 +++++ .../graph/pkg/service/v0/educationclasses.go | 162 ++++++++++++++++ .../pkg/service/v0/educationclasses_test.go | 115 ++++++++++++ services/graph/pkg/service/v0/instrument.go | 15 ++ services/graph/pkg/service/v0/logging.go | 15 ++ services/graph/pkg/service/v0/service.go | 9 + services/graph/pkg/service/v0/tracing.go | 15 ++ 10 files changed, 579 insertions(+) diff --git a/services/graph/pkg/identity/backend.go b/services/graph/pkg/identity/backend.go index 42aaea0525..4d2679773f 100644 --- a/services/graph/pkg/identity/backend.go +++ b/services/graph/pkg/identity/backend.go @@ -91,6 +91,13 @@ type EducationBackend interface { GetEducationUser(ctx context.Context, nameOrID string) (*libregraph.EducationUser, error) // GetEducationUsers lists all education users GetEducationUsers(ctx context.Context) ([]*libregraph.EducationUser, error) + + // GetEducationClassTeachers returns the EducationUser teachers for an EducationClass + GetEducationClassTeachers(ctx context.Context, classID string) ([]*libregraph.EducationUser, error) + // AddTeacherToEducationclass adds a teacher (by ID) to class in the identity backend. + AddTeacherToEducationClass(ctx context.Context, classID string, teacherID string) error + // RemoveTeacherFromEducationClass removes teacher (by ID) from a class + RemoveTeacherFromEducationClass(ctx context.Context, classID string, teacherID string) error } func CreateUserModelFromCS3(u *cs3.User) *libregraph.User { diff --git a/services/graph/pkg/identity/err_education.go b/services/graph/pkg/identity/err_education.go index e68e2e030c..297ca88696 100644 --- a/services/graph/pkg/identity/err_education.go +++ b/services/graph/pkg/identity/err_education.go @@ -118,3 +118,18 @@ func (i *ErrEducationBackend) GetEducationUser(ctx context.Context, nameOrID str func (i *ErrEducationBackend) GetEducationUsers(ctx context.Context) ([]*libregraph.EducationUser, error) { return nil, errNotImplemented } + +// GetEducationClassTeachers implements the EducationBackend interface for the ErrEducationBackend backend. +func (i *ErrEducationBackend) GetEducationClassTeachers(ctx context.Context, classID string) ([]*libregraph.EducationUser, error) { + return nil, errNotImplemented +} + +// AddTeacherToEducationclass implements the EducationBackend interface for the ErrEducationBackend backend. +func (i *ErrEducationBackend) AddTeacherToEducationClass(ctx context.Context, classID string, teacherID string) error { + return errNotImplemented +} + +// RemoveTeacherFromEducationClass implements the EducationBackend interface for the ErrEducationBackend backend. +func (i *ErrEducationBackend) RemoveTeacherFromEducationClass(ctx context.Context, classID string, teacherID string) error { + return errNotImplemented +} diff --git a/services/graph/pkg/identity/ldap_education_class.go b/services/graph/pkg/identity/ldap_education_class.go index 3ea687c462..cb55178713 100644 --- a/services/graph/pkg/identity/ldap_education_class.go +++ b/services/graph/pkg/identity/ldap_education_class.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/go-ldap/ldap/v3" + "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/services/graph/pkg/service/v0/errorcode" @@ -14,12 +15,14 @@ import ( type educationClassAttributeMap struct { externalID string classification string + teachers string } func newEducationClassAttributeMap() educationClassAttributeMap { return educationClassAttributeMap{ externalID: "ocEducationExternalId", classification: "ocEducationClassType", + teachers: "ocEducationTeacherMember", } } @@ -283,6 +286,7 @@ func (i *LDAP) getEducationClassAttrTypes(requestMembers bool) []string { i.educationConfig.classAttributeMap.classification, i.educationConfig.classAttributeMap.externalID, i.educationConfig.memberOfSchoolAttribute, + i.educationConfig.classAttributeMap.teachers, } if requestMembers { attrs = append(attrs, i.groupAttributeMap.member) @@ -337,3 +341,174 @@ func (i *LDAP) getEducationClassByID(nameOrID string, requestMembers bool) (*lda i.getEducationClassAttrTypes(requestMembers), ) } + +// GetEducationClassTeachers returns the EducationUser teachers for an EducationClass +func (i *LDAP) GetEducationClassTeachers(ctx context.Context, classID string) ([]*libregraph.EducationUser, error) { + logger := i.logger.SubloggerWithRequestID(ctx) + class, err := i.getEducationClassByID(classID, false) + if err != nil { + logger.Debug().Err(err).Msg("could not get class: backend error") + return nil, err + } + + teacherEntries, err := i.expandLDAPClassTeachers(ctx, class) + result := make([]*libregraph.EducationUser, 0, len(teacherEntries)) + if err != nil { + return nil, err + } + for _, teacher := range teacherEntries { + if u := i.createEducationUserModelFromLDAP(teacher); u != nil { + result = append(result, u) + } + } + + return result, nil + +} + +func (i *LDAP) expandLDAPClassTeachers(ctx context.Context, e *ldap.Entry) ([]*ldap.Entry, error) { + logger := i.logger.SubloggerWithRequestID(ctx) + logger.Debug().Str("backend", "ldap").Msg("expandLDAPClassTeachers") + result := []*ldap.Entry{} + + for _, teacherDN := range e.GetEqualFoldAttributeValues(i.educationConfig.classAttributeMap.teachers) { + if teacherDN == "" { + continue + } + logger.Debug().Str("teacherDN", teacherDN).Msg("lookup") + ue, err := i.getUserByDN(teacherDN) + if err != nil { + // Ignore errors when reading a specific teacher fails, just log them and continue + logger.Debug().Err(err).Str("teacher", teacherDN).Msg("error reading class teacher") + continue + } + result = append(result, ue) + } + + return result, nil +} + +// AddTeacherToEducationclass adds a teacher (by ID) to class in the identity backend. +func (i *LDAP) AddTeacherToEducationClass(ctx context.Context, classID string, teacherID string) error { + logger := i.logger.SubloggerWithRequestID(ctx) + class, err := i.getEducationClassByID(classID, false) + if err != nil { + logger.Debug().Err(err).Msg("could not get class: backend error") + return err + } + + logger.Debug().Str("classDn", class.DN).Msg("got a class") + teacher, err := i.getEducationUserByNameOrID(teacherID) + + if err != nil { + logger.Debug().Err(err).Msg("could not get education user: error fetching education user from backend") + return err + } + + logger.Debug().Str("userDn", teacher.DN).Msg("got a user") + + mr := ldap.ModifyRequest{DN: class.DN} + // Handle empty teacher list + current := class.GetEqualFoldAttributeValues(i.educationConfig.classAttributeMap.teachers) + if len(current) == 1 && current[0] == "" { + mr.Delete(i.educationConfig.classAttributeMap.teachers, []string{""}) + } + + // Create a Set of current teachers + currentSet := make(map[string]struct{}, len(current)) + for _, currentTeacher := range current { + if currentTeacher == "" { + continue + } + nCurrentTeacher, err := ldapdn.ParseNormalize(currentTeacher) + if err != nil { + // Couldn't parse teacher value as a DN, skipping + logger.Warn().Str("teacherDN", currentTeacher).Err(err).Msg("Couldn't parse DN") + continue + } + currentSet[nCurrentTeacher] = struct{}{} + } + + var newTeacherDN []string + nDN, err := ldapdn.ParseNormalize(teacher.DN) + if err != nil { + logger.Error().Str("new teacher", teacher.DN).Err(err).Msg("Couldn't parse DN") + return err + } + if _, present := currentSet[nDN]; !present { + newTeacherDN = append(newTeacherDN, teacher.DN) + } else { + logger.Debug().Str("teacherDN", teacher.DN).Msg("Member already present in group. Skipping") + } + + if len(newTeacherDN) > 0 { + mr.Add(i.educationConfig.classAttributeMap.teachers, newTeacherDN) + + if err := i.conn.Modify(&mr); err != nil { + return err + } + } + + return nil +} + +// RemoveTeacherFromEducationClass removes teacher (by ID) from a class +func (i *LDAP) RemoveTeacherFromEducationClass(ctx context.Context, classID string, teacherID string) error { + logger := i.logger.SubloggerWithRequestID(ctx) + class, err := i.getEducationClassByID(classID, false) + if err != nil { + logger.Debug().Err(err).Msg("could not get class: backend error") + return err + } + + teacher, err := i.getEducationUserByNameOrID(teacherID) + if err != nil { + logger.Debug().Err(err).Msg("could not get education user: error fetching education user from backend") + return err + } + + if mr, err := i.removeTeacherFromClassEntry(class, teacher.DN); 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/mocks/education_backend.go b/services/graph/pkg/identity/mocks/education_backend.go index 42e7cbdbaf..1de863c454 100644 --- a/services/graph/pkg/identity/mocks/education_backend.go +++ b/services/graph/pkg/identity/mocks/education_backend.go @@ -29,6 +29,20 @@ func (_m *EducationBackend) AddClassesToEducationSchool(ctx context.Context, sch return r0 } +// AddTeacherToEducationClass provides a mock function with given fields: ctx, classID, teacherID +func (_m *EducationBackend) AddTeacherToEducationClass(ctx context.Context, classID string, teacherID string) error { + ret := _m.Called(ctx, classID, teacherID) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string) error); ok { + r0 = rf(ctx, classID, teacherID) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // AddUsersToEducationSchool provides a mock function with given fields: ctx, schoolID, memberID func (_m *EducationBackend) AddUsersToEducationSchool(ctx context.Context, schoolID string, memberID []string) error { ret := _m.Called(ctx, schoolID, memberID) @@ -200,6 +214,29 @@ func (_m *EducationBackend) GetEducationClassMembers(ctx context.Context, nameOr return r0, r1 } +// GetEducationClassTeachers provides a mock function with given fields: ctx, classID +func (_m *EducationBackend) GetEducationClassTeachers(ctx context.Context, classID string) ([]*libregraph.EducationUser, error) { + ret := _m.Called(ctx, classID) + + var r0 []*libregraph.EducationUser + if rf, ok := ret.Get(0).(func(context.Context, string) []*libregraph.EducationUser); ok { + r0 = rf(ctx, classID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*libregraph.EducationUser) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, classID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetEducationClasses provides a mock function with given fields: ctx func (_m *EducationBackend) GetEducationClasses(ctx context.Context) ([]*libregraph.EducationClass, error) { ret := _m.Called(ctx) @@ -375,6 +412,20 @@ func (_m *EducationBackend) RemoveClassFromEducationSchool(ctx context.Context, return r0 } +// RemoveTeacherFromEducationClass provides a mock function with given fields: ctx, classID, teacherID +func (_m *EducationBackend) RemoveTeacherFromEducationClass(ctx context.Context, classID string, teacherID string) error { + ret := _m.Called(ctx, classID, teacherID) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string) error); ok { + r0 = rf(ctx, classID, teacherID) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // RemoveUserFromEducationSchool provides a mock function with given fields: ctx, schoolID, memberID func (_m *EducationBackend) RemoveUserFromEducationSchool(ctx context.Context, schoolID string, memberID string) error { ret := _m.Called(ctx, schoolID, memberID) diff --git a/services/graph/pkg/service/v0/educationclasses.go b/services/graph/pkg/service/v0/educationclasses.go index 989a8be7f6..0591190f37 100644 --- a/services/graph/pkg/service/v0/educationclasses.go +++ b/services/graph/pkg/service/v0/educationclasses.go @@ -443,6 +443,168 @@ func (g Graph) DeleteEducationClassMember(w http.ResponseWriter, r *http.Request render.NoContent(w, r) } +// GetEducationClassTeachers implements the Service interface. +func (g Graph) GetEducationClassTeachers(w http.ResponseWriter, r *http.Request) { + logger := g.logger.SubloggerWithRequestID(r.Context()) + logger.Info().Msg("calling get class teachers") + classID := chi.URLParam(r, "classID") + classID, err := url.PathUnescape(classID) + if err != nil { + logger.Debug().Str("id", classID).Msg("could not get class teachers: unescaping class id failed") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "unescaping class id failed") + return + } + + if classID == "" { + logger.Debug().Msg("could not get class teachers: missing class id") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing class id") + return + } + + logger.Debug().Str("id", classID).Msg("calling get class teachers on backend") + teachers, err := g.identityEducationBackend.GetEducationClassTeachers(r.Context(), classID) + // teachers, err := g.identityEducationBackend.GetEducationClassMembers(r.Context(), classID) + if err != nil { + logger.Debug().Err(err).Msg("could not get class teachers: backend error") + var errcode errorcode.Error + if errors.As(err, &errcode) { + errcode.Render(w, r) + } else { + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + } + return + } + + render.Status(r, http.StatusOK) + render.JSON(w, r, teachers) +} + +// PostEducationClassTeacher implements the Service interface. +func (g Graph) PostEducationClassTeacher(w http.ResponseWriter, r *http.Request) { + logger := g.logger.SubloggerWithRequestID(r.Context()) + logger.Info().Msg("Calling post class teacher") + + classID := chi.URLParam(r, "classID") + classID, err := url.PathUnescape(classID) + if err != nil { + logger.Debug(). + Err(err). + Str("id", classID). + Msg("could not add teacher to class: unescaping class id failed") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "unescaping class id failed") + return + } + + if classID == "" { + logger.Debug().Msg("could not add class teacher: missing class id") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing class id") + return + } + memberRef := libregraph.NewMemberReference() + err = json.NewDecoder(r.Body).Decode(memberRef) + if err != nil { + logger.Debug(). + Err(err). + Interface("body", r.Body). + Msg("could not add class teacher: invalid request body") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid request body: %s", err.Error())) + return + } + memberRefURL, ok := memberRef.GetOdataIdOk() + if !ok { + logger.Debug().Msg("could not add class teacher: @odata.id reference is missing") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "@odata.id reference is missing") + return + } + memberType, id, err := g.parseMemberRef(*memberRefURL) + if err != nil { + logger.Debug().Err(err).Msg("could not add class teacher: error parsing @odata.id url") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "Error parsing @odata.id url") + return + } + // The MS Graph spec allows "directoryObject", "user", "class" and "organizational Contact" + // we restrict this to users for now. Might add EducationClass as members later + if memberType != memberTypeUsers { + logger.Debug().Str("type", memberType).Msg("could not add class member: Only users are allowed as class teachers") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "Only users are allowed as class teachers") + return + } + + logger.Debug().Str("memberType", memberType).Str("id", id).Msg("calling add teacher on backend") + err = g.identityEducationBackend.AddTeacherToEducationClass(r.Context(), classID, id) + + if err != nil { + logger.Debug().Err(err).Msg("could not add class teacher: backend error") + var errcode errorcode.Error + if errors.As(err, &errcode) { + errcode.Render(w, r) + } else { + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + } + return + } + + /* TODO requires reva changes + currentUser := revactx.ContextMustGetUser(r.Context()) + g.publishEvent(events.EducationClassTeacherAdded{Executant: currentUser.Id, EducationClassID: classID, UserID: id}) + */ + render.Status(r, http.StatusNoContent) + render.NoContent(w, r) +} + +// DeleteEducationClassTeacher implements the Service interface. +func (g Graph) DeleteEducationClassTeacher(w http.ResponseWriter, r *http.Request) { + logger := g.logger.SubloggerWithRequestID(r.Context()) + logger.Info().Msg("calling delete class teacher") + + classID := chi.URLParam(r, "classID") + classID, err := url.PathUnescape(classID) + if err != nil { + logger.Debug().Err(err).Str("id", classID).Msg("could not delete class teacher: unescaping class id failed") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "unescaping class id failed") + return + } + + if classID == "" { + logger.Debug().Msg("could not delete class teacher: missing class id") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing class id") + return + } + + teacherID := chi.URLParam(r, "teacherID") + teacherID, err = url.PathUnescape(teacherID) + if err != nil { + logger.Debug().Err(err).Str("id", teacherID).Msg("could not delete class teacher: unescaping teacher id failed") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "unescaping teacher id failed") + return + } + + if teacherID == "" { + logger.Debug().Msg("could not delete class teacher: missing teacher id") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing teacher id") + return + } + logger.Debug().Str("classID", classID).Str("teacherID", teacherID).Msg("calling delete teacher on backend") + err = g.identityEducationBackend.RemoveTeacherFromEducationClass(r.Context(), classID, teacherID) + + if err != nil { + logger.Debug().Err(err).Msg("could not delete class teacher: backend error") + var errcode errorcode.Error + if errors.As(err, &errcode) { + errcode.Render(w, r) + } else { + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + } + return + } + /* TODO requires reva changes + currentUser := revactx.ContextMustGetUser(r.Context()) + g.publishEvent(events.EducationClassTeacherRemoved{Executant: currentUser.Id, EducationClassID: classID, UserID: teacherID}) + */ + render.Status(r, http.StatusNoContent) + render.NoContent(w, r) +} + func sortClasses(req *godata.GoDataRequest, classes []*libregraph.EducationClass) ([]*libregraph.EducationClass, error) { if req.Query.OrderBy == nil || len(req.Query.OrderBy.OrderByItems) != 1 { return classes, nil diff --git a/services/graph/pkg/service/v0/educationclasses_test.go b/services/graph/pkg/service/v0/educationclasses_test.go index ba90311bd2..b8db6df100 100644 --- a/services/graph/pkg/service/v0/educationclasses_test.go +++ b/services/graph/pkg/service/v0/educationclasses_test.go @@ -530,4 +530,119 @@ var _ = Describe("EducationClass", func() { identityBackend.AssertNumberOfCalls(GinkgoT(), "RemoveMemberFromGroup", 1) }) }) + + Describe("GetEducationClassTeachers", func() { + It("gets the list of teachers", func() { + user := libregraph.NewEducationUser() + user.SetId("user") + identityEducationBackend.On("GetEducationClassTeachers", mock.Anything, mock.Anything, mock.Anything). + Return([]*libregraph.EducationUser{user}, nil) + + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/education/classes/{classID}/teachers", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("classID", *newClass.Id) + r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx)) + svc.GetEducationClassTeachers(rr, r) + Expect(rr.Code).To(Equal(http.StatusOK)) + + data, err := io.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + var teachers []*libregraph.EducationUser + err = json.Unmarshal(data, &teachers) + Expect(err).ToNot(HaveOccurred()) + + Expect(len(teachers)).To(Equal(1)) + Expect(teachers[0].GetId()).To(Equal("user")) + }) + }) + + Describe("PostEducationClassTeacher", func() { + It("fails on invalid body", func() { + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/education/classes/{classID}/teachers", bytes.NewBufferString("{invalid")) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("classID", *newClass.Id) + r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx)) + svc.PostEducationClassTeacher(rr, r) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + }) + + It("fails on missing teacher refs", func() { + member := libregraph.NewMemberReference() + data, err := json.Marshal(member) + Expect(err).ToNot(HaveOccurred()) + + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/education/classes/{classID}/teachers", bytes.NewBuffer(data)) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("classID", *newClass.Id) + r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx)) + svc.PostEducationClassTeacher(rr, r) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + }) + + It("fails on invalid teacher refs", func() { + member := libregraph.NewMemberReference() + member.SetOdataId("/invalidtype/user") + data, err := json.Marshal(member) + Expect(err).ToNot(HaveOccurred()) + + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/education/classes/{classID}/teachers", bytes.NewBuffer(data)) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("classID", *newClass.Id) + r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx)) + svc.PostEducationClassTeacher(rr, r) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + }) + + It("adds a new teacher", func() { + member := libregraph.NewMemberReference() + member.SetOdataId("/users/user") + data, err := json.Marshal(member) + Expect(err).ToNot(HaveOccurred()) + identityEducationBackend.On("AddTeacherToEducationClass", mock.Anything, mock.Anything, mock.Anything).Return(nil) + + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/education/classes/{classID}/teachers", bytes.NewBuffer(data)) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("classID", *newClass.Id) + r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx)) + svc.PostEducationClassTeacher(rr, r) + Expect(rr.Code).To(Equal(http.StatusNoContent)) + + identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "AddTeacherToEducationClass", 1) + }) + }) + + Describe("DeleteEducationClassTeacher", func() { + It("handles missing or empty teacher id", func() { + r := httptest.NewRequest(http.MethodDelete, "/graph/v1.0/education/classes/{classID}/teachers/{teacherID}/$ref", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("classID", *newClass.Id) + r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx)) + svc.DeleteEducationClassTeacher(rr, r) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + }) + + It("handles missing or empty teacher id", func() { + r := httptest.NewRequest(http.MethodDelete, "/graph/v1.0/education/classes/{classID}/teachers/{teacherID}/$ref", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("teacherID", "/users/user") + r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx)) + svc.DeleteEducationClassTeacher(rr, r) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + }) + + It("deletes teacher", func() { + identityEducationBackend.On("RemoveTeacherFromEducationClass", mock.Anything, mock.Anything, mock.Anything).Return(nil) + + r := httptest.NewRequest(http.MethodDelete, "/graph/v1.0/education/classes/{classID}/teachers/{teacherID}/$ref", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("classID", *newClass.Id) + rctx.URLParams.Add("teacherID", "/users/user1") + r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx)) + svc.DeleteEducationClassTeacher(rr, r) + Expect(rr.Code).To(Equal(http.StatusNoContent)) + + identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "RemoveTeacherFromEducationClass", 1) + }) + }) }) diff --git a/services/graph/pkg/service/v0/instrument.go b/services/graph/pkg/service/v0/instrument.go index 1defd6210b..18125ece96 100644 --- a/services/graph/pkg/service/v0/instrument.go +++ b/services/graph/pkg/service/v0/instrument.go @@ -293,3 +293,18 @@ func (i instrument) AssignTags(w http.ResponseWriter, r *http.Request) { func (i instrument) UnassignTags(w http.ResponseWriter, r *http.Request) { i.next.UnassignTags(w, r) } + +// GetEducationClassTeachers implements the Service interface. +func (i instrument) GetEducationClassTeachers(w http.ResponseWriter, r *http.Request) { + i.next.UnassignTags(w, r) +} + +// PostEducationClassTeacher implements the Service interface. +func (i instrument) PostEducationClassTeacher(w http.ResponseWriter, r *http.Request) { + i.next.UnassignTags(w, r) +} + +// DeleteEducationClassTeacher implements the Service interface. +func (i instrument) DeleteEducationClassTeacher(w http.ResponseWriter, r *http.Request) { + i.next.UnassignTags(w, r) +} diff --git a/services/graph/pkg/service/v0/logging.go b/services/graph/pkg/service/v0/logging.go index 818d1d2e03..82156c9019 100644 --- a/services/graph/pkg/service/v0/logging.go +++ b/services/graph/pkg/service/v0/logging.go @@ -293,3 +293,18 @@ func (l logging) AssignTags(w http.ResponseWriter, r *http.Request) { func (l logging) UnassignTags(w http.ResponseWriter, r *http.Request) { l.next.UnassignTags(w, r) } + +// GetEducationClassTeachers implements the Service interface. +func (l logging) GetEducationClassTeachers(w http.ResponseWriter, r *http.Request) { + l.next.UnassignTags(w, r) +} + +// PostEducationClassTeacher implements the Service interface. +func (l logging) PostEducationClassTeacher(w http.ResponseWriter, r *http.Request) { + l.next.UnassignTags(w, r) +} + +// DeleteEducationClassTeacher implements the Service interface. +func (l logging) DeleteEducationClassTeacher(w http.ResponseWriter, r *http.Request) { + l.next.UnassignTags(w, r) +} diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index 5de44badb3..0d4cb643ca 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -84,6 +84,10 @@ type Service interface { PatchEducationUser(http.ResponseWriter, *http.Request) DeleteEducationClassMember(w http.ResponseWriter, r *http.Request) + GetEducationClassTeachers(w http.ResponseWriter, r *http.Request) + PostEducationClassTeacher(w http.ResponseWriter, r *http.Request) + DeleteEducationClassTeacher(w http.ResponseWriter, r *http.Request) + GetDrives(w http.ResponseWriter, r *http.Request) GetSingleDrive(w http.ResponseWriter, r *http.Request) GetAllDrives(w http.ResponseWriter, r *http.Request) @@ -270,6 +274,11 @@ func NewService(opts ...Option) (Graph, error) { r.Post("/$ref", svc.PostEducationClassMember) r.Delete("/{memberID}/$ref", svc.DeleteEducationClassMember) }) + r.Route("/teachers", func(r chi.Router) { + r.Get("/", svc.GetEducationClassTeachers) + r.Post("/$ref", svc.PostEducationClassTeacher) + r.Delete("/{teacherID}/$ref", svc.DeleteEducationClassTeacher) + }) }) }) }) diff --git a/services/graph/pkg/service/v0/tracing.go b/services/graph/pkg/service/v0/tracing.go index 10505a5b43..ad4a6b5c7d 100644 --- a/services/graph/pkg/service/v0/tracing.go +++ b/services/graph/pkg/service/v0/tracing.go @@ -289,3 +289,18 @@ func (t tracing) AssignTags(w http.ResponseWriter, r *http.Request) { func (t tracing) UnassignTags(w http.ResponseWriter, r *http.Request) { t.next.UnassignTags(w, r) } + +// GetEducationClassTeachers implements the Service interface. +func (t tracing) GetEducationClassTeachers(w http.ResponseWriter, r *http.Request) { + t.next.UnassignTags(w, r) +} + +// PostEducationClassTeacher implements the Service interface. +func (t tracing) PostEducationClassTeacher(w http.ResponseWriter, r *http.Request) { + t.next.UnassignTags(w, r) +} + +// DeleteEducationClassTeacher implements the Service interface. +func (t tracing) DeleteEducationClassTeacher(w http.ResponseWriter, r *http.Request) { + t.next.UnassignTags(w, r) +} 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 2/6] 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) From 4c1325f02ad11bfc2217e905a709702b95f93bf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sw=C3=A4rd?= Date: Thu, 9 Feb 2023 11:56:10 +0100 Subject: [PATCH 3/6] Remove duplication and make method generic. --- services/graph/pkg/identity/backend.go | 2 ++ services/graph/pkg/identity/cs3.go | 5 ++++ services/graph/pkg/identity/ldap.go | 23 +++++++++++++++ .../pkg/identity/ldap_education_class.go | 26 ++--------------- services/graph/pkg/identity/ldap_group.go | 28 ++----------------- services/graph/pkg/identity/mocks/backend.go | 23 +++++++++++++++ 6 files changed, 58 insertions(+), 49 deletions(-) diff --git a/services/graph/pkg/identity/backend.go b/services/graph/pkg/identity/backend.go index 2a2ccddb4a..d8a295d214 100644 --- a/services/graph/pkg/identity/backend.go +++ b/services/graph/pkg/identity/backend.go @@ -43,6 +43,8 @@ type Backend interface { 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) + // ExpandLDAPAttributeEntries reads an attribute from an ldap entry and expands to users + ExpandLDAPAttributeEntries(ctx context.Context, e *ldap.Entry, attribute string) ([]*ldap.Entry, 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 e963b2fba2..b6d9f3b9b9 100644 --- a/services/graph/pkg/identity/cs3.go +++ b/services/graph/pkg/identity/cs3.go @@ -213,6 +213,11 @@ func (i *CS3) RemoveEntryByDNAndAttributeFromEntry(entry *ldap.Entry, dn string, return nil, errorcode.New(errorcode.NotSupported, "not implemented") } +// ExpandLDAPAttributeEntries implements the Backend Interface. It's currently not supported for the CS3 backend +func (i *CS3) ExpandLDAPAttributeEntries(ctx context.Context, e *ldap.Entry, attribute string) ([]*ldap.Entry, 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 8913e6299e..a18d7a55f1 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -646,3 +646,26 @@ func (i *LDAP) RemoveEntryByDNAndAttributeFromEntry(entry *ldap.Entry, dn string mr.Delete(attribute, []string{dn}) return &mr, nil } + +// ExpandLDAPAttributeEntries reads an attribute from an ldap entry and expands to users +func (i *LDAP) ExpandLDAPAttributeEntries(ctx context.Context, e *ldap.Entry, attribute string) ([]*ldap.Entry, error) { + logger := i.logger.SubloggerWithRequestID(ctx) + logger.Debug().Str("backend", "ldap").Msg("ExpandLDAPAttributeEntries") + result := []*ldap.Entry{} + + for _, entryDN := range e.GetEqualFoldAttributeValues(attribute) { + if entryDN == "" { + continue + } + logger.Debug().Str("entryDN", entryDN).Msg("lookup") + ue, err := i.getUserByDN(entryDN) + if err != nil { + // Ignore errors when reading a specific entry fails, just log them and continue + logger.Debug().Err(err).Str("entry", entryDN).Msg("error reading attribute member entry") + continue + } + result = append(result, ue) + } + + return result, nil +} diff --git a/services/graph/pkg/identity/ldap_education_class.go b/services/graph/pkg/identity/ldap_education_class.go index 8e4bad7f79..677c2ff70a 100644 --- a/services/graph/pkg/identity/ldap_education_class.go +++ b/services/graph/pkg/identity/ldap_education_class.go @@ -228,7 +228,7 @@ func (i *LDAP) GetEducationClassMembers(ctx context.Context, id string) ([]*libr return nil, err } - memberEntries, err := i.expandLDAPGroupMembers(ctx, e) + memberEntries, err := i.ExpandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member) result := make([]*libregraph.EducationUser, 0, len(memberEntries)) if err != nil { return nil, err @@ -351,7 +351,7 @@ func (i *LDAP) GetEducationClassTeachers(ctx context.Context, classID string) ([ return nil, err } - teacherEntries, err := i.expandLDAPClassTeachers(ctx, class) + teacherEntries, err := i.ExpandLDAPAttributeEntries(ctx, class, i.educationConfig.classAttributeMap.teachers) result := make([]*libregraph.EducationUser, 0, len(teacherEntries)) if err != nil { return nil, err @@ -366,28 +366,6 @@ func (i *LDAP) GetEducationClassTeachers(ctx context.Context, classID string) ([ } -func (i *LDAP) expandLDAPClassTeachers(ctx context.Context, e *ldap.Entry) ([]*ldap.Entry, error) { - logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("expandLDAPClassTeachers") - result := []*ldap.Entry{} - - for _, teacherDN := range e.GetEqualFoldAttributeValues(i.educationConfig.classAttributeMap.teachers) { - if teacherDN == "" { - continue - } - logger.Debug().Str("teacherDN", teacherDN).Msg("lookup") - ue, err := i.getUserByDN(teacherDN) - if err != nil { - // Ignore errors when reading a specific teacher fails, just log them and continue - logger.Debug().Err(err).Str("teacher", teacherDN).Msg("error reading class teacher") - continue - } - result = append(result, ue) - } - - return result, nil -} - // AddTeacherToEducationclass adds a teacher (by ID) to class in the identity backend. func (i *LDAP) AddTeacherToEducationClass(ctx context.Context, classID string, teacherID string) error { logger := i.logger.SubloggerWithRequestID(ctx) diff --git a/services/graph/pkg/identity/ldap_group.go b/services/graph/pkg/identity/ldap_group.go index b572ddd93d..65e2986617 100644 --- a/services/graph/pkg/identity/ldap_group.go +++ b/services/graph/pkg/identity/ldap_group.go @@ -37,7 +37,7 @@ func (i *LDAP) GetGroup(ctx context.Context, nameOrID string, queryParam url.Val return nil, errorcode.New(errorcode.ItemNotFound, "not found") } if slices.Contains(sel, "members") || slices.Contains(exp, "members") { - members, err := i.expandLDAPGroupMembers(ctx, e) + members, err := i.ExpandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member) if err != nil { return nil, err } @@ -115,7 +115,7 @@ func (i *LDAP) GetGroups(ctx context.Context, queryParam url.Values) ([]*libregr continue } if expandMembers { - members, err := i.expandLDAPGroupMembers(ctx, e) + members, err := i.ExpandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member) if err != nil { return nil, err } @@ -142,7 +142,7 @@ func (i *LDAP) GetGroupMembers(ctx context.Context, groupID string) ([]*libregra return nil, err } - memberEntries, err := i.expandLDAPGroupMembers(ctx, e) + memberEntries, err := i.ExpandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member) result := make([]*libregraph.User, 0, len(memberEntries)) if err != nil { return nil, err @@ -323,28 +323,6 @@ func (i *LDAP) groupToLDAPAttrValues(group libregraph.Group) (map[string][]strin 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") - result := []*ldap.Entry{} - - for _, memberDN := range e.GetEqualFoldAttributeValues(i.groupAttributeMap.member) { - if memberDN == "" { - continue - } - logger.Debug().Str("memberDN", memberDN).Msg("lookup") - ue, err := i.getUserByDN(memberDN) - if err != nil { - // Ignore errors when reading a specific member fails, just log them and continue - logger.Debug().Err(err).Str("member", memberDN).Msg("error reading group member") - continue - } - result = append(result, ue) - } - - return result, nil -} - func (i *LDAP) getLDAPGroupByID(id string, requestMembers bool) (*ldap.Entry, error) { id = ldap.EscapeFilter(id) filter := fmt.Sprintf("(%s=%s)", i.groupAttributeMap.id, id) diff --git a/services/graph/pkg/identity/mocks/backend.go b/services/graph/pkg/identity/mocks/backend.go index 997b5a04fe..595a3db85b 100644 --- a/services/graph/pkg/identity/mocks/backend.go +++ b/services/graph/pkg/identity/mocks/backend.go @@ -109,6 +109,29 @@ func (_m *Backend) DeleteUser(ctx context.Context, nameOrID string) error { return r0 } +// ExpandLDAPAttributeEntries provides a mock function with given fields: ctx, e, attribute +func (_m *Backend) ExpandLDAPAttributeEntries(ctx context.Context, e *ldap.Entry, attribute string) ([]*ldap.Entry, error) { + ret := _m.Called(ctx, e, attribute) + + var r0 []*ldap.Entry + if rf, ok := ret.Get(0).(func(context.Context, *ldap.Entry, string) []*ldap.Entry); ok { + r0 = rf(ctx, e, attribute) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*ldap.Entry) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *ldap.Entry, string) error); ok { + r1 = rf(ctx, e, attribute) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetGroup provides a mock function with given fields: ctx, nameOrID, queryParam func (_m *Backend) GetGroup(ctx context.Context, nameOrID string, queryParam url.Values) (*libregraph.Group, error) { ret := _m.Called(ctx, nameOrID, queryParam) From a21f485d2cd7a4c29aaa45302173b11d3932508d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sw=C3=A4rd?= Date: Fri, 10 Feb 2023 11:21:55 +0100 Subject: [PATCH 4/6] Fix forgotten expected return values in tests. --- services/graph/pkg/identity/ldap_education_class_test.go | 6 +++--- services/graph/pkg/identity/ldap_education_school_test.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/services/graph/pkg/identity/ldap_education_class_test.go b/services/graph/pkg/identity/ldap_education_class_test.go index 21594310bf..df48c36e85 100644 --- a/services/graph/pkg/identity/ldap_education_class_test.go +++ b/services/graph/pkg/identity/ldap_education_class_test.go @@ -139,7 +139,7 @@ func TestGetEducationClass(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: tt.filter, - Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "ocMemberOfSchool"}, + Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "ocMemberOfSchool", "ocEducationTeacherMember"}, Controls: []ldap.Control(nil), } if tt.expectedItemNotFound { @@ -206,7 +206,7 @@ func TestDeleteEducationClass(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: tt.filter, - Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "ocMemberOfSchool"}, + Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "ocMemberOfSchool", "ocEducationTeacherMember"}, Controls: []ldap.Control(nil), } if tt.expectedItemNotFound { @@ -284,7 +284,7 @@ func TestGetEducationClassMembers(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: tt.filter, - Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "ocMemberOfSchool", "member"}, + Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "ocMemberOfSchool", "ocEducationTeacherMember", "member"}, Controls: []ldap.Control(nil), } if tt.expectedItemNotFound { diff --git a/services/graph/pkg/identity/ldap_education_school_test.go b/services/graph/pkg/identity/ldap_education_school_test.go index 47cc36437b..a26982e518 100644 --- a/services/graph/pkg/identity/ldap_education_school_test.go +++ b/services/graph/pkg/identity/ldap_education_school_test.go @@ -461,7 +461,7 @@ var classesBySchoolIDSearch *ldap.SearchRequest = &ldap.SearchRequest{ Scope: 2, SizeLimit: 0, Filter: "(&(objectClass=ocEducationClass)(ocMemberOfSchool=abcd-defg))", - Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "ocMemberOfSchool"}, + Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "ocMemberOfSchool", "ocEducationTeacherMember"}, Controls: []ldap.Control(nil), } @@ -484,7 +484,7 @@ var classesByUUIDSearchNotFound *ldap.SearchRequest = &ldap.SearchRequest{ Scope: 2, SizeLimit: 1, Filter: "(&(objectClass=ocEducationClass)(|(entryUUID=does-not-exist)(ocEducationExternalId=does-not-exist)))", - Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "ocMemberOfSchool"}, + Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "ocMemberOfSchool", "ocEducationTeacherMember"}, Controls: []ldap.Control(nil), } @@ -493,7 +493,7 @@ var classesByUUIDSearchFound *ldap.SearchRequest = &ldap.SearchRequest{ Scope: 2, SizeLimit: 1, Filter: "(&(objectClass=ocEducationClass)(|(entryUUID=abcd-defg)(ocEducationExternalId=abcd-defg)))", - Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "ocMemberOfSchool"}, + Attributes: []string{"cn", "entryUUID", "ocEducationClassType", "ocEducationExternalId", "ocMemberOfSchool", "ocEducationTeacherMember"}, Controls: []ldap.Control(nil), } From 82e312ac2fe64985f57c471029b56d14dddd5799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sw=C3=A4rd?= Date: Fri, 10 Feb 2023 11:50:46 +0100 Subject: [PATCH 5/6] Fix sonarcloud comment nitpick. --- services/graph/pkg/identity/err_education.go | 2 +- services/graph/pkg/identity/ldap_education_class.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/graph/pkg/identity/err_education.go b/services/graph/pkg/identity/err_education.go index 297ca88696..b1ed28da04 100644 --- a/services/graph/pkg/identity/err_education.go +++ b/services/graph/pkg/identity/err_education.go @@ -124,7 +124,7 @@ func (i *ErrEducationBackend) GetEducationClassTeachers(ctx context.Context, cla return nil, errNotImplemented } -// AddTeacherToEducationclass implements the EducationBackend interface for the ErrEducationBackend backend. +// AddTeacherToEducationClass implements the EducationBackend interface for the ErrEducationBackend backend. func (i *ErrEducationBackend) AddTeacherToEducationClass(ctx context.Context, classID string, teacherID string) error { return errNotImplemented } diff --git a/services/graph/pkg/identity/ldap_education_class.go b/services/graph/pkg/identity/ldap_education_class.go index 677c2ff70a..5288de4d18 100644 --- a/services/graph/pkg/identity/ldap_education_class.go +++ b/services/graph/pkg/identity/ldap_education_class.go @@ -366,7 +366,7 @@ func (i *LDAP) GetEducationClassTeachers(ctx context.Context, classID string) ([ } -// AddTeacherToEducationclass adds a teacher (by ID) to class in the identity backend. +// AddTeacherToEducationClass adds a teacher (by ID) to class in the identity backend. func (i *LDAP) AddTeacherToEducationClass(ctx context.Context, classID string, teacherID string) error { logger := i.logger.SubloggerWithRequestID(ctx) class, err := i.getEducationClassByID(classID, false) From 2338515444883931f2e51bd189bf69609c28a66e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sw=C3=A4rd?= Date: Fri, 10 Feb 2023 15:12:19 +0100 Subject: [PATCH 6/6] Make ldap functions package local and remove a superfluous comment. --- services/graph/pkg/identity/backend.go | 5 -- services/graph/pkg/identity/cs3.go | 11 ----- services/graph/pkg/identity/ldap.go | 10 ++-- .../pkg/identity/ldap_education_class.go | 6 +-- services/graph/pkg/identity/ldap_group.go | 8 ++-- services/graph/pkg/identity/mocks/backend.go | 48 ------------------- .../graph/pkg/service/v0/educationclasses.go | 1 - 7 files changed, 12 insertions(+), 77 deletions(-) diff --git a/services/graph/pkg/identity/backend.go b/services/graph/pkg/identity/backend.go index d8a295d214..4d2679773f 100644 --- a/services/graph/pkg/identity/backend.go +++ b/services/graph/pkg/identity/backend.go @@ -6,7 +6,6 @@ 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" ) @@ -41,10 +40,6 @@ 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) - // ExpandLDAPAttributeEntries reads an attribute from an ldap entry and expands to users - ExpandLDAPAttributeEntries(ctx context.Context, e *ldap.Entry, attribute string) ([]*ldap.Entry, 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 b6d9f3b9b9..d6eab4029d 100644 --- a/services/graph/pkg/identity/cs3.go +++ b/services/graph/pkg/identity/cs3.go @@ -9,7 +9,6 @@ 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" @@ -208,16 +207,6 @@ 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") -} - -// ExpandLDAPAttributeEntries implements the Backend Interface. It's currently not supported for the CS3 backend -func (i *CS3) ExpandLDAPAttributeEntries(ctx context.Context, e *ldap.Entry, attribute string) ([]*ldap.Entry, 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 a18d7a55f1..867750e5a7 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.RemoveEntryByDNAndAttributeFromEntry(group, e.DN, i.groupAttributeMap.member); 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 @@ -610,8 +610,8 @@ 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) { +// 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 @@ -647,8 +647,8 @@ func (i *LDAP) RemoveEntryByDNAndAttributeFromEntry(entry *ldap.Entry, dn string return &mr, nil } -// ExpandLDAPAttributeEntries reads an attribute from an ldap entry and expands to users -func (i *LDAP) ExpandLDAPAttributeEntries(ctx context.Context, e *ldap.Entry, attribute string) ([]*ldap.Entry, error) { +// expandLDAPAttributeEntries reads an attribute from an ldap entry and expands to users +func (i *LDAP) expandLDAPAttributeEntries(ctx context.Context, e *ldap.Entry, attribute string) ([]*ldap.Entry, error) { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("ExpandLDAPAttributeEntries") result := []*ldap.Entry{} diff --git a/services/graph/pkg/identity/ldap_education_class.go b/services/graph/pkg/identity/ldap_education_class.go index 5288de4d18..b2a34a5018 100644 --- a/services/graph/pkg/identity/ldap_education_class.go +++ b/services/graph/pkg/identity/ldap_education_class.go @@ -228,7 +228,7 @@ func (i *LDAP) GetEducationClassMembers(ctx context.Context, id string) ([]*libr return nil, err } - memberEntries, err := i.ExpandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member) + memberEntries, err := i.expandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member) result := make([]*libregraph.EducationUser, 0, len(memberEntries)) if err != nil { return nil, err @@ -351,7 +351,7 @@ func (i *LDAP) GetEducationClassTeachers(ctx context.Context, classID string) ([ return nil, err } - teacherEntries, err := i.ExpandLDAPAttributeEntries(ctx, class, i.educationConfig.classAttributeMap.teachers) + teacherEntries, err := i.expandLDAPAttributeEntries(ctx, class, i.educationConfig.classAttributeMap.teachers) result := make([]*libregraph.EducationUser, 0, len(teacherEntries)) if err != nil { return nil, err @@ -445,7 +445,7 @@ func (i *LDAP) RemoveTeacherFromEducationClass(ctx context.Context, classID stri return err } - if mr, err := i.RemoveEntryByDNAndAttributeFromEntry(class, teacher.DN, i.educationConfig.classAttributeMap.teachers); err == nil { + if mr, err := i.removeEntryByDNAndAttributeFromEntry(class, teacher.DN, i.educationConfig.classAttributeMap.teachers); err == nil { return i.conn.Modify(mr) } diff --git a/services/graph/pkg/identity/ldap_group.go b/services/graph/pkg/identity/ldap_group.go index 65e2986617..e82e8d5682 100644 --- a/services/graph/pkg/identity/ldap_group.go +++ b/services/graph/pkg/identity/ldap_group.go @@ -37,7 +37,7 @@ func (i *LDAP) GetGroup(ctx context.Context, nameOrID string, queryParam url.Val return nil, errorcode.New(errorcode.ItemNotFound, "not found") } if slices.Contains(sel, "members") || slices.Contains(exp, "members") { - members, err := i.ExpandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member) + members, err := i.expandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member) if err != nil { return nil, err } @@ -115,7 +115,7 @@ func (i *LDAP) GetGroups(ctx context.Context, queryParam url.Values) ([]*libregr continue } if expandMembers { - members, err := i.ExpandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member) + members, err := i.expandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member) if err != nil { return nil, err } @@ -142,7 +142,7 @@ func (i *LDAP) GetGroupMembers(ctx context.Context, groupID string) ([]*libregra return nil, err } - memberEntries, err := i.ExpandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member) + memberEntries, err := i.expandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member) result := make([]*libregraph.User, 0, len(memberEntries)) if err != nil { return nil, err @@ -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.RemoveEntryByDNAndAttributeFromEntry(ge, me.DN, i.groupAttributeMap.member); err == nil { + if mr, err := i.removeEntryByDNAndAttributeFromEntry(ge, me.DN, i.groupAttributeMap.member); err == nil { return i.conn.Modify(mr) } return nil diff --git a/services/graph/pkg/identity/mocks/backend.go b/services/graph/pkg/identity/mocks/backend.go index 595a3db85b..9fcdcd8c19 100644 --- a/services/graph/pkg/identity/mocks/backend.go +++ b/services/graph/pkg/identity/mocks/backend.go @@ -7,8 +7,6 @@ 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" @@ -109,29 +107,6 @@ func (_m *Backend) DeleteUser(ctx context.Context, nameOrID string) error { return r0 } -// ExpandLDAPAttributeEntries provides a mock function with given fields: ctx, e, attribute -func (_m *Backend) ExpandLDAPAttributeEntries(ctx context.Context, e *ldap.Entry, attribute string) ([]*ldap.Entry, error) { - ret := _m.Called(ctx, e, attribute) - - var r0 []*ldap.Entry - if rf, ok := ret.Get(0).(func(context.Context, *ldap.Entry, string) []*ldap.Entry); ok { - r0 = rf(ctx, e, attribute) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]*ldap.Entry) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func(context.Context, *ldap.Entry, string) error); ok { - r1 = rf(ctx, e, attribute) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // GetGroup provides a mock function with given fields: ctx, nameOrID, queryParam func (_m *Backend) GetGroup(ctx context.Context, nameOrID string, queryParam url.Values) (*libregraph.Group, error) { ret := _m.Called(ctx, nameOrID, queryParam) @@ -247,29 +222,6 @@ 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) diff --git a/services/graph/pkg/service/v0/educationclasses.go b/services/graph/pkg/service/v0/educationclasses.go index 0591190f37..33abbbaf05 100644 --- a/services/graph/pkg/service/v0/educationclasses.go +++ b/services/graph/pkg/service/v0/educationclasses.go @@ -463,7 +463,6 @@ func (g Graph) GetEducationClassTeachers(w http.ResponseWriter, r *http.Request) logger.Debug().Str("id", classID).Msg("calling get class teachers on backend") teachers, err := g.identityEducationBackend.GetEducationClassTeachers(r.Context(), classID) - // teachers, err := g.identityEducationBackend.GetEducationClassMembers(r.Context(), classID) if err != nil { logger.Debug().Err(err).Msg("could not get class teachers: backend error") var errcode errorcode.Error