From 6f404096cee34d2564338675c5fc422289249472 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 3 Mar 2026 16:22:00 +0100 Subject: [PATCH] feat(graph/education): Add support of 'eq' filters on users This adds support of simple OData filters on the 'education/users' endpoint. Filters of the type '$filter= eq ' are supported now for the following educationUser properties: "displayname", "mail", "userType", "primaryRole" and "externalId" Closes: #1599 --- services/graph/pkg/identity/backend.go | 2 + services/graph/pkg/identity/err_education.go | 5 ++ .../graph/pkg/identity/ldap_education_user.go | 54 ++++++++++++++ .../pkg/identity/ldap_education_user_test.go | 30 ++++++-- .../pkg/identity/mocks/education_backend.go | 74 +++++++++++++++++++ .../graph/pkg/service/v0/educationuser.go | 68 +++++++++++++++-- .../pkg/service/v0/educationuser_test.go | 12 +++ services/graph/pkg/service/v0/users_filter.go | 7 +- 8 files changed, 236 insertions(+), 16 deletions(-) diff --git a/services/graph/pkg/identity/backend.go b/services/graph/pkg/identity/backend.go index 93684311d5..dc1f2b1121 100644 --- a/services/graph/pkg/identity/backend.go +++ b/services/graph/pkg/identity/backend.go @@ -107,6 +107,8 @@ type EducationBackend interface { GetEducationUser(ctx context.Context, nameOrID string) (*libregraph.EducationUser, error) // GetEducationUsers lists all education users GetEducationUsers(ctx context.Context) ([]*libregraph.EducationUser, error) + // FilterEducationUsersByAttribute list all education users where and attribute matches a value, e.g. all users with a given externalid + FilterEducationUsersByAttribute(ctx context.Context, attr, value string) ([]*libregraph.EducationUser, error) // GetEducationClassTeachers returns the EducationUser teachers for an EducationClass GetEducationClassTeachers(ctx context.Context, classID string) ([]*libregraph.EducationUser, error) diff --git a/services/graph/pkg/identity/err_education.go b/services/graph/pkg/identity/err_education.go index 08609d2749..8c0e048534 100644 --- a/services/graph/pkg/identity/err_education.go +++ b/services/graph/pkg/identity/err_education.go @@ -119,6 +119,11 @@ func (i *ErrEducationBackend) GetEducationUsers(ctx context.Context) ([]*libregr return nil, errNotImplemented } +// FilterEducationUsersByAttribute implements the EducationBackend interface for the ErrEducationBackend backend. +func (i *ErrEducationBackend) FilterEducationUsersByAttribute(ctx context.Context, attr, value string) ([]*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 diff --git a/services/graph/pkg/identity/ldap_education_user.go b/services/graph/pkg/identity/ldap_education_user.go index 9fb33f78fb..5a5bf0bb66 100644 --- a/services/graph/pkg/identity/ldap_education_user.go +++ b/services/graph/pkg/identity/ldap_education_user.go @@ -252,6 +252,60 @@ func (i *LDAP) GetEducationUsers(ctx context.Context) ([]*libregraph.EducationUs return users, nil } +func (i *LDAP) FilterEducationUsersByAttribute(ctx context.Context, attr, value string) ([]*libregraph.EducationUser, error) { + logger := i.logger.SubloggerWithRequestID(ctx).With().Str("func", "FilterEducationUsersByAttribute").Logger() + logger.Debug().Str("backend", "ldap").Str("attribute", attr).Str("value", value).Msg("") + + var ldapAttr string + switch attr { + case "displayname": + ldapAttr = i.userAttributeMap.displayName + case "mail": + ldapAttr = i.userAttributeMap.mail + case "userType": + ldapAttr = i.userAttributeMap.userType + case "primaryRole": + ldapAttr = i.educationConfig.userAttributeMap.primaryRole + case "externalId": + ldapAttr = i.educationConfig.userAttributeMap.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.userFilter, i.educationConfig.userObjectClass, ldap.EscapeFilter(ldapAttr), ldap.EscapeFilter(value)) + + searchRequest := ldap.NewSearchRequest( + i.userBaseDN, + i.userScope, + ldap.NeverDerefAliases, 0, 0, false, + filter, + i.getEducationUserAttrTypes(), + 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()) + } + + users := make([]*libregraph.EducationUser, 0, len(res.Entries)) + + for _, e := range res.Entries { + u := i.createEducationUserModelFromLDAP(e) + // Skip invalid LDAP users + if u == nil { + continue + } + users = append(users, u) + } + return users, nil +} + func (i *LDAP) educationUserToUser(eduUser libregraph.EducationUser) *libregraph.User { user := libregraph.NewUser(*eduUser.DisplayName, *eduUser.OnPremisesSamAccountName) user.Surname = eduUser.Surname diff --git a/services/graph/pkg/identity/ldap_education_user_test.go b/services/graph/pkg/identity/ldap_education_user_test.go index bfe047d9b6..d85a73b448 100644 --- a/services/graph/pkg/identity/ldap_education_user_test.go +++ b/services/graph/pkg/identity/ldap_education_user_test.go @@ -5,12 +5,14 @@ import ( "testing" "github.com/go-ldap/ldap/v3" - "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" libregraph "github.com/opencloud-eu/libre-graph-api-go" + "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) +const peopleBaseDN = "ou=people,dc=test" + var eduUserAttrs = []string{ "displayname", "entryUUID", @@ -69,7 +71,7 @@ var eduUserEntryWithSchool = ldap.NewEntry("uid=user,ou=people,dc=test", }) var sr1 *ldap.SearchRequest = &ldap.SearchRequest{ - BaseDN: "ou=people,dc=test", + BaseDN: peopleBaseDN, Scope: 2, SizeLimit: 1, Filter: "(&(objectClass=openCloudEducationUser)(|(uid=abcd-defg)(entryUUID=abcd-defg)))", @@ -77,7 +79,7 @@ var sr1 *ldap.SearchRequest = &ldap.SearchRequest{ Controls: []ldap.Control(nil), } var sr2 *ldap.SearchRequest = &ldap.SearchRequest{ - BaseDN: "ou=people,dc=test", + BaseDN: peopleBaseDN, Scope: 2, SizeLimit: 1, Filter: "(&(objectClass=openCloudEducationUser)(|(uid=xxxx-xxxx)(entryUUID=xxxx-xxxx)))", @@ -164,7 +166,7 @@ func TestGetEducationUser(t *testing.T) { func TestGetEducationUsers(t *testing.T) { lm := &mocks.Client{} sr := &ldap.SearchRequest{ - BaseDN: "ou=people,dc=test", + BaseDN: peopleBaseDN, Scope: 2, SizeLimit: 0, Filter: "(objectClass=openCloudEducationUser)", @@ -179,12 +181,30 @@ func TestGetEducationUsers(t *testing.T) { assert.Nil(t, err) } +func TestFilterEducationUsersByAttr(t *testing.T) { + lm := &mocks.Client{} + sr := &ldap.SearchRequest{ + BaseDN: peopleBaseDN, + Scope: 2, + SizeLimit: 0, + Filter: "(&(objectClass=openCloudEducationUser)(openCloudEducationExternalId=id1234))", + Attributes: eduUserAttrs, + Controls: []ldap.Control(nil), + } + lm.On("Search", sr).Return(&ldap.SearchResult{Entries: []*ldap.Entry{eduUserEntry}}, nil) + b, err := getMockedBackend(lm, eduConfig, &logger) + assert.Nil(t, err) + _, err = b.FilterEducationUsersByAttribute(context.Background(), "externalId", "id1234") + lm.AssertNumberOfCalls(t, "Search", 1) + assert.Nil(t, err) +} + func TestUpdateEducationUser(t *testing.T) { lm := &mocks.Client{} b, err := getMockedBackend(lm, eduConfig, &logger) assert.Nil(t, err) userSearchReq := &ldap.SearchRequest{ - BaseDN: "ou=people,dc=test", + BaseDN: peopleBaseDN, Scope: 2, SizeLimit: 1, Filter: "(&(objectClass=openCloudEducationUser)(|(uid=testuser)(entryUUID=testuser)))", diff --git a/services/graph/pkg/identity/mocks/education_backend.go b/services/graph/pkg/identity/mocks/education_backend.go index 08e075bd2f..8933df7104 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 } +// 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) + + if len(ret) == 0 { + panic("no return value specified for FilterEducationUsersByAttribute") + } + + var r0 []*libregraph.EducationUser + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, string, string) ([]*libregraph.EducationUser, error)); ok { + return returnFunc(ctx, attr, value) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, string, string) []*libregraph.EducationUser); ok { + r0 = returnFunc(ctx, attr, value) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*libregraph.EducationUser) + } + } + 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_FilterEducationUsersByAttribute_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'FilterEducationUsersByAttribute' +type EducationBackend_FilterEducationUsersByAttribute_Call struct { + *mock.Call +} + +// FilterEducationUsersByAttribute is a helper method to define mock.On call +// - ctx context.Context +// - attr string +// - value string +func (_e *EducationBackend_Expecter) FilterEducationUsersByAttribute(ctx interface{}, attr interface{}, value interface{}) *EducationBackend_FilterEducationUsersByAttribute_Call { + return &EducationBackend_FilterEducationUsersByAttribute_Call{Call: _e.mock.On("FilterEducationUsersByAttribute", ctx, attr, value)} +} + +func (_c *EducationBackend_FilterEducationUsersByAttribute_Call) Run(run func(ctx context.Context, attr string, value string)) *EducationBackend_FilterEducationUsersByAttribute_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_FilterEducationUsersByAttribute_Call) Return(educationUsers []*libregraph.EducationUser, err error) *EducationBackend_FilterEducationUsersByAttribute_Call { + _c.Call.Return(educationUsers, err) + return _c +} + +func (_c *EducationBackend_FilterEducationUsersByAttribute_Call) RunAndReturn(run func(ctx context.Context, attr string, value string) ([]*libregraph.EducationUser, error)) *EducationBackend_FilterEducationUsersByAttribute_Call { + _c.Call.Return(run) + return _c +} + // GetEducationClass provides a mock function for the type EducationBackend func (_mock *EducationBackend) GetEducationClass(ctx context.Context, namedOrID string) (*libregraph.EducationClass, error) { ret := _mock.Called(ctx, namedOrID) diff --git a/services/graph/pkg/service/v0/educationuser.go b/services/graph/pkg/service/v0/educationuser.go index ee23e3e2a2..8fa87c5eec 100644 --- a/services/graph/pkg/service/v0/educationuser.go +++ b/services/graph/pkg/service/v0/educationuser.go @@ -1,6 +1,8 @@ package svc import ( + "context" + "errors" "fmt" "net/http" "net/url" @@ -21,8 +23,7 @@ import ( // GetEducationUsers implements the Service interface. func (g Graph) GetEducationUsers(w http.ResponseWriter, r *http.Request) { - logger := g.logger.SubloggerWithRequestID(r.Context()) - logger.Info().Interface("query", r.URL.Query()).Msg("calling get education users") + logger := g.logger.SubloggerWithRequestID(r.Context()).With().Str("func", "GetEducationUsers").Logger() sanitizedPath := strings.TrimPrefix(r.URL.Path, "/graph/v1.0/") odataReq, err := godata.ParseRequest(r.Context(), sanitizedPath, r.URL.Query()) if err != nil { @@ -31,12 +32,36 @@ func (g Graph) GetEducationUsers(w http.ResponseWriter, r *http.Request) { return } - logger.Debug().Interface("query", r.URL.Query()).Msg("calling get education users on backend") - users, err := g.identityEducationBackend.GetEducationUsers(r.Context()) - if err != nil { - logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("could not get education users from backend") - errorcode.RenderError(w, r, err) - return + var users []*libregraph.EducationUser + 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 + } + users, err = g.identityEducationBackend.FilterEducationUsersByAttribute(r.Context(), attr, value) + if err != nil { + logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("could not get education users from backend") + errorcode.RenderError(w, r, err) + return + } + } else { + users, err = g.identityEducationBackend.GetEducationUsers(r.Context()) + if err != nil { + logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("could not get education users from backend") + errorcode.RenderError(w, r, err) + return + } } users, err = sortEducationUsers(odataReq, users) @@ -387,3 +412,30 @@ func sortEducationUsers(req *godata.GoDataRequest, users []*libregraph.Education } return users, nil } + +func (g Graph) getEqualityFilter(ctx context.Context, req *godata.GoDataRequest) (string, string, error) { + logger := g.logger.SubloggerWithRequestID(ctx) + + root := req.Query.Filter.Tree + + if root.Token.Type != godata.ExpressionTokenLogical { + logger.Debug().Str("filter", req.Query.Filter.RawValue).Msg(unsupportedFilter) + return "", "", unsupportedFilterError() + } + if root.Token.Value != "eq" { + logger.Debug().Str("filter", req.Query.Filter.RawValue).Msg(unsupportedFilter) + return "", "", unsupportedFilterError() + } + if len(root.Children) != 2 { + logger.Debug().Str("filter", req.Query.Filter.RawValue).Msg(unsupportedFilter) + return "", "", unsupportedFilterError() + } + if root.Children[0].Token.Type != godata.ExpressionTokenLiteral || root.Children[1].Token.Type != godata.ExpressionTokenString { + logger.Debug().Str("filter", req.Query.Filter.RawValue).Msg(unsupportedFilter) + return "", "", unsupportedFilterError() + } + + // unquote + value := strings.Trim(root.Children[1].Token.Value, "'") + return root.Children[0].Token.Value, value, nil +} diff --git a/services/graph/pkg/service/v0/educationuser_test.go b/services/graph/pkg/service/v0/educationuser_test.go index 292cbd824f..077f270bb1 100644 --- a/services/graph/pkg/service/v0/educationuser_test.go +++ b/services/graph/pkg/service/v0/educationuser_test.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -202,6 +203,17 @@ var _ = Describe("EducationUsers", func() { Expect(rr.Code).To(Equal(http.StatusBadRequest)) }) + When("used with a filter", func() { + It("fails with an unsupported filter ", func() { + svc.GetEducationUsers(rr, httptest.NewRequest(http.MethodGet, "/graph/v1.0/education/users?$filter="+url.QueryEscape("displayName ne 'test'"), nil)) + Expect(rr.Code).To(Equal(http.StatusNotImplemented)) + }) + It("calls the backend with the filter", func() { + identityEducationBackend.On("FilterEducationUsersByAttribute", mock.Anything, "externalId", "id1234").Return([]*libregraph.EducationUser{}, nil) + svc.GetEducationUsers(rr, httptest.NewRequest(http.MethodGet, "/graph/v1.0/education/users?$filter="+url.QueryEscape("externalId eq 'id1234'"), nil)) + Expect(rr.Code).To(Equal(http.StatusOK)) + }) + }) }) Describe("GetEducationUser", func() { diff --git a/services/graph/pkg/service/v0/users_filter.go b/services/graph/pkg/service/v0/users_filter.go index 0fc0b1c27b..0232e06f2e 100644 --- a/services/graph/pkg/service/v0/users_filter.go +++ b/services/graph/pkg/service/v0/users_filter.go @@ -5,14 +5,15 @@ import ( "strings" "github.com/CiscoM31/godata" + libregraph "github.com/opencloud-eu/libre-graph-api-go" settingsmsg "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/settings/v0" settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" - libregraph "github.com/opencloud-eu/libre-graph-api-go" ) const ( appRoleID = "appRoleId" appRoleAssignments = "appRoleAssignments" + unsupportedFilter = "unsupported filter" ) func invalidFilterError() error { @@ -20,7 +21,7 @@ func invalidFilterError() error { } func unsupportedFilterError() error { - return godata.NotImplementedError("unsupported filter") + return godata.NotImplementedError(unsupportedFilter) } func (g Graph) applyUserFilter(ctx context.Context, req *godata.GoDataRequest, root *godata.ParseNode) (users []*libregraph.User, err error) { @@ -38,7 +39,7 @@ func (g Graph) applyUserFilter(ctx context.Context, req *godata.GoDataRequest, r case godata.ExpressionTokenFunc: return g.applyFilterFunction(ctx, req, root) } - logger.Debug().Str("filter", req.Query.Filter.RawValue).Msg("filter is not supported") + logger.Debug().Str("filter", req.Query.Filter.RawValue).Msg(unsupportedFilter) return users, unsupportedFilterError() }