From 318acd4bc3be7ee43a5eedc8e401801c987bd394 Mon Sep 17 00:00:00 2001 From: Silvan <27845747+adlerhurst@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:56:07 +0200 Subject: [PATCH] perf(query): speed up ListUsers login name equality filters (#12460) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `ListUsers` with `LoginNameQuery` + equals/equals-ignore-case was very slow on large orgs (e.g. ~330k users) because the generated SQL filtered the `projections.login_names3` **view** on the computed `login_name_lower` expression and correlated that subquery on `users14.id`. Postgres nested-looped every user and never used `login_names3_users_search (instance_id, user_name_lower)`. This change rewrites the **query planner** for that hot path: when an equals/equals-ignore-case login-name filter is present (and not under `OR`/`NOT`), the user list query **INNER JOINs** an indexed matches subquery instead of filtering via the view expression. The matches SQL mirrors `user_by_login_name.sql` (`user_name_lower` / domain paths + `preferred` / `is_primary`). Non-equals methods and OR combinations keep the previous view-based filter so semantics stay unchanged. Also adds a k6 use case that mirrors login v2 discovery (`loginNameQuery` EQUALS_IGNORE_CASE + `organizationIdQuery`, `limit: 2`): ```bash cd benchmark make users_by_login_name USER_AMOUNT=100000 VUS=10 DURATION=60s ``` ## Approach 1. `NewLoginNameSearchQuery` for equals / equals-ignore-case returns a marker `loginNameEqualsFilter` (other methods unchanged). 2. `prepareUsersQuery` extracts that marker when safe, then: - builds the usual `sq.SelectBuilder` **without** the login-name view predicate - adds `JoinClause` to `user_login_name_matches(.sql)` / `_case_sensitive.sql` as `login_name_matches` - keeps metadata JOIN/`DISTINCT` only when metadata filters are present (same as before) 3. Embedded SQL files under `internal/query/` for the matches subquery. Local smoke against ~330k synthetic users: baseline ~1392ms → rewritten path ~0.4ms for a single equals-ignore-case lookup. ## Test plan - [x] `go test ./internal/query/ -run 'TestLoginName|TestUsers|TestUserByLoginName'` (after generate-stubs) - [x] Existing `user_test` expected SQL updated (no always-on metadata join; login-name equals uses JOIN) - [ ] Run k6 before/after on a large `USER_AMOUNT` (e.g. 50k–100k+) and compare `list_users_duration` p50/p95/p99 - [x] Manual login v2 username discovery against a large org - [x] Confirm OR / NOT / CONTAINS login-name queries still return expected results
Open in Web Open in Cursor 
--------- Co-authored-by: Cursor Agent Co-authored-by: Silvan --- benchmark/Makefile | 4 + benchmark/README.md | 64 +++--- benchmark/src/pool.ts | 32 +++ benchmark/src/url.ts | 4 +- .../src/use_cases/users_by_login_name.ts | 79 +++++++ .../src/use_cases/users_by_metadata_key.ts | 21 +- .../src/use_cases/users_by_metadata_value.ts | 21 +- benchmark/src/user.ts | 20 ++ internal/query/user.go | 195 ++++++++++++++++-- internal/query/user_login_name_exists_test.go | 175 ++++++++++++++++ internal/query/user_login_name_matches.sql | 29 +++ ...user_login_name_matches_case_sensitive.sql | 29 +++ internal/query/user_test.go | 3 +- 13 files changed, 614 insertions(+), 62 deletions(-) create mode 100644 benchmark/src/pool.ts create mode 100644 benchmark/src/use_cases/users_by_login_name.ts create mode 100644 internal/query/user_login_name_exists_test.go create mode 100644 internal/query/user_login_name_matches.sql create mode 100644 internal/query/user_login_name_matches_case_sensitive.sql diff --git a/benchmark/Makefile b/benchmark/Makefile index 40f4792856..fa0e1241e2 100644 --- a/benchmark/Makefile +++ b/benchmark/Makefile @@ -74,6 +74,10 @@ users_by_metadata_key: ensure_modules bundle users_by_metadata_value: ensure_modules bundle ${K6} run --summary-trend-stats "min,avg,max,p(50),p(95),p(99)" dist/users_by_metadata_value.js --vus ${VUS} --duration ${DURATION} --out csv=output/users_by_metadata_${DATE}.csv +.PHONY: users_by_login_name +users_by_login_name: ensure_modules bundle + ${K6} run --summary-trend-stats "min,avg,max,p(50),p(95),p(99)" dist/users_by_login_name.js --vus ${VUS} --duration ${DURATION} --out csv=output/users_by_login_name_${DATE}.csv + .PHONY: lint lint: npm i diff --git a/benchmark/README.md b/benchmark/README.md index 4dcea967ca..4e70e76571 100644 --- a/benchmark/README.md +++ b/benchmark/README.md @@ -4,11 +4,11 @@ This package contains code for benchmarking specific endpoints of the API using ## Prerequisite -* [npm](https://docs.npmjs.com/downloading-and-installing-node-js-and-npm) -* [k6](https://k6.io/docs/get-started/installation/) -* [go](https://go.dev/doc/install) -* [xk6](https://github.com/grafana/xk6#local-installation) (make sure `~/go/bin` is in your `${PATH}`) -* running the API +- [npm](https://docs.npmjs.com/downloading-and-installing-node-js-and-npm) +- [k6](https://k6.io/docs/get-started/installation/) +- [go](https://go.dev/doc/install) +- [xk6](https://github.com/grafana/xk6#local-installation) (make sure `~/go/bin` is in your `${PATH}`) +- running the API ## Structure @@ -18,63 +18,69 @@ The use cases under tests are defined in `src/use_cases`. The implementation of ### Env vars -* `VUS`: Amount of parallel processes execute the test (default is 20) -* `DURATION`: Defines how long the tests are executed (default is `200s`) -* `ZITADEL_HOST`: URL of ZITADEL (default is `http://localhost:8080`) -* `ADMIN_LOGIN_NAME`: Loginanme of a human user with `IAM_OWNER`-role -* `ADMIN_PASSWORD`: password of the human user +- `VUS`: Amount of parallel processes execute the test (default is 20) +- `DURATION`: Defines how long the tests are executed (default is `200s`) +- `ZITADEL_HOST`: URL of ZITADEL (default is `http://localhost:8080`) +- `ADMIN_LOGIN_NAME`: Loginanme of a human user with `IAM_OWNER`-role +- `ADMIN_PASSWORD`: password of the human user +- `USER_AMOUNT`: Number of users created during setup for list-users benchmarks (default is `2500`) +- `SETUP_CONCURRENCY`: Max in-flight user-create requests during list-users setup (default is `50`). Large `USER_AMOUNT` with unbounded parallelism can exhaust ephemeral ports (`can't assign requested address`). To setup the tests we use the credentials of management console and log in using an admin. The user must be able to create organizations and all resources inside organizations. -* `ADMIN_LOGIN_NAME`: `zitadel-admin@zitadel.localhost` -* `ADMIN_PASSWORD`: `Password1!` +- `ADMIN_LOGIN_NAME`: `zitadel-admin@zitadel.localhost` +- `ADMIN_PASSWORD`: `Password1!` ### Test Before you run the tests you need an initialized user. The tests don't implement the change password screen during login. -* `make human_password_login` +- `make human_password_login` setup: creates human users test: uses the previously created humans to sign in using the login ui -* `make machine_pat_login` +- `make machine_pat_login` setup: creates machines and a pat for each machine test: calls user info endpoint with the given pats -* `make machine_client_credentials_login` +- `make machine_client_credentials_login` setup: creates machines and a client credential secret for each machine test: calls token endpoint with the `client_credentials` grant type. -* `make user_info` +- `make user_info` setup: creates human users and signs them in test: calls user info endpoint using the given humans -* `make manipulate_user` - test: creates a human, updates its profile, locks the user and then deletes it -* `make introspect` +- `make manipulate_user` + test: creates a human, updates its profile, locks the user and then deletes it +- `make introspect` setup: creates projects, one api per project, one key per api and generates the jwt from the given keys test: calls introspection endpoint using the given JWTs -* `make add_session` +- `make add_session` setup: creates human users test: creates new sessions with user id check -* `make oidc_session` +- `make oidc_session` setup: creates a service account to create the auth request and session. test: creates an auth request, a session and links the session to the auth request. Implementation of [this flow](https://zitadel.com/docs/guides/integrate/login-ui/oidc-standard). -* `make otp_session` +- `make otp_session` setup: creates 1 human user for each VU and adds OTP Email to it test: creates a session based on the login name of the user, sets the OTP Email challenge to the session and afterwards checks the OTP code -* `make password_session` +- `make password_session` setup: creates 1 human user for each VU and adds OTP Email to it test: creates a session based on the login name of the user and checks for the password on a second step -* `make machine_jwt_profile_grant` +- `make machine_jwt_profile_grant` setup: generates private/public key, creates service accounts, adds a key - test: creates a token and calls user info -* `make machine_jwt_profile_grant_single_user` + test: creates a token and calls user info +- `make machine_jwt_profile_grant_single_user` setup: generates private/public key, creates service account, adds a key test: creates a token and calls user info in parallel for the same user -* `make users_by_metadata_key` +- `make users_by_metadata_key` setup: creates for half of the VUS a human user and a machine for the other half, adds 3 metadata to each user test: calls the list users endpoint and filters by a metadata key -* `make users_by_metadata_value` +- `make users_by_metadata_value` setup: creates for half of the VUS a human user and a machine for the other half, adds 3 metadata to each user test: calls the list users endpoint and filters by a metadata value -* `make verify_all_user_grants_exists` +- `make users_by_login_name` + setup: creates `USER_AMOUNT` human users (default `2500`) in a new org, with `SETUP_CONCURRENCY` parallel creates (default `50`) + test: calls ListUsers the same way as login v2 (`loginNameQuery` with `EQUALS_IGNORE_CASE`, `organizationIdQuery`, `limit: 2`) + note: to reproduce multi-second latency on the old query plan, use a large dataset, e.g. `USER_AMOUNT=100000 VUS=10 DURATION=60s` +- `make verify_all_user_grants_exists` setup: creates 50 projects, 1 machine per VU test: creates a machine and grants all projects to the machine teardown: the organization is not removed to verify the data of the projections are correct. You can find additional information [at the bottom of this file](./src/use_cases/verify_all_user_grants_exist.ts) diff --git a/benchmark/src/pool.ts b/benchmark/src/pool.ts new file mode 100644 index 0000000000..ce9cb2a8ed --- /dev/null +++ b/benchmark/src/pool.ts @@ -0,0 +1,32 @@ +/** + * Run async work over items with a fixed number of in-flight tasks. + * Avoids unbounded Promise.all which can exhaust ephemeral ports + * when seeding large USER_AMOUNT datasets. + */ +export async function mapPool( + items: T[], + concurrency: number, + fn: (item: T, index: number) => Promise, +): Promise { + if (items.length === 0) { + return []; + } + + const limit = Math.max(1, Math.min(concurrency, items.length)); + const results: R[] = new Array(items.length); + let nextIndex = 0; + + async function worker(): Promise { + while (true) { + const index = nextIndex; + nextIndex += 1; + if (index >= items.length) { + return; + } + results[index] = await fn(items[index], index); + } + } + + await Promise.all(Array.from({ length: limit }, () => worker())); + return results; +} diff --git a/benchmark/src/url.ts b/benchmark/src/url.ts index 9d25ec51cb..97c007dd7b 100644 --- a/benchmark/src/url.ts +++ b/benchmark/src/url.ts @@ -1,4 +1,6 @@ -import { options } from 'k6/http'; +// @ts-ignore Import module +import { URL } from 'https://jslib.k6.io/url/1.0.0/index.js'; + import { Config } from './config'; export type options = { diff --git a/benchmark/src/use_cases/users_by_login_name.ts b/benchmark/src/use_cases/users_by_login_name.ts new file mode 100644 index 0000000000..2a17ae52c3 --- /dev/null +++ b/benchmark/src/use_cases/users_by_login_name.ts @@ -0,0 +1,79 @@ +import { check } from 'k6'; + +import { Config } from '../config'; +import { loginByUsernamePassword } from '../login_ui'; +import { createOrg, Org, removeOrg } from '../org'; +import { mapPool } from '../pool'; +import { createHuman, listUsers, User } from '../user'; + +const userAmount = parseInt(__ENV.USER_AMOUNT) || 2500; +// Unbounded Promise.all over large USER_AMOUNT exhausts ephemeral ports +// (dial tcp ... can't assign requested address). Keep a modest in-flight cap. +const setupConcurrency = parseInt(__ENV.SETUP_CONCURRENCY) || 50; + +type SetupData = { + tokens: { accessToken?: string }; + org: Org; + targetLoginName: string; +}; + +export async function setup(): Promise { + const tokens = loginByUsernamePassword(Config.admin as User); + console.info('setup: admin signed in'); + + const org = await createOrg(tokens.accessToken!); + console.info(`setup: org (${org.organizationId}) created`); + + const progressEvery = Math.max(1, Math.floor(userAmount / 100)); + const users = await mapPool( + Array.from({ length: userAmount }, (_, i) => i), + setupConcurrency, + async (i) => { + const user = await createHuman(`zitizen-${i}`, org, tokens.accessToken!); + if (i % progressEvery === 0 || i === userAmount - 1) { + console.log(`setup: ${i + 1} of ${userAmount} users setup`); + } + return user; + }, + ); + console.info(`setup: ${users.length} users created (concurrency=${setupConcurrency})`); + + const targetLoginName = users[0].loginNames[0]; + console.info(`setup: target login name ${targetLoginName}`); + + return { tokens, org, targetLoginName }; +} + +export default async function (data: SetupData) { + const result = await listUsers( + { + query: { limit: 2 }, + queries: [ + { + loginNameQuery: { + loginName: data.targetLoginName, + method: 'TEXT_QUERY_METHOD_EQUALS_IGNORE_CASE', + }, + }, + { + organizationIdQuery: { + organizationId: data.org.organizationId, + }, + }, + ], + }, + data.tokens.accessToken!, + ); + + check(result, { + 'exact one user found': (res) => (res.result?.length ?? 0) === 1 || res.details.totalResult == 1, + }) || + console.log( + `unexpected list users result. expected 1 user but got result=${result.result?.length} totalResult=${result.details.totalResult}`, + ); +} + +export function teardown(data: SetupData) { + removeOrg(data.org, data.tokens.accessToken!); + console.info('teardown: org removed'); +} diff --git a/benchmark/src/use_cases/users_by_metadata_key.ts b/benchmark/src/use_cases/users_by_metadata_key.ts index 87adfef65a..8a2974db26 100644 --- a/benchmark/src/use_cases/users_by_metadata_key.ts +++ b/benchmark/src/use_cases/users_by_metadata_key.ts @@ -1,11 +1,13 @@ import { loginByUsernamePassword } from '../login_ui'; import { createOrg, removeOrg } from '../org'; +import { mapPool } from '../pool'; import { createHuman, User, createMachine, setUserMetadata, listUsers } from '../user'; import { Config } from '../config'; import { check } from 'k6'; import encoding from 'k6/encoding'; const userAmount = parseInt(__ENV.USER_AMOUNT) || 2500; +const setupConcurrency = parseInt(__ENV.SETUP_CONCURRENCY) || 50; export async function setup() { const tokens = loginByUsernamePassword(Config.admin as User); @@ -14,10 +16,11 @@ export async function setup() { const org = await createOrg(tokens.accessToken!); console.info(`setup: org (${org.organizationId}) created`); - const users: User[] = []; - - await Promise.all( - Array.from({ length: userAmount }, async (_, i) => { + const progressEvery = Math.max(1, Math.floor(userAmount / 100)); + const users = await mapPool( + Array.from({ length: userAmount }, (_, i) => i), + setupConcurrency, + async (i) => { let user: User; let type: 'human' | 'machine'; if (i % 2 === 0) { @@ -27,7 +30,6 @@ export async function setup() { user = await createMachine(`zitachine-${i}`, org, tokens.accessToken!); type = 'machine'; } - users.push(user); await setUserMetadata( [ { key: 'type', value: encoding.b64encode(type, 'rawurl') }, @@ -37,12 +39,13 @@ export async function setup() { user.userId, tokens.accessToken!, ); - if (i % 10 === 0) { - console.log(`setup: ${i} of ${userAmount} users setup`); + if (i % progressEvery === 0 || i === userAmount - 1) { + console.log(`setup: ${i + 1} of ${userAmount} users setup`); } - }), + return user; + }, ); - console.info(`setup: ${users.length} users created`); + console.info(`setup: ${users.length} users created (concurrency=${setupConcurrency})`); return { tokens, org, users }; } diff --git a/benchmark/src/use_cases/users_by_metadata_value.ts b/benchmark/src/use_cases/users_by_metadata_value.ts index 10b067955f..86c58c2e22 100644 --- a/benchmark/src/use_cases/users_by_metadata_value.ts +++ b/benchmark/src/use_cases/users_by_metadata_value.ts @@ -1,11 +1,13 @@ import { loginByUsernamePassword } from '../login_ui'; import { createOrg, removeOrg } from '../org'; +import { mapPool } from '../pool'; import { createHuman, User, createMachine, setUserMetadata, listUsers } from '../user'; import { Config } from '../config'; import { check } from 'k6'; import encoding from 'k6/encoding'; const userAmount = parseInt(__ENV.USER_AMOUNT) || 2500; +const setupConcurrency = parseInt(__ENV.SETUP_CONCURRENCY) || 50; export async function setup() { const tokens = loginByUsernamePassword(Config.admin as User); @@ -14,10 +16,11 @@ export async function setup() { const org = await createOrg(tokens.accessToken!); console.info(`setup: org (${org.organizationId}) created`); - const users: User[] = []; - - await Promise.all( - Array.from({ length: userAmount }, async (_, i) => { + const progressEvery = Math.max(1, Math.floor(userAmount / 100)); + const users = await mapPool( + Array.from({ length: userAmount }, (_, i) => i), + setupConcurrency, + async (i) => { let user: User; let type: 'human' | 'machine'; if (i % 2 === 0) { @@ -27,7 +30,6 @@ export async function setup() { user = await createMachine(`zitachine-${i}`, org, tokens.accessToken!); type = 'machine'; } - users.push(user); await setUserMetadata( [ { key: 'type', value: encoding.b64encode(type, 'rawurl') }, @@ -37,12 +39,13 @@ export async function setup() { user.userId, tokens.accessToken!, ); - if (i % 10 === 0) { - console.log(`setup: ${i} of ${userAmount} users setup`); + if (i % progressEvery === 0 || i === userAmount - 1) { + console.log(`setup: ${i + 1} of ${userAmount} users setup`); } - }), + return user; + }, ); - console.info(`setup: ${users.length} users created`); + console.info(`setup: ${users.length} users created (concurrency=${setupConcurrency})`); return { tokens, org, users }; } diff --git a/benchmark/src/user.ts b/benchmark/src/user.ts index 032e4967ad..2435b14b1d 100644 --- a/benchmark/src/user.ts +++ b/benchmark/src/user.ts @@ -325,7 +325,26 @@ export function setUserMetadata(metadata: Metadata[], userId: string, accessToke } export type ListUsersRequest = { + query?: { + limit?: number; + offset?: number; + }; queries: { + loginNameQuery?: { + loginName: string; + method: + | 'TEXT_QUERY_METHOD_EQUALS' + | 'TEXT_QUERY_METHOD_EQUALS_IGNORE_CASE' + | 'TEXT_QUERY_METHOD_STARTS_WITH' + | 'TEXT_QUERY_METHOD_STARTS_WITH_IGNORE_CASE' + | 'TEXT_QUERY_METHOD_CONTAINS' + | 'TEXT_QUERY_METHOD_CONTAINS_IGNORE_CASE' + | 'TEXT_QUERY_METHOD_ENDS_WITH' + | 'TEXT_QUERY_METHOD_ENDS_WITH_IGNORE_CASE'; + }; + organizationIdQuery?: { + organizationId: string; + }; metadataKeyFilter?: { key: string; method: 'TEXT_FILTER_METHOD_EQUALS' | 'TEXT_FILTER_METHOD_CONTAINS' | 'TEXT_FILTER_METHOD_CONTAINS_IGNORE_CASE'; @@ -341,6 +360,7 @@ export type ListUsersResult = { details: { totalResult: number; }; + result?: unknown[]; }; const listUsersTrend = new Trend('list_users_duration', true); diff --git a/internal/query/user.go b/internal/query/user.go index 5abb16e53b..78018d9ec1 100644 --- a/internal/query/user.go +++ b/internal/query/user.go @@ -5,6 +5,7 @@ import ( "database/sql" _ "embed" "errors" + "slices" "strings" "time" @@ -793,8 +794,23 @@ func NewUserPreferredLoginNameSearchQuery(value string, comparison TextCompariso return NewTextQuery(userPreferredLoginNameCol, value, comparison) } +//go:embed user_login_name_matches.sql +var userLoginNameMatchesQuery string + +//go:embed user_login_name_matches_case_sensitive.sql +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. func NewUserLoginNameExistsQuery(value string, comparison TextComparison) (SearchQuery, error) { - // linking queries for the sub select + if comparison == TextEquals || comparison == TextEqualsIgnoreCase { + return newLoginNameEqualsFilter(value, comparison == TextEqualsIgnoreCase) + } + return newLoginNameExistsViewQuery(value, comparison) +} + +func newLoginNameExistsViewQuery(value string, comparison TextComparison) (SearchQuery, error) { instanceQuery, err := NewColumnComparisonQuery(LoginNameInstanceIDCol, UserInstanceIDCol, ColumnEquals) if err != nil { return nil, err @@ -807,7 +823,6 @@ func NewUserLoginNameExistsQuery(value string, comparison TextComparison) (Searc if err != nil { return nil, err } - // text query to select data from the linked sub select var loginNameQuery SearchQuery loginNameQuery, err = NewTextQuery(LoginNameNameCol, value, comparison) if comparison == TextEqualsIgnoreCase { @@ -816,12 +831,10 @@ func NewUserLoginNameExistsQuery(value string, comparison TextComparison) (Searc if err != nil { return nil, err } - // full definition of the sub select subSelect, err := NewSubSelect(LoginNameUserIDCol, []SearchQuery{instanceQuery, userIDQuery, resourceOwnerQuery, loginNameQuery}) if err != nil { return nil, err } - // "WHERE * IN (*)" query with subquery as list-data provider return NewListQuery( UserIDCol, subSelect, @@ -829,6 +842,147 @@ func NewUserLoginNameExistsQuery(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) +} + +func searchQueriesHaveMetadataFilter(queries []SearchQuery) bool { + return slices.ContainsFunc(queries, searchQueryHasMetadataFilter) +} + +func searchQueryHasMetadataFilter(qry SearchQuery) bool { + switch v := qry.(type) { + case *OrQuery: + return searchQueriesHaveMetadataFilter(v.queries) + case *AndQuery: + return searchQueriesHaveMetadataFilter(v.queries) + case *NotQuery: + return searchQueryHasMetadataFilter(v.query) + default: + return qry.Col().table.name == userMetadataTable.name + } +} + func triggerUserProjections(ctx context.Context) { triggerBatch(ctx, projection.UserProjection, projection.LoginNameProjection) } @@ -1257,14 +1411,25 @@ func prepareUserUniqueQuery() (sq.SelectBuilder, func(*sql.Row) (bool, error)) { } // prepareUsersQuery creates the select query for searching users and returns a matching scan function. -// Permissions, filters and sorting are applied in a `SELECT FROM` distinct sub-select. +// Permissions, filters and sorting are applied in a `SELECT FROM` sub-select. // The count over window function and limit are applied in the outer query. // 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. 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 + } + needsMetadataJoin := q.hasMetadataFilter() + // start building the sub-select query := sq.Select( UserIDCol.identifier(), @@ -1298,18 +1463,24 @@ func (q *UserSearchQueries) prepareUsersQuery(ctx context.Context, permissionChe MachineSecretCol.identifier(), MachineAccessTokenTypeCol.identifier(), q.SortingColumn.orderBy()). - Distinct(). From(userTable.identifier()). LeftJoin(join(HumanUserIDCol, UserIDCol)). LeftJoin(join(MachineUserIDCol, UserIDCol)). - LeftJoin(join(UserMetadataUserIDCol, UserIDCol)). JoinClause(joinLoginNames). - Where(sq.Eq{UserInstanceIDCol.identifier(): authz.GetInstance(ctx).InstanceID()}) + Where(sq.Eq{UserInstanceIDCol.identifier(): instanceID}) - query = userPermissionCheckV2(ctx, query, permissionCheckV2, q.Queries) - // apply requested filters - for _, q := range q.Queries { - query = q.toQuery(query) + if loginNameExtracted { + query = query.JoinClause(loginNameFilter.joinMatches(instanceID)) + } + + if needsMetadataJoin { + query = query.Distinct(). + LeftJoin(join(UserMetadataUserIDCol, UserIDCol)) + } + + query = userPermissionCheckV2(ctx, query, permissionCheckV2, filters) + for _, filter := range filters { + query = filter.toQuery(query) } // apply sorting in the sub-select,because the identifier is fully qualified. query = q.consumeSorting(query) diff --git a/internal/query/user_login_name_exists_test.go b/internal/query/user_login_name_exists_test.go new file mode 100644 index 0000000000..430c9aacde --- /dev/null +++ b/internal/query/user_login_name_exists_test.go @@ -0,0 +1,175 @@ +package query + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zitadel/zitadel/internal/api/authz" +) + +func TestNewUserLoginNameExistsQuery_EqualsIgnoreCaseIsMarker(t *testing.T) { + qry, err := NewUserLoginNameExistsQuery("User.Name@Org.Localhost", TextEqualsIgnoreCase) + require.NoError(t, err) + + ln, ok := qry.(*loginNameEqualsFilter) + require.True(t, ok) + assert.Equal(t, "user.name", ln.username) + assert.Equal(t, "org.localhost", ln.domain) + assert.Equal(t, "user.name@org.localhost", ln.loginName) + assert.True(t, ln.ignoreCase) +} + +func TestNewUserLoginNameExistsQuery_EqualsIsMarker(t *testing.T) { + qry, err := NewUserLoginNameExistsQuery("User.Name@Org.Localhost", TextEquals) + require.NoError(t, err) + + ln, ok := qry.(*loginNameEqualsFilter) + require.True(t, ok) + assert.Equal(t, "User.Name", ln.username) + assert.Equal(t, "Org.Localhost", ln.domain) + assert.Equal(t, "User.Name@Org.Localhost", ln.loginName) + assert.False(t, ln.ignoreCase) +} + +func TestNewUserLoginNameExistsQuery_ContainsFallsBackToView(t *testing.T) { + qry, err := NewUserLoginNameExistsQuery("user", TextContains) + require.NoError(t, err) + + _, ok := qry.(*loginNameEqualsFilter) + assert.False(t, ok) + + sql, _, err := qry.comp().ToSql() + require.NoError(t, err) + assert.Contains(t, sql, "projections.login_names3") + assert.Contains(t, strings.ToLower(sql), "login_name") +} + +func TestExtractLoginNameEqualsFilter_TopLevel(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}) + require.True(t, ok) + require.NotNil(t, filter) + assert.Equal(t, "user", filter.username) + require.Len(t, remaining, 1) + assert.Equal(t, orgQuery, remaining[0]) +} + +func TestExtractLoginNameEqualsFilter_SkipsOrQuery(t *testing.T) { + loginNameQuery, err := NewUserLoginNameExistsQuery("user@org.localhost", TextEqualsIgnoreCase) + require.NoError(t, err) + emailQuery, err := NewUserEmailSearchQuery("user@example.com", TextEqualsIgnoreCase) + require.NoError(t, err) + orQuery, err := NewOrQuery(loginNameQuery, emailQuery) + require.NoError(t, err) + + _, remaining, ok := extractLoginNameEqualsFilter([]SearchQuery{orQuery}) + assert.False(t, ok) + require.Len(t, remaining, 1) + assert.Equal(t, orQuery, remaining[0]) +} + +func TestPrepareUsersQuery_LoginNameEqualsUsesIndexedJoin(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + loginNameQuery, err := NewUserLoginNameExistsQuery("user165000@org.localhost", TextEqualsIgnoreCase) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{loginNameQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, args, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "INNER JOIN") + assert.Contains(t, sql, "login_name_matches") + assert.Contains(t, sql, "login_names3_users") + assert.Contains(t, sql, "user_name_lower") + assert.NotContains(t, sql, "login_name_lower") + assert.NotContains(t, sql, "user_metadata5") + assert.NotContains(t, sql, "SELECT DISTINCT") + assert.Contains(t, args, "inst-1") + assert.Contains(t, args, "user165000") + assert.Contains(t, args, "org.localhost") +} + +func TestPrepareUsersQuery_LoginNameEqualsCaseSensitive(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + loginNameQuery, err := NewUserLoginNameExistsQuery("User165000@Org.Localhost", TextEquals) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{loginNameQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, args, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "login_name_matches") + assert.Contains(t, sql, "u.user_name IN") + assert.NotContains(t, sql, "user_name_lower") + assert.Contains(t, args, "User165000") + assert.Contains(t, args, "Org.Localhost") +} + +func TestPrepareUsersQuery_LoginNameEqualsWithOrgFilter(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + loginNameQuery, err := NewUserLoginNameExistsQuery("user@org.localhost", TextEqualsIgnoreCase) + require.NoError(t, err) + orgQuery, err := NewUserResourceOwnerSearchQuery("org1", TextEquals) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{loginNameQuery, orgQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, args, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "login_name_matches") + assert.Contains(t, sql, "resource_owner") + assert.Contains(t, args, "org1") + assert.Contains(t, args, "user") +} + +func TestPrepareUsersQuery_LoginNameOrEmailDoesNotRewrite(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + loginNameQuery, err := NewUserLoginNameExistsQuery("user@org.localhost", TextEqualsIgnoreCase) + require.NoError(t, err) + emailQuery, err := NewUserEmailSearchQuery("user@example.com", TextEqualsIgnoreCase) + require.NoError(t, err) + orQuery, err := NewOrQuery(loginNameQuery, emailQuery) + 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, "login_name_matches") + assert.Contains(t, sql, "projections.login_names3") +} + +func TestPrepareUsersQuery_MetadataFilterKeepsDistinctJoin(t *testing.T) { + ctx := authz.WithInstanceID(t.Context(), "inst-1") + metadataQuery, err := NewUserMetadataKeySearchQuery("key", TextContains) + require.NoError(t, err) + + q := &UserSearchQueries{ + Queries: []SearchQuery{metadataQuery}, + } + builder, _ := q.prepareUsersQuery(ctx, false) + sql, _, err := builder.ToSql() + require.NoError(t, err) + + assert.Contains(t, sql, "SELECT DISTINCT") + assert.Contains(t, sql, "user_metadata5") +} diff --git a/internal/query/user_login_name_matches.sql b/internal/query/user_login_name_matches.sql new file mode 100644 index 0000000000..7498f554d3 --- /dev/null +++ b/internal/query/user_login_name_matches.sql @@ -0,0 +1,29 @@ +SELECT u.id AS user_id +FROM projections.login_names3_users u +LEFT JOIN LATERAL ( + SELECT p.must_be_domain + FROM projections.login_names3_policies AS p + WHERE + ( + p.instance_id = ? + AND NOT p.is_default + AND p.resource_owner = u.resource_owner + ) OR ( + p.instance_id = ? + AND p.is_default + ) + ORDER BY p.is_default + LIMIT 1 +) AS p ON TRUE +LEFT JOIN projections.login_names3_domains d + ON p.must_be_domain + AND u.resource_owner = d.resource_owner + AND u.instance_id = d.instance_id + AND d.name_lower = ? +WHERE + u.instance_id = ? + AND u.user_name_lower IN (?, ?) + AND ( + (p.must_be_domain AND u.user_name_lower = ? AND d.name_lower = ?) + OR (NOT COALESCE(p.must_be_domain, FALSE) AND u.user_name_lower = ?) + ) diff --git a/internal/query/user_login_name_matches_case_sensitive.sql b/internal/query/user_login_name_matches_case_sensitive.sql new file mode 100644 index 0000000000..f752d8b3a1 --- /dev/null +++ b/internal/query/user_login_name_matches_case_sensitive.sql @@ -0,0 +1,29 @@ +SELECT u.id AS user_id +FROM projections.login_names3_users u +LEFT JOIN LATERAL ( + SELECT p.must_be_domain + FROM projections.login_names3_policies AS p + WHERE + ( + p.instance_id = ? + AND NOT p.is_default + AND p.resource_owner = u.resource_owner + ) OR ( + p.instance_id = ? + AND p.is_default + ) + ORDER BY p.is_default + LIMIT 1 +) AS p ON TRUE +LEFT JOIN projections.login_names3_domains d + ON p.must_be_domain + AND u.resource_owner = d.resource_owner + AND u.instance_id = d.instance_id + AND d.name = ? +WHERE + u.instance_id = ? + AND u.user_name IN (?, ?) + AND ( + (p.must_be_domain AND u.user_name = ? AND d.name = ?) + OR (NOT COALESCE(p.must_be_domain, FALSE) AND u.user_name = ?) + ) diff --git a/internal/query/user_test.go b/internal/query/user_test.go index 8aed577379..1946ebd60e 100644 --- a/internal/query/user_test.go +++ b/internal/query/user_test.go @@ -244,7 +244,7 @@ var ( "count", } usersQuery = `SELECT *, COUNT(*) OVER () FROM (` + - `SELECT DISTINCT projections.users14.id,` + + `SELECT projections.users14.id,` + ` projections.users14.creation_date,` + ` projections.users14.change_date,` + ` projections.users14.resource_owner,` + @@ -278,7 +278,6 @@ var ( ` FROM projections.users14` + ` LEFT JOIN projections.users14_humans ON projections.users14.id = projections.users14_humans.user_id AND projections.users14.instance_id = projections.users14_humans.instance_id` + ` LEFT JOIN projections.users14_machines ON projections.users14.id = projections.users14_machines.user_id AND projections.users14.instance_id = projections.users14_machines.instance_id` + - ` LEFT JOIN projections.user_metadata5 ON projections.users14.id = projections.user_metadata5.user_id AND projections.users14.instance_id = projections.user_metadata5.instance_id` + ` LEFT JOIN LATERAL (SELECT ARRAY_AGG(ln.login_name ORDER BY ln.login_name) AS login_names, MAX(CASE WHEN ln.is_primary THEN ln.login_name ELSE NULL END) AS preferred_login_name FROM projections.login_names3 AS ln WHERE ln.user_id = projections.users14.id AND ln.instance_id = projections.users14.instance_id) AS login_names ON TRUE` + ` WHERE projections.users14.instance_id = $1 ORDER BY projections.users14.id DESC` + `) AS results`