fix(auth): use 127.0.0.1 for OAuth loopback and surface --no-localhost hint - #11000
fix(auth): use 127.0.0.1 for OAuth loopback and surface --no-localhost hint#11000Prajeeth-12 wants to merge 2 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request updates the local authentication server to bind explicitly to 127.0.0.1 instead of localhost and introduces a 30-second timeout that displays troubleshooting instructions if authentication is delayed. The review feedback correctly points out that the setTimeout timer is not cleared when the server closes, which could keep the Node.js event loop active or trigger unexpected warnings. It is recommended to capture the timeout ID and clear it on the server's close event.
| setTimeout(() => { | ||
| if (server.listening) { | ||
| logger.info(); | ||
| logger.info( | ||
| "Having trouble? Try " + | ||
| clc.bold("firebase login --no-localhost") + | ||
| " or " + | ||
| clc.bold("firebase login:ci"), | ||
| ); | ||
| } | ||
| }, 30_000); |
There was a problem hiding this comment.
The setTimeout timer is not cleared when the server closes (either upon successful login or failure). This can keep the Node.js event loop active, preventing the CLI process from exiting cleanly, or it can cause the warning message to be printed unexpectedly in long-running programmatic environments (like the VS Code extension) long after the login has completed.
We should capture the timeout ID and clear it when the server is closed.
| setTimeout(() => { | |
| if (server.listening) { | |
| logger.info(); | |
| logger.info( | |
| "Having trouble? Try " + | |
| clc.bold("firebase login --no-localhost") + | |
| " or " + | |
| clc.bold("firebase login:ci"), | |
| ); | |
| } | |
| }, 30_000); | |
| const timeoutId = setTimeout(() => { | |
| if (server.listening) { | |
| logger.info(); | |
| logger.info( | |
| "Having trouble? Try " + | |
| clc.bold("firebase login --no-localhost") + | |
| " or " + | |
| clc.bold("firebase login:ci"), | |
| ); | |
| } | |
| }, 30_000); | |
| server.on("close", () => { | |
| clearTimeout(timeoutId); | |
| }); |
…t hint On dual-stack Windows and corp-managed machines, `localhost` can resolve to `::1` or a hosts-file override while the HTTP server binds to `0.0.0.0`. This causes the OAuth callback to silently fail. Per Google's OAuth 2.0 for Native Apps guidance, use `127.0.0.1` for both the redirect_uri and the server bind address to avoid resolver ambiguity. Also prints a hint about `--no-localhost` and `login:ci` after 30s of no callback, so users stuck on a hang have an actionable next step. Fixes firebase#10750
e05202f to
7221441
Compare
|
@googlebot I fixed it. |
Summary
Fixes #10750
On dual-stack Windows and corp-managed machines,
localhostcan resolve to::1or a hosts-file override while the HTTP server binds to0.0.0.0, causing the OAuth callback to silently fail and the CLI to hang indefinitely on "Waiting for authentication...".Changes
src/auth.ts:getCallbackUrl(): Changed redirect URI fromhttp://localhost:<port>tohttp://127.0.0.1:<port>, per Google's OAuth 2.0 for Native Apps guidance which explicitly recommends127.0.0.1overlocalhostto avoid resolver ambiguity.loginWithLocalhost(): Bind the HTTP server to127.0.0.1explicitly instead of all interfaces, ensuring the listener and redirect URI agree on the address.Timeout hint: After 30 seconds with no callback, print a message pointing users to
firebase login --no-localhostandfirebase login:ci. Today the CLI hangs silently — this gives corp/VPN users an actionable next step.Test plan
getCallbackUrl(3000)returnshttp://127.0.0.1:3000127.0.0.1only (not0.0.0.0or::)🤖 Generated with Claude Code