Skip to content

Commit 27f94e8

Browse files
kushsharmakushsharma
committed
fix: cascade delete organization user roles
Signed-off-by: Kush Sharma <thekushsharma@gmail.com>
1 parent 11a2df9 commit 27f94e8

File tree

11 files changed

+143
-60
lines changed

11 files changed

+143
-60
lines changed

‎cmd/serve.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,8 @@ func buildAPIDependencies(
260260
)
261261

262262
organizationRepository := postgres.NewOrganizationRepository(dbc)
263-
organizationService := organization.NewService(organizationRepository, relationService, userService, authnService, cfg.App.DisableOrgsOnCreate)
263+
organizationService := organization.NewService(organizationRepository, relationService, userService,
264+
authnService, cfg.App.DisableOrgsOnCreate)
264265

265266
domainRepository := postgres.NewDomainRepository(logger, dbc)
266267
domainService := domain.NewService(logger, domainRepository, userService, organizationService)

‎core/deleter/service.go‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@ package deleter
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67

8+
"github.com/raystack/frontier/internal/bootstrap/schema"
9+
710
"github.com/google/uuid"
811
"github.com/raystack/frontier/core/invitation"
912

@@ -25,6 +28,7 @@ type ProjectService interface {
2528
type OrganizationService interface {
2629
Get(ctx context.Context, id string) (organization.Organization, error)
2730
DeleteModel(ctx context.Context, id string) error
31+
RemoveUsers(ctx context.Context, orgID string, userIDs []string) error
2832
}
2933

3034
type RoleService interface {
@@ -158,3 +162,70 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error {
158162

159163
return d.orgService.DeleteModel(ctx, id)
160164
}
165+
166+
// RemoveUsersFromOrg removes users from an organization as members
167+
func (d Service) RemoveUsersFromOrg(ctx context.Context, orgID string, userIDs []string) error {
168+
var err error
169+
for _, userID := range userIDs {
170+
userPolicies, policyErr := d.policyService.List(ctx, policy.Filter{
171+
PrincipalID: userID,
172+
PrincipalType: schema.UserPrincipal,
173+
})
174+
if policyErr != nil && !errors.Is(err, policy.ErrNotExist) {
175+
err = errors.Join(err, policyErr)
176+
continue
177+
}
178+
for _, pol := range userPolicies {
179+
// delete org level roles
180+
if pol.ResourceID == orgID && pol.ResourceType == schema.OrganizationNamespace {
181+
if policyErr := d.policyService.Delete(ctx, pol.ID); policyErr != nil {
182+
err = errors.Join(err, policyErr)
183+
}
184+
}
185+
186+
// delete project level roles
187+
if pol.ResourceType == schema.ProjectNamespace {
188+
orgProjects, err := d.projService.List(ctx, project.Filter{
189+
OrgID: orgID,
190+
})
191+
if err != nil && !errors.Is(err, project.ErrNotExist) {
192+
return err
193+
}
194+
for _, p := range orgProjects {
195+
if pol.ResourceID == p.ID {
196+
if policyErr := d.policyService.Delete(ctx, pol.ID); policyErr != nil {
197+
err = errors.Join(err, policyErr)
198+
}
199+
}
200+
}
201+
202+
// delete resource level roles
203+
// not doing it at the moment as it can be pretty expensive
204+
}
205+
206+
// delete group level roles
207+
if pol.ResourceType == schema.GroupNamespace {
208+
orgGroups, err := d.groupService.List(ctx, group.Filter{
209+
OrganizationID: orgID,
210+
})
211+
if err != nil && !errors.Is(err, group.ErrNotExist) {
212+
return err
213+
}
214+
for _, g := range orgGroups {
215+
if pol.ResourceID == g.ID {
216+
if policyErr := d.policyService.Delete(ctx, pol.ID); policyErr != nil {
217+
err = errors.Join(err, policyErr)
218+
}
219+
}
220+
}
221+
}
222+
}
223+
}
224+
if err != nil {
225+
// abort if any error occurred
226+
return err
227+
}
228+
229+
// remove user from org
230+
return d.orgService.RemoveUsers(ctx, orgID, userIDs)
231+
}

‎core/organization/service.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ type Service struct {
3737
}
3838

3939
func NewService(repository Repository, relationService RelationService,
40-
userService UserService, authnService AuthnService, disableOrgsOnCreate bool) *Service {
40+
userService UserService, authnService AuthnService,
41+
disableOrgsOnCreate bool) *Service {
4142
defaultState := Enabled
4243
if disableOrgsOnCreate {
4344
defaultState = Disabled
@@ -197,9 +198,12 @@ func (s Service) AddUsers(ctx context.Context, orgID string, userIDs []string) e
197198
}
198199

199200
// RemoveUsers removes users from an organization as members
201+
// it doesn't remove user access to projects or other resources provided
202+
// by policies
200203
func (s Service) RemoveUsers(ctx context.Context, orgID string, userIDs []string) error {
201204
var err error
202205
for _, userID := range userIDs {
206+
// remove user from org
203207
if currentErr := s.relationService.Delete(ctx, relation.Relation{
204208
Object: relation.Object{
205209
ID: orgID,
@@ -209,7 +213,6 @@ func (s Service) RemoveUsers(ctx context.Context, orgID string, userIDs []string
209213
ID: userID,
210214
Namespace: schema.UserPrincipal,
211215
},
212-
RelationName: schema.MemberRelationName,
213216
}); err != nil {
214217
err = errors.Join(err, currentErr)
215218
}

‎internal/api/v1beta1/deleter.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
type CascadeDeleter interface {
1313
DeleteProject(ctx context.Context, id string) error
1414
DeleteOrganization(ctx context.Context, id string) error
15+
RemoveUsersFromOrg(ctx context.Context, orgID string, userIDs []string) error
1516
}
1617

1718
func (h Handler) DeleteProject(ctx context.Context, request *frontierv1beta1.DeleteProjectRequest) (*frontierv1beta1.DeleteProjectResponse, error) {

‎internal/api/v1beta1/mocks/cascade_deleter.go‎

Lines changed: 44 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/api/v1beta1/mocks/organization_service.go‎

Lines changed: 0 additions & 44 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/api/v1beta1/org.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ type OrganizationService interface {
3939
Update(ctx context.Context, toUpdate organization.Organization) (organization.Organization, error)
4040
ListByUser(ctx context.Context, userID string) ([]organization.Organization, error)
4141
AddUsers(ctx context.Context, orgID string, userID []string) error
42-
RemoveUsers(ctx context.Context, orgID string, userID []string) error
4342
Enable(ctx context.Context, id string) error
4443
Disable(ctx context.Context, id string) error
4544
}
@@ -415,7 +414,7 @@ func (h Handler) RemoveOrganizationUser(ctx context.Context, request *frontierv1
415414
return nil, grpcMinAdminCountErr
416415
}
417416

418-
if err := h.orgService.RemoveUsers(ctx, orgResp.ID, []string{request.GetUserId()}); err != nil {
417+
if err := h.deleterService.RemoveUsersFromOrg(ctx, orgResp.ID, []string{request.GetUserId()}); err != nil {
419418
logger.Error(err.Error())
420419
return nil, grpcInternalServerError
421420
}

‎internal/api/v1beta1/org_test.go‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,14 +1061,14 @@ func TestHandler_AddOrganizationUser(t *testing.T) {
10611061
func TestHandler_RemoveOrganizationUser(t *testing.T) {
10621062
tests := []struct {
10631063
name string
1064-
setup func(os *mocks.OrganizationService, us *mocks.UserService)
1064+
setup func(os *mocks.OrganizationService, us *mocks.UserService, ds *mocks.CascadeDeleter)
10651065
req *frontierv1beta1.RemoveOrganizationUserRequest
10661066
want *frontierv1beta1.RemoveOrganizationUserResponse
10671067
wantErr error
10681068
}{
10691069
{
10701070
name: "should return internal error if org service return some error",
1071-
setup: func(os *mocks.OrganizationService, us *mocks.UserService) {
1071+
setup: func(os *mocks.OrganizationService, us *mocks.UserService, ds *mocks.CascadeDeleter) {
10721072
os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), testOrgID).Return(organization.Organization{}, errors.New("some error"))
10731073
},
10741074
req: &frontierv1beta1.RemoveOrganizationUserRequest{
@@ -1080,12 +1080,12 @@ func TestHandler_RemoveOrganizationUser(t *testing.T) {
10801080
},
10811081
{
10821082
name: "should return the error and not remove user if it is the last admin user",
1083-
setup: func(os *mocks.OrganizationService, us *mocks.UserService) {
1083+
setup: func(os *mocks.OrganizationService, us *mocks.UserService, ds *mocks.CascadeDeleter) {
10841084
os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), testOrgID).Return(testOrgMap[testOrgID], nil)
10851085
us.EXPECT().ListByOrg(mock.AnythingOfType("*context.emptyCtx"), testOrgID, "update").Return([]user.User{
10861086
testUserMap[testUserID],
10871087
}, nil)
1088-
os.EXPECT().RemoveUsers(mock.AnythingOfType("*context.emptyCtx"), testOrgID, []string{testUserID}).Return(nil)
1088+
ds.EXPECT().RemoveUsersFromOrg(mock.AnythingOfType("*context.emptyCtx"), testOrgID, []string{testUserID}).Return(nil)
10891089
},
10901090
req: &frontierv1beta1.RemoveOrganizationUserRequest{
10911091
Id: testOrgID,
@@ -1096,7 +1096,7 @@ func TestHandler_RemoveOrganizationUser(t *testing.T) {
10961096
},
10971097
{
10981098
name: "should remove user from org successfully",
1099-
setup: func(os *mocks.OrganizationService, us *mocks.UserService) {
1099+
setup: func(os *mocks.OrganizationService, us *mocks.UserService, ds *mocks.CascadeDeleter) {
11001100
os.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), testOrgID).Return(testOrgMap[testOrgID], nil)
11011101
us.EXPECT().ListByOrg(mock.AnythingOfType("*context.emptyCtx"), testOrgID, "update").Return([]user.User{
11021102
testUserMap[testUserID],
@@ -1110,7 +1110,7 @@ func TestHandler_RemoveOrganizationUser(t *testing.T) {
11101110
UpdatedAt: time.Time{},
11111111
},
11121112
}, nil)
1113-
os.EXPECT().RemoveUsers(mock.AnythingOfType("*context.emptyCtx"), testOrgID, []string{"some-user-id"}).Return(nil)
1113+
ds.EXPECT().RemoveUsersFromOrg(mock.AnythingOfType("*context.emptyCtx"), testOrgID, []string{"some-user-id"}).Return(nil)
11141114
},
11151115
req: &frontierv1beta1.RemoveOrganizationUserRequest{
11161116
Id: testOrgID,
@@ -1125,11 +1125,16 @@ func TestHandler_RemoveOrganizationUser(t *testing.T) {
11251125
t.Run(tt.name, func(t *testing.T) {
11261126
mockOrgService := new(mocks.OrganizationService)
11271127
mockUserService := new(mocks.UserService)
1128+
mockDeleterService := new(mocks.CascadeDeleter)
11281129
ctx := context.Background()
11291130
if tt.setup != nil {
1130-
tt.setup(mockOrgService, mockUserService)
1131+
tt.setup(mockOrgService, mockUserService, mockDeleterService)
1132+
}
1133+
mockDep := Handler{
1134+
orgService: mockOrgService,
1135+
userService: mockUserService,
1136+
deleterService: mockDeleterService,
11311137
}
1132-
mockDep := Handler{orgService: mockOrgService, userService: mockUserService}
11331138
got, err := mockDep.RemoveOrganizationUser(ctx, tt.req)
11341139
assert.EqualValues(t, tt.wantErr, err)
11351140
assert.EqualValues(t, tt.want, got)

‎internal/api/v1beta1/user.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ func (h Handler) ListCurrentUserGroups(ctx context.Context, request *frontierv1b
442442
groupsPb = append(groupsPb, &groupPB)
443443
}
444444

445-
if len(request.WithPermissions) == 0 {
445+
if len(request.WithPermissions) > 0 {
446446
resourceIds := utils.Map(groupsList, func(res group.Group) string {
447447
return res.ID
448448
})
@@ -612,7 +612,7 @@ func (h Handler) ListProjectsByCurrentUser(ctx context.Context, request *frontie
612612
}
613613
projects = append(projects, projPB)
614614
}
615-
if len(request.WithPermissions) == 0 {
615+
if len(request.WithPermissions) > 0 {
616616
resourceIds := utils.Map(projList, func(res project.Project) string {
617617
return res.ID
618618
})

‎internal/store/postgres/policy_repository.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ func (r PolicyRepository) List(ctx context.Context, flt policy.Filter) ([]policy
8484
"resource_type": schema.OrganizationNamespace,
8585
})
8686
}
87-
8887
if flt.GroupID != "" {
8988
stmt = stmt.Where(goqu.Ex{
9089
"resource_id": flt.GroupID,

0 commit comments

Comments
 (0)