feat: browser-login flow for newapi CLI (server side)#1
Conversation
Extract the inline access-token minting logic from GenerateAccessToken into a package-level helper, so a follow-up CliLoginExchange handler can mint tokens for the new browser-login flow without duplicating the logic. No behavior change to GET /api/user/token.
Browser-side companion to the new CLI login flow: takes the caller's session cookie, mints a 30-day access token via the helper extracted in 3ff63d9, echoes the CLI-supplied state nonce back to the browser so the CLI's loopback can verify it. Session-auth middleware on the /api/user group handles the unauthenticated case before reaching the handler.
Address code-review feedback on 51296f5: - session-missing branch now uses common.ApiErrorI18n(c, i18n.MsgAuthNotLoggedIn) to match middleware/auth.go behavior and house style - state field gains binding:"required,max=128" so protocol mistakes surface immediately and the echoed value can't grow unbounded
Companion page for the newapi CLI's browser-login handshake: reads port + state from the URL, polls /api/user/self until the user has a live session, calls /api/user/cli-login/exchange to mint a 30-day token, then cross-origin POSTs the result to the CLI's local loopback. Self-contained — no LoginForm redirect coupling.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f97759a53
ℹ️ 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".
|
|
||
| setStatus(STATUS.RETURNING); | ||
| try { | ||
| const cbResp = await fetch(`http://127.0.0.1:${params.port}/callback`, { |
There was a problem hiding this comment.
Restrict callback target to a numeric loopback port
The port query parameter is interpolated directly into a URL authority string, so crafted values like 80@evil.com are parsed with evil.com as the host (http://127.0.0.1:80@evil.com/callback), causing the freshly issued access token to be POSTed off-host instead of to the CLI loopback. This is exploitable when a user opens a malicious /cli-login?... link while logged in (or logs in during the flow), especially on HTTP deployments where mixed-content blocking does not mitigate the request. Validate port as digits in [1,65535] and construct the callback URL via new URL() with fixed hostname 127.0.0.1.
Useful? React with 👍 / 👎.
Summary
Adds the server-side half of a browser-login flow for the
newapiCLI. The CLI starts a local 127.0.0.1 loopback, opens the browser here, and a new/cli-loginpage swaps the signed-in session for a 30-day access token. The CLI side ships in a separate PR.Changes
controller/user.go: extractissueAccessTokenForUser(userID)from the inline body ofGenerateAccessTokenso the new endpoint can mint identical tokens.controller/cli_login.go(new):CliLoginExchangehandler.router/api-router.go: registerPOST /api/user/cli-login/exchangeon the existingselfRoutegroup (inheritsmiddleware.UserAuth()).web/src/components/auth/CliLogin.jsx(new) +web/src/App.jsx: React page at/cli-loginthat readsport+statefrom the URL, polls/api/user/selfuntil logged in (5-min cap), calls the exchange API, then cross-origin POSTs the token to the loopback.Security
/api/usergroup; unauthenticated requests get 401 from middleware before reaching the handler.Test Plan
Notes