From 6dc9129ae5a0655e65188d521dfa8d1b2c4b0b71 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 19 Aug 2026 12:45:31 +0530 Subject: [PATCH 1/7] fix(sdk): scope PAT detail to active org and show a unified not-found page --- web/sdk/client/views/pat/pat-details-view.tsx | 61 +++++++++++++++---- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/web/sdk/client/views/pat/pat-details-view.tsx b/web/sdk/client/views/pat/pat-details-view.tsx index 17be5629a6..6d0b82f9ca 100644 --- a/web/sdk/client/views/pat/pat-details-view.tsx +++ b/web/sdk/client/views/pat/pat-details-view.tsx @@ -1,8 +1,9 @@ 'use client'; -import { ReactNode, useCallback, useEffect, useMemo } from 'react'; +import { ReactNode, useCallback, useMemo } from 'react'; import { DotsHorizontalIcon, + LockClosedIcon, Pencil1Icon, UpdateIcon } from '@radix-ui/react-icons'; @@ -11,13 +12,13 @@ import { Breadcrumb, Button, Dialog, + EmptyState, Flex, IconButton, Image, Menu, Skeleton, Text, - toastManager, Tooltip } from '@raystack/apsara'; import deleteIcon from '../../assets/delete.svg'; @@ -108,20 +109,13 @@ export function PATDetailsView({ create(GetCurrentUserPATRequestSchema, { id: patId }), { enabled: Boolean(patId), + // A bad or unknown id is a definite answer, not a transient failure, so + // don't retry: surface the "not found" state immediately. + retry: false, select: d => d?.pat } ); - useEffect(() => { - if (patError) { - toastManager.add({ - title: 'Something went wrong', - description: patError.message, - type: 'error' - }); - } - }, [patError]); - const { data: orgRolesData, isLoading: isOrgRolesLoading } = useQuery( FrontierServiceQueries.listRolesForPAT, create(ListRolesForPATRequestSchema, { scopes: [PERMISSIONS.OrganizationNamespace] }), @@ -238,6 +232,49 @@ export function PATDetailsView({ const patTitle = pat?.title || ''; + // getCurrentUserPAT only scopes by the logged-in user, not the org, so a token + // from another org the user belongs to can still load here. Treat that, a + // missing token, and an invalid id the same way: one generic "not found" that + // reveals nothing and blocks opening, editing, regenerating or revoking. + const isForeignPat = Boolean(orgId) && pat != null && pat.orgId !== orgId; + const showTokenNotFound = Boolean(patError) || isForeignPat; + + if (showTokenNotFound) { + return ( + + + onNavigateToPats?.()} + data-test-id="frontier-sdk-pat-not-found-breadcrumb" + > + Personal access token + + + } + /> + } + heading="Token not found" + subHeading="This personal access token doesn't exist." + primaryAction={ + + } + /> + + ); + } + return ( Date: Wed, 19 Aug 2026 13:55:20 +0530 Subject: [PATCH 2/7] fix(sdk): show a distinct error state for non-not-found PAT fetch failures --- web/sdk/client/views/pat/pat-details-view.tsx | 81 +++++++++++++------ 1 file changed, 57 insertions(+), 24 deletions(-) diff --git a/web/sdk/client/views/pat/pat-details-view.tsx b/web/sdk/client/views/pat/pat-details-view.tsx index 6d0b82f9ca..7f0b5c2c1f 100644 --- a/web/sdk/client/views/pat/pat-details-view.tsx +++ b/web/sdk/client/views/pat/pat-details-view.tsx @@ -3,6 +3,7 @@ import { ReactNode, useCallback, useMemo } from 'react'; import { DotsHorizontalIcon, + ExclamationTriangleIcon, LockClosedIcon, Pencil1Icon, UpdateIcon @@ -23,6 +24,7 @@ import { } from '@raystack/apsara'; import deleteIcon from '../../assets/delete.svg'; import { useQuery } from '@connectrpc/connect-query'; +import { Code, ConnectError } from '@connectrpc/connect'; import { create } from '@bufbuild/protobuf'; import { FrontierServiceQueries, @@ -109,9 +111,16 @@ export function PATDetailsView({ create(GetCurrentUserPATRequestSchema, { id: patId }), { enabled: Boolean(patId), - // A bad or unknown id is a definite answer, not a transient failure, so - // don't retry: surface the "not found" state immediately. - retry: false, + // A missing token or an invalid id is a definite answer, so don't retry + // those and surface the result immediately. Other failures may be + // transient, so allow a couple of retries before showing an error. + retry: (failureCount, error) => { + const code = error instanceof ConnectError ? error.code : undefined; + if (code === Code.NotFound || code === Code.InvalidArgument) { + return false; + } + return failureCount < 2; + }, select: d => d?.pat } ); @@ -233,13 +242,18 @@ export function PATDetailsView({ const patTitle = pat?.title || ''; // getCurrentUserPAT only scopes by the logged-in user, not the org, so a token - // from another org the user belongs to can still load here. Treat that, a - // missing token, and an invalid id the same way: one generic "not found" that - // reveals nothing and blocks opening, editing, regenerating or revoking. + // from another org the user belongs to can still load here. A missing token, + // an invalid id, and a foreign token are all shown as a generic "not found" + // that reveals nothing. Any other failure is a real error, not a not-found. + const patErrorCode = + patError instanceof ConnectError ? patError.code : undefined; + const isNotFoundError = + patErrorCode === Code.NotFound || patErrorCode === Code.InvalidArgument; const isForeignPat = Boolean(orgId) && pat != null && pat.orgId !== orgId; - const showTokenNotFound = Boolean(patError) || isForeignPat; + const showTokenNotFound = isForeignPat || isNotFoundError; + const hasUnexpectedError = patError != null && !isNotFoundError; - if (showTokenNotFound) { + if (showTokenNotFound || hasUnexpectedError) { return ( } /> - } - heading="Token not found" - subHeading="This personal access token doesn't exist." - primaryAction={ - - } - /> + {showTokenNotFound ? ( + } + heading="Token not found" + subHeading="This personal access token doesn't exist." + primaryAction={ + + } + /> + ) : ( + } + heading="Something went wrong" + subHeading="We couldn't load this personal access token. Please try again." + primaryAction={ + + } + /> + )} ); } From 098490b60f782a9d2653ef049a0ce0d709ce54d8 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Thu, 20 Aug 2026 10:08:17 +0530 Subject: [PATCH 3/7] chore(sdk): remove verbose comment in PAT detail not-found guard --- web/sdk/client/views/pat/pat-details-view.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/web/sdk/client/views/pat/pat-details-view.tsx b/web/sdk/client/views/pat/pat-details-view.tsx index 7f0b5c2c1f..f261452f4a 100644 --- a/web/sdk/client/views/pat/pat-details-view.tsx +++ b/web/sdk/client/views/pat/pat-details-view.tsx @@ -241,10 +241,6 @@ export function PATDetailsView({ const patTitle = pat?.title || ''; - // getCurrentUserPAT only scopes by the logged-in user, not the org, so a token - // from another org the user belongs to can still load here. A missing token, - // an invalid id, and a foreign token are all shown as a generic "not found" - // that reveals nothing. Any other failure is a real error, not a not-found. const patErrorCode = patError instanceof ConnectError ? patError.code : undefined; const isNotFoundError = From 070da82d5cbb5f7536a001e77ff4f3497e358fc9 Mon Sep 17 00:00:00 2001 From: Rohan Chakraborty Date: Thu, 27 Aug 2026 03:14:19 +0530 Subject: [PATCH 4/7] feat: rfc for explicit consent --- docs/rfcs/0002-explicit-consent-at-signup.md | 508 +++++++++++++++++++ 1 file changed, 508 insertions(+) create mode 100644 docs/rfcs/0002-explicit-consent-at-signup.md diff --git a/docs/rfcs/0002-explicit-consent-at-signup.md b/docs/rfcs/0002-explicit-consent-at-signup.md new file mode 100644 index 0000000000..674e666ed3 --- /dev/null +++ b/docs/rfcs/0002-explicit-consent-at-signup.md @@ -0,0 +1,508 @@ +# RFC 0002: Explicit consent at signup + +| | | +|---|---| +| **Status** | Draft. Not implemented. | +| **Author** | Rohan Chakraborty | +| **Created** | 2026-08-26 | +| **Updated** | 2026-08-27 | + +## Summary + +Frontier can make a user accept a set of documents before their account is created, and store one +consent record for it. + +A deployment lists its documents in server config with an id, title, version and URL. The client +sends the document ids the user accepted. Frontier checks the ids cover every document in config, +then creates the user and the consent record together. The record lists each accepted document +with the version and URL from config. + +If `app.consent` is not enabled, nothing changes. Frontier never parses document contents or +version strings. + +## Problem + +Frontier creates a user at the end of a registration flow and stores nothing about what the user +agreed to. Three gaps. + +Finishing signup is the only evidence of agreement. A record built from that only says the user +signed up, which we already know. It cannot tell apart a user who read the documents from one who +never saw them. + +There is nothing to show when someone asks which documents a user accepted, and when. +`users.metadata` does not work for this: `UpdateUser` and `UpdateCurrentUser` both replace the +whole map, so a user can delete their own consent by editing their profile, and the metadata goes +away with the user row. + +Documents have versions, but nothing records which version a user accepted, so you cannot tell +which one applies to them. + +## Goals + +- The client sends consent explicitly. The server rejects signup when it is missing. +- No API can update or delete a consent record. Deleting the user does not delete it either. +- A consent record stores the version and URL of every document it covers. +- Document ids, titles, versions and URLs come from config. Frontier does not read them. +- Deployments that do not set or enable `app.consent` behave exactly as before. + +## Non-goals + +Re-consent, withdrawal, and users who already exist. When a document version changes, old consent +records keep pointing at the old version and nobody is asked again. + +Repairing a missing consent record. A record is written when a user is created and at no other +time. There is no path that adds one to an account that already exists. + +Frontier does not host the document text and does not serve the document list. Clients keep their +own copy of the list. The SDK sign-up view gets a checkbox so it still works when consent is +enabled, but the copy, the links and any second checkbox are the consumer's. + +## Document config + +A map under `app.consent`, keyed by document id: + +```yaml +app: + consent: + enabled: true + documents: + terms_of_service: + title: Terms & Conditions + version: "2026-04-01" + url: https://example.org/legal/terms/2026-04-01 + privacy_policy: + title: Privacy Policy + version: "2026-04-01" + url: https://example.org/legal/privacy/2026-04-01 +``` + +`app.consent` sits next to `app.authentication` and `app.pat` on `server.Config`. A document has an +id, a title, a version and a URL, and nothing else. The example lists two; a deployment lists all of +its documents, and they have to be in config from the first release, for the reason below. + +A map, not a list, for three reasons. It matches `authenticate.Config`, which already keys +`oidc_config` by strategy name. The map key is the document id, so two documents cannot use the +same one. And you can override a single field with an env var, which you cannot do for one item in +a list of objects. + +There is no per-document `required` flag. Every document in the map is required at signup. A flag +would put two lists in config, all documents and the required subset, where one does the job, and +an optional document is a different feature: it needs withdrawal, which is out of scope. The cost +is that adding a new document id breaks signup for clients still sending the old list, so a new +document and the client release that sends it have to ship together. Version bumps are unaffected, +since the client only sends ids. + +`version` is an opaque string that frontier only compares for equality, so a deployment can use +dates, semver or commit SHAs. + +`enabled` is one switch for the whole feature. With `app.consent` absent or `enabled` false, +frontier behaves exactly as it does today: no check runs, no record is written, and +`accepted_document_ids` is ignored rather than rejected, so one client build works against both +kinds of deployment. With `enabled` true, every document in the map is required. `enabled` true +with no documents fails at boot instead of quietly turning the feature off. + +Config is read once at boot, so changing a document version needs a restart. Keys are +env-overridable like the rest of frontier's config. An override cannot change an existing consent +record, because records are immutable and each holds its own copy of the version, but it can +produce wrong new ones. So the resolved document set is logged at boot. That log, not the config +repo, tells you what a deployment was actually serving. + +Bad config fails at boot instead of at the first signup: document ids, versions and URLs must be +non-empty, URLs must parse, and an enabled block needs at least one document. + +## The request field + +One repeated field on the existing `AuthenticateRequest`: + +```proto +repeated string accepted_document_ids = 6; +``` + +Field 6 is free: `AuthenticateRequest` ends at `callback_url = 5` today. + +Document ids, not a single boolean, because the client has its own copy of the document list and +the two can go out of sync. With ids, frontier sees the mismatch and rejects the signup. With a +boolean it would stamp whatever config holds and write a consent record saying the user accepted +a document they were never shown, and nothing would catch it. Ids also leave room for consenting +to a subset later without changing the field. + +Ids and nothing else. The client never sends versions, titles or URLs, and frontier would not use +them if it did, since everything on a consent record is stamped from config. Duplicate ids are +de-duplicated before the check. + +## Carrying consent across the redirect + +In an OIDC flow the user accepts the documents before the browser leaves for the identity +provider, and the account is created after it comes back: + +```mermaid +flowchart TD + accept["User accepts the documents"] + auth["Authenticate: document ids, IP and time
written to flows.metadata"] + idp[("Identity provider")] + cb["AuthCallback: flow row read back"] + q{"Would this create
a new user?"} + reject["Reject with FailedPrecondition,
no user created"] + create["One transaction: create the user
and the consent record"] + login["Return the existing user,
write nothing"] + + accept --> auth --> idp + idp -->|"state = flow id, code"| cb + cb --> q + q -->|"yes, consent incomplete"| reject + q -->|"yes, consent complete"| create + q -->|"no"| login +``` + +The only thing that survives the redirect is `state`, which already holds the flow id and is +visible to the browser and the provider. So the consent goes where the flow id points. +`Flow.Metadata` is a JSONB column that already carries `callback_url`, and `StartFlow` writes one +more key: + +```go +flow.Metadata["consent"] = map[string]any{ + "accepted_document_ids": ids, + "ip_address": ip, + "at": s.Now(), +} +``` + +The flow row is written before the redirect and read after it comes back, so the consent never +goes through the browser and cannot be changed on the way. The IP and time stored are from when +the user accepted, not from the callback. Mail OTP and passkey use the same path, so there is one +code path for every strategy. + +`Authenticate` and `AuthCallback` are both in `authenticationSkipList`, so the authentication +interceptor does not run and nothing puts session metadata in the context. The `Authenticate` +handler has to call `sessionutils.ExtractSessionMetadata` itself and pass the IP into `StartFlow`. +That helper returns `session.SessionMetadata`, whose `IpAddress` is the leftmost value of the +configured client IP header. It parses the user agent into an OS and a browser family and drops the +raw string, which is why the record keeps only the IP. + +`Flow.Metadata` is `map[string]any` stored as JSONB, so it does not return the types it was given: +the ids come back as `[]any` and `at` as an RFC 3339 string. One typed parser handles the read, the +way `otpAttempts` already does for the attempt counter, rather than an unchecked assertion like +`flow.Metadata["callback_url"].(string)`. A missing or unparseable consent key counts as no consent. + +This also fixes something unrelated. `applyOIDC` never calls `consumeFlow`, so OIDC flow rows sit +around until the expiry cron while mail OTP rows are deleted on use. That is hard to justify once +those rows hold consent. + +## Enforcement + +The consent service owns the config, so it owns both checks. They are separate functions because +signup and any later use want different rules: + +```go +// Resolve maps ids to their config snapshots. Rejects unknown ids. +// Says nothing about whether the set is complete. +func (s Service) Resolve(ids []string) ([]Document, error) + +// ResolveAll is Resolve plus the completeness rule: the ids must cover +// every document in config, no more and no less. +func (s Service) ResolveAll(ids []string) ([]Document, error) + +// Grant writes one consent record for the documents given. +// No completeness rule here at all. +func (s Service) Grant(ctx context.Context, tx *sqlx.Tx, req GrantRequest) error +``` + +`ResolveAll` compares the two sets in both directions, so the error names what is wrong: which +required ids are missing, or which sent ids config does not know. `Grant` takes whatever it is +given, which is what leaves room for a later re-consent covering one document without a second +write path. + +Consent is stored at the start of the flow and required at user creation. It cannot be required at +the start: in an OIDC flow the email is not known yet, so a signup and a login look like the same +request. What the `Authenticate` handler can do is call `Resolve` on any ids it was sent, so an +unknown id fails before the browser leaves for the provider. `ResolveAll` runs at user creation, +which is the first point where frontier knows who the user is and that they are new. + +So `getOrCreateUser` takes the flow, which is nil for the one caller that has none, and: + +- New user, `ResolveAll` passes: one transaction creates the user row and the consent record. +- New user, `ResolveAll` fails: return `ErrConsentRequired` and create nothing. `AuthCallback` maps + it to `FailedPrecondition` so the client can tell it apart from a bad code or an expired flow and + ask again. +- Existing user: log them in and write nothing, whatever the flow holds. + +A rejection ends the flow. The user starts a new one with a complete set, and for mail OTP that +means a fresh code, since `applyMailOTP` calls `consumeFlow` before it creates the user. Reusing +the flow would only let the same client assert the same wrong set again. + +The third case is absolute. There is no branch that notices a missing consent record and fills it +in. A record written outside a user creation would carry that moment's timestamp and IP for an +agreement that happened somewhere else, which is worse than having no record: it reads like real +evidence. + +That is why the transaction matters. `ResolveAll` runs before the transaction opens, so an +incomplete payload never starts one. Inside it, the user insert and the consent insert either both +land or neither does. Without the transaction a failed consent insert would leave an account with +no consent record and nothing able to repair it, which is the gap this feature exists to close. +`pkg/db` has `WithTxn`, but no context-carried transaction, so both repositories need a `Create` +that accepts the `*sqlx.Tx`. That is additive and breaks no existing caller. If threading the +transaction through the user repository is rejected, the fallback is to delete the user row when +the consent insert fails and log loudly if that delete also fails. + +### The other paths that create users + +`getOrCreateUser` has five callers, and two paths create a user row without going through it at all: + +| Path | With consent enabled | +|---|---| +| `applyOIDC` | gated: the flow carries the consent | +| `applyMailOTP` | gated: the flow carries the consent | +| `finishPassKeyRegisterMethod` | gated: the flow carries the consent | +| `finishPassKeyLoginMethod` | gated: a first-time passkey login is a signup | +| `authenticateWithPassthroughHeader` | not gated: no flow exists | +| `organization.Service.AdminCreate` | not gated: operator action, no flow | +| `CreateUser` RPC | not gated: operator action, no flow | + +The four flow-based paths are the ones a person signs up through, and they are the ones this RFC +closes. The other three are exempt because no account holder is present to consent: +`authenticateWithPassthroughHeader` provisions from `app.identity_proxy_header`, which already +warns that it bypasses authorization, and `AdminCreate` and `CreateUser` are operator actions. + +Exempt means those accounts get no consent record, the same as users who already exist. A +deployment that wants full coverage keeps all three out of its signup path: `identity_proxy_header` +unset outside development, and the two operator RPCs used only for accounts nobody signs up for. +Limitations records the residual gap. + +## Storage + +One consent record per consent, listing the documents it covers. + +```sql +CREATE TABLE user_consents ( + id UUID PRIMARY KEY DEFAULT uuid_generate_v7(), + user_id UUID NOT NULL, + user_email TEXT NOT NULL, + documents JSONB NOT NULL, + source TEXT NOT NULL DEFAULT 'signup', + auth_method TEXT, + ip_address TEXT, + consented_at TIMESTAMPTZ NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + metadata JSONB NOT NULL DEFAULT '{}', + + CONSTRAINT documents_not_empty CHECK ( + jsonb_typeof(documents) = 'array' AND jsonb_array_length(documents) > 0 + ) +); + +CREATE UNIQUE INDEX uq_user_consents_signup + ON user_consents(user_id) WHERE source = 'signup'; + +CREATE INDEX idx_user_consents_documents + ON user_consents USING GIN (documents jsonb_path_ops); +``` + +`documents` holds one object per accepted document, copied from config at write time: + +```json +[ + { + "id": "terms_of_service", + "title": "Terms & Conditions", + "version": "2026-04-01", + "url": "https://example.org/legal/terms/2026-04-01" + }, + { + "id": "privacy_policy", + "title": "Privacy Policy", + "version": "2026-04-01", + "url": "https://example.org/legal/privacy/2026-04-01" + } +] +``` + +Four fields per document, the same four config holds. A field added later appears on new records +only, since old ones keep the shape they were written with. + +`metadata` is written once at insert like every other column and is empty today. It gives a later +re-consent somewhere to record its own context without a migration. Nothing reads it. + +The grain is the consent, not the document. A user accepts a set of documents in one act, at one +time, from one IP, so `user_email`, `ip_address` and `consented_at` describe the act and are stored +once. It also means the same write path covers a later re-consent for any subset, because the +document list is an argument rather than something the schema fixes. The cost is that the +per-document fields sit inside JSON, which the queries below cover. + +Four things in there are deliberate and look like mistakes otherwise. + +There is no foreign key to `users`. `UserRepository.Delete` does a hard `DELETE`, so +`ON DELETE CASCADE` would drop the consent records and `ON DELETE RESTRICT` would block account +deletion. Consent records have to survive the user being deleted, which is also why `user_email` +is denormalized: once the user row is gone there is nothing left to join to. `ip_address` is +`TEXT` and not `INET` because the value comes from a request header, and a bad one must not fail a +signup. For the same reason it is nullable: a deployment that does not set the header gets a record +with no IP rather than a failed signup. + +The document versions and URLs are copies, not references. A consent record has to stay readable +years later and stay correct after the document is dropped from config. It is also why a document +table can be added later with no backfill: no consent record points at one. + +The unique index means a user gets at most one signup consent. Since nothing repairs a record, a +second signup write for the same user is a bug, and this makes it fail instead of leaving two rows +disagreeing about what happened. + +Consent records cannot be changed. A `BEFORE UPDATE` and a `BEFORE DELETE` trigger both raise +`45000`, following `20250904105226_add_audit_records_immutability.up.sql`, which does the same for +`audit_records`. That migration guards only `UPDATE`; `DELETE` is guarded here too, because a +deleted consent record leaves a user who looks like they never consented. The repository has +`Create` and nothing else, so no admin API path reaches a record even if a trigger gets dropped. +`DROP TABLE` still works, so `migrate down` is fine. + +A signup also writes one `user.consent_granted` audit record so the audit trail shows it happened. +The consent record is the source of truth; if the audit write fails, log it and carry on. + +## Reading it back + +There is no read API and no view. A reporting tool points at `user_consents` and reads the rows as +they are, one per consent, with the accepted documents in `documents`. + +A record is self-contained, so "what did this user accept, and when" is one row. The reverse, "who +accepted privacy policy 2026-04-01", is a containment filter: + +```sql +SELECT user_email, consented_at, ip_address +FROM user_consents +WHERE documents @> '[{"id": "privacy_policy", "version": "2026-04-01"}]'; +``` + +`@>` is what the GIN `jsonb_path_ops` index on `documents` serves, so that filter uses the index. A +view would be one more object to keep in step with the table it summarizes. + +## Client + +Frontier ships its own sign-up view in `web/sdk/client/views/auth/sign-up`. It calls `authenticate` +for the OIDC buttons and hands mail OTP to `MagicLinkView`, which calls `authenticate` itself. +Neither sends ids, so with consent enabled the shipped view cannot complete a signup. It gets an +Apsara `Checkbox`: + +```tsx +export type SignUpViewProps = /* ... */ & { + consent?: { + documentIds: string[]; + label?: ReactNode; + }; +}; +``` + +The prop is optional and the checkbox renders only when it is passed, so a deployment with consent +disabled sees the view it sees today. When it is passed the checkbox starts unchecked, every +sign-up control stays disabled until it is checked, and `documentIds` goes out as +`accepted_document_ids`. `MagicLinkView` takes the ids as a prop, so both strategies go through one +control. + +`label` takes a `ReactNode` so the consumer supplies the copy and the links. The default is plain +text without links, since the SDK does not know the documents or their URLs. A second checkbox or a +per-document link is a consumer rendering its own view and calling `authenticate` directly, which +already works today. + +`documentIds` comes from the consumer because frontier does not serve the document list. It is the +duplication Limitations describes, now visible in a prop. + +## Alternatives considered + +1. Consent in `users.metadata`. Both update paths replace the whole map, so a user can delete + their own consent record, and it goes away with the user row. + +2. Consent in `audit_records`. `CreateAuditRecord` takes any event string with a client-supplied + `occurred_at`, gated on platform `check`, which both the admin and member relations grant. Any + platform member could insert a backdated consent record. A table with no write RPC can only be + written by the signup path. + +3. One row per accepted document instead of one per consent. It puts every field in a column, but + it repeats the email, IP and timestamp on every row, and it needs a synthetic event id to answer + "what did this user accept in one sitting" once there is more than one occasion. The act is what + is being recorded, so the act is the row. + +4. A `consent_documents` table instead of config. Each consent record already copies its document + versions, so the records are the version history and the table answers no query they cannot. + Config sits in git, which is a better change log than rows an admin can edit, and a table would + need a write API, which is one more way to change what the server stamps. + +5. A `ConsentDocument` reconcile kind. The reconciler drives the admin API over RPC, so a kind + needs list and write RPCs plus a table, and it would make the document list editable by any + superuser. A restart is the narrower path. + +6. An RPC for the document list. Clients keeping their own copy costs one proto field for the + whole feature instead of a new endpoint. Deployments that want one source of truth can generate + both the config block and the client's copy from a single file. + +7. Taking consent in `AuthCallback`. The client would have to stash it locally and resend it after + the redirect, so the consent record would attest to a client re-assertion made after the fact, + the IP would be the post-redirect one, and it would break when the provider comes back into a + different tab. + +8. Requiring the full document set at flow start. In an OIDC flow that looks the same as a login, + so it would block returning users. Only the unknown-id check can run there. + +9. A login or signup intent on `AuthenticateRequest`, which would make a signup identifiable at + flow start and let the full check run before the redirect. It also turns an unknown email on + login into an account enumeration oracle on an unauthenticated endpoint, which frontier does + not have today because it auto-provisions. Not worth it for an earlier error. + +10. Repairing a missing consent record on a later login. See Enforcement. + +## Limitations + +Changing a document version needs a redeploy, since config is read at boot. + +The client's document list and the server's are declared separately and can go out of sync. Ids +catch a set mismatch. A version mismatch is undetectable, since the client sends no versions: a +client showing version A against a config holding version B produces a record that says B. Only +frontier serving the list would close that. + +A record ties to a version string, not to the document text. Editing the file at a URL without +bumping the version leaves every record for that version describing something the user did not see. +A per-document hash would close it and can be added later, since a document object is just JSON. + +The IP is only as good as the header it comes from. A proxy that appends to `X-Forwarded-For` +instead of overwriting it leaves the value under the caller's control, and a deployment that does +not set the header at all gets records with no IP. + +Users who already exist get no consent record, and nothing will ever give them one, so you never +get to full coverage. Neither do users created through the three exempt paths under Enforcement. +"No account without consent" holds for the signup flow, not for every row in `users`, and a +deployment has to accept that distinction before it relies on the records. + +Sharing a transaction with the user insert means the user repository gains a create that takes a +transaction. It is additive, but it is the one place this feature reaches outside its own domain. + +## Future work + +Re-consent when a document version changes. The write path already handles it: `Grant` takes the +document list as an argument and `source` separates one occasion from another. What is missing is +enforcement, which has to move from user creation to a gate on authenticated requests, roughly +what Keycloak's terms and conditions required action does. That is its own feature. + +A document list endpoint, if clients keeping their own copy turns out to be too fragile. +`ListAuthStrategies` is unauthenticated and already fetched by the sign-in page, so the resolved +document set could ride along on it instead of needing a new RPC, and the SDK sign-up view could +render the documents rather than be handed their ids. + +A per-document hash in config, copied into the record, to tie a consent to the document text and +not to a version string someone typed into config. + +A `ConsentDocument` reconcile kind, if restarts become a problem. It needs list and write RPCs and +a `consent_documents` table keyed by `(document_id, version)`. The config map maps onto that +directly and nothing needs backfilling, since no consent record points at it. + +Withdrawal, which needs a decision on what happens to the account. + +A reserved-event guard on `CreateAuditRecord`, so events frontier writes itself cannot be injected +through the public RPC. Not part of this feature, but the same gap lets any platform member forge +a backdated `pat.revoked`. + +## References + +- [RFC 0001: Declarative management of platform resources](0001-declarative-reconcile.md), for the + reconcile flow mentioned above. +- ISO/IEC 29184:2020, *Online privacy notices and consent*, and the Kantara Consent Receipt + specification, which the field list follows. +- RFC 9126, *OAuth 2.0 Pushed Authorization Requests*, for keeping request data server-side behind + a handle. +- Digital Personal Data Protection Act, 2023, section 6(1), for the "clear affirmative action" + standard. From eba00f76a2040b4511aa77d56f0bff4e1faac313 Mon Sep 17 00:00:00 2001 From: Rohan Chakraborty Date: Thu, 27 Aug 2026 11:44:17 +0530 Subject: [PATCH 5/7] feat: explicit consent rfc --- docs/rfcs/0002-explicit-consent-at-signup.md | 395 +++++++++++++------ 1 file changed, 283 insertions(+), 112 deletions(-) diff --git a/docs/rfcs/0002-explicit-consent-at-signup.md b/docs/rfcs/0002-explicit-consent-at-signup.md index 674e666ed3..e0f43a39e3 100644 --- a/docs/rfcs/0002-explicit-consent-at-signup.md +++ b/docs/rfcs/0002-explicit-consent-at-signup.md @@ -17,13 +17,18 @@ sends the document ids the user accepted. Frontier checks the ids cover every do then creates the user and the consent record together. The record lists each accepted document with the version and URL from config. +Consent belongs to a signup, and frontier cannot currently tell a signup from a login. Both are the +same request, and a login creates the account it should have refused. So this RFC also adds a flow +intent that separates the two, which is what lets the consent check run before the browser leaves +for the identity provider. + If `app.consent` is not enabled, nothing changes. Frontier never parses document contents or version strings. ## Problem Frontier creates a user at the end of a registration flow and stores nothing about what the user -agreed to. Three gaps. +agreed to. Finishing signup is the only evidence of agreement. A record built from that only says the user signed up, which we already know. It cannot tell apart a user who read the documents from one who @@ -37,13 +42,24 @@ away with the user row. Documents have versions, but nothing records which version a user accepted, so you cannot tell which one applies to them. +And frontier does not know which requests are signups. `SignInView` and `SignUpView` are the same +view with different strings: both call `authenticate({ strategyName, callbackUrl })`, both hand +mail OTP to the same `MagicLinkView`, and only the title, the footer link and one `gap` value +differ. `AuthenticateRequest` carries no intent, `Flow` carries none, and every strategy ends at +`getOrCreateUser` (`core/authenticate/service.go:784`), which returns the existing user or creates +one. So logging in with an address that has no account creates it, through a view that never showed +the documents, and no client can ask for anything else. + ## Goals - The client sends consent explicitly. The server rejects signup when it is missing. - No API can update or delete a consent record. Deleting the user does not delete it either. - A consent record stores the version and URL of every document it covers. - Document ids, titles, versions and URLs come from config. Frontier does not read them. -- Deployments that do not set or enable `app.consent` behave exactly as before. +- A login never creates an account. A signup never silently logs an existing user in. +- One RPC and one callback, unchanged. +- Deployments that do not set or enable `app.consent`, and clients that send no intent, behave + exactly as before. ## Non-goals @@ -51,7 +67,11 @@ Re-consent, withdrawal, and users who already exist. When a document version cha records keep pointing at the old version and nobody is asked again. Repairing a missing consent record. A record is written when a user is created and at no other -time. There is no path that adds one to an account that already exists. +time. There is no path that adds one to an account that already exists, and none for the three +paths that create a user without starting a flow, which Enforcement lists. + +Making the login gate a security boundary, and hiding whether an address has an account. Both are +given up deliberately, and Limitations says what they cost. Frontier does not host the document text and does not serve the document list. Clients keep their own copy of the list. The SDK sign-up view gets a checkbox so it still works when consent is @@ -86,11 +106,10 @@ same one. And you can override a single field with an env var, which you cannot a list of objects. There is no per-document `required` flag. Every document in the map is required at signup. A flag -would put two lists in config, all documents and the required subset, where one does the job, and -an optional document is a different feature: it needs withdrawal, which is out of scope. The cost -is that adding a new document id breaks signup for clients still sending the old list, so a new -document and the client release that sends it have to ship together. Version bumps are unaffected, -since the client only sends ids. +would put two lists in config where one does the job, and an optional document is a different +feature that needs withdrawal. The cost is that adding a document id breaks signup for clients +still sending the old list, so a new document and the client release that sends it ship together. +Version bumps are unaffected, since the client only sends ids. `version` is an opaque string that frontier only compares for equality, so a deployment can use dates, semver or commit SHAs. @@ -98,39 +117,54 @@ dates, semver or commit SHAs. `enabled` is one switch for the whole feature. With `app.consent` absent or `enabled` false, frontier behaves exactly as it does today: no check runs, no record is written, and `accepted_document_ids` is ignored rather than rejected, so one client build works against both -kinds of deployment. With `enabled` true, every document in the map is required. `enabled` true +kinds of deployment. With `enabled` true every document in the map is required, and `enabled` true with no documents fails at boot instead of quietly turning the feature off. Config is read once at boot, so changing a document version needs a restart. Keys are -env-overridable like the rest of frontier's config. An override cannot change an existing consent -record, because records are immutable and each holds its own copy of the version, but it can -produce wrong new ones. So the resolved document set is logged at boot. That log, not the config -repo, tells you what a deployment was actually serving. +env-overridable like the rest of frontier's config. An override cannot change an existing record, +since records are immutable and each holds its own copy of the version, but it can produce wrong +new ones, so the resolved document set is logged at boot. That log, not the config repo, tells you +what a deployment was actually serving. Bad config fails at boot instead of at the first signup: document ids, versions and URLs must be non-empty, URLs must parse, and an enabled block needs at least one document. -## The request field +## The request fields -One repeated field on the existing `AuthenticateRequest`: +Two new fields on the existing `AuthenticateRequest`, authored in `raystack/proton` and generated +here through `PROTON_COMMIT`: ```proto -repeated string accepted_document_ids = 6; +enum FlowIntent { + FLOW_INTENT_UNSPECIFIED = 0; + FLOW_INTENT_LOGIN = 1; + FLOW_INTENT_SIGNUP = 2; +} + +FlowIntent flow_intent = 6; +repeated string accepted_document_ids = 7; ``` -Field 6 is free: `AuthenticateRequest` ends at `callback_url = 5` today. +Both fields are assigned in one proton PR, so neither can claim the other's number. -Document ids, not a single boolean, because the client has its own copy of the document list and -the two can go out of sync. With ids, frontier sees the mismatch and rejects the signup. With a -boolean it would stamp whatever config holds and write a consent record saying the user accepted -a document they were never shown, and nothing would catch it. Ids also leave room for consenting -to a subset later without changing the field. +An enum rather than a string for the intent, because the set is closed. The zero value carries +backward compatibility for free: unspecified means today's behaviour, so no existing caller has to +change. `AuthCallback` needs neither field. Both ride in the flow. + +The ids only ever accompany `FLOW_INTENT_SIGNUP`. Sent with a login intent they are a client bug +and the handler rejects them, since a login creates no user and writes no record. Dropping them +quietly would leave a client believing it had recorded a consent that does not exist. + +Document ids, not a single boolean, because the client has its own copy of the list and the two can +go out of sync. With ids frontier sees the mismatch and rejects the signup. With a boolean it would +stamp whatever config holds and write a record saying the user accepted a document they were never +shown, with nothing to catch it. Ids also leave room for consenting to a subset later. Ids and nothing else. The client never sends versions, titles or URLs, and frontier would not use them if it did, since everything on a consent record is stamped from config. Duplicate ids are de-duplicated before the check. -## Carrying consent across the redirect +## Carrying them across the redirect In an OIDC flow the user accepts the documents before the browser leaves for the identity provider, and the account is created after it comes back: @@ -138,28 +172,40 @@ provider, and the account is created after it comes back: ```mermaid flowchart TD accept["User accepts the documents"] - auth["Authenticate: document ids, IP and time
written to flows.metadata"] + auth["Authenticate: intent = signup,
ResolveAll on the ids"] + early["Reject with FailedPrecondition,
nothing written, no redirect"] + store["Intent, ids, IP and time
written to flows.metadata"] idp[("Identity provider")] cb["AuthCallback: flow row read back"] - q{"Would this create
a new user?"} - reject["Reject with FailedPrecondition,
no user created"] + q{"Does the user
already exist?"} + exists["The signup gate rejects it,
no consent record"] create["One transaction: create the user
and the consent record"] - login["Return the existing user,
write nothing"] - accept --> auth --> idp + accept --> auth + auth -->|"incomplete"| early + auth -->|"complete"| store --> idp idp -->|"state = flow id, code"| cb cb --> q - q -->|"yes, consent incomplete"| reject - q -->|"yes, consent complete"| create - q -->|"no"| login + q -->|"yes"| exists + q -->|"no"| create ``` The only thing that survives the redirect is `state`, which already holds the flow id and is -visible to the browser and the provider. So the consent goes where the flow id points. -`Flow.Metadata` is a JSONB column that already carries `callback_url`, and `StartFlow` writes one -more key: +visible to the browser and the provider. So both fields go where the flow id points. +`Flow.Metadata` is a JSONB column that already carries `callback_url`, so nothing needs a +migration. `RegistrationStartRequest` gains the intent and the ids, `core/authenticate` gains the +type, and `StartFlow` writes two more keys: ```go +type FlowIntent string + +const ( + FlowIntentUnspecified FlowIntent = "" + FlowIntentLogin FlowIntent = "login" + FlowIntentSignup FlowIntent = "signup" +) + +flow.Metadata["intent"] = string(intent) flow.Metadata["consent"] = map[string]any{ "accepted_document_ids": ids, "ip_address": ip, @@ -167,8 +213,8 @@ flow.Metadata["consent"] = map[string]any{ } ``` -The flow row is written before the redirect and read after it comes back, so the consent never -goes through the browser and cannot be changed on the way. The IP and time stored are from when +The flow row is written before the redirect and read after it comes back, so neither value goes +through the browser and neither can be changed on the way. The IP and time stored are from when the user accepted, not from the callback. Mail OTP and passkey use the same path, so there is one code path for every strategy. @@ -180,9 +226,11 @@ configured client IP header. It parses the user agent into an OS and a browser f raw string, which is why the record keeps only the IP. `Flow.Metadata` is `map[string]any` stored as JSONB, so it does not return the types it was given: -the ids come back as `[]any` and `at` as an RFC 3339 string. One typed parser handles the read, the -way `otpAttempts` already does for the attempt counter, rather than an unchecked assertion like -`flow.Metadata["callback_url"].(string)`. A missing or unparseable consent key counts as no consent. +the ids come back as `[]any` and `at` as an RFC 3339 string. One typed accessor per key handles the +read, the way `otpAttempts` already does for the attempt counter, rather than an unchecked +assertion like `flow.Metadata["callback_url"].(string)`. Both accessors are methods on `*Flow` and +are nil-receiver safe, returning `FlowIntentUnspecified` and no consent, so a caller with no flow +needs no branch of its own. A missing or unparseable consent key counts as no consent. This also fixes something unrelated. `applyOIDC` never calls `consumeFlow`, so OIDC flow rows sit around until the expiry cron while mail OTP rows are deleted on use. That is hard to justify once @@ -190,6 +238,29 @@ those rows hold consent. ## Enforcement +Two gates. `StartFlow` is the fast path and exists for the error message. User creation is the gate +that matters: it is the only point every strategy reaches, and it is where the account would +otherwise be created. OIDC does not know the email until the callback, which is why the login gate +needs both points. + +### Login and signup + +| Intent | Strategy | At `StartFlow` | At user creation | +|---|---|---|---| +| login | mailotp, passkey | reject if no user exists | reject if no user exists | +| login | oidc | email unknown, no check | reject if no user exists | +| signup | mailotp, passkey | reject if a user exists | reject if a user exists | +| signup | oidc | email unknown, no check | reject if a user exists | +| unspecified | all | no check | create or get, as today | + +`StartFlow` guesses signup from login for passkey by looking the user up +(`core/authenticate/service.go:220`), and `finishPassKeyLoginMethod` calls `getOrCreateUser`, so a +passkey login can create an account today. The intent replaces the guess: signup picks +`startPassKeyRegisterMethod`, login picks `startPassKeyLoginMethod`, and unspecified keeps the +guess so nothing existing breaks. + +### Consent + The consent service owns the config, so it owns both checks. They are separate functions because signup and any later use want different rules: @@ -212,37 +283,68 @@ required ids are missing, or which sent ids config does not know. `Grant` takes given, which is what leaves room for a later re-consent covering one document without a second write path. -Consent is stored at the start of the flow and required at user creation. It cannot be required at -the start: in an OIDC flow the email is not known yet, so a signup and a login look like the same -request. What the `Authenticate` handler can do is call `Resolve` on any ids it was sent, so an -unknown id fails before the browser leaves for the provider. `ResolveAll` runs at user creation, -which is the first point where frontier knows who the user is and that they are new. +The intent decides where the completeness check runs. -So `getOrCreateUser` takes the flow, which is nil for the one caller that has none, and: +With `FLOW_INTENT_SIGNUP` the `Authenticate` handler knows a signup when it sees one, so +`ResolveAll` runs there, before the browser leaves for the provider and before mail OTP sends +anything. An incomplete set is rejected with nothing written and no redirect. This holds for every +strategy, OIDC included: the email is still unknown at flow start, but the intent is not. -- New user, `ResolveAll` passes: one transaction creates the user row and the consent record. -- New user, `ResolveAll` fails: return `ErrConsentRequired` and create nothing. `AuthCallback` maps - it to `FailedPrecondition` so the client can tell it apart from a bad code or an expired flow and - ask again. -- Existing user: log them in and write nothing, whatever the flow holds. +With the intent unset, a signup and a login look like the same request, so the check splits. +`Resolve` at flow start catches an unknown id before the redirect, and `ResolveAll` runs at user +creation, the first point where frontier knows the user is new. An unset intent is permissive for +the login gate but not for this one: `ResolveAll` still has to pass before a user row is written, +so omitting the intent skips the login gate and not consent. -A rejection ends the flow. The user starts a new one with a complete set, and for mail OTP that -means a fresh code, since `applyMailOTP` calls `consumeFlow` before it creates the user. Reusing -the flow would only let the same client assert the same wrong set again. +`ResolveAll` runs again at user creation either way. Not for the error, which under a signup intent +has already been returned, but as the invariant guarding the write. It is the last point before the +insert, and it is what makes a user row without a consent record impossible however the flow +reached it. -The third case is absolute. There is no branch that notices a missing consent record and fills it -in. A record written outside a user creation would carry that moment's timestamp and IP for an -agreement that happened somewhere else, which is worse than having no record: it reads like real -evidence. +`getOrCreateUser` takes the flow, which is a signature change. The flow carries both the intent +and the consent, so one parameter serves both gates, and the nil-safe accessors mean +`authenticateWithPassthroughHeader`, which has no flow, passes nil and needs no branch of its own. +Then: + +- New user, `ResolveAll` passes: one transaction creates the user row and the consent record. +- New user, `ResolveAll` fails: return `ErrConsentRequired` and create nothing. +- Existing user: write nothing. Under a signup intent the login gate has already rejected the + request; under an unset intent they are logged in, whatever the flow holds. + +The third case is absolute. A record written outside a user creation would carry that moment's +timestamp and IP for an agreement that happened somewhere else, which is worse than having no +record: it reads like real evidence. That is why the transaction matters. `ResolveAll` runs before the transaction opens, so an incomplete payload never starts one. Inside it, the user insert and the consent insert either both -land or neither does. Without the transaction a failed consent insert would leave an account with -no consent record and nothing able to repair it, which is the gap this feature exists to close. -`pkg/db` has `WithTxn`, but no context-carried transaction, so both repositories need a `Create` -that accepts the `*sqlx.Tx`. That is additive and breaks no existing caller. If threading the -transaction through the user repository is rejected, the fallback is to delete the user row when -the consent insert fails and log loudly if that delete also fails. +land or neither does; without it a failed consent insert would leave an account with no record and +nothing able to repair it, which is the gap this feature exists to close. `pkg/db` has `WithTxn` +but no context-carried transaction, so both repositories need a `Create` that accepts the +`*sqlx.Tx`. That is additive and breaks no existing caller. If threading it through the user +repository is rejected, the fallback is to delete the user row when the consent insert fails and +log loudly if that delete also fails. + +### Errors + +Three errors alongside the existing block at `core/authenticate/service.go:53`: + +```go +ErrLoginUserNotFound = errors.New("no account for this email") +ErrSignupUserExists = errors.New("an account already exists for this email") +ErrConsentRequired = errors.New("consent required for the configured documents") +``` + +They map to `CodeNotFound`, `CodeAlreadyExists` and `CodeFailedPrecondition`, returned from both +`Authenticate` and `AuthCallback`. `AuthCallback` maps a fixed list of errors to `InvalidArgument` +and everything else to `Internal` (`internal/api/v1beta1connect/authenticate.go:117`), so all +three have to be added to that list or they surface as 500s. `FailedPrecondition` is what lets a +client tell a consent rejection apart from a bad code or an expired flow and ask again. + +A rejection ends the flow. Under a signup intent it happens at `Authenticate`, before an OTP goes +out or a redirect is issued, so the user retries and loses nothing. Under an unset intent a consent +rejection happens at user creation, and for mail OTP that means a fresh code, since `applyMailOTP` +calls `consumeFlow` before it creates the user. Either way, reusing the flow would only let the +same client assert the same wrong set again. ### The other paths that create users @@ -322,21 +424,19 @@ only, since old ones keep the shape they were written with. `metadata` is written once at insert like every other column and is empty today. It gives a later re-consent somewhere to record its own context without a migration. Nothing reads it. -The grain is the consent, not the document. A user accepts a set of documents in one act, at one -time, from one IP, so `user_email`, `ip_address` and `consented_at` describe the act and are stored -once. It also means the same write path covers a later re-consent for any subset, because the -document list is an argument rather than something the schema fixes. The cost is that the -per-document fields sit inside JSON, which the queries below cover. +The grain is the consent, not the document. A user accepts a set in one act, so `user_email`, +`ip_address` and `consented_at` describe the act and are stored once, and the document list is an +argument to the write rather than something the schema fixes, which is what covers a later +re-consent for any subset. Alternative 3 has the tradeoff. Four things in there are deliberate and look like mistakes otherwise. There is no foreign key to `users`. `UserRepository.Delete` does a hard `DELETE`, so `ON DELETE CASCADE` would drop the consent records and `ON DELETE RESTRICT` would block account -deletion. Consent records have to survive the user being deleted, which is also why `user_email` -is denormalized: once the user row is gone there is nothing left to join to. `ip_address` is -`TEXT` and not `INET` because the value comes from a request header, and a bad one must not fail a -signup. For the same reason it is nullable: a deployment that does not set the header gets a record -with no IP rather than a failed signup. +deletion. Records have to survive the user, which is also why `user_email` is denormalized: once +the user row is gone there is nothing left to join to. `ip_address` is `TEXT` and nullable, not +`INET`, because the value comes from a request header and a bad or absent one must not fail a +signup. The document versions and URLs are copies, not references. A consent record has to stay readable years later and stay correct after the document is dropped from config. It is also why a document @@ -353,16 +453,36 @@ deleted consent record leaves a user who looks like they never consented. The re `Create` and nothing else, so no admin API path reaches a record even if a trigger gets dropped. `DROP TABLE` still works, so `migrate down` is fine. -A signup also writes one `user.consent_granted` audit record so the audit trail shows it happened. -The consent record is the source of truth; if the audit write fails, log it and carry on. +A signup also writes one audit record so the audit trail shows it happened. `pkg/auditrecord` gains +`UserConsentGrantedEvent Event = "user.consent_granted"` and `ConsentType EntityType = "consent"`, +following the `entity.verb` naming already there. Every field is set explicitly: + +| Field | Value | +|---|---| +| `Actor` | the new user: its id, `app/user`, email as name | +| `Resource` | the same user | +| `Target` | the consent record id, `consent` type, document ids and versions in `Metadata` | +| `OccurredAt` | `consented_at` from the flow, not the write time | +| `OrgID`, `IdempotencyKey` | empty. Both are nullable, and a signup has no org | + +`Actor` cannot be left to enrichment, and this is the part that looks fine and is not. +`AuditRecordRepository.Create` calls `enrichActorFromContext` when the actor is empty, and nothing +puts an actor in the context of a skip-listed endpoint, so the record would land with `uuid.Nil` and +the `system` actor for an act a person performed. Going through `auditrecord.Service.Create` +instead is worse: its `enrichUserActor` reads `Actor.ID` as a session id and resolves the user from +`session.UserID`, and no session exists yet, so it returns `ErrActorNotFound`. So the write goes +through the repository with the actor filled in, the way `userpat` writes its PAT events. +`actor_id` is `UUID NOT NULL` and the user id exists by then, because the row is already committed. + +The write happens after the transaction commits, since `Create` has no `*sqlx.Tx` variant, so it +cannot be atomic with the consent record. That is why the consent record is the source of truth: if +the audit write fails, log it and carry on. ## Reading it back There is no read API and no view. A reporting tool points at `user_consents` and reads the rows as -they are, one per consent, with the accepted documents in `documents`. - -A record is self-contained, so "what did this user accept, and when" is one row. The reverse, "who -accepted privacy policy 2026-04-01", is a containment filter: +they are. A record is self-contained, so "what did this user accept, and when" is one row. The +reverse, "who accepted privacy policy 2026-04-01", is a containment filter: ```sql SELECT user_email, consented_at, ip_address @@ -375,10 +495,12 @@ view would be one more object to keep in step with the table it summarizes. ## Client -Frontier ships its own sign-up view in `web/sdk/client/views/auth/sign-up`. It calls `authenticate` -for the OIDC buttons and hands mail OTP to `MagicLinkView`, which calls `authenticate` itself. -Neither sends ids, so with consent enabled the shipped view cannot complete a signup. It gets an -Apsara `Checkbox`: +Three files under `web/sdk/client/views/auth`, which are the only callers of `authenticate` in the +repo. `sign-up/sign-up-view.tsx` sends signup on the OIDC buttons, `sign-in/sign-in-view.tsx` sends +login, and `magic-link/magic-link-view.tsx` is shared by both, so it takes the intent as a prop. + +Neither sends document ids, so with consent enabled the shipped sign-up view cannot complete a +signup. It gets an Apsara `Checkbox`: ```tsx export type SignUpViewProps = /* ... */ & { @@ -392,16 +514,22 @@ export type SignUpViewProps = /* ... */ & { The prop is optional and the checkbox renders only when it is passed, so a deployment with consent disabled sees the view it sees today. When it is passed the checkbox starts unchecked, every sign-up control stays disabled until it is checked, and `documentIds` goes out as -`accepted_document_ids`. `MagicLinkView` takes the ids as a prop, so both strategies go through one -control. +`accepted_document_ids`. `MagicLinkView` takes the ids alongside the intent, so both strategies go +through one control. `label` takes a `ReactNode` so the consumer supplies the copy and the links. The default is plain -text without links, since the SDK does not know the documents or their URLs. A second checkbox or a -per-document link is a consumer rendering its own view and calling `authenticate` directly, which -already works today. +text without links, since the SDK does not know the documents or their URLs, and `documentIds` +comes from the consumer for the same reason. It is the duplication Limitations describes, now +visible in a prop. A second checkbox or a per-document link is a consumer rendering its own view +and calling `authenticate` directly, which already works today. + +`magicLinkHandler` handles only `status === 400` today and writes the message into the email field. +It needs the three new codes, with copy that points at the other view for the two gate errors; +`config.redirectSignup` and `config.redirectLogin` already exist for the links. -`documentIds` comes from the consumer because frontier does not serve the document list. It is the -duplication Limitations describes, now visible in a prop. +The OIDC rejections arrive at `AuthCallback`, which is the callback page rather than the view that +started the flow, and that page has no error UI. Redirecting back to the originating view with an +error param is the smaller change, rendering it in place is the other option, and this is undecided. ## Alternatives considered @@ -418,33 +546,44 @@ duplication Limitations describes, now visible in a prop. "what did this user accept in one sitting" once there is more than one occasion. The act is what is being recorded, so the act is the row. -4. A `consent_documents` table instead of config. Each consent record already copies its document - versions, so the records are the version history and the table answers no query they cannot. - Config sits in git, which is a better change log than rows an admin can edit, and a table would - need a write API, which is one more way to change what the server stamps. - -5. A `ConsentDocument` reconcile kind. The reconciler drives the admin API over RPC, so a kind - needs list and write RPCs plus a table, and it would make the document list editable by any - superuser. A restart is the narrower path. +4. The documents in the database instead of config, whether as a plain `consent_documents` table, + a `ConsentDocument` reconcile kind or an RPC serving the list. Each consent record already + copies its document versions, so the records are the version history and a table answers no + query they cannot. Config sits in git, which is a better change log than rows an admin can + edit, and any of the three needs a write path, which is one more way to change what the server + stamps. A deployment that wants one source of truth can generate both the config block and the + client's copy from a single file. Future work has the conditions under which this is revisited. -6. An RPC for the document list. Clients keeping their own copy costs one proto field for the - whole feature instead of a new endpoint. Deployments that want one source of truth can generate - both the config block and the client's copy from a single file. - -7. Taking consent in `AuthCallback`. The client would have to stash it locally and resend it after +5. Taking consent in `AuthCallback`. The client would have to stash it locally and resend it after the redirect, so the consent record would attest to a client re-assertion made after the fact, the IP would be the post-redirect one, and it would break when the provider comes back into a different tab. -8. Requiring the full document set at flow start. In an OIDC flow that looks the same as a login, - so it would block returning users. Only the unknown-id check can run there. +6. Repairing a missing consent record on a later login. See Enforcement. + +7. Two RPCs, `Login` and `Signup`, instead of the intent. `AuthCallback` cannot be split alongside + them: `state` is the flow id, both gates run at callback time, and two callback URLs would have + to be registered with every provider. The intent would still have to ride on the flow, so the + split duplicates the entry point without moving either gate, and leaves `Authenticate` as a + permanent ungated path since no existing caller can be broken. Frontier already discriminates + strategies with `strategy_name` on one RPC. -9. A login or signup intent on `AuthenticateRequest`, which would make a signup identifiable at - flow start and let the full check run before the redirect. It also turns an unknown email on - login into an account enumeration oracle on an unauthenticated endpoint, which frontier does - not have today because it auto-provisions. Not worth it for an earlier error. +8. A `oneof` carrying a `LoginIntent` and a `SignupIntent` message, with the accepted ids on the + signup arm only. It makes a signup-only field unrepresentable on a login rather than merely + rejected, and there is precedent in `ChangeSubscriptionRequest.Change`. But + `AuthenticateRequest.email` is already a field only some strategies use, checked at runtime, so + the flat field is the shape this message has. Moving later means deprecating field 6 and + carrying both for a window, so adopt it now if more signup-only fields are expected. -10. Repairing a missing consent record on a later login. See Enforcement. +9. Enforcing the login gate only at user creation, or accepting the flow and failing at + verification without sending anything. Both keep `Authenticate` quiet about whether an address + has an account. The first costs a wasted OTP mail and puts "no account for this email" on the + verification screen, where it reads like a bad code; the second wastes nothing but leaves the + user waiting for a code that never arrives. The clearer error was chosen over the quieter + endpoint. + +10. A first-class `Intent` field on `Flow`. It types better than metadata and costs a migration on + `flows` for one string, when the consent payload has to go in `metadata` regardless. ## Limitations @@ -468,6 +607,16 @@ get to full coverage. Neither do users created through the three exempt paths un "No account without consent" holds for the signup flow, not for every row in `users`, and a deployment has to accept that distinction before it relies on the records. +The unauthenticated `Authenticate` endpoint now answers whether an address has an account, for +mailotp and passkey where the email is known at flow start. Frontier does not answer that today, +because it auto-provisions instead. Rate limiting per address and per IP is the mitigation, not +hiding the answer. Passkey already leaks existence through its response shape, since register and +login return different options, and the intent neither widens that nor closes it. + +The login gate is a UX boundary and not a security one: an unset intent keeps create-or-get, so any +client can opt out by omitting the field. Consent cannot be opted out that way, since `ResolveAll` +runs at user creation under every intent. + Sharing a transaction with the user insert means the user repository gains a create that takes a transaction. It is additive, but it is the one place this feature reaches outside its own domain. @@ -478,6 +627,9 @@ document list as an argument and `source` separates one occasion from another. W enforcement, which has to move from user creation to a gate on authenticated requests, roughly what Keycloak's terms and conditions required action does. That is its own feature. +A server switch that rejects an unset intent, which turns the login gate into a boundary. It needs +a deprecation window first, since it breaks every client that has not moved. + A document list endpoint, if clients keeping their own copy turns out to be too fragile. `ListAuthStrategies` is unauthenticated and already fetched by the sign-in page, so the resolved document set could ride along on it instead of needing a new RPC, and the SDK sign-up view could @@ -493,8 +645,27 @@ directly and nothing needs backfilling, since no consent record points at it. Withdrawal, which needs a decision on what happens to the account. A reserved-event guard on `CreateAuditRecord`, so events frontier writes itself cannot be injected -through the public RPC. Not part of this feature, but the same gap lets any platform member forge -a backdated `pat.revoked`. +through the public RPC. Any platform member can forge a backdated `user.consent_granted` through +it, the same way they can forge `pat.revoked`. That does not weaken `user_consents`, which has no +write RPC, but it does mean the audit record is a breadcrumb and not evidence. + +## Work in order + +1. proton PR adding `FlowIntent`, `flow_intent = 6` and `accepted_document_ids = 7` in one change. +2. Bump `PROTON_COMMIT` in `Makefile:7` and run `make proto`. +3. Core, the login gate first: the `FlowIntent` type, the request fields, the metadata write and + the nil-safe accessors, the `StartFlow` checks, the passkey branch, and the `getOrCreateUser` + signature. +4. Core, consent: `app.consent` with boot validation, the consent service, the migration and + repository, and the transactional write in `getOrCreateUser`. +5. Handlers: the intent and ids into `StartFlow`, `ExtractSessionMetadata` in `Authenticate`, and + the three errors mapped in both `Authenticate` and `AuthCallback`. +6. SDK: the intent and ids through `MagicLinkView`, both views, the checkbox, and the error copy. +7. Tests. `core/authenticate/service_test.go` covers three intents against an address that does and + does not have an account, at both enforcement points, plus one case per strategy, since OIDC, + mail OTP and both passkey methods reach `getOrCreateUser` by different routes. Consent adds the + complete, incomplete and unknown-id sets, and a consent insert failure rolling back the user + row. ## References From 6349560c253dcac72c286e57c85512436db0482c Mon Sep 17 00:00:00 2001 From: Rohan Chakraborty Date: Thu, 27 Aug 2026 12:15:30 +0530 Subject: [PATCH 6/7] docs: tighten RFC 0002 prose Shorter sentences and plainer wording throughout, and cut the paragraphs that restated a decision already made earlier in the document. No design changes: every decision, file reference and verified claim is unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- docs/rfcs/0002-explicit-consent-at-signup.md | 593 ++++++++----------- 1 file changed, 258 insertions(+), 335 deletions(-) diff --git a/docs/rfcs/0002-explicit-consent-at-signup.md b/docs/rfcs/0002-explicit-consent-at-signup.md index e0f43a39e3..a1f2c1db50 100644 --- a/docs/rfcs/0002-explicit-consent-at-signup.md +++ b/docs/rfcs/0002-explicit-consent-at-signup.md @@ -9,73 +9,64 @@ ## Summary -Frontier can make a user accept a set of documents before their account is created, and store one -consent record for it. +Frontier can require a user to accept a set of documents before their account is created, and store +one consent record for it. A deployment lists its documents in server config with an id, title, version and URL. The client -sends the document ids the user accepted. Frontier checks the ids cover every document in config, -then creates the user and the consent record together. The record lists each accepted document -with the version and URL from config. +sends the ids the user accepted. Frontier checks that the ids cover every document in config, then +creates the user and the consent record in one transaction. The record copies each document's +version and URL from config. -Consent belongs to a signup, and frontier cannot currently tell a signup from a login. Both are the -same request, and a login creates the account it should have refused. So this RFC also adds a flow -intent that separates the two, which is what lets the consent check run before the browser leaves -for the identity provider. +Consent applies to signups only, and frontier cannot tell a signup from a login today. So this RFC +also adds a flow intent, which is what lets the consent check run before the browser leaves for the +identity provider. -If `app.consent` is not enabled, nothing changes. Frontier never parses document contents or -version strings. +With `app.consent` disabled, nothing changes. Frontier never reads document contents and never +parses version strings. ## Problem Frontier creates a user at the end of a registration flow and stores nothing about what the user -agreed to. +agreed to. Four gaps. -Finishing signup is the only evidence of agreement. A record built from that only says the user -signed up, which we already know. It cannot tell apart a user who read the documents from one who -never saw them. +Finishing signup is the only evidence of agreement, and it proves nothing. It cannot separate a user +who read the documents from one who never saw them. -There is nothing to show when someone asks which documents a user accepted, and when. -`users.metadata` does not work for this: `UpdateUser` and `UpdateCurrentUser` both replace the -whole map, so a user can delete their own consent by editing their profile, and the metadata goes -away with the user row. +Nothing answers "which documents did this user accept, and when". `users.metadata` cannot hold it, +because `UpdateUser` and `UpdateCurrentUser` replace the whole map, so a user could delete their own +consent by editing their profile. -Documents have versions, but nothing records which version a user accepted, so you cannot tell -which one applies to them. +Documents are versioned, but nothing records which version a user accepted. -And frontier does not know which requests are signups. `SignInView` and `SignUpView` are the same -view with different strings: both call `authenticate({ strategyName, callbackUrl })`, both hand -mail OTP to the same `MagicLinkView`, and only the title, the footer link and one `gap` value -differ. `AuthenticateRequest` carries no intent, `Flow` carries none, and every strategy ends at +Frontier does not know which requests are signups. `SignInView` and `SignUpView` are the same view +with different strings, `AuthenticateRequest` and `Flow` carry no intent, and every strategy ends at `getOrCreateUser` (`core/authenticate/service.go:784`), which returns the existing user or creates -one. So logging in with an address that has no account creates it, through a view that never showed -the documents, and no client can ask for anything else. +one. So a login with an unknown address creates the account, through a view that never showed the +documents. ## Goals -- The client sends consent explicitly. The server rejects signup when it is missing. -- No API can update or delete a consent record. Deleting the user does not delete it either. -- A consent record stores the version and URL of every document it covers. -- Document ids, titles, versions and URLs come from config. Frontier does not read them. -- A login never creates an account. A signup never silently logs an existing user in. +- The client sends consent explicitly. Signup fails without it. +- No API can update or delete a consent record. Deleting the user does not delete it. +- A record stores the version and URL of every document it covers. +- Document ids, titles, versions and URLs come from config. +- A login never creates an account. A signup never logs an existing user in. - One RPC and one callback, unchanged. -- Deployments that do not set or enable `app.consent`, and clients that send no intent, behave - exactly as before. +- Deployments without `app.consent`, and clients without an intent, behave as before. ## Non-goals -Re-consent, withdrawal, and users who already exist. When a document version changes, old consent -records keep pointing at the old version and nobody is asked again. +Re-consent and withdrawal. When a version changes, old records keep the old version and nobody is +asked again. -Repairing a missing consent record. A record is written when a user is created and at no other -time. There is no path that adds one to an account that already exists, and none for the three -paths that create a user without starting a flow, which Enforcement lists. +Consent for users who already exist, or for the three paths that create a user without a flow. A +record is written at user creation and nowhere else. -Making the login gate a security boundary, and hiding whether an address has an account. Both are -given up deliberately, and Limitations says what they cost. +Making the login gate a security boundary, or hiding whether an address has an account. Limitations +says what both cost. -Frontier does not host the document text and does not serve the document list. Clients keep their -own copy of the list. The SDK sign-up view gets a checkbox so it still works when consent is -enabled, but the copy, the links and any second checkbox are the consumer's. +Serving the document text or the document list. Clients keep their own copy. The SDK sign-up view +gets a checkbox, but the copy and the links are the consumer's. ## Document config @@ -96,43 +87,34 @@ app: url: https://example.org/legal/privacy/2026-04-01 ``` -`app.consent` sits next to `app.authentication` and `app.pat` on `server.Config`. A document has an -id, a title, a version and a URL, and nothing else. The example lists two; a deployment lists all of -its documents, and they have to be in config from the first release, for the reason below. +`app.consent` sits next to `app.authentication` and `app.pat` on `server.Config`. The example lists +two documents; a deployment lists all of its own. -A map, not a list, for three reasons. It matches `authenticate.Config`, which already keys -`oidc_config` by strategy name. The map key is the document id, so two documents cannot use the -same one. And you can override a single field with an env var, which you cannot do for one item in -a list of objects. +A map, not a list: it matches `authenticate.Config` keying `oidc_config` by strategy name, the key +enforces unique ids, and a single field stays env-overridable. -There is no per-document `required` flag. Every document in the map is required at signup. A flag -would put two lists in config where one does the job, and an optional document is a different -feature that needs withdrawal. The cost is that adding a document id breaks signup for clients -still sending the old list, so a new document and the client release that sends it ship together. -Version bumps are unaffected, since the client only sends ids. +Every document in the map is required at signup, so there is no per-document `required` flag. An +optional document would need withdrawal, which is out of scope. The cost is that adding a document +id breaks signup for clients still sending the old list, so a new document ships with the client +release that sends it. Version bumps are safe, since the client sends only ids. -`version` is an opaque string that frontier only compares for equality, so a deployment can use -dates, semver or commit SHAs. +`version` is opaque. Frontier compares it for equality, so dates, semver or commit SHAs all work. -`enabled` is one switch for the whole feature. With `app.consent` absent or `enabled` false, -frontier behaves exactly as it does today: no check runs, no record is written, and +`enabled` switches the whole feature. Absent or false, frontier behaves as it does today, and `accepted_document_ids` is ignored rather than rejected, so one client build works against both -kinds of deployment. With `enabled` true every document in the map is required, and `enabled` true -with no documents fails at boot instead of quietly turning the feature off. +kinds of deployment. True with no documents fails at boot rather than silently disabling itself. -Config is read once at boot, so changing a document version needs a restart. Keys are -env-overridable like the rest of frontier's config. An override cannot change an existing record, -since records are immutable and each holds its own copy of the version, but it can produce wrong -new ones, so the resolved document set is logged at boot. That log, not the config repo, tells you -what a deployment was actually serving. +Config is read at boot, so a version change needs a restart. An env override cannot alter an +existing record, but it can produce wrong new ones, so the resolved set is logged at boot. That log, +not the config repo, says what a deployment was serving. -Bad config fails at boot instead of at the first signup: document ids, versions and URLs must be -non-empty, URLs must parse, and an enabled block needs at least one document. +Bad config fails at boot: ids, versions and URLs must be non-empty, URLs must parse, and an enabled +block needs at least one document. ## The request fields -Two new fields on the existing `AuthenticateRequest`, authored in `raystack/proton` and generated -here through `PROTON_COMMIT`: +Two new fields on `AuthenticateRequest`, authored in `raystack/proton` and generated here through +`PROTON_COMMIT`: ```proto enum FlowIntent { @@ -145,29 +127,24 @@ FlowIntent flow_intent = 6; repeated string accepted_document_ids = 7; ``` -Both fields are assigned in one proton PR, so neither can claim the other's number. +Both fields land in one proton PR, so neither can claim the other's number. An enum, not a string, +because the set is closed, and its zero value gives backward compatibility for free. +`AuthCallback` needs neither field, since both ride on the flow. -An enum rather than a string for the intent, because the set is closed. The zero value carries -backward compatibility for free: unspecified means today's behaviour, so no existing caller has to -change. `AuthCallback` needs neither field. Both ride in the flow. +The ids accompany `FLOW_INTENT_SIGNUP` only. With a login intent they are a client bug and the +handler rejects them, because a login writes no record. Accepting them silently would leave a client +believing it recorded a consent that does not exist. -The ids only ever accompany `FLOW_INTENT_SIGNUP`. Sent with a login intent they are a client bug -and the handler rejects them, since a login creates no user and writes no record. Dropping them -quietly would leave a client believing it had recorded a consent that does not exist. - -Document ids, not a single boolean, because the client has its own copy of the list and the two can -go out of sync. With ids frontier sees the mismatch and rejects the signup. With a boolean it would -stamp whatever config holds and write a record saying the user accepted a document they were never -shown, with nothing to catch it. Ids also leave room for consenting to a subset later. - -Ids and nothing else. The client never sends versions, titles or URLs, and frontier would not use -them if it did, since everything on a consent record is stamped from config. Duplicate ids are -de-duplicated before the check. +Ids rather than one boolean, because the client's copy of the list can drift from config. Ids expose +the mismatch; a boolean would stamp whatever config holds, writing a record that says the user +accepted a document they never saw. Ids also allow consenting to a subset later. Versions, titles +and URLs all come from config, so a client sending them would be ignored. Duplicates are removed +before the check. ## Carrying them across the redirect -In an OIDC flow the user accepts the documents before the browser leaves for the identity -provider, and the account is created after it comes back: +In an OIDC flow the user accepts before the browser leaves for the provider, and the account is +created after it returns: ```mermaid flowchart TD @@ -190,11 +167,10 @@ flowchart TD q -->|"no"| create ``` -The only thing that survives the redirect is `state`, which already holds the flow id and is -visible to the browser and the provider. So both fields go where the flow id points. -`Flow.Metadata` is a JSONB column that already carries `callback_url`, so nothing needs a -migration. `RegistrationStartRequest` gains the intent and the ids, `core/authenticate` gains the -type, and `StartFlow` writes two more keys: +Only `state` survives the redirect, and it already holds the flow id, so both fields go where the +flow id points. `Flow.Metadata` is a JSONB column that already carries `callback_url`, so no +migration is needed. `RegistrationStartRequest` gains the intent and the ids, and `StartFlow` writes +two more keys: ```go type FlowIntent string @@ -213,35 +189,30 @@ flow.Metadata["consent"] = map[string]any{ } ``` -The flow row is written before the redirect and read after it comes back, so neither value goes -through the browser and neither can be changed on the way. The IP and time stored are from when -the user accepted, not from the callback. Mail OTP and passkey use the same path, so there is one -code path for every strategy. - -`Authenticate` and `AuthCallback` are both in `authenticationSkipList`, so the authentication -interceptor does not run and nothing puts session metadata in the context. The `Authenticate` -handler has to call `sessionutils.ExtractSessionMetadata` itself and pass the IP into `StartFlow`. -That helper returns `session.SessionMetadata`, whose `IpAddress` is the leftmost value of the -configured client IP header. It parses the user agent into an OS and a browser family and drops the -raw string, which is why the record keeps only the IP. - -`Flow.Metadata` is `map[string]any` stored as JSONB, so it does not return the types it was given: -the ids come back as `[]any` and `at` as an RFC 3339 string. One typed accessor per key handles the -read, the way `otpAttempts` already does for the attempt counter, rather than an unchecked -assertion like `flow.Metadata["callback_url"].(string)`. Both accessors are methods on `*Flow` and -are nil-receiver safe, returning `FlowIntentUnspecified` and no consent, so a caller with no flow -needs no branch of its own. A missing or unparseable consent key counts as no consent. - -This also fixes something unrelated. `applyOIDC` never calls `consumeFlow`, so OIDC flow rows sit -around until the expiry cron while mail OTP rows are deleted on use. That is hard to justify once -those rows hold consent. +The flow row is written before the redirect and read after it returns, so neither value passes +through the browser. The stored IP and time are from when the user accepted, not from the callback. +Every strategy shares this path. + +`Authenticate` and `AuthCallback` are in `authenticationSkipList`, so nothing puts session metadata +in the context. The handler calls `sessionutils.ExtractSessionMetadata` itself and passes the IP +into `StartFlow`. That helper parses the user agent into an OS and browser family and drops the raw +string, so the record keeps only the IP. + +JSONB does not return the types it was given: the ids come back as `[]any` and `at` as an RFC 3339 +string. One typed accessor per key handles the read, as `otpAttempts` already does, instead of an +unchecked assertion like `flow.Metadata["callback_url"].(string)`. Both accessors are +nil-receiver-safe methods on `*Flow`, so a caller without a flow needs no branch. A missing or +unparseable consent key means no consent. + +One unrelated fix comes with this. `applyOIDC` never calls `consumeFlow`, so OIDC flow rows survive +until the expiry cron while mail OTP rows are deleted on use. That is hard to justify once the rows +hold consent. ## Enforcement Two gates. `StartFlow` is the fast path and exists for the error message. User creation is the gate -that matters: it is the only point every strategy reaches, and it is where the account would -otherwise be created. OIDC does not know the email until the callback, which is why the login gate -needs both points. +that matters: every strategy reaches it, and it is where the account would be created. OIDC does not +know the email until the callback, so the login gate needs both points. ### Login and signup @@ -253,16 +224,15 @@ needs both points. | signup | oidc | email unknown, no check | reject if a user exists | | unspecified | all | no check | create or get, as today | -`StartFlow` guesses signup from login for passkey by looking the user up +`StartFlow` currently guesses signup from login for passkey by looking the user up (`core/authenticate/service.go:220`), and `finishPassKeyLoginMethod` calls `getOrCreateUser`, so a -passkey login can create an account today. The intent replaces the guess: signup picks -`startPassKeyRegisterMethod`, login picks `startPassKeyLoginMethod`, and unspecified keeps the -guess so nothing existing breaks. +passkey login can create an account. The intent replaces the guess: signup picks +`startPassKeyRegisterMethod`, login picks `startPassKeyLoginMethod`, unspecified keeps the guess. ### Consent -The consent service owns the config, so it owns both checks. They are separate functions because -signup and any later use want different rules: +The consent service owns the config, so it owns the checks. Three functions, because signup and any +later use need different rules: ```go // Resolve maps ids to their config snapshots. Rejects unknown ids. @@ -278,51 +248,40 @@ func (s Service) ResolveAll(ids []string) ([]Document, error) func (s Service) Grant(ctx context.Context, tx *sqlx.Tx, req GrantRequest) error ``` -`ResolveAll` compares the two sets in both directions, so the error names what is wrong: which -required ids are missing, or which sent ids config does not know. `Grant` takes whatever it is -given, which is what leaves room for a later re-consent covering one document without a second -write path. +`ResolveAll` compares both sets in both directions, so the error names what is wrong: missing +required ids, or ids config does not know. `Grant` takes what it is given, which leaves room for a +re-consent covering one document without a second write path. -The intent decides where the completeness check runs. - -With `FLOW_INTENT_SIGNUP` the `Authenticate` handler knows a signup when it sees one, so -`ResolveAll` runs there, before the browser leaves for the provider and before mail OTP sends -anything. An incomplete set is rejected with nothing written and no redirect. This holds for every -strategy, OIDC included: the email is still unknown at flow start, but the intent is not. - -With the intent unset, a signup and a login look like the same request, so the check splits. +The intent decides where the completeness check runs. Under `FLOW_INTENT_SIGNUP`, `Authenticate` +knows it has a signup, so `ResolveAll` runs there, before the redirect and before mail OTP sends +anything. This holds for every strategy including OIDC: the email is unknown at flow start, but the +intent is not. Without an intent the check splits, since a signup and a login look identical: `Resolve` at flow start catches an unknown id before the redirect, and `ResolveAll` runs at user creation, the first point where frontier knows the user is new. An unset intent is permissive for -the login gate but not for this one: `ResolveAll` still has to pass before a user row is written, -so omitting the intent skips the login gate and not consent. +the login gate but never for consent. -`ResolveAll` runs again at user creation either way. Not for the error, which under a signup intent -has already been returned, but as the invariant guarding the write. It is the last point before the -insert, and it is what makes a user row without a consent record impossible however the flow -reached it. +`ResolveAll` runs at user creation either way, not for the error but as the invariant guarding the +write. It is the last point before the insert, and it makes a user row without a consent record +impossible. -`getOrCreateUser` takes the flow, which is a signature change. The flow carries both the intent -and the consent, so one parameter serves both gates, and the nil-safe accessors mean -`authenticateWithPassthroughHeader`, which has no flow, passes nil and needs no branch of its own. -Then: +`getOrCreateUser` takes the flow, which is a signature change. The flow carries both the intent and +the consent, so one parameter serves both gates, and the nil-safe accessors let +`authenticateWithPassthroughHeader` pass nil without a branch. Then: - New user, `ResolveAll` passes: one transaction creates the user row and the consent record. -- New user, `ResolveAll` fails: return `ErrConsentRequired` and create nothing. +- New user, `ResolveAll` fails: return `ErrConsentRequired`, create nothing. - Existing user: write nothing. Under a signup intent the login gate has already rejected the - request; under an unset intent they are logged in, whatever the flow holds. + request; without an intent they are logged in, whatever the flow holds. The third case is absolute. A record written outside a user creation would carry that moment's -timestamp and IP for an agreement that happened somewhere else, which is worse than having no -record: it reads like real evidence. - -That is why the transaction matters. `ResolveAll` runs before the transaction opens, so an -incomplete payload never starts one. Inside it, the user insert and the consent insert either both -land or neither does; without it a failed consent insert would leave an account with no record and -nothing able to repair it, which is the gap this feature exists to close. `pkg/db` has `WithTxn` -but no context-carried transaction, so both repositories need a `Create` that accepts the -`*sqlx.Tx`. That is additive and breaks no existing caller. If threading it through the user -repository is rejected, the fallback is to delete the user row when the consent insert fails and -log loudly if that delete also fails. +timestamp and IP for an agreement made elsewhere, which is worse than no record: it reads like +evidence. + +Hence the transaction. `ResolveAll` runs before it opens, so an incomplete payload never starts one. +Inside, the user insert and the consent insert both land or neither does. `pkg/db` has `WithTxn` but +no context-carried transaction, so both repositories need a `Create` that takes a `*sqlx.Tx`. That +is additive. If threading it through the user repository is rejected, the fallback is deleting the +user row when the consent insert fails, and logging loudly if that delete also fails. ### Errors @@ -335,20 +294,19 @@ ErrConsentRequired = errors.New("consent required for the configured documents ``` They map to `CodeNotFound`, `CodeAlreadyExists` and `CodeFailedPrecondition`, returned from both -`Authenticate` and `AuthCallback`. `AuthCallback` maps a fixed list of errors to `InvalidArgument` -and everything else to `Internal` (`internal/api/v1beta1connect/authenticate.go:117`), so all -three have to be added to that list or they surface as 500s. `FailedPrecondition` is what lets a -client tell a consent rejection apart from a bad code or an expired flow and ask again. +`Authenticate` and `AuthCallback`. `AuthCallback` maps a fixed list to `InvalidArgument` and +everything else to `Internal` (`internal/api/v1beta1connect/authenticate.go:117`), so all three must +join that list or surface as 500s. `FailedPrecondition` is what lets a client separate a consent +rejection from a bad code or an expired flow. -A rejection ends the flow. Under a signup intent it happens at `Authenticate`, before an OTP goes -out or a redirect is issued, so the user retries and loses nothing. Under an unset intent a consent -rejection happens at user creation, and for mail OTP that means a fresh code, since `applyMailOTP` -calls `consumeFlow` before it creates the user. Either way, reusing the flow would only let the -same client assert the same wrong set again. +A rejection ends the flow. Under a signup intent it happens at `Authenticate`, before an OTP or a +redirect, so the user retries and loses nothing. Without an intent a consent rejection happens at +user creation, and mail OTP needs a fresh code, since `applyMailOTP` calls `consumeFlow` before +creating the user. ### The other paths that create users -`getOrCreateUser` has five callers, and two paths create a user row without going through it at all: +`getOrCreateUser` has five callers, and two paths create a user row without going through it: | Path | With consent enabled | |---|---| @@ -360,15 +318,11 @@ same client assert the same wrong set again. | `organization.Service.AdminCreate` | not gated: operator action, no flow | | `CreateUser` RPC | not gated: operator action, no flow | -The four flow-based paths are the ones a person signs up through, and they are the ones this RFC -closes. The other three are exempt because no account holder is present to consent: -`authenticateWithPassthroughHeader` provisions from `app.identity_proxy_header`, which already -warns that it bypasses authorization, and `AdminCreate` and `CreateUser` are operator actions. - -Exempt means those accounts get no consent record, the same as users who already exist. A -deployment that wants full coverage keeps all three out of its signup path: `identity_proxy_header` -unset outside development, and the two operator RPCs used only for accounts nobody signs up for. -Limitations records the residual gap. +The four flow-based paths are the ones people sign up through, and this RFC closes them. The other +three are exempt, because no account holder is present to consent: +`authenticateWithPassthroughHeader` provisions from `app.identity_proxy_header`, which already warns +that it bypasses authorization, and the other two are operator actions. Exempt means no consent +record, the same as users who already exist. Limitations records the gap. ## Storage @@ -399,16 +353,11 @@ CREATE INDEX idx_user_consents_documents ON user_consents USING GIN (documents jsonb_path_ops); ``` -`documents` holds one object per accepted document, copied from config at write time: +`documents` holds one object per accepted document, copied from config at write time, with the same +four fields config holds: ```json [ - { - "id": "terms_of_service", - "title": "Terms & Conditions", - "version": "2026-04-01", - "url": "https://example.org/legal/terms/2026-04-01" - }, { "id": "privacy_policy", "title": "Privacy Policy", @@ -418,42 +367,33 @@ CREATE INDEX idx_user_consents_documents ] ``` -Four fields per document, the same four config holds. A field added later appears on new records -only, since old ones keep the shape they were written with. - -`metadata` is written once at insert like every other column and is empty today. It gives a later -re-consent somewhere to record its own context without a migration. Nothing reads it. - The grain is the consent, not the document. A user accepts a set in one act, so `user_email`, `ip_address` and `consented_at` describe the act and are stored once, and the document list is an -argument to the write rather than something the schema fixes, which is what covers a later -re-consent for any subset. Alternative 3 has the tradeoff. - -Four things in there are deliberate and look like mistakes otherwise. - -There is no foreign key to `users`. `UserRepository.Delete` does a hard `DELETE`, so -`ON DELETE CASCADE` would drop the consent records and `ON DELETE RESTRICT` would block account -deletion. Records have to survive the user, which is also why `user_email` is denormalized: once -the user row is gone there is nothing left to join to. `ip_address` is `TEXT` and nullable, not -`INET`, because the value comes from a request header and a bad or absent one must not fail a -signup. - -The document versions and URLs are copies, not references. A consent record has to stay readable -years later and stay correct after the document is dropped from config. It is also why a document -table can be added later with no backfill: no consent record points at one. - -The unique index means a user gets at most one signup consent. Since nothing repairs a record, a -second signup write for the same user is a bug, and this makes it fail instead of leaving two rows -disagreeing about what happened. - -Consent records cannot be changed. A `BEFORE UPDATE` and a `BEFORE DELETE` trigger both raise -`45000`, following `20250904105226_add_audit_records_immutability.up.sql`, which does the same for -`audit_records`. That migration guards only `UPDATE`; `DELETE` is guarded here too, because a -deleted consent record leaves a user who looks like they never consented. The repository has -`Create` and nothing else, so no admin API path reaches a record even if a trigger gets dropped. -`DROP TABLE` still works, so `migrate down` is fine. - -A signup also writes one audit record so the audit trail shows it happened. `pkg/auditrecord` gains +argument to the write rather than something the schema fixes. Alternative 3 has the tradeoff. +`metadata` is empty today; it gives a later re-consent somewhere to record its context without a +migration. + +Four choices are deliberate and look like mistakes otherwise. + +No foreign key to `users`. `UserRepository.Delete` does a hard `DELETE`, so `ON DELETE CASCADE` +would drop the records and `ON DELETE RESTRICT` would block account deletion. Records must outlive +the user, which is also why `user_email` is denormalized. `ip_address` is `TEXT` and nullable, not +`INET`: the value comes from a request header, and a bad or missing one must not fail a signup. + +Versions and URLs are copies, not references, so a record stays readable years later and stays +correct after the document leaves config. It also means a document table can be added later with no +backfill. + +The unique index gives a user at most one signup consent. Nothing repairs a record, so a second +signup write is a bug, and this makes it fail rather than leave two rows disagreeing. + +Records cannot be changed. `BEFORE UPDATE` and `BEFORE DELETE` triggers both raise `45000`, +following `20250904105226_add_audit_records_immutability.up.sql`. That migration guards `UPDATE` +only; `DELETE` is guarded here too, because a deleted record leaves a user who looks like they never +consented. The repository has `Create` and nothing else. `DROP TABLE` still works, so `migrate down` +is fine. + +A signup also writes one audit record. `pkg/auditrecord` gains `UserConsentGrantedEvent Event = "user.consent_granted"` and `ConsentType EntityType = "consent"`, following the `entity.verb` naming already there. Every field is set explicitly: @@ -465,24 +405,22 @@ following the `entity.verb` naming already there. Every field is set explicitly: | `OccurredAt` | `consented_at` from the flow, not the write time | | `OrgID`, `IdempotencyKey` | empty. Both are nullable, and a signup has no org | -`Actor` cannot be left to enrichment, and this is the part that looks fine and is not. -`AuditRecordRepository.Create` calls `enrichActorFromContext` when the actor is empty, and nothing -puts an actor in the context of a skip-listed endpoint, so the record would land with `uuid.Nil` and -the `system` actor for an act a person performed. Going through `auditrecord.Service.Create` -instead is worse: its `enrichUserActor` reads `Actor.ID` as a session id and resolves the user from -`session.UserID`, and no session exists yet, so it returns `ErrActorNotFound`. So the write goes -through the repository with the actor filled in, the way `userpat` writes its PAT events. -`actor_id` is `UUID NOT NULL` and the user id exists by then, because the row is already committed. +`Actor` cannot be left to enrichment. `AuditRecordRepository.Create` calls `enrichActorFromContext` +when the actor is empty, and a skip-listed endpoint has no actor in context, so the record would +land with `uuid.Nil` and the `system` actor for an act a person performed. +`auditrecord.Service.Create` is worse: its `enrichUserActor` reads `Actor.ID` as a session id, and +no session exists yet, so it returns `ErrActorNotFound`. So the write goes through the repository +with the actor filled in, as `userpat` does for its PAT events. -The write happens after the transaction commits, since `Create` has no `*sqlx.Tx` variant, so it -cannot be atomic with the consent record. That is why the consent record is the source of truth: if -the audit write fails, log it and carry on. +That write happens after the transaction commits, since `Create` has no `*sqlx.Tx` variant. It +cannot be atomic with the consent record, which is why that record is the source of truth: if the +audit write fails, log it and carry on. ## Reading it back -There is no read API and no view. A reporting tool points at `user_consents` and reads the rows as -they are. A record is self-contained, so "what did this user accept, and when" is one row. The -reverse, "who accepted privacy policy 2026-04-01", is a containment filter: +No read API and no view. A reporting tool reads `user_consents` as it is. A record is +self-contained, so "what did this user accept, and when" is one row. The reverse, "who accepted +privacy policy 2026-04-01", is a containment filter served by the GIN index: ```sql SELECT user_email, consented_at, ip_address @@ -490,16 +428,13 @@ FROM user_consents WHERE documents @> '[{"id": "privacy_policy", "version": "2026-04-01"}]'; ``` -`@>` is what the GIN `jsonb_path_ops` index on `documents` serves, so that filter uses the index. A -view would be one more object to keep in step with the table it summarizes. - ## Client -Three files under `web/sdk/client/views/auth`, which are the only callers of `authenticate` in the -repo. `sign-up/sign-up-view.tsx` sends signup on the OIDC buttons, `sign-in/sign-in-view.tsx` sends -login, and `magic-link/magic-link-view.tsx` is shared by both, so it takes the intent as a prop. +Three files under `web/sdk/client/views/auth`, the only callers of `authenticate` in the repo. +`sign-up/sign-up-view.tsx` sends signup on the OIDC buttons, `sign-in/sign-in-view.tsx` sends login, +and `magic-link/magic-link-view.tsx` is shared by both, so it takes the intent as a prop. -Neither sends document ids, so with consent enabled the shipped sign-up view cannot complete a +Neither view sends document ids, so with consent enabled the shipped sign-up view cannot complete a signup. It gets an Apsara `Checkbox`: ```tsx @@ -511,76 +446,68 @@ export type SignUpViewProps = /* ... */ & { }; ``` -The prop is optional and the checkbox renders only when it is passed, so a deployment with consent -disabled sees the view it sees today. When it is passed the checkbox starts unchecked, every -sign-up control stays disabled until it is checked, and `documentIds` goes out as -`accepted_document_ids`. `MagicLinkView` takes the ids alongside the intent, so both strategies go -through one control. +The prop is optional, so a deployment with consent disabled sees today's view. When it is passed, +the checkbox starts unchecked, every sign-up control stays disabled until it is checked, and +`documentIds` goes out as `accepted_document_ids`. `MagicLinkView` takes the ids alongside the +intent, so both strategies use one control. -`label` takes a `ReactNode` so the consumer supplies the copy and the links. The default is plain -text without links, since the SDK does not know the documents or their URLs, and `documentIds` -comes from the consumer for the same reason. It is the duplication Limitations describes, now -visible in a prop. A second checkbox or a per-document link is a consumer rendering its own view -and calling `authenticate` directly, which already works today. +`label` takes a `ReactNode`, so the consumer supplies the copy and the links. The default is plain +text without links, because the SDK does not know the documents or their URLs, and `documentIds` +comes from the consumer for the same reason. A second checkbox or a per-document link means a +consumer rendering its own view and calling `authenticate` directly, which already works. `magicLinkHandler` handles only `status === 400` today and writes the message into the email field. -It needs the three new codes, with copy that points at the other view for the two gate errors; +It needs the three new codes, with copy pointing at the other view for the two gate errors. `config.redirectSignup` and `config.redirectLogin` already exist for the links. -The OIDC rejections arrive at `AuthCallback`, which is the callback page rather than the view that -started the flow, and that page has no error UI. Redirecting back to the originating view with an -error param is the smaller change, rendering it in place is the other option, and this is undecided. +OIDC rejections arrive at `AuthCallback`, which has no error UI. Redirecting back to the originating +view with an error param is the smaller change, rendering it in place is the other option, and this +is undecided. ## Alternatives considered -1. Consent in `users.metadata`. Both update paths replace the whole map, so a user can delete - their own consent record, and it goes away with the user row. +1. Consent in `users.metadata`. Both update paths replace the whole map, so a user can delete their + own consent record, and it goes away with the user row. 2. Consent in `audit_records`. `CreateAuditRecord` takes any event string with a client-supplied - `occurred_at`, gated on platform `check`, which both the admin and member relations grant. Any + `occurred_at`, gated on platform `check`, which both the admin and member relations grant, so any platform member could insert a backdated consent record. A table with no write RPC can only be written by the signup path. -3. One row per accepted document instead of one per consent. It puts every field in a column, but - it repeats the email, IP and timestamp on every row, and it needs a synthetic event id to answer - "what did this user accept in one sitting" once there is more than one occasion. The act is what - is being recorded, so the act is the row. +3. One row per accepted document instead of one per consent. It puts every field in a column, but it + repeats the email, IP and timestamp on every row, and it needs a synthetic event id to answer + "what did this user accept in one sitting". The act is what is recorded, so the act is the row. -4. The documents in the database instead of config, whether as a plain `consent_documents` table, - a `ConsentDocument` reconcile kind or an RPC serving the list. Each consent record already - copies its document versions, so the records are the version history and a table answers no - query they cannot. Config sits in git, which is a better change log than rows an admin can - edit, and any of the three needs a write path, which is one more way to change what the server - stamps. A deployment that wants one source of truth can generate both the config block and the - client's copy from a single file. Future work has the conditions under which this is revisited. +4. The documents in the database instead of config, as a table, a reconcile kind or an RPC serving + the list. Each record already copies its document versions, so the records are the version + history and a table answers no query they cannot. Config sits in git, which is a better change + log than rows an admin can edit, and all three need a write path, which is one more way to change + what the server stamps. Future work has the conditions for revisiting this. -5. Taking consent in `AuthCallback`. The client would have to stash it locally and resend it after - the redirect, so the consent record would attest to a client re-assertion made after the fact, - the IP would be the post-redirect one, and it would break when the provider comes back into a - different tab. +5. Taking consent in `AuthCallback`. The client would stash it locally and resend it after the + redirect, so the record would attest to a re-assertion made after the fact, the IP would be the + post-redirect one, and it would break when the provider returns into a different tab. 6. Repairing a missing consent record on a later login. See Enforcement. 7. Two RPCs, `Login` and `Signup`, instead of the intent. `AuthCallback` cannot be split alongside them: `state` is the flow id, both gates run at callback time, and two callback URLs would have - to be registered with every provider. The intent would still have to ride on the flow, so the - split duplicates the entry point without moving either gate, and leaves `Authenticate` as a - permanent ungated path since no existing caller can be broken. Frontier already discriminates - strategies with `strategy_name` on one RPC. - -8. A `oneof` carrying a `LoginIntent` and a `SignupIntent` message, with the accepted ids on the - signup arm only. It makes a signup-only field unrepresentable on a login rather than merely - rejected, and there is precedent in `ChangeSubscriptionRequest.Change`. But - `AuthenticateRequest.email` is already a field only some strategies use, checked at runtime, so - the flat field is the shape this message has. Moving later means deprecating field 6 and - carrying both for a window, so adopt it now if more signup-only fields are expected. - -9. Enforcing the login gate only at user creation, or accepting the flow and failing at - verification without sending anything. Both keep `Authenticate` quiet about whether an address - has an account. The first costs a wasted OTP mail and puts "no account for this email" on the - verification screen, where it reads like a bad code; the second wastes nothing but leaves the - user waiting for a code that never arrives. The clearer error was chosen over the quieter - endpoint. + to be registered with every provider. The intent would still ride on the flow, so the split + duplicates the entry point without moving either gate, and leaves `Authenticate` permanently + ungated. Frontier already discriminates strategies with `strategy_name` on one RPC. + +8. A `oneof` carrying a `LoginIntent` and a `SignupIntent` message, with the ids on the signup arm + only. It makes a signup-only field unrepresentable on a login rather than merely rejected, and + there is precedent in `ChangeSubscriptionRequest.Change`. But `AuthenticateRequest.email` is + already a field only some strategies use, checked at runtime, so the flat field is the shape this + message has. Moving later means deprecating field 6 and carrying both for a window, so adopt it + now if more signup-only fields are expected. + +9. Enforcing the login gate only at user creation, or failing at verification without sending + anything. Both keep `Authenticate` quiet about whether an address has an account. The first + wastes an OTP mail and puts "no account for this email" on the verification screen, where it + reads like a bad code. The second leaves the user waiting for a code that never arrives. The + clearer error was chosen over the quieter endpoint. 10. A first-class `Intent` field on `Flow`. It types better than metadata and costs a migration on `flows` for one string, when the consent payload has to go in `metadata` regardless. @@ -589,83 +516,79 @@ error param is the smaller change, rendering it in place is the other option, an Changing a document version needs a redeploy, since config is read at boot. -The client's document list and the server's are declared separately and can go out of sync. Ids -catch a set mismatch. A version mismatch is undetectable, since the client sends no versions: a -client showing version A against a config holding version B produces a record that says B. Only -frontier serving the list would close that. +The client's document list and the server's are declared separately and can drift. Ids catch a set +mismatch, but a version mismatch cannot be caught, since the client sends no versions: a client +showing version A against a config holding version B produces a record that says B. Only frontier +serving the list would close that. -A record ties to a version string, not to the document text. Editing the file at a URL without +A record ties to a version string, not to the document text, so editing the file at a URL without bumping the version leaves every record for that version describing something the user did not see. -A per-document hash would close it and can be added later, since a document object is just JSON. +A per-document hash would close this and can be added later. The IP is only as good as the header it comes from. A proxy that appends to `X-Forwarded-For` instead of overwriting it leaves the value under the caller's control, and a deployment that does -not set the header at all gets records with no IP. +not set the header gets records with no IP. -Users who already exist get no consent record, and nothing will ever give them one, so you never -get to full coverage. Neither do users created through the three exempt paths under Enforcement. -"No account without consent" holds for the signup flow, not for every row in `users`, and a -deployment has to accept that distinction before it relies on the records. +Users who already exist get no consent record, and nothing will ever give them one, so coverage is +never complete. Neither do users from the three exempt paths. "No account without consent" holds for +the signup flow, not for every row in `users`. The unauthenticated `Authenticate` endpoint now answers whether an address has an account, for -mailotp and passkey where the email is known at flow start. Frontier does not answer that today, -because it auto-provisions instead. Rate limiting per address and per IP is the mitigation, not -hiding the answer. Passkey already leaks existence through its response shape, since register and -login return different options, and the intent neither widens that nor closes it. +mailotp and passkey where the email is known at flow start. Rate limiting per address and per IP is +the mitigation, not hiding the answer. Passkey already leaks existence through its response shape, +and the intent neither widens nor closes that. -The login gate is a UX boundary and not a security one: an unset intent keeps create-or-get, so any -client can opt out by omitting the field. Consent cannot be opted out that way, since `ResolveAll` -runs at user creation under every intent. +The login gate is a UX boundary, not a security one. An unset intent keeps create-or-get, so any +client can opt out by omitting the field. Consent cannot be opted out of that way, since +`ResolveAll` runs at user creation under every intent. -Sharing a transaction with the user insert means the user repository gains a create that takes a -transaction. It is additive, but it is the one place this feature reaches outside its own domain. +The user repository gains a create that takes a transaction. It is additive, but it is the one place +this feature reaches outside its own domain. ## Future work Re-consent when a document version changes. The write path already handles it: `Grant` takes the -document list as an argument and `source` separates one occasion from another. What is missing is -enforcement, which has to move from user creation to a gate on authenticated requests, roughly -what Keycloak's terms and conditions required action does. That is its own feature. +document list as an argument, and `source` separates one occasion from another. What is missing is +enforcement, which has to move from user creation to a gate on authenticated requests, roughly what +Keycloak's terms and conditions required action does. -A server switch that rejects an unset intent, which turns the login gate into a boundary. It needs -a deprecation window first, since it breaks every client that has not moved. +A server switch that rejects an unset intent, turning the login gate into a boundary. It needs a +deprecation window first, since it breaks every client that has not moved. -A document list endpoint, if clients keeping their own copy turns out to be too fragile. -`ListAuthStrategies` is unauthenticated and already fetched by the sign-in page, so the resolved -document set could ride along on it instead of needing a new RPC, and the SDK sign-up view could -render the documents rather than be handed their ids. +A document list endpoint, if clients keeping their own copy proves too fragile. `ListAuthStrategies` +is unauthenticated and already fetched by the sign-in page, so the resolved set could ride along on +it instead of needing a new RPC. -A per-document hash in config, copied into the record, to tie a consent to the document text and -not to a version string someone typed into config. +A per-document hash in config, copied into the record, tying a consent to the document text rather +than to a version string someone typed into config. -A `ConsentDocument` reconcile kind, if restarts become a problem. It needs list and write RPCs and -a `consent_documents` table keyed by `(document_id, version)`. The config map maps onto that -directly and nothing needs backfilling, since no consent record points at it. +A `ConsentDocument` reconcile kind, if restarts become a problem. It needs list and write RPCs and a +`consent_documents` table keyed by `(document_id, version)`, which the config map maps onto +directly. Withdrawal, which needs a decision on what happens to the account. A reserved-event guard on `CreateAuditRecord`, so events frontier writes itself cannot be injected -through the public RPC. Any platform member can forge a backdated `user.consent_granted` through -it, the same way they can forge `pat.revoked`. That does not weaken `user_consents`, which has no -write RPC, but it does mean the audit record is a breadcrumb and not evidence. +through the public RPC. Any platform member can forge a backdated `user.consent_granted` through it, +the same way they can forge `pat.revoked`. That does not weaken `user_consents`, which has no write +RPC, but it does mean the audit record is a breadcrumb and not evidence. ## Work in order 1. proton PR adding `FlowIntent`, `flow_intent = 6` and `accepted_document_ids = 7` in one change. 2. Bump `PROTON_COMMIT` in `Makefile:7` and run `make proto`. -3. Core, the login gate first: the `FlowIntent` type, the request fields, the metadata write and - the nil-safe accessors, the `StartFlow` checks, the passkey branch, and the `getOrCreateUser` +3. Core, the login gate first: the `FlowIntent` type, the request fields, the metadata write and the + nil-safe accessors, the `StartFlow` checks, the passkey branch, and the `getOrCreateUser` signature. 4. Core, consent: `app.consent` with boot validation, the consent service, the migration and repository, and the transactional write in `getOrCreateUser`. 5. Handlers: the intent and ids into `StartFlow`, `ExtractSessionMetadata` in `Authenticate`, and the three errors mapped in both `Authenticate` and `AuthCallback`. 6. SDK: the intent and ids through `MagicLinkView`, both views, the checkbox, and the error copy. -7. Tests. `core/authenticate/service_test.go` covers three intents against an address that does and - does not have an account, at both enforcement points, plus one case per strategy, since OIDC, - mail OTP and both passkey methods reach `getOrCreateUser` by different routes. Consent adds the - complete, incomplete and unknown-id sets, and a consent insert failure rolling back the user - row. +7. Tests. Three intents against an address that does and does not have an account, at both + enforcement points, plus one case per strategy, since OIDC, mail OTP and both passkey methods + reach `getOrCreateUser` by different routes. Consent adds the complete, incomplete and + unknown-id sets, and a consent insert failure rolling back the user row. ## References From 249af92726e330d9f0c387e8d088180070dd07ee Mon Sep 17 00:00:00 2001 From: Rohan Chakraborty Date: Fri, 28 Aug 2026 01:43:57 +0530 Subject: [PATCH 7/7] chore: update rfc comments --- docs/rfcs/0002-explicit-consent-at-signup.md | 193 ++++++++++++------- 1 file changed, 127 insertions(+), 66 deletions(-) diff --git a/docs/rfcs/0002-explicit-consent-at-signup.md b/docs/rfcs/0002-explicit-consent-at-signup.md index a1f2c1db50..6d0b7334e7 100644 --- a/docs/rfcs/0002-explicit-consent-at-signup.md +++ b/docs/rfcs/0002-explicit-consent-at-signup.md @@ -12,10 +12,10 @@ Frontier can require a user to accept a set of documents before their account is created, and store one consent record for it. -A deployment lists its documents in server config with an id, title, version and URL. The client -sends the ids the user accepted. Frontier checks that the ids cover every document in config, then -creates the user and the consent record in one transaction. The record copies each document's -version and URL from config. +A deployment lists its documents in server config with an id, title, version and URL. A public +endpoint serves that list, and the client sends back the ids the user accepted. Frontier checks that +the ids cover every document in config, then creates the user and the consent record in one +transaction. The record copies each document's version and URL from config. Consent applies to signups only, and frontier cannot tell a signup from a login today. So this RFC also adds a flow intent, which is what lets the consent check run before the browser leaves for the @@ -49,9 +49,9 @@ documents. - The client sends consent explicitly. Signup fails without it. - No API can update or delete a consent record. Deleting the user does not delete it. - A record stores the version and URL of every document it covers. -- Document ids, titles, versions and URLs come from config. +- Document ids, titles, versions and URLs come from config, and the client reads them from frontier. - A login never creates an account. A signup never logs an existing user in. -- One RPC and one callback, unchanged. +- The auth flow stays one RPC and one callback, plus one read-only endpoint for the list. - Deployments without `app.consent`, and clients without an intent, behave as before. ## Non-goals @@ -65,8 +65,8 @@ record is written at user creation and nowhere else. Making the login gate a security boundary, or hiding whether an address has an account. Limitations says what both cost. -Serving the document text or the document list. Clients keep their own copy. The SDK sign-up view -gets a checkbox, but the copy and the links are the consumer's. +Serving the document text. The list endpoint returns URLs, and frontier never reads what is behind +them. ## Document config @@ -94,9 +94,9 @@ A map, not a list: it matches `authenticate.Config` keying `oidc_config` by stra enforces unique ids, and a single field stays env-overridable. Every document in the map is required at signup, so there is no per-document `required` flag. An -optional document would need withdrawal, which is out of scope. The cost is that adding a document -id breaks signup for clients still sending the old list, so a new document ships with the client -release that sends it. Version bumps are safe, since the client sends only ids. +optional document would need withdrawal, which is out of scope. Adding a document id breaks signup +for a client sending a hardcoded list, which is why the list is served over an endpoint. Version +bumps are safe either way, since the client sends only ids. `version` is opaque. Frontier compares it for equality, so dates, semver or commit SHAs all work. @@ -111,6 +111,44 @@ not the config repo, says what a deployment was serving. Bad config fails at boot: ids, versions and URLs must be non-empty, URLs must parse, and an enabled block needs at least one document. +## The document list endpoint + +`ListConsentDocuments`, unauthenticated, so a sign-up view can render the documents it is asking the +user to accept: + +```proto +message ConsentDocument { + string id = 1; + string title = 2; + string version = 3; + string url = 4; +} + +message ListConsentDocumentsRequest {} + +message ListConsentDocumentsResponse { + repeated ConsentDocument documents = 1; +} +``` + +It returns the resolved config set, all four fields per document, ordered by id so the response is +stable. + +With `app.consent` disabled it returns an empty list rather than an error, so one client build works +against both kinds of deployment: no documents means no checkbox. + +The handler mirrors `ListAuthStrategies` (`internal/api/v1beta1connect/authenticate.go:302`). It +reads the consent service, touches no database, and joins `authenticationSkipList` beside +`ListAuthStrategies` and `Authenticate`. + +Public, because the documents are already public: the URLs are meant to be read by anyone +considering an account, and the ids are an input to an unauthenticated `Authenticate`. Requiring a +session to learn what to accept before the account exists is a cycle. + +Not folded into `ListAuthStrategies`. Consent is not a strategy, and `AuthStrategy` carries `name` +and `params` and nothing else, so the documents would go in a `params` map every client has to +parse. Two thin endpoints beat one that means two things. + ## The request fields Two new fields on `AuthenticateRequest`, authored in `raystack/proton` and generated here through @@ -135,11 +173,12 @@ The ids accompany `FLOW_INTENT_SIGNUP` only. With a login intent they are a clie handler rejects them, because a login writes no record. Accepting them silently would leave a client believing it recorded a consent that does not exist. -Ids rather than one boolean, because the client's copy of the list can drift from config. Ids expose -the mismatch; a boolean would stamp whatever config holds, writing a record that says the user -accepted a document they never saw. Ids also allow consenting to a subset later. Versions, titles -and URLs all come from config, so a client sending them would be ignored. Duplicates are removed -before the check. +Ids rather than one boolean, because the list the client rendered can still differ from config: it +may have been fetched before a restart, or hardcoded by a consumer rendering its own view. Ids +expose the mismatch; a boolean would stamp whatever config holds, writing a record that says the +user accepted a document they never saw. Ids also allow consenting to a subset later. Versions, +titles and URLs all come from config, so a client sending them would be ignored. Duplicates are +removed before the check. ## Carrying them across the redirect @@ -231,10 +270,14 @@ passkey login can create an account. The intent replaces the guess: signup picks ### Consent -The consent service owns the config, so it owns the checks. Three functions, because signup and any -later use need different rules: +The consent service owns the config, so it owns the checks. Four functions, because signup, the list +endpoint and any later use need different rules: ```go +// Documents returns every document in config, ordered by id. +// Empty when the feature is disabled. Serves ListConsentDocuments. +func (s Service) Documents() []Document + // Resolve maps ids to their config snapshots. Rejects unknown ids. // Says nothing about whether the set is complete. func (s Service) Resolve(ids []string) ([]Document, error) @@ -330,16 +373,15 @@ One consent record per consent, listing the documents it covers. ```sql CREATE TABLE user_consents ( - id UUID PRIMARY KEY DEFAULT uuid_generate_v7(), - user_id UUID NOT NULL, - user_email TEXT NOT NULL, - documents JSONB NOT NULL, - source TEXT NOT NULL DEFAULT 'signup', - auth_method TEXT, - ip_address TEXT, - consented_at TIMESTAMPTZ NOT NULL, - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), - metadata JSONB NOT NULL DEFAULT '{}', + id UUID PRIMARY KEY DEFAULT uuid_generate_v7(), + user_id UUID NOT NULL, + user_email TEXT NOT NULL, + documents JSONB NOT NULL, + source TEXT NOT NULL DEFAULT 'signup', + auth_strategy TEXT, + ip_address TEXT, + consented_at TIMESTAMPTZ NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), CONSTRAINT documents_not_empty CHECK ( jsonb_typeof(documents) = 'array' AND jsonb_array_length(documents) > 0 @@ -348,9 +390,6 @@ CREATE TABLE user_consents ( CREATE UNIQUE INDEX uq_user_consents_signup ON user_consents(user_id) WHERE source = 'signup'; - -CREATE INDEX idx_user_consents_documents - ON user_consents USING GIN (documents jsonb_path_ops); ``` `documents` holds one object per accepted document, copied from config at write time, with the same @@ -370,8 +409,11 @@ four fields config holds: The grain is the consent, not the document. A user accepts a set in one act, so `user_email`, `ip_address` and `consented_at` describe the act and are stored once, and the document list is an argument to the write rather than something the schema fixes. Alternative 3 has the tradeoff. -`metadata` is empty today; it gives a later re-consent somewhere to record its context without a -migration. +`auth_strategy` holds the `strategy_name` the consent came through: `oidc`, `mailotp` or `passkey`, +the flow's own word for it. + +No `metadata` column. Nothing would write it today, and a re-consent that needs one can add it in +its own migration. Four choices are deliberate and look like mistakes otherwise. @@ -403,7 +445,8 @@ following the `entity.verb` naming already there. Every field is set explicitly: | `Resource` | the same user | | `Target` | the consent record id, `consent` type, document ids and versions in `Metadata` | | `OccurredAt` | `consented_at` from the flow, not the write time | -| `OrgID`, `IdempotencyKey` | empty. Both are nullable, and a signup has no org | +| `OrgID` | `schema.PlatformOrgID` | +| `IdempotencyKey` | empty. It is nullable, and there is nothing to deduplicate | `Actor` cannot be left to enrichment. `AuditRecordRepository.Create` calls `enrichActorFromContext` when the actor is empty, and a skip-listed endpoint has no actor in context, so the record would @@ -412,6 +455,10 @@ land with `uuid.Nil` and the `system` actor for an act a person performed. no session exists yet, so it returns `ErrActorNotFound`. So the write goes through the repository with the actor filled in, as `userpat` does for its PAT events. +`OrgID` is `schema.PlatformOrgID`, the nil UUID, rather than empty. `user.Service` and +`userpat.Service` already stamp it on the platform-level events they write, and it saves every +reader special-casing a blank org. + That write happens after the transaction commits, since `Create` has no `*sqlx.Tx` variant. It cannot be atomic with the consent record, which is why that record is the source of truth: if the audit write fails, log it and carry on. @@ -419,8 +466,10 @@ audit write fails, log it and carry on. ## Reading it back No read API and no view. A reporting tool reads `user_consents` as it is. A record is -self-contained, so "what did this user accept, and when" is one row. The reverse, "who accepted -privacy policy 2026-04-01", is a containment filter served by the GIN index: +self-contained, so "what did this user accept, and when" is one row, found by `user_id`. + +There is no index on `documents`. The reverse question, "who accepted privacy policy 2026-04-01", is +a containment filter, answerable as a scan: ```sql SELECT user_email, consented_at, ip_address @@ -428,6 +477,9 @@ FROM user_consents WHERE documents @> '[{"id": "privacy_policy", "version": "2026-04-01"}]'; ``` +That is an occasional compliance query, not a request path, and a GIN index would be maintained on +every insert to serve it. If it becomes a real access pattern, the index is one migration away. + ## Client Three files under `web/sdk/client/views/auth`, the only callers of `authenticate` in the repo. @@ -435,26 +487,33 @@ Three files under `web/sdk/client/views/auth`, the only callers of `authenticate and `magic-link/magic-link-view.tsx` is shared by both, so it takes the intent as a prop. Neither view sends document ids, so with consent enabled the shipped sign-up view cannot complete a -signup. It gets an Apsara `Checkbox`: +signup. `SignUpView` gains a `listConsentDocuments` query beside the `listAuthStrategies` one it +already runs, and an Apsara `Checkbox` fed from the response: ```tsx export type SignUpViewProps = /* ... */ & { - consent?: { - documentIds: string[]; - label?: ReactNode; - }; + consentLabel?: ReactNode | ((documents: ConsentDocument[]) => ReactNode); }; ``` -The prop is optional, so a deployment with consent disabled sees today's view. When it is passed, -the checkbox starts unchecked, every sign-up control stays disabled until it is checked, and -`documentIds` goes out as `accepted_document_ids`. `MagicLinkView` takes the ids alongside the +An empty list means no checkbox, so a deployment with consent disabled sees today's view. With +documents, the checkbox starts unchecked, every sign-up control stays disabled until it is checked, +and the fetched ids go out as `accepted_document_ids`. `MagicLinkView` takes the ids alongside the intent, so both strategies use one control. -`label` takes a `ReactNode`, so the consumer supplies the copy and the links. The default is plain -text without links, because the SDK does not know the documents or their URLs, and `documentIds` -comes from the consumer for the same reason. A second checkbox or a per-document link means a -consumer rendering its own view and calling `authenticate` directly, which already works. +The default label is built from the response: the copy around it is the SDK's, and each document +contributes an Apsara `Link` to its `url`, titled with its `title`. Changing the documents in config +changes the label, with no client release. + +`consentLabel` overrides it: a `ReactNode` for static copy, or a function of the documents for copy +that links them. `ReactNode` does not admit functions, so one `typeof === 'function'` check tells +the two apart, and both resolve to a node before the `Checkbox` sees it. + +A second checkbox, or one per document, means a consumer rendering its own view and calling +`authenticate` directly, which already works. + +The ids the view sends are the ids the server just gave it, so a set mismatch takes a config change +between the two calls. Nothing in the SDK hardcodes a document. `magicLinkHandler` handles only `status === 400` today and writes the message into the email field. It needs the three new codes, with copy pointing at the other view for the two gate errors. @@ -478,11 +537,12 @@ is undecided. repeats the email, IP and timestamp on every row, and it needs a synthetic event id to answer "what did this user accept in one sitting". The act is what is recorded, so the act is the row. -4. The documents in the database instead of config, as a table, a reconcile kind or an RPC serving - the list. Each record already copies its document versions, so the records are the version - history and a table answers no query they cannot. Config sits in git, which is a better change - log than rows an admin can edit, and all three need a write path, which is one more way to change - what the server stamps. Future work has the conditions for revisiting this. +4. The documents in the database instead of config, as a table or a reconcile kind. Each record + already copies its document versions, so the records are the version history and a table answers + no query they cannot. Config sits in git, which is a better change log than rows an admin can + edit, and both need a write path, which is one more way to change what the server stamps. + `ListConsentDocuments` serves the list from config, so storage is a separate question. Future + work has the conditions for revisiting it. 5. Taking consent in `AuthCallback`. The client would stash it locally and resend it after the redirect, so the record would attest to a re-assertion made after the fact, the IP would be the @@ -516,10 +576,11 @@ is undecided. Changing a document version needs a redeploy, since config is read at boot. -The client's document list and the server's are declared separately and can drift. Ids catch a set -mismatch, but a version mismatch cannot be caught, since the client sends no versions: a client -showing version A against a config holding version B produces a record that says B. Only frontier -serving the list would close that. +Drift between what the user read and what the record says is narrowed, not closed. +`ListConsentDocuments` and `Authenticate` are two calls, so a config change between them leaves a +user who read version A with a record that says version B. The window is a page load rather than a +release cycle. Ids catch a set mismatch; a version mismatch cannot be caught, since the client sends +no versions. A record ties to a version string, not to the document text, so editing the file at a URL without bumping the version leaves every record for that version describing something the user did not see. @@ -555,10 +616,6 @@ Keycloak's terms and conditions required action does. A server switch that rejects an unset intent, turning the login gate into a boundary. It needs a deprecation window first, since it breaks every client that has not moved. -A document list endpoint, if clients keeping their own copy proves too fragile. `ListAuthStrategies` -is unauthenticated and already fetched by the sign-in page, so the resolved set could ride along on -it instead of needing a new RPC. - A per-document hash in config, copied into the record, tying a consent to the document text rather than to a version string someone typed into config. @@ -575,20 +632,24 @@ RPC, but it does mean the audit record is a breadcrumb and not evidence. ## Work in order -1. proton PR adding `FlowIntent`, `flow_intent = 6` and `accepted_document_ids = 7` in one change. +1. proton PR, one change: `FlowIntent`, `flow_intent = 6` and `accepted_document_ids = 7`, plus + `ListConsentDocuments` with its request, response and `ConsentDocument` messages. 2. Bump `PROTON_COMMIT` in `Makefile:7` and run `make proto`. 3. Core, the login gate first: the `FlowIntent` type, the request fields, the metadata write and the nil-safe accessors, the `StartFlow` checks, the passkey branch, and the `getOrCreateUser` signature. 4. Core, consent: `app.consent` with boot validation, the consent service, the migration and repository, and the transactional write in `getOrCreateUser`. -5. Handlers: the intent and ids into `StartFlow`, `ExtractSessionMetadata` in `Authenticate`, and - the three errors mapped in both `Authenticate` and `AuthCallback`. -6. SDK: the intent and ids through `MagicLinkView`, both views, the checkbox, and the error copy. +5. Handlers: the intent and ids into `StartFlow`, `ExtractSessionMetadata` in `Authenticate`, the + three errors mapped in both `Authenticate` and `AuthCallback`, and the `ListConsentDocuments` + handler with its skip-list entry. +6. SDK: the intent and ids through `MagicLinkView`, both views, the documents query, the checkbox, + and the error copy. 7. Tests. Three intents against an address that does and does not have an account, at both enforcement points, plus one case per strategy, since OIDC, mail OTP and both passkey methods reach `getOrCreateUser` by different routes. Consent adds the complete, incomplete and - unknown-id sets, and a consent insert failure rolling back the user row. + unknown-id sets, a consent insert failure rolling back the user row, and `ListConsentDocuments` + enabled and disabled. ## References