TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2025-12-26 04:56:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(query): distinct count in user list (#10840)

Browse Source 
# Which Problems Are Solved

When listing / searching users, each user got multiplied by the amount
of metadata entries they have, towards the `total_results` count. In
PostgreSQL the `COUNT(*) OVER()` window function does not support
`DISTINCT`. Even tho the query did a distinct select, the count would
still include duplicates.

# How the Problems Are Solved

Wrap the original query in a sub-select, so that the `DISTINCT` gets
handled before the count window function is executed in the outer
function. Filters, permission and solting is applied to the inner query.
Offset, limit and count are applied to the outer query.

# Additional Changes

- none

# Additional Context

- Closes https://github.com/zitadel/zitadel/issues/10825
- Backport to 4v

(cherry picked from commit f27ca69749)
This commit is contained in:
Tim Möhlmann
2025-10-14 12:43:59 +03:00
committed by Livio Spring
parent 45c7354234
commit cd059dc0cb
4 changed files with 345 additions and 153 deletions
Download Patch File Download Diff File Expand all files Collapse all files

160
internal/api/grpc/user/v2/integration_test/query_test.go
View File

@@ -6,6 +6,7 @@ import (
"context"
"encoding/base64"
"errors"
"fmt"
"slices"
"testing"
"time"
@@ -716,6 +717,86 @@ func TestServer_ListUsers(t *testing.T) {
},
},
},
{
// https://github.com/zitadel/zitadel/issues/10825
name: "list user with metadata by IDs with offset and limit, ok",
args: args{
IamCTX,
&user.ListUsersRequest{},
func(ctx context.Context, request *user.ListUsersRequest) userAttrs {
infos := createUsers(ctx, orgResp.OrganizationId, 3, false)
request.Queries = []*user.SearchQuery{}
request.Queries = append(request.Queries, OrganizationIdQuery(orgResp.OrganizationId))
request.Queries = append(request.Queries, InUserIDsQuery(infos.userIDs()))
// With the original bug, this would multiply the "TotalResult" by 2.
for _, user := range infos {
for i := 0; i < 2; i++ {
Instance.SetUserMetadata(ctx, user.UserID, fmt.Sprintf("key-%d", i), fmt.Sprintf("value-%d", i))
}
}
infos = infos[1:] // prevent panic in user ID setting below
request.SortingColumn = user.UserFieldName_USER_FIELD_NAME_CREATION_DATE
request.Query = &object.ListQuery{
Offset: 1,
Limit: 2,
}
return infos
},
},
want: &user.ListUsersResponse{
Details: &object.ListDetails{
TotalResult: 3,
Timestamp: timestamppb.Now(),
},
SortingColumn: user.UserFieldName_USER_FIELD_NAME_CREATION_DATE,
Result: []*user.User{
{
State: user.UserState_USER_STATE_ACTIVE,
Type: &user.User_Human{
Human: &user.HumanUser{
Profile: &user.HumanProfile{
GivenName: "Mickey",
FamilyName: "Mouse",
NickName: gu.Ptr("Mickey"),
DisplayName: gu.Ptr("Mickey Mouse"),
PreferredLanguage: gu.Ptr("nl"),
Gender: user.Gender_GENDER_MALE.Enum(),
},
Email: &user.HumanEmail{
IsVerified: true,
},
Phone: &user.HumanPhone{
IsVerified: true,
},
},
},
},
{
State: user.UserState_USER_STATE_ACTIVE,
Type: &user.User_Human{
Human: &user.HumanUser{
Profile: &user.HumanProfile{
GivenName: "Mickey",
FamilyName: "Mouse",
NickName: gu.Ptr("Mickey"),
DisplayName: gu.Ptr("Mickey Mouse"),
PreferredLanguage: gu.Ptr("nl"),
Gender: user.Gender_GENDER_MALE.Enum(),
},
Email: &user.HumanEmail{
IsVerified: true,
},
Phone: &user.HumanPhone{
IsVerified: true,
},
},
},
},
},
},
},
{
name: "list user by username, ok",
args: args{
@@ -900,6 +981,85 @@ func TestServer_ListUsers(t *testing.T) {
},
},
},
{
// https://github.com/zitadel/zitadel/issues/10825
name: "list user with metadata by emails with offset and limit, ok",
args: args{
IamCTX,
&user.ListUsersRequest{},
func(ctx context.Context, request *user.ListUsersRequest) userAttrs {
infos := createUsers(ctx, orgResp.OrganizationId, 3, false)
request.Queries = []*user.SearchQuery{}
request.Queries = append(request.Queries, OrganizationIdQuery(orgResp.OrganizationId))
request.Queries = append(request.Queries, InUserEmailsQuery(infos.emails()))
// With the original bug, this would multiply the "TotalResult" by 2.
for _, user := range infos {
for i := 0; i < 2; i++ {
Instance.SetUserMetadata(ctx, user.UserID, fmt.Sprintf("key-%d", i), fmt.Sprintf("value-%d", i))
}
}
infos = infos[1:] // prevent panic in user ID setting below
request.SortingColumn = user.UserFieldName_USER_FIELD_NAME_CREATION_DATE
request.Query = &object.ListQuery{
Offset: 1,
Limit: 2,
}
return infos
},
},
want: &user.ListUsersResponse{
Details: &object.ListDetails{
TotalResult: 3,
Timestamp: timestamppb.Now(),
},
SortingColumn: user.UserFieldName_USER_FIELD_NAME_CREATION_DATE,
Result: []*user.User{
{
State: user.UserState_USER_STATE_ACTIVE,
Type: &user.User_Human{
Human: &user.HumanUser{
Profile: &user.HumanProfile{
GivenName: "Mickey",
FamilyName: "Mouse",
NickName: gu.Ptr("Mickey"),
DisplayName: gu.Ptr("Mickey Mouse"),
PreferredLanguage: gu.Ptr("nl"),
Gender: user.Gender_GENDER_MALE.Enum(),
},
Email: &user.HumanEmail{
IsVerified: true,
},
Phone: &user.HumanPhone{
IsVerified: true,
},
},
},
}, {
State: user.UserState_USER_STATE_ACTIVE,
Type: &user.User_Human{
Human: &user.HumanUser{
Profile: &user.HumanProfile{
GivenName: "Mickey",
FamilyName: "Mouse",
NickName: gu.Ptr("Mickey"),
DisplayName: gu.Ptr("Mickey Mouse"),
PreferredLanguage: gu.Ptr("nl"),
Gender: user.Gender_GENDER_MALE.Enum(),
},
Email: &user.HumanEmail{
IsVerified: true,
},
Phone: &user.HumanPhone{
IsVerified: true,
},
},
},
},
},
},
},
{
name: "list user in emails no found, ok",
args: args{

11
internal/query/search_query.go
View File

@@ -23,6 +23,8 @@ type SearchRequest struct {
Limit uint64
SortingColumn Column
Asc bool
sortingConsumed bool
}
func (req *SearchRequest) toQuery(query sq.SelectBuilder) sq.SelectBuilder {
@@ -32,15 +34,20 @@ func (req *SearchRequest) toQuery(query sq.SelectBuilder) sq.SelectBuilder {
if req.Limit > 0 {
query = query.Limit(req.Limit)
}
return req.consumeSorting(query)
}
if !req.SortingColumn.isZero() {
// consumeSorting sets the sorting column to the query once.
// subsequent calls will not set the sorting column again.
func (req *SearchRequest) consumeSorting(query sq.SelectBuilder) sq.SelectBuilder {
if !req.sortingConsumed && !req.SortingColumn.isZero() {
clause := req.SortingColumn.orderBy()
if !req.Asc {
clause += " DESC"
}
query = query.OrderByClause(clause)
req.sortingConsumed = true
}
return query
}

293
internal/query/user.go
View File

@@ -634,11 +634,8 @@ func (q *Queries) searchUsers(ctx context.Context, queries *UserSearchQueries, p
ctx, span := tracing.NewSpan(ctx)
defer func() { span.EndWithError(err) }()
query, scan := prepareUsersQuery(queries.SortingColumn)
query = userPermissionCheckV2(ctx, query, permissionCheckV2, queries.Queries)
stmt, args, err := queries.toQuery(query).Where(sq.Eq{
UserInstanceIDCol.identifier(): authz.GetInstance(ctx).InstanceID(),
}).ToSql()
query, scan := queries.prepareUsersQuery(ctx, permissionCheckV2)
stmt, args, err := queries.toQuery(query).ToSql()
if err != nil {
return nil, zerrors.ThrowInternal(err, "QUERY-Dgbg2", "Errors.Query.SQLStatement")
}
@@ -726,14 +723,6 @@ func (q *Queries) SearchClaimedUserIDsOfOrgDomain(ctx context.Context, domain, o
return userIDs, err
}
func (q *UserSearchQueries) toQuery(query sq.SelectBuilder) sq.SelectBuilder {
query = q.SearchRequest.toQuery(query)
for _, q := range q.Queries {
query = q.toQuery(query)
}
return query
}
func (r *UserSearchQueries) AppendMyResourceOwnerQuery(orgID string) error {
query, err := NewUserResourceOwnerSearchQuery(orgID, TextEquals)
if err != nil {
@@ -1270,147 +1259,171 @@ func prepareUserUniqueQuery() (sq.SelectBuilder, func(*sql.Row) (bool, error)) {
}
}
func prepareUsersQuery(orderBy Column) (sq.SelectBuilder, func(*sql.Rows) (*Users, error)) {
if orderBy.isZero() {
orderBy = UserIDCol
// 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.
// 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.
func (q *UserSearchQueries) prepareUsersQuery(ctx context.Context, permissionCheckV2 bool) (sq.SelectBuilder, func(*sql.Rows) (*Users, error)) {
if q.SortingColumn.isZero() {
q.SortingColumn = UserIDCol
}
return sq.Select(
UserIDCol.identifier(),
UserCreationDateCol.identifier(),
UserChangeDateCol.identifier(),
UserResourceOwnerCol.identifier(),
UserSequenceCol.identifier(),
UserStateCol.identifier(),
UserTypeCol.identifier(),
UserUsernameCol.identifier(),
userLoginNamesListCol.identifier(),
userPreferredLoginNameCol.identifier(),
HumanUserIDCol.identifier(),
HumanFirstNameCol.identifier(),
HumanLastNameCol.identifier(),
HumanNickNameCol.identifier(),
HumanDisplayNameCol.identifier(),
HumanPreferredLanguageCol.identifier(),
HumanGenderCol.identifier(),
HumanAvatarURLCol.identifier(),
HumanEmailCol.identifier(),
HumanIsEmailVerifiedCol.identifier(),
HumanPhoneCol.identifier(),
HumanIsPhoneVerifiedCol.identifier(),
HumanPasswordChangeRequiredCol.identifier(),
HumanPasswordChangedCol.identifier(),
HumanMFAInitSkippedCol.identifier(),
MachineUserIDCol.identifier(),
MachineNameCol.identifier(),
MachineDescriptionCol.identifier(),
MachineSecretCol.identifier(),
MachineAccessTokenTypeCol.identifier(),
orderBy.orderBy(),
countColumn.identifier()).
Distinct().
From(userTable.identifier()).
LeftJoin(join(HumanUserIDCol, UserIDCol)).
LeftJoin(join(MachineUserIDCol, UserIDCol)).
LeftJoin(join(UserMetadataUserIDCol, UserIDCol)).
JoinClause(joinLoginNames).
PlaceholderFormat(sq.Dollar),
func(rows *sql.Rows) (*Users, error) {
users := make([]*User, 0)
var count uint64
for rows.Next() {
u := new(User)
loginNames := database.TextArray[string]{}
preferredLoginName := sql.NullString{}
human, machine := sqlHuman{}, sqlMachine{}
var orderByValue any
// start building the sub-select
query := sq.Select(
UserIDCol.identifier(),
UserCreationDateCol.identifier(),
UserChangeDateCol.identifier(),
UserResourceOwnerCol.identifier(),
UserSequenceCol.identifier(),
UserStateCol.identifier(),
UserTypeCol.identifier(),
UserUsernameCol.identifier(),
userLoginNamesListCol.identifier(),
userPreferredLoginNameCol.identifier(),
HumanUserIDCol.identifier(),
HumanFirstNameCol.identifier(),
HumanLastNameCol.identifier(),
HumanNickNameCol.identifier(),
HumanDisplayNameCol.identifier(),
HumanPreferredLanguageCol.identifier(),
HumanGenderCol.identifier(),
HumanAvatarURLCol.identifier(),
HumanEmailCol.identifier(),
HumanIsEmailVerifiedCol.identifier(),
HumanPhoneCol.identifier(),
HumanIsPhoneVerifiedCol.identifier(),
HumanPasswordChangeRequiredCol.identifier(),
HumanPasswordChangedCol.identifier(),
HumanMFAInitSkippedCol.identifier(),
MachineUserIDCol.identifier(),
MachineNameCol.identifier(),
MachineDescriptionCol.identifier(),
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()})
err := rows.Scan(
&u.ID,
&u.CreationDate,
&u.ChangeDate,
&u.ResourceOwner,
&u.Sequence,
&u.State,
&u.Type,
&u.Username,
&loginNames,
&preferredLoginName,
query = userPermissionCheckV2(ctx, query, permissionCheckV2, q.Queries)
// apply requested filters
for _, q := range q.Queries {
query = q.toQuery(query)
}
// apply sorting in the sub-select,because the identifier is fully qualified.
query = q.consumeSorting(query)
&human.humanID,
&human.firstName,
&human.lastName,
&human.nickName,
&human.displayName,
&human.preferredLanguage,
&human.gender,
&human.avatarKey,
&human.email,
&human.isEmailVerified,
&human.phone,
&human.isPhoneVerified,
&human.passwordChangeRequired,
&human.passwordChanged,
&human.mfaInitSkipped,
// set the sub-select as source for the outer query
query = sq.Select(
"*",
countColumn.identifier(),
).FromSelect(query, "results")
&machine.machineID,
&machine.name,
&machine.description,
&machine.encodedSecret,
&machine.accessTokenType,
// apply limit and offset in the outer query
query = q.toQuery(query)
query = query.PlaceholderFormat(sq.Dollar)
&orderByValue,
&count,
)
if err != nil {
return nil, err
}
return query, func(rows *sql.Rows) (*Users, error) {
users := make([]*User, 0)
var count uint64
for rows.Next() {
u := new(User)
loginNames := database.TextArray[string]{}
preferredLoginName := sql.NullString{}
u.LoginNames = loginNames
if preferredLoginName.Valid {
u.PreferredLoginName = preferredLoginName.String
}
human, machine := sqlHuman{}, sqlMachine{}
var orderByValue any
if human.humanID.Valid {
u.Human = &Human{
FirstName: human.firstName.String,
LastName: human.lastName.String,
NickName: human.nickName.String,
DisplayName: human.displayName.String,
AvatarKey: human.avatarKey.String,
PreferredLanguage: language.Make(human.preferredLanguage.String),
Gender: domain.Gender(human.gender.Int32),
Email: domain.EmailAddress(human.email.String),
IsEmailVerified: human.isEmailVerified.Bool,
Phone: domain.PhoneNumber(human.phone.String),
IsPhoneVerified: human.isPhoneVerified.Bool,
PasswordChangeRequired: human.passwordChangeRequired.Bool,
PasswordChanged: human.passwordChanged.Time,
MFAInitSkipped: human.mfaInitSkipped.Time,
}
} else if machine.machineID.Valid {
u.Machine = &Machine{
Name: machine.name.String,
Description: machine.description.String,
EncodedSecret: machine.encodedSecret.String,
AccessTokenType: domain.OIDCTokenType(machine.accessTokenType.Int32),
}
}
err := rows.Scan(
&u.ID,
&u.CreationDate,
&u.ChangeDate,
&u.ResourceOwner,
&u.Sequence,
&u.State,
&u.Type,
&u.Username,
&loginNames,
&preferredLoginName,
users = append(users, u)
&human.humanID,
&human.firstName,
&human.lastName,
&human.nickName,
&human.displayName,
&human.preferredLanguage,
&human.gender,
&human.avatarKey,
&human.email,
&human.isEmailVerified,
&human.phone,
&human.isPhoneVerified,
&human.passwordChangeRequired,
&human.passwordChanged,
&human.mfaInitSkipped,
&machine.machineID,
&machine.name,
&machine.description,
&machine.encodedSecret,
&machine.accessTokenType,
&orderByValue,
&count,
)
if err != nil {
return nil, err
}
if err := rows.Close(); err != nil {
return nil, zerrors.ThrowInternal(err, "QUERY-frhbd", "Errors.Query.CloseRows")
u.LoginNames = loginNames
if preferredLoginName.Valid {
u.PreferredLoginName = preferredLoginName.String
}
return &Users{
Users: users,
SearchResponse: SearchResponse{
Count: count,
},
}, nil
if human.humanID.Valid {
u.Human = &Human{
FirstName: human.firstName.String,
LastName: human.lastName.String,
NickName: human.nickName.String,
DisplayName: human.displayName.String,
AvatarKey: human.avatarKey.String,
PreferredLanguage: language.Make(human.preferredLanguage.String),
Gender: domain.Gender(human.gender.Int32),
Email: domain.EmailAddress(human.email.String),
IsEmailVerified: human.isEmailVerified.Bool,
Phone: domain.PhoneNumber(human.phone.String),
IsPhoneVerified: human.isPhoneVerified.Bool,
PasswordChangeRequired: human.passwordChangeRequired.Bool,
PasswordChanged: human.passwordChanged.Time,
MFAInitSkipped: human.mfaInitSkipped.Time,
}
} else if machine.machineID.Valid {
u.Machine = &Machine{
Name: machine.name.String,
Description: machine.description.String,
EncodedSecret: machine.encodedSecret.String,
AccessTokenType: domain.OIDCTokenType(machine.accessTokenType.Int32),
}
}
users = append(users, u)
}
if err := rows.Close(); err != nil {
return nil, zerrors.ThrowInternal(err, "QUERY-frhbd", "Errors.Query.CloseRows")
}
return &Users{
Users: users,
SearchResponse: SearchResponse{
Count: count,
},
}, nil
}
}
type sqlHuman struct {

34
internal/query/user_test.go
View File

@@ -367,7 +367,8 @@ var (
"password_set",
"count",
}
usersQuery = `SELECT DISTINCT projections.users14.id,` +
usersQuery = `SELECT *, COUNT(*) OVER () FROM (` +
`SELECT DISTINCT projections.users14.id,` +
` projections.users14.creation_date,` +
` projections.users14.change_date,` +
` projections.users14.resource_owner,` +
@@ -397,13 +398,14 @@ var (
` projections.users14_machines.description,` +
` projections.users14_machines.secret,` +
` projections.users14_machines.access_token_type,` +
` projections.users14.id,` +
` COUNT(*) OVER ()` +
` projections.users14.id` +
` 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`
` 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`
usersCols = []string{
"id",
"creation_date",
@@ -949,7 +951,8 @@ func Test_UserPrepares(t *testing.T) {
{
name: "prepareUsersQuery no result",
prepare: func() (sq.SelectBuilder, func(*sql.Rows) (*Users, error)) {
return prepareUsersQuery(UserIDCol)
q := &UserSearchQueries{}
return q.prepareUsersQuery(t.Context(), false)
},
want: want{
sqlExpectations: mockQuery(
@@ -969,7 +972,8 @@ func Test_UserPrepares(t *testing.T) {
{
name: "prepareUsersQuery one result",
prepare: func() (sq.SelectBuilder, func(*sql.Rows) (*Users, error)) {
return prepareUsersQuery(UserIDCol)
q := &UserSearchQueries{}
return q.prepareUsersQuery(t.Context(), false)
},
want: want{
sqlExpectations: mockQueries(
@@ -1053,7 +1057,8 @@ func Test_UserPrepares(t *testing.T) {
{
name: "prepareUsersQuery one result, no sorting",
prepare: func() (sq.SelectBuilder, func(*sql.Rows) (*Users, error)) {
return prepareUsersQuery(Column{})
q := &UserSearchQueries{}
return q.prepareUsersQuery(t.Context(), false)
},
want: want{
sqlExpectations: mockQueries(
@@ -1135,13 +1140,19 @@ func Test_UserPrepares(t *testing.T) {
},
},
{
name: "prepareUsersQuery multiple results",
name: "prepareUsersQuery multiple results, offset and limit",
prepare: func() (sq.SelectBuilder, func(*sql.Rows) (*Users, error)) {
return prepareUsersQuery(UserIDCol)
q := &UserSearchQueries{
SearchRequest: SearchRequest{
Offset: 1,
Limit: 2,
},
}
return q.prepareUsersQuery(t.Context(), false)
},
want: want{
sqlExpectations: mockQueries(
regexp.QuoteMeta(usersQuery),
regexp.QuoteMeta(usersQuery+` LIMIT 2 OFFSET 1`),
usersCols,
[][]driver.Value{
{
@@ -1274,7 +1285,8 @@ func Test_UserPrepares(t *testing.T) {
{
name: "prepareUsersQuery sql err",
prepare: func() (sq.SelectBuilder, func(*sql.Rows) (*Users, error)) {
return prepareUsersQuery(UserIDCol)
q := &UserSearchQueries{}
return q.prepareUsersQuery(t.Context(), false)
},
want: want{
sqlExpectations: mockQueryErr(