From 370cd19a834925c616f1099ede05cc25bf9585b9 Mon Sep 17 00:00:00 2001 From: Livio Amstutz Date: Tue, 1 Sep 2020 16:38:34 +0200 Subject: [PATCH] fix: improve permission checks (#682) * separate roles for global org * remove old user grant permissions * allow context permissions Co-authored-by: Fabi <38692350+fgerschwiler@users.noreply.github.com> --- cmd/zitadel/authz.yaml | 46 +++++++++++++------ internal/api/authz/permissions.go | 1 + .../api/grpc/management/project_member.go | 6 ++- .../repository/eventsourcing/repository.go | 21 +++++---- .../eventsourcing/eventstore/project.go | 22 +++++++-- .../repository/eventsourcing/repository.go | 2 +- internal/management/repository/project.go | 2 +- .../repository/eventsourcing/eventstore.go | 11 +++-- .../eventsourcing/eventstore_test.go | 21 ++++++++- internal/setup/setup.go | 2 +- 10 files changed, 98 insertions(+), 36 deletions(-) diff --git a/cmd/zitadel/authz.yaml b/cmd/zitadel/authz.yaml index 281357e807..f53b4a2d4b 100644 --- a/cmd/zitadel/authz.yaml +++ b/cmd/zitadel/authz.yaml @@ -47,18 +47,12 @@ InternalAuthZ: - "project.app.read" - "project.app.write" - "project.app.delete" - - "project.user.grant.read" - - "project.user.grant.write" - - "project.user.grant.delete" - "project.grant.read" - "project.grant.write" - "project.grant.delete" - "project.grant.member.read" - "project.grant.member.write" - "project.grant.member.delete" - - "project.grant.user.grant.read" - - "project.grant.user.grant.write" - - "project.grant.user.grant.delete" - Role: 'IAM_OWNER_VIEWER' Permissions: - "iam.read" @@ -77,10 +71,8 @@ InternalAuthZ: - "project.member.read" - "project.role.read" - "project.app.read" - - "project.user.grant.read" - "project.grant.read" - "project.grant.member.read" - - "project.grant.user.grant.read" - Role: 'ORG_OWNER' Permissions: - "org.read" @@ -116,18 +108,12 @@ InternalAuthZ: - "project.role.delete" - "project.app.read" - "project.app.write" - - "project.user.grant.read" - - "project.user.grant.write" - - "project.user.grant.delete" - "project.grant.read" - "project.grant.write" - "project.grant.delete" - "project.grant.member.read" - "project.grant.member.write" - "project.grant.member.delete" - - "project.grant.user.grant.read" - - "project.grant.user.grant.write" - - "project.grant.user.grant.delete" - Role: 'ORG_OWNER_VIEWER' Permissions: - "org.read" @@ -142,7 +128,6 @@ InternalAuthZ: - "project.member.read" - "project.role.read" - "project.app.read" - - "project.user.grant.read" - "project.grant.read" - "project.grant.member.read" - "project.grant.user.grant.read" @@ -224,6 +209,37 @@ InternalAuthZ: - "user.global.read" - "user.grant.read" - "user.membership.read" + - Role: 'PROJECT_OWNER_GLOBAL' + Permissions: + - "org.global.read" + - "project.read" + - "project.write" + - "project.delete" + - "project.member.read" + - "project.member.write" + - "project.member.delete" + - "project.role.read" + - "project.role.write" + - "project.role.delete" + - "project.app.read" + - "project.app.write" + - "project.app.delete" + - "user.global.read" + - "user.grant.read" + - "user.grant.write" + - "user.grant.delete" + - "user.membership.read" + - Role: 'PROJECT_OWNER_VIEWER_GLOBAL' + Permissions: + - "project.read" + - "project.member.read" + - "project.role.read" + - "project.app.read" + - "project.grant.read" + - "project.grant.member.read" + - "user.global.read" + - "user.grant.read" + - "user.membership.read" - Role: 'PROJECT_GRANT_OWNER' Permissions: - "org.global.read" diff --git a/internal/api/authz/permissions.go b/internal/api/authz/permissions.go index 3af78f49a6..cb1bf2da0a 100644 --- a/internal/api/authz/permissions.go +++ b/internal/api/authz/permissions.go @@ -43,6 +43,7 @@ func mapRoleToPerm(requiredPerm, actualRole string, authConfig Config, requestPe allPermissions = append(allPermissions, permWithCtx) } + p, _ = SplitPermission(p) if p == requiredPerm { if !ExistsPerm(requestPermissions, permWithCtx) { requestPermissions = append(requestPermissions, permWithCtx) diff --git a/internal/api/grpc/management/project_member.go b/internal/api/grpc/management/project_member.go index 0d8d9999fa..cbb16bc816 100644 --- a/internal/api/grpc/management/project_member.go +++ b/internal/api/grpc/management/project_member.go @@ -9,7 +9,11 @@ import ( ) func (s *Server) GetProjectMemberRoles(ctx context.Context, _ *empty.Empty) (*management.ProjectMemberRoles, error) { - return &management.ProjectMemberRoles{Roles: s.project.GetProjectMemberRoles()}, nil + roles, err := s.project.GetProjectMemberRoles(ctx) + if err != nil { + return nil, err + } + return &management.ProjectMemberRoles{Roles: roles}, nil } func (s *Server) SearchProjectMembers(ctx context.Context, in *management.ProjectMemberSearchRequest) (*management.ProjectMemberSearchResponse, error) { diff --git a/internal/auth/repository/eventsourcing/repository.go b/internal/auth/repository/eventsourcing/repository.go index 1642cc718f..739803fe5b 100644 --- a/internal/auth/repository/eventsourcing/repository.go +++ b/internal/auth/repository/eventsourcing/repository.go @@ -98,6 +98,16 @@ func Start(conf Config, authZ authz.Config, systemDefaults sd.SystemDefaults, au if err != nil { return nil, err } + iam, err := es_iam.StartIAM( + es_iam.IAMConfig{ + Eventstore: es, + Cache: conf.Eventstore.Cache, + }, + systemDefaults, + ) + if err != nil { + return nil, err + } project, err := es_proj.StartProject( es_proj.ProjectConfig{ @@ -109,16 +119,7 @@ func Start(conf Config, authZ authz.Config, systemDefaults sd.SystemDefaults, au if err != nil { return nil, err } - iam, err := es_iam.StartIAM( - es_iam.IAMConfig{ - Eventstore: es, - Cache: conf.Eventstore.Cache, - }, - systemDefaults, - ) - if err != nil { - return nil, err - } + org := es_org.StartOrg(es_org.OrgConfig{Eventstore: es, IAMDomain: conf.Domain}, systemDefaults) repos := handler.EventstoreRepos{UserEvents: user, ProjectEvents: project, OrgEvents: org, IamEvents: iam} diff --git a/internal/management/repository/eventsourcing/eventstore/project.go b/internal/management/repository/eventsourcing/eventstore/project.go index 6761a019ca..096927bf3b 100644 --- a/internal/management/repository/eventsourcing/eventstore/project.go +++ b/internal/management/repository/eventsourcing/eventstore/project.go @@ -12,6 +12,7 @@ import ( "github.com/caos/zitadel/internal/eventstore/models" es_models "github.com/caos/zitadel/internal/eventstore/models" es_sdk "github.com/caos/zitadel/internal/eventstore/sdk" + iam_event "github.com/caos/zitadel/internal/iam/repository/eventsourcing" "github.com/caos/zitadel/internal/management/repository/eventsourcing/view" global_model "github.com/caos/zitadel/internal/model" proj_model "github.com/caos/zitadel/internal/project/model" @@ -29,8 +30,10 @@ type ProjectRepo struct { ProjectEvents *proj_event.ProjectEventstore UserGrantEvents *usr_grant_event.UserGrantEventStore UserEvents *usr_event.UserEventstore + IAMEvents *iam_event.IAMEventstore View *view.View Roles []string + IAMID string } func (repo *ProjectRepo) ProjectByID(ctx context.Context, id string) (*proj_model.ProjectView, error) { @@ -64,8 +67,13 @@ func (repo *ProjectRepo) ProjectByID(ctx context.Context, id string) (*proj_mode } func (repo *ProjectRepo) CreateProject(ctx context.Context, name string) (*proj_model.Project, error) { + ctxData := authz.GetCtxData(ctx) + iam, err := repo.IAMEvents.IAMByID(ctx, repo.IAMID) + if err != nil { + return nil, err + } project := &proj_model.Project{Name: name} - return repo.ProjectEvents.CreateProject(ctx, project) + return repo.ProjectEvents.CreateProject(ctx, project, iam.GlobalOrgID == ctxData.OrgID) } func (repo *ProjectRepo) UpdateProject(ctx context.Context, project *proj_model.Project) (*proj_model.Project, error) { @@ -612,14 +620,22 @@ func (repo *ProjectRepo) SearchProjectGrantMembers(ctx context.Context, request return result, nil } -func (repo *ProjectRepo) GetProjectMemberRoles() []string { +func (repo *ProjectRepo) GetProjectMemberRoles(ctx context.Context) ([]string, error) { + iam, err := repo.IAMEvents.IAMByID(ctx, repo.IAMID) + if err != nil { + return nil, err + } roles := make([]string, 0) + global := authz.GetCtxData(ctx).OrgID == iam.GlobalOrgID for _, roleMap := range repo.Roles { if strings.HasPrefix(roleMap, "PROJECT") && !strings.HasPrefix(roleMap, "PROJECT_GRANT") { + if global && !strings.HasSuffix(roleMap, "GLOBAL") { + continue + } roles = append(roles, roleMap) } } - return roles + return roles, nil } func (repo *ProjectRepo) GetProjectGrantMemberRoles() []string { diff --git a/internal/management/repository/eventsourcing/repository.go b/internal/management/repository/eventsourcing/repository.go index d304d4c9f5..a87124efa8 100644 --- a/internal/management/repository/eventsourcing/repository.go +++ b/internal/management/repository/eventsourcing/repository.go @@ -95,7 +95,7 @@ func Start(conf Config, systemDefaults sd.SystemDefaults, roles []string) (*EsRe return &EsRepository{ spooler: spool, OrgRepository: eventstore.OrgRepository{conf.SearchLimit, org, user, view, roles, systemDefaults}, - ProjectRepo: eventstore.ProjectRepo{es, conf.SearchLimit, project, usergrant, user, view, roles}, + ProjectRepo: eventstore.ProjectRepo{es, conf.SearchLimit, project, usergrant, user, iam, view, roles, systemDefaults.IamID}, UserRepo: eventstore.UserRepo{conf.SearchLimit, user, policy, org, view, systemDefaults}, UserGrantRepo: eventstore.UserGrantRepo{conf.SearchLimit, usergrant, view}, PolicyRepo: eventstore.PolicyRepo{policy}, diff --git a/internal/management/repository/project.go b/internal/management/repository/project.go index f07f49f470..382b9df089 100644 --- a/internal/management/repository/project.go +++ b/internal/management/repository/project.go @@ -23,7 +23,7 @@ type ProjectRepository interface { ChangeProjectMember(ctx context.Context, member *model.ProjectMember) (*model.ProjectMember, error) RemoveProjectMember(ctx context.Context, projectID, userID string) error SearchProjectMembers(ctx context.Context, request *model.ProjectMemberSearchRequest) (*model.ProjectMemberSearchResponse, error) - GetProjectMemberRoles() []string + GetProjectMemberRoles(ctx context.Context) ([]string, error) AddProjectRole(ctx context.Context, role *model.ProjectRole) (*model.ProjectRole, error) ChangeProjectRole(ctx context.Context, role *model.ProjectRole) (*model.ProjectRole, error) diff --git a/internal/project/repository/eventsourcing/eventstore.go b/internal/project/repository/eventsourcing/eventstore.go index c7e552e441..a821e76703 100644 --- a/internal/project/repository/eventsourcing/eventstore.go +++ b/internal/project/repository/eventsourcing/eventstore.go @@ -23,7 +23,8 @@ import ( ) const ( - projectOwnerRole = "PROJECT_OWNER" + projectOwnerRole = "PROJECT_OWNER" + projectOwnerGlobalRole = "PROJECT_OWNER_GLOBAL" ) type ProjectEventstore struct { @@ -78,7 +79,7 @@ func (es *ProjectEventstore) ProjectEventsByID(ctx context.Context, id string, s return es.FilterEvents(ctx, query) } -func (es *ProjectEventstore) CreateProject(ctx context.Context, project *proj_model.Project) (*proj_model.Project, error) { +func (es *ProjectEventstore) CreateProject(ctx context.Context, project *proj_model.Project, global bool) (*proj_model.Project, error) { if !project.IsValid() { return nil, caos_errs.ThrowPreconditionFailed(nil, "EVENT-IOVCC", "Errors.Project.Invalid") } @@ -89,9 +90,13 @@ func (es *ProjectEventstore) CreateProject(ctx context.Context, project *proj_mo project.AggregateID = id project.State = proj_model.ProjectStateActive repoProject := model.ProjectFromModel(project) + projectRole := projectOwnerRole + if global { + projectRole = projectOwnerGlobalRole + } member := &model.ProjectMember{ UserID: authz.GetCtxData(ctx).UserID, - Roles: []string{projectOwnerRole}, + Roles: []string{projectRole}, } createAggregate := ProjectCreateAggregate(es.AggregateCreator(), repoProject, member) diff --git a/internal/project/repository/eventsourcing/eventstore_test.go b/internal/project/repository/eventsourcing/eventstore_test.go index 6c954e589d..6c80a03291 100644 --- a/internal/project/repository/eventsourcing/eventstore_test.go +++ b/internal/project/repository/eventsourcing/eventstore_test.go @@ -82,9 +82,11 @@ func TestCreateProject(t *testing.T) { es *ProjectEventstore ctx context.Context project *model.Project + global bool } type res struct { project *model.Project + role string wantErr bool errFunc func(err error) bool } @@ -102,6 +104,20 @@ func TestCreateProject(t *testing.T) { }, res: res{ project: &model.Project{ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 1}, Name: "Name"}, + role: projectOwnerRole, + }, + }, + { + name: "create global project, ok", + args: args{ + es: GetMockManipulateProject(ctrl), + ctx: authz.NewMockContext("orgID", "userID"), + project: &model.Project{ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 1}, Name: "Name"}, + global: true, + }, + res: res{ + project: &model.Project{ObjectRoot: es_models.ObjectRoot{AggregateID: "AggregateID", Sequence: 1}, Name: "Name"}, + role: projectOwnerGlobalRole, }, }, { @@ -119,7 +135,7 @@ func TestCreateProject(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := tt.args.es.CreateProject(tt.args.ctx, tt.args.project) + result, err := tt.args.es.CreateProject(tt.args.ctx, tt.args.project, tt.args.global) if !tt.res.wantErr && result.AggregateID == "" { t.Errorf("result has no id") @@ -127,6 +143,9 @@ func TestCreateProject(t *testing.T) { if !tt.res.wantErr && result.Name != tt.res.project.Name { t.Errorf("got wrong result name: expected: %v, actual: %v ", tt.res.project.Name, result.Name) } + if !tt.res.wantErr && result.Members[0].Roles[0] != tt.res.role { + t.Errorf("got wrong result role: expected: %v, actual: %v ", tt.res.role, result.Members[0].Roles[0]) + } if tt.res.wantErr && !tt.res.errFunc(err) { t.Errorf("got wrong err: %v ", err) } diff --git a/internal/setup/setup.go b/internal/setup/setup.go index ea2f00b238..4d26cda0c1 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -386,7 +386,7 @@ func (setUp *initializer) project(ctx context.Context, project Project) (*proj_m addProject := &proj_model.Project{ Name: project.Name, } - return setUp.ProjectEvents.CreateProject(ctx, addProject) + return setUp.ProjectEvents.CreateProject(ctx, addProject, false) } func (setUp *initializer) oidcApp(ctx context.Context, project *proj_model.Project, oidc OIDCApp) (*proj_model.Application, error) {