Skip to content

Commit

Permalink
fix: org members should be allowed to get org details
Browse files Browse the repository at this point in the history
Signed-off-by: Kush Sharma <[email protected]>
  • Loading branch information
kushsharma committed Sep 21, 2023
1 parent 5e00304 commit 1e508f5
Show file tree
Hide file tree
Showing 17 changed files with 90 additions and 68 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
steps:
- uses: actions/setup-go@v4
with:
go-version: '1.20'
go-version: '1.21'
- uses: actions/checkout@v3
with:
fetch-depth: 0
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v4
with:
go-version: "1.20"
go-version: "1.21"
- name: Login to DockerHub
uses: docker/login-action@v1
with:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v4
with:
go-version: "1.20"
go-version: "1.21"
- name: Login to DockerHub
uses: docker/login-action@v1
with:
Expand All @@ -41,7 +41,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v4
with:
go-version: "1.20"
go-version: "1.21"
- name: Login to DockerHub
uses: docker/login-action@v1
with:
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v4
with:
go-version: '1.20'
go-version: '1.21'
- name: install dependencies
run: go mod tidy
- name: run unit tests
Expand All @@ -42,7 +42,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v4
with:
go-version: '1.20'
go-version: '1.21'
- name: install dependencies
run: go mod tidy
- name: install spicedb binary
Expand All @@ -65,7 +65,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v4
with:
go-version: '1.20'
go-version: '1.21'
- name: install dependencies
run: go mod tidy
- name: run regression tests
Expand All @@ -81,7 +81,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v4
with:
go-version: '1.20'
go-version: '1.21'
- name: install dependencies
run: go mod tidy
- name: run integration tests
Expand Down
2 changes: 1 addition & 1 deletion cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 21 additions & 1 deletion core/deleter/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -78,6 +84,7 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic
policyService: policyService,
roleService: roleService,
invitationService: invitationService,
userService: userService,
}
}

Expand Down Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion core/organization/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion core/serviceuser/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
6 changes: 4 additions & 2 deletions core/user/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
10 changes: 5 additions & 5 deletions internal/api/v1beta1/audit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ func TestHandler_ListOrganizationAuditLogs(t *testing.T) {
{
name: "should return list of audit logs",
setup: func(as *mocks.AuditService, os *mocks.OrganizationService) {
os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), "org-id").Return(testOrgMap[testOrgID], nil)
as.EXPECT().List(mock.AnythingOfType("*context.emptyCtx"), audit.Filter{
os.EXPECT().Get(mock.Anything, "org-id").Return(testOrgMap[testOrgID], nil)
as.EXPECT().List(mock.Anything, audit.Filter{
OrgID: testOrgMap[testOrgID].ID,
Source: "guardian-service",
Action: "project.create",
Expand Down Expand Up @@ -84,8 +84,8 @@ func TestHandler_ListOrganizationAuditLogs(t *testing.T) {
{
name: "should return error when audit service returns error",
setup: func(as *mocks.AuditService, os *mocks.OrganizationService) {
os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), "org-id").Return(testOrgMap[testOrgID], nil)
as.EXPECT().List(mock.AnythingOfType("*context.emptyCtx"), audit.Filter{
os.EXPECT().Get(mock.Anything, "org-id").Return(testOrgMap[testOrgID], nil)
as.EXPECT().List(mock.Anything, audit.Filter{
OrgID: testOrgMap[testOrgID].ID,
Source: "guardian-service",
Action: "project.create",
Expand All @@ -106,7 +106,7 @@ func TestHandler_ListOrganizationAuditLogs(t *testing.T) {
{
name: "should return error when org is disabled",
setup: func(as *mocks.AuditService, os *mocks.OrganizationService) {
os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), "org-id").Return(organization.Organization{}, organization.ErrDisabled)
os.EXPECT().Get(mock.Anything, "org-id").Return(organization.Organization{}, organization.ErrDisabled)
},
request: &frontierv1beta1.ListOrganizationAuditLogsRequest{
OrgId: "org-id",
Expand Down
1 change: 1 addition & 0 deletions internal/api/v1beta1/deleter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
43 changes: 43 additions & 0 deletions internal/api/v1beta1/mocks/cascade_deleter.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 0 additions & 43 deletions internal/api/v1beta1/mocks/user_service.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions internal/api/v1beta1/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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())
}
Expand Down
2 changes: 1 addition & 1 deletion internal/bootstrap/schema/base_schema.zed
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/bootstrap/testdata/compiled_schema.zed
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/regression/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ func (s *APIRegressionTestSuite) TestUserAPI() {
PermissionFilter: schema.GetPermission,
})
s.Assert().NoError(err)
s.Assert().Equal(2, len(orgUsersRespAfterRelation.GetUsers()))
s.Assert().Equal(3, len(orgUsersRespAfterRelation.GetUsers()))
groupUsersResp, err := s.testBench.Client.ListGroupUsers(ctxOrgAdminAuth, &frontierv1beta1.ListGroupUsersRequest{
Id: existingGroup.Id,
OrgId: existingOrg.GetOrganization().GetId(),
Expand Down Expand Up @@ -735,7 +735,7 @@ func (s *APIRegressionTestSuite) TestUserAPI() {
PermissionFilter: schema.GetPermission,
})
s.Assert().NoError(err)
s.Assert().Equal(1, len(orgUsersRespAfterDeletion.GetUsers()))
s.Assert().Equal(2, len(orgUsersRespAfterDeletion.GetUsers()))

// check its relations with group
groupUsersRespAfterDeletion, err := s.testBench.Client.ListGroupUsers(ctxOrgAdminAuth, &frontierv1beta1.ListGroupUsersRequest{
Expand Down

0 comments on commit 1e508f5

Please sign in to comment.