From 427cbe06f38f8f18fc7f643cffa024d0111f9d32 Mon Sep 17 00:00:00 2001 From: Silvan <27845747+adlerhurst@users.noreply.github.com> Date: Tue, 12 Aug 2025 11:37:08 +0200 Subject: [PATCH] fix: query organization directly from event store (#10463) Querying an organization by id allowed to trigger the org projection. This could lead to performance impacts if the projection gets triggered too often. Instead of executing the trigger the organization by id query is now always executed on the eventstore and reduces all event types required of the organization requested. --------- Co-authored-by: Livio Spring --- internal/api/grpc/admin/import.go | 2 +- internal/api/grpc/admin/org.go | 4 +- internal/api/grpc/feature/v2/converter.go | 4 -- .../api/grpc/feature/v2/converter_test.go | 8 ++-- internal/api/grpc/feature/v2beta/converter.go | 4 -- .../api/grpc/feature/v2beta/converter_test.go | 8 ++-- internal/api/grpc/management/org.go | 2 +- internal/api/oidc/client.go | 2 +- internal/api/ui/login/renderer.go | 2 +- .../eventsourcing/eventstore/auth_request.go | 6 +-- .../eventstore/auth_request_test.go | 4 +- internal/feature/feature.go | 13 ++++--- internal/query/org.go | 38 +------------------ proto/buf.yaml | 3 ++ proto/zitadel/feature/v2/feature.proto | 8 ++-- proto/zitadel/feature/v2beta/feature.proto | 8 ++-- 16 files changed, 39 insertions(+), 77 deletions(-) diff --git a/internal/api/grpc/admin/import.go b/internal/api/grpc/admin/import.go index ac085c135..a4e7f3f7c 100644 --- a/internal/api/grpc/admin/import.go +++ b/internal/api/grpc/admin/import.go @@ -310,7 +310,7 @@ func importOrg1(ctx context.Context, s *Server, errors *[]*admin_pb.ImportDataEr _, err = s.command.AddOrgWithID(ctx, org.GetOrg().GetName(), ctxData.UserID, ctxData.ResourceOwner, org.GetOrgId(), setOrgInactive, []string{}) if err != nil { *errors = append(*errors, &admin_pb.ImportDataError{Type: "org", Id: org.GetOrgId(), Message: err.Error()}) - if _, err := s.query.OrgByID(ctx, true, org.OrgId); err != nil { + if _, err := s.query.OrgByID(ctx, org.OrgId); err != nil { // TODO: Only nil if err != not found return nil } diff --git a/internal/api/grpc/admin/org.go b/internal/api/grpc/admin/org.go index a2b55cf21..ca5731c43 100644 --- a/internal/api/grpc/admin/org.go +++ b/internal/api/grpc/admin/org.go @@ -38,7 +38,7 @@ func (s *Server) RemoveOrg(ctx context.Context, req *admin_pb.RemoveOrgRequest) } func (s *Server) GetDefaultOrg(ctx context.Context, _ *admin_pb.GetDefaultOrgRequest) (*admin_pb.GetDefaultOrgResponse, error) { - org, err := s.query.OrgByID(ctx, true, authz.GetInstance(ctx).DefaultOrganisationID()) + org, err := s.query.OrgByID(ctx, authz.GetInstance(ctx).DefaultOrganisationID()) if err != nil { return nil, err } @@ -46,7 +46,7 @@ func (s *Server) GetDefaultOrg(ctx context.Context, _ *admin_pb.GetDefaultOrgReq } func (s *Server) GetOrgByID(ctx context.Context, req *admin_pb.GetOrgByIDRequest) (*admin_pb.GetOrgByIDResponse, error) { - org, err := s.query.OrgByID(ctx, true, req.Id) + org, err := s.query.OrgByID(ctx, req.Id) if err != nil { return nil, err } diff --git a/internal/api/grpc/feature/v2/converter.go b/internal/api/grpc/feature/v2/converter.go index ab8ddc7d7..a91b7b15e 100644 --- a/internal/api/grpc/feature/v2/converter.go +++ b/internal/api/grpc/feature/v2/converter.go @@ -164,8 +164,6 @@ func improvedPerformanceTypeToPb(typ feature.ImprovedPerformanceType) feature_pb switch typ { case feature.ImprovedPerformanceTypeUnspecified: return feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_UNSPECIFIED - case feature.ImprovedPerformanceTypeOrgByID: - return feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_ORG_BY_ID case feature.ImprovedPerformanceTypeProjectGrant: return feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_PROJECT_GRANT case feature.ImprovedPerformanceTypeProject: @@ -196,8 +194,6 @@ func improvedPerformanceToDomain(typ feature_pb.ImprovedPerformance) feature.Imp switch typ { case feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_UNSPECIFIED: return feature.ImprovedPerformanceTypeUnspecified - case feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_ORG_BY_ID: - return feature.ImprovedPerformanceTypeOrgByID case feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_PROJECT_GRANT: return feature.ImprovedPerformanceTypeProjectGrant case feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_PROJECT: diff --git a/internal/api/grpc/feature/v2/converter_test.go b/internal/api/grpc/feature/v2/converter_test.go index 7b11fc0d1..103b23c01 100644 --- a/internal/api/grpc/feature/v2/converter_test.go +++ b/internal/api/grpc/feature/v2/converter_test.go @@ -66,7 +66,7 @@ func Test_systemFeaturesToPb(t *testing.T) { }, ImprovedPerformance: query.FeatureSource[[]feature.ImprovedPerformanceType]{ Level: feature.LevelSystem, - Value: []feature.ImprovedPerformanceType{feature.ImprovedPerformanceTypeOrgByID}, + Value: []feature.ImprovedPerformanceType{feature.ImprovedPerformanceTypeOrgDomainVerified}, }, OIDCSingleV1SessionTermination: query.FeatureSource[bool]{ Level: feature.LevelSystem, @@ -107,7 +107,7 @@ func Test_systemFeaturesToPb(t *testing.T) { Source: feature_pb.Source_SOURCE_SYSTEM, }, ImprovedPerformance: &feature_pb.ImprovedPerformanceFeatureFlag{ - ExecutionPaths: []feature_pb.ImprovedPerformance{feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_ORG_BY_ID}, + ExecutionPaths: []feature_pb.ImprovedPerformance{feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_ORG_DOMAIN_VERIFIED}, Source: feature_pb.Source_SOURCE_SYSTEM, }, OidcSingleV1SessionTermination: &feature_pb.FeatureFlag{ @@ -191,7 +191,7 @@ func Test_instanceFeaturesToPb(t *testing.T) { }, ImprovedPerformance: query.FeatureSource[[]feature.ImprovedPerformanceType]{ Level: feature.LevelSystem, - Value: []feature.ImprovedPerformanceType{feature.ImprovedPerformanceTypeOrgByID}, + Value: []feature.ImprovedPerformanceType{feature.ImprovedPerformanceTypeOrgDomainVerified}, }, OIDCSingleV1SessionTermination: query.FeatureSource[bool]{ Level: feature.LevelInstance, @@ -236,7 +236,7 @@ func Test_instanceFeaturesToPb(t *testing.T) { Source: feature_pb.Source_SOURCE_SYSTEM, }, ImprovedPerformance: &feature_pb.ImprovedPerformanceFeatureFlag{ - ExecutionPaths: []feature_pb.ImprovedPerformance{feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_ORG_BY_ID}, + ExecutionPaths: []feature_pb.ImprovedPerformance{feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_ORG_DOMAIN_VERIFIED}, Source: feature_pb.Source_SOURCE_SYSTEM, }, DebugOidcParentError: &feature_pb.FeatureFlag{ diff --git a/internal/api/grpc/feature/v2beta/converter.go b/internal/api/grpc/feature/v2beta/converter.go index dc791d4c5..918bad6e7 100644 --- a/internal/api/grpc/feature/v2beta/converter.go +++ b/internal/api/grpc/feature/v2beta/converter.go @@ -101,8 +101,6 @@ func improvedPerformanceTypeToPb(typ feature.ImprovedPerformanceType) feature_pb switch typ { case feature.ImprovedPerformanceTypeUnspecified: return feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_UNSPECIFIED - case feature.ImprovedPerformanceTypeOrgByID: - return feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_ORG_BY_ID case feature.ImprovedPerformanceTypeProjectGrant: return feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_PROJECT_GRANT case feature.ImprovedPerformanceTypeProject: @@ -133,8 +131,6 @@ func improvedPerformanceToDomain(typ feature_pb.ImprovedPerformance) feature.Imp switch typ { case feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_UNSPECIFIED: return feature.ImprovedPerformanceTypeUnspecified - case feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_ORG_BY_ID: - return feature.ImprovedPerformanceTypeOrgByID case feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_PROJECT_GRANT: return feature.ImprovedPerformanceTypeProjectGrant case feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_PROJECT: diff --git a/internal/api/grpc/feature/v2beta/converter_test.go b/internal/api/grpc/feature/v2beta/converter_test.go index ec681011f..7ec9970ad 100644 --- a/internal/api/grpc/feature/v2beta/converter_test.go +++ b/internal/api/grpc/feature/v2beta/converter_test.go @@ -56,7 +56,7 @@ func Test_systemFeaturesToPb(t *testing.T) { }, ImprovedPerformance: query.FeatureSource[[]feature.ImprovedPerformanceType]{ Level: feature.LevelSystem, - Value: []feature.ImprovedPerformanceType{feature.ImprovedPerformanceTypeOrgByID}, + Value: []feature.ImprovedPerformanceType{feature.ImprovedPerformanceTypeOrgDomainVerified}, }, OIDCSingleV1SessionTermination: query.FeatureSource[bool]{ Level: feature.LevelSystem, @@ -82,7 +82,7 @@ func Test_systemFeaturesToPb(t *testing.T) { Source: feature_pb.Source_SOURCE_SYSTEM, }, ImprovedPerformance: &feature_pb.ImprovedPerformanceFeatureFlag{ - ExecutionPaths: []feature_pb.ImprovedPerformance{feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_ORG_BY_ID}, + ExecutionPaths: []feature_pb.ImprovedPerformance{feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_ORG_DOMAIN_VERIFIED}, Source: feature_pb.Source_SOURCE_SYSTEM, }, OidcSingleV1SessionTermination: &feature_pb.FeatureFlag{ @@ -134,7 +134,7 @@ func Test_instanceFeaturesToPb(t *testing.T) { }, ImprovedPerformance: query.FeatureSource[[]feature.ImprovedPerformanceType]{ Level: feature.LevelSystem, - Value: []feature.ImprovedPerformanceType{feature.ImprovedPerformanceTypeOrgByID}, + Value: []feature.ImprovedPerformanceType{feature.ImprovedPerformanceTypeOrgDomainVerified}, }, OIDCSingleV1SessionTermination: query.FeatureSource[bool]{ Level: feature.LevelInstance, @@ -160,7 +160,7 @@ func Test_instanceFeaturesToPb(t *testing.T) { Source: feature_pb.Source_SOURCE_SYSTEM, }, ImprovedPerformance: &feature_pb.ImprovedPerformanceFeatureFlag{ - ExecutionPaths: []feature_pb.ImprovedPerformance{feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_ORG_BY_ID}, + ExecutionPaths: []feature_pb.ImprovedPerformance{feature_pb.ImprovedPerformance_IMPROVED_PERFORMANCE_ORG_DOMAIN_VERIFIED}, Source: feature_pb.Source_SOURCE_SYSTEM, }, DebugOidcParentError: &feature_pb.FeatureFlag{ diff --git a/internal/api/grpc/management/org.go b/internal/api/grpc/management/org.go index a006db063..092dcdd6e 100644 --- a/internal/api/grpc/management/org.go +++ b/internal/api/grpc/management/org.go @@ -21,7 +21,7 @@ import ( ) func (s *Server) GetMyOrg(ctx context.Context, req *mgmt_pb.GetMyOrgRequest) (*mgmt_pb.GetMyOrgResponse, error) { - org, err := s.query.OrgByID(ctx, true, authz.GetCtxData(ctx).OrgID) + org, err := s.query.OrgByID(ctx, authz.GetCtxData(ctx).OrgID) if err != nil { return nil, err } diff --git a/internal/api/oidc/client.go b/internal/api/oidc/client.go index 6a2639f21..ced270e54 100644 --- a/internal/api/oidc/client.go +++ b/internal/api/oidc/client.go @@ -261,7 +261,7 @@ func (s *Server) checkOrgScopes(ctx context.Context, resourceOwner string, scope if slices.ContainsFunc(scopes, func(scope string) bool { return strings.HasPrefix(scope, domain.OrgDomainPrimaryScope) }) { - org, err := s.query.OrgByID(ctx, false, resourceOwner) + org, err := s.query.OrgByID(ctx, resourceOwner) if err != nil { return nil, err } diff --git a/internal/api/ui/login/renderer.go b/internal/api/ui/login/renderer.go index ac6246575..a5773eea9 100644 --- a/internal/api/ui/login/renderer.go +++ b/internal/api/ui/login/renderer.go @@ -571,7 +571,7 @@ func (l *Login) getOrgPrimaryDomain(r *http.Request, authReq *domain.AuthRequest if authReq != nil && authReq.RequestedPrimaryDomain != "" { return authReq.RequestedPrimaryDomain } - org, err := l.query.OrgByID(r.Context(), false, orgID) + org, err := l.query.OrgByID(r.Context(), orgID) if err != nil { logging.New().WithError(err).Error("cannot get default org") return "" diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index b40e7d906..4081b66d6 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -114,7 +114,7 @@ type userCommandProvider interface { } type orgViewProvider interface { - OrgByID(context.Context, bool, string) (*query.Org, error) + OrgByID(context.Context, string) (*query.Org, error) OrgByPrimaryDomain(context.Context, string) (*query.Org, error) } @@ -1550,7 +1550,7 @@ func (repo *AuthRequestRepo) getDomainPolicy(ctx context.Context, orgID string) func setOrgID(ctx context.Context, orgViewProvider orgViewProvider, request *domain.AuthRequest) error { orgID := request.GetScopeOrgID() if orgID != "" { - org, err := orgViewProvider.OrgByID(ctx, false, orgID) + org, err := orgViewProvider.OrgByID(ctx, orgID) if err != nil { return err } @@ -1723,7 +1723,7 @@ func activeUserByID(ctx context.Context, userViewProvider userViewProvider, user if !(user.State == user_model.UserStateActive || user.State == user_model.UserStateInitial) { return nil, zerrors.ThrowPreconditionFailed(nil, "EVENT-FJ262", "Errors.User.NotActive") } - org, err := queries.OrgByID(ctx, false, user.ResourceOwner) + org, err := queries.OrgByID(ctx, user.ResourceOwner) if err != nil { return nil, err } diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go b/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go index 78edd2a7e..079d65a94 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go @@ -237,7 +237,7 @@ type mockViewOrg struct { State domain.OrgState } -func (m *mockViewOrg) OrgByID(context.Context, bool, string) (*query.Org, error) { +func (m *mockViewOrg) OrgByID(context.Context, string) (*query.Org, error) { return &query.Org{ State: m.State, }, nil @@ -251,7 +251,7 @@ func (m *mockViewOrg) OrgByPrimaryDomain(context.Context, string) (*query.Org, e type mockViewErrOrg struct{} -func (m *mockViewErrOrg) OrgByID(context.Context, bool, string) (*query.Org, error) { +func (m *mockViewErrOrg) OrgByID(context.Context, string) (*query.Org, error) { return nil, zerrors.ThrowInternal(nil, "id", "internal error") } diff --git a/internal/feature/feature.go b/internal/feature/feature.go index 5e2833890..46d2e143d 100644 --- a/internal/feature/feature.go +++ b/internal/feature/feature.go @@ -57,12 +57,13 @@ type Features struct { type ImprovedPerformanceType int32 const ( - ImprovedPerformanceTypeUnspecified ImprovedPerformanceType = iota - ImprovedPerformanceTypeOrgByID - ImprovedPerformanceTypeProjectGrant - ImprovedPerformanceTypeProject - ImprovedPerformanceTypeUserGrant - ImprovedPerformanceTypeOrgDomainVerified + // Reserved: 1 + + ImprovedPerformanceTypeUnspecified ImprovedPerformanceType = 0 + ImprovedPerformanceTypeProjectGrant ImprovedPerformanceType = 2 + ImprovedPerformanceTypeProject ImprovedPerformanceType = 3 + ImprovedPerformanceTypeUserGrant ImprovedPerformanceType = 4 + ImprovedPerformanceTypeOrgDomainVerified ImprovedPerformanceType = 5 ) func (f Features) ShouldUseImprovedPerformance(typ ImprovedPerformanceType) bool { diff --git a/internal/query/org.go b/internal/query/org.go index e2d9e205d..a6f3b2a00 100644 --- a/internal/query/org.go +++ b/internal/query/org.go @@ -8,13 +8,10 @@ import ( "time" sq "github.com/Masterminds/squirrel" - "github.com/zitadel/logging" "github.com/zitadel/zitadel/internal/api/authz" domain_pkg "github.com/zitadel/zitadel/internal/domain" es "github.com/zitadel/zitadel/internal/eventstore" - "github.com/zitadel/zitadel/internal/eventstore/handler/v2" - "github.com/zitadel/zitadel/internal/feature" "github.com/zitadel/zitadel/internal/query/projection" "github.com/zitadel/zitadel/internal/telemetry/tracing" "github.com/zitadel/zitadel/internal/v2/eventstore" @@ -121,7 +118,7 @@ func (q *OrgSearchQueries) toQuery(query sq.SelectBuilder) sq.SelectBuilder { return query } -func (q *Queries) OrgByID(ctx context.Context, shouldTriggerBulk bool, id string) (org *Org, err error) { +func (q *Queries) OrgByID(ctx context.Context, id string) (org *Org, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -134,10 +131,6 @@ func (q *Queries) OrgByID(ctx context.Context, shouldTriggerBulk bool, id string } }() - if !authz.GetInstance(ctx).Features().ShouldUseImprovedPerformance(feature.ImprovedPerformanceTypeOrgByID) { - return q.oldOrgByID(ctx, shouldTriggerBulk, id) - } - foundOrg := readmodel.NewOrg(id) eventCount, err := q.eventStoreV4.Query( ctx, @@ -168,33 +161,6 @@ func (q *Queries) OrgByID(ctx context.Context, shouldTriggerBulk bool, id string }, nil } -func (q *Queries) oldOrgByID(ctx context.Context, shouldTriggerBulk bool, id string) (org *Org, err error) { - ctx, span := tracing.NewSpan(ctx) - defer func() { span.EndWithError(err) }() - - if shouldTriggerBulk { - _, traceSpan := tracing.NewNamedSpan(ctx, "TriggerOrgProjection") - ctx, err = projection.OrgProjection.Trigger(ctx, handler.WithAwaitRunning()) - logging.OnError(err).Debug("trigger failed") - traceSpan.EndWithError(err) - } - - stmt, scan := prepareOrgQuery() - query, args, err := stmt.Where(sq.Eq{ - OrgColumnID.identifier(): id, - OrgColumnInstanceID.identifier(): authz.GetInstance(ctx).InstanceID(), - }).ToSql() - if err != nil { - return nil, zerrors.ThrowInternal(err, "QUERY-AWx52", "Errors.Query.SQLStatement") - } - - err = q.client.QueryRowContext(ctx, func(row *sql.Row) error { - org, err = scan(row) - return err - }, query, args...) - return org, err -} - func (q *Queries) OrgByPrimaryDomain(ctx context.Context, domain string) (org *Org, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -288,7 +254,7 @@ func (q *Queries) ExistsOrg(ctx context.Context, id, domain string) (verifiedID var org *Org if id != "" { - org, err = q.OrgByID(ctx, true, id) + org, err = q.OrgByID(ctx, id) } else { org, err = q.OrgByVerifiedDomain(ctx, domain) } diff --git a/proto/buf.yaml b/proto/buf.yaml index abe35b305..6ef7b2936 100644 --- a/proto/buf.yaml +++ b/proto/buf.yaml @@ -9,8 +9,11 @@ breaking: - FILE - FIELD_NO_DELETE_UNLESS_NAME_RESERVED - FIELD_NO_DELETE_UNLESS_NUMBER_RESERVED + - ENUM_VALUE_NO_DELETE_UNLESS_NAME_RESERVED + - ENUM_VALUE_NO_DELETE_UNLESS_NUMBER_RESERVED except: - FIELD_NO_DELETE + - ENUM_VALUE_NO_DELETE ignore_unstable_packages: true lint: use: diff --git a/proto/zitadel/feature/v2/feature.proto b/proto/zitadel/feature/v2/feature.proto index 1748706fa..53f56147e 100644 --- a/proto/zitadel/feature/v2/feature.proto +++ b/proto/zitadel/feature/v2/feature.proto @@ -60,13 +60,13 @@ message LoginV2FeatureFlag { } enum ImprovedPerformance { + reserved 1; + reserved "IMPROVED_PERFORMANCE_ORG_BY_ID"; + IMPROVED_PERFORMANCE_UNSPECIFIED = 0; - // Uses the eventstore to query the org by id - // instead of the sql table. - IMPROVED_PERFORMANCE_ORG_BY_ID = 1; // Improves performance on write side by using // optimized processes to query data to determine - // correctnes of data. + // correctness of data. IMPROVED_PERFORMANCE_PROJECT_GRANT = 2; IMPROVED_PERFORMANCE_PROJECT = 3; IMPROVED_PERFORMANCE_USER_GRANT = 4; diff --git a/proto/zitadel/feature/v2beta/feature.proto b/proto/zitadel/feature/v2beta/feature.proto index d5756ee05..d600c02ea 100644 --- a/proto/zitadel/feature/v2beta/feature.proto +++ b/proto/zitadel/feature/v2beta/feature.proto @@ -50,13 +50,13 @@ message ImprovedPerformanceFeatureFlag { } enum ImprovedPerformance { + reserved 1; + reserved "IMPROVED_PERFORMANCE_ORG_BY_ID"; + IMPROVED_PERFORMANCE_UNSPECIFIED = 0; - // Uses the eventstore to query the org by id - // instead of the sql table. - IMPROVED_PERFORMANCE_ORG_BY_ID = 1; // Improves performance on write side by using // optimized processes to query data to determine - // correctnes of data. + // correctness of data. IMPROVED_PERFORMANCE_PROJECT_GRANT = 2; IMPROVED_PERFORMANCE_PROJECT = 3; IMPROVED_PERFORMANCE_USER_GRANT = 4;