From 5bd9badbcfe3b8859285cbda15640d07c25bb362 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Wed, 27 Jul 2022 09:55:44 +0200 Subject: [PATCH] fix: project grants (#4031) * fix: filter granted memberships correctly * fix: only show changes of granted project * Apply suggestions from code review Co-authored-by: Fabi <38692350+hifabienne@users.noreply.github.com> * Update internal/query/user_membership.go Co-authored-by: Fabi <38692350+hifabienne@users.noreply.github.com> Co-authored-by: Fabi <38692350+hifabienne@users.noreply.github.com> --- .../app/modules/changes/changes.component.ts | 7 +++ .../granted-project-detail.component.html | 2 +- console/src/app/services/mgmt.service.ts | 24 ++++++++++ docs/docs/apis/proto/management.md | 37 +++++++++++++++ internal/api/grpc/auth/user.go | 8 +++- internal/api/grpc/management/project.go | 11 +++++ .../eventstore/user_membership.go | 6 ++- internal/query/changes.go | 11 +++++ internal/query/search_query.go | 22 +++++++++ internal/query/user_membership.go | 47 ++++++++++++------- internal/query/user_membership_test.go | 27 ++++++++--- internal/query/zitadel_permission.go | 6 ++- pkg/grpc/management/changes.go | 32 ++++++------- proto/zitadel/management.proto | 28 +++++++++++ 14 files changed, 224 insertions(+), 44 deletions(-) diff --git a/console/src/app/modules/changes/changes.component.ts b/console/src/app/modules/changes/changes.component.ts index 3153290ba6..117f3dece4 100644 --- a/console/src/app/modules/changes/changes.component.ts +++ b/console/src/app/modules/changes/changes.component.ts @@ -19,6 +19,7 @@ export enum ChangeType { USER = 'user', ORG = 'org', PROJECT = 'project', + PROJECT_GRANT= 'project-grant', APP = 'app', } @@ -93,6 +94,9 @@ export class ChangesComponent implements OnInit, OnDestroy { case ChangeType.PROJECT: first = this.mgmtUserService.listProjectChanges(this.id, 30, 0); break; + case ChangeType.PROJECT_GRANT: + first = this.mgmtUserService.listProjectGrantChanges(this.id, this.secId, 30, 0); + break; case ChangeType.ORG: first = this.mgmtUserService.listOrgChanges(30, 0); break; @@ -126,6 +130,9 @@ export class ChangesComponent implements OnInit, OnDestroy { case ChangeType.PROJECT: more = this.mgmtUserService.listProjectChanges(this.id, 20, cursor); break; + case ChangeType.PROJECT_GRANT: + more = this.mgmtUserService.listProjectGrantChanges(this.id, this.secId, 20, cursor); + break; case ChangeType.ORG: more = this.mgmtUserService.listOrgChanges(20, cursor); break; diff --git a/console/src/app/pages/projects/granted-projects/granted-project-detail/granted-project-detail.component.html b/console/src/app/pages/projects/granted-projects/granted-project-detail/granted-project-detail.component.html index d671050449..eb1a3751fe 100644 --- a/console/src/app/pages/projects/granted-projects/granted-project-detail/granted-project-detail.component.html +++ b/console/src/app/pages/projects/granted-projects/granted-project-detail/granted-project-detail.component.html @@ -54,7 +54,7 @@
- +
diff --git a/console/src/app/services/mgmt.service.ts b/console/src/app/services/mgmt.service.ts index 11cf9ed74e..d25cce37c1 100644 --- a/console/src/app/services/mgmt.service.ts +++ b/console/src/app/services/mgmt.service.ts @@ -221,6 +221,8 @@ import { ListPersonalAccessTokensResponse, ListProjectChangesRequest, ListProjectChangesResponse, + ListProjectGrantChangesRequest, + ListProjectGrantChangesResponse, ListProjectGrantMemberRolesRequest, ListProjectGrantMemberRolesResponse, ListProjectGrantMembersRequest, @@ -1776,6 +1778,28 @@ export class ManagementService { return this.grpcService.mgmt.listProjectChanges(req, null).then((resp) => resp.toObject()); } + public listProjectGrantChanges( + projectId: string, + grantId: string, + limit: number, + sequence: number, + ): Promise { + const req = new ListProjectGrantChangesRequest(); + req.setProjectId(projectId); + req.setGrantId(grantId); + const query = new ChangeQuery(); + + if (limit) { + query.setLimit(limit); + } + if (sequence) { + query.setSequence(sequence); + } + + req.setQuery(query); + return this.grpcService.mgmt.listProjectGrantChanges(req, null).then((resp) => resp.toObject()); + } + public listUserChanges(userId: string, limit: number, sequence: number): Promise { const req = new ListUserChangesRequest(); req.setUserId(userId); diff --git a/docs/docs/apis/proto/management.md b/docs/docs/apis/proto/management.md index 44505cc2d7..2eac7a106c 100644 --- a/docs/docs/apis/proto/management.md +++ b/docs/docs/apis/proto/management.md @@ -1417,6 +1417,19 @@ Removes an app key DELETE: /projects/{project_id}/apps/{app_id}/keys/{key_id} +### ListProjectGrantChanges + +> **rpc** ListProjectGrantChanges([ListProjectGrantChangesRequest](#listprojectgrantchangesrequest)) +[ListProjectGrantChangesResponse](#listprojectgrantchangesresponse) + +Returns the history of the project grant (each event) +Limit should always be set, there is a default limit set by the service + + + + POST: /projects/{project_id}/grants/{grant_id}/changes/_search + + ### GetProjectGrantByID > **rpc** GetProjectGrantByID([GetProjectGrantByIDRequest](#getprojectgrantbyidrequest)) @@ -5743,6 +5756,30 @@ This is an empty request +### ListProjectGrantChangesRequest + + + +| Field | Type | Description | Validation | +| ----- | ---- | ----------- | ----------- | +| query | zitadel.change.v1.ChangeQuery | list limitations and ordering | | +| project_id | string | - | string.min_len: 1
string.max_len: 200
| +| grant_id | string | - | string.min_len: 1
string.max_len: 200
| + + + + +### ListProjectGrantChangesResponse + + + +| Field | Type | Description | Validation | +| ----- | ---- | ----------- | ----------- | +| result | repeated zitadel.change.v1.Change | zitadel.v1.ListDetails details = 1; was always returned empty (as we cannot get the necessary infos) | | + + + + ### ListProjectGrantMemberRolesRequest diff --git a/internal/api/grpc/auth/user.go b/internal/api/grpc/auth/user.go index 7f31b51567..e43d018ece 100644 --- a/internal/api/grpc/auth/user.go +++ b/internal/api/grpc/auth/user.go @@ -177,8 +177,12 @@ func (s *Server) ListMyProjectOrgs(ctx context.Context, req *auth_pb.ListMyProje if !isIAMAdmin(memberships.Memberships) { ids := make([]string, 0, len(memberships.Memberships)) - for _, grant := range memberships.Memberships { - ids = appendIfNotExists(ids, grant.ResourceOwner) + for _, membership := range memberships.Memberships { + orgID := membership.ResourceOwner + if membership.ProjectGrant != nil && membership.ProjectGrant.GrantedOrgID != "" { + orgID = membership.ProjectGrant.GrantedOrgID + } + ids = appendIfNotExists(ids, orgID) } idsQuery, err := query.NewOrgIDsSearchQuery(ids...) diff --git a/internal/api/grpc/management/project.go b/internal/api/grpc/management/project.go index 27be7798e2..a55a2fb576 100644 --- a/internal/api/grpc/management/project.go +++ b/internal/api/grpc/management/project.go @@ -55,6 +55,17 @@ func (s *Server) ListProjects(ctx context.Context, req *mgmt_pb.ListProjectsRequ }, nil } +func (s *Server) ListProjectGrantChanges(ctx context.Context, req *mgmt_pb.ListProjectGrantChangesRequest) (*mgmt_pb.ListProjectGrantChangesResponse, error) { + sequence, limit, asc := change_grpc.ChangeQueryToQuery(req.Query) + res, err := s.query.ProjectGrantChanges(ctx, req.ProjectId, req.GrantId, sequence, limit, asc, s.auditLogRetention) + if err != nil { + return nil, err + } + return &mgmt_pb.ListProjectGrantChangesResponse{ + Result: change_grpc.ChangesToPb(res.Changes, s.assetAPIPrefix(ctx)), + }, nil +} + func (s *Server) ListGrantedProjects(ctx context.Context, req *mgmt_pb.ListGrantedProjectsRequest) (*mgmt_pb.ListGrantedProjectsResponse, error) { queries, err := listGrantedProjectsRequestToModel(req) if err != nil { diff --git a/internal/authz/repository/eventsourcing/eventstore/user_membership.go b/internal/authz/repository/eventsourcing/eventstore/user_membership.go index 9dc5bb57c9..6239730e74 100644 --- a/internal/authz/repository/eventsourcing/eventstore/user_membership.go +++ b/internal/authz/repository/eventsourcing/eventstore/user_membership.go @@ -34,8 +34,12 @@ func (repo *UserMembershipRepo) searchUserMemberships(ctx context.Context) (_ [] if err != nil { return nil, err } + grantedIDQuery, err := query.NewMembershipGrantedOrgIDSearchQuery(ctxData.OrgID) + if err != nil { + return nil, err + } memberships, err := repo.Queries.Memberships(ctx, &query.MembershipSearchQuery{ - Queries: []query.SearchQuery{userIDQuery, orgIDsQuery}, + Queries: []query.SearchQuery{userIDQuery, query.Or(orgIDsQuery, grantedIDQuery)}, }) if err != nil { return nil, err diff --git a/internal/query/changes.go b/internal/query/changes.go index fa5989f636..6e1e79f1a4 100644 --- a/internal/query/changes.go +++ b/internal/query/changes.go @@ -50,6 +50,17 @@ func (q *Queries) ProjectChanges(ctx context.Context, projectID string, lastSequ return q.changes(ctx, query, lastSequence, limit, sortAscending, auditLogRetention) } +func (q *Queries) ProjectGrantChanges(ctx context.Context, projectID, grantID string, lastSequence uint64, limit uint64, sortAscending bool, auditLogRetention time.Duration) (*Changes, error) { + query := func(query *eventstore.SearchQuery) { + query.AggregateTypes(project.AggregateType). + AggregateIDs(projectID). + EventData(map[string]interface{}{ + "grantId": grantID, + }) + } + return q.changes(ctx, query, lastSequence, limit, sortAscending, auditLogRetention) +} + func (q *Queries) ApplicationChanges(ctx context.Context, projectID, appID string, lastSequence uint64, limit uint64, sortAscending bool, auditLogRetention time.Duration) (*Changes, error) { query := func(query *eventstore.SearchQuery) { query.AggregateTypes(project.AggregateType). diff --git a/internal/query/search_query.go b/internal/query/search_query.go index 2f71a906ce..c2f2500cda 100644 --- a/internal/query/search_query.go +++ b/internal/query/search_query.go @@ -312,6 +312,28 @@ func ListComparisonFromMethod(m domain.SearchMethod) ListComparison { } } +type or struct { + queries []SearchQuery +} + +func Or(queries ...SearchQuery) *or { + return &or{ + queries: queries, + } +} + +func (q *or) toQuery(query sq.SelectBuilder) sq.SelectBuilder { + return query.Where(q.comp()) +} + +func (q *or) comp() sq.Sqlizer { + queries := make([]sq.Sqlizer, 0) + for _, query := range q.queries { + queries = append(queries, query.comp()) + } + return sq.Or(queries) +} + type BoolQuery struct { Column Column Value bool diff --git a/internal/query/user_membership.go b/internal/query/user_membership.go index 75c23902a6..2204312367 100644 --- a/internal/query/user_membership.go +++ b/internal/query/user_membership.go @@ -48,9 +48,10 @@ type ProjectMembership struct { } type ProjectGrantMembership struct { - ProjectID string - ProjectName string - GrantID string + ProjectID string + ProjectName string + GrantID string + GrantedOrgID string } type MembershipSearchQuery struct { @@ -78,6 +79,10 @@ func NewMembershipResourceOwnersSearchQuery(ids ...string) (SearchQuery, error) return NewListQuery(membershipResourceOwner, list, ListIn) } +func NewMembershipGrantedOrgIDSearchQuery(id string) (SearchQuery, error) { + return NewTextQuery(ProjectGrantColumnGrantedOrgID, id, TextEquals) +} + func NewMembershipProjectIDQuery(value string) (SearchQuery, error) { return NewTextQuery(membershipProjectID, value, TextEquals) } @@ -173,6 +178,10 @@ var ( name: projection.ProjectGrantMemberGrantIDCol, table: membershipAlias, } + membershipGrantGrantedOrgID = Column{ + name: projection.ProjectGrantColumnGrantedOrgID, + table: membershipAlias, + } membershipFrom = "(" + prepareOrgMember() + @@ -197,12 +206,14 @@ func prepareMembershipsQuery() (sq.SelectBuilder, func(*sql.Rows) (*Memberships, membershipIAMID.identifier(), membershipProjectID.identifier(), membershipGrantID.identifier(), + ProjectGrantColumnGrantedOrgID.identifier(), ProjectColumnName.identifier(), OrgColumnName.identifier(), countColumn.identifier(), ).From(membershipFrom). LeftJoin(join(ProjectColumnID, membershipProjectID)). LeftJoin(join(OrgColumnID, membershipOrgID)). + LeftJoin(join(ProjectGrantColumnGrantID, membershipGrantID)). PlaceholderFormat(sq.Dollar), func(rows *sql.Rows) (*Memberships, error) { memberships := make([]*Membership, 0) @@ -210,14 +221,15 @@ func prepareMembershipsQuery() (sq.SelectBuilder, func(*sql.Rows) (*Memberships, for rows.Next() { var ( - membership = new(Membership) - orgID = sql.NullString{} - iamID = sql.NullString{} - projectID = sql.NullString{} - grantID = sql.NullString{} - roles = pq.StringArray{} - projectName = sql.NullString{} - orgName = sql.NullString{} + membership = new(Membership) + orgID = sql.NullString{} + iamID = sql.NullString{} + projectID = sql.NullString{} + grantID = sql.NullString{} + grantedOrgID = sql.NullString{} + roles = pq.StringArray{} + projectName = sql.NullString{} + orgName = sql.NullString{} ) err := rows.Scan( @@ -231,6 +243,7 @@ func prepareMembershipsQuery() (sq.SelectBuilder, func(*sql.Rows) (*Memberships, &iamID, &projectID, &grantID, + &grantedOrgID, &projectName, &orgName, &count, @@ -252,11 +265,12 @@ func prepareMembershipsQuery() (sq.SelectBuilder, func(*sql.Rows) (*Memberships, IAMID: iamID.String, Name: iamID.String, } - } else if projectID.Valid && grantID.Valid { + } else if projectID.Valid && grantID.Valid && grantedOrgID.Valid { membership.ProjectGrant = &ProjectGrantMembership{ - ProjectID: projectID.String, - ProjectName: projectName.String, - GrantID: grantID.String, + ProjectID: projectID.String, + ProjectName: projectName.String, + GrantID: grantID.String, + GrantedOrgID: grantedOrgID.String, } } else if projectID.Valid { membership.Project = &ProjectMembership{ @@ -346,7 +360,8 @@ func prepareProjectGrantMember() string { "NULL::STRING AS "+membershipIAMID.name, ProjectGrantMemberProjectID.identifier(), ProjectGrantMemberGrantID.identifier(), - ).From(projectGrantMemberTable.identifier()).MustSql() + ).From(projectGrantMemberTable.identifier()). + MustSql() return stmt } diff --git a/internal/query/user_membership_test.go b/internal/query/user_membership_test.go index 8f160680bd..88b74528a2 100644 --- a/internal/query/user_membership_test.go +++ b/internal/query/user_membership_test.go @@ -23,6 +23,7 @@ var ( ", memberships.id" + ", memberships.project_id" + ", memberships.grant_id" + + ", projections.project_grants.granted_org_id" + ", projections.projects.name" + ", projections.orgs.name" + ", COUNT(*) OVER ()" + @@ -80,7 +81,8 @@ var ( " FROM projections.project_grant_members as members" + ") AS memberships" + " LEFT JOIN projections.projects ON memberships.project_id = projections.projects.id" + - " LEFT JOIN projections.orgs ON memberships.org_id = projections.orgs.id") + " LEFT JOIN projections.orgs ON memberships.org_id = projections.orgs.id" + + " LEFT JOIN projections.project_grants ON memberships.grant_id = projections.project_grants.grant_id") membershipCols = []string{ "user_id", "roles", @@ -92,6 +94,7 @@ var ( "instance_id", "project_id", "grant_id", + "granted_org_id", "name", //project name "name", //org name "count", @@ -141,6 +144,7 @@ func Test_MembershipPrepares(t *testing.T) { nil, nil, nil, + nil, "org-name", }, }, @@ -184,6 +188,7 @@ func Test_MembershipPrepares(t *testing.T) { nil, nil, nil, + nil, }, }, ), @@ -224,6 +229,7 @@ func Test_MembershipPrepares(t *testing.T) { nil, "project-id", nil, + nil, "project-name", nil, }, @@ -266,6 +272,7 @@ func Test_MembershipPrepares(t *testing.T) { nil, "project-id", "grant-id", + "granted-org-id", "project-name", nil, }, @@ -285,9 +292,10 @@ func Test_MembershipPrepares(t *testing.T) { Sequence: 20211202, ResourceOwner: "ro", ProjectGrant: &ProjectGrantMembership{ - GrantID: "grant-id", - ProjectID: "project-id", - ProjectName: "project-name", + GrantID: "grant-id", + ProjectID: "project-id", + ProjectName: "project-name", + GrantedOrgID: "granted-org-id", }, }, }, @@ -313,6 +321,7 @@ func Test_MembershipPrepares(t *testing.T) { nil, nil, nil, + nil, "org-name", }, { @@ -328,6 +337,7 @@ func Test_MembershipPrepares(t *testing.T) { nil, nil, nil, + nil, }, { "user-id", @@ -340,6 +350,7 @@ func Test_MembershipPrepares(t *testing.T) { nil, "project-id", nil, + nil, "project-name", nil, }, @@ -354,6 +365,7 @@ func Test_MembershipPrepares(t *testing.T) { nil, "project-id", "grant-id", + "granted-org-id", "project-name", nil, }, @@ -400,9 +412,10 @@ func Test_MembershipPrepares(t *testing.T) { Sequence: 20211202, ResourceOwner: "ro", ProjectGrant: &ProjectGrantMembership{ - ProjectID: "project-id", - GrantID: "grant-id", - ProjectName: "project-name", + ProjectID: "project-id", + GrantID: "grant-id", + ProjectName: "project-name", + GrantedOrgID: "granted-org-id", }, }, }, diff --git a/internal/query/zitadel_permission.go b/internal/query/zitadel_permission.go index c24ecd659f..f274783a0e 100644 --- a/internal/query/zitadel_permission.go +++ b/internal/query/zitadel_permission.go @@ -16,8 +16,12 @@ func (q *Queries) MyZitadelPermissions(ctx context.Context, orgID, userID string if err != nil { return nil, err } + grantedOrgIDQuery, err := NewMembershipGrantedOrgIDSearchQuery(orgID) + if err != nil { + return nil, err + } memberships, err := q.Memberships(ctx, &MembershipSearchQuery{ - Queries: []SearchQuery{userIDQuery, orgIDsQuery}, + Queries: []SearchQuery{userIDQuery, Or(orgIDsQuery, grantedOrgIDQuery)}, }) if err != nil { return nil, err diff --git a/pkg/grpc/management/changes.go b/pkg/grpc/management/changes.go index 403d4ee309..5cee5b9c1f 100644 --- a/pkg/grpc/management/changes.go +++ b/pkg/grpc/management/changes.go @@ -2,47 +2,47 @@ package management import ( "github.com/zitadel/zitadel/internal/api/grpc/server/middleware" + "github.com/zitadel/zitadel/pkg/grpc/change" ) func (c *ListUserChangesResponse) Localizers() []middleware.Localizer { if c == nil { return nil } - localizers := make([]middleware.Localizer, len(c.Result)) - for i, change := range c.Result { - localizers[i] = change.EventType - } - return localizers + return changesLocalizers(c.Result) } func (c *ListOrgChangesResponse) Localizers() []middleware.Localizer { if c == nil { return nil } - localizers := make([]middleware.Localizer, len(c.Result)) - for i, change := range c.Result { - localizers[i] = change.EventType - } - return localizers + return changesLocalizers(c.Result) } func (c *ListProjectChangesResponse) Localizers() []middleware.Localizer { if c == nil { return nil } - localizers := make([]middleware.Localizer, len(c.Result)) - for i, change := range c.Result { - localizers[i] = change.EventType + return changesLocalizers(c.Result) +} + +func (c *ListProjectGrantChangesResponse) Localizers() []middleware.Localizer { + if c == nil { + return nil } - return localizers + return changesLocalizers(c.Result) } func (c *ListAppChangesResponse) Localizers() []middleware.Localizer { if c == nil { return nil } - localizers := make([]middleware.Localizer, len(c.Result)) - for i, change := range c.Result { + return changesLocalizers(c.Result) +} + +func changesLocalizers(changes []*change.Change) []middleware.Localizer { + localizers := make([]middleware.Localizer, len(changes)) + for i, change := range changes { localizers[i] = change.EventType } return localizers diff --git a/proto/zitadel/management.proto b/proto/zitadel/management.proto index ce0ff6b528..11da17b0d5 100644 --- a/proto/zitadel/management.proto +++ b/proto/zitadel/management.proto @@ -1045,6 +1045,7 @@ service ManagementService { option (zitadel.v1.auth_option) = { permission: "project.read" + check_field_name: "ProjectId" }; } @@ -1470,6 +1471,19 @@ service ManagementService { }; } + // Returns the history of the project grant (each event) + // Limit should always be set, there is a default limit set by the service + rpc ListProjectGrantChanges(ListProjectGrantChangesRequest) returns (ListProjectGrantChangesResponse) { + option (google.api.http) = { + post: "/projects/{project_id}/grants/{grant_id}/changes/_search" + }; + + option (zitadel.v1.auth_option) = { + permission: "project.grant.read" + check_field_name: "GrantId" + }; + } + // Returns a project grant (ProjectGrant = Grant another organisation for my project) rpc GetProjectGrantByID(GetProjectGrantByIDRequest) returns (GetProjectGrantByIDResponse) { option (google.api.http) = { @@ -4100,6 +4114,20 @@ message RemoveAppKeyResponse { zitadel.v1.ObjectDetails details = 1; } +message ListProjectGrantChangesRequest { + //list limitations and ordering + zitadel.change.v1.ChangeQuery query = 1; + string project_id = 2 [(validate.rules).string = {min_len: 1, max_len: 200}]; + string grant_id = 3 [(validate.rules).string = {min_len: 1, max_len: 200}]; +} + +message ListProjectGrantChangesResponse { + reserved 1; + reserved "details"; + // zitadel.v1.ListDetails details = 1; was always returned empty (as we cannot get the necessary infos) + repeated zitadel.change.v1.Change result = 2; +} + message GetProjectGrantByIDRequest { string project_id = 1 [(validate.rules).string = {min_len: 1, max_len: 200}]; string grant_id = 2 [(validate.rules).string = {min_len: 1, max_len: 200}];