From c60c015e619aebc973148d995abb315ec91a43b1 Mon Sep 17 00:00:00 2001 From: aman Date: Sun, 25 Oct 2026 13:41:02 +0530 Subject: [PATCH 1/2] fix(user): user delete ends the user's sessions first Delete soft-deletes the user's sessions before it removes the relations and the user row. Today the sessions go only through the sessions.user_id ON DELETE CASCADE, which stops firing once the user row is kept. --- core/user/service.go | 3 +++ core/user/service_test.go | 28 +++++++++++++++++++++------- test/e2e/regression/api_test.go | 30 ++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/core/user/service.go b/core/user/service.go index 6763ed83f..d7ef7ef96 100644 --- a/core/user/service.go +++ b/core/user/service.go @@ -160,6 +160,9 @@ func (s Service) Disable(ctx context.Context, id string) error { // Delete by user uuid // don't call this directly, use cascade deleter func (s Service) Delete(ctx context.Context, id string) error { + if err := s.sessionService.DeleteByUserID(ctx, id); err != nil { + return err + } if err := s.relationService.Delete(ctx, relation.Relation{Subject: relation.Subject{ ID: id, Namespace: schema.UserPrincipal, diff --git a/core/user/service_test.go b/core/user/service_test.go index 7c98dd5c5..6e7038c18 100644 --- a/core/user/service_test.go +++ b/core/user/service_test.go @@ -592,6 +592,23 @@ func TestService_Enable(t *testing.T) { } func TestService_Delete(t *testing.T) { + t.Run("ends the sessions, then removes the relations, then the user", func(t *testing.T) { + repo, relationService, sessionService, auditRecordRepository := mockService(t) + var steps []string + sessionService.EXPECT().DeleteByUserID(mock.Anything, "test-id"). + Run(func(context.Context, string) { steps = append(steps, "sessions") }).Return(nil) + relationService.EXPECT().Delete(mock.Anything, relation.Relation{Subject: relation.Subject{ + ID: "test-id", + Namespace: schema.UserPrincipal, + }}).Run(func(context.Context, relation.Relation) { steps = append(steps, "relations") }).Return(nil) + repo.EXPECT().Delete(mock.Anything, "test-id"). + Run(func(context.Context, string) { steps = append(steps, "user") }).Return(nil) + + err := user.NewService(repo, relationService, sessionService, auditRecordRepository).Delete(context.Background(), "test-id") + assert.NoError(t, err) + assert.Equal(t, []string{"sessions", "relations", "user"}, steps) + }) + tests := []struct { name string id string @@ -599,16 +616,12 @@ func TestService_Delete(t *testing.T) { wantErr bool }{ { - name: "while deleting user, delete it's relations", + name: "fail on ending sessions shouldn't remove relations or user", id: "test-id", - wantErr: false, + wantErr: true, setup: func() *user.Service { repo, relationService, sessionService, auditRecordRepository := mockService(t) - relationService.EXPECT().Delete(mock.Anything, relation.Relation{Subject: relation.Subject{ - ID: "test-id", - Namespace: schema.UserPrincipal, - }}).Return(nil) - repo.EXPECT().Delete(mock.Anything, "test-id").Return(nil) + sessionService.EXPECT().DeleteByUserID(mock.Anything, "test-id").Return(errors.New("failed to end sessions")) return user.NewService(repo, relationService, sessionService, auditRecordRepository) }, }, @@ -618,6 +631,7 @@ func TestService_Delete(t *testing.T) { wantErr: true, setup: func() *user.Service { repo, relationService, sessionService, auditRecordRepository := mockService(t) + sessionService.EXPECT().DeleteByUserID(mock.Anything, "test-id").Return(nil) relationService.EXPECT().Delete(mock.Anything, relation.Relation{Subject: relation.Subject{ ID: "test-id", Namespace: schema.UserPrincipal, diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index 62be22653..a9a1c24ba 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -1721,6 +1721,36 @@ func (s *APIRegressionTestSuite) TestUserAPI() { })) s.Require().NoError(err) + _, err = s.testBench.Client.GetCurrentUser(ctxTarget, connect.NewRequest(&frontierv1beta1.GetCurrentUserRequest{})) + s.Require().Error(err) + s.Assert().Equal(connect.CodeUnauthenticated, connect.CodeOf(err)) + }) + s.Run("13. session is rejected on the next request after the user is deleted", func() { + const targetEmail = "delete-session-target@raystack.org" + + createResp, err := s.testBench.Client.CreateUser(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateUserRequest{ + Body: &frontierv1beta1.UserRequestBody{ + Title: "delete session target", + Email: targetEmail, + Name: "delete_session_target", + }, + })) + s.Require().NoError(err) + targetID := createResp.Msg.GetUser().GetId() + + targetCookie, err := testbench.AuthenticateUser(context.Background(), s.testBench.Client, targetEmail) + s.Require().NoError(err) + ctxTarget := testbench.ContextWithAuth(context.Background(), targetCookie) + + meResp, err := s.testBench.Client.GetCurrentUser(ctxTarget, connect.NewRequest(&frontierv1beta1.GetCurrentUserRequest{})) + s.Require().NoError(err) + s.Assert().Equal(targetEmail, meResp.Msg.GetUser().GetEmail()) + + _, err = s.testBench.Client.DeleteUser(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeleteUserRequest{ + Id: targetID, + })) + s.Require().NoError(err) + _, err = s.testBench.Client.GetCurrentUser(ctxTarget, connect.NewRequest(&frontierv1beta1.GetCurrentUserRequest{})) s.Require().Error(err) s.Assert().Equal(connect.CodeUnauthenticated, connect.CodeOf(err)) From 717deab6592f3a7252d5f60e511f187cc46e7205 Mon Sep 17 00:00:00 2001 From: aman Date: Tue, 27 Oct 2026 17:12:28 +0530 Subject: [PATCH 2/2] fix(user): user delete ends the sessions just before the user row Delete now removes the relations first, then ends the sessions, then deletes the row. If the relations step fails, the user stays signed in and can retry. --- core/user/service.go | 6 +++--- core/user/service_test.go | 21 ++++++++++++--------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/core/user/service.go b/core/user/service.go index d7ef7ef96..a88bf4cb9 100644 --- a/core/user/service.go +++ b/core/user/service.go @@ -160,15 +160,15 @@ func (s Service) Disable(ctx context.Context, id string) error { // Delete by user uuid // don't call this directly, use cascade deleter func (s Service) Delete(ctx context.Context, id string) error { - if err := s.sessionService.DeleteByUserID(ctx, id); err != nil { - return err - } if err := s.relationService.Delete(ctx, relation.Relation{Subject: relation.Subject{ ID: id, Namespace: schema.UserPrincipal, }}); err != nil { return err } + if err := s.sessionService.DeleteByUserID(ctx, id); err != nil { + return err + } return s.repository.Delete(ctx, id) } diff --git a/core/user/service_test.go b/core/user/service_test.go index 6e7038c18..792568857 100644 --- a/core/user/service_test.go +++ b/core/user/service_test.go @@ -592,21 +592,21 @@ func TestService_Enable(t *testing.T) { } func TestService_Delete(t *testing.T) { - t.Run("ends the sessions, then removes the relations, then the user", func(t *testing.T) { + t.Run("removes the relations, then ends the sessions, then removes the user", func(t *testing.T) { repo, relationService, sessionService, auditRecordRepository := mockService(t) var steps []string - sessionService.EXPECT().DeleteByUserID(mock.Anything, "test-id"). - Run(func(context.Context, string) { steps = append(steps, "sessions") }).Return(nil) relationService.EXPECT().Delete(mock.Anything, relation.Relation{Subject: relation.Subject{ ID: "test-id", Namespace: schema.UserPrincipal, }}).Run(func(context.Context, relation.Relation) { steps = append(steps, "relations") }).Return(nil) + sessionService.EXPECT().DeleteByUserID(mock.Anything, "test-id"). + Run(func(context.Context, string) { steps = append(steps, "sessions") }).Return(nil) repo.EXPECT().Delete(mock.Anything, "test-id"). Run(func(context.Context, string) { steps = append(steps, "user") }).Return(nil) err := user.NewService(repo, relationService, sessionService, auditRecordRepository).Delete(context.Background(), "test-id") assert.NoError(t, err) - assert.Equal(t, []string{"sessions", "relations", "user"}, steps) + assert.Equal(t, []string{"relations", "sessions", "user"}, steps) }) tests := []struct { @@ -616,26 +616,29 @@ func TestService_Delete(t *testing.T) { wantErr bool }{ { - name: "fail on ending sessions shouldn't remove relations or user", + name: "fail on delete relations shouldn't end sessions or remove user", id: "test-id", wantErr: true, setup: func() *user.Service { repo, relationService, sessionService, auditRecordRepository := mockService(t) - sessionService.EXPECT().DeleteByUserID(mock.Anything, "test-id").Return(errors.New("failed to end sessions")) + relationService.EXPECT().Delete(mock.Anything, relation.Relation{Subject: relation.Subject{ + ID: "test-id", + Namespace: schema.UserPrincipal, + }}).Return(errors.New("failed to delete relation")) return user.NewService(repo, relationService, sessionService, auditRecordRepository) }, }, { - name: "fail on delete relations shouldn't remove user", + name: "fail on ending sessions shouldn't remove user", id: "test-id", wantErr: true, setup: func() *user.Service { repo, relationService, sessionService, auditRecordRepository := mockService(t) - sessionService.EXPECT().DeleteByUserID(mock.Anything, "test-id").Return(nil) relationService.EXPECT().Delete(mock.Anything, relation.Relation{Subject: relation.Subject{ ID: "test-id", Namespace: schema.UserPrincipal, - }}).Return(errors.New("failed to delete relation")) + }}).Return(nil) + sessionService.EXPECT().DeleteByUserID(mock.Anything, "test-id").Return(errors.New("failed to end sessions")) return user.NewService(repo, relationService, sessionService, auditRecordRepository) }, },