Repository navigation
feat(auth): real accounts — gate the app behind Supabase Auth (008) - #28
Merged
Merged
Conversation
Closes the anonymous-only session model. Every route except /signin now requires a permanent account; sessions come from Supabase Auth email/password rather than signInAnonymously(). - config: enable_anonymous_sign_ins = false, minimum_password_length = 8, and site_url fixed (was 127.0.0.1:3000, but vite dev serves 5173 and preview 4173 — confirmation redirects would have landed nowhere). - $lib/auth: thin wrapper over Supabase Auth (signUp / signInWithPassword / signOut / reset / resend), error→product-voice mapping, and client-side pre-checks. No hashing, tokens or storage of our own (INV-SEC-001). - session.svelte.ts now observes rather than creates a session; ensureSession() → currentUser() across the six data services. - /signin wall + /account (email, password change, sign-out). The layout guard remembers the intended path in ?next= and refuses off-site targets. - No email-confirmation callback route: supabase-js reads the session from the redirect URL fragment (detectSessionInUrl defaults to true). signUp() handles both the auto-confirm and check-your-inbox outcomes. Also fixes a raw NUL byte in shoppingListIntegration.ts's mergeKey template literal (now \0), which made the file read as binary and silently excluded it from grep/rg. Tests: 324 unit (49 new), 40 e2e (11 new auth specs); the four existing suites now sign in first. Refs #19
…ration Follow-up to 21bb654, from a multi-agent review of the 008 branch. - signOut() now uses scope 'local'. supabase-js defaults to 'global', which revokes refresh tokens for every session on the account, so signing out on a phone silently signed the user out of their laptop too — the opposite of what the sign-up screen promises. - signUp() no longer strands a duplicate sign-up. With confirmations on, Supabase answers an existing address with a 200 carrying an obfuscated user (no identities, no session, no mail sent); reported as "confirmation-required" that told the user to wait for an email nobody sent. Now detected explicitly and folded together with the confirmations-off error path, so behaviour no longer changes with project configuration. - Sign-up no longer reveals whether an address is registered, matching the stance the reset flow already took. Duplicate outcomes render identically to new sign-ups, the enumerating strings are gone from authErrors, a failed resend is acknowledged neutrally, and authErrorMessage refuses to pass Supabase's raw "User already registered" text through. - Expired sessions recover instead of dead-ending. A non-terminal refresh failure left the client holding a stale session with no SIGNED_OUT: the UI looked signed in while every request failed 401/PGRST301, mapped to generic "please try again" copy that retrying could never fix. A fetch wrapper in supabaseClient clears the session on a non-auth 401, so the layout guard routes to /signin?next=<path> and the user resumes where they left off. - The ?next= guard parses instead of prefix-matching. `/\evil.com` passed startsWith('/') && !startsWith('//') and resolved to a foreign origin; SvelteKit refused the navigation, but the rejection was voided, stranding a signed-in user on the sign-in wall. Both goto calls now report failures. - restoreSession() catches a thrown getSession(), which previously escaped as an unhandled rejection and made services throw raw Errors instead of their own domain errors, defeating instanceof checks downstream. - Auth failures are logged with their cause. None of the four catch blocks recorded anything, so the least-recognised errors were also the least diagnosable in production. - seed-dev: --create-user is create-or-reuse by email rather than a silent no-op whenever any account already exists, and the script refuses to run against a non-localhost host, since it provisions a known-password account. - Dead code removed (isSignedIn, currentUserId), sessionState exported readonly, and two comments corrected that described the opposite of what the code does. Gates: svelte-check 0 errors/0 warnings, prettier + eslint clean, 333 unit tests, 40 e2e. Claude-Session: https://claude.ai/code/session_017nHLGE2QXdBnW5tLxUotrC
The workflow was succeeding while posting nothing. Without `track_progress`, a `prompt` on a pull_request event runs the action in agent mode, which provisions no GitHub comment-posting tools. The review ran to completion (14 turns, ~15 min) and then every attempt to publish it was denied — run 30229845477 on PR #28 ended with permission_denials_count: 69 and "No buffered inline comments", leaving zero comments on the PR. `track_progress: true` forces tag mode, which creates the tracking comment and preserves GitHub context. This matches the working workflows in lattice, spacelab, and spacelab-hud. Also bumps actions/checkout v4 -> v6 to clear the Node 20 deprecation warning. ci.yml and claude.yml still pin v4. Claude-Session: https://claude.ai/code/session_017nHLGE2QXdBnW5tLxUotrC
Clears the Node 20 deprecation warning in the three ci.yml jobs and in claude.yml, matching the bump already applied to claude-code-review.yml. No actions/checkout@v4 references remain. Claude-Session: https://claude.ai/code/session_017nHLGE2QXdBnW5tLxUotrC
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Feature 008: the app is gated behind real Supabase Auth. Two commits — the feature, then fixes from a multi-agent review of it.
What ships
src/routes/+layout.svelteredirects to/signinuntil a session exists, remembering the intended path in?next=. Rendering is gated onreadyso a returning user never sees a flash of the wall while the session restores.$lib/auth— a thin wrapper over Supabase Auth. Every credential operation delegates to supabase-js; no hand-rolled hashing, tokens or storage (INV-SEC-001).authErrorstranslatesAuthErrorinto the product's calm voice, preferringcodeover message text.$lib/sessionnow only observes the session.ensureSession()(which minted anonymous sessions) is replaced bycurrentUser()across all six data services; anonymous sign-ins are off inconfig.toml.AuthForm/AccountPanel— sign in, sign up, password reset, password change, sign out.detectSessionInUrl), so confirmation and recovery links only needsite_url/additional_redirect_urlspointed at the app.Review fixes (second commit)
Seven review agents ran over the feature commit. The findings worth acting on:
scope: 'global', revoking refresh tokens for every session on the account — signing out on a phone signed you out of your laptop, contradicting the sign-up screen's "keeps everything in sync across your devices". Nowscope: 'local'.confirmation-required, that told people to wait for an email nobody sent. Production-only — locallyenable_confirmations = falsereturns a real error, so the whole test suite exercised the working path.SIGNED_OUT: the UI looked signed in while every request failed 401/PGRST301 and mapped to generic "please try again" copy that retrying could never fix. A fetch wrapper clears the session on a non-auth 401 so the guard routes to/signin?next=<path>.?next=guard was string-based./\evil.compassedstartsWith('/') && !startsWith('//')and resolved to a foreign origin. Not an exploitable open redirect — SvelteKit refuses off-origin navigation — but the rejection wasvoided, stranding a signed-in user on the sign-in wall with no error. Now parsed and origin-compared, with bothgotocalls reporting failures.restoreSession()catches a throwngetSession();--create-useris create-or-reuse rather than a no-op when any account exists, and the seed script refuses non-localhost hosts; dead code removed;sessionStateexported readonly; two inverted comments corrected.One correction worth recording: an earlier reported "broken password recovery" was not a bug. GoTrue's
IsRedirectURLValidmatchessite_urlon scheme + hostname + port and never compares the path, so${origin}/accountis accepted and the allow-list is never consulted.Verification
svelte-check0 errors / 0 warnings (543 files) · prettier + eslint clean · 333 unit tests · 40 e2e.Not included
Specs for
session.svelte.tsandAccountPanel.svelte, and an e2e drivingPASSWORD_RECOVERYthrough to a completed password change — the largest remaining coverage gaps. Production also needs SMTP configured in Supabase before confirmations are switched on, or confirmation mail never sends.https://claude.ai/code/session_017nHLGE2QXdBnW5tLxUotrC