From 6780c5a07ca491690e0af6d8baeac9aa5d69cabe Mon Sep 17 00:00:00 2001 From: Stefan Benz <46600784+stebenz@users.noreply.github.com> Date: Wed, 30 Oct 2024 09:53:00 +0100 Subject: [PATCH] fix: add resourceowner to check for project in project grant (#8785) # Which Problems Are Solved Resource owner can be different than expected if the provided x-zitadel-orgid header is provided. # How the Problems Are Solved Check that the project is only checked with the correct resource owner to avoid unexpected situations. # Additional Changes None # Additional Context Closes #8685 --------- Co-authored-by: Livio Spring --- internal/command/project_grant.go | 14 ++-- internal/command/project_grant_model.go | 15 ++++- internal/command/project_grant_test.go | 90 +++++++++++++++++++++++++ internal/command/project_old.go | 4 +- 4 files changed, 114 insertions(+), 9 deletions(-) diff --git a/internal/command/project_grant.go b/internal/command/project_grant.go index ef602c4b3e..feb8a29e4b 100644 --- a/internal/command/project_grant.go +++ b/internal/command/project_grant.go @@ -27,7 +27,7 @@ func (c *Commands) AddProjectGrant(ctx context.Context, grant *domain.ProjectGra if !grant.IsValid() { return nil, zerrors.ThrowInvalidArgument(nil, "PROJECT-3b8fs", "Errors.Project.Grant.Invalid") } - err = c.checkProjectGrantPreCondition(ctx, grant) + err = c.checkProjectGrantPreCondition(ctx, grant, resourceOwner) if err != nil { return nil, err } @@ -67,7 +67,7 @@ func (c *Commands) ChangeProjectGrant(ctx context.Context, grant *domain.Project return nil, err } grant.GrantedOrgID = existingGrant.GrantedOrgID - err = c.checkProjectGrantPreCondition(ctx, grant) + err = c.checkProjectGrantPreCondition(ctx, grant, resourceOwner) if err != nil { return nil, err } @@ -255,11 +255,11 @@ func (c *Commands) projectGrantWriteModelByID(ctx context.Context, grantID, proj return writeModel, nil } -func (c *Commands) checkProjectGrantPreCondition(ctx context.Context, projectGrant *domain.ProjectGrant) error { +func (c *Commands) checkProjectGrantPreCondition(ctx context.Context, projectGrant *domain.ProjectGrant, resourceOwner string) error { if !authz.GetFeatures(ctx).ShouldUseImprovedPerformance(feature.ImprovedPerformanceTypeProjectGrant) { - return c.checkProjectGrantPreConditionOld(ctx, projectGrant) + return c.checkProjectGrantPreConditionOld(ctx, projectGrant, resourceOwner) } - existingRoleKeys, err := c.searchProjectGrantState(ctx, projectGrant.AggregateID, projectGrant.GrantedOrgID) + existingRoleKeys, err := c.searchProjectGrantState(ctx, projectGrant.AggregateID, projectGrant.GrantedOrgID, resourceOwner) if err != nil { return err } @@ -270,11 +270,12 @@ func (c *Commands) checkProjectGrantPreCondition(ctx context.Context, projectGra return nil } -func (c *Commands) searchProjectGrantState(ctx context.Context, projectID, grantedOrgID string) (existingRoleKeys []string, err error) { +func (c *Commands) searchProjectGrantState(ctx context.Context, projectID, grantedOrgID, resourceOwner string) (existingRoleKeys []string, err error) { results, err := c.eventstore.Search( ctx, // project state query map[eventstore.FieldType]any{ + eventstore.FieldTypeResourceOwner: resourceOwner, eventstore.FieldTypeAggregateType: project.AggregateType, eventstore.FieldTypeAggregateID: projectID, eventstore.FieldTypeFieldName: project.ProjectStateSearchField, @@ -289,6 +290,7 @@ func (c *Commands) searchProjectGrantState(ctx context.Context, projectID, grant }, // role query map[eventstore.FieldType]any{ + eventstore.FieldTypeResourceOwner: resourceOwner, eventstore.FieldTypeAggregateType: project.AggregateType, eventstore.FieldTypeAggregateID: projectID, eventstore.FieldTypeFieldName: project.ProjectRoleKeySearchField, diff --git a/internal/command/project_grant_model.go b/internal/command/project_grant_model.go index 791fd54fe3..c658b00b69 100644 --- a/internal/command/project_grant_model.go +++ b/internal/command/project_grant_model.go @@ -121,8 +121,9 @@ type ProjectGrantPreConditionReadModel struct { ExistingRoleKeys []string } -func NewProjectGrantPreConditionReadModel(projectID, grantedOrgID string) *ProjectGrantPreConditionReadModel { +func NewProjectGrantPreConditionReadModel(projectID, grantedOrgID, resourceOwner string) *ProjectGrantPreConditionReadModel { return &ProjectGrantPreConditionReadModel{ + WriteModel: eventstore.WriteModel{ResourceOwner: resourceOwner}, ProjectID: projectID, GrantedOrgID: grantedOrgID, } @@ -132,12 +133,24 @@ func (wm *ProjectGrantPreConditionReadModel) Reduce() error { for _, event := range wm.Events { switch e := event.(type) { case *project.ProjectAddedEvent: + if e.Aggregate().ResourceOwner != wm.ResourceOwner { + continue + } wm.ProjectExists = true case *project.ProjectRemovedEvent: + if e.Aggregate().ResourceOwner != wm.ResourceOwner { + continue + } wm.ProjectExists = false case *project.RoleAddedEvent: + if e.Aggregate().ResourceOwner != wm.ResourceOwner { + continue + } wm.ExistingRoleKeys = append(wm.ExistingRoleKeys, e.Key) case *project.RoleRemovedEvent: + if e.Aggregate().ResourceOwner != wm.ResourceOwner { + continue + } for i, key := range wm.ExistingRoleKeys { if key == e.Key { copy(wm.ExistingRoleKeys[i:], wm.ExistingRoleKeys[i+1:]) diff --git a/internal/command/project_grant_test.go b/internal/command/project_grant_test.go index 42c6875f06..b6f44c27dd 100644 --- a/internal/command/project_grant_test.go +++ b/internal/command/project_grant_test.go @@ -79,6 +79,50 @@ func TestCommandSide_AddProjectGrant(t *testing.T) { err: zerrors.IsPreconditionFailed, }, }, + { + name: "project not existing in org, precondition error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + project.NewProjectAddedEvent(context.Background(), + &project.NewAggregate("project1", "otherorg").Aggregate, + "projectname1", true, true, true, + domain.PrivateLabelingSettingUnspecified, + ), + ), + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("grantedorg1").Aggregate, + "granted org", + ), + ), + eventFromEventPusher( + project.NewRoleAddedEvent(context.Background(), + &project.NewAggregate("project1", "otherorg").Aggregate, + "key1", + "key", + "", + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + projectGrant: &domain.ProjectGrant{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "project1", + }, + GrantedOrgID: "grantedorg1", + }, + resourceOwner: "org1", + }, + res: res{ + err: zerrors.IsPreconditionFailed, + }, + }, { name: "granted org not existing, precondition error", fields: fields{ @@ -325,6 +369,52 @@ func TestCommandSide_ChangeProjectGrant(t *testing.T) { err: zerrors.IsPreconditionFailed, }, }, + { + name: "project not existing in org, precondition error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher(project.NewGrantAddedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, + "projectgrant1", + "grantedorg1", + []string{"key1"}, + )), + ), + expectFilter( + eventFromEventPusher( + project.NewProjectAddedEvent(context.Background(), + &project.NewAggregate("project1", "otherorg").Aggregate, + "projectname1", true, true, true, + domain.PrivateLabelingSettingUnspecified, + ), + ), + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("grantedorg1").Aggregate, + "granted org", + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + projectGrant: &domain.ProjectGrant{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "project1", + }, + GrantID: "projectgrant1", + GrantedOrgID: "grantedorg1", + RoleKeys: []string{"key1"}, + }, + resourceOwner: "org1", + }, + res: res{ + err: zerrors.IsPreconditionFailed, + }, + }, { name: "granted org not existing, precondition error", fields: fields{ diff --git a/internal/command/project_old.go b/internal/command/project_old.go index 434df38a7f..b31b4e58bf 100644 --- a/internal/command/project_old.go +++ b/internal/command/project_old.go @@ -161,8 +161,8 @@ func (c *Commands) removeProjectOld(ctx context.Context, projectID, resourceOwne return writeModelToObjectDetails(&existingProject.WriteModel), nil } -func (c *Commands) checkProjectGrantPreConditionOld(ctx context.Context, projectGrant *domain.ProjectGrant) error { - preConditions := NewProjectGrantPreConditionReadModel(projectGrant.AggregateID, projectGrant.GrantedOrgID) +func (c *Commands) checkProjectGrantPreConditionOld(ctx context.Context, projectGrant *domain.ProjectGrant, resourceOwner string) error { + preConditions := NewProjectGrantPreConditionReadModel(projectGrant.AggregateID, projectGrant.GrantedOrgID, resourceOwner) err := c.eventstore.FilterToQueryReducer(ctx, preConditions) if err != nil { return err