From 6602a9e6c334d2f6fb74f8aab58185ce406d46bd 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 5bbcab27cf..bb278ec838 100644 --- a/internal/api/grpc/admin/import.go +++ b/internal/api/grpc/admin/import.go @@ -308,7 +308,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(), []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 90b99ca208..0439b4e901 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 e146ac2db6..cde89abd68 100644 --- a/internal/api/grpc/feature/v2/converter.go +++ b/internal/api/grpc/feature/v2/converter.go @@ -174,8 +174,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: @@ -206,8 +204,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 f481e4f65a..8f04f4b861 100644 --- a/internal/api/grpc/feature/v2/converter_test.go +++ b/internal/api/grpc/feature/v2/converter_test.go @@ -78,7 +78,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, @@ -127,7 +127,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{ @@ -225,7 +225,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}, }, WebKey: query.FeatureSource[bool]{ Level: feature.LevelInstance, @@ -282,7 +282,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, }, WebKey: &feature_pb.FeatureFlag{ diff --git a/internal/api/grpc/feature/v2beta/converter.go b/internal/api/grpc/feature/v2beta/converter.go index 9739e1c4c8..21061f3f68 100644 --- a/internal/api/grpc/feature/v2beta/converter.go +++ b/internal/api/grpc/feature/v2beta/converter.go @@ -111,8 +111,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: @@ -143,8 +141,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 72d91b10d4..41026ec8d2 100644 --- a/internal/api/grpc/feature/v2beta/converter_test.go +++ b/internal/api/grpc/feature/v2beta/converter_test.go @@ -68,7 +68,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, @@ -102,7 +102,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{ @@ -168,7 +168,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}, }, WebKey: query.FeatureSource[bool]{ Level: feature.LevelInstance, @@ -206,7 +206,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, }, WebKey: &feature_pb.FeatureFlag{ diff --git a/internal/api/grpc/management/org.go b/internal/api/grpc/management/org.go index 57caeda3ce..967c8d1183 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 08ed8c31b9..3038a3040c 100644 --- a/internal/api/oidc/client.go +++ b/internal/api/oidc/client.go @@ -1049,7 +1049,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 ac62465758..a5773eea91 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 984a1e7145..fadfc8fcbd 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) } @@ -1548,7 +1548,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 } @@ -1721,7 +1721,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 7d71ddecd9..0350a88aaf 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 b5f5a901d4..398b619c32 100644 --- a/internal/feature/feature.go +++ b/internal/feature/feature.go @@ -62,12 +62,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 58b0dad4a5..37ff8ac466 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" @@ -109,7 +106,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) }() @@ -122,10 +119,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, @@ -156,33 +149,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) }() @@ -276,7 +242,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 31bc7b4ccc..a8f397ff99 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 1748706fae..53f56147ef 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 d5756ee056..d600c02ea3 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;