diff --git a/core/deleter/deleter.go b/core/deleter/deleter.go index e63abf68e..e00457078 100644 --- a/core/deleter/deleter.go +++ b/core/deleter/deleter.go @@ -89,9 +89,8 @@ func labelWithOrg(orgName string, b Blocker) Blocker { return b } -// BlockedError carries every blocker the up-front check found, so the -// caller gets one checklist instead of discovering blockers one retry at -// a time. +// BlockedError carries the blockers that stop a delete, so the caller gets +// one checklist instead of discovering blockers one retry at a time. type BlockedError struct { OrgID string ProjectID string diff --git a/core/deleter/mocks/membership_service.go b/core/deleter/mocks/membership_service.go index c99693ae9..406890b0c 100644 --- a/core/deleter/mocks/membership_service.go +++ b/core/deleter/mocks/membership_service.go @@ -24,55 +24,6 @@ func (_m *MembershipService) EXPECT() *MembershipService_Expecter { return &MembershipService_Expecter{mock: &_m.Mock} } -// ForceRemoveOrganizationMember provides a mock function with given fields: ctx, orgID, principalID, principalType -func (_m *MembershipService) ForceRemoveOrganizationMember(ctx context.Context, orgID string, principalID string, principalType string) error { - ret := _m.Called(ctx, orgID, principalID, principalType) - - if len(ret) == 0 { - panic("no return value specified for ForceRemoveOrganizationMember") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, string) error); ok { - r0 = rf(ctx, orgID, principalID, principalType) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// MembershipService_ForceRemoveOrganizationMember_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ForceRemoveOrganizationMember' -type MembershipService_ForceRemoveOrganizationMember_Call struct { - *mock.Call -} - -// ForceRemoveOrganizationMember is a helper method to define mock.On call -// - ctx context.Context -// - orgID string -// - principalID string -// - principalType string -func (_e *MembershipService_Expecter) ForceRemoveOrganizationMember(ctx interface{}, orgID interface{}, principalID interface{}, principalType interface{}) *MembershipService_ForceRemoveOrganizationMember_Call { - return &MembershipService_ForceRemoveOrganizationMember_Call{Call: _e.mock.On("ForceRemoveOrganizationMember", ctx, orgID, principalID, principalType)} -} - -func (_c *MembershipService_ForceRemoveOrganizationMember_Call) Run(run func(ctx context.Context, orgID string, principalID string, principalType string)) *MembershipService_ForceRemoveOrganizationMember_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(string)) - }) - return _c -} - -func (_c *MembershipService_ForceRemoveOrganizationMember_Call) Return(_a0 error) *MembershipService_ForceRemoveOrganizationMember_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *MembershipService_ForceRemoveOrganizationMember_Call) RunAndReturn(run func(context.Context, string, string, string) error) *MembershipService_ForceRemoveOrganizationMember_Call { - _c.Call.Return(run) - return _c -} - // ListPrincipalsByResource provides a mock function with given fields: ctx, resourceID, resourceType, filter func (_m *MembershipService) ListPrincipalsByResource(ctx context.Context, resourceID string, resourceType string, filter membership.MemberFilter) ([]membership.Member, error) { ret := _m.Called(ctx, resourceID, resourceType, filter) @@ -242,6 +193,55 @@ func (_c *MembershipService_OnGroupDeleted_Call) RunAndReturn(run func(context.C return _c } +// RemoveOrganizationMemberOnDelete provides a mock function with given fields: ctx, orgID, principalID, principalType +func (_m *MembershipService) RemoveOrganizationMemberOnDelete(ctx context.Context, orgID string, principalID string, principalType string) error { + ret := _m.Called(ctx, orgID, principalID, principalType) + + if len(ret) == 0 { + panic("no return value specified for RemoveOrganizationMemberOnDelete") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string, string) error); ok { + r0 = rf(ctx, orgID, principalID, principalType) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MembershipService_RemoveOrganizationMemberOnDelete_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RemoveOrganizationMemberOnDelete' +type MembershipService_RemoveOrganizationMemberOnDelete_Call struct { + *mock.Call +} + +// RemoveOrganizationMemberOnDelete is a helper method to define mock.On call +// - ctx context.Context +// - orgID string +// - principalID string +// - principalType string +func (_e *MembershipService_Expecter) RemoveOrganizationMemberOnDelete(ctx interface{}, orgID interface{}, principalID interface{}, principalType interface{}) *MembershipService_RemoveOrganizationMemberOnDelete_Call { + return &MembershipService_RemoveOrganizationMemberOnDelete_Call{Call: _e.mock.On("RemoveOrganizationMemberOnDelete", ctx, orgID, principalID, principalType)} +} + +func (_c *MembershipService_RemoveOrganizationMemberOnDelete_Call) Run(run func(ctx context.Context, orgID string, principalID string, principalType string)) *MembershipService_RemoveOrganizationMemberOnDelete_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(string)) + }) + return _c +} + +func (_c *MembershipService_RemoveOrganizationMemberOnDelete_Call) Return(_a0 error) *MembershipService_RemoveOrganizationMemberOnDelete_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MembershipService_RemoveOrganizationMemberOnDelete_Call) RunAndReturn(run func(context.Context, string, string, string) error) *MembershipService_RemoveOrganizationMemberOnDelete_Call { + _c.Call.Return(run) + return _c +} + // RemovePrincipalPolicies provides a mock function with given fields: ctx, principalID, principalType func (_m *MembershipService) RemovePrincipalPolicies(ctx context.Context, principalID string, principalType string) error { ret := _m.Called(ctx, principalID, principalType) diff --git a/core/deleter/service.go b/core/deleter/service.go index dd35825cc..f0e267cc3 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -84,7 +84,7 @@ type MembershipService interface { OnGroupDeleted(ctx context.Context, groupID string) error ListResourcesByPrincipal(ctx context.Context, principal authenticate.Principal, resourceType string, filter membership.ResourceFilter) ([]string, error) ListPrincipalsByResource(ctx context.Context, resourceID, resourceType string, filter membership.MemberFilter) ([]membership.Member, error) - ForceRemoveOrganizationMember(ctx context.Context, orgID, principalID, principalType string) error + RemoveOrganizationMemberOnDelete(ctx context.Context, orgID, principalID, principalType string) error RemovePrincipalPolicies(ctx context.Context, principalID, principalType string) error } @@ -542,21 +542,6 @@ func (d Service) deleteCustomers(ctx context.Context, id string, customers []cus return nil } -// RemoveUsersFromOrg removes users from an organization as members. The policy -// and relation cleanup — org, project, group, and custom resource — is -// delegated to membership.ForceRemoveOrganizationMember. It uses the force -// variant because a deletion cascade must succeed even when the user is the -// org's last owner. -func (d Service) RemoveUsersFromOrg(ctx context.Context, orgID string, userIDs []string) error { - var errs error - for _, userID := range userIDs { - if memberErr := d.membershipService.ForceRemoveOrganizationMember(ctx, orgID, userID, schema.UserPrincipal); memberErr != nil { - errs = errors.Join(errs, memberErr) - } - } - return errs -} - func (d Service) CheckUserDelete(ctx context.Context, userID string) (UserDeleteCheck, error) { check, err := d.checkUser(ctx, userID, false) if err != nil { @@ -626,18 +611,22 @@ func (d Service) checkUserOrg(ctx context.Context, userID, orgID string, fromPro toLeave.RoleTitle = self.Roles[0].Title } if isOrgOwner(*self) && otherOwners == 0 { - toLeave.Blockers = []Blocker{{ - Type: BlockerSoleOwnerWithMembers, - Subject: org.ID, - SubjectType: SubjectOrganization, - Message: fmt.Sprintf("organization[%s] has other members and this user is its only owner: make another member an owner or remove the other members, then retry the delete", org.Name), - }} + toLeave.Blockers = []Blocker{soleOwnerBlocker(org)} } check.OrganizationsToLeave = append(check.OrganizationsToLeave, toLeave) } return nil } +func soleOwnerBlocker(org organization.Organization) Blocker { + return Blocker{ + Type: BlockerSoleOwnerWithMembers, + Subject: org.ID, + SubjectType: SubjectOrganization, + Message: fmt.Sprintf("organization[%s] has other members and this user is its only owner: make another member an owner or remove the other members, then retry the delete", org.Name), + } +} + func (d Service) existingOrgUsers(ctx context.Context, orgID string) ([]membership.Member, error) { members, err := d.membershipService.ListPrincipalsByResource(ctx, orgID, schema.OrganizationNamespace, membership.MemberFilter{ PrincipalType: schema.UserPrincipal, @@ -660,8 +649,8 @@ func isOrgOwner(m membership.Member) bool { return slices.ContainsFunc(m.Roles, func(r role.Role) bool { return r.Name == schema.RoleOrganizationOwner }) } -// DeleteUser visits every org the user has a policy on (disabled orgs too), -// otherwise userService.Delete would leave orphan policy rows behind. +// DeleteUser stops on any blocker the check finds. Otherwise it deletes the orgs +// where the user is the only member and leaves the others, disabled orgs too. func (d Service) DeleteUser(ctx context.Context, userID string) error { check, err := d.checkUser(ctx, userID, true) if err != nil { @@ -676,7 +665,7 @@ func (d Service) DeleteUser(ctx context.Context, userID string) error { } } for _, o := range check.OrganizationsToLeave { - if err := d.RemoveUsersFromOrg(ctx, o.Organization.ID, []string{userID}); err != nil { + if err := d.leaveOrg(ctx, userID, o.Organization); err != nil { return fmt.Errorf("failed to delete user from org[%s]: %w", o.Organization.ID, err) } } @@ -689,6 +678,18 @@ func (d Service) DeleteUser(ctx context.Context, userID string) error { return d.userService.Delete(ctx, userID) } +// leaveOrg runs after the check, and the org can change in between. +func (d Service) leaveOrg(ctx context.Context, userID string, org organization.Organization) error { + err := d.membershipService.RemoveOrganizationMemberOnDelete(ctx, org.ID, userID, schema.UserPrincipal) + switch { + case errors.Is(err, membership.ErrNotMember), errors.Is(err, organization.ErrNotExist): + return nil + case errors.Is(err, membership.ErrLastOwnerRole): + return &BlockedError{UserID: userID, Blockers: []Blocker{soleOwnerBlocker(org)}} + } + return err +} + func (d Service) listOrgProjects(ctx context.Context, orgID string) ([]project.Project, error) { return d.projService.List(ctx, project.Filter{OrgID: orgID, IncludeDisabled: true}) } diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index eba56fe42..fb42d2988 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -1790,43 +1790,6 @@ func TestDeleteCustomers(t *testing.T) { }) } -func TestRemoveUsersFromOrg(t *testing.T) { - const orgID = "org-1" - const userID = "user-1" - - t.Run("delegates the whole removal to the membership cascade", func(t *testing.T) { - m := newMocks(t) - - // org, project, group, and custom-resource cleanup all happen inside the - // cascade — strict mocks fail if the deleter touches them itself - m.mbrSvc.EXPECT().ForceRemoveOrganizationMember(mock.Anything, orgID, userID, schema.UserPrincipal).Return(nil) - - err := m.build().RemoveUsersFromOrg(context.Background(), orgID, []string{userID}) - assert.NoError(t, err) - }) - - t.Run("propagates membership cascade failure", func(t *testing.T) { - m := newMocks(t) - - m.mbrSvc.EXPECT().ForceRemoveOrganizationMember(mock.Anything, orgID, userID, schema.UserPrincipal). - Return(errors.New("cascade boom")) - - err := m.build().RemoveUsersFromOrg(context.Background(), orgID, []string{userID}) - assert.ErrorContains(t, err, "cascade boom") - }) - - t.Run("keeps going to the next user after one fails", func(t *testing.T) { - m := newMocks(t) - - m.mbrSvc.EXPECT().ForceRemoveOrganizationMember(mock.Anything, orgID, userID, schema.UserPrincipal). - Return(errors.New("cascade boom")) - m.mbrSvc.EXPECT().ForceRemoveOrganizationMember(mock.Anything, orgID, "user-2", schema.UserPrincipal).Return(nil) - - err := m.build().RemoveUsersFromOrg(context.Background(), orgID, []string{userID, "user-2"}) - assert.ErrorContains(t, err, "cascade boom") - }) -} - var ( orgOwner = role.Role{Name: schema.RoleOrganizationOwner, Title: "Owner"} orgViewer = role.Role{Name: schema.RoleOrganizationViewer, Title: "Member"} @@ -1891,7 +1854,7 @@ func TestDeleteUser(t *testing.T) { expectNoBillingAccounts(m, solo.ID) expectUserOrg(m, shared, []membership.Member{orgMember(userID, orgViewer), orgMember("user-2", orgOwner)}, userID, "user-2") expectEmptyOrgDelete(m, &steps) - m.mbrSvc.EXPECT().ForceRemoveOrganizationMember(mock.Anything, shared.ID, userID, schema.UserPrincipal). + m.mbrSvc.EXPECT().RemoveOrganizationMemberOnDelete(mock.Anything, shared.ID, userID, schema.UserPrincipal). Run(func(_ context.Context, orgID, _, _ string) { steps = append(steps, "leave:"+orgID) }).Return(nil) m.mbrSvc.EXPECT().RemovePrincipalPolicies(mock.Anything, userID, schema.UserPrincipal). Run(func(context.Context, string, string) { steps = append(steps, "policies") }).Return(nil) @@ -1996,12 +1959,54 @@ func TestDeleteUser(t *testing.T) { assert.Equal(t, []string{"policies", "pats", "user"}, steps) }) + t.Run("a user removed from an org since the check does not stop the delete", func(t *testing.T) { + m := newMocks(t) + + expectUserOrgs(m, shared.ID) + expectUserOrg(m, shared, []membership.Member{orgMember(userID, orgViewer), orgMember("user-2", orgOwner)}, userID, "user-2") + m.mbrSvc.EXPECT().RemoveOrganizationMemberOnDelete(mock.Anything, shared.ID, userID, schema.UserPrincipal). + Return(membership.ErrNotMember) + m.mbrSvc.EXPECT().RemovePrincipalPolicies(mock.Anything, userID, schema.UserPrincipal).Return(nil) + m.patSvc.EXPECT().DeleteAllByUser(mock.Anything, userID).Return(nil) + m.usrSvc.EXPECT().Delete(mock.Anything, userID).Return(nil) + + assert.NoError(t, m.build().DeleteUser(context.Background(), userID)) + }) + + t.Run("an org the user leaves that was deleted since the check does not stop the delete", func(t *testing.T) { + m := newMocks(t) + + expectUserOrgs(m, shared.ID) + expectUserOrg(m, shared, []membership.Member{orgMember(userID, orgViewer), orgMember("user-2", orgOwner)}, userID, "user-2") + m.mbrSvc.EXPECT().RemoveOrganizationMemberOnDelete(mock.Anything, shared.ID, userID, schema.UserPrincipal). + Return(organization.ErrNotExist) + m.mbrSvc.EXPECT().RemovePrincipalPolicies(mock.Anything, userID, schema.UserPrincipal).Return(nil) + m.patSvc.EXPECT().DeleteAllByUser(mock.Anything, userID).Return(nil) + m.usrSvc.EXPECT().Delete(mock.Anything, userID).Return(nil) + + assert.NoError(t, m.build().DeleteUser(context.Background(), userID)) + }) + + t.Run("an org that lost its other owner since the check refuses the delete with the blocker", func(t *testing.T) { + m := newMocks(t) + + expectUserOrgs(m, shared.ID) + expectUserOrg(m, shared, []membership.Member{orgMember(userID, orgOwner), orgMember("user-2", orgOwner)}, userID, "user-2") + m.mbrSvc.EXPECT().RemoveOrganizationMemberOnDelete(mock.Anything, shared.ID, userID, schema.UserPrincipal). + Return(membership.ErrLastOwnerRole) + + err := m.build().DeleteUser(context.Background(), userID) + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Equal(t, &deleter.BlockedError{UserID: userID, Blockers: []deleter.Blocker{soleOwnerBlocker(shared)}}, blocked) + }) + t.Run("an org removal failure stops the delete before the remaining policies", func(t *testing.T) { m := newMocks(t) expectUserOrgs(m, shared.ID) expectUserOrg(m, shared, []membership.Member{orgMember(userID, orgViewer), orgMember("user-2", orgOwner)}, userID, "user-2") - m.mbrSvc.EXPECT().ForceRemoveOrganizationMember(mock.Anything, shared.ID, userID, schema.UserPrincipal). + m.mbrSvc.EXPECT().RemoveOrganizationMemberOnDelete(mock.Anything, shared.ID, userID, schema.UserPrincipal). Return(errors.New("cascade boom")) err := m.build().DeleteUser(context.Background(), userID) diff --git a/core/membership/org.go b/core/membership/org.go index 71bfc030f..220a024e7 100644 --- a/core/membership/org.go +++ b/core/membership/org.go @@ -205,20 +205,17 @@ func (s *Service) SetPATAllProjectsRole(ctx context.Context, orgID, patID, roleI // relations. Returns ErrNotMember if the principal has no policies on this org, // and ErrLastOwnerRole when removing the org's last owner. func (s *Service) RemoveOrganizationMember(ctx context.Context, orgID, principalID, principalType string) error { - return s.removeOrganizationMember(ctx, orgID, principalID, principalType, true) + return s.removeOrganizationMember(ctx, orgID, principalID, principalType, false) } -// ForceRemoveOrganizationMember is RemoveOrganizationMember without the -// member-and-owner guards: it does not fail when the principal has no -// org-level policies and it removes the org's last owner without complaint. -// It exists for deletion cascades (see core/deleter), where the principal is -// leaving the system entirely and the org may legitimately be left ownerless. -// Anything user-initiated should go through RemoveOrganizationMember instead. -func (s *Service) ForceRemoveOrganizationMember(ctx context.Context, orgID, principalID, principalType string) error { - return s.removeOrganizationMember(ctx, orgID, principalID, principalType, false) +// RemoveOrganizationMemberOnDelete is RemoveOrganizationMember for deleting a +// user. It also works on a disabled org, because the user has to leave every +// org before the account can go. +func (s *Service) RemoveOrganizationMemberOnDelete(ctx context.Context, orgID, principalID, principalType string) error { + return s.removeOrganizationMember(ctx, orgID, principalID, principalType, true) } -func (s *Service) removeOrganizationMember(ctx context.Context, orgID, principalID, principalType string, guarded bool) error { +func (s *Service) removeOrganizationMember(ctx context.Context, orgID, principalID, principalType string, allowDisabledOrg bool) error { targetAuditType, err := principalTypeToAuditType(principalType) if err != nil { return err @@ -226,11 +223,9 @@ func (s *Service) removeOrganizationMember(ctx context.Context, orgID, principal org, err := s.orgService.Get(ctx, orgID) if err != nil { - // deletion cascades must keep working on disabled orgs — the deleter - // visits them deliberately so user deletion doesn't leave orphan - // policies behind. Get discards the org payload on ErrDisabled, so - // reconstruct the minimal org the cascade needs. - if guarded || !errors.Is(err, organization.ErrDisabled) { + // a user delete leaves disabled orgs too. Get discards the org payload + // on ErrDisabled, so reconstruct the minimal org the cascade needs. + if !allowDisabledOrg || !errors.Is(err, organization.ErrDisabled) { return err } org = organization.Organization{ID: orgID} @@ -245,19 +240,17 @@ func (s *Service) removeOrganizationMember(ctx context.Context, orgID, principal if err != nil { return fmt.Errorf("list existing policies: %w", err) } + if len(orgPolicies) == 0 { + return ErrNotMember + } // only humans can be the last owner — skip for service users and PATs. // an empty ownerRoleID makes the cascade delete owner policies unguarded. var ownerRoleID string - if guarded { - if len(orgPolicies) == 0 { - return ErrNotMember - } - if principalType == schema.UserPrincipal { - ownerRoleID, err = s.validateMinOwnerConstraint(ctx, orgID, "", orgPolicies) - if err != nil { - return err - } + if principalType == schema.UserPrincipal { + ownerRoleID, err = s.validateMinOwnerConstraint(ctx, orgID, "", orgPolicies) + if err != nil { + return err } } @@ -336,6 +329,9 @@ func (s *Service) cascadeRemovePrincipal(ctx context.Context, org organization.O if errors.Is(err, policy.ErrLastRoleGuard) { return ErrLastOwnerRole } + if errors.Is(err, policy.ErrNotExist) { + continue + } return fmt.Errorf("delete org policy %s: %w", pol.ID, err) } } @@ -343,7 +339,7 @@ func (s *Service) cascadeRemovePrincipal(ctx context.Context, org organization.O // guard passed — delete sub-resource policies var errs error for _, pol := range subResourcePolicies { - if err := s.policyService.Delete(ctx, pol.ID); err != nil { + if err := s.policyService.Delete(ctx, pol.ID); err != nil && !errors.Is(err, policy.ErrNotExist) { errs = errors.Join(errs, fmt.Errorf("delete sub-resource policy %s: %w", pol.ID, err)) } } diff --git a/core/membership/org_test.go b/core/membership/org_test.go index d7788ed03..9de68b50d 100644 --- a/core/membership/org_test.go +++ b/core/membership/org_test.go @@ -1000,17 +1000,14 @@ func TestService_RemoveOrganizationMember(t *testing.T) { } } -func TestService_ForceRemoveOrganizationMember(t *testing.T) { +func TestService_RemoveOrganizationMemberOnDelete(t *testing.T) { ctx := context.Background() orgID := uuid.New().String() userID := uuid.New().String() ownerRoleID := uuid.New().String() - enabledOrg := organization.Organization{ID: orgID, Title: "Test Org"} - type testDeps struct { policySvc *mocks.PolicyService - relSvc *mocks.RelationService roleSvc *mocks.RoleService orgSvc *mocks.OrgService projSvc *mocks.ProjectService @@ -1018,6 +1015,12 @@ func TestService_ForceRemoveOrganizationMember(t *testing.T) { resSvc *mocks.ResourceService auditRepo *mocks.AuditRecordRepository } + expectDisabledOrgOwner := func(d testDeps, owners []policy.Policy) { + d.orgSvc.EXPECT().Get(ctx, orgID).Return(organization.Organization{}, organization.ErrDisabled) + d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: ownerRoleID}}, nil) + d.roleSvc.EXPECT().Get(ctx, schema.RoleOrganizationOwner).Return(role.Role{ID: ownerRoleID, Name: schema.RoleOrganizationOwner}, nil) + d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, RoleID: ownerRoleID}).Return(owners, nil) + } tests := []struct { name string @@ -1025,51 +1028,49 @@ func TestService_ForceRemoveOrganizationMember(t *testing.T) { wantErr error }{ { - name: "removes the org's last owner without the guard", + name: "removes an owner from a disabled org with the owner guard", setup: func(d testDeps) { - d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) - d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: ownerRoleID}}, nil) - // no roleSvc.Get / owner-count query: the guard is skipped entirely + expectDisabledOrgOwner(d, []policy.Policy{{ID: "org-p1"}, {ID: "other-owner-p"}}) d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{}, nil) d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID, RoleID: ownerRoleID}, }, nil) - // plain delete, not DeleteWithMinRoleGuard - d.policySvc.EXPECT().Delete(ctx, "org-p1").Return(nil) + d.policySvc.EXPECT().DeleteWithMinRoleGuard(ctx, "org-p1", ownerRoleID).Return(nil) d.resSvc.EXPECT().RemovePrincipalAccess(ctx, userID, schema.UserPrincipal, orgID, []string{}).Return(nil) d.auditRepo.EXPECT().Create(ctx, mock.Anything).Return(auditrecord.AuditRecord{}, nil) }, }, { - name: "does not return ErrNotMember when principal has no org policies and still cascades", + name: "skips policies that are already gone", setup: func(d testDeps) { - d.orgSvc.EXPECT().Get(ctx, orgID).Return(enabledOrg, nil) - d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{}, nil) + expectDisabledOrgOwner(d, []policy.Policy{{ID: "org-p1"}, {ID: "other-owner-p"}}) + d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{{ID: "proj-1"}}, nil) d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) - d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{}, nil) - d.resSvc.EXPECT().RemovePrincipalAccess(ctx, userID, schema.UserPrincipal, orgID, []string{}).Return(nil) + d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ + {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID, RoleID: ownerRoleID}, + {ID: "proj-p1", ResourceType: schema.ProjectNamespace, ResourceID: "proj-1"}, + }, nil) + d.policySvc.EXPECT().DeleteWithMinRoleGuard(ctx, "org-p1", ownerRoleID).Return(policy.ErrNotExist) + d.policySvc.EXPECT().Delete(ctx, "proj-p1").Return(policy.ErrNotExist) + d.resSvc.EXPECT().RemovePrincipalAccess(ctx, userID, schema.UserPrincipal, orgID, []string{"proj-1"}).Return(nil) d.auditRepo.EXPECT().Create(ctx, mock.Anything).Return(auditrecord.AuditRecord{}, nil) }, }, { - name: "proceeds with the cascade when the org is disabled", + name: "refuses the last owner of a disabled org", + setup: func(d testDeps) { + expectDisabledOrgOwner(d, []policy.Policy{{ID: "org-p1"}}) + }, + wantErr: membership.ErrLastOwnerRole, + }, + { + name: "returns ErrNotMember when the principal has no org policies", setup: func(d testDeps) { - // Get discards the org payload on ErrDisabled; the force path - // must reconstruct the org from the input ID and continue — - // the deleter visits disabled orgs deliberately d.orgSvc.EXPECT().Get(ctx, orgID).Return(organization.Organization{}, organization.ErrDisabled) - d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{{ID: "org-p1", RoleID: ownerRoleID}}, nil) - d.projSvc.EXPECT().List(ctx, project.Filter{OrgID: orgID}).Return([]project.Project{}, nil) - d.grpSvc.EXPECT().List(ctx, group.Filter{OrganizationID: orgID}).Return([]group.Group{}, nil) - d.policySvc.EXPECT().List(ctx, policy.Filter{PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{ - {ID: "org-p1", ResourceType: schema.OrganizationNamespace, ResourceID: orgID, RoleID: ownerRoleID}, - }, nil) - d.policySvc.EXPECT().Delete(ctx, "org-p1").Return(nil) - d.resSvc.EXPECT().RemovePrincipalAccess(ctx, userID, schema.UserPrincipal, orgID, []string{}).Return(nil) - d.auditRepo.EXPECT().Create(ctx, mock.Anything).Return(auditrecord.AuditRecord{}, nil) + d.policySvc.EXPECT().List(ctx, policy.Filter{OrgID: orgID, PrincipalID: userID, PrincipalType: schema.UserPrincipal}).Return([]policy.Policy{}, nil) }, + wantErr: membership.ErrNotMember, }, { name: "propagates org lookup failure", @@ -1084,7 +1085,6 @@ func TestService_ForceRemoveOrganizationMember(t *testing.T) { t.Run(tt.name, func(t *testing.T) { d := testDeps{ policySvc: mocks.NewPolicyService(t), - relSvc: mocks.NewRelationService(t), roleSvc: mocks.NewRoleService(t), orgSvc: mocks.NewOrgService(t), projSvc: mocks.NewProjectService(t), @@ -1092,14 +1092,12 @@ func TestService_ForceRemoveOrganizationMember(t *testing.T) { resSvc: mocks.NewResourceService(t), auditRepo: mocks.NewAuditRecordRepository(t), } - if tt.setup != nil { - tt.setup(d) - } + tt.setup(d) - svc := membership.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), d.policySvc, d.relSvc, d.roleSvc, d.orgSvc, mocks.NewUserService(t), d.projSvc, d.grpSvc, mocks.NewServiceuserService(t), d.auditRepo) + svc := membership.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), d.policySvc, mocks.NewRelationService(t), d.roleSvc, d.orgSvc, mocks.NewUserService(t), d.projSvc, d.grpSvc, mocks.NewServiceuserService(t), d.auditRepo) svc.SetResourceService(d.resSvc) - err := svc.ForceRemoveOrganizationMember(ctx, orgID, userID, schema.UserPrincipal) + err := svc.RemoveOrganizationMemberOnDelete(ctx, orgID, userID, schema.UserPrincipal) if tt.wantErr != nil { assert.ErrorIs(t, err, tt.wantErr) } else { diff --git a/core/resource/service.go b/core/resource/service.go index 997c8ac4b..0df69f2fb 100644 --- a/core/resource/service.go +++ b/core/resource/service.go @@ -574,7 +574,7 @@ func (s Service) RemovePrincipalAccess(ctx context.Context, principalID, princip var errs error for _, pol := range policies { if _, ok := resourceIDs[pol.ResourceID]; ok { - if err := s.policyService.Delete(ctx, pol.ID); err != nil { + if err := s.policyService.Delete(ctx, pol.ID); err != nil && !errors.Is(err, policy.ErrNotExist) { errs = errors.Join(errs, fmt.Errorf("delete resource policy %s: %w", pol.ID, err)) } } diff --git a/core/resource/service_test.go b/core/resource/service_test.go index 97b599439..a151ff254 100644 --- a/core/resource/service_test.go +++ b/core/resource/service_test.go @@ -946,6 +946,20 @@ func TestRemovePrincipalAccess(t *testing.T) { assert.NoError(t, err) }) + t.Run("a policy that is already gone counts as deleted", func(t *testing.T) { + repo, _, _, _, _, _, _, policySvc, svc := newTestService(t) + + policySvc.EXPECT().List(mock.Anything, mock.Anything).Return([]policy.Policy{ + {ID: "store-p1", ResourceType: "resource/store", ResourceID: "store-1"}, + }, nil) + repo.EXPECT().List(mock.Anything, resource.Filter{OrgID: "org-1"}). + Return([]resource.Resource{{ID: "store-1"}}, nil) + policySvc.EXPECT().Delete(mock.Anything, "store-p1").Return(policy.ErrNotExist) + + err := svc.RemovePrincipalAccess(context.Background(), userID, schema.UserPrincipal, "org-1", nil) + assert.NoError(t, err) + }) + t.Run("aggregates delete errors and keeps going", func(t *testing.T) { repo, _, _, _, _, _, _, policySvc, svc := newTestService(t) diff --git a/internal/api/v1beta1connect/authenticate.go b/internal/api/v1beta1connect/authenticate.go index 8315d20a2..2a3ce2ee4 100644 --- a/internal/api/v1beta1connect/authenticate.go +++ b/internal/api/v1beta1connect/authenticate.go @@ -364,10 +364,12 @@ func (h *ConnectHandler) AuthLogout(ctx context.Context, request *connect.Reques } resp := connect.NewResponse(&frontierv1beta1.AuthLogoutResponse{}) + clearSessionCookie(resp) + return resp, nil +} - // instruct interceptor to invalidate cookie +func clearSessionCookie(resp connect.AnyResponse) { resp.Header().Set(consts.SessionDeleteGatewayKey, "true") - return resp, nil } func (h *ConnectHandler) getLoggedInSessionID(ctx context.Context) (uuid.UUID, error) { @@ -400,6 +402,17 @@ func (h *ConnectHandler) GetLoggedInPrincipal(ctx context.Context, via ...authen return principal, nil } +func (h *ConnectHandler) getLoggedInUser(ctx context.Context) (authenticate.Principal, error) { + principal, err := h.GetLoggedInPrincipal(ctx) + if err != nil { + return principal, err + } + if principal.Type != schema.UserPrincipal { + return principal, connect.NewError(connect.CodePermissionDenied, ErrUnauthenticated) + } + return principal, nil +} + func (h *ConnectHandler) ListAuthStrategies(ctx context.Context, request *connect.Request[frontierv1beta1.ListAuthStrategiesRequest]) (*connect.Response[frontierv1beta1.ListAuthStrategiesResponse], error) { var pbstrategy []*frontierv1beta1.AuthStrategy for _, strategy := range h.authnService.SupportedStrategies() { diff --git a/internal/api/v1beta1connect/interfaces.go b/internal/api/v1beta1connect/interfaces.go index 1b8ef7191..50288490d 100644 --- a/internal/api/v1beta1connect/interfaces.go +++ b/internal/api/v1beta1connect/interfaces.go @@ -400,6 +400,7 @@ type CascadeDeleter interface { CheckOrganizationDelete(ctx context.Context, id string) ([]deleter.Blocker, error) DeleteGroup(ctx context.Context, id string) error DeleteUser(ctx context.Context, userID string) error + CheckUserDelete(ctx context.Context, userID string) (deleter.UserDeleteCheck, error) } type AuditRecordService interface { diff --git a/internal/api/v1beta1connect/mocks/cascade_deleter.go b/internal/api/v1beta1connect/mocks/cascade_deleter.go index f4e871657..ab6648bcd 100644 --- a/internal/api/v1beta1connect/mocks/cascade_deleter.go +++ b/internal/api/v1beta1connect/mocks/cascade_deleter.go @@ -140,6 +140,63 @@ func (_c *CascadeDeleter_CheckProjectDelete_Call) RunAndReturn(run func(context. return _c } +// CheckUserDelete provides a mock function with given fields: ctx, userID +func (_m *CascadeDeleter) CheckUserDelete(ctx context.Context, userID string) (deleter.UserDeleteCheck, error) { + ret := _m.Called(ctx, userID) + + if len(ret) == 0 { + panic("no return value specified for CheckUserDelete") + } + + var r0 deleter.UserDeleteCheck + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) (deleter.UserDeleteCheck, error)); ok { + return rf(ctx, userID) + } + if rf, ok := ret.Get(0).(func(context.Context, string) deleter.UserDeleteCheck); ok { + r0 = rf(ctx, userID) + } else { + r0 = ret.Get(0).(deleter.UserDeleteCheck) + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, userID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// CascadeDeleter_CheckUserDelete_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CheckUserDelete' +type CascadeDeleter_CheckUserDelete_Call struct { + *mock.Call +} + +// CheckUserDelete is a helper method to define mock.On call +// - ctx context.Context +// - userID string +func (_e *CascadeDeleter_Expecter) CheckUserDelete(ctx interface{}, userID interface{}) *CascadeDeleter_CheckUserDelete_Call { + return &CascadeDeleter_CheckUserDelete_Call{Call: _e.mock.On("CheckUserDelete", ctx, userID)} +} + +func (_c *CascadeDeleter_CheckUserDelete_Call) Run(run func(ctx context.Context, userID string)) *CascadeDeleter_CheckUserDelete_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *CascadeDeleter_CheckUserDelete_Call) Return(_a0 deleter.UserDeleteCheck, _a1 error) *CascadeDeleter_CheckUserDelete_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *CascadeDeleter_CheckUserDelete_Call) RunAndReturn(run func(context.Context, string) (deleter.UserDeleteCheck, error)) *CascadeDeleter_CheckUserDelete_Call { + _c.Call.Return(run) + return _c +} + // DeleteGroup provides a mock function with given fields: ctx, id func (_m *CascadeDeleter) DeleteGroup(ctx context.Context, id string) error { ret := _m.Called(ctx, id) diff --git a/internal/api/v1beta1connect/user.go b/internal/api/v1beta1connect/user.go index b9eeafb21..c20d4ea30 100644 --- a/internal/api/v1beta1connect/user.go +++ b/internal/api/v1beta1connect/user.go @@ -409,32 +409,93 @@ func (h *ConnectHandler) DisableUser(ctx context.Context, request *connect.Reque } func (h *ConnectHandler) DeleteUser(ctx context.Context, request *connect.Request[frontierv1beta1.DeleteUserRequest]) (*connect.Response[frontierv1beta1.DeleteUserResponse], error) { + if err := h.deleteUser(ctx, request, "DeleteUser", request.Msg.GetId()); err != nil { + return nil, err + } + return connect.NewResponse(&frontierv1beta1.DeleteUserResponse{}), nil +} + +func (h *ConnectHandler) CheckCurrentUserDelete(ctx context.Context, request *connect.Request[frontierv1beta1.CheckCurrentUserDeleteRequest]) (*connect.Response[frontierv1beta1.CheckCurrentUserDeleteResponse], error) { + errorLogger := NewErrorLogger() + + principal, err := h.getLoggedInUser(ctx) + if err != nil { + return nil, err + } + check, err := h.deleterService.CheckUserDelete(ctx, principal.ID) + if err != nil { + errorLogger.LogServiceError(ctx, request, "CheckCurrentUserDelete", err, "user_id", principal.ID) + return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("CheckCurrentUserDelete: user_id=%s: %w", principal.ID, err)) + } + return connect.NewResponse(transformUserDeleteCheckToPB(check)), nil +} + +func (h *ConnectHandler) DeleteCurrentUser(ctx context.Context, request *connect.Request[frontierv1beta1.DeleteCurrentUserRequest]) (*connect.Response[frontierv1beta1.DeleteCurrentUserResponse], error) { + principal, err := h.getLoggedInUser(ctx) + if err != nil { + return nil, err + } + if err := h.deleteUser(ctx, request, "DeleteCurrentUser", principal.ID); err != nil { + return nil, err + } + resp := connect.NewResponse(&frontierv1beta1.DeleteCurrentUserResponse{}) + clearSessionCookie(resp) + return resp, nil +} + +func (h *ConnectHandler) deleteUser(ctx context.Context, request connect.AnyRequest, operation, userID string) error { errorLogger := NewErrorLogger() - userID := request.Msg.GetId() if err := h.deleterService.DeleteUser(ctx, userID); err != nil { var blocked *deleter.BlockedError if errors.As(err, &blocked) { - return nil, deleteBlockedError(ctx, blocked) + return deleteBlockedError(ctx, blocked) } - errorLogger.LogServiceError(ctx, request, "DeleteUser", err, + errorLogger.LogServiceError(ctx, request, operation, err, "user_id", userID) switch { case errors.Is(err, user.ErrNotExist): - return nil, connect.NewError(connect.CodeNotFound, ErrUserNotExist) + return connect.NewError(connect.CodeNotFound, ErrUserNotExist) case errors.Is(err, user.ErrInvalidID): - return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadRequest) + return connect.NewError(connect.CodeInvalidArgument, ErrBadRequest) default: - return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("DeleteUser: user_id=%s: %w", userID, err)) + return connect.NewError(connect.CodeInternal, fmt.Errorf("%s: user_id=%s: %w", operation, userID, err)) } } - if err := audit.GetAuditor(ctx, schema.PlatformOrgID.String()).Log(audit.UserDeletedEvent, audit.UserTarget(request.Msg.GetId())); err != nil { - errorLogger.LogServiceError(ctx, request, "DeleteUser.AuditLog", err, + if err := audit.GetAuditor(ctx, schema.PlatformOrgID.String()).Log(audit.UserDeletedEvent, audit.UserTarget(userID)); err != nil { + errorLogger.LogServiceError(ctx, request, operation+".AuditLog", err, "user_id", userID) } - return connect.NewResponse(&frontierv1beta1.DeleteUserResponse{}), nil + return nil +} + +func transformUserDeleteCheckToPB(check deleter.UserDeleteCheck) *frontierv1beta1.CheckCurrentUserDeleteResponse { + resp := &frontierv1beta1.CheckCurrentUserDeleteResponse{ + CanDelete: check.CanDelete(), + Blockers: transformBlockersToPB(check.Blockers), + PatCount: int32(check.PATCount), + } + for _, o := range check.OrganizationsToDelete { + resp.OrganizationsToDelete = append(resp.OrganizationsToDelete, &frontierv1beta1.CheckCurrentUserDeleteResponse_OrganizationToDelete{ + Id: o.Organization.ID, + Name: o.Organization.Name, + Title: o.Organization.Title, + CanDelete: len(o.Blockers) == 0, + Blockers: transformBlockersToPB(o.Blockers), + }) + } + for _, o := range check.OrganizationsToLeave { + resp.OrganizationsToLeave = append(resp.OrganizationsToLeave, &frontierv1beta1.CheckCurrentUserDeleteResponse_OrganizationToLeave{ + Id: o.Organization.ID, + Name: o.Organization.Name, + Title: o.Organization.Title, + Role: o.RoleTitle, + Blockers: transformBlockersToPB(o.Blockers), + }) + } + return resp } func (h *ConnectHandler) ListUserGroups(ctx context.Context, request *connect.Request[frontierv1beta1.ListUserGroupsRequest]) (*connect.Response[frontierv1beta1.ListUserGroupsResponse], error) { diff --git a/internal/api/v1beta1connect/user_pat.go b/internal/api/v1beta1connect/user_pat.go index 8ee157ebf..4c5ba8d3b 100644 --- a/internal/api/v1beta1connect/user_pat.go +++ b/internal/api/v1beta1connect/user_pat.go @@ -10,7 +10,6 @@ import ( "github.com/raystack/frontier/core/userpat" paterrors "github.com/raystack/frontier/core/userpat/errors" "github.com/raystack/frontier/core/userpat/models" - "github.com/raystack/frontier/internal/bootstrap/schema" "github.com/raystack/frontier/pkg/metadata" "github.com/raystack/frontier/pkg/utils" frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1" @@ -61,13 +60,10 @@ func mapPATError(err error) *connect.Error { func (h *ConnectHandler) CreateCurrentUserPAT(ctx context.Context, request *connect.Request[frontierv1beta1.CreateCurrentUserPATRequest]) (*connect.Response[frontierv1beta1.CreateCurrentUserPATResponse], error) { errorLogger := NewErrorLogger() - principal, err := h.GetLoggedInPrincipal(ctx) + principal, err := h.getLoggedInUser(ctx) if err != nil { return nil, err } - if principal.Type != schema.UserPrincipal { - return nil, connect.NewError(connect.CodePermissionDenied, ErrUnauthenticated) - } if err := h.userPATService.ValidateExpiry(request.Msg.GetExpiresAt().AsTime()); err != nil { return nil, connect.NewError(connect.CodeInvalidArgument, err) diff --git a/internal/api/v1beta1connect/user_test.go b/internal/api/v1beta1connect/user_test.go index 407d7a276..635648cd6 100644 --- a/internal/api/v1beta1connect/user_test.go +++ b/internal/api/v1beta1connect/user_test.go @@ -22,6 +22,7 @@ import ( "github.com/raystack/frontier/internal/bootstrap/schema" "github.com/raystack/frontier/pkg/errors" "github.com/raystack/frontier/pkg/metadata" + "github.com/raystack/frontier/pkg/server/consts" "github.com/raystack/frontier/pkg/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -910,6 +911,7 @@ func TestConnectHandler_DeleteUser(t *testing.T) { if tt.err == connect.Code(0) { assert.NoError(t, err) assert.EqualValues(t, tt.want, resp.Msg) + assert.Empty(t, resp.Header().Get(consts.SessionDeleteGatewayKey), "the admin's own session cookie stays") } else { assert.Nil(t, resp) assert.Equal(t, tt.err, connect.CodeOf(err)) @@ -955,6 +957,194 @@ func TestConnectHandler_DeleteUser(t *testing.T) { }) } +func TestConnectHandler_CheckCurrentUserDelete(t *testing.T) { + userID := uuid.New().String() + currentUser := authenticate.Principal{ID: userID, Type: schema.UserPrincipal, User: &user.User{ID: userID}} + solo := organization.Organization{ID: uuid.New().String(), Name: "solo", Title: "Solo"} + debt := organization.Organization{ID: uuid.New().String(), Name: "debt", Title: "Debt"} + shared := organization.Organization{ID: uuid.New().String(), Name: "shared", Title: "Shared"} + unpaid := deleter.Blocker{Type: deleter.BlockerUnpaidInvoice, Subject: "inv-1", SubjectType: deleter.SubjectInvoice, Message: "invoice[inv-1] is unpaid"} + soleOwner := deleter.Blocker{Type: deleter.BlockerSoleOwnerWithMembers, Subject: shared.ID, SubjectType: deleter.SubjectOrganization, Message: "organization[shared] has other members and this user is its only owner"} + + tests := []struct { + title string + setup func(*mocks.AuthnService, *mocks.CascadeDeleter) + want *frontierv1beta1.CheckCurrentUserDeleteResponse + err connect.Code + }{ + { + title: "should list the orgs deleted with the user and the orgs it leaves, each with its blockers", + setup: func(as *mocks.AuthnService, cd *mocks.CascadeDeleter) { + as.EXPECT().GetPrincipal(mock.Anything).Return(currentUser, nil) + cd.EXPECT().CheckUserDelete(mock.Anything, userID).Return(deleter.UserDeleteCheck{ + OrganizationsToDelete: []deleter.OrganizationToDelete{ + {Organization: solo}, + {Organization: debt, Blockers: []deleter.Blocker{unpaid}}, + }, + OrganizationsToLeave: []deleter.OrganizationToLeave{ + {Organization: shared, RoleTitle: "Owner", Blockers: []deleter.Blocker{soleOwner}}, + }, + PATCount: 2, + }, nil) + }, + want: &frontierv1beta1.CheckCurrentUserDeleteResponse{ + CanDelete: false, + OrganizationsToDelete: []*frontierv1beta1.CheckCurrentUserDeleteResponse_OrganizationToDelete{ + {Id: solo.ID, Name: "solo", Title: "Solo", CanDelete: true}, + {Id: debt.ID, Name: "debt", Title: "Debt", CanDelete: false, Blockers: []*frontierv1beta1.Blocker{ + {Type: deleter.BlockerUnpaidInvoice, Subject: "inv-1", SubjectType: deleter.SubjectInvoice, Message: "invoice[inv-1] is unpaid"}, + }}, + }, + OrganizationsToLeave: []*frontierv1beta1.CheckCurrentUserDeleteResponse_OrganizationToLeave{ + {Id: shared.ID, Name: "shared", Title: "Shared", Role: "Owner", Blockers: []*frontierv1beta1.Blocker{ + {Type: deleter.BlockerSoleOwnerWithMembers, Subject: shared.ID, SubjectType: deleter.SubjectOrganization, Message: "organization[shared] has other members and this user is its only owner"}, + }}, + }, + PatCount: 2, + }, + }, + { + title: "should allow the delete when nothing blocks it", + setup: func(as *mocks.AuthnService, cd *mocks.CascadeDeleter) { + as.EXPECT().GetPrincipal(mock.Anything).Return(currentUser, nil) + cd.EXPECT().CheckUserDelete(mock.Anything, userID).Return(deleter.UserDeleteCheck{ + OrganizationsToLeave: []deleter.OrganizationToLeave{{Organization: shared, RoleTitle: "Member"}}, + }, nil) + }, + want: &frontierv1beta1.CheckCurrentUserDeleteResponse{ + CanDelete: true, + OrganizationsToLeave: []*frontierv1beta1.CheckCurrentUserDeleteResponse_OrganizationToLeave{ + {Id: shared.ID, Name: "shared", Title: "Shared", Role: "Member"}, + }, + }, + }, + { + title: "should return internal error when the check fails", + setup: func(as *mocks.AuthnService, cd *mocks.CascadeDeleter) { + as.EXPECT().GetPrincipal(mock.Anything).Return(currentUser, nil) + cd.EXPECT().CheckUserDelete(mock.Anything, userID).Return(deleter.UserDeleteCheck{}, errors.New("read failed")) + }, + err: connect.CodeInternal, + }, + { + title: "should return unauthenticated when nobody is logged in", + setup: func(as *mocks.AuthnService, cd *mocks.CascadeDeleter) { + as.EXPECT().GetPrincipal(mock.Anything).Return(authenticate.Principal{}, errors.ErrUnauthenticated) + }, + err: connect.CodeUnauthenticated, + }, + { + title: "should return permission denied for a service user", + setup: func(as *mocks.AuthnService, cd *mocks.CascadeDeleter) { + as.EXPECT().GetPrincipal(mock.Anything).Return(authenticate.Principal{ID: "sv-1", Type: schema.ServiceUserPrincipal}, nil) + }, + err: connect.CodePermissionDenied, + }, + { + title: "should return permission denied for a PAT", + setup: func(as *mocks.AuthnService, cd *mocks.CascadeDeleter) { + as.EXPECT().GetPrincipal(mock.Anything).Return(authenticate.Principal{ID: "pat-1", Type: schema.PATPrincipal, User: &user.User{ID: userID}}, nil) + }, + err: connect.CodePermissionDenied, + }, + } + + for _, tt := range tests { + t.Run(tt.title, func(t *testing.T) { + as := mocks.NewAuthnService(t) + cd := mocks.NewCascadeDeleter(t) + tt.setup(as, cd) + handler := &ConnectHandler{authnService: as, deleterService: cd} + + resp, err := handler.CheckCurrentUserDelete(context.Background(), connect.NewRequest(&frontierv1beta1.CheckCurrentUserDeleteRequest{})) + if tt.err != connect.Code(0) { + assert.Nil(t, resp) + assert.Equal(t, tt.err, connect.CodeOf(err)) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.want, resp.Msg) + }) + } +} + +func TestConnectHandler_DeleteCurrentUser(t *testing.T) { + userID := uuid.New().String() + currentUser := authenticate.Principal{ID: userID, Type: schema.UserPrincipal, User: &user.User{ID: userID}} + + tests := []struct { + title string + setup func(*mocks.AuthnService, *mocks.CascadeDeleter) + err connect.Code + }{ + { + title: "should delete the logged-in user and clear the session cookie", + setup: func(as *mocks.AuthnService, cd *mocks.CascadeDeleter) { + as.EXPECT().GetPrincipal(mock.Anything).Return(currentUser, nil) + cd.EXPECT().DeleteUser(mock.Anything, userID).Return(nil) + }, + }, + { + title: "should return failed precondition when the delete is blocked", + setup: func(as *mocks.AuthnService, cd *mocks.CascadeDeleter) { + as.EXPECT().GetPrincipal(mock.Anything).Return(currentUser, nil) + cd.EXPECT().DeleteUser(mock.Anything, userID).Return(&deleter.BlockedError{ + UserID: userID, + Blockers: []deleter.Blocker{{Type: deleter.BlockerSoleOwnerWithMembers, Subject: "org-1"}}, + }) + }, + err: connect.CodeFailedPrecondition, + }, + { + title: "should return internal error when the delete fails", + setup: func(as *mocks.AuthnService, cd *mocks.CascadeDeleter) { + as.EXPECT().GetPrincipal(mock.Anything).Return(currentUser, nil) + cd.EXPECT().DeleteUser(mock.Anything, userID).Return(errors.New("spicedb unavailable")) + }, + err: connect.CodeInternal, + }, + { + title: "should return unauthenticated when nobody is logged in", + setup: func(as *mocks.AuthnService, cd *mocks.CascadeDeleter) { + as.EXPECT().GetPrincipal(mock.Anything).Return(authenticate.Principal{}, errors.ErrUnauthenticated) + }, + err: connect.CodeUnauthenticated, + }, + { + title: "should return permission denied for a service user", + setup: func(as *mocks.AuthnService, cd *mocks.CascadeDeleter) { + as.EXPECT().GetPrincipal(mock.Anything).Return(authenticate.Principal{ID: "sv-1", Type: schema.ServiceUserPrincipal}, nil) + }, + err: connect.CodePermissionDenied, + }, + { + title: "should return permission denied for a PAT", + setup: func(as *mocks.AuthnService, cd *mocks.CascadeDeleter) { + as.EXPECT().GetPrincipal(mock.Anything).Return(authenticate.Principal{ID: "pat-1", Type: schema.PATPrincipal, User: &user.User{ID: userID}}, nil) + }, + err: connect.CodePermissionDenied, + }, + } + + for _, tt := range tests { + t.Run(tt.title, func(t *testing.T) { + as := mocks.NewAuthnService(t) + cd := mocks.NewCascadeDeleter(t) + tt.setup(as, cd) + handler := &ConnectHandler{authnService: as, deleterService: cd} + + resp, err := handler.DeleteCurrentUser(context.Background(), connect.NewRequest(&frontierv1beta1.DeleteCurrentUserRequest{})) + if tt.err != connect.Code(0) { + assert.Nil(t, resp) + assert.Equal(t, tt.err, connect.CodeOf(err)) + return + } + assert.NoError(t, err) + assert.Equal(t, "true", resp.Header().Get(consts.SessionDeleteGatewayKey)) + }) + } +} + func TestConnectHandler_ListUserGroups(t *testing.T) { userID := uuid.New().String() orgID := uuid.New().String() diff --git a/pkg/server/connect_interceptors/authorization.go b/pkg/server/connect_interceptors/authorization.go index 6c9b41e5b..9b520ae6c 100644 --- a/pkg/server/connect_interceptors/authorization.go +++ b/pkg/server/connect_interceptors/authorization.go @@ -118,6 +118,8 @@ var authorizationSkipEndpoints = map[string]bool{ "/raystack.frontier.v1beta1.FrontierService/ListOrganizationsByCurrentUser": true, "/raystack.frontier.v1beta1.FrontierService/ListProjectsByCurrentUser": true, "/raystack.frontier.v1beta1.FrontierService/ListCurrentUserInvitations": true, + "/raystack.frontier.v1beta1.FrontierService/CheckCurrentUserDelete": true, + "/raystack.frontier.v1beta1.FrontierService/DeleteCurrentUser": true, "/raystack.frontier.v1beta1.FrontierService/JoinOrganization": true, "/raystack.frontier.v1beta1.FrontierService/CreateOrganization": true, @@ -157,6 +159,8 @@ var patDeniedEndpoints = map[string]bool{ "/raystack.frontier.v1beta1.FrontierService/PingUserSession": true, "/raystack.frontier.v1beta1.FrontierService/RevokeSession": true, "/raystack.frontier.v1beta1.FrontierService/CreateCurrentUserPAT": true, + "/raystack.frontier.v1beta1.FrontierService/CheckCurrentUserDelete": true, + "/raystack.frontier.v1beta1.FrontierService/DeleteCurrentUser": true, } // authorizationValidationMap stores path to validation function diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index a4af23d3f..d2463a011 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -2,6 +2,7 @@ package e2e_test import ( "context" + "encoding/base64" "encoding/json" "io" "net/http" @@ -2222,6 +2223,241 @@ CREATE TRIGGER trg_e2e_fail_user_delete_retry BEFORE DELETE OR UPDATE OF deleted }) } +func (s *APIRegressionTestSuite) TestDeleteCurrentUser() { + ctxOrgAdminAuth := testbench.ContextWithAuth(context.Background(), s.adminCookie) + + createOrg := func(ctx context.Context, name string) string { + resp, err := s.testBench.Client.CreateOrganization(ctx, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + Body: &frontierv1beta1.OrganizationRequestBody{Title: name, Name: name}, + })) + s.Require().NoError(err) + return resp.Msg.GetOrganization().GetId() + } + createProject := func(ctx context.Context, orgID, name string) string { + resp, err := s.testBench.Client.CreateProject(ctx, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{ + Body: &frontierv1beta1.ProjectRequestBody{Name: name, OrgId: orgID}, + })) + s.Require().NoError(err) + return resp.Msg.GetProject().GetId() + } + addMember := func(orgID, userID, roleID string) { + resp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: orgID, + Members: []*frontierv1beta1.OrgMemberEntry{{UserId: userID, RoleId: roleID}}, + })) + requireAddOrgMembersSuccess(s.T(), resp, err) + } + canGet := func(subject, resource string) bool { + resp, err := s.testBench.AdminClient.CheckFederatedResourcePermission(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CheckFederatedResourcePermissionRequest{ + Resource: resource, + Permission: schema.GetPermission, + Subject: subject, + })) + s.Require().NoError(err) + return resp.Msg.GetStatus() + } + orgIsLive := func(id string) bool { + var count int + s.Require().NoError(s.dbc.GetContext(context.Background(), &count, + "SELECT count(*) FROM organizations WHERE id = $1 AND deleted_at IS NULL", id)) + return count == 1 + } + listedInOrgUsers := func(orgID, userID string) (bool, bool) { + resp, err := s.testBench.Client.ListOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListOrganizationUsersRequest{Id: orgID})) + s.Require().NoError(err) + inUsers := slices.ContainsFunc(resp.Msg.GetUsers(), func(u *frontierv1beta1.User) bool { return u.GetId() == userID }) + inRolePairs := slices.ContainsFunc(resp.Msg.GetRolePairs(), func(p *frontierv1beta1.ListOrganizationUsersResponse_RolePair) bool { + return p.GetUserId() == userID + }) + return inUsers, inRolePairs + } + + userID, userCtx := signUpUser(s.T(), s.testBench.Client, "delete-current-user@raystack.org") + userSubject := schema.JoinNamespaceAndResourceID(schema.UserPrincipal, userID) + otherResp, err := s.testBench.Client.CreateUser(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateUserRequest{ + Body: &frontierv1beta1.UserRequestBody{Email: "delete-current-user-other@raystack.org", Name: "delete-current-user-other"}, + })) + s.Require().NoError(err) + otherID := otherResp.Msg.GetUser().GetId() + + aloneOrgID := createOrg(userCtx, "delete-current-user-alone") + aloneProjectID := createProject(userCtx, aloneOrgID, "delete-current-user-alone-project") + sharedOrgID := createOrg(userCtx, "delete-current-user-shared") + sharedProjectID := createProject(userCtx, sharedOrgID, "delete-current-user-shared-project") + addMember(sharedOrgID, otherID, s.orgViewerRole) + memberOrgID := createOrg(ctxOrgAdminAuth, "delete-current-user-member") + memberProjectID := createProject(ctxOrgAdminAuth, memberOrgID, "delete-current-user-member-project") + addMember(memberOrgID, userID, s.orgViewerRole) + _, err = s.testBench.Client.CreatePolicy(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePolicyRequest{ + Body: &frontierv1beta1.PolicyRequestBody{ + RoleId: schema.RoleProjectViewer, + Resource: schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, memberProjectID), + Principal: userSubject, + }, + })) + s.Require().NoError(err) + + resources := []struct{ name, resource string }{ + {"the org that goes with the user", schema.JoinNamespaceAndResourceID(schema.OrganizationNamespace, aloneOrgID)}, + {"a project in the org that goes with the user", schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, aloneProjectID)}, + {"the org it owns with another member", schema.JoinNamespaceAndResourceID(schema.OrganizationNamespace, sharedOrgID)}, + {"a project in the org it owns with another member", schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, sharedProjectID)}, + {"the org it is a member of", schema.JoinNamespaceAndResourceID(schema.OrganizationNamespace, memberOrgID)}, + {"a project it was given in that org", schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, memberProjectID)}, + } + for _, r := range resources { + s.Require().Truef(canGet(userSubject, r.resource), "access to %s before the delete", r.name) + } + for _, orgID := range []string{sharedOrgID, memberOrgID} { + inUsers, inRolePairs := listedInOrgUsers(orgID, userID) + s.Require().Truef(inUsers && inRolePairs, "listed in the users and role pairs of org %s before the delete", orgID) + } + + s.Run("the check lists the org that goes with the user and the orgs it leaves, and the only owner blocks the delete", func() { + resp, err := s.testBench.Client.CheckCurrentUserDelete(userCtx, connect.NewRequest(&frontierv1beta1.CheckCurrentUserDeleteRequest{})) + s.Require().NoError(err) + s.Assert().False(resp.Msg.GetCanDelete()) + s.Assert().Empty(resp.Msg.GetBlockers()) + s.Assert().Zero(resp.Msg.GetPatCount()) + + s.Require().Len(resp.Msg.GetOrganizationsToDelete(), 1) + toDelete := resp.Msg.GetOrganizationsToDelete()[0] + s.Assert().Equal(aloneOrgID, toDelete.GetId()) + s.Assert().Equal("delete-current-user-alone", toDelete.GetName()) + s.Assert().Equal("delete-current-user-alone", toDelete.GetTitle()) + s.Assert().True(toDelete.GetCanDelete()) + + toLeave := map[string]*frontierv1beta1.CheckCurrentUserDeleteResponse_OrganizationToLeave{} + for _, o := range resp.Msg.GetOrganizationsToLeave() { + toLeave[o.GetId()] = o + } + s.Require().Len(toLeave, 2) + s.Assert().Equal("Owner", toLeave[sharedOrgID].GetRole()) + s.Require().Len(toLeave[sharedOrgID].GetBlockers(), 1) + s.Assert().Equal(deleter.BlockerSoleOwnerWithMembers, toLeave[sharedOrgID].GetBlockers()[0].GetType()) + s.Assert().Equal(sharedOrgID, toLeave[sharedOrgID].GetBlockers()[0].GetSubject()) + s.Assert().Equal("Member", toLeave[memberOrgID].GetRole()) + s.Assert().Empty(toLeave[memberOrgID].GetBlockers()) + }) + + s.Run("a blocked delete fails with the blocker and changes nothing", func() { + _, err := s.testBench.Client.DeleteCurrentUser(userCtx, connect.NewRequest(&frontierv1beta1.DeleteCurrentUserRequest{})) + message, violations := requireDeleteRefused(s.T(), err) + s.Assert().True(strings.HasPrefix(message, "user cannot be deleted yet"), message) + s.Require().Len(violations, 1) + s.Assert().Equal(deleter.BlockerSoleOwnerWithMembers, violations[0].GetType()) + s.Assert().Equal(sharedOrgID, violations[0].GetSubject()) + + _, err = s.testBench.Client.GetCurrentUser(userCtx, connect.NewRequest(&frontierv1beta1.GetCurrentUserRequest{})) + s.Require().NoError(err) + s.Assert().True(orgIsLive(aloneOrgID)) + for _, r := range resources { + s.Assert().Truef(canGet(userSubject, r.resource), "access to %s", r.name) + } + }) + + s.Run("making the other member an owner clears the blocker", func() { + _, err := s.testBench.Client.SetOrganizationMemberRole(userCtx, connect.NewRequest(&frontierv1beta1.SetOrganizationMemberRoleRequest{ + OrgId: sharedOrgID, + UserId: otherID, + RoleId: s.orgOwnerRole, + })) + s.Require().NoError(err) + + resp, err := s.testBench.Client.CheckCurrentUserDelete(userCtx, connect.NewRequest(&frontierv1beta1.CheckCurrentUserDeleteRequest{})) + s.Require().NoError(err) + s.Assert().True(resp.Msg.GetCanDelete()) + }) + + s.Run("the delete goes through and clears the session cookie", func() { + resp, err := s.testBench.Client.DeleteCurrentUser(userCtx, connect.NewRequest(&frontierv1beta1.DeleteCurrentUserRequest{})) + s.Require().NoError(err) + setCookie := resp.Header().Get("Set-Cookie") + s.Assert().True(strings.HasPrefix(setCookie, consts.SessionRequestKey+"=;"), setCookie) + s.Assert().Contains(setCookie, "Max-Age=0") + }) + + s.Run("the old session cookie is rejected as unauthenticated", func() { + _, err := s.testBench.Client.GetCurrentUser(userCtx, connect.NewRequest(&frontierv1beta1.GetCurrentUserRequest{})) + s.Require().Error(err) + s.Assert().Equal(connect.CodeUnauthenticated, connect.CodeOf(err)) + }) + + s.Run("the deleted user has no access left in any org or project it belonged to", func() { + for _, r := range resources { + s.Assert().Falsef(canGet(userSubject, r.resource), "access to %s", r.name) + } + }) + + s.Run("no live policy names the deleted user", func() { + s.Assert().Empty(livePoliciesMatching(s.T(), ctxOrgAdminAuth, s.testBench.Client, &frontierv1beta1.ListPoliciesRequest{UserId: userID})) + var count int + s.Require().NoError(s.dbc.GetContext(context.Background(), &count, + "SELECT count(*) FROM policies WHERE principal_id = $1 AND deleted_at IS NULL", userID)) + s.Assert().Zero(count) + }) + + s.Run("org users names the deleted user in neither users nor role pairs", func() { + for _, orgID := range []string{sharedOrgID, memberOrgID} { + inUsers, inRolePairs := listedInOrgUsers(orgID, userID) + s.Assert().Falsef(inUsers, "listed in the users of org %s", orgID) + s.Assert().Falsef(inRolePairs, "listed in the role pairs of org %s", orgID) + } + }) + + s.Run("the org that went with the user is deleted, and the orgs it left stay", func() { + s.Assert().False(orgIsLive(aloneOrgID)) + s.Assert().True(orgIsLive(sharedOrgID)) + s.Assert().True(orgIsLive(memberOrgID)) + _, err := s.testBench.Client.GetUser(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.GetUserRequest{Id: userID})) + s.Assert().Equal(connect.CodeNotFound, connect.CodeOf(err)) + }) + + s.Run("the delete writes one app.user.deleted log and one user.deleted record", func() { + var logs, records int + s.Require().NoError(s.dbc.GetContext(context.Background(), &logs, + "SELECT count(*) FROM auditlogs WHERE action = $1 AND org_id = $2 AND target->>'ID' = $3", + "app.user.deleted", schema.PlatformOrgID.String(), userID)) + s.Require().NoError(s.dbc.GetContext(context.Background(), &records, + "SELECT count(*) FROM audit_records WHERE event = $1 AND target_id = $2", "user.deleted", userID)) + s.Assert().Equal(1, logs) + s.Assert().Equal(1, records) + }) + + s.Run("the same email signs up again as a new user", func() { + newID, newCtx := signUpUser(s.T(), s.testBench.Client, "delete-current-user@raystack.org") + s.Assert().NotEqual(userID, newID) + _, err := s.testBench.Client.GetUser(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.GetUserRequest{Id: userID})) + s.Assert().Equal(connect.CodeNotFound, connect.CodeOf(err)) + + resp, err := s.testBench.Client.CheckCurrentUserDelete(newCtx, connect.NewRequest(&frontierv1beta1.CheckCurrentUserDeleteRequest{})) + s.Require().NoError(err) + s.Assert().Empty(resp.Msg.GetOrganizationsToDelete()) + s.Assert().Empty(resp.Msg.GetOrganizationsToLeave()) + }) + + s.Run("a service user can neither check nor delete an account", func() { + serviceUserResp, err := s.testBench.Client.CreateServiceUser(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateServiceUserRequest{OrgId: memberOrgID})) + s.Require().NoError(err) + credentialResp, err := s.testBench.Client.CreateServiceUserCredential(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateServiceUserCredentialRequest{ + Id: serviceUserResp.Msg.GetServiceuser().GetId(), + OrgId: memberOrgID, + })) + s.Require().NoError(err) + secret := credentialResp.Msg.GetSecret() + serviceUserCtx := testbench.ContextWithHeaders(context.Background(), map[string]string{ + "Authorization": "Basic " + base64.StdEncoding.EncodeToString([]byte(secret.GetId()+":"+secret.GetSecret())), + }) + + _, err = s.testBench.Client.CheckCurrentUserDelete(serviceUserCtx, connect.NewRequest(&frontierv1beta1.CheckCurrentUserDeleteRequest{})) + s.Assert().Equal(connect.CodePermissionDenied, connect.CodeOf(err)) + _, err = s.testBench.Client.DeleteCurrentUser(serviceUserCtx, connect.NewRequest(&frontierv1beta1.DeleteCurrentUserRequest{})) + s.Assert().Equal(connect.CodePermissionDenied, connect.CodeOf(err)) + _, err = s.testBench.Client.GetCurrentUser(serviceUserCtx, connect.NewRequest(&frontierv1beta1.GetCurrentUserRequest{})) + s.Assert().NoError(err) + }) +} + func (s *APIRegressionTestSuite) TestRelationAPI() { ctxOrgAdminAuth := testbench.ContextWithAuth(context.Background(), s.adminCookie) diff --git a/test/e2e/regression/pat_test.go b/test/e2e/regression/pat_test.go index 5c5ec683e..07aaf5be7 100644 --- a/test/e2e/regression/pat_test.go +++ b/test/e2e/regression/pat_test.go @@ -861,6 +861,43 @@ func (s *PATRegressionTestSuite) TestPATOfDeletedOrgStopsAuthenticating() { }) } +func (s *PATRegressionTestSuite) TestPATCannotCheckOrDeleteItsOwnersAccount() { + _, userCtx := signUpUser(s.T(), s.testBench.Client, "pat-account-delete@raystack.org") + orgResp, err := s.testBench.Client.CreateOrganization(userCtx, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + Body: &frontierv1beta1.OrganizationRequestBody{Name: "org-pat-account-delete"}, + })) + s.Require().NoError(err) + _, patToken := s.createPAT(userCtx, orgResp.Msg.GetOrganization().GetId(), "pat-account-delete", []*frontierv1beta1.PATScope{ + {RoleId: s.roleID(schema.RoleOrganizationViewer), ResourceType: schema.OrganizationNamespace}, + }) + patCtx := getPATCtx(patToken) + _, err = s.testBench.Client.GetCurrentUser(patCtx, connect.NewRequest(&frontierv1beta1.GetCurrentUserRequest{})) + s.Require().NoError(err) + + s.Run("the token is refused for both calls", func() { + _, err := s.testBench.Client.CheckCurrentUserDelete(patCtx, connect.NewRequest(&frontierv1beta1.CheckCurrentUserDeleteRequest{})) + s.Assert().Equal(connect.CodePermissionDenied, connect.CodeOf(err)) + s.Assert().ErrorContains(err, "personal access token") + _, err = s.testBench.Client.DeleteCurrentUser(patCtx, connect.NewRequest(&frontierv1beta1.DeleteCurrentUserRequest{})) + s.Assert().Equal(connect.CodePermissionDenied, connect.CodeOf(err)) + s.Assert().ErrorContains(err, "personal access token") + }) + + s.Run("the check counts the token", func() { + resp, err := s.testBench.Client.CheckCurrentUserDelete(userCtx, connect.NewRequest(&frontierv1beta1.CheckCurrentUserDeleteRequest{})) + s.Require().NoError(err) + s.Assert().Equal(int32(1), resp.Msg.GetPatCount()) + }) + + s.Run("the token stops working once its owner deletes the account", func() { + _, err := s.testBench.Client.DeleteCurrentUser(userCtx, connect.NewRequest(&frontierv1beta1.DeleteCurrentUserRequest{})) + s.Require().NoError(err) + _, err = s.testBench.Client.GetCurrentUser(patCtx, connect.NewRequest(&frontierv1beta1.GetCurrentUserRequest{})) + s.Require().Error(err) + s.Assert().Equal(connect.CodeUnauthenticated, connect.CodeOf(err)) + }) +} + func TestEndToEndPATRegressionTestSuite(t *testing.T) { suite.Run(t, new(PATRegressionTestSuite)) }