From c64fd054ef12fba7b8a09372503651e181d5f54c Mon Sep 17 00:00:00 2001 From: aman Date: Sun, 25 Oct 2026 13:22:27 +0530 Subject: [PATCH 1/2] feat(store): user delete keeps the row The user row gets deleted_at instead of being removed, and the user.deleted audit record takes its occurred_at from deleted_at. A second delete answers not found. --- internal/store/postgres/user_repository.go | 4 +-- .../store/postgres/user_repository_test.go | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/internal/store/postgres/user_repository.go b/internal/store/postgres/user_repository.go index 2b402ac54..65cf4b323 100644 --- a/internal/store/postgres/user_repository.go +++ b/internal/store/postgres/user_repository.go @@ -556,7 +556,7 @@ func (r UserRepository) SetState(ctx context.Context, id string, state user.Stat } func (r UserRepository) Delete(ctx context.Context, id string) error { - query, params, err := dialect.Delete(TABLE_USERS).Where( + query, params, err := softDelete(TABLE_USERS).Where( goqu.Ex{ "id": id, }, @@ -572,7 +572,7 @@ func (r UserRepository) Delete(ctx context.Context, id string) error { }); err != nil { return err } - return InsertAuditRecordInTx(ctx, tx, buildUserAuditRecord(ctx, auditrecord.UserDeletedEvent, userModel, time.Now().UTC())) + return InsertAuditRecordInTx(ctx, tx, buildUserAuditRecord(ctx, auditrecord.UserDeletedEvent, userModel, userModel.DeletedAt.Time)) }); err != nil { err = checkPostgresError(err) switch { diff --git a/internal/store/postgres/user_repository_test.go b/internal/store/postgres/user_repository_test.go index 26bec102c..7838208da 100644 --- a/internal/store/postgres/user_repository_test.go +++ b/internal/store/postgres/user_repository_test.go @@ -2,9 +2,11 @@ package postgres_test import ( "context" + "database/sql" "errors" "fmt" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -506,6 +508,32 @@ func (s *UserRepositoryTestSuite) TestDelete() { } } +func (s *UserRepositoryTestSuite) TestDeleteKeepsTheRow() { + deleted := s.users[0] + + s.Require().NoError(s.repository.Delete(s.ctx, deleted.ID)) + + var deletedAt sql.NullTime + s.Require().NoError(s.client.GetContext(s.ctx, &deletedAt, "SELECT deleted_at FROM users WHERE id = $1", deleted.ID)) + s.Require().True(deletedAt.Valid) + + var occurredAt time.Time + s.Require().NoError(s.client.GetContext(s.ctx, &occurredAt, + "SELECT occurred_at FROM audit_records WHERE event = $1 AND target_id = $2", + pkgAuditRecord.UserDeletedEvent.String(), deleted.ID)) + s.Assert().WithinDuration(deletedAt.Time, occurredAt, 0) + + _, err := s.repository.GetByID(s.ctx, deleted.ID) + s.Assert().ErrorIs(err, user.ErrNotExist) + + s.Assert().ErrorIs(s.repository.Delete(s.ctx, deleted.ID), user.ErrNotExist) + s.assertAudited(pkgAuditRecord.UserDeletedEvent.String(), deleted.ID, uuid.Nil.String()) + + again, err := s.repository.Create(s.ctx, user.User{Name: deleted.Name, Email: deleted.Email}) + s.Require().NoError(err) + s.Assert().NotEqual(deleted.ID, again.ID) +} + func (s *UserRepositoryTestSuite) TestSetState() { type testCase struct { Description string From a9ace05cbd6e165d634bb4411f11b091bb968ecd Mon Sep 17 00:00:00 2001 From: aman Date: Sun, 25 Oct 2026 13:22:28 +0530 Subject: [PATCH 2/2] feat(store): org PAT search skips PATs whose owner is deleted The search inner joins users with the live filter instead of a left join, so the list and the count skip a PAT whose owner is soft-deleted. --- .../store/postgres/org_pats_repository.go | 4 +- .../postgres/org_pats_repository_pg_test.go | 90 +++++++++++++++++++ .../postgres/org_pats_repository_test.go | 4 + 3 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 internal/store/postgres/org_pats_repository_pg_test.go diff --git a/internal/store/postgres/org_pats_repository.go b/internal/store/postgres/org_pats_repository.go index f6d8918b3..bf0df723c 100644 --- a/internal/store/postgres/org_pats_repository.go +++ b/internal/store/postgres/org_pats_repository.go @@ -141,9 +141,9 @@ func (r OrgPATsRepository) Search(ctx context.Context, orgID string, rqlQuery *r // buildInnerSubquery builds the base PAT query with filters and search applied. func (r OrgPATsRepository) buildInnerSubquery(orgID string, rqlQuery *rql.Query) (*goqu.SelectDataset, error) { inner := dialect.From(goqu.T(TABLE_USER_PATS).As("p")). - LeftJoin( + InnerJoin( goqu.T(TABLE_USERS).As("u"), - goqu.On(goqu.I("p.user_id").Eq(goqu.I("u.id"))), + goqu.On(goqu.I("p.user_id").Eq(goqu.I("u.id")), live("u")), ). Where( goqu.I("p.org_id").Eq(orgID), diff --git a/internal/store/postgres/org_pats_repository_pg_test.go b/internal/store/postgres/org_pats_repository_pg_test.go new file mode 100644 index 000000000..23fcfa59f --- /dev/null +++ b/internal/store/postgres/org_pats_repository_pg_test.go @@ -0,0 +1,90 @@ +package postgres_test + +import ( + "context" + "fmt" + "testing" + + "github.com/raystack/frontier/internal/store/postgres" + "github.com/raystack/frontier/pkg/db" + "github.com/raystack/salt/rql" + "github.com/stretchr/testify/suite" +) + +type OrgPATsRepositoryPGTestSuite struct { + suite.Suite + ctx context.Context + client *db.Client + repository *postgres.OrgPATsRepository + orgID string +} + +func (s *OrgPATsRepositoryPGTestSuite) SetupSuite() { + var err error + s.client, err = newTestClient() + if err != nil { + s.T().Fatal(err) + } + s.ctx = context.TODO() + s.repository = postgres.NewOrgPATsRepository(s.client) +} + +func (s *OrgPATsRepositoryPGTestSuite) TearDownSuite() { + if err := closeTestClient(s.client); err != nil { + s.T().Fatal(err) + } +} + +func (s *OrgPATsRepositoryPGTestSuite) SetupTest() { + execSQL(s.T(), s.ctx, s.client, `INSERT INTO organizations (name, title) VALUES ('op-org', 'Org PATs Org')`) + s.orgID = scalarSQL(s.T(), s.ctx, s.client, `SELECT id FROM organizations WHERE name = 'op-org'`) + + execSQL(s.T(), s.ctx, s.client, `INSERT INTO users (name, email, title) VALUES + ('op-uma', 'op-uma@example.com', 'Uma'), + ('op-gary', 'op-gary@example.com', 'Gary')`) + execSQL(s.T(), s.ctx, s.client, `INSERT INTO user_pats (user_id, org_id, title, secret_hash, expires_at) VALUES + ((SELECT id FROM users WHERE name = 'op-uma'), $1, 'deploy-token', 'op-hash-uma', now() + interval '1 day'), + ((SELECT id FROM users WHERE name = 'op-gary'), $1, 'backup-token', 'op-hash-gary', now() + interval '1 day')`, + s.orgID) + execSQL(s.T(), s.ctx, s.client, `UPDATE users SET deleted_at = now() WHERE name = 'op-gary'`) +} + +func (s *OrgPATsRepositoryPGTestSuite) TearDownTest() { + queries := []string{} + for _, table := range []string{postgres.TABLE_USER_PATS, postgres.TABLE_USERS, postgres.TABLE_ORGANIZATIONS} { + queries = append(queries, fmt.Sprintf("TRUNCATE TABLE %s RESTART IDENTITY CASCADE", table)) + } + if err := execQueries(s.ctx, s.client, queries); err != nil { + s.T().Fatal(err) + } +} + +func (s *OrgPATsRepositoryPGTestSuite) search(text string) ([]string, int64) { + s.T().Helper() + res, err := s.repository.Search(s.ctx, s.orgID, &rql.Query{Limit: 50, Search: text}) + s.Require().NoError(err) + titles := make([]string, 0, len(res.PATs)) + for _, pat := range res.PATs { + titles = append(titles, pat.Title) + } + return titles, res.Pagination.TotalCount +} + +func (s *OrgPATsRepositoryPGTestSuite) TestSkipsPATsOfSoftDeletedUsers() { + titles, total := s.search("") + s.Equal([]string{"deploy-token"}, titles) + s.Equal(int64(1), total) +} + +func (s *OrgPATsRepositoryPGTestSuite) TestSearchMatchesLiveOwnersOnly() { + titles, _ := s.search("Uma") + s.Equal([]string{"deploy-token"}, titles) + + titles, total := s.search("Gary") + s.Empty(titles, "the owner is soft-deleted") + s.Zero(total) +} + +func TestOrgPATsRepositoryPG(t *testing.T) { + suite.Run(t, new(OrgPATsRepositoryPGTestSuite)) +} diff --git a/internal/store/postgres/org_pats_repository_test.go b/internal/store/postgres/org_pats_repository_test.go index aff8a7e09..28895e22c 100644 --- a/internal/store/postgres/org_pats_repository_test.go +++ b/internal/store/postgres/org_pats_repository_test.go @@ -12,6 +12,8 @@ import ( "github.com/stretchr/testify/assert" ) +const liveOwnerJoin = `INNER JOIN "users" AS "u" ON (("p"."user_id" = "u"."id") AND ("u"."deleted_at" IS NULL))` + func TestOrgPATsRepository_buildCountQuery(t *testing.T) { r := OrgPATsRepository{} @@ -66,6 +68,7 @@ func TestOrgPATsRepository_buildCountQuery(t *testing.T) { assert.NoError(t, err) assert.NotEmpty(t, sql) assert.Contains(t, sql, "COUNT(*)") + assert.Contains(t, sql, liveOwnerJoin) assert.NotContains(t, sql, "ORDER BY") assert.NotEmpty(t, params) }) @@ -135,6 +138,7 @@ func TestOrgPATsRepository_buildDataQuery(t *testing.T) { assert.Contains(t, sql, "pat_id") assert.Contains(t, sql, "created_by_title") assert.Contains(t, sql, "pol") + assert.Contains(t, sql, liveOwnerJoin) assert.NotEmpty(t, params) // the live filter sits in the policies join, not in a WHERE clause