fix: improve user grants precondition checks (#4237)

* fix: improve user grants precondition checks

* build rc

* fix prerelease

* fix: build image

* remove branch from releaserc
This commit is contained in:
Livio Spring 2022-08-24 11:38:59 +02:00 committed by GitHub
parent 06fd70d457
commit 4c26665b93
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 116 additions and 10 deletions

View File

@ -33,7 +33,7 @@ func (c *Commands) addUserGrant(ctx context.Context, userGrant *domain.UserGrant
if !userGrant.IsValid() { if !userGrant.IsValid() {
return nil, nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-4M0fs", "Errors.UserGrant.Invalid") return nil, nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-4M0fs", "Errors.UserGrant.Invalid")
} }
err = c.checkUserGrantPreCondition(ctx, userGrant) err = c.checkUserGrantPreCondition(ctx, userGrant, resourceOwner)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -91,7 +91,7 @@ func (c *Commands) changeUserGrant(ctx context.Context, userGrant *domain.UserGr
} }
userGrant.ProjectID = existingUserGrant.ProjectID userGrant.ProjectID = existingUserGrant.ProjectID
userGrant.ProjectGrantID = existingUserGrant.ProjectGrantID userGrant.ProjectGrantID = existingUserGrant.ProjectGrantID
err = c.checkUserGrantPreCondition(ctx, userGrant) err = c.checkUserGrantPreCondition(ctx, userGrant, resourceOwner)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -285,8 +285,8 @@ func (c *Commands) userGrantWriteModelByID(ctx context.Context, userGrantID, res
return writeModel, nil return writeModel, nil
} }
func (c *Commands) checkUserGrantPreCondition(ctx context.Context, usergrant *domain.UserGrant) error { func (c *Commands) checkUserGrantPreCondition(ctx context.Context, usergrant *domain.UserGrant, resourceOwner string) error {
preConditions := NewUserGrantPreConditionReadModel(usergrant.UserID, usergrant.ProjectID, usergrant.ProjectGrantID) preConditions := NewUserGrantPreConditionReadModel(usergrant.UserID, usergrant.ProjectID, usergrant.ProjectGrantID, resourceOwner)
err := c.eventstore.FilterToQueryReducer(ctx, preConditions) err := c.eventstore.FilterToQueryReducer(ctx, preConditions)
if err != nil { if err != nil {
return err return err
@ -294,7 +294,7 @@ func (c *Commands) checkUserGrantPreCondition(ctx context.Context, usergrant *do
if !preConditions.UserExists { if !preConditions.UserExists {
return caos_errs.ThrowPreconditionFailed(err, "COMMAND-4f8sg", "Errors.User.NotFound") return caos_errs.ThrowPreconditionFailed(err, "COMMAND-4f8sg", "Errors.User.NotFound")
} }
if !preConditions.ProjectExists { if usergrant.ProjectGrantID == "" && !preConditions.ProjectExists {
return caos_errs.ThrowPreconditionFailed(err, "COMMAND-3n77S", "Errors.Project.NotFound") return caos_errs.ThrowPreconditionFailed(err, "COMMAND-3n77S", "Errors.Project.NotFound")
} }
if usergrant.ProjectGrantID != "" && !preConditions.ProjectGrantExists { if usergrant.ProjectGrantID != "" && !preConditions.ProjectGrantExists {

View File

@ -89,17 +89,19 @@ type UserGrantPreConditionReadModel struct {
UserID string UserID string
ProjectID string ProjectID string
ProjectGrantID string ProjectGrantID string
ResourceOwner string
UserExists bool UserExists bool
ProjectExists bool ProjectExists bool
ProjectGrantExists bool ProjectGrantExists bool
ExistingRoleKeys []string ExistingRoleKeys []string
} }
func NewUserGrantPreConditionReadModel(userID, projectID, projectGrantID string) *UserGrantPreConditionReadModel { func NewUserGrantPreConditionReadModel(userID, projectID, projectGrantID, resourceOwner string) *UserGrantPreConditionReadModel {
return &UserGrantPreConditionReadModel{ return &UserGrantPreConditionReadModel{
UserID: userID, UserID: userID,
ProjectID: projectID, ProjectID: projectID,
ProjectGrantID: projectGrantID, ProjectGrantID: projectGrantID,
ResourceOwner: resourceOwner,
} }
} }
@ -115,17 +117,18 @@ func (wm *UserGrantPreConditionReadModel) Reduce() error {
case *user.UserRemovedEvent: case *user.UserRemovedEvent:
wm.UserExists = false wm.UserExists = false
case *project.ProjectAddedEvent: case *project.ProjectAddedEvent:
wm.ProjectExists = true if wm.ProjectGrantID == "" && wm.ResourceOwner == e.Aggregate().ResourceOwner {
wm.ProjectExists = true
}
case *project.ProjectRemovedEvent: case *project.ProjectRemovedEvent:
wm.ProjectExists = false wm.ProjectExists = false
case *project.GrantAddedEvent: case *project.GrantAddedEvent:
if wm.ProjectGrantID == e.GrantID { if wm.ProjectGrantID == e.GrantID && wm.ResourceOwner == e.GrantedOrgID {
wm.ProjectGrantExists = true wm.ProjectGrantExists = true
wm.ExistingRoleKeys = e.RoleKeys wm.ExistingRoleKeys = e.RoleKeys
} }
case *project.GrantChangedEvent: case *project.GrantChangedEvent:
if wm.ProjectGrantID == e.GrantID { if wm.ProjectGrantID == e.GrantID {
wm.ProjectGrantExists = true
wm.ExistingRoleKeys = e.RoleKeys wm.ExistingRoleKeys = e.RoleKeys
} }
case *project.GrantRemovedEvent: case *project.GrantRemovedEvent:

View File

@ -5,6 +5,8 @@ import (
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"golang.org/x/text/language"
"github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/domain"
caos_errs "github.com/zitadel/zitadel/internal/errors" caos_errs "github.com/zitadel/zitadel/internal/errors"
@ -16,7 +18,6 @@ import (
"github.com/zitadel/zitadel/internal/repository/project" "github.com/zitadel/zitadel/internal/repository/project"
"github.com/zitadel/zitadel/internal/repository/user" "github.com/zitadel/zitadel/internal/repository/user"
"github.com/zitadel/zitadel/internal/repository/usergrant" "github.com/zitadel/zitadel/internal/repository/usergrant"
"golang.org/x/text/language"
) )
func TestCommandSide_AddUserGrant(t *testing.T) { func TestCommandSide_AddUserGrant(t *testing.T) {
@ -149,6 +150,48 @@ func TestCommandSide_AddUserGrant(t *testing.T) {
err: caos_errs.IsPreconditionFailed, err: caos_errs.IsPreconditionFailed,
}, },
}, },
{
name: "project on other org, precondition error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username1",
"firstname1",
"lastname1",
"nickname1",
"displayname1",
language.German,
domain.GenderMale,
"email1",
true,
),
),
eventFromEventPusher(
project.NewProjectAddedEvent(context.Background(),
&project.NewAggregate("project1", "org1").Aggregate,
"projectname1", true, true, true,
domain.PrivateLabelingSettingUnspecified,
),
),
),
),
},
args: args{
ctx: authz.NewMockContextWithPermissions("", "org2", "user", []string{domain.RoleProjectOwner}),
userGrant: &domain.UserGrant{
UserID: "user1",
ProjectID: "project1",
},
resourceOwner: "org2",
},
res: res{
err: caos_errs.IsPreconditionFailed,
},
},
{ {
name: "project roles not existing, precondition error", name: "project roles not existing, precondition error",
fields: fields{ fields: fields{
@ -296,6 +339,66 @@ func TestCommandSide_AddUserGrant(t *testing.T) {
err: caos_errs.IsPreconditionFailed, err: caos_errs.IsPreconditionFailed,
}, },
}, },
{
name: "project grant on other org, precondition error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username1",
"firstname1",
"lastname1",
"nickname1",
"displayname1",
language.German,
domain.GenderMale,
"email1",
true,
),
),
eventFromEventPusher(
project.NewProjectAddedEvent(context.Background(),
&project.NewAggregate("project1", "org1").Aggregate,
"projectname1", true, true, true,
domain.PrivateLabelingSettingUnspecified,
),
),
eventFromEventPusher(
project.NewRoleAddedEvent(context.Background(),
&project.NewAggregate("project1", "org1").Aggregate,
"rolekey1",
"rolekey",
"",
),
),
eventFromEventPusher(
project.NewGrantAddedEvent(context.Background(),
&project.NewAggregate("project1", "org1").Aggregate,
"projectgrant1",
"org3",
[]string{"rolekey1"},
),
),
),
),
},
args: args{
ctx: authz.NewMockContextWithPermissions("", "org2", "user", []string{domain.RoleProjectOwner}),
userGrant: &domain.UserGrant{
UserID: "user1",
ProjectID: "project1",
ProjectGrantID: "projectgrant1",
RoleKeys: []string{"rolekey1"},
},
resourceOwner: "org2",
},
res: res{
err: caos_errs.IsPreconditionFailed,
},
},
{ {
name: "usergrant for project, ok", name: "usergrant for project, ok",
fields: fields{ fields: fields{