diff --git a/pkg/coredata/membership.go b/pkg/coredata/membership.go index 6f5050656..599b83506 100644 --- a/pkg/coredata/membership.go +++ b/pkg/coredata/membership.go @@ -593,6 +593,7 @@ func (m *Memberships) LoadByOrganizationID( scope Scoper, organizationID gid.GID, cursor *page.Cursor[MembershipOrderField], + filter *MembershipFilter, ) error { query := ` WITH membership_with_profile AS ( @@ -615,6 +616,7 @@ WITH membership_with_profile AS ( WHERE m.organization_id = @organization_id AND m.%s + AND %s ) SELECT id, @@ -632,12 +634,13 @@ WHERE %s ` - query = fmt.Sprintf(query, scope.SQLFragment(), cursor.SQLFragment()) + query = fmt.Sprintf(query, scope.SQLFragment(), filter.SQLFragment(), cursor.SQLFragment()) args := pgx.StrictNamedArgs{ "organization_id": organizationID, } maps.Copy(args, scope.SQLArguments()) + maps.Copy(args, filter.SQLArguments()) maps.Copy(args, cursor.SQLArguments()) rows, err := conn.Query(ctx, query, args) @@ -659,21 +662,26 @@ func (m *Memberships) CountByOrganizationID( conn pg.Conn, scope Scoper, organizationID gid.GID, + filter *MembershipFilter, ) (int, error) { query := ` SELECT COUNT(*) FROM - iam_memberships + iam_memberships m +JOIN + identities i ON m.identity_id = i.id WHERE - organization_id = @organization_id + m.organization_id = @organization_id + AND m.%s AND %s ` - query = fmt.Sprintf(query, scope.SQLFragment()) + query = fmt.Sprintf(query, scope.SQLFragment(), filter.SQLFragment()) args := pgx.StrictNamedArgs{ "organization_id": organizationID, } maps.Copy(args, scope.SQLArguments()) + maps.Copy(args, filter.SQLArguments()) row := conn.QueryRow(ctx, query, args) var count int if err := row.Scan(&count); err != nil { diff --git a/pkg/coredata/membership_filter.go b/pkg/coredata/membership_filter.go new file mode 100644 index 000000000..4971b3193 --- /dev/null +++ b/pkg/coredata/membership_filter.go @@ -0,0 +1,54 @@ +// Copyright (c) 2025 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +package coredata + +import ( + "github.com/jackc/pgx/v5" + "go.probo.inc/probo/pkg/mail" +) + +type MembershipFilter struct { + email *mail.Addr +} + +func NewMembershipFilter() *MembershipFilter { + return &MembershipFilter{} +} + +func (f *MembershipFilter) WithEmail(email *mail.Addr) *MembershipFilter { + f.email = email + return f +} + +func (f *MembershipFilter) Email() *mail.Addr { + return f.email +} + +func (f *MembershipFilter) SQLArguments() pgx.StrictNamedArgs { + return pgx.StrictNamedArgs{ + "filter_email": f.email, + } +} + +func (f *MembershipFilter) SQLFragment() string { + return ` +( + CASE + WHEN @filter_email::text IS NOT NULL THEN + i.email_address = @filter_email::text + ELSE TRUE + END +)` +} diff --git a/pkg/iam/organization_service.go b/pkg/iam/organization_service.go index 2d1e18697..f4139e276 100644 --- a/pkg/iam/organization_service.go +++ b/pkg/iam/organization_service.go @@ -173,7 +173,7 @@ func (s *OrganizationService) CountMemberships( ctx, func(conn pg.Conn) (err error) { memberships := coredata.Memberships{} - count, err = memberships.CountByOrganizationID(ctx, conn, scope, organizationID) + count, err = memberships.CountByOrganizationID(ctx, conn, scope, organizationID, coredata.NewMembershipFilter()) if err != nil { return fmt.Errorf("cannot count memberships: %w", err) } @@ -824,7 +824,7 @@ func (s *OrganizationService) ListMembers( err := s.pg.WithConn( ctx, func(conn pg.Conn) error { - err := memberships.LoadByOrganizationID(ctx, conn, scope, organizationID, cursor) + err := memberships.LoadByOrganizationID(ctx, conn, scope, organizationID, cursor, coredata.NewMembershipFilter()) if err != nil { return fmt.Errorf("cannot load memberships: %w", err) } diff --git a/pkg/iam/scim/service.go b/pkg/iam/scim/service.go index 63a24e190..43b65b246 100644 --- a/pkg/iam/scim/service.go +++ b/pkg/iam/scim/service.go @@ -27,6 +27,7 @@ import ( "github.com/elimity-com/scim" scimerrors "github.com/elimity-com/scim/errors" "github.com/elimity-com/scim/optional" + scimfilter "github.com/scim2/filter-parser/v2" "go.gearno.de/kit/log" "go.gearno.de/kit/pg" "go.probo.inc/probo/pkg/coredata" @@ -245,44 +246,24 @@ func (s *Service) GetUser( func (s *Service) ListUsers( ctx context.Context, config *coredata.SCIMConfiguration, - filter *UserFilter, + filterExpr scimfilter.Expression, startIndex int, count int, ipAddress net.IP, ) ([]scim.Resource, int, error) { + filter, err := ParseUserFilter(filterExpr) + if err != nil { + return nil, 0, err + } + scope := coredata.NewScopeFromObjectID(config.OrganizationID) var memberships coredata.Memberships var totalCount int - err := s.pg.WithConn(ctx, func(conn pg.Conn) error { - // If we have a userName filter, query by email directly - if filter != nil && filter.UserName != nil { - emailAddr, err := mail.ParseAddr(*filter.UserName) - if err != nil { - // Invalid email format - return empty result - totalCount = 0 - return nil - } - - membership := &coredata.Membership{} - err = membership.LoadByEmailAndOrganization(ctx, conn, scope, emailAddr, config.OrganizationID) - if err == coredata.ErrResourceNotFound { - totalCount = 0 - return nil - } - if err != nil { - return fmt.Errorf("cannot load membership by email: %w", err) - } - - memberships = append(memberships, membership) - totalCount = 1 - return nil - } - - // No filter - return all memberships with pagination + err = s.pg.WithConn(ctx, func(conn pg.Conn) error { var err error - totalCount, err = memberships.CountByOrganizationID(ctx, conn, scope, config.OrganizationID) + totalCount, err = memberships.CountByOrganizationID(ctx, conn, scope, config.OrganizationID, filter) if err != nil { return fmt.Errorf("cannot count memberships: %w", err) } @@ -293,7 +274,7 @@ func (s *Service) ListUsers( } cursor := page.NewCursor(count, nil, page.Head, orderBy) - err = memberships.LoadByOrganizationID(ctx, conn, scope, config.OrganizationID, cursor) + err = memberships.LoadByOrganizationID(ctx, conn, scope, config.OrganizationID, cursor, filter) if err != nil { return fmt.Errorf("cannot load memberships: %w", err) } diff --git a/pkg/iam/scim/types.go b/pkg/iam/scim/types.go index 10e0ec862..7cefa1273 100644 --- a/pkg/iam/scim/types.go +++ b/pkg/iam/scim/types.go @@ -20,95 +20,70 @@ import ( scimerrors "github.com/elimity-com/scim/errors" scimfilter "github.com/scim2/filter-parser/v2" + "go.probo.inc/probo/pkg/coredata" + "go.probo.inc/probo/pkg/mail" ) -// UserFilter represents filter criteria for listing SCIM users -type UserFilter struct { - // UserName filters by userName (email) with exact match - UserName *string -} +func ParseUserFilter(expr scimfilter.Expression) (*coredata.MembershipFilter, error) { + filter := coredata.NewMembershipFilter() -// ParseUserFilter converts a SCIM filter AST expression to a UserFilter. -// Returns (nil, nil) if no filter is provided. -// Returns an error if the filter uses unsupported operators or attributes. -func ParseUserFilter(expr scimfilter.Expression) (*UserFilter, error) { if expr == nil { - return nil, nil + return filter, nil } - filter := &UserFilter{} + stack := []scimfilter.Expression{expr} - switch e := expr.(type) { - case *scimfilter.AttributeExpression: - if err := parseAttributeExpression(e, filter); err != nil { - return nil, err + for len(stack) > 0 { + current := stack[len(stack)-1] + stack = stack[:len(stack)-1] + + switch e := current.(type) { + case *scimfilter.AttributeExpression: + if e.Operator != scimfilter.EQ { + return nil, scimerrors.ScimErrorBadRequest( + fmt.Sprintf("operator '%s' is not supported, only 'eq' is supported", e.Operator)) + } + + value, ok := e.CompareValue.(string) + if !ok { + return nil, scimerrors.ScimErrorBadRequest("filter value must be a string") + } + + attrName := strings.ToLower(e.AttributePath.AttributeName) + switch attrName { + case "username": + email, err := mail.ParseAddr(value) + if err != nil { + return nil, scimerrors.ScimErrorBadRequest( + fmt.Sprintf("invalid email format for userName: %s", value)) + } + filter.WithEmail(&email) + default: + return nil, scimerrors.ScimErrorBadRequest( + fmt.Sprintf("attribute '%s' is not supported for filtering, only 'userName' is supported", e.AttributePath.AttributeName)) + } + + case *scimfilter.LogicalExpression: + if e.Operator != scimfilter.AND { + return nil, scimerrors.ScimErrorBadRequest( + fmt.Sprintf("logical operator '%s' is not supported, only 'and' is supported", e.Operator)) + } + stack = append(stack, e.Left, e.Right) + + case *scimfilter.NotExpression: + return nil, scimerrors.ScimErrorBadRequest("NOT expressions are not supported") + + case *scimfilter.ValuePath: + return nil, scimerrors.ScimErrorBadRequest("value path expressions are not supported") + + default: + return nil, scimerrors.ScimErrorBadRequest("unknown filter expression type") } - case *scimfilter.LogicalExpression: - if e.Operator != scimfilter.AND { - return nil, scimerrors.ScimErrorBadRequest(fmt.Sprintf("logical operator '%s' is not supported, only 'and' is supported", e.Operator)) - } - if err := parseLogicalExpression(e, filter); err != nil { - return nil, err - } - case *scimfilter.NotExpression: - return nil, scimerrors.ScimErrorBadRequest("NOT expressions are not supported") - case *scimfilter.ValuePath: - return nil, scimerrors.ScimErrorBadRequest("value path expressions are not supported") - default: - return nil, scimerrors.ScimErrorBadRequest("unknown filter expression type") } return filter, nil } -func parseAttributeExpression(e *scimfilter.AttributeExpression, filter *UserFilter) error { - // Only support "eq" operator - if e.Operator != scimfilter.EQ { - return scimerrors.ScimErrorBadRequest(fmt.Sprintf("operator '%s' is not supported, only 'eq' is supported", e.Operator)) - } - - // Get the attribute name (lowercase for comparison) - attrName := strings.ToLower(e.AttributePath.AttributeName) - - // Extract the string value - value, ok := e.CompareValue.(string) - if !ok { - return scimerrors.ScimErrorBadRequest("filter value must be a string") - } - - switch attrName { - case "username": - filter.UserName = &value - default: - return scimerrors.ScimErrorBadRequest(fmt.Sprintf("attribute '%s' is not supported for filtering, only 'userName' is supported", e.AttributePath.AttributeName)) - } - - return nil -} - -func parseLogicalExpression(e *scimfilter.LogicalExpression, filter *UserFilter) error { - // Process left expression - if left, ok := e.Left.(*scimfilter.AttributeExpression); ok { - if err := parseAttributeExpression(left, filter); err != nil { - return err - } - } else { - return scimerrors.ScimErrorBadRequest("nested logical expressions are not supported") - } - - // Process right expression - if right, ok := e.Right.(*scimfilter.AttributeExpression); ok { - if err := parseAttributeExpression(right, filter); err != nil { - return err - } - } else { - return scimerrors.ScimErrorBadRequest("nested logical expressions are not supported") - } - - return nil -} - -// User represents parsed SCIM user attributes with extracted values. type User struct { Email string FullName string diff --git a/pkg/iam/scim/types_test.go b/pkg/iam/scim/types_test.go new file mode 100644 index 000000000..2d736afb0 --- /dev/null +++ b/pkg/iam/scim/types_test.go @@ -0,0 +1,128 @@ +// Copyright (c) 2025 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +package scim + +import ( + "testing" + + scimfilter "github.com/scim2/filter-parser/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseUserFilter(t *testing.T) { + t.Run("nil expression returns empty filter", func(t *testing.T) { + filter, err := ParseUserFilter(nil) + require.NoError(t, err) + require.NotNil(t, filter) + assert.Nil(t, filter.Email()) + }) + + t.Run("simple userName eq filter", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`userName eq "test@example.com"`)) + require.NoError(t, err) + + filter, err := ParseUserFilter(expr) + require.NoError(t, err) + require.NotNil(t, filter) + require.NotNil(t, filter.Email()) + assert.Equal(t, "test@example.com", filter.Email().String()) + }) + + t.Run("userName filter is case insensitive", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`UserName eq "test@example.com"`)) + require.NoError(t, err) + + filter, err := ParseUserFilter(expr) + require.NoError(t, err) + require.NotNil(t, filter) + require.NotNil(t, filter.Email()) + assert.Equal(t, "test@example.com", filter.Email().String()) + }) + + t.Run("logical AND expression", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`userName eq "user1@example.com" and userName eq "user2@example.com"`)) + require.NoError(t, err) + + filter, err := ParseUserFilter(expr) + require.NoError(t, err) + require.NotNil(t, filter) + // Last processed value wins + require.NotNil(t, filter.Email()) + }) + + t.Run("unsupported operator returns error", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`userName co "test"`)) + require.NoError(t, err) + + filter, err := ParseUserFilter(expr) + assert.Error(t, err) + assert.Nil(t, filter) + assert.Contains(t, err.Error(), "operator") + assert.Contains(t, err.Error(), "not supported") + }) + + t.Run("unsupported attribute returns error", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`displayName eq "John"`)) + require.NoError(t, err) + + filter, err := ParseUserFilter(expr) + assert.Error(t, err) + assert.Nil(t, filter) + assert.Contains(t, err.Error(), "displayName") + assert.Contains(t, err.Error(), "not supported") + }) + + t.Run("OR operator returns error", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`userName eq "a@b.com" or userName eq "c@d.com"`)) + require.NoError(t, err) + + filter, err := ParseUserFilter(expr) + assert.Error(t, err) + assert.Nil(t, filter) + assert.Contains(t, err.Error(), "logical operator") + assert.Contains(t, err.Error(), "not supported") + }) + + t.Run("NOT expression returns error", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`not userName eq "test@example.com"`)) + require.NoError(t, err) + + filter, err := ParseUserFilter(expr) + assert.Error(t, err) + assert.Nil(t, filter) + assert.Contains(t, err.Error(), "NOT expressions are not supported") + }) + + t.Run("nested AND expressions", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`(userName eq "a@b.com" and userName eq "c@d.com") and userName eq "e@f.com"`)) + require.NoError(t, err) + + filter, err := ParseUserFilter(expr) + require.NoError(t, err) + require.NotNil(t, filter) + require.NotNil(t, filter.Email()) + }) + + t.Run("invalid email format returns error", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`userName eq "not-an-email"`)) + require.NoError(t, err) + + filter, err := ParseUserFilter(expr) + assert.Error(t, err) + assert.Nil(t, filter) + assert.Contains(t, err.Error(), "invalid email format") + }) +} diff --git a/pkg/server/api/connect/v1/scim_handler.go b/pkg/server/api/connect/v1/scim_handler.go index e336bb75b..238394453 100644 --- a/pkg/server/api/connect/v1/scim_handler.go +++ b/pkg/server/api/connect/v1/scim_handler.go @@ -174,18 +174,7 @@ func (h *scimResourceHandler) GetAll(r *http.Request, params scim.ListRequestPar return scim.Page{}, scimerrors.ScimErrorBadRequest(err.Error()) } - // Parse SCIM filter AST into our filter type - filter, err := scimservice.ParseUserFilter(params.FilterValidator.GetFilter()) - if err != nil { - var scimErr scimerrors.ScimError - if errors.As(err, &scimErr) { - return scim.Page{}, err - } - h.handler.logger.ErrorCtx(ctx, "cannot parse filter", log.Error(err)) - return scim.Page{}, scimerrors.ScimErrorInternal - } - - resources, totalCount, err := h.handler.iam.SCIMService.ListUsers(ctx, config, filter, params.StartIndex, params.Count, getIPAddress(r)) + resources, totalCount, err := h.handler.iam.SCIMService.ListUsers(ctx, config, params.FilterValidator.GetFilter(), params.StartIndex, params.Count, getIPAddress(r)) if err != nil { var scimErr scimerrors.ScimError if errors.As(err, &scimErr) {