The sign-in redirect URL is built by hand in five places

Aperta
  • Qualcosa non va?
#816 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
55/100
Tipo di issue
Refactoring
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
next.js, typescript
Ambito
authentication, backend-api-design

Direzione di ricerca

Read src/auth/callback-url.ts and the five call sites named in the issue, then check the three listed e2e specs and the existing test setup. Add and test the shared URL helper, replace the five literals, and run the listed lint, format, typecheck, knip, and test commands. Done means no hand-built /login?callbackUrl= remains in src and the e2e specs pass unchanged.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

enhancement

What happens

Five places write the same template literal, /login?callbackUrl=${encodeURIComponent(x)}, with no shared helper:

  • requireOrganizerSession in src/auth/session.ts
  • the consent page src/app/oauth/consent/[uid]/page.tsx, twice: the signed-out redirect and switchAccount
  • src/app/oauth/consent/[uid]/decision/route.ts
  • switchAccount in src/components/signup/SignupLoadError.tsx

The two "Sign in as someone else" actions also each explain the same reason in their own words: plain /login would bounce a signed-in organizer straight back to /app, so they sign out first. src/auth/callback-url.ts already owns the callback rules and is the natural home for the URL. Nobody using OpenSignup would notice a fix.

Fix, in outline

  • Add loginUrl(callbackUrl: string) to src/auth/callback-url.ts. It builds the URL and nothing else: don't call safeCallbackUrl inside it. The callers already do that where it's needed, and the consent paths are pre-validated.
  • Optionally, add switchOrganizer(callbackUrl) in src/auth/sign-out.ts, which calls signOutOrganizer(loginUrl(callbackUrl)) and carries the shared "why sign out first" comment once. Each switchAccount keeps only its site-specific note.
  • Replace the five literals. redirect() and signOutOrganizer() calls stay where they are.

Watch for

  • This touches src/auth and the OAuth consent pages, so the PR waits for the owner even though output is byte-identical. A scratch test showed the helper matches all five current expressions for every input tried: paths with queries, unicode, already-encoded input, and 128-character consent uids.
  • The e2e specs that assert the URL (tests/e2e/organizer.spec.ts, login-code.spec.ts, oauth-consent.spec.ts) must pass unchanged.
  • The net line count is about zero; the gain is one owner for the URL shape.

Done when

  • No hand-built /login?callbackUrl= string is left in src.
  • A unit test pins loginUrl.
  • pnpm lint && pnpm format:check && pnpm typecheck && pnpm knip && pnpm test pass.

Found by the pipeline's refactor scout in sign-in and OAuth (src/auth, src/app/oauth), at c1e1cad. Checked by an independent verify agent, which grepped every callbackUrl= and /login in src and tests and ran a scratch vitest comparing a shared helper with each copy. https://claude.ai/code/session_016YZ9ST4ox4Tmm6UZcobKgf

Held back by the scout's cap. Add pipeline:ready to queue it.

Lingua principale
TypeScript
Stelle
19
Fork
4
Merge medio
19h 15m
PR unite (30g)
205

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di richshaw/OpenSignup

Tutte le issue di richshaw/OpenSignup

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.