diff --git a/AGENTS.md b/AGENTS.md index ca03943..1d20caf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,11 +46,13 @@ lfx-cli/ ### Current State -`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. +The CLI implements `lfx auth login` / `status` / `token` / `logout` +(Auth0 Device Code flow, refresh-token exchange, and credential storage +via the system keychain with `99designs/keyring`, or a plain +`--insecure-storage` fallback) and `lfx api` (raw authenticated calls +against LFX platform APIs, with custom methods/headers, JSON body +construction via `--field`/`--raw-field`, raw bodies via `--input`/stdin, +and response filtering via `--query`). **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 45cda85..9d215a2 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,9 @@ lfx auth token lfx auth logout # Make an authenticated call to an LFX platform API endpoint. -lfx api +lfx api '/my-grants?v=1&object_type=projects' +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 ``` Credentials (refresh token, cached access token) are stored in your @@ -50,11 +52,11 @@ Service reachable in one shell session but not another); pass `--backend` to pin it to one explicitly (see `lfx auth backends` for the available names). Once a login has pinned a backend, later commands must pass the same `--backend` value. Pass `--insecure-storage` to -any `auth` subcommand to instead store credentials in a plain, unencrypted, -owner-only file, at the cost of weaker protection for the stored tokens. On -Windows, this owner-only mode relies on inherited directory permissions -rather than a real ACL, since Go's `Chmod(0600)` maps to the read-only -attribute there rather than restricting access to the current user. +instead store credentials in a plain, unencrypted, owner-only file, at the +cost of weaker protection for the stored tokens. On Windows, this +owner-only mode relies on inherited directory permissions rather than a +real ACL, since Go's `Chmod(0600)` maps to the read-only attribute there +rather than restricting access to the current user. ```bash lfx auth login --insecure-storage @@ -63,11 +65,6 @@ lfx auth login --backend=keychain Run `lfx --help` or `lfx --help` for full details on any command. -> **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. - ## Development ```bash diff --git a/cmd/lfx/main.go b/cmd/lfx/main.go index 6a34b18..a108cec 100644 --- a/cmd/lfx/main.go +++ b/cmd/lfx/main.go @@ -38,6 +38,7 @@ func main() { Usage: "Authenticate with and call the LFX platform APIs", Version: version, EnableShellCompletion: true, + Flags: commands.CredentialStoreFlags, Commands: []*cli.Command{ commands.NewAuthCommand(), commands.NewAPICommand(), diff --git a/go.mod b/go.mod index e66af03..79ad9f7 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ go 1.25.14 require ( github.com/99designs/keyring v1.2.2 + github.com/tidwall/gjson v1.19.0 github.com/urfave/cli-docs/v3 v3.1.0 github.com/urfave/cli/v3 v3.11.0 golang.org/x/oauth2 v0.36.0 @@ -20,6 +21,8 @@ require ( github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c // indirect github.com/mtibben/percent v0.2.1 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect + github.com/tidwall/match v1.1.1 // indirect + github.com/tidwall/pretty v1.2.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/term v0.45.0 // indirect ) diff --git a/go.sum b/go.sum index 44c705b..c0e834f 100644 --- a/go.sum +++ b/go.sum @@ -29,6 +29,12 @@ github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +github.com/tidwall/gjson v1.19.0 h1:xwxm7n691Uf3u5OFjzngavjGTh55KX5q/9w9xHW88JU= +github.com/tidwall/gjson v1.19.0/go.mod h1:V37/opeE/JbLUOfH0QTXiNez2l0RUjYUhpT4szFQAfc= +github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= +github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= +github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs= +github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= github.com/urfave/cli-docs/v3 v3.1.0 h1:Sa5xm19IpE5gpm6tZzXdfjdFxn67PnEsE4dpXF7vsKw= github.com/urfave/cli-docs/v3 v3.1.0/go.mod h1:59d+5Hz1h6GSGJ10cvcEkbIe3j233t4XDqI72UIx7to= github.com/urfave/cli/v3 v3.11.0 h1:P/euJp99kb9p0tlVY+iYTLYYTAQlfl0hR2gUO1Img1Q= diff --git a/internal/commands/api.go b/internal/commands/api.go index cea6f7f..58aa4c3 100644 --- a/internal/commands/api.go +++ b/internal/commands/api.go @@ -5,24 +5,379 @@ package commands import ( + "bytes" "context" + "encoding/json" + "errors" "fmt" + "io" + "net/http" + "net/url" + "os" + "regexp" + "strings" + "github.com/tidwall/gjson" "github.com/urfave/cli/v3" ) +// Flag names for the `api` command. +const ( + apiMethodFlagName = "method" + apiHeaderFlagName = "header" + apiInputFlagName = "input" + apiFieldFlagName = "field" + apiRawFieldFlagName = "raw-field" + apiQueryFlagName = "query" + apiHostnameFlagName = "hostname" +) + +// apiAllowedMethods enumerates the HTTP methods `lfx api` accepts via +// --method. PATCH is deliberately excluded: the LFX API, by design, +// currently includes no PATCH endpoints, favoring PUT with ETag/If-Match +// concurrency control instead. +var apiAllowedMethods = map[string]bool{ + http.MethodGet: true, + http.MethodPost: true, + http.MethodPut: true, + http.MethodDelete: true, +} + // NewAPICommand builds the `lfx api` command for making raw authenticated // calls against LFX platform APIs. -// -// This is currently a stub; the real implementation lands in LFXV2-2517. func NewAPICommand() *cli.Command { return &cli.Command{ Name: "api", Usage: "Make an authenticated call to an LFX platform API endpoint", - ArgsUsage: " ", - Action: func(_ context.Context, _ *cli.Command) error { - fmt.Println("lfx api: not yet implemented (see LFXV2-2517)") - return nil + ArgsUsage: "", + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: apiMethodFlagName, + Aliases: []string{"X"}, + Usage: "HTTP method: GET, POST, PUT, or DELETE (default: GET, or POST if a body is explicitly supplied)", + Value: http.MethodGet, + }, + &cli.StringSliceFlag{ + Name: apiHeaderFlagName, + Aliases: []string{"H"}, + Usage: "Add an additional request header as 'key:value' (repeatable)", + }, + &cli.StringFlag{ + Name: apiInputFlagName, + Usage: "Read the request body from a file, or '-' for stdin (Content-Type defaults to application/json for POST/PUT unless overridden with -H)", + }, + &cli.StringSliceFlag{ + Name: apiFieldFlagName, + Aliases: []string{"F"}, + Usage: "Add a typed JSON body field as 'key=value' (repeatable)", + }, + &cli.StringSliceFlag{ + Name: apiRawFieldFlagName, + Usage: "Add a string JSON body field as 'key=value' (repeatable)", + }, + &cli.StringFlag{ + Name: apiQueryFlagName, + 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, + } +} + +func runAPI(ctx context.Context, cmd *cli.Command) error { + if cmd.Args().Len() != 1 { + return errors.New("usage: lfx api ") + } + path := cmd.Args().First() + + method := strings.ToUpper(cmd.String(apiMethodFlagName)) + if !cmd.IsSet(apiMethodFlagName) && apiHasExplicitBody(cmd) { + // No --method was passed, but the caller explicitly supplied a + // body via --field/--raw-field/--input; a GET request wouldn't + // carry it anywhere useful. Auto-promote to POST, matching the + // conventional default `gh api` and `curl` both use when a body + // is present. This never triggers for a body implicitly picked + // up from piped stdin with no explicit body flag, since that's + // not a clear enough signal of intent to override the GET + // default. + method = http.MethodPost + } + if !apiAllowedMethods[method] { + return fmt.Errorf("invalid --%s %q (must be one of GET, POST, PUT, DELETE)", apiMethodFlagName, method) + } + + token, audience, 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 audience 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 = audience + } + if baseURL == "" { + return errors.New("no API base URL available; log in with `lfx auth login` or pass --hostname") + } + if err := apiRequireHTTPS(baseURL); err != nil { + return err + } + + body, contentType, err := apiRequestBody(cmd) + if err != nil { + return err + } + + endpoint, err := apiJoinURL(baseURL, path) + if err != nil { + return err + } + + req, err := http.NewRequestWithContext(ctx, method, endpoint, bytes.NewReader(body)) + if err != nil { + return fmt.Errorf("build request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+token) + if contentType != "" { + req.Header.Set("Content-Type", contentType) + } + for _, h := range cmd.StringSlice(apiHeaderFlagName) { + key, value, ok := strings.Cut(h, ":") + if !ok { + return fmt.Errorf("invalid --%s %q (expected 'key:value')", apiHeaderFlagName, h) + } + // Header.Set (rather than Add) is deliberate: it lets a + // repeated -H for the same key (e.g. an explicit + // "Authorization:" or "Content-Type:") cleanly override the + // value set above, at the cost of not accumulating multiple + // values for the same header name the way curl/gh api do. + req.Header.Set(strings.TrimSpace(key), strings.TrimSpace(value)) + } + if len(body) > 0 && req.Header.Get("Content-Type") == "" && (method == http.MethodPost || method == http.MethodPut) { + // LFX platform APIs are overwhelmingly JSON, so default to that + // for any POST/PUT body unless the caller set Content-Type + // explicitly (via --field/--raw-field, which already produce + // JSON and set this above, or via -H, handled just above). + req.Header.Set("Content-Type", "application/json") + } + + // Don't auto-follow redirects: http.DefaultClient can preserve the + // Authorization header across a same-host redirect even when it + // downgrades from HTTPS to HTTP, which would bypass + // apiRequireHTTPS and leak the bearer token. A 3xx response is + // instead reported like any other non-2xx status below. + client := &http.Client{ + CheckRedirect: func(_ *http.Request, _ []*http.Request) error { + return http.ErrUseLastResponse + }, + } + resp, err := client.Do(req) + if err != nil { + return fmt.Errorf("request failed: %w", err) + } + defer func() { + _ = resp.Body.Close() + }() + + if query := cmd.String(apiQueryFlagName); query != "" { + // --query needs the whole body in memory to run gjson against + // it, so buffer and filter before writing. + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("read response body: %w", err) + } + output := []byte(gjson.GetBytes(respBody, query).String()) + // Write output verbatim with no added trailing newline, + // matching `gh api -q` so a scalar result (e.g. a single + // field) can be captured cleanly by a shell command + // substitution without a stray newline. + if _, err := os.Stdout.Write(output); err != nil { + return fmt.Errorf("write response body: %w", err) + } + } else { + // Stream the body straight through rather than buffering it + // all in memory first, since this command explicitly supports + // redirecting its output for large or binary responses. + if _, err := io.Copy(os.Stdout, resp.Body); err != nil { + return fmt.Errorf("write response body: %w", err) + } + } + + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + // Use cli.Exit's message (rather than writing directly to + // stderr) so main's generic `Error: ` handling only emits + // one diagnostic line instead of two. + return cli.Exit(fmt.Sprintf("HTTP %d", resp.StatusCode), 1) + } + + return nil +} + +// apiHasExplicitBody reports whether cmd was given an explicit +// body-supplying flag (--input, --field, or --raw-field), as opposed to a +// body implicitly picked up from piped stdin with none of those flags set. +// Used to decide whether the default --method should be promoted from GET; +// see runAPI. +func apiHasExplicitBody(cmd *cli.Command) bool { + return cmd.IsSet(apiInputFlagName) || + len(cmd.StringSlice(apiFieldFlagName)) > 0 || + len(cmd.StringSlice(apiRawFieldFlagName)) > 0 +} + +// apiRequestBody constructs the request body and its Content-Type from the +// command's --input, --field, and --raw-field flags (mutually exclusive: +// --input vs. --field/--raw-field), falling back to stdin when none are +// passed and stdin is piped (non-TTY). +func apiRequestBody(cmd *cli.Command) (body []byte, contentType string, err error) { + inputSet := cmd.IsSet(apiInputFlagName) + input := cmd.String(apiInputFlagName) + fields := cmd.StringSlice(apiFieldFlagName) + rawFields := cmd.StringSlice(apiRawFieldFlagName) + + if inputSet && (len(fields) > 0 || len(rawFields) > 0) { + return nil, "", fmt.Errorf("--%s cannot be combined with --%s or --%s", apiInputFlagName, apiFieldFlagName, apiRawFieldFlagName) + } + + if inputSet { + if input == "" { + return nil, "", fmt.Errorf("--%s was set to an empty value; omit the flag to read the body from stdin instead", apiInputFlagName) + } + if input == "-" { + data, err := io.ReadAll(os.Stdin) + if err != nil { + return nil, "", fmt.Errorf("read stdin: %w", err) + } + return data, "", nil + } + data, err := os.ReadFile(input) + if err != nil { + return nil, "", fmt.Errorf("read --%s file: %w", apiInputFlagName, err) + } + return data, "", nil + } + + if len(fields) > 0 || len(rawFields) > 0 { + obj := make(map[string]any, len(fields)+len(rawFields)) + for _, f := range fields { + key, value, ok := strings.Cut(f, "=") + if !ok { + return nil, "", fmt.Errorf("invalid --%s %q (expected 'key=value')", apiFieldFlagName, f) + } + obj[key] = coerceFieldValue(value) + } + for _, f := range rawFields { + key, value, ok := strings.Cut(f, "=") + if !ok { + return nil, "", fmt.Errorf("invalid --%s %q (expected 'key=value')", apiRawFieldFlagName, f) + } + obj[key] = value + } + data, err := json.Marshal(obj) + if err != nil { + return nil, "", fmt.Errorf("build JSON body: %w", err) + } + return data, "application/json", nil + } + + stat, err := os.Stdin.Stat() + if err != nil { + return nil, "", fmt.Errorf("stat stdin: %w", err) + } + if stat.Mode()&os.ModeCharDevice == 0 { + data, err := io.ReadAll(os.Stdin) + if err != nil { + return nil, "", fmt.Errorf("read stdin: %w", err) + } + return data, "", nil + } + + return nil, "", nil +} + +// jsonNumberPattern matches valid JSON number syntax (RFC 8259), which is +// stricter than Go's strconv.ParseFloat: it rejects "NaN", "Inf", and +// hexadecimal floats (e.g. "0x1p2"), all of which ParseFloat accepts but +// which are not valid JSON numbers and would otherwise either fail +// json.Marshal outright or silently coerce a value the user likely meant +// as a literal string. +var jsonNumberPattern = regexp.MustCompile(`^-?(0|[1-9]\d*)(\.\d+)?([eE][+-]?\d+)?$`) + +// coerceFieldValue applies gh-style type coercion to a --field value: +// "true"/"false" become booleans, "null" becomes nil, and values matching +// JSON number syntax become json.Number (preserving the original digits +// verbatim in the request body, rather than round-tripping through +// float64 and silently losing precision for integers beyond 2^53). +// Everything else stays a string. +func coerceFieldValue(value string) any { + switch value { + case "true": + return true + case "false": + return false + case "null": + return nil + } + if jsonNumberPattern.MatchString(value) { + return json.Number(value) + } + return value +} + +// 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). +func apiJoinURL(base, path string) (string, error) { + if base == "" { + return "", errors.New("empty base URL") + } + return strings.TrimRight(base, "/") + "/" + strings.TrimLeft(path, "/"), nil +} + +// 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. +func apiRequireHTTPS(rawURL string) error { + parsed, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("invalid base URL %q: %w", rawURL, err) + } + if strings.EqualFold(parsed.Scheme, "https") { + return nil + } + switch parsed.Hostname() { + 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) } diff --git a/internal/commands/api_test.go b/internal/commands/api_test.go new file mode 100644 index 0000000..4aa4ae0 --- /dev/null +++ b/internal/commands/api_test.go @@ -0,0 +1,305 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +package commands + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/urfave/cli/v3" +) + +// newAPITestCommand builds a *cli.Command with the `api` command's flags +// registered, parses args against it, and hands the parsed *cli.Command to +// fn, mirroring newTestCommand in auth_test.go for the auth flags. +func newAPITestCommand(t *testing.T, args []string, fn func(cmd *cli.Command)) { + t.Helper() + cmd := &cli.Command{ + Name: "test", + Flags: NewAPICommand().Flags, + Action: func(_ context.Context, cmd *cli.Command) error { + fn(cmd) + return nil + }, + } + if err := cmd.Run(context.Background(), append([]string{"test"}, args...)); err != nil { + t.Fatalf("cmd.Run: %v", err) + } +} + +func TestCoerceFieldValue(t *testing.T) { + tests := []struct { + name string + value string + want any + }{ + {name: "true", value: "true", want: true}, + {name: "false", value: "false", want: false}, + {name: "null", value: "null", want: nil}, + {name: "integer", value: "42", want: json.Number("42")}, + {name: "float", value: "3.14", want: json.Number("3.14")}, + {name: "plain string", value: "hello", want: "hello"}, + {name: "numeric-looking but not fully numeric", value: "42abc", want: "42abc"}, + {name: "empty string", value: "", want: ""}, + {name: "NaN stays a string", value: "NaN", want: "NaN"}, + {name: "Infinity stays a string", value: "Infinity", want: "Infinity"}, + {name: "hex float stays a string", value: "0x1p2", want: "0x1p2"}, + {name: "leading zero stays a string (not valid JSON number)", value: "042", want: "042"}, + {name: "large integer preserves precision", value: "9007199254740993", want: json.Number("9007199254740993")}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := coerceFieldValue(tc.value); got != tc.want { + t.Errorf("coerceFieldValue(%q) = %#v, want %#v", tc.value, got, tc.want) + } + }) + } +} + +func TestAPIJoinURL(t *testing.T) { + tests := []struct { + name string + base string + path string + want string + }{ + {name: "no trailing/leading slash", base: "https://api.example.com", path: "projects", want: "https://api.example.com/projects"}, + {name: "trailing slash on base", base: "https://api.example.com/", path: "projects", want: "https://api.example.com/projects"}, + {name: "leading slash on path", base: "https://api.example.com", path: "/projects", want: "https://api.example.com/projects"}, + {name: "both slashes", base: "https://api.example.com/", path: "/projects", want: "https://api.example.com/projects"}, + {name: "query string preserved", base: "https://api.example.com", path: "/my-grants?v=1&object_type=projects", want: "https://api.example.com/my-grants?v=1&object_type=projects"}, + {name: "fragment preserved", base: "https://api.example.com", path: "/projects#frag", want: "https://api.example.com/projects#frag"}, + {name: "encoded slash preserved verbatim", base: "https://api.example.com", path: "/objects/a%2Fb", want: "https://api.example.com/objects/a%2Fb"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := apiJoinURL(tc.base, tc.path) + if err != nil { + t.Fatalf("apiJoinURL(%q, %q): %v", tc.base, tc.path, err) + } + if got != tc.want { + t.Errorf("apiJoinURL(%q, %q) = %q, want %q", tc.base, tc.path, got, tc.want) + } + }) + } + + t.Run("empty base", func(t *testing.T) { + if _, err := apiJoinURL("", "/projects"); err == nil { + t.Fatal("apiJoinURL: got nil error, want error for empty base") + } + }) +} + +func TestAPIRequireHTTPS(t *testing.T) { + tests := []struct { + name string + rawURL string + wantErr bool + }{ + {name: "https", rawURL: "https://api.example.com", wantErr: false}, + {name: "https uppercase scheme", rawURL: "HTTPS://api.example.com", wantErr: false}, + {name: "https with port", rawURL: "https://api.example.com:8443", wantErr: false}, + {name: "http rejected", rawURL: "http://api.example.com", wantErr: true}, + {name: "http localhost allowed", rawURL: "http://localhost:8080", wantErr: false}, + {name: "http 127.0.0.1 allowed", rawURL: "http://127.0.0.1:8080", wantErr: false}, + {name: "http ::1 allowed", rawURL: "http://[::1]:8080", wantErr: false}, + {name: "http on non-loopback hostname resembling localhost rejected", rawURL: "http://localhost.attacker.example", wantErr: true}, + {name: "invalid URL rejected", rawURL: "http://[::1", wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := apiRequireHTTPS(tc.rawURL) + if tc.wantErr && err == nil { + t.Fatalf("apiRequireHTTPS(%q): got nil error, want error", tc.rawURL) + } + if !tc.wantErr && err != nil { + t.Fatalf("apiRequireHTTPS(%q): got error %v, want nil", tc.rawURL, err) + } + }) + } +} + +func TestAPIRequestBodyFields(t *testing.T) { + newAPITestCommand(t, []string{ + "--field", "name=example", + "--field", "active=true", + "--field", "count=3", + "--raw-field", "note=42", + }, func(cmd *cli.Command) { + body, contentType, err := apiRequestBody(cmd) + if err != nil { + t.Fatalf("apiRequestBody: %v", err) + } + if contentType != "application/json" { + t.Errorf("contentType = %q, want application/json", contentType) + } + + var got map[string]any + if err := json.Unmarshal(body, &got); err != nil { + t.Fatalf("json.Unmarshal(body): %v", err) + } + want := map[string]any{ + "name": "example", + "active": true, + "count": float64(3), + // --raw-field always stays a JSON string, even though "42" + // would otherwise coerce to a number via --field. + "note": "42", + } + if len(got) != len(want) { + t.Fatalf("body = %v, want %v", got, want) + } + for k, v := range want { + if got[k] != v { + t.Errorf("body[%q] = %#v, want %#v", k, got[k], v) + } + } + }) +} + +func TestAPIRequestBodyInvalidField(t *testing.T) { + newAPITestCommand(t, []string{"--field", "no-equals-sign"}, func(cmd *cli.Command) { + if _, _, err := apiRequestBody(cmd); err == nil { + t.Fatal("apiRequestBody: got nil error, want error for malformed --field") + } + }) +} + +func TestAPIRequestBodyInvalidRawField(t *testing.T) { + newAPITestCommand(t, []string{"--raw-field", "no-equals-sign"}, func(cmd *cli.Command) { + if _, _, err := apiRequestBody(cmd); err == nil { + t.Fatal("apiRequestBody: got nil error, want error for malformed --raw-field") + } + }) +} + +func TestAPIRequestBodyInputFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "body.json") + if err := os.WriteFile(path, []byte(`{"raw":true}`), 0o600); err != nil { + t.Fatalf("os.WriteFile: %v", err) + } + + newAPITestCommand(t, []string{"--input", path}, func(cmd *cli.Command) { + body, contentType, err := apiRequestBody(cmd) + if err != nil { + t.Fatalf("apiRequestBody: %v", err) + } + if contentType != "" { + t.Errorf("contentType = %q, want empty (raw --input sets no Content-Type)", contentType) + } + if string(body) != `{"raw":true}` { + t.Errorf("body = %q, want %q", body, `{"raw":true}`) + } + }) +} + +func TestAPIRequestBodyInputRejectsCombiningWithFields(t *testing.T) { + newAPITestCommand(t, []string{"--input", "/dev/null", "--field", "name=example"}, func(cmd *cli.Command) { + if _, _, err := apiRequestBody(cmd); err == nil { + t.Fatal("apiRequestBody: got nil error, want error for --input combined with --field") + } + }) +} + +func TestAPIRequestBodyRejectsExplicitEmptyInput(t *testing.T) { + newAPITestCommand(t, []string{"--input="}, func(cmd *cli.Command) { + if _, _, err := apiRequestBody(cmd); err == nil { + t.Fatal("apiRequestBody: got nil error, want error for explicit empty --input=") + } + }) +} + +func TestAPIRequestBodyExplicitEmptyInputRejectsCombiningWithFields(t *testing.T) { + newAPITestCommand(t, []string{"--input=", "--field", "name=example"}, func(cmd *cli.Command) { + if _, _, err := apiRequestBody(cmd); err == nil { + t.Fatal("apiRequestBody: got nil error, want error for explicit empty --input= combined with --field") + } + }) +} + +func TestAPIHasExplicitBody(t *testing.T) { + tests := []struct { + name string + args []string + want bool + }{ + {name: "no body flags", args: nil, want: false}, + {name: "field", args: []string{"--field", "name=example"}, want: true}, + {name: "raw-field", args: []string{"--raw-field", "name=example"}, want: true}, + {name: "input file", args: []string{"--input", "/dev/null"}, want: true}, + {name: "input stdin marker", args: []string{"--input", "-"}, want: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + newAPITestCommand(t, tc.args, func(cmd *cli.Command) { + if got := apiHasExplicitBody(cmd); got != tc.want { + t.Errorf("apiHasExplicitBody() = %v, want %v", got, tc.want) + } + }) + }) + } +} + +func TestAPIRequestBodyStatError(t *testing.T) { + // Substitute an already-closed file for os.Stdin so Stat() fails, + // simulating the rare platforms/conditions where stdin can't be + // probed at all; apiRequestBody should surface that error rather + // than silently falling back to an empty body. + f, err := os.CreateTemp(t.TempDir(), "closed-stdin") + if err != nil { + t.Fatalf("os.CreateTemp: %v", err) + } + if err := f.Close(); err != nil { + t.Fatalf("close temp file: %v", err) + } + + orig := os.Stdin + os.Stdin = f + t.Cleanup(func() { os.Stdin = orig }) + + newAPITestCommand(t, nil, func(cmd *cli.Command) { + if _, _, err := apiRequestBody(cmd); err == nil { + t.Fatal("apiRequestBody: got nil error, want error from failed stdin Stat()") + } + }) +} + +func TestAPIRequestBodyNoneWithPipedStdin(t *testing.T) { + // os.Stdin under `go test` is not a real TTY, so apiRequestBody's + // non-char-device check treats it as piped; substitute an explicit, + // already-closed pipe end so the test deterministically exercises + // the "read piped stdin" branch without depending on -- or + // blocking on -- whatever stdin the test binary happened to + // inherit. + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe: %v", err) + } + if _, err := w.WriteString(`{"from":"stdin"}`); err != nil { + t.Fatalf("write to pipe: %v", err) + } + if err := w.Close(); err != nil { + t.Fatalf("close pipe writer: %v", err) + } + + orig := os.Stdin + os.Stdin = r + t.Cleanup(func() { os.Stdin = orig }) + + newAPITestCommand(t, nil, func(cmd *cli.Command) { + body, contentType, err := apiRequestBody(cmd) + if err != nil { + t.Fatalf("apiRequestBody: %v", err) + } + if contentType != "" { + t.Errorf("contentType = %q, want empty (piped stdin sets no Content-Type)", contentType) + } + if string(body) != `{"from":"stdin"}` { + t.Errorf("body = %q, want %q", body, `{"from":"stdin"}`) + } + }) +} diff --git a/internal/commands/auth.go b/internal/commands/auth.go index d43ffb4..79ef3a6 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -43,35 +43,41 @@ const ( audienceFlagName = "audience" ) +// CredentialStoreFlags are the --insecure-storage and --backend flags +// used by credStoreFromCommand. They are registered on the root `lfx` +// command in cmd/lfx/main.go (rather than per-subcommand) so they're +// inherited via cmd.Bool/cmd.String without redeclaration wherever +// they're needed -- both the auth-specific commands and API/method calls. +var CredentialStoreFlags = []cli.Flag{ + &cli.BoolFlag{ + Name: insecureStorageFlagName, + Usage: "Store & retrieve credentials in a plain (unencrypted) file instead of the system backend", + }, + &cli.StringFlag{ + Name: backendFlagName, + Usage: "Pin credential storage to a specific system backend (see `lfx auth backends`); mutually exclusive with --insecure-storage", + }, +} + // 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 and --backend flags are shared by all -// subcommands (they are not declared as "Local" flags, so urfave/cli -// resolves them for subcommand actions via cmd.Bool/cmd.String). -// --insecure-storage controls whether credentials bypass the system -// backend in favor of credstore's plain (unencrypted) file fallback, e.g. -// for headless/CI use. --backend pins credential storage to a -// single system backend rather than letting keyring.Open silently pick -// whichever one currently opens; see credstore.DeviceState.Backend for why -// that matters once a login has pinned one. +// --insecure-storage and --backend (registered on the root `lfx` command; +// see CredentialStoreFlags) control credential storage for all +// subcommands here. --insecure-storage controls whether credentials +// bypass the system backend in favor of credstore's plain (unencrypted) +// file fallback, e.g. for headless/CI use. --backend pins credential +// storage to a single system backend rather than letting keyring.Open +// silently pick whichever one currently opens; see +// credstore.DeviceState.Backend for why that matters once a login has +// pinned one. func NewAuthCommand() *cli.Command { return &cli.Command{ Name: "auth", Usage: "Manage authentication with the LFX platform", - Flags: []cli.Flag{ - &cli.BoolFlag{ - Name: insecureStorageFlagName, - Usage: "Store credentials in a plain (unencrypted) file instead of the system backend", - }, - &cli.StringFlag{ - Name: backendFlagName, - Usage: "Pin credential storage to a specific system backend (see `lfx auth backends`); mutually exclusive with --insecure-storage", - }, - }, Commands: []*cli.Command{ newAuthLoginCommand(), newAuthTokenCommand(), @@ -438,70 +444,85 @@ 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 { - store, creds, found, err := loadStoredCredentials(cmd) + token, _, _, err := resolveAccessToken(ctx, cmd) if err != nil { return err } - if !found { - return errors.New("not logged in; run `lfx auth login` first") - } + fmt.Println(token) + return nil + }, + } +} - // Validate the persisted device state (insecure-storage and - // --backend pinning) before trusting or returning anything - // from creds, including the ValidAccessToken fast path below - // -- otherwise omitting a pinned --backend could still open - // some other auto-detected backend and print its cached - // token, defeating the pin. - _, domain, clientID, err := loadDeviceStateForBackend(store, cmd) - if err != nil { - return fmt.Errorf("load device state: %w", err) - } +// 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 +// 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) { + store, creds, found, err := loadStoredCredentials(cmd) + if err != nil { + return "", "", "", err + } + if !found { + return "", "", "", errors.New("not logged in; run `lfx auth login` first") + } - if creds.ValidAccessToken() { - fmt.Println(creds.AccessToken) - return nil - } + // Validate the persisted device state (insecure-storage and + // --backend pinning) before trusting or returning anything from + // creds, including the ValidAccessToken fast path below -- otherwise + // omitting a pinned --backend could still open some other + // auto-detected backend and return its cached token, defeating the + // pin. + state, domain, clientID, err := loadDeviceStateForBackend(store, cmd) + if err != nil { + return "", "", "", fmt.Errorf("load device state: %w", err) + } - if creds.RefreshToken == "" { - return errors.New("no refresh token available; run `lfx auth login` again") - } + if creds.ValidAccessToken() { + return creds.AccessToken, state.Audience, authEnvironment(state.Environment), nil + } - cfg := &oauth2.Config{ - ClientID: clientID, - Endpoint: oauth2.Endpoint{ - DeviceAuthURL: "https://" + domain + "/oauth/device/code", - TokenURL: "https://" + domain + "/oauth/token", - AuthStyle: oauth2.AuthStyleInParams, - }, - } + if creds.RefreshToken == "" { + return "", "", "", errors.New("no refresh token available; run `lfx auth login` again") + } - token, 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") - } - if err != nil { - return fmt.Errorf("refresh access token: %w", err) - } + cfg := &oauth2.Config{ + ClientID: clientID, + Endpoint: oauth2.Endpoint{ + DeviceAuthURL: "https://" + domain + "/oauth/device/code", + TokenURL: "https://" + domain + "/oauth/token", + AuthStyle: oauth2.AuthStyleInParams, + }, + } - 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) - } + 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") + } + if err != nil { + return "", "", "", fmt.Errorf("refresh access token: %w", err) + } - fmt.Println(token.AccessToken) - return nil - }, + refreshToken := refreshed.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: refreshed.AccessToken, + AccessTokenExpiry: refreshed.Expiry, + }); err != nil { + return "", "", "", fmt.Errorf("save refreshed credentials: %w", err) } + + return refreshed.AccessToken, state.Audience, authEnvironment(state.Environment), nil } func newAuthStatusCommand() *cli.Command { diff --git a/internal/commands/auth_test.go b/internal/commands/auth_test.go index a795d1b..078a7aa 100644 --- a/internal/commands/auth_test.go +++ b/internal/commands/auth_test.go @@ -15,18 +15,15 @@ import ( ) // newTestCommand builds a *cli.Command with the --insecure-storage and -// --backend flags registered (as newAuthLoginCommand and friends -// do), parses args against it, and returns the parsed *cli.Command handed -// to fn's Action so tests can read flag values the way the real commands -// do. +// --backend flags registered (as the root `lfx` command does via +// CredentialStoreFlags), parses args against it, and returns the parsed +// *cli.Command handed to fn's Action so tests can read flag values the +// way the real commands do. func newTestCommand(t *testing.T, args []string, fn func(cmd *cli.Command)) { t.Helper() cmd := &cli.Command{ - Name: "test", - Flags: []cli.Flag{ - &cli.BoolFlag{Name: insecureStorageFlagName}, - &cli.StringFlag{Name: backendFlagName}, - }, + Name: "test", + Flags: CredentialStoreFlags, Action: func(_ context.Context, cmd *cli.Command) error { fn(cmd) return nil diff --git a/internal/commands/environment.go b/internal/commands/environment.go index 04a4667..dfd49da 100644 --- a/internal/commands/environment.go +++ b/internal/commands/environment.go @@ -30,6 +30,16 @@ 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{ envProd: "sso.linuxfoundation.org", envStaging: "linuxfoundation-staging.auth0.com", diff --git a/internal/credstore/credstore.go b/internal/credstore/credstore.go index f69626e..69e317b 100644 --- a/internal/credstore/credstore.go +++ b/internal/credstore/credstore.go @@ -178,8 +178,8 @@ func (c Credentials) ValidAccessToken() bool { // 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 +// was considered 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