diff --git a/devtools/deployments/multi-tenancy/initialize_users.go b/devtools/deployments/multi-tenancy/initialize_users.go index 9c64182b44..b376e3476a 100644 --- a/devtools/deployments/multi-tenancy/initialize_users.go +++ b/devtools/deployments/multi-tenancy/initialize_users.go @@ -31,11 +31,13 @@ var demoTenants = []tenantWithUsers{ DisplayName: libregraph.PtrString("Dennis Ritchie"), OnPremisesSamAccountName: libregraph.PtrString("dennis"), Mail: libregraph.PtrString("dennis@example.org"), + ExternalId: libregraph.PtrString("ExternalID1"), }, { DisplayName: libregraph.PtrString("Grace Hopper"), OnPremisesSamAccountName: libregraph.PtrString("grace"), Mail: libregraph.PtrString("grace@example.org"), + ExternalId: libregraph.PtrString("ExternalID2"), }, }, }, @@ -49,11 +51,13 @@ var demoTenants = []tenantWithUsers{ DisplayName: libregraph.PtrString("Albert Einstein"), OnPremisesSamAccountName: libregraph.PtrString("einstein"), Mail: libregraph.PtrString("einstein@example.org"), + ExternalId: libregraph.PtrString("ExternalID3"), }, { DisplayName: libregraph.PtrString("Marie Curie"), OnPremisesSamAccountName: libregraph.PtrString("marie"), Mail: libregraph.PtrString("marie@example.org"), + ExternalId: libregraph.PtrString("ExternalID4"), }, }, }, diff --git a/services/graph/pkg/identity/backend.go b/services/graph/pkg/identity/backend.go index 93684311d5..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 @@ -107,6 +109,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..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 @@ -119,6 +124,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_school.go b/services/graph/pkg/identity/ldap_education_school.go index 2aecc3d9bb..68ee855b61 100644 --- a/services/graph/pkg/identity/ldap_education_school.go +++ b/services/graph/pkg/identity/ldap_education_school.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "slices" "time" "github.com/go-ldap/ldap/v3" @@ -49,10 +50,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 +136,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 +284,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 +314,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 +328,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 +343,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 } @@ -371,13 +356,36 @@ func (i *LDAP) GetEducationSchool(ctx context.Context, numberOrIDOrExternalID st // GetEducationSchools implements the EducationBackend interface for the LDAP backend. func (i *LDAP) GetEducationSchools(ctx context.Context) ([]*libregraph.EducationSchool, error) { - var filter string - filter = fmt.Sprintf("(objectClass=%s)", i.educationConfig.schoolObjectClass) - + filter := fmt.Sprintf("(objectClass=%s)", i.educationConfig.schoolObjectClass) if i.educationConfig.schoolFilter != "" { filter = fmt.Sprintf("(&%s%s)", i.educationConfig.schoolFilter, filter) } + return i.searchEducationSchools(ctx, filter) +} + +// 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), + ) + return i.searchEducationSchools(ctx, filter) +} +// searchEducationSchools builds and executes an LDAP search for education schools and converts the results to EducationSchool models. +func (i *LDAP) searchEducationSchools(ctx context.Context, filter string) ([]*libregraph.EducationSchool, error) { searchRequest := ldap.NewSearchRequest( i.educationConfig.schoolBaseDN, i.educationConfig.schoolScope, @@ -386,13 +394,15 @@ func (i *LDAP) GetEducationSchools(ctx context.Context) ([]*libregraph.Education i.getEducationSchoolAttrTypes(), nil, ) - i.logger.Debug().Str("backend", "ldap"). + logger := i.logger.SubloggerWithRequestID(ctx) + logger.Debug().Str("backend", "ldap"). Str("base", searchRequest.BaseDN). Str("filter", searchRequest.Filter). Int("scope", searchRequest.Scope). Int("sizelimit", searchRequest.SizeLimit). Interface("attributes", searchRequest.Attributes). - Msg("GetEducationSchools") + Msg("searchEducationSchools") + res, err := i.conn.Search(searchRequest) if err != nil { return nil, errorcode.New(errorcode.ItemNotFound, err.Error()) @@ -436,11 +446,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 } @@ -462,32 +472,31 @@ func (i *LDAP) AddUsersToEducationSchool(ctx context.Context, schoolNumberOrIDOr } for _, userEntry := range userEntries { - currentSchools := userEntry.GetEqualFoldAttributeValues(i.educationConfig.memberOfSchoolAttribute) - found := false - for _, currentSchool := range currentSchools { - if currentSchool == schoolID { - found = true - break - } - } - if !found { - mr := ldap.ModifyRequest{DN: userEntry.DN} - mr.Add(i.educationConfig.memberOfSchoolAttribute, []string{schoolID}) - if err := i.conn.Modify(&mr); err != nil { - return err - } + if err := i.addEntryToSchool(userEntry, schoolID); err != nil { + return err } } return nil } +// addEntryToSchool adds the schoolID to the entry's memberOfSchool attribute if not already present. +func (i *LDAP) addEntryToSchool(entry *ldap.Entry, schoolID string) error { + currentSchools := entry.GetEqualFoldAttributeValues(i.educationConfig.memberOfSchoolAttribute) + if slices.Contains(currentSchools, schoolID) { + return nil + } + mr := ldap.ModifyRequest{DN: entry.DN} + mr.Add(i.educationConfig.memberOfSchoolAttribute, []string{schoolID}) + return i.conn.Modify(&mr) +} + // 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 +551,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 +593,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 } @@ -610,20 +619,8 @@ func (i *LDAP) AddClassesToEducationSchool(ctx context.Context, schoolNumberOrID } for _, classEntry := range classEntries { - currentSchools := classEntry.GetEqualFoldAttributeValues(i.educationConfig.memberOfSchoolAttribute) - found := false - for _, currentSchool := range currentSchools { - if currentSchool == schoolID { - found = true - break - } - } - if !found { - mr := ldap.ModifyRequest{DN: classEntry.DN} - mr.Add(i.educationConfig.memberOfSchoolAttribute, []string{schoolID}) - if err := i.conn.Modify(&mr); err != nil { - return err - } + if err := i.addEntryToSchool(classEntry, schoolID); err != nil { + return err } } @@ -631,11 +628,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 +670,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 +692,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 +805,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..0eb4fcc048 100644 --- a/services/graph/pkg/identity/ldap_education_school_test.go +++ b/services/graph/pkg/identity/ldap_education_school_test.go @@ -65,10 +65,12 @@ 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)))" + + schoolLDAPAttributeTypes = []string{"ou", "openCloudUUID", "openCloudEducationExternalId", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"} ) func TestCreateEducationSchool(t *testing.T) { @@ -128,7 +130,7 @@ func TestCreateEducationSchool(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: "(&(objectClass=openCloudEducationSchool)(openCloudEducationSchoolNumber=0123))", - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: schoolLDAPAttributeTypes, Controls: []ldap.Control(nil), } lm.On("Search", schoolNumberSearchRequest). @@ -142,7 +144,7 @@ func TestCreateEducationSchool(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: "(&(objectClass=openCloudEducationSchool)(openCloudEducationSchoolNumber=0666))", - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: schoolLDAPAttributeTypes, Controls: []ldap.Control(nil), } lm.On("Search", existingSchoolNumberSearchRequest). @@ -156,7 +158,7 @@ func TestCreateEducationSchool(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: "(&(objectClass=openCloudEducationSchool)(openCloudEducationSchoolNumber=1111))", - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: schoolLDAPAttributeTypes, Controls: []ldap.Control(nil), } lm.On("Search", schoolNumberSearchRequestError). @@ -170,7 +172,7 @@ func TestCreateEducationSchool(t *testing.T) { Scope: 0, SizeLimit: 1, Filter: "(objectClass=openCloudEducationSchool)", - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: schoolLDAPAttributeTypes, Controls: []ldap.Control(nil), } lm.On("Search", schoolLookupAfterCreate). @@ -358,7 +360,7 @@ func TestDeleteEducationSchool(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: tt.filter, - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: schoolLDAPAttributeTypes, Controls: []ldap.Control(nil), } if tt.expectedItemNotFound { @@ -427,7 +429,7 @@ func TestGetEducationSchool(t *testing.T) { Scope: 2, SizeLimit: 1, Filter: tt.filter, - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: schoolLDAPAttributeTypes, Controls: []ldap.Control(nil), } if tt.expectedItemNotFound { @@ -461,7 +463,7 @@ func TestGetEducationSchools(t *testing.T) { Scope: 2, SizeLimit: 0, Filter: "(objectClass=openCloudEducationSchool)", - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: schoolLDAPAttributeTypes, Controls: []ldap.Control(nil), } lm.On("Search", sr1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{schoolEntry, schoolEntry1}}, nil) @@ -478,7 +480,7 @@ var schoolByIDSearch1 *ldap.SearchRequest = &ldap.SearchRequest{ Scope: 2, SizeLimit: 1, Filter: filterSchoolSearchByIdExisting, - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: schoolLDAPAttributeTypes, Controls: []ldap.Control(nil), } @@ -487,7 +489,7 @@ var schoolByNumberSearch *ldap.SearchRequest = &ldap.SearchRequest{ Scope: 2, SizeLimit: 1, Filter: filterSchoolSearchByNumberExisting, - Attributes: []string{"ou", "openCloudUUID", "openCloudEducationSchoolNumber", "openCloudEducationSchoolTerminationTimestamp"}, + Attributes: schoolLDAPAttributeTypes, Controls: []ldap.Control(nil), } diff --git a/services/graph/pkg/identity/ldap_education_user.go b/services/graph/pkg/identity/ldap_education_user.go index c2700195ed..88855d52e0 100644 --- a/services/graph/pkg/identity/ldap_education_user.go +++ b/services/graph/pkg/identity/ldap_education_user.go @@ -12,11 +12,13 @@ import ( type educationUserAttributeMap struct { primaryRole string + externalID string } func newEducationUserAttributeMap() educationUserAttributeMap { return educationUserAttributeMap{ primaryRole: "userClass", + externalID: "openCloudEducationExternalId", } } @@ -33,7 +35,7 @@ func (i *LDAP) CreateEducationUser(ctx context.Context, user libregraph.Educatio return nil, err } - if err := i.conn.Add(ar); err != nil { + if err = i.conn.Add(ar); err != nil { var lerr *ldap.Error logger.Debug().Err(err).Msg("error adding user") if errors.As(err, &lerr) { @@ -118,6 +120,7 @@ func (i *LDAP) UpdateEducationUser(ctx context.Context, nameOrID string, user li i.userAttributeMap.givenName: user.GetGivenName(), i.userAttributeMap.userType: user.GetUserType(), i.educationConfig.userAttributeMap.primaryRole: user.GetPrimaryRole(), + i.educationConfig.userAttributeMap.externalID: user.GetExternalId(), } for attribute, value := range properties { @@ -205,39 +208,63 @@ func (i *LDAP) GetEducationUser(ctx context.Context, nameOrID string) (*libregra // GetEducationUsers implements the EducationBackend interface for the LDAP backend. func (i *LDAP) GetEducationUsers(ctx context.Context) ([]*libregraph.EducationUser, error) { - logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetEducationUsers") - - var userFilter string - + var filter string if i.userFilter == "" { - userFilter = fmt.Sprintf("(objectClass=%s)", i.educationConfig.userObjectClass) + filter = fmt.Sprintf("(objectClass=%s)", i.educationConfig.userObjectClass) } else { - userFilter = fmt.Sprintf("(&%s(objectClass=%s))", i.userFilter, i.educationConfig.userObjectClass) + filter = fmt.Sprintf("(&%s(objectClass=%s))", i.userFilter, i.educationConfig.userObjectClass) } + return i.searchEducationUsers(ctx, filter) +} +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)) + return i.searchEducationUsers(ctx, filter) +} + +// searchEducationUsers builds and executes an LDAP search for education users and converts the results to EducationUser models. +func (i *LDAP) searchEducationUsers(ctx context.Context, filter string) ([]*libregraph.EducationUser, error) { searchRequest := ldap.NewSearchRequest( i.userBaseDN, i.userScope, ldap.NeverDerefAliases, 0, 0, false, - userFilter, + filter, i.getEducationUserAttrTypes(), nil, ) + logger := i.logger.SubloggerWithRequestID(ctx) logger.Debug().Str("backend", "ldap"). Str("base", searchRequest.BaseDN). Str("filter", searchRequest.Filter). Int("scope", searchRequest.Scope). Int("sizelimit", searchRequest.SizeLimit). Interface("attributes", searchRequest.Attributes). - Msg("GetEducationUsers") + Msg("searchEducationUsers") + 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 @@ -277,6 +304,10 @@ func (i *LDAP) userToEducationUser(user libregraph.User, e *ldap.Entry) *libregr if primaryRole := e.GetEqualFoldAttributeValue(i.educationConfig.userAttributeMap.primaryRole); primaryRole != "" { eduUser.SetPrimaryRole(primaryRole) } + + if externalID := e.GetEqualFoldAttributeValue(i.educationConfig.userAttributeMap.externalID); externalID != "" { + eduUser.SetExternalId(externalID) + } } return eduUser @@ -286,6 +317,10 @@ func (i *LDAP) educationUserToLDAPAttrValues(user libregraph.EducationUser, attr if role, ok := user.GetPrimaryRoleOk(); ok { attrs[i.educationConfig.userAttributeMap.primaryRole] = []string{*role} } + + if externalID, ok := user.GetExternalIdOk(); ok { + attrs[i.educationConfig.userAttributeMap.externalID] = []string{*externalID} + } attrs["objectClass"] = append(attrs["objectClass"], i.educationConfig.userObjectClass) return attrs, nil } @@ -326,6 +361,7 @@ func (i *LDAP) getEducationUserAttrTypes() []string { i.userAttributeMap.userType, i.userAttributeMap.identities, i.educationConfig.userAttributeMap.primaryRole, + i.educationConfig.userAttributeMap.externalID, i.educationConfig.memberOfSchoolAttribute, } } diff --git a/services/graph/pkg/identity/ldap_education_user_test.go b/services/graph/pkg/identity/ldap_education_user_test.go index 5fbefa5d82..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", @@ -22,6 +24,7 @@ var eduUserAttrs = []string{ "userTypeAttribute", "openCloudExternalIdentity", "userClass", + "openCloudEducationExternalId", "openCloudMemberOfSchool", } @@ -68,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)))", @@ -76,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)))", @@ -163,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)", @@ -178,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..f710fa9467 100644 --- a/services/graph/pkg/identity/mocks/education_backend.go +++ b/services/graph/pkg/identity/mocks/education_backend.go @@ -602,6 +602,154 @@ 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) + + 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/educationschools.go b/services/graph/pkg/service/v0/educationschools.go index 6e1a8756b2..d363a43494 100644 --- a/services/graph/pkg/service/v0/educationschools.go +++ b/services/graph/pkg/service/v0/educationschools.go @@ -1,6 +1,7 @@ package svc import ( + "context" "errors" "fmt" "net/http" @@ -30,10 +31,10 @@ func (g Graph) GetEducationSchools(w http.ResponseWriter, r *http.Request) { return } - schools, err := g.identityEducationBackend.GetEducationSchools(r.Context()) + schools, err := g.getEducationSchoolsFromBackend(r.Context(), odataReq) if err != nil { - logger.Debug().Err(err).Msg("could not get schools: backend error") - errorcode.RenderError(w, r, err) + logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("could not get schools") + renderEqualityFilterError(w, r, err) return } @@ -616,3 +617,15 @@ func sortEducationSchools(req *godata.GoDataRequest, schools []*libregraph.Educa return schools, nil } + +// getEducationSchoolsFromBackend fetches schools from the backend, applying an OData $filter if present. +func (g Graph) getEducationSchoolsFromBackend(ctx context.Context, odataReq *godata.GoDataRequest) ([]*libregraph.EducationSchool, error) { + if odataReq.Query.Filter != nil { + attr, value, err := g.getEqualityFilter(ctx, odataReq) + if err != nil { + return nil, err + } + return g.identityEducationBackend.FilterEducationSchoolsByAttribute(ctx, attr, value) + } + return g.identityEducationBackend.GetEducationSchools(ctx) +} 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() { diff --git a/services/graph/pkg/service/v0/educationuser.go b/services/graph/pkg/service/v0/educationuser.go index ee23e3e2a2..389eccf665 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,14 +32,12 @@ 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()) + users, err := g.getEducationUsersFromBackend(r.Context(), odataReq) 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) + logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("could not get education users") + renderEqualityFilterError(w, r, err) return } - users, err = sortEducationUsers(odataReq, users) if err != nil { logger.Debug().Interface("query", odataReq).Msg("error while sorting education users according to query") @@ -387,3 +386,56 @@ func sortEducationUsers(req *godata.GoDataRequest, users []*libregraph.Education } return users, nil } + +// getEducationUsersFromBackend fetches users from the backend, applying an OData $filter if present. +func (g Graph) getEducationUsersFromBackend(ctx context.Context, odataReq *godata.GoDataRequest) ([]*libregraph.EducationUser, error) { + if odataReq.Query.Filter != nil { + attr, value, err := g.getEqualityFilter(ctx, odataReq) + if err != nil { + return nil, err + } + return g.identityEducationBackend.FilterEducationUsersByAttribute(ctx, attr, value) + } + return g.identityEducationBackend.GetEducationUsers(ctx) +} + +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 +} + +// renderEqualityFilterError writes the appropriate HTTP error response for errors returned by getEqualityFilter. +func renderEqualityFilterError(w http.ResponseWriter, r *http.Request, err error) { + 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()) + } +} 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() }