feat(auth): implement Device Code login flow and token refresh

[emsearcy]

Summary

Implements lfx auth login, lfx auth token, lfx auth status, and lfx auth logout on top of golang.org/x/oauth2's device authorization grant support (Config.DeviceAuth/DeviceAccessToken) and refresh-token TokenSource, targeting the static per-environment Auth0 client IDs provisioned in LFXV2-2513.

• internal/commands/environment.go: resolves --env to its IdP domain and compiled-in client ID.
• internal/commands/auth.go: real login (interactive device code flow, or --with-token for headless/CI use), token (cached-token fast path, refresh-and-recache on expiry, clear error message on invalid/expired refresh token), status, logout, and backends implementations. Calls golang.org/x/oauth2's Config.DeviceAuth/DeviceAccessToken and refresh-token TokenSource directly (no wrapper package/types in between) for the device code request, poll, and refresh-token exchange.
• internal/credstore: extends DeviceState with environment/audience (needed to rebuild the same IdP client on refresh), insecure (guards against mixing state between the keychain and --insecure-storage backends), and adds DeleteDeviceState for logout. Also adds AvailableBackends, which reports the system credential-store backends compiled into the binary for the current OS.
• lfx auth backends: lists the system credential-store backend(s) available on this OS (e.g. macOS Keychain, pass, Linux Secret Service/KWallet, Windows Credential Manager), in the priority order lfx auth login tries them. This only reflects what's compiled in per-OS build tags, not whether a given backend is actually usable at runtime (e.g. no D-Bus session for Secret Service). User-facing "system keychain" wording elsewhere was renamed to the more accurate, OS-agnostic "system backend" to match.
• Includes outstanding .cspell.json wordlist additions/fixes (nolint, containedctx, rundll, plus suppressing spellcheck on the opaque Auth0 client ID literals).

Notably NOT implemented: persistent "device ID"

The epic's background/design notes called for a "Device ID persistence in ~/.local/state/lfx-cli/," modeled on an assumed gh CLI precedent. I checked gh's actual behavior directly: ~/.local/state/gh/device-id exists, but it's generated by internal/telemetry.getOrCreateDeviceID in github.com/cli/cli — an anonymous telemetry identifier, not something used in gh's own OAuth device flow or credential storage. Auth0's device authorization grant has no concept of a device ID either. Since the LFX CLI has no telemetry pipeline in scope here, there's nothing for one to do; left out of credstore.DeviceState with a comment explaining the rationale. See ticket comments on LFXV2-2515/LFXV2-2516 for the full writeup.

Related tickets

• LFXV2-2515 — Implement Device Code login flow (lfx auth login)
• LFXV2-2516 — Implement lfx auth token command

Testing

• go build ./..., go vet ./..., golangci-lint run, revive ./... all pass clean
• Unit tests (go test ./...) cover credstore's Save/Load/Delete round-trip and ValidAccessToken, persistLogin's success/rollback paths, loadDeviceStateForBackend's backend/IdP-domain mismatch guards, and identityFromIDToken's claim-selection logic
• Manually smoke-tested the full login/status/token/logout cycle end-to-end against the dev and prod Auth0 tenants (--env development/--env prod), including the interactive device code flow, cached-token fast path, and refresh-token exchange
• Found and fixed a real bug during that prod testing: prod's IdP domain must be its custom domain sso.linuxfoundation.org, not linuxfoundation.auth0.com (see auth0-terraform's own auth0_domain variable) — using the wrong domain surfaced as an Auth0 invalid_request: Missing required parameter: response_type error and a login that never completed

🤖 Generated with GitHub Copilot (via OpenCode)

[Copilot]

Pull request overview

Implements Auth0 device-code authentication and token refresh for the LFX CLI.

Changes:

• Adds environment-aware Auth0 device authorization and refresh support.
• Implements login, token, status, logout, and credential-state cleanup.
• Adds OAuth dependency and spellcheck terms.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
internal/auth0/device.go	Adds Auth0 device and refresh flows.
internal/commands/auth.go	Implements authentication commands.
internal/credstore/credstore.go	Extends and deletes persisted device state.
go.mod	Adds OAuth2 dependency.
go.sum	Records OAuth2 checksums.
.cspell.json	Adds required technical terms.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (2)

