fix(auth): refuse aal1 tokens from users with a verified MFA factor

[JOY (JOY)]

Problem

A user with a verified TOTP factor could still use an aal1 access token (password only, or a recovery link) to call /api directly: every query and file, and R1 and R2 commands (upload, edit, reject, session revoke). Only R3 asked for aal2 (P11).

Design

• Self-host (AUTH_SESSION_CHECK=db). crove_kernel.session_check now takes the token's assurance level (p_aal, 5 arguments; the 4-argument version is dropped so no caller can skip the check). After the upstream session check passes, an aal1 token whose user has a verified factor raises 28000 MFA_REQUIRED. The kernel answers it as 401 MFA_REQUIRED. The check runs in the request transaction before any write, so reads, R1 and R2 commands, uploads, downloads and org-less calls (me/get, org/create) are all refused, and nothing is written.
• Factor reader. crove_kernel.auth_user_has_verified_factor(uuid) is a SECURITY DEFINER owned by crove_auth_reader, like auth_session_valid. It has no EXECUTE for crove_app or PUBLIC and pins search_path. Any verified factor counts, of any type.
• auth-grants.
  • It grants exactly auth.mfa_factors (user_id, status) plus a SELECT policy (Supabase Auth enables RLS on the table). The factor secrets are never granted.
  • auth_grants_ready() requires that grant and policy whenever the table exists. A missing grant or policy therefore fails closed with 503 AUTH_NOT_READY, as on the existing path.
  • If the table is absent, the check treats the user as having no factor, and auth-grants logs a warning. Rationale: Supabase Auth creates auth.mfa_factors in a migration that runs before the one adding auth.sessions.not_after, which auth-grants already waits for. I checked this against the auth.schema_migrations order of the pinned supabase/gotrue:v2.197.0. A Supabase Auth without the table has no MFA at all.
  • This branch also keeps the base-branch invariant copy green: its mirror of the auth schema has no mfa_factors.
