From 8141d902b8b883c48e4e117a330767c224ad356c Mon Sep 17 00:00:00 2001 From: Elio Bischof Date: Thu, 6 Apr 2023 07:46:12 +0200 Subject: [PATCH] fix: delete org project mapping by grant id (#5607) * fix: delete org project mapping by grant id * fix: check for project on authentication using projections * fix tests --------- Co-authored-by: Livio Spring --- .../eventsourcing/eventstore/auth_request.go | 37 +++-- .../eventstore/auth_request_test.go | 25 ++-- .../eventsourcing/handler/handler.go | 4 +- .../handler/org_project_mapping.go | 130 ------------------ .../repository/eventsourcing/repository.go | 2 +- .../eventsourcing/spooler/spooler.go | 5 +- .../eventsourcing/view/org_project_mapping.go | 81 ----------- 7 files changed, 41 insertions(+), 243 deletions(-) delete mode 100644 internal/auth/repository/eventsourcing/handler/org_project_mapping.go delete mode 100644 internal/auth/repository/eventsourcing/view/org_project_mapping.go diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index 4b1ca88910..6a879a3e8e 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -18,7 +18,6 @@ import ( v1 "github.com/zitadel/zitadel/internal/eventstore/v1" es_models "github.com/zitadel/zitadel/internal/eventstore/v1/models" "github.com/zitadel/zitadel/internal/id" - project_view_model "github.com/zitadel/zitadel/internal/project/repository/view/model" "github.com/zitadel/zitadel/internal/query" user_repo "github.com/zitadel/zitadel/internal/repository/user" "github.com/zitadel/zitadel/internal/telemetry/tracing" @@ -106,7 +105,7 @@ type userGrantProvider interface { type projectProvider interface { ProjectByClientID(context.Context, string, bool) (*query.Project, error) - OrgProjectMappingByIDs(orgID, projectID, instanceID string) (*project_view_model.OrgProjectMapping, error) + SearchProjectGrants(ctx context.Context, queries *query.ProjectGrantSearchQueries, withOwnerRemoved bool) (projects *query.ProjectGrants, err error) } type applicationProvider interface { @@ -1153,11 +1152,11 @@ func privacyPolicyToDomain(p *query.PrivacyPolicy) *domain.PrivacyPolicy { CreationDate: p.CreationDate, ChangeDate: p.ChangeDate, }, - State: p.State, - Default: p.IsDefault, - TOSLink: p.TOSLink, - PrivacyLink: p.PrivacyLink, - HelpLink: p.HelpLink, + State: p.State, + Default: p.IsDefault, + TOSLink: p.TOSLink, + PrivacyLink: p.PrivacyLink, + HelpLink: p.HelpLink, SupportEmail: p.SupportEmail, } } @@ -1465,7 +1464,7 @@ func userGrantRequired(ctx context.Context, request *domain.AuthRequest, user *u return len(grants) == 0, nil } -func projectRequired(ctx context.Context, request *domain.AuthRequest, projectProvider projectProvider) (_ bool, err error) { +func projectRequired(ctx context.Context, request *domain.AuthRequest, projectProvider projectProvider) (missingGrant bool, err error) { var project *query.Project switch request.Request.Type() { case domain.AuthRequestTypeOIDC, domain.AuthRequestTypeSAML: @@ -1476,13 +1475,23 @@ func projectRequired(ctx context.Context, request *domain.AuthRequest, projectPr default: return false, errors.ThrowPreconditionFailed(nil, "EVENT-dfrw2", "Errors.AuthRequest.RequestTypeNotSupported") } - if !project.HasProjectCheck { + // if the user and project are part of the same organisation we do not need to check if the project exists on that org + if !project.HasProjectCheck || project.ResourceOwner == request.UserOrgID { return false, nil } - _, err = projectProvider.OrgProjectMappingByIDs(request.UserOrgID, project.ID, request.InstanceID) - if errors.IsNotFound(err) { - // if not found there is no error returned - return true, nil + + // else just check if there is a project grant for that org + projectID, err := query.NewProjectGrantProjectIDSearchQuery(project.ID) + if err != nil { + return false, err } - return false, err + grantedOrg, err := query.NewProjectGrantGrantedOrgIDSearchQuery(request.UserOrgID) + if err != nil { + return false, err + } + grants, err := projectProvider.SearchProjectGrants(ctx, &query.ProjectGrantSearchQueries{Queries: []query.SearchQuery{projectID, grantedOrg}}, false) + if err != nil { + return false, err + } + return len(grants.ProjectGrants) != 1, nil } diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go b/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go index bc6983bfba..04d9e06646 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go @@ -14,7 +14,6 @@ import ( "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/errors" es_models "github.com/zitadel/zitadel/internal/eventstore/v1/models" - proj_view_model "github.com/zitadel/zitadel/internal/project/repository/view/model" "github.com/zitadel/zitadel/internal/query" user_repo "github.com/zitadel/zitadel/internal/repository/user" user_model "github.com/zitadel/zitadel/internal/user/model" @@ -208,19 +207,21 @@ func (m *mockUserGrants) UserGrantsByProjectAndUserID(ctx context.Context, s str } type mockProject struct { - hasProject bool - projectCheck bool + hasProject bool + projectCheck bool + resourceOwner string } func (m *mockProject) ProjectByClientID(ctx context.Context, s string, _ bool) (*query.Project, error) { - return &query.Project{HasProjectCheck: m.projectCheck}, nil + return &query.Project{ResourceOwner: m.resourceOwner, HasProjectCheck: m.projectCheck}, nil } -func (m *mockProject) OrgProjectMappingByIDs(orgID, projectID, instanceID string) (*proj_view_model.OrgProjectMapping, error) { +func (m *mockProject) SearchProjectGrants(ctx context.Context, queries *query.ProjectGrantSearchQueries, _ bool) (*query.ProjectGrants, error) { if m.hasProject { - return &proj_view_model.OrgProjectMapping{OrgID: orgID, ProjectID: projectID}, nil + mockProjectGrant := new(query.ProjectGrant) + return &query.ProjectGrants{ProjectGrants: []*query.ProjectGrant{mockProjectGrant}}, nil } - return nil, errors.ThrowNotFound(nil, "ERROR", "error") + return &query.ProjectGrants{}, nil } type mockApp struct { @@ -1258,8 +1259,9 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { orgViewProvider: &mockViewOrg{State: domain.OrgStateActive}, userGrantProvider: &mockUserGrants{}, projectProvider: &mockProject{ - projectCheck: true, - hasProject: false, + projectCheck: true, + hasProject: false, + resourceOwner: "other-org", }, lockoutPolicyProvider: &mockLockoutPolicy{ policy: &query.LockoutPolicy{ @@ -1297,8 +1299,9 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { orgViewProvider: &mockViewOrg{State: domain.OrgStateActive}, userGrantProvider: &mockUserGrants{}, projectProvider: &mockProject{ - projectCheck: true, - hasProject: true, + projectCheck: true, + hasProject: true, + resourceOwner: "other-org", }, applicationProvider: &mockApp{app: &query.App{OIDCConfig: &query.OIDCApp{AppType: domain.OIDCApplicationTypeWeb}}}, lockoutPolicyProvider: &mockLockoutPolicy{ diff --git a/internal/auth/repository/eventsourcing/handler/handler.go b/internal/auth/repository/eventsourcing/handler/handler.go index 161500de3a..3f0baecdba 100644 --- a/internal/auth/repository/eventsourcing/handler/handler.go +++ b/internal/auth/repository/eventsourcing/handler/handler.go @@ -6,7 +6,6 @@ import ( "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/auth/repository/eventsourcing/view" - sd "github.com/zitadel/zitadel/internal/config/systemdefaults" v1 "github.com/zitadel/zitadel/internal/eventstore/v1" "github.com/zitadel/zitadel/internal/eventstore/v1/models" "github.com/zitadel/zitadel/internal/eventstore/v1/query" @@ -33,7 +32,7 @@ func (h *handler) Eventstore() v1.Eventstore { return h.es } -func Register(ctx context.Context, configs Configs, bulkLimit, errorCount uint64, view *view.View, es v1.Eventstore, systemDefaults sd.SystemDefaults, queries *query2.Queries) []query.Handler { +func Register(ctx context.Context, configs Configs, bulkLimit, errorCount uint64, view *view.View, es v1.Eventstore, queries *query2.Queries) []query.Handler { return []query.Handler{ newUser(ctx, handler{view, bulkLimit, configs.cycleDuration("User"), errorCount, es}, queries), @@ -42,7 +41,6 @@ func Register(ctx context.Context, configs Configs, bulkLimit, errorCount uint64 newToken(ctx, handler{view, bulkLimit, configs.cycleDuration("Token"), errorCount, es}), newRefreshToken(ctx, handler{view, bulkLimit, configs.cycleDuration("RefreshToken"), errorCount, es}), - newOrgProjectMapping(ctx, handler{view, bulkLimit, configs.cycleDuration("OrgProjectMapping"), errorCount, es}), } } diff --git a/internal/auth/repository/eventsourcing/handler/org_project_mapping.go b/internal/auth/repository/eventsourcing/handler/org_project_mapping.go deleted file mode 100644 index ffa7688866..0000000000 --- a/internal/auth/repository/eventsourcing/handler/org_project_mapping.go +++ /dev/null @@ -1,130 +0,0 @@ -package handler - -import ( - "context" - - "github.com/zitadel/logging" - - "github.com/zitadel/zitadel/internal/eventstore" - v1 "github.com/zitadel/zitadel/internal/eventstore/v1" - es_models "github.com/zitadel/zitadel/internal/eventstore/v1/models" - "github.com/zitadel/zitadel/internal/eventstore/v1/query" - "github.com/zitadel/zitadel/internal/eventstore/v1/spooler" - view_model "github.com/zitadel/zitadel/internal/project/repository/view/model" - "github.com/zitadel/zitadel/internal/repository/instance" - "github.com/zitadel/zitadel/internal/repository/org" - "github.com/zitadel/zitadel/internal/repository/project" -) - -const ( - orgProjectMappingTable = "auth.org_project_mapping2" -) - -type OrgProjectMapping struct { - handler - subscription *v1.Subscription -} - -func newOrgProjectMapping( - ctx context.Context, - handler handler, -) *OrgProjectMapping { - h := &OrgProjectMapping{ - handler: handler, - } - - h.subscribe(ctx) - - return h -} - -func (p *OrgProjectMapping) subscribe(ctx context.Context) { - p.subscription = p.es.Subscribe(p.AggregateTypes()...) - go func() { - for event := range p.subscription.Events { - query.ReduceEvent(ctx, p, event) - } - }() -} - -func (p *OrgProjectMapping) ViewModel() string { - return orgProjectMappingTable -} - -func (p *OrgProjectMapping) Subscription() *v1.Subscription { - return p.subscription -} - -func (_ *OrgProjectMapping) AggregateTypes() []es_models.AggregateType { - return []es_models.AggregateType{project.AggregateType, instance.AggregateType} -} - -func (p *OrgProjectMapping) CurrentSequence(instanceID string) (uint64, error) { - sequence, err := p.view.GetLatestOrgProjectMappingSequence(instanceID) - if err != nil { - return 0, err - } - return sequence.CurrentSequence, nil -} - -func (p *OrgProjectMapping) EventQuery(instanceIDs []string) (*es_models.SearchQuery, error) { - sequences, err := p.view.GetLatestOrgProjectMappingSequences(instanceIDs) - if err != nil { - return nil, err - } - return newSearchQuery(sequences, p.AggregateTypes(), instanceIDs), nil -} - -func (p *OrgProjectMapping) Reduce(event *es_models.Event) (err error) { - mapping := new(view_model.OrgProjectMapping) - switch eventstore.EventType(event.Type) { - case project.ProjectAddedType: - mapping.OrgID = event.ResourceOwner - mapping.ProjectID = event.AggregateID - mapping.InstanceID = event.InstanceID - case project.ProjectRemovedType: - err := p.view.DeleteOrgProjectMappingsByProjectID(event.AggregateID, event.InstanceID) - if err == nil { - return p.view.ProcessedOrgProjectMappingSequence(event) - } - case project.GrantAddedType: - projectGrant := new(view_model.ProjectGrant) - err := projectGrant.SetData(event) - if err != nil { - return err - } - mapping.OrgID = projectGrant.GrantedOrgID - mapping.ProjectID = event.AggregateID - mapping.ProjectGrantID = projectGrant.GrantID - mapping.InstanceID = event.InstanceID - case project.GrantRemovedType: - projectGrant := new(view_model.ProjectGrant) - err := projectGrant.SetData(event) - if err != nil { - return err - } - err = p.view.DeleteOrgProjectMappingsByProjectGrantID(event.AggregateID, event.InstanceID) - if err == nil { - return p.view.ProcessedOrgProjectMappingSequence(event) - } - case instance.InstanceRemovedEventType: - return p.view.DeleteInstanceOrgProjectMappings(event) - case org.OrgRemovedEventType: - return p.view.UpdateOwnerRemovedOrgProjectMappings(event) - default: - return p.view.ProcessedOrgProjectMappingSequence(event) - } - if err != nil { - return err - } - return p.view.PutOrgProjectMapping(mapping, event) -} - -func (p *OrgProjectMapping) OnError(event *es_models.Event, err error) error { - logging.WithFields("id", event.AggregateID).WithError(err).Warn("something went wrong in org project mapping handler") - return spooler.HandleError(event, err, p.view.GetLatestOrgProjectMappingFailedEvent, p.view.ProcessedOrgProjectMappingFailedEvent, p.view.ProcessedOrgProjectMappingSequence, p.errorCountUntilSkip) -} - -func (p *OrgProjectMapping) OnSuccess(instanceIDs []string) error { - return spooler.HandleSuccess(p.view.UpdateOrgProjectMappingSpoolerRunTimestamp, instanceIDs) -} diff --git a/internal/auth/repository/eventsourcing/repository.go b/internal/auth/repository/eventsourcing/repository.go index b06e46a139..852ebcca91 100644 --- a/internal/auth/repository/eventsourcing/repository.go +++ b/internal/auth/repository/eventsourcing/repository.go @@ -48,7 +48,7 @@ func Start(ctx context.Context, conf Config, systemDefaults sd.SystemDefaults, c authReq := cache.Start(dbClient) - spool := spooler.StartSpooler(ctx, conf.Spooler, es, esV2, view, dbClient, systemDefaults, queries) + spool := spooler.StartSpooler(ctx, conf.Spooler, es, esV2, view, dbClient, queries) userRepo := eventstore.UserRepo{ SearchLimit: conf.SearchLimit, diff --git a/internal/auth/repository/eventsourcing/spooler/spooler.go b/internal/auth/repository/eventsourcing/spooler/spooler.go index 4da8340154..dfbfc73b27 100644 --- a/internal/auth/repository/eventsourcing/spooler/spooler.go +++ b/internal/auth/repository/eventsourcing/spooler/spooler.go @@ -5,7 +5,6 @@ import ( "github.com/zitadel/zitadel/internal/auth/repository/eventsourcing/handler" "github.com/zitadel/zitadel/internal/auth/repository/eventsourcing/view" - sd "github.com/zitadel/zitadel/internal/config/systemdefaults" "github.com/zitadel/zitadel/internal/database" "github.com/zitadel/zitadel/internal/eventstore" v1 "github.com/zitadel/zitadel/internal/eventstore/v1" @@ -21,14 +20,14 @@ type SpoolerConfig struct { Handlers handler.Configs } -func StartSpooler(ctx context.Context, c SpoolerConfig, es v1.Eventstore, esV2 *eventstore.Eventstore, view *view.View, client *database.DB, systemDefaults sd.SystemDefaults, queries *query.Queries) *spooler.Spooler { +func StartSpooler(ctx context.Context, c SpoolerConfig, es v1.Eventstore, esV2 *eventstore.Eventstore, view *view.View, client *database.DB, queries *query.Queries) *spooler.Spooler { spoolerConfig := spooler.Config{ Eventstore: es, EventstoreV2: esV2, Locker: &locker{dbClient: client.DB}, ConcurrentWorkers: c.ConcurrentWorkers, ConcurrentInstances: c.ConcurrentInstances, - ViewHandlers: handler.Register(ctx, c.Handlers, c.BulkLimit, c.FailureCountUntilSkip, view, es, systemDefaults, queries), + ViewHandlers: handler.Register(ctx, c.Handlers, c.BulkLimit, c.FailureCountUntilSkip, view, es, queries), } spool := spoolerConfig.New() spool.Start() diff --git a/internal/auth/repository/eventsourcing/view/org_project_mapping.go b/internal/auth/repository/eventsourcing/view/org_project_mapping.go deleted file mode 100644 index 9cd3f20899..0000000000 --- a/internal/auth/repository/eventsourcing/view/org_project_mapping.go +++ /dev/null @@ -1,81 +0,0 @@ -package view - -import ( - "github.com/zitadel/zitadel/internal/errors" - "github.com/zitadel/zitadel/internal/eventstore/v1/models" - "github.com/zitadel/zitadel/internal/project/repository/view" - "github.com/zitadel/zitadel/internal/project/repository/view/model" - "github.com/zitadel/zitadel/internal/view/repository" -) - -const ( - orgProjectMappingTable = "auth.org_project_mapping2" -) - -func (v *View) OrgProjectMappingByIDs(orgID, projectID, instanceID string) (*model.OrgProjectMapping, error) { - return view.OrgProjectMappingByIDs(v.Db, orgProjectMappingTable, orgID, projectID, instanceID) -} - -func (v *View) PutOrgProjectMapping(mapping *model.OrgProjectMapping, event *models.Event) error { - err := view.PutOrgProjectMapping(v.Db, orgProjectMappingTable, mapping) - if err != nil { - return err - } - return v.ProcessedOrgProjectMappingSequence(event) -} - -func (v *View) DeleteOrgProjectMapping(orgID, projectID, instanceID string, event *models.Event) error { - err := view.DeleteOrgProjectMapping(v.Db, orgProjectMappingTable, orgID, projectID, instanceID) - if err != nil && !errors.IsNotFound(err) { - return err - } - return v.ProcessedOrgProjectMappingSequence(event) -} - -func (v *View) DeleteInstanceOrgProjectMappings(event *models.Event) error { - err := view.DeleteInstanceOrgProjectMappings(v.Db, orgProjectMappingTable, event.InstanceID) - if err != nil && !errors.IsNotFound(err) { - return err - } - return v.ProcessedOrgProjectMappingSequence(event) -} - -func (v *View) UpdateOwnerRemovedOrgProjectMappings(event *models.Event) error { - err := view.UpdateOwnerRemovedOrgProjectMappings(v.Db, orgProjectMappingTable, event.InstanceID, event.AggregateID) - if err != nil && !errors.IsNotFound(err) { - return err - } - return v.ProcessedOrgProjectMappingSequence(event) -} - -func (v *View) DeleteOrgProjectMappingsByProjectID(projectID, instanceID string) error { - return view.DeleteOrgProjectMappingsByProjectID(v.Db, orgProjectMappingTable, projectID, instanceID) -} - -func (v *View) DeleteOrgProjectMappingsByProjectGrantID(projectGrantID, instanceID string) error { - return view.DeleteOrgProjectMappingsByProjectGrantID(v.Db, orgProjectMappingTable, projectGrantID, instanceID) -} - -func (v *View) GetLatestOrgProjectMappingSequence(instanceID string) (*repository.CurrentSequence, error) { - return v.latestSequence(orgProjectMappingTable, instanceID) -} - -func (v *View) GetLatestOrgProjectMappingSequences(instanceIDs []string) ([]*repository.CurrentSequence, error) { - return v.latestSequences(orgProjectMappingTable, instanceIDs) -} - -func (v *View) ProcessedOrgProjectMappingSequence(event *models.Event) error { - return v.saveCurrentSequence(orgProjectMappingTable, event) -} - -func (v *View) UpdateOrgProjectMappingSpoolerRunTimestamp(instanceIDs []string) error { - return v.updateSpoolerRunSequence(orgProjectMappingTable, instanceIDs) -} - -func (v *View) GetLatestOrgProjectMappingFailedEvent(sequence uint64, instanceID string) (*repository.FailedEvent, error) { - return v.latestFailedEvent(orgProjectMappingTable, instanceID, sequence) -} - -func (v *View) ProcessedOrgProjectMappingFailedEvent(failedEvent *repository.FailedEvent) error { - return v.saveFailedEvent(failedEvent) -}