diff --git a/cmd/serve.go b/cmd/serve.go index 0bb9540d1a..e321818537 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -577,8 +577,8 @@ func buildAPIDependencies( ) cascadeDeleter := deleter.NewCascadeDeleter(organizationService, projectService, resourceService, - groupService, membershipService, policyService, roleService, invitationService, userService, serviceUserService, - customerService, subscriptionService, invoiceService, + groupService, membershipService, policyService, roleService, invitationService, userService, userPATService, + serviceUserService, customerService, subscriptionService, invoiceService, ) // we should default it with a stdout logger repository as postgres can start to bloat really fast diff --git a/core/deleter/mocks/user_pat_service.go b/core/deleter/mocks/user_pat_service.go new file mode 100644 index 0000000000..eb38761c72 --- /dev/null +++ b/core/deleter/mocks/user_pat_service.go @@ -0,0 +1,83 @@ +// Code generated by mockery v2.53.5. DO NOT EDIT. + +package mocks + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" +) + +// UserPATService is an autogenerated mock type for the UserPATService type +type UserPATService struct { + mock.Mock +} + +type UserPATService_Expecter struct { + mock *mock.Mock +} + +func (_m *UserPATService) EXPECT() *UserPATService_Expecter { + return &UserPATService_Expecter{mock: &_m.Mock} +} + +// 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) + + if len(ret) == 0 { + panic("no return value specified for DeleteAllByUser") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = rf(ctx, userID) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// UserPATService_DeleteAllByUser_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteAllByUser' +type UserPATService_DeleteAllByUser_Call struct { + *mock.Call +} + +// DeleteAllByUser is a helper method to define mock.On call +// - ctx context.Context +// - userID string +func (_e *UserPATService_Expecter) DeleteAllByUser(ctx interface{}, userID interface{}) *UserPATService_DeleteAllByUser_Call { + return &UserPATService_DeleteAllByUser_Call{Call: _e.mock.On("DeleteAllByUser", ctx, userID)} +} + +func (_c *UserPATService_DeleteAllByUser_Call) Run(run func(ctx context.Context, userID string)) *UserPATService_DeleteAllByUser_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *UserPATService_DeleteAllByUser_Call) Return(_a0 error) *UserPATService_DeleteAllByUser_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *UserPATService_DeleteAllByUser_Call) RunAndReturn(run func(context.Context, string) error) *UserPATService_DeleteAllByUser_Call { + _c.Call.Return(run) + return _c +} + +// NewUserPATService creates a new instance of UserPATService. 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 NewUserPATService(t interface { + mock.TestingT + Cleanup(func()) +}) *UserPATService { + mock := &UserPATService{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/core/deleter/service.go b/core/deleter/service.go index 4cbee75b3f..f1490b8578 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -83,6 +83,10 @@ type UserService interface { Delete(ctx context.Context, id string) error } +type UserPATService interface { + DeleteAllByUser(ctx context.Context, userID string) error +} + type ServiceUserService interface { List(ctx context.Context, flt serviceuser.Filter) ([]serviceuser.ServiceUser, error) Delete(ctx context.Context, id string) error @@ -112,6 +116,7 @@ type Service struct { roleService RoleService invitationService InvitationService userService UserService + userPATService UserPATService serviceUserService ServiceUserService customerService CustomerService subService SubscriptionService @@ -123,6 +128,7 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic membershipService MembershipService, policyService PolicyService, roleService RoleService, invitationService InvitationService, userService UserService, + userPATService UserPATService, serviceUserService ServiceUserService, customerService CustomerService, subService SubscriptionService, invoiceService InvoiceService) *Service { @@ -136,6 +142,7 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic roleService: roleService, invitationService: invitationService, userService: userService, + userPATService: userPATService, serviceUserService: serviceUserService, customerService: customerService, subService: subService, @@ -394,6 +401,9 @@ func (d Service) DeleteUser(ctx context.Context, userID string) error { return fmt.Errorf("failed to delete user from org[%s]: %w", orgID, err) } } + if err := d.userPATService.DeleteAllByUser(ctx, userID); err != nil { + return fmt.Errorf("failed to delete user PATs: %w", err) + } return d.userService.Delete(ctx, userID) } diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index fb16ec2ae9..459324823a 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -32,6 +32,7 @@ func newMocks(t *testing.T) ( *mocks.RoleService, *mocks.InvitationService, *mocks.UserService, + *mocks.UserPATService, *mocks.ServiceUserService, *mocks.CustomerService, *mocks.SubscriptionService, @@ -47,6 +48,7 @@ func newMocks(t *testing.T) ( mocks.NewRoleService(t), mocks.NewInvitationService(t), mocks.NewUserService(t), + mocks.NewUserPATService(t), mocks.NewServiceUserService(t), mocks.NewCustomerService(t), mocks.NewSubscriptionService(t), @@ -55,7 +57,7 @@ func newMocks(t *testing.T) ( func TestDeleteProject(t *testing.T) { t.Run("deletes policies, resources, then project model", func(t *testing.T) { - orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) + orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}). Return([]policy.Policy{{ID: "pol-1"}, {ID: "pol-2"}}, nil) @@ -68,38 +70,38 @@ func TestDeleteProject(t *testing.T) { projSvc.EXPECT().DeleteModel(mock.Anything, "proj-1").Return(nil) - svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc) + svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc) err := svc.DeleteProject(context.Background(), "proj-1") assert.NoError(t, err) }) t.Run("returns error when policy list fails", func(t *testing.T) { - _, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) + _, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) orgSvc := mocks.NewOrganizationService(t) polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}). Return(nil, errors.New("db error")) - svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc) + svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc) err := svc.DeleteProject(context.Background(), "proj-1") assert.ErrorContains(t, err, "db error") }) t.Run("returns error when policy delete fails", func(t *testing.T) { - _, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) + _, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) orgSvc := mocks.NewOrganizationService(t) polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}). Return([]policy.Policy{{ID: "pol-fail"}}, nil) polSvc.EXPECT().Delete(mock.Anything, "pol-fail").Return(errors.New("delete error")) - svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc) + svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc) err := svc.DeleteProject(context.Background(), "proj-1") assert.ErrorContains(t, err, "pol-fail") }) t.Run("no policies — still deletes resources and project", func(t *testing.T) { - orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) + orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}). Return([]policy.Policy{}, nil) @@ -107,7 +109,7 @@ func TestDeleteProject(t *testing.T) { Return([]resource.Resource{}, nil) projSvc.EXPECT().DeleteModel(mock.Anything, "proj-1").Return(nil) - svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc) + svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc) err := svc.DeleteProject(context.Background(), "proj-1") assert.NoError(t, err) }) @@ -115,7 +117,7 @@ func TestDeleteProject(t *testing.T) { func TestDeleteOrganization(t *testing.T) { t.Run("full cascade delete", func(t *testing.T) { - orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) + orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) // canDelete: no customers custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). @@ -165,13 +167,13 @@ func TestDeleteOrganization(t *testing.T) { // finally delete org model orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) - svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc) + svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc) err := svc.DeleteOrganization(context.Background(), "org-1") assert.NoError(t, err) }) t.Run("blocked when billed customer has invoices", func(t *testing.T) { - _, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) + _, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) orgSvc := mocks.NewOrganizationService(t) custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). @@ -179,13 +181,13 @@ func TestDeleteOrganization(t *testing.T) { invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1"}). Return([]invoice.Invoice{{ID: "inv-1"}}, nil) - svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc) + svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc) err := svc.DeleteOrganization(context.Background(), "org-1") assert.ErrorIs(t, err, deleter.ErrDeleteNotAllowed) }) t.Run("propagates error when service user list fails", func(t *testing.T) { - orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) + orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) @@ -198,13 +200,13 @@ func TestDeleteOrganization(t *testing.T) { suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). Return(nil, errors.New("su list failed")) - svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc) + svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc) err := svc.DeleteOrganization(context.Background(), "org-1") assert.ErrorContains(t, err, "su list failed") }) t.Run("propagates error when service user delete fails", func(t *testing.T) { - orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) + orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) @@ -218,7 +220,7 @@ func TestDeleteOrganization(t *testing.T) { Return([]serviceuser.ServiceUser{{ID: "su-1"}}, nil) suSvc.EXPECT().Delete(mock.Anything, "su-1").Return(errors.New("su delete failed")) - svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc) + svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc) err := svc.DeleteOrganization(context.Background(), "org-1") assert.ErrorContains(t, err, "su delete failed") assert.ErrorContains(t, err, "su-1") @@ -227,7 +229,7 @@ func TestDeleteOrganization(t *testing.T) { func TestDeleteCustomers(t *testing.T) { t.Run("deletes subscriptions invoices and customer", func(t *testing.T) { - orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) + orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). @@ -236,13 +238,13 @@ func TestDeleteCustomers(t *testing.T) { invocSvc.EXPECT().DeleteByCustomer(mock.Anything, c).Return(nil) custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) - svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc) + svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc) err := svc.DeleteCustomers(context.Background(), "org-1") assert.NoError(t, err) }) t.Run("skips subscription and invoice delete when no provider", func(t *testing.T) { - orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) + orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) c := customer.Customer{ID: "cust-no-provider", ProviderID: ""} custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). @@ -250,22 +252,36 @@ func TestDeleteCustomers(t *testing.T) { // no sub or invoice delete expected custSvc.EXPECT().Delete(mock.Anything, "cust-no-provider").Return(nil) - svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc) + svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc) err := svc.DeleteCustomers(context.Background(), "org-1") assert.NoError(t, err) }) } func TestDeleteUser(t *testing.T) { - t.Run("removes user from all orgs then deletes", func(t *testing.T) { - orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) + t.Run("removes user from all orgs, cleans PATs, then deletes user", func(t *testing.T) { + orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) mbrSvc.EXPECT().ListResourcesByPrincipal(mock.Anything, mock.Anything, schema.OrganizationNamespace, mock.Anything). Return(nil, nil) + patSvc.EXPECT().DeleteAllByUser(mock.Anything, "user-1").Return(nil) usrSvc.EXPECT().Delete(mock.Anything, "user-1").Return(nil) - svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, suSvc, custSvc, subSvc, invocSvc) + svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc) err := svc.DeleteUser(context.Background(), "user-1") assert.NoError(t, err) }) + + t.Run("aborts before userService.Delete when PAT cleanup fails", func(t *testing.T) { + orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc := newMocks(t) + + mbrSvc.EXPECT().ListResourcesByPrincipal(mock.Anything, mock.Anything, schema.OrganizationNamespace, mock.Anything). + Return(nil, nil) + patSvc.EXPECT().DeleteAllByUser(mock.Anything, "user-1").Return(errors.New("pat cleanup boom")) + // usrSvc.Delete must NOT be called — strict mock fails on unexpected call. + + svc := deleter.NewCascadeDeleter(orgSvc, projSvc, resSvc, grpSvc, mbrSvc, polSvc, roleSvc, invSvc, usrSvc, patSvc, suSvc, custSvc, subSvc, invocSvc) + err := svc.DeleteUser(context.Background(), "user-1") + assert.ErrorContains(t, err, "pat cleanup boom") + }) } diff --git a/core/userpat/mocks/repository.go b/core/userpat/mocks/repository.go index 0708fca020..a9f9e43bd4 100644 --- a/core/userpat/mocks/repository.go +++ b/core/userpat/mocks/repository.go @@ -420,6 +420,65 @@ func (_c *Repository_List_Call) RunAndReturn(run func(context.Context, string, s 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) + + if len(ret) == 0 { + panic("no return value specified for ListByUser") + } + + var r0 []models.PAT + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) ([]models.PAT, error)); ok { + return rf(ctx, userID) + } + if rf, ok := ret.Get(0).(func(context.Context, string) []models.PAT); ok { + r0 = rf(ctx, userID) + } 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, userID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Repository_ListByUser_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListByUser' +type Repository_ListByUser_Call struct { + *mock.Call +} + +// ListByUser is a helper method to define mock.On call +// - ctx context.Context +// - userID string +func (_e *Repository_Expecter) ListByUser(ctx interface{}, userID interface{}) *Repository_ListByUser_Call { + return &Repository_ListByUser_Call{Call: _e.mock.On("ListByUser", ctx, userID)} +} + +func (_c *Repository_ListByUser_Call) Run(run func(ctx context.Context, userID string)) *Repository_ListByUser_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *Repository_ListByUser_Call) Return(_a0 []models.PAT, _a1 error) *Repository_ListByUser_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *Repository_ListByUser_Call) RunAndReturn(run func(context.Context, string) ([]models.PAT, error)) *Repository_ListByUser_Call { + _c.Call.Return(run) + return _c +} + // ListExpiredNoticePending provides a mock function with given fields: ctx func (_m *Repository) ListExpiredNoticePending(ctx context.Context) ([]models.PAT, error) { ret := _m.Called(ctx) diff --git a/core/userpat/service.go b/core/userpat/service.go index 3fc2d8a873..cd2f8fbd45 100644 --- a/core/userpat/service.go +++ b/core/userpat/service.go @@ -7,12 +7,11 @@ import ( "encoding/hex" "fmt" "io" + "log/slog" "maps" "slices" "time" - "log/slog" - "github.com/raystack/frontier/core/auditrecord/models" "github.com/raystack/frontier/core/authenticate" "github.com/raystack/frontier/core/organization" @@ -144,9 +143,6 @@ func (s *Service) Get(ctx context.Context, userID, id string) (patmodels.PAT, er // Soft-delete before policy cleanup prevents concurrent Update from re-creating // policies for a deleted PAT (TOCTOU mitigation). func (s *Service) Delete(ctx context.Context, userID, id string) error { - if !s.config.Enabled { - return paterrors.ErrDisabled - } pat, err := s.repo.GetByID(ctx, id) if err != nil { return err @@ -170,6 +166,20 @@ func (s *Service) Delete(ctx context.Context, userID, id string) error { return nil } +// DeleteAllByUser deletes every PAT owned by the user via the per-PAT cascade. +func (s *Service) DeleteAllByUser(ctx context.Context, userID string) error { + pats, err := s.repo.ListByUser(ctx, userID) + if err != nil { + return fmt.Errorf("listing PATs for user: %w", err) + } + for _, pat := range pats { + if err := s.Delete(ctx, userID, pat.ID); 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 be832e8cf4..649513a5e9 100644 --- a/core/userpat/service_test.go +++ b/core/userpat/service_test.go @@ -23,6 +23,7 @@ import ( "github.com/raystack/frontier/core/userpat/mocks" "github.com/raystack/frontier/core/userpat/models" "github.com/raystack/frontier/internal/bootstrap/schema" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "golang.org/x/crypto/sha3" ) @@ -1770,21 +1771,6 @@ func TestService_Delete(t *testing.T) { wantErr bool wantErrIs error }{ - { - name: "should return ErrDisabled when PAT feature is disabled", - userID: "user-1", - patID: "pat-1", - setup: func() *userpat.Service { - repo := mocks.NewRepository(t) - orgSvc := mocks.NewOrganizationService(t) - auditRepo := mocks.NewAuditRecordRepository(t) - return userpat.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), repo, userpat.Config{ - Enabled: false, - }, orgSvc, nil, nil, nil, auditRepo) - }, - wantErr: true, - wantErrIs: paterrors.ErrDisabled, - }, { name: "should return ErrNotFound when PAT does not exist", userID: "user-1", @@ -1916,6 +1902,70 @@ func TestService_Delete(t *testing.T) { } } +func TestService_DeleteAllByUser(t *testing.T) { + t.Run("no-op when user has no PATs", func(t *testing.T) { + repo := mocks.NewRepository(t) + repo.EXPECT().ListByUser(mock.Anything, "user-1").Return(nil, nil) + svc := userpat.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), repo, defaultConfig, + mocks.NewOrganizationService(t), nil, nil, nil, mocks.NewAuditRecordRepository(t)) + assert.NoError(t, svc.DeleteAllByUser(context.Background(), "user-1")) + }) + + t.Run("returns error when repo list fails", func(t *testing.T) { + repo := mocks.NewRepository(t) + repo.EXPECT().ListByUser(mock.Anything, "user-1").Return(nil, errors.New("db down")) + svc := userpat.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), repo, defaultConfig, + mocks.NewOrganizationService(t), nil, nil, nil, mocks.NewAuditRecordRepository(t)) + err := svc.DeleteAllByUser(context.Background(), "user-1") + assert.ErrorContains(t, err, "listing PATs for user") + assert.ErrorContains(t, err, "db down") + }) + + t.Run("deletes every PAT, cleaning policies and recording audit per PAT", func(t *testing.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-1", OrgID: "org-2", Title: "t2", ExpiresAt: time.Now().Add(time.Hour)}, + } + repo := mocks.NewRepository(t) + repo.EXPECT().ListByUser(mock.Anything, "user-1").Return(pats, nil) + repo.EXPECT().GetByID(mock.Anything, "pat-1").Return(pats[0], nil) + repo.EXPECT().GetByID(mock.Anything, "pat-2").Return(pats[1], nil) + repo.EXPECT().Delete(mock.Anything, "pat-1").Return(nil) + repo.EXPECT().Delete(mock.Anything, "pat-2").Return(nil) + + membershipSvc := mocks.NewMembershipService(t) + membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-1").Return(nil) + membershipSvc.EXPECT().RemoveAllPATPolicies(mock.Anything, "pat-2").Return(nil) + + 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) + auditRepo.On("Create", mock.Anything, mock.Anything). + Return(auditmodels.AuditRecord{}, nil).Maybe() + + svc := userpat.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), repo, defaultConfig, + orgSvc, nil, membershipSvc, nil, auditRepo) + assert.NoError(t, svc.DeleteAllByUser(context.Background(), "user-1")) + }) + + t.Run("aborts the cascade when a per-PAT delete fails", func(t *testing.T) { + pats := []models.PAT{ + {ID: "pat-bad", UserID: "user-1", OrgID: "org-1", Title: "t1", ExpiresAt: time.Now().Add(time.Hour)}, + } + repo := mocks.NewRepository(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")) + + svc := userpat.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), repo, defaultConfig, + mocks.NewOrganizationService(t), nil, mocks.NewMembershipService(t), 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_Update(t *testing.T) { testPAT := models.PAT{ ID: "pat-1", diff --git a/core/userpat/userpat.go b/core/userpat/userpat.go index 902ec4325d..d1236339b3 100644 --- a/core/userpat/userpat.go +++ b/core/userpat/userpat.go @@ -13,6 +13,7 @@ type Repository interface { CountActive(ctx context.Context, userID, orgID string) (int64, error) 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) 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 7b19c292cd..e4964d43f1 100644 --- a/internal/store/postgres/userpat_repository.go +++ b/internal/store/postgres/userpat_repository.go @@ -408,6 +408,32 @@ func (r UserPATRepository) ListExpiredNoticePending(ctx context.Context) ([]mode return pats, nil } +// ListByUser returns all active PATs for a user across every org. +func (r UserPATRepository) ListByUser(ctx context.Context, userID string) ([]models.PAT, error) { + query, params, err := dialect.From(TABLE_USER_PATS).Where( + goqu.Ex{"user_id": userID}, + goqu.Ex{"deleted_at": nil}, + ).ToSQL() + if err != nil { + return nil, fmt.Errorf("%w: %w", queryErr, err) + } + var rows []UserPAT + if err = r.dbc.WithTimeout(ctx, TABLE_USER_PATS, "ListByUser", func(ctx context.Context) error { + return r.dbc.SelectContext(ctx, &rows, query, params...) + }); err != nil { + return nil, fmt.Errorf("%w: %w", dbErr, 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 005fb3b886..752031fc11 100644 --- a/internal/store/postgres/userpat_repository_test.go +++ b/internal/store/postgres/userpat_repository_test.go @@ -313,6 +313,51 @@ func (s *UserPATRepositoryTestSuite) TestCountActive_MultipleTokens() { s.Equal(int64(3), count) } +func (s *UserPATRepositoryTestSuite) TestListByUser_ReturnsActivePATsAcrossOrgs() { + s.truncateTokens() + + created := make(map[string]bool) + for _, spec := range []struct { + user, org, title, hash string + }{ + {s.users[0].ID, s.orgs[0].ID, "u0-o0-a", "hashU0O0A"}, + {s.users[0].ID, s.orgs[1].ID, "u0-o1-a", "hashU0O1A"}, + {s.users[1].ID, s.orgs[0].ID, "u1-o0-a", "hashU1O0A"}, + } { + 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.user == s.users[0].ID { + created[pat.ID] = true + } + } + + // Soft-delete one of user[0]'s tokens — ListByUser must skip it. + for id := range created { + s.Require().NoError(s.repository.Delete(s.ctx, id)) + delete(created, id) + break + } + + got, err := s.repository.ListByUser(s.ctx, s.users[0].ID) + s.Require().NoError(err) + s.Require().Len(got, 1) + s.Equal(s.users[0].ID, got[0].UserID) +} + +func (s *UserPATRepositoryTestSuite) TestListByUser_EmptyWhenNoTokens() { + s.truncateTokens() + + got, err := s.repository.ListByUser(s.ctx, s.users[0].ID) + s.Require().NoError(err) + s.Empty(got) +} + func TestUserPATRepository(t *testing.T) { suite.Run(t, new(UserPATRepositoryTestSuite)) }