From 13f14d2e99ff9f8dd9d20b654191c0a575891b39 Mon Sep 17 00:00:00 2001 From: Bazyli Brzoska Date: Wed, 3 Jun 2026 19:02:44 -0700 Subject: [PATCH 1/6] feat: add proxy auth mode for Atlassian CLIs --- shared/auth/auth.go | 7 +- shared/auth/auth_test.go | 4 + shared/client/client.go | 8 +- shared/client/client_test.go | 39 ++++++ shared/client/options.go | 21 +++ shared/credstore/conndivergence_test.go | 12 ++ shared/credstore/credstore.go | 7 +- shared/credstore/credstore_test.go | 16 +++ shared/url/url.go | 22 ++- shared/url/url_test.go | 28 ++++ tools/cfl/README.md | 16 ++- tools/cfl/api/client.go | 19 ++- tools/cfl/api/client_test.go | 29 ++++ tools/cfl/docs/development.md | 2 +- tools/cfl/integration-tests.md | 11 +- tools/cfl/internal/cmd/init/init.go | 107 ++++++++++---- tools/cfl/internal/cmd/init/init_test.go | 106 ++++++++++++-- tools/cfl/internal/cmd/root/root.go | 3 + tools/cfl/internal/cmd/root/root_test.go | 25 ++++ tools/cfl/internal/config/config.go | 38 +++-- tools/cfl/internal/config/config_test.go | 38 +++++ tools/jtk/README.md | 18 ++- tools/jtk/api/client.go | 50 +++++-- tools/jtk/api/client_test.go | 52 +++++++ tools/jtk/docs/development.md | 2 +- tools/jtk/integration-tests.md | 7 +- tools/jtk/internal/cmd/initcmd/initcmd.go | 132 ++++++++++++------ .../jtk/internal/cmd/initcmd/initcmd_test.go | 65 +++++++-- tools/jtk/internal/cmd/root/root.go | 14 +- tools/jtk/internal/cmd/root/root_test.go | 26 ++++ tools/jtk/internal/config/config.go | 19 +-- tools/jtk/internal/config/config_test.go | 11 ++ 32 files changed, 807 insertions(+), 147 deletions(-) diff --git a/shared/auth/auth.go b/shared/auth/auth.go index a287f680..faba6255 100644 --- a/shared/auth/auth.go +++ b/shared/auth/auth.go @@ -13,15 +13,18 @@ const ( // AuthMethodBearer is the authentication method for service accounts with scoped API tokens. AuthMethodBearer = "bearer" + + // AuthMethodProxy sends no Authorization header and relies on a local or upstream proxy. + AuthMethodProxy = "proxy" ) // ErrInvalidAuthMethod is returned when an unrecognized auth method is provided. -var ErrInvalidAuthMethod = errors.New("invalid auth method: must be \"basic\" or \"bearer\"") +var ErrInvalidAuthMethod = errors.New("invalid auth method: must be \"basic\", \"bearer\", or \"proxy\"") // ValidateAuthMethod returns nil if method is a recognized auth method, or ErrInvalidAuthMethod otherwise. func ValidateAuthMethod(method string) error { switch method { - case AuthMethodBasic, AuthMethodBearer: + case AuthMethodBasic, AuthMethodBearer, AuthMethodProxy: return nil default: return fmt.Errorf("%w: got %q", ErrInvalidAuthMethod, method) diff --git a/shared/auth/auth_test.go b/shared/auth/auth_test.go index a8018337..d731ed9e 100644 --- a/shared/auth/auth_test.go +++ b/shared/auth/auth_test.go @@ -114,6 +114,7 @@ func TestValidateAuthMethod(t *testing.T) { }{ {name: "basic is valid", method: "basic", wantErr: false}, {name: "bearer is valid", method: "bearer", wantErr: false}, + {name: "proxy is valid", method: "proxy", wantErr: false}, {name: "empty string is invalid", method: "", wantErr: true}, {name: "capitalized Bearer is invalid", method: "Bearer", wantErr: true}, {name: "unknown method is invalid", method: "oauth", wantErr: true}, @@ -147,4 +148,7 @@ func TestAuthMethodConstants(t *testing.T) { if AuthMethodBearer != "bearer" { t.Errorf("AuthMethodBearer = %q, want %q", AuthMethodBearer, "bearer") } + if AuthMethodProxy != "proxy" { + t.Errorf("AuthMethodProxy = %q, want %q", AuthMethodProxy, "proxy") + } } diff --git a/shared/client/client.go b/shared/client/client.go index 084e81f6..a78f9417 100644 --- a/shared/client/client.go +++ b/shared/client/client.go @@ -44,6 +44,7 @@ func New(baseURL, email, apiToken string, opts *Options) *Client { var verbose bool var verboseOut io.Writer = os.Stderr var authHeader string + var skipAuthHeader bool if opts != nil { timeout = opts.timeoutOrDefault() @@ -52,9 +53,10 @@ func New(baseURL, email, apiToken string, opts *Options) *Client { verboseOut = opts.VerboseOut } authHeader = opts.AuthHeader + skipAuthHeader = opts.SkipAuthHeader } - if authHeader == "" { + if authHeader == "" && !skipAuthHeader { authHeader = auth.BasicAuthHeader(email, apiToken) } @@ -106,7 +108,9 @@ func (c *Client) Do(ctx context.Context, method, path string, body any) ([]byte, } // Set headers - req.Header.Set("Authorization", c.AuthHeader) + if c.AuthHeader != "" { + req.Header.Set("Authorization", c.AuthHeader) + } req.Header.Set("Accept", "application/json") req.Header.Set("Content-Type", "application/json") diff --git a/shared/client/client_test.go b/shared/client/client_test.go index 4dc604e7..fabacb2b 100644 --- a/shared/client/client_test.go +++ b/shared/client/client_test.go @@ -555,6 +555,27 @@ func TestNew_AuthHeaderOverride(t *testing.T) { } }) + t.Run("SkipAuthHeader sends no Authorization header", func(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if auth := r.Header.Get("Authorization"); auth != "" { + t.Errorf("Authorization = %v, want empty", auth) + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{}`)) + })) + defer server.Close() + + c := New(server.URL, "", "", &Options{SkipAuthHeader: true}) + if c.AuthHeader != "" { + t.Errorf("AuthHeader = %v, want empty", c.AuthHeader) + } + + if _, err := c.Get(context.Background(), "/api/test"); err != nil { + t.Fatalf("Get() error = %v", err) + } + }) + t.Run("nil options uses Basic auth", func(t *testing.T) { t.Parallel() c := New("https://example.atlassian.net", "user@example.com", "token", nil) @@ -565,6 +586,24 @@ func TestNew_AuthHeaderOverride(t *testing.T) { }) } +func TestGatewayBaseURLFromEnv(t *testing.T) { + t.Setenv("JIRA_GATEWAY_BASE_URL", "") + t.Setenv("ATLASSIAN_GATEWAY_BASE_URL", "") + if got := GatewayBaseURLFromEnv("JIRA_GATEWAY_BASE_URL"); got != GatewayBaseURL { + t.Fatalf("default gateway = %q, want %q", got, GatewayBaseURL) + } + + t.Setenv("ATLASSIAN_GATEWAY_BASE_URL", "https://shared.example/") + if got := GatewayBaseURLFromEnv("JIRA_GATEWAY_BASE_URL"); got != "https://shared.example" { + t.Fatalf("shared gateway = %q", got) + } + + t.Setenv("JIRA_GATEWAY_BASE_URL", "https://jira.example/") + if got := GatewayBaseURLFromEnv("JIRA_GATEWAY_BASE_URL"); got != "https://jira.example" { + t.Fatalf("tool gateway = %q", got) + } +} + func TestOptions_timeoutOrDefault(t *testing.T) { t.Parallel() t.Run("nil options", func(t *testing.T) { diff --git a/shared/client/options.go b/shared/client/options.go index e28ec8c4..c988396d 100644 --- a/shared/client/options.go +++ b/shared/client/options.go @@ -2,6 +2,8 @@ package client import ( "io" + "os" + "strings" "time" ) @@ -12,6 +14,21 @@ const DefaultTimeout = 60 * time.Second // with scoped API tokens (service accounts). const GatewayBaseURL = "https://api.atlassian.com" +// GatewayBaseURLFromEnv returns a gateway base URL using tool-specific +// precedence, then the shared ATLASSIAN_GATEWAY_BASE_URL override, then +// the Atlassian Cloud gateway default. +func GatewayBaseURLFromEnv(primaryEnv string) string { + for _, name := range []string{primaryEnv, "ATLASSIAN_GATEWAY_BASE_URL"} { + if name == "" { + continue + } + if v := strings.TrimRight(strings.TrimSpace(os.Getenv(name)), "/"); v != "" { + return v + } + } + return GatewayBaseURL +} + // Options configures client behavior. type Options struct { // Timeout for HTTP requests. Defaults to 60 seconds if not set. @@ -27,6 +44,10 @@ type Options struct { // Use auth.BearerAuthHeader() for service accounts with scoped tokens. // When empty, New() computes BasicAuthHeader(email, apiToken) as before. AuthHeader string + + // SkipAuthHeader suppresses the Authorization header entirely. + // Use this for proxy auth, where a trusted proxy injects credentials. + SkipAuthHeader bool } // timeoutOrDefault returns the configured timeout or the default. diff --git a/shared/credstore/conndivergence_test.go b/shared/credstore/conndivergence_test.go index 228528ec..d1cd4e6b 100644 --- a/shared/credstore/conndivergence_test.go +++ b/shared/credstore/conndivergence_test.go @@ -123,6 +123,18 @@ func TestDetectConnDivergence(t *testing.T) { testutil.Equal(t, basic, got.AuthMethod) }) + t.Run("explicit proxy is preserved without email", func(t *testing.T) { + t.Parallel() + got, conf := DetectConnDivergence([]NamedConn{ + nc("shared config", "default", "/c.yml", ConnProfile{ + URL: "http://127.0.0.1:8080/atlassian", AuthMethod: "proxy", + }), + }) + testutil.Equal(t, 0, len(conf)) + testutil.Equal(t, "proxy", got.AuthMethod) + testutil.Equal(t, "http://127.0.0.1:8080/atlassian", got.URL) + }) + t.Run("all-empty source ignored", func(t *testing.T) { t.Parallel() got, conf := DetectConnDivergence([]NamedConn{ diff --git a/shared/credstore/credstore.go b/shared/credstore/credstore.go index 0c3fae48..f80d8b0f 100644 --- a/shared/credstore/credstore.go +++ b/shared/credstore/credstore.go @@ -199,8 +199,9 @@ func (s *Store) ResolveWithSource(_, field string) (string, Source) { // complete enough to authenticate once a token is supplied. The api_token // is no longer part of this store (it lives in the keyring), so callers // must compose this with keyring.HasToken for full readiness. Basic -// requires url + email; bearer requires url + cloud_id. Empty auth_method -// defaults to basic, matching the rest of the codebase. +// requires url + email; bearer requires url + cloud_id; proxy requires +// only url because authentication is delegated to the proxy. Empty +// auth_method defaults to basic, matching the rest of the codebase. func (s *Store) HasUsableConfig(tool string) bool { r := s.Resolve(tool) method := r.AuthMethod @@ -210,6 +211,8 @@ func (s *Store) HasUsableConfig(tool string) bool { switch method { case auth.AuthMethodBearer: return r.URL != "" && r.CloudID != "" + case auth.AuthMethodProxy: + return r.URL != "" case auth.AuthMethodBasic: return r.URL != "" && r.Email != "" default: diff --git a/shared/credstore/credstore_test.go b/shared/credstore/credstore_test.go index 87aecbea..4d089037 100644 --- a/shared/credstore/credstore_test.go +++ b/shared/credstore/credstore_test.go @@ -224,6 +224,22 @@ func TestHasUsableConfig(t *testing.T) { tool: ToolJTK, want: true, }, + { + name: "proxy needs only URL", + s: &Store{Default: Section{ + URL: "http://127.0.0.1:8080/atlassian", AuthMethod: auth.AuthMethodProxy, + }}, + tool: ToolCFL, + want: true, + }, + { + name: "proxy missing URL", + s: &Store{Default: Section{ + AuthMethod: auth.AuthMethodProxy, + }}, + tool: ToolJTK, + want: false, + }, { name: "empty store", s: &Store{}, diff --git a/shared/url/url.go b/shared/url/url.go index 48545c86..94b93953 100644 --- a/shared/url/url.go +++ b/shared/url/url.go @@ -1,7 +1,11 @@ // Package url provides URL normalization utilities for Atlassian CLI tools. package url -import "strings" +import ( + "net/netip" + neturl "net/url" + "strings" +) // NormalizeURL ensures the URL has an https scheme and no trailing slashes. // If the URL is empty, it returns an empty string. @@ -36,3 +40,19 @@ func HasScheme(u string) bool { func TrimTrailingSlashes(u string) string { return strings.TrimRight(u, "/") } + +// IsLoopbackHTTP reports whether u is an http:// URL whose host is localhost +// or a loopback IP address. It is intentionally narrow so CLIs can allow local +// development/proxy endpoints without allowing arbitrary cleartext URLs. +func IsLoopbackHTTP(u string) bool { + parsed, err := neturl.Parse(u) + if err != nil || parsed.Scheme != "http" || parsed.Host == "" { + return false + } + host := strings.ToLower(parsed.Hostname()) + if host == "localhost" { + return true + } + addr, err := netip.ParseAddr(host) + return err == nil && addr.IsLoopback() +} diff --git a/shared/url/url_test.go b/shared/url/url_test.go index 76ad91d5..35645241 100644 --- a/shared/url/url_test.go +++ b/shared/url/url_test.go @@ -115,3 +115,31 @@ func TestTrimTrailingSlashes(t *testing.T) { }) } } + +func TestIsLoopbackHTTP(t *testing.T) { + t.Parallel() + tests := []struct { + input string + want bool + }{ + {"http://localhost:8080/atlassian", true}, + {"http://LOCALHOST/atlassian", true}, + {"http://127.0.0.1:8080/atlassian", true}, + {"http://127.42.0.1:8080/atlassian", true}, + {"http://[::1]:8080/atlassian", true}, + {"https://localhost:8080/atlassian", false}, + {"http://example.com/atlassian", false}, + {"http://10.0.0.1/atlassian", false}, + {"localhost:8080/atlassian", false}, + {"", false}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + t.Parallel() + if got := IsLoopbackHTTP(tt.input); got != tt.want { + t.Errorf("IsLoopbackHTTP(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} diff --git a/tools/cfl/README.md b/tools/cfl/README.md index a8d96cf0..25b294d1 100644 --- a/tools/cfl/README.md +++ b/tools/cfl/README.md @@ -209,18 +209,25 @@ cfl init --url https://mycompany.atlassian.net --email you@example.com # Service account with scoped token (Bearer Auth) cfl init --auth-method bearer cfl init --auth-method bearer --url https://mycompany.atlassian.net \ - --token YOUR_SCOPED_TOKEN --cloud-id YOUR_CLOUD_ID --no-verify + --token-from-env CFL_API_TOKEN --cloud-id YOUR_CLOUD_ID --no-verify + +# Trusted proxy (no CLI-side Authorization header) +cfl init --auth-method proxy --url http://127.0.0.1:8080/atlassian --no-verify ``` | Flag | Short | Default | Description | |------|-------|---------|-------------| | `--url` | | | Pre-populate Confluence URL | | `--email` | | | Pre-populate email address | -| `--auth-method` | | | Auth method: `basic` (default) or `bearer` | +| `--token-stdin` | | `false` | Read the API token from stdin | +| `--token-from-env` | | | Read the API token from an environment variable | +| `--auth-method` | | | Auth method: `basic` (default), `bearer`, or `proxy` | | `--cloud-id` | | | Cloud ID for bearer auth (find at `https://your-site.atlassian.net/_edge/tenant_info`) | | `--no-verify` | | `false` | Skip connection verification | > **Bearer Auth:** For [Atlassian service accounts](https://support.atlassian.com/user-management/docs/manage-api-tokens-for-service-accounts/) with scoped API tokens. Email is not required. Requests route through the `api.atlassian.com` gateway. +> +> **Proxy Auth:** Use `--auth-method proxy` for a trusted proxy that injects credentials upstream. The CLI requires only a URL and sends no `Authorization` header. HTTPS URLs are accepted; `http://` URLs are accepted only for loopback hosts such as `localhost`, `127.0.0.1`, or `[::1]`. cfl appends `/wiki` for Confluence API calls. After a successful save, `cfl init` prints the equivalent of `cfl me` so you can confirm which user the saved credentials authenticate as. (Skipped when `--no-verify` is set, since no live API call is made and there is no user to render.) @@ -823,7 +830,7 @@ shared store at `~/.config/atlassian-cli/config.yml`: default: url: https://mycompany.atlassian.net # base URL; cfl appends /wiki on read email: you@example.com - auth_method: basic # or "bearer" + auth_method: basic # or "bearer" / "proxy" cloud_id: "" # required for bearer cfl: default_space: DEV # cfl-only defaults @@ -885,9 +892,12 @@ Environment variables override file-based config. Variables are checked in order | Default Space | `CFL_DEFAULT_SPACE` → shared `cfl.default_space` → legacy | | Auth Method | `CFL_AUTH_METHOD` → `ATLASSIAN_AUTH_METHOD` → shared `default` → legacy → `basic` | | Cloud ID | `CFL_CLOUD_ID` → `ATLASSIAN_CLOUD_ID` → shared `default` → legacy | +| Bearer Gateway Base URL | `CFL_GATEWAY_BASE_URL` → `ATLASSIAN_GATEWAY_BASE_URL` → `https://api.atlassian.com` | Per §2.2 connection config is single-sourced from the shared `default` section — per-tool `cfl:`/`jtk:` sections carry only non-secret defaults and may not override `url`/`email`/`auth_method`/`cloud_id`. +Set `ATLASSIAN_AUTH_METHOD=proxy` (or `CFL_AUTH_METHOD=proxy`) with `ATLASSIAN_URL`/`CFL_URL` to use a trusted proxy. Proxy auth ignores email, token, and cloud ID because the proxy is responsible for upstream authentication. + **Shared credentials:** If you use both `cfl` and `jtk` (Jira CLI), set `ATLASSIAN_*` variables once: ```bash diff --git a/tools/cfl/api/client.go b/tools/cfl/api/client.go index bc523c0d..2aa61652 100644 --- a/tools/cfl/api/client.go +++ b/tools/cfl/api/client.go @@ -11,6 +11,7 @@ import ( "github.com/open-cli-collective/atlassian-go/auth" "github.com/open-cli-collective/atlassian-go/client" + sharedurl "github.com/open-cli-collective/atlassian-go/url" ) // Validation errors for bearer auth. @@ -32,6 +33,14 @@ func NewClient(baseURL, email, apiToken string) *Client { } } +// NewProxyClient creates a Confluence client for a trusted proxy that injects +// authentication upstream. No Authorization header is sent by the CLI. +func NewProxyClient(baseURL string) *Client { + return &Client{ + Client: client.New(normalizeWikiBaseURL(baseURL), "", "", &client.Options{SkipAuthHeader: true}), + } +} + // NewBearerClient creates a new Confluence API client using bearer auth via the API gateway. // The cloudID is used to construct the gateway URL: https://api.atlassian.com/ex/confluence/{cloudId}/wiki func NewBearerClient(apiToken, cloudID string) (*Client, error) { @@ -41,7 +50,7 @@ func NewBearerClient(apiToken, cloudID string) (*Client, error) { if cloudID == "" { return nil, ErrCloudIDRequired } - gatewayBase := fmt.Sprintf("%s/ex/confluence/%s/wiki", client.GatewayBaseURL, cloudID) + gatewayBase := fmt.Sprintf("%s/ex/confluence/%s/wiki", client.GatewayBaseURLFromEnv("CFL_GATEWAY_BASE_URL"), cloudID) opts := &client.Options{ AuthHeader: auth.BearerAuthHeader(apiToken), } @@ -50,6 +59,14 @@ func NewBearerClient(apiToken, cloudID string) (*Client, error) { }, nil } +func normalizeWikiBaseURL(baseURL string) string { + baseURL = sharedurl.NormalizeURL(baseURL) + if !strings.HasSuffix(baseURL, "/wiki") { + baseURL += "/wiki" + } + return baseURL +} + // GetHTTPClient returns the underlying HTTP client for custom requests. func (c *Client) GetHTTPClient() *http.Client { return c.HTTPClient diff --git a/tools/cfl/api/client_test.go b/tools/cfl/api/client_test.go index 9e9fd9bb..38874d1a 100644 --- a/tools/cfl/api/client_test.go +++ b/tools/cfl/api/client_test.go @@ -23,6 +23,27 @@ func TestNewClient(t *testing.T) { testutil.Contains(t, client.GetAuthHeader(), "Basic ") } +func TestNewProxyClient(t *testing.T) { + t.Parallel() + var capturedAuth string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedAuth = r.Header.Get("Authorization") + testutil.Equal(t, "/wiki/api/v2/spaces", r.URL.Path) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{}`)) + })) + defer server.Close() + + client := NewProxyClient(server.URL) + testutil.NotNil(t, client) + testutil.Equal(t, server.URL+"/wiki", client.GetBaseURL()) + testutil.Equal(t, "", client.GetAuthHeader()) + + _, err := client.Get(context.Background(), "/api/v2/spaces") + testutil.RequireNoError(t, err) + testutil.Equal(t, "", capturedAuth) +} + func TestClient_AuthHeader(t *testing.T) { t.Parallel() var capturedAuth string @@ -243,3 +264,11 @@ func TestNewBearerClient(t *testing.T) { testutil.Equal(t, "https://example.atlassian.net/wiki", c.GetBaseURL()) }) } + +func TestNewBearerClient_GatewayBaseFromEnv(t *testing.T) { + t.Setenv("CFL_GATEWAY_BASE_URL", "https://gateway.example/") + t.Setenv("ATLASSIAN_GATEWAY_BASE_URL", "") + c, err := NewBearerClient("scoped-token", "abc-123") + testutil.RequireNoError(t, err) + testutil.Equal(t, "https://gateway.example/ex/confluence/abc-123/wiki", c.GetBaseURL()) +} diff --git a/tools/cfl/docs/development.md b/tools/cfl/docs/development.md index 96b0dfc2..e2297c97 100644 --- a/tools/cfl/docs/development.md +++ b/tools/cfl/docs/development.md @@ -84,7 +84,7 @@ Add new Confluence macros through `MacroRegistry` in `macro.go`; the tokenizer, `cfl` participates in the shared Atlassian credential/config model described by the monorepo guide. `ATLASSIAN_*` variables apply across both tools; `CFL_*` variables override for cfl. The cfl-specific config section carries non-secret defaults such as `default_space` and `output_format`. -Basic auth uses an instance URL plus email and token. Bearer auth routes through `api.atlassian.com` and requires a cloud ID. `cfl init` and `cfl me` verify against Confluence's current-user endpoint. +Basic auth uses an instance URL plus email and token. Bearer auth routes through `api.atlassian.com` and requires a cloud ID. Proxy auth uses only a URL, sends no `Authorization` header, and allows loopback `http://` URLs for trusted local proxies while keeping arbitrary cleartext proxy URLs rejected. Bearer gateway routing can be overridden with `CFL_GATEWAY_BASE_URL` or `ATLASSIAN_GATEWAY_BASE_URL`. `cfl init` and `cfl me` verify against Confluence's current-user endpoint. ## Output diff --git a/tools/cfl/integration-tests.md b/tools/cfl/integration-tests.md index f83bef89..123630b8 100644 --- a/tools/cfl/integration-tests.md +++ b/tools/cfl/integration-tests.md @@ -4,12 +4,13 @@ This document catalogs the manual integration test suite for `cfl`. These tests ## Auth Methods -cfl supports two authentication methods. The full integration test suite should be run with both: +cfl supports three authentication methods. The full integration test suite should be run with each configured auth method: - **Basic Auth** (default): Classic API tokens using `email:token` against the instance URL. - **Bearer Auth**: Scoped API tokens for service accounts using `Authorization: Bearer ` against the `api.atlassian.com` gateway. +- **Proxy Auth**: Trusted proxy setup using `--auth-method proxy`; the CLI sends no `Authorization` header and requires only a URL. -All cfl commands should work with both auth methods (no scope limitations for Confluence). +All cfl commands should work with all configured auth methods (no scope limitations for Confluence). --- @@ -25,6 +26,10 @@ All cfl commands should work with both auth methods (no scope limitations for Co - Your Cloud ID (find at `https://your-site.atlassian.net/_edge/tenant_info`) - `cfl init --auth-method bearer` completed +### Proxy Auth Prerequisites +- A trusted proxy that authenticates upstream and exposes Confluence `/wiki` paths. +- `cfl init --auth-method proxy --url ` completed. + ### Test Data Conventions - Test pages use `[Test]` prefix: `[Test] My Page` - Baseline pages (for comparison) use `[Baseline]` prefix @@ -574,7 +579,7 @@ Copies in the TEST space (originals from INT, CUS, PROD, PLAYBOOK): ## Test Execution Checklist -All cfl commands work with both auth methods (no scope restrictions for Confluence). Run the full checklist twice with separate passes to ensure both auth paths work. +All cfl commands work with all configured auth methods (no scope restrictions for Confluence). Run the full checklist with separate passes to ensure each auth path works. ### Pass 1: Basic Auth diff --git a/tools/cfl/internal/cmd/init/init.go b/tools/cfl/internal/cmd/init/init.go index c2b57b41..bf811834 100644 --- a/tools/cfl/internal/cmd/init/init.go +++ b/tools/cfl/internal/cmd/init/init.go @@ -23,13 +23,16 @@ import ( // clientBuilder constructs an *api.Client from a config. // Pulled out as a parameter so tests can inject an httptest-pointed client -// without depending on api.NewBearerClient's hardcoded gateway URL. +// without depending on api.NewBearerClient's gateway URL. type clientBuilder func(cfg *config.Config) (*api.Client, error) func defaultClientBuilder(cfg *config.Config) (*api.Client, error) { if cfg.AuthMethod == auth.AuthMethodBearer { return api.NewBearerClient(cfg.APIToken, cfg.CloudID) } + if cfg.AuthMethod == auth.AuthMethodProxy { + return api.NewProxyClient(cfg.URL), nil + } return api.NewClient(cfg.URL, cfg.Email, cfg.APIToken), nil } @@ -67,6 +70,10 @@ For service account scoped tokens (bearer auth): Use --auth-method bearer with your scoped API token and Cloud ID. Find your Cloud ID at: https://your-site.atlassian.net/_edge/tenant_info +For trusted local proxies: + Use --auth-method proxy with the proxy URL. The CLI sends no + Authorization header; the proxy is expected to authenticate upstream. + Scripted ingress (§1.5.1): use --token-stdin or --token-from-env VAR for the API token. cfl init has never had a --token flag because flag-passed plaintext secrets leak into shell history and process @@ -84,7 +91,10 @@ listings.`, # Service account (bearer auth) setup cfl init --auth-method bearer --url https://mycompany.atlassian.net \ - --token-from-env CFL_API_TOKEN --cloud-id YOUR_CLOUD_ID`, + --token-from-env CFL_API_TOKEN --cloud-id YOUR_CLOUD_ID + + # Local proxy setup + cfl init --auth-method proxy --url http://127.0.0.1:8080/atlassian --no-verify`, RunE: func(cmd *cobra.Command, _ []string) error { return runInit(cmd.Context(), opts, url, email, tokenStdin, tokenFromEnv, authMethod, cloudID, noVerify) }, @@ -94,7 +104,7 @@ listings.`, cmd.Flags().StringVar(&email, "email", "", "Your Atlassian account email") cmd.Flags().BoolVar(&tokenStdin, "token-stdin", false, "Read the API token from stdin (xor with --token-from-env)") cmd.Flags().StringVar(&tokenFromEnv, "token-from-env", "", "Read the API token from this env var (xor with --token-stdin)") - cmd.Flags().StringVar(&authMethod, "auth-method", "", "Authentication method: basic (default) or bearer") + cmd.Flags().StringVar(&authMethod, "auth-method", "", "Authentication method: basic (default), bearer, or proxy") cmd.Flags().StringVar(&cloudID, "cloud-id", "", "Atlassian Cloud ID (required for bearer auth)") cmd.Flags().BoolVar(&noVerify, "no-verify", false, "Skip connection verification") @@ -132,14 +142,17 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s return err } cfg := result.prefill - - // Now the one-time §1.8 token migration: relocate any pre-existing - // legacy plaintext token into the single shared keyring api_token - // (token-only, connection-preserving scrub) before the user sets a - // new one. - if err := keyring.EnsureMigrated(); err != nil { - v.Error("Could not prepare secure credential storage: %v", err) - return err + isProxy := cfg.AuthMethod == auth.AuthMethodProxy + + if !isProxy { + // Now the one-time §1.8 token migration: relocate any pre-existing + // legacy plaintext token into the single shared keyring api_token + // (token-only, connection-preserving scrub) before the user sets a + // new one. + if err := keyring.EnsureMigrated(); err != nil { + v.Error("Could not prepare secure credential storage: %v", err) + return err + } } // EnsureMigrated relocated any legacy plaintext token into the @@ -178,7 +191,7 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s cfg.APIToken = scripted } - if cfg.APIToken == "" { + if !isProxy && cfg.APIToken == "" { if tok, _, terr := keyring.ResolveTokenNoMigrate(credstore.ToolCFL); terr == nil { cfg.APIToken = tok } @@ -190,7 +203,28 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s // Build the form based on auth method var formGroups []*huh.Group - if isBearer { + if isProxy { + // Proxy auth: URL only. The proxy injects authentication upstream. + formGroups = append(formGroups, huh.NewGroup( + huh.NewInput(). + Title("Confluence Proxy URL"). + Description("Trusted proxy URL; loopback http or https"). + Placeholder("http://127.0.0.1:8080/atlassian"). + Value(&cfg.URL). + Validate(func(s string) error { + if s == "" { + return fmt.Errorf("URL is required") + } + return nil + }), + + huh.NewInput(). + Title("Default Space (optional)"). + Description("Default space key for page operations"). + Placeholder("MYSPACE"). + Value(&cfg.DefaultSpace), + )) + } else if isBearer { // Bearer auth: URL + token + cloud ID (no email) formGroups = append(formGroups, huh.NewGroup( huh.NewInput(). @@ -288,7 +322,7 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s // (via `cfl set-credential`). Fail loud naming the first missing // field. if !prompt.WantPrompt(opts.NonInteractive, opts.Stdin) { - if err := requireNonInteractiveFields(cfg, isBearer); err != nil { + if err := requireNonInteractiveFields(cfg); err != nil { return err } } else { @@ -299,6 +333,7 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s } cfg.NormalizeURL() + normalizeAuthConfig(cfg) if err := cfg.Validate(); err != nil { return fmt.Errorf("invalid configuration: %w", err) @@ -372,16 +407,19 @@ func finalizeInit( return fmt.Errorf("saving shared store: %w", err) } - // The token never lands in the plaintext store (Save strips it) — it - // goes to the OS keyring under the single shared api_token (§1.11.10: - // one key for both jtk and cfl; the reconcile write-target governs - // only NON-secret placement, untouched here). - if err := keyring.PersistToken(cfg.APIToken); err != nil { - v.Error("Saved the non-secret config to %s, but could not store the API token in the keyring: %v", sharedPath, err) - v.Error("Recover by storing just the token (no need to re-run init): `cfl set-credential --ref atlassian-cli/default --key api_token --stdin --overwrite` (reads stdin; use --from-env VAR for env-driven setup).") - return err + // Direct auth tokens never land in the plaintext store (Save strips them). + // Proxy auth has no CLI-side token; basic/bearer tokens go to the OS + // keyring under the single shared api_token (§1.11.10). + if cfg.AuthMethod == auth.AuthMethodProxy { + v.Success("Configuration saved to %s (proxy auth; no token stored)", sharedPath) + } else { + if err := keyring.PersistToken(cfg.APIToken); err != nil { + v.Error("Saved the non-secret config to %s, but could not store the API token in the keyring: %v", sharedPath, err) + v.Error("Recover by storing just the token (no need to re-run init): `cfl set-credential --ref atlassian-cli/default --key api_token --stdin --overwrite` (reads stdin; use --from-env VAR for env-driven setup).") + return err + } + v.Success("Configuration saved to %s (token stored in the OS keyring)", sharedPath) } - v.Success("Configuration saved to %s (token stored in the OS keyring)", sharedPath) // Optional: clean up legacy files we just migrated. for _, lp := range result.consumedLegacies { @@ -428,24 +466,41 @@ func finalizeInit( if cfg.AuthMethod == auth.AuthMethodBearer { v.Println("") v.Info("To switch back to basic auth later, run: cfl init --auth-method basic") + } else if cfg.AuthMethod == auth.AuthMethodProxy { + v.Println("") + v.Info("To switch to direct authentication later, run: cfl init --auth-method basic") } return nil } +func normalizeAuthConfig(cfg *config.Config) { + if cfg.AuthMethod == "" { + cfg.AuthMethod = auth.AuthMethodBasic + } + if cfg.AuthMethod == auth.AuthMethodProxy { + cfg.Email = "" + cfg.APIToken = "" + cfg.CloudID = "" + } +} + // requireNonInteractiveFields enforces the §3.4 fail-loud contract for // scripted/CI runs of `cfl init`. cfl init has no --token flag, so the // token MUST come from a pre-staged keyring entry via cfl set-credential; // the error names that path explicitly. -func requireNonInteractiveFields(cfg *config.Config, isBearer bool) error { +func requireNonInteractiveFields(cfg *config.Config) error { if cfg.URL == "" { return fmt.Errorf("--non-interactive: missing required value for --url") } - if isBearer { + switch cfg.AuthMethod { + case auth.AuthMethodProxy: + return nil + case auth.AuthMethodBearer: if cfg.CloudID == "" { return fmt.Errorf("--non-interactive: missing required value for --cloud-id (bearer auth)") } - } else { + default: if cfg.Email == "" { return fmt.Errorf("--non-interactive: missing required value for --email (basic auth)") } diff --git a/tools/cfl/internal/cmd/init/init_test.go b/tools/cfl/internal/cmd/init/init_test.go index a6199b23..cd7a68c3 100644 --- a/tools/cfl/internal/cmd/init/init_test.go +++ b/tools/cfl/internal/cmd/init/init_test.go @@ -131,30 +131,28 @@ func TestRunInit_InvalidAuthMethod(t *testing.T) { func TestRequireNonInteractiveFields_NamesFirstMissing(t *testing.T) { t.Parallel() tests := []struct { - name string - cfg *config.Config - isBearer bool - wants []string + name string + cfg *config.Config + wants []string }{ - {"basic — missing URL", &config.Config{}, false, []string{"--url"}}, - {"basic — missing email", &config.Config{URL: "https://acme.atlassian.net"}, false, []string{"--email"}}, - {"bearer — missing cloud-id", &config.Config{URL: "https://acme.atlassian.net"}, true, []string{"--cloud-id"}}, + {"basic — missing URL", &config.Config{}, []string{"--url"}}, + {"basic — missing email", &config.Config{URL: "https://acme.atlassian.net"}, []string{"--email"}}, + {"bearer — missing cloud-id", &config.Config{URL: "https://acme.atlassian.net", AuthMethod: auth.AuthMethodBearer}, []string{"--cloud-id"}}, { name: "basic — missing token recommends --token-stdin + --token-from-env + set-credential", cfg: &config.Config{URL: "https://acme.atlassian.net", Email: "u@x.io"}, wants: []string{"--token-stdin", "--token-from-env", "set-credential"}, }, { - name: "bearer — missing token recommends --token-stdin + --token-from-env + set-credential", - cfg: &config.Config{URL: "https://acme.atlassian.net", CloudID: "cid"}, - isBearer: true, - wants: []string{"--token-stdin", "--token-from-env", "set-credential"}, + name: "bearer — missing token recommends --token-stdin + --token-from-env + set-credential", + cfg: &config.Config{URL: "https://acme.atlassian.net", CloudID: "cid", AuthMethod: auth.AuthMethodBearer}, + wants: []string{"--token-stdin", "--token-from-env", "set-credential"}, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - err := requireNonInteractiveFields(tc.cfg, tc.isBearer) + err := requireNonInteractiveFields(tc.cfg) testutil.RequireError(t, err) if !strings.Contains(err.Error(), "--non-interactive") { t.Fatalf("error must mention --non-interactive: %v", err) @@ -174,7 +172,18 @@ func TestRequireNonInteractiveFields_AllSupplied_NoError(t *testing.T) { URL: "https://acme.atlassian.net", Email: "u@x.io", APIToken: "tok-1234567890", } - if err := requireNonInteractiveFields(cfg, false); err != nil { + if err := requireNonInteractiveFields(cfg); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestRequireNonInteractiveFields_ProxyNeedsOnlyURL(t *testing.T) { + t.Parallel() + cfg := &config.Config{ + URL: "http://127.0.0.1:8080/atlassian", + AuthMethod: auth.AuthMethodProxy, + } + if err := requireNonInteractiveFields(cfg); err != nil { t.Fatalf("unexpected error: %v", err) } } @@ -250,6 +259,35 @@ func TestRunInit_UsesConfiguredPathForReconciliation(t *testing.T) { testutil.Equal(t, "https://configured.atlassian.net", store.Default.URL) } +func TestRunInit_Proxy_NoTokenRequiredOrPersisted(t *testing.T) { + credtest.Hermetic(t) + opts := &root.Options{ + Output: "table", + NoColor: true, + NonInteractive: true, + Stdin: strings.NewReader(""), + Stdout: &bytes.Buffer{}, + Stderr: &bytes.Buffer{}, + } + err := runInit(context.Background(), opts, + "http://127.0.0.1:8080/atlassian", "", false, "", auth.AuthMethodProxy, "", true) + testutil.RequireNoError(t, err) + + store, err := credstore.Load(credtest.SharedConfigPath(t)) + testutil.RequireNoError(t, err) + testutil.Equal(t, "http://127.0.0.1:8080/atlassian", store.Default.URL) + testutil.Equal(t, "", store.Default.Email) + testutil.Equal(t, auth.AuthMethodProxy, store.Default.AuthMethod) + testutil.Equal(t, "", store.Default.CloudID) + + s, err := keyring.OpenNoMigrate() + testutil.RequireNoError(t, err) + defer func() { _ = s.Close() }() + ok, err := s.HasToken(keyring.KeyAPIToken) + testutil.RequireNoError(t, err) + testutil.False(t, ok) +} + // finalizeInit tests use t.TempDir() for paths and an httptest-backed // clientBuilder so the user's real config is never touched and no real // network call is made. @@ -441,6 +479,48 @@ func TestDefaultClientBuilder(t *testing.T) { _, err := defaultClientBuilder(cfg) testutil.RequireError(t, err) }) + + t.Run("proxy constructs no-auth client", func(t *testing.T) { + t.Parallel() + cfg := &config.Config{ + URL: "http://127.0.0.1:8080/atlassian", + AuthMethod: auth.AuthMethodProxy, + } + c, err := defaultClientBuilder(cfg) + testutil.RequireNoError(t, err) + testutil.Equal(t, "http://127.0.0.1:8080/atlassian/wiki", c.BaseURL) + testutil.Equal(t, "", c.AuthHeader) + }) +} + +func TestFinalizeInit_ProxyNoTokenPersisted(t *testing.T) { + credtest.Hermetic(t) + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + opts := newFinalizeOpts() + cfg := &config.Config{ + URL: "http://127.0.0.1:8080/atlassian/wiki", + AuthMethod: auth.AuthMethodProxy, + DefaultSpace: "DEV", + } + + err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, true, defaultClientBuilder) + testutil.RequireNoError(t, err) + + loaded, err := credstore.Load(configPath) + testutil.RequireNoError(t, err) + testutil.Equal(t, "http://127.0.0.1:8080/atlassian", loaded.Default.URL) + testutil.Equal(t, "", loaded.Default.Email) + testutil.Equal(t, auth.AuthMethodProxy, loaded.Default.AuthMethod) + testutil.Equal(t, "", loaded.Default.CloudID) + testutil.Equal(t, "DEV", loaded.CFL.DefaultSpace) + + s, err := keyring.OpenNoMigrate() + testutil.RequireNoError(t, err) + defer func() { _ = s.Close() }() + ok, err := s.HasToken(keyring.KeyAPIToken) + testutil.RequireNoError(t, err) + testutil.False(t, ok) } func TestFinalizeInit_AuthFailure(t *testing.T) { diff --git a/tools/cfl/internal/cmd/root/root.go b/tools/cfl/internal/cmd/root/root.go index 2116da2e..dd3fc950 100644 --- a/tools/cfl/internal/cmd/root/root.go +++ b/tools/cfl/internal/cmd/root/root.go @@ -144,6 +144,9 @@ func (o *Options) APIClient() (*api.Client, error) { if cfg.AuthMethod == auth.AuthMethodBearer { return api.NewBearerClient(cfg.APIToken, cfg.CloudID) } + if cfg.AuthMethod == auth.AuthMethodProxy { + return api.NewProxyClient(cfg.URL), nil + } return api.NewClient(cfg.URL, cfg.Email, cfg.APIToken), nil } diff --git a/tools/cfl/internal/cmd/root/root_test.go b/tools/cfl/internal/cmd/root/root_test.go index dbc98226..4d78e584 100644 --- a/tools/cfl/internal/cmd/root/root_test.go +++ b/tools/cfl/internal/cmd/root/root_test.go @@ -3,6 +3,8 @@ package root import ( "bytes" "fmt" + "net/http" + "net/http/httptest" "path/filepath" "testing" @@ -140,6 +142,29 @@ func TestRegisterCommands(t *testing.T) { testutil.True(t, called) } +func TestOptions_APIClient_ProxySkipsAuthHeader(t *testing.T) { + var capturedAuth string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedAuth = r.Header.Get("Authorization") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{}`)) + })) + defer server.Close() + + _, opts := NewCmd() + opts.SetConfig(&config.Config{ + URL: server.URL, + AuthMethod: auth.AuthMethodProxy, + }) + c, err := opts.APIClient() + testutil.RequireNoError(t, err) + testutil.Equal(t, "", c.GetAuthHeader()) + + _, err = c.Get(t.Context(), "/api/v2/spaces") + testutil.RequireNoError(t, err) + testutil.Equal(t, "", capturedAuth) +} + func TestValidateOutputFormat(t *testing.T) { t.Parallel() diff --git a/tools/cfl/internal/config/config.go b/tools/cfl/internal/config/config.go index 2c35f9bf..b33569a1 100644 --- a/tools/cfl/internal/config/config.go +++ b/tools/cfl/internal/config/config.go @@ -13,6 +13,7 @@ import ( sharedconfig "github.com/open-cli-collective/atlassian-go/config" "github.com/open-cli-collective/atlassian-go/credstore" "github.com/open-cli-collective/atlassian-go/keyring" + sharedurl "github.com/open-cli-collective/atlassian-go/url" "gopkg.in/yaml.v3" ) @@ -23,7 +24,7 @@ type Config struct { APIToken string `yaml:"api_token"` DefaultSpace string `yaml:"default_space,omitempty"` OutputFormat string `yaml:"output_format,omitempty"` - AuthMethod string `yaml:"auth_method,omitempty"` // "basic" (default) or "bearer" + AuthMethod string `yaml:"auth_method,omitempty"` // "basic" (default), "bearer", or "proxy" CloudID string `yaml:"cloud_id,omitempty"` // Required for bearer auth (gateway URL) Keyring KeyringConfig `yaml:"keyring,omitempty"` } @@ -41,13 +42,11 @@ type KeyringConfig struct { // Validate checks that all required fields are present and valid. // For bearer auth: URL + API token + Cloud ID are required (no email). // For basic auth: URL + email + API token are required. +// For proxy auth: only URL is required; no Authorization header is sent. func (c *Config) Validate() error { if c.URL == "" { return errors.New("url is required") } - if c.APIToken == "" { - return errors.New("api_token is required") - } // Validate auth method if set (empty defaults to basic) if c.AuthMethod != "" { @@ -56,21 +55,33 @@ func (c *Config) Validate() error { } } - if c.AuthMethod == auth.AuthMethodBearer { + // Validate URL scheme. Proxy auth may use loopback http for a local proxy; + // all other cleartext URLs are rejected. + if !strings.HasPrefix(c.URL, "https://") { + if c.AuthMethod != auth.AuthMethodProxy || !sharedurl.IsLoopbackHTTP(c.URL) { + return errors.New("url must use https unless proxy auth uses loopback http") + } + } + + switch c.AuthMethod { + case auth.AuthMethodProxy: + return nil + case auth.AuthMethodBearer: + if c.APIToken == "" { + return errors.New("api_token is required") + } if c.CloudID == "" { return errors.New("cloud_id is required for bearer auth") } - } else { + default: + if c.APIToken == "" { + return errors.New("api_token is required") + } if c.Email == "" { return errors.New("email is required") } } - // Validate URL scheme - if !strings.HasPrefix(c.URL, "https://") { - return errors.New("url must use https") - } - return nil } @@ -280,6 +291,11 @@ func LoadWithEnv(path string, resolveToken, loadShared bool) (*Config, error) { // ResolveToken applies the authoritative token source to cfg. func ResolveToken(cfg *Config) error { + if cfg.AuthMethod == auth.AuthMethodProxy { + cfg.APIToken = "" + return nil + } + // Authoritative token resolution: overwrites any token a legacy-file // parse may have populated, so plaintext can never reach the client. tok, _, kErr := keyring.ResolveToken(credstore.ToolCFL) diff --git a/tools/cfl/internal/config/config_test.go b/tools/cfl/internal/config/config_test.go index 9531b2d5..1f692278 100644 --- a/tools/cfl/internal/config/config_test.go +++ b/tools/cfl/internal/config/config_test.go @@ -66,6 +66,31 @@ func TestConfig_Validate(t *testing.T) { wantErr: true, errMsg: "url must use https", }, + { + name: "proxy with loopback http URL needs no token or email", + config: Config{ + URL: "http://127.0.0.1:8080/atlassian/wiki", + AuthMethod: "proxy", + }, + wantErr: false, + }, + { + name: "proxy with https URL needs no token or email", + config: Config{ + URL: "https://proxy.example.com/atlassian/wiki", + AuthMethod: "proxy", + }, + wantErr: false, + }, + { + name: "proxy rejects arbitrary http URL", + config: Config{ + URL: "http://example.com/atlassian/wiki", + AuthMethod: "proxy", + }, + wantErr: true, + errMsg: "url must use https", + }, { name: "valid bearer config", config: Config{ @@ -592,3 +617,16 @@ func TestLoadWithEnv_CorruptSharedFallsBackToLegacy(t *testing.T) { // but the command still works (no error). testutil.Equal(t, "", cfg.APIToken) } + +func TestLoadWithEnv_ProxySkipsTokenResolution(t *testing.T) { + credtest.Hermetic(t) + t.Setenv("ATLASSIAN_URL", "http://127.0.0.1:8080/atlassian") + t.Setenv("ATLASSIAN_AUTH_METHOD", "proxy") + t.Setenv("ATLASSIAN_API_TOKEN", "env-token-that-should-be-ignored") + + cfg, err := LoadWithEnv(filepath.Join(t.TempDir(), "missing.yml")) + testutil.RequireNoError(t, err) + testutil.Equal(t, "http://127.0.0.1:8080/atlassian", cfg.URL) + testutil.Equal(t, "proxy", cfg.AuthMethod) + testutil.Equal(t, "", cfg.APIToken) +} diff --git a/tools/jtk/README.md b/tools/jtk/README.md index 2924ce51..26582118 100644 --- a/tools/jtk/README.md +++ b/tools/jtk/README.md @@ -175,21 +175,28 @@ jtk init --url https://mycompany.atlassian.net --email user@example.com # Service account with scoped token (Bearer Auth) jtk init --auth-method bearer jtk init --auth-method bearer --url https://mycompany.atlassian.net \ - --token YOUR_SCOPED_TOKEN --cloud-id YOUR_CLOUD_ID --no-verify + --token-from-env JIRA_API_TOKEN --cloud-id YOUR_CLOUD_ID --no-verify + +# Trusted proxy (no CLI-side Authorization header) +jtk init --auth-method proxy --url http://127.0.0.1:8080/atlassian --no-verify ``` | Flag | Default | Description | |------|---------|-------------| | `--url` | | Jira URL (e.g., `https://mycompany.atlassian.net`) | | `--email` | | Email address for authentication | -| `--token` | | API token | -| `--auth-method` | | Auth method: `basic` (default) or `bearer` | +| `--token` | | Deprecated API token literal; prefer `--token-stdin` or `--token-from-env` | +| `--token-stdin` | `false` | Read the API token from stdin | +| `--token-from-env` | | Read the API token from an environment variable | +| `--auth-method` | | Auth method: `basic` (default), `bearer`, or `proxy` | | `--cloud-id` | | Cloud ID for bearer auth (find at `https://your-site.atlassian.net/_edge/tenant_info`) | | `--no-verify` | `false` | Skip connection verification | > **Bearer Auth:** For [Atlassian service accounts](https://support.atlassian.com/user-management/docs/manage-api-tokens-for-service-accounts/) with scoped API tokens. Email is not required. Requests route through the `api.atlassian.com` gateway. > > **Scope limitations:** Scoped tokens don't have scopes for Agile (boards/sprints), Automation, Dashboards, or the private Development API. These commands are unavailable with bearer auth — this is an Atlassian platform limitation. +> +> **Proxy Auth:** Use `--auth-method proxy` for a trusted proxy that injects credentials upstream. The CLI requires only a URL and sends no `Authorization` header. HTTPS URLs are accepted; `http://` URLs are accepted only for loopback hosts such as `localhost`, `127.0.0.1`, or `[::1]`. --- @@ -1663,7 +1670,7 @@ shared store at `~/.config/atlassian-cli/config.yml`: default: url: https://mycompany.atlassian.net email: user@example.com - auth_method: basic # or "bearer" + auth_method: basic # or "bearer" / "proxy" cloud_id: "" # required for bearer jtk: default_project: MYPROJECT # jtk-only defaults @@ -1722,9 +1729,12 @@ Environment variables override file-based config. Variables are checked in order | Default Project | `JIRA_DEFAULT_PROJECT` → shared `jtk.default_project` → legacy | | Auth Method | `JIRA_AUTH_METHOD` → `ATLASSIAN_AUTH_METHOD` → shared `default` → legacy → `basic` | | Cloud ID | `JIRA_CLOUD_ID` → `ATLASSIAN_CLOUD_ID` → shared `default` → legacy | +| Bearer Gateway Base URL | `JIRA_GATEWAY_BASE_URL` → `ATLASSIAN_GATEWAY_BASE_URL` → `https://api.atlassian.com` | Per §2.2 connection config is single-sourced from the shared `default` section — per-tool `cfl:`/`jtk:` sections carry only non-secret defaults and may not override `url`/`email`/`auth_method`/`cloud_id`. +Set `ATLASSIAN_AUTH_METHOD=proxy` (or `JIRA_AUTH_METHOD=proxy`) with `ATLASSIAN_URL`/`JIRA_URL` to use a trusted proxy. Proxy auth ignores email, token, and cloud ID because the proxy is responsible for upstream authentication. + **Shared credentials:** If you use both `jtk` and `cfl` (Confluence CLI), set `ATLASSIAN_*` variables once: ```bash diff --git a/tools/jtk/api/client.go b/tools/jtk/api/client.go index 1808ba24..2a8eff6a 100644 --- a/tools/jtk/api/client.go +++ b/tools/jtk/api/client.go @@ -39,20 +39,18 @@ type ClientConfig struct { Email string APIToken string Verbose bool - AuthMethod string // "basic" (default) or "bearer" + AuthMethod string // "basic" (default), "bearer", or "proxy" CloudID string // Required for bearer auth (used to construct gateway URL) } // New creates a new Jira API client from config. // For bearer auth: URL + API token + Cloud ID are required (no email). // For basic auth: URL + email + API token are required. +// For proxy auth: only URL is required; no Authorization header is sent. func New(cfg ClientConfig) (*Client, error) { if cfg.URL == "" { return nil, ErrURLRequired } - if cfg.APIToken == "" { - return nil, ErrAPITokenRequired - } if cfg.AuthMethod != "" { if err := auth.ValidateAuthMethod(cfg.AuthMethod); err != nil { @@ -60,14 +58,20 @@ func New(cfg ClientConfig) (*Client, error) { } } - if cfg.AuthMethod == auth.AuthMethodBearer { + switch cfg.AuthMethod { + case auth.AuthMethodBearer: return newBearerClient(cfg) + case auth.AuthMethodProxy: + return newProxyClient(cfg) } // Basic auth (default) if cfg.Email == "" { return nil, ErrEmailRequired } + if cfg.APIToken == "" { + return nil, ErrAPITokenRequired + } // Normalize URL: ensure https and no trailing slash baseURL := url.NormalizeURL(cfg.URL) @@ -89,6 +93,9 @@ func New(cfg ClientConfig) (*Client, error) { // newBearerClient creates a client configured for bearer auth via the API gateway. func newBearerClient(cfg ClientConfig) (*Client, error) { + if cfg.APIToken == "" { + return nil, ErrAPITokenRequired + } if cfg.CloudID == "" { return nil, ErrCloudIDRequired } @@ -97,7 +104,7 @@ func newBearerClient(cfg ClientConfig) (*Client, error) { instanceURL := url.NormalizeURL(cfg.URL) // Gateway URLs for bearer auth - gatewayBase := fmt.Sprintf("%s/ex/jira/%s", client.GatewayBaseURL, cfg.CloudID) + gatewayBase := fmt.Sprintf("%s/ex/jira/%s", client.GatewayBaseURLFromEnv("JIRA_GATEWAY_BASE_URL"), cfg.CloudID) restURL := gatewayBase + "/rest/api/3" opts := &client.Options{ @@ -117,6 +124,28 @@ func newBearerClient(cfg ClientConfig) (*Client, error) { }, nil } +// newProxyClient creates a client configured for a trusted proxy that injects +// authentication upstream. No Authorization header is sent by the CLI. +func newProxyClient(cfg ClientConfig) (*Client, error) { + baseURL := url.NormalizeURL(cfg.URL) + if strings.HasPrefix(baseURL, "http://") && !url.IsLoopbackHTTP(baseURL) { + return nil, ErrProxyURLRequiresHTTPS + } + restURL := baseURL + "/rest/api/3" + + opts := &client.Options{SkipAuthHeader: true} + if cfg.Verbose { + opts.Verbose = true + } + + return &Client{ + Client: client.New(restURL, "", "", opts), + URL: baseURL, + BaseURL: restURL, + AgileURL: baseURL + "/rest/agile/1.0", + }, nil +} + // SupportsAgile returns true if the client can access the Agile REST API. // Bearer auth clients (service accounts with scoped tokens) cannot access // the Agile API because Atlassian does not provide an Agile scope. @@ -133,10 +162,11 @@ func (c *Client) IsBearerAuth() bool { // Validation errors var ( - ErrURLRequired = stderrors.New("URL is required") - ErrEmailRequired = stderrors.New("email is required") - ErrAPITokenRequired = stderrors.New("API token is required") - ErrCloudIDRequired = stderrors.New("cloud ID is required for bearer auth") + ErrURLRequired = stderrors.New("URL is required") + ErrEmailRequired = stderrors.New("email is required") + ErrAPITokenRequired = stderrors.New("API token is required") + ErrCloudIDRequired = stderrors.New("cloud ID is required for bearer auth") + ErrProxyURLRequiresHTTPS = stderrors.New("proxy auth URL must use https unless it is loopback http") ) // ErrAgileUnavailable is returned when a command requires the Agile API diff --git a/tools/jtk/api/client_test.go b/tools/jtk/api/client_test.go index 478747fb..1985cc73 100644 --- a/tools/jtk/api/client_test.go +++ b/tools/jtk/api/client_test.go @@ -424,6 +424,45 @@ func TestNew_BearerAuth(t *testing.T) { testutil.RequireNoError(t, err) }) + t.Run("proxy auth requires URL only and sends no auth header", func(t *testing.T) { + t.Parallel() + var capturedAuth string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedAuth = r.Header.Get("Authorization") + testutil.Equal(t, "/rest/api/3/myself", r.URL.Path) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{}`)) + })) + defer server.Close() + + c, err := New(ClientConfig{ + URL: server.URL, + AuthMethod: "proxy", + }) + testutil.RequireNoError(t, err) + testutil.Equal(t, server.URL+"/rest/api/3", c.BaseURL) + testutil.Equal(t, "", c.GetAuthHeader()) + testutil.True(t, c.SupportsAgile()) + testutil.False(t, c.IsBearerAuth()) + + _, err = c.Get(context.Background(), "/myself") + testutil.RequireNoError(t, err) + testutil.Equal(t, "", capturedAuth) + }) + + t.Run("proxy auth rejects arbitrary http URLs", func(t *testing.T) { + t.Parallel() + c, err := New(ClientConfig{ + URL: "http://example.com", + AuthMethod: "proxy", + }) + testutil.Error(t, err) + testutil.Nil(t, c) + if !errors.Is(err, ErrProxyURLRequiresHTTPS) { + t.Errorf("got error %v, want %v", err, ErrProxyURLRequiresHTTPS) + } + }) + t.Run("basic auth unchanged when AuthMethod empty", func(t *testing.T) { t.Parallel() c, err := New(ClientConfig{ @@ -473,3 +512,16 @@ func TestNew_BearerAuth(t *testing.T) { } }) } + +func TestNew_BearerAuth_GatewayBaseFromEnv(t *testing.T) { + t.Setenv("JIRA_GATEWAY_BASE_URL", "https://gateway.example/") + t.Setenv("ATLASSIAN_GATEWAY_BASE_URL", "") + c, err := New(ClientConfig{ + URL: "https://example.atlassian.net", + APIToken: "scoped-token", + AuthMethod: "bearer", + CloudID: "abc-123", + }) + testutil.RequireNoError(t, err) + testutil.Equal(t, "https://gateway.example/ex/jira/abc-123/rest/api/3", c.BaseURL) +} diff --git a/tools/jtk/docs/development.md b/tools/jtk/docs/development.md index f9a3014f..a8167ebd 100644 --- a/tools/jtk/docs/development.md +++ b/tools/jtk/docs/development.md @@ -75,7 +75,7 @@ Read `internal/cmd/OUTPUT_SPEC.md` before changing default output, `--id`, `--fi `jtk` participates in the shared Atlassian credential/config model described by the monorepo guide. `ATLASSIAN_*` variables apply across both tools; `JIRA_*` variables override for jtk. The jtk-specific config section carries non-secret defaults such as `default_project`. -Basic auth uses an instance URL plus email and token. Bearer auth routes through `api.atlassian.com`, requires a cloud ID, and has Atlassian platform scope limitations for some Jira surfaces. +Basic auth uses an instance URL plus email and token. Bearer auth routes through `api.atlassian.com`, requires a cloud ID, and has Atlassian platform scope limitations for some Jira surfaces. Proxy auth uses only a URL, sends no `Authorization` header, and allows loopback `http://` URLs for trusted local proxies while keeping arbitrary cleartext proxy URLs rejected. Bearer gateway routing can be overridden with `JIRA_GATEWAY_BASE_URL` or `ATLASSIAN_GATEWAY_BASE_URL`. ## Testing Notes diff --git a/tools/jtk/integration-tests.md b/tools/jtk/integration-tests.md index a1be315f..df28d7dd 100644 --- a/tools/jtk/integration-tests.md +++ b/tools/jtk/integration-tests.md @@ -6,10 +6,11 @@ If a test reveals a bug, **record the bug and continue testing** rather than sto ## Auth Methods -jtk supports two authentication methods. The full integration test suite should be run with both: +jtk supports three authentication methods. The full integration test suite should be run with Basic Auth and Proxy Auth, and the bearer-compatible subset should be run with Bearer Auth: - **Basic Auth** (default): Classic API tokens using `email:token` against the instance URL. - **Bearer Auth**: Scoped API tokens for service accounts using `Authorization: Bearer ` against the `api.atlassian.com` gateway. +- **Proxy Auth**: Trusted proxy setup using `--auth-method proxy`; the CLI sends no `Authorization` header and requires only a URL. > **Scope limitations:** Scoped tokens don't have scopes for Agile (boards/sprints), Automation, or Dashboards. Sections 4 (Boards & Sprints), 6 (Dashboards), 8 (Automation), 13 (Dashboard Mutations), 14 (Automation Mutations), and 15 (Sprint Mutations) must be **skipped** when testing with Bearer Auth. Section 19 (Bearer Auth Guards) should be run **only** with Bearer Auth. @@ -30,6 +31,10 @@ jtk supports two authentication methods. The full integration test suite should - Your Cloud ID (find at `https://your-site.atlassian.net/_edge/tenant_info`) - `jtk init --auth-method bearer` completed +### Proxy Auth Prerequisites +- A trusted proxy that authenticates upstream and exposes Jira REST paths. +- `jtk init --auth-method proxy --url ` completed. + ### Build ```bash diff --git a/tools/jtk/internal/cmd/initcmd/initcmd.go b/tools/jtk/internal/cmd/initcmd/initcmd.go index 4cfa6689..1a145686 100644 --- a/tools/jtk/internal/cmd/initcmd/initcmd.go +++ b/tools/jtk/internal/cmd/initcmd/initcmd.go @@ -47,6 +47,10 @@ For service account scoped tokens (bearer auth): Use --auth-method bearer with your scoped API token and Cloud ID. Find your Cloud ID at: https://your-site.atlassian.net/_edge/tenant_info +For trusted local proxies: + Use --auth-method proxy with the proxy URL. The CLI sends no + Authorization header; the proxy is expected to authenticate upstream. + Scripted ingress (§1.5.1): use --token-stdin or --token-from-env VAR for the API token. The legacy --token flag is deprecated — it leaks the secret into shell history and process listings — and will be removed @@ -66,6 +70,9 @@ in a future release.`, jtk init --auth-method bearer --url https://mycompany.atlassian.net \ --token-from-env JIRA_API_TOKEN --cloud-id YOUR_CLOUD_ID + # Local proxy setup + jtk init --auth-method proxy --url http://127.0.0.1:8080/atlassian --no-verify + # Skip connection verification jtk init --no-verify`, RunE: func(cmd *cobra.Command, _ []string) error { @@ -78,7 +85,7 @@ in a future release.`, cmd.Flags().StringVar(&token, "token", "", "DEPRECATED: API token literal (use --token-stdin or --token-from-env; §1.5.1)") cmd.Flags().BoolVar(&tokenStdin, "token-stdin", false, "Read the API token from stdin (xor with --token-from-env)") cmd.Flags().StringVar(&tokenFromEnv, "token-from-env", "", "Read the API token from this env var (xor with --token-stdin)") - cmd.Flags().StringVar(&authMethod, "auth-method", "", "Authentication method: basic (default) or bearer") + cmd.Flags().StringVar(&authMethod, "auth-method", "", "Authentication method: basic (default), bearer, or proxy") cmd.Flags().StringVar(&cloudID, "cloud-id", "", "Atlassian Cloud ID (required for bearer auth)") cmd.Flags().BoolVar(&noVerify, "no-verify", false, "Skip connection verification") @@ -155,22 +162,25 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, return err } cfg := result.prefill + isProxy := cfg.AuthMethod == auth.AuthMethodProxy - // Now the one-time §1.8 token migration (token-only, - // connection-preserving scrub). - if err := keyring.EnsureMigrated(); err != nil { - v.Error("Could not prepare secure credential storage: %v", err) - return err - } + if !isProxy { + // Now the one-time §1.8 token migration (token-only, + // connection-preserving scrub). + if err := keyring.EnsureMigrated(); err != nil { + v.Error("Could not prepare secure credential storage: %v", err) + return err + } - // EnsureMigrated relocated any legacy plaintext token into the - // keyring and scrubbed it from disk, so prefill.APIToken is empty - // even though the token still exists. Backfill from the keyring so a - // returning user isn't forced to re-enter a just-migrated token. - // NoMigrate: migration already ran. Value stays password-masked. - if cfg.APIToken == "" { - if tok, _, terr := keyring.ResolveTokenNoMigrate(credstore.ToolJTK); terr == nil { - cfg.APIToken = tok + // EnsureMigrated relocated any legacy plaintext token into the + // keyring and scrubbed it from disk, so prefill.APIToken is empty + // even though the token still exists. Backfill from the keyring so a + // returning user isn't forced to re-enter a just-migrated token. + // NoMigrate: migration already ran. Value stays password-masked. + if cfg.APIToken == "" { + if tok, _, terr := keyring.ResolveTokenNoMigrate(credstore.ToolJTK); terr == nil { + cfg.APIToken = tok + } } } @@ -180,7 +190,28 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, // Build the form based on auth method var formGroups []*huh.Group - if isBearer { + if isProxy { + // Proxy auth: URL only. The proxy injects authentication upstream. + formGroups = append(formGroups, huh.NewGroup( + huh.NewInput(). + Title("Jira Proxy URL"). + Description("Trusted proxy URL; loopback http or https"). + Placeholder("http://127.0.0.1:8080/atlassian"). + Value(&cfg.URL). + Validate(func(s string) error { + if s == "" { + return fmt.Errorf("URL is required") + } + return nil + }), + + huh.NewInput(). + Title("Default Project (optional)"). + Description("Default project key for commands"). + Placeholder("MYPROJ"). + Value(&cfg.DefaultProject), + )) + } else if isBearer { // Bearer auth: URL + token + cloud ID (no email) formGroups = append(formGroups, huh.NewGroup( huh.NewInput(). @@ -275,7 +306,7 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, // can't run — every required value must already be in cfg from the // flag prefills. Fail loud naming the first missing field. if !prompt.WantPrompt(opts.NonInteractive, opts.Stdin) { - if err := requireNonInteractiveFields(cfg, isBearer); err != nil { + if err := requireNonInteractiveFields(cfg); err != nil { return err } } else { @@ -287,22 +318,23 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, // Normalize URL cfg.URL = sharedurl.NormalizeURL(cfg.URL) + normalizeAuthConfig(cfg) + + client, err := api.New(api.ClientConfig{ + URL: cfg.URL, + Email: cfg.Email, + APIToken: cfg.APIToken, + AuthMethod: cfg.AuthMethod, + CloudID: cfg.CloudID, + }) + if err != nil { + return fmt.Errorf("creating client: %w", err) + } // Verify connection unless --no-verify if !noVerify { v.Println("Testing connection...") - client, err := api.New(api.ClientConfig{ - URL: cfg.URL, - Email: cfg.Email, - APIToken: cfg.APIToken, - AuthMethod: cfg.AuthMethod, - CloudID: cfg.CloudID, - }) - if err != nil { - return fmt.Errorf("creating client: %w", err) - } - user, err := client.GetCurrentUser(ctx, "") if err != nil { v.Error("Connection failed: %v", err) @@ -348,16 +380,19 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, return fmt.Errorf("saving shared store: %w", err) } - // The token never lands in the plaintext store (Save strips it) — it - // goes to the OS keyring under the single shared api_token (§1.11.10: - // one key for both jtk and cfl; the reconcile write-target governs - // only NON-secret placement, untouched here). - if err := keyring.PersistToken(cfg.APIToken); err != nil { - v.Error("Saved the non-secret config to %s, but could not store the API token in the keyring: %v", sharedPath, err) - v.Error("Recover by storing just the token (no need to re-run init): `jtk set-credential --ref atlassian-cli/default --key api_token --stdin --overwrite` (reads stdin; use --from-env VAR for env-driven setup).") - return err + // Direct auth tokens never land in the plaintext store (Save strips them). + // Proxy auth has no CLI-side token; basic/bearer tokens go to the OS + // keyring under the single shared api_token (§1.11.10). + if cfg.AuthMethod == auth.AuthMethodProxy { + v.Success("Configuration saved to %s (proxy auth; no token stored)", sharedPath) + } else { + if err := keyring.PersistToken(cfg.APIToken); err != nil { + v.Error("Saved the non-secret config to %s, but could not store the API token in the keyring: %v", sharedPath, err) + v.Error("Recover by storing just the token (no need to re-run init): `jtk set-credential --ref atlassian-cli/default --key api_token --stdin --overwrite` (reads stdin; use --from-env VAR for env-driven setup).") + return err + } + v.Success("Configuration saved to %s (token stored in the OS keyring)", sharedPath) } - v.Success("Configuration saved to %s (token stored in the OS keyring)", sharedPath) for _, lp := range result.consumedLegacies { if !prompt.WantPrompt(opts.NonInteractive, opts.Stdin) { @@ -394,25 +429,42 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, if isBearer { v.Println("") v.Info("To switch back to basic auth later, run: jtk init --auth-method basic") + } else if isProxy { + v.Println("") + v.Info("To switch to direct authentication later, run: jtk init --auth-method basic") } return nil } +func normalizeAuthConfig(cfg *config.Config) { + if cfg.AuthMethod == "" { + cfg.AuthMethod = auth.AuthMethodBasic + } + if cfg.AuthMethod == auth.AuthMethodProxy { + cfg.Email = "" + cfg.APIToken = "" + cfg.CloudID = "" + } +} + // requireNonInteractiveFields enforces the §3.4 fail-loud contract for // scripted/CI runs of `jtk init`: any required value missing from the // flag prefills (which already populated cfg) produces an error naming // the first missing field, with the auth-mode shape baked into the // message so the operator knows which flag set is required. -func requireNonInteractiveFields(cfg *config.Config, isBearer bool) error { +func requireNonInteractiveFields(cfg *config.Config) error { if cfg.URL == "" { return fmt.Errorf("--non-interactive: missing required value for --url") } - if isBearer { + switch cfg.AuthMethod { + case auth.AuthMethodProxy: + return nil + case auth.AuthMethodBearer: if cfg.CloudID == "" { return fmt.Errorf("--non-interactive: missing required value for --cloud-id (bearer auth)") } - } else { + default: if cfg.Email == "" { return fmt.Errorf("--non-interactive: missing required value for --email (basic auth)") } diff --git a/tools/jtk/internal/cmd/initcmd/initcmd_test.go b/tools/jtk/internal/cmd/initcmd/initcmd_test.go index 4c43b707..5e6573ea 100644 --- a/tools/jtk/internal/cmd/initcmd/initcmd_test.go +++ b/tools/jtk/internal/cmd/initcmd/initcmd_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/open-cli-collective/atlassian-go/auth" "github.com/open-cli-collective/atlassian-go/credstore" "github.com/open-cli-collective/atlassian-go/credtest" "github.com/open-cli-collective/atlassian-go/keyring" @@ -73,10 +74,9 @@ func TestRequireNonInteractiveFields_NamesFirstMissing(t *testing.T) { t.Parallel() tests := []struct { - name string - cfg *config.Config - isBearer bool - wants []string + name string + cfg *config.Config + wants []string }{ { name: "basic auth — missing URL", @@ -89,10 +89,9 @@ func TestRequireNonInteractiveFields_NamesFirstMissing(t *testing.T) { wants: []string{"--email"}, }, { - name: "bearer — missing cloud-id", - cfg: &config.Config{URL: "https://acme.atlassian.net"}, - isBearer: true, - wants: []string{"--cloud-id"}, + name: "bearer — missing cloud-id", + cfg: &config.Config{URL: "https://acme.atlassian.net", AuthMethod: auth.AuthMethodBearer}, + wants: []string{"--cloud-id"}, }, { name: "basic auth — missing token recommends --token-stdin + --token-from-env + set-credential", @@ -100,16 +99,15 @@ func TestRequireNonInteractiveFields_NamesFirstMissing(t *testing.T) { wants: []string{"--token-stdin", "--token-from-env", "set-credential"}, }, { - name: "bearer — missing token recommends --token-stdin + --token-from-env + set-credential", - cfg: &config.Config{URL: "https://acme.atlassian.net", CloudID: "cid"}, - isBearer: true, - wants: []string{"--token-stdin", "--token-from-env", "set-credential"}, + name: "bearer — missing token recommends --token-stdin + --token-from-env + set-credential", + cfg: &config.Config{URL: "https://acme.atlassian.net", CloudID: "cid", AuthMethod: auth.AuthMethodBearer}, + wants: []string{"--token-stdin", "--token-from-env", "set-credential"}, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - err := requireNonInteractiveFields(tc.cfg, tc.isBearer) + err := requireNonInteractiveFields(tc.cfg) testutil.RequireError(t, err) if !strings.Contains(err.Error(), "--non-interactive: missing") { t.Fatalf("error must mention --non-interactive prefix: %v", err) @@ -131,7 +129,18 @@ func TestRequireNonInteractiveFields_AllSupplied_NoError(t *testing.T) { URL: "https://acme.atlassian.net", Email: "u@x.io", APIToken: "tok-1234567890", } - if err := requireNonInteractiveFields(cfg, false); err != nil { + if err := requireNonInteractiveFields(cfg); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestRequireNonInteractiveFields_ProxyNeedsOnlyURL(t *testing.T) { + t.Parallel() + cfg := &config.Config{ + URL: "http://127.0.0.1:8080/atlassian", + AuthMethod: auth.AuthMethodProxy, + } + if err := requireNonInteractiveFields(cfg); err != nil { t.Fatalf("unexpected error: %v", err) } } @@ -181,6 +190,34 @@ func TestRunInit_NonInteractive_MissingToken_FlagAndKeyringEmpty(t *testing.T) { } } +func TestRunInit_Proxy_NoTokenRequiredOrPersisted(t *testing.T) { + credtest.Hermetic(t) + opts := &root.Options{ + NoColor: true, + NonInteractive: true, + Stdin: strings.NewReader(""), + Stdout: &bytes.Buffer{}, + Stderr: &bytes.Buffer{}, + } + err := runInit(context.Background(), opts, + "http://127.0.0.1:8080/atlassian", "", "", false, "", auth.AuthMethodProxy, "", true) + testutil.RequireNoError(t, err) + + store, err := credstore.Load(credtest.SharedConfigPath(t)) + testutil.RequireNoError(t, err) + testutil.Equal(t, "http://127.0.0.1:8080/atlassian", store.Default.URL) + testutil.Equal(t, "", store.Default.Email) + testutil.Equal(t, auth.AuthMethodProxy, store.Default.AuthMethod) + testutil.Equal(t, "", store.Default.CloudID) + + s, err := keyring.OpenNoMigrate() + testutil.RequireNoError(t, err) + defer func() { _ = s.Close() }() + ok, err := s.HasToken(keyring.KeyAPIToken) + testutil.RequireNoError(t, err) + testutil.False(t, ok) +} + func TestInitCommand_Flags(t *testing.T) { t.Parallel() rootCmd := &cobra.Command{Use: "jtk", Short: "Test CLI"} diff --git a/tools/jtk/internal/cmd/root/root.go b/tools/jtk/internal/cmd/root/root.go index f6af0ad4..2ff85c00 100644 --- a/tools/jtk/internal/cmd/root/root.go +++ b/tools/jtk/internal/cmd/root/root.go @@ -12,6 +12,7 @@ import ( cccredstore "github.com/open-cli-collective/cli-common/credstore" "github.com/open-cli-collective/atlassian-go/artifact" + "github.com/open-cli-collective/atlassian-go/auth" "github.com/open-cli-collective/atlassian-go/keyring" "github.com/open-cli-collective/atlassian-go/present" "github.com/open-cli-collective/atlassian-go/version" @@ -88,16 +89,21 @@ func (o *Options) APIClient() (*api.Client, error) { if o.cachedClient != nil { return o.cachedClient, nil } - token, err := config.ResolveAPIToken() - if err != nil { - return nil, err + authMethod := config.GetAuthMethod() + var token string + if authMethod != auth.AuthMethodProxy { + var err error + token, err = config.ResolveAPIToken() + if err != nil { + return nil, err + } } c, err := api.New(api.ClientConfig{ URL: config.GetURL(), Email: config.GetEmail(), APIToken: token, Verbose: o.Verbose, - AuthMethod: config.GetAuthMethod(), + AuthMethod: authMethod, CloudID: config.GetCloudID(), }) if err != nil { diff --git a/tools/jtk/internal/cmd/root/root_test.go b/tools/jtk/internal/cmd/root/root_test.go index c10ad1ef..5daac049 100644 --- a/tools/jtk/internal/cmd/root/root_test.go +++ b/tools/jtk/internal/cmd/root/root_test.go @@ -2,11 +2,14 @@ package root import ( "bytes" + "net/http" + "net/http/httptest" "regexp" "strings" "testing" "github.com/open-cli-collective/atlassian-go/artifact" + "github.com/open-cli-collective/atlassian-go/credtest" "github.com/open-cli-collective/atlassian-go/present" "github.com/open-cli-collective/atlassian-go/testutil" "github.com/open-cli-collective/atlassian-go/view" @@ -130,6 +133,29 @@ func TestOptions_SetAPIClient(t *testing.T) { testutil.Equal(t, got, client) } +func TestOptions_APIClient_ProxySkipsToken(t *testing.T) { + credtest.Hermetic(t) + var capturedAuth string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedAuth = r.Header.Get("Authorization") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{}`)) + })) + defer server.Close() + + t.Setenv("JIRA_URL", server.URL) + t.Setenv("JIRA_AUTH_METHOD", "proxy") + + opts := &Options{Stdout: &bytes.Buffer{}, Stderr: &bytes.Buffer{}} + c, err := opts.APIClient() + testutil.RequireNoError(t, err) + testutil.Equal(t, "", c.GetAuthHeader()) + + _, err = c.Get(t.Context(), "/myself") + testutil.RequireNoError(t, err) + testutil.Equal(t, "", capturedAuth) +} + func TestRegisterCommands(t *testing.T) { cmd, opts := NewCmd() diff --git a/tools/jtk/internal/config/config.go b/tools/jtk/internal/config/config.go index a2e1d7e8..e7a955b3 100644 --- a/tools/jtk/internal/config/config.go +++ b/tools/jtk/internal/config/config.go @@ -76,7 +76,7 @@ type Config struct { Email string `json:"email"` APIToken string `json:"api_token"` DefaultProject string `json:"default_project,omitempty"` - AuthMethod string `json:"auth_method,omitempty"` // "basic" (default) or "bearer" + AuthMethod string `json:"auth_method,omitempty"` // "basic" (default), "bearer", or "proxy" CloudID string `json:"cloud_id,omitempty"` // Required for bearer auth (gateway URL) // Keyring's `omitempty` is a no-op on this struct type — encoding/json // emits an empty {} for zero-valued struct fields regardless. Kept for @@ -269,16 +269,19 @@ func GetAPIToken() string { return tok } -// IsConfigured returns true if the NON-SECRET config is complete and a -// token is resolvable (env or keyring, non-migrating). The token left -// the plaintext config store, so completeness is composed from both -// halves. For bearer auth: URL + Cloud ID + token; for basic: URL + -// email + token. +// IsConfigured returns true if the config is complete for the selected auth +// method. For proxy auth, only the URL is required because credentials are +// injected by the proxy. For bearer auth: URL + Cloud ID + token. For basic: +// URL + email + token. func IsConfigured() bool { - if GetAuthMethod() == auth.AuthMethodBearer { + switch GetAuthMethod() { + case auth.AuthMethodProxy: + return GetURL() != "" + case auth.AuthMethodBearer: return GetURL() != "" && GetCloudID() != "" && GetAPIToken() != "" + default: + return GetURL() != "" && GetEmail() != "" && GetAPIToken() != "" } - return GetURL() != "" && GetEmail() != "" && GetAPIToken() != "" } // GetAuthMethod returns the auth method from config or environment. diff --git a/tools/jtk/internal/config/config_test.go b/tools/jtk/internal/config/config_test.go index ba460ffe..0b9680d6 100644 --- a/tools/jtk/internal/config/config_test.go +++ b/tools/jtk/internal/config/config_test.go @@ -485,6 +485,17 @@ func TestIsConfigured_Bearer(t *testing.T) { testutil.True(t, IsConfigured()) } +func TestIsConfigured_Proxy(t *testing.T) { + _, cleanup := setupTestConfig(t) + defer cleanup() + + t.Setenv("JIRA_AUTH_METHOD", "proxy") + testutil.False(t, IsConfigured()) + + t.Setenv("JIRA_URL", "http://127.0.0.1:8080/atlassian") + testutil.True(t, IsConfigured()) +} + func TestConfig_SaveAndLoad_WithAuthFields(t *testing.T) { _, cleanup := setupTestConfig(t) defer cleanup() From 7e03a90d2086beae4d06777ce46a1b7d8db19fd7 Mon Sep 17 00:00:00 2001 From: Bazyli Brzoska Date: Sat, 27 Jun 2026 19:10:15 -0700 Subject: [PATCH 2/6] fix: address proxy auth review feedback --- shared/auth/auth.go | 16 ++++ shared/auth/auth_test.go | 83 +++++++++++++++++++ tools/cfl/api/client.go | 17 ++-- tools/cfl/api/client_test.go | 16 +++- tools/cfl/internal/cmd/init/init.go | 17 +--- tools/cfl/internal/cmd/root/root.go | 2 +- tools/cfl/internal/cmd/root/root_test.go | 18 ++++ tools/jtk/integration-tests.md | 31 ++++++- tools/jtk/internal/cmd/initcmd/initcmd.go | 68 ++++++++------- .../jtk/internal/cmd/initcmd/initcmd_test.go | 45 ++++++++++ 10 files changed, 262 insertions(+), 51 deletions(-) diff --git a/shared/auth/auth.go b/shared/auth/auth.go index faba6255..28080626 100644 --- a/shared/auth/auth.go +++ b/shared/auth/auth.go @@ -31,6 +31,22 @@ func ValidateAuthMethod(method string) error { } } +// NormalizeConfig applies auth-method policy to config credential fields. +// +// Empty auth method defaults to basic. Proxy auth sends no CLI-side +// credentials, so direct credential fields are cleared. +func NormalizeConfig(authMethod, email, apiToken, cloudID string) (string, string, string, string) { + if authMethod == "" { + authMethod = AuthMethodBasic + } + if authMethod == AuthMethodProxy { + email = "" + apiToken = "" + cloudID = "" + } + return authMethod, email, apiToken, cloudID +} + // BasicAuthHeader returns the HTTP Basic Authentication header value // for use with Atlassian Cloud APIs. // diff --git a/shared/auth/auth_test.go b/shared/auth/auth_test.go index d731ed9e..9780f8f7 100644 --- a/shared/auth/auth_test.go +++ b/shared/auth/auth_test.go @@ -140,6 +140,89 @@ func TestValidateAuthMethod(t *testing.T) { } } +func TestNormalizeConfig(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + authMethod string + email string + apiToken string + cloudID string + wantAuthMethod string + wantEmail string + wantAPIToken string + wantCloudID string + }{ + { + name: "empty auth method defaults to basic", + email: "user@example.com", + apiToken: "token", + cloudID: "cloud-id", + wantAuthMethod: AuthMethodBasic, + wantEmail: "user@example.com", + wantAPIToken: "token", + wantCloudID: "cloud-id", + }, + { + name: "basic auth keeps direct credentials", + authMethod: AuthMethodBasic, + email: "user@example.com", + apiToken: "token", + cloudID: "cloud-id", + wantAuthMethod: AuthMethodBasic, + wantEmail: "user@example.com", + wantAPIToken: "token", + wantCloudID: "cloud-id", + }, + { + name: "bearer auth keeps token and cloud ID", + authMethod: AuthMethodBearer, + email: "user@example.com", + apiToken: "token", + cloudID: "cloud-id", + wantAuthMethod: AuthMethodBearer, + wantEmail: "user@example.com", + wantAPIToken: "token", + wantCloudID: "cloud-id", + }, + { + name: "proxy auth strips direct credentials", + authMethod: AuthMethodProxy, + email: "user@example.com", + apiToken: "token", + cloudID: "cloud-id", + wantAuthMethod: AuthMethodProxy, + wantEmail: "", + wantAPIToken: "", + wantCloudID: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + gotAuthMethod, gotEmail, gotAPIToken, gotCloudID := NormalizeConfig( + tt.authMethod, tt.email, tt.apiToken, tt.cloudID, + ) + + if gotAuthMethod != tt.wantAuthMethod { + t.Errorf("auth method = %q, want %q", gotAuthMethod, tt.wantAuthMethod) + } + if gotEmail != tt.wantEmail { + t.Errorf("email = %q, want %q", gotEmail, tt.wantEmail) + } + if gotAPIToken != tt.wantAPIToken { + t.Errorf("api token = %q, want %q", gotAPIToken, tt.wantAPIToken) + } + if gotCloudID != tt.wantCloudID { + t.Errorf("cloud ID = %q, want %q", gotCloudID, tt.wantCloudID) + } + }) + } +} + func TestAuthMethodConstants(t *testing.T) { t.Parallel() if AuthMethodBasic != "basic" { diff --git a/tools/cfl/api/client.go b/tools/cfl/api/client.go index 2aa61652..dd2cc9d0 100644 --- a/tools/cfl/api/client.go +++ b/tools/cfl/api/client.go @@ -14,10 +14,11 @@ import ( sharedurl "github.com/open-cli-collective/atlassian-go/url" ) -// Validation errors for bearer auth. +// Validation errors for auth configuration. var ( - ErrAPITokenRequired = errors.New("API token is required") - ErrCloudIDRequired = errors.New("cloud ID is required for bearer auth") + ErrAPITokenRequired = errors.New("API token is required") + ErrCloudIDRequired = errors.New("cloud ID is required for bearer auth") + ErrProxyURLRequiresHTTPS = errors.New("proxy auth URL must use https unless it is loopback http") ) // Client is the Confluence Cloud API client. @@ -35,10 +36,14 @@ func NewClient(baseURL, email, apiToken string) *Client { // NewProxyClient creates a Confluence client for a trusted proxy that injects // authentication upstream. No Authorization header is sent by the CLI. -func NewProxyClient(baseURL string) *Client { - return &Client{ - Client: client.New(normalizeWikiBaseURL(baseURL), "", "", &client.Options{SkipAuthHeader: true}), +func NewProxyClient(baseURL string) (*Client, error) { + normalized := normalizeWikiBaseURL(baseURL) + if strings.HasPrefix(normalized, "http://") && !sharedurl.IsLoopbackHTTP(normalized) { + return nil, ErrProxyURLRequiresHTTPS } + return &Client{ + Client: client.New(normalized, "", "", &client.Options{SkipAuthHeader: true}), + }, nil } // NewBearerClient creates a new Confluence API client using bearer auth via the API gateway. diff --git a/tools/cfl/api/client_test.go b/tools/cfl/api/client_test.go index 38874d1a..418c0bc4 100644 --- a/tools/cfl/api/client_test.go +++ b/tools/cfl/api/client_test.go @@ -34,16 +34,28 @@ func TestNewProxyClient(t *testing.T) { })) defer server.Close() - client := NewProxyClient(server.URL) + client, err := NewProxyClient(server.URL) + testutil.RequireNoError(t, err) testutil.NotNil(t, client) testutil.Equal(t, server.URL+"/wiki", client.GetBaseURL()) testutil.Equal(t, "", client.GetAuthHeader()) - _, err := client.Get(context.Background(), "/api/v2/spaces") + _, err = client.Get(context.Background(), "/api/v2/spaces") testutil.RequireNoError(t, err) testutil.Equal(t, "", capturedAuth) } +func TestNewProxyClient_RejectsArbitraryHTTP(t *testing.T) { + t.Parallel() + + client, err := NewProxyClient("http://example.com/atlassian") + testutil.RequireError(t, err) + testutil.Nil(t, client) + if !errors.Is(err, ErrProxyURLRequiresHTTPS) { + t.Errorf("got error %v, want %v", err, ErrProxyURLRequiresHTTPS) + } +} + func TestClient_AuthHeader(t *testing.T) { t.Parallel() var capturedAuth string diff --git a/tools/cfl/internal/cmd/init/init.go b/tools/cfl/internal/cmd/init/init.go index bf811834..732d0cb7 100644 --- a/tools/cfl/internal/cmd/init/init.go +++ b/tools/cfl/internal/cmd/init/init.go @@ -31,7 +31,7 @@ func defaultClientBuilder(cfg *config.Config) (*api.Client, error) { return api.NewBearerClient(cfg.APIToken, cfg.CloudID) } if cfg.AuthMethod == auth.AuthMethodProxy { - return api.NewProxyClient(cfg.URL), nil + return api.NewProxyClient(cfg.URL) } return api.NewClient(cfg.URL, cfg.Email, cfg.APIToken), nil } @@ -333,7 +333,9 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s } cfg.NormalizeURL() - normalizeAuthConfig(cfg) + cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID = auth.NormalizeConfig( + cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID, + ) if err := cfg.Validate(); err != nil { return fmt.Errorf("invalid configuration: %w", err) @@ -474,17 +476,6 @@ func finalizeInit( return nil } -func normalizeAuthConfig(cfg *config.Config) { - if cfg.AuthMethod == "" { - cfg.AuthMethod = auth.AuthMethodBasic - } - if cfg.AuthMethod == auth.AuthMethodProxy { - cfg.Email = "" - cfg.APIToken = "" - cfg.CloudID = "" - } -} - // requireNonInteractiveFields enforces the §3.4 fail-loud contract for // scripted/CI runs of `cfl init`. cfl init has no --token flag, so the // token MUST come from a pre-staged keyring entry via cfl set-credential; diff --git a/tools/cfl/internal/cmd/root/root.go b/tools/cfl/internal/cmd/root/root.go index dd3fc950..6eecb357 100644 --- a/tools/cfl/internal/cmd/root/root.go +++ b/tools/cfl/internal/cmd/root/root.go @@ -145,7 +145,7 @@ func (o *Options) APIClient() (*api.Client, error) { return api.NewBearerClient(cfg.APIToken, cfg.CloudID) } if cfg.AuthMethod == auth.AuthMethodProxy { - return api.NewProxyClient(cfg.URL), nil + return api.NewProxyClient(cfg.URL) } return api.NewClient(cfg.URL, cfg.Email, cfg.APIToken), nil } diff --git a/tools/cfl/internal/cmd/root/root_test.go b/tools/cfl/internal/cmd/root/root_test.go index 4d78e584..533a1057 100644 --- a/tools/cfl/internal/cmd/root/root_test.go +++ b/tools/cfl/internal/cmd/root/root_test.go @@ -2,6 +2,7 @@ package root import ( "bytes" + "errors" "fmt" "net/http" "net/http/httptest" @@ -165,6 +166,23 @@ func TestOptions_APIClient_ProxySkipsAuthHeader(t *testing.T) { testutil.Equal(t, "", capturedAuth) } +func TestOptions_APIClient_ProxyRejectsArbitraryHTTP(t *testing.T) { + t.Parallel() + + _, opts := NewCmd() + opts.SetConfig(&config.Config{ + URL: "http://example.com/atlassian", + AuthMethod: auth.AuthMethodProxy, + }) + + c, err := opts.APIClient() + testutil.RequireError(t, err) + testutil.Nil(t, c) + if !errors.Is(err, api.ErrProxyURLRequiresHTTPS) { + t.Errorf("got error %v, want %v", err, api.ErrProxyURLRequiresHTTPS) + } +} + func TestValidateOutputFormat(t *testing.T) { t.Parallel() diff --git a/tools/jtk/integration-tests.md b/tools/jtk/integration-tests.md index df28d7dd..f2703ea4 100644 --- a/tools/jtk/integration-tests.md +++ b/tools/jtk/integration-tests.md @@ -1361,6 +1361,8 @@ Verify each alias produces the same output as the full command: ## Test Execution Checklist +jtk supports three auth methods. Run the full checklist with separate Basic Auth and Proxy Auth passes, and run the bearer-compatible subset plus bearer guard checks during the Bearer Auth pass. + ### Pass 1: Basic Auth #### Setup (Basic Auth) @@ -1532,6 +1534,33 @@ Verify each alias produces the same output as the full command: --- +### Pass 3: Proxy Auth + +#### Setup (Proxy Auth) +- [ ] Configure a trusted proxy that authenticates upstream and exposes a loopback `http://` URL or an `https://` URL +- [ ] `jtk init --auth-method proxy --url http://127.0.0.1:8080/atlassian --no-verify` (adjust URL for your proxy) +- [ ] `jtk config show` — auth_method = proxy; email, api_token, and cloud_id are empty +- [ ] `jtk config test` — Authentication successful through the proxy +- [ ] `jtk me` works +- [ ] Discover: `$PROJECT`, `$BOARD_ID`, `$SPRINT_ID`, `$ACCOUNT_ID`, `$AUTO_UUID`, `$DASHBOARD_ID`, `$EXISTING_ISSUE`, `$LINK_TYPE`, `$CUSTOM_FIELD`, `$SELECT_FIELD` +- [ ] `jtk issues types -p $PROJECT` to learn `$ISSUE_TYPE` + +#### Full Checklist (Proxy Auth) +- [ ] Run Sections 1-18 using the same steps as Pass 1 +- [ ] Include Agile, Dashboard, Automation, and Sprint sections +- [ ] Skip Section 19; bearer guard checks are bearer-auth specific + +#### Cleanup (Proxy Auth) +- [ ] Delete test projects: `jtk projects delete ZTEST --force` (etc.) +- [ ] Delete test issues: search for `[Test]` prefix, delete with `--force` +- [ ] Delete test dashboards: `jtk dashboards delete $TEST_DASH_ID` +- [ ] Trash test fields: `jtk fields delete $TEST_FIELD --force` +- [ ] Delete automation test rules: `jtk auto list | grep '\[Test\]' | awk '{print $1}' | xargs -I{} jtk auto delete {}` +- [ ] Verify: `jtk auto list | grep -E '\[Test\]|\[DELETEME\]'` — should be empty +- [ ] **Accepted residuals:** `$ARCHIVE_ISSUE_1` and `$ARCHIVE_ISSUE_2` remain archived (no CLI restore); `$TEST_FIELD` remains trashed (no purge API) + +--- + ## Adding New Tests When adding new features or fixing bugs: @@ -1539,6 +1568,6 @@ When adding new features or fixing bugs: 1. Add test steps to the appropriate numbered section above 2. Include both happy path and error cases with exact expected output 3. Document gotchas inline, immediately before the step where they matter -4. Update both Pass 1 and Pass 2 in the Test Execution Checklist +4. Update Pass 1, Pass 2, and Pass 3 in the Test Execution Checklist 5. If the feature is scope-restricted, add guard tests to Section 19 6. Record bugs discovered during testing and continue — don't stop to fix diff --git a/tools/jtk/internal/cmd/initcmd/initcmd.go b/tools/jtk/internal/cmd/initcmd/initcmd.go index 1a145686..f7e2f440 100644 --- a/tools/jtk/internal/cmd/initcmd/initcmd.go +++ b/tools/jtk/internal/cmd/initcmd/initcmd.go @@ -27,6 +27,12 @@ import ( "github.com/open-cli-collective/jira-ticket-cli/internal/config" ) +type clientBuilder func(api.ClientConfig) (*api.Client, error) + +func defaultClientBuilder(cfg api.ClientConfig) (*api.Client, error) { + return api.New(cfg) +} + // Register registers the init command func Register(parent *cobra.Command, opts *root.Options) { var url, email, token, tokenFromEnv, authMethod, cloudID string @@ -232,7 +238,7 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, EchoMode(huh.EchoModePassword). Value(&cfg.APIToken). Validate(func(s string) error { - if s == "" { + if s == "" && !noVerify { return fmt.Errorf("API token is required") } return nil @@ -288,7 +294,7 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, EchoMode(huh.EchoModePassword). Value(&cfg.APIToken). Validate(func(s string) error { - if s == "" { + if s == "" && !noVerify { return fmt.Errorf("API token is required") } return nil @@ -318,23 +324,38 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, // Normalize URL cfg.URL = sharedurl.NormalizeURL(cfg.URL) - normalizeAuthConfig(cfg) - - client, err := api.New(api.ClientConfig{ - URL: cfg.URL, - Email: cfg.Email, - APIToken: cfg.APIToken, - AuthMethod: cfg.AuthMethod, - CloudID: cfg.CloudID, - }) - if err != nil { - return fmt.Errorf("creating client: %w", err) - } + cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID = auth.NormalizeConfig( + cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID, + ) + + return finalizeInit(ctx, opts, cfg, result, sharedPath, noVerify, defaultClientBuilder) +} + +func finalizeInit( + ctx context.Context, + opts *root.Options, + cfg *config.Config, + result *reconcileResult, + sharedPath string, + noVerify bool, + build clientBuilder, +) error { + v := opts.View() - // Verify connection unless --no-verify if !noVerify { v.Println("Testing connection...") + client, err := build(api.ClientConfig{ + URL: cfg.URL, + Email: cfg.Email, + APIToken: cfg.APIToken, + AuthMethod: cfg.AuthMethod, + CloudID: cfg.CloudID, + }) + if err != nil { + return fmt.Errorf("creating client: %w", err) + } + user, err := client.GetCurrentUser(ctx, "") if err != nil { v.Error("Connection failed: %v", err) @@ -385,6 +406,8 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, // keyring under the single shared api_token (§1.11.10). if cfg.AuthMethod == auth.AuthMethodProxy { v.Success("Configuration saved to %s (proxy auth; no token stored)", sharedPath) + } else if cfg.APIToken == "" { + v.Success("Configuration saved to %s (token not stored; run `jtk set-credential --ref atlassian-cli/default --key api_token --stdin --overwrite` before authenticated commands)", sharedPath) } else { if err := keyring.PersistToken(cfg.APIToken); err != nil { v.Error("Saved the non-secret config to %s, but could not store the API token in the keyring: %v", sharedPath, err) @@ -426,10 +449,10 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, v.Println(" jtk me") v.Println(" jtk issues list --project ") - if isBearer { + if cfg.AuthMethod == auth.AuthMethodBearer { v.Println("") v.Info("To switch back to basic auth later, run: jtk init --auth-method basic") - } else if isProxy { + } else if cfg.AuthMethod == auth.AuthMethodProxy { v.Println("") v.Info("To switch to direct authentication later, run: jtk init --auth-method basic") } @@ -437,17 +460,6 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, return nil } -func normalizeAuthConfig(cfg *config.Config) { - if cfg.AuthMethod == "" { - cfg.AuthMethod = auth.AuthMethodBasic - } - if cfg.AuthMethod == auth.AuthMethodProxy { - cfg.Email = "" - cfg.APIToken = "" - cfg.CloudID = "" - } -} - // requireNonInteractiveFields enforces the §3.4 fail-loud contract for // scripted/CI runs of `jtk init`: any required value missing from the // flag prefills (which already populated cfg) produces an error naming diff --git a/tools/jtk/internal/cmd/initcmd/initcmd_test.go b/tools/jtk/internal/cmd/initcmd/initcmd_test.go index 5e6573ea..3cf81c57 100644 --- a/tools/jtk/internal/cmd/initcmd/initcmd_test.go +++ b/tools/jtk/internal/cmd/initcmd/initcmd_test.go @@ -3,6 +3,8 @@ package initcmd import ( "bytes" "context" + "errors" + "path/filepath" "strings" "testing" @@ -13,6 +15,7 @@ import ( "github.com/open-cli-collective/atlassian-go/testutil" "github.com/spf13/cobra" + "github.com/open-cli-collective/jira-ticket-cli/api" "github.com/open-cli-collective/jira-ticket-cli/internal/cmd/root" "github.com/open-cli-collective/jira-ticket-cli/internal/config" ) @@ -218,6 +221,48 @@ func TestRunInit_Proxy_NoTokenRequiredOrPersisted(t *testing.T) { testutil.False(t, ok) } +func TestFinalizeInit_NoVerify_BasicAuthWithoutTokenSavesConfig(t *testing.T) { + credtest.Hermetic(t) + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + opts := &root.Options{ + NoColor: true, + Stdout: &bytes.Buffer{}, + Stderr: &bytes.Buffer{}, + } + cfg := &config.Config{ + URL: "https://acme.atlassian.net", + Email: "user@example.com", + AuthMethod: auth.AuthMethodBasic, + } + builderCalled := false + build := func(api.ClientConfig) (*api.Client, error) { + builderCalled = true + return nil, errors.New("client builder should not run with --no-verify") + } + + err := finalizeInit(context.Background(), opts, cfg, &reconcileResult{store: &credstore.Store{}}, configPath, true, build) + testutil.RequireNoError(t, err) + testutil.False(t, builderCalled, "client builder should not be invoked when --no-verify is set") + + store, err := credstore.Load(configPath) + testutil.RequireNoError(t, err) + testutil.Equal(t, "https://acme.atlassian.net", store.Default.URL) + testutil.Equal(t, "user@example.com", store.Default.Email) + testutil.Equal(t, auth.AuthMethodBasic, store.Default.AuthMethod) + + s, err := keyring.OpenNoMigrate() + testutil.RequireNoError(t, err) + defer func() { _ = s.Close() }() + ok, err := s.HasToken(keyring.KeyAPIToken) + testutil.RequireNoError(t, err) + testutil.False(t, ok) + + stdout := opts.Stdout.(*bytes.Buffer).String() + testutil.Contains(t, stdout, "Configuration saved") + testutil.Contains(t, stdout, "token not stored") +} + func TestInitCommand_Flags(t *testing.T) { t.Parallel() rootCmd := &cobra.Command{Use: "jtk", Short: "Test CLI"} From a1b5ee0208529ff133bfc9dde5c9fbb114cabfd8 Mon Sep 17 00:00:00 2001 From: Bazyli Brzoska Date: Thu, 16 Jul 2026 17:48:12 -0700 Subject: [PATCH 3/6] fix: address follow-up proxy auth review --- shared/auth/auth.go | 27 ++ shared/auth/auth_test.go | 76 ++++ tools/cfl/integration-tests.md | 23 +- tools/cfl/internal/cmd/init/finalize_test.go | 400 +++++++++++++++++ tools/cfl/internal/cmd/init/init.go | 44 +- tools/cfl/internal/cmd/init/init_test.go | 406 ------------------ tools/cfl/internal/config/config.go | 15 +- tools/cfl/internal/config/config_test.go | 69 +++ tools/jtk/internal/cmd/initcmd/initcmd.go | 37 +- .../jtk/internal/cmd/initcmd/initcmd_test.go | 80 ---- 10 files changed, 626 insertions(+), 551 deletions(-) create mode 100644 tools/cfl/internal/cmd/init/finalize_test.go diff --git a/shared/auth/auth.go b/shared/auth/auth.go index 28080626..cef563a5 100644 --- a/shared/auth/auth.go +++ b/shared/auth/auth.go @@ -47,6 +47,33 @@ func NormalizeConfig(authMethod, email, apiToken, cloudID string) (string, strin return authMethod, email, apiToken, cloudID } +// RequireNonInteractiveFields validates the auth fields needed by scripted +// init flows and names the first missing CLI value. toolHint is the +// tool-specific set-credential command shown when the API token is absent. +func RequireNonInteractiveFields(url, authMethod, email, apiToken, cloudID, toolHint string) error { + if url == "" { + return errors.New("--non-interactive: missing required value for --url") + } + + switch authMethod { + case AuthMethodProxy: + return nil + case AuthMethodBearer: + if cloudID == "" { + return errors.New("--non-interactive: missing required value for --cloud-id (bearer auth)") + } + default: + if email == "" { + return errors.New("--non-interactive: missing required value for --email (basic auth)") + } + } + + if apiToken == "" { + return fmt.Errorf("--non-interactive: missing required value for --token-stdin or --token-from-env VAR (or pre-stage with `%s`)", toolHint) + } + return nil +} + // BasicAuthHeader returns the HTTP Basic Authentication header value // for use with Atlassian Cloud APIs. // diff --git a/shared/auth/auth_test.go b/shared/auth/auth_test.go index 9780f8f7..5a23ef30 100644 --- a/shared/auth/auth_test.go +++ b/shared/auth/auth_test.go @@ -223,6 +223,82 @@ func TestNormalizeConfig(t *testing.T) { } } +func TestRequireNonInteractiveFields(t *testing.T) { + t.Parallel() + + const toolHint = "cfl set-credential --ref atlassian-cli/default --key api_token --stdin" + tests := []struct { + name string + url string + authMethod string + email string + apiToken string + cloudID string + wantError string + }{ + {name: "missing URL", wantError: "--url"}, + { + name: "basic auth missing email", + url: "https://example.atlassian.net", + apiToken: "token", + wantError: "--email (basic auth)", + }, + { + name: "bearer auth missing cloud ID", + url: "https://example.atlassian.net", + authMethod: AuthMethodBearer, + apiToken: "token", + wantError: "--cloud-id (bearer auth)", + }, + { + name: "basic auth missing token includes tool hint", + url: "https://example.atlassian.net", + email: "user@example.com", + wantError: toolHint, + }, + { + name: "bearer auth missing token includes tool hint", + url: "https://example.atlassian.net", + authMethod: AuthMethodBearer, + cloudID: "cloud-id", + wantError: toolHint, + }, + { + name: "proxy auth needs only URL", + url: "http://127.0.0.1:8080/atlassian", + authMethod: AuthMethodProxy, + }, + { + name: "complete basic auth", + url: "https://example.atlassian.net", + email: "user@example.com", + apiToken: "token", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := RequireNonInteractiveFields( + tt.url, tt.authMethod, tt.email, tt.apiToken, tt.cloudID, toolHint, + ) + if tt.wantError == "" { + if err != nil { + t.Fatalf("RequireNonInteractiveFields() error = %v, want nil", err) + } + return + } + if err == nil { + t.Fatalf("RequireNonInteractiveFields() error = nil, want containing %q", tt.wantError) + } + if !strings.Contains(err.Error(), tt.wantError) { + t.Fatalf("RequireNonInteractiveFields() error = %q, want containing %q", err, tt.wantError) + } + }) + } +} + func TestAuthMethodConstants(t *testing.T) { t.Parallel() if AuthMethodBasic != "basic" { diff --git a/tools/cfl/integration-tests.md b/tools/cfl/integration-tests.md index 123630b8..45bcaf21 100644 --- a/tools/cfl/integration-tests.md +++ b/tools/cfl/integration-tests.md @@ -758,6 +758,27 @@ All cfl commands work with all configured auth methods (no scope restrictions fo --- +### Pass 3: Proxy Auth + +#### Setup (Proxy Auth) +- [ ] Configure a trusted proxy that authenticates upstream and exposes a loopback `http://` URL or an `https://` URL +- [ ] `cfl init --auth-method proxy --url http://127.0.0.1:8080/atlassian --no-verify` (adjust URL for your proxy) +- [ ] `cfl config show` — auth_method = proxy; email, api_token, and cloud_id are empty +- [ ] `cfl config test` — Connection verified through the proxy +- [ ] `cfl space list` works + +#### Full Checklist (Proxy Auth) +- [ ] Run Page CRUD, ADF Body Fallback, Wiki Links, Attachment CRUD, Search, Space CRUD, and Edge Cases using the same steps as Pass 1 +- [ ] No sections are skipped; proxy auth has no cfl scope restrictions + +#### Cleanup (Proxy Auth) +- [ ] Delete all [Test] prefixed pages +- [ ] `cfl space delete INTTEST --force` +- [ ] `cfl space delete INTTEST2 --force` +- [ ] Verify no test data remains + +--- + ## Adding New Tests When adding new features or fixing bugs: @@ -765,4 +786,4 @@ When adding new features or fixing bugs: 1. Add test cases to the appropriate section above 2. Include both happy path and error cases 3. Document any known limitations or edge cases -4. Update the "Test Execution Checklist" if needed +4. Update Pass 1, Pass 2, and Pass 3 in the Test Execution Checklist diff --git a/tools/cfl/internal/cmd/init/finalize_test.go b/tools/cfl/internal/cmd/init/finalize_test.go new file mode 100644 index 00000000..5ae23d6e --- /dev/null +++ b/tools/cfl/internal/cmd/init/finalize_test.go @@ -0,0 +1,400 @@ +package init + +import ( + "bytes" + "context" + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/open-cli-collective/atlassian-go/auth" + sharedclient "github.com/open-cli-collective/atlassian-go/client" + "github.com/open-cli-collective/atlassian-go/credstore" + "github.com/open-cli-collective/atlassian-go/credtest" + "github.com/open-cli-collective/atlassian-go/keyring" + "github.com/open-cli-collective/atlassian-go/testutil" + + "github.com/open-cli-collective/confluence-cli/api" + "github.com/open-cli-collective/confluence-cli/internal/cmd/root" + "github.com/open-cli-collective/confluence-cli/internal/config" +) + +// finalizeInit tests use t.TempDir() for paths and an httptest-backed +// clientBuilder so the user's real config is never touched and no real +// network call is made. + +func newFinalizeReconcileResult() *reconcileResult { + return &reconcileResult{store: &credstore.Store{}} +} + +func newFinalizeOpts() *root.Options { + return &root.Options{ + Output: "table", + NoColor: true, + Stdout: &bytes.Buffer{}, + Stderr: &bytes.Buffer{}, + } +} + +func userResponseServer(t *testing.T, body string, status int) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + testutil.Equal(t, "/wiki/rest/api/user/current", r.URL.Path) + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + })) +} + +// TestFinalizeInit_WritesConnectionToDefault verifies the §2.2 +// (MON-5328) single-source model: connection always lands in the shared +// `default` section (no per-tool override target), the token NEVER +// touches the plaintext store (keyring only, single api_token), and the +// cfl section carries no connection fields. +func TestFinalizeInit_WritesConnectionToDefault(t *testing.T) { + credtest.Hermetic(t) // t.Setenv → no t.Parallel + server := userResponseServer(t, `{"accountId":"abc","displayName":"X","email":"x@e"}`, http.StatusOK) + defer server.Close() + build := func(_ *config.Config) (*api.Client, error) { + return api.NewClient(server.URL, "x@e", "tok"), nil + } + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + cfg := &config.Config{URL: server.URL + "/wiki", Email: "x@e", APIToken: "tok", DefaultSpace: "SP"} + result := &reconcileResult{store: &credstore.Store{}} + + testutil.RequireNoError(t, + finalizeInit(context.Background(), newFinalizeOpts(), cfg, result, configPath, false, build)) + + loaded, err := credstore.Load(configPath) + testutil.RequireNoError(t, err) + + // Connection in default; token never in plaintext. + testutil.Equal(t, "", loaded.Default.APIToken) + testutil.Equal(t, "x@e", loaded.Default.Email) + testutil.Equal(t, server.URL, loaded.Default.URL) // /wiki stripped + // cfl section carries only the non-secret default, no connection. + testutil.Equal(t, "SP", loaded.CFL.DefaultSpace) + + // Raw file must contain no api_token and no per-tool connection key. + raw, rerr := os.ReadFile(configPath) //nolint:gosec // test reads its own temp file + testutil.RequireNoError(t, rerr) + if strings.Contains(string(raw), "api_token") { + t.Fatalf("plaintext store must never contain api_token:\n%s", raw) + } + + s, err := keyring.OpenNoMigrate() + testutil.RequireNoError(t, err) + defer func() { _ = s.Close() }() + ok, err := s.HasToken(keyring.KeyAPIToken) + testutil.RequireNoError(t, err) + testutil.True(t, ok) +} + +func TestFinalizeInit_BasicHappyPath(t *testing.T) { + credtest.Hermetic(t) // t.Setenv → no t.Parallel + server := userResponseServer(t, `{"accountId":"abc123","displayName":"Rian Stockbower","email":"rian@example.com"}`, http.StatusOK) + defer server.Close() + + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + opts := newFinalizeOpts() + cfg := &config.Config{ + URL: server.URL, + Email: "rian@example.com", + APIToken: "test-token", + } + + build := func(_ *config.Config) (*api.Client, error) { + return api.NewClient(server.URL, "rian@example.com", "test-token"), nil + } + + err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, false, build) + testutil.RequireNoError(t, err) + + stdout := opts.Stdout.(*bytes.Buffer).String() + testutil.Contains(t, stdout, "Connected to") + testutil.Contains(t, stdout, "Configuration saved to") + testutil.Contains(t, stdout, "abc123 | Rian Stockbower | rian@example.com") + + _, err = os.Stat(configPath) + testutil.RequireNoError(t, err) +} + +func TestFinalizeInit_BearerHappyPath(t *testing.T) { + credtest.Hermetic(t) // t.Setenv → no t.Parallel + // Server asserts that the verify request actually carries a Bearer + // Authorization header — i.e. the bearer code path emits bearer auth on + // the wire, not just bearer-themed UI copy. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + testutil.Equal(t, "/wiki/rest/api/user/current", r.URL.Path) + testutil.Equal(t, "Bearer scoped-token", r.Header.Get("Authorization")) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"accountId":"svc456","displayName":"Service Account","email":"svc@example.com"}`)) + })) + defer server.Close() + + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + opts := newFinalizeOpts() + cfg := &config.Config{ + URL: server.URL, + APIToken: "scoped-token", + AuthMethod: auth.AuthMethodBearer, + CloudID: "test-cloud-id", + } + + // Construct a real bearer-style client (auth header injected via Options) + // pointed at the httptest URL. This mirrors what api.NewBearerClient + // produces, just with a routable base URL. + build := func(c *config.Config) (*api.Client, error) { + return &api.Client{ + Client: sharedclient.New(server.URL, "", "", &sharedclient.Options{ + AuthHeader: auth.BearerAuthHeader(c.APIToken), + }), + }, nil + } + + err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, false, build) + testutil.RequireNoError(t, err) + + stdout := opts.Stdout.(*bytes.Buffer).String() + testutil.Contains(t, stdout, "Connected to") + testutil.Contains(t, stdout, "Configuration saved to") + testutil.Contains(t, stdout, "svc456 | Service Account | svc@example.com") + testutil.Contains(t, stdout, "switch back to basic auth") + + _, err = os.Stat(configPath) + testutil.RequireNoError(t, err) +} + +// TestDefaultClientBuilder verifies the production wiring between +// cfg.AuthMethod and which client constructor runs. In the finalizeInit +// tests the builder is always replaced; this test pins the default. +func TestDefaultClientBuilder(t *testing.T) { + t.Parallel() + + t.Run("basic constructs basic-auth client", func(t *testing.T) { + t.Parallel() + cfg := &config.Config{ + URL: "https://example.atlassian.net", + Email: "user@example.com", + APIToken: "secret", + } + c, err := defaultClientBuilder(cfg) + testutil.RequireNoError(t, err) + testutil.Equal(t, "https://example.atlassian.net", c.BaseURL) + // Basic auth header is "Basic "; presence of the + // "Basic " prefix is enough to confirm dispatch. + testutil.True(t, strings.HasPrefix(c.AuthHeader, "Basic "), "expected Basic prefix, got: "+c.AuthHeader) + }) + + t.Run("bearer constructs bearer-auth client at gateway", func(t *testing.T) { + t.Parallel() + cfg := &config.Config{ + APIToken: "scoped-token", + AuthMethod: auth.AuthMethodBearer, + CloudID: "cloud-abc", + } + c, err := defaultClientBuilder(cfg) + testutil.RequireNoError(t, err) + testutil.Contains(t, c.BaseURL, "/ex/confluence/cloud-abc/wiki") + testutil.Equal(t, "Bearer scoped-token", c.AuthHeader) + }) + + t.Run("bearer rejects empty cloud ID", func(t *testing.T) { + t.Parallel() + cfg := &config.Config{ + APIToken: "scoped-token", + AuthMethod: auth.AuthMethodBearer, + } + _, err := defaultClientBuilder(cfg) + testutil.RequireError(t, err) + }) + + t.Run("proxy constructs no-auth client", func(t *testing.T) { + t.Parallel() + cfg := &config.Config{ + URL: "http://127.0.0.1:8080/atlassian", + AuthMethod: auth.AuthMethodProxy, + } + c, err := defaultClientBuilder(cfg) + testutil.RequireNoError(t, err) + testutil.Equal(t, "http://127.0.0.1:8080/atlassian/wiki", c.BaseURL) + testutil.Equal(t, "", c.AuthHeader) + }) +} + +func TestFinalizeInit_ProxyNoTokenPersisted(t *testing.T) { + credtest.Hermetic(t) + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + opts := newFinalizeOpts() + cfg := &config.Config{ + URL: "http://127.0.0.1:8080/atlassian/wiki", + AuthMethod: auth.AuthMethodProxy, + DefaultSpace: "DEV", + } + + err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, true, defaultClientBuilder) + testutil.RequireNoError(t, err) + + loaded, err := credstore.Load(configPath) + testutil.RequireNoError(t, err) + testutil.Equal(t, "http://127.0.0.1:8080/atlassian", loaded.Default.URL) + testutil.Equal(t, "", loaded.Default.Email) + testutil.Equal(t, auth.AuthMethodProxy, loaded.Default.AuthMethod) + testutil.Equal(t, "", loaded.Default.CloudID) + testutil.Equal(t, "DEV", loaded.CFL.DefaultSpace) + + s, err := keyring.OpenNoMigrate() + testutil.RequireNoError(t, err) + defer func() { _ = s.Close() }() + ok, err := s.HasToken(keyring.KeyAPIToken) + testutil.RequireNoError(t, err) + testutil.False(t, ok) +} + +func TestFinalizeInit_AuthFailure(t *testing.T) { + t.Parallel() + server := userResponseServer(t, `{"message":"Unauthorized"}`, http.StatusUnauthorized) + defer server.Close() + + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + opts := newFinalizeOpts() + cfg := &config.Config{ + URL: server.URL, + Email: "rian@example.com", + APIToken: "wrong-token", + } + + build := func(_ *config.Config) (*api.Client, error) { + return api.NewClient(server.URL, "rian@example.com", "wrong-token"), nil + } + + err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, false, build) + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "authentication failed") + + // Both the error and the remediation hint must land on stderr — splitting + // them across stdout/stderr would mean a script capturing only stderr + // sees the failure with no actionable next step. + stderr := opts.Stderr.(*bytes.Buffer).String() + testutil.Contains(t, stderr, "Connection failed") + testutil.Contains(t, stderr, "Check your credentials and try again") + + _, statErr := os.Stat(configPath) + testutil.True(t, os.IsNotExist(statErr), "config file should not exist after auth failure") +} + +func TestFinalizeInit_BuildFailureSurfacesError(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + opts := newFinalizeOpts() + cfg := &config.Config{ + URL: "https://example.atlassian.net", + Email: "rian@example.com", + APIToken: "test-token", + } + + build := func(_ *config.Config) (*api.Client, error) { + return nil, errors.New("simulated builder failure") + } + + err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, false, build) + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "simulated builder failure") + + // User must see WHY init failed, not just a non-zero exit. + stderr := opts.Stderr.(*bytes.Buffer).String() + testutil.Contains(t, stderr, "Could not construct API client") + + _, statErr := os.Stat(configPath) + testutil.True(t, os.IsNotExist(statErr), "config should not be saved when builder fails") +} + +func TestFinalizeInit_NoVerify(t *testing.T) { + credtest.Hermetic(t) // t.Setenv → no t.Parallel + httpCalled := false + server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { + httpCalled = true + })) + defer server.Close() + + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + opts := newFinalizeOpts() + cfg := &config.Config{ + URL: server.URL, + Email: "rian@example.com", + APIToken: "test-token", + } + + // Track builder invocation directly. If the noVerify guard regresses + // (e.g. moves below the build call), the server-not-called assertion + // alone wouldn't catch it — the builder running but never being used + // would still leave httpCalled=false. + builderCalled := false + build := func(_ *config.Config) (*api.Client, error) { + builderCalled = true + return api.NewClient(server.URL, "rian@example.com", "test-token"), nil + } + + err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, true, build) + testutil.RequireNoError(t, err) + + testutil.False(t, builderCalled, "clientBuilder should not be invoked when --no-verify is set") + testutil.False(t, httpCalled, "no API call should be made when --no-verify is set") + + stdout := opts.Stdout.(*bytes.Buffer).String() + testutil.Contains(t, stdout, "Configuration saved to") + // No verify → no "Connected to" confirmation, no user one-liner. + testutil.False(t, strings.Contains(stdout, "Connected to"), "verify confirmation should not appear without verify") + + _, err = os.Stat(configPath) + testutil.RequireNoError(t, err) +} + +func TestFinalizeInit_NoVerify_BearerWithoutTokenSavesConfig(t *testing.T) { + credtest.Hermetic(t) + configPath := filepath.Join(t.TempDir(), "config.yml") + opts := newFinalizeOpts() + cfg := &config.Config{ + URL: "https://example.atlassian.net/wiki", + AuthMethod: auth.AuthMethodBearer, + CloudID: "cloud-id", + } + + builderCalled := false + build := func(_ *config.Config) (*api.Client, error) { + builderCalled = true + return nil, errors.New("client builder should not run with --no-verify") + } + + err := finalizeInit( + context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, true, build, + ) + testutil.RequireNoError(t, err) + testutil.False(t, builderCalled, "clientBuilder should not be invoked when --no-verify is set") + + store, err := credstore.Load(configPath) + testutil.RequireNoError(t, err) + testutil.Equal(t, auth.AuthMethodBearer, store.Default.AuthMethod) + testutil.Equal(t, "cloud-id", store.Default.CloudID) + + s, err := keyring.OpenNoMigrate() + testutil.RequireNoError(t, err) + defer func() { _ = s.Close() }() + ok, err := s.HasToken(keyring.KeyAPIToken) + testutil.RequireNoError(t, err) + testutil.False(t, ok) + + stdout := opts.Stdout.(*bytes.Buffer).String() + testutil.Contains(t, stdout, "token not stored") + testutil.Contains(t, stdout, "cfl set-credential") +} diff --git a/tools/cfl/internal/cmd/init/init.go b/tools/cfl/internal/cmd/init/init.go index 732d0cb7..d4ed47bd 100644 --- a/tools/cfl/internal/cmd/init/init.go +++ b/tools/cfl/internal/cmd/init/init.go @@ -245,7 +245,7 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s EchoMode(huh.EchoModePassword). Value(&cfg.APIToken). Validate(func(s string) error { - if s == "" { + if s == "" && !noVerify { return fmt.Errorf("API token is required") } return nil @@ -301,7 +301,7 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s EchoMode(huh.EchoModePassword). Value(&cfg.APIToken). Validate(func(s string) error { - if s == "" { + if s == "" && !noVerify { return fmt.Errorf("API token is required") } return nil @@ -322,7 +322,10 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s // (via `cfl set-credential`). Fail loud naming the first missing // field. if !prompt.WantPrompt(opts.NonInteractive, opts.Stdin) { - if err := requireNonInteractiveFields(cfg); err != nil { + if err := auth.RequireNonInteractiveFields( + cfg.URL, cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID, + "cfl set-credential --ref atlassian-cli/default --key api_token --stdin", + ); err != nil { return err } } else { @@ -337,7 +340,7 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID, ) - if err := cfg.Validate(); err != nil { + if err := cfg.ValidateForInit(noVerify); err != nil { return fmt.Errorf("invalid configuration: %w", err) } @@ -414,6 +417,8 @@ func finalizeInit( // keyring under the single shared api_token (§1.11.10). if cfg.AuthMethod == auth.AuthMethodProxy { v.Success("Configuration saved to %s (proxy auth; no token stored)", sharedPath) + } else if cfg.APIToken == "" { + v.Success("Configuration saved to %s (token not stored; run `cfl set-credential --ref atlassian-cli/default --key api_token --stdin` before authenticated commands)", sharedPath) } else { if err := keyring.PersistToken(cfg.APIToken); err != nil { v.Error("Saved the non-secret config to %s, but could not store the API token in the keyring: %v", sharedPath, err) @@ -465,39 +470,14 @@ func finalizeInit( v.Println(" cfl space list") v.Println(" cfl page list --space ") - if cfg.AuthMethod == auth.AuthMethodBearer { + switch cfg.AuthMethod { + case auth.AuthMethodBearer: v.Println("") v.Info("To switch back to basic auth later, run: cfl init --auth-method basic") - } else if cfg.AuthMethod == auth.AuthMethodProxy { + case auth.AuthMethodProxy: v.Println("") v.Info("To switch to direct authentication later, run: cfl init --auth-method basic") } return nil } - -// requireNonInteractiveFields enforces the §3.4 fail-loud contract for -// scripted/CI runs of `cfl init`. cfl init has no --token flag, so the -// token MUST come from a pre-staged keyring entry via cfl set-credential; -// the error names that path explicitly. -func requireNonInteractiveFields(cfg *config.Config) error { - if cfg.URL == "" { - return fmt.Errorf("--non-interactive: missing required value for --url") - } - switch cfg.AuthMethod { - case auth.AuthMethodProxy: - return nil - case auth.AuthMethodBearer: - if cfg.CloudID == "" { - return fmt.Errorf("--non-interactive: missing required value for --cloud-id (bearer auth)") - } - default: - if cfg.Email == "" { - return fmt.Errorf("--non-interactive: missing required value for --email (basic auth)") - } - } - if cfg.APIToken == "" { - return fmt.Errorf("--non-interactive: missing required value for --token-stdin or --token-from-env VAR (or pre-stage with `cfl set-credential --ref atlassian-cli/default --key api_token --stdin`)") - } - return nil -} diff --git a/tools/cfl/internal/cmd/init/init_test.go b/tools/cfl/internal/cmd/init/init_test.go index cd7a68c3..b384ac75 100644 --- a/tools/cfl/internal/cmd/init/init_test.go +++ b/tools/cfl/internal/cmd/init/init_test.go @@ -3,23 +3,18 @@ package init import ( "bytes" "context" - "errors" - "net/http" - "net/http/httptest" "os" "path/filepath" "strings" "testing" "github.com/open-cli-collective/atlassian-go/auth" - sharedclient "github.com/open-cli-collective/atlassian-go/client" "github.com/open-cli-collective/atlassian-go/credstore" "github.com/open-cli-collective/atlassian-go/credtest" "github.com/open-cli-collective/atlassian-go/keyring" "github.com/open-cli-collective/atlassian-go/testutil" "github.com/spf13/cobra" - "github.com/open-cli-collective/confluence-cli/api" "github.com/open-cli-collective/confluence-cli/internal/cmd/root" "github.com/open-cli-collective/confluence-cli/internal/config" ) @@ -124,70 +119,6 @@ func TestRunInit_InvalidAuthMethod(t *testing.T) { testutil.Contains(t, err.Error(), "invalid auth method") } -// TestRequireNonInteractiveFields_NamesFirstMissing — cfl variant. -// The token error must recommend --token-stdin / --token-from-env (added -// in #390) first AND name `cfl set-credential` as the alternate -// pre-stage path. -func TestRequireNonInteractiveFields_NamesFirstMissing(t *testing.T) { - t.Parallel() - tests := []struct { - name string - cfg *config.Config - wants []string - }{ - {"basic — missing URL", &config.Config{}, []string{"--url"}}, - {"basic — missing email", &config.Config{URL: "https://acme.atlassian.net"}, []string{"--email"}}, - {"bearer — missing cloud-id", &config.Config{URL: "https://acme.atlassian.net", AuthMethod: auth.AuthMethodBearer}, []string{"--cloud-id"}}, - { - name: "basic — missing token recommends --token-stdin + --token-from-env + set-credential", - cfg: &config.Config{URL: "https://acme.atlassian.net", Email: "u@x.io"}, - wants: []string{"--token-stdin", "--token-from-env", "set-credential"}, - }, - { - name: "bearer — missing token recommends --token-stdin + --token-from-env + set-credential", - cfg: &config.Config{URL: "https://acme.atlassian.net", CloudID: "cid", AuthMethod: auth.AuthMethodBearer}, - wants: []string{"--token-stdin", "--token-from-env", "set-credential"}, - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - err := requireNonInteractiveFields(tc.cfg) - testutil.RequireError(t, err) - if !strings.Contains(err.Error(), "--non-interactive") { - t.Fatalf("error must mention --non-interactive: %v", err) - } - for _, want := range tc.wants { - if !strings.Contains(err.Error(), want) { - t.Fatalf("error must mention %s, got %v", want, err) - } - } - }) - } -} - -func TestRequireNonInteractiveFields_AllSupplied_NoError(t *testing.T) { - t.Parallel() - cfg := &config.Config{ - URL: "https://acme.atlassian.net", Email: "u@x.io", - APIToken: "tok-1234567890", - } - if err := requireNonInteractiveFields(cfg); err != nil { - t.Fatalf("unexpected error: %v", err) - } -} - -func TestRequireNonInteractiveFields_ProxyNeedsOnlyURL(t *testing.T) { - t.Parallel() - cfg := &config.Config{ - URL: "http://127.0.0.1:8080/atlassian", - AuthMethod: auth.AuthMethodProxy, - } - if err := requireNonInteractiveFields(cfg); err != nil { - t.Fatalf("unexpected error: %v", err) - } -} - // TestRunInit_NonInteractive_MissingURL_Fails — drives runInit through // the public surface; fail-loud surfaces before any keyring work runs. func TestRunInit_NonInteractive_MissingURL_Fails(t *testing.T) { @@ -288,343 +219,6 @@ func TestRunInit_Proxy_NoTokenRequiredOrPersisted(t *testing.T) { testutil.False(t, ok) } -// finalizeInit tests use t.TempDir() for paths and an httptest-backed -// clientBuilder so the user's real config is never touched and no real -// network call is made. - -func newFinalizeReconcileResult() *reconcileResult { - return &reconcileResult{store: &credstore.Store{}} -} - -func newFinalizeOpts() *root.Options { - return &root.Options{ - Output: "table", - NoColor: true, - Stdout: &bytes.Buffer{}, - Stderr: &bytes.Buffer{}, - } -} - -func userResponseServer(t *testing.T, body string, status int) *httptest.Server { - t.Helper() - return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - testutil.Equal(t, "/wiki/rest/api/user/current", r.URL.Path) - w.WriteHeader(status) - _, _ = w.Write([]byte(body)) - })) -} - -// TestFinalizeInit_WritesConnectionToDefault verifies the §2.2 -// (MON-5328) single-source model: connection always lands in the shared -// `default` section (no per-tool override target), the token NEVER -// touches the plaintext store (keyring only, single api_token), and the -// cfl section carries no connection fields. -func TestFinalizeInit_WritesConnectionToDefault(t *testing.T) { - credtest.Hermetic(t) // t.Setenv → no t.Parallel - server := userResponseServer(t, `{"accountId":"abc","displayName":"X","email":"x@e"}`, http.StatusOK) - defer server.Close() - build := func(_ *config.Config) (*api.Client, error) { - return api.NewClient(server.URL, "x@e", "tok"), nil - } - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "config.yml") - cfg := &config.Config{URL: server.URL + "/wiki", Email: "x@e", APIToken: "tok", DefaultSpace: "SP"} - result := &reconcileResult{store: &credstore.Store{}} - - testutil.RequireNoError(t, - finalizeInit(context.Background(), newFinalizeOpts(), cfg, result, configPath, false, build)) - - loaded, err := credstore.Load(configPath) - testutil.RequireNoError(t, err) - - // Connection in default; token never in plaintext. - testutil.Equal(t, "", loaded.Default.APIToken) - testutil.Equal(t, "x@e", loaded.Default.Email) - testutil.Equal(t, server.URL, loaded.Default.URL) // /wiki stripped - // cfl section carries only the non-secret default, no connection. - testutil.Equal(t, "SP", loaded.CFL.DefaultSpace) - - // Raw file must contain no api_token and no per-tool connection key. - raw, rerr := os.ReadFile(configPath) //nolint:gosec // test reads its own temp file - testutil.RequireNoError(t, rerr) - if strings.Contains(string(raw), "api_token") { - t.Fatalf("plaintext store must never contain api_token:\n%s", raw) - } - - s, err := keyring.OpenNoMigrate() - testutil.RequireNoError(t, err) - defer func() { _ = s.Close() }() - ok, err := s.HasToken(keyring.KeyAPIToken) - testutil.RequireNoError(t, err) - testutil.True(t, ok) -} - -func TestFinalizeInit_BasicHappyPath(t *testing.T) { - credtest.Hermetic(t) // t.Setenv → no t.Parallel - server := userResponseServer(t, `{"accountId":"abc123","displayName":"Rian Stockbower","email":"rian@example.com"}`, http.StatusOK) - defer server.Close() - - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "config.yml") - opts := newFinalizeOpts() - cfg := &config.Config{ - URL: server.URL, - Email: "rian@example.com", - APIToken: "test-token", - } - - build := func(_ *config.Config) (*api.Client, error) { - return api.NewClient(server.URL, "rian@example.com", "test-token"), nil - } - - err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, false, build) - testutil.RequireNoError(t, err) - - stdout := opts.Stdout.(*bytes.Buffer).String() - testutil.Contains(t, stdout, "Connected to") - testutil.Contains(t, stdout, "Configuration saved to") - testutil.Contains(t, stdout, "abc123 | Rian Stockbower | rian@example.com") - - _, err = os.Stat(configPath) - testutil.RequireNoError(t, err) -} - -func TestFinalizeInit_BearerHappyPath(t *testing.T) { - credtest.Hermetic(t) // t.Setenv → no t.Parallel - // Server asserts that the verify request actually carries a Bearer - // Authorization header — i.e. the bearer code path emits bearer auth on - // the wire, not just bearer-themed UI copy. - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - testutil.Equal(t, "/wiki/rest/api/user/current", r.URL.Path) - testutil.Equal(t, "Bearer scoped-token", r.Header.Get("Authorization")) - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"accountId":"svc456","displayName":"Service Account","email":"svc@example.com"}`)) - })) - defer server.Close() - - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "config.yml") - opts := newFinalizeOpts() - cfg := &config.Config{ - URL: server.URL, - APIToken: "scoped-token", - AuthMethod: auth.AuthMethodBearer, - CloudID: "test-cloud-id", - } - - // Construct a real bearer-style client (auth header injected via Options) - // pointed at the httptest URL. This mirrors what api.NewBearerClient - // produces, just with a routable base URL. - build := func(c *config.Config) (*api.Client, error) { - return &api.Client{ - Client: sharedclient.New(server.URL, "", "", &sharedclient.Options{ - AuthHeader: auth.BearerAuthHeader(c.APIToken), - }), - }, nil - } - - err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, false, build) - testutil.RequireNoError(t, err) - - stdout := opts.Stdout.(*bytes.Buffer).String() - testutil.Contains(t, stdout, "Connected to") - testutil.Contains(t, stdout, "Configuration saved to") - testutil.Contains(t, stdout, "svc456 | Service Account | svc@example.com") - testutil.Contains(t, stdout, "switch back to basic auth") - - _, err = os.Stat(configPath) - testutil.RequireNoError(t, err) -} - -// TestDefaultClientBuilder verifies the production wiring between -// cfg.AuthMethod and which client constructor runs. In the finalizeInit -// tests the builder is always replaced; this test pins the default. -func TestDefaultClientBuilder(t *testing.T) { - t.Parallel() - - t.Run("basic constructs basic-auth client", func(t *testing.T) { - t.Parallel() - cfg := &config.Config{ - URL: "https://example.atlassian.net", - Email: "user@example.com", - APIToken: "secret", - } - c, err := defaultClientBuilder(cfg) - testutil.RequireNoError(t, err) - testutil.Equal(t, "https://example.atlassian.net", c.BaseURL) - // Basic auth header is "Basic "; presence of the - // "Basic " prefix is enough to confirm dispatch. - testutil.True(t, strings.HasPrefix(c.AuthHeader, "Basic "), "expected Basic prefix, got: "+c.AuthHeader) - }) - - t.Run("bearer constructs bearer-auth client at gateway", func(t *testing.T) { - t.Parallel() - cfg := &config.Config{ - APIToken: "scoped-token", - AuthMethod: auth.AuthMethodBearer, - CloudID: "cloud-abc", - } - c, err := defaultClientBuilder(cfg) - testutil.RequireNoError(t, err) - testutil.Contains(t, c.BaseURL, "/ex/confluence/cloud-abc/wiki") - testutil.Equal(t, "Bearer scoped-token", c.AuthHeader) - }) - - t.Run("bearer rejects empty cloud ID", func(t *testing.T) { - t.Parallel() - cfg := &config.Config{ - APIToken: "scoped-token", - AuthMethod: auth.AuthMethodBearer, - } - _, err := defaultClientBuilder(cfg) - testutil.RequireError(t, err) - }) - - t.Run("proxy constructs no-auth client", func(t *testing.T) { - t.Parallel() - cfg := &config.Config{ - URL: "http://127.0.0.1:8080/atlassian", - AuthMethod: auth.AuthMethodProxy, - } - c, err := defaultClientBuilder(cfg) - testutil.RequireNoError(t, err) - testutil.Equal(t, "http://127.0.0.1:8080/atlassian/wiki", c.BaseURL) - testutil.Equal(t, "", c.AuthHeader) - }) -} - -func TestFinalizeInit_ProxyNoTokenPersisted(t *testing.T) { - credtest.Hermetic(t) - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "config.yml") - opts := newFinalizeOpts() - cfg := &config.Config{ - URL: "http://127.0.0.1:8080/atlassian/wiki", - AuthMethod: auth.AuthMethodProxy, - DefaultSpace: "DEV", - } - - err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, true, defaultClientBuilder) - testutil.RequireNoError(t, err) - - loaded, err := credstore.Load(configPath) - testutil.RequireNoError(t, err) - testutil.Equal(t, "http://127.0.0.1:8080/atlassian", loaded.Default.URL) - testutil.Equal(t, "", loaded.Default.Email) - testutil.Equal(t, auth.AuthMethodProxy, loaded.Default.AuthMethod) - testutil.Equal(t, "", loaded.Default.CloudID) - testutil.Equal(t, "DEV", loaded.CFL.DefaultSpace) - - s, err := keyring.OpenNoMigrate() - testutil.RequireNoError(t, err) - defer func() { _ = s.Close() }() - ok, err := s.HasToken(keyring.KeyAPIToken) - testutil.RequireNoError(t, err) - testutil.False(t, ok) -} - -func TestFinalizeInit_AuthFailure(t *testing.T) { - t.Parallel() - server := userResponseServer(t, `{"message":"Unauthorized"}`, http.StatusUnauthorized) - defer server.Close() - - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "config.yml") - opts := newFinalizeOpts() - cfg := &config.Config{ - URL: server.URL, - Email: "rian@example.com", - APIToken: "wrong-token", - } - - build := func(_ *config.Config) (*api.Client, error) { - return api.NewClient(server.URL, "rian@example.com", "wrong-token"), nil - } - - err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, false, build) - testutil.RequireError(t, err) - testutil.Contains(t, err.Error(), "authentication failed") - - // Both the error and the remediation hint must land on stderr — splitting - // them across stdout/stderr would mean a script capturing only stderr - // sees the failure with no actionable next step. - stderr := opts.Stderr.(*bytes.Buffer).String() - testutil.Contains(t, stderr, "Connection failed") - testutil.Contains(t, stderr, "Check your credentials and try again") - - _, statErr := os.Stat(configPath) - testutil.True(t, os.IsNotExist(statErr), "config file should not exist after auth failure") -} - -func TestFinalizeInit_BuildFailureSurfacesError(t *testing.T) { - t.Parallel() - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "config.yml") - opts := newFinalizeOpts() - cfg := &config.Config{ - URL: "https://example.atlassian.net", - Email: "rian@example.com", - APIToken: "test-token", - } - - build := func(_ *config.Config) (*api.Client, error) { - return nil, errors.New("simulated builder failure") - } - - err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, false, build) - testutil.RequireError(t, err) - testutil.Contains(t, err.Error(), "simulated builder failure") - - // User must see WHY init failed, not just a non-zero exit. - stderr := opts.Stderr.(*bytes.Buffer).String() - testutil.Contains(t, stderr, "Could not construct API client") - - _, statErr := os.Stat(configPath) - testutil.True(t, os.IsNotExist(statErr), "config should not be saved when builder fails") -} - -func TestFinalizeInit_NoVerify(t *testing.T) { - credtest.Hermetic(t) // t.Setenv → no t.Parallel - httpCalled := false - server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { - httpCalled = true - })) - defer server.Close() - - tmpDir := t.TempDir() - configPath := filepath.Join(tmpDir, "config.yml") - opts := newFinalizeOpts() - cfg := &config.Config{ - URL: server.URL, - Email: "rian@example.com", - APIToken: "test-token", - } - - // Track builder invocation directly. If the noVerify guard regresses - // (e.g. moves below the build call), the server-not-called assertion - // alone wouldn't catch it — the builder running but never being used - // would still leave httpCalled=false. - builderCalled := false - build := func(_ *config.Config) (*api.Client, error) { - builderCalled = true - return api.NewClient(server.URL, "rian@example.com", "test-token"), nil - } - - err := finalizeInit(context.Background(), opts, cfg, newFinalizeReconcileResult(), configPath, true, build) - testutil.RequireNoError(t, err) - - testutil.False(t, builderCalled, "clientBuilder should not be invoked when --no-verify is set") - testutil.False(t, httpCalled, "no API call should be made when --no-verify is set") - - stdout := opts.Stdout.(*bytes.Buffer).String() - testutil.Contains(t, stdout, "Configuration saved to") - // No verify → no "Connected to" confirmation, no user one-liner. - testutil.False(t, strings.Contains(stdout, "Connected to"), "verify confirmation should not appear without verify") - - _, err = os.Stat(configPath) - testutil.RequireNoError(t, err) -} - const cflInitSentinel = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAcflInitTok" // TestRunInit_TokenStdin_PopulatesAPIToken — under --non-interactive, diff --git a/tools/cfl/internal/config/config.go b/tools/cfl/internal/config/config.go index b33569a1..c7100c98 100644 --- a/tools/cfl/internal/config/config.go +++ b/tools/cfl/internal/config/config.go @@ -44,6 +44,17 @@ type KeyringConfig struct { // For basic auth: URL + email + API token are required. // For proxy auth: only URL is required; no Authorization header is sent. func (c *Config) Validate() error { + return c.validate(true) +} + +// ValidateForInit validates config produced by init. A skipped connection +// verification permits deferring the API token to set-credential while all +// non-secret connection fields remain required and validated. +func (c *Config) ValidateForInit(noVerify bool) error { + return c.validate(!noVerify) +} + +func (c *Config) validate(requireToken bool) error { if c.URL == "" { return errors.New("url is required") } @@ -67,14 +78,14 @@ func (c *Config) Validate() error { case auth.AuthMethodProxy: return nil case auth.AuthMethodBearer: - if c.APIToken == "" { + if requireToken && c.APIToken == "" { return errors.New("api_token is required") } if c.CloudID == "" { return errors.New("cloud_id is required for bearer auth") } default: - if c.APIToken == "" { + if requireToken && c.APIToken == "" { return errors.New("api_token is required") } if c.Email == "" { diff --git a/tools/cfl/internal/config/config_test.go b/tools/cfl/internal/config/config_test.go index 1f692278..d3a0eb84 100644 --- a/tools/cfl/internal/config/config_test.go +++ b/tools/cfl/internal/config/config_test.go @@ -148,6 +148,75 @@ func TestConfig_Validate(t *testing.T) { } } +func TestConfig_ValidateForInit(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + config Config + noVerify bool + wantErr string + }{ + { + name: "basic auth without token allowed when verification skipped", + config: Config{ + URL: "https://example.atlassian.net/wiki", + Email: "user@example.com", + }, + noVerify: true, + }, + { + name: "bearer auth without token allowed when verification skipped", + config: Config{ + URL: "https://example.atlassian.net/wiki", + AuthMethod: "bearer", + CloudID: "cloud-id", + }, + noVerify: true, + }, + { + name: "token remains required when verification runs", + config: Config{ + URL: "https://example.atlassian.net/wiki", + Email: "user@example.com", + }, + wantErr: "api_token is required", + }, + { + name: "bearer cloud ID remains required when verification skipped", + config: Config{ + URL: "https://example.atlassian.net/wiki", + AuthMethod: "bearer", + }, + noVerify: true, + wantErr: "cloud_id is required", + }, + { + name: "URL scheme remains validated when verification skipped", + config: Config{ + URL: "http://example.atlassian.net/wiki", + Email: "user@example.com", + }, + noVerify: true, + wantErr: "url must use https", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := tt.config.ValidateForInit(tt.noVerify) + if tt.wantErr == "" { + testutil.NoError(t, err) + return + } + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), tt.wantErr) + }) + } +} + func TestConfig_NormalizeURL(t *testing.T) { t.Parallel() tests := []struct { diff --git a/tools/jtk/internal/cmd/initcmd/initcmd.go b/tools/jtk/internal/cmd/initcmd/initcmd.go index f7e2f440..1da875df 100644 --- a/tools/jtk/internal/cmd/initcmd/initcmd.go +++ b/tools/jtk/internal/cmd/initcmd/initcmd.go @@ -312,7 +312,10 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, // can't run — every required value must already be in cfg from the // flag prefills. Fail loud naming the first missing field. if !prompt.WantPrompt(opts.NonInteractive, opts.Stdin) { - if err := requireNonInteractiveFields(cfg); err != nil { + if err := auth.RequireNonInteractiveFields( + cfg.URL, cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID, + "jtk set-credential --ref atlassian-cli/default --key api_token --stdin", + ); err != nil { return err } } else { @@ -449,40 +452,14 @@ func finalizeInit( v.Println(" jtk me") v.Println(" jtk issues list --project ") - if cfg.AuthMethod == auth.AuthMethodBearer { + switch cfg.AuthMethod { + case auth.AuthMethodBearer: v.Println("") v.Info("To switch back to basic auth later, run: jtk init --auth-method basic") - } else if cfg.AuthMethod == auth.AuthMethodProxy { + case auth.AuthMethodProxy: v.Println("") v.Info("To switch to direct authentication later, run: jtk init --auth-method basic") } return nil } - -// requireNonInteractiveFields enforces the §3.4 fail-loud contract for -// scripted/CI runs of `jtk init`: any required value missing from the -// flag prefills (which already populated cfg) produces an error naming -// the first missing field, with the auth-mode shape baked into the -// message so the operator knows which flag set is required. -func requireNonInteractiveFields(cfg *config.Config) error { - if cfg.URL == "" { - return fmt.Errorf("--non-interactive: missing required value for --url") - } - switch cfg.AuthMethod { - case auth.AuthMethodProxy: - return nil - case auth.AuthMethodBearer: - if cfg.CloudID == "" { - return fmt.Errorf("--non-interactive: missing required value for --cloud-id (bearer auth)") - } - default: - if cfg.Email == "" { - return fmt.Errorf("--non-interactive: missing required value for --email (basic auth)") - } - } - if cfg.APIToken == "" { - return fmt.Errorf("--non-interactive: missing required value for --token-stdin or --token-from-env VAR (or pre-stage with `jtk set-credential --ref atlassian-cli/default --key api_token --stdin`)") - } - return nil -} diff --git a/tools/jtk/internal/cmd/initcmd/initcmd_test.go b/tools/jtk/internal/cmd/initcmd/initcmd_test.go index 3cf81c57..ec82a326 100644 --- a/tools/jtk/internal/cmd/initcmd/initcmd_test.go +++ b/tools/jtk/internal/cmd/initcmd/initcmd_test.go @@ -68,86 +68,6 @@ func TestRunInit_InvalidAuthMethod(t *testing.T) { // The non-interactive paths (all flags provided) still use huh forms internally, // so we test config loading/saving separately -// TestRequireNonInteractiveFields_NamesFirstMissing pins the §3.4 -// fail-loud message shape so the family pattern (jtk + cfl + future -// nrq-aligned ports) stays consistent. The wizard wrapper isn't tested -// directly because it depends on huh form state we can't easily fake -// — the helper IS the contract for what gets named. -func TestRequireNonInteractiveFields_NamesFirstMissing(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - cfg *config.Config - wants []string - }{ - { - name: "basic auth — missing URL", - cfg: &config.Config{}, - wants: []string{"--url"}, - }, - { - name: "basic auth — missing email", - cfg: &config.Config{URL: "https://acme.atlassian.net"}, - wants: []string{"--email"}, - }, - { - name: "bearer — missing cloud-id", - cfg: &config.Config{URL: "https://acme.atlassian.net", AuthMethod: auth.AuthMethodBearer}, - wants: []string{"--cloud-id"}, - }, - { - name: "basic auth — missing token recommends --token-stdin + --token-from-env + set-credential", - cfg: &config.Config{URL: "https://acme.atlassian.net", Email: "u@x.io"}, - wants: []string{"--token-stdin", "--token-from-env", "set-credential"}, - }, - { - name: "bearer — missing token recommends --token-stdin + --token-from-env + set-credential", - cfg: &config.Config{URL: "https://acme.atlassian.net", CloudID: "cid", AuthMethod: auth.AuthMethodBearer}, - wants: []string{"--token-stdin", "--token-from-env", "set-credential"}, - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - err := requireNonInteractiveFields(tc.cfg) - testutil.RequireError(t, err) - if !strings.Contains(err.Error(), "--non-interactive: missing") { - t.Fatalf("error must mention --non-interactive prefix: %v", err) - } - for _, want := range tc.wants { - if !strings.Contains(err.Error(), want) { - t.Fatalf("error must name %s, got %v", want, err) - } - } - }) - } -} - -// TestRequireNonInteractiveFields_AllSupplied_NoError — happy path; no -// error returned when every required field is present. -func TestRequireNonInteractiveFields_AllSupplied_NoError(t *testing.T) { - t.Parallel() - cfg := &config.Config{ - URL: "https://acme.atlassian.net", Email: "u@x.io", - APIToken: "tok-1234567890", - } - if err := requireNonInteractiveFields(cfg); err != nil { - t.Fatalf("unexpected error: %v", err) - } -} - -func TestRequireNonInteractiveFields_ProxyNeedsOnlyURL(t *testing.T) { - t.Parallel() - cfg := &config.Config{ - URL: "http://127.0.0.1:8080/atlassian", - AuthMethod: auth.AuthMethodProxy, - } - if err := requireNonInteractiveFields(cfg); err != nil { - t.Fatalf("unexpected error: %v", err) - } -} - // TestRunInit_NonInteractive_MissingURL_Fails — drives runInit through // the public surface with --non-interactive but no flags supplied. The // fail-loud message must surface BEFORE any keyring/migration work runs. From f1b653728ce0d0bf2868b2eb240647fda60cb7ad Mon Sep 17 00:00:00 2001 From: Bazyli Brzoska Date: Wed, 29 Jul 2026 23:31:48 -0700 Subject: [PATCH 4/6] test(cfl): fix config tests after rebase Update proxy config coverage for the current loader API and isolate shared-config precedence tests from ambient connection environment variables. --- tools/cfl/internal/cmd/root/root_test.go | 9 ++++++++- tools/cfl/internal/config/config_test.go | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tools/cfl/internal/cmd/root/root_test.go b/tools/cfl/internal/cmd/root/root_test.go index 533a1057..e08457e1 100644 --- a/tools/cfl/internal/cmd/root/root_test.go +++ b/tools/cfl/internal/cmd/root/root_test.go @@ -451,6 +451,13 @@ func TestDefaultConfigLayersSharedValues(t *testing.T) { credtest.Hermetic(t) keyring.SetBackendSelection("", "") t.Cleanup(func() { keyring.SetBackendSelection("", "") }) + for _, name := range []string{ + "CFL_URL", "ATLASSIAN_URL", "CFL_EMAIL", "ATLASSIAN_EMAIL", + "CFL_AUTH_METHOD", "ATLASSIAN_AUTH_METHOD", "CFL_CLOUD_ID", + "ATLASSIAN_CLOUD_ID", "CFL_DEFAULT_SPACE", + } { + t.Setenv(name, "") + } t.Setenv("CFL_API_TOKEN", "token") legacy := &config.Config{ @@ -483,7 +490,7 @@ func TestDefaultConfigLayersSharedValues(t *testing.T) { if cfg.URL != "https://shared.atlassian.net/wiki" || cfg.Email != "shared@example.com" || cfg.AuthMethod != auth.AuthMethodBearer || cfg.CloudID != "shared-cloud" || cfg.DefaultSpace != "SHARED" || cfg.OutputFormat != "shared-output" { - t.Fatal("default config did not layer shared values") + t.Fatalf("default config did not layer shared values: %+v", cfg) } return nil }, diff --git a/tools/cfl/internal/config/config_test.go b/tools/cfl/internal/config/config_test.go index d3a0eb84..8fb1a8c8 100644 --- a/tools/cfl/internal/config/config_test.go +++ b/tools/cfl/internal/config/config_test.go @@ -693,7 +693,7 @@ func TestLoadWithEnv_ProxySkipsTokenResolution(t *testing.T) { t.Setenv("ATLASSIAN_AUTH_METHOD", "proxy") t.Setenv("ATLASSIAN_API_TOKEN", "env-token-that-should-be-ignored") - cfg, err := LoadWithEnv(filepath.Join(t.TempDir(), "missing.yml")) + cfg, err := LoadWithEnv(filepath.Join(t.TempDir(), "missing.yml"), true, true) testutil.RequireNoError(t, err) testutil.Equal(t, "http://127.0.0.1:8080/atlassian", cfg.URL) testutil.Equal(t, "proxy", cfg.AuthMethod) From fa09b17da31a85bd69ccd873dc204700b11ee8cf Mon Sep 17 00:00:00 2001 From: Bazyli Brzoska Date: Wed, 29 Jul 2026 23:33:21 -0700 Subject: [PATCH 5/6] test(cfl): isolate config show environment Clear ambient connection overrides so the explicit-config path fixture verifies file selection deterministically. --- tools/cfl/internal/cmd/configcmd/show_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/cfl/internal/cmd/configcmd/show_test.go b/tools/cfl/internal/cmd/configcmd/show_test.go index c5129f04..bcb7b5ed 100644 --- a/tools/cfl/internal/cmd/configcmd/show_test.go +++ b/tools/cfl/internal/cmd/configcmd/show_test.go @@ -83,6 +83,13 @@ func TestRunShow_ExactOutput(t *testing.T) { func TestRunShow_UsesConfiguredPath(t *testing.T) { credtest.Hermetic(t) + for _, name := range []string{ + "CFL_URL", "ATLASSIAN_URL", "CFL_EMAIL", "ATLASSIAN_EMAIL", + "CFL_DEFAULT_SPACE", "CFL_AUTH_METHOD", "ATLASSIAN_AUTH_METHOD", + "CFL_CLOUD_ID", "ATLASSIAN_CLOUD_ID", + } { + t.Setenv(name, "") + } defaultPath := cflconfig.DefaultConfigPath() explicitPath := filepath.Join(t.TempDir(), "explicit.yml") testutil.RequireNoError(t, (&cflconfig.Config{URL: "https://default.atlassian.net/wiki", Email: "default@example.com"}).Save(defaultPath)) From 7988e1f45256beb8ef5b41530deaebf110767e21 Mon Sep 17 00:00:00 2001 From: Bazyli Brzoska Date: Sat, 26 Sep 2026 18:57:59 -0700 Subject: [PATCH 6/6] fix: address remaining proxy auth review findings --- shared/auth/auth.go | 42 ++++++----- shared/auth/auth_test.go | 24 +++---- shared/url/url.go | 21 ++++++ shared/url/url_test.go | 34 ++++++++- tools/cfl/README.md | 2 + tools/cfl/api/client.go | 72 ++++++++++++++++--- tools/cfl/api/client_test.go | 20 ++++-- tools/cfl/docs/development.md | 2 +- tools/cfl/internal/cmd/init/init.go | 30 ++++---- tools/cfl/internal/cmd/root/root.go | 14 ++-- tools/cfl/internal/cmd/root/root_test.go | 9 +++ tools/cfl/internal/config/config.go | 8 ++- tools/cfl/internal/config/config_test.go | 5 ++ tools/jtk/README.md | 2 + tools/jtk/api/client.go | 30 +++++--- tools/jtk/api/client_test.go | 27 +++++-- tools/jtk/docs/development.md | 2 +- tools/jtk/internal/cmd/initcmd/initcmd.go | 34 +++++---- .../jtk/internal/cmd/initcmd/initcmd_test.go | 18 +++++ tools/jtk/internal/cmd/root/root.go | 14 ++-- tools/jtk/internal/cmd/root/root_test.go | 14 ++++ 21 files changed, 314 insertions(+), 110 deletions(-) diff --git a/shared/auth/auth.go b/shared/auth/auth.go index cef563a5..4aa85034 100644 --- a/shared/auth/auth.go +++ b/shared/auth/auth.go @@ -31,44 +31,50 @@ func ValidateAuthMethod(method string) error { } } -// NormalizeConfig applies auth-method policy to config credential fields. -// -// Empty auth method defaults to basic. Proxy auth sends no CLI-side -// credentials, so direct credential fields are cleared. -func NormalizeConfig(authMethod, email, apiToken, cloudID string) (string, string, string, string) { - if authMethod == "" { - authMethod = AuthMethodBasic +// Credentials groups fields governed by auth-method policy. +type Credentials struct { + Method string + Email string + APIToken string + CloudID string +} + +// Normalize defaults an empty method to basic and clears direct credentials +// for proxy auth, which sends no CLI-side credentials. +func (c Credentials) Normalize() Credentials { + if c.Method == "" { + c.Method = AuthMethodBasic } - if authMethod == AuthMethodProxy { - email = "" - apiToken = "" - cloudID = "" + if c.Method == AuthMethodProxy { + c.Email = "" + c.APIToken = "" + c.CloudID = "" } - return authMethod, email, apiToken, cloudID + return c } -// RequireNonInteractiveFields validates the auth fields needed by scripted +// RequireNonInteractive validates the auth fields needed by scripted // init flows and names the first missing CLI value. toolHint is the // tool-specific set-credential command shown when the API token is absent. -func RequireNonInteractiveFields(url, authMethod, email, apiToken, cloudID, toolHint string) error { +func (c Credentials) RequireNonInteractive(url, toolHint string) error { if url == "" { return errors.New("--non-interactive: missing required value for --url") } - switch authMethod { + switch c.Method { case AuthMethodProxy: return nil case AuthMethodBearer: - if cloudID == "" { + if c.CloudID == "" { return errors.New("--non-interactive: missing required value for --cloud-id (bearer auth)") } default: - if email == "" { + if c.Email == "" { return errors.New("--non-interactive: missing required value for --email (basic auth)") } } - if apiToken == "" { + if c.APIToken == "" { return fmt.Errorf("--non-interactive: missing required value for --token-stdin or --token-from-env VAR (or pre-stage with `%s`)", toolHint) } return nil diff --git a/shared/auth/auth_test.go b/shared/auth/auth_test.go index 5a23ef30..49e59d62 100644 --- a/shared/auth/auth_test.go +++ b/shared/auth/auth_test.go @@ -203,21 +203,19 @@ func TestNormalizeConfig(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - gotAuthMethod, gotEmail, gotAPIToken, gotCloudID := NormalizeConfig( - tt.authMethod, tt.email, tt.apiToken, tt.cloudID, - ) + got := (Credentials{Method: tt.authMethod, Email: tt.email, APIToken: tt.apiToken, CloudID: tt.cloudID}).Normalize() - if gotAuthMethod != tt.wantAuthMethod { - t.Errorf("auth method = %q, want %q", gotAuthMethod, tt.wantAuthMethod) + if got.Method != tt.wantAuthMethod { + t.Errorf("auth method = %q, want %q", got.Method, tt.wantAuthMethod) } - if gotEmail != tt.wantEmail { - t.Errorf("email = %q, want %q", gotEmail, tt.wantEmail) + if got.Email != tt.wantEmail { + t.Errorf("email = %q, want %q", got.Email, tt.wantEmail) } - if gotAPIToken != tt.wantAPIToken { - t.Errorf("api token = %q, want %q", gotAPIToken, tt.wantAPIToken) + if got.APIToken != tt.wantAPIToken { + t.Errorf("api token = %q, want %q", got.APIToken, tt.wantAPIToken) } - if gotCloudID != tt.wantCloudID { - t.Errorf("cloud ID = %q, want %q", gotCloudID, tt.wantCloudID) + if got.CloudID != tt.wantCloudID { + t.Errorf("cloud ID = %q, want %q", got.CloudID, tt.wantCloudID) } }) } @@ -280,9 +278,7 @@ func TestRequireNonInteractiveFields(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := RequireNonInteractiveFields( - tt.url, tt.authMethod, tt.email, tt.apiToken, tt.cloudID, toolHint, - ) + err := (Credentials{Method: tt.authMethod, Email: tt.email, APIToken: tt.apiToken, CloudID: tt.cloudID}).RequireNonInteractive(tt.url, toolHint) if tt.wantError == "" { if err != nil { t.Fatalf("RequireNonInteractiveFields() error = %v, want nil", err) diff --git a/shared/url/url.go b/shared/url/url.go index 94b93953..b671927c 100644 --- a/shared/url/url.go +++ b/shared/url/url.go @@ -2,11 +2,32 @@ package url import ( + "errors" "net/netip" neturl "net/url" "strings" ) +// ErrRequiresHTTPS is returned for a cleartext non-loopback URL. +var ErrRequiresHTTPS = errors.New("url must use https unless it is loopback http") + +// RequireSecureOrLoopback permits HTTPS endpoints and local HTTP endpoints. +// Scheme-less URLs are normalized to HTTPS, as they are by API clients. +func RequireSecureOrLoopback(u string) error { + if strings.Contains(u, "://") && !HasScheme(u) { + return ErrRequiresHTTPS + } + u = NormalizeURL(u) + parsed, err := neturl.Parse(u) + if err != nil || parsed.Host == "" { + return ErrRequiresHTTPS + } + if parsed.Scheme == "https" || IsLoopbackHTTP(u) { + return nil + } + return ErrRequiresHTTPS +} + // NormalizeURL ensures the URL has an https scheme and no trailing slashes. // If the URL is empty, it returns an empty string. // If the URL has no scheme, https:// is prepended. diff --git a/shared/url/url_test.go b/shared/url/url_test.go index 35645241..41615317 100644 --- a/shared/url/url_test.go +++ b/shared/url/url_test.go @@ -1,6 +1,38 @@ package url //nolint:revive // test file for url package -import "testing" +import ( + "errors" + "testing" +) + +func TestRequireSecureOrLoopback(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + url string + valid bool + }{ + {"https://proxy.example.com/path", true}, + {"proxy.example.com/path", true}, + {"http://localhost:8080/path", true}, + {"http://127.0.0.1:8080/path", true}, + {"http://[::1]:8080/path", true}, + {"http://proxy.example.com/path", false}, + {"ftp://proxy.example.com/path", false}, + {"https://", false}, + {"", false}, + } { + t.Run(tt.url, func(t *testing.T) { + t.Parallel() + err := RequireSecureOrLoopback(tt.url) + if tt.valid && err != nil { + t.Fatalf("RequireSecureOrLoopback(%q) = %v, want nil", tt.url, err) + } + if !tt.valid && !errors.Is(err, ErrRequiresHTTPS) { + t.Fatalf("RequireSecureOrLoopback(%q) = %v, want ErrRequiresHTTPS", tt.url, err) + } + }) + } +} func TestNormalizeURL(t *testing.T) { t.Parallel() diff --git a/tools/cfl/README.md b/tools/cfl/README.md index 25b294d1..5da7586e 100644 --- a/tools/cfl/README.md +++ b/tools/cfl/README.md @@ -894,6 +894,8 @@ Environment variables override file-based config. Variables are checked in order | Cloud ID | `CFL_CLOUD_ID` → `ATLASSIAN_CLOUD_ID` → shared `default` → legacy | | Bearer Gateway Base URL | `CFL_GATEWAY_BASE_URL` → `ATLASSIAN_GATEWAY_BASE_URL` → `https://api.atlassian.com` | +Gateway overrides must use HTTPS, except for loopback HTTP endpoints used in local testing. + Per §2.2 connection config is single-sourced from the shared `default` section — per-tool `cfl:`/`jtk:` sections carry only non-secret defaults and may not override `url`/`email`/`auth_method`/`cloud_id`. Set `ATLASSIAN_AUTH_METHOD=proxy` (or `CFL_AUTH_METHOD=proxy`) with `ATLASSIAN_URL`/`CFL_URL` to use a trusted proxy. Proxy auth ignores email, token, and cloud ID because the proxy is responsible for upstream authentication. diff --git a/tools/cfl/api/client.go b/tools/cfl/api/client.go index dd2cc9d0..657ecd2a 100644 --- a/tools/cfl/api/client.go +++ b/tools/cfl/api/client.go @@ -18,7 +18,9 @@ import ( var ( ErrAPITokenRequired = errors.New("API token is required") ErrCloudIDRequired = errors.New("cloud ID is required for bearer auth") - ErrProxyURLRequiresHTTPS = errors.New("proxy auth URL must use https unless it is loopback http") + ErrURLRequired = errors.New("URL is required") + ErrEmailRequired = errors.New("email is required") + ErrProxyURLRequiresHTTPS = sharedurl.ErrRequiresHTTPS ) // Client is the Confluence Cloud API client. @@ -27,6 +29,42 @@ type Client struct { *client.Client } +// ClientConfig contains the inputs for a Confluence API client. +type ClientConfig struct { + URL string + Email string + APIToken string + AuthMethod string + CloudID string + GatewayBaseURL string // Optional bearer gateway base; defaults to api.atlassian.com +} + +// New dispatches all supported authentication methods through one entry point. +func New(cfg ClientConfig) (*Client, error) { + if cfg.AuthMethod != "" { + if err := auth.ValidateAuthMethod(cfg.AuthMethod); err != nil { + return nil, err + } + } + switch cfg.AuthMethod { + case auth.AuthMethodBearer: + return newBearerClient(cfg) + case auth.AuthMethodProxy: + return newProxyClient(cfg.URL) + default: + if cfg.URL == "" { + return nil, ErrURLRequired + } + if cfg.Email == "" { + return nil, ErrEmailRequired + } + if cfg.APIToken == "" { + return nil, ErrAPITokenRequired + } + return NewClient(cfg.URL, cfg.Email, cfg.APIToken), nil + } +} + // NewClient creates a new Confluence API client using basic auth. func NewClient(baseURL, email, apiToken string) *Client { return &Client{ @@ -37,10 +75,17 @@ func NewClient(baseURL, email, apiToken string) *Client { // NewProxyClient creates a Confluence client for a trusted proxy that injects // authentication upstream. No Authorization header is sent by the CLI. func NewProxyClient(baseURL string) (*Client, error) { - normalized := normalizeWikiBaseURL(baseURL) - if strings.HasPrefix(normalized, "http://") && !sharedurl.IsLoopbackHTTP(normalized) { - return nil, ErrProxyURLRequiresHTTPS + return New(ClientConfig{URL: baseURL, AuthMethod: auth.AuthMethodProxy}) +} + +func newProxyClient(baseURL string) (*Client, error) { + if baseURL == "" { + return nil, ErrURLRequired + } + if err := sharedurl.RequireSecureOrLoopback(baseURL); err != nil { + return nil, err } + normalized := normalizeWikiBaseURL(baseURL) return &Client{ Client: client.New(normalized, "", "", &client.Options{SkipAuthHeader: true}), }, nil @@ -49,15 +94,26 @@ func NewProxyClient(baseURL string) (*Client, error) { // NewBearerClient creates a new Confluence API client using bearer auth via the API gateway. // The cloudID is used to construct the gateway URL: https://api.atlassian.com/ex/confluence/{cloudId}/wiki func NewBearerClient(apiToken, cloudID string) (*Client, error) { - if apiToken == "" { + return New(ClientConfig{APIToken: apiToken, CloudID: cloudID, AuthMethod: auth.AuthMethodBearer}) +} + +func newBearerClient(cfg ClientConfig) (*Client, error) { + if cfg.APIToken == "" { return nil, ErrAPITokenRequired } - if cloudID == "" { + if cfg.CloudID == "" { return nil, ErrCloudIDRequired } - gatewayBase := fmt.Sprintf("%s/ex/confluence/%s/wiki", client.GatewayBaseURLFromEnv("CFL_GATEWAY_BASE_URL"), cloudID) + gatewayURL := cfg.GatewayBaseURL + if gatewayURL == "" { + gatewayURL = client.GatewayBaseURL + } + if err := sharedurl.RequireSecureOrLoopback(gatewayURL); err != nil { + return nil, fmt.Errorf("invalid gateway base URL: %w", err) + } + gatewayBase := fmt.Sprintf("%s/ex/confluence/%s/wiki", sharedurl.NormalizeURL(gatewayURL), cfg.CloudID) opts := &client.Options{ - AuthHeader: auth.BearerAuthHeader(apiToken), + AuthHeader: auth.BearerAuthHeader(cfg.APIToken), } return &Client{ Client: client.New(gatewayBase, "", "", opts), diff --git a/tools/cfl/api/client_test.go b/tools/cfl/api/client_test.go index 418c0bc4..2fe7dbeb 100644 --- a/tools/cfl/api/client_test.go +++ b/tools/cfl/api/client_test.go @@ -277,10 +277,22 @@ func TestNewBearerClient(t *testing.T) { }) } -func TestNewBearerClient_GatewayBaseFromEnv(t *testing.T) { - t.Setenv("CFL_GATEWAY_BASE_URL", "https://gateway.example/") - t.Setenv("ATLASSIAN_GATEWAY_BASE_URL", "") - c, err := NewBearerClient("scoped-token", "abc-123") +func TestNew_BearerGatewayOverride(t *testing.T) { + c, err := New(ClientConfig{APIToken: "scoped-token", CloudID: "abc-123", AuthMethod: "bearer", GatewayBaseURL: "https://gateway.example/"}) testutil.RequireNoError(t, err) testutil.Equal(t, "https://gateway.example/ex/confluence/abc-123/wiki", c.GetBaseURL()) } + +func TestNew_BearerRejectsCleartextGateway(t *testing.T) { + c, err := New(ClientConfig{APIToken: "token", CloudID: "cloud", AuthMethod: "bearer", GatewayBaseURL: "http://external.example"}) + if !errors.Is(err, ErrProxyURLRequiresHTTPS) || c != nil { + t.Fatalf("New() = (%v, %v), want secure URL error", c, err) + } +} + +func TestNew_BearerDoesNotReadGatewayEnv(t *testing.T) { + t.Setenv("CFL_GATEWAY_BASE_URL", "https://ambient.example") + c, err := New(ClientConfig{APIToken: "token", CloudID: "cloud", AuthMethod: "bearer"}) + testutil.RequireNoError(t, err) + testutil.Equal(t, "https://api.atlassian.com/ex/confluence/cloud/wiki", c.GetBaseURL()) +} diff --git a/tools/cfl/docs/development.md b/tools/cfl/docs/development.md index e2297c97..6cfa2a74 100644 --- a/tools/cfl/docs/development.md +++ b/tools/cfl/docs/development.md @@ -84,7 +84,7 @@ Add new Confluence macros through `MacroRegistry` in `macro.go`; the tokenizer, `cfl` participates in the shared Atlassian credential/config model described by the monorepo guide. `ATLASSIAN_*` variables apply across both tools; `CFL_*` variables override for cfl. The cfl-specific config section carries non-secret defaults such as `default_space` and `output_format`. -Basic auth uses an instance URL plus email and token. Bearer auth routes through `api.atlassian.com` and requires a cloud ID. Proxy auth uses only a URL, sends no `Authorization` header, and allows loopback `http://` URLs for trusted local proxies while keeping arbitrary cleartext proxy URLs rejected. Bearer gateway routing can be overridden with `CFL_GATEWAY_BASE_URL` or `ATLASSIAN_GATEWAY_BASE_URL`. `cfl init` and `cfl me` verify against Confluence's current-user endpoint. +Basic auth uses an instance URL plus email and token. Bearer auth routes through `api.atlassian.com` and requires a cloud ID. Proxy auth uses only a URL, sends no `Authorization` header, and allows loopback `http://` URLs for trusted local proxies while keeping arbitrary cleartext proxy URLs rejected. Bearer gateway routing can be overridden with `CFL_GATEWAY_BASE_URL` or `ATLASSIAN_GATEWAY_BASE_URL`; overrides must use HTTPS or loopback HTTP. `cfl init` and `cfl me` verify against Confluence's current-user endpoint. ## Output diff --git a/tools/cfl/internal/cmd/init/init.go b/tools/cfl/internal/cmd/init/init.go index d4ed47bd..a702ca63 100644 --- a/tools/cfl/internal/cmd/init/init.go +++ b/tools/cfl/internal/cmd/init/init.go @@ -11,6 +11,7 @@ import ( "github.com/spf13/cobra" "github.com/open-cli-collective/atlassian-go/auth" + "github.com/open-cli-collective/atlassian-go/client" "github.com/open-cli-collective/atlassian-go/credstore" "github.com/open-cli-collective/atlassian-go/keyring" "github.com/open-cli-collective/atlassian-go/prompt" @@ -23,17 +24,15 @@ import ( // clientBuilder constructs an *api.Client from a config. // Pulled out as a parameter so tests can inject an httptest-pointed client -// without depending on api.NewBearerClient's gateway URL. +// without depending on the bearer gateway URL. type clientBuilder func(cfg *config.Config) (*api.Client, error) func defaultClientBuilder(cfg *config.Config) (*api.Client, error) { - if cfg.AuthMethod == auth.AuthMethodBearer { - return api.NewBearerClient(cfg.APIToken, cfg.CloudID) - } - if cfg.AuthMethod == auth.AuthMethodProxy { - return api.NewProxyClient(cfg.URL) - } - return api.NewClient(cfg.URL, cfg.Email, cfg.APIToken), nil + return api.New(api.ClientConfig{ + URL: cfg.URL, Email: cfg.Email, APIToken: cfg.APIToken, + AuthMethod: cfg.AuthMethod, CloudID: cfg.CloudID, + GatewayBaseURL: client.GatewayBaseURLFromEnv("CFL_GATEWAY_BASE_URL"), + }) } // Register adds the init command to the root command. @@ -322,10 +321,9 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s // (via `cfl set-credential`). Fail loud naming the first missing // field. if !prompt.WantPrompt(opts.NonInteractive, opts.Stdin) { - if err := auth.RequireNonInteractiveFields( - cfg.URL, cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID, - "cfl set-credential --ref atlassian-cli/default --key api_token --stdin", - ); err != nil { + credentials := auth.Credentials{Method: cfg.AuthMethod, Email: cfg.Email, APIToken: cfg.APIToken, CloudID: cfg.CloudID} + if err := credentials.RequireNonInteractive(cfg.URL, + "cfl set-credential --ref atlassian-cli/default --key api_token --stdin"); err != nil { return err } } else { @@ -336,9 +334,11 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s } cfg.NormalizeURL() - cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID = auth.NormalizeConfig( - cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID, - ) + credentials := (auth.Credentials{Method: cfg.AuthMethod, Email: cfg.Email, APIToken: cfg.APIToken, CloudID: cfg.CloudID}).Normalize() + cfg.AuthMethod = credentials.Method + cfg.Email = credentials.Email + cfg.APIToken = credentials.APIToken + cfg.CloudID = credentials.CloudID if err := cfg.ValidateForInit(noVerify); err != nil { return fmt.Errorf("invalid configuration: %w", err) diff --git a/tools/cfl/internal/cmd/root/root.go b/tools/cfl/internal/cmd/root/root.go index 6eecb357..94fbe015 100644 --- a/tools/cfl/internal/cmd/root/root.go +++ b/tools/cfl/internal/cmd/root/root.go @@ -11,7 +11,7 @@ import ( cccredstore "github.com/open-cli-collective/cli-common/credstore" "github.com/open-cli-collective/atlassian-go/artifact" - "github.com/open-cli-collective/atlassian-go/auth" + "github.com/open-cli-collective/atlassian-go/client" "github.com/open-cli-collective/atlassian-go/keyring" "github.com/open-cli-collective/atlassian-go/present" "github.com/open-cli-collective/atlassian-go/version" @@ -141,13 +141,11 @@ func (o *Options) APIClient() (*api.Client, error) { if err != nil { return nil, err } - if cfg.AuthMethod == auth.AuthMethodBearer { - return api.NewBearerClient(cfg.APIToken, cfg.CloudID) - } - if cfg.AuthMethod == auth.AuthMethodProxy { - return api.NewProxyClient(cfg.URL) - } - return api.NewClient(cfg.URL, cfg.Email, cfg.APIToken), nil + return api.New(api.ClientConfig{ + URL: cfg.URL, Email: cfg.Email, APIToken: cfg.APIToken, + AuthMethod: cfg.AuthMethod, CloudID: cfg.CloudID, + GatewayBaseURL: client.GatewayBaseURLFromEnv("CFL_GATEWAY_BASE_URL"), + }) } // SetAPIClient sets a test client (for testing only) diff --git a/tools/cfl/internal/cmd/root/root_test.go b/tools/cfl/internal/cmd/root/root_test.go index e08457e1..31191b4d 100644 --- a/tools/cfl/internal/cmd/root/root_test.go +++ b/tools/cfl/internal/cmd/root/root_test.go @@ -183,6 +183,15 @@ func TestOptions_APIClient_ProxyRejectsArbitraryHTTP(t *testing.T) { } } +func TestOptions_APIClient_UsesGatewayOverride(t *testing.T) { + t.Setenv("CFL_GATEWAY_BASE_URL", "https://gateway.example/") + _, opts := NewCmd() + opts.SetConfig(&config.Config{URL: "https://example.atlassian.net/wiki", APIToken: "token", AuthMethod: auth.AuthMethodBearer, CloudID: "cloud-123"}) + c, err := opts.APIClient() + testutil.RequireNoError(t, err) + testutil.Equal(t, "https://gateway.example/ex/confluence/cloud-123/wiki", c.GetBaseURL()) +} + func TestValidateOutputFormat(t *testing.T) { t.Parallel() diff --git a/tools/cfl/internal/config/config.go b/tools/cfl/internal/config/config.go index c7100c98..cc54b6f2 100644 --- a/tools/cfl/internal/config/config.go +++ b/tools/cfl/internal/config/config.go @@ -68,10 +68,12 @@ func (c *Config) validate(requireToken bool) error { // Validate URL scheme. Proxy auth may use loopback http for a local proxy; // all other cleartext URLs are rejected. - if !strings.HasPrefix(c.URL, "https://") { - if c.AuthMethod != auth.AuthMethodProxy || !sharedurl.IsLoopbackHTTP(c.URL) { - return errors.New("url must use https unless proxy auth uses loopback http") + if c.AuthMethod == auth.AuthMethodProxy { + if err := sharedurl.RequireSecureOrLoopback(c.URL); err != nil { + return err } + } else if !strings.HasPrefix(c.URL, "https://") { + return errors.New("url must use https unless proxy auth uses loopback http") } switch c.AuthMethod { diff --git a/tools/cfl/internal/config/config_test.go b/tools/cfl/internal/config/config_test.go index 8fb1a8c8..6fb25d24 100644 --- a/tools/cfl/internal/config/config_test.go +++ b/tools/cfl/internal/config/config_test.go @@ -82,6 +82,11 @@ func TestConfig_Validate(t *testing.T) { }, wantErr: false, }, + { + name: "proxy with scheme-less URL defaults to https", + config: Config{URL: "proxy.example.com/atlassian/wiki", AuthMethod: "proxy"}, + wantErr: false, + }, { name: "proxy rejects arbitrary http URL", config: Config{ diff --git a/tools/jtk/README.md b/tools/jtk/README.md index 26582118..219d2ca2 100644 --- a/tools/jtk/README.md +++ b/tools/jtk/README.md @@ -1731,6 +1731,8 @@ Environment variables override file-based config. Variables are checked in order | Cloud ID | `JIRA_CLOUD_ID` → `ATLASSIAN_CLOUD_ID` → shared `default` → legacy | | Bearer Gateway Base URL | `JIRA_GATEWAY_BASE_URL` → `ATLASSIAN_GATEWAY_BASE_URL` → `https://api.atlassian.com` | +Gateway overrides must use HTTPS, except for loopback HTTP endpoints used in local testing. + Per §2.2 connection config is single-sourced from the shared `default` section — per-tool `cfl:`/`jtk:` sections carry only non-secret defaults and may not override `url`/`email`/`auth_method`/`cloud_id`. Set `ATLASSIAN_AUTH_METHOD=proxy` (or `JIRA_AUTH_METHOD=proxy`) with `ATLASSIAN_URL`/`JIRA_URL` to use a trusted proxy. Proxy auth ignores email, token, and cloud ID because the proxy is responsible for upstream authentication. diff --git a/tools/jtk/api/client.go b/tools/jtk/api/client.go index 2a8eff6a..bb75d5dc 100644 --- a/tools/jtk/api/client.go +++ b/tools/jtk/api/client.go @@ -35,12 +35,13 @@ type Client struct { // ClientConfig contains configuration for creating a new client type ClientConfig struct { - URL string // Full Jira URL (e.g., https://mycompany.atlassian.net or https://jira.internal.corp.com) - Email string - APIToken string - Verbose bool - AuthMethod string // "basic" (default), "bearer", or "proxy" - CloudID string // Required for bearer auth (used to construct gateway URL) + URL string // Full Jira URL (e.g., https://mycompany.atlassian.net or https://jira.internal.corp.com) + Email string + APIToken string + Verbose bool + AuthMethod string // "basic" (default), "bearer", or "proxy" + CloudID string // Required for bearer auth (used to construct gateway URL) + GatewayBaseURL string // Optional bearer gateway base; defaults to api.atlassian.com } // New creates a new Jira API client from config. @@ -104,7 +105,14 @@ func newBearerClient(cfg ClientConfig) (*Client, error) { instanceURL := url.NormalizeURL(cfg.URL) // Gateway URLs for bearer auth - gatewayBase := fmt.Sprintf("%s/ex/jira/%s", client.GatewayBaseURLFromEnv("JIRA_GATEWAY_BASE_URL"), cfg.CloudID) + gatewayURL := cfg.GatewayBaseURL + if gatewayURL == "" { + gatewayURL = client.GatewayBaseURL + } + if err := url.RequireSecureOrLoopback(gatewayURL); err != nil { + return nil, fmt.Errorf("invalid gateway base URL: %w", err) + } + gatewayBase := fmt.Sprintf("%s/ex/jira/%s", url.NormalizeURL(gatewayURL), cfg.CloudID) restURL := gatewayBase + "/rest/api/3" opts := &client.Options{ @@ -127,10 +135,10 @@ func newBearerClient(cfg ClientConfig) (*Client, error) { // newProxyClient creates a client configured for a trusted proxy that injects // authentication upstream. No Authorization header is sent by the CLI. func newProxyClient(cfg ClientConfig) (*Client, error) { - baseURL := url.NormalizeURL(cfg.URL) - if strings.HasPrefix(baseURL, "http://") && !url.IsLoopbackHTTP(baseURL) { - return nil, ErrProxyURLRequiresHTTPS + if err := url.RequireSecureOrLoopback(cfg.URL); err != nil { + return nil, err } + baseURL := url.NormalizeURL(cfg.URL) restURL := baseURL + "/rest/api/3" opts := &client.Options{SkipAuthHeader: true} @@ -166,7 +174,7 @@ var ( ErrEmailRequired = stderrors.New("email is required") ErrAPITokenRequired = stderrors.New("API token is required") ErrCloudIDRequired = stderrors.New("cloud ID is required for bearer auth") - ErrProxyURLRequiresHTTPS = stderrors.New("proxy auth URL must use https unless it is loopback http") + ErrProxyURLRequiresHTTPS = url.ErrRequiresHTTPS ) // ErrAgileUnavailable is returned when a command requires the Agile API diff --git a/tools/jtk/api/client_test.go b/tools/jtk/api/client_test.go index 1985cc73..c432749e 100644 --- a/tools/jtk/api/client_test.go +++ b/tools/jtk/api/client_test.go @@ -513,15 +513,28 @@ func TestNew_BearerAuth(t *testing.T) { }) } -func TestNew_BearerAuth_GatewayBaseFromEnv(t *testing.T) { - t.Setenv("JIRA_GATEWAY_BASE_URL", "https://gateway.example/") - t.Setenv("ATLASSIAN_GATEWAY_BASE_URL", "") +func TestNew_BearerAuth_GatewayBaseOverride(t *testing.T) { c, err := New(ClientConfig{ - URL: "https://example.atlassian.net", - APIToken: "scoped-token", - AuthMethod: "bearer", - CloudID: "abc-123", + URL: "https://example.atlassian.net", + APIToken: "scoped-token", + AuthMethod: "bearer", + CloudID: "abc-123", + GatewayBaseURL: "https://gateway.example/", }) testutil.RequireNoError(t, err) testutil.Equal(t, "https://gateway.example/ex/jira/abc-123/rest/api/3", c.BaseURL) } + +func TestNew_BearerAuth_RejectsCleartextGateway(t *testing.T) { + c, err := New(ClientConfig{URL: "https://example.atlassian.net", APIToken: "token", AuthMethod: "bearer", CloudID: "cloud", GatewayBaseURL: "http://external.example"}) + if !errors.Is(err, ErrProxyURLRequiresHTTPS) || c != nil { + t.Fatalf("New() = (%v, %v), want secure URL error", c, err) + } +} + +func TestNew_BearerAuth_DoesNotReadGatewayEnv(t *testing.T) { + t.Setenv("JIRA_GATEWAY_BASE_URL", "https://ambient.example") + c, err := New(ClientConfig{URL: "https://example.atlassian.net", APIToken: "token", AuthMethod: "bearer", CloudID: "cloud"}) + testutil.RequireNoError(t, err) + testutil.Equal(t, "https://api.atlassian.com/ex/jira/cloud/rest/api/3", c.BaseURL) +} diff --git a/tools/jtk/docs/development.md b/tools/jtk/docs/development.md index a8167ebd..fff87279 100644 --- a/tools/jtk/docs/development.md +++ b/tools/jtk/docs/development.md @@ -75,7 +75,7 @@ Read `internal/cmd/OUTPUT_SPEC.md` before changing default output, `--id`, `--fi `jtk` participates in the shared Atlassian credential/config model described by the monorepo guide. `ATLASSIAN_*` variables apply across both tools; `JIRA_*` variables override for jtk. The jtk-specific config section carries non-secret defaults such as `default_project`. -Basic auth uses an instance URL plus email and token. Bearer auth routes through `api.atlassian.com`, requires a cloud ID, and has Atlassian platform scope limitations for some Jira surfaces. Proxy auth uses only a URL, sends no `Authorization` header, and allows loopback `http://` URLs for trusted local proxies while keeping arbitrary cleartext proxy URLs rejected. Bearer gateway routing can be overridden with `JIRA_GATEWAY_BASE_URL` or `ATLASSIAN_GATEWAY_BASE_URL`. +Basic auth uses an instance URL plus email and token. Bearer auth routes through `api.atlassian.com`, requires a cloud ID, and has Atlassian platform scope limitations for some Jira surfaces. Proxy auth uses only a URL, sends no `Authorization` header, and allows loopback `http://` URLs for trusted local proxies while keeping arbitrary cleartext proxy URLs rejected. Bearer gateway routing can be overridden with `JIRA_GATEWAY_BASE_URL` or `ATLASSIAN_GATEWAY_BASE_URL`; overrides must use HTTPS or loopback HTTP. ## Testing Notes diff --git a/tools/jtk/internal/cmd/initcmd/initcmd.go b/tools/jtk/internal/cmd/initcmd/initcmd.go index 1da875df..70c6c0e2 100644 --- a/tools/jtk/internal/cmd/initcmd/initcmd.go +++ b/tools/jtk/internal/cmd/initcmd/initcmd.go @@ -17,6 +17,7 @@ import ( "github.com/spf13/cobra" "github.com/open-cli-collective/atlassian-go/auth" + "github.com/open-cli-collective/atlassian-go/client" "github.com/open-cli-collective/atlassian-go/credstore" "github.com/open-cli-collective/atlassian-go/keyring" "github.com/open-cli-collective/atlassian-go/prompt" @@ -312,10 +313,9 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, // can't run — every required value must already be in cfg from the // flag prefills. Fail loud naming the first missing field. if !prompt.WantPrompt(opts.NonInteractive, opts.Stdin) { - if err := auth.RequireNonInteractiveFields( - cfg.URL, cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID, - "jtk set-credential --ref atlassian-cli/default --key api_token --stdin", - ); err != nil { + credentials := auth.Credentials{Method: cfg.AuthMethod, Email: cfg.Email, APIToken: cfg.APIToken, CloudID: cfg.CloudID} + if err := credentials.RequireNonInteractive(cfg.URL, + "jtk set-credential --ref atlassian-cli/default --key api_token --stdin"); err != nil { return err } } else { @@ -325,11 +325,18 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, } } - // Normalize URL + credentials := (auth.Credentials{Method: cfg.AuthMethod, Email: cfg.Email, APIToken: cfg.APIToken, CloudID: cfg.CloudID}).Normalize() + cfg.AuthMethod = credentials.Method + cfg.Email = credentials.Email + cfg.APIToken = credentials.APIToken + cfg.CloudID = credentials.CloudID + if cfg.AuthMethod == auth.AuthMethodProxy { + if err := sharedurl.RequireSecureOrLoopback(cfg.URL); err != nil { + return fmt.Errorf("invalid proxy URL: %w", err) + } + } + // Normalize URL after validation so unsupported schemes are rejected. cfg.URL = sharedurl.NormalizeURL(cfg.URL) - cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID = auth.NormalizeConfig( - cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID, - ) return finalizeInit(ctx, opts, cfg, result, sharedPath, noVerify, defaultClientBuilder) } @@ -349,11 +356,12 @@ func finalizeInit( v.Println("Testing connection...") client, err := build(api.ClientConfig{ - URL: cfg.URL, - Email: cfg.Email, - APIToken: cfg.APIToken, - AuthMethod: cfg.AuthMethod, - CloudID: cfg.CloudID, + URL: cfg.URL, + Email: cfg.Email, + APIToken: cfg.APIToken, + AuthMethod: cfg.AuthMethod, + CloudID: cfg.CloudID, + GatewayBaseURL: client.GatewayBaseURLFromEnv("JIRA_GATEWAY_BASE_URL"), }) if err != nil { return fmt.Errorf("creating client: %w", err) diff --git a/tools/jtk/internal/cmd/initcmd/initcmd_test.go b/tools/jtk/internal/cmd/initcmd/initcmd_test.go index ec82a326..433c5305 100644 --- a/tools/jtk/internal/cmd/initcmd/initcmd_test.go +++ b/tools/jtk/internal/cmd/initcmd/initcmd_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "os" "path/filepath" "strings" "testing" @@ -141,6 +142,23 @@ func TestRunInit_Proxy_NoTokenRequiredOrPersisted(t *testing.T) { testutil.False(t, ok) } +func TestRunInit_Proxy_NoVerifyRejectsCleartextRemoteBeforeSave(t *testing.T) { + credtest.Hermetic(t) + opts := &root.Options{ + NoColor: true, NonInteractive: true, Stdin: strings.NewReader(""), + Stdout: &bytes.Buffer{}, Stderr: &bytes.Buffer{}, + } + err := runInit(context.Background(), opts, + "http://external.example/atlassian", "", "", false, "", auth.AuthMethodProxy, "", true) + if !errors.Is(err, api.ErrProxyURLRequiresHTTPS) { + t.Fatalf("runInit() error = %v, want insecure URL error", err) + } + _, err = os.Stat(credtest.SharedConfigPath(t)) + if !os.IsNotExist(err) { + t.Fatalf("runInit() wrote invalid proxy configuration: stat error = %v", err) + } +} + func TestFinalizeInit_NoVerify_BasicAuthWithoutTokenSavesConfig(t *testing.T) { credtest.Hermetic(t) tmpDir := t.TempDir() diff --git a/tools/jtk/internal/cmd/root/root.go b/tools/jtk/internal/cmd/root/root.go index 2ff85c00..aa00793b 100644 --- a/tools/jtk/internal/cmd/root/root.go +++ b/tools/jtk/internal/cmd/root/root.go @@ -13,6 +13,7 @@ import ( "github.com/open-cli-collective/atlassian-go/artifact" "github.com/open-cli-collective/atlassian-go/auth" + "github.com/open-cli-collective/atlassian-go/client" "github.com/open-cli-collective/atlassian-go/keyring" "github.com/open-cli-collective/atlassian-go/present" "github.com/open-cli-collective/atlassian-go/version" @@ -99,12 +100,13 @@ func (o *Options) APIClient() (*api.Client, error) { } } c, err := api.New(api.ClientConfig{ - URL: config.GetURL(), - Email: config.GetEmail(), - APIToken: token, - Verbose: o.Verbose, - AuthMethod: authMethod, - CloudID: config.GetCloudID(), + URL: config.GetURL(), + Email: config.GetEmail(), + APIToken: token, + Verbose: o.Verbose, + AuthMethod: authMethod, + CloudID: config.GetCloudID(), + GatewayBaseURL: client.GatewayBaseURLFromEnv("JIRA_GATEWAY_BASE_URL"), }) if err != nil { return nil, err diff --git a/tools/jtk/internal/cmd/root/root_test.go b/tools/jtk/internal/cmd/root/root_test.go index 5daac049..2e537350 100644 --- a/tools/jtk/internal/cmd/root/root_test.go +++ b/tools/jtk/internal/cmd/root/root_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/open-cli-collective/atlassian-go/artifact" + "github.com/open-cli-collective/atlassian-go/auth" "github.com/open-cli-collective/atlassian-go/credtest" "github.com/open-cli-collective/atlassian-go/present" "github.com/open-cli-collective/atlassian-go/testutil" @@ -156,6 +157,19 @@ func TestOptions_APIClient_ProxySkipsToken(t *testing.T) { testutil.Equal(t, "", capturedAuth) } +func TestOptions_APIClient_UsesGatewayOverride(t *testing.T) { + credtest.Hermetic(t) + t.Setenv("JIRA_URL", "https://example.atlassian.net") + t.Setenv("JIRA_AUTH_METHOD", auth.AuthMethodBearer) + t.Setenv("JIRA_CLOUD_ID", "cloud-123") + t.Setenv("JIRA_API_TOKEN", "token") + t.Setenv("JIRA_GATEWAY_BASE_URL", "https://gateway.example/") + opts := &Options{} + c, err := opts.APIClient() + testutil.RequireNoError(t, err) + testutil.Equal(t, "https://gateway.example/ex/jira/cloud-123/rest/api/3", c.BaseURL) +} + func TestRegisterCommands(t *testing.T) { cmd, opts := NewCmd()