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

オープン
  • 何か問題がありますか？
#816 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
3/5
見積もり時間
1〜2日
初心者へのやさしさ
55/100
issue の種類
リファクタリング
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
next.js, typescript
領域
authentication, backend-api-design

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

説明

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.

主要言語
TypeScript
スター
19
フォーク
4
平均マージ
19時間 15分
マージ済み PR（30日）
205

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

richshaw/OpenSignup のほかの issue

richshaw/OpenSignup の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。