Add OAuth login (hb auth login/logout/status) - #31
Open
stympy wants to merge 7 commits into
Open
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds an OAuth 2.0 login option for the Data API in lieu of a personal auth token: - hb auth login: authorization code flow with PKCE over a loopback redirect (RFC 8252/7636), with dynamic client registration (RFC 7591) against the server's advertised registration endpoint. Server capabilities come from RFC 8414 discovery. - hb auth login --device: device authorization grant (RFC 8628) for SSH/headless machines; activates automatically once the server advertises the grant, with a clear error until then. - hb auth logout: best-effort token revocation (RFC 7009) plus local credential removal. hb auth status reports the current auth source. - Tokens are stored 0600 in ~/.honeybadger-cli-credentials.json (keyed by issuer host so US/EU coexist) and refreshed transparently. - Data API commands now use a shared newDataAPIClient() helper: personal auth tokens (flag/env/config) take precedence via basic auth; stored OAuth tokens are sent as Authorization: Bearer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Addresses findings from a Codex review of the OAuth login branch: - Key stored credentials by canonical issuer URL (scheme + host + path) instead of bare host, so an http:// endpoint can never receive a token obtained over https, and path-based issuers don't collide. - Validate discovery results per RFC 8414: the advertised issuer must exactly match the requested one (mix-up protection) and all endpoints must be HTTPS (loopback hosts excepted, for local development). - Build the well-known URL per RFC 8414 §3.1 for issuers that carry a path component. - Write the credentials file atomically (0600 temp file + rename), eliminating the permissions window and torn writes. - Don't let stray or forged loopback callbacks (wrong/missing state) abort a pending login; they get an error page and the flow keeps waiting for the real redirect. - Preserve an unreadable credentials file as .corrupt instead of silently overwriting it (which would drop other issuers' logins). - Treat a failed save of a rotated refresh token as an error instead of a warning, since the old token is already invalid server-side. - Merge query parameters properly when the authorization endpoint already has a query string. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
'hb auth login' now picks the sign-in method for the environment: the browser-based authorization code flow when a browser is likely available, or the device flow in SSH sessions and on displayless machines (when the server advertises the grant). An explicit --device or new --web flag overrides the choice, and each mode prints a one-line tip pointing at the other, gated on server support so the tip never suggests a method that would fail. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Serialize credential mutations across CLI processes: login, logout, and token refresh now go through credentials.Update, which holds an exclusive interprocess lock (flock on Unix, LockFileEx on Windows, via a sidecar lock file), reloads the file, mutates, and saves — so concurrent commands can't lose each other's writes or clobber a rotated refresh token. Refresh re-checks expiry under the lock, so the second of two racing processes reuses the first one's token instead of refreshing again. - Reject issuer URLs containing a query or fragment (RFC 8414 §2) instead of silently stripping them. - Document that the 0600 permission guarantee is Unix-only (Windows relies on profile-directory ACLs) and skip the permission assertion there; verify GOOS=windows builds. - Refresh the design doc to match the implemented behavior (strict discovery validation, issuer-keyed locked storage, flow auto-selection, corrupt-file handling). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Windows reports 0666 for the mode bits regardless of the Chmod; the guarantee is Unix-only (documented in the package comment), and the dedicated permission test was already gated — this assertion inside TestSaveAndLoadRoundTrip was missed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Summary
Adds an OAuth 2.0 login option for the Data API in lieu of a personal auth token.
hb auth loginpicks the best sign-in method for the environment — the browser-based authorization code flow with PKCE (RFC 8252/7636) locally, or the device authorization flow (RFC 8628) in SSH/headless sessions — with--web/--deviceoverrides and a printed hint for switching, backed by RFC 8414 discovery (with strict issuer validation), RFC 7591 dynamic client registration, transparent token refresh with rotation, and RFC 7009 revocation onhb auth logout.Tokens are stored per-issuer in
~/.honeybadger-cli-credentials.json(0600, atomic writes, interprocess-locked mutations so concurrent commands can't lose a rotated refresh token), and all ~76 Data API call sites now go through a sharednewDataAPIClient()helper that prefers a personal auth token (basic auth) and falls back to the stored OAuth bearer token. Personal tokens keep working unchanged and take precedence; the README documents that OAuth tokens are account-scoped (sohb accounts listrequires a personal token).The device flow activates automatically once the server advertises the grant (pairs with the server-side
oauth-device-code-flowbranch (see https://github.com/honeybadger-io/honeybadger/pull/6356)) and was verified end-to-end against a dev server running that branch: login, bearer-token API reads, forced refresh with rotation, and revocation. Two rounds of adversarial code review (Codex) are addressed in the branch history.Closes #26
Test plan
go test ./...— unit + flow-level coverage for discovery validation, PKCE/state handling, device-flow polling semantics (authorization_pending/slow_down/access_denied/expired_token), refresh rotation, credential locking/permissions, flow auto-selection, and bearer-vs-basic client selectiongolangci-lint runclean;GOOS=windows go buildverified--deviceerror) and against the dev server with the device grant (full approval-flow walkthrough)🤖 Generated with Claude Code