feat(auth): OAuth device-flow sign-in (wherobots auth login) by ClayMav · Pull Request #33 · wherobots/wbc-cli

ClayMav · 2026-07-02T21:22:39Z

Summary

Adds browser-based sign-in to the CLI so users no longer need to manually create and export an API key. wherobots auth login runs the OAuth 2.0 Device Authorization Grant (RFC 8628 / WorkOS "CLI Auth") against WorkOS AuthKit; the API key remains fully supported.

New commands:

wherobots auth login — prints a confirmation code, opens the browser, polls for approval (--no-browser prints the URL only, for SSH).
wherobots auth logout — removes the stored session (--all for every environment).
wherobots auth status — shows which credential is active, account, and token expiry.

How it works

internal/auth: device-flow client (OIDC discovery with conventional-path fallback, interval/slow_down-aware polling, refresh-token grant with rotation, invalid_grant handling), a domain-keyed credentials.json session store (os.UserConfigDir()/wherobots/, 0600, atomic temp+rename writes), unverified JWT claim decode for display, and a request-time credential resolver.
Credential precedence: WHEROBOTS_API_KEY always wins over a stored OAuth session (gh/aws convention), so CI and scripts stay predictable.
executor: BuildRequest applies credentials through a Credentials interface; DoWithReauth force-refreshes and replays once on a 401 with an OAuth credential; --dry-run redacts the bearer.
auth and upgrade dispatch through a bare root, so they work on a fresh machine with no API key, no cached spec, and no API connectivity.
Environment selection mirrors WHEROBOTS_API_URL: WHEROBOTS_OAUTH_DOMAIN / WHEROBOTS_OAUTH_CLIENT_ID override the baked-in production defaults.

No backend change required

The device-flow access token is a WorkOS AuthKit session bearer. studio-backend's WorkOSTokenValidator resolves it via the existing AuthKit session profile (issuer-checked, no audience check) — the same path the web app uses — so no WORKOS_CONNECT_CLIENTS entry or other backend change is needed. A dedicated WorkOS CLI Connect client (public/PKCE, device grant enabled) exists per environment for independent control; IDs are in docs/oauth-setup.md.

Verification

Full unit coverage (device-flow state machine, store perms/atomicity/corruption, resolver precedence + refresh rotation, 401-replay, dry-run redaction, spec-free dispatch). Verified end-to-end against staging with the dedicated client:

auth login → session stored; /users/me → 200
refresh-token rotation persists on a proactive refresh
WHEROBOTS_API_KEY correctly shadows the stored session
--dry-run redacts the bearer

Follow-up (not in this PR)

Replace the baked-in production defaultOAuthClientID verification with a real prod login once convenient; prod device grant is already confirmed enabled at WorkOS.

https://claude.ai/code/session_01GFDWxfQrcQob6dv2Wa7AMA

Adds `wherobots auth login|logout|status` using the OAuth 2.0 Device Authorization Grant (RFC 8628, WorkOS "CLI Auth") against WorkOS AuthKit. - internal/auth: device-flow client (OIDC discovery with conventional-path fallback, interval/slow_down-aware polling, refresh-token grant with rotation, invalid_grant handling), domain-keyed 0600 credentials.json session store with atomic writes, unverified JWT claim decoding for display, and a request-time credential resolver. - Precedence: WHEROBOTS_API_KEY always wins over a stored OAuth session (gh/aws convention) so CI and scripts stay predictable. - executor: BuildRequest applies credentials via the Credentials interface; DoWithReauth force-refreshes and replays once on a 401 with an OAuth credential; --dry-run redacts the bearer. - spec loader authenticates best-effort and never blocks credential-free commands. - auth and upgrade dispatch through a bare root, so they work with no API key, no cached spec, and no API connectivity. - Config: WHEROBOTS_OAUTH_DOMAIN / WHEROBOTS_OAUTH_CLIENT_ID env overrides with baked-in production defaults. The access token is a WorkOS AuthKit session bearer that studio-backend already validates on every REST route (the same path as the web app), so no backend change is required. Verified end-to-end against staging: login, /users/me, refresh-token rotation, and API-key precedence. Claude-Session: https://claude.ai/code/session_01GFDWxfQrcQob6dv2Wa7AMA

