fix(security): restrict Supabase redirect wildcard to project namespace#195
Closed
EmiyaKiritsugu3 wants to merge 23 commits into
Closed
fix(security): restrict Supabase redirect wildcard to project namespace#195EmiyaKiritsugu3 wants to merge 23 commits into
EmiyaKiritsugu3 wants to merge 23 commits into
Conversation
Q01: aluno-header aria-label 'Perfil do usuario' → 'Menu do aluno' Q02: dashboard achievements justify-around → justify-center Q03: landing page 'Academia Five Star' → 'Five Star Gym' cleanup: remove stale .claude/workflows/ and old audit plan
A01: CardTitle <div> → <h3> semantic heading A02: login email/password autocomplete + type attributes A03: auth errors inline with role=alert (replaces toast) A04: circular-progress SVG viewBox + aria-hidden
U01: card-treino press feedback (active:scale + focus-visible ring) U02: meus-treinos exercise names truncated below count U03: enable Perfil dropdown + /aluno/perfil stub page
…-md buttons P01: align button spec (rounded-full → rounded-md matches code) P02: document Inter/Outfit font split, remove stale SF Pro reference - --font-headline: 'Outfit', 'Inter', sans-serif - Typography table: add dedicated Headlines row (Outfit)
…b/magic-link, middleware bypass Wave 1: AUTH_CALLBACK_PATH constant, middleware excludes /auth/callback Wave 2: signInWithMagicLink, signInWithGoogle, signInWithGitHub Server Actions + WorkoutSession integration on aluno dashboard
/aluno/login: social buttons (inline SVGs) + magic link form + separators /login: social buttons between form and footer
…message extraction
Student + admin login pages now offer Google, GitHub, and Apple All OAuth providers share the same architecture (Server Action → callback)
- .env.example: fix NEXT_PUBLIC_APP_URL port 3000→3001 (prevents auth break) - admin login: replace Globe/Code with brand SVGs (matches student page) - admin login: add setError() in OAuth handlers (was silently swallowing errors) - route.test.ts: 5 tests (code exchange, redirect validation, open-redirect) - auth.test.ts: 9 tests (magic link, Google/GitHub/Apple OAuth)
signInWithGoogle/GitHub/Apple now pass next=/dashboard so callback redirects correctly after authentication
…header - page.test: 'Academia Five Star' → 'Five Star Gym' (matches Q03 brand rename) - aluno-header.test: mock useRouter from next/navigation (UserMenu now uses router for Perfil)
P0: Extract shared `validateNext` to `src/lib/auth-redirect.ts` so the
OAuth callback route and auth server actions cannot drift — normalizes
`next` via `URL.pathname` (defeats `/aluno/../admin` traversal), enforces
path-segment boundaries (`/aluno$` ok, `/alunox` rejected), and blocks
protocol-relative `//` open redirects. Add Sentry.captureException on
every auth failure path (magic-link + 3 OAuth providers + callback
exchange). Add Zod email validation in `signInWithMagicLink`.
P1: Wrap magic-link submit in try/finally so `setMagicLinkLoading(false)`
always runs even if the server action throws (button no longer stuck
disabled). Stack admin social buttons (drop `sm:flex-row`) — `max-w-sm`
card (384px) cannot fit 3 long-label buttons in a row without overflow.
P2: Dedup 3 OAuth handlers per login page into `handleOAuthSignIn` /
`handleOAuthLogin` helpers; admin variant clears stale password error via
`setError('')` before OAuth attempt. Dedup 3 `signInWith{Google,GitHub,Apple}`
server actions into shared `signInWithOAuth(provider, next)`. Rename
stale "Aegis Fitness OS" template brand to "Five Star Gym" in DESIGN.md.
Fix plan doc P01 radius contradiction (spec standardizes `rounded-md`,
not `rounded-full`) and add `npm run e2e` to the gate command.
Tests: 1151 pass, 0 fail. Typecheck clean.
Co-Authored-By: Claude <noreply@anthropic.com>
sfU (account enumeration): the signUp fallback in `handleFormSubmit`
surfaces raw Supabase error messages. When an attacker enters a
registered email with a wrong password, signIn fails with "Invalid
login credentials" and the code falls back to signUp, which throws
"User already registered" — leaking account existence to the caller.
Detect that class of message via regex and surface the same generic
credential error as a failed login, so the error reveals nothing.
se1 (stale dashboard after save): `registrarHistoricoTreinoAction` calls
`revalidatePath('/aluno/dashboard')`, which invalidates the cache but
does not update this already-mounted client component's `aluno` props.
Add `router.refresh()` after a successful save so the RSC payload is
re-fetched and XP/level/streak/treinosNoMes reflect the new state
without a manual reload. Mock `next/navigation` in
`dashboard-client.test.tsx` to satisfy `useRouter`.
Tests: 1151 pass, 0 fail. Typecheck + lint clean.
Co-Authored-By: Claude <noreply@anthropic.com>
- auth-redirect.ts: wrap new URL() in try/catch; malformed next returns '/' instead of 500 (cubic PMD5t P1) - callback/route.ts: strip query string from Sentry callbackUrl to avoid persisting OAuth code in third-party logs (cubic PMD50 P1) - actions/auth.ts: replace silent localhost:3001 fallback with explicit throw when NEXT_PUBLIC_APP_URL unset, read lazily at call time (coderabbit sfi) - .env.example: move NEXT_PUBLIC_APP_URL above NODE_ENV (coderabbit PLDGo) Co-Authored-By: Claude <noreply@anthropic.com>
… + a11y
Address remaining cubic/coderabbit deferred review threads:
- card-treino: focus-visible→focus-within on exercise row divs (non-focusable
container; focus-within fires when child button/link receives keyboard focus)
- .env.example: revert NEXT_PUBLIC_APP_URL to :3000 (matches `next dev` default;
:3001 broke test env + dev parity)
- auth: split OAuth return contract — dedicated `url`/`error` fields replace the
brittle `startsWith('http')` heuristic; client never disambiguates via error
string. Both login pages + auth.test.ts updated to the new contract.
- auth: lazy NEXT_PUBLIC_APP_URL read inside callbackUrl() (not module load) so
test harnesses injecting env after import still work.
- meus-treinos-client.test: assert exercise names render, sliced to 3, joined;
omit name list when 0 exercises (O8Gaq).
- dashboard-client.test: mock @/lib/actions/treinos (unblocks Prisma DATABASE_URL
import-time failure) + WorkoutSession gate; cover WorkoutSession state toggle
(O9FaX), router.refresh() after registrarHistorico success (PMTVI), error path.
- aluno/login.test: account-enumeration guard — "User already registered" and
"User has already been registered" both masked to generic credential error,
no leak, no push (PMTVN).
All 112 test files / 1159 tests pass. Typecheck clean.
Co-Authored-By: Claude <noreply@anthropic.com>
Document final commit 641831d: P2/P3 test coverage (O8Gaq, O9FaX, PMTVI, PMTVN) + a11y (O8GbK) + .env parity (O9Faj) + OAuth url/error contract (O9Fap, O9Fa4). 8 cubic threads resolved, 0 unresolved. 112 files / 1159 tests. Co-Authored-By: Claude <noreply@anthropic.com>
20/21 E2E failures stemmed from selectors resolving to multiple elements
under Playwright strict mode:
- helpers/auth.ts + auth.spec.ts: getByRole('button', {name: /entrar|login/i})
matched 3 buttons (submit + Google + GitHub). Anchored to
/^entrar( no sistema)?$/i to hit only the staff "Entrar no Sistema" and
aluno "Entrar" submit buttons.
- helpers/auth.ts: getByLabel(/senha|password/i) on /aluno/login resolved to
the visibility-toggle button aria-label "Mostrar senha" instead of the
RHF-controlled input (FormLabel has no htmlFor binding). Switched to
input[type="password"] locator, which works on both /login (classic
label) and /aluno/login (RHF).
- student-portal.spec.ts: getByRole('heading', {name:'Meus Treinos'}) hit 2
elements — PageHeader h1 plus a nav/sidebar mirror reusing i18n
myWorkouts. Pinned level:1.
Result: 21/21 pass (1.8m).
Co-Authored-By: Claude <noreply@anthropic.com>
…w/prod) Design for fixing silent magic-link + OAuth auth failures in Vercel Preview/Production. Root causes: NEXT_PUBLIC_APP_URL set to empty string in Vercel Production (empty-string defeats ?? fallback), missing VERCEL_URL tier, supabase/config.toml allowlist mismatches (127.0.0.1 vs localhost, https-on-local, no localhost/vercel.app glob/prod), stale cloud Supabase allowlist. callbackUrl() rewritten to tiered resolution (explicit → VERCEL_URL → localhost), config.toml allowlist fixed, Vercel prod env set. Password login unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
Context7 audit found Fix B glob https://*-*.vercel.app/auth/callback wrong: - Supabase * matches non-separator chars (separators . and /); - is not a separator so * technically spans it, but ambiguous for multi-hyphen preview hostnames (smartmanagementsystem-6rfj2hapr-emiyakiritsugu3s-projects .vercel.app). - ** matches any sequence of chars — documented Supabase-recommended pattern for Netlify/Vercel preview URLs (supabase.com/docs/guides/auth/redirect-urls). - Supabase docs recommend exact paths for production; explicit prod URL retained. Fix E references 'mirror Fix B entries' so cloud allowlist propagates automatically. Co-Authored-By: Claude <noreply@anthropic.com>
…fallback Fix A — callbackUrl() tiered resolution (src/lib/actions/auth.ts): - Tier 1: NEXT_PUBLIC_APP_URL?.trim() — empty string converted to null via explicit || null so ?? falls through to VERCEL_URL. - Tier 2: VERCEL_URL — Vercel auto-injected deploy hostname, prefix https://. - Tier 3: http://localhost:3000 — zero-config local dev default. - Removed throw-on-unset; local dev works without any env vars. Fix B — supabase/config.toml allowlist: site_url → localhost:3000, additional_redirect_urls with https://**.vercel.app/auth/callback glob (** matches any chars per Context7 Supabase docs) + explicit prod URL. Fix F — .env.example: annotated NEXT_PUBLIC_APP_URL with preview/prod guidance (leave unset for VERCEL_URL fallback in preview). Fix G — auth.test.ts: 5 new tier-resolution tests (14 total, all pass) through public signInWithMagicLink API; afterEach cleans VERCEL_URL only. Quality: tsc strict clean, 1164/1164 tests green. Co-Authored-By: Claude <noreply@anthropic.com>
Matched local [auth.email] and [auth.mfa.totp] to cloud production values so supabase config push only changes redirect URLs: - auth.email: enable_confirmations=true, max_frequency=1m0s, otp_length=8 - auth.mfa.totp: enroll_enabled=true, verify_enabled=true Co-Authored-By: Claude <noreply@anthropic.com>
Context7 audit of Supabase official docs (supabase.com/docs/guides/auth/redirect-urls) shows the recommended pattern: process?.env?.NEXT_PUBLIC_VERCEL_URL VERCEL_URL is Vercel's system env var; NEXT_PUBLIC_VERCEL_URL is the Next.js framework-specific equivalent that Supabase explicitly recommends. Changes: - auth.ts: process.env.VERCEL_URL → process.env.NEXT_PUBLIC_VERCEL_URL - auth.test.ts: all VERCEL_URL refs → NEXT_PUBLIC_VERCEL_URL (tests + afterEach) - .env.example: comment updated to NEXT_PUBLIC_VERCEL_URL - spec: code snippet + tier descriptions + Fix D all updated Quality: tsc strict clean, 14/14 auth tests green. Co-Authored-By: Claude <noreply@anthropic.com>
Replaced https://**.vercel.app/auth/callback with https://smartmanagementsystem-*-emiyakiritsugu3s-projects.vercel.app/auth/callback ** glob matched every Vercel deployment worldwide — latent risk if any auth flow ever passes user-influenced redirectTo. Scoped to team namespace: * matches non-sep chars (hash portion), -team.vercel.app is literal suffix. Explicit production URL retained for safety. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (32)
WalkthroughThis PR fixes OAuth/magic-link redirect resolution across environments (new callbackUrl/validateNext logic, auth callback route, Supabase config/env updates), adds a workout session flow to the aluno dashboard, applies aluno UI/accessibility fixes (profile navigation, card heading semantics, exercise list truncation), rebrands landing page text, and adds design/plan documentation. ChangesAuth Redirect & OAuth/Magic-Link Fix
Estimated code review effort: 4 (Complex) | ~60 minutes Aluno Dashboard Workout Session Flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Aluno UI Fixes, Accessibility, and Landing Rebrand
Estimated code review effort: 2 (Simple) | ~15 minutes Documentation Updates
Estimated code review effort: 1 (Trivial) | ~5 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant CallbackRoute as /auth/callback
participant Supabase
participant Sentry
Browser->>CallbackRoute: GET ?code&next
CallbackRoute->>CallbackRoute: validateNext(next)
CallbackRoute->>Supabase: exchangeCodeForSession(code)
alt success
Supabase-->>CallbackRoute: session/cookies
CallbackRoute-->>Browser: redirect(validatedNext)
else failure
CallbackRoute->>Sentry: captureException
CallbackRoute-->>Browser: redirect(/login?error=auth_callback_failed)
end
sequenceDiagram
participant CardTreino
participant DashboardClient
participant TreinosAction as registrarHistoricoTreinoAction
participant Router
CardTreino->>DashboardClient: onStartWorkout()
DashboardClient->>DashboardClient: setTreinoEmSessao(true)
DashboardClient->>DashboardClient: render WorkoutSession
WorkoutSession->>DashboardClient: onFinish(payload)
DashboardClient->>TreinosAction: registrarHistoricoTreinoAction(payload)
TreinosAction-->>DashboardClient: success/error
DashboardClient->>Router: refresh() (on success)
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
Problem
** glob in additional_redirect_urls (**.vercel.app) matches every Vercel deployment worldwide.
Latent risk: if any auth flow ever passes user-influenced redirectTo, tokens could be intercepted.
Fix
https://smartmanagementsystem-*-emiyakiritsugu3s-projects.vercel.app/auth/callback*matches non-separator chars only (the hash portion). Team slug suffix is literal.Only our project's Vercel preview deploys match — not any arbitrary vercel.app subdomain.
Already synced to cloud
supabase config pushapplied to production project (zqrlsupezgltofwotven).Summary by cubic
Restricts Supabase Auth redirects to our Vercel project namespace to prevent token interception. Adds a validated
/auth/callbackflow and safer callback URL resolution for magic-link and OAuth.Bug Fixes
supabase/config.tomladditional_redirect_urlstohttps://smartmanagementsystem-*-emiyakiritsugu3s-projects.vercel.app/auth/callbackplus the explicit production URL; removed the globalvercel.appwildcard./auth/callbackroute and sharedvalidateNexthelper to block open redirects, traversal, and protocol-relative URLs; middleware bypass for this route.NEXT_PUBLIC_APP_URLwithNEXT_PUBLIC_VERCEL_URLand localhost fallbacks..env.exampleupdated with preview/production guidance; auth/login tests and E2E selectors adjusted to the new flow.Migration
NEXT_PUBLIC_APP_URLtohttps://smartmanagementsystem.vercel.appin production; leave unset on previews to useNEXT_PUBLIC_VERCEL_URL.http://localhost:3000is allowlisted.Written for commit 641d57b. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation