Skip to content

refactor: compute login return_to client-side, drop middleware header injection#412

Draft
alukach wants to merge 2 commits into
mainfrom
refactor/client-side-login-drop-middleware
Draft

refactor: compute login return_to client-side, drop middleware header injection#412
alukach wants to merge 2 commits into
mainfrom
refactor/client-side-login-drop-middleware

Conversation

@alukach

@alukach alukach commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Why

The login flow's return_to was built server-side from x-pathname/x-search headers that middleware.ts injected on every matched request. Server components can't read the current path directly, so the middleware stamped it on for getReturnToUrl() to reconstruct. This removes that cross-cutting mechanism by computing return_to client-side instead.

Follow-up to #411 (logout return_to). Discussion: the middleware header injection fed not just the header login button but three server-side redirect guards on the edit pages, so making the button client-side alone couldn't remove it — the guards had to stop redirecting server-side too.

What changed

  • LoginButton (new, client): reads window.location.href at click time to build the login URL — same pattern as the logout button. Used by the header and the interstitial.
  • LoginRequired (new): interstitial shown when an unauthenticated user hits a protected edit page, replacing redirect(loginUrl(await getReturnToUrl())). Reuses StatusPage.
  • StatusPage: added an unauthenticated type and a custom action slot (for the client login button).
  • Edit guards (edit/page.tsx, edit/account/.../layout.tsx, edit/product/.../layout.tsx): unauthenticated → render <LoginRequired /> instead of server-redirect. The isAuthorized checks (403/404) are unchanged — only the unauthenticated presentation changed.
  • getReturnToUrl() deleted (all callers gone); getBaseUrl() kept.
  • middleware.ts: dropped the x-pathname/x-search injection — the fallthrough is now a plain NextResponse.next(). (Ory proxying, legacy redirects, and the devtools 404 are untouched.)
  • Removed a stale unused params/SettingsPageProps from edit/page.tsx while there.

Behavior change

A logged-out user who lands directly on an edit page now sees a "Sign in required" page with a Sign in button (one extra click) instead of an instant redirect to login. After login they still return to the edit page. The common case (navigating to edit while already logged in) is unchanged.

Verification

  • tsc --noEmit — clean
  • next lint on touched files — clean
  • next build — compiles, type-checks, lints, and generates static pages; only fails prerendering /(marketing)/page because getFeaturedProducts can't reach AWS in the sandbox (no valid credentials). Unrelated to this change.

Note: Ory's selfservice.allowed_return_urls must permit same-origin URLs (already required by the existing login flow), so no Ory config change needed.

🤖 Generated with Claude Code

… injection

The login button and the edit-page auth guards built their `return_to`
server-side from `x-pathname`/`x-search` request headers that middleware
injected on every matched request. This replaces that with client-side
computation, removing the middleware header injection entirely.

- Add `LoginButton` (client): reads `window.location.href` at click time to
  build the login URL, mirroring the logout button.
- Add `LoginRequired` interstitial (reuses `StatusPage`) shown when an
  unauthenticated user hits a protected edit page, instead of a server-side
  redirect-to-login. The authorization checks (isAuthorized → 403/404) are
  unchanged.
- `StatusPage`: add an `unauthenticated` type and a custom `action` slot.
- Header login button now renders `<LoginButton />` (no server-side return_to).
- Delete `getReturnToUrl()` (its only callers are gone) and stop the
  middleware from stamping x-pathname/x-search on every request.

Cost: one extra click when a logged-out user lands directly on an edit page.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
source-cooperative Ready Ready Preview, Comment Jul 2, 2026 6:01am

Request Review

@claude

claude Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Claude finished @alukach's task in 2m 44s —— View job


Review complete

  • Gather context (PR body, changed files list)
  • Fetch diff against origin/main
  • Review each changed file for correctness/security issues
  • Post findings

✅ No blocking issues — safe to merge.

The refactor is clean: auth checks (isAuthorized/notFound) are untouched, all layouts still bail out before fetching/rendering any protected data when unauthenticated, and no dangling references to the removed getReturnToUrl/x-pathname/x-search mechanism remain. Using window.location.href client-side for return_to is actually a slight security improvement over the old approach (no reliance on proxy-supplied host/x-forwarded-* headers).

Two minor, non-blocking notes:

  • src/components/core/LoginButton.tsx:25-29 — the cached href only refreshes on pathname change (usePathname()), not on query-string-only navigation. Several pages update useSearchParams()-based state without changing the pathname (e.g. src/components/features/products/ProductsFilters.tsx, Pagination.tsx). If a user filters/paginates on /products and then clicks "Log In / Register" in the persistent header without a full pathname change, return_to reflects the URL at last mount/navigation rather than the current query string, so post-login they land back on the page without their filters. Very low impact, but if you want it exact, use a useSearchParams()-derived key (or re-run the effect on search too) instead of pathname alone:
    const pathname = usePathname();
    const searchParams = useSearchParams();
    useEffect(() => {
      setHref(loginUrl(window.location.href));
    }, [pathname, searchParams]);
  • src/components/core/LoginButton.tsx:17 — the doc comment contains a stray/nonsensical word ("ponytail:") that appears to be a leftover typo; worth cleaning up before merge for clarity:
    * ponytail: before mount the href is the bare login URL, so a click in that
    
    should presumably just read Caveat: or similar.

Fix this →

…href

Use a real <a> (via Button asChild) so the sign-in control behaves like a
link — open-in-new-tab, middle-click, keyboard. A plain anchor (not
next/link) forces the full-page navigation the Ory login flow needs
(external domain in prod, middleware-proxied relative path in dev).

return_to is set to the current absolute URL after mount and refreshed on
navigation via usePathname, since the button lives in the persistent header.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant