From cbe57825cd4fc97c8c06dd083720f61d752fdca2 Mon Sep 17 00:00:00 2001 From: David Deal Date: Thu, 1 Oct 2026 22:16:39 +0000 Subject: [PATCH 1/3] fix(api): derive default API base URL from the login environment Without --hostname, `lfx api` now always uses the compiled-in default API audience for the login's environment as its base URL, rather than the audience value persisted in state.json. A stored audience that differs from that default is refused with an error asking the user to run `lfx auth login` again (or, for development logins, to pass --hostname explicitly), mirroring the existing IdP domain consistency check in loadDeviceStateForBackend. Logins made with a non-default --audience therefore need --hostname (development only) to use `lfx api`; `lfx auth token` is unaffected. Signed-off-by: David Deal Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01L8qMuq3je1uuU2JMLHuYBG --- README.md | 6 ++ internal/commands/api.go | 50 ++++++++++++--- internal/commands/api_test.go | 107 ++++++++++++++++++++++++++++++++ internal/commands/auth.go | 8 ++- internal/credstore/credstore.go | 7 ++- 5 files changed, 163 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 108abe5..a383e99 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,12 @@ lfx api /projects --field name=example # auto-promotes to POST lfx api -X PUT /projects/123 --input - -H "If-Match: " < input.json # Content-Type: application/json is added automatically ``` +`lfx api` always sends requests to the default API for the login's +environment (see `lfx auth environments`). If the stored login was made +with a non-default `--audience`, `lfx api` refuses to use it as the base +URL; for development logins, pass `--hostname` explicitly on each call to +target a different API. + Credentials (refresh token, cached access token) are stored in your operating system's credential store by default (macOS Keychain, Windows Credential Manager, Linux Secret Service/KWallet/`pass`). Which of these is diff --git a/internal/commands/api.go b/internal/commands/api.go index 84d2ece..b5ef6e5 100644 --- a/internal/commands/api.go +++ b/internal/commands/api.go @@ -119,7 +119,7 @@ func runAPI(ctx context.Context, cmd *cli.Command) error { baseURL := cmd.String(apiHostnameFlagName) if cmd.IsSet(apiHostnameFlagName) { if baseURL == "" { - return errors.New("--hostname was set to an empty value; omit the flag to use the login audience instead") + return errors.New("--hostname was set to an empty value; omit the flag to use the login environment's default API instead") } // --hostname sends the live bearer token to whatever host is // named. The purpose of this restriction is to keep a prod or @@ -138,10 +138,10 @@ func runAPI(ctx context.Context, cmd *cli.Command) error { } } if baseURL == "" { - baseURL = audience - } - if baseURL == "" { - return errors.New("no API base URL available; log in with `lfx auth login` or pass --hostname") + baseURL, err = apiDefaultBaseURL(audience, env) + if err != nil { + return err + } } if err := apiRequireHTTPS(baseURL); err != nil { return err @@ -347,14 +347,44 @@ func coerceFieldValue(value string) any { return value } +// apiDefaultBaseURL returns the API base URL to use when --hostname isn't +// passed: the compiled-in default audience for env (see +// defaultAudienceForEnvironment), never the persisted login audience +// as-is. Mirroring loadDeviceStateForBackend's IdP domain check, a stored +// audience that differs from env's default is refused, so the host that +// receives the bearer token is only ever chosen by the compiled-in +// environment table or by an explicit, per-invocation --hostname (subject +// to its development-only gate). This applies uniformly to every +// environment, including development. A deliberate non-default `lfx auth +// login --audience` must therefore be re-confirmed on each call via +// --hostname (development logins only). +func apiDefaultBaseURL(audience string, env authEnvironment) (string, error) { + expected, err := defaultAudienceForEnvironment(env) + if err != nil { + return "", err + } + if audience != expected { + hint := "" + if env == envDevelopment { + hint = fmt.Sprintf(", or pass --%s explicitly to target a non-default API", apiHostnameFlagName) + } + return "", fmt.Errorf( + "stored login audience %q does not match the default audience %q for environment %q; refusing to use it as the API base URL. Run `lfx auth login --%s=%s` again%s", + audience, expected, env, envFlagName, env, hint, + ) + } + return expected, nil +} + // apiJoinURL joins base and path into a single URL, ensuring exactly one // slash separates them. path is passed through unmodified (aside from a // leading-slash trim) rather than parsed/re-escaped: base is already -// trusted (the login audience, or --hostname, which is gated to -// development-environment logins in runAPI) and path is meant to be sent -// verbatim, including its query string, exactly as the caller wrote it -- -// a URL-parsing round trip risks subtly rewriting it (see the query -// string and escaped-slash regressions caught in review). +// trusted (env's compiled-in default audience via apiDefaultBaseURL, or +// --hostname, which is gated to development-environment logins in runAPI) +// and path is meant to be sent verbatim, including its query string, +// exactly as the caller wrote it -- a URL-parsing round trip risks subtly +// rewriting it (see the query string and escaped-slash regressions caught +// in review). func apiJoinURL(base, path string) (string, error) { if base == "" { return "", errors.New("empty base URL") diff --git a/internal/commands/api_test.go b/internal/commands/api_test.go index 6e514d8..537871c 100644 --- a/internal/commands/api_test.go +++ b/internal/commands/api_test.go @@ -6,10 +6,16 @@ package commands import ( "context" "encoding/json" + "net/http" + "net/http/httptest" "os" "path/filepath" + "strings" + "sync/atomic" "testing" + "time" + "github.com/linuxfoundation/lfx-cli/internal/credstore" "github.com/urfave/cli/v3" ) @@ -303,3 +309,104 @@ func TestAPIRequestBodyNoneWithPipedStdin(t *testing.T) { } }) } + +func TestAPIDefaultBaseURL(t *testing.T) { + tests := []struct { + name string + audience string + env authEnvironment + want string + wantErr bool + }{ + {name: "production default", audience: defaultAudiences[envProduction], env: envProduction, want: defaultAudiences[envProduction]}, + {name: "staging default", audience: defaultAudiences[envStaging], env: envStaging, want: defaultAudiences[envStaging]}, + {name: "development default", audience: defaultAudiences[envDevelopment], env: envDevelopment, want: defaultAudiences[envDevelopment]}, + {name: "production tampered audience rejected", audience: "https://attacker.example/", env: envProduction, wantErr: true}, + {name: "staging tampered audience rejected", audience: "https://attacker.example/", env: envStaging, wantErr: true}, + {name: "development tampered audience rejected", audience: "https://attacker.example/", env: envDevelopment, wantErr: true}, + {name: "loopback audience rejected", audience: "http://127.0.0.1:8080/", env: envProduction, wantErr: true}, + {name: "other environment's default rejected", audience: defaultAudiences[envDevelopment], env: envProduction, wantErr: true}, + {name: "empty audience rejected", audience: "", env: envProduction, wantErr: true}, + {name: "unknown environment rejected", audience: "https://attacker.example/", env: authEnvironment("invalid"), wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := apiDefaultBaseURL(tc.audience, tc.env) + if tc.wantErr { + if err == nil { + t.Fatalf("apiDefaultBaseURL(%q, %q) = %q, want error", tc.audience, tc.env, got) + } + return + } + if err != nil { + t.Fatalf("apiDefaultBaseURL(%q, %q): %v", tc.audience, tc.env, err) + } + if got != tc.want { + t.Errorf("apiDefaultBaseURL(%q, %q) = %q, want %q", tc.audience, tc.env, got, tc.want) + } + }) + } +} + +// TestRunAPIRefusesTamperedAudience checks that `lfx api` refuses a stored +// login whose audience differs from its environment's default, rather than +// sending the cached access token to that host. +func TestRunAPIRefusesTamperedAudience(t *testing.T) { + tests := []struct { + name string + env authEnvironment + }{ + {name: "production", env: envProduction}, + {name: "staging", env: envStaging}, + {name: "development", env: envDevelopment}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var hits atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + hits.Add(1) + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(server.Close) + + stateDir := t.TempDir() + t.Setenv("XDG_STATE_HOME", stateDir) + store, err := credstore.New(credstore.Options{Insecure: true}) + if err != nil { + t.Fatalf("credstore.New: %v", err) + } + if err := persistLogin( + store, + credstore.Credentials{ + RefreshToken: "refresh", + AccessToken: "secret-access-token", + AccessTokenExpiry: time.Now().Add(time.Hour), + }, + credstore.DeviceState{ + IDPDomain: authDomains[tc.env], + Environment: string(tc.env), + Audience: server.URL + "/", + Insecure: true, + }, + ); err != nil { + t.Fatalf("persistLogin: %v", err) + } + + app := &cli.Command{ + Name: "lfx", + Flags: CredentialStoreFlags, + Commands: []*cli.Command{NewAPICommand()}, + } + err = app.Run(context.Background(), []string{"lfx", "--insecure-storage", "api", "--field", "k=v", "/x"}) + if err == nil { + t.Fatal("lfx api: got nil error, want refusal of tampered audience") + } + if !strings.Contains(err.Error(), "does not match the default audience") { + t.Errorf("lfx api error = %q, want audience mismatch error", err) + } + if n := hits.Load(); n != 0 { + t.Fatalf("tampered audience host received %d request(s); want 0", n) + } + }) + } +} diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 7a897e2..91d5c1a 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -463,9 +463,11 @@ func newAuthTokenCommand() *cli.Command { // resolveAccessToken returns a valid access token for the current login, // refreshing it (and persisting the refreshed credentials) if the cached // one is missing or expired. It also returns the audience and login -// environment recorded at login time, so callers (e.g. `lfx api`) can use -// the audience as their default API base URL and the environment to gate -// features like --hostname to non-production logins. Both `lfx auth +// environment recorded at login time, so callers (e.g. `lfx api`) can check +// the audience against the environment's compiled-in default (see +// apiDefaultBaseURL; the persisted audience is never trusted as a base URL +// on its own) and use the environment to gate features like --hostname to +// non-production logins. Both `lfx auth // token` and `lfx api` share this single code path so their refresh, // error, and credential-persistence behavior never drifts apart. func resolveAccessToken(ctx context.Context, cmd *cli.Command) (token, audience string, env authEnvironment, err error) { diff --git a/internal/credstore/credstore.go b/internal/credstore/credstore.go index 69e317b..d7dc56f 100644 --- a/internal/credstore/credstore.go +++ b/internal/credstore/credstore.go @@ -196,8 +196,11 @@ type DeviceState struct { // 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`. + // back on refresh; it's persisted for display in `lfx auth status` + // and so `lfx api` can refuse a login whose audience differs from + // its environment's compiled-in default. Like the rest of state.json + // it is unauthenticated, so it must never on its own choose which + // host receives a bearer token. 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 From 7f5f815932fbc571416e89883c692f2d6eb25a27 Mon Sep 17 00:00:00 2001 From: David Deal Date: Thu, 1 Oct 2026 22:26:35 +0000 Subject: [PATCH 2/3] docs: qualify lfx api default base URL wording Describe the environment's default API as the behavior when --hostname is not passed, rather than implying it applies unconditionally. Signed-off-by: David Deal Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01L8qMuq3je1uuU2JMLHuYBG --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index a383e99..f33bce9 100644 --- a/README.md +++ b/README.md @@ -60,11 +60,11 @@ lfx api /projects --field name=example # auto-promotes to POST lfx api -X PUT /projects/123 --input - -H "If-Match: " < input.json # Content-Type: application/json is added automatically ``` -`lfx api` always sends requests to the default API for the login's -environment (see `lfx auth environments`). If the stored login was made -with a non-default `--audience`, `lfx api` refuses to use it as the base -URL; for development logins, pass `--hostname` explicitly on each call to -target a different API. +By default (when `--hostname` is not passed), `lfx api` sends requests to +the default API for the login's environment (see `lfx auth environments`). +If the stored login was made with a non-default `--audience`, `lfx api` +refuses to use it as the base URL; for development logins, pass +`--hostname` explicitly on each call to target a different API. Credentials (refresh token, cached access token) are stored in your operating system's credential store by default (macOS Keychain, Windows From 0b6754692df6bc064ca7f0f6d4ddbb91b58577ce Mon Sep 17 00:00:00 2001 From: David Deal Date: Mon, 19 Oct 2026 12:50:05 -0700 Subject: [PATCH 3/3] fix(api): remove --audience and --hostname, derive audience from env only Per review feedback on this PR: the prior approach (validating a persisted `--audience` against the environment's compiled-in default) left `--audience` effectively dead outside `development`, since `lfx api` could never use a non-default stored audience and `--hostname` was independently gated to development logins. Instead of validating, remove the ability to configure either value at all: - `lfx auth login` no longer accepts `--audience`; the audience sent to Auth0's device code endpoint is always the compiled-in default for `--env` (defaultAudienceForEnvironment). - `lfx api` no longer accepts `--hostname`; its base URL is always the compiled-in default audience for the login's environment. - `credstore.DeviceState` no longer persists `Audience`; nothing in state.json can influence which host receives a bearer token anymore. - `lfx auth status`'s "Audience:" line is now computed from the stored Environment via defaultAudienceForEnvironment, not read back from state.json. - Removed `apiDefaultBaseURL` and its now-obsolete tests (the audience-mismatch validation and tampered-audience end-to-end test), since there's nothing left to validate against. Suggested release: minor (removes two user-facing flags). Signed-off-by: David Deal --- README.md | 8 +- internal/commands/api.go | 83 +++------------------ internal/commands/api_test.go | 107 --------------------------- internal/commands/auth.go | 77 ++++++++----------- internal/commands/environment.go | 16 +--- internal/credstore/credstore.go | 15 +--- internal/credstore/credstore_test.go | 1 - 7 files changed, 53 insertions(+), 254 deletions(-) diff --git a/README.md b/README.md index 903ec2a..a646b2f 100644 --- a/README.md +++ b/README.md @@ -67,11 +67,9 @@ Automatic browser opening accepts only absolute HTTPS URLs with a hostname. Other verification URI values are displayed for manual inspection without launching them; control characters in login instructions are escaped. -By default (when `--hostname` is not passed), `lfx api` sends requests to -the default API for the login's environment (see `lfx auth environments`). -If the stored login was made with a non-default `--audience`, `lfx api` -refuses to use it as the base URL; for development logins, pass -`--hostname` explicitly on each call to target a different API. +`lfx api` always sends requests to the compiled-in default API for the +login's environment (see `lfx auth environments`); the base URL is not +configurable per call. Credentials (refresh token, cached access token) are stored in your operating system's credential store by default (macOS Keychain, Windows diff --git a/internal/commands/api.go b/internal/commands/api.go index 49ab521..11e382d 100644 --- a/internal/commands/api.go +++ b/internal/commands/api.go @@ -29,7 +29,6 @@ const ( apiFieldFlagName = "field" apiRawFieldFlagName = "raw-field" apiQueryFlagName = "query" - apiHostnameFlagName = "hostname" ) // apiAllowedMethods enumerates the HTTP methods `lfx api` accepts via @@ -89,10 +88,6 @@ func NewAPICommand() *cli.Command { Aliases: []string{"q"}, Usage: "Filter the response body through a gjson expression before output", }, - &cli.StringFlag{ - Name: apiHostnameFlagName, - Usage: "Override the LFX API base URL (advanced; independent of the IdP domain). Requires a development-environment login ('lfx auth login --env=development').", - }, }, Action: runAPI, } @@ -120,37 +115,14 @@ func runAPI(ctx context.Context, cmd *cli.Command) error { return fmt.Errorf("invalid --%s %q (must be one of GET, POST, PUT, DELETE)", apiMethodFlagName, method) } - token, audience, env, err := resolveAccessToken(ctx, cmd) + token, env, err := resolveAccessToken(ctx, cmd) if err != nil { return err } - baseURL := cmd.String(apiHostnameFlagName) - if cmd.IsSet(apiHostnameFlagName) { - if baseURL == "" { - return errors.New("--hostname was set to an empty value; omit the flag to use the login environment's default API instead") - } - // --hostname sends the live bearer token to whatever host is - // named. The purpose of this restriction is to keep a prod or - // staging token -- the ones with real authority -- from being - // redirected to a third-party host at all; --hostname's actual - // use case (pointing at a local mock server or alternate - // deployment for advanced troubleshooting) only comes up in - // development anyway. Allowing it there is also independently - // safe: OAuth2 resource servers validate a token's issuer - // (`iss`), not just its audience, against the specific IdP(s) - // they trust, so even in the hypothetical where a development - // login held a token claiming the prod audience, prod would - // still reject it as issued by an untrusted IdP. - if env != envDevelopment { - return fmt.Errorf("--%s requires a development-environment login (run `lfx auth login --%s=%s`); the current login's environment is %q", apiHostnameFlagName, envFlagName, envDevelopment, env) - } - } - if baseURL == "" { - baseURL, err = apiDefaultBaseURL(audience, env) - if err != nil { - return err - } + baseURL, err := defaultAudienceForEnvironment(env) + if err != nil { + return err } if err := apiRequireHTTPS(baseURL); err != nil { return err @@ -356,44 +328,14 @@ func coerceFieldValue(value string) any { return value } -// apiDefaultBaseURL returns the API base URL to use when --hostname isn't -// passed: the compiled-in default audience for env (see -// defaultAudienceForEnvironment), never the persisted login audience -// as-is. Mirroring loadDeviceStateForBackend's IdP domain check, a stored -// audience that differs from env's default is refused, so the host that -// receives the bearer token is only ever chosen by the compiled-in -// environment table or by an explicit, per-invocation --hostname (subject -// to its development-only gate). This applies uniformly to every -// environment, including development. A deliberate non-default `lfx auth -// login --audience` must therefore be re-confirmed on each call via -// --hostname (development logins only). -func apiDefaultBaseURL(audience string, env authEnvironment) (string, error) { - expected, err := defaultAudienceForEnvironment(env) - if err != nil { - return "", err - } - if audience != expected { - hint := "" - if env == envDevelopment { - hint = fmt.Sprintf(", or pass --%s explicitly to target a non-default API", apiHostnameFlagName) - } - return "", fmt.Errorf( - "stored login audience %q does not match the default audience %q for environment %q; refusing to use it as the API base URL. Run `lfx auth login --%s=%s` again%s", - audience, expected, env, envFlagName, env, hint, - ) - } - return expected, nil -} - // apiJoinURL joins base and path into a single URL, ensuring exactly one // slash separates them. path is passed through unmodified (aside from a // leading-slash trim) rather than parsed/re-escaped: base is already -// trusted (env's compiled-in default audience via apiDefaultBaseURL, or -// --hostname, which is gated to development-environment logins in runAPI) -// and path is meant to be sent verbatim, including its query string, -// exactly as the caller wrote it -- a URL-parsing round trip risks subtly -// rewriting it (see the query string and escaped-slash regressions caught -// in review). +// trusted (env's compiled-in default audience; see +// defaultAudienceForEnvironment), and path is meant to be sent verbatim, +// including its query string, exactly as the caller wrote it -- a +// URL-parsing round trip risks subtly rewriting it (see the query string +// and escaped-slash regressions caught in review). func apiJoinURL(base, path string) (string, error) { if base == "" { return "", errors.New("empty base URL") @@ -403,9 +345,8 @@ func apiJoinURL(base, path string) (string, error) { // apiRequireHTTPS rejects base URLs that would send the bearer token over // a non-HTTPS connection, since the request's Authorization header is -// otherwise transmitted in cleartext. Loopback hosts are exempted, since -// http:// there is a common and low-risk way to point --hostname at a -// local mock server for debugging. +// otherwise transmitted in cleartext. Loopback hosts are exempted as a +// low-risk allowance for local mock servers during development. func apiRequireHTTPS(rawURL string) error { parsed, err := url.Parse(rawURL) if err != nil { @@ -418,5 +359,5 @@ func apiRequireHTTPS(rawURL string) error { case "localhost", "127.0.0.1", "::1": return nil } - return fmt.Errorf("refusing to send credentials to non-HTTPS URL %q (use an https:// --hostname, or localhost/127.0.0.1 for local debugging)", rawURL) + return fmt.Errorf("refusing to send credentials to non-HTTPS URL %q (use an https:// URL, or localhost/127.0.0.1 for local debugging)", rawURL) } diff --git a/internal/commands/api_test.go b/internal/commands/api_test.go index 26ddfed..3184710 100644 --- a/internal/commands/api_test.go +++ b/internal/commands/api_test.go @@ -6,16 +6,10 @@ package commands import ( "context" "encoding/json" - "net/http" - "net/http/httptest" "os" "path/filepath" - "strings" - "sync/atomic" "testing" - "time" - "github.com/linuxfoundation/lfx-cli/internal/credstore" "github.com/urfave/cli/v3" ) @@ -369,104 +363,3 @@ func TestAPIRequestBodyNoneWithPipedStdin(t *testing.T) { } }) } - -func TestAPIDefaultBaseURL(t *testing.T) { - tests := []struct { - name string - audience string - env authEnvironment - want string - wantErr bool - }{ - {name: "production default", audience: defaultAudiences[envProduction], env: envProduction, want: defaultAudiences[envProduction]}, - {name: "staging default", audience: defaultAudiences[envStaging], env: envStaging, want: defaultAudiences[envStaging]}, - {name: "development default", audience: defaultAudiences[envDevelopment], env: envDevelopment, want: defaultAudiences[envDevelopment]}, - {name: "production tampered audience rejected", audience: "https://attacker.example/", env: envProduction, wantErr: true}, - {name: "staging tampered audience rejected", audience: "https://attacker.example/", env: envStaging, wantErr: true}, - {name: "development tampered audience rejected", audience: "https://attacker.example/", env: envDevelopment, wantErr: true}, - {name: "loopback audience rejected", audience: "http://127.0.0.1:8080/", env: envProduction, wantErr: true}, - {name: "other environment's default rejected", audience: defaultAudiences[envDevelopment], env: envProduction, wantErr: true}, - {name: "empty audience rejected", audience: "", env: envProduction, wantErr: true}, - {name: "unknown environment rejected", audience: "https://attacker.example/", env: authEnvironment("invalid"), wantErr: true}, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - got, err := apiDefaultBaseURL(tc.audience, tc.env) - if tc.wantErr { - if err == nil { - t.Fatalf("apiDefaultBaseURL(%q, %q) = %q, want error", tc.audience, tc.env, got) - } - return - } - if err != nil { - t.Fatalf("apiDefaultBaseURL(%q, %q): %v", tc.audience, tc.env, err) - } - if got != tc.want { - t.Errorf("apiDefaultBaseURL(%q, %q) = %q, want %q", tc.audience, tc.env, got, tc.want) - } - }) - } -} - -// TestRunAPIRefusesTamperedAudience checks that `lfx api` refuses a stored -// login whose audience differs from its environment's default, rather than -// sending the cached access token to that host. -func TestRunAPIRefusesTamperedAudience(t *testing.T) { - tests := []struct { - name string - env authEnvironment - }{ - {name: "production", env: envProduction}, - {name: "staging", env: envStaging}, - {name: "development", env: envDevelopment}, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - var hits atomic.Int32 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - hits.Add(1) - w.WriteHeader(http.StatusOK) - })) - t.Cleanup(server.Close) - - stateDir := t.TempDir() - t.Setenv("XDG_STATE_HOME", stateDir) - store, err := credstore.New(credstore.Options{Insecure: true}) - if err != nil { - t.Fatalf("credstore.New: %v", err) - } - if err := persistLogin( - store, - credstore.Credentials{ - RefreshToken: "refresh", - AccessToken: "secret-access-token", - AccessTokenExpiry: time.Now().Add(time.Hour), - }, - credstore.DeviceState{ - IDPDomain: authDomains[tc.env], - Environment: string(tc.env), - Audience: server.URL + "/", - Insecure: true, - }, - ); err != nil { - t.Fatalf("persistLogin: %v", err) - } - - app := &cli.Command{ - Name: "lfx", - Flags: CredentialStoreFlags, - Commands: []*cli.Command{NewAPICommand()}, - } - err = app.Run(context.Background(), []string{"lfx", "--insecure-storage", "api", "--field", "k=v", "/x"}) - if err == nil { - t.Fatal("lfx api: got nil error, want refusal of tampered audience") - } - if !strings.Contains(err.Error(), "does not match the default audience") { - t.Errorf("lfx api error = %q, want audience mismatch error", err) - } - if n := hits.Load(); n != 0 { - t.Fatalf("tampered audience host received %d request(s); want 0", n) - } - }) - } -} diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 3cd0716..af36be8 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -43,7 +43,6 @@ const ( webFlagName = "web" withTokenFlagName = "with-token" envFlagName = "env" - audienceFlagName = "audience" ) // CredentialStoreFlags are the --insecure-storage and --backend flags @@ -123,10 +122,6 @@ func newAuthLoginCommand() *cli.Command { Usage: "Target environment; see 'lfx auth environments' for accepted values", Value: string(envProduction), }, - &cli.StringFlag{ - Name: audienceFlagName, - Usage: "Auth0 API audience to request tokens for; defaults to --env's audience (see 'lfx auth environments')", - }, }, Action: runAuthLogin, } @@ -143,21 +138,14 @@ func runAuthLogin(ctx context.Context, cmd *cli.Command) error { if err != nil { return err } - audience := cmd.String(audienceFlagName) - if !cmd.IsSet(audienceFlagName) { - audience, err = defaultAudienceForEnvironment(env) - if err != nil { - return err - } - } insecure := cmd.Bool(insecureStorageFlagName) backend := cmd.String(backendFlagName) if cmd.Bool(withTokenFlagName) { - return loginWithToken(store, env, domain, audience, insecure, backend) + return loginWithToken(store, env, domain, insecure, backend) } - return loginWithDeviceCode(ctx, cmd, domain, clientID, store, env, audience, insecure, backend) + return loginWithDeviceCode(ctx, cmd, domain, clientID, store, env, insecure, backend) } // loginWithToken implements `--with-token`: it reads a refresh token from @@ -165,7 +153,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 authEnvironment, domain, audience string, insecure bool, backend string) error { +func loginWithToken(store credstore.Store, env authEnvironment, domain string, insecure bool, backend string) error { reader := bufio.NewReader(os.Stdin) line, err := reader.ReadString('\n') if err != nil && !errors.Is(err, io.EOF) { @@ -179,7 +167,7 @@ func loginWithToken(store credstore.Store, env authEnvironment, domain, audience if err := persistLogin( store, credstore.Credentials{RefreshToken: refreshToken}, - credstore.DeviceState{IDPDomain: domain, Environment: string(env), Audience: audience, Insecure: insecure, Backend: backend}, + credstore.DeviceState{IDPDomain: domain, Environment: string(env), Insecure: insecure, Backend: backend}, ); err != nil { return err } @@ -198,10 +186,13 @@ func loginWithDeviceCode( domain, clientID string, store credstore.Store, env authEnvironment, - audience string, insecure bool, backend string, ) error { + audience, err := defaultAudienceForEnvironment(env) + if err != nil { + return err + } cfg := &oauth2.Config{ ClientID: clientID, Scopes: loginScopes, @@ -211,10 +202,7 @@ func loginWithDeviceCode( AuthStyle: oauth2.AuthStyleInParams, }, } - var opts []oauth2.AuthCodeOption - if audience != "" { - opts = append(opts, oauth2.SetAuthURLParam("audience", audience)) - } + opts := []oauth2.AuthCodeOption{oauth2.SetAuthURLParam("audience", audience)} resp, err := cfg.DeviceAuth(ctx, opts...) if err != nil { return fmt.Errorf("request device code: %w", err) @@ -268,7 +256,7 @@ func loginWithDeviceCode( AccessToken: token.AccessToken, AccessTokenExpiry: token.Expiry, }, - credstore.DeviceState{IDPDomain: domain, Environment: string(env), Audience: audience, Insecure: insecure, Backend: backend}, + credstore.DeviceState{IDPDomain: domain, Environment: string(env), Insecure: insecure, Backend: backend}, ); err != nil { return err } @@ -439,7 +427,7 @@ func newAuthTokenCommand() *cli.Command { Name: "token", Usage: "Print a valid access token for the LFX platform", Action: func(ctx context.Context, cmd *cli.Command) error { - token, _, _, err := resolveAccessToken(ctx, cmd) + token, _, err := resolveAccessToken(ctx, cmd) if err != nil { return err } @@ -451,21 +439,19 @@ func newAuthTokenCommand() *cli.Command { // resolveAccessToken returns a valid access token for the current login, // refreshing it (and persisting the refreshed credentials) if the cached -// one is missing or expired. It also returns the audience and login -// environment recorded at login time, so callers (e.g. `lfx api`) can check -// the audience against the environment's compiled-in default (see -// apiDefaultBaseURL; the persisted audience is never trusted as a base URL -// on its own) and use the environment to gate features like --hostname to -// non-production logins. Both `lfx auth -// token` and `lfx api` share this single code path so their refresh, -// error, and credential-persistence behavior never drifts apart. -func resolveAccessToken(ctx context.Context, cmd *cli.Command) (token, audience string, env authEnvironment, err error) { +// one is missing or expired. It also returns the login environment, so +// callers (e.g. `lfx api`) can compute the environment's compiled-in +// default audience (see defaultAudienceForEnvironment) themselves, rather +// than trusting anything persisted in state.json. Both `lfx auth token` +// and `lfx api` share this single code path so their refresh, error, and +// credential-persistence behavior never drifts apart. +func resolveAccessToken(ctx context.Context, cmd *cli.Command) (token string, env authEnvironment, err error) { store, creds, found, err := loadStoredCredentials(cmd) if err != nil { - return "", "", "", err + return "", "", err } if !found { - return "", "", "", errors.New("not logged in; run `lfx auth login` first") + return "", "", errors.New("not logged in; run `lfx auth login` first") } // Validate the persisted device state (insecure-storage and @@ -476,15 +462,15 @@ func resolveAccessToken(ctx context.Context, cmd *cli.Command) (token, audience // pin. state, domain, clientID, err := loadDeviceStateForBackend(store, cmd) if err != nil { - return "", "", "", fmt.Errorf("load device state: %w", err) + return "", "", fmt.Errorf("load device state: %w", err) } if creds.ValidAccessToken() { - return creds.AccessToken, state.Audience, normalizeEnvironment(state.Environment), nil + return creds.AccessToken, normalizeEnvironment(state.Environment), nil } if creds.RefreshToken == "" { - return "", "", "", errors.New("no refresh token available; run `lfx auth login` again") + return "", "", errors.New("no refresh token available; run `lfx auth login` again") } cfg := &oauth2.Config{ @@ -499,10 +485,10 @@ func resolveAccessToken(ctx context.Context, cmd *cli.Command) (token, audience refreshed, err := cfg.TokenSource(ctx, &oauth2.Token{RefreshToken: creds.RefreshToken}).Token() var retrieveErr *oauth2.RetrieveError if errors.As(err, &retrieveErr) && retrieveErr.ErrorCode == "invalid_grant" { - return "", "", "", errors.New("session expired or revoked; run `lfx auth login` to log in again") + 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) + return "", "", fmt.Errorf("refresh access token: %w", err) } refreshToken := refreshed.RefreshToken @@ -516,10 +502,10 @@ func resolveAccessToken(ctx context.Context, cmd *cli.Command) (token, audience AccessToken: refreshed.AccessToken, AccessTokenExpiry: refreshed.Expiry, }); err != nil { - return "", "", "", fmt.Errorf("save refreshed credentials: %w", err) + return "", "", fmt.Errorf("save refreshed credentials: %w", err) } - return refreshed.AccessToken, state.Audience, normalizeEnvironment(state.Environment), nil + return refreshed.AccessToken, normalizeEnvironment(state.Environment), nil } func newAuthStatusCommand() *cli.Command { @@ -555,14 +541,15 @@ func newAuthStatusCommand() *cli.Command { ) } if state.Environment != "" { - fmt.Printf(" %-22s %s\n", "Environment:", normalizeEnvironment(state.Environment)) + env := normalizeEnvironment(state.Environment) + fmt.Printf(" %-22s %s\n", "Environment:", env) + if audience, err := defaultAudienceForEnvironment(env); err == nil { + fmt.Printf(" %-22s %s\n", "Audience:", audience) + } } if state.IDPDomain != "" { fmt.Printf(" %-22s %s\n", "IdP domain:", state.IDPDomain) } - if state.Audience != "" { - fmt.Printf(" %-22s %s\n", "Audience:", state.Audience) - } fmt.Printf(" %-22s %s\n", "Credential backend:", backend) if state.Backend != "" { fmt.Printf(" %-22s %s\n", "Pinned backend:", state.Backend) diff --git a/internal/commands/environment.go b/internal/commands/environment.go index 3d0e7cd..fd94590 100644 --- a/internal/commands/environment.go +++ b/internal/commands/environment.go @@ -31,16 +31,6 @@ const ( // since this CLI never calls the Auth0 Management API (only the device // code and token endpoints), there's no need to separately track the // underlying tenant name. -// -// `runAPI`'s --hostname gate (api.go) restricts redirecting the API base -// URL to development-environment logins: the goal is to keep a prod or -// staging token -- the ones with real authority -- from ever being sent -// to a third-party host, since --hostname's troubleshooting use case only -// comes up in development anyway. That restriction is also independently -// safe, since each tenant here is a distinct OAuth2 issuer and resource -// servers validate a token's issuer (`iss`), not just its audience: even -// a development-issued token claiming the prod audience would still be -// rejected by prod as untrusted. var authDomains = map[authEnvironment]string{ envProduction: "sso.linuxfoundation.org", envStaging: "linuxfoundation-staging.auth0.com", @@ -59,9 +49,9 @@ var authClientIDs = map[authEnvironment]string{ // cspell:enable -// defaultAudiences maps each authEnvironment to the matching LFX v2 API -// audience used when `lfx auth login` is run without an explicit -// `--audience` override. +// defaultAudiences maps each authEnvironment to the compiled-in LFX v2 API +// audience that `lfx auth login` requests and `lfx api` sends requests to +// for that environment. var defaultAudiences = map[authEnvironment]string{ envProduction: "https://lfx-api.v2.cluster.lfx.dev/", envStaging: "https://lfx-api.staging.v2.cluster.linuxfound.info/", diff --git a/internal/credstore/credstore.go b/internal/credstore/credstore.go index d7dc56f..e95b298 100644 --- a/internal/credstore/credstore.go +++ b/internal/credstore/credstore.go @@ -26,8 +26,8 @@ // acceptable, and is deliberately less secure than the keyring-backed // storage. // -// Non-sensitive state (environment, IdP domain, and audience used at login) -// is always stored as plain JSON under the XDG state directory +// Non-sensitive state (environment and IdP domain 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 @@ -175,7 +175,7 @@ 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 environment, IdP domain, or audience used at login. +// re-specify the environment or IdP domain used at login. // // Note: this deliberately does not include a persistent "device ID". One // was considered on the assumption that `gh` uses one as part of its @@ -193,15 +193,6 @@ type DeviceState struct { // 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 for display in `lfx auth status` - // and so `lfx api` can refuse a login whose audience differs from - // its environment's compiled-in default. Like the rest of state.json - // it is unauthenticated, so it must never on its own choose which - // host receives a bearer token. - 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 diff --git a/internal/credstore/credstore_test.go b/internal/credstore/credstore_test.go index 8d7d363..e4ff536 100644 --- a/internal/credstore/credstore_test.go +++ b/internal/credstore/credstore_test.go @@ -83,7 +83,6 @@ func TestDeviceStateRoundTrip(t *testing.T) { want := DeviceState{ IDPDomain: "linuxfoundation-dev.auth0.com", Environment: "development", - Audience: "https://example.test/", Insecure: true, } if err := store.SaveDeviceState(want); err != nil {