diff --git a/shared/auth/auth.go b/shared/auth/auth.go index a287f680..4aa85034 100644 --- a/shared/auth/auth.go +++ b/shared/auth/auth.go @@ -13,21 +13,73 @@ 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) } } +// 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 c.Method == AuthMethodProxy { + c.Email = "" + c.APIToken = "" + c.CloudID = "" + } + return c +} + +// 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 (c Credentials) RequireNonInteractive(url, toolHint string) error { + if url == "" { + return errors.New("--non-interactive: missing required value for --url") + } + + switch c.Method { + case AuthMethodProxy: + return nil + case AuthMethodBearer: + if c.CloudID == "" { + return errors.New("--non-interactive: missing required value for --cloud-id (bearer auth)") + } + default: + if c.Email == "" { + return errors.New("--non-interactive: missing required value for --email (basic auth)") + } + } + + 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 +} + // 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 a8018337..49e59d62 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}, @@ -139,6 +140,161 @@ 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() + + got := (Credentials{Method: tt.authMethod, Email: tt.email, APIToken: tt.apiToken, CloudID: tt.cloudID}).Normalize() + + if got.Method != tt.wantAuthMethod { + t.Errorf("auth method = %q, want %q", got.Method, tt.wantAuthMethod) + } + if got.Email != tt.wantEmail { + t.Errorf("email = %q, want %q", got.Email, tt.wantEmail) + } + if got.APIToken != tt.wantAPIToken { + t.Errorf("api token = %q, want %q", got.APIToken, tt.wantAPIToken) + } + if got.CloudID != tt.wantCloudID { + t.Errorf("cloud ID = %q, want %q", got.CloudID, tt.wantCloudID) + } + }) + } +} + +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 := (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) + } + 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" { @@ -147,4 +303,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..b671927c 100644 --- a/shared/url/url.go +++ b/shared/url/url.go @@ -1,7 +1,32 @@ // Package url provides URL normalization utilities for Atlassian CLI tools. package url -import "strings" +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. @@ -36,3 +61,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..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() @@ -115,3 +147,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..5da7586e 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,14 @@ 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` | + +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. + **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..657ecd2a 100644 --- a/tools/cfl/api/client.go +++ b/tools/cfl/api/client.go @@ -11,12 +11,16 @@ 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. +// 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") + ErrURLRequired = errors.New("URL is required") + ErrEmailRequired = errors.New("email is required") + ErrProxyURLRequiresHTTPS = sharedurl.ErrRequiresHTTPS ) // Client is the Confluence Cloud API client. @@ -25,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{ @@ -32,24 +72,62 @@ 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) { + 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 +} + // 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.GatewayBaseURL, 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), }, 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..2fe7dbeb 100644 --- a/tools/cfl/api/client_test.go +++ b/tools/cfl/api/client_test.go @@ -23,6 +23,39 @@ 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, 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") + 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 @@ -243,3 +276,23 @@ func TestNewBearerClient(t *testing.T) { testutil.Equal(t, "https://example.atlassian.net/wiki", c.GetBaseURL()) }) } + +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 96b0dfc2..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. `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/integration-tests.md b/tools/cfl/integration-tests.md index f83bef89..45bcaf21 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 @@ -753,6 +758,27 @@ All cfl commands work with both auth methods (no scope restrictions for Confluen --- +### 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: @@ -760,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/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)) 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 c2b57b41..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,14 +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 hardcoded 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) - } - 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. @@ -67,6 +69,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 +90,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 +103,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 +141,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 +190,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 +202,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(). @@ -211,7 +244,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 @@ -267,7 +300,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 @@ -288,7 +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 := requireNonInteractiveFields(cfg, isBearer); 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 { @@ -299,8 +334,13 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail s } cfg.NormalizeURL() + 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.Validate(); err != nil { + if err := cfg.ValidateForInit(noVerify); err != nil { return fmt.Errorf("invalid configuration: %w", err) } @@ -372,16 +412,21 @@ 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 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) + 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 { @@ -425,33 +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") + 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, isBearer bool) error { - if cfg.URL == "" { - return fmt.Errorf("--non-interactive: missing required value for --url") - } - if isBearer { - if cfg.CloudID == "" { - return fmt.Errorf("--non-interactive: missing required value for --cloud-id (bearer auth)") - } - } else { - 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 a6199b23..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,61 +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 - isBearer bool - 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"}}, - { - 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"}, - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - err := requireNonInteractiveFields(tc.cfg, tc.isBearer) - 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, false); 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) { @@ -250,299 +190,33 @@ func TestRunInit_UsesConfiguredPathForReconciliation(t *testing.T) { testutil.Equal(t, "https://configured.atlassian.net", store.Default.URL) } -// 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 +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{}, } - 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) + err := runInit(context.Background(), opts, + "http://127.0.0.1:8080/atlassian", "", false, "", auth.AuthMethodProxy, "", true) 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) - } + 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.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) - }) -} - -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) + testutil.False(t, ok) } const cflInitSentinel = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAcflInitTok" diff --git a/tools/cfl/internal/cmd/root/root.go b/tools/cfl/internal/cmd/root/root.go index 2116da2e..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,10 +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) - } - 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 dbc98226..31191b4d 100644 --- a/tools/cfl/internal/cmd/root/root_test.go +++ b/tools/cfl/internal/cmd/root/root_test.go @@ -2,7 +2,10 @@ package root import ( "bytes" + "errors" "fmt" + "net/http" + "net/http/httptest" "path/filepath" "testing" @@ -140,6 +143,55 @@ 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 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 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() @@ -408,6 +460,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{ @@ -440,7 +499,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.go b/tools/cfl/internal/config/config.go index 2c35f9bf..cc54b6f2 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,22 @@ 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 { + 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") } - if c.APIToken == "" { - return errors.New("api_token is required") - } // Validate auth method if set (empty defaults to basic) if c.AuthMethod != "" { @@ -56,21 +66,35 @@ 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 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 { + case auth.AuthMethodProxy: + return nil + case auth.AuthMethodBearer: + if requireToken && 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 requireToken && 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 +304,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..6fb25d24 100644 --- a/tools/cfl/internal/config/config_test.go +++ b/tools/cfl/internal/config/config_test.go @@ -66,6 +66,36 @@ 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 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{ + URL: "http://example.com/atlassian/wiki", + AuthMethod: "proxy", + }, + wantErr: true, + errMsg: "url must use https", + }, { name: "valid bearer config", config: Config{ @@ -123,6 +153,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 { @@ -592,3 +691,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"), true, true) + 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..219d2ca2 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,14 @@ 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` | + +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. + **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..bb75d5dc 100644 --- a/tools/jtk/api/client.go +++ b/tools/jtk/api/client.go @@ -35,24 +35,23 @@ 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) or "bearer" - 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. // 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 +59,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 +94,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 +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.GatewayBaseURL, 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{ @@ -117,6 +132,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) { + 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} + 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 +170,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 = 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 478747fb..c432749e 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,29 @@ func TestNew_BearerAuth(t *testing.T) { } }) } + +func TestNew_BearerAuth_GatewayBaseOverride(t *testing.T) { + c, err := New(ClientConfig{ + 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 f9a3014f..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. +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/integration-tests.md b/tools/jtk/integration-tests.md index a1be315f..f2703ea4 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 @@ -1356,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) @@ -1527,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: @@ -1534,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 4cfa6689..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" @@ -27,6 +28,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 @@ -47,6 +54,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 +77,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 +92,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 +169,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 +197,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(). @@ -201,7 +239,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 @@ -257,7 +295,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 @@ -275,7 +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 := requireNonInteractiveFields(cfg, isBearer); 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 { @@ -285,19 +325,43 @@ 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) - // Verify connection unless --no-verify + 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() + 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, + client, err := build(api.ClientConfig{ + 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) @@ -348,16 +412,21 @@ 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 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) + 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) { @@ -391,34 +460,14 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail, v.Println(" jtk me") v.Println(" jtk issues list --project ") - if isBearer { + switch cfg.AuthMethod { + case auth.AuthMethodBearer: v.Println("") v.Info("To switch back to basic auth later, run: jtk init --auth-method basic") + 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, isBearer bool) error { - if cfg.URL == "" { - return fmt.Errorf("--non-interactive: missing required value for --url") - } - if isBearer { - if cfg.CloudID == "" { - return fmt.Errorf("--non-interactive: missing required value for --cloud-id (bearer auth)") - } - } else { - 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 4c43b707..433c5305 100644 --- a/tools/jtk/internal/cmd/initcmd/initcmd_test.go +++ b/tools/jtk/internal/cmd/initcmd/initcmd_test.go @@ -3,15 +3,20 @@ package initcmd import ( "bytes" "context" + "errors" + "os" + "path/filepath" "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" "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" ) @@ -64,78 +69,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 - isBearer bool - 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"}, - isBearer: true, - 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"}, - isBearer: true, - 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) - 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, false); 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. @@ -181,6 +114,93 @@ 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 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() + 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"} diff --git a/tools/jtk/internal/cmd/root/root.go b/tools/jtk/internal/cmd/root/root.go index f6af0ad4..aa00793b 100644 --- a/tools/jtk/internal/cmd/root/root.go +++ b/tools/jtk/internal/cmd/root/root.go @@ -12,6 +12,8 @@ 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" @@ -88,17 +90,23 @@ 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(), - 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 c10ad1ef..2e537350 100644 --- a/tools/jtk/internal/cmd/root/root_test.go +++ b/tools/jtk/internal/cmd/root/root_test.go @@ -2,11 +2,15 @@ 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/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" "github.com/open-cli-collective/atlassian-go/view" @@ -130,6 +134,42 @@ 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 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() 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()