diff --git a/pkg/roles/handlers.go b/pkg/roles/handlers.go index fd1ccfc51..e4cc3ad5c 100644 --- a/pkg/roles/handlers.go +++ b/pkg/roles/handlers.go @@ -62,6 +62,7 @@ func (a *API) RegisterEndpoints(mux *chi.Mux) { mux.Delete("/api/v0/roles/{id:.+}/entitlements/{e_id:.+}", a.handleRemovePermission) mux.Get("/api/v0/roles/{id:.+}/groups", a.handleListRoleGroup) } + func (a *API) RegisterValidation(v validation.ValidationRegistryInterface) { err := v.RegisterPayloadValidator("roles", a.payloadValidator) if err != nil { diff --git a/pkg/roles/service.go b/pkg/roles/service.go index 8d7057fa0..1f69d8a42 100644 --- a/pkg/roles/service.go +++ b/pkg/roles/service.go @@ -253,9 +253,12 @@ func (s *Service) DeleteRole(ctx context.Context, ID string) error { // https://go.dev/ref/spec#Send_statements // A send on an unbuffered channel can proceed if a receiver is ready. // A send on a buffered channel can proceed if there is room in the buffer - results := make(chan *pool.Result[any], len(s.permissionTypes())) + jobs := len(s.permissionTypes()) + 1 + + results := make(chan *pool.Result[any], jobs) wg := sync.WaitGroup{} - wg.Add(len(s.permissionTypes())) + // number of types + 1 for assignees job + wg.Add(jobs) // TODO @shipperizer use a background operator for _, t := range s.permissionTypes() { @@ -266,6 +269,12 @@ func (s *Service) DeleteRole(ctx context.Context, ID string) error { ) } + s.wpool.Submit( + s.removeAssigneesFunc(ctx, ID), + results, + &wg, + ) + // wait for tasks to finish wg.Wait() @@ -326,11 +335,51 @@ func (s *Service) removePermissionsByType(ctx context.Context, ID, pType string) break } + if len(permissions) == 0 { + return + } + if err := s.ofga.DeleteTuples(ctx, permissions...); err != nil { s.logger.Error(err.Error()) } } +func (s *Service) removeAssignees(ctx context.Context, ID string) { + ctx, span := s.tracer.Start(ctx, "roles.Service.removeAssignees") + defer span.End() + + cToken := "" + assignees := make([]ofga.Tuple, 0) + for { + r, err := s.ofga.ReadTuples(ctx, "", ASSIGNEE_RELATION, fmt.Sprintf("role:%s", ID), cToken) + + if err != nil { + s.logger.Errorf("error when retrieving tuples for %s role:%s", ASSIGNEE_RELATION, ID) + return + } + + for _, t := range r.Tuples { + assignees = append(assignees, *ofga.NewTuple(t.Key.User, t.Key.Relation, t.Key.Object)) + } + + // if there are more pages, keep going with the loop + if cToken = r.ContinuationToken; cToken != "" { + continue + } + + // TODO @shipperizer understand if better breaking at every cycle or reverting if clause + break + } + + if len(assignees) == 0 { + return + } + + if err := s.ofga.DeleteTuples(ctx, assignees...); err != nil { + s.logger.Error(err.Error()) + } +} + func (s *Service) listPermissionsFunc(ctx context.Context, roleID, ofgaType, cToken string) func() any { return func() any { p, token, err := s.listPermissionsByType( @@ -355,6 +404,12 @@ func (s *Service) removePermissionsFunc(ctx context.Context, roleID, ofgaType st } } +func (s *Service) removeAssigneesFunc(ctx context.Context, roleID string) func() { + return func() { + s.removeAssignees(ctx, roleID) + } +} + func (s *Service) permissionTypes() []string { return []string{"role", "group", "identity", "scheme", "provider", "client"} } diff --git a/pkg/roles/service_test.go b/pkg/roles/service_test.go index 931ee2222..c2fdfedcf 100644 --- a/pkg/roles/service_test.go +++ b/pkg/roles/service_test.go @@ -461,7 +461,7 @@ func TestServiceDeleteRole(t *testing.T) { mockOpenFGA := NewMockOpenFGAClientInterface(ctrl) workerPool := pool.NewMockWorkerPoolInterface(ctrl) - for i := 0; i < 6; i++ { + for i := 0; i < 7; i++ { setupMockSubmit(workerPool, nil) } @@ -469,6 +469,7 @@ func TestServiceDeleteRole(t *testing.T) { mockTracer.EXPECT().Start(gomock.Any(), "roles.Service.DeleteRole").Times(1).Return(context.TODO(), trace.SpanFromContext(context.TODO())) mockTracer.EXPECT().Start(gomock.Any(), "roles.Service.removePermissionsByType").Times(6).Return(context.TODO(), trace.SpanFromContext(context.TODO())) + mockTracer.EXPECT().Start(gomock.Any(), "roles.Service.removeAssignees").Times(1).Return(context.TODO(), trace.SpanFromContext(context.TODO())) pTypes := []string{"role", "group", "identity", "scheme", "provider", "client"} @@ -504,28 +505,78 @@ func TestServiceDeleteRole(t *testing.T) { } + calls = append( + calls, + mockOpenFGA.EXPECT().ReadTuples(gomock.Any(), "", ASSIGNEE_RELATION, fmt.Sprintf("role:%s", test.input), "").Times(1).DoAndReturn( + func(ctx context.Context, user, relation, object, continuationToken string) (*client.ClientReadResponse, error) { + if test.expected != nil { + return nil, test.expected + } + + tuples := []openfga.Tuple{ + *openfga.NewTuple( + *openfga.NewTupleKey( + "user:test", ASSIGNEE_RELATION, object, + ), + time.Now(), + ), + *openfga.NewTuple( + *openfga.NewTupleKey( + "group:test#member", ASSIGNEE_RELATION, object, + ), + time.Now(), + ), + } + + r := new(client.ClientReadResponse) + r.SetContinuationToken("") + r.SetTuples(tuples) + + return r, nil + }, + ), + ) + if test.expected == nil { mockOpenFGA.EXPECT().DeleteTuples( gomock.Any(), gomock.Any(), - ).Times(7).DoAndReturn( + ).Times(8).DoAndReturn( func(ctx context.Context, tuples ...ofga.Tuple) error { - if len(tuples) != 1 { - t.Errorf("too many tuples") - } - tuple := tuples[0] + switch len(tuples) { + case 1: + tuple := tuples[0] - if tuple.User != fmt.Sprintf("role:%s#%s", test.input, ASSIGNEE_RELATION) && tuple.User != authorization.ADMIN_PRIVILEGE { - t.Errorf("expected user to be one of %v got %v", []string{fmt.Sprintf("role:%s#%s", test.input, ASSIGNEE_RELATION), authorization.ADMIN_PRIVILEGE}, tuple.User) - } + if tuple.User != fmt.Sprintf("role:%s#%s", test.input, ASSIGNEE_RELATION) && tuple.User != authorization.ADMIN_PRIVILEGE { + t.Errorf("expected user to be one of %v got %v", []string{fmt.Sprintf("role:%s#%s", test.input, ASSIGNEE_RELATION), authorization.ADMIN_PRIVILEGE}, tuple.User) + } - if tuple.Relation != "privileged" && tuple.Relation != "can_edit" { - t.Errorf("expected relation to be one of %v got %v", []string{"privileged", "can_edit"}, tuple.Relation) - } + if tuple.Relation != "privileged" && tuple.Relation != "can_edit" { + t.Errorf("expected relation to be one of %v got %v", []string{"privileged", "can_edit"}, tuple.Relation) + } + + if tuple.Object != fmt.Sprintf("role:%s", test.input) && !strings.HasSuffix(tuple.Object, ":test") { + t.Errorf("expected object to be one of %v got %v", []string{fmt.Sprintf("role:%s", test.input), "<*>:test"}, tuple.Object) + } + case 2: + + for _, tuple := range tuples { + if tuple.User != "user:test" && tuple.User != "group:test#member" { + t.Errorf("expected user to be one of %v got %v", []string{"user:test", "group:test#member"}, tuple.User) + } + + if tuple.Relation != ASSIGNEE_RELATION { + t.Errorf("expected relation to be of %v got %v", ASSIGNEE_RELATION, tuple.Relation) + } - if tuple.Object != fmt.Sprintf("role:%s", test.input) && !strings.HasSuffix(tuple.Object, ":test") { - t.Errorf("expected object to be one of %v got %v", []string{fmt.Sprintf("role:%s", test.input), "<*>:test"}, tuple.Object) + if tuple.Object != fmt.Sprintf("role:%s", test.input) { + t.Errorf("expected object to be one of %v got %v", fmt.Sprintf("role:%s", test.input), tuple.Object) + } + } + + default: + t.Errorf("too many tuples") } return nil