From 43dc925f1662d21aad09c3ef4ccdb395afc0ed48 Mon Sep 17 00:00:00 2001 From: Fabi <38692350+fgerschwiler@users.noreply.github.com> Date: Thu, 11 Jun 2020 13:27:25 +0200 Subject: [PATCH] fix: bugs (#208) * fix: add iam roles to permissions * fix: show state initial on usersearch * fix: search project roles returns only roles of project * fix: add project member owner on project create * fix: create new object oon failed event * feat: parse error body on chat message * feat: remove comment --- cmd/zitadel/authz.yaml | 2 + cmd/zitadel/caos_local.sh | 2 +- .../notification/providers/chat/provider.go | 13 +++++- internal/project/model/project_member_view.go | 3 ++ internal/project/model/project_role_view.go | 3 ++ .../repository/eventsourcing/eventstore.go | 11 ++++- .../repository/eventsourcing/project.go | 14 ++++-- .../repository/eventsourcing/project_test.go | 46 +++++++++++++------ internal/view/failed_events.go | 9 ++-- pkg/management/api/grpc/project.go | 5 +- pkg/management/api/grpc/project_member.go | 5 +- pkg/management/api/grpc/user_converter.go | 4 ++ 12 files changed, 88 insertions(+), 29 deletions(-) diff --git a/cmd/zitadel/authz.yaml b/cmd/zitadel/authz.yaml index 979fdbe0ec..2583609d5a 100644 --- a/cmd/zitadel/authz.yaml +++ b/cmd/zitadel/authz.yaml @@ -2,6 +2,8 @@ InternalAuthZ: RolePermissionMappings: - Role: 'IAM_OWNER' Permissions: + - "iam.read" + - "iam.write" - "org.read" - "org.write" - "org.member.read" diff --git a/cmd/zitadel/caos_local.sh b/cmd/zitadel/caos_local.sh index ca95a147dd..e80515d387 100755 --- a/cmd/zitadel/caos_local.sh +++ b/cmd/zitadel/caos_local.sh @@ -36,7 +36,7 @@ export SMTP_PASSWORD=$(gopass zitadel-secrets/zitadel/google/emailappkey) export EMAIL_SENDER_ADDRESS=noreply@caos.ch export EMAIL_SENDER_NAME=CAOS AG export SMTP_TLS=TRUE -export CHAT_URL=$(gopass zitadel-secrets/zitadel/dev/google-chat-url | base64 -D) +export CHAT_URL=$(gopass zitadel-secrets/zitadel/dev/google-chat-url) #OIDC export ZITADEL_ISSUER=http://localhost:50022 diff --git a/internal/notification/providers/chat/provider.go b/internal/notification/providers/chat/provider.go index fe8856f595..ae8ccdd02e 100644 --- a/internal/notification/providers/chat/provider.go +++ b/internal/notification/providers/chat/provider.go @@ -3,8 +3,10 @@ package chat import ( "bytes" "encoding/json" + "github.com/caos/logging" caos_errs "github.com/caos/zitadel/internal/errors" "github.com/caos/zitadel/internal/notification/providers" + "io/ioutil" "net/http" "net/url" "unicode/utf8" @@ -51,10 +53,19 @@ func (chat *Chat) SendMessage(message providers.Message) error { return caos_errs.ThrowInternal(err, "PROVI-s8uie", "Could not unmarshal content") } - _, err = http.Post(chat.URL.String(), "application/json; charset=UTF-8", bytes.NewReader(req)) + response, err := http.Post(chat.URL.String(), "application/json; charset=UTF-8", bytes.NewReader(req)) if err != nil { return caos_errs.ThrowInternal(err, "PROVI-si93s", "unable to send message") } + if response.StatusCode != 200 { + defer response.Body.Close() + bodyBytes, err := ioutil.ReadAll(response.Body) + if err != nil { + return caos_errs.ThrowInternal(err, "PROVI-PSLd3", "unable to read response message") + } + logging.LogWithFields("PROVI-PS0kx", "Body", string(bodyBytes)).Warn("Chat Message post didnt get 200 OK") + return caos_errs.ThrowInternal(nil, "PROVI-LSopw", string(bodyBytes)) + } return nil } diff --git a/internal/project/model/project_member_view.go b/internal/project/model/project_member_view.go index a1d1005966..1d7202b7c1 100644 --- a/internal/project/model/project_member_view.go +++ b/internal/project/model/project_member_view.go @@ -56,3 +56,6 @@ func (r *ProjectMemberSearchRequest) EnsureLimit(limit uint64) { r.Limit = limit } } +func (r *ProjectMemberSearchRequest) AppendProjectQuery(projectID string) { + r.Queries = append(r.Queries, &ProjectMemberSearchQuery{Key: PROJECTMEMBERSEARCHKEY_PROJECT_ID, Method: model.SEARCHMETHOD_EQUALS, Value: projectID}) +} diff --git a/internal/project/model/project_role_view.go b/internal/project/model/project_role_view.go index 5465e9ea61..2da3241041 100644 --- a/internal/project/model/project_role_view.go +++ b/internal/project/model/project_role_view.go @@ -51,6 +51,9 @@ type ProjectRoleSearchResponse struct { func (r *ProjectRoleSearchRequest) AppendMyOrgQuery(orgID string) { r.Queries = append(r.Queries, &ProjectRoleSearchQuery{Key: PROJECTROLESEARCHKEY_ORGID, Method: model.SEARCHMETHOD_EQUALS, Value: orgID}) } +func (r *ProjectRoleSearchRequest) AppendProjectQuery(projectID string) { + r.Queries = append(r.Queries, &ProjectRoleSearchQuery{Key: PROJECTROLESEARCHKEY_PROJECTID, Method: model.SEARCHMETHOD_EQUALS, Value: projectID}) +} func (r *ProjectRoleSearchRequest) EnsureLimit(limit uint64) { if r.Limit == 0 || r.Limit > limit { diff --git a/internal/project/repository/eventsourcing/eventstore.go b/internal/project/repository/eventsourcing/eventstore.go index 91446bea32..92d90b77e9 100644 --- a/internal/project/repository/eventsourcing/eventstore.go +++ b/internal/project/repository/eventsourcing/eventstore.go @@ -2,6 +2,7 @@ package eventsourcing import ( "context" + "github.com/caos/zitadel/internal/api/auth" "github.com/caos/zitadel/internal/cache/config" sd "github.com/caos/zitadel/internal/config/systemdefaults" es_models "github.com/caos/zitadel/internal/eventstore/models" @@ -15,6 +16,10 @@ import ( proj_model "github.com/caos/zitadel/internal/project/model" ) +const ( + projectOwnerRole = "PROJECT_OWNER" +) + type ProjectEventstore struct { es_int.Eventstore projectCache *ProjectCache @@ -70,8 +75,12 @@ func (es *ProjectEventstore) CreateProject(ctx context.Context, project *proj_mo project.AggregateID = id project.State = proj_model.PROJECTSTATE_ACTIVE repoProject := model.ProjectFromModel(project) + member := &model.ProjectMember{ + UserID: auth.GetCtxData(ctx).UserID, + Roles: []string{projectOwnerRole}, + } - createAggregate := ProjectCreateAggregate(es.AggregateCreator(), repoProject) + createAggregate := ProjectCreateAggregate(es.AggregateCreator(), repoProject, member) err = es_sdk.Push(ctx, es.PushAggregates, repoProject.AppendEvents, createAggregate) if err != nil { return nil, err diff --git a/internal/project/repository/eventsourcing/project.go b/internal/project/repository/eventsourcing/project.go index 54c152f712..fb94fadf3b 100644 --- a/internal/project/repository/eventsourcing/project.go +++ b/internal/project/repository/eventsourcing/project.go @@ -33,10 +33,10 @@ func ProjectAggregate(ctx context.Context, aggCreator *es_models.AggregateCreato return aggCreator.NewAggregate(ctx, project.AggregateID, model.ProjectAggregate, model.ProjectVersion, project.Sequence) } -func ProjectCreateAggregate(aggCreator *es_models.AggregateCreator, project *model.Project) func(ctx context.Context) (*es_models.Aggregate, error) { +func ProjectCreateAggregate(aggCreator *es_models.AggregateCreator, project *model.Project, member *model.ProjectMember) func(ctx context.Context) (*es_models.Aggregate, error) { return func(ctx context.Context) (*es_models.Aggregate, error) { - if project == nil { - return nil, errors.ThrowPreconditionFailed(nil, "EVENT-kdie6", "project should not be nil") + if project == nil || member == nil { + return nil, errors.ThrowPreconditionFailed(nil, "EVENT-kdie6", "project and member should not be nil") } agg, err := ProjectAggregate(ctx, aggCreator, project) @@ -48,7 +48,11 @@ func ProjectCreateAggregate(aggCreator *es_models.AggregateCreator, project *mod EventTypesFilter(model.ProjectAdded, model.ProjectChanged, model.ProjectRemoved) validation := addProjectValidation(project.Name) - return agg.SetPrecondition(validationQuery, validation).AppendEvent(model.ProjectAdded, project) + agg, err = agg.SetPrecondition(validationQuery, validation).AppendEvent(model.ProjectAdded, project) + if err != nil { + return nil, err + } + return agg.AppendEvent(model.ProjectMemberAdded, member) } } @@ -488,7 +492,7 @@ func addProjectValidation(projectName string) func(...*es_models.Event) error { } for _, p := range projects { if p.Name == projectName { - return errors.ThrowPreconditionFailed(nil, "EVENT-s9oPw", "conditions not met") + return errors.ThrowPreconditionFailed(nil, "EVENT-s9oPw", "project already exists on resourceowner") } } return nil diff --git a/internal/project/repository/eventsourcing/project_test.go b/internal/project/repository/eventsourcing/project_test.go index e4c05e8036..177a405824 100644 --- a/internal/project/repository/eventsourcing/project_test.go +++ b/internal/project/repository/eventsourcing/project_test.go @@ -164,11 +164,12 @@ func TestProjectCreateAggregate(t *testing.T) { type args struct { ctx context.Context new *model.Project + member *model.ProjectMember aggCreator *models.AggregateCreator } type res struct { eventLen int - eventType models.EventType + eventType []models.EventType wantErr bool errFunc func(err error) bool } @@ -182,11 +183,12 @@ func TestProjectCreateAggregate(t *testing.T) { args: args{ ctx: auth.NewMockContext("orgID", "userID"), new: &model.Project{ObjectRoot: models.ObjectRoot{AggregateID: "AggregateID"}, Name: "ProjectName", State: int32(proj_model.PROJECTSTATE_ACTIVE)}, + member: &model.ProjectMember{UserID: "UserID"}, aggCreator: models.NewAggregateCreator("Test"), }, res: res{ - eventLen: 1, - eventType: model.ProjectAdded, + eventLen: 2, + eventType: []models.EventType{model.ProjectAdded, model.ProjectMemberAdded}, }, }, { @@ -194,29 +196,47 @@ func TestProjectCreateAggregate(t *testing.T) { args: args{ ctx: auth.NewMockContext("orgID", "userID"), new: nil, + member: &model.ProjectMember{UserID: "UserID"}, aggCreator: models.NewAggregateCreator("Test"), }, res: res{ - eventLen: 1, - eventType: model.ProjectAdded, - wantErr: true, - errFunc: caos_errs.IsPreconditionFailed, + wantErr: true, + errFunc: caos_errs.IsPreconditionFailed, + }, + }, + { + name: "new member nil", + args: args{ + ctx: auth.NewMockContext("orgID", "userID"), + new: &model.Project{ObjectRoot: models.ObjectRoot{AggregateID: "AggregateID"}, Name: "ProjectName", State: int32(proj_model.PROJECTSTATE_ACTIVE)}, + member: nil, + aggCreator: models.NewAggregateCreator("Test"), + }, + res: res{ + wantErr: true, + errFunc: caos_errs.IsPreconditionFailed, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - agg, err := ProjectCreateAggregate(tt.args.aggCreator, tt.args.new)(tt.args.ctx) + agg, err := ProjectCreateAggregate(tt.args.aggCreator, tt.args.new, tt.args.member)(tt.args.ctx) if !tt.res.wantErr && len(agg.Events) != tt.res.eventLen { t.Errorf("got wrong event len: expected: %v, actual: %v ", tt.res.eventLen, len(agg.Events)) } - if !tt.res.wantErr && agg.Events[0].Type != tt.res.eventType { - t.Errorf("got wrong event type: expected: %v, actual: %v ", tt.res.eventType, agg.Events[0].Type.String()) - } - if !tt.res.wantErr && agg.Events[0].Data == nil { - t.Errorf("should have data in event") + + if !tt.res.wantErr { + for i, _ := range agg.Events { + if !tt.res.wantErr && agg.Events[i].Type != tt.res.eventType[i] { + t.Errorf("got wrong event type: expected: %v, actual: %v ", tt.res.eventType, agg.Events[i].Type.String()) + } + if !tt.res.wantErr && agg.Events[i].Data == nil { + t.Errorf("should have data in event") + } + } } + if tt.res.wantErr && !tt.res.errFunc(err) { t.Errorf("got wrong err: %v ", err) } diff --git a/internal/view/failed_events.go b/internal/view/failed_events.go index 23f7d1145c..c46579955c 100644 --- a/internal/view/failed_events.go +++ b/internal/view/failed_events.go @@ -81,10 +81,11 @@ func LatestFailedEvent(db *gorm.DB, table, viewName string, sequence uint64) (*F } if errors.IsNotFound(err) { - failedEvent.ViewName = viewName - failedEvent.FailedSequence = sequence - failedEvent.FailureCount = 0 - return failedEvent, nil + return &FailedEvent{ + ViewName: viewName, + FailedSequence: sequence, + FailureCount: 0, + }, nil } return nil, errors.ThrowInternalf(err, "VIEW-9LyCB", "unable to get failed events of %s", viewName) diff --git a/pkg/management/api/grpc/project.go b/pkg/management/api/grpc/project.go index 1176638067..7e3e00d4ee 100644 --- a/pkg/management/api/grpc/project.go +++ b/pkg/management/api/grpc/project.go @@ -3,6 +3,7 @@ package grpc import ( "context" "github.com/caos/zitadel/internal/api" + "github.com/caos/zitadel/internal/api/auth" grpc_util "github.com/caos/zitadel/internal/api/grpc" "github.com/caos/zitadel/internal/errors" "github.com/golang/protobuf/ptypes/empty" @@ -104,8 +105,8 @@ func (s *Server) RemoveProjectRole(ctx context.Context, in *ProjectRoleRemove) ( func (s *Server) SearchProjectRoles(ctx context.Context, in *ProjectRoleSearchRequest) (*ProjectRoleSearchResponse, error) { request := projectRoleSearchRequestsToModel(in) - orgID := grpc_util.GetHeader(ctx, api.ZitadelOrgID) - request.AppendMyOrgQuery(orgID) + request.AppendMyOrgQuery(auth.GetCtxData(ctx).OrgID) + request.AppendProjectQuery(in.ProjectId) response, err := s.project.SearchProjectRoles(ctx, request) if err != nil { return nil, err diff --git a/pkg/management/api/grpc/project_member.go b/pkg/management/api/grpc/project_member.go index 9dd6b17247..ddb35eef0c 100644 --- a/pkg/management/api/grpc/project_member.go +++ b/pkg/management/api/grpc/project_member.go @@ -2,7 +2,6 @@ package grpc import ( "context" - "github.com/golang/protobuf/ptypes/empty" ) @@ -11,7 +10,9 @@ func (s *Server) GetProjectMemberRoles(ctx context.Context, _ *empty.Empty) (*Pr } func (s *Server) SearchProjectMembers(ctx context.Context, in *ProjectMemberSearchRequest) (*ProjectMemberSearchResponse, error) { - response, err := s.project.SearchProjectMembers(ctx, projectMemberSearchRequestsToModel(in)) + request := projectMemberSearchRequestsToModel(in) + request.AppendProjectQuery(in.ProjectId) + response, err := s.project.SearchProjectMembers(ctx, request) if err != nil { return nil, err } diff --git a/pkg/management/api/grpc/user_converter.go b/pkg/management/api/grpc/user_converter.go index 33a006736d..14022b045b 100644 --- a/pkg/management/api/grpc/user_converter.go +++ b/pkg/management/api/grpc/user_converter.go @@ -341,6 +341,10 @@ func userStateFromModel(state usr_model.UserState) UserState { return UserState_USERSTATE_INACTIVE case usr_model.USERSTATE_LOCKED: return UserState_USERSTATE_LOCKED + case usr_model.USERSTATE_INITIAL: + return UserState_USERSTATE_INITIAL + case usr_model.USERSTATE_SUSPEND: + return UserState_USERSTATE_SUSPEND default: return UserState_USERSTATE_UNSPECIFIED }