Merge pull request #2421 from rhafer/issue/1599

feat(graph/education): Add support of 'eq' filters on users

Author: butonic

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"),
 			},
 		},
 	},

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)

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

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,32 +619,20 @@ 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
 		}
 	}
 
 	return nil
 }
 
 // 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,
 	}