• Order of checks. A signed-out, expired, banned or deleted session still gets 401 UNAUTHENTICATED, and the SPA signs it out. Only a live session that has not passed MFA gets MFA_REQUIRED.
• SPA.
  • isUnauthenticated() no longer treats MFA_REQUIRED as a sign-out, and queries do not retry it.
  • There is no /mfa route on main yet (PR feat: SPA auth flows: PKCE callbacks, password recovery, TOTP MFA and step-up #101, issue Auth flows in the SPA: password recovery, MFA enrollment and callbacks #32), so for now the user sees the server's Vietnamese message ("Tài khoản này đã bật xác thực hai lớp (MFA): hãy nhập mã xác thực để tiếp tục").
  • The change is compatible with feat: SPA auth flows: PKCE callbacks, password recovery, TOTP MFA and step-up #101, which can add the redirect to /mfa on isMfaRequired.
• Crove Cloud (jwt, since feat: Cloud sessions use the Supabase default with live checks for R3 #149).
  • An aal1 access token does not say whether its user has a verified factor, and Crove cannot read a remote Supabase Auth. ADR-0028 section 3 now records the requirement: the identity provider must state the fact in the token it signs (for example, a claim from a custom access token hook) or not issue such tokens.
  • Until then, jwt asks for the second factor only on R3, and gate T1 keeps this item open for Cloud.
  • Nothing DOS-side is implemented here.
• Deliberate limits (recorded in the ADR):
  • identity_resolve still maps an aal1 token to its account. It writes only the email and name that the token itself carries, and it grants no access.
  • AUTH_SESSION_CHECK=off (local URLs only) does not enforce the rule.

Migration

20261016180000_crove_kernel_mfa_required.sql:

• Idempotent, with SET search_path = pg_catalog, pg_temp on every function.
• Ownership handover is restated with the same SET ROLE and brief CREATE pattern as 20261015090000, and REVOKE/GRANT are restated.
• The timestamp is greater than every file on main and every pending PR at push time: feat(finance): journal reversal and a property test of the posting path (T3) #164 uses 150000, Inbound: tell the owner how many messages to a retired address were dropped #139 160000, Upload pipeline per plan 7.6: staging key, presigned URLs, generation in file references #109 170000.

Tests

New invariant file apps/kernel/test/invariant/mfa-required.db.test.ts. It runs through the HTTP API on PostgreSQL 17, against a mirror of auth.users, auth.sessions and auth.mfa_factors with the enum types and RLS of the pinned GoTrue. It checks:

• An aal1 token of a user with a verified factor is refused with 401 MFA_REQUIRED on a read query (inbox/list, files/recent), an R1 command (sessions/revoke), an upload (kernel.file.ingest) and me/get. No file and no revoke is written.
• An aal2 token of the same kind of user passes all of them.
• A user without factors, and a user with only an unverified factor, keep aal1 access.
• Verifying a factor blocks the old aal1 token on the next request, and deleting the factor restores access.
• A signed-out session gets UNAUTHENTICATED, not MFA_REQUIRED.
• Grants: exactly (user_id, status) and never secret. The factor reader's owner, definer, search_path and EXECUTE are asserted, and the 4-argument session_check is gone.
• Revoking the grant or dropping the policy gives 503 AUTH_NOT_READY, and re-running auth-grants restores it.

Also:

• packages/db/test/invariant/migrate.db.test.ts covers the new reader as a non-superuser runner.
• A unit test pins the 28000 message mapping.
• apps/kernel/test/session-check-stale.db.test.ts (from fix(auth): session revocation follow-ups from #90 #166) now expects the 5-argument check.

Mutation check: when sessions.ts was made to always send aal2, the refusal tests failed.

Verification

• pnpm lint and pnpm typecheck: clean.
• pnpm exec vitest run: 293 passed.
• pnpm exec vitest run --config vitest.invariant.config.ts: 15 files, 251 passed.
• vitest --project db: 289 passed, 5 skipped.
• pnpm docs:check: OK.
• Base copy: I also ran main's copy of the invariant suite and vitest.invariant.config.ts against this PR's code, as Invariant suite (from base) does: 14 files, 244 passed.
• The database runs above used a throwaway postgres:17 container. On the shared dev cluster, other sessions' parallel test databases caused unrelated tuple concurrently updated errors in ALTER ROLE.
• Independent security review (agent): no must-fix findings. The should-fix items are applied: the warning when the table is absent, the limits recorded in the ADR, honest SPA wording, and the migration runner test.

Docs

• ADR-0028: an amendment line; the rule in section 3 (db, and the requirement for jwt); a row in the section 8 error table; a self-host acceptance test.
• Plan: section 7.5 (login, db adapter, jwt adapter, on main), External-tenant gate rows T1 and T2, and the T1 row in the Vietnamese page.
• AGENTS.md: the invariant suite list.

Closes #126

🤖 Generated with Claude Code

[vercel]

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

Project	Deployment	Actions	Updated
crove	Ready	Preview	Oct 15, 2026 5:28pm UTC

[claude]

⚠️ Code review skipped — your organization has no extra usage available to pay for this review.

If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.

Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 47 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9b9a0bc2-eb65-4bb2-b007-d9bad691cdc3

📥 Commits

Reviewing files that changed from the base of the PR and between f8a96ab and 4299393.

📒 Files selected for processing: 14

• AGENTS.md
• apps/app/src/lib/api.ts
• apps/kernel/src/main.ts
• apps/kernel/src/sessions.ts
• apps/kernel/test/auth.test.ts
• apps/kernel/test/invariant/mfa-required.db.test.ts
• apps/kernel/test/session-check-stale.db.test.ts
• docs/adr/0028-identity-sessions-and-revocation.md
• docs/plan/crove-os-kernel.md
• docs/plan/crove-os-kernel.vi.html
• packages/db/src/migrate.ts
• packages/db/test/invariant/migrate.db.test.ts
• packages/kernel-runtime/src/errors.ts
• supabase/migrations/20261016180000_crove_kernel_mfa_required.sql

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[gemini-code-assist]

Code Review

This pull request implements a security enhancement (issue #126) requiring users with a verified multi-factor authentication (MFA) factor to use an aal2 token for all requests, rather than just R3 operations. To support this, the database session_check function has been updated to accept the token's assurance level, and the auth-grants step now configures permissions for auth.mfa_factors. The changes also include client-side handling of the MFA_REQUIRED error, comprehensive documentation updates in ADR-0028 and the project plan, and a new invariant test suite. The reviewer suggested updating an error message in the migration script to accurately reflect that mfa_factors is now checked alongside other auth tables.

[gemini-code-assist]

The error message is slightly outdated and misleading because auth_grants_ready() now also checks permissions on auth.mfa_factors if it exists. Updating the error message to include mfa_factors will make debugging easier if the grant fails on that table.

Suggested change

	if (!rows[0]?.ok) throw new Error('the session check still cannot read auth.sessions and auth.users after the grant');
	if (!rows[0]?.ok) throw new Error('the session check still cannot read auth tables (sessions, users, or mfa_factors) after the grant');