README gains an Authentication section (browser sign-in vs API key, the env-var-wins precedence rule, per-environment OAuth env vars); CLAUDE.md covers the internal/auth package and spec-free dispatch; docs/oauth-setup.md records the WorkOS dashboard steps (public/PKCE client, device grant) and notes that no studio-backend change is needed because the token validates via the existing AuthKit session profile. Claude-Session: https://claude.ai/code/session_01GFDWxfQrcQob6dv2Wa7AMA

ClayMav · 2026-07-02T21:37:06Z

@salty-hambot rubric

salty-hambot

Reviewed by Salty Hambot 🤖🧂 — rubric mode

Verdict: ✅ pass

Dimension	Verdict	Notes
correctness	✅ pass	The DoWithReauth 'empty body on POST replay' concern is a false positive — http.NewRequestWithContext sets GetBody for strings.Reader bodies and req.Clone preserves it; the passing replay test confirms the original body is re-sent.
security	✅ pass	The baked-in and documented client IDs are public OAuth clients (token-endpoint auth 'none'), intentionally shippable; JWT is decoded unverified only for local display/expiry hints with no backend trust.
privacy	✅ pass	Tokens stored 0600, bearer redacted in dry-run, email/sub local-only, nothing sensitive logged.
reliability	✅ pass	401 refresh-replay correctly re-sends the body, transient refresh failures keep the session, invalid_grant deletes it; the alleged body-replay gap does not exist.
scalability	✅ pass	Single-file read-modify-write store with atomic rename; concurrent CLI auth writes are rare and corruption is prevented.
observability	✅ pass	Auth errors consistently point to `auth login`/`WHEROBOTS_API_KEY`; X-Wherobots-Client carries the invoked command; dry-run redacts credentials deterministically.
clarity/maintainability	✅ pass	Clear separation across device flow, store, JWT, resolver; the Credentials interface cleanly decouples the executor; test seams are explicit. Minor io.Writer inconsistency noted.
test quality	✅ pass	Device-flow state machine, store atomicity/permissions/corruption, resolver precedence and rotation, 401-replay (with body), dry-run redaction, and spec-free dispatch are all directly tested.

Clean, well-tested OAuth device-flow addition — the reviewer's only real bug hunt (empty-body replay on 401) is a false alarm: strings.NewReader bodies get GetBody set by the stdlib, and the passing POST-replay test proves it. What's left is two fair questions and a couple of style nits.

_{5 finding(s) posted.}
_{💬 To request a re-review, comment @salty-hambot review}

- DoWithReauth: surface the request-body rewind error on a 401 retry instead of returning the original 401, which was misleading to debug. - removeCorruptStore: take io.Writer instead of an anonymous writer interface. - Add comments clarifying the device-poll deadline guard and why the login tests are not parallel (shared package-level openBrowser stub). Claude-Session: https://claude.ai/code/session_01GFDWxfQrcQob6dv2Wa7AMA

ClayMav · 2026-07-02T21:41:38Z

@salty-hambot rubric

salty-hambot

Reviewed by Salty Hambot 🤖🧂 — rubric mode

Verdict: ✅ pass

Dimension	Verdict	Notes
correctness	✅ pass	The prior body-rewind concern is resolved — DoWithReauth now wraps and returns bodyErr; 401 replay, refresh, and precedence logic are all sound.
security	✅ pass	Public OAuth client IDs are intentionally shippable; JWT decoded unverified only for local display; bearer redacted in dry-run; tokens stored 0600.
privacy	✅ pass	Access/refresh tokens stored owner-only (0600/0700), bearer redacted in dry-run curl, email/sub kept local, nothing sensitive logged.
reliability	✅ pass	Proactive refresh within 2m skew, invalid_grant deletes session and guides re-login, transient errors preserve session, 401-replay re-sends body via GetBody.
scalability	✅ pass	Single-file atomic read-modify-write store; concurrent writes rare; no unbounded polling or memory growth.
observability	✅ pass	All error paths point users to `wherobots auth login` or WHEROBOTS_API_KEY; X-Wherobots-Client carries the invoked command; dry-run redacts credentials deterministically.
clarity/maintainability	✅ pass	Both prior style nits are resolved: removeCorruptStore uses io.Writer, and TestAuthLoginHappyPath documents why it isn't parallel. Clean package separation throughout.
test quality	✅ pass	Device-flow state machine, store atomicity/permissions/corruption, resolver precedence and rotation, 401-replay with body re-send, dry-run redaction, and spec-free dispatch are all directly tested.

