diff --git a/DECISIONS.md b/DECISIONS.md index b0cc7fdc..747282cb 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1522,3 +1522,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-09-02] Nyckeltal "Resultat per månad" shows the exact per-month figures as an always-on list under the bars (#2198), not behind an "Anpassa" toggle: a preference would touch the type, the PUT schema, the strict preferences-body validator, the dialog and its tests for a switch nobody turns off. Per-bar compact labels are conditional on a glyph-width fit rule and fall back to the single latest label, so they never overlap. Left alone: the monthly path counts only posted entries while the year-total path also counts reversed originals (pinned as intended in tests/pg/kpi-report-aggregates-rpc.pg.test.ts), so a same-year storno makes the sum of months differ from Nettoresultat; visible as numbers now, founder call whether to align the two. [2026-09-03] Kundorder PGRST201 fixed by hinting the three sales_order_items embeds AND by teaching scripts/checks/ambiguous-embed.mjs to parse composite FOREIGN KEY (a, b) constraints, instead of a bespoke source-scan test: the guard is the sanctioned owner of this class (decision 2026-09-01) and it missed the pair only because its parser was single-column; with the fix it derives the same 17 pairs prod reports and flags all three shipped sites on main. [2026-09-03] Email-change double-submit gate is a dedicated per-user claim table + SECURITY DEFINER RPC (migration 20260903083000), not idempotency_keys and not an advisory lock: idempotency_keys requires a company_id the account-level route does not have, and a transaction-scoped advisory lock cannot cover the GoTrue call that happens outside the transaction. +[2026-09-03] Old-address social identities are unlinked by a BEFORE UPDATE trigger on auth.users (migration 20260903110000), not by the /auth/callback done path: the callback never runs for a completing click from a browser without a session, and admin-side changes bypass it entirely; the trigger covers every path and keeps the email identity, password and BankID intact. diff --git a/docs/SELF-HOSTING.md b/docs/SELF-HOSTING.md index 07033e0b..d0c018ed 100644 --- a/docs/SELF-HOSTING.md +++ b/docs/SELF-HOSTING.md @@ -22,7 +22,7 @@ In the Supabase dashboard under **Authentication > URL Configuration**: 1. Set **Site URL** to your deployment URL (e.g., `https://gnubok.example.com`). 2. Add `https://gnubok.example.com/auth/callback` to the **Redirect URLs** allowlist. -Accounted uses email + password authentication with magic link as a fallback. The default Supabase email auth settings work out of the box. For production, configure a custom SMTP provider under **Authentication > SMTP Settings** to avoid Supabase's built-in rate limits. +Accounted uses email + password authentication with magic link as a fallback. The default Supabase email auth settings work out of the box. For production, configure a custom SMTP provider under **Authentication > SMTP Settings** to avoid Supabase's built-in rate limits. Keep **Secure email change** enabled (Authentication > Settings, on by default): a login-email change also removes social logins tied to the old address, so the old mailbox must confirm the change before it can lose its access. MFA (two-factor authentication via TOTP) is **not enforced** for self-hosted deployments: the Docker image sets `NEXT_PUBLIC_SELF_HOSTED=true` by default, which disables MFA enforcement. Users can still optionally enable 2FA in Settings > Säkerhet if they wish. Idle and absolute session timeouts are also off by default for self-hosted installs; operators can opt in with the variables below. diff --git a/lib/auth/__tests__/unlink-old-address-identities.pg.test.ts b/lib/auth/__tests__/unlink-old-address-identities.pg.test.ts new file mode 100644 index 00000000..0a0519e7 --- /dev/null +++ b/lib/auth/__tests__/unlink-old-address-identities.pg.test.ts @@ -0,0 +1,266 @@ +import { randomUUID } from 'node:crypto' +import { describe, expect, it } from 'vitest' +import { getPool } from '@/tests/pg/setup' +import { insertAuthUser } from '@/tests/pg/fixtures' + +// on_auth_user_email_updated_unlink_old_identities +// (20260903110000_unlink_old_address_identities_on_email_change.sql): when +// auth.users.email changes, social identities bound to the OLD address are +// removed, app_metadata.providers is recomputed, an email identity for the +// new address is guaranteed, and the removal is written to GoTrue's audit +// table. Everything else on the account (email identity, social identities +// on other addresses) stays. + +async function insertIdentity(params: { + userId: string + provider: string + email: string + providerId?: string +}): Promise { + const id = randomUUID() + await getPool().query( + `INSERT INTO auth.identities + (id, user_id, provider, provider_id, identity_data, created_at, updated_at, last_sign_in_at) + VALUES ($1, $2, $3, $4, $5::jsonb, now(), now(), now())`, + [ + id, + params.userId, + params.provider, + params.providerId ?? (params.provider === 'email' ? params.userId : randomUUID()), + JSON.stringify({ sub: params.providerId ?? params.userId, email: params.email }), + ], + ) + return id +} + +async function setProviders(userId: string, providers: string[]): Promise { + await getPool().query( + `UPDATE auth.users + SET raw_app_meta_data = jsonb_build_object('provider', $2::text, 'providers', $3::jsonb) + WHERE id = $1`, + [userId, providers[0] ?? 'email', JSON.stringify(providers)], + ) +} + +// Mirrors GoTrue's ConfirmEmailChange: the pending address becomes the +// address and the pending column is cleared in the same statement. +async function confirmEmailChange(userId: string, email: string): Promise { + await getPool().query(`UPDATE auth.users SET email_change = $2 WHERE id = $1`, [userId, email]) + await getPool().query( + `UPDATE auth.users SET email = $2, email_change = '' WHERE id = $1`, + [userId, email], + ) +} + +// An admin-side or SQL change: no pending address involved. +async function adminSetEmail(userId: string, email: string): Promise { + await getPool().query(`UPDATE auth.users SET email = $2 WHERE id = $1`, [userId, email]) +} + +async function identities(userId: string): Promise> { + const { rows } = await getPool().query<{ provider: string; email: string }>( + `SELECT provider, lower(identity_data->>'email') AS email + FROM auth.identities WHERE user_id = $1 ORDER BY provider, email`, + [userId], + ) + return rows +} + +async function emailIdentity( + userId: string, +): Promise<{ provider_id: string; identity_data: Record } | undefined> { + const { rows } = await getPool().query<{ + provider_id: string + identity_data: Record + }>(`SELECT provider_id, identity_data FROM auth.identities WHERE user_id = $1 AND provider = 'email'`, [ + userId, + ]) + return rows[0] +} + +async function providers(userId: string): Promise { + const { rows } = await getPool().query<{ providers: unknown }>( + `SELECT raw_app_meta_data->'providers' AS providers FROM auth.users WHERE id = $1`, + [userId], + ) + return rows[0]!.providers +} + +async function currentEmail(userId: string): Promise { + const { rows } = await getPool().query<{ email: string }>( + `SELECT email FROM auth.users WHERE id = $1`, + [userId], + ) + return rows[0]!.email +} + +async function auditEntries(userId: string): Promise>> { + const { rows } = await getPool().query<{ payload: Record }>( + `SELECT payload FROM auth.audit_log_entries + WHERE payload->>'actor_id' = $1 AND payload->>'action' = 'identity_unlink' + ORDER BY created_at`, + [userId], + ) + return rows.map((r) => r.payload) +} + +describe('unlink old-address identities on auth email change', () => { + it('removes the social identity bound to the old address and keeps the rest', async () => { + const userId = await insertAuthUser() + const oldEmail = await currentEmail(userId) + const newEmail = `pg-real-new-${userId}@test.invalid` + await insertIdentity({ userId, provider: 'email', email: oldEmail }) + await insertIdentity({ userId, provider: 'google', email: oldEmail }) + await insertIdentity({ userId, provider: 'google', email: `other-${userId}@test.invalid` }) + await setProviders(userId, ['email', 'google']) + + await confirmEmailChange(userId, newEmail) + + expect(await identities(userId)).toEqual([ + { provider: 'email', email: oldEmail }, + { provider: 'google', email: `other-${userId}@test.invalid` }, + ]) + // Still has a google identity (the other address), so the list is unchanged. + expect(await providers(userId)).toEqual(['email', 'google']) + }) + + it('drops the provider from app_metadata when its last identity goes', async () => { + const userId = await insertAuthUser() + const oldEmail = await currentEmail(userId) + await insertIdentity({ userId, provider: 'email', email: oldEmail }) + await insertIdentity({ userId, provider: 'google', email: oldEmail }) + await setProviders(userId, ['email', 'google']) + + await confirmEmailChange(userId, `pg-real-new-${userId}@test.invalid`) + + expect(await identities(userId)).toEqual([{ provider: 'email', email: oldEmail }]) + expect(await providers(userId)).toEqual(['email']) + }) + + it('gives a Google-only account a verified email identity after a confirmed change', async () => { + // Signed up with Google, never set a password: no 'email' identity. + // Removing the Google identity must not leave zero identities; the email + // identity is what Google with the new address links through and what + // password recovery resolves. Old address matched case-insensitively. + const userId = await insertAuthUser() + const oldEmail = await currentEmail(userId) + const newEmail = `pg-real-new-${userId}@test.invalid` + await insertIdentity({ userId, provider: 'google', email: oldEmail.toUpperCase() }) + await setProviders(userId, ['google']) + + await confirmEmailChange(userId, newEmail) + + expect(await identities(userId)).toEqual([{ provider: 'email', email: newEmail }]) + const created = await emailIdentity(userId) + expect(created?.provider_id).toBe(userId) + expect(created?.identity_data).toMatchObject({ + sub: userId, + email: newEmail, + email_verified: true, + }) + expect(await providers(userId)).toEqual(['email']) + }) + + it('creates the email identity unverified after an admin-side change', async () => { + // No pending address was confirmed by the user, so the trigger must not + // vouch for the new address. + const userId = await insertAuthUser() + const oldEmail = await currentEmail(userId) + const newEmail = `pg-real-new-${userId}@test.invalid` + await insertIdentity({ userId, provider: 'google', email: oldEmail }) + await setProviders(userId, ['google']) + + await adminSetEmail(userId, newEmail) + + expect(await identities(userId)).toEqual([{ provider: 'email', email: newEmail }]) + expect((await emailIdentity(userId))?.identity_data).toMatchObject({ + email: newEmail, + email_verified: false, + }) + }) + + it('does not create a second email identity when one already exists', async () => { + const userId = await insertAuthUser() + const oldEmail = await currentEmail(userId) + await insertIdentity({ userId, provider: 'email', email: oldEmail }) + await insertIdentity({ userId, provider: 'google', email: oldEmail }) + + await confirmEmailChange(userId, `pg-real-new-${userId}@test.invalid`) + + const { rows } = await getPool().query<{ n: number }>( + `SELECT count(*)::int AS n FROM auth.identities WHERE user_id = $1 AND provider = 'email'`, + [userId], + ) + expect(rows[0]!.n).toBe(1) + }) + + it('never touches the email identity, even though it carries the old address', async () => { + // GoTrue moves the email identity itself as part of the change; the + // trigger must not race it by deleting the row. + const userId = await insertAuthUser() + const oldEmail = await currentEmail(userId) + await insertIdentity({ userId, provider: 'email', email: oldEmail }) + await setProviders(userId, ['email']) + + await confirmEmailChange(userId, `pg-real-new-${userId}@test.invalid`) + + expect(await identities(userId)).toEqual([{ provider: 'email', email: oldEmail }]) + expect(await providers(userId)).toEqual(['email']) + expect(await auditEntries(userId)).toEqual([]) + }) + + it('leaves identities alone when the update does not change the email', async () => { + const userId = await insertAuthUser() + const oldEmail = await currentEmail(userId) + await insertIdentity({ userId, provider: 'google', email: oldEmail }) + await setProviders(userId, ['google']) + + await getPool().query(`UPDATE auth.users SET updated_at = now() WHERE id = $1`, [userId]) + await adminSetEmail(userId, oldEmail) + + expect(await identities(userId)).toEqual([{ provider: 'google', email: oldEmail }]) + expect(await providers(userId)).toEqual(['google']) + }) + + it('does not touch other users with a social identity on the same address', async () => { + const a = await insertAuthUser() + const b = await insertAuthUser() + const shared = `shared-${a}@test.invalid` + await adminSetEmail(a, shared) + await insertIdentity({ userId: a, provider: 'google', email: shared }) + await insertIdentity({ userId: b, provider: 'google', email: shared }) + const newEmail = `pg-real-new-${a}@test.invalid` + + await confirmEmailChange(a, newEmail) + + // a lost its Google login and got an email identity for the new address. + expect(await identities(a)).toEqual([{ provider: 'email', email: newEmail }]) + expect(await identities(b)).toEqual([{ provider: 'google', email: shared }]) + }) + + it('writes an identity_unlink entry to the auth audit log', async () => { + const userId = await insertAuthUser() + const oldEmail = await currentEmail(userId) + const newEmail = `pg-real-new-${userId}@test.invalid` + await insertIdentity({ userId, provider: 'email', email: oldEmail }) + await insertIdentity({ userId, provider: 'google', email: oldEmail }) + + await confirmEmailChange(userId, newEmail) + + const entries = await auditEntries(userId) + expect(entries).toHaveLength(1) + expect(entries[0]).toMatchObject({ + action: 'identity_unlink', + actor_id: userId, + actor_username: oldEmail, + log_type: 'user', + traits: { + reason: 'email_change', + providers: ['google'], + old_email: oldEmail, + new_email: newEmail, + confirmed: true, + }, + }) + }) +}) diff --git a/supabase/migrations/20260903110000_unlink_old_address_identities_on_email_change.sql b/supabase/migrations/20260903110000_unlink_old_address_identities_on_email_change.sql new file mode 100644 index 00000000..67d9b270 --- /dev/null +++ b/supabase/migrations/20260903110000_unlink_old_address_identities_on_email_change.sql @@ -0,0 +1,131 @@ +-- A completed login-email change must close every door that was opened by +-- the old address, the social login included. +-- +-- GoTrue keys OAuth identities (Google, ...) on the provider's subject, not +-- on the address, so after a secure email change from A to B the Google +-- identity that was auto-linked for A stays on the account: "Logga in med +-- Google" while signed into the A mailbox still opens the company, although +-- the user just told us A is no longer theirs (reproduced on prod +-- 2026-09-03 with a test account: both Google logins kept working after +-- the change). Product decision (Emil, 2026-09-03): a change is a change; +-- only identities bound to the address the user switched FROM go, +-- everything else stays. Google with the NEW address keeps working: GoTrue +-- auto-links it on the first sign-in through the email identity for that +-- address, which this trigger guarantees exists. Password, BankID and +-- social identities on other addresses are untouched, so the account always +-- keeps a way in (at minimum "Glömt lösenord" to the new address). +-- +-- A trigger rather than app code so every completion path is covered: the +-- hook-built link, the stock GoTrue link, a click from a phone mail app with +-- no session, and an admin-side change. Sits next to sync_profile_email +-- (20260828191950) on the same event. + +create or replace function public.unlink_old_address_identities() +returns trigger as $$ +declare + v_removed integer; + v_providers text[]; + -- GoTrue's ConfirmEmailChange writes email = email_change and clears + -- email_change in the same UPDATE, so "the pending address became the + -- address" is the signature of a change the user confirmed from both + -- mailboxes. Anything else (admin API, SQL) is unconfirmed. + v_confirmed boolean := old.email_change is not null + and old.email_change <> '' + and lower(old.email_change) = lower(new.email); +begin + -- auth.identities is created by GoTrue at startup, not by the Postgres + -- image. Where GoTrue has never run (a bare pg-real container, a fresh + -- self-hosted database before first boot) there is nothing to unlink and + -- an email change must not fail on a missing table. + if to_regclass('auth.identities') is null then + return new; + end if; + + with removed as ( + delete from auth.identities i + where i.user_id = new.id + and i.provider not in ('email', 'phone') + and lower(i.identity_data->>'email') = lower(old.email) + returning i.provider + ) + select count(*), array_agg(provider order by provider) + into v_removed, v_providers + from removed; + + if v_removed > 0 then + -- A Google-only account (signed up with Google, never set a password) has + -- no 'email' identity at all, so removing its Google identity would leave + -- zero identities. GoTrue links a later "Sign in with Google" for the NEW + -- address, and resolves password recovery, through the email identity, + -- so make sure one exists for the new address. Same shape GoTrue writes + -- itself (provider_id = user id). email_verified is only claimed for a + -- change the user confirmed; an admin-side change gets an unverified + -- identity, exactly as GoTrue would create it. If GoTrue creates or + -- updates the email identity later in the same change, it finds this row + -- and updates it. + insert into auth.identities (id, user_id, provider, provider_id, identity_data, created_at, updated_at) + select gen_random_uuid(), new.id, 'email', new.id::text, + jsonb_build_object('sub', new.id::text, 'email', new.email, + 'email_verified', v_confirmed, 'phone_verified', false), + now(), now() + where not exists ( + select 1 from auth.identities i where i.user_id = new.id and i.provider = 'email' + ); + + -- GoTrue mirrors the linked providers into app_metadata.providers on + -- link/unlink; keep that list truthful so nothing offers a login button + -- for a provider that is no longer linked. Recomputed from what is left + -- rather than by removing one entry, so it is right whatever was there. + new.raw_app_meta_data := jsonb_set( + coalesce(new.raw_app_meta_data, '{}'::jsonb), + '{providers}', + coalesce( + (select jsonb_agg(distinct i.provider order by i.provider) + from auth.identities i + where i.user_id = new.id), + '[]'::jsonb + ) + ); + + -- Removing a login method is a security-relevant event; leave the same + -- trail GoTrue leaves for its own identity_unlink, in its own audit + -- table, so the account history reads as one sequence. + if to_regclass('auth.audit_log_entries') is not null then + insert into auth.audit_log_entries (instance_id, id, payload, created_at, ip_address) + values ( + new.instance_id, + gen_random_uuid(), + jsonb_build_object( + 'action', 'identity_unlink', + 'actor_id', new.id, + 'actor_username', old.email, + 'actor_via_sso', false, + 'log_type', 'user', + 'traits', jsonb_build_object( + 'reason', 'email_change', + 'providers', to_jsonb(v_providers), + 'old_email', old.email, + 'new_email', new.email, + 'confirmed', v_confirmed + ) + )::json, + now(), + '' + ); + end if; + end if; + + return new; +end; +$$ language plpgsql security definer set search_path = public; + +-- BEFORE so the providers list lands in the same row write; the identity +-- delete does not depend on the users row having been updated yet. +drop trigger if exists on_auth_user_email_updated_unlink_old_identities on auth.users; +create trigger on_auth_user_email_updated_unlink_old_identities + before update of email on auth.users + for each row + when (old.email is not null and new.email is distinct from old.email) + execute function public.unlink_old_address_identities(); + +revoke all on function public.unlink_old_address_identities() from public, anon, authenticated; diff --git a/tests/pg/bootstrap.sql b/tests/pg/bootstrap.sql index cbbfa480..d5d6154b 100644 --- a/tests/pg/bootstrap.sql +++ b/tests/pg/bootstrap.sql @@ -46,3 +46,29 @@ CREATE OR REPLACE FUNCTION storage.foldername(name text) AS $$ SELECT string_to_array(name, '/'); $$; + +-- auth.identities is created by GoTrue at startup, not by the Postgres +-- image, and GoTrue does not run in CI. Triggers on auth.users that touch +-- identities (20260903110000 unlink_old_address_identities) need the table +-- to exist so an email change in a pg-real test does not fail with 42P01. +-- Shape mirrors GoTrue's migration (same PK, unique key, generated email). +CREATE TABLE IF NOT EXISTS auth.identities ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + provider_id text NOT NULL, + user_id uuid NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE, + identity_data jsonb NOT NULL, + provider text NOT NULL, + last_sign_in_at timestamptz, + created_at timestamptz, + updated_at timestamptz, + email text GENERATED ALWAYS AS (lower(identity_data ->> 'email')) STORED, + CONSTRAINT identities_provider_id_provider_unique UNIQUE (provider_id, provider) +); +CREATE INDEX IF NOT EXISTS identities_user_id_idx ON auth.identities (user_id); + +-- auth.audit_log_entries ships in the Postgres image without the ip_address +-- column GoTrue adds on first boot (NOT NULL DEFAULT '' on every hosted +-- project). unlink_old_address_identities writes GoTrue's audit table with +-- that column, so the CI double must carry it too. +ALTER TABLE IF EXISTS auth.audit_log_entries + ADD COLUMN IF NOT EXISTS ip_address varchar(64) NOT NULL DEFAULT '';