feat(consent): record consent in the same transaction as the user by rohanchkrabrty · Pull Request #1914 · raystack/frontier

rohanchkrabrty · 2026-08-31T10:13:28Z

Summary

Closes the invariant the whole feature rests on: a user row without a consent record is impossible. ResolveAll runs before the transaction opens, so an incomplete payload never starts one; inside it, the user insert and the consent insert both land or neither does. Per RFC 0002, Enforcement and Storage.
user.Repository gains CreateWithTx, which is the one place this feature reaches outside its own domain. pkg/db has WithTxn but carries no transaction on the context, so it is threaded through explicitly. Both create paths share one query builder, so they cannot drift.
An existing user is written nothing, whatever the flow carries. A record written outside a user creation would stamp this moment's timestamp and IP on an agreement made elsewhere, which is worse than no record because it reads like evidence. The IP and time come from when the user accepted, carried on the flow across the redirect.
A user.consent_granted audit record is written after the commit, through the repository rather than the service, with Actor filled in explicitly — these endpoints are skip-listed, so an empty actor would be enriched to the nil UUID and the system actor for an act a person performed. It cannot be atomic with the consent row, which is why that row is the source of truth and a failed audit write is logged and carried past.
The rollback is proved against a real database, not a mocked transaction — a mock can only pretend to roll back.

vercel · 2026-08-31T10:13:34Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
frontier	Ready	Preview	Sep 7, 2026 7:15am UTC

coderabbitai · 2026-08-31T10:13:38Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: a1a8ef97-e7b2-46a7-837d-e39d2420c19e

📥 Commits

Reviewing files that changed from the base of the PR and between 57fc138 and 0447c33.

📒 Files selected for processing (13)

core/authenticate/service.go
core/authenticate/service_test.go
core/consent/consent.go
core/consent/errors.go
core/consent/service.go
core/consent/service_test.go
core/user/service.go
core/user/service_test.go
core/user/user.go
internal/store/postgres/user_consent.go
internal/store/postgres/user_consent_repository.go
internal/store/postgres/user_consent_repository_test.go
internal/store/postgres/user_repository.go

💤 Files with no reviewable changes (2)

internal/store/postgres/user_consent.go
core/user/service.go

🚧 Files skipped from review as they are similar to previous changes (11)

core/user/user.go
core/user/service_test.go
internal/store/postgres/user_repository.go
internal/store/postgres/user_consent_repository.go
internal/store/postgres/user_consent_repository_test.go
core/consent/errors.go
core/consent/consent.go
core/consent/service_test.go
core/authenticate/service.go
core/authenticate/service_test.go
core/consent/service.go

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

📝 Summary

Summary by CodeRabbit

New Features
- Added explicit consent collection during signup for configured documents.
- Consent records preserve accepted document snapshots and signup details.
- User creation and consent recording occur atomically.
- Added audit events for granted consent.
Bug Fixes
- Existing-user flows no longer create unnecessary consent records.
- Signups requiring consent now fail clearly when consent is missing or invalid.
- Duplicate consent submissions are rejected consistently.
Documentation
- Updated consent documentation to describe document snapshots and audit metadata.

Walkthrough

The change adds consent records to signup. It defines consent contracts and audit events, persists users and consent atomically, integrates consent into authentication flows, and adds unit and PostgreSQL integration coverage.

Changes

Signup consent flow

Layer / File(s)	Summary
Consent contracts and audit recording `core/consent/*`, `pkg/auditrecord/consts.go`, `docs/rfcs/...`	Adds consent and grant data types, validation errors, grant preparation, audit recording, document snapshots, and related tests.
Atomic user and consent persistence `core/user/`, `internal/store/postgres/`	Adds consent-aware user creation, PostgreSQL consent storage, JSON document mapping, duplicate handling, transaction rollback, and integration tests.
Authentication consent orchestration `core/authenticate/*`, `cmd/serve.go`	Resolves required documents during signup, creates users with consent in one transaction, records consent after commit, wires services, and updates tests and mocks.

Estimated code review effort: 4 (Complex) | ~60 minutes

Merge Risk: 🔵 Low · up to 0447c

Signup now creates users and consent atomically, but a duplicate-signup conflict can leave a database connection checked out. This is a bounded availability risk that should be addressed before sustained conflict traffic.

Suggested reviewers: amangit07

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coveralls · 2026-08-31T10:20:16Z

Coverage Report for CI Build 34094313603

Coverage increased (+0.3%) to 50.011%

Details

Coverage increased (+0.3%) from the base build.
Patch coverage: 34 uncovered changes across 5 files (219 of 253 lines covered, 86.56%).
No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
internal/store/postgres/user_repository.go	51	32	62.75%
internal/store/postgres/user_consent_repository.go	40	34	85.0%
cmd/serve.go	5	0	0.0%
core/authenticate/service.go	45	43	95.56%
internal/store/postgres/user_consent.go	37	35	94.59%
Total (7 files)	253	219	86.56%

Coverage Regressions

No coverage regressions found.

Coverage Stats


