From 5d439ded3dca18b7bcefa7438473b55bfbcf5e44 Mon Sep 17 00:00:00 2001 From: kris1027 Date: Sun, 11 Oct 2026 12:13:19 +0200 Subject: [PATCH 01/18] feat(supabase): add profiles with a username and its weekly lock Co-Authored-By: Claude Opus 5.5 --- lib/supabase/database.types.ts | 25 +++ .../20261011120000_account_username.sql | 144 ++++++++++++++ supabase/schemas/02-tables.sql | 31 +++ supabase/schemas/03-triggers.sql | 45 +++++ supabase/schemas/04-policies.sql | 36 ++++ supabase/schemas/05-functions.sql | 27 +++ supabase/tests/140-security-sweep.test.sql | 19 +- supabase/tests/170-profiles-username.test.sql | 182 ++++++++++++++++++ 8 files changed, 507 insertions(+), 2 deletions(-) create mode 100644 supabase/migrations/20261011120000_account_username.sql create mode 100644 supabase/tests/170-profiles-username.test.sql diff --git a/lib/supabase/database.types.ts b/lib/supabase/database.types.ts index 21da10c..9ae866e 100644 --- a/lib/supabase/database.types.ts +++ b/lib/supabase/database.types.ts @@ -9,6 +9,30 @@ export type Json = export type Database = { public: { Tables: { + profiles: { + Row: { + created_at: string + updated_at: string + user_id: string + username: string + username_changed_at: string + } + Insert: { + created_at?: string + updated_at?: string + user_id: string + username: string + username_changed_at?: string + } + Update: { + created_at?: string + updated_at?: string + user_id?: string + username?: string + username_changed_at?: string + } + Relationships: [] + } user_episode_state: { Row: { created_at: string @@ -273,6 +297,7 @@ export type Database = { tracked_at: string }[] } + username_available: { Args: { p_username: string }; Returns: boolean } } Enums: { [_ in never]: never diff --git a/supabase/migrations/20261011120000_account_username.sql b/supabase/migrations/20261011120000_account_username.sql new file mode 100644 index 0000000..868f964 --- /dev/null +++ b/supabase/migrations/20261011120000_account_username.sql @@ -0,0 +1,144 @@ +-- Account username (prompts/account-username.md): the `profiles` table, its +-- rules, owner only policies, and `username_available` for the sign up form. +-- +-- Written by hand from `supabase/schemas/`, which stays the source of truth. +-- The grants are written out because the declarative diff does not track the +-- default privileges Supabase hands `anon` and `authenticated` on a new table +-- (see the caveat at the end of `04-policies.sql`). It only adds objects, so +-- the app deployed before it keeps working; push it before merging its PR +-- (docs/deploy.md). + +-- One row per person who has chosen a username (prompts/account-username.md). +-- It is the only account field the auth account does not already hold, which +-- is what AGENTS.md section 8 asks before a profile table exists. Never public: +-- only its owner reads it, and no route shows one person's name to another. +-- +-- The rules live on the table, not in the app, so a direct API write obeys +-- them too: the two checks hold the format and the reserved names (both kept +-- equal to `lib/auth/username.ts` by `username-sql.test.ts`), the index below +-- holds uniqueness ignoring case, and `guard_username_change` in +-- `03-triggers.sql` owns `username_changed_at` and the weekly lock. +create table public.profiles ( + user_id uuid primary key references auth.users (id) on delete cascade, + username text not null, + username_changed_at timestamptz not null default now(), + created_at timestamptz not null default now(), + updated_at timestamptz not null default now(), + constraint profiles_username_format_check + check (username ~ '^[A-Za-z0-9_.-]{2,30}$' and username !~ '^[.-]|[.-]$'), + constraint profiles_username_reserved_check + check (lower(username) <> all (array[ + 'admin', 'administrator', 'root', 'support', 'help', 'bestats', + 'system', 'moderator', 'staff', 'official', 'tmdb', 'api', 'account', + 'null', 'undefined' + ])) +); + +-- Stored as typed, unique as lowercased: `Kris` and `kris` are one name. Also +-- the index `username_available` reads. +create unique index profiles_username_lower_key + on public.profiles (lower(username)); + +-- Owns `profiles.username_changed_at` and the weekly lock +-- (prompts/account-username.md). A username can change once per rolling 7 +-- days; the first one, set at sign up or on `/account`, starts the clock. +-- Saving the identical name is not a change and keeps the clock where it was, +-- while a change of case is one. The client can never choose the timestamp, +-- and `user_id` never moves, whichever write path is used. +-- +-- On an upsert that hits an existing row, the insert branch runs first on the +-- proposed row and the update branch then decides, as in `set_watchlisted_at`. +create or replace function public.guard_username_change() +returns trigger +language plpgsql +security invoker +set search_path = '' +as $$ +begin + if tg_op = 'INSERT' then + new.username_changed_at := now(); + return new; + end if; + + new.user_id := old.user_id; + + if new.username = old.username then + new.username_changed_at := old.username_changed_at; + return new; + end if; + + if old.username_changed_at > now() - interval '7 days' then + raise exception 'username_locked' using errcode = 'P0001'; + end if; + + new.username_changed_at := now(); + return new; +end; +$$; + +create trigger profiles_guard_username_change + before insert or update on public.profiles + for each row execute function public.guard_username_change(); + +create trigger profiles_set_updated_at + before update on public.profiles + for each row execute function public.set_updated_at(); + +alter table public.profiles enable row level security; +alter table public.profiles force row level security; + +revoke all on table public.profiles from anon, public, authenticated; +-- No delete: deleting and re-inserting the row would restart the weekly lock. +grant select, insert, update on table public.profiles to authenticated; + +-- profiles + +create policy "Users select their own profile" + on public.profiles for select + to authenticated + using ((select auth.uid()) = user_id); + +create policy "Users insert their own profile" + on public.profiles for insert + to authenticated + with check ((select auth.uid()) = user_id); + +create policy "Users update their own profile" + on public.profiles for update + to authenticated + using ((select auth.uid()) = user_id) + with check ((select auth.uid()) = user_id); + +create policy "Users delete their own profile" + on public.profiles for delete + to authenticated + using ((select auth.uid()) = user_id); + +revoke all on function public.guard_username_change() from public, anon, authenticated; + +-- Whether a username is free, asked by the sign up form before the account +-- exists (prompts/account-username.md, Q16). +-- +-- The one SECURITY DEFINER function, and the one `anon` can call, in `public`: +-- a signed out visitor cannot read `profiles`, and asking after the account is +-- created would leave it with no name when the name is taken. +-- `140-security-sweep` names this exact signature as its only exception. It +-- returns a boolean and nothing else, so all it can tell anyone is that a name +-- is in use, which the plan accepts. The unique index, not this answer, is +-- what guarantees uniqueness: two sign ups racing for one name both hear +-- true, and the second insert fails. +create or replace function public.username_available(p_username text) +returns boolean +language sql +stable +security definer +set search_path = '' +as $$ + select not exists ( + select 1 from public.profiles + where lower(username) = lower(p_username) + ); +$$; + +revoke all on function public.username_available(text) from public, anon, authenticated; +grant execute on function public.username_available(text) to anon, authenticated; diff --git a/supabase/schemas/02-tables.sql b/supabase/schemas/02-tables.sql index 08b63c2..4e81372 100644 --- a/supabase/schemas/02-tables.sql +++ b/supabase/schemas/02-tables.sql @@ -110,3 +110,34 @@ create table public.user_episode_state ( -- show, already in display order. create index user_episode_state_show_order_idx on public.user_episode_state (user_id, show_id, season_number, episode_number); + +-- One row per person who has chosen a username (prompts/account-username.md). +-- It is the only account field the auth account does not already hold, which +-- is what AGENTS.md section 8 asks before a profile table exists. Never public: +-- only its owner reads it, and no route shows one person's name to another. +-- +-- The rules live on the table, not in the app, so a direct API write obeys +-- them too: the two checks hold the format and the reserved names (both kept +-- equal to `lib/auth/username.ts` by `username-sql.test.ts`), the index below +-- holds uniqueness ignoring case, and `guard_username_change` in +-- `03-triggers.sql` owns `username_changed_at` and the weekly lock. +create table public.profiles ( + user_id uuid primary key references auth.users (id) on delete cascade, + username text not null, + username_changed_at timestamptz not null default now(), + created_at timestamptz not null default now(), + updated_at timestamptz not null default now(), + constraint profiles_username_format_check + check (username ~ '^[A-Za-z0-9_.-]{2,30}$' and username !~ '^[.-]|[.-]$'), + constraint profiles_username_reserved_check + check (lower(username) <> all (array[ + 'admin', 'administrator', 'root', 'support', 'help', 'bestats', + 'system', 'moderator', 'staff', 'official', 'tmdb', 'api', 'account', + 'null', 'undefined' + ])) +); + +-- Stored as typed, unique as lowercased: `Kris` and `kris` are one name. Also +-- the index `username_available` reads. +create unique index profiles_username_lower_key + on public.profiles (lower(username)); diff --git a/supabase/schemas/03-triggers.sql b/supabase/schemas/03-triggers.sql index 0025d05..16c73b9 100644 --- a/supabase/schemas/03-triggers.sql +++ b/supabase/schemas/03-triggers.sql @@ -86,6 +86,43 @@ begin end; $$; +-- Owns `profiles.username_changed_at` and the weekly lock +-- (prompts/account-username.md). A username can change once per rolling 7 +-- days; the first one, set at sign up or on `/account`, starts the clock. +-- Saving the identical name is not a change and keeps the clock where it was, +-- while a change of case is one. The client can never choose the timestamp, +-- and `user_id` never moves, whichever write path is used. +-- +-- On an upsert that hits an existing row, the insert branch runs first on the +-- proposed row and the update branch then decides, as in `set_watchlisted_at`. +create or replace function public.guard_username_change() +returns trigger +language plpgsql +security invoker +set search_path = '' +as $$ +begin + if tg_op = 'INSERT' then + new.username_changed_at := now(); + return new; + end if; + + new.user_id := old.user_id; + + if new.username = old.username then + new.username_changed_at := old.username_changed_at; + return new; + end if; + + if old.username_changed_at > now() - interval '7 days' then + raise exception 'username_locked' using errcode = 'P0001'; + end if; + + new.username_changed_at := now(); + return new; +end; +$$; + -- Triggers on one table fire in name order. This one does not read -- `updated_at`, so its place beside `user_movie_state_set_updated_at` does not -- matter. @@ -108,3 +145,11 @@ create trigger user_show_state_set_updated_at create trigger user_episode_state_set_updated_at before update on public.user_episode_state for each row execute function public.set_updated_at(); + +create trigger profiles_guard_username_change + before insert or update on public.profiles + for each row execute function public.guard_username_change(); + +create trigger profiles_set_updated_at + before update on public.profiles + for each row execute function public.set_updated_at(); diff --git a/supabase/schemas/04-policies.sql b/supabase/schemas/04-policies.sql index 43258da..90ae61d 100644 --- a/supabase/schemas/04-policies.sql +++ b/supabase/schemas/04-policies.sql @@ -29,10 +29,13 @@ alter table public.user_show_state enable row level security; alter table public.user_show_state force row level security; alter table public.user_episode_state enable row level security; alter table public.user_episode_state force row level security; +alter table public.profiles enable row level security; +alter table public.profiles force row level security; revoke all on table public.user_movie_state from anon, public; revoke all on table public.user_show_state from anon, public; revoke all on table public.user_episode_state from anon, public; +revoke all on table public.profiles from anon, public; -- `authenticated` is revoked from as well, then granted back only the four -- commands. The same default privileges that reach `anon` also hand @@ -42,11 +45,20 @@ revoke all on table public.user_episode_state from anon, public; revoke all on table public.user_movie_state from authenticated; revoke all on table public.user_show_state from authenticated; revoke all on table public.user_episode_state from authenticated; +revoke all on table public.profiles from authenticated; grant select, insert, update, delete on table public.user_movie_state to authenticated; grant select, insert, update, delete on table public.user_show_state to authenticated; grant select, insert, update, delete on table public.user_episode_state to authenticated; +-- No delete on `profiles`, deliberately (prompts/account-username.md). +-- Deleting the row and inserting it again would restart the weekly lock, so +-- the privilege layer refuses a delete before its policy is ever consulted. +-- The policy below still exists so every table has the same shape, which +-- `140-security-sweep` checks; an account's row goes when the account does, +-- through the cascade. +grant select, insert, update on table public.profiles to authenticated; + -- user_movie_state create policy "Users select their own movie state" @@ -116,12 +128,36 @@ create policy "Users delete their own episode state" to authenticated using ((select auth.uid()) = user_id); +-- profiles + +create policy "Users select their own profile" + on public.profiles for select + to authenticated + using ((select auth.uid()) = user_id); + +create policy "Users insert their own profile" + on public.profiles for insert + to authenticated + with check ((select auth.uid()) = user_id); + +create policy "Users update their own profile" + on public.profiles for update + to authenticated + using ((select auth.uid()) = user_id) + with check ((select auth.uid()) = user_id); + +create policy "Users delete their own profile" + on public.profiles for delete + to authenticated + using ((select auth.uid()) = user_id); + -- Every trigger function: Postgres grants EXECUTE to PUBLIC on every new -- function, so narrow them here too. They return `trigger`, which Postgres -- refuses to call directly, so this is tidiness rather than a live hole. revoke all on function public.set_updated_at() from public, anon, authenticated; revoke all on function public.set_watchlisted_at() from public, anon, authenticated; revoke all on function public.set_tracked_at() from public, anon, authenticated; +revoke all on function public.guard_username_change() from public, anon, authenticated; -- Caveat worth knowing before you edit this file. The declarative diff engine -- does not track grants that come from Supabase's default privileges, so the diff --git a/supabase/schemas/05-functions.sql b/supabase/schemas/05-functions.sql index c9f262c..c4c9ec2 100644 --- a/supabase/schemas/05-functions.sql +++ b/supabase/schemas/05-functions.sql @@ -648,3 +648,30 @@ revoke all on function public.restore_episodes_watched(integer, jsonb) from publ grant execute on function public.mark_season_watched(integer, smallint, integer[], smallint[]) to authenticated; grant execute on function public.unmark_episodes_watched(integer, integer[]) to authenticated; grant execute on function public.restore_episodes_watched(integer, jsonb) to authenticated; + +-- Whether a username is free, asked by the sign up form before the account +-- exists (prompts/account-username.md, Q16). +-- +-- The one SECURITY DEFINER function, and the one `anon` can call, in `public`: +-- a signed out visitor cannot read `profiles`, and asking after the account is +-- created would leave it with no name when the name is taken. +-- `140-security-sweep` names this exact signature as its only exception. It +-- returns a boolean and nothing else, so all it can tell anyone is that a name +-- is in use, which the plan accepts. The unique index, not this answer, is +-- what guarantees uniqueness: two sign ups racing for one name both hear +-- true, and the second insert fails. +create or replace function public.username_available(p_username text) +returns boolean +language sql +stable +security definer +set search_path = '' +as $$ + select not exists ( + select 1 from public.profiles + where lower(username) = lower(p_username) + ); +$$; + +revoke all on function public.username_available(text) from public, anon, authenticated; +grant execute on function public.username_available(text) to anon, authenticated; diff --git a/supabase/tests/140-security-sweep.test.sql b/supabase/tests/140-security-sweep.test.sql index 0009a18..cdbd0dc 100644 --- a/supabase/tests/140-security-sweep.test.sql +++ b/supabase/tests/140-security-sweep.test.sql @@ -12,7 +12,7 @@ -- through `pg_depend`. begin; -select plan(13); +select plan(14); -- The sweep must see something, or every empty result below is vacuous. select ok( @@ -122,17 +122,31 @@ select is_empty( ); -- Functions: security definer runs as the owner and would step around the --- caller's policies. Every function here is security invoker by design. +-- caller's policies. Every function here is security invoker by design, with +-- one named exception: `username_available(text)`, which the signed out sign +-- up form asks before an account exists (prompts/account-username.md). It is +-- excluded by its exact signature, here and in the anon check below, so any +-- other definer or anon callable function still fails. select is_empty( $$select p.oid::regprocedure::text from pg_proc p where p.pronamespace = 'public'::regnamespace and p.prosecdef + and p.oid <> 'public.username_available(text)'::regprocedure and not exists ( select 1 from pg_depend d where d.objid = p.oid and d.deptype = 'e' )$$, 'no function in public is security definer' ); +-- The exception can only ever answer yes or no. Widening its return type +-- would turn it into a way for anyone to read the table. +select is( + (select p.prorettype::regtype::text from pg_proc p + where p.oid = 'public.username_available(text)'::regprocedure), + 'boolean', + 'the one exception returns a boolean and nothing else' +); + -- An unpinned search_path lets a caller shadow a table or operator the -- function names. select is_empty( @@ -155,6 +169,7 @@ select is_empty( where p.pronamespace = 'public'::regnamespace and p.prorettype <> 'trigger'::regtype and has_function_privilege('anon', p.oid, 'execute') + and p.oid <> 'public.username_available(text)'::regprocedure and not exists ( select 1 from pg_depend d where d.objid = p.oid and d.deptype = 'e' )$$, diff --git a/supabase/tests/170-profiles-username.test.sql b/supabase/tests/170-profiles-username.test.sql new file mode 100644 index 0000000..1697766 --- /dev/null +++ b/supabase/tests/170-profiles-username.test.sql @@ -0,0 +1,182 @@ +-- Account username (prompts/account-username.md, R1 to R4). +-- +-- Pins `profiles`: the format and reserved checks, uniqueness ignoring case, +-- the trigger that owns `username_changed_at` and the weekly lock, the owner +-- only policies with no delete, and `username_available`, the one definer +-- function anon may call. Past times are pinned as `postgres` with +-- `session_replication_role = replica`, which skips the triggers. + +begin; +select plan(31); + +-- Shape + +select ok( + (select prosecdef and proconfig = array['search_path=""'] + and prorettype = 'boolean'::regtype + from pg_proc where oid = 'public.username_available(text)'::regprocedure), + 'username_available is security definer, returns boolean, with an empty search_path' +); +select ok( + has_function_privilege('anon', 'public.username_available(text)', 'execute') + and has_function_privilege('authenticated', 'public.username_available(text)', 'execute'), + 'anon and authenticated can ask whether a name is free' +); +select ok( + (select not prosecdef and proconfig = array['search_path=""'] + from pg_proc where oid = 'public.guard_username_change()'::regprocedure) + and not has_function_privilege('authenticated', 'public.guard_username_change()', 'execute'), + 'the trigger function is security invoker and not executable by authenticated' +); +select ok( + not has_table_privilege('authenticated', 'public.profiles', 'delete') + and has_table_privilege('authenticated', 'public.profiles', 'select') + and has_table_privilege('authenticated', 'public.profiles', 'insert') + and has_table_privilege('authenticated', 'public.profiles', 'update'), + 'authenticated may select, insert and update, but never delete' +); + +-- User B already has a name. +set local session_replication_role = replica; +insert into public.profiles (user_id, username, username_changed_at) +values ('22222222-2222-2222-2222-222222222222', 'Bee_Two', now() - interval '30 days'); +set local session_replication_role = origin; + +-- As user A + +set local request.jwt.claims = '{"sub":"11111111-1111-1111-1111-111111111111","role":"authenticated"}'; +set local role authenticated; + +select is_empty( + $$select 1 from public.profiles where user_id = '22222222-2222-2222-2222-222222222222'$$, + 'A cannot read B''s profile' +); +select is(public.username_available('bee_two'), false, 'a taken name is unavailable, ignoring case'); +select is(public.username_available('free_name'), true, 'a free name is available'); + +-- Format and reserved names (R1) +select throws_ok( + $$insert into public.profiles (user_id, username) values ('11111111-1111-1111-1111-111111111111', 'a')$$, + '23514', null, 'one character is refused' +); +select throws_ok( + $$insert into public.profiles (user_id, username) values ('11111111-1111-1111-1111-111111111111', 'has space')$$, + '23514', null, 'a space is refused' +); +select throws_ok( + $$insert into public.profiles (user_id, username) values ('11111111-1111-1111-1111-111111111111', '-dash')$$, + '23514', null, 'a leading dash is refused' +); +select throws_ok( + $$insert into public.profiles (user_id, username) values ('11111111-1111-1111-1111-111111111111', 'dot.')$$, + '23514', null, 'a trailing dot is refused' +); +select throws_ok( + $$insert into public.profiles (user_id, username) values ('11111111-1111-1111-1111-111111111111', 'émile')$$, + '23514', null, 'a non ASCII letter is refused' +); +select throws_ok( + $$insert into public.profiles (user_id, username) values ('11111111-1111-1111-1111-111111111111', repeat('a', 31))$$, + '23514', null, '31 characters is refused' +); +select throws_ok( + $$insert into public.profiles (user_id, username) values ('11111111-1111-1111-1111-111111111111', 'Admin')$$, + '23514', null, 'a reserved name is refused, ignoring case' +); +select throws_ok( + $$insert into public.profiles (user_id, username) values ('11111111-1111-1111-1111-111111111111', 'BEE_TWO')$$, + '23505', null, 'a name taken in another case is refused' +); + +-- Ownership (R3) +select throws_ok( + $$insert into public.profiles (user_id, username) values ('22222222-2222-2222-2222-222222222222', 'stolen')$$, + '42501', null, 'A cannot insert a row for B' +); + +select lives_ok( + $$insert into public.profiles (user_id, username, username_changed_at) + values ('11111111-1111-1111-1111-111111111111', 'Ay_One', now() - interval '1 year')$$, + 'A sets a valid name' +); +select is( + (select username_changed_at from public.profiles where user_id = '11111111-1111-1111-1111-111111111111'), + now(), + 'the first set stamps the current time, whatever the client sent' +); + +-- The weekly lock (R2) +select throws_ok( + $$update public.profiles set username = 'Ay_Two' where user_id = '11111111-1111-1111-1111-111111111111'$$, + 'P0001', 'username_locked', 'a change straight after the first set is locked' +); +select throws_ok( + $$update public.profiles set username = 'ay_one' where user_id = '11111111-1111-1111-1111-111111111111'$$, + 'P0001', 'username_locked', 'a change of case counts as a change' +); +select lives_ok( + $$update public.profiles set username = 'Ay_One', username_changed_at = now() - interval '1 year' + where user_id = '11111111-1111-1111-1111-111111111111'$$, + 'saving the identical name is allowed while locked' +); +select is( + (select username_changed_at from public.profiles where user_id = '11111111-1111-1111-1111-111111111111'), + now(), + 'and it neither moves the clock nor lets the client rewind it' +); + +-- Eight days later +reset role; +set local session_replication_role = replica; +update public.profiles set username_changed_at = now() - interval '8 days' +where user_id = '11111111-1111-1111-1111-111111111111'; +set local session_replication_role = origin; +set local role authenticated; + +select lives_ok( + $$insert into public.profiles (user_id, username) values ('11111111-1111-1111-1111-111111111111', 'Ay_Two') + on conflict (user_id) do update set username = excluded.username$$, + 'an upsert changes the name once the week is over' +); +select is( + (select username_changed_at from public.profiles where user_id = '11111111-1111-1111-1111-111111111111'), + now(), + 'and restarts the clock' +); +select throws_ok( + $$update public.profiles set username = 'Ay_Three' where user_id = '11111111-1111-1111-1111-111111111111'$$, + 'P0001', 'username_locked', 'so a second change is locked again' +); +select is(public.username_available('ay_one'), true, 'the old name is free again'); + +select lives_ok( + $$update public.profiles set username = 'x_x' where user_id = '22222222-2222-2222-2222-222222222222'$$, + 'an update aimed at B''s row runs' +); +reset role; +select is( + (select username from public.profiles where user_id = '22222222-2222-2222-2222-222222222222'), + 'Bee_Two', + 'but changes nothing' +); +set local role authenticated; + +select throws_ok( + $$delete from public.profiles where user_id = '11111111-1111-1111-1111-111111111111'$$, + '42501', null, 'A cannot delete their own row, which would reset the lock' +); + +-- Signed out +reset role; +set local request.jwt.claims = '{"role":"anon"}'; +set local role anon; + +select throws_ok( + $$select 1 from public.profiles$$, + '42501', null, 'anon cannot read profiles' +); +select is(public.username_available('Ay_Two'), false, 'anon learns only that a name is in use'); + +reset role; +select * from finish(); +rollback; From e45a983cc1428362965bf43aec187743562f6bb1 Mon Sep 17 00:00:00 2001 From: kris1027 Date: Sun, 11 Oct 2026 13:34:35 +0200 Subject: [PATCH 02/18] docs(prompts): plan the account username Co-Authored-By: Claude Opus 5.5 --- prompts/account-username.md | 238 ++++++++++++++++++++++++++++++++++++ 1 file changed, 238 insertions(+) create mode 100644 prompts/account-username.md diff --git a/prompts/account-username.md b/prompts/account-username.md new file mode 100644 index 0000000..3cb3532 --- /dev/null +++ b/prompts/account-username.md @@ -0,0 +1,238 @@ +# Account username + +No spec: an account feature decided in grilling (2026-10-11, rounds 1 to 3). It adds the first `profiles` table, which section 8 of `AGENTS.md` allows once the UI needs a field the auth account does not hold, and it reverses the "no `profiles` table" note in `lib/auth/AGENTS.md` and `lib/auth/identity.ts`. Branch `feat/account-username`. + +## Goal + +Every account can have a **username**: chosen on the sign up form, changeable on `/account` at most once every 7 days, shown in the navbar in place of the email derived name. + +- 2 to 30 characters, ASCII letters, digits, `_`, `.` and `-`; no spaces; never starting or ending with `.` or `-`. +- Unique ignoring case (`Kris` and `kris` are the same name), stored as typed. +- A short reserved list (`admin`, `bestats`, ...) is refused. +- Not shown to anyone but its owner: there are still no public profiles (section 7). + +## Inspected code + +- `app/(auth)/sign-up/sign-up-form.tsx`, `page.tsx` and their tests: email and password fields, `useActionState(signUpAction)`, field errors through `AuthActionState.fieldErrors`. +- `app/(auth)/actions.ts`, `signUpAction`: + - parses `signUpSchema`, then calls `supabase.auth.signUp`; + - with `NEXT_PUBLIC_AUTH_EMAIL_DELIVERY=off` (production) a session comes back and it redirects to `next`; + - with delivery on, it pads every answer to `SIGN_UP_PAD_MS` (1200 ms). +- `supabase/config.toml`: `enable_confirmations = false`, so a sign up returns a session; Google is not enabled. +- `app/account/page.tsx`: `instant = false`, `requireUser()`, an identity card with Sign out, then `ChangePasswordForm` in its own glass section. `app/account/actions.ts`, `changePasswordAction`: the pattern for an `/account` Server Action (Zod, `getOptionalUser()`, a returned `AuthActionState`, never a redirect). +- `lib/auth/schemas.ts`: the one place for input rules, with constants exported so a test can compare them with the database. +- `lib/auth/action-state.ts` (`AuthField`, `failure`, `fromZodError`, `success`) and `lib/auth/messages.ts` (`AUTH_OUTCOME`, all copy). +- `lib/auth/identity.ts`: `displayName(email)` and `avatarLetter(email)`, pure, used only by `components/layout/account-slot.tsx`. +- `lib/auth/user.ts`: `getOptionalUser` is `cache()`d and JWT only; the navbar slot renders twice (desktop and mobile) inside Suspense in `app/layout.tsx`. +- `supabase/schemas/02-06*.sql`: the declarative source of truth; migrations are written by hand from them. `04-policies.sql` revokes everything from `anon`/`public`/`authenticated` and grants back only what is needed, and narrows every trigger function. +- `supabase/tests/140-security-sweep.test.sql`: in `public`, no `security definer` function, no function `anon` can execute, every table with RLS forced, an owner policy for each of the four commands for `authenticated` only, and a `user_id` that references `auth.users` on delete cascade. +- `app/privacy/page.tsx` ("Account data: your email address, and your password ...") and `lib/legal/operator.ts` (`LEGAL_LAST_UPDATED`). + +## Skills + +- `supabase-postgres-best-practices` and `supabase` for the table, constraints, trigger, function, grants, migration and pgTAP. +- `next-dev-loop` for the running app check. + +## Decisions (settled in grilling, 2026-10-11) + +1. **Where it shows** (Q1): the navbar account button, the avatar letter and the `/account` header use the username; with none, they fall back to `displayName(email)` as today. +2. **Uniqueness** (Q2): unique index on `lower(username)`; stored as typed. +3. **Format** (Q3): `^[A-Za-z0-9_.-]{2,30}$`, not starting or ending with `.` or `-`. ASCII only, so no look-alike names. +4. **Sign up** (Q4): the field is required. +5. **Accounts without one** (Q5): existing accounts and, later, Google accounts are not forced; they keep the fallback and can set one on `/account`. +6. **The weekly lock** (Q6): + - rolling: 7 days (168 hours) after the last change; + - the first set, at sign up or on `/account`, starts the clock; + - saving the identical name is a no-op and does not restart it; + - a case-only change (`kris` to `Kris`) counts as a change. +7. **Enforcement in the database** (Q7, revised in round 3 to fit the security sweep): `profiles` is an ordinary owner-only RLS table. Format, reserved names, uniqueness and the lock are table rules (constraints and a trigger), so they hold for a direct API write too. No `security definer` writer. +8. **Reserved names** (Q8): `admin`, `administrator`, `root`, `support`, `help`, `bestats`, `system`, `moderator`, `staff`, `official`, `tmdb`, `api`, `account`, `null`, `undefined`. Defined once in TypeScript and once in SQL, and a test keeps the two equal. +9. **Sign up order** (Q9, Q16): check availability, then `signUp`, then insert the profile as the new user. No trigger on `auth.users`. +10. **Availability check** (Q10, Q16): on submit only. It needs one named exception to the sweep, `public.username_available(text) returns boolean`, `security definer`, callable by `anon` and `authenticated`, returning only true or false. The sweep lists it by its exact signature as its only exception. That someone can learn a name is in use is accepted. +11. **Unconfirmed accounts** (Q11): they keep their name. Accepted; confirmation is off. +12. **Navbar read** (Q12): one `cache()`d profile read per request inside the existing Suspense boundary. +13. **`/account` form** (Q13): a "Username" section between the identity card and Change password; see Design. +14. **Privacy** (Q14): policy text and `LEGAL_LAST_UPDATED` change in this PR. +15. **Deletion** (Q15): `on delete cascade` from `auth.users` frees the name. +16. **Timing pad** (Q17): a taken username is refused before `signUp`, unpadded. The pad is unchanged for the email address branches. + +## Design + +### Database + +`supabase/schemas/02-tables.sql`: + +```sql +create table public.profiles ( + user_id uuid primary key references auth.users (id) on delete cascade, + username text not null, + username_changed_at timestamptz not null default now(), + created_at timestamptz not null default now(), + updated_at timestamptz not null default now(), + constraint profiles_username_format_check + check (username ~ '^[A-Za-z0-9_.-]{2,30}$' and username !~ '^[.-]|[.-]$'), + constraint profiles_username_reserved_check + check (lower(username) <> all (array['admin', ...])) +); +create unique index profiles_username_lower_key on public.profiles (lower(username)); +``` + +`supabase/schemas/03-triggers.sql`: the existing `set_updated_at` trigger, plus `guard_username_change()`, `before insert or update`, security invoker, empty `search_path`: + +- **insert**: `new.username_changed_at := now()`, whatever the client sent. +- **update, same username** (exact compare): `new.username_changed_at := old.username_changed_at`, so the timestamp can't be rewound by writing it directly. +- **update, different username**: if `old.username_changed_at > now() - interval '7 days'`, raise `username_locked` (`errcode 'P0001'`); otherwise `new.username_changed_at := now()`. +- **`user_id`**: cannot change (`new.user_id := old.user_id`); the update policy's `with check` also pins it to `auth.uid()`. + +`supabase/schemas/04-policies.sql`: + +- RLS enabled and forced. +- Owner policies (`user_id = (select auth.uid())`) for select, insert, update and delete, for `authenticated` only, matching the sweep. +- Grants: `revoke all` from `anon`, `public` and `authenticated`, then `grant select, insert, update` to `authenticated`. **No delete grant**: deleting and re-inserting the row would reset the lock. The delete policy stays so the sweep's shape holds; the privilege layer refuses first. pgTAP proves a delete fails. +- `guard_username_change()` narrowed like the other trigger functions. + +`supabase/schemas/05-functions.sql`: + +```sql +create function public.username_available(p_username text) returns boolean +language sql stable security definer set search_path = '' +as $$ select not exists (select 1 from public.profiles where lower(username) = lower(p_username)) $$; +revoke all on function public.username_available(text) from public; +grant execute on function public.username_available(text) to anon, authenticated; +``` + +It reads only that one column, returns only a boolean, and the sign up action alone calls it. + +Migration `supabase/migrations/20261011120000_account_username.sql`, written by hand from the schema files. It only adds objects, so it is backward compatible: the deployed app ignores the new table. + +`supabase/tests/140-security-sweep.test.sql`: the definer and anon execute assertions exclude exactly `public.username_available(text)`, with a comment saying why. A new assertion fails if that function ever stops returning `boolean`. + +### TypeScript + +- `lib/auth/username.ts` (pure, no `server-only`): + - `USERNAME_MIN = 2`, `USERNAME_MAX = 30`, `RESERVED_USERNAMES`, `USERNAME_CHANGE_INTERVAL_MS`; + - `usernameSchema` (Zod: `trim()`, then the format rules with one message each); + - `usernameUnlocksAt(changedAt): Date` and `isUsernameLocked(changedAt, now): boolean`. +- `lib/auth/schemas.ts`: `signUpSchema` gains `username: usernameSchema`; a new `changeUsernameSchema`. +- `lib/auth/action-state.ts`: `AuthField` gains `"username"`; `values` may carry `username` back too, so a refused sign up keeps what was typed. +- `lib/auth/messages.ts`: + - `usernameTaken`: "That username is taken." Also used for reserved names, so the list isn't revealed one name at a time. + - `usernameLocked`: "You can change your username once a week." + - `usernameUnchanged`: no message, a quiet success. + - `usernameTakenAfterSignUp`: "Your account is ready, but that username was just taken. Choose another." +- `lib/auth/profile.ts` (`server-only`): `getOwnProfile = cache(...)` returns `{ username, usernameChangedAt } | null`. A read error returns `null` and is logged without user data, so the navbar never breaks over it. +- `lib/auth/identity.ts`: `displayName(email, username?)` and `avatarLetter(email, username?)` prefer the username. The "no `profiles` table" comment is rewritten. + +### Sign up + +`signUpAction` order: + +1. Parse. +2. `rpc('username_available')`. If false, `failure(usernameTaken, ["username"])` with email and username kept. An RPC error is reported as `signUpUnavailable`, never treated as available. +3. `signUp`, unchanged. +4. If there is a session: `insert into profiles (user_id, username)`. + - A unique violation (`23505`), from two people choosing the same name at once, redirects to `/account?username=taken`, which shows the `usernameTakenAfterSignUp` notice above an empty, enabled Username field. + - Any other error redirects to `/account` with the field empty; the account exists and the name can be set there. +5. Redirect as today. + +With no session (confirmation on in some other environment), the username is not stored and is set on `/account`; a known limitation, not reachable with today's config. + +The form gets a Username input before the email: + +- `autocomplete="username"`, `autocapitalize="none"`, `spellcheck=false`; +- hint text "2 to 30 characters: letters, numbers, _ . -"; +- `aria-describedby` on the error. + +### /account + +The page reads `getOwnProfile()`. + +- The identity card says "Signed in as **{username}** ({email})", or the email alone when there is no username. +- A new "Username" glass section, the same shell as Change password, holds `ChangeUsernameForm`, a client form on `useActionState(changeUsernameAction)`: + - **No username yet**: an empty, enabled field, "Choose a username" and Save. + - **Unlocked**: the current name prefilled, enabled, with Save. + - **Locked**: the field disabled with the current name, Save disabled, and "You can change it again on {date and time}". The time is formatted in the browser's own time zone by a small client `` that renders the ISO value on the server and formats it after mount, because the server does not know the viewer's zone and a date alone can be hours off. + - **Success**: "Username updated." The form then shows the locked state; the action calls `refresh()` (or `revalidatePath`, whichever the installed Next.js docs say a Server Action uses under `cacheComponents`) so the navbar updates. +- `changeUsernameAction`: + - Zod first, then `getOptionalUser()`; a missing session returns `sessionExpired`, as change password does. + - An upsert on `user_id`. The trigger decides the lock and the no-op. + - `23505` maps to `usernameTaken`, `P0001 username_locked` to `usernameLocked`, a `23514` check violation (a reserved name or a format the Zod check missed) to `usernameTaken` or `invalidInput`, and anything else to the generic unavailable message. + - Never a redirect. + +### Navbar + +`AccountSlot` reads `getOwnProfile()` after `getOptionalUser()` and passes the username into `displayName` and `avatarLetter`. The read sits inside the existing Suspense boundaries, so `/shows` and `/movies` keep their prerendered shells; `app/layout.test.ts` keeps passing because the read lives in the slot. + +### Privacy + +"Account data: your email address, **the username you choose**, and your password ..." `LEGAL_LAST_UPDATED = "2026-10-11"`. No new processor. + +## Expected files + +- `supabase/schemas/02-tables.sql`, `03-triggers.sql`, `04-policies.sql`, `05-functions.sql` +- `supabase/migrations/20261011120000_account_username.sql` +- `supabase/tests/170-profiles-username.test.sql` (new), `140-security-sweep.test.sql`, and `010`/`020` if they list tables by name +- `lib/supabase/database.types.ts` (regenerated with `pnpm db:types`) +- `lib/auth/username.ts` + test, `profile.ts`, `schemas.ts` + test, `action-state.ts`, `messages.ts` + test, `identity.ts` + test, `AGENTS.md` +- `app/(auth)/actions.ts` + test, `app/(auth)/sign-up/sign-up-form.tsx` + test +- `app/account/page.tsx`, `actions.ts` + test, `change-username-form.tsx` (new) + test +- `components/layout/account-slot.tsx`, a small `LocalDateTime` client component +- `app/privacy/page.tsx`, `lib/legal/operator.ts` +- `AGENTS.md` (section 8 user account note, the repo facts line), `docs/scope/scope.md` via `/sync` + +## Requirements + +- R1: a username matches the format, is unique ignoring case, and is not reserved, enforced by the database for any writer. +- R2: a second change within 7 days of the last one is refused by the database; the identical name is a no-op that does not move the clock. +- R3: only the owner can read, insert or update their row; nobody can delete it; `anon` can read nothing. +- R4: `username_available` is the only `security definer` and the only anon callable function in `public`, and it returns only a boolean. +- R5: sign up requires a valid username and refuses a taken one before creating the account. +- R6: `/account` sets, changes and shows the lock state of the username; accounts without one see an empty field. +- R7: the navbar shows the username, or the email name without one. +- R8: Zod and SQL rules (min, max, pattern, reserved list) cannot drift: a test reads both. + +## Security + +- The user id always comes from the session (`auth.uid()` in policies, `getOptionalUser()` in actions), never from form data. +- The availability function reveals only whether a name is in use (accepted, Q10), and sits behind the existing auth rate limit and Vercel Firewall rule for sign up. +- No elevated key is used; the one definer function is read only and narrow. +- Usernames are rendered as text by React, never as HTML; the ASCII pattern rules out look-alike and invisible characters. +- No username, email or token goes into logs. + +## Acceptance criteria + +1. Signing up with `kris_01` creates the account and the profile, and the navbar shows `kris_01`. +2. Signing up with `KRIS_01` afterwards is refused on the Username field with "That username is taken.", no account is created, and the typed email and username are kept. +3. `a`, `has space`, `-dash`, `dot.`, `émile`, a 31 character name and `Admin` are refused by the form; the same values written directly through the API as the owner are refused by the database. +4. On `/account`, a fresh account (name set at sign up) sees the field locked with the unlock date and time; after moving `username_changed_at` back 8 days in the database it can change the name once, then is locked again. +5. Saving the identical name succeeds quietly and does not move the unlock time. +6. An account with no profile row sees an empty field, can set a name immediately, and is then locked. +7. User B cannot read, insert, update or delete user A's profile row through the API; `anon` cannot read the table; nobody can delete their own row. +8. Signed out, `username_available` answers true or false and nothing else. +9. `/shows` and `/movies` are still prerendered (build output), and the navbar falls back to the email name when the profile read fails. +10. `/privacy` lists the username and shows the new last updated date. + +## Automated checks + +- `pnpm typecheck`, `pnpm lint`, `pnpm test`. +- `supabase db reset`, then `pnpm test:db`; the reset comes first, see the pgTAP memory. +- `pnpm db:types:check`. +- `pnpm build`. + +## Remote migration + +The migration only adds objects, so per `docs/deploy.md` it is pushed with `supabase db push` before the PR is merged, once you approve that step. No `config push` is needed. + +## Manual test steps + +Run against the local stack with `pnpm dev:docker`. + +1. Open `/sign-up` and try `a`, `has space` and `admin` as the username. Each is refused under the Username field and keeps what you typed. +2. Sign up with `tester_one` and a new email. You land signed in, and the navbar shows `tester_one` with the avatar letter `T`. +3. Open `/account`. It reads "Signed in as tester_one", and the Username field is disabled with "You can change it again on {date about a week ahead, your local time}". +4. In Studio (or psql), set `username_changed_at = now() - interval '8 days'` for that row and reload `/account`. The field is enabled. +5. Save `Tester_One`. You see "Username updated.", the field locks again and the navbar shows `Tester_One`. +6. Sign out, then sign up with `TESTER_ONE` and another email. You get "That username is taken." and no account is created (check the Auth users list). +7. Delete one account's profile row in Studio and reload `/account` as that user. The field is empty and enabled, and the navbar falls back to the email name. Set a name and it saves and locks. +8. Narrow the window to 375 px and repeat step 3. The section fits, the field and button have 44 px targets, and focus is visible when you tab through. +9. Open `/privacy` and check that the username line and the 2026-10-11 date are there. From d9fc9467321ee361fa1c1025a1c21381577ff189 Mon Sep 17 00:00:00 2001 From: kris1027 Date: Sun, 11 Oct 2026 13:34:35 +0200 Subject: [PATCH 03/18] feat(auth): ask for a username on sign up Co-Authored-By: Claude Opus 5.5 --- app/(auth)/actions.ts | 96 +++++++++++++++++-- app/(auth)/email-delivery.test.ts | 95 +++++++++++++++++- app/(auth)/sign-up/sign-up-form.test.tsx | 37 +++++++ app/(auth)/sign-up/sign-up-form.tsx | 15 +++ components/auth/auth-field.tsx | 25 ++++- lib/auth/AGENTS.md | 4 +- lib/auth/action-state.ts | 28 +++++- lib/auth/identity.test.ts | 13 +++ lib/auth/identity.ts | 19 ++-- lib/auth/log.ts | 2 + lib/auth/messages.ts | 34 +++++++ lib/auth/profile.ts | 51 ++++++++++ lib/auth/schemas.ts | 7 ++ lib/auth/username.test.ts | 87 +++++++++++++++++ lib/auth/username.ts | 90 +++++++++++++++++ .../20261011120000_account_username.sql | 2 +- supabase/schemas/02-tables.sql | 2 +- 17 files changed, 583 insertions(+), 24 deletions(-) create mode 100644 lib/auth/profile.ts create mode 100644 lib/auth/username.test.ts create mode 100644 lib/auth/username.ts diff --git a/app/(auth)/actions.ts b/app/(auth)/actions.ts index 59d3138..b8443c7 100644 --- a/app/(auth)/actions.ts +++ b/app/(auth)/actions.ts @@ -7,12 +7,14 @@ import { failure, fromZodError, keepEmail, + keepUsername, passwordFieldFor, success, } from "@/lib/auth/action-state"; import { isRecoverySession } from "@/lib/auth/identity"; import { AUTH_EVENT, logAuthEvent } from "@/lib/auth/log"; import { + ACCOUNT_NOTICE, AUTH_OUTCOME, RESEND_REQUESTED_MESSAGE, RESET_REQUESTED_MESSAGE, @@ -82,6 +84,57 @@ async function padTo(startedAt: number, floorMs: number): Promise { } } +/** + * Keeps both typed values on a refused sign up, so neither field empties. + */ +function keepTyped( + state: AuthActionState, + formData: FormData, +): AuthActionState { + return keepUsername(keepEmail(state, formData), formData); +} + +/** + * Stores the new account's username and says where to go next + * (prompts/account-username.md, Q9, Q16). + * + * Runs as the new user, under the session `signUp` just created, so row level + * security applies like any other write. The availability check before + * `signUp` makes a refusal here rare: it means someone claimed the same name + * in between. The account exists by then and cannot be undone from here, so + * the person is sent to `/account` to choose another, with a notice when the + * name was taken and without one for any other failure, where the empty field + * says enough. + * + * @returns The path to redirect to. Never throws, so the caller's `redirect()` + * stays outside any `try`. + */ +async function claimUsername( + supabase: Awaited>, + userId: string | undefined, + username: string, + nextPath: string, +): Promise { + if (!userId) { + logAuthEvent(AUTH_EVENT.signUp, "error", AUTH_OUTCOME.unexpected); + return "/account"; + } + + const { error } = await supabase + .from("profiles") + .insert({ user_id: userId, username }); + + if (!error) return nextPath; + + if (error.code === "23505") { + logAuthEvent(AUTH_EVENT.signUp, "refused", AUTH_OUTCOME.usernameTaken); + return `/account?notice=${ACCOUNT_NOTICE.usernameTaken}`; + } + + logAuthEvent(AUTH_EVENT.signUp, "error", AUTH_OUTCOME.unexpected); + return "/account"; +} + /** * Creates an account, then signs in or asks for confirmation (spec 0005 AC-1, * AC-2; spec 0018 AC-11, AC-12). @@ -99,6 +152,7 @@ export async function signUpAction( const startedAt = Date.now(); const parsed = signUpSchema.safeParse({ + username: formData.get("username"), email: formData.get("email"), password: formData.get("password"), next: formData.get("next") ?? undefined, @@ -106,14 +160,36 @@ export async function signUpAction( if (!parsed.success) { logAuthEvent(AUTH_EVENT.signUp, "refused", AUTH_OUTCOME.invalidInput); - return keepEmail(fromZodError(parsed.error), formData); + return keepTyped(fromZodError(parsed.error), formData); } - const { email, password, next } = parsed.data; + const { username, email, password, next } = parsed.data; const nextPath = safeNextPath(next); const emailDelivery = getAuthEmailDelivery(); const supabase = await createClient(); + + // Asked before the account exists, so a taken name never leaves behind an + // account without one. Unpadded: that a name is in use is not a secret the + // plan keeps (prompts/account-username.md, Q10, Q17). An error is never + // read as "available". + const { data: available, error: availabilityError } = await supabase.rpc( + "username_available", + { p_username: username }, + ); + + if (availabilityError) { + logAuthEvent(AUTH_EVENT.signUp, "error", AUTH_OUTCOME.signUpUnavailable); + return keepTyped(failure(AUTH_OUTCOME.signUpUnavailable), formData); + } + + if (!available) { + logAuthEvent(AUTH_EVENT.signUp, "refused", AUTH_OUTCOME.usernameTaken); + return keepTyped( + failure(AUTH_OUTCOME.usernameTaken, ["username"]), + formData, + ); + } const { data, error } = await supabase.auth.signUp({ email, password, @@ -137,7 +213,7 @@ export async function signUpAction( if (error && !addressAlreadyRegistered) { const outcome = classifyAuthError(error); logAuthEvent(AUTH_EVENT.signUp, "refused", outcome); - return keepEmail(failure(outcome, passwordFieldFor(outcome)), formData); + return keepTyped(failure(outcome, passwordFieldFor(outcome)), formData); } if (emailDelivery === "off") { @@ -150,7 +226,7 @@ export async function signUpAction( "refused", AUTH_OUTCOME.alreadyRegistered, ); - return keepEmail( + return keepTyped( { ...failure(AUTH_OUTCOME.alreadyRegistered), link: { @@ -167,17 +243,23 @@ export async function signUpAction( // sends, so refuse plainly and leave the mismatch in the log. if (!data.session) { logAuthEvent(AUTH_EVENT.signUp, "error", AUTH_OUTCOME.signUpUnavailable); - return keepEmail(failure(AUTH_OUTCOME.signUpUnavailable), formData); + return keepTyped(failure(AUTH_OUTCOME.signUpUnavailable), formData); } - redirect(nextPath); + redirect(await claimUsername(supabase, data.user?.id, username, nextPath)); } // Both branches, never one. See SIGN_UP_PAD_MS: padding only the refusal // makes the refusal the slow one and leaks the answer just as loudly. await padTo(startedAt, SIGN_UP_PAD_MS); - if (data.session) redirect(nextPath); + if (data.session) { + redirect(await claimUsername(supabase, data.user?.id, username, nextPath)); + } + + // No session means confirmation is on, and nobody can write the profile + // until there is one. The name is then chosen on `/account`; unreachable + // with today's config (`enable_confirmations = false`). redirect( `/check-email?email=${encodeURIComponent(email)}&next=${encodeURIComponent(nextPath)}`, diff --git a/app/(auth)/email-delivery.test.ts b/app/(auth)/email-delivery.test.ts index 33b8c80..9bf481a 100644 --- a/app/(auth)/email-delivery.test.ts +++ b/app/(auth)/email-delivery.test.ts @@ -19,6 +19,11 @@ const auth = { signInWithPassword: vi.fn(), }; +/** The username check and the profile insert that sign up adds. */ +const rpc = vi.fn(); +const insert = vi.fn(); +const from = vi.fn(); + const createClient = vi.fn(); vi.mock("@/lib/supabase/server", () => ({ @@ -49,6 +54,8 @@ const PASSWORD = "a-long-enough-password"; function form(fields: Record): FormData { const data = new FormData(); + // Sign up needs a username; the other actions ignore the extra field. + data.set("username", "tester_one"); for (const [name, value] of Object.entries(fields)) data.set(name, value); return data; } @@ -68,7 +75,10 @@ const NEW_USER = { id: "user-a", identities: [{ id: "identity" }] }; let warn: ReturnType; beforeEach(() => { - createClient.mockResolvedValue({ auth }); + createClient.mockResolvedValue({ auth, rpc, from }); + rpc.mockResolvedValue({ data: true, error: null }); + insert.mockResolvedValue({ error: null }); + from.mockReturnValue({ insert }); vi.stubEnv("NEXT_PUBLIC_SUPABASE_URL", "http://127.0.0.1:54321"); vi.stubEnv("NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY", "sb_publishable_test"); vi.stubEnv("NEXT_PUBLIC_SITE_URL", "http://localhost:3000"); @@ -381,3 +391,86 @@ describe("the email actions with email delivery on", () => { expect(state.message).toBe(AUTH_MESSAGES[AUTH_OUTCOME.emailNotConfirmed]); }); }); + +/** covers: prompts/account-username.md, R5 (Q9, Q16) */ +describe("signUpAction and the username", () => { + beforeEach(() => { + vi.stubEnv("NEXT_PUBLIC_AUTH_EMAIL_DELIVERY", "off"); + auth.signUp.mockResolvedValue({ + data: { user: NEW_USER, session: SESSION }, + error: null, + }); + }); + + it("stores the username as the new user, then goes on", async () => { + const url = await redirectOf( + signUpAction(IDLE_STATE, form({ email: ADDRESS, password: PASSWORD })), + ); + + expect(url).toBe("/shows"); + expect(rpc).toHaveBeenCalledWith("username_available", { + p_username: "tester_one", + }); + expect(from).toHaveBeenCalledWith("profiles"); + expect(insert).toHaveBeenCalledWith({ + user_id: "user-a", + username: "tester_one", + }); + }); + + it("refuses a taken name on its field before any account exists", async () => { + rpc.mockResolvedValue({ data: false, error: null }); + + const state = await signUpAction( + IDLE_STATE, + form({ email: ADDRESS, password: PASSWORD }), + ); + + expect(state.outcome).toBe(AUTH_OUTCOME.usernameTaken); + expect(state.fieldErrors?.username).toBe("That username is taken."); + expect(state.values).toEqual({ email: ADDRESS, username: "tester_one" }); + expect(auth.signUp).not.toHaveBeenCalled(); + }); + + it("never reads a failed check as available", async () => { + rpc.mockResolvedValue({ data: null, error: { code: "08006" } }); + + const state = await signUpAction( + IDLE_STATE, + form({ email: ADDRESS, password: PASSWORD }), + ); + + expect(state.outcome).toBe(AUTH_OUTCOME.signUpUnavailable); + expect(auth.signUp).not.toHaveBeenCalled(); + }); + + it("sends a name lost to a race to /account with the notice", async () => { + insert.mockResolvedValue({ error: { code: "23505" } }); + + const url = await redirectOf( + signUpAction(IDLE_STATE, form({ email: ADDRESS, password: PASSWORD })), + ); + + expect(url).toBe("/account?notice=username-taken"); + }); + + it("sends any other failed insert to /account without one", async () => { + insert.mockResolvedValue({ error: { code: "08006" } }); + + const url = await redirectOf( + signUpAction(IDLE_STATE, form({ email: ADDRESS, password: PASSWORD })), + ); + + expect(url).toBe("/account"); + }); + + it("requires a username", async () => { + const data = form({ email: ADDRESS, password: PASSWORD }); + data.delete("username"); + + const state = await signUpAction(IDLE_STATE, data); + + expect(state.fieldErrors?.username).toBeDefined(); + expect(rpc).not.toHaveBeenCalled(); + }); +}); diff --git a/app/(auth)/sign-up/sign-up-form.test.tsx b/app/(auth)/sign-up/sign-up-form.test.tsx index 2b507c4..6c5fa37 100644 --- a/app/(auth)/sign-up/sign-up-form.test.tsx +++ b/app/(auth)/sign-up/sign-up-form.test.tsx @@ -48,3 +48,40 @@ describe("SignUpForm's already registered refusal", () => { ).not.toBeInTheDocument(); }); }); + +/** covers: prompts/account-username.md, R5 */ +describe("SignUpForm's username field", () => { + it("asks for a required username and states the rules up front", () => { + render(); + const field = screen.getByLabelText("Username"); + expect(field).toBeRequired(); + expect(field).toHaveAttribute("maxlength", "30"); + expect(field).toHaveAccessibleDescription( + "2 to 30 characters: letters, numbers, _ . -", + ); + }); + + it("marks a taken name on the field and keeps what was typed", async () => { + vi.mocked(signUpAction).mockResolvedValue({ + status: "error", + message: AUTH_MESSAGES[AUTH_OUTCOME.usernameTaken], + outcome: AUTH_OUTCOME.usernameTaken, + fieldErrors: { username: "That username is taken." }, + values: { email: "someone@example.com", username: "tester_one" }, + }); + + render(); + const form = screen + .getByRole("button", { name: "Create account" }) + .closest("form"); + if (!form) throw new Error("the submit button is outside a form"); + fireEvent.submit(form); + + const field = await screen.findByDisplayValue("tester_one"); + expect(field).toHaveAttribute("aria-invalid", "true"); + expect(field).toHaveAccessibleDescription( + expect.stringContaining("That username is taken."), + ); + expect(screen.getByLabelText("Email")).toHaveValue("someone@example.com"); + }); +}); diff --git a/app/(auth)/sign-up/sign-up-form.tsx b/app/(auth)/sign-up/sign-up-form.tsx index 9f24c79..efd68e9 100644 --- a/app/(auth)/sign-up/sign-up-form.tsx +++ b/app/(auth)/sign-up/sign-up-form.tsx @@ -8,6 +8,7 @@ import { } from "@/components/auth/auth-feedback"; import { AuthField } from "@/components/auth/auth-field"; import { type AuthActionState, IDLE_STATE } from "@/lib/auth/action-state"; +import { USERNAME_HINT, USERNAME_MAX_LENGTH } from "@/lib/auth/username"; import { signUpAction } from "../actions"; /** @@ -24,6 +25,10 @@ import { signUpAction } from "../actions"; * as you type. With email delivery off, a submitted taken address is reported * with a link back to sign in (spec 0018, AC-12); with it on, the action masks * that case as AC-2 asks. + * + * The username comes first and is required (prompts/account-username.md). Its + * rules are shown up front for the same reason as the password's, and a taken + * one is reported on submit only, never as you type. */ function SignUpForm({ next }: { next?: string }) { const [state, formAction] = useActionState( @@ -46,6 +51,16 @@ function SignUpForm({ next }: { next?: string }) { ) : null} + + @@ -110,6 +127,12 @@ function AuthField({ ) : null} + {hint ? ( +

+ {hint} +

+ ) : null} + {error ? (

{error} diff --git a/lib/auth/AGENTS.md b/lib/auth/AGENTS.md index c78635e..56e509c 100644 --- a/lib/auth/AGENTS.md +++ b/lib/auth/AGENTS.md @@ -12,7 +12,9 @@ Authentication rules shared by every auth surface: the forms in `app/(auth)/`, ` - `supabase-error.ts`: `classifyAuthError()`, the only code that touches a raw Supabase error. It matches on `error.code` (and `reasons` for weak passwords), never message text. - `log.ts`: `logAuthEvent()`, the only way an auth path writes to the log. - `schemas.ts`: the email and password Zod rules. `MINIMUM_PASSWORD_LENGTH` must equal `minimum_password_length` in `supabase/config.toml`. -- `identity.ts`: pure display derivations (name, avatar letter). Nothing is stored, and there is no `profiles` table. +- `identity.ts`: pure display derivations (name, avatar letter). The username, when there is one, is passed in; otherwise the email name is used. +- `username.ts`: the username Zod rule, reserved list and weekly lock helpers. Must match the `profiles` checks in `supabase/schemas/02-tables.sql` (`username.test.ts` checks). The database is the judge: the trigger `guard_username_change` owns the lock and the timestamp. +- `profile.ts`: `getOwnProfile()`, the `cache()`d read of the signed in account's `profiles` row, used by the navbar slot, `/account` and `changeUsernameAction`. A failed read returns null so the navbar falls back, never breaks. ## Rules diff --git a/lib/auth/action-state.ts b/lib/auth/action-state.ts index f573952..7c55bf4 100644 --- a/lib/auth/action-state.ts +++ b/lib/auth/action-state.ts @@ -15,7 +15,7 @@ import { AUTH_OUTCOME, type AuthOutcome, authMessage } from "./messages"; * the field it belongs to, which is how AC-21 is satisfied without each form * inventing its own error plumbing. */ -export type AuthField = "email" | "password" | "currentPassword"; +export type AuthField = "email" | "password" | "currentPassword" | "username"; export type AuthActionState = { status: "idle" | "success" | "error"; @@ -31,9 +31,10 @@ export type AuthActionState = { * What was typed into the email field, handed back so the form can use it as * `defaultValue`. React 19 resets a form's uncontrolled inputs once its * action finishes, so without this every refusal empties the field. Only the - * email is ever carried; a password must never travel back to the page. + * email and the username are ever carried; a password must never travel + * back to the page. */ - values?: { email?: string }; + values?: { email?: string; username?: string }; /** * A next step the action points to, rendered by the form under its message. * Today only sign up's "already exists" refusal carries one, the way back to @@ -112,7 +113,23 @@ export function keepEmail( formData: FormData, ): AuthActionState { const email = formData.get("email"); - return typeof email === "string" ? { ...state, values: { email } } : state; + return typeof email === "string" + ? { ...state, values: { ...state.values, email } } + : state; +} + +/** + * Carries the submitted username back, for the same reason as `keepEmail`. + * The two compose, so sign up keeps both. + */ +export function keepUsername( + state: AuthActionState, + formData: FormData, +): AuthActionState { + const username = formData.get("username"); + return typeof username === "string" + ? { ...state, values: { ...state.values, username } } + : state; } /** @@ -132,7 +149,8 @@ export function fromZodError(error: z.ZodError): AuthActionState { if ( (field === "email" || field === "password" || - field === "currentPassword") && + field === "currentPassword" || + field === "username") && !fieldErrors[field] ) { fieldErrors[field] = issue.message; diff --git a/lib/auth/identity.test.ts b/lib/auth/identity.test.ts index 5026c63..a5e14c6 100644 --- a/lib/auth/identity.test.ts +++ b/lib/auth/identity.test.ts @@ -101,3 +101,16 @@ describe("the recovery session gate (AC-24)", () => { expect(isRecoverySession(amr)).toBe(false); }); }); + +/** covers: prompts/account-username.md, R7 */ +describe("with a username", () => { + it("prefers the username to the email name", () => { + expect(displayName("kris@example.com", "tester_one")).toBe("tester_one"); + expect(avatarLetter("kris@example.com", "tester_one")).toBe("T"); + }); + + it("falls back to the email name without one", () => { + expect(displayName("kris@example.com", null)).toBe("kris"); + expect(avatarLetter("kris@example.com", undefined)).toBe("K"); + }); +}); diff --git a/lib/auth/identity.ts b/lib/auth/identity.ts index c9e93a9..99732d7 100644 --- a/lib/auth/identity.ts +++ b/lib/auth/identity.ts @@ -7,13 +7,18 @@ * them, which is the difference between covering the rule and covering a * restatement of it. * - * Nothing here is ever stored. That is the whole reason this feature adds no - * `profiles` table: a value that is recomputed every render cannot drift out of - * step with the account it came from. + * The username is the one chosen value (prompts/account-username.md), read + * from `profiles` by the caller and passed in. Everything else is derived from + * the account and recomputed every render, so it cannot drift out of step. */ -/** Shown beside the avatar: the part of the address before the `@`. */ -export function displayName(email: string): string { +/** + * Shown beside the avatar: the username when there is one, otherwise the part + * of the address before the `@`, which is what every account without a + * username keeps (existing accounts and, later, Google ones). + */ +export function displayName(email: string, username?: string | null): string { + if (username) return username; const local = email.split("@")[0]; return local && local.length > 0 ? local : "Account"; } @@ -24,8 +29,8 @@ export function displayName(email: string): string { * Falls back rather than returning an empty string, because an empty circle * reads as a broken control rather than as an unknown account. */ -export function avatarLetter(email: string): string { - return displayName(email).charAt(0).toUpperCase(); +export function avatarLetter(email: string, username?: string | null): string { + return displayName(email, username).charAt(0).toUpperCase(); } /** diff --git a/lib/auth/log.ts b/lib/auth/log.ts index 7c50d14..06d3da0 100644 --- a/lib/auth/log.ts +++ b/lib/auth/log.ts @@ -24,6 +24,8 @@ export const AUTH_EVENT = { requestPasswordReset: "auth.request_password_reset", resetPassword: "auth.reset_password", changePassword: "auth.change_password", + changeUsername: "auth.change_username", + readProfile: "auth.read_profile", callback: "auth.callback", guard: "auth.guard", } as const; diff --git a/lib/auth/messages.ts b/lib/auth/messages.ts index dbe2b3f..4724385 100644 --- a/lib/auth/messages.ts +++ b/lib/auth/messages.ts @@ -49,6 +49,14 @@ export const AUTH_OUTCOME = { accountNotReady: "account_not_ready", /** A resend or recovery email was asked for while none can be sent. */ emailUnavailable: "email_unavailable", + /** + * The username is in use, in any case, or reserved + * (prompts/account-username.md). One outcome for both, so the reserved list + * cannot be read off the form. + */ + usernameTaken: "username_taken", + /** The username was changed less than a week ago. */ + usernameLocked: "username_locked", } as const; export type AuthOutcome = (typeof AUTH_OUTCOME)[keyof typeof AUTH_OUTCOME]; @@ -102,6 +110,8 @@ export const AUTH_MESSAGES: Record = { "Sign up is unavailable right now. Please try again later.", [AUTH_OUTCOME.accountNotReady]: `This account can't sign in yet. Write to ${CONTACT_EMAIL} for help.`, [AUTH_OUTCOME.emailUnavailable]: RECOVERY_UNAVAILABLE_MESSAGE, + [AUTH_OUTCOME.usernameTaken]: "That username is taken.", + [AUTH_OUTCOME.usernameLocked]: "You can change your username once a week.", }; /** @@ -142,3 +152,27 @@ export const SIGN_IN_NOTICES: Record = { export function authMessage(outcome: AuthOutcome): string { return AUTH_MESSAGES[outcome]; } + +/** + * What `/account` shows when sign up created the account but lost its + * username to someone who claimed it a moment earlier + * (prompts/account-username.md, Q16). Looked up by key from the query string, + * like `SIGN_IN_NOTICES`, so the URL can never put its own text on the page. + */ +export const ACCOUNT_NOTICE = { + usernameTaken: "username-taken", +} as const; + +export type AccountNotice = + (typeof ACCOUNT_NOTICE)[keyof typeof ACCOUNT_NOTICE]; + +export const ACCOUNT_NOTICES: Record = { + [ACCOUNT_NOTICE.usernameTaken]: + "Your account is ready, but that username was just taken. Choose another.", +}; + +/** Shown after a username is saved. */ +export const USERNAME_UPDATED_MESSAGE = "Username updated."; + +/** Shown when the name saved is the one already set; nothing changes. */ +export const USERNAME_UNCHANGED_MESSAGE = "That's already your username."; diff --git a/lib/auth/profile.ts b/lib/auth/profile.ts new file mode 100644 index 0000000..20b5d40 --- /dev/null +++ b/lib/auth/profile.ts @@ -0,0 +1,51 @@ +import "server-only"; + +import { cache } from "react"; +import { createClient } from "@/lib/supabase/server"; +import { AUTH_EVENT, logAuthEvent } from "./log"; +import { AUTH_OUTCOME } from "./messages"; +import { getOptionalUser } from "./user"; + +/** The signed in account's own profile (prompts/account-username.md). */ +export type OwnProfile = { + username: string; + usernameChangedAt: Date; +}; + +/** + * Reads the signed in account's profile, or null. + * + * Null covers three cases the callers treat alike: signed out, no username + * chosen yet, and a failed read. The navbar is on every page, so a database + * hiccup must cost the person their username for one render, never the page; + * the failure is logged by class only, with no user data. + * + * Memoized per request with React `cache()`, because the navbar renders two + * account slots and `/account` reads it too. Row level security limits the + * read to the caller's own row; the `user_id` filter only lets the query use + * the primary key. + */ +export const getOwnProfile = cache(readOwnProfile); + +async function readOwnProfile(): Promise { + const user = await getOptionalUser(); + if (!user) return null; + + const supabase = await createClient(); + const { data, error } = await supabase + .from("profiles") + .select("username, username_changed_at") + .eq("user_id", user.id) + .maybeSingle(); + + if (error) { + logAuthEvent(AUTH_EVENT.readProfile, "error", AUTH_OUTCOME.unexpected); + return null; + } + if (!data) return null; + + return { + username: data.username, + usernameChangedAt: new Date(data.username_changed_at), + }; +} diff --git a/lib/auth/schemas.ts b/lib/auth/schemas.ts index 587a652..1eca711 100644 --- a/lib/auth/schemas.ts +++ b/lib/auth/schemas.ts @@ -1,5 +1,7 @@ import { z } from "zod"; +import { usernameSchema } from "./username"; + /** * The password and address rules, defined once (spec 0005, AC-9). * @@ -60,6 +62,7 @@ export const signInSchema = z.object({ }); export const signUpSchema = z.object({ + username: usernameSchema, email: emailSchema, password: newPasswordSchema, next: nextField, @@ -77,3 +80,7 @@ export const changePasswordSchema = z.object({ currentPassword: existingPasswordSchema, password: newPasswordSchema, }); + +export const changeUsernameSchema = z.object({ + username: usernameSchema, +}); diff --git a/lib/auth/username.test.ts b/lib/auth/username.test.ts new file mode 100644 index 0000000..f378043 --- /dev/null +++ b/lib/auth/username.test.ts @@ -0,0 +1,87 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; + +import { + isUsernameLocked, + RESERVED_USERNAMES, + USERNAME_MAX_LENGTH, + USERNAME_MIN_LENGTH, + usernameSchema, + usernameUnlocksAt, +} from "./username"; + +/** covers: prompts/account-username.md, R1, R8 */ +describe("usernameSchema", () => { + it.each(["ab", "kris_01", "Kris.B-1", "a".repeat(30)])( + "accepts %s", + (value) => { + expect(usernameSchema.safeParse(value).success).toBe(true); + }, + ); + + it.each([ + "a", + "a".repeat(31), + "has space", + "tab\there", + "émile", + "kris@home", + ".dot", + "dot.", + "-dash", + "dash-", + "ADMIN", + "bestats", + ])("refuses %s", (value) => { + expect(usernameSchema.safeParse(value).success).toBe(false); + }); + + it("trims the edges before judging", () => { + expect(usernameSchema.parse(" kris ")).toBe("kris"); + }); +}); + +describe("the weekly lock", () => { + const changedAt = new Date("2026-10-11T12:00:00Z"); + + it("unlocks exactly seven days later", () => { + expect(usernameUnlocksAt(changedAt).toISOString()).toBe( + "2026-10-18T12:00:00.000Z", + ); + }); + + it("holds until that moment and not after", () => { + expect(isUsernameLocked(changedAt, new Date("2026-10-18T11:59:59Z"))).toBe( + true, + ); + expect(isUsernameLocked(changedAt, new Date("2026-10-18T12:00:00Z"))).toBe( + false, + ); + }); +}); + +/** + * The database enforces the same rules for every writer. Read the schema file + * and fail if the two copies drift apart. + */ +describe("the SQL rules match", () => { + const sql = readFileSync( + join(process.cwd(), "supabase/schemas/02-tables.sql"), + "utf8", + ); + + it("has the same length bounds", () => { + expect(sql).toContain(`{${USERNAME_MIN_LENGTH},${USERNAME_MAX_LENGTH}}$'`); + }); + + it("has the same reserved names", () => { + const block = sql.match( + /profiles_username_reserved_check[\s\S]*?array\[([\s\S]*?)\]/, + ); + const names = [...(block?.[1] ?? "").matchAll(/'([^']+)'/g)].map( + (match) => match[1], + ); + expect([...names].sort()).toEqual([...RESERVED_USERNAMES].sort()); + }); +}); diff --git a/lib/auth/username.ts b/lib/auth/username.ts new file mode 100644 index 0000000..d1415be --- /dev/null +++ b/lib/auth/username.ts @@ -0,0 +1,90 @@ +import { z } from "zod"; + +/** + * The username rules, defined once on the app side (prompts/account-username.md). + * + * The database enforces the same rules on `profiles` for every writer, so the + * values below exist to give the forms a precise message before a round trip. + * The two copies must stay equal: `lib/auth/username.test.ts` reads the schema file + * and fails if the pattern bounds or the reserved list drift apart, the same + * way the password minimum is checked against `supabase/config.toml`. + * + * Deliberately pure and not `server-only`, so the sign up form, the account + * form and the tests share one copy. + */ +export const USERNAME_MIN_LENGTH = 2; +export const USERNAME_MAX_LENGTH = 30; + +/** A rolling week. The database uses `interval '7 days'`. */ +export const USERNAME_CHANGE_INTERVAL_MS = 7 * 24 * 60 * 60 * 1000; + +/** + * Names nobody may take, compared ignoring case. + * + * Kept short on purpose: it guards against someone posing as the app or its + * operator, not against every word a person might dislike. + */ +export const RESERVED_USERNAMES: readonly string[] = [ + "admin", + "administrator", + "root", + "support", + "help", + "bestats", + "system", + "moderator", + "staff", + "official", + "tmdb", + "api", + "account", + "null", + "undefined", +]; + +/** The hint under both username inputs. */ +export const USERNAME_HINT = `${USERNAME_MIN_LENGTH} to ${USERNAME_MAX_LENGTH} characters: letters, numbers, _ . -`; + +/** + * A username being chosen. + * + * ASCII only, so no two names can look alike and no invisible character can + * hide in one. A reserved name is refused with the same words as a taken one, + * so probing the form does not reveal the list a name at a time. + */ +export const usernameSchema = z + .string() + .trim() + .min( + USERNAME_MIN_LENGTH, + `Your username needs at least ${USERNAME_MIN_LENGTH} characters.`, + ) + .max( + USERNAME_MAX_LENGTH, + `Your username can have at most ${USERNAME_MAX_LENGTH} characters.`, + ) + .regex(/^\S+$/, "Your username can't contain spaces.") + .regex( + /^[A-Za-z0-9_.-]+$/, + "Use only letters, numbers, underscores, dots and dashes.", + ) + .regex(/^[^.-](.*[^.-])?$/, "Your username can't start or end with . or -.") + .refine( + (value) => !RESERVED_USERNAMES.includes(value.toLowerCase()), + "That username is taken.", + ); + +/** + * When a username set at `changedAt` may next be changed. + * + * Rolling, not calendar based, matching the trigger that enforces it. The + * database stays the judge: this only decides what the form shows. + */ +export function usernameUnlocksAt(changedAt: Date): Date { + return new Date(changedAt.getTime() + USERNAME_CHANGE_INTERVAL_MS); +} + +/** Whether the weekly lock still holds at `now`. */ +export function isUsernameLocked(changedAt: Date, now: Date): boolean { + return usernameUnlocksAt(changedAt).getTime() > now.getTime(); +} diff --git a/supabase/migrations/20261011120000_account_username.sql b/supabase/migrations/20261011120000_account_username.sql index 868f964..28cdd70 100644 --- a/supabase/migrations/20261011120000_account_username.sql +++ b/supabase/migrations/20261011120000_account_username.sql @@ -15,7 +15,7 @@ -- -- The rules live on the table, not in the app, so a direct API write obeys -- them too: the two checks hold the format and the reserved names (both kept --- equal to `lib/auth/username.ts` by `username-sql.test.ts`), the index below +-- equal to `lib/auth/username.ts` by `lib/auth/username.test.ts`), the index below -- holds uniqueness ignoring case, and `guard_username_change` in -- `03-triggers.sql` owns `username_changed_at` and the weekly lock. create table public.profiles ( diff --git a/supabase/schemas/02-tables.sql b/supabase/schemas/02-tables.sql index 4e81372..a39a118 100644 --- a/supabase/schemas/02-tables.sql +++ b/supabase/schemas/02-tables.sql @@ -118,7 +118,7 @@ create index user_episode_state_show_order_idx -- -- The rules live on the table, not in the app, so a direct API write obeys -- them too: the two checks hold the format and the reserved names (both kept --- equal to `lib/auth/username.ts` by `username-sql.test.ts`), the index below +-- equal to `lib/auth/username.ts` by `lib/auth/username.test.ts`), the index below -- holds uniqueness ignoring case, and `guard_username_change` in -- `03-triggers.sql` owns `username_changed_at` and the weekly lock. create table public.profiles ( From b52d9652d4f3595df1e5d35a19443997f95f2cce Mon Sep 17 00:00:00 2001 From: kris1027 Date: Sun, 11 Oct 2026 13:34:35 +0200 Subject: [PATCH 04/18] feat(account): set and change the username once a week Co-Authored-By: Claude Opus 5.5 --- app/account/actions.ts | 103 ++++++++++++++- app/account/change-username-action.test.ts | 138 +++++++++++++++++++++ app/account/change-username-form.test.tsx | 40 ++++++ app/account/change-username-form.tsx | 80 ++++++++++++ app/account/page.tsx | 62 ++++++++- components/auth/auth-feedback.tsx | 5 +- components/local-date-time.tsx | 36 ++++++ private-actions-boundary.test.ts | 8 ++ 8 files changed, 467 insertions(+), 5 deletions(-) create mode 100644 app/account/change-username-action.test.ts create mode 100644 app/account/change-username-form.test.tsx create mode 100644 app/account/change-username-form.tsx create mode 100644 components/local-date-time.tsx diff --git a/app/account/actions.ts b/app/account/actions.ts index 0f039f1..515f1ba 100644 --- a/app/account/actions.ts +++ b/app/account/actions.ts @@ -1,15 +1,23 @@ "use server"; +import { refresh } from "next/cache"; + import { type AuthActionState, failure, fromZodError, + keepUsername, passwordFieldFor, success, } from "@/lib/auth/action-state"; import { AUTH_EVENT, logAuthEvent } from "@/lib/auth/log"; -import { AUTH_OUTCOME } from "@/lib/auth/messages"; -import { changePasswordSchema } from "@/lib/auth/schemas"; +import { + AUTH_OUTCOME, + USERNAME_UNCHANGED_MESSAGE, + USERNAME_UPDATED_MESSAGE, +} from "@/lib/auth/messages"; +import { getOwnProfile } from "@/lib/auth/profile"; +import { changePasswordSchema, changeUsernameSchema } from "@/lib/auth/schemas"; import { classifyAuthError } from "@/lib/auth/supabase-error"; import { getOptionalUser } from "@/lib/auth/user"; import { createClient } from "@/lib/supabase/server"; @@ -98,3 +106,94 @@ export async function changePasswordAction( return success("Your password has been changed."); } + +/** + * Sets or changes the signed in account's username + * (prompts/account-username.md, Q6, Q13). + * + * One upsert, and the database decides the rest: the trigger on `profiles` + * refuses a change inside the week, treats the identical name as no change, + * and stamps the time itself; the checks and the unique index refuse a bad, + * reserved or taken name. Deciding the lock here as well would only add a + * second clock to disagree with the first. + * + * The session is resolved here, not trusted from the page, as in + * `changePasswordAction`, and the user id comes only from it. On success + * `refresh()` re-renders the page and the navbar with the new name. + */ +export async function changeUsernameAction( + _previous: AuthActionState, + formData: FormData, +): Promise { + const parsed = changeUsernameSchema.safeParse({ + username: formData.get("username"), + }); + + if (!parsed.success) { + logAuthEvent( + AUTH_EVENT.changeUsername, + "refused", + AUTH_OUTCOME.invalidInput, + ); + // A reserved name fails here with the taken message, so it is reported + // as taken, not as a highlighted field to fix. + return keepUsername(fromZodError(parsed.error), formData); + } + + const user = await getOptionalUser(); + + if (!user) { + logAuthEvent( + AUTH_EVENT.changeUsername, + "refused", + AUTH_OUTCOME.sessionExpired, + ); + return failure(AUTH_OUTCOME.sessionExpired); + } + + // The identical name is no change, and the trigger would keep the clock + // where it was. Saying "updated" for it would be a small lie. + const current = await getOwnProfile(); + if (current?.username === parsed.data.username) { + return keepUsername(success(USERNAME_UNCHANGED_MESSAGE), formData); + } + + const supabase = await createClient(); + const { error } = await supabase + .from("profiles") + .upsert( + { user_id: user.id, username: parsed.data.username }, + { onConflict: "user_id" }, + ); + + if (error) { + const outcome = usernameWriteOutcome(error); + logAuthEvent(AUTH_EVENT.changeUsername, "refused", outcome); + return keepUsername( + failure( + outcome, + outcome === AUTH_OUTCOME.usernameTaken ? ["username"] : [], + ), + formData, + ); + } + + refresh(); + return success(USERNAME_UPDATED_MESSAGE); +} + +/** + * Maps a refused `profiles` write to what the person is told. Matches on the + * SQLSTATE and the trigger's own message, never on free text Postgres writes. + */ +function usernameWriteOutcome(error: { code?: string; message?: string }) { + // 23505: the unique index. 23514: the reserved list (the format is caught + // by Zod first, so a check failure here is a reserved name in practice). + if (error.code === "23505" || error.code === "23514") { + return AUTH_OUTCOME.usernameTaken; + } + if (error.code === "P0001" && error.message === "username_locked") { + return AUTH_OUTCOME.usernameLocked; + } + return AUTH_OUTCOME.unexpected; +} diff --git a/app/account/change-username-action.test.ts b/app/account/change-username-action.test.ts new file mode 100644 index 0000000..953b9ce --- /dev/null +++ b/app/account/change-username-action.test.ts @@ -0,0 +1,138 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { IDLE_STATE } from "@/lib/auth/action-state"; +import { + AUTH_OUTCOME, + USERNAME_UNCHANGED_MESSAGE, + USERNAME_UPDATED_MESSAGE, +} from "@/lib/auth/messages"; + +/** + * The database is the judge of the lock and of uniqueness, so the client is + * replaced and each refusal it can send is played back. + */ +const upsert = vi.fn(); +const from = vi.fn(); +const getOptionalUser = vi.fn(); +const getOwnProfile = vi.fn(); +const refresh = vi.fn(); + +vi.mock("@/lib/supabase/server", () => ({ + createClient: async () => ({ from }), +})); +vi.mock("@/lib/auth/user", () => ({ getOptionalUser })); +vi.mock("@/lib/auth/profile", () => ({ getOwnProfile })); +vi.mock("next/cache", () => ({ refresh })); + +const { changeUsernameAction } = await import("./actions"); + +function usernameForm(username: string): FormData { + const form = new FormData(); + form.set("username", username); + return form; +} + +let warn: ReturnType; + +beforeEach(() => { + from.mockReturnValue({ upsert }); + upsert.mockResolvedValue({ error: null }); + getOptionalUser.mockResolvedValue({ id: "user-a", email: "a@example.com" }); + getOwnProfile.mockResolvedValue(null); + warn = vi.spyOn(console, "warn").mockImplementation(() => {}); +}); + +afterEach(() => { + vi.resetAllMocks(); + warn.mockRestore(); +}); + +/** covers: prompts/account-username.md, R2, R6 */ +describe("changeUsernameAction", () => { + it("saves a valid name for the session's user and refreshes the page", async () => { + const state = await changeUsernameAction( + IDLE_STATE, + usernameForm(" tester_one "), + ); + + expect(state).toMatchObject({ + status: "success", + message: USERNAME_UPDATED_MESSAGE, + }); + expect(from).toHaveBeenCalledWith("profiles"); + expect(upsert).toHaveBeenCalledWith( + { user_id: "user-a", username: "tester_one" }, + { onConflict: "user_id" }, + ); + expect(refresh).toHaveBeenCalled(); + }); + + it.each([ + ["a", "at least 2"], + ["has space", "spaces"], + ["émile", "only letters"], + ["-dash", "start or end"], + ])("refuses %s on the field before any write", async (value, fragment) => { + const state = await changeUsernameAction(IDLE_STATE, usernameForm(value)); + + expect(state.status).toBe("error"); + expect(state.fieldErrors?.username).toContain(fragment); + expect(state.values?.username).toBe(value); + expect(upsert).not.toHaveBeenCalled(); + }); + + it("calls a reserved name taken, so the list is not revealed", async () => { + const state = await changeUsernameAction(IDLE_STATE, usernameForm("Admin")); + + expect(state.fieldErrors?.username).toBe("That username is taken."); + expect(upsert).not.toHaveBeenCalled(); + }); + + it("says nothing changed for the name already set, without writing", async () => { + getOwnProfile.mockResolvedValue({ + username: "tester_one", + usernameChangedAt: new Date(), + }); + + const state = await changeUsernameAction( + IDLE_STATE, + usernameForm("tester_one"), + ); + + expect(state.message).toBe(USERNAME_UNCHANGED_MESSAGE); + expect(upsert).not.toHaveBeenCalled(); + }); + + it.each([ + ["the unique index", { code: "23505" }, AUTH_OUTCOME.usernameTaken], + ["a check", { code: "23514" }, AUTH_OUTCOME.usernameTaken], + [ + "the weekly lock", + { code: "P0001", message: "username_locked" }, + AUTH_OUTCOME.usernameLocked, + ], + ["anything else", { code: "08006" }, AUTH_OUTCOME.unexpected], + ])("reports a refusal from %s", async (_, error, outcome) => { + upsert.mockResolvedValue({ error }); + + const state = await changeUsernameAction( + IDLE_STATE, + usernameForm("tester_one"), + ); + + expect(state.outcome).toBe(outcome); + expect(refresh).not.toHaveBeenCalled(); + }); + + it("writes nothing without a session", async () => { + getOptionalUser.mockResolvedValue(null); + + const state = await changeUsernameAction( + IDLE_STATE, + usernameForm("tester_one"), + ); + + expect(state.outcome).toBe(AUTH_OUTCOME.sessionExpired); + expect(from).not.toHaveBeenCalled(); + }); +}); diff --git a/app/account/change-username-form.test.tsx b/app/account/change-username-form.test.tsx new file mode 100644 index 0000000..d1a72f9 --- /dev/null +++ b/app/account/change-username-form.test.tsx @@ -0,0 +1,40 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +import { ChangeUsernameForm } from "./change-username-form"; + +vi.mock("./actions", () => ({ changeUsernameAction: vi.fn() })); + +/** covers: prompts/account-username.md, R6 (Q13) */ +describe("ChangeUsernameForm", () => { + it("offers an empty field when no username is set", () => { + render(); + expect(screen.getByLabelText("Username")).toHaveValue(""); + expect(screen.getByLabelText("Username")).toBeEnabled(); + expect(screen.getByRole("button", { name: "Save username" })).toBeEnabled(); + }); + + it("prefills the current name while unlocked", () => { + render(); + expect(screen.getByLabelText("Username")).toHaveValue("tester_one"); + expect(screen.getByLabelText("Username")).toBeEnabled(); + }); + + it("disables the field and says when it unlocks while locked", () => { + render( + , + ); + expect(screen.getByLabelText("Username")).toBeDisabled(); + expect( + screen.getByRole("button", { name: "Save username" }), + ).toBeDisabled(); + expect(screen.getByText(/You can change it again on/)).toBeInTheDocument(); + expect(document.querySelector("time")).toHaveAttribute( + "datetime", + "2026-10-18T12:00:00.000Z", + ); + }); +}); diff --git a/app/account/change-username-form.tsx b/app/account/change-username-form.tsx new file mode 100644 index 0000000..095ebd1 --- /dev/null +++ b/app/account/change-username-form.tsx @@ -0,0 +1,80 @@ +"use client"; + +import Link from "next/link"; +import { useActionState } from "react"; +import { + AuthFeedback, + AuthSubmitButton, +} from "@/components/auth/auth-feedback"; +import { AuthField } from "@/components/auth/auth-field"; +import { LocalDateTime } from "@/components/local-date-time"; +import { type AuthActionState, IDLE_STATE } from "@/lib/auth/action-state"; +import { AUTH_OUTCOME } from "@/lib/auth/messages"; +import { USERNAME_HINT, USERNAME_MAX_LENGTH } from "@/lib/auth/username"; +import { changeUsernameAction } from "./actions"; + +/** + * The username form on `/account` (prompts/account-username.md, Q13). + * + * Three states, all decided by the page from the stored profile: no username + * yet (an empty field), unlocked (the current name, editable), and locked (the + * current name, disabled, with when it unlocks). The lock shown here is only a + * courtesy; the trigger on `profiles` is what refuses an early change, and its + * refusal still reaches this form if the two ever disagree. + * + * @param username The current username, or null when none is set. + * @param unlocksAt When the weekly lock ends, as an ISO string, or null when + * it does not hold now. + */ +function ChangeUsernameForm({ + username, + unlocksAt, +}: { + username: string | null; + unlocksAt: string | null; +}) { + const [state, formAction] = useActionState( + changeUsernameAction, + IDLE_STATE, + ); + + const locked = unlocksAt !== null; + + return ( +

+ + + {state.outcome === AUTH_OUTCOME.sessionExpired ? ( + + Sign in again + + ) : null} + + + + {locked ? ( +

+ You can change it again on . +

+ ) : null} + + + Save username + + + ); +} + +export { ChangeUsernameForm }; diff --git a/app/account/page.tsx b/app/account/page.tsx index 777894b..00c11f9 100644 --- a/app/account/page.tsx +++ b/app/account/page.tsx @@ -3,9 +3,13 @@ import type { Metadata } from "next"; import { signOutAction } from "@/app/(auth)/actions"; import { Button, ButtonLink } from "@/components/ui/button"; import { hasPasswordIdentity } from "@/lib/auth/identity"; +import { ACCOUNT_NOTICE, ACCOUNT_NOTICES } from "@/lib/auth/messages"; +import { getOwnProfile } from "@/lib/auth/profile"; import { requireUser } from "@/lib/auth/user"; +import { isUsernameLocked, usernameUnlocksAt } from "@/lib/auth/username"; import { createClient } from "@/lib/supabase/server"; import { ChangePasswordForm } from "./change-password-form"; +import { ChangeUsernameForm } from "./change-username-form"; /** * This route opts out of prerendering (root `AGENTS.md`, `cacheComponents`). @@ -40,8 +44,26 @@ export const metadata: Metadata = { * and spacing unchanged rather than inventing a second look, which is the * approach `AGENTS.md` section 3 asks for and spec 0005 records. */ -export default async function AccountPage() { +export default async function AccountPage({ + searchParams, +}: { + searchParams: Promise<{ notice?: string | string[] }>; +}) { const user = await requireUser(); + const profile = await getOwnProfile(); + const { notice } = await searchParams; + + // Only a known key shows anything, so the URL cannot put its own text here. + const noticeText = + notice === ACCOUNT_NOTICE.usernameTaken + ? ACCOUNT_NOTICES[ACCOUNT_NOTICE.usernameTaken] + : null; + + // Read once per request; the page is dynamic (`instant = false`). + const unlocksAt = + profile && isUsernameLocked(profile.usernameChangedAt, new Date()) + ? usernameUnlocksAt(profile.usernameChangedAt).toISOString() + : null; // The one network call on this page. `getClaims()` has already established // who this is, but the identities list is not in the JWT and AC-16 needs it. @@ -69,7 +91,19 @@ export default async function AccountPage() {

You are signed in as{" "} - {user.email}. + {profile ? ( + <> + + {profile.username} + {" "} + ({user.email}) + + ) : ( + + {user.email} + + )} + .

@@ -80,6 +114,30 @@ export default async function AccountPage() { +
+
+

+ Username +

+

+ {profile + ? "You can change your username once a week." + : "Choose a username. You can change it once a week."} +

+
+ + {noticeText && !profile ? ( +

+ {noticeText} +

+ ) : null} + + +
+ {canChangePassword ? (
diff --git a/components/auth/auth-feedback.tsx b/components/auth/auth-feedback.tsx index c7fc70c..154346d 100644 --- a/components/auth/auth-feedback.tsx +++ b/components/auth/auth-feedback.tsx @@ -48,9 +48,12 @@ function AuthFeedback({ state }: { state: AuthActionState }) { function AuthSubmitButton({ children, pendingLabel, + disabled = false, }: { children: React.ReactNode; pendingLabel: string; + /** Off for a reason other than a pending submit, such as a locked field. */ + disabled?: boolean; }) { const { pending } = useFormStatus(); @@ -59,7 +62,7 @@ function AuthSubmitButton({ type="submit" variant="selected" size="lg" - disabled={pending} + disabled={pending || disabled} className="w-full" > {pending ? pendingLabel : children} diff --git a/components/local-date-time.tsx b/components/local-date-time.tsx new file mode 100644 index 0000000..ed25435 --- /dev/null +++ b/components/local-date-time.tsx @@ -0,0 +1,36 @@ +"use client"; + +import { useEffect, useState } from "react"; + +/** + * A moment shown in the viewer's own time zone (prompts/account-username.md, + * Q13). + * + * The server does not know the viewer's zone, and a date alone can be hours + * off for a lock that ends mid day. So the server renders the moment in UTC, + * labelled as such, and the browser replaces it with local time after mount. + * The `