Clean OAuth device-flow addition — every prior thread is resolved in the code (body-rewind error surfaced, io.Writer aligned, non-parallel test explained), and there's nothing new to nitpick. Ship it.

Prior findings: ✅ 5 resolved

_{0 finding(s) posted.}
_{💬 To request a re-review, comment @salty-hambot review}

ClayMav · 2026-07-07T21:35:05Z

@salty-hambot rubric

Review feedback from #33: - refresh with the session's stored client_id, not the current default, so a future default-client change can't orphan old sessions - tolerate up to 3 consecutive transient failures while polling for device-flow sign-in instead of aborting the whole login - on invalid_grant, re-read the store once and adopt a session rotated by a concurrent CLI process instead of signing both processes out - propagate corrupt-store errors from ForceRefresh (with the sign-in hint) instead of surfacing a bare 401 - drop the unused Resolver.Source API (auth status reads the store)

…atch Review feedback from #33: - config.Load no longer fails when os.UserConfigDir/os.UserCacheDir are unresolvable (e.g. HOME unset in CI): API-key-only usage works, the spec loader skips caching, and the credentials store surfaces an actionable error only when credentials are actually needed - warn at login when WHEROBOTS_API_URL is custom but the OAuth domain is still the production default (sign-in would target the wrong tenant) - route `wherobots help auth` through the bare spec-free root, matching `wherobots auth --help`

ClayMav · 2026-07-07T22:05:04Z

@salty-hambot rubric

salty-hambot

Reviewed by Salty Hambot 🤖🧂 — rubric mode

Verdict: ✅ pass

Dimension	Verdict	Notes
correctness	✅ pass	bodyErr now wrapped/surfaced, 401-replay re-sends body via GetBody, client-side deadline documented as belt-and-suspenders.
security	✅ pass	Public OAuth client IDs intentionally shippable; JWT decoded unverified for display only; bearer redacted in dry-run; tokens stored 0600.
privacy	✅ pass	Access/refresh tokens owner-only (0600/0700), bearer redacted in --dry-run, email/sub kept local, nothing sensitive logged.
reliability	✅ pass	Proactive refresh within 2m skew, invalid_grant adopts winner or re-guides login, transient errors preserve session, 401-replay re-sends body.
scalability	✅ pass	Single-file atomic read-modify-write store; concurrent writes rare and corruption-safe via temp+rename; no unbounded growth.
observability	✅ pass	Credential-error paths point to auth login/WHEROBOTS_API_KEY; X-Wherobots-Client carries the invoked command; dry-run redacts deterministically.
clarity/maintainability	✅ pass	Both prior style nits resolved; clean package separation and a well-decoupled Credentials interface throughout.
test quality	✅ pass	Device-flow state machine, store atomicity/permissions/corruption, resolver precedence/rotation, 401-replay with body, dry-run redaction, spec-free dispatch all directly tested.

Third pass and it's still spotless — every prior thread landed (bodyErr surfaced, io.Writer aligned, non-parallel test documented), rubric's green across all eight. Ship it.

Prior findings: ✅ 5 resolved

_{0 finding(s) posted.}
_{⏱️ 51.2s}
_{💬 To request a re-review, comment @salty-hambot review}

salty-hambot

Reviewed by Salty Hambot 🤖🧂 — rubric mode

Verdict: ✅ pass

