From c94ced75037630c1e0238c9eb56bcae53f4c03f5 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Tue, 18 Aug 2026 14:33:29 -0700 Subject: [PATCH 1/9] feat(auth): implement Device Code login flow and token refresh 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/auth0: new package resolving --env to its IdP domain and compiled-in client ID, and driving the device code request, poll, and refresh-token exchange. - internal/commands/auth.go: real login (interactive device code flow or --with-token for headless use), token (cached-token fast path, refresh-and-recache on expiry, clear error on invalid/expired refresh token), status, and logout implementations. - internal/credstore: extends DeviceState with environment/audience (needed to replay the same IdP/client on refresh) and adds DeleteDeviceState for logout. Deliberately does not persist a device ID: Auth0's device flow has no such concept, and the `gh` CLI precedent cited when this story was written turned out to be an unrelated telemetry identifier, not part of its OAuth flow. - Includes outstanding .cspell.json wordlist additions. LFXV2-2515, LFXV2-2516 Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- .cspell.json | 5 + go.mod | 1 + go.sum | 2 + internal/auth0/device.go | 190 +++++++++++++++++ internal/commands/auth.go | 355 ++++++++++++++++++++++++++++++-- internal/credstore/credstore.go | 42 +++- 6 files changed, 574 insertions(+), 21 deletions(-) create mode 100644 internal/auth0/device.go diff --git a/.cspell.json b/.cspell.json index 4de95cc..6601992 100644 --- a/.cspell.json +++ b/.cspell.json @@ -6,8 +6,10 @@ "artipacked", "binname", "binpath", + "bsdtar", "cimd", "clidocs", + "containedctx", "coverprofile", "cpuprof", "credstore", @@ -28,7 +30,10 @@ "memprof", "mgechev", "mktemp", + "mtimes", + "nolint", "pipefail", + "rundll", "techdocs", "trimpath", "urfave", diff --git a/go.mod b/go.mod index 7199b68..c967d6c 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/99designs/keyring v1.2.2 github.com/urfave/cli-docs/v3 v3.1.0 github.com/urfave/cli/v3 v3.10.1 + golang.org/x/oauth2 v0.36.0 ) require ( diff --git a/go.sum b/go.sum index 77c7dc3..652414f 100644 --- a/go.sum +++ b/go.sum @@ -33,6 +33,8 @@ github.com/urfave/cli-docs/v3 v3.1.0 h1:Sa5xm19IpE5gpm6tZzXdfjdFxn67PnEsE4dpXF7v github.com/urfave/cli-docs/v3 v3.1.0/go.mod h1:59d+5Hz1h6GSGJ10cvcEkbIe3j233t4XDqI72UIx7to= github.com/urfave/cli/v3 v3.10.1 h1:7Kx9H50hrHbRbyxgO1KP6/BcbiGRz0uYh5YyQ30JEEY= github.com/urfave/cli/v3 v3.10.1/go.mod h1:ysVLtOEmg2tOy6PknnYVhDoouyC/6N42TMeoMzskhso= +golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= +golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= diff --git a/internal/auth0/device.go b/internal/auth0/device.go new file mode 100644 index 0000000..4bbe920 --- /dev/null +++ b/internal/auth0/device.go @@ -0,0 +1,190 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +// Package auth0 configures the Auth0 Device Authorization Grant (RFC 8628) +// and refresh-token exchange used by `lfx auth login` and `lfx auth token`, +// on top of golang.org/x/oauth2's device flow support +// (Config.DeviceAuth/DeviceAccessToken and TokenSource-based refresh). +package auth0 + +import ( + "context" + "errors" + "fmt" + "net/http" + + "golang.org/x/oauth2" +) + +// Environment identifies which LFX Auth0 tenant/IdP a login targets. +type Environment string + +// Supported environments, selected via the `--env` flag. Each maps to a +// fixed IdP domain and a static, pre-provisioned Auth0 native application +// client ID (device_code + refresh_token grants; see LFXV2-2513 and +// auth0-terraform PR #348). CIMD was evaluated and abandoned for this flow: +// Auth0 silently drops CIMD client registration for the device_code grant. +const ( + EnvProd Environment = "prod" + EnvStaging Environment = "staging" + EnvDevelopment Environment = "development" +) + +// domains maps each Environment to its Auth0 tenant domain. +var domains = map[Environment]string{ + EnvProd: "linuxfoundation.auth0.com", + EnvStaging: "linuxfoundation-staging.auth0.com", + EnvDevelopment: "linuxfoundation-dev.auth0.com", +} + +// clientIDs maps each Environment to its compiled-in, pre-provisioned +// Auth0 native application client ID for the device code grant. +// cspell:disable -- opaque, randomly-generated Auth0 client IDs, not words. +var clientIDs = map[Environment]string{ + EnvProd: "kkCpM0c9zJ0vNZZDDOGqcyzocOBircOn", + EnvStaging: "9XzXgDfAB9O7IoHqhBj5mg4VLvdBM8ci", + EnvDevelopment: "0TN1OElqQY146vLEPdV5qfejRKpc9IAZ", +} + +// cspell:enable + +// DefaultAudience is the production LFX v2 API audience used unless +// overridden via `--audience`. It intentionally does not vary with `--env`: +// per LFXV2-2515, the audience is independent of the selected environment +// and must be set explicitly when testing a non-prod API. +const DefaultAudience = "https://lfx-api.v2.cluster.lfx.dev/" + +// ErrInvalidEnvironment is returned by Resolve for an unrecognized +// Environment value. +var ErrInvalidEnvironment = errors.New("auth0: invalid environment") + +// Resolve returns the IdP domain and client ID for env. +func Resolve(env Environment) (domain, clientID string, err error) { + domain, ok := domains[env] + if !ok { + return "", "", fmt.Errorf("%w: %q (must be one of prod, staging, development)", ErrInvalidEnvironment, env) + } + return domain, clientIDs[env], nil +} + +// Client drives the Auth0 device code and refresh-token exchanges for a +// single environment, via an underlying oauth2.Config. Auth0's device code +// application is a public native client (Token Endpoint Authentication +// Method: None; is_first_party = true), so ClientSecret is deliberately +// left unset. +type Client struct { + // Domain is the Auth0 tenant domain, e.g. "linuxfoundation.auth0.com". + Domain string + // ClientID is the Auth0 application client ID used for both the + // device code request and subsequent token/refresh exchanges. + ClientID string + // HTTPClient is used for all requests. Defaults to + // http.DefaultClient if nil. + HTTPClient *http.Client +} + +// config builds the oauth2.Config used for a device code flow requesting +// scopes, pointed at this Client's Auth0 tenant. +func (c *Client) config(scopes []string) *oauth2.Config { + return &oauth2.Config{ + ClientID: c.ClientID, + Scopes: scopes, + Endpoint: oauth2.Endpoint{ + DeviceAuthURL: "https://" + c.Domain + "/oauth/device/code", + TokenURL: "https://" + c.Domain + "/oauth/token", + AuthStyle: oauth2.AuthStyleInParams, + }, + } +} + +// context attaches Client's HTTPClient to ctx, per the convention +// documented on oauth2.HTTPClient, so all requests made by the resulting +// oauth2.Config use it. +func (c *Client) context(ctx context.Context) context.Context { + if c.HTTPClient == nil { + return ctx + } + return context.WithValue(ctx, oauth2.HTTPClient, c.HTTPClient) +} + +// DeviceCode wraps the RFC 8628 §3.2 device authorization response, +// carrying the oauth2.Config needed to complete the flow via Poll. +type DeviceCode struct { + *oauth2.DeviceAuthResponse + cfg *oauth2.Config + ctx context.Context //nolint:containedctx // ctx is captured for reuse by Poll, matching Client.RequestDeviceCode/Poll's split across a user-facing pause. +} + +// RequestDeviceCode starts the device authorization flow for the given +// audience and scopes. +func (c *Client) RequestDeviceCode(ctx context.Context, audience string, scopes []string) (*DeviceCode, error) { + cfg := c.config(scopes) + authCtx := c.context(ctx) + + var opts []oauth2.AuthCodeOption + if audience != "" { + opts = append(opts, oauth2.SetAuthURLParam("audience", audience)) + } + + resp, err := cfg.DeviceAuth(authCtx, opts...) + if err != nil { + return nil, fmt.Errorf("auth0: request device code: %w", err) + } + return &DeviceCode{DeviceAuthResponse: resp, cfg: cfg, ctx: authCtx}, nil +} + +// Errors surfaced by Poll when the token endpoint reports the user has +// explicitly declined (RFC 8628 §3.5's "access_denied") or the device code +// expired before the flow completed ("expired_token"). Poll otherwise +// blocks internally on "authorization_pending"/"slow_down" until one of +// these, success, or ctx's deadline (bounded by the device code's own +// expiry). +var ( + // ErrAccessDenied indicates the user explicitly declined the request. + ErrAccessDenied = errors.New("auth0: access denied") + // ErrExpiredToken indicates the device code expired before the user + // completed the flow. + ErrExpiredToken = errors.New("auth0: device code expired") + // ErrInvalidGrant indicates the supplied refresh token is expired, + // revoked, or otherwise no longer valid (Auth0's "invalid_grant" + // error from the refresh_token grant). Callers should treat this as + // requiring the user to run `lfx auth login` again. + ErrInvalidGrant = errors.New("auth0: invalid or expired refresh token") +) + +// Poll blocks until the user completes (or rejects) the device flow, +// polling the token endpoint at the interval Auth0 specified (respecting +// "slow_down" backoff) via oauth2.Config.DeviceAccessToken. It returns +// ErrAccessDenied or ErrExpiredToken for those respective outcomes. +func (dc *DeviceCode) Poll() (*oauth2.Token, error) { + tok, err := dc.cfg.DeviceAccessToken(dc.ctx, dc.DeviceAuthResponse) + if err == nil { + return tok, nil + } + + var retrieveErr *oauth2.RetrieveError + if errors.As(err, &retrieveErr) { + switch retrieveErr.ErrorCode { + case "access_denied": + return nil, ErrAccessDenied + case "expired_token": + return nil, ErrExpiredToken + } + } + return nil, fmt.Errorf("auth0: poll for token: %w", err) +} + +// RefreshToken exchanges a refresh token for a new access token. +func (c *Client) RefreshToken(ctx context.Context, refreshToken string) (*oauth2.Token, error) { + cfg := c.config(nil) + tok, err := cfg.TokenSource(c.context(ctx), &oauth2.Token{RefreshToken: refreshToken}).Token() + if err == nil { + return tok, nil + } + + var retrieveErr *oauth2.RetrieveError + if errors.As(err, &retrieveErr) && retrieveErr.ErrorCode == "invalid_grant" { + return nil, ErrInvalidGrant + } + return nil, fmt.Errorf("auth0: refresh token: %w", err) +} diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 407629c..def32a9 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -5,9 +5,21 @@ package commands import ( + "bufio" "context" + "encoding/base64" + "encoding/json" + "errors" "fmt" + "io" + "os" + "os/exec" + "runtime" + "strings" + "time" + "github.com/linuxfoundation/lfx-cli/internal/auth0" + "github.com/linuxfoundation/lfx-cli/internal/credstore" "github.com/urfave/cli/v3" ) @@ -16,17 +28,25 @@ import ( // unencrypted file. const insecureStorageFlagName = "insecure-storage" +// Flag names shared by the login command. +const ( + webFlagName = "web" + withTokenFlagName = "with-token" + envFlagName = "env" + audienceFlagName = "audience" +) + +// scopes requested during the device code flow. offline_access is required +// to receive a refresh token; the rest identify the user for `auth status`. +var loginScopes = []string{"openid", "profile", "email", "offline_access"} + // NewAuthCommand builds the `lfx auth` command group with its subcommands. // -// The --insecure-storage flag is shared by all subcommands and controls -// whether credentials bypass the system keychain in favor of credstore's -// plain (unencrypted) file fallback, e.g. for headless/CI use. -// -// The login, token, status, and logout actions are currently stubs; real -// implementations land in LFXV2-2515 (login flow) and LFXV2-2516 (token -// command), at which point they'll build a credstore.Store via -// credstore.New(credstore.Options{Insecure: -// cmd.Bool(insecureStorageFlagName)}). +// The --insecure-storage flag is shared by all subcommands (it is not +// declared as a "Local" flag, so urfave/cli resolves it for subcommand +// actions via cmd.Bool) and controls whether credentials bypass the system +// keychain in favor of credstore's plain (unencrypted) file fallback, e.g. +// for headless/CI use. func NewAuthCommand() *cli.Command { return &cli.Command{ Name: "auth", @@ -46,23 +66,222 @@ func NewAuthCommand() *cli.Command { } } +// credStoreFromCommand builds a credstore.Store using the group-level +// --insecure-storage flag, however deep in the `auth` subcommand tree cmd +// is. +func credStoreFromCommand(cmd *cli.Command) (credstore.Store, error) { + return credstore.New(credstore.Options{Insecure: cmd.Bool(insecureStorageFlagName)}) +} + func newAuthLoginCommand() *cli.Command { return &cli.Command{ Name: "login", Usage: "Log in to the LFX platform via the Auth0 Device Code flow", - Action: func(_ context.Context, _ *cli.Command) error { - fmt.Println("lfx auth login: not yet implemented (see LFXV2-2515)") - return nil + Flags: []cli.Flag{ + &cli.BoolFlag{ + Name: webFlagName, + Aliases: []string{"w"}, + Usage: "Automatically open the verification URL in the default browser", + }, + &cli.BoolFlag{ + Name: withTokenFlagName, + Usage: "Read a refresh token from stdin instead of performing the interactive Device Code flow", + }, + &cli.StringFlag{ + Name: envFlagName, + Usage: "Target environment: prod, staging, or development", + Value: string(auth0.EnvProd), + }, + &cli.StringFlag{ + Name: audienceFlagName, + Usage: "Auth0 API audience to request tokens for (independent of --env)", + Value: auth0.DefaultAudience, + }, }, + Action: runAuthLogin, + } +} + +func runAuthLogin(ctx context.Context, cmd *cli.Command) error { + store, err := credStoreFromCommand(cmd) + if err != nil { + return err + } + + env := auth0.Environment(cmd.String(envFlagName)) + domain, clientID, err := auth0.Resolve(env) + if err != nil { + return err + } + audience := cmd.String(audienceFlagName) + + if cmd.Bool(withTokenFlagName) { + return loginWithToken(store, env, domain, audience) + } + + client := &auth0.Client{Domain: domain, ClientID: clientID} + return loginWithDeviceCode(ctx, cmd, client, store, env, domain, audience) +} + +// loginWithToken implements `--with-token`: it reads a refresh token from +// stdin (one line, trimmed) for headless/CI use, e.g. +// `echo "$REFRESH_TOKEN" | lfx auth login --with-token`. No access token is +// cached; the next `lfx auth token` call exchanges the refresh token for +// one. +func loginWithToken(store credstore.Store, env auth0.Environment, domain, audience string) error { + reader := bufio.NewReader(os.Stdin) + line, err := reader.ReadString('\n') + if err != nil && !errors.Is(err, io.EOF) { + return fmt.Errorf("read refresh token from stdin: %w", err) + } + refreshToken := strings.TrimSpace(line) + if refreshToken == "" { + return errors.New("no refresh token provided on stdin") + } + + if err := store.SaveCredentials(credstore.Credentials{RefreshToken: refreshToken}); err != nil { + return fmt.Errorf("save credentials: %w", err) + } + if err := store.SaveDeviceState(credstore.DeviceState{ + IDPDomain: domain, + Environment: string(env), + Audience: audience, + }); err != nil { + return fmt.Errorf("save device state: %w", err) + } + + fmt.Println("Logged in with a supplied refresh token.") + return nil +} + +// loginWithDeviceCode performs the interactive Auth0 Device Code flow: +// request a device code, show the user code and verification URL +// (optionally opening it in a browser), then poll until the user completes +// or the flow expires/is denied. +func loginWithDeviceCode( + ctx context.Context, + cmd *cli.Command, + client *auth0.Client, + store credstore.Store, + env auth0.Environment, + domain, audience string, +) error { + dc, err := client.RequestDeviceCode(ctx, audience, loginScopes) + if err != nil { + return err + } + + fmt.Printf("First copy your one-time code: %s\n", dc.UserCode) + if cmd.Bool(webFlagName) { + fmt.Printf("Opening %s in your browser...\n", dc.VerificationURIComplete) + if err := openBrowser(dc.VerificationURIComplete); err != nil { + fmt.Printf("Couldn't open browser automatically: %v\n", err) + fmt.Printf("Please visit: %s\n", dc.VerificationURIComplete) + } + } else { + fmt.Printf("Then visit: %s\n", dc.VerificationURIComplete) + } + fmt.Println("Waiting for authentication...") + + // Poll blocks internally until success or failure, honoring the + // device code's own expiry and Auth0's requested polling interval + // (including "slow_down" backoff). + token, err := dc.Poll() + switch { + case err == nil: + case errors.Is(err, auth0.ErrAccessDenied): + return errors.New("login was denied") + case errors.Is(err, auth0.ErrExpiredToken): + return errors.New("device code expired before login completed") + default: + return err + } + + if err := store.SaveCredentials(credstore.Credentials{ + RefreshToken: token.RefreshToken, + AccessToken: token.AccessToken, + AccessTokenExpiry: token.Expiry, + }); err != nil { + return fmt.Errorf("save credentials: %w", err) } + if err := store.SaveDeviceState(credstore.DeviceState{ + IDPDomain: domain, + Environment: string(env), + Audience: audience, + }); err != nil { + return fmt.Errorf("save device state: %w", err) + } + + fmt.Println("Login successful.") + if idToken, ok := token.Extra("id_token").(string); ok { + if identity := identityFromIDToken(idToken); identity != "" { + fmt.Printf("Logged in as %s.\n", identity) + } + } + return nil } func newAuthTokenCommand() *cli.Command { return &cli.Command{ Name: "token", Usage: "Print a valid access token for the LFX platform", - Action: func(_ context.Context, _ *cli.Command) error { - fmt.Println("lfx auth token: not yet implemented (see LFXV2-2516)") + Action: func(ctx context.Context, cmd *cli.Command) error { + store, err := credStoreFromCommand(cmd) + if err != nil { + return err + } + + creds, err := store.LoadCredentials() + if errors.Is(err, credstore.ErrNotFound) { + return errors.New("not logged in; run `lfx auth login` first") + } + if err != nil { + return err + } + + if creds.ValidAccessToken() { + fmt.Println(creds.AccessToken) + return nil + } + + if creds.RefreshToken == "" { + return errors.New("no refresh token available; run `lfx auth login` again") + } + + state, err := store.LoadDeviceState() + if err != nil { + return fmt.Errorf("load device state: %w", err) + } + + _, clientID, err := auth0.Resolve(auth0.Environment(state.Environment)) + if err != nil { + return err + } + client := &auth0.Client{Domain: state.IDPDomain, ClientID: clientID} + + token, err := client.RefreshToken(ctx, creds.RefreshToken) + if errors.Is(err, auth0.ErrInvalidGrant) { + return errors.New("session expired or revoked; run `lfx auth login` to log in again") + } + if err != nil { + return fmt.Errorf("refresh access token: %w", err) + } + + refreshToken := token.RefreshToken + if refreshToken == "" { + // Auth0 may not rotate the refresh token on every + // exchange; keep the existing one in that case. + refreshToken = creds.RefreshToken + } + if err := store.SaveCredentials(credstore.Credentials{ + RefreshToken: refreshToken, + AccessToken: token.AccessToken, + AccessTokenExpiry: token.Expiry, + }); err != nil { + return fmt.Errorf("save refreshed credentials: %w", err) + } + + fmt.Println(token.AccessToken) return nil }, } @@ -72,8 +291,48 @@ func newAuthStatusCommand() *cli.Command { return &cli.Command{ Name: "status", Usage: "Show the current authentication status", - Action: func(_ context.Context, _ *cli.Command) error { - fmt.Println("lfx auth status: not yet implemented (see LFXV2-2515)") + Action: func(_ context.Context, cmd *cli.Command) error { + store, err := credStoreFromCommand(cmd) + if err != nil { + return err + } + + creds, err := store.LoadCredentials() + if errors.Is(err, credstore.ErrNotFound) { + fmt.Println("Not logged in.") + return nil + } + if err != nil { + return err + } + + state, err := store.LoadDeviceState() + if err != nil && !errors.Is(err, credstore.ErrNotFound) { + return fmt.Errorf("load device state: %w", err) + } + + backend := "system keychain" + if cmd.Bool(insecureStorageFlagName) { + backend = "plain file (--insecure-storage)" + } + + fmt.Println("Logged in.") + if state.Environment != "" { + fmt.Printf(" Environment: %s\n", state.Environment) + } + if state.IDPDomain != "" { + fmt.Printf(" IdP domain: %s\n", state.IDPDomain) + } + if state.Audience != "" { + fmt.Printf(" Audience: %s\n", state.Audience) + } + fmt.Printf(" Credential backend: %s\n", backend) + if creds.ValidAccessToken() { + fmt.Printf(" Access token expires: %s\n", creds.AccessTokenExpiry.Format(time.RFC3339)) + } else { + fmt.Println(" Access token: expired or not cached (will refresh on next `lfx auth token`)") + } + return nil }, } @@ -83,9 +342,69 @@ func newAuthLogoutCommand() *cli.Command { return &cli.Command{ Name: "logout", Usage: "Remove stored LFX platform credentials", - Action: func(_ context.Context, _ *cli.Command) error { - fmt.Println("lfx auth logout: not yet implemented (see LFXV2-2515)") + Action: func(_ context.Context, cmd *cli.Command) error { + store, err := credStoreFromCommand(cmd) + if err != nil { + return err + } + + if err := store.DeleteCredentials(); err != nil { + return fmt.Errorf("delete credentials: %w", err) + } + if err := store.DeleteDeviceState(); err != nil { + return fmt.Errorf("delete device state: %w", err) + } + + fmt.Println("Logged out.") return nil }, } } + +// identityFromIDToken extracts a human-readable identifier (email or +// subject) from an unverified decode of the ID token's JWT payload. It is +// used only for a friendly "Logged in as ..." message; the access token +// (verified server-side on every API call) is the actual credential. +func identityFromIDToken(idToken string) string { + parts := strings.Split(idToken, ".") + if len(parts) != 3 { + return "" + } + + payload, err := base64URLDecode(parts[1]) + if err != nil { + return "" + } + + var claims struct { + Email string `json:"email"` + Sub string `json:"sub"` + } + if err := json.Unmarshal(payload, &claims); err != nil { + return "" + } + + if claims.Email != "" { + return claims.Email + } + return claims.Sub +} + +// base64URLDecode decodes a base64url-encoded JWT segment, tolerating the +// missing padding that JWTs conventionally omit. +func base64URLDecode(s string) ([]byte, error) { + return base64.RawURLEncoding.DecodeString(s) +} + +// openBrowser opens url in the user's default browser, following the +// per-OS conventions used by tools like `gh`. +func openBrowser(url string) error { + switch runtime.GOOS { + case "darwin": + return exec.Command("open", url).Start() + case "windows": + return exec.Command("rundll32", "url.dll,FileProtocolHandler", url).Start() + default: + return exec.Command("xdg-open", url).Start() + } +} diff --git a/internal/credstore/credstore.go b/internal/credstore/credstore.go index 660c015..9f83c07 100644 --- a/internal/credstore/credstore.go +++ b/internal/credstore/credstore.go @@ -22,7 +22,8 @@ // acceptable, and is deliberately less secure than the keyring-backed // storage. // -// Non-sensitive state (device ID, IdP domain used at login) is always stored +// Non-sensitive state (environment, IdP domain, and audience used at login) +// is always stored // as plain JSON under the XDG state directory (~/.local/state/lfx-cli/ by // default), per XDG Base Directory conventions for mutable runtime state. package credstore @@ -94,10 +95,30 @@ func (c Credentials) ValidAccessToken() bool { // DeviceState holds non-sensitive information persisted between CLI // invocations so that commands like `lfx auth token` don't need to -// re-specify the IdP domain used at login. +// re-specify the environment, IdP domain, or audience used at login. +// +// Note: this deliberately does not include a persistent "device ID". One +// was considered (see LFXV2-2515/LFXV2-2509 discussion) on the assumption +// that `gh` uses one as part of its OAuth device flow, but `gh`'s +// `~/.local/state/gh/device-id` is actually just an anonymous telemetry +// identifier (see `internal/telemetry.getOrCreateDeviceID` in +// github.com/cli/cli) -- it plays no role in the OAuth device +// authorization grant and isn't sent to GitHub's API. Since the LFX CLI +// has no telemetry pipeline, and Auth0's device flow has no concept of a +// device ID at all, there's nothing here for one to do. Revisit if/when +// opt-in CLI telemetry is added. type DeviceState struct { - DeviceID string `json:"device_id"` IDPDomain string `json:"idp_domain,omitempty"` + // Environment is the `--env` value used at login (prod, staging, or + // development), determining which compiled-in client ID is used to + // refresh the access token. + Environment string `json:"environment,omitempty"` + // Audience is the `--audience` value used at login. Auth0's + // refresh_token grant automatically ties the refreshed access token + // to the audience it was originally issued for, so this isn't sent + // back on refresh; it's persisted purely for display in + // `lfx auth status`. + Audience string `json:"audience,omitempty"` } // Store is the credential storage abstraction used by the auth commands. @@ -117,6 +138,9 @@ type Store interface { // LoadDeviceState returns the persisted device state, or ErrNotFound if // none has been saved. LoadDeviceState() (DeviceState, error) + // DeleteDeviceState removes any persisted device state. It is a no-op + // if none exists. + DeleteDeviceState() error } // Options configures a Store returned by New. @@ -297,6 +321,18 @@ func (s *store) LoadDeviceState() (DeviceState, error) { return state, nil } +// DeleteDeviceState implements Store. +func (s *store) DeleteDeviceState() error { + path := filepath.Join(s.stateDir, stateFileName) + + err := os.Remove(path) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("credstore: delete device state: %w", err) + } + + return nil +} + // keyringSecrets is a secretsBackend that stores Credentials in a real // system credential store via keyring.Keyring (see systemBackends). type keyringSecrets struct { From 05b5affc67a67853492bf504249c25351df1ab52 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Wed, 19 Aug 2026 09:32:10 -0700 Subject: [PATCH 2/9] fix(review): address PR #4 Copilot review feedback - auth token: use auth0.Resolve's domain (trusted) for the refresh client instead of the persisted state.json value, while still sanity-checking it against the stored domain to catch a tampered or corrupted state file before it can redirect a refresh request. - credstore: add DeviceState.Insecure, recording which credential backend (keychain vs. --insecure-storage) wrote state.json. auth token/status/logout now validate this against the invocation's own --insecure-storage flag before trusting or deleting state.json, so switching backends no longer silently corrupts or destroys the other backend's metadata. - README.md/AGENTS.md: update the stale "auth commands are stubs" note now that lfx auth is fully implemented; only lfx api remains a stub. - auth0: map DeviceAccessToken's context.DeadlineExceeded (returned once the device code's own expiry passes, ahead of the token endpoint ever reporting expired_token) to ErrExpiredToken, so the documented error and "device code expired" message are reliably produced. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- AGENTS.md | 14 ++-- README.md | 4 +- internal/auth0/device.go | 13 ++++ internal/commands/auth.go | 112 ++++++++++++++++++++++++++++---- internal/credstore/credstore.go | 9 +++ 5 files changed, 129 insertions(+), 23 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c761719..ee97003 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,15 +46,11 @@ lfx-cli/ ### Current State -This repo is under active scaffolding. Auth and API commands are currently -stubs; real implementations land in follow-on work: - -- `lfx auth login` / `status` / `logout` -- `lfx auth token` -- `lfx api` - -Credential storage (system keychain via `99designs/keyring`) and the Auth0 -CIMD client are tracked separately. +`lfx auth login` / `status` / `token` / `logout` are fully implemented, +including the Auth0 Device Code flow, refresh-token exchange, and +credential storage (system keychain via `99designs/keyring`, with a plain +`--insecure-storage` fallback). `lfx api` remains a stub; its +implementation lands in follow-on work. **No container build**: this project produces binary artifacts only, distributed via GitHub Releases, the `install.sh` curl-style installer diff --git a/README.md b/README.md index 44f4b9d..0f7f734 100644 --- a/README.md +++ b/README.md @@ -58,8 +58,8 @@ lfx auth login --insecure-storage Run `lfx --help` or `lfx --help` for full details on any command. -> **Note:** This project is under active development. Authentication and API -> commands are currently stubs; see the +> **Note:** This project is under active development. `lfx auth` is fully +> implemented; `lfx api` is currently a stub. See the > [LFXV2-2509 epic](https://linuxfoundation.atlassian.net/browse/LFXV2-2509) > for status. diff --git a/internal/auth0/device.go b/internal/auth0/device.go index 4bbe920..4cf0504 100644 --- a/internal/auth0/device.go +++ b/internal/auth0/device.go @@ -171,6 +171,19 @@ func (dc *DeviceCode) Poll() (*oauth2.Token, error) { return nil, ErrExpiredToken } } + + // DeviceAccessToken derives its own polling deadline from the device + // code's Expiry and returns the bare context.DeadlineExceeded (not a + // *oauth2.RetrieveError) once that deadline passes, rather than + // waiting for the token endpoint to report "expired_token" itself. + // Since this Client never sets its own deadline on the context it + // 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 + } + return nil, fmt.Errorf("auth0: poll for token: %w", err) } diff --git a/internal/commands/auth.go b/internal/commands/auth.go index def32a9..eeb28ed 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -114,13 +114,14 @@ func runAuthLogin(ctx context.Context, cmd *cli.Command) error { return err } audience := cmd.String(audienceFlagName) + insecure := cmd.Bool(insecureStorageFlagName) if cmd.Bool(withTokenFlagName) { - return loginWithToken(store, env, domain, audience) + return loginWithToken(store, env, domain, audience, insecure) } client := &auth0.Client{Domain: domain, ClientID: clientID} - return loginWithDeviceCode(ctx, cmd, client, store, env, domain, audience) + return loginWithDeviceCode(ctx, cmd, client, store, env, domain, audience, insecure) } // loginWithToken implements `--with-token`: it reads a refresh token from @@ -128,7 +129,7 @@ func runAuthLogin(ctx context.Context, cmd *cli.Command) error { // `echo "$REFRESH_TOKEN" | lfx auth login --with-token`. No access token is // cached; the next `lfx auth token` call exchanges the refresh token for // one. -func loginWithToken(store credstore.Store, env auth0.Environment, domain, audience string) error { +func loginWithToken(store credstore.Store, env auth0.Environment, domain, audience string, insecure bool) error { reader := bufio.NewReader(os.Stdin) line, err := reader.ReadString('\n') if err != nil && !errors.Is(err, io.EOF) { @@ -146,6 +147,7 @@ func loginWithToken(store credstore.Store, env auth0.Environment, domain, audien IDPDomain: domain, Environment: string(env), Audience: audience, + Insecure: insecure, }); err != nil { return fmt.Errorf("save device state: %w", err) } @@ -165,6 +167,7 @@ func loginWithDeviceCode( store credstore.Store, env auth0.Environment, domain, audience string, + insecure bool, ) error { dc, err := client.RequestDeviceCode(ctx, audience, loginScopes) if err != nil { @@ -208,6 +211,7 @@ func loginWithDeviceCode( IDPDomain: domain, Environment: string(env), Audience: audience, + Insecure: insecure, }); err != nil { return fmt.Errorf("save device state: %w", err) } @@ -221,6 +225,69 @@ func loginWithDeviceCode( return nil } +// loadDeviceStateForBackend loads the persisted device state and validates +// it against the current invocation before returning it: +// +// - state.Insecure must match cmd's --insecure-storage flag. state.json is +// shared by both the keychain and plain-file credential backends (see +// credstore.DeviceState.Insecure), so a mismatch means this invocation's +// credentials were saved under a different backend than the one that +// last wrote state.json -- trusting it here would silently mix a +// refresh token from one backend with IdP/environment metadata written +// for the other. +// - the IdP domain implied by state.Environment (via auth0.Resolve, the +// source of truth) must match the persisted state.IDPDomain, guarding +// against a tampered or corrupted state.json redirecting a refresh +// request -- and its long-lived refresh token -- to another host. +// +// On success it returns the state along with the trusted domain and client +// ID to use for any Auth0 request (always auth0.Resolve's values, never the +// persisted ones). +func loadDeviceStateForBackend(store credstore.Store, cmd *cli.Command) (state credstore.DeviceState, domain, clientID string, err error) { + state, err = store.LoadDeviceState() + if err != nil { + return credstore.DeviceState{}, "", "", err + } + + if state.Insecure != cmd.Bool(insecureStorageFlagName) { + return credstore.DeviceState{}, "", "", fmt.Errorf( + "stored login state belongs to %s; pass %s to match, or run `lfx auth login` again", + backendDescription(state.Insecure), insecureStorageUsageHint(state.Insecure), + ) + } + + domain, clientID, err = auth0.Resolve(auth0.Environment(state.Environment)) + if err != nil { + return credstore.DeviceState{}, "", "", err + } + if domain != state.IDPDomain { + return credstore.DeviceState{}, "", "", fmt.Errorf( + "stored IdP domain %q does not match %q for environment %q; run `lfx auth login` again", + state.IDPDomain, domain, state.Environment, + ) + } + + return state, domain, clientID, nil +} + +// backendDescription renders a human-readable name for a credential +// backend, for use in error messages. +func backendDescription(insecure bool) string { + if insecure { + return "the plain-file (--insecure-storage) backend" + } + return "the system keychain" +} + +// insecureStorageUsageHint renders the flag (or its absence) needed to +// select the given backend, for use in error messages. +func insecureStorageUsageHint(insecure bool) string { + if insecure { + return "--insecure-storage" + } + return "no --insecure-storage" +} + func newAuthTokenCommand() *cli.Command { return &cli.Command{ Name: "token", @@ -248,16 +315,11 @@ func newAuthTokenCommand() *cli.Command { return errors.New("no refresh token available; run `lfx auth login` again") } - state, err := store.LoadDeviceState() + _, domain, clientID, err := loadDeviceStateForBackend(store, cmd) if err != nil { return fmt.Errorf("load device state: %w", err) } - - _, clientID, err := auth0.Resolve(auth0.Environment(state.Environment)) - if err != nil { - return err - } - client := &auth0.Client{Domain: state.IDPDomain, ClientID: clientID} + client := &auth0.Client{Domain: domain, ClientID: clientID} token, err := client.RefreshToken(ctx, creds.RefreshToken) if errors.Is(err, auth0.ErrInvalidGrant) { @@ -317,6 +379,13 @@ func newAuthStatusCommand() *cli.Command { } fmt.Println("Logged in.") + if err == nil && state.Insecure != cmd.Bool(insecureStorageFlagName) { + fmt.Printf( + " Note: stored login state below belongs to %s, not this credential backend; "+ + "it may not describe these credentials. Run `lfx auth login` to refresh it.\n", + backendDescription(state.Insecure), + ) + } if state.Environment != "" { fmt.Printf(" Environment: %s\n", state.Environment) } @@ -351,8 +420,27 @@ func newAuthLogoutCommand() *cli.Command { if err := store.DeleteCredentials(); err != nil { return fmt.Errorf("delete credentials: %w", err) } - if err := store.DeleteDeviceState(); err != nil { - return fmt.Errorf("delete device state: %w", err) + + // state.json is shared by both credential backends (see + // credstore.DeviceState.Insecure), so only delete it when it + // actually describes this invocation's backend; otherwise + // logging out of one backend would destroy metadata (env, IdP + // domain) still needed by the other backend's credentials. + state, err := store.LoadDeviceState() + switch { + case errors.Is(err, credstore.ErrNotFound): + // Nothing to delete. + case err != nil: + return fmt.Errorf("load device state: %w", err) + case state.Insecure == cmd.Bool(insecureStorageFlagName): + if err := store.DeleteDeviceState(); err != nil { + return fmt.Errorf("delete device state: %w", err) + } + default: + fmt.Printf( + "Note: leaving stored login state in place; it belongs to %s.\n", + backendDescription(state.Insecure), + ) } fmt.Println("Logged out.") diff --git a/internal/credstore/credstore.go b/internal/credstore/credstore.go index 9f83c07..9b9136d 100644 --- a/internal/credstore/credstore.go +++ b/internal/credstore/credstore.go @@ -119,6 +119,15 @@ type DeviceState struct { // back on refresh; it's persisted purely for display in // `lfx auth status`. Audience string `json:"audience,omitempty"` + // Insecure records whether `--insecure-storage` was passed at login, + // i.e. whether Credentials live in the plain-file backend rather than + // the system keychain. state.json itself is not namespaced by + // backend (both share the same file), so callers must check this + // against the invocation's own --insecure-storage flag before trusting + // the rest of the state: without that check, logging into one backend + // silently overwrites the metadata (env, IdP domain) that the other + // backend's still-present credentials depend on. + Insecure bool `json:"insecure,omitempty"` } // Store is the credential storage abstraction used by the auth commands. From 161c203764ffadb16f57e098338efc5a10a48ae7 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Wed, 19 Aug 2026 10:32:52 -0700 Subject: [PATCH 3/9] fix(auth0): use prod custom domain sso.linuxfoundation.org lfx auth login --env prod was sending device code and token requests to linuxfoundation.auth0.com, but end users only ever authenticate at the prod tenant's custom domain, sso.linuxfoundation.org (see auth0-terraform's auth0_domain variable). The mismatch surfaced as an Auth0 invalid_request: Missing required parameter: response_type error completing login, and a login that never resolved when polling. Since this Client only talks to the device code and token endpoints (never the Auth0 Management API), there's no need to separately track each environment's underlying tenant name -- only the IdP domain end users authenticate against, which for prod is the custom domain rather than the *.auth0.com domain. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- internal/auth0/device.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/internal/auth0/device.go b/internal/auth0/device.go index 4cf0504..18d7b13 100644 --- a/internal/auth0/device.go +++ b/internal/auth0/device.go @@ -30,9 +30,15 @@ const ( EnvDevelopment Environment = "development" ) -// domains maps each Environment to its Auth0 tenant domain. +// domains maps each Environment to the Auth0 IdP domain end users +// authenticate against, matching auth0-terraform's own `auth0_domain` +// variable. This is deliberately not each tenant's *.auth0.com domain: prod +// fronts its tenant with the custom domain sso.linuxfoundation.org, and +// since this Client never calls the Auth0 Management API (only the device +// code and token endpoints), there's no need to separately track the +// underlying tenant name. var domains = map[Environment]string{ - EnvProd: "linuxfoundation.auth0.com", + EnvProd: "sso.linuxfoundation.org", EnvStaging: "linuxfoundation-staging.auth0.com", EnvDevelopment: "linuxfoundation-dev.auth0.com", } @@ -73,7 +79,9 @@ func Resolve(env Environment) (domain, clientID string, err error) { // Method: None; is_first_party = true), so ClientSecret is deliberately // left unset. type Client struct { - // Domain is the Auth0 tenant domain, e.g. "linuxfoundation.auth0.com". + // Domain is the Auth0 IdP domain end users authenticate against, e.g. + // "linuxfoundation-dev.auth0.com" or, for a tenant fronted by a custom + // domain, "sso.linuxfoundation.org". Domain string // ClientID is the Auth0 application client ID used for both the // device code request and subsequent token/refresh exchanges. From d94eda8135f7c210ef2e0249edbec0713a3f0b0b Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Wed, 19 Aug 2026 10:35:45 -0700 Subject: [PATCH 4/9] fix(review): address second round of Copilot review comments - Poll(): distinguish a caller-supplied context deadline from DeviceAccessToken's own internally-derived one before mapping DeadlineExceeded to ErrExpiredToken, so an unrelated caller timeout is no longer misreported as an expired device code. - loginWithDeviceCode: fall back to VerificationURI when VerificationURIComplete is empty (it's optional per RFC 8628 section 3.2), so a login doesn't try to open/print an empty URL. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- internal/auth0/device.go | 11 ++++++----- internal/commands/auth.go | 15 +++++++++++---- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/internal/auth0/device.go b/internal/auth0/device.go index 18d7b13..7389bf9 100644 --- a/internal/auth0/device.go +++ b/internal/auth0/device.go @@ -184,11 +184,12 @@ func (dc *DeviceCode) Poll() (*oauth2.Token, error) { // code's Expiry and returns the bare context.DeadlineExceeded (not a // *oauth2.RetrieveError) once that deadline passes, rather than // waiting for the token endpoint to report "expired_token" itself. - // Since this Client never sets its own deadline on the context it - // 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) { + // dc.ctx may also carry a caller-supplied deadline of its own (e.g. if + // RequestDeviceCode was called with one), so only treat + // DeadlineExceeded as an expired device code if dc.ctx itself isn't + // what expired -- otherwise this would misreport an unrelated caller + // timeout as ErrExpiredToken. + if errors.Is(err, context.DeadlineExceeded) && dc.ctx.Err() == nil { return nil, ErrExpiredToken } diff --git a/internal/commands/auth.go b/internal/commands/auth.go index eeb28ed..1c40e3f 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -175,14 +175,21 @@ func loginWithDeviceCode( } fmt.Printf("First copy your one-time code: %s\n", dc.UserCode) + // VerificationURIComplete is optional per RFC 8628 §3.2; fall back to + // VerificationURI (always present) plus the user code if the IdP + // doesn't supply it. + verificationURI := dc.VerificationURIComplete + if verificationURI == "" { + verificationURI = dc.VerificationURI + } if cmd.Bool(webFlagName) { - fmt.Printf("Opening %s in your browser...\n", dc.VerificationURIComplete) - if err := openBrowser(dc.VerificationURIComplete); err != nil { + fmt.Printf("Opening %s in your browser...\n", verificationURI) + if err := openBrowser(verificationURI); err != nil { fmt.Printf("Couldn't open browser automatically: %v\n", err) - fmt.Printf("Please visit: %s\n", dc.VerificationURIComplete) + fmt.Printf("Please visit: %s\n", verificationURI) } } else { - fmt.Printf("Then visit: %s\n", dc.VerificationURIComplete) + fmt.Printf("Then visit: %s\n", verificationURI) } fmt.Println("Waiting for authentication...") From b6388d1e8a0e814ec2d26d928d75560f212f0568 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Wed, 19 Aug 2026 10:41:55 -0700 Subject: [PATCH 5/9] feat(auth): show LFID username in the login greeting lfx auth login's "Logged in as ..." message now prefers the LFID username (the https://sso.linuxfoundation.org/claims/ custom ID token claim) over email, since username is the conventional LFX identifier. Shows "username (email)" when both are present, falling back to "email (no username)" in the unexpected case the custom claim is missing, and finally the subject claim if neither is present. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- internal/commands/auth.go | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 1c40e3f..e64a832 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -456,10 +456,18 @@ func newAuthLogoutCommand() *cli.Command { } } -// identityFromIDToken extracts a human-readable identifier (email or -// subject) from an unverified decode of the ID token's JWT payload. It is -// used only for a friendly "Logged in as ..." message; the access token -// (verified server-side on every API call) is the actual credential. +// lfidClaimsNamespace prefixes the custom LFID claims Auth0 adds to the ID +// token, namely username. Distinct from the shorter "http://lfx.dev/claims" +// LFX claims namespace used elsewhere. +const lfidClaimsNamespace = "https://sso.linuxfoundation.org/claims/" + +// identityFromIDToken extracts a human-readable identity from an unverified +// decode of the ID token's JWT payload, for a friendly "Logged in as ..." +// message; the access token (verified server-side on every API call) is the +// actual credential. LFX usernames (the custom claim above) are the +// conventional identifier; email is included alongside it when present, and +// email or subject alone are used as fallbacks if the custom claim is +// unexpectedly missing. func identityFromIDToken(idToken string) string { parts := strings.Split(idToken, ".") if len(parts) != 3 { @@ -471,18 +479,25 @@ func identityFromIDToken(idToken string) string { return "" } - var claims struct { - Email string `json:"email"` - Sub string `json:"sub"` - } + var claims map[string]any if err := json.Unmarshal(payload, &claims); err != nil { return "" } - if claims.Email != "" { - return claims.Email + username, _ := claims[lfidClaimsNamespace+"username"].(string) + email, _ := claims["email"].(string) + sub, _ := claims["sub"].(string) + + switch { + case username != "" && email != "": + return fmt.Sprintf("%s (%s)", username, email) + case username != "": + return username + case email != "": + return fmt.Sprintf("%s (no username)", email) + default: + return sub } - return claims.Sub } // base64URLDecode decodes a base64url-encoded JWT segment, tolerating the From b632b5eeed1d12305f1b87a17a894d206015248d Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Wed, 19 Aug 2026 16:57:04 -0700 Subject: [PATCH 6/9] fix(auth): remove code duplication flagged by jscpd Extract persistLogin and loadStoredCredentials helpers to eliminate the two Go clones MegaLinter's jscpd check flagged in internal/commands/auth.go: the duplicated SaveCredentials + SaveDeviceState pairs in the --with-token and device-code login paths, and the duplicated LoadCredentials/ErrNotFound loading pattern shared by the token and status commands. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- internal/commands/auth.go | 91 +++++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 38 deletions(-) diff --git a/internal/commands/auth.go b/internal/commands/auth.go index e64a832..701e04f 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -140,16 +140,12 @@ func loginWithToken(store credstore.Store, env auth0.Environment, domain, audien return errors.New("no refresh token provided on stdin") } - if err := store.SaveCredentials(credstore.Credentials{RefreshToken: refreshToken}); err != nil { - return fmt.Errorf("save credentials: %w", err) - } - 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) + if err := persistLogin( + store, + credstore.Credentials{RefreshToken: refreshToken}, + credstore.DeviceState{IDPDomain: domain, Environment: string(env), Audience: audience, Insecure: insecure}, + ); err != nil { + return err } fmt.Println("Logged in with a supplied refresh token.") @@ -207,20 +203,16 @@ func loginWithDeviceCode( return err } - if err := store.SaveCredentials(credstore.Credentials{ - RefreshToken: token.RefreshToken, - AccessToken: token.AccessToken, - AccessTokenExpiry: token.Expiry, - }); err != nil { - return fmt.Errorf("save credentials: %w", err) - } - 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) + if err := persistLogin( + store, + credstore.Credentials{ + RefreshToken: token.RefreshToken, + AccessToken: token.AccessToken, + AccessTokenExpiry: token.Expiry, + }, + credstore.DeviceState{IDPDomain: domain, Environment: string(env), Audience: audience, Insecure: insecure}, + ); err != nil { + return err } fmt.Println("Login successful.") @@ -295,23 +287,51 @@ func insecureStorageUsageHint(insecure bool) string { return "no --insecure-storage" } +// persistLogin saves creds and state together, wrapping any failure with +// context on which write failed. Note: the two writes are not atomic; if +// SaveDeviceState fails after SaveCredentials succeeds, the caller is left +// with new credentials paired with stale or missing environment metadata. +func persistLogin(store credstore.Store, creds credstore.Credentials, state credstore.DeviceState) error { + 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) + } + return nil +} + +// loadStoredCredentials builds a credstore.Store for cmd and loads its +// credentials, returning (creds, false, nil) when none are stored +// (credstore.ErrNotFound) instead of treating that as an error, since +// callers report "not logged in" differently. +func loadStoredCredentials(cmd *cli.Command) (store credstore.Store, creds credstore.Credentials, found bool, err error) { + store, err = credStoreFromCommand(cmd) + if err != nil { + return nil, credstore.Credentials{}, false, err + } + creds, err = store.LoadCredentials() + if errors.Is(err, credstore.ErrNotFound) { + return store, credstore.Credentials{}, false, nil + } + if err != nil { + return nil, credstore.Credentials{}, false, err + } + return store, creds, true, nil +} + func newAuthTokenCommand() *cli.Command { return &cli.Command{ Name: "token", Usage: "Print a valid access token for the LFX platform", Action: func(ctx context.Context, cmd *cli.Command) error { - store, err := credStoreFromCommand(cmd) + store, creds, found, err := loadStoredCredentials(cmd) if err != nil { return err } - - creds, err := store.LoadCredentials() - if errors.Is(err, credstore.ErrNotFound) { + if !found { return errors.New("not logged in; run `lfx auth login` first") } - if err != nil { - return err - } if creds.ValidAccessToken() { fmt.Println(creds.AccessToken) @@ -361,19 +381,14 @@ func newAuthStatusCommand() *cli.Command { Name: "status", Usage: "Show the current authentication status", Action: func(_ context.Context, cmd *cli.Command) error { - store, err := credStoreFromCommand(cmd) + store, creds, found, err := loadStoredCredentials(cmd) if err != nil { return err } - - creds, err := store.LoadCredentials() - if errors.Is(err, credstore.ErrNotFound) { + if !found { fmt.Println("Not logged in.") return nil } - if err != nil { - return err - } state, err := store.LoadDeviceState() if err != nil && !errors.Is(err, credstore.ErrNotFound) { From 5488a60f822bcf2e4de8a6ff15a64312bbd5e3b8 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Wed, 19 Aug 2026 16:57:12 -0700 Subject: [PATCH 7/9] docs(agents): add Go toolchain version policy, downgrade go.mod Document a Go toolchain upgrade policy in the Contributing Guidelines: freely bump go.mod's go directive to the latest patch release, but only bump the minor version when the user explicitly asks for it and it has been validated against the Go version MegaLinter itself bundles -- MegaLinter runs several linters (e.g. golangci-lint) against its own bundled Go version, and a go.mod directive newer than that bundled version breaks those checks. Includes the concrete steps to look up MegaLinter's bundled Go version from its pinned flavor tag, and a one-liner using the go.dev/dl JSON feed to find the latest patch release for the minor version currently pinned in go.mod. Downgrade go.mod's go directive from 1.26.5 to 1.25.14 (the latest 1.25.x patch release). Verified against .github/workflows/mega-linter.yml, which pins oxsecurity/megalinter's go flavor to v9.6.0; that flavor's Dockerfile bundles Go 1.26.3 (GO_ALPINE_VERSION=1.26.3-r0), so the previous 1.26.5 directive was already newer than MegaLinter's own Go toolchain -- exactly the failure mode this policy exists to prevent. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- AGENTS.md | 47 ++++++++++++++++++++++++++++++++++++++++++++--- go.mod | 2 +- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ee97003..c82ec77 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -229,6 +229,47 @@ release binaries may be missing even though the GitHub Release exists. 2. **Package Comments**: Every new `*.go` file must include the same `// Package ...` doc comment as the rest of its package 3. **Dependencies**: Run `go get -u ./... && go mod tidy` before every PR to - keep dependencies current -4. **Code Quality**: Run `make check` before commits -5. **Documentation**: Update README.md for user-facing changes + keep dependencies current. This upgrades module dependencies only, not the + Go toolchain itself (`go.mod`'s `go` directive) -- see the toolchain policy + below before touching that. +4. **Go toolchain version**: Freely bump `go.mod`'s `go` directive to the + latest available *patch* release (e.g. `1.X.Y` → `1.X.{Y+1}`) to pick up + security fixes. Do **not** bump the *minor* version (e.g. `1.X.x` → + `1.{X+1}.x`) unless the user explicitly asks for it, **and** you've + validated it against the Go version MegaLinter itself bundles -- + MegaLinter runs several linters (e.g. `golangci-lint`) against its own + bundled Go version, and a `go.mod` directive newer than that bundled + version breaks those checks. + + To find MegaLinter's bundled Go version: + + ```bash + # 1. Find the MegaLinter flavor and pinned version tag used in CI. + grep -A1 'oxsecurity/megalinter' .github/workflows/*.yml + # e.g. "uses: oxsecurity/megalinter/flavors/@ # " + + # 2. Fetch that flavor's Dockerfile and read its GO_ALPINE_VERSION (or + # GO_IMAGE_VERSION) build arg. + curl -s "https://raw.githubusercontent.com/oxsecurity/megalinter//flavors//Dockerfile" \ + | grep -i 'GO_ALPINE_VERSION\|GO_IMAGE_VERSION' + ``` + + `go.mod`'s `go` directive must never exceed that bundled version. Staying + one minor version behind it (rather than matching its minor *and* patch + exactly) leaves room to always take the latest patch release for security + fixes without ever being blocked by MegaLinter's own bundled patch version + lagging behind a newly disclosed vulnerability. + + There's no built-in `go` subcommand to look up the latest patch release + for a given minor version -- query the official `go.dev/dl` JSON feed + instead: + + ```bash + # Find the latest patch release for the minor version pinned in go.mod. + MINOR=$(grep '^go ' go.mod | awk '{print $2}' | cut -d. -f1,2) + curl -s "https://go.dev/dl/?mode=json&include=all" \ + | jq -r --arg m "go${MINOR}." '.[].version | select(startswith($m))' \ + | sort -V | tail -1 + ``` +5. **Code Quality**: Run `make check` before commits +6. **Documentation**: Update README.md for user-facing changes diff --git a/go.mod b/go.mod index c967d6c..e65b3ed 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ // SPDX-License-Identifier: MIT module github.com/linuxfoundation/lfx-cli -go 1.26.5 +go 1.25.14 require ( github.com/99designs/keyring v1.2.2 From 98b10ccf6221428b72e01982c34da25190bbed37 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Wed, 19 Aug 2026 17:09:48 -0700 Subject: [PATCH 8/9] fix(auth): roll back saved credentials if device-state save fails persistLogin saves credentials and device state in two separate, non-atomic writes. Per Copilot review feedback on PR #4, if SaveDeviceState fails after SaveCredentials has already succeeded, the new refresh token was left paired with stale or missing environment metadata, so a later refresh could target the wrong Auth0 tenant. Delete the newly saved credentials when SaveDeviceState fails, so a partial failure leaves the previous login state undisturbed rather than a mismatched credentials/metadata pair. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- internal/commands/auth.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 701e04f..3cde10f 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -287,15 +287,19 @@ func insecureStorageUsageHint(insecure bool) string { return "no --insecure-storage" } -// persistLogin saves creds and state together, wrapping any failure with -// context on which write failed. Note: the two writes are not atomic; if -// SaveDeviceState fails after SaveCredentials succeeds, the caller is left -// with new credentials paired with stale or missing environment metadata. +// persistLogin saves creds and state together. The two writes are not +// atomic, so if SaveDeviceState fails after SaveCredentials succeeds, the +// newly saved credentials are rolled back (deleted) rather than left paired +// with stale or missing environment metadata, which could otherwise send a +// later refresh to the wrong Auth0 tenant. func persistLogin(store credstore.Store, creds credstore.Credentials, state credstore.DeviceState) error { if err := store.SaveCredentials(creds); err != nil { return fmt.Errorf("save credentials: %w", err) } if err := store.SaveDeviceState(state); err != nil { + if delErr := store.DeleteCredentials(); delErr != nil { + return fmt.Errorf("save device state: %w (and rollback of saved credentials also failed: %v)", err, delErr) + } return fmt.Errorf("save device state: %w", err) } return nil From 7ad645bdf318b321d1be336ba51d056b88fbbcbb Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Thu, 20 Aug 2026 13:07:04 -0700 Subject: [PATCH 9/9] feat(auth): add `lfx auth backends` to list credential-store backends Lists the system credential-store backends compiled into this binary for the current OS (macOS Keychain, Linux Secret Service/KWallet, Windows Credential Manager, or pass), in the priority order `lfx auth login` tries them via keyring.Open. This only reports what's compiled in per-OS build tags (credstore.AvailableBackends wraps keyring.AvailableBackends), not whether a given backend is actually usable at runtime. Also renames user-facing "system keychain" wording to "system backend" (--insecure-storage flag usage text, `auth status` output, and backendDescription's error-message text) since it's a more accurate, OS-agnostic term now that `auth backends` exists to name individual backends like the macOS Keychain specifically. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- internal/commands/auth.go | 35 ++++++++++++++++++++++--- internal/credstore/credstore.go | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 3cde10f..6f2e225 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -45,7 +45,7 @@ var loginScopes = []string{"openid", "profile", "email", "offline_access"} // The --insecure-storage flag is shared by all subcommands (it is not // declared as a "Local" flag, so urfave/cli resolves it for subcommand // actions via cmd.Bool) and controls whether credentials bypass the system -// keychain in favor of credstore's plain (unencrypted) file fallback, e.g. +// backend in favor of credstore's plain (unencrypted) file fallback, e.g. // for headless/CI use. func NewAuthCommand() *cli.Command { return &cli.Command{ @@ -54,7 +54,7 @@ func NewAuthCommand() *cli.Command { Flags: []cli.Flag{ &cli.BoolFlag{ Name: insecureStorageFlagName, - Usage: "Store credentials in a plain (unencrypted) file instead of the system keychain", + Usage: "Store credentials in a plain (unencrypted) file instead of the system backend", }, }, Commands: []*cli.Command{ @@ -62,6 +62,7 @@ func NewAuthCommand() *cli.Command { newAuthTokenCommand(), newAuthStatusCommand(), newAuthLogoutCommand(), + newAuthBackendsCommand(), }, } } @@ -275,7 +276,7 @@ func backendDescription(insecure bool) string { if insecure { return "the plain-file (--insecure-storage) backend" } - return "the system keychain" + return "the system backend" } // insecureStorageUsageHint renders the flag (or its absence) needed to @@ -399,7 +400,7 @@ func newAuthStatusCommand() *cli.Command { return fmt.Errorf("load device state: %w", err) } - backend := "system keychain" + backend := "system backend" if cmd.Bool(insecureStorageFlagName) { backend = "plain file (--insecure-storage)" } @@ -475,6 +476,32 @@ func newAuthLogoutCommand() *cli.Command { } } +// newAuthBackendsCommand builds `lfx auth backends`, which lists the system +// credential-store backends compiled into this binary for the current OS +// (see credstore.AvailableBackends), in the priority order `lfx auth login` +// would try them. It does not attempt to open any backend, so a listed +// backend may still turn out to be unusable at runtime (e.g. no D-Bus +// session for Secret Service, `pass` not initialized). +func newAuthBackendsCommand() *cli.Command { + return &cli.Command{ + Name: "backends", + Usage: "List the system credential-store backends available on this OS", + Action: func(_ context.Context, _ *cli.Command) error { + backends := credstore.AvailableBackends() + if len(backends) == 0 { + fmt.Println("No system credential-store backends are available on this OS; `lfx auth login` requires --insecure-storage.") + return nil + } + + fmt.Println("Available credential-store backends, in the order `lfx auth login` tries them:") + for _, b := range backends { + fmt.Printf(" %-15s %s\n", b.Name, b.DisplayName) + } + return nil + }, + } +} + // lfidClaimsNamespace prefixes the custom LFID claims Auth0 adds to the ID // token, namely username. Distinct from the shorter "http://lfx.dev/claims" // LFX claims namespace used elsewhere. diff --git a/internal/credstore/credstore.go b/internal/credstore/credstore.go index 9b9136d..ad8cf4d 100644 --- a/internal/credstore/credstore.go +++ b/internal/credstore/credstore.go @@ -77,6 +77,52 @@ var systemBackends = []keyring.BackendType{ keyring.PassBackend, } +// backendDisplayNames maps keyring.BackendType values to the human-readable +// names shown by `lfx auth backends`. +var backendDisplayNames = map[keyring.BackendType]string{ + keyring.SecretServiceBackend: "Secret Service (GNOME Keyring, KeePassXC, etc.)", + keyring.KeychainBackend: "macOS Keychain", + keyring.KWalletBackend: "KDE Wallet (kwallet)", + keyring.WinCredBackend: "Windows Credential Manager", + keyring.PassBackend: "gpg-encrypted vault (passwordstore.org)", +} + +// Backend describes one of the system credential-store backends compiled +// into this binary for the current OS, as reported by `lfx auth backends`. +type Backend struct { + // Name is the keyring.BackendType identifier (e.g. "keychain"). + Name string + // DisplayName is a human-readable label for Name. + DisplayName string +} + +// AvailableBackends reports the system credential-store backends compiled +// into this binary for the current OS (Go build tags determine which +// backends are even possible per-platform; see the per-backend source +// files in github.com/99designs/keyring), in the same priority order (see +// systemBackends) that New passes to keyring.Open as AllowedBackends. It +// does not attempt to open any backend, so a backend listed here may still +// fail at login time if it isn't actually usable at runtime (e.g. no D-Bus +// session for Secret Service, `pass` not initialized, etc.). +func AvailableBackends() []Backend { + available := make(map[keyring.BackendType]bool) + for _, b := range keyring.AvailableBackends() { + available[b] = true + } + + var backends []Backend + for _, b := range systemBackends { + if !available[b] { + continue + } + backends = append(backends, Backend{ + Name: string(b), + DisplayName: backendDisplayNames[b], + }) + } + return backends +} + // Credentials holds the secrets needed to authenticate with the LFX // platform: the long-lived Auth0 refresh token, and an optional cached // access token with its expiry.