Relevant Lines:	41005
Covered Lines:	20507
Line Coverage:	50.01%
Coverage Strength:	15.94 hits per line

💛 - Coveralls

The invariant this establishes: a user row without a consent record is impossible. ResolveAll runs before the transaction opens, so an incomplete payload never starts one; inside it, the user insert and the consent insert both land or neither does. user.Repository and the new user_consents repository each gain a Create that takes a *sqlx.Tx. pkg/db has WithTxn but carries no transaction on the context, so the transaction is threaded through explicitly rather than found on one. Both are additive, and the user repository change is the one place this feature reaches outside its own domain. The consent repository has Create and nothing else, because the table is immutable. consent.Grant writes one record for the documents it is given and has no completeness rule of its own. ResolveAll is what decides a signup covers every configured document, and keeping that out of Grant leaves room for a later re-consent covering a subset without a second write path. getOrCreateUser now has three outcomes for a new user. A complete payload writes both rows in one transaction. An incomplete one returns ErrConsentRequired and writes nothing. An existing user gets no record at all, which is absolute: a record written outside a user creation would carry that moment's timestamp and IP for an agreement made elsewhere, which is worse than no record because it reads like evidence. A nil flow means one of the paths that create a user without one, and those stay exempt because no account holder is present to consent. The completeness check runs at user creation under every intent, not for the error but as the invariant guarding the write. An unset intent is permissive for the login gate but never for consent. With app.consent disabled ResolveAll resolves nothing, and an empty document set means write no record, so nothing changes for a deployment that does not ask for consent. Each signup also writes one audit record, with UserConsentGrantedEvent and ConsentType added to pkg/auditrecord following the entity.verb naming already there. It goes through the repository with the actor filled in, as userpat does for its PAT events: the repository enriches an empty actor from the context, and these endpoints are on the authentication skip list with no actor in it, so the record would otherwise land as the system actor for an act a person performed. It is written after the commit, since the audit repository has no transactional create, so it cannot be atomic with the record it describes. That is why the consent record is the source of truth and this one is a breadcrumb: a failure is logged and the signup stands. Its target metadata carries the whole document snapshot rather than the id and the version alone, so a reader working from the audit trail can say what was accepted without reading back a record they may not have access to. The rollback is tested against a real Postgres rather than a mocked transaction, since a mock can only pretend to roll back. See docs/rfcs/0002-explicit-consent-at-signup.md, Enforcement and Storage.

AmanGIT07 · 2026-09-03T17:55:26Z

+type Repository interface {
+	Create(ctx context.Context, tx *sqlx.Tx, cnst Consent) (Consent, error)
+}


We will need to establish a pattern across the codebase on how to move transactions across services — this doesn't exist as of now, and will be done later. The approach may vary as well. So instead of doing it as a one-off case and making db trnx as dependency in service, can we instead look at alternatives, e.g. a postgres-layer method (say CreateWithConsent with the interface declared on authenticate's side since it already imports both domains) which opens WithTxn internally and inserts all the rows required?

@rohilsurana thoughts?

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/store/postgres/user_repository.go (1)
225-226: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Roll back Create on duplicate-key errors

