feat(login): browser-default flow with email/password CI fallback#1
feat(login): browser-default flow with email/password CI fallback#1Xbang0222 wants to merge 9 commits into
Conversation
Design and implementation plan for replacing the email/password default in `newapi login` with a gh-style loopback callback flow. Email/password remains as the non-interactive (CI) fallback. Spec: docs/superpowers/specs/2026-05-25-web-login-design.md Plan: docs/superpowers/plans/2026-05-25-web-login.md No code changes yet — implementation lives in subsequent commits per the plan's task decomposition.
Self-review caught four real bugs in the initial spec/plan pair: 1. Task 6's TestRun_ContextTimeout blocked inside onListen with an outer ctx that runWith never observed — would hang forever. Rewritten to pass the timeout ctx to RunWithHook directly. 2. LoopbackHint() and Result.loopbackURL were dead — login.go reads the URL via the onListen hook, never via the field. Removed both. 3. runWith was used in Task 6 tests but Task 7 step 4 then renamed it to runWithImpl + added a RunWithHook wrapper — cross-task drift. Promoted RunWithHook to the actual public function and dropped the rename step entirely. 4. CliLogin.jsx assumed new-api's LoginForm honors a `?return=` query — unverified. Replaced with a self-contained "poll /api/user/self until logged in (max 5 min)" loop and a hint linking to /login in a new tab. Also synced spec §4.2 path with the actual route mount: the endpoint lives at /api/user/cli-login/exchange (under selfRoute), not the top-level /api/cli-login/exchange that the first spec draft assumed.
First slice of the new webauth package powering the upcoming browser-login flow. Generate() returns a 43-char URL-safe-base64 nonce backed by crypto/rand; Equal() does a constant-time compare. Tests cover length, uniqueness across 100 invocations, and the known equality cases.
Best-effort launcher used by the browser-login flow: tries `open` (darwin) / `xdg-open` (linux) / `rundll32 url.dll,FileProtocolHandler` (windows). The caller is expected to print the URL to stderr first; this wrapper's failures are safely ignored and fall back to manual paste. Stdlib only.
127.0.0.1:0 HTTP server that receives the browser's POST after the user signs in. Validates the Origin header against the configured endpoint, validates state against a CLI-supplied nonce, and hands the access_token + user_id back via a channel. Shuts down cleanly on either success or context cancellation. Tests cover the happy path, state mismatch, Origin mismatch, and context-timeout cleanup.
Address code-review feedback on a9f70d5: - TrimRight slash on endpoint at RunWithHook entry so configs with trailing slashes match the browser's slash-less Origin header - add ReadHeaderTimeout (gosec G112) - unknown JSON user_id types fall back to "" instead of fmt %v noise
…lback newapi login now opens the user's default browser by default and receives the access token through a 127.0.0.1 loopback. Passing either --email or --password switches to the legacy non-interactive path; --no-browser prints the URL and skips the launcher (for headless or SSH usage). When stdin is not a TTY and neither --email nor --password is given, login fails with a clear instruction message instead of hanging. Token storage, profile save, and pickAPIKey are unchanged.
Address code-review polish items on 6fc96c3: - under --no-browser, lead-in says "Open this URL to sign in:" instead of "Opening your browser to:" (no browser is opened) - drop redundant strings.TrimRight; endpoint is pre-normalised by normalizeEndpoint - comment the 5-min context timeout so it stays in sync with the cli-login page's polling cap - mention --no-browser in the Long help text, not just the Example block
README's Logging in section now leads with the browser flow and keeps email/password as the CI fallback. CLAUDE.md §H grants an explicit exception for browser-launch shellouts (open/xdg-open/rundll32) since they are best-effort and never a hard dependency.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b704279b6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if origin := r.Header.Get("Origin"); origin != endpoint { | ||
| w.Header().Set("Access-Control-Allow-Origin", endpoint) | ||
| http.Error(w, "bad origin", http.StatusForbidden) |
There was a problem hiding this comment.
Comparing Origin to endpoint as raw strings causes valid callbacks to be rejected when users pass equivalent endpoint forms (for example https://host:443 vs browser-origin https://host, or host case normalization). In those cases /callback always returns 403 and browser login cannot complete, even though the request is same-origin by policy. Canonicalize the configured endpoint to origin form (scheme/host/port with default-port normalization) before this equality check.
Useful? React with 👍 / 👎.
| if !Equal(body.State, state) { | ||
| http.Error(w, "state mismatch", http.StatusForbidden) | ||
| return |
There was a problem hiding this comment.
Validation failures in the callback handler (bad origin/state/body/token) return an HTTP error to the browser but never send an error to resultCh, so RunWithHook keeps waiting until context timeout. This makes deterministic failures look like a long hang instead of an immediate login error. Send a terminal error result on these rejection paths so the CLI exits promptly when the callback is irrecoverably invalid.
Useful? React with 👍 / 👎.
1 participant
Summary
Replaces the email/password default of
newapi loginwith agh auth login-style browser flow: open the user's default browser, let them sign in with any method the server supports (password, OAuth, Passkey, 2FA), and receive a 30-day access token via a short-lived 127.0.0.1 loopback. Email/password remains the non-interactive (CI) fallback, triggered by passing--emailor--password.The server-side half lives in Xbang0222/new-api#1 and must land + deploy first for end-to-end smoke.
Changes
internal/cli/webauth/:state.go/state_test.go— 32-byte crypto/rand CSRF nonce, constant-time comparebrowser.go— cross-platformOpen(url)viaopen/xdg-open/rundll32, best-effortserver.go/server_test.go— loopback HTTP server on 127.0.0.1:0 with Origin + state validation, CORS handling, context-driven shutdowninternal/cli/login.go— three-way branch:--email/--password→ password path; non-TTY without those → clear error; otherwise → browser flow (default)--no-browserflag (local tologin) for headless/SSHREADME.md/CLAUDE.md— document the new flow and the §H exception foropen/xdg-open/rundll32Security
127.0.0.1(not 0.0.0.0)Originheader validated against the configured endpoint (with trailing-slash normalisation)statevalidated constant-timeUX details
--no-browserswitches the lead-in to "Open this URL to sign in:" and skips the launcher--no-input(or non-TTY) without--email/--passwordfails with a clear message naming the workaround flagsTest Plan
make test(go test -race -count=1 ./...) — all packages green locallymake lintclean./newapi login --helprenders new Long + ExampleCLAUDE.local.md§"1. Auth"Notes
2c4fdea+ad3085funderdocs/superpowers/.