From fefeaea56ad21dea5b808f8c7c89bfab6d9840e1 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Thu, 10 Jul 2025 11:17:49 -0400 Subject: [PATCH] perf: improve org and org domain creation (#10232) # Which Problems Are Solved When an organization domain is verified, e.g. also when creating a new organization (incl. generated domain), existing usernames are checked if the domain has been claimed. The query was not optimized for instances with many users and organizations. # How the Problems Are Solved - Replace the query, which was searching over the users projection with (computed loginnames) with a dedicated query checking the loginnames projection directly. - All occurrences have been updated to use the new query. # Additional Changes None # Additional Context - reported through support - requires backport to v3.x --- internal/api/grpc/admin/org.go | 15 +---- internal/api/grpc/management/org.go | 23 +------ .../org/v2beta/integration_test/org_test.go | 64 +++++++++++++++++++ internal/api/grpc/org/v2beta/org.go | 23 +------ internal/api/ui/login/login.go | 14 +--- internal/query/user.go | 29 +++++++++ internal/query/user_claimed_user_ids.sql | 13 ++++ 7 files changed, 110 insertions(+), 71 deletions(-) create mode 100644 internal/query/user_claimed_user_ids.sql diff --git a/internal/api/grpc/admin/org.go b/internal/api/grpc/admin/org.go index ef97e47bb0..a2b55cf21c 100644 --- a/internal/api/grpc/admin/org.go +++ b/internal/api/grpc/admin/org.go @@ -9,7 +9,6 @@ import ( http_utils "github.com/zitadel/zitadel/internal/api/http" "github.com/zitadel/zitadel/internal/command" "github.com/zitadel/zitadel/internal/domain" - "github.com/zitadel/zitadel/internal/query" admin_pb "github.com/zitadel/zitadel/pkg/grpc/admin" ) @@ -104,17 +103,5 @@ func (s *Server) SetUpOrg(ctx context.Context, req *admin_pb.SetUpOrgRequest) (* } func (s *Server) getClaimedUserIDsOfOrgDomain(ctx context.Context, orgDomain string) ([]string, error) { - loginName, err := query.NewUserPreferredLoginNameSearchQuery("@"+orgDomain, query.TextEndsWithIgnoreCase) - if err != nil { - return nil, err - } - users, err := s.query.SearchUsers(ctx, &query.UserSearchQueries{Queries: []query.SearchQuery{loginName}}, nil) - if err != nil { - return nil, err - } - userIDs := make([]string, len(users.Users)) - for i, user := range users.Users { - userIDs[i] = user.ID - } - return userIDs, nil + return s.query.SearchClaimedUserIDsOfOrgDomain(ctx, orgDomain, "") } diff --git a/internal/api/grpc/management/org.go b/internal/api/grpc/management/org.go index ee4f5eb633..a006db063d 100644 --- a/internal/api/grpc/management/org.go +++ b/internal/api/grpc/management/org.go @@ -316,28 +316,7 @@ func (s *Server) RemoveOrgMember(ctx context.Context, req *mgmt_pb.RemoveOrgMemb } func (s *Server) getClaimedUserIDsOfOrgDomain(ctx context.Context, orgDomain, orgID string) ([]string, error) { - queries := make([]query.SearchQuery, 0, 2) - loginName, err := query.NewUserPreferredLoginNameSearchQuery("@"+orgDomain, query.TextEndsWithIgnoreCase) - if err != nil { - return nil, err - } - queries = append(queries, loginName) - if orgID != "" { - owner, err := query.NewUserResourceOwnerSearchQuery(orgID, query.TextNotEquals) - if err != nil { - return nil, err - } - queries = append(queries, owner) - } - users, err := s.query.SearchUsers(ctx, &query.UserSearchQueries{Queries: queries}, nil) - if err != nil { - return nil, err - } - userIDs := make([]string, len(users.Users)) - for i, user := range users.Users { - userIDs[i] = user.ID - } - return userIDs, nil + return s.query.SearchClaimedUserIDsOfOrgDomain(ctx, orgDomain, orgID) } func (s *Server) ListOrgMetadata(ctx context.Context, req *mgmt_pb.ListOrgMetadataRequest) (*mgmt_pb.ListOrgMetadataResponse, error) { diff --git a/internal/api/grpc/org/v2beta/integration_test/org_test.go b/internal/api/grpc/org/v2beta/integration_test/org_test.go index 0d3b920afe..d36c570b92 100644 --- a/internal/api/grpc/org/v2beta/integration_test/org_test.go +++ b/internal/api/grpc/org/v2beta/integration_test/org_test.go @@ -1044,6 +1044,70 @@ func TestServer_AddOrganizationDomain(t *testing.T) { } } +func TestServer_AddOrganizationDomain_ClaimDomain(t *testing.T) { + domain := gofakeit.DomainName() + + // create an organization, ensure it has globally unique usernames + // and create a user with a loginname that matches the domain later on + organization, err := Client.CreateOrganization(CTX, &v2beta_org.CreateOrganizationRequest{ + Name: gofakeit.AppName(), + }) + require.NoError(t, err) + _, err = Instance.Client.Admin.AddCustomDomainPolicy(CTX, &admin.AddCustomDomainPolicyRequest{ + OrgId: organization.GetId(), + UserLoginMustBeDomain: false, + }) + require.NoError(t, err) + username := gofakeit.Username() + "@" + domain + ownUser := Instance.CreateHumanUserVerified(CTX, organization.GetId(), username, "") + + // create another organization, ensure it has globally unique usernames + // and create a user with a loginname that matches the domain later on + otherOrg, err := Client.CreateOrganization(CTX, &v2beta_org.CreateOrganizationRequest{ + Name: gofakeit.AppName(), + }) + require.NoError(t, err) + _, err = Instance.Client.Admin.AddCustomDomainPolicy(CTX, &admin.AddCustomDomainPolicyRequest{ + OrgId: otherOrg.GetId(), + UserLoginMustBeDomain: false, + }) + require.NoError(t, err) + + otherUsername := gofakeit.Username() + "@" + domain + otherUser := Instance.CreateHumanUserVerified(CTX, otherOrg.GetId(), otherUsername, "") + + // if we add the domain now to the first organization, it should be claimed on the second organization, resp. its user(s) + _, err = Client.AddOrganizationDomain(CTX, &v2beta_org.AddOrganizationDomainRequest{ + OrganizationId: organization.GetId(), + Domain: domain, + }) + require.NoError(t, err) + + // check both users: the first one must be untouched, the second one must be updated + users, err := Instance.Client.UserV2.ListUsers(CTX, &user.ListUsersRequest{ + Queries: []*user.SearchQuery{ + { + Query: &user.SearchQuery_InUserIdsQuery{ + InUserIdsQuery: &user.InUserIDQuery{UserIds: []string{ownUser.GetUserId(), otherUser.GetUserId()}}, + }, + }, + }, + }) + require.NoError(t, err) + require.Len(t, users.GetResult(), 2) + + for _, u := range users.GetResult() { + if u.GetUserId() == ownUser.GetUserId() { + assert.Equal(t, username, u.GetPreferredLoginName()) + continue + } + if u.GetUserId() == otherUser.GetUserId() { + assert.NotEqual(t, otherUsername, u.GetPreferredLoginName()) + assert.Contains(t, u.GetPreferredLoginName(), "@temporary.") + } + } +} + func TestServer_ListOrganizationDomains(t *testing.T) { domain := gofakeit.URL() tests := []struct { diff --git a/internal/api/grpc/org/v2beta/org.go b/internal/api/grpc/org/v2beta/org.go index 35e1d72d3c..edc667409a 100644 --- a/internal/api/grpc/org/v2beta/org.go +++ b/internal/api/grpc/org/v2beta/org.go @@ -250,26 +250,5 @@ func createOrganizationRequestAdminToCommand(admin *v2beta_org.CreateOrganizatio } func (s *Server) getClaimedUserIDsOfOrgDomain(ctx context.Context, orgDomain, orgID string) ([]string, error) { - queries := make([]query.SearchQuery, 0, 2) - loginName, err := query.NewUserPreferredLoginNameSearchQuery("@"+orgDomain, query.TextEndsWithIgnoreCase) - if err != nil { - return nil, err - } - queries = append(queries, loginName) - if orgID != "" { - owner, err := query.NewUserResourceOwnerSearchQuery(orgID, query.TextNotEquals) - if err != nil { - return nil, err - } - queries = append(queries, owner) - } - users, err := s.query.SearchUsers(ctx, &query.UserSearchQueries{Queries: queries}, nil) - if err != nil { - return nil, err - } - userIDs := make([]string, len(users.Users)) - for i, user := range users.Users { - userIDs[i] = user.ID - } - return userIDs, nil + return s.query.SearchClaimedUserIDsOfOrgDomain(ctx, orgDomain, orgID) } diff --git a/internal/api/ui/login/login.go b/internal/api/ui/login/login.go index f1ce9bfa2a..5ff27c14fc 100644 --- a/internal/api/ui/login/login.go +++ b/internal/api/ui/login/login.go @@ -178,19 +178,7 @@ func (l *Login) getClaimedUserIDsOfOrgDomain(ctx context.Context, orgName string if err != nil { return nil, err } - loginName, err := query.NewUserPreferredLoginNameSearchQuery("@"+orgDomain, query.TextEndsWithIgnoreCase) - if err != nil { - return nil, err - } - users, err := l.query.SearchUsers(ctx, &query.UserSearchQueries{Queries: []query.SearchQuery{loginName}}, nil) - if err != nil { - return nil, err - } - userIDs := make([]string, len(users.Users)) - for i, user := range users.Users { - userIDs[i] = user.ID - } - return userIDs, nil + return l.query.SearchClaimedUserIDsOfOrgDomain(ctx, orgDomain, "") } func setContext(ctx context.Context, resourceOwner string) context.Context { diff --git a/internal/query/user.go b/internal/query/user.go index ac3eb79fc9..66c7abb228 100644 --- a/internal/query/user.go +++ b/internal/query/user.go @@ -697,6 +697,35 @@ func (q *Queries) IsUserUnique(ctx context.Context, username, email, resourceOwn return isUnique, err } +//go:embed user_claimed_user_ids.sql +var userClaimedUserIDOfOrgDomain string + +func (q *Queries) SearchClaimedUserIDsOfOrgDomain(ctx context.Context, domain, orgID string) (userIDs []string, err error) { + ctx, span := tracing.NewSpan(ctx) + defer func() { span.EndWithError(err) }() + + err = q.client.QueryContext(ctx, + func(rows *sql.Rows) error { + userIDs = make([]string, 0) + for rows.Next() { + var userID string + err := rows.Scan(&userID) + if err != nil { + return err + } + userIDs = append(userIDs, userID) + } + return nil + }, + userClaimedUserIDOfOrgDomain, + authz.GetInstance(ctx).InstanceID(), + "%@"+domain, + orgID, + ) + + return userIDs, err +} + func (q *UserSearchQueries) toQuery(query sq.SelectBuilder) sq.SelectBuilder { query = q.SearchRequest.toQuery(query) for _, q := range q.Queries { diff --git a/internal/query/user_claimed_user_ids.sql b/internal/query/user_claimed_user_ids.sql new file mode 100644 index 0000000000..5d4639be46 --- /dev/null +++ b/internal/query/user_claimed_user_ids.sql @@ -0,0 +1,13 @@ +SELECT u.id +FROM projections.login_names3_users u + LEFT JOIN projections.login_names3_policies p_custom + ON u.instance_id = p_custom.instance_id + AND p_custom.instance_id = $1 + AND p_custom.resource_owner = u.resource_owner + JOIN projections.login_names3_policies p_default + ON u.instance_id = p_default.instance_id + AND p_default.instance_id = $1 AND p_default.is_default IS TRUE +WHERE u.instance_id = $1 + AND COALESCE(p_custom.must_be_domain, p_default.must_be_domain) = false + AND u.user_name_lower like $2 + AND u.resource_owner <> $3; \ No newline at end of file