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..f8218be686 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, @@ -345,6 +354,23 @@ 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 { + if errors.Is(err, domain.ErrNotExist) { + continue + } + 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{ diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index 0aabc63786..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" @@ -19,6 +20,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 +50,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 +75,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 +99,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 +239,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 +433,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 +483,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 +587,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 +769,164 @@ func TestDeleteOrganization(t *testing.T) { assert.ErrorContains(t, err, "su delete failed") assert.ErrorContains(t, err, "su-1") }) + + 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"). + 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{"domain", "pats", "policy", "role", "org"}, steps) + }) + + 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"). + 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: "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("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) + + 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{}, 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 +1080,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 +1131,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). diff --git a/core/userpat/mocks/repository.go b/core/userpat/mocks/repository.go index a9f9e43bd4..3faadf55e5 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 } +// 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 ListAllByOrg") + } + + 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_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 +} + +// ListAllByOrg is a helper method to define mock.On call +// - ctx context.Context +// - orgID string +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_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_ListAllByOrg_Call) Return(_a0 []models.PAT, _a1 error) *Repository_ListAllByOrg_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *Repository_ListAllByOrg_Call) RunAndReturn(run func(context.Context, string) ([]models.PAT, error)) *Repository_ListAllByOrg_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/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 ebb0e48c94..d2ba2033e2 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" @@ -150,17 +151,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 +184,44 @@ 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.ListAllByOrg(ctx, orgID) + if err != nil { + return fmt.Errorf("listing PATs for org: %w", err) + } + for _, pat := range pats { + 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 993bb26c07..17fdcda316 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,198 @@ 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().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().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") + }) + + 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().ListAllByOrg(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().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") + 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().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")) + + 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") + }) + + 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)}, + } + 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-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() + + assert.NoError(t, svc.DeleteAllByOrg(context.Background(), "org-1")) + }) + + 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-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).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) + }) +} + 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 d1236339b3..201999925d 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) + 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/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() { 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 d3b94b16d5..4d01c5fe29 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) 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, "ListAllByOrg", 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..72a29be5e6 100644 --- a/internal/store/postgres/userpat_repository_test.go +++ b/internal/store/postgres/userpat_repository_test.go @@ -351,6 +351,56 @@ func (s *UserPATRepositoryTestSuite) TestListByUser_EmptyWhenNoTokens() { s.Empty(got) } +func (s *UserPATRepositoryTestSuite) TestListAllByOrg_ReturnsLiveAndDeletedPATsOfTheOrg() { + s.truncateTokens() + + var idsInOrg0 []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 { + idsInOrg0 = append(idsInOrg0, 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)) + idsInOrg0 = append(idsInOrg0, deleted.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(idsInOrg0, gotIDs) + + none, err := s.repository.ListAllByOrg(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() diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index 0056543bc3..3bcb250e9f 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) TestOrganizationDeleteHidesTheOrgAndRemovesWhatItOwned() { + 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)) } diff --git a/test/e2e/regression/pat_test.go b/test/e2e/regression/pat_test.go index b34fe6b82e..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") @@ -771,6 +781,56 @@ 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)) + 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) + 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("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)) + }) +} + func TestEndToEndPATRegressionTestSuite(t *testing.T) { suite.Run(t, new(PATRegressionTestSuite)) }