Create starts a transaction with BeginTxx, but the ErrDuplicateKey branch returns without commit or rollback. WithTimeout only cancels the query context. The transaction keeps its connection until rollback, commit, or cancellation of the context passed to BeginTxx; repeated conflicts with an active context can reduce pool capacity or exhaust it. The new WithTxn path does not surround Create, and this PR leaves Create unchanged.
♻️ Proposed fix
 		err = checkPostgresError(err)
 		switch {
 		case errors.Is(err, ErrDuplicateKey):
+			if rbErr := tx.Rollback(); rbErr != nil {
+				return user.User{}, rbErr
+			}
 			return user.User{}, user.ErrConflict
 		default:

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 7b8c77c9-8812-4e98-89b7-4898d2d17d9f

📥 Commits

Reviewing files that changed from the base of the PR and between 2ef789b and aebd4f8.

📒 Files selected for processing (20)

cmd/serve.go
core/authenticate/mocks/consent_service.go
core/authenticate/mocks/user_service.go
core/authenticate/service.go
core/authenticate/service_test.go
core/consent/consent.go
core/consent/errors.go
core/consent/service.go
core/consent/service_test.go
core/user/mocks/repository.go
core/user/service.go
core/user/service_test.go
core/user/user.go
docs/rfcs/0002-explicit-consent-at-signup.md
internal/store/postgres/postgres.go
internal/store/postgres/user_consent.go
internal/store/postgres/user_consent_repository.go
internal/store/postgres/user_consent_repository_test.go
internal/store/postgres/user_repository.go
pkg/auditrecord/consts.go

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

The consent record still lands with the user row or not at all, but no service signature names a database transaction any more. core/user, core/consent and core/authenticate no longer import sqlx. user.Repository gains CreateWithConsent in place of CreateWithTx, and the Postgres implementation opens the transaction itself, writing the user row and then the consent record through the repository that already owns that table. Both user create paths still share buildUserInsertQuery, so they cannot drift. consent.Service keeps its config, its documents, its resolve rules and its audit record. What changed is that Grant, which wrote, becomes PrepareGrant, which builds and validates the record and performs no I/O. Its Repository interface had no other caller and is gone, and with it the last *sqlx.Tx in core. The identity is no longer required by validateGrant: the user id is generated by the insert, and the writer stamps it from the row it just wrote, with NOT NULL and a foreign key behind it. This is deliberately scoped to signup rather than general: the codebase has no pattern for a transaction spanning two domains, the read-only aggregates are not one, and organization.AdminCreate shows the current answer is to sequence the writes and accept partial failure. A general pattern needs its own research and RFC. Until then the composite write lives with the user domain, which is where user creation has always lived, and the comments say so at each site. The rollback is still proved against a real database, now through CreateWithConsent rather than a hand-assembled transaction. See docs/rfcs/0002-explicit-consent-at-signup.md, Enforcement and Storage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The comments added with the consent write explained more than a reader needs. Keep the reasons that are not in the code, and drop the ones repeated across layers: that the consent identity is stamped from the inserted row was stated in five places, the audit actor rationale in three. Two were wrong rather than only verbose: - three claimed a foreign key on user_consents.user_id, which the migration rules out on purpose: "no FK to users(id): Delete is a hard DELETE, so CASCADE would drop these records" - the fakeRepository doc comment was left standing over the wrong type once the fake it described was removed No behaviour change. gofmt realigns the struct literals whose interstitial comments went away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

rohanchkrabrty self-assigned this Aug 31, 2026

rohanchkrabrty requested a review from rohilsurana August 31, 2026 10:13

rohanchkrabrty mentioned this pull request Aug 31, 2026

feat(authenticate): map the gate and consent errors on both auth RPCs #1915

Open

vercel Bot deployed to Preview August 31, 2026 10:14 View deployment

rohanchkrabrty marked this pull request as draft August 31, 2026 10:36

rohanchkrabrty force-pushed the feature/featconsent-record-consent-in-the-same-transaction-as-the branch from 481c8eb to c396eaa Compare August 31, 2026 19:10

vercel Bot deployed to Preview August 31, 2026 19:11 View deployment

rohanchkrabrty force-pushed the feature/featconsent-record-consent-in-the-same-transaction-as-the branch from c396eaa to 6989f83 Compare September 2, 2026 16:29

vercel Bot deployed to Preview September 2, 2026 16:32 View deployment

rohanchkrabrty force-pushed the feature/featconsent-record-consent-in-the-same-transaction-as-the branch from 6989f83 to fa677e9 Compare September 2, 2026 19:00

vercel Bot deployed to Preview September 2, 2026 19:02 View deployment

rohanchkrabrty marked this pull request as ready for review September 2, 2026 20:06

rohanchkrabrty requested a review from AmanGIT07 September 3, 2026 17:30

AmanGIT07 reviewed Sep 3, 2026

View reviewed changes

vercel Bot deployed to Preview September 6, 2026 21:42 View deployment

coderabbitai Bot reviewed Sep 6, 2026

View reviewed changes

rohanchkrabrty force-pushed the feature/featconsent-record-consent-in-the-same-transaction-as-the branch from aebd4f8 to 57fc138 Compare September 7, 2026 05:29

vercel Bot deployed to Preview September 7, 2026 05:30 View deployment

rohanchkrabrty force-pushed the feature/featconsent-record-consent-in-the-same-transaction-as-the branch from 57fc138 to 55a7236 Compare September 7, 2026 06:09

vercel Bot deployed to Preview September 7, 2026 06:10 View deployment

rohanchkrabrty force-pushed the feature/featconsent-record-consent-in-the-same-transaction-as-the branch from 55a7236 to 0447c33 Compare September 7, 2026 07:12

vercel Bot deployed to Preview September 7, 2026 07:15 View deployment

Navigation Menu

Uh oh!

feat(consent): record consent in the same transaction as the user - #1914

feat(consent): record consent in the same transaction as the user#1914
rohanchkrabrty wants to merge 3 commits into
feature/featconsent-add-appconsent-config-the-consent-service-andfrom
feature/featconsent-record-consent-in-the-same-transaction-as-the

rohanchkrabrty commented Aug 31, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Aug 31, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 31, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

coveralls commented Aug 31, 2026 •

edited

Loading

Uh oh!

Uh oh!

AmanGIT07 Sep 3, 2026

Uh oh!

AmanGIT07 Sep 3, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

rohanchkrabrty commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

vercel Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

coveralls commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 34094313603

Coverage increased (+0.3%) to 50.011%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

Uh oh!

AmanGIT07 Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

AmanGIT07 Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rohanchkrabrty commented Aug 31, 2026 •

edited

Loading

vercel Bot commented Aug 31, 2026 •

edited

Loading

coderabbitai Bot commented Aug 31, 2026 •

edited

Loading

coveralls commented Aug 31, 2026 •

edited

Loading