Stop redirecting to the address the process bound to - #821
Merged
Conversation
Reported from the iOS side, iOS #96. `GET /api/auth/native/login` answers location: https://0.0.0.0:3000/auth/login?flow=native behind a proxy that does not pass `Host` through. `NextResponse.redirect` needs an absolute URL, the route built one with `new URL(path, req.url)`, and `req.url` there is the address the Node process bound to. That is the primary onboarding path. ASWebAuthenticationSession follows the redirect, iOS refuses the restricted port, and the person lands on a browser error page with no way forward. Distributed builds carry no associated domains on purpose, so the browser handoff is also how passkeys are meant to work; without it a new account is down to a password and a confirmation code. Two things hid it. Every error branch of that route builds the fixed healthlog:// scheme with no host at all, so the route behaved correctly on every failure and broke only when it was supposed to succeed. And its own test built a request whose host matched, so the absolute URL it produced looked right. The assertion there used toContain, which is satisfied by the broken value just as happily as the correct one. The same line was in six more places: the error redirects of the Withings, WHOOP and Fitbit connect routes, which send a browser to /settings/integrations. Behind the same proxy those land on the same unreachable address, so all seven move to a relative Location. A relative Location is valid per RFC 7231 and the client resolves it against the address it actually called, which is the public one by definition. The alternative is reading X-Forwarded-Host, and that would put one more proxy setting between a self-hoster and a working sign-in. Three checks, as the report asked for. The route test now drives a request whose URL names the bind address, which is what a proxy hop looks like from inside the handler. A structural test freezes the pattern out of the four files and, separately, requires each of them to still reach for the helper, because a file that simply deleted its redirect would pass the first check and fail its users. The helper refuses a protocol-relative value, which is a host in disguise. Verified by breaking it: putting the old call back fails four checks, the last of them reporting the reported value verbatim. Refs iOS #96.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported from the iOS side as iOS #96, reproduced in the code.
GET /api/auth/native/loginanswersbehind a proxy that does not pass
Hostthrough.NextResponse.redirectneeds an absolute URL, the route built one withnew URL(path, req.url), andreq.urlthere is the address the Node process bound to.Why it matters more than it looks
That is the primary onboarding path.
ASWebAuthenticationSessionfollows the redirect, iOS refuses the restricted port, and the person lands on a browser error page with no way forward. Distributed builds deliberately carry no associated domains, so the browser handoff is also how passkeys are supposed to work. Without it a new account is down to a password and a confirmation code.Why nothing caught it
Every error branch of that route builds the fixed
healthlog://scheme with no host at all. So the route behaved correctly on every failure and broke only when it was supposed to succeed. The reporter noticed exactly this: an invalidcode_challengebounces back cleanly.Its own test built a request whose host matched, so the absolute URL looked right, and the assertion was
toContain("/auth/login?flow=native")which the broken value satisfies just as happily as the correct one.The same line was in six more places
The error redirects of the Withings, WHOOP and Fitbit connect routes send a browser to
/settings/integrations. Behind the same proxy they land on the same unreachable address. All seven now use a relativeLocation.That is valid per RFC 7231 and the client resolves it against the address it actually called, which is the public one by definition. The alternative is reading
X-Forwarded-Host, which would put one more proxy setting between a self-hoster and a working sign-in.Three checks, as the report asked for
The route test now drives a request whose URL names the bind address, which is what a proxy hop looks like from inside the handler. A structural test freezes the pattern out of the four files and, separately, requires each of them to still reach for the helper, because a file that simply deleted its redirect would pass the first check and fail its users. The helper refuses a protocol-relative value, which is a host in disguise.
Verified by breaking it: putting the old call back fails four checks, the last reporting
http://0.0.0.0:3000/auth/login?flow=nativeverbatim.Refs iOS #96.