Repository navigation
fix: return to the chosen plan's checkout after sign-in - #29
Conversation
Sign-in is about to accept a `next` destination so people return to
where they started (e.g. a plan's checkout) instead of always landing
on /app. Because that value comes from the URL, it must never be able
to send users to another site.
safeNextPath() accepts only root-relative, same-site paths and returns
/app (DEFAULT_NEXT) for anything else:
- non-strings, empty values and values over 512 characters
- paths that don't start with "/"
- protocol-relative forms ("//host" and "/\host"), which browsers treat
as another origin
- anything that resolves to a different origin when parsed
The path, query and hash are kept, so /pricing?checkout=team survives.
Checks that same-site paths keep their query string, and that absolute URLs, protocol-relative "//" and "/\" forms, javascript: URLs, relative paths, non-strings and over-long values all fall back to /app.
GitHub sign-in is a round trip through github.com, so the destination has to survive it. Store it in a short-lived cookie: - setSignInNext(next) validates the path with safeNextPath() and stores it in jargons_sign_in_next (HttpOnly, SameSite=Lax, 10 minute max-age, Secure in production). Lax is enough because the cookie only needs to come back on the top-level redirect from GitHub. When the destination is just the default /app, any stale cookie is cleared instead. - consumeSignInNext() reads the destination, validates it again, and deletes the cookie so it is used only once. It returns /app when nothing was stored.
/auth/github/start now reads the optional `next` query parameter and hands it to setSignInNext() before redirecting to GitHub, so the destination is remembered across the OAuth round trip. Invalid values are ignored and sign-in falls back to /app.
After a successful GitHub sign-in the callback now redirects to the destination from consumeSignInNext() instead of always going to /app. With no stored destination the behaviour is unchanged. Error handling still sends users back to the sign-in page.
GitHubAuthButton takes an optional `next` prop and appends it (URL-encoded) to /auth/github/start. Without `next` the link is unchanged. The value is validated on the server, not here.
New buyers who click "Create an account" from sign-in should still come back to the plan they picked. The sign-up route now accepts an optional `next` search param, passes it to the GitHub button, and keeps it on the "Sign in" link.
The sign-in route now accepts an optional `next` search param alongside `error`, forwards it to the GitHub button, and keeps it on the "Create an account" link so switching to sign-up doesn't lose it.
Previously a signed-out visitor who clicked "Sign in to subscribe" was sent to /auth/sign-in and, after signing in, landed on /app with no sign of the plan they had picked, so the purchase was easy to abandon. - Signed-out clicks (and a 401 from the checkout API) now go to /auth/sign-in?next=/pricing?checkout=<plan>. - The pricing route accepts `?checkout=<plan>`, validated with isPaidPlan(). When it is present and the user is signed in on the free plan, the page starts that plan's checkout straight away. - The param is removed from the URL (history replace) before the checkout starts, so pressing Back from the Bachs checkout does not start it again. A ref guards against running it twice. - Signed-out visitors and workspaces already on a paid plan just see the pricing page; nothing is started automatically.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![jargons-ai[bot]](https://avatars.githubusercontent.com/in/4369042?s=60&v=4){kind=small}
There was a problem hiding this comment.
Jargons review
Found 2 issues worth a look:
🟡 Medium — Potential Open Redirect in signInThenCheckout
src/routes/pricing.tsx:29
The signInThenCheckout function constructs a next URL parameter using encodeURIComponent(next). While safeNextPath is used on the server to validate the next parameter, the client-side construction of the next parameter in signInThenCheckout could potentially be manipulated if target itself is not strictly controlled or validated before being passed to signInThenCheckout. An attacker could craft a malicious target value that, even after encodeURIComponent, could lead to an open redirect if the server-side safeNextPath has any bypasses or if the target is used in other contexts without proper validation.
Suggestion: Ensure that the target parameter passed to signInThenCheckout is always a strictly controlled and validated value, ideally an enum or a value derived from a trusted source. While safeNextPath provides server-side protection, client-side validation or stricter input control for target would add defense in depth.
🟡 Medium — Cookie maxAge for signInNextCookieName is too long
src/server/github-auth.ts:287
The signInNextCookieName cookie is set with a maxAge of 10 minutes (60 * 10). This cookie is intended to remember the post-sign-in destination for a single round trip. A 10-minute maxAge is longer than typically necessary for this purpose and could potentially allow an attacker to reuse a next value if they intercept the cookie before it's consumed, although the sameSite: 'lax' and httpOnly: true attributes mitigate some risks. A shorter maxAge would reduce the window of opportunity for such an attack.
Suggestion: Reduce the maxAge for the signInNextCookieName cookie to a shorter duration, such as 60 seconds (1 minute), which should be sufficient for the GitHub authentication round trip. This minimizes the time the cookie is valid and reduces potential exposure.
🔧 Suggested fixes: open the fix PR →
Summary
Signed-out visitors who clicked Sign in to subscribe ended up on
/appafter signing in, with no sign of the plan they picked. Now they come straight back into that plan's checkout.How it works
/auth/sign-in?next=/pricing?checkout=<plan>/app/pricing?checkout=<plan>starts that plan's checkout if you're signed in and on Free, then removes the param so Back from Bachs doesn't restart itThe destination is kept through the sign-up page too, so brand-new users get the same flow.
Security
nextonly accepts paths on this site (safeNextPath). Absolute URLs,//host,/\hostandjavascript:all fall back to/app.