Skip to content

feat(consent): add the user_consents table with immutability triggers - #1909

Closed
rohanchkrabrty wants to merge 1 commit into
feature/cld-3569-choreproto-pull-flowintent-and-the-consent-document-rpcfrom
feature/cld-3570-featconsent-add-the-user_consents-table-with-immutability
Closed

feat(consent): add the user_consents table with immutability triggers#1909
rohanchkrabrty wants to merge 1 commit into
feature/cld-3569-choreproto-pull-flowintent-and-the-consent-document-rpcfrom
feature/cld-3570-featconsent-add-the-user_consents-table-with-immutability

Conversation

@rohanchkrabrty

Copy link
Copy Markdown
Contributor

Part of RFC 0002: Explicit consent at signup. Based on #1908.

Summary

Adds the user_consents table from the RFC's Storage section. Schema only — there is no repository and no write path here. The repository lands with its caller in the transactional-write PR, so it arrives with something that exercises it rather than as dead code. audit_records shipped the same way: the table migration in #1118, the repository in #1124.

A consent record says which documents a user accepted, in what version, when, and from where. It has to be readable years later, it has to survive the user row, and nothing may edit it after the fact — those three requirements are what the rest of this description is about.

Changes

  • 20260830100000_create_user_consents.up.sql — the table, the documents_not_empty CHECK, the partial unique index uq_user_consents_signup, and BEFORE UPDATE / BEFORE DELETE triggers raising 45000.
  • 20260830100000_create_user_consents.down.sql — drops the triggers, the functions, the index and the table.

documents is a JSONB array holding one object per accepted document, copied from config at write time with the same four fields config holds (id, title, version, url). The CHECK enforces that it is an array and non-empty, so a consent that covers nothing cannot be stored.

consented_at is when the user accepted, taken from the flow, not when the row was written. created_at is the write time. They differ by an OIDC round trip, and conflating them would put the post-redirect moment on the record.

Technical Details

Four choices that read as mistakes without the RFC. Each is also commented in the migration, since the schema outlives this PR description.

No foreign key to users. UserRepository.Delete does a hard DELETE, so ON DELETE CASCADE would drop consent records along with the account and ON DELETE RESTRICT would block account deletion outright. Neither is acceptable: the records have to outlive the user. That is also why user_email is denormalized onto the row — after the user is gone, the email is the only thing left identifying whose consent it was.

ip_address is TEXT and nullable, not INET. The value comes from a request header. A proxy that sends a malformed value, or a deployment that sends none, must not fail a signup over it — INET would reject the row and take the account creation down with it. The RFC's Limitations section is explicit that the IP is only as good as the header it comes from.

Versions and URLs are copies, not references. A record stays readable after the document leaves config, and stays correct after the version bumps. It also means a consent_documents table can be added later with no backfill, since every record already carries its own snapshot. The tradeoff is that a record ties to a version string rather than to the document text — a per-document hash would close that and can be added later.

The partial unique index gives a user at most one signup consent. Nothing in the design repairs or rewrites a record, so a second signup write is a bug rather than a legitimate update. The index makes that bug fail loudly instead of leaving two rows that disagree about what the user accepted. It is partial on source = 'signup' so a later re-consent, written with a different source, is not blocked by it.

Also deliberate: there is no metadata column. Nothing would write it today, and a re-consent that needs one can add it in its own migration — an unused JSONB column invites unstructured writes that no reader expects.

Immutability, and why DELETE is guarded too. The triggers follow 20250904105226_add_audit_records_immutability.up.sql: a plpgsql function per operation that does nothing but RAISE EXCEPTION with ERRCODE = '45000', a BEFORE ... FOR EACH ROW trigger, and a COMMENT ON TRIGGER recording intent. That precedent guards UPDATE only. DELETE is guarded here as well, because the failure modes are not symmetric: a modified record is visibly wrong, whereas a deleted one leaves a user who simply looks like they never consented, which is indistinguishable from a user who never did. The triggers fire per row and so do not block DROP TABLE, which is what keeps the down migration working.

One migration pair, not two. The audit_records precedent is two pairs only because the immutability trigger was an afterthought that arrived days later with the repository. Here the table and its triggers are one design landing at one time, and splitting them would create an intermediate version in which user_consents is mutable — a state no deployment should ever be in. 20260218100000_create_user_pats.up.sql is the closer precedent: table, indexes, function and trigger in a single migration.

uuid_generate_v7() is the existing function from 20250901054744_create_audits_table.up.sql, which runs first. The down migration deliberately leaves it in place rather than dropping something it does not own.

Test Plan

Verified against Postgres 16 with the repo's own migrate path (migrations.MigrationFs through golang-migrate), the same code frontier server migrate runs.

  • migrate up from an empty database — clean, lands on 20260830100000, not dirty.
  • UPDATE on a stored row → ERROR: 45000: user_consents cannot be updated to maintain consent integrity.
  • DELETE on a stored row → ERROR: 45000: user_consents cannot be deleted to maintain consent integrity.
  • Second source = 'signup' row for the same user_id23505 on uq_user_consents_signup. A row for the same user with a different source inserts, confirming the index is partial.
  • documents as '[]' and as a JSON object → both rejected by documents_not_empty (23514).
  • Null ip_address and null auth_strategy insert fine; the RFC's documents @> ... containment query returns the expected row.
  • migrate down one step with rows present — table, triggers and functions gone, uuid_generate_v7() untouched. Confirms the BEFORE DELETE trigger does not block DROP TABLE.
  • Full up / down / up cycle, and a full Down() to version 0 followed by a fresh up.
  • make lint — 0 issues.
  • make test — passes, including internal/store/postgres, which boots a Dockerized Postgres and applies every migration.

SQL Safety

Not applicable — this PR adds two .sql migration files and touches no *_repository.go and no goqu.*. There is no query construction here at all; the repository that will query this table lands in a later PR, where the checklist applies.

Adds user_consents per the RFC's Storage section: the table, the
documents_not_empty CHECK, the partial unique index giving a user at
most one signup consent, and BEFORE UPDATE / BEFORE DELETE triggers
that both raise 45000.

Schema only. The repository and the write path land with their caller,
following audit_records, whose table migration shipped in #1118 and
whose repository followed in #1124.

Four choices are deliberate and read as mistakes without the RFC: no
foreign key to users, ip_address as nullable TEXT rather than INET,
document versions and URLs copied rather than referenced, and the
partial unique index. Each is commented in the migration itself.

Part of RFC 0002:
https://github.com/raystack/frontier/blob/main/docs/rfcs/0002-explicit-consent-at-signup.md

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VW3nysiE4H83VQk6BroMYc
@vercel

vercel Bot commented Aug 30, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
frontier Ready Ready Preview Aug 30, 2026 1:49pm

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5c1bd424-9185-4d20-8404-add4202b0c87

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@rohanchkrabrty
rohanchkrabrty deleted the feature/cld-3570-featconsent-add-the-user_consents-table-with-immutability branch August 30, 2026 13:49
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 33315196254

Coverage remained the same at 49.161%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 40516
Covered Lines: 19918
Line Coverage: 49.16%
Coverage Strength: 15.74 hits per line

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants