From 258c8c81eff6ed15b878d57f161425baa1c417ea Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 6 Oct 2026 16:36:40 +0530 Subject: [PATCH 01/18] feat(store): list the live personal access tokens of an organization --- core/userpat/mocks/repository.go | 65 +++++++++++++++++-- core/userpat/userpat.go | 1 + internal/store/postgres/userpat_repository.go | 22 +++++++ .../store/postgres/userpat_repository_test.go | 48 ++++++++++++++ 4 files changed, 132 insertions(+), 4 deletions(-) diff --git a/core/userpat/mocks/repository.go b/core/userpat/mocks/repository.go index a9f9e43bd4..45984b714e 100644 --- a/core/userpat/mocks/repository.go +++ b/core/userpat/mocks/repository.go @@ -4,13 +4,11 @@ package mocks import ( context "context" + time "time" models "github.com/raystack/frontier/core/userpat/models" - mock "github.com/stretchr/testify/mock" - rql "github.com/raystack/salt/rql" - - time "time" + mock "github.com/stretchr/testify/mock" ) // Repository is an autogenerated mock type for the Repository type @@ -420,6 +418,65 @@ func (_c *Repository_List_Call) RunAndReturn(run func(context.Context, string, s return _c } +// ListByOrg provides a mock function with given fields: ctx, orgID +func (_m *Repository) ListByOrg(ctx context.Context, orgID string) ([]models.PAT, error) { + ret := _m.Called(ctx, orgID) + + if len(ret) == 0 { + panic("no return value specified for ListByOrg") + } + + var r0 []models.PAT + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) ([]models.PAT, error)); ok { + return rf(ctx, orgID) + } + if rf, ok := ret.Get(0).(func(context.Context, string) []models.PAT); ok { + r0 = rf(ctx, orgID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]models.PAT) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, orgID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Repository_ListByOrg_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListByOrg' +type Repository_ListByOrg_Call struct { + *mock.Call +} + +// ListByOrg is a helper method to define mock.On call +// - ctx context.Context +// - orgID string +func (_e *Repository_Expecter) ListByOrg(ctx interface{}, orgID interface{}) *Repository_ListByOrg_Call { + return &Repository_ListByOrg_Call{Call: _e.mock.On("ListByOrg", ctx, orgID)} +} + +func (_c *Repository_ListByOrg_Call) Run(run func(ctx context.Context, orgID string)) *Repository_ListByOrg_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *Repository_ListByOrg_Call) Return(_a0 []models.PAT, _a1 error) *Repository_ListByOrg_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *Repository_ListByOrg_Call) RunAndReturn(run func(context.Context, string) ([]models.PAT, error)) *Repository_ListByOrg_Call { + _c.Call.Return(run) + return _c +} + // ListByUser provides a mock function with given fields: ctx, userID func (_m *Repository) ListByUser(ctx context.Context, userID string) ([]models.PAT, error) { ret := _m.Called(ctx, userID) diff --git a/core/userpat/userpat.go b/core/userpat/userpat.go index d1236339b3..a4b9dfc643 100644 --- a/core/userpat/userpat.go +++ b/core/userpat/userpat.go @@ -14,6 +14,7 @@ type Repository interface { GetByID(ctx context.Context, id string) (models.PAT, error) List(ctx context.Context, userID, orgID string, query *rql.Query) (models.PATList, error) ListByUser(ctx context.Context, userID string) ([]models.PAT, error) + ListByOrg(ctx context.Context, orgID string) ([]models.PAT, error) GetBySecretHash(ctx context.Context, secretHash string) (models.PAT, error) IsTitleAvailable(ctx context.Context, userID, orgID, title string) (bool, error) UpdateUsedAt(ctx context.Context, id string, at time.Time) error diff --git a/internal/store/postgres/userpat_repository.go b/internal/store/postgres/userpat_repository.go index d3b94b16d5..360673cae7 100644 --- a/internal/store/postgres/userpat_repository.go +++ b/internal/store/postgres/userpat_repository.go @@ -437,6 +437,28 @@ func (r UserPATRepository) ListByUser(ctx context.Context, userID string) ([]mod return pats, nil } +func (r UserPATRepository) ListByOrg(ctx context.Context, orgID string) ([]models.PAT, error) { + query, params, err := fromLive(TABLE_USER_PATS).Where(goqu.Ex{"org_id": orgID}).ToSQL() + if err != nil { + return nil, fmt.Errorf("%w: %w", errQuery, err) + } + var rows []UserPAT + if err = r.dbc.WithTimeout(ctx, TABLE_USER_PATS, "ListByOrg", func(ctx context.Context) error { + return r.dbc.SelectContext(ctx, &rows, query, params...) + }); err != nil { + return nil, fmt.Errorf("%w: %w", errDB, err) + } + pats := make([]models.PAT, 0, len(rows)) + for _, m := range rows { + pat, err := m.transform() + if err != nil { + return nil, err + } + pats = append(pats, pat) + } + return pats, nil +} + func (r UserPATRepository) SetAlertSentMetadata(ctx context.Context, id string, key string) error { now := time.Now().UTC().Format(time.RFC3339) query, params, err := dialect.Update(TABLE_USER_PATS). diff --git a/internal/store/postgres/userpat_repository_test.go b/internal/store/postgres/userpat_repository_test.go index 32d0399449..f04390d95d 100644 --- a/internal/store/postgres/userpat_repository_test.go +++ b/internal/store/postgres/userpat_repository_test.go @@ -351,6 +351,54 @@ func (s *UserPATRepositoryTestSuite) TestListByUser_EmptyWhenNoTokens() { s.Empty(got) } +func (s *UserPATRepositoryTestSuite) TestListByOrg_ReturnsLivePATsOfTheOrg() { + s.truncateTokens() + + var liveInOrg0 []string + for _, spec := range []struct { + user, org, title, hash string + }{ + {s.users[0].ID, s.orgs[0].ID, "u0-o0-a", "hashU0O0A"}, + {s.users[1].ID, s.orgs[0].ID, "u1-o0-a", "hashU1O0A"}, + {s.users[0].ID, s.orgs[1].ID, "u0-o1-a", "hashU0O1A"}, + } { + pat, err := s.repository.Create(s.ctx, models.PAT{ + UserID: spec.user, + OrgID: spec.org, + Title: spec.title, + SecretHash: spec.hash, + ExpiresAt: time.Now().Add(24 * time.Hour), + }) + s.Require().NoError(err) + if spec.org == s.orgs[0].ID { + liveInOrg0 = append(liveInOrg0, pat.ID) + } + } + + deleted, err := s.repository.Create(s.ctx, models.PAT{ + UserID: s.users[0].ID, + OrgID: s.orgs[0].ID, + Title: "u0-o0-gone", + SecretHash: "hashU0O0Gone", + ExpiresAt: time.Now().Add(24 * time.Hour), + }) + s.Require().NoError(err) + s.Require().NoError(s.repository.Delete(s.ctx, deleted.ID)) + + got, err := s.repository.ListByOrg(s.ctx, s.orgs[0].ID) + s.Require().NoError(err) + gotIDs := make([]string, 0, len(got)) + for _, pat := range got { + s.Equal(s.orgs[0].ID, pat.OrgID) + gotIDs = append(gotIDs, pat.ID) + } + s.ElementsMatch(liveInOrg0, gotIDs) + + none, err := s.repository.ListByOrg(s.ctx, uuid.NewString()) + s.Require().NoError(err) + s.Empty(none) +} + func (s *UserPATRepositoryTestSuite) TestRegenerate() { s.Run("clears alert-sent keys but preserves other metadata", func() { s.truncateTokens() From f1f44cb0137f259a9be4ab303165721271960487 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Fri, 9 Oct 2026 15:52:37 +0530 Subject: [PATCH 02/18] feat(userpat): delete every personal access token of an organization --- core/userpat/service.go | 22 ++++++-- core/userpat/service_test.go | 99 ++++++++++++++++++++++++++++++++++-- 2 files changed, 115 insertions(+), 6 deletions(-) diff --git a/core/userpat/service.go b/core/userpat/service.go index ebb0e48c94..80c354bfa9 100644 --- a/core/userpat/service.go +++ b/core/userpat/service.go @@ -150,17 +150,20 @@ func (s *Service) Delete(ctx context.Context, userID, id string) error { if pat.UserID != userID { return paterrors.ErrNotFound } + return s.softDeleteThenRemovePolicies(ctx, pat) +} - if err := s.repo.Delete(ctx, id); err != nil { +func (s *Service) softDeleteThenRemovePolicies(ctx context.Context, pat patmodels.PAT) error { + if err := s.repo.Delete(ctx, pat.ID); err != nil { return fmt.Errorf("soft deleting PAT: %w", err) } - if err := s.membershipService.RemoveAllPATPolicies(ctx, id); err != nil { + if err := s.membershipService.RemoveAllPATPolicies(ctx, pat.ID); err != nil { return fmt.Errorf("deleting policies: %w", err) } if err := s.createAuditRecord(ctx, pkgAuditRecord.PATRevokedEvent, pat, time.Now().UTC(), nil); err != nil { - s.logger.ErrorContext(ctx, "failed to create audit record for PAT revocation", "pat_id", id, "error", err) + s.logger.ErrorContext(ctx, "failed to create audit record for PAT revocation", "pat_id", pat.ID, "error", err) } return nil @@ -180,6 +183,19 @@ func (s *Service) DeleteAllByUser(ctx context.Context, userID string) error { return nil } +func (s *Service) DeleteAllByOrg(ctx context.Context, orgID string) error { + pats, err := s.repo.ListByOrg(ctx, orgID) + if err != nil { + return fmt.Errorf("listing PATs for org: %w", err) + } + for _, pat := range pats { + if err := s.softDeleteThenRemovePolicies(ctx, pat); err != nil { + return fmt.Errorf("deleting PAT[%s]: %w", pat.ID, err) + } + } + return nil +} + // Regenerate creates a new secret and updates the expiry for an existing PAT. // Scope (roles + projects) and policies are preserved. Expired PATs can be // regenerated; if reviving one, checks the active count limit. diff --git a/core/userpat/service_test.go b/core/userpat/service_test.go index 993bb26c07..6a97308d10 100644 --- a/core/userpat/service_test.go +++ b/core/userpat/service_test.go @@ -1811,14 +1811,15 @@ func TestService_Delete(t *testing.T) { Return(testPAT, nil) repo.EXPECT().Delete(mock.Anything, "pat-1"). Return(errors.New("db error")) + membershipSvc := mocks.NewMembershipService(t) orgSvc := mocks.NewOrganizationService(t) auditRepo := mocks.NewAuditRecordRepository(t) - return userpat.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), repo, defaultConfig, orgSvc, nil, nil, nil, auditRepo) + return userpat.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), repo, defaultConfig, orgSvc, nil, membershipSvc, nil, auditRepo) }, wantErr: true, }, { - name: "should return error when policy list fails after soft-delete", + name: "should return error when policy removal fails after the PAT is marked deleted", userID: "user-1", patID: "pat-1", setup: func() *userpat.Service { @@ -1957,15 +1958,107 @@ func TestService_DeleteAllByUser(t *testing.T) { repo.EXPECT().ListByUser(mock.Anything, "user-1").Return(pats, nil) repo.EXPECT().GetByID(mock.Anything, "pat-bad").Return(pats[0], nil) repo.EXPECT().Delete(mock.Anything, "pat-bad").Return(errors.New("delete boom")) + membershipSvc := mocks.NewMembershipService(t) svc := userpat.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), repo, defaultConfig, - mocks.NewOrganizationService(t), nil, mocks.NewMembershipService(t), nil, mocks.NewAuditRecordRepository(t)) + mocks.NewOrganizationService(t), nil, membershipSvc, nil, mocks.NewAuditRecordRepository(t)) err := svc.DeleteAllByUser(context.Background(), "user-1") assert.ErrorContains(t, err, "deleting PAT[pat-bad]") assert.ErrorContains(t, err, "delete boom") }) } +func TestService_DeleteAllByOrg(t *testing.T) { + newService := func(t *testing.T) (*userpat.Service, *mocks.Repository, *mocks.MembershipService, *mocks.AuditRecordRepository) { + t.Helper() + repo := mocks.NewRepository(t) + membershipSvc := mocks.NewMembershipService(t) + orgSvc := mocks.NewOrganizationService(t) + orgSvc.On("GetRaw", mock.Anything, mock.Anything). + Return(organization.Organization{ID: "org-1", Title: "Test Org"}, nil).Maybe() + auditRepo := mocks.NewAuditRecordRepository(t) + svc := userpat.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), repo, defaultConfig, + orgSvc, nil, membershipSvc, nil, auditRepo) + return svc, repo, membershipSvc, auditRepo + } + + t.Run("no-op when the org has no PATs", func(t *testing.T) { + svc, repo, _, _ := newService(t) + repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(nil, nil) + assert.NoError(t, svc.DeleteAllByOrg(context.Background(), "org-1")) + }) + + t.Run("returns error when the org list fails", func(t *testing.T) { + svc, repo, _, _ := newService(t) + repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(nil, errors.New("db down")) + err := svc.DeleteAllByOrg(context.Background(), "org-1") + assert.ErrorContains(t, err, "listing PATs for org") + assert.ErrorContains(t, err, "db down") + }) + + t.Run("marks each PAT deleted, then removes its policies, then records the revocation", func(t *testing.T) { + svc, repo, membershipSvc, auditRepo := newService(t) + pats := []models.PAT{ + {ID: "pat-1", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, + {ID: "pat-2", UserID: "user-2", OrgID: "org-1", Title: "t2", ExpiresAt: time.Now().Add(time.Hour)}, + } + repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(pats, nil) + + var calls []string + for _, pat := range pats { + membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, pat.ID). + Run(func(_ context.Context, patID string) { calls = append(calls, "policies:"+patID) }). + Return(nil) + repo.EXPECT().Delete(mock.Anything, pat.ID). + Run(func(_ context.Context, patID string) { calls = append(calls, "row:"+patID) }). + Return(nil) + } + auditRepo.EXPECT().Create(mock.Anything, mock.Anything). + Run(func(_ context.Context, record auditmodels.AuditRecord) { + calls = append(calls, "audit:"+record.Target.ID) + }). + Return(auditmodels.AuditRecord{}, nil). + Times(2) + + assert.NoError(t, svc.DeleteAllByOrg(context.Background(), "org-1")) + assert.Equal(t, []string{ + "row:pat-1", "policies:pat-1", "audit:pat-1", + "row:pat-2", "policies:pat-2", "audit:pat-2", + }, calls) + }) + + t.Run("stops at the first PAT that cannot be marked deleted and removes no policies", func(t *testing.T) { + svc, repo, _, _ := newService(t) + pats := []models.PAT{ + {ID: "pat-bad", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, + {ID: "pat-2", UserID: "user-1", OrgID: "org-1", Title: "t2", ExpiresAt: time.Now().Add(time.Hour)}, + } + repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(pats, nil) + repo.EXPECT().Delete(mock.Anything, "pat-bad").Return(errors.New("db down")) + + err := svc.DeleteAllByOrg(context.Background(), "org-1") + assert.ErrorContains(t, err, "deleting PAT[pat-bad]") + assert.ErrorContains(t, err, "soft deleting PAT") + assert.ErrorContains(t, err, "db down") + }) + + t.Run("stops at the first PAT whose policy removal fails and writes no audit record", func(t *testing.T) { + svc, repo, membershipSvc, _ := newService(t) + pats := []models.PAT{ + {ID: "pat-bad", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, + {ID: "pat-2", UserID: "user-1", OrgID: "org-1", Title: "t2", ExpiresAt: time.Now().Add(time.Hour)}, + } + repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(pats, nil) + repo.EXPECT().Delete(mock.Anything, "pat-bad").Return(nil) + membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-bad").Return(errors.New("spicedb down")) + + err := svc.DeleteAllByOrg(context.Background(), "org-1") + assert.ErrorContains(t, err, "deleting PAT[pat-bad]") + assert.ErrorContains(t, err, "deleting policies") + assert.ErrorContains(t, err, "spicedb down") + }) +} + func TestService_Update(t *testing.T) { testPAT := models.PAT{ ID: "pat-1", From 56aa7250e94c42c4741b4e5e0c607585edc878ea Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 12 Oct 2026 15:40:37 +0530 Subject: [PATCH 03/18] fix(userpat): org delete skips a token that was already revoked --- core/userpat/service.go | 4 ++++ core/userpat/service_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/core/userpat/service.go b/core/userpat/service.go index 80c354bfa9..0805101f55 100644 --- a/core/userpat/service.go +++ b/core/userpat/service.go @@ -6,6 +6,7 @@ import ( "crypto/sha3" "encoding/base64" "encoding/hex" + "errors" "fmt" "io" "log/slog" @@ -190,6 +191,9 @@ func (s *Service) DeleteAllByOrg(ctx context.Context, orgID string) error { } for _, pat := range pats { if err := s.softDeleteThenRemovePolicies(ctx, pat); err != nil { + if errors.Is(err, paterrors.ErrNotFound) { + continue + } return fmt.Errorf("deleting PAT[%s]: %w", pat.ID, err) } } diff --git a/core/userpat/service_test.go b/core/userpat/service_test.go index 6a97308d10..cd3229d1f0 100644 --- a/core/userpat/service_test.go +++ b/core/userpat/service_test.go @@ -2057,6 +2057,34 @@ func TestService_DeleteAllByOrg(t *testing.T) { assert.ErrorContains(t, err, "deleting policies") assert.ErrorContains(t, err, "spicedb down") }) + + t.Run("skips a PAT that was already revoked and still deletes the next one", func(t *testing.T) { + svc, repo, membershipSvc, auditRepo := newService(t) + pats := []models.PAT{ + {ID: "pat-gone", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, + {ID: "pat-2", UserID: "user-2", OrgID: "org-1", Title: "t2", ExpiresAt: time.Now().Add(time.Hour)}, + } + repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(pats, nil) + repo.EXPECT().Delete(mock.Anything, "pat-gone").Return(paterrors.ErrNotFound) + repo.EXPECT().Delete(mock.Anything, "pat-2").Return(nil) + membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-2").Return(nil) + auditRepo.EXPECT().Create(mock.Anything, mock.Anything). + Return(auditmodels.AuditRecord{}, nil). + Once() + + assert.NoError(t, svc.DeleteAllByOrg(context.Background(), "org-1")) + }) + + t.Run("a skipped PAT removes no policies and writes no audit record", func(t *testing.T) { + svc, repo, _, _ := newService(t) + pats := []models.PAT{ + {ID: "pat-gone", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, + } + repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(pats, nil) + repo.EXPECT().Delete(mock.Anything, "pat-gone").Return(paterrors.ErrNotFound) + + assert.NoError(t, svc.DeleteAllByOrg(context.Background(), "org-1")) + }) } func TestService_Update(t *testing.T) { From 3e1b59d4e146b8b3cef2296a2fc026a449fe6251 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 12 Oct 2026 15:45:10 +0530 Subject: [PATCH 04/18] feat(userpat): list deleted tokens of an org too --- core/userpat/mocks/repository.go | 22 +++++++++---------- core/userpat/models/pat.go | 1 + core/userpat/service.go | 2 +- core/userpat/service_test.go | 14 ++++++------ core/userpat/userpat.go | 2 +- internal/store/postgres/userpat.go | 1 + internal/store/postgres/userpat_repository.go | 6 ++--- .../store/postgres/userpat_repository_test.go | 14 +++++++----- 8 files changed, 33 insertions(+), 29 deletions(-) diff --git a/core/userpat/mocks/repository.go b/core/userpat/mocks/repository.go index 45984b714e..3faadf55e5 100644 --- a/core/userpat/mocks/repository.go +++ b/core/userpat/mocks/repository.go @@ -418,12 +418,12 @@ func (_c *Repository_List_Call) RunAndReturn(run func(context.Context, string, s return _c } -// ListByOrg provides a mock function with given fields: ctx, orgID -func (_m *Repository) ListByOrg(ctx context.Context, orgID string) ([]models.PAT, error) { +// ListAllByOrg provides a mock function with given fields: ctx, orgID +func (_m *Repository) ListAllByOrg(ctx context.Context, orgID string) ([]models.PAT, error) { ret := _m.Called(ctx, orgID) if len(ret) == 0 { - panic("no return value specified for ListByOrg") + panic("no return value specified for ListAllByOrg") } var r0 []models.PAT @@ -448,31 +448,31 @@ func (_m *Repository) ListByOrg(ctx context.Context, orgID string) ([]models.PAT return r0, r1 } -// Repository_ListByOrg_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListByOrg' -type Repository_ListByOrg_Call struct { +// Repository_ListAllByOrg_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListAllByOrg' +type Repository_ListAllByOrg_Call struct { *mock.Call } -// ListByOrg is a helper method to define mock.On call +// ListAllByOrg is a helper method to define mock.On call // - ctx context.Context // - orgID string -func (_e *Repository_Expecter) ListByOrg(ctx interface{}, orgID interface{}) *Repository_ListByOrg_Call { - return &Repository_ListByOrg_Call{Call: _e.mock.On("ListByOrg", ctx, orgID)} +func (_e *Repository_Expecter) ListAllByOrg(ctx interface{}, orgID interface{}) *Repository_ListAllByOrg_Call { + return &Repository_ListAllByOrg_Call{Call: _e.mock.On("ListAllByOrg", ctx, orgID)} } -func (_c *Repository_ListByOrg_Call) Run(run func(ctx context.Context, orgID string)) *Repository_ListByOrg_Call { +func (_c *Repository_ListAllByOrg_Call) Run(run func(ctx context.Context, orgID string)) *Repository_ListAllByOrg_Call { _c.Call.Run(func(args mock.Arguments) { run(args[0].(context.Context), args[1].(string)) }) return _c } -func (_c *Repository_ListByOrg_Call) Return(_a0 []models.PAT, _a1 error) *Repository_ListByOrg_Call { +func (_c *Repository_ListAllByOrg_Call) Return(_a0 []models.PAT, _a1 error) *Repository_ListAllByOrg_Call { _c.Call.Return(_a0, _a1) return _c } -func (_c *Repository_ListByOrg_Call) RunAndReturn(run func(context.Context, string) ([]models.PAT, error)) *Repository_ListByOrg_Call { +func (_c *Repository_ListAllByOrg_Call) RunAndReturn(run func(context.Context, string) ([]models.PAT, error)) *Repository_ListAllByOrg_Call { _c.Call.Return(run) return _c } diff --git a/core/userpat/models/pat.go b/core/userpat/models/pat.go index cc44c061d9..1e10aeb6ee 100644 --- a/core/userpat/models/pat.go +++ b/core/userpat/models/pat.go @@ -27,6 +27,7 @@ type PAT struct { ExpiresAt time.Time `rql:"name=expires_at,type=datetime"` CreatedAt time.Time `rql:"name=created_at,type=datetime"` UpdatedAt time.Time `rql:"name=updated_at,type=datetime"` + DeletedAt *time.Time } type PATList struct { diff --git a/core/userpat/service.go b/core/userpat/service.go index 0805101f55..7139d2b518 100644 --- a/core/userpat/service.go +++ b/core/userpat/service.go @@ -185,7 +185,7 @@ func (s *Service) DeleteAllByUser(ctx context.Context, userID string) error { } func (s *Service) DeleteAllByOrg(ctx context.Context, orgID string) error { - pats, err := s.repo.ListByOrg(ctx, orgID) + pats, err := s.repo.ListAllByOrg(ctx, orgID) if err != nil { return fmt.Errorf("listing PATs for org: %w", err) } diff --git a/core/userpat/service_test.go b/core/userpat/service_test.go index cd3229d1f0..b041acaf1e 100644 --- a/core/userpat/service_test.go +++ b/core/userpat/service_test.go @@ -1984,13 +1984,13 @@ func TestService_DeleteAllByOrg(t *testing.T) { t.Run("no-op when the org has no PATs", func(t *testing.T) { svc, repo, _, _ := newService(t) - repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(nil, nil) + repo.EXPECT().ListAllByOrg(mock.Anything, "org-1").Return(nil, nil) assert.NoError(t, svc.DeleteAllByOrg(context.Background(), "org-1")) }) t.Run("returns error when the org list fails", func(t *testing.T) { svc, repo, _, _ := newService(t) - repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(nil, errors.New("db down")) + repo.EXPECT().ListAllByOrg(mock.Anything, "org-1").Return(nil, errors.New("db down")) err := svc.DeleteAllByOrg(context.Background(), "org-1") assert.ErrorContains(t, err, "listing PATs for org") assert.ErrorContains(t, err, "db down") @@ -2002,7 +2002,7 @@ func TestService_DeleteAllByOrg(t *testing.T) { {ID: "pat-1", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, {ID: "pat-2", UserID: "user-2", OrgID: "org-1", Title: "t2", ExpiresAt: time.Now().Add(time.Hour)}, } - repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(pats, nil) + repo.EXPECT().ListAllByOrg(mock.Anything, "org-1").Return(pats, nil) var calls []string for _, pat := range pats { @@ -2033,7 +2033,7 @@ func TestService_DeleteAllByOrg(t *testing.T) { {ID: "pat-bad", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, {ID: "pat-2", UserID: "user-1", OrgID: "org-1", Title: "t2", ExpiresAt: time.Now().Add(time.Hour)}, } - repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(pats, nil) + repo.EXPECT().ListAllByOrg(mock.Anything, "org-1").Return(pats, nil) repo.EXPECT().Delete(mock.Anything, "pat-bad").Return(errors.New("db down")) err := svc.DeleteAllByOrg(context.Background(), "org-1") @@ -2048,7 +2048,7 @@ func TestService_DeleteAllByOrg(t *testing.T) { {ID: "pat-bad", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, {ID: "pat-2", UserID: "user-1", OrgID: "org-1", Title: "t2", ExpiresAt: time.Now().Add(time.Hour)}, } - repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(pats, nil) + repo.EXPECT().ListAllByOrg(mock.Anything, "org-1").Return(pats, nil) repo.EXPECT().Delete(mock.Anything, "pat-bad").Return(nil) membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-bad").Return(errors.New("spicedb down")) @@ -2064,7 +2064,7 @@ func TestService_DeleteAllByOrg(t *testing.T) { {ID: "pat-gone", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, {ID: "pat-2", UserID: "user-2", OrgID: "org-1", Title: "t2", ExpiresAt: time.Now().Add(time.Hour)}, } - repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(pats, nil) + repo.EXPECT().ListAllByOrg(mock.Anything, "org-1").Return(pats, nil) repo.EXPECT().Delete(mock.Anything, "pat-gone").Return(paterrors.ErrNotFound) repo.EXPECT().Delete(mock.Anything, "pat-2").Return(nil) membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-2").Return(nil) @@ -2080,7 +2080,7 @@ func TestService_DeleteAllByOrg(t *testing.T) { pats := []models.PAT{ {ID: "pat-gone", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, } - repo.EXPECT().ListByOrg(mock.Anything, "org-1").Return(pats, nil) + repo.EXPECT().ListAllByOrg(mock.Anything, "org-1").Return(pats, nil) repo.EXPECT().Delete(mock.Anything, "pat-gone").Return(paterrors.ErrNotFound) assert.NoError(t, svc.DeleteAllByOrg(context.Background(), "org-1")) diff --git a/core/userpat/userpat.go b/core/userpat/userpat.go index a4b9dfc643..201999925d 100644 --- a/core/userpat/userpat.go +++ b/core/userpat/userpat.go @@ -14,7 +14,7 @@ type Repository interface { GetByID(ctx context.Context, id string) (models.PAT, error) List(ctx context.Context, userID, orgID string, query *rql.Query) (models.PATList, error) ListByUser(ctx context.Context, userID string) ([]models.PAT, error) - ListByOrg(ctx context.Context, orgID string) ([]models.PAT, error) + ListAllByOrg(ctx context.Context, orgID string) ([]models.PAT, error) GetBySecretHash(ctx context.Context, secretHash string) (models.PAT, error) IsTitleAvailable(ctx context.Context, userID, orgID, title string) (bool, error) UpdateUsedAt(ctx context.Context, id string, at time.Time) error diff --git a/internal/store/postgres/userpat.go b/internal/store/postgres/userpat.go index b2fa7655ca..2114098282 100644 --- a/internal/store/postgres/userpat.go +++ b/internal/store/postgres/userpat.go @@ -41,5 +41,6 @@ func (t UserPAT) transform() (models.PAT, error) { ExpiresAt: t.ExpiresAt, CreatedAt: t.CreatedAt, UpdatedAt: t.UpdatedAt, + DeletedAt: t.DeletedAt, }, nil } diff --git a/internal/store/postgres/userpat_repository.go b/internal/store/postgres/userpat_repository.go index 360673cae7..4d01c5fe29 100644 --- a/internal/store/postgres/userpat_repository.go +++ b/internal/store/postgres/userpat_repository.go @@ -437,13 +437,13 @@ func (r UserPATRepository) ListByUser(ctx context.Context, userID string) ([]mod return pats, nil } -func (r UserPATRepository) ListByOrg(ctx context.Context, orgID string) ([]models.PAT, error) { - query, params, err := fromLive(TABLE_USER_PATS).Where(goqu.Ex{"org_id": orgID}).ToSQL() +func (r UserPATRepository) ListAllByOrg(ctx context.Context, orgID string) ([]models.PAT, error) { + query, params, err := dialect.From(TABLE_USER_PATS).Where(goqu.Ex{"org_id": orgID}).ToSQL() if err != nil { return nil, fmt.Errorf("%w: %w", errQuery, err) } var rows []UserPAT - if err = r.dbc.WithTimeout(ctx, TABLE_USER_PATS, "ListByOrg", func(ctx context.Context) error { + if err = r.dbc.WithTimeout(ctx, TABLE_USER_PATS, "ListAllByOrg", func(ctx context.Context) error { return r.dbc.SelectContext(ctx, &rows, query, params...) }); err != nil { return nil, fmt.Errorf("%w: %w", errDB, err) diff --git a/internal/store/postgres/userpat_repository_test.go b/internal/store/postgres/userpat_repository_test.go index f04390d95d..72a29be5e6 100644 --- a/internal/store/postgres/userpat_repository_test.go +++ b/internal/store/postgres/userpat_repository_test.go @@ -351,10 +351,10 @@ func (s *UserPATRepositoryTestSuite) TestListByUser_EmptyWhenNoTokens() { s.Empty(got) } -func (s *UserPATRepositoryTestSuite) TestListByOrg_ReturnsLivePATsOfTheOrg() { +func (s *UserPATRepositoryTestSuite) TestListAllByOrg_ReturnsLiveAndDeletedPATsOfTheOrg() { s.truncateTokens() - var liveInOrg0 []string + var idsInOrg0 []string for _, spec := range []struct { user, org, title, hash string }{ @@ -371,7 +371,7 @@ func (s *UserPATRepositoryTestSuite) TestListByOrg_ReturnsLivePATsOfTheOrg() { }) s.Require().NoError(err) if spec.org == s.orgs[0].ID { - liveInOrg0 = append(liveInOrg0, pat.ID) + idsInOrg0 = append(idsInOrg0, pat.ID) } } @@ -384,17 +384,19 @@ func (s *UserPATRepositoryTestSuite) TestListByOrg_ReturnsLivePATsOfTheOrg() { }) s.Require().NoError(err) s.Require().NoError(s.repository.Delete(s.ctx, deleted.ID)) + idsInOrg0 = append(idsInOrg0, deleted.ID) - got, err := s.repository.ListByOrg(s.ctx, s.orgs[0].ID) + got, err := s.repository.ListAllByOrg(s.ctx, s.orgs[0].ID) s.Require().NoError(err) gotIDs := make([]string, 0, len(got)) for _, pat := range got { s.Equal(s.orgs[0].ID, pat.OrgID) gotIDs = append(gotIDs, pat.ID) + s.Equal(pat.ID == deleted.ID, pat.DeletedAt != nil) } - s.ElementsMatch(liveInOrg0, gotIDs) + s.ElementsMatch(idsInOrg0, gotIDs) - none, err := s.repository.ListByOrg(s.ctx, uuid.NewString()) + none, err := s.repository.ListAllByOrg(s.ctx, uuid.NewString()) s.Require().NoError(err) s.Empty(none) } From fd742c0dffdd9c3c1817f91d9fcaf1a32f2a7edf Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 12 Oct 2026 15:47:01 +0530 Subject: [PATCH 05/18] fix(userpat): org delete removes the policies of tokens that are already deleted --- core/userpat/service.go | 30 +++++++++++-- core/userpat/service_test.go | 81 ++++++++++++++++++++++++++++++++---- 2 files changed, 98 insertions(+), 13 deletions(-) diff --git a/core/userpat/service.go b/core/userpat/service.go index 7139d2b518..d2ba2033e2 100644 --- a/core/userpat/service.go +++ b/core/userpat/service.go @@ -190,16 +190,38 @@ func (s *Service) DeleteAllByOrg(ctx context.Context, orgID string) error { return fmt.Errorf("listing PATs for org: %w", err) } for _, pat := range pats { - if err := s.softDeleteThenRemovePolicies(ctx, pat); err != nil { - if errors.Is(err, paterrors.ErrNotFound) { - continue - } + if err := s.revokeOrgPAT(ctx, pat); err != nil { return fmt.Errorf("deleting PAT[%s]: %w", pat.ID, err) } } return nil } +func (s *Service) revokeOrgPAT(ctx context.Context, pat patmodels.PAT) error { + softDeleted := false + if pat.DeletedAt == nil { + err := s.repo.Delete(ctx, pat.ID) + switch { + case err == nil: + softDeleted = true + case errors.Is(err, paterrors.ErrNotFound): + default: + return fmt.Errorf("soft deleting PAT: %w", err) + } + } + + if err := s.membershipService.RemoveAllPATPolicies(ctx, pat.ID); err != nil { + return fmt.Errorf("deleting policies: %w", err) + } + + if softDeleted { + if err := s.createAuditRecord(ctx, pkgAuditRecord.PATRevokedEvent, pat, time.Now().UTC(), nil); err != nil { + s.logger.ErrorContext(ctx, "failed to create audit record for PAT revocation", "pat_id", pat.ID, "error", err) + } + } + return nil +} + // Regenerate creates a new secret and updates the expiry for an existing PAT. // Scope (roles + projects) and policies are preserved. Expired PATs can be // regenerated; if reviving one, checks the active count limit. diff --git a/core/userpat/service_test.go b/core/userpat/service_test.go index b041acaf1e..17fdcda316 100644 --- a/core/userpat/service_test.go +++ b/core/userpat/service_test.go @@ -2058,16 +2058,41 @@ func TestService_DeleteAllByOrg(t *testing.T) { assert.ErrorContains(t, err, "spicedb down") }) - t.Run("skips a PAT that was already revoked and still deletes the next one", func(t *testing.T) { - svc, repo, membershipSvc, auditRepo := newService(t) + t.Run("still removes the policies of a PAT that is already deleted, without a delete or audit record", func(t *testing.T) { + svc, repo, membershipSvc, _ := newService(t) + deletedAt := time.Now().Add(-time.Hour) + pats := []models.PAT{ + {ID: "pat-old", UserID: "user-1", OrgID: "org-1", Title: "t1", DeletedAt: &deletedAt}, + } + repo.EXPECT().ListAllByOrg(mock.Anything, "org-1").Return(pats, nil) + membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-old").Return(nil).Once() + + assert.NoError(t, svc.DeleteAllByOrg(context.Background(), "org-1")) + }) + + t.Run("still removes the policies when the delete finds the PAT revoked meanwhile, with no audit record", func(t *testing.T) { + svc, repo, membershipSvc, _ := newService(t) pats := []models.PAT{ {ID: "pat-gone", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, - {ID: "pat-2", UserID: "user-2", OrgID: "org-1", Title: "t2", ExpiresAt: time.Now().Add(time.Hour)}, } repo.EXPECT().ListAllByOrg(mock.Anything, "org-1").Return(pats, nil) repo.EXPECT().Delete(mock.Anything, "pat-gone").Return(paterrors.ErrNotFound) + membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-gone").Return(nil).Once() + + assert.NoError(t, svc.DeleteAllByOrg(context.Background(), "org-1")) + }) + + t.Run("continues with the next PAT after one that was already deleted", func(t *testing.T) { + svc, repo, membershipSvc, auditRepo := newService(t) + deletedAt := time.Now().Add(-time.Hour) + pats := []models.PAT{ + {ID: "pat-old", UserID: "user-1", OrgID: "org-1", Title: "t1", DeletedAt: &deletedAt}, + {ID: "pat-2", UserID: "user-2", OrgID: "org-1", Title: "t2", ExpiresAt: time.Now().Add(time.Hour)}, + } + repo.EXPECT().ListAllByOrg(mock.Anything, "org-1").Return(pats, nil) repo.EXPECT().Delete(mock.Anything, "pat-2").Return(nil) - membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-2").Return(nil) + membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-old").Return(nil).Once() + membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-2").Return(nil).Once() auditRepo.EXPECT().Create(mock.Anything, mock.Anything). Return(auditmodels.AuditRecord{}, nil). Once() @@ -2075,15 +2100,53 @@ func TestService_DeleteAllByOrg(t *testing.T) { assert.NoError(t, svc.DeleteAllByOrg(context.Background(), "org-1")) }) - t.Run("a skipped PAT removes no policies and writes no audit record", func(t *testing.T) { - svc, repo, _, _ := newService(t) + t.Run("asks the repo for the exact org and removes policies only for the ids it returned", func(t *testing.T) { + svc, repo, membershipSvc, auditRepo := newService(t) + deletedAt := time.Now().Add(-time.Hour) pats := []models.PAT{ - {ID: "pat-gone", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, + {ID: "pat-live", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, + {ID: "pat-old", UserID: "user-2", OrgID: "org-1", Title: "t2", DeletedAt: &deletedAt}, } - repo.EXPECT().ListAllByOrg(mock.Anything, "org-1").Return(pats, nil) - repo.EXPECT().Delete(mock.Anything, "pat-gone").Return(paterrors.ErrNotFound) + repo.EXPECT().ListAllByOrg(mock.Anything, "org-1").Return(pats, nil).Once() + repo.EXPECT().Delete(mock.Anything, "pat-live").Return(nil).Once() + membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-live").Return(nil).Once() + membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-old").Return(nil).Once() + auditRepo.EXPECT().Create(mock.Anything, mock.Anything). + Return(auditmodels.AuditRecord{}, nil). + Once() assert.NoError(t, svc.DeleteAllByOrg(context.Background(), "org-1")) + membershipSvc.AssertNumberOfCalls(t, "RemoveAllPATPolicies", 2) + repo.AssertNumberOfCalls(t, "ListAllByOrg", 1) + }) + + t.Run("public Delete refuses a PAT of another user and removes no policies", func(t *testing.T) { + repo := mocks.NewRepository(t) + membershipSvc := mocks.NewMembershipService(t) + auditRepo := mocks.NewAuditRecordRepository(t) + svc := userpat.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), repo, defaultConfig, + nil, nil, membershipSvc, nil, auditRepo) + repo.EXPECT().GetByID(mock.Anything, "pat-1"). + Return(models.PAT{ID: "pat-1", UserID: "user-2", OrgID: "org-1"}, nil) + + err := svc.Delete(context.Background(), "user-1", "pat-1") + assert.ErrorIs(t, err, paterrors.ErrNotFound) + membershipSvc.AssertNotCalled(t, "RemoveAllPATPolicies", mock.Anything, mock.Anything) + repo.AssertNotCalled(t, "Delete", mock.Anything, mock.Anything) + }) + + t.Run("public Delete of a missing PAT returns not found and removes no policies", func(t *testing.T) { + repo := mocks.NewRepository(t) + membershipSvc := mocks.NewMembershipService(t) + auditRepo := mocks.NewAuditRecordRepository(t) + svc := userpat.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), repo, defaultConfig, + nil, nil, membershipSvc, nil, auditRepo) + repo.EXPECT().GetByID(mock.Anything, "pat-x").Return(models.PAT{}, paterrors.ErrNotFound) + + err := svc.Delete(context.Background(), "user-1", "pat-x") + assert.ErrorIs(t, err, paterrors.ErrNotFound) + membershipSvc.AssertNotCalled(t, "RemoveAllPATPolicies", mock.Anything, mock.Anything) + repo.AssertNotCalled(t, "Delete", mock.Anything, mock.Anything) }) } From cd8b5250077e590c679c6aae078871be877484d7 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 6 Oct 2026 16:41:21 +0530 Subject: [PATCH 06/18] feat(deleter): soft-delete the org's domains and tokens before the org row --- cmd/serve.go | 2 +- core/deleter/mocks/domain_service.go | 143 +++++++++++++++++++++++++ core/deleter/mocks/user_pat_service.go | 47 ++++++++ core/deleter/service.go | 25 ++++- core/deleter/service_test.go | 130 +++++++++++++++++++++- 5 files changed, 344 insertions(+), 3 deletions(-) create mode 100644 core/deleter/mocks/domain_service.go diff --git a/cmd/serve.go b/cmd/serve.go index 1535dc2120..d82a03fa63 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -597,7 +597,7 @@ func buildAPIDependencies( cascadeDeleter := deleter.NewCascadeDeleter(organizationService, projectService, resourceService, groupService, membershipService, policyService, roleService, invitationService, userService, userPATService, - serviceUserService, customerService, subscriptionService, invoiceService, checkoutService, + domainService, serviceUserService, customerService, subscriptionService, invoiceService, checkoutService, creditService, orgKycService, planService, mailDialer, cfg.Billing.OrgDeleteNotice, ) diff --git a/core/deleter/mocks/domain_service.go b/core/deleter/mocks/domain_service.go new file mode 100644 index 0000000000..257f6d392b --- /dev/null +++ b/core/deleter/mocks/domain_service.go @@ -0,0 +1,143 @@ +// Code generated by mockery v2.53.5. DO NOT EDIT. + +package mocks + +import ( + context "context" + + domain "github.com/raystack/frontier/core/domain" + mock "github.com/stretchr/testify/mock" +) + +// DomainService is an autogenerated mock type for the DomainService type +type DomainService struct { + mock.Mock +} + +type DomainService_Expecter struct { + mock *mock.Mock +} + +func (_m *DomainService) EXPECT() *DomainService_Expecter { + return &DomainService_Expecter{mock: &_m.Mock} +} + +// Delete provides a mock function with given fields: ctx, id +func (_m *DomainService) Delete(ctx context.Context, id string) error { + ret := _m.Called(ctx, id) + + if len(ret) == 0 { + panic("no return value specified for Delete") + } + + 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 +} + +// DomainService_Delete_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Delete' +type DomainService_Delete_Call struct { + *mock.Call +} + +// Delete is a helper method to define mock.On call +// - ctx context.Context +// - id string +func (_e *DomainService_Expecter) Delete(ctx interface{}, id interface{}) *DomainService_Delete_Call { + return &DomainService_Delete_Call{Call: _e.mock.On("Delete", ctx, id)} +} + +func (_c *DomainService_Delete_Call) Run(run func(ctx context.Context, id string)) *DomainService_Delete_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *DomainService_Delete_Call) Return(_a0 error) *DomainService_Delete_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *DomainService_Delete_Call) RunAndReturn(run func(context.Context, string) error) *DomainService_Delete_Call { + _c.Call.Return(run) + return _c +} + +// List provides a mock function with given fields: ctx, flt +func (_m *DomainService) List(ctx context.Context, flt domain.Filter) ([]domain.Domain, error) { + ret := _m.Called(ctx, flt) + + if len(ret) == 0 { + panic("no return value specified for List") + } + + var r0 []domain.Domain + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, domain.Filter) ([]domain.Domain, error)); ok { + return rf(ctx, flt) + } + if rf, ok := ret.Get(0).(func(context.Context, domain.Filter) []domain.Domain); ok { + r0 = rf(ctx, flt) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]domain.Domain) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, domain.Filter) error); ok { + r1 = rf(ctx, flt) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// DomainService_List_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'List' +type DomainService_List_Call struct { + *mock.Call +} + +// List is a helper method to define mock.On call +// - ctx context.Context +// - flt domain.Filter +func (_e *DomainService_Expecter) List(ctx interface{}, flt interface{}) *DomainService_List_Call { + return &DomainService_List_Call{Call: _e.mock.On("List", ctx, flt)} +} + +func (_c *DomainService_List_Call) Run(run func(ctx context.Context, flt domain.Filter)) *DomainService_List_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(domain.Filter)) + }) + return _c +} + +func (_c *DomainService_List_Call) Return(_a0 []domain.Domain, _a1 error) *DomainService_List_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *DomainService_List_Call) RunAndReturn(run func(context.Context, domain.Filter) ([]domain.Domain, error)) *DomainService_List_Call { + _c.Call.Return(run) + return _c +} + +// NewDomainService creates a new instance of DomainService. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewDomainService(t interface { + mock.TestingT + Cleanup(func()) +}) *DomainService { + mock := &DomainService{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/core/deleter/mocks/user_pat_service.go b/core/deleter/mocks/user_pat_service.go index eb38761c72..8b618153d3 100644 --- a/core/deleter/mocks/user_pat_service.go +++ b/core/deleter/mocks/user_pat_service.go @@ -21,6 +21,53 @@ func (_m *UserPATService) EXPECT() *UserPATService_Expecter { return &UserPATService_Expecter{mock: &_m.Mock} } +// DeleteAllByOrg provides a mock function with given fields: ctx, orgID +func (_m *UserPATService) DeleteAllByOrg(ctx context.Context, orgID string) error { + ret := _m.Called(ctx, orgID) + + if len(ret) == 0 { + panic("no return value specified for DeleteAllByOrg") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = rf(ctx, orgID) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// UserPATService_DeleteAllByOrg_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteAllByOrg' +type UserPATService_DeleteAllByOrg_Call struct { + *mock.Call +} + +// DeleteAllByOrg is a helper method to define mock.On call +// - ctx context.Context +// - orgID string +func (_e *UserPATService_Expecter) DeleteAllByOrg(ctx interface{}, orgID interface{}) *UserPATService_DeleteAllByOrg_Call { + return &UserPATService_DeleteAllByOrg_Call{Call: _e.mock.On("DeleteAllByOrg", ctx, orgID)} +} + +func (_c *UserPATService_DeleteAllByOrg_Call) Run(run func(ctx context.Context, orgID string)) *UserPATService_DeleteAllByOrg_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *UserPATService_DeleteAllByOrg_Call) Return(_a0 error) *UserPATService_DeleteAllByOrg_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *UserPATService_DeleteAllByOrg_Call) RunAndReturn(run func(context.Context, string) error) *UserPATService_DeleteAllByOrg_Call { + _c.Call.Return(run) + return _c +} + // DeleteAllByUser provides a mock function with given fields: ctx, userID func (_m *UserPATService) DeleteAllByUser(ctx context.Context, userID string) error { ret := _m.Called(ctx, userID) diff --git a/core/deleter/service.go b/core/deleter/service.go index 5cd2f885a2..7b6427d363 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -10,6 +10,7 @@ import ( "github.com/raystack/frontier/core/audit" "github.com/raystack/frontier/core/authenticate" + "github.com/raystack/frontier/core/domain" "github.com/raystack/frontier/billing" @@ -95,6 +96,12 @@ type UserService interface { type UserPATService interface { DeleteAllByUser(ctx context.Context, userID string) error + DeleteAllByOrg(ctx context.Context, orgID string) error +} + +type DomainService interface { + List(ctx context.Context, flt domain.Filter) ([]domain.Domain, error) + Delete(ctx context.Context, id string) error } type ServiceUserService interface { @@ -149,6 +156,7 @@ type Service struct { invitationService InvitationService userService UserService userPATService UserPATService + domainService DomainService serviceUserService ServiceUserService customerService CustomerService subService SubscriptionService @@ -168,7 +176,7 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic membershipService MembershipService, policyService PolicyService, roleService RoleService, invitationService InvitationService, userService UserService, - userPATService UserPATService, + userPATService UserPATService, domainService DomainService, serviceUserService ServiceUserService, customerService CustomerService, subService SubscriptionService, invoiceService InvoiceService, checkoutService CheckoutService, @@ -186,6 +194,7 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic invitationService: invitationService, userService: userService, userPATService: userPATService, + domainService: domainService, serviceUserService: serviceUserService, customerService: customerService, subService: subService, @@ -372,6 +381,20 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error { } } + domains, err := d.domainService.List(ctx, domain.Filter{OrgID: id}) + if err != nil { + return err + } + for _, dmn := range domains { + if err = d.domainService.Delete(ctx, dmn.ID); err != nil { + return fmt.Errorf("failed to delete org while deleting a domain[%s]: %w", dmn.Name, err) + } + } + + if err := d.userPATService.DeleteAllByOrg(ctx, id); err != nil { + return fmt.Errorf("failed to delete org while deleting its personal access tokens: %w", err) + } + if err := d.orgService.DeleteModel(ctx, id); err != nil { return err } diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index 0aabc63786..296c3cff16 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -19,6 +19,7 @@ import ( "github.com/raystack/frontier/core/audit" "github.com/raystack/frontier/core/deleter" "github.com/raystack/frontier/core/deleter/mocks" + "github.com/raystack/frontier/core/domain" "github.com/raystack/frontier/core/group" "github.com/raystack/frontier/core/invitation" "github.com/raystack/frontier/core/membership" @@ -48,6 +49,7 @@ type deleterMocks struct { invSvc *mocks.InvitationService usrSvc *mocks.UserService patSvc *mocks.UserPATService + domainSvc *mocks.DomainService suSvc *mocks.ServiceUserService custSvc *mocks.CustomerService subSvc *mocks.SubscriptionService @@ -72,6 +74,7 @@ func newMocks(t *testing.T) deleterMocks { invSvc: mocks.NewInvitationService(t), usrSvc: mocks.NewUserService(t), patSvc: mocks.NewUserPATService(t), + domainSvc: mocks.NewDomainService(t), suSvc: mocks.NewServiceUserService(t), custSvc: mocks.NewCustomerService(t), subSvc: mocks.NewSubscriptionService(t), @@ -95,7 +98,7 @@ func newMocks(t *testing.T) deleterMocks { func (m deleterMocks) build() *deleter.Service { return deleter.NewCascadeDeleter(m.orgSvc, m.projSvc, m.resSvc, m.grpSvc, m.mbrSvc, - m.polSvc, m.roleSvc, m.invSvc, m.usrSvc, m.patSvc, m.suSvc, + m.polSvc, m.roleSvc, m.invSvc, m.usrSvc, m.patSvc, m.domainSvc, m.suSvc, m.custSvc, m.subSvc, m.invocSvc, m.checkoutSvc, m.creditSvc, m.kycSvc, m.planSvc, m.dialer, billing.OrgDeleteNoticeConfig{}) } @@ -235,6 +238,10 @@ func TestDeleteOrganization(t *testing.T) { m.roleSvc.EXPECT().Delete(mock.Anything, "role-1").Return(nil) // org model + m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). + Return([]domain.Domain{{ID: "dom-1", Name: "acme.test"}}, nil) + m.domainSvc.EXPECT().Delete(mock.Anything, "dom-1").Return(nil) + m.patSvc.EXPECT().DeleteAllByOrg(mock.Anything, "org-1").Return(nil) m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) // every delete notifies the owners, tokens or not @@ -425,6 +432,9 @@ func TestDeleteOrganization(t *testing.T) { Return([]policy.Policy{}, nil) m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). Return([]role.Role{}, nil) + m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). + Return([]domain.Domain{}, nil) + m.patSvc.EXPECT().DeleteAllByOrg(mock.Anything, "org-1").Return(nil) m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) err := m.build().DeleteOrganization(context.Background(), "org-1") @@ -472,6 +482,9 @@ func TestDeleteOrganization(t *testing.T) { Return([]policy.Policy{}, nil) m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). Return([]role.Role{}, nil) + m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). + Return([]domain.Domain{}, nil) + m.patSvc.EXPECT().DeleteAllByOrg(mock.Anything, "org-1").Return(nil) m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) expectOwnerNotified(m) @@ -573,6 +586,9 @@ func TestDeleteOrganization(t *testing.T) { Return([]policy.Policy{}, nil) m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). Return([]role.Role{}, nil) + m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). + Return([]domain.Domain{}, nil) + m.patSvc.EXPECT().DeleteAllByOrg(mock.Anything, "org-1").Return(nil) m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) expectOwnerNotified(m) @@ -752,6 +768,112 @@ func TestDeleteOrganization(t *testing.T) { assert.ErrorContains(t, err, "su delete failed") assert.ErrorContains(t, err, "su-1") }) + + t.Run("domains and tokens go after the roles and before the org row", func(t *testing.T) { + m := newMocks(t) + + m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + Return([]project.Project{}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + Return([]group.Group{}, nil) + m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). + Return([]serviceuser.ServiceUser{}, nil) + m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). + Return([]invitation.Invitation{}, nil) + m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) + + var steps []string + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{OrgID: "org-1"}). + Return([]policy.Policy{{ID: "org-pol-1"}}, nil) + m.polSvc.EXPECT().Delete(mock.Anything, "org-pol-1"). + Run(func(context.Context, string) { steps = append(steps, "policy") }).Return(nil) + m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). + Return([]role.Role{{ID: "role-1", Name: "r1"}}, nil) + m.roleSvc.EXPECT().Delete(mock.Anything, "role-1"). + Run(func(context.Context, string) { steps = append(steps, "role") }).Return(nil) + m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). + Return([]domain.Domain{{ID: "dom-1", Name: "acme.test"}}, nil) + m.domainSvc.EXPECT().Delete(mock.Anything, "dom-1"). + Run(func(context.Context, string) { steps = append(steps, "domain") }).Return(nil) + m.patSvc.EXPECT().DeleteAllByOrg(mock.Anything, "org-1"). + Run(func(context.Context, string) { steps = append(steps, "pats") }).Return(nil) + m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1"). + Run(func(context.Context, string) { steps = append(steps, "org") }).Return(nil) + + m.roleSvc.EXPECT().Get(mock.Anything, schema.RoleOrganizationOwner). + Return(role.Role{}, errors.New("no owners in this test")) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.NoError(t, err) + assert.Equal(t, []string{"policy", "role", "domain", "pats", "org"}, steps) + }) + + t.Run("domain delete failure keeps the org row", func(t *testing.T) { + m := newMocks(t) + + m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + Return([]project.Project{}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + Return([]group.Group{}, nil) + m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). + Return([]serviceuser.ServiceUser{}, nil) + m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). + Return([]invitation.Invitation{}, nil) + m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{OrgID: "org-1"}). + Return([]policy.Policy{}, nil) + m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). + Return([]role.Role{}, nil) + m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). + Return([]domain.Domain{{ID: "dom-1", Name: "acme.test"}}, nil) + m.domainSvc.EXPECT().Delete(mock.Anything, "dom-1").Return(errors.New("domain delete failed")) + + m.roleSvc.EXPECT().Get(mock.Anything, schema.RoleOrganizationOwner). + Return(role.Role{}, errors.New("no owners in this test")) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.ErrorContains(t, err, "domain delete failed") + assert.ErrorContains(t, err, "acme.test") + }) + + t.Run("token delete failure keeps the org row", func(t *testing.T) { + m := newMocks(t) + + m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + Return([]project.Project{}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + Return([]group.Group{}, nil) + m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). + Return([]serviceuser.ServiceUser{}, nil) + m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). + Return([]invitation.Invitation{}, nil) + m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{OrgID: "org-1"}). + Return([]policy.Policy{}, nil) + m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). + Return([]role.Role{}, nil) + m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). + Return([]domain.Domain{}, nil) + m.patSvc.EXPECT().DeleteAllByOrg(mock.Anything, "org-1").Return(errors.New("pat delete failed")) + + m.roleSvc.EXPECT().Get(mock.Anything, schema.RoleOrganizationOwner). + Return(role.Role{}, errors.New("no owners in this test")) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.ErrorContains(t, err, "pat delete failed") + }) } func TestCheckOrganizationDelete(t *testing.T) { @@ -905,6 +1027,9 @@ func TestForfeitAuditRecord(t *testing.T) { Return([]policy.Policy{}, nil) m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). Return([]role.Role{}, nil) + m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). + Return([]domain.Domain{}, nil) + m.patSvc.EXPECT().DeleteAllByOrg(mock.Anything, "org-1").Return(nil) m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) expectOwnerNotified(m) @@ -953,6 +1078,9 @@ func TestForfeitAuditRecord(t *testing.T) { Return([]policy.Policy{}, nil) m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). Return([]role.Role{}, nil) + m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). + Return([]domain.Domain{}, nil) + m.patSvc.EXPECT().DeleteAllByOrg(mock.Anything, "org-1").Return(nil) m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) m.roleSvc.EXPECT().Get(mock.Anything, schema.RoleOrganizationOwner). From 9218314fb1151385af8f28ed6810be2ff535fb9a Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Thu, 8 Oct 2026 15:00:17 +0530 Subject: [PATCH 07/18] fix(deleter): remove domains and tokens before the org policies --- core/deleter/service.go | 28 ++++++++++++------------ core/deleter/service_test.go | 42 +++++++++++++++++++++++++----------- 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/core/deleter/service.go b/core/deleter/service.go index 7b6427d363..bf31751bbe 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -354,6 +354,20 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error { return fmt.Errorf("failed to delete org while deleting its kyc record: %w", err) } + domains, err := d.domainService.List(ctx, domain.Filter{OrgID: id}) + if err != nil { + return err + } + for _, dmn := range domains { + if err = d.domainService.Delete(ctx, dmn.ID); err != nil { + return fmt.Errorf("failed to delete org while deleting a domain[%s]: %w", dmn.Name, err) + } + } + + if err := d.userPATService.DeleteAllByOrg(ctx, id); err != nil { + return fmt.Errorf("failed to delete org while deleting its personal access tokens: %w", err) + } + // delete all policies; this removes the org owners, so it stays after the // steps above. Policies must go before roles as they refer to them. policies, err := d.policyService.List(ctx, policy.Filter{ @@ -381,20 +395,6 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error { } } - domains, err := d.domainService.List(ctx, domain.Filter{OrgID: id}) - if err != nil { - return err - } - for _, dmn := range domains { - if err = d.domainService.Delete(ctx, dmn.ID); err != nil { - return fmt.Errorf("failed to delete org while deleting a domain[%s]: %w", dmn.Name, err) - } - } - - if err := d.userPATService.DeleteAllByOrg(ctx, id); err != nil { - return fmt.Errorf("failed to delete org while deleting its personal access tokens: %w", err) - } - if err := d.orgService.DeleteModel(ctx, id); err != nil { return err } diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index 296c3cff16..f895a68e5d 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -769,7 +769,7 @@ func TestDeleteOrganization(t *testing.T) { assert.ErrorContains(t, err, "su-1") }) - t.Run("domains and tokens go after the roles and before the org row", func(t *testing.T) { + t.Run("domains and tokens go before the policies and roles and the org row last", func(t *testing.T) { m := newMocks(t) m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). @@ -809,10 +809,36 @@ func TestDeleteOrganization(t *testing.T) { err := m.build().DeleteOrganization(context.Background(), "org-1") assert.NoError(t, err) - assert.Equal(t, []string{"policy", "role", "domain", "pats", "org"}, steps) + assert.Equal(t, []string{"domain", "pats", "policy", "role", "org"}, steps) }) - t.Run("domain delete failure keeps the org row", func(t *testing.T) { + t.Run("domain list failure stops before the org policies", func(t *testing.T) { + m := newMocks(t) + + m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + Return([]project.Project{}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + Return([]group.Group{}, nil) + m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). + Return([]serviceuser.ServiceUser{}, nil) + m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). + Return([]invitation.Invitation{}, nil) + m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) + m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). + Return(nil, errors.New("domain list failed")) + + m.roleSvc.EXPECT().Get(mock.Anything, schema.RoleOrganizationOwner). + Return(role.Role{}, errors.New("no owners in this test")) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.ErrorContains(t, err, "domain list failed") + }) + + t.Run("domain delete failure stops before the org policies", func(t *testing.T) { m := newMocks(t) m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). @@ -828,10 +854,6 @@ func TestDeleteOrganization(t *testing.T) { m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). Return([]invitation.Invitation{}, nil) m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) - m.polSvc.EXPECT().List(mock.Anything, policy.Filter{OrgID: "org-1"}). - Return([]policy.Policy{}, nil) - m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). - Return([]role.Role{}, nil) m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). Return([]domain.Domain{{ID: "dom-1", Name: "acme.test"}}, nil) m.domainSvc.EXPECT().Delete(mock.Anything, "dom-1").Return(errors.New("domain delete failed")) @@ -844,7 +866,7 @@ func TestDeleteOrganization(t *testing.T) { assert.ErrorContains(t, err, "acme.test") }) - t.Run("token delete failure keeps the org row", func(t *testing.T) { + t.Run("token delete failure stops before the org policies", func(t *testing.T) { m := newMocks(t) m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). @@ -860,10 +882,6 @@ func TestDeleteOrganization(t *testing.T) { m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). Return([]invitation.Invitation{}, nil) m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) - m.polSvc.EXPECT().List(mock.Anything, policy.Filter{OrgID: "org-1"}). - Return([]policy.Policy{}, nil) - m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). - Return([]role.Role{}, nil) m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). Return([]domain.Domain{}, nil) m.patSvc.EXPECT().DeleteAllByOrg(mock.Anything, "org-1").Return(errors.New("pat delete failed")) From 14f0cf1c903b5399d30bc94b36a9553e4e2c9f31 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 12 Oct 2026 15:51:39 +0530 Subject: [PATCH 08/18] fix(deleter): org delete skips a domain that was already removed --- core/deleter/service.go | 3 +++ core/deleter/service_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/core/deleter/service.go b/core/deleter/service.go index bf31751bbe..f8218be686 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -360,6 +360,9 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error { } for _, dmn := range domains { if err = d.domainService.Delete(ctx, dmn.ID); err != nil { + if errors.Is(err, domain.ErrNotExist) { + continue + } return fmt.Errorf("failed to delete org while deleting a domain[%s]: %w", dmn.Name, err) } } diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index f895a68e5d..bc8975f4d0 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "fmt" "strings" "testing" @@ -866,6 +867,40 @@ func TestDeleteOrganization(t *testing.T) { assert.ErrorContains(t, err, "acme.test") }) + t.Run("a domain that is already gone is skipped and the next one is deleted", func(t *testing.T) { + m := newMocks(t) + + m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + Return([]project.Project{}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + Return([]group.Group{}, nil) + m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). + Return([]serviceuser.ServiceUser{}, nil) + m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). + Return([]invitation.Invitation{}, nil) + m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) + m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). + Return([]domain.Domain{{ID: "dom-1", Name: "gone.test"}, {ID: "dom-2", Name: "acme.test"}}, nil) + m.domainSvc.EXPECT().Delete(mock.Anything, "dom-1").Return(fmt.Errorf("get domain: %w", domain.ErrNotExist)) + m.domainSvc.EXPECT().Delete(mock.Anything, "dom-2").Return(nil).Once() + m.patSvc.EXPECT().DeleteAllByOrg(mock.Anything, "org-1").Return(nil) + m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) + + m.roleSvc.EXPECT().Get(mock.Anything, schema.RoleOrganizationOwner). + Return(role.Role{}, errors.New("no owners in this test")) + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{OrgID: "org-1"}). + Return([]policy.Policy{}, nil) + m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). + Return([]role.Role{}, nil) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.NoError(t, err) + }) + t.Run("token delete failure stops before the org policies", func(t *testing.T) { m := newMocks(t) From c0f4ae8dde88026e405c4a1d90e0b6c3836d3a44 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 6 Oct 2026 16:44:26 +0530 Subject: [PATCH 09/18] feat(store): organization delete keeps the row and frees its name --- .../store/postgres/organization_repository.go | 2 +- .../postgres/organization_repository_test.go | 54 ++++++++----------- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/internal/store/postgres/organization_repository.go b/internal/store/postgres/organization_repository.go index ba15574c9f..fae59b6f84 100644 --- a/internal/store/postgres/organization_repository.go +++ b/internal/store/postgres/organization_repository.go @@ -509,7 +509,7 @@ func (r OrganizationRepository) SetState(ctx context.Context, id string, state o } func (r OrganizationRepository) Delete(ctx context.Context, id string) error { - query, params, err := dialect.Delete(TABLE_ORGANIZATIONS).Where( + query, params, err := softDelete(TABLE_ORGANIZATIONS).Where( goqu.Ex{ "id": id, }, diff --git a/internal/store/postgres/organization_repository_test.go b/internal/store/postgres/organization_repository_test.go index 12f3a21bb1..9778fc713a 100644 --- a/internal/store/postgres/organization_repository_test.go +++ b/internal/store/postgres/organization_repository_test.go @@ -438,40 +438,30 @@ func (s *OrganizationRepositoryTestSuite) TestUpdateBySlug() { } func (s *OrganizationRepositoryTestSuite) TestDelete() { - type testCase struct { - Description string - OrganizationToDelete organization.Organization - ErrString string - } + s.Run("should return error if organization not found", func() { + s.Assert().ErrorIs(s.repository.Delete(s.ctx, uuid.NewString()), organization.ErrNotExist) + }) - var testCases = []testCase{ - { - Description: "should delete a organization", - OrganizationToDelete: organization.Organization{ - ID: s.orgs[0].ID, - }, - }, - { - Description: "should return error if organization not found", - OrganizationToDelete: organization.Organization{ - ID: uuid.NewString(), - }, - ErrString: organization.ErrNotExist.Error(), - }, - } + s.Run("should keep the row, hide it from reads, refuse a second delete and free its name", func() { + deleted := s.orgs[0] - for _, tc := range testCases { - s.Run(tc.Description, func() { - err := s.repository.Delete(s.ctx, tc.OrganizationToDelete.ID) - if tc.ErrString != "" { - if err.Error() != tc.ErrString { - s.T().Fatalf("got error %s, expected was %s", err.Error(), tc.ErrString) - } - } else { - s.Assert().NoError(err) - } - }) - } + s.Require().NoError(s.repository.Delete(s.ctx, deleted.ID)) + + s.Assert().Equal("1", scalarSQL(s.T(), s.ctx, s.client, + "SELECT count(*) FROM organizations WHERE id = $1 AND deleted_at IS NOT NULL", deleted.ID)) + s.Assert().Equal("1", scalarSQL(s.T(), s.ctx, s.client, + "SELECT count(*) FROM audit_records WHERE event = 'organization.delete' AND resource_id = $1", deleted.ID)) + + _, err := s.repository.GetByID(s.ctx, deleted.ID) + s.Assert().ErrorIs(err, organization.ErrNotExist) + + s.Assert().ErrorIs(s.repository.Delete(s.ctx, deleted.ID), organization.ErrNotExist) + + again, err := s.repository.Create(s.ctx, organization.Organization{Name: deleted.Name, Metadata: metadata.Metadata{}}) + s.Require().NoError(err) + s.Assert().NotEqual(deleted.ID, again.ID) + s.Assert().Equal(deleted.Name, again.Name) + }) } func (s *OrganizationRepositoryTestSuite) TestSetState() { From 6aeec07c062042d5f21f78f217e88e6e9a90c3a4 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 6 Oct 2026 16:46:04 +0530 Subject: [PATCH 10/18] test(e2e): deleting an org hides it, frees its name and removes its domains and policies --- test/e2e/regression/api_test.go | 70 +++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index 0056543bc3..bea4b5c4ba 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -3152,6 +3152,76 @@ func (s *APIRegressionTestSuite) TestOrganizationRoleDeleteInUse() { s.Require().NoError(err) } +func (s *APIRegressionTestSuite) TestOrganizationDeleteKeepsTheRow() { + ctxOrgAdminAuth := testbench.ContextWithAuth(context.Background(), s.adminCookie) + + createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + Body: &frontierv1beta1.OrganizationRequestBody{ + Title: "org soft delete", + Name: "org-soft-delete", + }, + })) + s.Require().NoError(err) + orgID := createOrgResp.Msg.GetOrganization().GetId() + + createDomainResp, err := s.testBench.Client.CreateOrganizationDomain(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationDomainRequest{ + OrgId: orgID, + Domain: "org-soft-delete.raystack.io", + })) + s.Require().NoError(err) + domainID := createDomainResp.Msg.GetDomain().GetId() + + policiesResp, err := s.testBench.Client.ListPolicies(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListPoliciesRequest{OrgId: orgID})) + s.Require().NoError(err) + policiesOnOrgBeforeDelete := policiesResp.Msg.GetPolicies() + s.Require().NotEmpty(policiesOnOrgBeforeDelete) + + _, err = s.testBench.Client.DeleteOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeleteOrganizationRequest{Id: orgID})) + s.Require().NoError(err) + + s.Run("no live policy names the org as its resource", func() { + for _, policyBeforeDelete := range policiesOnOrgBeforeDelete { + _, err := s.testBench.Client.GetPolicy(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.GetPolicyRequest{Id: policyBeforeDelete.GetId()})) + s.Require().Error(err) + s.Assert().Equal(connect.CodeNotFound, connect.CodeOf(err)) + } + }) + + s.Run("the org is hidden from the superuser listing", func() { + listResp, err := s.testBench.AdminClient.ListAllOrganizations(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListAllOrganizationsRequest{})) + s.Require().NoError(err) + for _, org := range listResp.Msg.GetOrganizations() { + s.Assert().NotEqual(orgID, org.GetId()) + } + }) + + s.Run("the org's domain is gone with it", func() { + _, err := s.testBench.Client.GetOrganizationDomain(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.GetOrganizationDomainRequest{ + OrgId: orgID, + Id: domainID, + })) + s.Require().Error(err) + s.Assert().Equal(connect.CodeNotFound, connect.CodeOf(err)) + }) + + s.Run("a second delete is refused", func() { + _, err := s.testBench.Client.DeleteOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeleteOrganizationRequest{Id: orgID})) + s.Require().Error(err) + s.Assert().Contains([]connect.Code{connect.CodeNotFound, connect.CodePermissionDenied}, connect.CodeOf(err)) + }) + + s.Run("a new org can take the name", func() { + recreateResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + Body: &frontierv1beta1.OrganizationRequestBody{ + Title: "org soft delete again", + Name: "org-soft-delete", + }, + })) + s.Require().NoError(err) + s.Assert().NotEqual(orgID, recreateResp.Msg.GetOrganization().GetId()) + }) +} + func TestEndToEndAPIRegressionTestSuite(t *testing.T) { suite.Run(t, new(APIRegressionTestSuite)) } From 05e318dbbe5db8d12a8ff802ce9ac3ac75cec640 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 7 Oct 2026 14:46:01 +0530 Subject: [PATCH 11/18] test(e2e): a personal access token of a deleted org stops authenticating --- test/e2e/regression/pat_test.go | 58 +++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/test/e2e/regression/pat_test.go b/test/e2e/regression/pat_test.go index b34fe6b82e..9a1b259177 100644 --- a/test/e2e/regression/pat_test.go +++ b/test/e2e/regression/pat_test.go @@ -771,6 +771,64 @@ func (s *PATRegressionTestSuite) TestPATCRUD_CreateErrors() { }) } +func (s *PATRegressionTestSuite) TestPATOfDeletedOrgStopsAuthenticating() { + ctxAdmin := testbench.ContextWithAuth(context.Background(), s.adminCookie) + orgID, proj1ID, _ := s.createOrgAndProjects(ctxAdmin, "org-pat-deleted-org", "pat-deleted-org-p1", "") + + patID, patToken := s.createPAT(ctxAdmin, orgID, "pat-of-deleted-org", []*frontierv1beta1.PATScope{ + {RoleId: s.roleID(schema.RoleOrganizationViewer), ResourceType: schema.OrganizationNamespace}, + {RoleId: s.roleID(schema.RoleProjectViewer), ResourceType: schema.ProjectNamespace, ResourceIds: []string{proj1ID}}, + }) + patCtx := getPATCtx(patToken) + s.Require().True(s.checkPermission(patCtx, schema.OrganizationNamespace, orgID, schema.GetPermission)) + + policiesResp, err := s.testBench.Client.ListPolicies(ctxAdmin, connect.NewRequest(&frontierv1beta1.ListPoliciesRequest{UserId: patID})) + s.Require().NoError(err) + patPoliciesBeforeDelete := policiesResp.Msg.GetPolicies() + s.Require().Len(patPoliciesBeforeDelete, 2) + + _, err = s.testBench.Client.DeleteOrganization(ctxAdmin, connect.NewRequest(&frontierv1beta1.DeleteOrganizationRequest{Id: orgID})) + s.Require().NoError(err) + + s.Run("the token is rejected", func() { + _, err := s.testBench.Client.GetCurrentUser(patCtx, connect.NewRequest(&frontierv1beta1.GetCurrentUserRequest{})) + s.Require().Error(err) + s.Assert().Equal(connect.CodeUnauthenticated, connect.CodeOf(err)) + }) + + s.Run("the token is hidden from its owner", func() { + _, err := s.testBench.Client.GetCurrentUserPAT(ctxAdmin, connect.NewRequest(&frontierv1beta1.GetCurrentUserPATRequest{Id: patID})) + s.Require().Error(err) + s.Assert().Equal(connect.CodeNotFound, connect.CodeOf(err)) + }) + + s.Run("no live policy is held by the token", func() { + policiesResp, err := s.testBench.Client.ListPolicies(ctxAdmin, connect.NewRequest(&frontierv1beta1.ListPoliciesRequest{UserId: patID})) + s.Require().NoError(err) + s.Assert().Empty(policiesResp.Msg.GetPolicies()) + for _, policyBeforeDelete := range patPoliciesBeforeDelete { + _, err := s.testBench.Client.GetPolicy(ctxAdmin, connect.NewRequest(&frontierv1beta1.GetPolicyRequest{Id: policyBeforeDelete.GetId()})) + s.Require().Error(err) + s.Assert().Equal(connect.CodeNotFound, connect.CodeOf(err)) + } + }) + + s.Run("no relation is left for the token", func() { + for _, policyBeforeDelete := range patPoliciesBeforeDelete { + resp, err := s.testBench.AdminClient.ListRelations(ctxAdmin, connect.NewRequest(&frontierv1beta1.ListRelationsRequest{ + Object: schema.JoinNamespaceAndResourceID(schema.RoleBindingNamespace, policyBeforeDelete.GetId()), + })) + s.Require().NoError(err) + s.Assert().Empty(resp.Msg.GetRelations()) + } + resp, err := s.testBench.AdminClient.ListRelations(ctxAdmin, connect.NewRequest(&frontierv1beta1.ListRelationsRequest{ + Subject: schema.JoinNamespaceAndResourceID(schema.PATPrincipal, patID), + })) + s.Require().NoError(err) + s.Assert().Empty(resp.Msg.GetRelations()) + }) +} + func TestEndToEndPATRegressionTestSuite(t *testing.T) { suite.Run(t, new(PATRegressionTestSuite)) } From 2a674b7cf86ce5c10392cd1e20f4e69b14870d5b Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Thu, 8 Oct 2026 12:43:12 +0530 Subject: [PATCH 12/18] test(e2e): name the org delete probe after what it checks --- test/e2e/regression/api_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index bea4b5c4ba..3bcb250e9f 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -3152,7 +3152,7 @@ func (s *APIRegressionTestSuite) TestOrganizationRoleDeleteInUse() { s.Require().NoError(err) } -func (s *APIRegressionTestSuite) TestOrganizationDeleteKeepsTheRow() { +func (s *APIRegressionTestSuite) TestOrganizationDeleteHidesTheOrgAndRemovesWhatItOwned() { ctxOrgAdminAuth := testbench.ContextWithAuth(context.Background(), s.adminCookie) createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ From 4b13c1e5760bb7704ca6cbfd03b82fadabc19734 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Thu, 8 Oct 2026 13:26:49 +0530 Subject: [PATCH 13/18] test(e2e): check the token's access in SpiceDB after its org is deleted --- test/e2e/regression/pat_test.go | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/test/e2e/regression/pat_test.go b/test/e2e/regression/pat_test.go index 9a1b259177..871369a141 100644 --- a/test/e2e/regression/pat_test.go +++ b/test/e2e/regression/pat_test.go @@ -155,6 +155,16 @@ func (s *PATRegressionTestSuite) checkPermission(ctx context.Context, namespace, return resp.Msg.GetStatus() } +func (s *PATRegressionTestSuite) patHasPermissionInSpiceDB(ctx context.Context, patID, namespace, id, permission string) bool { + resp, err := s.testBench.AdminClient.CheckFederatedResourcePermission(ctx, connect.NewRequest(&frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(namespace, id), + Permission: permission, + Subject: schema.JoinNamespaceAndResourceID(schema.PATPrincipal, patID), + })) + s.Require().NoError(err) + return resp.Msg.GetStatus() +} + func (s *PATRegressionTestSuite) TestPATScope_OrgViewer_ProjectViewer() { ctxAdmin := testbench.ContextWithAuth(context.Background(), s.adminCookie) orgID, proj1ID, proj2ID := s.createOrgAndProjects(ctxAdmin, "org-pat-ov-pv", "pat-ov-pv-p1", "pat-ov-pv-p2") @@ -781,6 +791,8 @@ func (s *PATRegressionTestSuite) TestPATOfDeletedOrgStopsAuthenticating() { }) patCtx := getPATCtx(patToken) s.Require().True(s.checkPermission(patCtx, schema.OrganizationNamespace, orgID, schema.GetPermission)) + s.Require().True(s.patHasPermissionInSpiceDB(ctxAdmin, patID, schema.OrganizationNamespace, orgID, schema.GetPermission)) + s.Require().True(s.patHasPermissionInSpiceDB(ctxAdmin, patID, schema.ProjectNamespace, proj1ID, schema.GetPermission)) policiesResp, err := s.testBench.Client.ListPolicies(ctxAdmin, connect.NewRequest(&frontierv1beta1.ListPoliciesRequest{UserId: patID})) s.Require().NoError(err) @@ -813,19 +825,9 @@ func (s *PATRegressionTestSuite) TestPATOfDeletedOrgStopsAuthenticating() { } }) - s.Run("no relation is left for the token", func() { - for _, policyBeforeDelete := range patPoliciesBeforeDelete { - resp, err := s.testBench.AdminClient.ListRelations(ctxAdmin, connect.NewRequest(&frontierv1beta1.ListRelationsRequest{ - Object: schema.JoinNamespaceAndResourceID(schema.RoleBindingNamespace, policyBeforeDelete.GetId()), - })) - s.Require().NoError(err) - s.Assert().Empty(resp.Msg.GetRelations()) - } - resp, err := s.testBench.AdminClient.ListRelations(ctxAdmin, connect.NewRequest(&frontierv1beta1.ListRelationsRequest{ - Subject: schema.JoinNamespaceAndResourceID(schema.PATPrincipal, patID), - })) - s.Require().NoError(err) - s.Assert().Empty(resp.Msg.GetRelations()) + s.Run("the token has no access left in SpiceDB", func() { + s.Assert().False(s.patHasPermissionInSpiceDB(ctxAdmin, patID, schema.OrganizationNamespace, orgID, schema.GetPermission)) + s.Assert().False(s.patHasPermissionInSpiceDB(ctxAdmin, patID, schema.ProjectNamespace, proj1ID, schema.GetPermission)) }) } From 1be7c1d91a087d141ddee751bec7dc75818965f3 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Fri, 9 Oct 2026 16:12:23 +0530 Subject: [PATCH 14/18] feat(deleter): delete disabled projects and groups with the org --- core/deleter/service.go | 30 ++++++-- core/deleter/service_test.go | 126 ++++++++++++++++++++++++++++++++++ core/group/service.go | 6 +- core/group/service_test.go | 28 ++++++++ core/membership/group.go | 18 ++++- core/membership/group_test.go | 31 +++++++++ 6 files changed, 233 insertions(+), 6 deletions(-) diff --git a/core/deleter/service.go b/core/deleter/service.go index f8218be686..d0347af322 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -303,9 +303,7 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error { } // delete all related projects - projects, err := d.projService.List(ctx, project.Filter{ - OrgID: id, - }) + projects, err := d.listOrgProjects(ctx, id) if err != nil { return err } @@ -316,7 +314,7 @@ func (d Service) DeleteOrganization(ctx context.Context, id string) error { } // delete all related groups - groups, err := d.groupService.List(ctx, group.Filter{OrganizationID: id}) + groups, err := d.listOrgGroups(ctx, id) if err != nil { return err } @@ -547,6 +545,30 @@ func (d Service) DeleteUser(ctx context.Context, userID string) error { return d.userService.Delete(ctx, userID) } +func (d Service) listOrgProjects(ctx context.Context, orgID string) ([]project.Project, error) { + enabled, err := d.projService.List(ctx, project.Filter{OrgID: orgID}) + if err != nil { + return nil, err + } + disabled, err := d.projService.List(ctx, project.Filter{OrgID: orgID, State: project.Disabled}) + if err != nil { + return nil, err + } + return append(enabled, disabled...), nil +} + +func (d Service) listOrgGroups(ctx context.Context, orgID string) ([]group.Group, error) { + enabled, err := d.groupService.List(ctx, group.Filter{OrganizationID: orgID}) + if err != nil { + return nil, err + } + disabled, err := d.groupService.List(ctx, group.Filter{OrganizationID: orgID, State: group.Disabled}) + if err != nil { + return nil, err + } + return append(enabled, disabled...), nil +} + // ensureDeletable collects everything that blocks deleting the organization and // returns it all as one BlockedError, so the caller gets a full checklist // instead of discovering blockers one retry at a time. diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index bc8975f4d0..c8ea4b4cca 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -63,6 +63,16 @@ type deleterMocks struct { } func newMocks(t *testing.T) deleterMocks { + t.Helper() + m := newBareMocks(t) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", State: project.Disabled}). + Return([]project.Project{}, nil).Maybe() + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", State: group.Disabled}). + Return([]group.Group{}, nil).Maybe() + return m +} + +func newBareMocks(t *testing.T) deleterMocks { t.Helper() m := deleterMocks{ orgSvc: mocks.NewOrganizationService(t), @@ -158,6 +168,25 @@ func TestDeleteProject(t *testing.T) { }) } +func expectEmptyOrgTail(m deleterMocks, steps *[]string) { + m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). + Return([]serviceuser.ServiceUser{}, nil) + m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). + Return([]invitation.Invitation{}, nil) + m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) + m.domainSvc.EXPECT().List(mock.Anything, domain.Filter{OrgID: "org-1"}). + Return([]domain.Domain{}, nil) + m.patSvc.EXPECT().DeleteAllByOrg(mock.Anything, "org-1").Return(nil) + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{OrgID: "org-1"}). + Return([]policy.Policy{}, nil) + m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). + Return([]role.Role{}, nil) + m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1"). + Run(func(context.Context, string) { *steps = append(*steps, "org") }).Return(nil) + m.roleSvc.EXPECT().Get(mock.Anything, schema.RoleOrganizationOwner). + Return(role.Role{}, errors.New("no owners in this test")) +} + // expectOwnerNotified wires the owner lookup and one delivered mail; a // successful delete must always end in the notice being sent. func expectOwnerNotified(m deleterMocks) { @@ -269,6 +298,103 @@ func TestDeleteOrganization(t *testing.T) { assert.NoError(t, err) }) + t.Run("disabled projects and groups are deleted with the org and the org row goes last", func(t *testing.T) { + m := newBareMocks(t) + var steps []string + + m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{}, nil) + + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + Return([]project.Project{{ID: "proj-on", Name: "on"}}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", State: project.Disabled}). + Return([]project.Project{{ID: "proj-off", Name: "off", State: project.Disabled}}, nil) + for _, id := range []string{"proj-on", "proj-off"} { + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: id}). + Return([]policy.Policy{}, nil) + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: id, IncludeDeleted: true}). + Return([]resource.Resource{}, nil) + m.projSvc.EXPECT().DeleteModel(mock.Anything, id). + Run(func(_ context.Context, id string) { steps = append(steps, "project:"+id) }).Return(nil) + } + + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + Return([]group.Group{{ID: "grp-on", Name: "on"}}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", State: group.Disabled}). + Return([]group.Group{{ID: "grp-off", Name: "off", State: group.Disabled}}, nil) + for _, id := range []string{"grp-on", "grp-off"} { + m.mbrSvc.EXPECT().OnGroupDeleted(mock.Anything, id).Return(nil) + m.grpSvc.EXPECT().DeleteModel(mock.Anything, id). + Run(func(_ context.Context, id string) { steps = append(steps, "group:"+id) }).Return(nil) + } + + expectEmptyOrgTail(m, &steps) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.NoError(t, err) + assert.Equal(t, []string{ + "project:proj-on", "project:proj-off", "group:grp-on", "group:grp-off", "org", + }, steps) + }) + + t.Run("an org with only enabled children lists nothing extra to delete", func(t *testing.T) { + m := newBareMocks(t) + var steps []string + + m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + Return([]project.Project{{ID: "proj-on", Name: "on"}}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", State: project.Disabled}). + Return([]project.Project{}, nil) + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-on"}). + Return([]policy.Policy{}, nil) + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-on", IncludeDeleted: true}). + Return([]resource.Resource{}, nil) + m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-on"). + Run(func(_ context.Context, id string) { steps = append(steps, "project:"+id) }).Return(nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + Return([]group.Group{{ID: "grp-on", Name: "on"}}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", State: group.Disabled}). + Return([]group.Group{}, nil) + m.mbrSvc.EXPECT().OnGroupDeleted(mock.Anything, "grp-on").Return(nil) + m.grpSvc.EXPECT().DeleteModel(mock.Anything, "grp-on"). + Run(func(_ context.Context, id string) { steps = append(steps, "group:"+id) }).Return(nil) + + expectEmptyOrgTail(m, &steps) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.NoError(t, err) + assert.Equal(t, []string{"project:proj-on", "group:grp-on", "org"}, steps) + }) + + t.Run("a failed disabled project delete stops before the org row", func(t *testing.T) { + m := newBareMocks(t) + + m.orgSvc.EXPECT().GetRaw(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + Return([]project.Project{}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", State: project.Disabled}). + Return([]project.Project{{ID: "proj-off", Name: "off", State: project.Disabled}}, nil) + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-off"}). + Return([]policy.Policy{}, nil) + m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-off", IncludeDeleted: true}). + Return([]resource.Resource{}, nil) + m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-off").Return(errors.New("boom")) + m.roleSvc.EXPECT().Get(mock.Anything, schema.RoleOrganizationOwner). + Return(role.Role{}, errors.New("no owners in this test")) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.ErrorContains(t, err, "project[off]") + }) + t.Run("already deleted org returns not found without touching anything", func(t *testing.T) { m := newMocks(t) diff --git a/core/group/service.go b/core/group/service.go index 3ab29980d8..4ff47bfc68 100644 --- a/core/group/service.go +++ b/core/group/service.go @@ -197,10 +197,14 @@ func (s Service) Disable(ctx context.Context, id string) error { // responsibility — see core/deleter.DeleteGroup for the orchestration that // pairs this with membership.OnGroupDeleted. func (s Service) DeleteModel(ctx context.Context, id string) error { - group, err := s.repository.GetByID(ctx, id) + found, err := s.repository.GetByIDs(ctx, []string{id}, Filter{IncludeDisabled: true}) if err != nil { return err } + if len(found) == 0 { + return ErrNotExist + } + group := found[0] if err := s.repository.Delete(ctx, id); err != nil { return err diff --git a/core/group/service_test.go b/core/group/service_test.go index f5a7da59f7..79c30b1608 100644 --- a/core/group/service_test.go +++ b/core/group/service_test.go @@ -411,3 +411,31 @@ func TestService_List_PrincipalFilter(t *testing.T) { assert.Empty(t, result) }) } + +func TestService_DeleteModel(t *testing.T) { + ctx := context.Background() + newSvc := func(t *testing.T) (*group.Service, *mocks.Repository) { + t.Helper() + mockRepo := mocks.NewRepository(t) + svc := group.NewService(mockRepo, mocks.NewRelationService(t), mocks.NewAuthnService(t), mocks.NewPolicyService(t)) + return svc, mockRepo + } + + t.Run("deletes a disabled group", func(t *testing.T) { + svc, repo := newSvc(t) + disabled := group.Group{ID: "grp-off", OrganizationID: "org-1", State: group.Disabled} + repo.EXPECT().GetByIDs(mock.Anything, []string{"grp-off"}, group.Filter{IncludeDisabled: true}). + Return([]group.Group{disabled}, nil) + repo.EXPECT().Delete(mock.Anything, "grp-off").Return(nil) + + assert.NoError(t, svc.DeleteModel(ctx, "grp-off")) + }) + + t.Run("a group that does not exist is not found", func(t *testing.T) { + svc, repo := newSvc(t) + repo.EXPECT().GetByIDs(mock.Anything, []string{"grp-x"}, group.Filter{IncludeDisabled: true}). + Return([]group.Group{}, nil) + + assert.ErrorIs(t, svc.DeleteModel(ctx, "grp-x"), group.ErrNotExist) + }) +} diff --git a/core/membership/group.go b/core/membership/group.go index a1e025c088..848a694625 100644 --- a/core/membership/group.go +++ b/core/membership/group.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" + "github.com/raystack/frontier/core/group" "github.com/raystack/frontier/core/policy" "github.com/raystack/frontier/core/relation" "github.com/raystack/frontier/core/role" @@ -217,7 +218,7 @@ func (s *Service) RemoveAllGroupMembers(ctx context.Context, groupID string) err // Errors are joined; partial failures are logged so a retry can complete // the cleanup. func (s *Service) OnGroupDeleted(ctx context.Context, groupID string) error { - grp, err := s.groupService.Get(ctx, groupID) + grp, err := s.getGroupIncludingDisabled(ctx, groupID) if err != nil { return err } @@ -235,6 +236,21 @@ func (s *Service) OnGroupDeleted(ctx context.Context, groupID string) error { return errs } +func (s *Service) getGroupIncludingDisabled(ctx context.Context, groupID string) (group.Group, error) { + grp, err := s.groupService.Get(ctx, groupID) + if !errors.Is(err, group.ErrNotExist) { + return grp, err + } + disabled, listErr := s.groupService.List(ctx, group.Filter{GroupIDs: []string{groupID}, State: group.Disabled}) + if listErr != nil { + return group.Group{}, listErr + } + if len(disabled) == 0 { + return group.Group{}, err + } + return disabled[0], nil +} + // removeGroupAsPrincipalPolicies deletes every policy where the given group // is the principal — e.g. policies created by `CreatePolicy(principal=group:X, // resource=project:Y, role=viewer)` that grant the group access to other diff --git a/core/membership/group_test.go b/core/membership/group_test.go index d607f6cedc..92c9594cbe 100644 --- a/core/membership/group_test.go +++ b/core/membership/group_test.go @@ -509,9 +509,40 @@ func TestService_OnGroupDeleted(t *testing.T) { assert.NoError(t, svc.OnGroupDeleted(ctx, groupID)) }) + t.Run("cleans up a disabled group that Get does not return", func(t *testing.T) { + policySvc := mocks.NewPolicyService(t) + relSvc := mocks.NewRelationService(t) + grpSvc := mocks.NewGroupService(t) + + disabled := grp + disabled.State = group.Disabled + grpSvc.EXPECT().Get(ctx, groupID).Return(group.Group{}, group.ErrNotExist) + grpSvc.EXPECT().List(ctx, group.Filter{GroupIDs: []string{groupID}, State: group.Disabled}). + Return([]group.Group{disabled}, nil) + policySvc.EXPECT().List(ctx, policy.Filter{GroupID: groupID}).Return([]policy.Policy{}, nil) + policySvc.EXPECT().List(ctx, policy.Filter{ + PrincipalType: schema.GroupPrincipal, + PrincipalID: groupID, + }).Return([]policy.Policy{}, nil) + relSvc.EXPECT().Delete(ctx, relation.Relation{ + Object: relation.Object{ID: groupID, Namespace: schema.GroupNamespace}, + Subject: relation.Subject{ID: orgID, Namespace: schema.OrganizationNamespace}, + RelationName: schema.OrganizationRelationName, + }).Return(nil) + + svc := membership.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), policySvc, relSvc, + mocks.NewRoleService(t), mocks.NewOrgService(t), mocks.NewUserService(t), + mocks.NewProjectService(t), grpSvc, mocks.NewServiceuserService(t), + mocks.NewAuditRecordRepository(t)) + + assert.NoError(t, svc.OnGroupDeleted(ctx, groupID)) + }) + t.Run("returns error if group not found", func(t *testing.T) { grpSvc := mocks.NewGroupService(t) grpSvc.EXPECT().Get(ctx, groupID).Return(group.Group{}, group.ErrNotExist) + grpSvc.EXPECT().List(ctx, group.Filter{GroupIDs: []string{groupID}, State: group.Disabled}). + Return([]group.Group{}, nil) svc := membership.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), mocks.NewPolicyService(t), mocks.NewRelationService(t), From d288a284d619ec9a3545b8a4132982151c88e8ae Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Fri, 9 Oct 2026 16:12:23 +0530 Subject: [PATCH 15/18] test(e2e): deleting an org also deletes its disabled projects and groups --- test/e2e/regression/api_test.go | 100 ++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index 3bcb250e9f..a16ff4d143 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -3222,6 +3222,106 @@ func (s *APIRegressionTestSuite) TestOrganizationDeleteHidesTheOrgAndRemovesWhat }) } +func (s *APIRegressionTestSuite) TestOrganizationDeleteAlsoDeletesDisabledProjectsAndGroups() { + ctxOrgAdminAuth := testbench.ContextWithAuth(context.Background(), s.adminCookie) + + orgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + Body: &frontierv1beta1.OrganizationRequestBody{Title: "org-disabled-children", Name: "org-disabled-children"}, + })) + s.Require().NoError(err) + orgID := orgResp.Msg.GetOrganization().GetId() + + createProject := func(name string) string { + resp, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{ + Body: &frontierv1beta1.ProjectRequestBody{Name: name, OrgId: orgID}, + })) + s.Require().NoError(err) + return resp.Msg.GetProject().GetId() + } + createGroup := func(name string) string { + resp, err := s.testBench.Client.CreateGroup(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateGroupRequest{ + OrgId: orgID, + Body: &frontierv1beta1.GroupRequestBody{Name: name}, + })) + s.Require().NoError(err) + return resp.Msg.GetGroup().GetId() + } + + enabledProjectID := createProject("enabled-project") + disabledProjectID := createProject("disabled-project") + enabledGroupID := createGroup("enabled-group") + disabledGroupID := createGroup("disabled-group") + + _, err = s.testBench.Client.DisableProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DisableProjectRequest{Id: disabledProjectID})) + s.Require().NoError(err) + _, err = s.testBench.Client.DisableGroup(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DisableGroupRequest{Id: disabledGroupID})) + s.Require().NoError(err) + + policyIDs := func(req *frontierv1beta1.ListPoliciesRequest) []string { + resp, err := s.testBench.Client.ListPolicies(ctxOrgAdminAuth, connect.NewRequest(req)) + s.Require().NoError(err) + ids := []string{} + for _, p := range resp.Msg.GetPolicies() { + ids = append(ids, p.GetId()) + } + return ids + } + disabledProjectPolicyIDs := policyIDs(&frontierv1beta1.ListPoliciesRequest{ProjectId: disabledProjectID}) + disabledGroupPolicyIDs := policyIDs(&frontierv1beta1.ListPoliciesRequest{GroupId: disabledGroupID}) + s.Require().NotEmpty(disabledProjectPolicyIDs) + s.Require().NotEmpty(disabledGroupPolicyIDs) + + enabledProjects, err := s.testBench.AdminClient.ListProjects(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListProjectsRequest{OrgId: orgID})) + s.Require().NoError(err) + s.Require().Len(enabledProjects.Msg.GetProjects(), 1) + s.Require().Equal(enabledProjectID, enabledProjects.Msg.GetProjects()[0].GetId()) + enabledGroups, err := s.testBench.AdminClient.ListGroups(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListGroupsRequest{OrgId: orgID})) + s.Require().NoError(err) + s.Require().Len(enabledGroups.Msg.GetGroups(), 1) + s.Require().Equal(enabledGroupID, enabledGroups.Msg.GetGroups()[0].GetId()) + disabledProjects, err := s.testBench.AdminClient.ListProjects(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListProjectsRequest{OrgId: orgID, State: "disabled"})) + s.Require().NoError(err) + s.Require().Len(disabledProjects.Msg.GetProjects(), 1) + s.Require().Equal(disabledProjectID, disabledProjects.Msg.GetProjects()[0].GetId()) + disabledGroups, err := s.testBench.AdminClient.ListGroups(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListGroupsRequest{OrgId: orgID, State: "disabled"})) + s.Require().NoError(err) + s.Require().Len(disabledGroups.Msg.GetGroups(), 1) + s.Require().Equal(disabledGroupID, disabledGroups.Msg.GetGroups()[0].GetId()) + + _, err = s.testBench.Client.DeleteOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeleteOrganizationRequest{Id: orgID})) + s.Require().NoError(err) + + s.Run("no project row of the org is left, disabled ones included", func() { + for _, state := range []string{"", "disabled"} { + resp, err := s.testBench.AdminClient.ListProjects(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListProjectsRequest{OrgId: orgID, State: state})) + s.Require().NoError(err) + s.Assert().Empty(resp.Msg.GetProjects(), "state %q", state) + } + }) + + s.Run("no group row of the org is left, disabled ones included", func() { + for _, state := range []string{"", "disabled"} { + resp, err := s.testBench.AdminClient.ListGroups(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListGroupsRequest{OrgId: orgID, State: state})) + s.Require().NoError(err) + s.Assert().Empty(resp.Msg.GetGroups(), "state %q", state) + } + }) + + s.Run("no policy of the disabled project or group is left", func() { + for _, id := range append(disabledProjectPolicyIDs, disabledGroupPolicyIDs...) { + _, err := s.testBench.Client.GetPolicy(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.GetPolicyRequest{Id: id})) + s.Require().Error(err) + s.Assert().Equal(connect.CodeNotFound, connect.CodeOf(err)) + } + }) + + s.Run("the org is gone, a second delete is refused", func() { + _, err := s.testBench.Client.DeleteOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeleteOrganizationRequest{Id: orgID})) + s.Require().Error(err) + s.Assert().Contains([]connect.Code{connect.CodeNotFound, connect.CodePermissionDenied}, connect.CodeOf(err)) + }) +} + func TestEndToEndAPIRegressionTestSuite(t *testing.T) { suite.Run(t, new(APIRegressionTestSuite)) } From 34bea62662927373fe45ede188d35f1e8b74ecfd Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 13 Oct 2026 12:11:43 +0530 Subject: [PATCH 16/18] refactor(membership): find a disabled group with one GetByIDs call --- core/membership/group.go | 16 +++--- core/membership/group_test.go | 10 ++-- core/membership/mocks/group_service.go | 74 ++++++++++++++++++++++++++ core/membership/service.go | 1 + 4 files changed, 85 insertions(+), 16 deletions(-) diff --git a/core/membership/group.go b/core/membership/group.go index 848a694625..79003b1579 100644 --- a/core/membership/group.go +++ b/core/membership/group.go @@ -237,18 +237,14 @@ func (s *Service) OnGroupDeleted(ctx context.Context, groupID string) error { } func (s *Service) getGroupIncludingDisabled(ctx context.Context, groupID string) (group.Group, error) { - grp, err := s.groupService.Get(ctx, groupID) - if !errors.Is(err, group.ErrNotExist) { - return grp, err - } - disabled, listErr := s.groupService.List(ctx, group.Filter{GroupIDs: []string{groupID}, State: group.Disabled}) - if listErr != nil { - return group.Group{}, listErr - } - if len(disabled) == 0 { + found, err := s.groupService.GetByIDs(ctx, []string{groupID}, group.Filter{IncludeDisabled: true}) + if err != nil { return group.Group{}, err } - return disabled[0], nil + if len(found) == 0 { + return group.Group{}, group.ErrNotExist + } + return found[0], nil } // removeGroupAsPrincipalPolicies deletes every policy where the given group diff --git a/core/membership/group_test.go b/core/membership/group_test.go index 92c9594cbe..184ff804a8 100644 --- a/core/membership/group_test.go +++ b/core/membership/group_test.go @@ -484,7 +484,7 @@ func TestService_OnGroupDeleted(t *testing.T) { relSvc := mocks.NewRelationService(t) grpSvc := mocks.NewGroupService(t) - grpSvc.EXPECT().Get(ctx, groupID).Return(grp, nil) + grpSvc.EXPECT().GetByIDs(ctx, []string{groupID}, group.Filter{IncludeDisabled: true}).Return([]group.Group{grp}, nil) // RemoveAllGroupMembers — no member policies policySvc.EXPECT().List(ctx, policy.Filter{GroupID: groupID}).Return([]policy.Policy{}, nil) // removeGroupAsPrincipalPolicies — one policy granting this group access elsewhere @@ -509,15 +509,14 @@ func TestService_OnGroupDeleted(t *testing.T) { assert.NoError(t, svc.OnGroupDeleted(ctx, groupID)) }) - t.Run("cleans up a disabled group that Get does not return", func(t *testing.T) { + t.Run("cleans up a disabled group", func(t *testing.T) { policySvc := mocks.NewPolicyService(t) relSvc := mocks.NewRelationService(t) grpSvc := mocks.NewGroupService(t) disabled := grp disabled.State = group.Disabled - grpSvc.EXPECT().Get(ctx, groupID).Return(group.Group{}, group.ErrNotExist) - grpSvc.EXPECT().List(ctx, group.Filter{GroupIDs: []string{groupID}, State: group.Disabled}). + grpSvc.EXPECT().GetByIDs(ctx, []string{groupID}, group.Filter{IncludeDisabled: true}). Return([]group.Group{disabled}, nil) policySvc.EXPECT().List(ctx, policy.Filter{GroupID: groupID}).Return([]policy.Policy{}, nil) policySvc.EXPECT().List(ctx, policy.Filter{ @@ -540,8 +539,7 @@ func TestService_OnGroupDeleted(t *testing.T) { t.Run("returns error if group not found", func(t *testing.T) { grpSvc := mocks.NewGroupService(t) - grpSvc.EXPECT().Get(ctx, groupID).Return(group.Group{}, group.ErrNotExist) - grpSvc.EXPECT().List(ctx, group.Filter{GroupIDs: []string{groupID}, State: group.Disabled}). + grpSvc.EXPECT().GetByIDs(ctx, []string{groupID}, group.Filter{IncludeDisabled: true}). Return([]group.Group{}, nil) svc := membership.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), diff --git a/core/membership/mocks/group_service.go b/core/membership/mocks/group_service.go index 2e84c67651..2bf581edbc 100644 --- a/core/membership/mocks/group_service.go +++ b/core/membership/mocks/group_service.go @@ -80,6 +80,80 @@ func (_c *GroupService_Get_Call) RunAndReturn(run func(context.Context, string) return _c } +// GetByIDs provides a mock function with given fields: ctx, ids, flt +func (_m *GroupService) GetByIDs(ctx context.Context, ids []string, flt ...group.Filter) ([]group.Group, error) { + _va := make([]interface{}, len(flt)) + for _i := range flt { + _va[_i] = flt[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, ids) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + if len(ret) == 0 { + panic("no return value specified for GetByIDs") + } + + var r0 []group.Group + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, []string, ...group.Filter) ([]group.Group, error)); ok { + return rf(ctx, ids, flt...) + } + if rf, ok := ret.Get(0).(func(context.Context, []string, ...group.Filter) []group.Group); ok { + r0 = rf(ctx, ids, flt...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]group.Group) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, []string, ...group.Filter) error); ok { + r1 = rf(ctx, ids, flt...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GroupService_GetByIDs_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetByIDs' +type GroupService_GetByIDs_Call struct { + *mock.Call +} + +// GetByIDs is a helper method to define mock.On call +// - ctx context.Context +// - ids []string +// - flt ...group.Filter +func (_e *GroupService_Expecter) GetByIDs(ctx interface{}, ids interface{}, flt ...interface{}) *GroupService_GetByIDs_Call { + return &GroupService_GetByIDs_Call{Call: _e.mock.On("GetByIDs", + append([]interface{}{ctx, ids}, flt...)...)} +} + +func (_c *GroupService_GetByIDs_Call) Run(run func(ctx context.Context, ids []string, flt ...group.Filter)) *GroupService_GetByIDs_Call { + _c.Call.Run(func(args mock.Arguments) { + variadicArgs := make([]group.Filter, len(args)-2) + for i, a := range args[2:] { + if a != nil { + variadicArgs[i] = a.(group.Filter) + } + } + run(args[0].(context.Context), args[1].([]string), variadicArgs...) + }) + return _c +} + +func (_c *GroupService_GetByIDs_Call) Return(_a0 []group.Group, _a1 error) *GroupService_GetByIDs_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *GroupService_GetByIDs_Call) RunAndReturn(run func(context.Context, []string, ...group.Filter) ([]group.Group, error)) *GroupService_GetByIDs_Call { + _c.Call.Return(run) + return _c +} + // List provides a mock function with given fields: ctx, flt func (_m *GroupService) List(ctx context.Context, flt group.Filter) ([]group.Group, error) { ret := _m.Called(ctx, flt) diff --git a/core/membership/service.go b/core/membership/service.go index d1c3b941d0..4708d53a99 100644 --- a/core/membership/service.go +++ b/core/membership/service.go @@ -54,6 +54,7 @@ type ProjectService interface { type GroupService interface { Get(ctx context.Context, idOrName string) (group.Group, error) List(ctx context.Context, flt group.Filter) ([]group.Group, error) + GetByIDs(ctx context.Context, ids []string, flt ...group.Filter) ([]group.Group, error) } type ServiceuserService interface { From 48581d0b857d1696570923dbe7a1d93b546cf9bb Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 13 Oct 2026 12:12:36 +0530 Subject: [PATCH 17/18] feat(store): group and project list can include disabled rows --- core/project/filter.go | 1 + internal/store/postgres/group_repository.go | 5 +++-- .../store/postgres/group_repository_test.go | 17 +++++++++++++++ internal/store/postgres/project_repository.go | 7 ++++--- .../store/postgres/project_repository_test.go | 21 +++++++++++++++++++ 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/core/project/filter.go b/core/project/filter.go index c55cde33a8..cbdb7b9343 100644 --- a/core/project/filter.go +++ b/core/project/filter.go @@ -10,6 +10,7 @@ type Filter struct { WithMemberCount bool ProjectIDs []string State State + IncludeDisabled bool // NonInherited filters out projects that are inherited from access given through an organization NonInherited bool Pagination *pagination.Pagination diff --git a/internal/store/postgres/group_repository.go b/internal/store/postgres/group_repository.go index 8e013a877a..f19238ad24 100644 --- a/internal/store/postgres/group_repository.go +++ b/internal/store/postgres/group_repository.go @@ -178,9 +178,10 @@ func (r GroupRepository) List(ctx context.Context, flt group.Filter) ([]group.Gr if flt.OrganizationID != "" { sqlStatement = sqlStatement.Where(goqu.Ex{"org_id": flt.OrganizationID}) } - if flt.State != "" { + switch { + case flt.State != "": sqlStatement = sqlStatement.Where(goqu.Ex{"state": flt.State}) - } else { + case !flt.IncludeDisabled: sqlStatement = sqlStatement.Where(notDisabledGroupExp) } if flt.GroupIDs != nil { diff --git a/internal/store/postgres/group_repository_test.go b/internal/store/postgres/group_repository_test.go index 0450a2e6bc..8496d9b9b4 100644 --- a/internal/store/postgres/group_repository_test.go +++ b/internal/store/postgres/group_repository_test.go @@ -657,3 +657,20 @@ func (s *GroupRepositoryTestSuite) TestSkipsSoftDeletedGroups() { err = s.repository.SetState(s.ctx, deleted.ID, group.Disabled) s.Assert().ErrorIs(err, group.ErrNotExist) } + +func (s *GroupRepositoryTestSuite) TestListIncludeDisabled() { + orgID := s.orgs[1].ID + + hidden, err := s.repository.List(s.ctx, group.Filter{OrganizationID: orgID}) + s.Require().NoError(err) + s.Assert().Len(hidden, 1) + s.Assert().Equal(group.Enabled, hidden[0].State) + + all, err := s.repository.List(s.ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}) + s.Require().NoError(err) + states := map[group.State]int{} + for _, g := range all { + states[g.State]++ + } + s.Assert().Equal(map[group.State]int{group.Enabled: 1, group.Disabled: 1}, states) +} diff --git a/internal/store/postgres/project_repository.go b/internal/store/postgres/project_repository.go index d75614444b..5c2dbf833b 100644 --- a/internal/store/postgres/project_repository.go +++ b/internal/store/postgres/project_repository.go @@ -201,12 +201,13 @@ func (r ProjectRepository) List(ctx context.Context, flt project.Filter) ([]proj "id": goqu.Op{"in": flt.ProjectIDs}, }) } - if flt.State == "" { - stmt = stmt.Where(notDisabledProjectExp) - } else { + switch { + case flt.State != "": stmt = stmt.Where(goqu.Ex{ "state": flt.State.String(), }) + case !flt.IncludeDisabled: + stmt = stmt.Where(notDisabledProjectExp) } if flt.Pagination != nil { diff --git a/internal/store/postgres/project_repository_test.go b/internal/store/postgres/project_repository_test.go index 283aaeecb4..575ab21763 100644 --- a/internal/store/postgres/project_repository_test.go +++ b/internal/store/postgres/project_repository_test.go @@ -521,3 +521,24 @@ func (s *ProjectRepositoryTestSuite) TestSkipsSoftDeletedProjects() { err = s.repository.SetState(s.ctx, deleted.ID, project.Disabled) s.Assert().ErrorIs(err, project.ErrNotExist) } + +func (s *ProjectRepositoryTestSuite) TestListIncludeDisabled() { + disabled := s.projects[0] + orgID := disabled.Organization.ID + s.Require().NoError(s.repository.SetState(s.ctx, disabled.ID, project.Disabled)) + + hidden, err := s.repository.List(s.ctx, project.Filter{OrgID: orgID}) + s.Require().NoError(err) + for _, p := range hidden { + s.Assert().NotEqual(disabled.ID, p.ID) + } + + all, err := s.repository.List(s.ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}) + s.Require().NoError(err) + s.Assert().Len(all, len(hidden)+1) + ids := []string{} + for _, p := range all { + ids = append(ids, p.ID) + } + s.Assert().Contains(ids, disabled.ID) +} From f0731ddd4d70b52b5b8823da9708f1ea1468968c Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 13 Oct 2026 12:13:06 +0530 Subject: [PATCH 18/18] refactor(deleter): list enabled and disabled children in one query --- core/deleter/service.go | 20 +-------- core/deleter/service_test.go | 84 +++++++++++++++--------------------- 2 files changed, 37 insertions(+), 67 deletions(-) diff --git a/core/deleter/service.go b/core/deleter/service.go index d0347af322..089c6c17d5 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -546,27 +546,11 @@ func (d Service) DeleteUser(ctx context.Context, userID string) error { } func (d Service) listOrgProjects(ctx context.Context, orgID string) ([]project.Project, error) { - enabled, err := d.projService.List(ctx, project.Filter{OrgID: orgID}) - if err != nil { - return nil, err - } - disabled, err := d.projService.List(ctx, project.Filter{OrgID: orgID, State: project.Disabled}) - if err != nil { - return nil, err - } - return append(enabled, disabled...), nil + return d.projService.List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}) } func (d Service) listOrgGroups(ctx context.Context, orgID string) ([]group.Group, error) { - enabled, err := d.groupService.List(ctx, group.Filter{OrganizationID: orgID}) - if err != nil { - return nil, err - } - disabled, err := d.groupService.List(ctx, group.Filter{OrganizationID: orgID, State: group.Disabled}) - if err != nil { - return nil, err - } - return append(enabled, disabled...), nil + return d.groupService.List(ctx, group.Filter{OrganizationID: orgID, IncludeDisabled: true}) } // ensureDeletable collects everything that blocks deleting the organization and diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index c8ea4b4cca..b59be04f5e 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -65,10 +65,6 @@ type deleterMocks struct { func newMocks(t *testing.T) deleterMocks { t.Helper() m := newBareMocks(t) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", State: project.Disabled}). - Return([]project.Project{}, nil).Maybe() - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", State: group.Disabled}). - Return([]group.Group{}, nil).Maybe() return m } @@ -229,7 +225,7 @@ func TestDeleteOrganization(t *testing.T) { m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) // projects (each triggers DeleteProject) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{{ID: "proj-1", Name: "p1"}}, nil) m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}). Return([]policy.Policy{}, nil) @@ -238,7 +234,7 @@ func TestDeleteOrganization(t *testing.T) { m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-1").Return(nil) // groups (DeleteGroup: OnGroupDeleted then DeleteModel) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{{ID: "grp-1", Name: "g1"}}, nil) m.mbrSvc.EXPECT().OnGroupDeleted(mock.Anything, "grp-1").Return(nil) m.grpSvc.EXPECT().DeleteModel(mock.Anything, "grp-1").Return(nil) @@ -307,10 +303,8 @@ func TestDeleteOrganization(t *testing.T) { m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). - Return([]project.Project{{ID: "proj-on", Name: "on"}}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", State: project.Disabled}). - Return([]project.Project{{ID: "proj-off", Name: "off", State: project.Disabled}}, nil) + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). + Return([]project.Project{{ID: "proj-on", Name: "on"}, {ID: "proj-off", Name: "off", State: project.Disabled}}, nil) for _, id := range []string{"proj-on", "proj-off"} { m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: id}). Return([]policy.Policy{}, nil) @@ -320,10 +314,8 @@ func TestDeleteOrganization(t *testing.T) { Run(func(_ context.Context, id string) { steps = append(steps, "project:"+id) }).Return(nil) } - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). - Return([]group.Group{{ID: "grp-on", Name: "on"}}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", State: group.Disabled}). - Return([]group.Group{{ID: "grp-off", Name: "off", State: group.Disabled}}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). + Return([]group.Group{{ID: "grp-on", Name: "on"}, {ID: "grp-off", Name: "off", State: group.Disabled}}, nil) for _, id := range []string{"grp-on", "grp-off"} { m.mbrSvc.EXPECT().OnGroupDeleted(mock.Anything, id).Return(nil) m.grpSvc.EXPECT().DeleteModel(mock.Anything, id). @@ -347,20 +339,16 @@ func TestDeleteOrganization(t *testing.T) { Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{{ID: "proj-on", Name: "on"}}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", State: project.Disabled}). - Return([]project.Project{}, nil) m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-on"}). Return([]policy.Policy{}, nil) m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-on", IncludeDeleted: true}). Return([]resource.Resource{}, nil) m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-on"). Run(func(_ context.Context, id string) { steps = append(steps, "project:"+id) }).Return(nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{{ID: "grp-on", Name: "on"}}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", State: group.Disabled}). - Return([]group.Group{}, nil) m.mbrSvc.EXPECT().OnGroupDeleted(mock.Anything, "grp-on").Return(nil) m.grpSvc.EXPECT().DeleteModel(mock.Anything, "grp-on"). Run(func(_ context.Context, id string) { steps = append(steps, "group:"+id) }).Return(nil) @@ -379,9 +367,7 @@ func TestDeleteOrganization(t *testing.T) { Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). - Return([]project.Project{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", State: project.Disabled}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{{ID: "proj-off", Name: "off", State: project.Disabled}}, nil) m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-off"}). Return([]policy.Policy{}, nil) @@ -546,9 +532,9 @@ func TestDeleteOrganization(t *testing.T) { m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{}, nil) @@ -596,9 +582,9 @@ func TestDeleteOrganization(t *testing.T) { m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{}, nil) @@ -700,9 +686,9 @@ func TestDeleteOrganization(t *testing.T) { m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{}, nil) @@ -805,9 +791,9 @@ func TestDeleteOrganization(t *testing.T) { Return(organization.Organization{ID: "org-1", State: organization.Disabled}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{}, nil) @@ -857,9 +843,9 @@ func TestDeleteOrganization(t *testing.T) { Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return(nil, errors.New("su list failed")) @@ -879,9 +865,9 @@ func TestDeleteOrganization(t *testing.T) { Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{{ID: "su-1"}}, nil) @@ -903,9 +889,9 @@ func TestDeleteOrganization(t *testing.T) { Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{}, nil) @@ -946,9 +932,9 @@ func TestDeleteOrganization(t *testing.T) { Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{}, nil) @@ -972,9 +958,9 @@ func TestDeleteOrganization(t *testing.T) { Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{}, nil) @@ -1000,9 +986,9 @@ func TestDeleteOrganization(t *testing.T) { Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{}, nil) @@ -1034,9 +1020,9 @@ func TestDeleteOrganization(t *testing.T) { Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{}, nil) @@ -1193,9 +1179,9 @@ func TestForfeitAuditRecord(t *testing.T) { m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{}, nil) @@ -1244,9 +1230,9 @@ func TestForfeitAuditRecord(t *testing.T) { Return(organization.Organization{ID: "org-1", Title: "Org One"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) - m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1", IncludeDisabled: true}). Return([]project.Project{}, nil) - m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1", IncludeDisabled: true}). Return([]group.Group{}, nil) m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return([]serviceuser.ServiceUser{}, nil)