internal/auth0/device.go:184

• The caller can pass a context with its own deadline to RequestDeviceCode; DeviceAccessToken derives its polling context from that parent, so DeadlineExceeded is not necessarily caused by the device-code expiry. This branch misreports an earlier caller timeout as an expired device code. Check dc.ctx.Err() (or otherwise distinguish the parent deadline) before mapping the error to ErrExpiredToken.

	// passes in (see RequestDeviceCode), any DeadlineExceeded seen here
	// can only come from that internal one, so it's safe to always treat
	// it as ErrExpiredToken.
	if errors.Is(err, context.DeadlineExceeded) {
		return nil, ErrExpiredToken

internal/commands/auth.go:186

• verification_uri_complete is optional, but both the browser and manual paths use it unconditionally. A valid device response that only supplies verification_uri therefore opens/prints an empty URL and leaves the user unable to complete login. Fall back to dc.VerificationURI when the complete URI is empty.

		fmt.Printf("Opening %s in your browser...\n", dc.VerificationURIComplete)
		if err := openBrowser(dc.VerificationURIComplete); err != nil {

[emsearcy]

AI-assisted: Fixed in d94eda8 — addressed the 2 suppressed Copilot comments from the follow-up review:

• Poll() now checks dc.ctx.Err() before mapping context.DeadlineExceeded to ErrExpiredToken, so a caller-supplied context deadline (distinct from DeviceAccessToken's own internally-derived one) is no longer misreported as an expired device code.
• loginWithDeviceCode now falls back to VerificationURI when VerificationURIComplete is empty (it's optional per RFC 8628 §3.2).

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (5)

internal/commands/auth.go:216

• As in the supplied-token path, credentials are saved before the required device state. A state-file write failure therefore reports login failure while leaving newly issued credentials paired with missing or previous environment metadata. Make the two persistence updates atomic from the caller’s perspective or restore/remove the credential update on failure.

	if err := store.SaveCredentials(credstore.Credentials{
		RefreshToken:      token.RefreshToken,
		AccessToken:       token.AccessToken,
		AccessTokenExpiry: token.Expiry,
	}); err != nil {
		return fmt.Errorf("save credentials: %w", err)
	}

internal/commands/auth.go:209

• A successful Auth0 device exchange can omit refresh_token when the selected custom API does not allow offline access, even though offline_access was requested. This currently persists an empty refresh token and reports “Login successful,” so the session becomes unusable as soon as the cached access token expires. Reject the login with a clear configuration error when token.RefreshToken is empty before saving credentials.

		return errors.New("login was denied")
	case errors.Is(err, auth0.ErrExpiredToken):
		return errors.New("device code expired before login completed")
	default:
		return err

internal/commands/auth.go:152

• Credentials have already been overwritten when SaveDeviceState runs. If writing state.json fails, the command returns an error but leaves the new refresh token paired with missing or stale environment metadata, so a later refresh cannot safely reconstruct its Auth0 client. Persist these as one consistent operation or roll back the credential update when state persistence fails.

This issue also appears on line 210 of the same file.

	if err := store.SaveDeviceState(credstore.DeviceState{
		IDPDomain:   domain,
		Environment: string(env),
		Audience:    audience,
		Insecure:    insecure,
	}); err != nil {
		return fmt.Errorf("save device state: %w", err)

AGENTS.md:50

• This implementation now uses compiled-in static client IDs, but AGENTS.md:177-180 still says the gh-pages CIMD URL is the Device Code client ID. That guidance directly contradicts internal/auth0/device.go:22-26 and can send future maintainers to update an unused client definition. Update the gh-pages section to explain that the CIMD asset is no longer used by this flow.

`lfx auth login` / `status` / `token` / `logout` are fully implemented,
including the Auth0 Device Code flow, refresh-token exchange, and

internal/auth0/device.go:91

• All production callers leave HTTPClient nil, and main supplies context.Background(), so device-code and refresh requests use http.DefaultClient without an overall response timeout. An Auth0 endpoint that accepts a connection but stalls can therefore hang lfx auth login or lfx auth token indefinitely. Provide a bounded default client or add per-request deadlines while retaining the device-flow polling deadline.

	// HTTPClient is used for all requests. Defaults to
	// http.DefaultClient if nil.
	HTTPClient *http.Client

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

internal/commands/auth.go:223

• This has the same partial-login failure mode as --with-token: credentials are replaced before device state is saved. If the state write fails, an older same-backend state remains usable and can direct this new refresh token to the previous environment's token endpoint. Roll back/delete the new credentials on failure, or make persistence of credentials and their routing metadata atomic.

	if err := store.SaveDeviceState(credstore.DeviceState{
		IDPDomain:   domain,
		Environment: string(env),
		Audience:    audience,
		Insecure:    insecure,
	}); err != nil {
		return fmt.Errorf("save device state: %w", err)

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (5)

internal/commands/auth.go:146

• SaveCredentials has already replaced the selected backend's credentials when SaveDeviceState fails. The command then reports a failed login while leaving the new refresh token paired with stale or missing environment metadata, so a later refresh can target the wrong configured tenant and fail. Persist the credential/state pair transactionally, or restore/delete the newly written credentials when saving the state fails.

	if err := store.SaveCredentials(credstore.Credentials{RefreshToken: refreshToken}); err != nil {
		return fmt.Errorf("save credentials: %w", err)
	}
	if err := store.SaveDeviceState(credstore.DeviceState{

internal/commands/auth.go:217

• This second write can fail after the newly issued credentials have already been saved, leaving credentials and device metadata from different login attempts. A later refresh then uses stale metadata with the new refresh token. Make these writes transactional or roll back the credential write when saving state fails.

	if err := store.SaveDeviceState(credstore.DeviceState{

internal/commands/auth.go:388

• The presence of a credential record does not establish that the user is logged in: --with-token accepts any non-empty string without validation, and stored refresh tokens can later be revoked. In both cases this command prints “Logged in.” even though token acquisition fails. Validate the session (for example by refreshing when no valid access token is cached), or report only that credentials are stored rather than asserting an authenticated status.

			fmt.Println("Logged in.")

internal/commands/auth.go:214

• A successful OAuth token response is not required to include a refresh token. Saving such a response and printing “Login successful” creates a session that necessarily stops working when the access token expires. Check token.RefreshToken before persisting and return an actionable error if Auth0 did not issue one.

	if err := store.SaveCredentials(credstore.Credentials{
		RefreshToken:      token.RefreshToken,
		AccessToken:       token.AccessToken,
		AccessTokenExpiry: token.Expiry,
	}); err != nil {

internal/commands/auth.go:40

• This comment is inaccurate: auth status never reads identity claims; these scopes are used only for the post-login “Logged in as …” message at lines 227-230. Describe that actual use so future scope changes do not rely on a nonexistent status dependency.

// scopes requested during the device code flow. offline_access is required
// to receive a refresh token; the rest identify the user for `auth status`.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (2)

internal/commands/auth.go:299

• If the credential write succeeds but the state write fails (disk full, permissions, or a partial state-file write), the command reports login failure after already replacing the credentials. The new refresh token is then paired with stale environment metadata—or no metadata—so later refreshes fail, and a previously valid login may have been destroyed. Make these writes transactional from the caller’s perspective by restoring/deleting the newly written credentials when state persistence fails (and preserving any prior login as appropriate).

	if err := store.SaveCredentials(creds); err != nil {
		return fmt.Errorf("save credentials: %w", err)
	}
	if err := store.SaveDeviceState(state); err != nil {
		return fmt.Errorf("save device state: %w", err)

internal/commands/auth.go:209

• A successful OAuth token response is not guaranteed to contain a refresh token, even when offline_access is requested (for example, an overridden audience may have offline access disabled). This path still persists the response and prints “Login successful,” so authentication works only until the cached access token expires, after which auth token can never refresh it. Require a non-empty token.RefreshToken before persisting the login and return an actionable configuration/login error otherwise.

			RefreshToken:      token.RefreshToken,

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

internal/commands/auth.go:435

• This cached-token fast path runs before loadDeviceStateForBackend, so it bypasses the newly required backend check. For example, if a login is pinned to Keychain but auto-detection later opens pass containing an older still-valid token, lfx auth token silently prints that token; the same mismatch is rejected only after it expires. Validate the stored state before either returning a cached token or refreshing it.

			if creds.ValidAccessToken() {
				fmt.Println(creds.AccessToken)
				return nil

internal/commands/auth.go:552

• Credentials are deleted before the persisted backend is checked. If a login is pinned to Keychain but this invocation opens pass (explicitly or through changed auto-detection), this deletes the wrong entry, leaves the actual Keychain credentials intact, and still prints “Logged out.” Load and validate DeviceState before this destructive operation; on a mismatch, return the existing actionable backend hint instead of reporting success.

			if err := store.DeleteCredentials(); err != nil {
				return fmt.Errorf("delete credentials: %w", err)
			}

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

internal/commands/auth.go:445

• The cached-token fast path returns before the persisted backend is validated. A login pinned with --backend can therefore be invoked with that flag omitted or changed while its access token is still valid, potentially printing a token from an auto-selected/different credential store. Load and validate the device state before this fast path.

This issue also appears on line 550 of the same file.

			if creds.ValidAccessToken() {
				fmt.Println(creds.AccessToken)
				return nil

internal/commands/auth.go:552

• Credentials are deleted before the pinned backend in state.json is checked. For example, after a --backend=pass login, omitting the flag can auto-open Secret Service, delete that store's entry, leave the actual pass login intact, and still print “Logged out.” Load and validate the state before mutating credentials, and reject a pinned-backend mismatch (or explicitly report that only the selected backend was cleared).

			if err := store.DeleteCredentials(); err != nil {
				return fmt.Errorf("delete credentials: %w", err)
			}

internal/commands/auth.go:72

• This help text says --backend is ignored with --insecure-storage, but credStoreFromCommand rejects that combination. Update the user-facing contract to match the implemented mutual exclusion.

			&cli.StringFlag{
				Name:  backendFlagName,
				Usage: "Pin credential storage to a specific system backend (see `lfx auth backends`); mutually exclusive with --insecure-storage",

internal/credstore/credstore.go:299

• The “available” list is built from every supported cross-platform backend, not the current OS's AvailableBackends. Consequently an invalid value on Linux can advertise keychain and wincred as available even though keyring.Open cannot use them there. Build this error's list from AvailableBackends and distinguish recognized-but-unavailable backends.

			path: filepath.Join(stateDir, insecureCredentialsFileName),
		}
	} else {
		allowedBackends := systemBackends
		if opts.Backend != "" {

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

internal/commands/auth.go:552

• Credentials are deleted before the pinned backend is checked. When --backend is omitted or differs but auto-detection opens the pinned store, this removes that login, preserves its state as “mismatched,” and still prints Logged out.; if another store opens, the active pinned login can remain while the command still reports success. Reject a same-storage-mode pinned-backend mismatch before deleting anything.

			if err := store.DeleteCredentials(); err != nil {
				return fmt.Errorf("delete credentials: %w", err)
			}

[dealako]

Hi @emsearcy 👋 — thanks for the fast, substantial turnaround on this. You addressed the blocking test gap head-on and the scope actually grew in a good direction. Nice iteration.

👏 Nice work this round

• Tests landed where it counts. credstore_test.go and auth_test.go now cover the credential/state round-trips, New's backend rejection, every loadDeviceStateForBackend branch (insecure mismatch, domain-tamper, backend-pin mismatch, unpinned-allows-any, OK), identityFromIDToken including malformed/base64/non-JSON inputs, and the persistLogin rollback path via a failingDeviceStateStore. The StateDir seam I called out as unused is now exercised.
• The internal/auth0 → internal/commands/environment.go consolidation simplified the domain-resolution trust model without weakening it: resolveEnvironment is still the single source of truth and the IDPDomain == resolveEnvironment(state.Environment) tamper check still gates every refresh.
• New --backend pinning is a real security improvement over letting keyring.Open silently auto-detect, and it fails closed (isAvailableBackend validates against OS-compiled backends, mutual-exclusion with --insecure-storage enforced before any store opens).
• Fixed the flaky UTC round-trip test and the orphaned doc-comment line.

Revision tracking (prior round)

• ✅ Resolved — Orphaned credstore doc-comment line (D): reflowed cleanly.
• ✅ Effectively resolved — Blocking "no tests" (A): the zero-tests condition is gone. The remaining untested surface (the DeviceAccessToken/refresh HTTP error-mapping switches and login flows) is a deliberate, documented scope call now that the HTTPClient injection seam was removed with the auth0 package. I'm not holding this blocking — that logic largely exercises golang.org/x/oauth2 itself. Worth a follow-up ticket if you ever want the expired_token/access_denied/invalid_grant mappings pinned.
• ⚠️ Accepted with rationale — persistLogin rollback data-loss window (B): behavior unchanged, but now explicitly documented as an accepted tradeoff (narrow window, recoverable by re-running login) and the delete-rollback path is tested. Reasonable call; leaving it to you.
• ⚠️ Partially addressed — README docs (C): --backend is now documented in prose + example. Still missing: lfx auth backends in the usage command block (README.md:29-42) and the --env / --with-token / --web / --audience login flags. Per AGENTS.md guideline #6, worth a line each (or an explicit "see lfx auth login --help").
• ❌ Still open (nit) — mixed keychain/backend wording (E): new code standardizes on "system backend", but the pre-existing insecureStorageFlagName doc comment (auth.go:27) still says "system keychain". One-line fix while you're in there.

Issue summary (open items, carried + new)

• 🔴 Blocking: 0
• 🟡 Minor: 3
  1. --backend status/logout mismatch note is misleading — reuses backendDescription(state.Insecure) and misidentifies a pin-mismatch as a system-vs-other-backend split, omitting the --backend=<name> fix (auth.go:369 / notes at 511, 571).
  2. backendFlagName const doc comment says --backend is "Ignored when --insecure-storage is set", but credStoreFromCommand rejects the combination — contradicts the adjacent Usage string and matches Copilot's open comment (auth.go:34).
  3. README docs still incomplete (item C above).
• ⚪ Nit: 1
  1. keychain/backend wording (item E above).

Bot reconciliation

• Agree with Copilot's open comment that the --backend/--insecure-storage "ignored vs. mutually exclusive" wording is contradictory — you fixed the Usage help and the credstore.go comment, but the auth.go:34 const doc comment still says "Ignored". Folded into minor Add gh-pages install script, CIMD metadata, and docs publishing #2.
• Copilot's cached-token fast path bypasses pinned-backend validation (auth.go:436): I traced this — the fast path reads from the backend credStoreFromCommand already opened per flags and does no refresh/network routing, so the IdP-domain tamper guard isn't bypassed in a way that leaks a secret or reaches an invalid state. It's a minor UX seam (a pinned login can still print a cached token without --backend), not a security hole. Not filing it as blocking.
• Copilot's cross-platform allowlist and concurrent-write comments: the first was addressed (isAvailableBackend now validates OS-compiled backends); the second you declined with PR Add credstore package for keychain-backed credential storage #3 precedent, which is consistent.

Decision

🔴 Needs changes before approval — no blockers remain and the security posture is solid, but there are 3 open minors (two of them quick doc/message fixes, one a small UX correction on the new --backend notes). Close those out and this is an easy approve.

[dealako]

Follow-up review . see the summary comment for full revision tracking and decision rationale. Two inline minors below; no blockers.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

internal/commands/auth.go:599

• Logged out. can be printed while the pinned login remains active. For example, after login with --backend=pass, running logout without that flag may auto-open Secret Service; DeleteCredentials removes nothing there, the mismatch branch preserves the pass state, and this line still reports success even though lfx auth token --backend=pass continues to work. Validate a pinned backend before deleting credentials and return the matching-flag error, or otherwise avoid reporting a successful logout when the stored login was not removed.

			fmt.Println("Logged out.")