sevenhill/QSfera
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

graph: fix adding users to educationSchool by schoolNumber (#5422)

Browse Source 
GET /education/schools/{school-id}/users
POST /education/schools/{school-id}/users/$ref
and
DELETE /education/schools/{school-id}/users/$ref

are supposed to also work when using the schoolNumber as the
'{school-id}' parameter. This fix that functionality. This also makes the
the mocks for the LDAP Modify operations more specific to avoid using
the generic mock.Anything
This commit is contained in:
Ralf Haferkamp
2023-01-23 10:37:24 +01:00
committed by GitHub
parent c2ed4ab526
commit 473c64fe03
2 changed files with 66 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
services/graph/pkg/identity/ldap_education_school.go
+13 -8
View File
@@ -348,11 +348,11 @@ func (i *LDAP) GetEducationSchools(ctx context.Context, queryParam url.Values) (
}
// GetEducationSchoolUsers implements the EducationBackend interface for the LDAP backend.
func (i *LDAP) GetEducationSchoolUsers(ctx context.Context, id string) ([]*libregraph.EducationUser, error) {
func (i *LDAP) GetEducationSchoolUsers(ctx context.Context, schoolNumberOrID string) ([]*libregraph.EducationUser, error) {
logger := i.logger.SubloggerWithRequestID(ctx)
logger.Debug().Str("backend", "ldap").Msg("GetEducationSchoolUsers")
schoolEntry, err := i.getSchoolByNumberOrID(id)
schoolEntry, err := i.getSchoolByNumberOrID(schoolNumberOrID)
if err != nil {
return nil, err
}
@@ -360,8 +360,9 @@ func (i *LDAP) GetEducationSchoolUsers(ctx context.Context, id string) ([]*libre
if schoolEntry == nil {
return nil, ErrNotFound
}
id = ldap.EscapeFilter(id)
idFilter := fmt.Sprintf("(%s=%s)", i.educationConfig.memberOfSchoolAttribute, id)
schoolID := schoolEntry.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.id)
schoolID = ldap.EscapeFilter(schoolID)
idFilter := fmt.Sprintf("(%s=%s)", i.educationConfig.memberOfSchoolAttribute, schoolID)
userFilter := fmt.Sprintf("(&%s(objectClass=%s)%s)", i.userFilter, i.educationConfig.userObjectClass, idFilter)
searchRequest := ldap.NewSearchRequest(
@@ -398,11 +399,11 @@ func (i *LDAP) GetEducationSchoolUsers(ctx context.Context, id string) ([]*libre
}
// AddUsersToEducationSchool adds new members (reference by a slice of IDs) to supplied school in the identity backend.
func (i *LDAP) AddUsersToEducationSchool(ctx context.Context, schoolID 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.getSchoolByNumberOrID(schoolID)
schoolEntry, err := i.getSchoolByNumberOrID(schoolNumberOrID)
if err != nil {
return err
}
@@ -411,6 +412,8 @@ func (i *LDAP) AddUsersToEducationSchool(ctx context.Context, schoolID string, m
return ErrNotFound
}
schoolID := schoolEntry.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.id)
userEntries := make([]*ldap.Entry, 0, len(memberIDs))
for _, memberID := range memberIDs {
user, err := i.getEducationUserByNameOrID(memberID)
@@ -443,11 +446,11 @@ func (i *LDAP) AddUsersToEducationSchool(ctx context.Context, schoolID string, m
}
// RemoveUserFromEducationSchool removes a single member (by ID) from a school
func (i *LDAP) RemoveUserFromEducationSchool(ctx context.Context, schoolID 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.getSchoolByNumberOrID(schoolID)
schoolEntry, err := i.getSchoolByNumberOrID(schoolNumberOrID)
if err != nil {
return err
}
@@ -455,6 +458,8 @@ func (i *LDAP) RemoveUserFromEducationSchool(ctx context.Context, schoolID strin
if schoolEntry == nil {
return ErrNotFound
}
schoolID := schoolEntry.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.id)
user, err := i.getEducationUserByNameOrID(memberID)
if err != nil {
i.logger.Warn().Str("userid", memberID).Msg("User does not exist")
services/graph/pkg/identity/ldap_education_school_test.go
+53 -5
View File
@@ -303,6 +303,14 @@ var schoolByIDSearch1 *ldap.SearchRequest = &ldap.SearchRequest{
Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber"},
Controls: []ldap.Control(nil),
}
var schoolByNumberSearch *ldap.SearchRequest = &ldap.SearchRequest{
BaseDN: "",
Scope: 2,
SizeLimit: 1,
Filter: filterSchoolSearchByNumberExisting,
Attributes: []string{"ou", "owncloudUUID", "ocEducationSchoolNumber"},
Controls: []ldap.Control(nil),
}
var userByIDSearch1 *ldap.SearchRequest = &ldap.SearchRequest{
BaseDN: "ou=people,dc=test",
Scope: 2,
@@ -320,12 +328,39 @@ var userByIDSearch2 *ldap.SearchRequest = &ldap.SearchRequest{
Controls: []ldap.Control(nil),
}
var userToSchoolModRequest *ldap.ModifyRequest = &ldap.ModifyRequest{
DN: "uid=user,ou=people,dc=test",
Changes: []ldap.Change{
{
Operation: ldap.AddAttribute,
Modification: ldap.PartialAttribute{
Type: "ocMemberOfSchool",
Vals: []string{"abcd-defg"},
},
},
},
}
var userFromSchoolModRequest *ldap.ModifyRequest = &ldap.ModifyRequest{
DN: "uid=user,ou=people,dc=test",
Changes: []ldap.Change{
{
Operation: ldap.DeleteAttribute,
Modification: ldap.PartialAttribute{
Type: "ocMemberOfSchool",
Vals: []string{"abcd-defg"},
},
},
},
}
func TestAddUsersToEducationSchool(t *testing.T) {
lm := &mocks.Client{}
lm.On("Search", schoolByIDSearch1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{schoolEntry, schoolEntry1}}, nil)
lm.On("Search", schoolByIDSearch1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{schoolEntry}}, nil)
lm.On("Search", schoolByNumberSearch).Return(&ldap.SearchResult{Entries: []*ldap.Entry{schoolEntry}}, nil)
lm.On("Search", userByIDSearch1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{eduUserEntry}}, nil)
lm.On("Search", userByIDSearch2).Return(&ldap.SearchResult{Entries: []*ldap.Entry{}}, nil)
lm.On("Modify", mock.Anything).Return(nil)
lm.On("Modify", userToSchoolModRequest).Return(nil)
b, err := getMockedBackend(lm, eduConfig, &logger)
assert.Nil(t, err)
err = b.AddUsersToEducationSchool(context.Background(), "abcd-defg", []string{"does-not-exist"})
@@ -337,14 +372,19 @@ func TestAddUsersToEducationSchool(t *testing.T) {
err = b.AddUsersToEducationSchool(context.Background(), "abcd-defg", []string{"abcd-defg"})
lm.AssertNumberOfCalls(t, "Search", 7)
assert.Nil(t, err)
// try to add by school number (instead or id)
err = b.AddUsersToEducationSchool(context.Background(), "0123", []string{"abcd-defg"})
lm.AssertNumberOfCalls(t, "Search", 9)
assert.Nil(t, err)
}
func TestRemoveMemberFromEducationSchool(t *testing.T) {
lm := &mocks.Client{}
lm.On("Search", schoolByIDSearch1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{schoolEntry, schoolEntry1}}, nil)
lm.On("Search", schoolByIDSearch1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{schoolEntry}}, nil)
lm.On("Search", schoolByNumberSearch).Return(&ldap.SearchResult{Entries: []*ldap.Entry{schoolEntry}}, nil)
lm.On("Search", userByIDSearch1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{eduUserEntryWithSchool}}, nil)
lm.On("Search", userByIDSearch2).Return(&ldap.SearchResult{Entries: []*ldap.Entry{}}, nil)
lm.On("Modify", mock.Anything).Return(nil)
lm.On("Modify", userFromSchoolModRequest).Return(nil)
b, err := getMockedBackend(lm, eduConfig, &logger)
assert.Nil(t, err)
err = b.RemoveUserFromEducationSchool(context.Background(), "abcd-defg", "does-not-exist")
@@ -354,6 +394,10 @@ func TestRemoveMemberFromEducationSchool(t *testing.T) {
err = b.RemoveUserFromEducationSchool(context.Background(), "abcd-defg", "abcd-defg")
lm.AssertNumberOfCalls(t, "Search", 4)
lm.AssertNumberOfCalls(t, "Modify", 1)
// try to remove by school number (instead or id)
err = b.RemoveUserFromEducationSchool(context.Background(), "0123", "abcd-defg")
lm.AssertNumberOfCalls(t, "Search", 6)
lm.AssertNumberOfCalls(t, "Modify", 2)
assert.Nil(t, err)
}
@@ -368,10 +412,14 @@ var usersBySchoolIDSearch *ldap.SearchRequest = &ldap.SearchRequest{
func TestGetEducationSchoolUsers(t *testing.T) {
lm := &mocks.Client{}
lm.On("Search", schoolByIDSearch1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{schoolEntry, schoolEntry1}}, nil)
lm.On("Search", schoolByIDSearch1).Return(&ldap.SearchResult{Entries: []*ldap.Entry{schoolEntry}}, nil)
lm.On("Search", schoolByNumberSearch).Return(&ldap.SearchResult{Entries: []*ldap.Entry{schoolEntry}}, nil)
lm.On("Search", usersBySchoolIDSearch).Return(&ldap.SearchResult{Entries: []*ldap.Entry{eduUserEntryWithSchool}}, nil)
b, _ := getMockedBackend(lm, eduConfig, &logger)
users, err := b.GetEducationSchoolUsers(context.Background(), "abcd-defg")
assert.Nil(t, err)
assert.Equal(t, 1, len(users))
users, err = b.GetEducationSchoolUsers(context.Background(), "0123")
assert.Nil(t, err)
assert.Equal(t, 1, len(users))
}