feat(consent): add the user_consents table with immutability triggers - #1909
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Coverage Report for CI Build 33315196254Coverage remained the same at 49.161%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Part of RFC 0002: Explicit consent at signup. Based on #1908.
Summary
Adds the
user_consentstable 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_recordsshipped 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, thedocuments_not_emptyCHECK, the partial unique indexuq_user_consents_signup, andBEFORE UPDATE/BEFORE DELETEtriggers raising45000.20260830100000_create_user_consents.down.sql— drops the triggers, the functions, the index and the table.documentsis 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_atis when the user accepted, taken from the flow, not when the row was written.created_atis 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.Deletedoes a hardDELETE, soON DELETE CASCADEwould drop consent records along with the account andON DELETE RESTRICTwould block account deletion outright. Neither is acceptable: the records have to outlive the user. That is also whyuser_emailis denormalized onto the row — after the user is gone, the email is the only thing left identifying whose consent it was.ip_addressisTEXTand nullable, notINET. 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 —INETwould 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_documentstable 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 differentsource, is not blocked by it.Also deliberate: there is no
metadatacolumn. 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
DELETEis guarded too. The triggers follow20250904105226_add_audit_records_immutability.up.sql: aplpgsqlfunction per operation that does nothing butRAISE EXCEPTIONwithERRCODE = '45000', aBEFORE ... FOR EACH ROWtrigger, and aCOMMENT ON TRIGGERrecording intent. That precedent guardsUPDATEonly.DELETEis 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 blockDROP TABLE, which is what keeps the down migration working.One migration pair, not two. The
audit_recordsprecedent 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 whichuser_consentsis mutable — a state no deployment should ever be in.20260218100000_create_user_pats.up.sqlis the closer precedent: table, indexes, function and trigger in a single migration.uuid_generate_v7()is the existing function from20250901054744_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.MigrationFsthroughgolang-migrate), the same codefrontier server migrateruns.migrate upfrom an empty database — clean, lands on20260830100000, not dirty.UPDATEon a stored row →ERROR: 45000: user_consents cannot be updated to maintain consent integrity.DELETEon a stored row →ERROR: 45000: user_consents cannot be deleted to maintain consent integrity.source = 'signup'row for the sameuser_id→23505onuq_user_consents_signup. A row for the same user with a differentsourceinserts, confirming the index is partial.documentsas'[]'and as a JSON object → both rejected bydocuments_not_empty(23514).ip_addressand nullauth_strategyinsert fine; the RFC'sdocuments @> ...containment query returns the expected row.migrate downone step with rows present — table, triggers and functions gone,uuid_generate_v7()untouched. Confirms theBEFORE DELETEtrigger does not blockDROP TABLE.Down()to version 0 followed by a freshup.make lint— 0 issues.make test— passes, includinginternal/store/postgres, which boots a Dockerized Postgres and applies every migration.SQL Safety
Not applicable — this PR adds two
.sqlmigration files and touches no*_repository.goand nogoqu.*. There is no query construction here at all; the repository that will query this table lands in a later PR, where the checklist applies.