diff --git a/cmd/setup/76.go b/cmd/setup/76.go new file mode 100644 index 000000000..3a839146e --- /dev/null +++ b/cmd/setup/76.go @@ -0,0 +1,93 @@ +package setup + +import ( + "context" + "database/sql" + "embed" + "fmt" + + "github.com/zitadel/zitadel/backend/v3/instrumentation/logging" + "github.com/zitadel/zitadel/internal/database" + "github.com/zitadel/zitadel/internal/eventstore" +) + +var ( + //go:embed 76/*.sql + users14LoginEqualityIndexes embed.FS +) + +const ( + users14TableExistsQuery = "SELECT exists(SELECT 1 FROM information_schema.tables WHERE table_schema = 'projections' AND table_name = 'users14')" + users14InvalidIndexQuery = `SELECT EXISTS ( + SELECT 1 + FROM pg_index i + JOIN pg_class c ON c.oid = i.indexrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = 'projections' + AND c.relname = $1 + AND NOT i.indisvalid +)` + users14UsernameLowerIdx = "users14_username_lower_idx" + users14HumansPhoneLowerIdx = "users14_humans_phone_lower_idx" + dropInvalidIndexConcurrently = "DROP INDEX CONCURRENTLY IF EXISTS projections." +) + +var users14LoginEqualityIndexNames = []string{ + users14UsernameLowerIdx, + users14HumansPhoneLowerIdx, +} + +type Users14LoginEqualityIndexes struct { + dbClient *database.DB +} + +func (mig *Users14LoginEqualityIndexes) Execute(ctx context.Context, _ eventstore.Event) error { + var exists bool + err := mig.dbClient.QueryRowContext(ctx, func(r *sql.Row) error { + return r.Scan(&exists) + }, users14TableExistsQuery) + if err != nil || !exists { + return err + } + + if err := mig.dropInvalidIndexes(ctx); err != nil { + return err + } + + statements, err := readStatements(users14LoginEqualityIndexes, "76") + if err != nil { + return err + } + for _, stmt := range statements { + logging.Info(ctx, "execute statement", "file", stmt.file, "migration", mig.String()) + if _, err := mig.dbClient.ExecContext(ctx, stmt.query); err != nil { + return fmt.Errorf("%s %s: %w", mig.String(), stmt.file, err) + } + } + return nil +} + +func (mig *Users14LoginEqualityIndexes) dropInvalidIndexes(ctx context.Context) error { + for _, name := range users14LoginEqualityIndexNames { + var invalid bool + err := mig.dbClient.QueryRowContext(ctx, func(r *sql.Row) error { + return r.Scan(&invalid) + }, users14InvalidIndexQuery, name) + if err != nil { + return fmt.Errorf("%s check invalid index %s: %w", mig.String(), name, err) + } + if !invalid { + continue + } + drop := dropInvalidIndexConcurrently + name + logging.Info(ctx, "drop invalid leftover index", "index", name, "migration", mig.String()) + if _, err := mig.dbClient.ExecContext(ctx, drop); err != nil { + return fmt.Errorf("%s drop invalid index %s: %w", mig.String(), name, err) + } + } + return nil +} + +func (mig *Users14LoginEqualityIndexes) String() string { + return "76_users14_login_equality_indexes" +} diff --git a/cmd/setup/76/01_users14_username_lower_idx.sql b/cmd/setup/76/01_users14_username_lower_idx.sql new file mode 100644 index 000000000..e48fd4500 --- /dev/null +++ b/cmd/setup/76/01_users14_username_lower_idx.sql @@ -0,0 +1 @@ +CREATE INDEX CONCURRENTLY IF NOT EXISTS users14_username_lower_idx ON projections.users14 (instance_id, LOWER(username)); diff --git a/cmd/setup/76/02_users14_humans_phone_lower_idx.sql b/cmd/setup/76/02_users14_humans_phone_lower_idx.sql new file mode 100644 index 000000000..a4553c563 --- /dev/null +++ b/cmd/setup/76/02_users14_humans_phone_lower_idx.sql @@ -0,0 +1 @@ +CREATE INDEX CONCURRENTLY IF NOT EXISTS users14_humans_phone_lower_idx ON projections.users14_humans (instance_id, LOWER(phone)); diff --git a/cmd/setup/76_test.go b/cmd/setup/76_test.go new file mode 100644 index 000000000..618327a85 --- /dev/null +++ b/cmd/setup/76_test.go @@ -0,0 +1,92 @@ +package setup + +import ( + "context" + "regexp" + "testing" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zitadel/zitadel/internal/database" +) + +const ( + users14UsernameLowerCreate = "CREATE INDEX CONCURRENTLY IF NOT EXISTS users14_username_lower_idx ON projections.users14 (instance_id, LOWER(username));\n" + users14PhoneLowerCreate = "CREATE INDEX CONCURRENTLY IF NOT EXISTS users14_humans_phone_lower_idx ON projections.users14_humans (instance_id, LOWER(phone));\n" +) + +func TestUsers14LoginEqualityIndexes_Execute(t *testing.T) { + tests := []struct { + name string + expects func(sqlmock.Sqlmock) + wantErr bool + }{ + { + name: "table missing, no drop or create", + expects: func(mock sqlmock.Sqlmock) { + mock.ExpectQuery(regexp.QuoteMeta(users14TableExistsQuery)). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) + }, + }, + { + name: "table exists, indexes valid, creates both concurrently", + expects: func(mock sqlmock.Sqlmock) { + expectUsers14TableExists(mock, true) + expectInvalidIndex(mock, users14UsernameLowerIdx, false) + expectInvalidIndex(mock, users14HumansPhoneLowerIdx, false) + mock.ExpectExec(regexp.QuoteMeta(users14UsernameLowerCreate)). + WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectExec(regexp.QuoteMeta(users14PhoneLowerCreate)). + WillReturnResult(sqlmock.NewResult(0, 0)) + }, + }, + { + name: "username index invalid, drop then create both", + expects: func(mock sqlmock.Sqlmock) { + expectUsers14TableExists(mock, true) + expectInvalidIndex(mock, users14UsernameLowerIdx, true) + mock.ExpectExec(regexp.QuoteMeta(dropInvalidIndexConcurrently + users14UsernameLowerIdx)). + WillReturnResult(sqlmock.NewResult(0, 0)) + expectInvalidIndex(mock, users14HumansPhoneLowerIdx, false) + mock.ExpectExec(regexp.QuoteMeta(users14UsernameLowerCreate)). + WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectExec(regexp.QuoteMeta(users14PhoneLowerCreate)). + WillReturnResult(sqlmock.NewResult(0, 0)) + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + db, mock, err := sqlmock.New() + require.NoError(t, err) + defer func() { + require.NoError(t, mock.ExpectationsWereMet()) + }() + defer db.Close() + tt.expects(mock) + + mig := &Users14LoginEqualityIndexes{ + dbClient: &database.DB{DB: db}, + } + err = mig.Execute(context.Background(), nil) + if tt.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + }) + } +} + +func expectUsers14TableExists(mock sqlmock.Sqlmock, exists bool) { + mock.ExpectQuery(regexp.QuoteMeta(users14TableExistsQuery)). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(exists)) +} + +func expectInvalidIndex(mock sqlmock.Sqlmock, name string, invalid bool) { + mock.ExpectQuery(regexp.QuoteMeta(users14InvalidIndexQuery)). + WithArgs(name). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(invalid)) +} diff --git a/cmd/setup/config.go b/cmd/setup/config.go index f0eb6e637..0cfc5c15f 100644 --- a/cmd/setup/config.go +++ b/cmd/setup/config.go @@ -195,6 +195,7 @@ type Steps struct { s73FixUserGrantRoles *FixUserGrantRoles s74Apps7OIDCConfigsAddRegistrationToken *Apps7OIDCConfigsAddRegistrationToken s75Apps7OIDCConfigsAddAppLinkConfig *Apps7OIDCConfigsAddAppLinkConfig + s76Users14LoginEqualityIndexes *Users14LoginEqualityIndexes RelationalTables *TransactionalTables } diff --git a/cmd/setup/setup.go b/cmd/setup/setup.go index 4c9942d81..4013b6071 100644 --- a/cmd/setup/setup.go +++ b/cmd/setup/setup.go @@ -258,6 +258,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string) steps.s73FixUserGrantRoles = &FixUserGrantRoles{eventstore: eventstoreClient} steps.s74Apps7OIDCConfigsAddRegistrationToken = &Apps7OIDCConfigsAddRegistrationToken{dbClient: dbClient} steps.s75Apps7OIDCConfigsAddAppLinkConfig = &Apps7OIDCConfigsAddAppLinkConfig{dbClient: dbClient} + steps.s76Users14LoginEqualityIndexes = &Users14LoginEqualityIndexes{dbClient: dbClient} err = projection.Create(ctx, dbClient, eventstoreClient, config.Projections, nil, nil, nil) if err != nil { @@ -316,6 +317,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string) steps.s67SyncMemberRoleFields, steps.s69CacheTablesLogged, steps.s70AddEventStoreCommandEnforceOwner, + steps.s76Users14LoginEqualityIndexes, } { setupErr = executeMigration(ctx, eventstoreClient, step, "migration failed") if setupErr != nil { diff --git a/internal/api/grpc/user/v2/integration_test/query_test.go b/internal/api/grpc/user/v2/integration_test/query_test.go index f96adb556..1786d0889 100644 --- a/internal/api/grpc/user/v2/integration_test/query_test.go +++ b/internal/api/grpc/user/v2/integration_test/query_test.go @@ -1252,6 +1252,64 @@ func TestServer_SystemUsers_ListUsers(t *testing.T) { } } +func TestServer_ListUsers_UsernameAndOr(t *testing.T) { + iamOwnerCtx := InstancePermissionV2.WithAuthorizationToken(OrgCTX, integration.UserTypeIAMOwner) + orgResp := InstancePermissionV2.CreateOrganization(iamOwnerCtx, integration.OrganizationName(), integration.Email()) + alice := createUser(iamOwnerCtx, InstancePermissionV2, orgResp.OrganizationId, false) + bob := createUser(iamOwnerCtx, InstancePermissionV2, orgResp.OrganizationId, false) + orgQuery := OrganizationIdQuery(orgResp.OrganizationId) + + tests := []struct { + name string + queries []*user.SearchQuery + wantUsernames []string + }{ + { + name: "org and username or username", + queries: []*user.SearchQuery{ + orgQuery, + OrQuery([]*user.SearchQuery{UsernameQuery(alice.Username), UsernameQuery(bob.Username)}), + }, + wantUsernames: []string{alice.Username, bob.Username}, + }, + { + name: "org and sibling usernames", + queries: []*user.SearchQuery{ + orgQuery, + UsernameQuery(alice.Username), + UsernameQuery(bob.Username), + }, + }, + { + name: "nested and of or and org", + queries: []*user.SearchQuery{ + AndQuery([]*user.SearchQuery{ + OrQuery([]*user.SearchQuery{UsernameQuery(alice.Username), UsernameQuery(bob.Username)}), + orgQuery, + }), + }, + wantUsernames: []string{alice.Username, bob.Username}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := &user.ListUsersRequest{Queries: tt.queries} + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(iamOwnerCtx, 20*time.Second) + require.EventuallyWithT(t, func(ttt *assert.CollectT) { + got, err := InstancePermissionV2.Client.UserV2.ListUsers(iamOwnerCtx, req) + require.NoError(ttt, err) + + gotUsernames := make([]string, 0, len(got.GetResult())) + for _, u := range got.GetResult() { + gotUsernames = append(gotUsernames, u.GetUsername()) + } + assert.ElementsMatch(ttt, tt.wantUsernames, gotUsernames) + }, retryDuration, tick, "timeout waiting for expected user result") + }) + } +} + func InUserIDsQuery(ids []string) *user.SearchQuery { return &user.SearchQuery{ Query: &user.SearchQuery_InUserIdsQuery{ @@ -1312,6 +1370,16 @@ func OrQuery(queries []*user.SearchQuery) *user.SearchQuery { } } +func AndQuery(queries []*user.SearchQuery) *user.SearchQuery { + return &user.SearchQuery{ + Query: &user.SearchQuery_AndQuery{ + AndQuery: &user.AndQuery{ + Queries: queries, + }, + }, + } +} + func MetadataKeyContainsQuery(metadataKey string) *user.SearchQuery { return &user.SearchQuery{ Query: &user.SearchQuery_MetadataKeyFilter{ diff --git a/internal/query/projection/user.go b/internal/query/projection/user.go index d11c4855f..1a1d56e4e 100644 --- a/internal/query/projection/user.go +++ b/internal/query/projection/user.go @@ -101,6 +101,7 @@ func (*userProjection) Init() *old_handler.Check { }, handler.NewPrimaryKey(UserInstanceIDCol, UserIDCol), handler.WithIndex(handler.NewIndex("username", []string{UserUsernameCol})), + handler.WithIndex(handler.NewIndex("username_lower", []string{UserInstanceIDCol, "LOWER(" + UserUsernameCol + ")"})), handler.WithIndex(handler.NewIndex("resource_owner", []string{UserResourceOwnerCol})), ), handler.NewSuffixedTable([]*handler.InitColumn{ @@ -125,6 +126,7 @@ func (*userProjection) Init() *old_handler.Check { UserHumanSuffix, handler.WithForeignKey(handler.NewForeignKeyOfPublicKeys()), handler.WithIndex(handler.NewIndex("email", []string{HumanUserInstanceIDCol, "LOWER(" + HumanEmailCol + ")"})), + handler.WithIndex(handler.NewIndex("phone_lower", []string{HumanUserInstanceIDCol, "LOWER(" + HumanPhoneCol + ")"})), ), handler.NewSuffixedTable([]*handler.InitColumn{ handler.NewColumn(MachineUserIDCol, handler.ColumnTypeText), diff --git a/internal/query/user.go b/internal/query/user.go index 78018d9ec..b07e28893 100644 --- a/internal/query/user.go +++ b/internal/query/user.go @@ -747,7 +747,7 @@ func NewUserResourceOwnerSearchQuery(value string, comparison TextComparison) (S } func NewUserUsernameSearchQuery(value string, comparison TextComparison) (SearchQuery, error) { - return NewTextQuery(UserUsernameCol, value, comparison) + return newLowerEqualsSearchQuery(UserUsernameCol, userTable, UserIDCol, value, comparison) } func NewUserFirstNameSearchQuery(value string, comparison TextComparison) (SearchQuery, error) { @@ -767,11 +767,11 @@ func NewUserDisplayNameSearchQuery(value string, comparison TextComparison) (Sea } func NewUserEmailSearchQuery(value string, comparison TextComparison) (SearchQuery, error) { - return NewTextQuery(HumanEmailCol, value, comparison) + return newLowerEqualsSearchQuery(HumanEmailCol, humanTable, HumanUserIDCol, value, comparison) } func NewUserPhoneSearchQuery(value string, comparison TextComparison) (SearchQuery, error) { - return NewTextQuery(HumanPhoneCol, value, comparison) + return newLowerEqualsSearchQuery(HumanPhoneCol, humanTable, HumanUserIDCol, value, comparison) } func NewUserVerifiedEmailSearchQuery(value string) (SearchQuery, error) { @@ -801,8 +801,8 @@ var userLoginNameMatchesQuery string var userLoginNameMatchesCaseSensitiveQuery string // NewUserLoginNameExistsQuery filters users by login name. -// Equals / EqualsIgnoreCase use a planner marker rewritten in prepareUsersQuery -// into an indexed join on login_names3_users. Other comparisons use the view. +// Equals / EqualsIgnoreCase are rewritten in prepareUsersQuery into an indexed +// ID-seek. Other comparisons use the login_names3 view. func NewUserLoginNameExistsQuery(value string, comparison TextComparison) (SearchQuery, error) { if comparison == TextEquals || comparison == TextEqualsIgnoreCase { return newLoginNameEqualsFilter(value, comparison == TextEqualsIgnoreCase) @@ -842,126 +842,6 @@ func newLoginNameExistsViewQuery(value string, comparison TextComparison) (Searc ) } -// loginNameEqualsFilter marks a login-name equality filter for prepareUsersQuery -// to rewrite as an indexed join. Unextracted markers (e.g. inside OrQuery) fall -// back to the view-based exists query. -type loginNameEqualsFilter struct { - username string - domain string - loginName string - ignoreCase bool -} - -func newLoginNameEqualsFilter(value string, ignoreCase bool) (*loginNameEqualsFilter, error) { - if ignoreCase { - value = strings.ToLower(value) - } - username := value - domainIndex := strings.LastIndex(value, "@") - var domainSuffix string - // split between the last @ (so ignore it if the login name ends with it) - if domainIndex > 0 && domainIndex != len(value)-1 { - domainSuffix = value[domainIndex+1:] - username = value[:domainIndex] - } - return &loginNameEqualsFilter{ - username: username, - domain: domainSuffix, - loginName: value, - ignoreCase: ignoreCase, - }, nil -} - -func (q *loginNameEqualsFilter) comparison() TextComparison { - if q.ignoreCase { - return TextEqualsIgnoreCase - } - return TextEquals -} - -func (q *loginNameEqualsFilter) fallback() SearchQuery { - // Equals / EqualsIgnoreCase construction cannot fail for valid columns. - fallback, _ := newLoginNameExistsViewQuery(q.loginName, q.comparison()) - return fallback -} - -func (q *loginNameEqualsFilter) toQuery(query sq.SelectBuilder) sq.SelectBuilder { - return q.fallback().toQuery(query) -} - -func (q *loginNameEqualsFilter) Col() Column { - return UserIDCol -} - -func (q *loginNameEqualsFilter) comp() sq.Sqlizer { - return q.fallback().comp() -} - -func (q *loginNameEqualsFilter) matchesArgs(instanceID string) []interface{} { - return []interface{}{ - instanceID, - instanceID, - q.domain, - instanceID, - q.username, - q.loginName, - q.username, - q.domain, - q.loginName, - } -} - -func (q *loginNameEqualsFilter) joinMatches(instanceID string) sq.Sqlizer { - subQuery := userLoginNameMatchesQuery - if !q.ignoreCase { - subQuery = userLoginNameMatchesCaseSensitiveQuery - } - return sq.Expr( - "INNER JOIN ("+subQuery+") AS login_name_matches ON "+UserIDCol.identifier()+" = login_name_matches.user_id", - q.matchesArgs(instanceID)..., - ) -} - -// extractLoginNameEqualsFilter extracts one login-name equals marker from -// top-level or AndQuery filters. Markers inside OrQuery / NotQuery are kept. -func extractLoginNameEqualsFilter(queries []SearchQuery) (filter *loginNameEqualsFilter, remaining []SearchQuery, ok bool) { - remaining = make([]SearchQuery, 0, len(queries)) - var found *loginNameEqualsFilter - - for _, qry := range queries { - switch v := qry.(type) { - case *loginNameEqualsFilter: - if found != nil { - return nil, queries, false - } - found = v - case *AndQuery: - inner, rest, extracted := extractLoginNameEqualsFilter(v.queries) - if !extracted { - remaining = append(remaining, qry) - continue - } - if found != nil { - return nil, queries, false - } - found = inner - if len(rest) == 1 { - remaining = append(remaining, rest[0]) - } else if len(rest) > 1 { - andQuery, _ := NewAndQuery(rest...) - remaining = append(remaining, andQuery) - } - default: - remaining = append(remaining, qry) - } - } - - if found == nil { - return nil, queries, false - } - return found, remaining, true -} - func (q *UserSearchQueries) hasMetadataFilter() bool { return searchQueriesHaveMetadataFilter(q.Queries) } @@ -1416,18 +1296,15 @@ func prepareUserUniqueQuery() (sq.SelectBuilder, func(*sql.Row) (bool, error)) { // It is not possible to pass more filters to the returned query, as they need to be applied in the sub-select. // // Metadata JOIN and DISTINCT are only applied when a metadata filter is present. -// Login-name equals markers are rewritten into an indexed join when extractable. +// Login-equality filters (username, email, phone, login name EQUALS / +// EQUALS_IGNORE_CASE) become an indexed UNION of ID seeks. func (q *UserSearchQueries) prepareUsersQuery(ctx context.Context, permissionCheckV2 bool) (sq.SelectBuilder, func(*sql.Rows) (*Users, error)) { if q.SortingColumn.isZero() { q.SortingColumn = UserIDCol } instanceID := authz.GetInstance(ctx).InstanceID() - loginNameFilter, remainingFilters, loginNameExtracted := extractLoginNameEqualsFilter(q.Queries) - filters := q.Queries - if loginNameExtracted { - filters = remainingFilters - } + loginEqualitySeeks, filters, loginEqualityExtracted := extractLoginEqualitySeeks(instanceID, q.Queries) needsMetadataJoin := q.hasMetadataFilter() // start building the sub-select @@ -1469,8 +1346,8 @@ func (q *UserSearchQueries) prepareUsersQuery(ctx context.Context, permissionChe JoinClause(joinLoginNames). Where(sq.Eq{UserInstanceIDCol.identifier(): instanceID}) - if loginNameExtracted { - query = query.JoinClause(loginNameFilter.joinMatches(instanceID)) + if loginEqualityExtracted { + query = query.JoinClause(joinLoginEqualitySeeks(loginEqualitySeeks)) } if needsMetadataJoin { diff --git a/internal/query/user_login_equality.go b/internal/query/user_login_equality.go new file mode 100644 index 000000000..deb9128b9 --- /dev/null +++ b/internal/query/user_login_equality.go @@ -0,0 +1,227 @@ +package query + +import ( + "strings" + + sq "github.com/Masterminds/squirrel" +) + +type loginEqualitySeek struct { + sql string + args []interface{} +} + +// loginNameEqualsFilter marks login-name equality for an indexed ID-seek in +// prepareUsersQuery. Unextracted markers fall back to the view-based exists query. +type loginNameEqualsFilter struct { + username string + domain string + loginName string + ignoreCase bool +} + +func newLoginNameEqualsFilter(value string, ignoreCase bool) (*loginNameEqualsFilter, error) { + if ignoreCase { + value = strings.ToLower(value) + } + username := value + domainIndex := strings.LastIndex(value, "@") + var domainSuffix string + // split between the last @ (so ignore it if the login name ends with it) + if domainIndex > 0 && domainIndex != len(value)-1 { + domainSuffix = value[domainIndex+1:] + username = value[:domainIndex] + } + return &loginNameEqualsFilter{ + username: username, + domain: domainSuffix, + loginName: value, + ignoreCase: ignoreCase, + }, nil +} + +func (q *loginNameEqualsFilter) comparison() TextComparison { + if q.ignoreCase { + return TextEqualsIgnoreCase + } + return TextEquals +} + +func (q *loginNameEqualsFilter) fallback() SearchQuery { + fallback, _ := newLoginNameExistsViewQuery(q.loginName, q.comparison()) + return fallback +} + +func (q *loginNameEqualsFilter) toQuery(query sq.SelectBuilder) sq.SelectBuilder { + return q.fallback().toQuery(query) +} + +func (q *loginNameEqualsFilter) Col() Column { + return UserIDCol +} + +func (q *loginNameEqualsFilter) comp() sq.Sqlizer { + return q.fallback().comp() +} + +func (q *loginNameEqualsFilter) matchesArgs(instanceID string) []interface{} { + return []interface{}{ + instanceID, + instanceID, + q.domain, + instanceID, + q.username, + q.loginName, + q.username, + q.domain, + q.loginName, + } +} + +func (q *loginNameEqualsFilter) idSeek(instanceID string) loginEqualitySeek { + // userLoginNameMatchesQuery uses *_lower columns; the _case_sensitive embed uses raw columns. + subQuery := userLoginNameMatchesQuery + if !q.ignoreCase { + subQuery = userLoginNameMatchesCaseSensitiveQuery + } + return loginEqualitySeek{ + sql: "SELECT user_id AS id FROM (" + subQuery + ") AS login_name_matches", + args: q.matchesArgs(instanceID), + } +} + +// lowerEqualsFilter is username, email, or phone EQUALS / EQUALS_IGNORE_CASE. +// Unextracted filters compile to the same WHERE clause as textQuery. +type lowerEqualsFilter struct { + textQuery + tbl table + idCol Column +} + +func newLowerEqualsSearchQuery(valueCol Column, tbl table, idCol Column, value string, comparison TextComparison) (SearchQuery, error) { + if comparison != TextEquals && comparison != TextEqualsIgnoreCase { + return NewTextQuery(valueCol, value, comparison) + } + tq, err := NewTextQuery(valueCol, value, comparison) + if err != nil { + return nil, err + } + return &lowerEqualsFilter{textQuery: *tq, tbl: tbl, idCol: idCol}, nil +} + +func (q *lowerEqualsFilter) idSeek(instanceID string) loginEqualitySeek { + sql := "SELECT " + q.idCol.identifier() + " AS id FROM " + q.tbl.identifier() + + " WHERE " + q.tbl.InstanceIDIdentifier() + " = ? AND LOWER(" + q.Column.identifier() + ") = " + args := []interface{}{instanceID} + if q.Compare == TextEquals { + sql += "LOWER(?) AND " + q.Column.identifier() + " = ?" + args = append(args, q.Text, q.Text) + } else { + sql += "?" + args = append(args, strings.ToLower(q.Text)) + } + return loginEqualitySeek{sql: sql, args: args} +} + +func loginEqualitySeekFromLeaf(instanceID string, qry SearchQuery) (loginEqualitySeek, bool) { + switch v := qry.(type) { + case *loginNameEqualsFilter: + if v.loginName == "" { + return loginEqualitySeek{}, false + } + return v.idSeek(instanceID), true + case *lowerEqualsFilter: + if v.Text == "" { + return loginEqualitySeek{}, false + } + return v.idSeek(instanceID), true + default: + return loginEqualitySeek{}, false + } +} + +func loginEqualitySeeksFromQuery(instanceID string, qry SearchQuery) ([]loginEqualitySeek, bool) { + if seek, ok := loginEqualitySeekFromLeaf(instanceID, qry); ok { + return []loginEqualitySeek{seek}, true + } + or, ok := qry.(*OrQuery) + if !ok { + return nil, false + } + seeks := make([]loginEqualitySeek, 0, len(or.queries)) + for _, inner := range or.queries { + seek, ok := loginEqualitySeekFromLeaf(instanceID, inner) + if !ok { + return nil, false + } + seeks = append(seeks, seek) + } + if len(seeks) == 0 { + return nil, false + } + return seeks, true +} + +// extractLoginEqualitySeeks pulls one login-equality filter from queries. +// A match may be a top-level equals leaf, a flat OrQuery of those leaves, or +// the same nested under AndQuery. Remaining AND conjuncts stay as WHERE. +// Equality nested under OrQuery or NotQuery is not extracted. +func extractLoginEqualitySeeks(instanceID string, queries []SearchQuery) ([]loginEqualitySeek, []SearchQuery, bool) { + for i, qry := range queries { + if and, ok := qry.(*AndQuery); ok { + seeks, andRest, ok := extractLoginEqualitySeeks(instanceID, and.queries) + if !ok { + continue + } + return seeks, spliceRemainingQuery(queries, i, remainingAndQuery(andRest)), true + } + seeks, ok := loginEqualitySeeksFromQuery(instanceID, qry) + if !ok { + continue + } + return seeks, spliceRemainingQuery(queries, i, nil), true + } + return nil, queries, false +} + +func remainingAndQuery(rest []SearchQuery) SearchQuery { + switch len(rest) { + case 0: + return nil + case 1: + return rest[0] + default: + and, err := NewAndQuery(rest...) + if err != nil { + return nil + } + return and + } +} + +func spliceRemainingQuery(queries []SearchQuery, i int, replacement SearchQuery) []SearchQuery { + n := len(queries) - 1 + if replacement != nil { + n++ + } + remaining := make([]SearchQuery, 0, n) + remaining = append(remaining, queries[:i]...) + if replacement != nil { + remaining = append(remaining, replacement) + } + remaining = append(remaining, queries[i+1:]...) + return remaining +} + +func joinLoginEqualitySeeks(seeks []loginEqualitySeek) sq.Sqlizer { + parts := make([]string, len(seeks)) + args := make([]interface{}, 0, len(seeks)*2) + for i, seek := range seeks { + parts[i] = seek.sql + args = append(args, seek.args...) + } + return sq.Expr( + "INNER JOIN ("+strings.Join(parts, " UNION ")+") AS matches ON "+UserIDCol.identifier()+" = matches.id", + args..., + ) +} diff --git a/internal/query/user_login_name_exists_test.go b/internal/query/user_login_name_exists_test.go index 430c9aacd..5fb2c87a4 100644 --- a/internal/query/user_login_name_exists_test.go +++ b/internal/query/user_login_name_exists_test.go @@ -47,21 +47,33 @@ func TestNewUserLoginNameExistsQuery_ContainsFallsBackToView(t *testing.T) { assert.Contains(t, strings.ToLower(sql), "login_name") } -func TestExtractLoginNameEqualsFilter_TopLevel(t *testing.T) { +func TestExtractLoginEqualitySeeks_LoginNameAndOrg(t *testing.T) { loginNameQuery, err := NewUserLoginNameExistsQuery("user@org.localhost", TextEqualsIgnoreCase) require.NoError(t, err) orgQuery, err := NewUserResourceOwnerSearchQuery("org1", TextEquals) require.NoError(t, err) - filter, remaining, ok := extractLoginNameEqualsFilter([]SearchQuery{loginNameQuery, orgQuery}) + seeks, remaining, ok := extractLoginEqualitySeeks("inst-1", []SearchQuery{loginNameQuery, orgQuery}) require.True(t, ok) - require.NotNil(t, filter) - assert.Equal(t, "user", filter.username) + require.Len(t, seeks, 1) + assert.Contains(t, seeks[0].sql, "login_name_matches") require.Len(t, remaining, 1) assert.Equal(t, orgQuery, remaining[0]) } -func TestExtractLoginNameEqualsFilter_SkipsOrQuery(t *testing.T) { +func TestNewUserEmailSearchQuery_EqualsIsLowerEqualsFilter(t *testing.T) { + qry, err := NewUserEmailSearchQuery("Ada@Example.com", TextEquals) + require.NoError(t, err) + _, ok := qry.(*lowerEqualsFilter) + require.True(t, ok) + + contains, err := NewUserEmailSearchQuery("Ada", TextContains) + require.NoError(t, err) + _, ok = contains.(*lowerEqualsFilter) + assert.False(t, ok) +} + +func TestExtractLoginEqualitySeeks_OrLoginNameAndEmail(t *testing.T) { loginNameQuery, err := NewUserLoginNameExistsQuery("user@org.localhost", TextEqualsIgnoreCase) require.NoError(t, err) emailQuery, err := NewUserEmailSearchQuery("user@example.com", TextEqualsIgnoreCase) @@ -69,12 +81,45 @@ func TestExtractLoginNameEqualsFilter_SkipsOrQuery(t *testing.T) { orQuery, err := NewOrQuery(loginNameQuery, emailQuery) require.NoError(t, err) - _, remaining, ok := extractLoginNameEqualsFilter([]SearchQuery{orQuery}) + seeks, remaining, ok := extractLoginEqualitySeeks("inst-1", []SearchQuery{orQuery}) + require.True(t, ok) + require.Len(t, seeks, 2) + assert.Contains(t, seeks[0].sql, "login_name_matches") + assert.Contains(t, seeks[1].sql, "LOWER("+HumanEmailCol.identifier()+")") + assert.Empty(t, remaining) +} + +func TestExtractLoginEqualitySeeks_EmptyOrArmDoesNotExtract(t *testing.T) { + emailQuery, err := NewUserEmailSearchQuery("", TextEqualsIgnoreCase) + require.NoError(t, err) + phoneQuery, err := NewUserPhoneSearchQuery("+41000000000", TextEquals) + require.NoError(t, err) + orQuery, err := NewOrQuery(emailQuery, phoneQuery) + require.NoError(t, err) + + seeks, remaining, ok := extractLoginEqualitySeeks("inst-1", []SearchQuery{orQuery}) assert.False(t, ok) + assert.Nil(t, seeks) require.Len(t, remaining, 1) assert.Equal(t, orQuery, remaining[0]) } +func TestExtractLoginEqualitySeeks_NestedAndExtractsLoginName(t *testing.T) { + loginNameQuery, err := NewUserLoginNameExistsQuery("user@org.localhost", TextEqualsIgnoreCase) + require.NoError(t, err) + orgQuery, err := NewUserResourceOwnerSearchQuery("org1", TextEquals) + require.NoError(t, err) + andQuery, err := NewAndQuery(loginNameQuery, orgQuery) + require.NoError(t, err) + + seeks, remaining, ok := extractLoginEqualitySeeks("inst-1", []SearchQuery{andQuery}) + require.True(t, ok) + require.Len(t, seeks, 1) + assert.Contains(t, seeks[0].sql, "login_name_matches") + require.Len(t, remaining, 1) + assert.Equal(t, orgQuery, remaining[0]) +} + func TestPrepareUsersQuery_LoginNameEqualsUsesIndexedJoin(t *testing.T) { ctx := authz.WithInstanceID(t.Context(), "inst-1") loginNameQuery, err := NewUserLoginNameExistsQuery("user165000@org.localhost", TextEqualsIgnoreCase) @@ -138,7 +183,7 @@ func TestPrepareUsersQuery_LoginNameEqualsWithOrgFilter(t *testing.T) { assert.Contains(t, args, "user") } -func TestPrepareUsersQuery_LoginNameOrEmailDoesNotRewrite(t *testing.T) { +func TestPrepareUsersQuery_LoginNameOrEmailUsesUnion(t *testing.T) { ctx := authz.WithInstanceID(t.Context(), "inst-1") loginNameQuery, err := NewUserLoginNameExistsQuery("user@org.localhost", TextEqualsIgnoreCase) require.NoError(t, err) @@ -151,11 +196,268 @@ func TestPrepareUsersQuery_LoginNameOrEmailDoesNotRewrite(t *testing.T) { Queries: []SearchQuery{orQuery}, } builder, _ := q.prepareUsersQuery(ctx, false) + sql, args, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "UNION") + assert.Contains(t, sql, "AS matches") + assert.Contains(t, sql, "login_name_matches") + assert.Contains(t, sql, "login_names3_users") + assert.Contains(t, sql, "LOWER("+HumanEmailCol.identifier()+")") + assert.NotContains(t, sql, "login_name_lower") + assert.Contains(t, args, "user@example.com") +} + +func TestPrepareUsersQuery_UsernameOrEmailUsesUnion(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + usernameQuery, err := NewUserUsernameSearchQuery("user165000", TextEqualsIgnoreCase) + require.NoError(t, err) + emailQuery, err := NewUserEmailSearchQuery("user@example.com", TextEqualsIgnoreCase) + require.NoError(t, err) + orQuery, err := NewOrQuery(usernameQuery, emailQuery) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{orQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, args, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "UNION") + assert.Contains(t, sql, "AS matches") + assert.Contains(t, sql, "LOWER("+UserUsernameCol.identifier()+")") + assert.Contains(t, sql, "LOWER("+HumanEmailCol.identifier()+")") + assert.NotContains(t, sql, "login_name_matches") + assert.Contains(t, args, "user165000") + assert.Contains(t, args, "user@example.com") +} + +func TestPrepareUsersQuery_EmailOrPhoneUsesUnion(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + emailQuery, err := NewUserEmailSearchQuery("user@example.com", TextEqualsIgnoreCase) + require.NoError(t, err) + phoneQuery, err := NewUserPhoneSearchQuery("+41000000000", TextEquals) + require.NoError(t, err) + orQuery, err := NewOrQuery(emailQuery, phoneQuery) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{orQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, args, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "UNION") + assert.Contains(t, sql, "LOWER("+HumanEmailCol.identifier()+")") + assert.Contains(t, sql, "LOWER("+HumanPhoneCol.identifier()+")") + assert.Contains(t, args, "user@example.com") + assert.Contains(t, args, "+41000000000") +} + +func TestPrepareUsersQuery_EmailEqualsRechecksExactColumn(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + emailQuery, err := NewUserEmailSearchQuery("Ada@Example.com", TextEquals) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{emailQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, args, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "AS matches") + assert.Contains(t, sql, "LOWER("+HumanEmailCol.identifier()+")") + assert.Contains(t, sql, "LOWER($") + assert.Contains(t, sql, HumanEmailCol.identifier()+" =") + assert.Contains(t, args, "Ada@Example.com") + assert.NotContains(t, args, "ada@example.com") +} + +func TestPrepareUsersQuery_EmailIgnoreCaseDoesNotRecheckExactColumn(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + emailQuery, err := NewUserEmailSearchQuery("Ada@Example.com", TextEqualsIgnoreCase) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{emailQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, args, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "AS matches") + assert.Contains(t, sql, "LOWER("+HumanEmailCol.identifier()+")") + assert.NotContains(t, sql, "LOWER($") + assert.NotContains(t, sql, HumanEmailCol.identifier()+" =") + assert.Contains(t, args, "ada@example.com") + assert.NotContains(t, args, "Ada@Example.com") +} + +func TestPrepareUsersQuery_PhoneIgnoreCaseUsesUnion(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + phoneQuery, err := NewUserPhoneSearchQuery("+41000000000", TextEqualsIgnoreCase) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{phoneQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, args, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "AS matches") + assert.Contains(t, sql, "LOWER("+HumanPhoneCol.identifier()+")") + assert.Contains(t, args, "+41000000000") +} + +func TestPrepareUsersQuery_ContainsDoesNotUseUnion(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + usernameQuery, err := NewUserUsernameSearchQuery("user", TextContainsIgnoreCase) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{usernameQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) sql, _, err := builder.ToSql() require.NoError(t, err) - assert.NotContains(t, sql, "login_name_matches") - assert.Contains(t, sql, "projections.login_names3") + assert.NotContains(t, sql, "AS matches") + assert.NotContains(t, sql, " UNION ") +} + +func TestPrepareUsersQuery_DisplayNameDoesNotUseUnion(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + displayNameQuery, err := NewUserDisplayNameSearchQuery("Ada Lovelace", TextEqualsIgnoreCase) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{displayNameQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, _, err := builder.ToSql() + require.NoError(t, err) + + assert.NotContains(t, sql, "AS matches") + assert.NotContains(t, sql, " UNION ") +} + +func TestPrepareUsersQuery_UsernameOrContainsDoesNotUseUnion(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + usernameQuery, err := NewUserUsernameSearchQuery("user165000", TextEqualsIgnoreCase) + require.NoError(t, err) + displayNameQuery, err := NewUserDisplayNameSearchQuery("user", TextContainsIgnoreCase) + require.NoError(t, err) + orQuery, err := NewOrQuery(usernameQuery, displayNameQuery) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{orQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, _, err := builder.ToSql() + require.NoError(t, err) + + assert.NotContains(t, sql, "AS matches") + assert.NotContains(t, sql, " UNION ") +} + +func TestPrepareUsersQuery_UsernameOrUsernameUsesUnion(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + alice, err := NewUserUsernameSearchQuery("alice", TextEqualsIgnoreCase) + require.NoError(t, err) + bob, err := NewUserUsernameSearchQuery("bob", TextEqualsIgnoreCase) + require.NoError(t, err) + orQuery, err := NewOrQuery(alice, bob) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{orQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, args, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "UNION") + assert.Contains(t, sql, "AS matches") + assert.Equal(t, 2, strings.Count(sql, "LOWER("+UserUsernameCol.identifier()+")")) + assert.Contains(t, args, "alice") + assert.Contains(t, args, "bob") +} + +func TestPrepareUsersQuery_SiblingUsernameAndKeepsRemainingWhere(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + alice, err := NewUserUsernameSearchQuery("alice", TextEqualsIgnoreCase) + require.NoError(t, err) + bob, err := NewUserUsernameSearchQuery("bob", TextEqualsIgnoreCase) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{alice, bob}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, args, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "AS matches") + assert.NotContains(t, sql, " UNION ") + assert.Contains(t, args, "alice") + assert.Contains(t, args, "bob") +} + +func TestPrepareUsersQuery_NestedAndOfOrUsesUnion(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + alice, err := NewUserUsernameSearchQuery("alice", TextEqualsIgnoreCase) + require.NoError(t, err) + bob, err := NewUserUsernameSearchQuery("bob", TextEqualsIgnoreCase) + require.NoError(t, err) + orQuery, err := NewOrQuery(alice, bob) + require.NoError(t, err) + orgQuery, err := NewUserResourceOwnerSearchQuery("org1", TextEquals) + require.NoError(t, err) + andQuery, err := NewAndQuery(orQuery, orgQuery) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{andQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, args, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "AS matches") + assert.Contains(t, sql, " UNION ") + assert.Contains(t, sql, "resource_owner") + assert.Contains(t, args, "org1") + assert.Contains(t, args, "alice") + assert.Contains(t, args, "bob") +} + +func TestPrepareUsersQuery_NestedOrOfAndDoesNotUseUnion(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + alice, err := NewUserUsernameSearchQuery("alice", TextEqualsIgnoreCase) + require.NoError(t, err) + orgQuery, err := NewUserResourceOwnerSearchQuery("org1", TextEquals) + require.NoError(t, err) + andQuery, err := NewAndQuery(alice, orgQuery) + require.NoError(t, err) + bob, err := NewUserUsernameSearchQuery("bob", TextEqualsIgnoreCase) + require.NoError(t, err) + orQuery, err := NewOrQuery(andQuery, bob) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{orQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, _, err := builder.ToSql() + require.NoError(t, err) + + assert.NotContains(t, sql, "AS matches") + assert.NotContains(t, sql, " UNION ") } func TestPrepareUsersQuery_MetadataFilterKeepsDistinctJoin(t *testing.T) {