From 9d938edbf5eadbf17e20007926af78228487a8c3 Mon Sep 17 00:00:00 2001 From: aman Date: Mon, 5 Oct 2026 16:04:00 +0530 Subject: [PATCH 1/5] feat(store): policy and relation tuples are unique over live rows only The plain unique constraints on policies and relations are replaced by partial unique indexes over rows where deleted_at is null, and both upserts name those indexes in their conflict target. A soft-deleted row no longer blocks the same tuple from being written again, and a duplicate of a live row still updates it in place. --- ...00_policies_relations_live_unique.down.sql | 5 ++ ...0000_policies_relations_live_unique.up.sql | 5 ++ internal/store/postgres/policy_repository.go | 2 +- .../store/postgres/policy_repository_test.go | 48 +++++++++++++++++++ .../store/postgres/relation_repository.go | 2 +- .../postgres/relation_repository_test.go | 37 ++++++++++++++ 6 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 internal/store/postgres/migrations/20261005100000_policies_relations_live_unique.down.sql create mode 100644 internal/store/postgres/migrations/20261005100000_policies_relations_live_unique.up.sql diff --git a/internal/store/postgres/migrations/20261005100000_policies_relations_live_unique.down.sql b/internal/store/postgres/migrations/20261005100000_policies_relations_live_unique.down.sql new file mode 100644 index 000000000..6557ceabc --- /dev/null +++ b/internal/store/postgres/migrations/20261005100000_policies_relations_live_unique.down.sql @@ -0,0 +1,5 @@ +ALTER TABLE policies ADD CONSTRAINT policies_role_id_resource_id_resource_type_principal_id_pri_key UNIQUE (role_id, resource_id, resource_type, principal_id, principal_type); +DROP INDEX IF EXISTS uq_policies_role_resource_principal_live; + +ALTER TABLE relations ADD CONSTRAINT relations_subject_namespace_name_subject_id_object_namespac_key UNIQUE (subject_namespace_name, subject_id, object_namespace_name, object_id, relation_name); +DROP INDEX IF EXISTS uq_relations_subject_object_relation_live; diff --git a/internal/store/postgres/migrations/20261005100000_policies_relations_live_unique.up.sql b/internal/store/postgres/migrations/20261005100000_policies_relations_live_unique.up.sql new file mode 100644 index 000000000..897073b53 --- /dev/null +++ b/internal/store/postgres/migrations/20261005100000_policies_relations_live_unique.up.sql @@ -0,0 +1,5 @@ +CREATE UNIQUE INDEX IF NOT EXISTS uq_policies_role_resource_principal_live ON policies (role_id, resource_id, resource_type, principal_id, principal_type) WHERE deleted_at IS NULL; +ALTER TABLE policies DROP CONSTRAINT IF EXISTS policies_role_id_resource_id_resource_type_principal_id_pri_key; + +CREATE UNIQUE INDEX IF NOT EXISTS uq_relations_subject_object_relation_live ON relations (subject_namespace_name, subject_id, object_namespace_name, object_id, relation_name) WHERE deleted_at IS NULL; +ALTER TABLE relations DROP CONSTRAINT IF EXISTS relations_subject_namespace_name_subject_id_object_namespac_key; diff --git a/internal/store/postgres/policy_repository.go b/internal/store/postgres/policy_repository.go index 3887c90ae..05d101160 100644 --- a/internal/store/postgres/policy_repository.go +++ b/internal/store/postgres/policy_repository.go @@ -219,7 +219,7 @@ func (r PolicyRepository) Upsert(ctx context.Context, pol policy.Policy) (policy "principal_type": pol.PrincipalType, "grant_relation": pol.GrantRelation, "metadata": marshaledMetadata, - }).OnConflict(goqu.DoUpdate("role_id, resource_id, resource_type, principal_id, principal_type", goqu.Record{ + }).OnConflict(goqu.DoUpdate(liveConflictTarget("role_id, resource_id, resource_type, principal_id, principal_type"), goqu.Record{ "grant_relation": pol.GrantRelation, "metadata": marshaledMetadata, "updated_at": goqu.L("now()"), diff --git a/internal/store/postgres/policy_repository_test.go b/internal/store/postgres/policy_repository_test.go index cd07fed37..50be78f1b 100644 --- a/internal/store/postgres/policy_repository_test.go +++ b/internal/store/postgres/policy_repository_test.go @@ -18,6 +18,7 @@ import ( "github.com/raystack/frontier/core/policy" "github.com/raystack/frontier/internal/store/postgres" "github.com/raystack/frontier/pkg/db" + "github.com/raystack/frontier/pkg/metadata" ) type PolicyRepositoryTestSuite struct { @@ -203,6 +204,53 @@ func (s *PolicyRepositoryTestSuite) TestCreate() { } }) } + + s.Run("should create a new row when the same policy was soft-deleted", func() { + pol := policy.Policy{ + RoleID: s.roles[0].ID, + ResourceID: uuid.NewString(), + ResourceType: "ns1", + PrincipalID: s.userID, + PrincipalType: schema.UserPrincipal, + } + first, err := s.repository.Upsert(s.ctx, pol) + s.Require().NoError(err) + if _, err := s.client.ExecContext(s.ctx, "UPDATE policies SET deleted_at = now() WHERE id = $1", first.ID); err != nil { + s.T().Fatal(err) + } + + second, err := s.repository.Upsert(s.ctx, pol) + s.Assert().NoError(err) + s.Assert().NotEqual(first.ID, second.ID) + + var firstStillDeleted bool + if err := s.client.QueryRowxContext(s.ctx, "SELECT deleted_at IS NOT NULL FROM policies WHERE id = $1", first.ID).Scan(&firstStillDeleted); err != nil { + s.T().Fatal(err) + } + s.Assert().True(firstStillDeleted) + + _, err = s.repository.Get(s.ctx, second.ID) + s.Assert().NoError(err) + }) + + s.Run("should update the live policy in place when the same policy exists", func() { + pol := policy.Policy{ + RoleID: s.roles[0].ID, + ResourceID: uuid.NewString(), + ResourceType: "ns1", + PrincipalID: s.userID, + PrincipalType: schema.UserPrincipal, + } + before, err := s.repository.Upsert(s.ctx, pol) + s.Require().NoError(err) + + pol.Metadata = metadata.Metadata{"team": "maps"} + got, err := s.repository.Upsert(s.ctx, pol) + s.Assert().NoError(err) + s.Assert().Equal(before.ID, got.ID) + s.Assert().Equal(metadata.Metadata{"team": "maps"}, got.Metadata) + s.Assert().True(got.UpdatedAt.After(before.UpdatedAt)) + }) } func (s *PolicyRepositoryTestSuite) TestList() { diff --git a/internal/store/postgres/relation_repository.go b/internal/store/postgres/relation_repository.go index 9d93b9007..c3f71311b 100644 --- a/internal/store/postgres/relation_repository.go +++ b/internal/store/postgres/relation_repository.go @@ -35,7 +35,7 @@ func (r RelationRepository) Upsert(ctx context.Context, relationToCreate relatio "created_at": goqu.L("now()"), "updated_at": goqu.L("now()"), }).OnConflict( - goqu.DoUpdate("subject_namespace_name, subject_id, object_namespace_name, object_id, relation_name", goqu.Record{ + goqu.DoUpdate(liveConflictTarget("subject_namespace_name, subject_id, object_namespace_name, object_id, relation_name"), goqu.Record{ "subject_namespace_name": relationToCreate.Subject.Namespace, })).Returning(&relationCols{}).ToSQL() if err != nil { diff --git a/internal/store/postgres/relation_repository_test.go b/internal/store/postgres/relation_repository_test.go index c80afa06b..37c17496d 100644 --- a/internal/store/postgres/relation_repository_test.go +++ b/internal/store/postgres/relation_repository_test.go @@ -207,6 +207,43 @@ func (s *RelationRepositoryTestSuite) TestUpsert() { } }) } + + s.Run("should create a new row when the same relation was soft-deleted", func() { + rel := relation.Relation{ + Subject: relation.Subject{ID: uuid.NewString(), Namespace: "ns1"}, + Object: relation.Object{ID: uuid.NewString(), Namespace: "ns1"}, + RelationName: "relation1", + } + first, err := s.repository.Upsert(s.ctx, rel) + s.Require().NoError(err) + if _, err := s.client.ExecContext(s.ctx, "UPDATE relations SET deleted_at = now() WHERE id = $1", first.ID); err != nil { + s.T().Fatal(err) + } + + second, err := s.repository.Upsert(s.ctx, rel) + s.Assert().NoError(err) + s.Assert().NotEqual(first.ID, second.ID) + + var firstStillDeleted bool + if err := s.client.QueryRowxContext(s.ctx, "SELECT deleted_at IS NOT NULL FROM relations WHERE id = $1", first.ID).Scan(&firstStillDeleted); err != nil { + s.T().Fatal(err) + } + s.Assert().True(firstStillDeleted) + }) + + s.Run("should return the live relation when the same relation exists", func() { + rel := relation.Relation{ + Subject: relation.Subject{ID: uuid.NewString(), Namespace: "ns1"}, + Object: relation.Object{ID: uuid.NewString(), Namespace: "ns1"}, + RelationName: "relation1", + } + first, err := s.repository.Upsert(s.ctx, rel) + s.Require().NoError(err) + + second, err := s.repository.Upsert(s.ctx, rel) + s.Assert().NoError(err) + s.Assert().Equal(first.ID, second.ID) + }) } func (s *RelationRepositoryTestSuite) TestList() { From f6ce7bbfc3b05a8d5b69714146231bb7e8d781ae Mon Sep 17 00:00:00 2001 From: aman Date: Mon, 5 Oct 2026 16:24:59 +0530 Subject: [PATCH 2/5] feat(store): policy and relation deletes keep the row Policy delete, the owner-guarded policy delete, and relation delete set deleted_at instead of removing the row. The SpiceDB tuples are still removed by the services as before. A delete that finds no live row reports not found, so a second delete of the same policy or relation no longer writes an audit record or passes silently. --- internal/store/postgres/policy_repository.go | 19 +++-- .../store/postgres/policy_repository_test.go | 69 +++++++++++++++++++ .../store/postgres/relation_repository.go | 2 +- .../postgres/relation_repository_test.go | 23 +++++++ 4 files changed, 107 insertions(+), 6 deletions(-) diff --git a/internal/store/postgres/policy_repository.go b/internal/store/postgres/policy_repository.go index 05d101160..47171750d 100644 --- a/internal/store/postgres/policy_repository.go +++ b/internal/store/postgres/policy_repository.go @@ -347,15 +347,24 @@ func (r PolicyRepository) Delete(ctx context.Context, id string) error { if err := r.dbc.WithTxn(ctx, sql.TxOptions{}, func(tx *sqlx.Tx) error { return r.dbc.WithTimeout(ctx, TABLE_POLICIES, "Delete", func(ctx context.Context) error { - deleteQuery, deleteParams, err := dialect.Delete(TABLE_POLICIES). + deleteQuery, deleteParams, err := softDelete(TABLE_POLICIES). Where(goqu.Ex{"id": id}). ToSQL() if err != nil { return fmt.Errorf("%w: %w", errQuery, err) } - if _, err := tx.ExecContext(ctx, deleteQuery, deleteParams...); err != nil { + result, err := tx.ExecContext(ctx, deleteQuery, deleteParams...) + if err != nil { return err } + deleted, err := result.RowsAffected() + if err != nil { + return err + } + if deleted == 0 { + // the row went away after the Get above, so there is nothing to audit + return sql.ErrNoRows + } policyDB := Policy{ ID: existingPolicy.ID, @@ -380,7 +389,7 @@ func (r PolicyRepository) Delete(ctx context.Context, id string) error { return nil } -// DeleteWithMinRoleGuard atomically deletes a policy only if at least one other +// DeleteWithMinRoleGuard atomically marks a policy deleted only if at least one other // policy with the same guarded role remains for the resource. Uses SELECT FOR UPDATE // to serialize concurrent deletions under READ COMMITTED isolation, preventing the // TOCTOU race where two concurrent requests both pass a count check then both delete. @@ -402,7 +411,7 @@ func (r PolicyRepository) DeleteWithMinRoleGuard(ctx context.Context, id string, ORDER BY id FOR UPDATE ) - DELETE FROM ` + TABLE_POLICIES + ` WHERE id = $1 AND ( + UPDATE ` + TABLE_POLICIES + ` SET deleted_at = now() WHERE id = $1 AND deleted_at IS NULL AND ( (SELECT role_id FROM ` + TABLE_POLICIES + ` WHERE id = $1) != $4 OR (SELECT COUNT(*) FROM locked WHERE id != $1) > 0 )` @@ -422,7 +431,7 @@ func (r PolicyRepository) DeleteWithMinRoleGuard(ctx context.Context, id string, if rowsAffected == 0 { var existingID string err := tx.QueryRowContext(ctx, - `SELECT id FROM `+TABLE_POLICIES+` WHERE id = $1`, id, + `SELECT id FROM `+TABLE_POLICIES+` WHERE id = $1 AND deleted_at IS NULL`, id, ).Scan(&existingID) if errors.Is(err, sql.ErrNoRows) { return sql.ErrNoRows diff --git a/internal/store/postgres/policy_repository_test.go b/internal/store/postgres/policy_repository_test.go index 50be78f1b..8d36aa835 100644 --- a/internal/store/postgres/policy_repository_test.go +++ b/internal/store/postgres/policy_repository_test.go @@ -389,6 +389,36 @@ func (s *PolicyRepositoryTestSuite) TestDelete() { } }) } + + newPolicy := func() policy.Policy { + created, err := s.repository.Upsert(s.ctx, policy.Policy{ + RoleID: s.roles[0].ID, + ResourceID: uuid.NewString(), + ResourceType: "ns1", + PrincipalID: s.userID, + PrincipalType: schema.UserPrincipal, + }) + s.Require().NoError(err) + return created + } + kept := newPolicy() + + s.Run("should keep the row and mark it deleted", func() { + s.Assert().NoError(s.repository.Delete(s.ctx, kept.ID)) + + var markedDeleted bool + if err := s.client.QueryRowxContext(s.ctx, "SELECT deleted_at IS NOT NULL FROM policies WHERE id = $1", kept.ID).Scan(&markedDeleted); err != nil { + s.T().Fatal(err) + } + s.Assert().True(markedDeleted) + + _, err := s.repository.Get(s.ctx, kept.ID) + s.Assert().ErrorIs(err, policy.ErrNotExist) + }) + + s.Run("should return not found when the policy is already deleted", func() { + s.Assert().ErrorIs(s.repository.Delete(s.ctx, kept.ID), policy.ErrNotExist) + }) } func TestPolicyRepository(t *testing.T) { @@ -636,3 +666,42 @@ func (s *PolicyRepositoryTestSuite) TestDeleteWithMinRoleGuardCountsLiveHoldersO _, err = s.repository.Get(s.ctx, second.ID) s.Assert().NoError(err) } + +func (s *PolicyRepositoryTestSuite) TestDeleteWithMinRoleGuardKeepsTheRow() { + guardRole := s.roles[0].ID + resourceID := uuid.NewString() + newHolder := func() policy.Policy { + created, err := s.repository.Upsert(s.ctx, policy.Policy{ + RoleID: guardRole, + ResourceID: resourceID, + ResourceType: schema.OrganizationNamespace, + PrincipalID: uuid.NewString(), + PrincipalType: schema.UserPrincipal, + }) + s.Require().NoError(err) + return created + } + first, second := newHolder(), newHolder() + + s.Run("should keep the row and mark it deleted", func() { + s.Assert().NoError(s.repository.DeleteWithMinRoleGuard(s.ctx, first.ID, guardRole)) + + var markedDeleted bool + if err := s.client.QueryRowxContext(s.ctx, "SELECT deleted_at IS NOT NULL FROM policies WHERE id = $1", first.ID).Scan(&markedDeleted); err != nil { + s.T().Fatal(err) + } + s.Assert().True(markedDeleted) + + _, err := s.repository.Get(s.ctx, first.ID) + s.Assert().ErrorIs(err, policy.ErrNotExist) + + _, err = s.repository.Get(s.ctx, second.ID) + s.Assert().NoError(err) + }) + + s.Run("should return not found instead of the guard error when the policy is already deleted", func() { + err := s.repository.DeleteWithMinRoleGuard(s.ctx, first.ID, guardRole) + s.Assert().ErrorIs(err, policy.ErrNotExist) + s.Assert().NotErrorIs(err, policy.ErrLastRoleGuard) + }) +} diff --git a/internal/store/postgres/relation_repository.go b/internal/store/postgres/relation_repository.go index c3f71311b..65a8f8204 100644 --- a/internal/store/postgres/relation_repository.go +++ b/internal/store/postgres/relation_repository.go @@ -131,7 +131,7 @@ func (r RelationRepository) DeleteByID(ctx context.Context, id string) error { if strings.TrimSpace(id) == "" { return relation.ErrInvalidID } - query, params, err := dialect.Delete(TABLE_RELATIONS).Where(goqu.Ex{ + query, params, err := softDelete(TABLE_RELATIONS).Where(goqu.Ex{ "id": id, }).ToSQL() if err != nil { diff --git a/internal/store/postgres/relation_repository_test.go b/internal/store/postgres/relation_repository_test.go index 37c17496d..6ddb4d533 100644 --- a/internal/store/postgres/relation_repository_test.go +++ b/internal/store/postgres/relation_repository_test.go @@ -347,6 +347,29 @@ func (s *RelationRepositoryTestSuite) TestDeleteByID() { } }) } + + kept := s.relations[1] + + s.Run("should keep the row and mark it deleted", func() { + s.Assert().NoError(s.repository.DeleteByID(s.ctx, kept.ID)) + + var markedDeleted bool + if err := s.client.QueryRowxContext(s.ctx, "SELECT deleted_at IS NOT NULL FROM relations WHERE id = $1", kept.ID).Scan(&markedDeleted); err != nil { + s.T().Fatal(err) + } + s.Assert().True(markedDeleted) + + _, err := s.repository.Get(s.ctx, kept.ID) + s.Assert().ErrorIs(err, relation.ErrNotExist) + + byFields, err := s.repository.GetByFields(s.ctx, kept) + s.Assert().NoError(err) + s.Assert().Empty(byFields) + }) + + s.Run("should return not found when the relation is already deleted", func() { + s.Assert().ErrorIs(s.repository.DeleteByID(s.ctx, kept.ID), relation.ErrNotExist) + }) } func TestRelationRepository(t *testing.T) { From c6e69f3cb9500a07f9e3529d8104c6ced2de889c Mon Sep 17 00:00:00 2001 From: aman Date: Mon, 5 Oct 2026 17:25:41 +0530 Subject: [PATCH 3/5] feat(store): org PATs listing joins live policies only Policy rows now stay after a delete, so the org PATs listing filters the policies join on deleted_at. A PAT whose policy was removed lists with no scope instead of its old role. --- internal/store/postgres/org_pats_repository.go | 2 ++ internal/store/postgres/org_pats_repository_test.go | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/internal/store/postgres/org_pats_repository.go b/internal/store/postgres/org_pats_repository.go index 6a813532e..2323f383a 100644 --- a/internal/store/postgres/org_pats_repository.go +++ b/internal/store/postgres/org_pats_repository.go @@ -217,6 +217,8 @@ func (r OrgPATsRepository) buildDataQuery(orgID string, rqlQuery *rql.Query) (st goqu.On( goqu.I("pol.principal_id").Eq(goqu.I("p.id")), goqu.I("pol.principal_type").Eq(schema.PATPrincipal), + // kept in the join so a PAT with no live policy still lists, with empty policy columns + live("pol"), ), ) diff --git a/internal/store/postgres/org_pats_repository_test.go b/internal/store/postgres/org_pats_repository_test.go index 7262f9aed..aff8a7e09 100644 --- a/internal/store/postgres/org_pats_repository_test.go +++ b/internal/store/postgres/org_pats_repository_test.go @@ -1,6 +1,7 @@ package postgres import ( + "strings" "testing" "time" @@ -135,6 +136,12 @@ func TestOrgPATsRepository_buildDataQuery(t *testing.T) { assert.Contains(t, sql, "created_by_title") assert.Contains(t, sql, "pol") assert.NotEmpty(t, params) + + // the live filter sits in the policies join, not in a WHERE clause + _, policiesJoin, found := strings.Cut(sql, `LEFT JOIN "policies" AS "pol" ON`) + assert.True(t, found) + assert.Contains(t, policiesJoin, `"pol"."deleted_at" IS NULL`) + assert.NotContains(t, policiesJoin, "WHERE") }) } } From de2a3ecdd699392a4110729b241188c233fadc3d Mon Sep 17 00:00:00 2001 From: aman Date: Mon, 5 Oct 2026 17:25:41 +0530 Subject: [PATCH 4/5] test(e2e): a deleted policy can be granted again --- test/e2e/regression/api_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index 4a0f77aa7..0056543bc 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -1727,6 +1727,31 @@ func (s *APIRegressionTestSuite) TestRelationAPI() { })) s.Assert().NoError(err) s.Assert().Equal(false, checkAfterDeletePermission.Msg.GetStatus()) + + // the deleted policy cannot be deleted twice + _, err = s.testBench.Client.DeletePolicy(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.DeletePolicyRequest{ + Id: createPolicyResp.Msg.GetPolicy().GetId(), + })) + s.Require().Error(err) + s.Assert().Equal(connect.CodeNotFound, connect.CodeOf(err)) + + // the same grant can be made again and lands on a new policy + recreatePolicyResp, err := s.testBench.Client.CreatePolicy(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePolicyRequest{ + Body: &frontierv1beta1.PolicyRequestBody{ + RoleId: schema.RoleOrganizationOwner, + Resource: schema.JoinNamespaceAndResourceID(schema.OrganizationNamespace, existingOrg.Msg.GetOrganization().GetId()), + Principal: schema.JoinNamespaceAndResourceID(schema.UserPrincipal, createUserResp.Msg.GetUser().GetId()), + }, + })) + s.Require().NoError(err) + s.Assert().NotEqual(createPolicyResp.Msg.GetPolicy().GetId(), recreatePolicyResp.Msg.GetPolicy().GetId()) + + checkAfterRecreatePermission, err := s.testBench.Client.CheckResourcePermission(ctxOrgUserAuth, connect.NewRequest(&frontierv1beta1.CheckResourcePermissionRequest{ + Resource: schema.JoinNamespaceAndResourceID(schema.OrganizationNamespace, existingOrg.Msg.GetOrganization().GetId()), + Permission: schema.DeletePermission, + })) + s.Assert().NoError(err) + s.Assert().Equal(true, checkAfterRecreatePermission.Msg.GetStatus()) }) } From 227a9421b14a63fc77d1a48d0a04fb5f0e74b2e2 Mon Sep 17 00:00:00 2001 From: Aman Prasad Date: Tue, 6 Oct 2026 12:35:08 +0530 Subject: [PATCH 5/5] Remove unnecessary comment in org_pats_repository.go Removed comment about PAT with no live policy listing. --- internal/store/postgres/org_pats_repository.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/store/postgres/org_pats_repository.go b/internal/store/postgres/org_pats_repository.go index 2323f383a..f6d8918b3 100644 --- a/internal/store/postgres/org_pats_repository.go +++ b/internal/store/postgres/org_pats_repository.go @@ -217,7 +217,6 @@ func (r OrgPATsRepository) buildDataQuery(orgID string, rqlQuery *rql.Query) (st goqu.On( goqu.I("pol.principal_id").Eq(goqu.I("p.id")), goqu.I("pol.principal_type").Eq(schema.PATPrincipal), - // kept in the join so a PAT with no live policy still lists, with empty policy columns live("pol"), ), )