From 020a37b017164dcd7a44929414174922f8287148 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 4 Mar 2026 13:47:54 +0100 Subject: [PATCH] feat(graph): replace externalId school lookup with OData $filter support Remove the ability to look up schools by externalId directly (from LDAP filters, duplicate checks, and the EducationBackend interface). This approach was somewhat unclean, we shouldn't add more an more attributes as keys for direct lookup. Instead, expose externalId filtering via the OData $filter query parameter on GET /education/schools, following the same pattern as for education users. Related: #1598 --- services/graph/pkg/identity/backend.go | 2 + services/graph/pkg/identity/err_education.go | 5 + .../pkg/identity/ldap_education_school.go | 127 +++++++++++------- .../identity/ldap_education_school_test.go | 26 ++-- .../pkg/identity/mocks/education_backend.go | 74 ++++++++++ .../graph/pkg/service/v0/educationschools.go | 35 ++++- .../pkg/service/v0/educationschools_test.go | 13 ++ 7 files changed, 212 insertions(+), 70 deletions(-) diff --git a/services/graph/pkg/identity/backend.go b/services/graph/pkg/identity/backend.go index dc1f2b1121..342af7ade9 100644 --- a/services/graph/pkg/identity/backend.go +++ b/services/graph/pkg/identity/backend.go @@ -68,6 +68,8 @@ type EducationBackend interface { GetEducationSchool(ctx context.Context, nameOrID string) (*libregraph.EducationSchool, error) // GetEducationSchools lists all schools GetEducationSchools(ctx context.Context) ([]*libregraph.EducationSchool, error) + // FilterEducationSchoolsByAttribute list all schools where an attribute matches a value, e.g. all schools with a given externalId + FilterEducationSchoolsByAttribute(ctx context.Context, attr, value string) ([]*libregraph.EducationSchool, error) // UpdateEducationSchool updates attributes of a school UpdateEducationSchool(ctx context.Context, numberOrID string, school libregraph.EducationSchool) (*libregraph.EducationSchool, error) // GetEducationSchoolUsers lists all members of a school diff --git a/services/graph/pkg/identity/err_education.go b/services/graph/pkg/identity/err_education.go index 8c0e048534..4138299035 100644 --- a/services/graph/pkg/identity/err_education.go +++ b/services/graph/pkg/identity/err_education.go @@ -29,6 +29,11 @@ func (i *ErrEducationBackend) GetEducationSchools(ctx context.Context) ([]*libre return nil, errNotImplemented } +// FilterEducationSchoolsByAttribute implements the EducationBackend interface for the ErrEducationBackend backend. +func (i *ErrEducationBackend) FilterEducationSchoolsByAttribute(ctx context.Context, attr, value string) ([]*libregraph.EducationSchool, error) { + return nil, errNotImplemented +} + // UpdateEducationSchool implements the EducationBackend interface for the ErrEducationBackend backend. func (i *ErrEducationBackend) UpdateEducationSchool(ctx context.Context, numberOrID string, school libregraph.EducationSchool) (*libregraph.EducationSchool, error) { return nil, errNotImplemented diff --git a/services/graph/pkg/identity/ldap_education_school.go b/services/graph/pkg/identity/ldap_education_school.go index 2aecc3d9bb..68f2b4225b 100644 --- a/services/graph/pkg/identity/ldap_education_school.go +++ b/services/graph/pkg/identity/ldap_education_school.go @@ -49,10 +49,9 @@ const ( ) var ( - errNotSet = errors.New("attribute not set") - errSchoolNameExists = errorcode.New(errorcode.NameAlreadyExists, "A school with that name is already present") - errSchoolNumberExists = errorcode.New(errorcode.NameAlreadyExists, "A school with that number is already present") - errSchoolExternalIdExists = errorcode.New(errorcode.NameAlreadyExists, "A school with that external id is already present") + errNotSet = errors.New("attribute not set") + errSchoolNameExists = errorcode.New(errorcode.NameAlreadyExists, "A school with that name is already present") + errSchoolNumberExists = errorcode.New(errorcode.NameAlreadyExists, "A school with that number is already present") ) func defaultEducationConfig() educationConfig { @@ -136,21 +135,6 @@ func (i *LDAP) CreateEducationSchool(ctx context.Context, school libregraph.Educ } } - // Check that the school external id is not already used - if school.HasExternalId() { - _, err := i.getSchoolByExternalId(school.GetExternalId()) - switch { - case err == nil: - logger.Debug().Err(errSchoolExternalIdExists).Str("externalId", school.GetExternalId()).Msg("duplicate school external id") - return nil, errSchoolExternalIdExists - case errors.Is(err, ErrNotFound): - break - default: - logger.Error().Err(err).Str("externalId", school.GetExternalId()).Msg("error looking up school by external id") - return nil, errorcode.New(errorcode.GeneralException, "error looking up school by external id") - } - } - attributeTypeAndValue := ldap.AttributeTypeAndValue{ Type: i.educationConfig.schoolAttributeMap.displayName, Value: school.GetDisplayName(), @@ -299,14 +283,14 @@ func (i *LDAP) updateSchoolProperties(ctx context.Context, dn string, currentSch } // UpdateEducationSchool updates the supplied school in the identity backend -func (i *LDAP) UpdateEducationSchool(ctx context.Context, numberOrIDOrExternalID string, school libregraph.EducationSchool) (*libregraph.EducationSchool, error) { +func (i *LDAP) UpdateEducationSchool(ctx context.Context, numberOrID string, school libregraph.EducationSchool) (*libregraph.EducationSchool, error) { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("UpdateEducationSchool") if !i.writeEnabled { return nil, ErrReadOnly } - e, err := i.getSchoolByNumberOrIDOrExternalID(numberOrIDOrExternalID) + e, err := i.getSchoolByNumberOrID(numberOrID) if err != nil { return nil, err } @@ -329,7 +313,7 @@ func (i *LDAP) UpdateEducationSchool(ctx context.Context, numberOrIDOrExternalID } // Read back school from LDAP - e, err = i.getSchoolByNumberOrIDOrExternalID(i.getID(e)) + e, err = i.getSchoolByNumberOrID(i.getID(e)) if err != nil { return nil, err } @@ -343,7 +327,7 @@ func (i *LDAP) DeleteEducationSchool(ctx context.Context, id string) error { if !i.writeEnabled { return ErrReadOnly } - e, err := i.getSchoolByNumberOrIDOrExternalID(id) + e, err := i.getSchoolByNumberOrID(id) if err != nil { return err } @@ -358,10 +342,10 @@ func (i *LDAP) DeleteEducationSchool(ctx context.Context, id string) error { } // GetEducationSchool implements the EducationBackend interface for the LDAP backend. -func (i *LDAP) GetEducationSchool(ctx context.Context, numberOrIDOrExternalID string) (*libregraph.EducationSchool, error) { +func (i *LDAP) GetEducationSchool(ctx context.Context, numberOrID string) (*libregraph.EducationSchool, error) { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("GetEducationSchool") - e, err := i.getSchoolByNumberOrIDOrExternalID(numberOrIDOrExternalID) + e, err := i.getSchoolByNumberOrID(numberOrID) if err != nil { return nil, err } @@ -410,6 +394,56 @@ func (i *LDAP) GetEducationSchools(ctx context.Context) ([]*libregraph.Education return schools, nil } +// FilterEducationSchoolsByAttribute implements the EducationBackend interface for the LDAP backend. +func (i *LDAP) FilterEducationSchoolsByAttribute(ctx context.Context, attr, value string) ([]*libregraph.EducationSchool, error) { + logger := i.logger.SubloggerWithRequestID(ctx).With().Str("func", "FilterEducationSchoolsByAttribute").Logger() + logger.Debug().Str("backend", "ldap").Str("attribute", attr).Str("value", value).Msg("") + + var ldapAttr string + switch attr { + case "externalId": + ldapAttr = i.educationConfig.schoolAttributeMap.externalId + default: + return nil, errorcode.New(errorcode.InvalidRequest, fmt.Sprintf("filtering by attribute '%s' is not supported", attr)) + } + filter := fmt.Sprintf("(&%s(objectClass=%s)(%s=%s))", + i.educationConfig.schoolFilter, + i.educationConfig.schoolObjectClass, + ldap.EscapeFilter(ldapAttr), + ldap.EscapeFilter(value), + ) + + searchRequest := ldap.NewSearchRequest( + i.educationConfig.schoolBaseDN, + i.educationConfig.schoolScope, + ldap.NeverDerefAliases, 0, 0, false, + filter, + i.getEducationSchoolAttrTypes(), + nil, + ) + logger.Debug().Str("base", searchRequest.BaseDN). + Str("filter", searchRequest.Filter). + Int("scope", searchRequest.Scope). + Int("sizelimit", searchRequest.SizeLimit). + Interface("attributes", searchRequest.Attributes). + Msg("LDAP Search Request") + + res, err := i.conn.Search(searchRequest) + if err != nil { + return nil, errorcode.New(errorcode.ItemNotFound, err.Error()) + } + + schools := make([]*libregraph.EducationSchool, 0, len(res.Entries)) + for _, e := range res.Entries { + school := i.createSchoolModelFromLDAP(e) + if school == nil { + continue + } + schools = append(schools, school) + } + return schools, nil +} + // GetEducationSchoolUsers implements the EducationBackend interface for the LDAP backend. func (i *LDAP) GetEducationSchoolUsers(ctx context.Context, schoolNumberOrID string) ([]*libregraph.EducationUser, error) { logger := i.logger.SubloggerWithRequestID(ctx) @@ -436,11 +470,11 @@ func (i *LDAP) GetEducationSchoolUsers(ctx context.Context, schoolNumberOrID str } // AddUsersToEducationSchool adds new members (reference by a slice of IDs) to supplied school in the identity backend. -func (i *LDAP) AddUsersToEducationSchool(ctx context.Context, schoolNumberOrIDOrExternalID string, memberIDs []string) error { +func (i *LDAP) AddUsersToEducationSchool(ctx context.Context, schoolNumberOrID string, memberIDs []string) error { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("AddUsersToEducationSchool") - schoolEntry, err := i.getSchoolByNumberOrIDOrExternalID(schoolNumberOrIDOrExternalID) + schoolEntry, err := i.getSchoolByNumberOrID(schoolNumberOrID) if err != nil { return err } @@ -483,11 +517,11 @@ func (i *LDAP) AddUsersToEducationSchool(ctx context.Context, schoolNumberOrIDOr } // RemoveUserFromEducationSchool removes a single member (by ID) from a school -func (i *LDAP) RemoveUserFromEducationSchool(ctx context.Context, schoolNumberOrIDOrExternalID string, memberID string) error { +func (i *LDAP) RemoveUserFromEducationSchool(ctx context.Context, schoolNumberOrID string, memberID string) error { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("RemoveUserFromEducationSchool") - schoolEntry, err := i.getSchoolByNumberOrIDOrExternalID(schoolNumberOrIDOrExternalID) + schoolEntry, err := i.getSchoolByNumberOrID(schoolNumberOrID) if err != nil { return err } @@ -542,12 +576,12 @@ func (i *LDAP) GetEducationSchoolClasses(ctx context.Context, schoolNumberOrID s } func (i *LDAP) getEducationSchoolEntries( - schoolNumberOrIDOrExternalID, filter, objectClass, baseDN string, + schoolNumberOrID, filter, objectClass, baseDN string, scope int, attributes []string, logger log.Logger, ) ([]*ldap.Entry, error) { - schoolEntry, err := i.getSchoolByNumberOrIDOrExternalID(schoolNumberOrIDOrExternalID) + schoolEntry, err := i.getSchoolByNumberOrID(schoolNumberOrID) if err != nil { return nil, err } @@ -584,11 +618,11 @@ func (i *LDAP) getEducationSchoolEntries( } // AddClassesToEducationSchool adds new members (reference by a slice of IDs) to supplied school in the identity backend. -func (i *LDAP) AddClassesToEducationSchool(ctx context.Context, schoolNumberOrIDOrExternalID string, memberIDs []string) error { +func (i *LDAP) AddClassesToEducationSchool(ctx context.Context, schoolNumberOrID string, memberIDs []string) error { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("AddClassesToEducationSchool") - schoolEntry, err := i.getSchoolByNumberOrIDOrExternalID(schoolNumberOrIDOrExternalID) + schoolEntry, err := i.getSchoolByNumberOrID(schoolNumberOrID) if err != nil { return err } @@ -631,11 +665,11 @@ func (i *LDAP) AddClassesToEducationSchool(ctx context.Context, schoolNumberOrID } // RemoveClassFromEducationSchool removes a single member (by ID) from a school -func (i *LDAP) RemoveClassFromEducationSchool(ctx context.Context, schoolNumberOrIDOrExternalID string, memberID string) error { +func (i *LDAP) RemoveClassFromEducationSchool(ctx context.Context, schoolNumberOrID string, memberID string) error { logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap").Msg("RemoveClassFromEducationSchool") - schoolEntry, err := i.getSchoolByNumberOrIDOrExternalID(schoolNumberOrIDOrExternalID) + schoolEntry, err := i.getSchoolByNumberOrID(schoolNumberOrID) if err != nil { return err } @@ -673,16 +707,14 @@ func (i *LDAP) getSchoolByDN(dn string) (*ldap.Entry, error) { return i.getEntryByDN(dn, i.getEducationSchoolAttrTypes(), filter) } -func (i *LDAP) getSchoolByNumberOrIDOrExternalID(numberOrIDOrExternalID string) (*ldap.Entry, error) { - numberOrIDOrExternalID = ldap.EscapeFilter(numberOrIDOrExternalID) +func (i *LDAP) getSchoolByNumberOrID(numberOrID string) (*ldap.Entry, error) { + numberOrID = ldap.EscapeFilter(numberOrID) filter := fmt.Sprintf( - "(|(%s=%s)(%s=%s)(%s=%s))", + "(|(%s=%s)(%s=%s))", i.educationConfig.schoolAttributeMap.id, - numberOrIDOrExternalID, + numberOrID, i.educationConfig.schoolAttributeMap.schoolNumber, - numberOrIDOrExternalID, - i.educationConfig.schoolAttributeMap.externalId, - numberOrIDOrExternalID, + numberOrID, ) return i.getSchoolByFilter(filter) } @@ -697,16 +729,6 @@ func (i *LDAP) getSchoolByNumber(schoolNumber string) (*ldap.Entry, error) { return i.getSchoolByFilter(filter) } -func (i *LDAP) getSchoolByExternalId(schoolExternalId string) (*ldap.Entry, error) { - schoolExternalId = ldap.EscapeFilter(schoolExternalId) - filter := fmt.Sprintf( - "(%s=%s)", - i.educationConfig.schoolAttributeMap.externalId, - schoolExternalId, - ) - return i.getSchoolByFilter(filter) -} - func (i *LDAP) getSchoolByFilter(filter string) (*ldap.Entry, error) { filter = fmt.Sprintf("(&%s(objectClass=%s)%s)", i.educationConfig.schoolFilter, @@ -820,6 +842,7 @@ func (i *LDAP) getEducationSchoolAttrTypes() []string { return []string{ i.educationConfig.schoolAttributeMap.displayName, i.educationConfig.schoolAttributeMap.id, + i.educationConfig.schoolAttributeMap.externalId, i.educationConfig.schoolAttributeMap.schoolNumber, i.educationConfig.schoolAttributeMap.terminationDate, } diff --git a/services/graph/pkg/identity/ldap_education_school_test.go b/services/graph/pkg/identity/ldap_education_school_test.go index 5bc7718184..b47f088d6b 100644 --- a/services/graph/pkg/identity/ldap_education_school_test.go +++ b/services/graph/pkg/identity/ldap_education_school_test.go @@ -65,10 +65,10 @@ var schoolEntryWithTermination = ldap.NewEntry("ou=Test School", }) var ( - filterSchoolSearchByIdExisting = "(&(objectClass=openCloudEducationSchool)(|(openCloudUUID=abcd-defg)(openCloudEducationSchoolNumber=abcd-defg)(openCloudEducationExternalId=abcd-defg)))" - filterSchoolSearchByIdNonexistant = "(&(objectClass=openCloudEducationSchool)(|(openCloudUUID=xxxx-xxxx)(openCloudEducationSchoolNumber=xxxx-xxxx)(openCloudEducationExternalId=xxxx-xxxx)))" - filterSchoolSearchByNumberExisting = "(&(objectClass=openCloudEducationSchool)(|(openCloudUUID=0123)(openCloudEducationSchoolNumber=0123)(openCloudEducationExternalId=0123)))" - filterSchoolSearchByNumberNonexistant = "(&(objectClass=openCloudEducationSchool)(|(openCloudUUID=3210)(openCloudEducationSchoolNumber=3210)(openCloudEducationExternalId=3210)))" + filterSchoolSearchByIdExisting = "(&(objectClass=openCloudEducationSchool)(|(openCloudUUID=abcd-defg)(openCloudEducationSchoolNumber=abcd-defg)))" + filterSchoolSearchByIdNonexistant = "(&(objectClass=openCloudEducationSchool)(|(openCloudUUID=xxxx-xxxx)(openCloudEducationSchoolNumber=xxxx-xxxx)))" + filterSchoolSearchByNumberExisting = "(&(objectClass=openCloudEducationSchool)(|(openCloudUUID=0123)(openCloudEducationSchoolNumber=0123)))" + filterSchoolSearchByNumberNonexistant = "(&(objectClass=openCloudEducationSchool)(|(openCloudUUID=3210)(openCloudEducationSchoolNumber=3210)))" ) func TestCreateEducationSchool(t *testing.T) { @@ -128,7 +128,7 @@ func TestCreateEducationSchool(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: "(&(objectClass=openCloudEducationSchool)(openCloudEducationSchoolNumber=0123))", - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: []string{"ou", "openCloudUUID", "openCloudEducationExternalId", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, Controls: []ldap.Control(nil), } lm.On("Search", schoolNumberSearchRequest). @@ -142,7 +142,7 @@ func TestCreateEducationSchool(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: "(&(objectClass=openCloudEducationSchool)(openCloudEducationSchoolNumber=0666))", - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: []string{"ou", "openCloudUUID", "openCloudEducationExternalId", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, Controls: []ldap.Control(nil), } lm.On("Search", existingSchoolNumberSearchRequest). @@ -156,7 +156,7 @@ func TestCreateEducationSchool(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: "(&(objectClass=openCloudEducationSchool)(openCloudEducationSchoolNumber=1111))", - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: []string{"ou", "openCloudUUID", "openCloudEducationExternalId", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, Controls: []ldap.Control(nil), } lm.On("Search", schoolNumberSearchRequestError). @@ -170,7 +170,7 @@ func TestCreateEducationSchool(t *testing.T) { Scope: 0, SizeLimit: 1, Filter: "(objectClass=openCloudEducationSchool)", - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: []string{"ou", "openCloudUUID", "openCloudEducationExternalId", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, Controls: []ldap.Control(nil), } lm.On("Search", schoolLookupAfterCreate). @@ -358,7 +358,7 @@ func TestDeleteEducationSchool(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: tt.filter, - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: []string{"ou", "openCloudUUID", "openCloudEducationExternalId", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, Controls: []ldap.Control(nil), } if tt.expectedItemNotFound { @@ -427,7 +427,7 @@ func TestGetEducationSchool(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: tt.filter, - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: []string{"ou", "openCloudUUID", "openCloudEducationExternalId", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, Controls: []ldap.Control(nil), } if tt.expectedItemNotFound { @@ -461,7 +461,7 @@ func TestGetEducationSchools(t *testing.T) { Scope: 2, SizeLimit: 0, Filter: "(objectClass=openCloudEducationSchool)", - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: []string{"ou", "openCloudUUID", "openCloudEducationExternalId", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, Controls: []ldap.Control(nil), } lm.On("Search", sr1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{schoolEntry, schoolEntry1}}, nil) @@ -478,7 +478,7 @@ var schoolByIDSearch1 *ldap.SearchRequest = &ldap.SearchRequest{ Scope: 2, SizeLimit: 1, Filter: filterSchoolSearchByIdExisting, - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: []string{"ou", "openCloudUUID", "openCloudEducationExternalId", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, Controls: []ldap.Control(nil), } @@ -487,7 +487,7 @@ var schoolByNumberSearch *ldap.SearchRequest = &ldap.SearchRequest{ Scope: 2, SizeLimit: 1, Filter: filterSchoolSearchByNumberExisting, - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: []string{"ou", "openCloudUUID", "openCloudEducationExternalId", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, Controls: []ldap.Control(nil), } diff --git a/services/graph/pkg/identity/mocks/education_backend.go b/services/graph/pkg/identity/mocks/education_backend.go index 8933df7104..f710fa9467 100644 --- a/services/graph/pkg/identity/mocks/education_backend.go +++ b/services/graph/pkg/identity/mocks/education_backend.go @@ -602,6 +602,80 @@ func (_c *EducationBackend_DeleteEducationUser_Call) RunAndReturn(run func(ctx c return _c } +// FilterEducationSchoolsByAttribute provides a mock function for the type EducationBackend +func (_mock *EducationBackend) FilterEducationSchoolsByAttribute(ctx context.Context, attr string, value string) ([]*libregraph.EducationSchool, error) { + ret := _mock.Called(ctx, attr, value) + + if len(ret) == 0 { + panic("no return value specified for FilterEducationSchoolsByAttribute") + } + + var r0 []*libregraph.EducationSchool + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, string, string) ([]*libregraph.EducationSchool, error)); ok { + return returnFunc(ctx, attr, value) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, string, string) []*libregraph.EducationSchool); ok { + r0 = returnFunc(ctx, attr, value) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*libregraph.EducationSchool) + } + } + if returnFunc, ok := ret.Get(1).(func(context.Context, string, string) error); ok { + r1 = returnFunc(ctx, attr, value) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// EducationBackend_FilterEducationSchoolsByAttribute_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'FilterEducationSchoolsByAttribute' +type EducationBackend_FilterEducationSchoolsByAttribute_Call struct { + *mock.Call +} + +// FilterEducationSchoolsByAttribute is a helper method to define mock.On call +// - ctx context.Context +// - attr string +// - value string +func (_e *EducationBackend_Expecter) FilterEducationSchoolsByAttribute(ctx interface{}, attr interface{}, value interface{}) *EducationBackend_FilterEducationSchoolsByAttribute_Call { + return &EducationBackend_FilterEducationSchoolsByAttribute_Call{Call: _e.mock.On("FilterEducationSchoolsByAttribute", ctx, attr, value)} +} + +func (_c *EducationBackend_FilterEducationSchoolsByAttribute_Call) Run(run func(ctx context.Context, attr string, value string)) *EducationBackend_FilterEducationSchoolsByAttribute_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 string + if args[1] != nil { + arg1 = args[1].(string) + } + var arg2 string + if args[2] != nil { + arg2 = args[2].(string) + } + run( + arg0, + arg1, + arg2, + ) + }) + return _c +} + +func (_c *EducationBackend_FilterEducationSchoolsByAttribute_Call) Return(educationSchools []*libregraph.EducationSchool, err error) *EducationBackend_FilterEducationSchoolsByAttribute_Call { + _c.Call.Return(educationSchools, err) + return _c +} + +func (_c *EducationBackend_FilterEducationSchoolsByAttribute_Call) RunAndReturn(run func(ctx context.Context, attr string, value string) ([]*libregraph.EducationSchool, error)) *EducationBackend_FilterEducationSchoolsByAttribute_Call { + _c.Call.Return(run) + return _c +} + // FilterEducationUsersByAttribute provides a mock function for the type EducationBackend func (_mock *EducationBackend) FilterEducationUsersByAttribute(ctx context.Context, attr string, value string) ([]*libregraph.EducationUser, error) { ret := _mock.Called(ctx, attr, value) diff --git a/services/graph/pkg/service/v0/educationschools.go b/services/graph/pkg/service/v0/educationschools.go index 6e1a8756b2..119783c418 100644 --- a/services/graph/pkg/service/v0/educationschools.go +++ b/services/graph/pkg/service/v0/educationschools.go @@ -30,11 +30,36 @@ func (g Graph) GetEducationSchools(w http.ResponseWriter, r *http.Request) { return } - schools, err := g.identityEducationBackend.GetEducationSchools(r.Context()) - if err != nil { - logger.Debug().Err(err).Msg("could not get schools: backend error") - errorcode.RenderError(w, r, err) - return + var schools []*libregraph.EducationSchool + if odataReq.Query.Filter != nil { + attr, value, err := g.getEqualityFilter(r.Context(), odataReq) + if err != nil { + logger.Debug().Err(err).Str("filter", odataReq.Query.Filter.RawValue).Msg("failed to parse filter") + var errcode errorcode.Error + var godataerr *godata.GoDataError + switch { + case errors.As(err, &errcode): + errcode.Render(w, r) + case errors.As(err, &godataerr): + errorcode.GeneralException.Render(w, r, godataerr.ResponseCode, err.Error()) + default: + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + } + return + } + schools, err = g.identityEducationBackend.FilterEducationSchoolsByAttribute(r.Context(), attr, value) + if err != nil { + logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("could not get schools from backend") + errorcode.RenderError(w, r, err) + return + } + } else { + schools, err = g.identityEducationBackend.GetEducationSchools(r.Context()) + if err != nil { + logger.Debug().Err(err).Msg("could not get schools: backend error") + errorcode.RenderError(w, r, err) + return + } } schools, err = sortEducationSchools(odataReq, schools) diff --git a/services/graph/pkg/service/v0/educationschools_test.go b/services/graph/pkg/service/v0/educationschools_test.go index c9ede03947..79f4c2357a 100644 --- a/services/graph/pkg/service/v0/educationschools_test.go +++ b/services/graph/pkg/service/v0/educationschools_test.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "time" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" @@ -176,6 +177,18 @@ var _ = Describe("Schools", func() { Expect(len(res.Value)).To(Equal(1)) Expect(res.Value[0].GetId()).To(Equal("school1")) }) + + When("used with a filter", func() { + It("fails with an unsupported filter", func() { + svc.GetEducationSchools(rr, httptest.NewRequest(http.MethodGet, "/graph/v1.0/education/schools?$filter="+url.QueryEscape("displayName ne 'test'"), nil)) + Expect(rr.Code).To(Equal(http.StatusNotImplemented)) + }) + It("calls the backend with the externalId filter", func() { + identityEducationBackend.On("FilterEducationSchoolsByAttribute", mock.Anything, "externalId", "ext1234").Return([]*libregraph.EducationSchool{newSchool}, nil) + svc.GetEducationSchools(rr, httptest.NewRequest(http.MethodGet, "/graph/v1.0/education/schools?$filter="+url.QueryEscape("externalId eq 'ext1234'"), nil)) + Expect(rr.Code).To(Equal(http.StatusOK)) + }) + }) }) Describe("GetEducationSchool", func() {