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

Đang mở
  • Có gì không ổn?
#816 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức phù hợp với người mới
55/100
Loại issue
Tái cấu trúc
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
next.js, typescript
Lĩnh vực
authentication, backend-api-design

Hướng nghiên cứu

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.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

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.

Ngôn ngữ chính
TypeScript
Star
19
Fork
4
Merge trung bình
19 giờ 15 phút
Pull request đã merge (30 ngày)
205

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của richshaw/OpenSignup

Tất cả issue của richshaw/OpenSignup

Issue tương tự

Thêm issue về TypeScript

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.