Dimension	Verdict	Notes
correctness	✅ pass	Prior body-rewind concern resolved via fmt.Errorf wrapping; 401-replay, refresh, and precedence logic are all sound.
security	✅ pass	Public OAuth client IDs are intentionally shippable; JWT decoded unverified for local display only; bearer redacted in dry-run; tokens stored 0600/0700.
privacy	✅ pass	Access/refresh tokens stored owner-only, bearer redacted in dry-run curl, email/sub kept local, nothing sensitive logged.
reliability	✅ pass	Proactive refresh within 2m skew, invalid_grant deletes and guides re-login, transient errors preserve session, concurrent-rotation race handled.
scalability	✅ pass	Single-file atomic read-modify-write store; concurrent writes rare; no unbounded polling or memory growth.
observability	✅ pass	All credential-failure paths point to auth login or WHEROBOTS_API_KEY; X-Wherobots-Client carries the command; dry-run redacts deterministically.
clarity/maintainability	✅ pass	Both prior style nits resolved (io.Writer, non-parallel test comment); clean package separation across device flow, store, JWT, and resolver.
test quality	✅ pass	Device-flow state machine, store atomicity/permissions/corruption, resolver precedence and rotation, 401-replay with body re-send, dry-run redaction, and spec-free dispatch are all directly tested.

Third pass and it's still clean — every prior thread landed in the code (body-rewind error surfaced, io.Writer aligned, non-parallel test explained), and there's nothing new worth flagging. Ship it.

Prior findings: ✅ 5 resolved

_{0 finding(s) posted.}
_{⏱️ 1m15.1s}
_{💬 To request a re-review, comment @salty-hambot review}

sfishel18 · 2026-07-08T17:49:52Z

do we have any control over this page?

ClayMav · 2026-07-08T18:16:50Z

Just updated the buttons to be correct!

Review feedback from #33: - refresh with the session's stored client_id, not the current default, so a future default-client change can't orphan old sessions - tolerate up to 3 consecutive transient failures while polling for device-flow sign-in instead of aborting the whole login - on invalid_grant, re-read the store once and adopt a session rotated by a concurrent CLI process instead of signing both processes out - propagate corrupt-store errors from ForceRefresh (with the sign-in hint) instead of surfacing a bare 401 - drop the unused Resolver.Source API (auth status reads the store)

ClayMav added 2 commits July 2, 2026 14:36

ClayMav force-pushed the clay/oauth-device-login branch from 58b8cb1 to e39eb50 Compare July 2, 2026 21:36

salty-hambot Bot reviewed Jul 2, 2026

View reviewed changes

Comment thread internal/executor/request.go

Comment thread internal/executor/request.go

Comment thread internal/auth/device.go

Comment thread internal/commands/auth.go

Comment thread internal/commands/auth_test.go

salty-hambot Bot reviewed Jul 2, 2026

View reviewed changes

ClayMav requested review from sfishel18 and zongsizhang July 2, 2026 21:43

ClayMav marked this pull request as ready for review July 2, 2026 21:50

ClayMav requested a review from a team as a code owner July 2, 2026 21:50

sfishel18 reviewed Jul 7, 2026

View reviewed changes

ClayMav added 2 commits July 7, 2026 15:04

salty-hambot Bot reviewed Jul 7, 2026

View reviewed changes

ClayMav requested a review from sfishel18 July 7, 2026 22:07

sfishel18 approved these changes Jul 8, 2026

View reviewed changes

ClayMav merged commit e1dbb7d into main Jul 8, 2026
3 checks passed

ClayMav deleted the clay/oauth-device-login branch July 8, 2026 18:16

Uh oh!

Conversation

ClayMav commented Jul 2, 2026

Summary

How it works

No backend change required

Verification

Follow-up (not in this PR)

Uh oh!

ClayMav commented Jul 2, 2026

Uh oh!

salty-hambot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ClayMav commented Jul 2, 2026

Uh oh!

salty-hambot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ClayMav commented Jul 7, 2026

Uh oh!

ClayMav commented Jul 7, 2026

Uh oh!

salty-hambot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

salty-hambot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

sfishel18 commented Jul 8, 2026

Uh oh!

ClayMav commented Jul 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants