login: default to browser sign-in, add --device fallback#1366
Draft
toothbrush wants to merge 8 commits into
Draft
login: default to browser sign-in, add --device fallback#1366toothbrush wants to merge 8 commits into
toothbrush wants to merge 8 commits into
Conversation
`entire login` now defaults to the loopback authorization-code flow (auth-go's new authcode package): open a browser, get redirected back to a local 127.0.0.1 listener, exchange the code — no code to type, no poll latency. The device-code flow stays available behind `--device` and is also the automatic fallback when there's no interactive terminal (CI, piped, SSH without a tty), matching gh / gcloud / aws sso. - provider: add AuthorizePath (v1 /oauth/authorize, v2 /authorize — CONFIRM the v2 path against the auth host's OIDC discovery doc before relying on it in production). - auth.Client: build an authcode.Client alongside the deviceflow client and expose StartBrowserAuth + the BrowserAuthFlow shim (parallel to the existing deviceflow shim), flattening TokenSet to (access, refresh). - login.go: shouldUseBrowserLogin() flow selection, runBrowserLogin(), and a shared persistLogin() tail reused by both flows. openBrowser is a no-op under test (ENTIRE_TEST_TTY) so the browser flow is testable without launching a real browser. Tests: unit coverage for the flow selector and runBrowserLogin error/ messaging paths; an integration test drives the full loopback flow by playing the browser (GET the callback with the state parsed off the printed authorize URL). Existing device-flow tests now exercise the --device / non-interactive fallback path. Stacked on auth-go's authcode package: before merge, bump the auth-go dependency to the released tag (go get github.com/entireio/auth-go@<tag>). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pulls in the new authcode package (RFC 8252 loopback + PKCE) that the login browser flow depends on. Pinned to the branch pseudo-version (entireio/auth-go#16); re-pin to the released tag once that PR merges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 72b11bca1f11
Make the default loopback flow read like --device: show "Login URL:", pause on "Press Enter to open in browser...", then print "Waiting for sign-in... " with no newline so persistLogin's "Login complete." lands on the same line. waitForEnter now short-circuits under test (interactive.UnderTest) like openBrowser, so forcing interactive mode in tests no longer blocks on a real /dev/tty read. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: eec99cd78cba
The browser flow now prints "Logging in to: <auth host>" rather than the full authorize URL (the PKCE challenge + loopback redirect made it long and unreadable). The full URL is shown only as a fallback when the browser can't be opened, which is also what a headless host hits. openBrowser reports failure under test (no usable browser on a CI host, and we mustn't spawn a real one), so the integration test recovers the ephemeral loopback callback URL from that fallback line on stdout — replacing the file-based seam from the previous iteration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 3ae392c3a1eb
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates entire login to default to a browser-based loopback authorization-code (PKCE) flow when an interactive terminal is available, while preserving the existing device-code flow via --device and as an automatic fallback for non-interactive environments.
Changes:
- Default interactive login to loopback browser auth; add
--deviceto force device-code flow and auto-fallback to device flow when non-interactive. - Extend the auth client/provider wiring to support the authorization endpoint and a new authcode (browser) client.
- Add unit and integration coverage for the new browser login flow.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Bumps github.com/entireio/auth-go to a newer pseudo-version that includes the authcode support. |
| go.sum | Adds checksums for the updated auth-go pseudo-version. |
| cmd/entire/cli/login.go | Implements browser login flow, routing logic, and shared token persistence. |
| cmd/entire/cli/login_test.go | Adds unit tests for browser-flow routing and runBrowserLogin behavior. |
| cmd/entire/cli/integration_test/login_test.go | Adds an end-to-end integration test for the browser loopback flow and refactors login process helpers. |
| cmd/entire/cli/auth/provider.go | Adds AuthorizePath to provider configuration for both provider versions. |
| cmd/entire/cli/auth/client.go | Adds an authcode client and a StartBrowserAuth entrypoint with an interface-friendly flow wrapper. |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 2c42a3117eae
- Assert the stubbed-Wait error in runBrowserLogin tests instead of blank-assigning it (errcheck check-blank). - nolint:ireturn on StartBrowserAuth and its test fake — the BrowserAuthFlow interface return is deliberate for test substitution. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: a0b237ab3218
Avoid the ireturn lint suppression by following "accept interfaces, return structs" (as the device-flow shim already does): - auth.BrowserAuthFlow is now a concrete struct (was an interface); StartBrowserAuth returns *BrowserAuthFlow. - runBrowserLogin accepts a cli-local browserAuthFlow interface plus the base URL, with StartBrowserAuth moved up into newLoginCmd. The fake flow satisfies the local interface; no nolint anywhere. Output: one space after "Logging in to:" and a single blank line before "Waiting for sign-in..." (the leading newline doubled up with the post-Enter newline). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: e72efb851121
openBrowser reports failure under test (not a no-op), forcing the fallback URL path the test scrapes — update the doc comment to match. Addresses a PR review note. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 86eeeb1f9d81
Contributor
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5ea776a. Configure here.
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.
What
entire loginnow opens your browser by default and finishes automatically — no code to copy.Falls back to the device-code flow automatically when there's no interactive terminal (CI, piped, SSH without a tty), so headless logins keep working — same shape as
gh/gcloud/aws sso.How
RFC 8252 loopback authorization-code flow with PKCE, via auth-go's new
authcodepackage: bind a127.0.0.1listener, open the browser, catch the redirect, exchange the code. Shares the token-validate/save/context-record tail with the device flow.Verified against prod (
us.auth.entire.io)authorization_endpoint=/authorize,code_challenge_methods_supported=["S256"],authorization_codegrant, public-client (none) auth.entire-cliis registered with ahttp://127.0.0.1/callbackloopback redirect, any-port (probed: unknown client and unregistered redirect both 400 at/authorize; two ephemeral ports accepted).Stacked on entireio/auth-go#16 (
authcodepackage).go.modis pinned to that branch's pseudo-version so this builds today; once #16 merges + tags, re-pin:go get github.com/entireio/auth-go@<tag> && go mod tidy.Note
High Risk
Changes the default authentication path and OAuth loopback/PKCE handling; mistakes could break login or weaken token handling across environments.
Overview
entire loginnow defaults to a loopback browser (PKCE authorization-code) sign-in when an interactive terminal is available, with--deviceto keep the previous device-code flow. Non-interactive environments (CI, pipes, SSH without a tty) still use device-code automatically.The auth client wires
auth-goauthcodealongside device flow, addsAuthorizePathon v1/v2 providers, and exposesStartBrowserAuth/BrowserAuthFlow. Login shares token validation and persistence viapersistLoginfor both paths. Test-only behavior skips blocking on/dev/ttyand refuses to spawn a real browser so integration tests can drive the loopback callback from printed URLs.go.modpins a newergithub.com/entireio/auth-gopseudo-version that includes theauthcodepackage (stacked on auth-go#16).Reviewed by Cursor Bugbot for commit 5ea776a. Configure here.