From 28060581c61a640ac34a1368f2262575529c1163 Mon Sep 17 00:00:00 2001 From: Kush Sharma Date: Thu, 31 Oct 2024 15:44:25 +0530 Subject: [PATCH] feat: role filter while listing organization users - https://github.com/raystack/proton/pull/371 - a list of role name/ids are accepted as inputs, it can't be used when `with_roles` is toggled on. Signed-off-by: Kush Sharma --- core/policy/filter.go | 1 + internal/api/v1beta1/org.go | 121 ++++++++++++------- internal/store/postgres/policy_repository.go | 5 + test/e2e/regression/api_test.go | 82 ++++++++++++- 4 files changed, 160 insertions(+), 49 deletions(-) diff --git a/core/policy/filter.go b/core/policy/filter.go index d15807f7f..ee7743e13 100644 --- a/core/policy/filter.go +++ b/core/policy/filter.go @@ -5,6 +5,7 @@ type Filter struct { ProjectID string GroupID string RoleID string + RoleIDs []string PrincipalType string PrincipalID string diff --git a/internal/api/v1beta1/org.go b/internal/api/v1beta1/org.go index 87b598652..0160090bd 100644 --- a/internal/api/v1beta1/org.go +++ b/internal/api/v1beta1/org.go @@ -3,26 +3,20 @@ package v1beta1 import ( "context" - "github.com/raystack/frontier/core/serviceuser" - - "github.com/raystack/frontier/core/authenticate" - - "go.uber.org/zap" - + grpczap "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" + "github.com/pkg/errors" "github.com/raystack/frontier/core/audit" - "github.com/raystack/frontier/core/role" - "github.com/raystack/frontier/pkg/pagination" - "github.com/raystack/frontier/pkg/utils" - - "github.com/raystack/frontier/internal/bootstrap/schema" - + "github.com/raystack/frontier/core/authenticate" + "github.com/raystack/frontier/core/policy" "github.com/raystack/frontier/core/project" - - "github.com/pkg/errors" + "github.com/raystack/frontier/core/role" + "github.com/raystack/frontier/core/serviceuser" "github.com/raystack/frontier/core/user" + "github.com/raystack/frontier/internal/bootstrap/schema" "github.com/raystack/frontier/pkg/metadata" - - grpczap "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" + "github.com/raystack/frontier/pkg/pagination" + "github.com/raystack/frontier/pkg/utils" + "go.uber.org/zap" "github.com/raystack/frontier/core/organization" @@ -248,6 +242,10 @@ func (h Handler) ListOrganizationAdmins(ctx context.Context, request *frontierv1 } func (h Handler) ListOrganizationUsers(ctx context.Context, request *frontierv1beta1.ListOrganizationUsersRequest) (*frontierv1beta1.ListOrganizationUsersResponse, error) { + if len(request.GetRoleFilters()) > 0 && request.GetWithRoles() { + return nil, status.Errorf(codes.InvalidArgument, "cannot use role filters and with_roles together") + } + logger := grpczap.Extract(ctx) orgResp, err := h.orgService.Get(ctx, request.GetId()) if err != nil { @@ -261,9 +259,67 @@ func (h Handler) ListOrganizationUsers(ctx context.Context, request *frontierv1b } } - users, err := h.userService.ListByOrg(ctx, orgResp.ID, request.GetPermissionFilter()) - if err != nil { - return nil, err + var users []user.User + var rolePairPBs []*frontierv1beta1.ListOrganizationUsersResponse_RolePair + + if len(request.GetRoleFilters()) > 0 { + // convert role names to ids if needed + roleIDs := request.GetRoleFilters() + for i, roleFilter := range request.GetRoleFilters() { + if !utils.IsValidUUID(roleFilter) { + role, err := h.roleService.Get(ctx, roleFilter) + if err != nil { + return nil, err + } + roleIDs[i] = role.ID + } + } + + // need to fetch users with roles assigned to them + policies, err := h.policyService.List(ctx, policy.Filter{ + OrgID: request.GetId(), + PrincipalType: schema.UserPrincipal, + ResourceType: schema.OrganizationNamespace, + RoleIDs: roleIDs, + }) + if err != nil { + return nil, err + } + users = utils.Filter(utils.Map(policies, func(pol policy.Policy) user.User { + u, _ := h.userService.GetByID(ctx, pol.PrincipalID) + return u + }), func(u user.User) bool { + return u.ID != "" + }) + } else { + // list all users + users, err = h.userService.ListByOrg(ctx, orgResp.ID, request.GetPermissionFilter()) + if err != nil { + return nil, err + } + if request.GetWithRoles() { + for _, user := range users { + roles, err := h.policyService.ListRoles(ctx, schema.UserPrincipal, user.ID, schema.OrganizationNamespace, request.GetId()) + if err != nil { + return nil, err + } + + rolesPb := utils.Filter(utils.Map(roles, func(role role.Role) *frontierv1beta1.Role { + pb, err := transformRoleToPB(role) + if err != nil { + logger.Error("failed to transform role for group", zap.Error(err)) + return nil + } + return &pb + }), func(role *frontierv1beta1.Role) bool { + return role != nil + }) + rolePairPBs = append(rolePairPBs, &frontierv1beta1.ListOrganizationUsersResponse_RolePair{ + UserId: user.ID, + Roles: rolesPb, + }) + } + } } var usersPB []*frontierv1beta1.User @@ -272,35 +328,8 @@ func (h Handler) ListOrganizationUsers(ctx context.Context, request *frontierv1b if err != nil { return nil, err } - usersPB = append(usersPB, u) } - - var rolePairPBs []*frontierv1beta1.ListOrganizationUsersResponse_RolePair - if request.GetWithRoles() { - for _, user := range users { - roles, err := h.policyService.ListRoles(ctx, schema.UserPrincipal, user.ID, schema.OrganizationNamespace, request.GetId()) - if err != nil { - return nil, err - } - - rolesPb := utils.Filter(utils.Map(roles, func(role role.Role) *frontierv1beta1.Role { - pb, err := transformRoleToPB(role) - if err != nil { - logger.Error("failed to transform role for group", zap.Error(err)) - return nil - } - return &pb - }), func(role *frontierv1beta1.Role) bool { - return role != nil - }) - rolePairPBs = append(rolePairPBs, &frontierv1beta1.ListOrganizationUsersResponse_RolePair{ - UserId: user.ID, - Roles: rolesPb, - }) - } - } - return &frontierv1beta1.ListOrganizationUsersResponse{ Users: usersPB, RolePairs: rolePairPBs, diff --git a/internal/store/postgres/policy_repository.go b/internal/store/postgres/policy_repository.go index 2bfc25fff..e3215b313 100644 --- a/internal/store/postgres/policy_repository.go +++ b/internal/store/postgres/policy_repository.go @@ -118,6 +118,11 @@ func applyListFilter(stmt *goqu.SelectDataset, flt policy.Filter) *goqu.SelectDa "role_id": flt.RoleID, }) } + if len(flt.RoleIDs) > 0 { + stmt = stmt.Where(goqu.Ex{ + "role_id": flt.RoleIDs, + }) + } return stmt } diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index 2fdc003fb..cf995a0ab 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -345,6 +345,51 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { }) s.Assert().NoError(err) }) + s.Run("6. a user should successfully list organization users via it's filter", func() { + createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, &frontierv1beta1.CreateOrganizationRequest{ + Body: &frontierv1beta1.OrganizationRequestBody{ + Title: "org acme 1-6", + Name: "org-acme-1-6", + }, + }) + s.Assert().NoError(err) + + createUser1Resp, err := s.testBench.Client.CreateUser(ctxOrgAdminAuth, &frontierv1beta1.CreateUserRequest{ + Body: &frontierv1beta1.UserRequestBody{ + Email: "user-for-org-1-6-p1@raystack.org", + Name: "user-for-org-1-6-p1", + }, + }) + s.Assert().NoError(err) + s.Assert().NotNil(createUser1Resp) + + // add user to org + _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, &frontierv1beta1.AddOrganizationUsersRequest{ + Id: createOrgResp.GetOrganization().GetId(), + UserIds: []string{createUser1Resp.GetUser().GetId()}, + }) + s.Assert().NoError(err) + + orgUsersResp, err := s.testBench.Client.ListOrganizationUsers(ctxOrgAdminAuth, &frontierv1beta1.ListOrganizationUsersRequest{ + Id: createOrgResp.GetOrganization().GetId(), + }) + s.Assert().NoError(err) + s.Assert().Equal(2, len(orgUsersResp.GetUsers())) + emails := utils.Map(orgUsersResp.GetUsers(), func(u *frontierv1beta1.User) string { + return u.GetEmail() + }) + s.Assert().Contains(emails, createUser1Resp.GetUser().GetEmail()) + s.Assert().Contains(emails, testbench.OrgAdminEmail) + + // list only owner + orgUsersRespOwner, err := s.testBench.Client.ListOrganizationUsers(ctxOrgAdminAuth, &frontierv1beta1.ListOrganizationUsersRequest{ + Id: createOrgResp.GetOrganization().GetId(), + RoleFilters: []string{schema.RoleOrganizationOwner}, + }) + s.Assert().NoError(err) + s.Assert().Equal(1, len(orgUsersRespOwner.GetUsers())) + s.Assert().Equal(testbench.OrgAdminEmail, orgUsersRespOwner.GetUsers()[0].GetEmail()) + }) } func (s *APIRegressionTestSuite) TestProjectAPI() { @@ -501,12 +546,15 @@ func (s *APIRegressionTestSuite) TestProjectAPI() { }) s.Assert().NoError(err) s.Assert().NotNil(createUserResp) + createUserRespAuth := metadata.NewOutgoingContext(context.Background(), metadata.New(map[string]string{ + testbench.IdentityHeader: createUserResp.GetUser().GetEmail(), + })) // add user to project - _, err = s.testBench.Client.CreatePolicy(ctxOrgAdminAuth, &frontierv1beta1.CreatePolicyRequest{ - Body: &frontierv1beta1.PolicyRequestBody{ + _, err = s.testBench.Client.CreatePolicyForProject(ctxOrgAdminAuth, &frontierv1beta1.CreatePolicyForProjectRequest{ + ProjectId: createProjectP1Response.GetProject().GetId(), + Body: &frontierv1beta1.CreatePolicyForProjectBody{ RoleId: schema.RoleProjectViewer, - Resource: schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, createProjectP1Response.GetProject().GetId()), Principal: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, createUserResp.GetUser().GetId()), }, }) @@ -527,6 +575,34 @@ func (s *APIRegressionTestSuite) TestProjectAPI() { return p.GetName() == "org-project-2-p2" })) + // viewer should only have get permission + listProjCurrentUsersResp, err = s.testBench.Client.ListProjectsByCurrentUser(createUserRespAuth, &frontierv1beta1.ListProjectsByCurrentUserRequest{ + WithPermissions: []string{ + "update", + "get", + "delete", + }, + }) + s.Assert().NoError(err) + s.Assert().True(slices.ContainsFunc[[]*frontierv1beta1.Project](listProjCurrentUsersResp.GetProjects(), func(p *frontierv1beta1.Project) bool { + return p.GetName() == "org-project-2-p1" + })) + s.Assert().Len(listProjCurrentUsersResp.GetAccessPairs(), 1) + + // check permission for viewer + checkResourcePermissionResp, err := s.testBench.Client.CheckResourcePermission(createUserRespAuth, &frontierv1beta1.CheckResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, createProjectP1Response.GetProject().GetId()), + Permission: schema.GetPermission, + }) + s.Assert().NoError(err) + s.Assert().True(checkResourcePermissionResp.GetStatus()) + checkResourcePermissionResp, err = s.testBench.Client.CheckResourcePermission(createUserRespAuth, &frontierv1beta1.CheckResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, createProjectP1Response.GetProject().GetId()), + Permission: schema.UpdatePermission, + }) + s.Assert().NoError(err) + s.Assert().False(checkResourcePermissionResp.GetStatus()) + // create a group and add user to it createGroupResp, err := s.testBench.Client.CreateGroup(ctxOrgAdminAuth, &frontierv1beta1.CreateGroupRequest{ OrgId: existingOrg.GetOrganization().GetId(),