From 84a397b6cb99fb6efdeb39dad3dd9b15e1bf91a4 Mon Sep 17 00:00:00 2001 From: Kush Sharma Date: Thu, 21 Sep 2023 12:07:36 +0530 Subject: [PATCH] fix: org members should be allowed to get org details Signed-off-by: Kush Sharma --- cmd/serve.go | 2 +- core/deleter/service.go | 22 +++++++++- core/organization/service.go | 2 +- core/serviceuser/service.go | 2 +- core/user/service.go | 6 ++- internal/api/v1beta1/deleter.go | 1 + internal/api/v1beta1/mocks/cascade_deleter.go | 43 +++++++++++++++++++ internal/api/v1beta1/mocks/user_service.go | 43 ------------------- internal/api/v1beta1/user.go | 3 +- internal/bootstrap/schema/base_schema.zed | 2 +- .../bootstrap/testdata/compiled_schema.zed | 2 +- 11 files changed, 75 insertions(+), 53 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index 75bf9db0c..7aefb9f3e 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -297,7 +297,7 @@ func buildAPIDependencies( invitationService := invitation.NewService(mailDialer, postgres.NewInvitationRepository(logger, dbc), organizationService, groupService, userService, relationService, policyService, cfg.App.Invite) cascadeDeleter := deleter.NewCascadeDeleter(organizationService, projectService, resourceService, - groupService, policyService, roleService, invitationService) + groupService, policyService, roleService, invitationService, userService) // we should default it with a stdout logger repository as postgres can start to bloat really fast var auditRepository audit.Repository diff --git a/core/deleter/service.go b/core/deleter/service.go index b1e12968b..d6ac7f329 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -29,6 +29,7 @@ type OrganizationService interface { Get(ctx context.Context, id string) (organization.Organization, error) DeleteModel(ctx context.Context, id string) error RemoveUsers(ctx context.Context, orgID string, userIDs []string) error + ListByUser(ctx context.Context, userID string) ([]organization.Organization, error) } type RoleService interface { @@ -56,6 +57,10 @@ type InvitationService interface { Delete(ctx context.Context, id uuid.UUID) error } +type UserService interface { + Delete(ctx context.Context, id string) error +} + type Service struct { projService ProjectService orgService OrganizationService @@ -64,12 +69,13 @@ type Service struct { policyService PolicyService roleService RoleService invitationService InvitationService + userService UserService } func NewCascadeDeleter(orgService OrganizationService, projService ProjectService, resService ResourceService, groupService GroupService, policyService PolicyService, roleService RoleService, - invitationService InvitationService) *Service { + invitationService InvitationService, userService UserService) *Service { return &Service{ projService: projService, orgService: orgService, @@ -78,6 +84,7 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic policyService: policyService, roleService: roleService, invitationService: invitationService, + userService: userService, } } @@ -229,3 +236,16 @@ func (d Service) RemoveUsersFromOrg(ctx context.Context, orgID string, userIDs [ // remove user from org return d.orgService.RemoveUsers(ctx, orgID, userIDs) } + +func (d Service) DeleteUser(ctx context.Context, userID string) error { + userOrgs, err := d.orgService.ListByUser(ctx, userID) + if err != nil { + return err + } + for _, org := range userOrgs { + if err = d.RemoveUsersFromOrg(ctx, org.ID, []string{userID}); err != nil { + return fmt.Errorf("failed to delete user from org[%s]: %w", org.Name, err) + } + } + return d.userService.Delete(ctx, userID) +} diff --git a/core/organization/service.go b/core/organization/service.go index 2f4224e1d..92b09ec2e 100644 --- a/core/organization/service.go +++ b/core/organization/service.go @@ -199,7 +199,7 @@ func (s Service) AddUsers(ctx context.Context, orgID string, userIDs []string) e // RemoveUsers removes users from an organization as members // it doesn't remove user access to projects or other resources provided -// by policies +// by policies, don't call directly, use cascade deleter func (s Service) RemoveUsers(ctx context.Context, orgID string, userIDs []string) error { var err error for _, userID := range userIDs { diff --git a/core/serviceuser/service.go b/core/serviceuser/service.go index 276be90e3..e0920c4bd 100644 --- a/core/serviceuser/service.go +++ b/core/serviceuser/service.go @@ -141,7 +141,7 @@ func (s Service) Delete(ctx context.Context, id string) error { // delete all of serviceuser relationships // before deleting the serviceuser if err := s.relService.Delete(ctx, relation.Relation{ - Object: relation.Object{ + Subject: relation.Subject{ ID: id, Namespace: schema.ServiceUserPrincipal, }, diff --git a/core/user/service.go b/core/user/service.go index 488d18868..0bd53ac2a 100644 --- a/core/user/service.go +++ b/core/user/service.go @@ -108,10 +108,12 @@ func (s Service) Disable(ctx context.Context, id string) error { return s.repository.SetState(ctx, id, Disabled) } +// Delete by user uuid +// don't call this directly, use cascade deleter func (s Service) Delete(ctx context.Context, id string) error { - if err := s.relationService.Delete(ctx, relation.Relation{Object: relation.Object{ + if err := s.relationService.Delete(ctx, relation.Relation{Subject: relation.Subject{ ID: id, - Namespace: schema.ProjectNamespace, + Namespace: schema.UserPrincipal, }}); err != nil { return err } diff --git a/internal/api/v1beta1/deleter.go b/internal/api/v1beta1/deleter.go index ffa10f258..bb08eb16c 100644 --- a/internal/api/v1beta1/deleter.go +++ b/internal/api/v1beta1/deleter.go @@ -13,6 +13,7 @@ type CascadeDeleter interface { DeleteProject(ctx context.Context, id string) error DeleteOrganization(ctx context.Context, id string) error RemoveUsersFromOrg(ctx context.Context, orgID string, userIDs []string) error + DeleteUser(ctx context.Context, userID string) error } func (h Handler) DeleteProject(ctx context.Context, request *frontierv1beta1.DeleteProjectRequest) (*frontierv1beta1.DeleteProjectResponse, error) { diff --git a/internal/api/v1beta1/mocks/cascade_deleter.go b/internal/api/v1beta1/mocks/cascade_deleter.go index 697f96395..b6d607612 100644 --- a/internal/api/v1beta1/mocks/cascade_deleter.go +++ b/internal/api/v1beta1/mocks/cascade_deleter.go @@ -107,6 +107,49 @@ func (_c *CascadeDeleter_DeleteProject_Call) RunAndReturn(run func(context.Conte return _c } +// DeleteUser provides a mock function with given fields: ctx, userID +func (_m *CascadeDeleter) DeleteUser(ctx context.Context, userID string) error { + ret := _m.Called(ctx, userID) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = rf(ctx, userID) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// CascadeDeleter_DeleteUser_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteUser' +type CascadeDeleter_DeleteUser_Call struct { + *mock.Call +} + +// DeleteUser is a helper method to define mock.On call +// - ctx context.Context +// - userID string +func (_e *CascadeDeleter_Expecter) DeleteUser(ctx interface{}, userID interface{}) *CascadeDeleter_DeleteUser_Call { + return &CascadeDeleter_DeleteUser_Call{Call: _e.mock.On("DeleteUser", ctx, userID)} +} + +func (_c *CascadeDeleter_DeleteUser_Call) Run(run func(ctx context.Context, userID string)) *CascadeDeleter_DeleteUser_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *CascadeDeleter_DeleteUser_Call) Return(_a0 error) *CascadeDeleter_DeleteUser_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *CascadeDeleter_DeleteUser_Call) RunAndReturn(run func(context.Context, string) error) *CascadeDeleter_DeleteUser_Call { + _c.Call.Return(run) + return _c +} + // RemoveUsersFromOrg provides a mock function with given fields: ctx, orgID, userIDs func (_m *CascadeDeleter) RemoveUsersFromOrg(ctx context.Context, orgID string, userIDs []string) error { ret := _m.Called(ctx, orgID, userIDs) diff --git a/internal/api/v1beta1/mocks/user_service.go b/internal/api/v1beta1/mocks/user_service.go index d91a744fb..1c57354a3 100644 --- a/internal/api/v1beta1/mocks/user_service.go +++ b/internal/api/v1beta1/mocks/user_service.go @@ -75,49 +75,6 @@ func (_c *UserService_Create_Call) RunAndReturn(run func(context.Context, user.U return _c } -// Delete provides a mock function with given fields: ctx, id -func (_m *UserService) Delete(ctx context.Context, id string) error { - ret := _m.Called(ctx, id) - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { - r0 = rf(ctx, id) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// UserService_Delete_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Delete' -type UserService_Delete_Call struct { - *mock.Call -} - -// Delete is a helper method to define mock.On call -// - ctx context.Context -// - id string -func (_e *UserService_Expecter) Delete(ctx interface{}, id interface{}) *UserService_Delete_Call { - return &UserService_Delete_Call{Call: _e.mock.On("Delete", ctx, id)} -} - -func (_c *UserService_Delete_Call) Run(run func(ctx context.Context, id string)) *UserService_Delete_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string)) - }) - return _c -} - -func (_c *UserService_Delete_Call) Return(_a0 error) *UserService_Delete_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *UserService_Delete_Call) RunAndReturn(run func(context.Context, string) error) *UserService_Delete_Call { - _c.Call.Return(run) - return _c -} - // Disable provides a mock function with given fields: ctx, id func (_m *UserService) Disable(ctx context.Context, id string) error { ret := _m.Called(ctx, id) diff --git a/internal/api/v1beta1/user.go b/internal/api/v1beta1/user.go index 8a5be873c..1bab7ca33 100644 --- a/internal/api/v1beta1/user.go +++ b/internal/api/v1beta1/user.go @@ -44,7 +44,6 @@ type UserService interface { Update(ctx context.Context, toUpdate user.User) (user.User, error) Enable(ctx context.Context, id string) error Disable(ctx context.Context, id string) error - Delete(ctx context.Context, id string) error IsSudo(ctx context.Context, id string) (bool, error) } @@ -664,7 +663,7 @@ func (h Handler) DisableUser(ctx context.Context, request *frontierv1beta1.Disab func (h Handler) DeleteUser(ctx context.Context, request *frontierv1beta1.DeleteUserRequest) (*frontierv1beta1.DeleteUserResponse, error) { logger := grpczap.Extract(ctx) - if err := h.userService.Delete(ctx, request.GetId()); err != nil { + if err := h.deleterService.DeleteUser(ctx, request.GetId()); err != nil { logger.Error(err.Error()) return nil, status.Errorf(codes.Internal, err.Error()) } diff --git a/internal/bootstrap/schema/base_schema.zed b/internal/bootstrap/schema/base_schema.zed index e86836b7f..5f64344d2 100644 --- a/internal/bootstrap/schema/base_schema.zed +++ b/internal/bootstrap/schema/base_schema.zed @@ -32,7 +32,7 @@ definition app/organization { permission delete = platform->superuser + granted->app_organization_administer + granted->app_organization_delete + owner permission update = platform->superuser + granted->app_organization_administer + granted->app_organization_update + owner - permission get = platform->superuser + granted->app_organization_administer + granted->app_organization_get + owner + permission get = platform->superuser + granted->app_organization_administer + granted->app_organization_get + owner + member permission rolemanage = platform->superuser + granted->app_organization_administer + granted->app_organization_rolemanage + owner permission policymanage = platform->superuser + granted->app_organization_administer + granted->app_organization_policymanage + owner permission projectlist = platform->superuser + granted->app_organization_administer + granted->app_organization_projectlist + owner diff --git a/internal/bootstrap/testdata/compiled_schema.zed b/internal/bootstrap/testdata/compiled_schema.zed index 017b271fa..0957b7b71 100644 --- a/internal/bootstrap/testdata/compiled_schema.zed +++ b/internal/bootstrap/testdata/compiled_schema.zed @@ -31,7 +31,7 @@ definition app/organization { permission compute_receipt_get = owner + platform->superuser + granted->app_organization_administer + granted->compute_receipt_get permission compute_receipt_update = owner + platform->superuser + granted->app_organization_administer + granted->compute_receipt_update permission delete = platform->superuser + granted->app_organization_administer + granted->app_organization_delete + owner - permission get = platform->superuser + granted->app_organization_administer + granted->app_organization_get + owner + permission get = platform->superuser + granted->app_organization_administer + granted->app_organization_get + owner + member relation granted: app/rolebinding // synthetic permissions - group