Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions shared/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,18 @@ const (

// AuthMethodBearer is the authentication method for service accounts with scoped API tokens.
AuthMethodBearer = "bearer"

// AuthMethodProxy sends no Authorization header and relies on a local or upstream proxy.
AuthMethodProxy = "proxy"
)

// ErrInvalidAuthMethod is returned when an unrecognized auth method is provided.
var ErrInvalidAuthMethod = errors.New("invalid auth method: must be \"basic\" or \"bearer\"")
var ErrInvalidAuthMethod = errors.New("invalid auth method: must be \"basic\", \"bearer\", or \"proxy\"")

// ValidateAuthMethod returns nil if method is a recognized auth method, or ErrInvalidAuthMethod otherwise.
func ValidateAuthMethod(method string) error {
switch method {
case AuthMethodBasic, AuthMethodBearer:
case AuthMethodBasic, AuthMethodBearer, AuthMethodProxy:
return nil
default:
return fmt.Errorf("%w: got %q", ErrInvalidAuthMethod, method)
Expand Down
4 changes: 4 additions & 0 deletions shared/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -147,4 +148,7 @@ func TestAuthMethodConstants(t *testing.T) {
if AuthMethodBearer != "bearer" {
t.Errorf("AuthMethodBearer = %q, want %q", AuthMethodBearer, "bearer")
}
if AuthMethodProxy != "proxy" {
t.Errorf("AuthMethodProxy = %q, want %q", AuthMethodProxy, "proxy")
}
}
8 changes: 6 additions & 2 deletions shared/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)
}

Expand Down Expand Up @@ -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")

Expand Down
39 changes: 39 additions & 0 deletions shared/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
21 changes: 21 additions & 0 deletions shared/client/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package client

import (
"io"
"os"
"strings"
"time"
)

Expand All @@ -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.
Expand All @@ -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.
Expand Down
12 changes: 12 additions & 0 deletions shared/credstore/conndivergence_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
7 changes: 5 additions & 2 deletions shared/credstore/credstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
16 changes: 16 additions & 0 deletions shared/credstore/credstore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down
22 changes: 21 additions & 1 deletion shared/url/url.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
// Package url provides URL normalization utilities for Atlassian CLI tools.
package url

import "strings"
import (
"net/netip"
neturl "net/url"
"strings"
)

// NormalizeURL ensures the URL has an https scheme and no trailing slashes.
// If the URL is empty, it returns an empty string.
Expand Down Expand Up @@ -36,3 +40,19 @@ func HasScheme(u string) bool {
func TrimTrailingSlashes(u string) string {
return strings.TrimRight(u, "/")
}

// IsLoopbackHTTP reports whether u is an http:// URL whose host is localhost
// or a loopback IP address. It is intentionally narrow so CLIs can allow local
// development/proxy endpoints without allowing arbitrary cleartext URLs.
func IsLoopbackHTTP(u string) bool {
parsed, err := neturl.Parse(u)
if err != nil || parsed.Scheme != "http" || parsed.Host == "" {
return false
}
host := strings.ToLower(parsed.Hostname())
if host == "localhost" {
return true
}
addr, err := netip.ParseAddr(host)
return err == nil && addr.IsLoopback()
}
28 changes: 28 additions & 0 deletions shared/url/url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,31 @@ func TestTrimTrailingSlashes(t *testing.T) {
})
}
}

func TestIsLoopbackHTTP(t *testing.T) {
t.Parallel()
tests := []struct {
input string
want bool
}{
{"http://localhost:8080/atlassian", true},
{"http://LOCALHOST/atlassian", true},
{"http://127.0.0.1:8080/atlassian", true},
{"http://127.42.0.1:8080/atlassian", true},
{"http://[::1]:8080/atlassian", true},
{"https://localhost:8080/atlassian", false},
{"http://example.com/atlassian", false},
{"http://10.0.0.1/atlassian", false},
{"localhost:8080/atlassian", false},
{"", false},
}

for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
t.Parallel()
if got := IsLoopbackHTTP(tt.input); got != tt.want {
t.Errorf("IsLoopbackHTTP(%q) = %v, want %v", tt.input, got, tt.want)
}
})
}
}
16 changes: 13 additions & 3 deletions tools/cfl/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,18 +202,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.)

Expand Down Expand Up @@ -782,7 +789,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
Expand Down Expand Up @@ -844,9 +851,12 @@ Environment variables override file-based config. Variables are checked in order
| Default Space | `CFL_DEFAULT_SPACE` → shared `cfl.default_space` → legacy |
| Auth Method | `CFL_AUTH_METHOD` → `ATLASSIAN_AUTH_METHOD` → shared `default` → legacy → `basic` |
| Cloud ID | `CFL_CLOUD_ID` → `ATLASSIAN_CLOUD_ID` → shared `default` → legacy |
| Bearer Gateway Base URL | `CFL_GATEWAY_BASE_URL` → `ATLASSIAN_GATEWAY_BASE_URL` → `https://api.atlassian.com` |

Per §2.2 connection config is single-sourced from the shared `default` section — per-tool `cfl:`/`jtk:` sections carry only non-secret defaults and may not override `url`/`email`/`auth_method`/`cloud_id`.

Set `ATLASSIAN_AUTH_METHOD=proxy` (or `CFL_AUTH_METHOD=proxy`) with `ATLASSIAN_URL`/`CFL_URL` to use a trusted proxy. Proxy auth ignores email, token, and cloud ID because the proxy is responsible for upstream authentication.

**Shared credentials:** If you use both `cfl` and `jtk` (Jira CLI), set `ATLASSIAN_*` variables once:

```bash
Expand Down
19 changes: 18 additions & 1 deletion tools/cfl/api/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/open-cli-collective/atlassian-go/auth"
"github.com/open-cli-collective/atlassian-go/client"
sharedurl "github.com/open-cli-collective/atlassian-go/url"
)

// Validation errors for bearer auth.
Expand All @@ -32,6 +33,14 @@ func NewClient(baseURL, email, apiToken string) *Client {
}
}

// NewProxyClient creates a Confluence client for a trusted proxy that injects

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NewProxyClient skips the loopback-HTTP safety check that newProxyClient (jtk) enforces at the client boundary.

The invariant introduced by this diff is: proxy auth must never send unauthenticated requests to a non-loopback cleartext endpoint. tools/jtk/api/client.go:newProxyClient enforces this at client-construction time:

if strings.HasPrefix(baseURL, "http://") && !url.IsLoopbackHTTP(baseURL) {
    return nil, ErrProxyURLRequiresHTTPS
}

tools/cfl/api/client.go:NewProxyClient has no such check — it passes the URL straight to normalizeWikiBaseURL and client.New. The cfl path relies entirely on config.Validate() being called during cfl init, but Validate() is not called at runtime from root.go's APIClient(). A manually edited config (auth_method: proxy, url: http://external-host/...) would bypass validation and silently send unauthenticated requests to an arbitrary cleartext server.

Fix: add the identical guard in NewProxyClient before calling normalizeWikiBaseURL:

func NewProxyClient(baseURL string) (*Client, error) {
    normalized := normalizeWikiBaseURL(baseURL)
    if strings.HasPrefix(normalized, "http://") && !sharedurl.IsLoopbackHTTP(normalized) {
        return nil, ErrProxyURLRequiresHTTPS  // add this sentinel error
    }
    return &Client{Client: client.New(normalized, "", "", &client.Options{SkipAuthHeader: true})}, nil
}

This also makes the two tools' public API surfaces symmetric.

Reply inline to this comment.

// authentication upstream. No Authorization header is sent by the CLI.
func NewProxyClient(baseURL string) *Client {
return &Client{
Client: client.New(normalizeWikiBaseURL(baseURL), "", "", &client.Options{SkipAuthHeader: true}),
}
}

// NewBearerClient creates a new Confluence API client using bearer auth via the API gateway.
// The cloudID is used to construct the gateway URL: https://api.atlassian.com/ex/confluence/{cloudId}/wiki
func NewBearerClient(apiToken, cloudID string) (*Client, error) {
Expand All @@ -41,7 +50,7 @@ func NewBearerClient(apiToken, cloudID string) (*Client, error) {
if cloudID == "" {
return nil, ErrCloudIDRequired
}
gatewayBase := fmt.Sprintf("%s/ex/confluence/%s/wiki", client.GatewayBaseURL, cloudID)
gatewayBase := fmt.Sprintf("%s/ex/confluence/%s/wiki", client.GatewayBaseURLFromEnv("CFL_GATEWAY_BASE_URL"), cloudID)
opts := &client.Options{
AuthHeader: auth.BearerAuthHeader(apiToken),
}
Expand All @@ -50,6 +59,14 @@ func NewBearerClient(apiToken, cloudID string) (*Client, error) {
}, nil
}

func normalizeWikiBaseURL(baseURL string) string {
baseURL = sharedurl.NormalizeURL(baseURL)
if !strings.HasSuffix(baseURL, "/wiki") {
baseURL += "/wiki"
}
return baseURL
}

// GetHTTPClient returns the underlying HTTP client for custom requests.
func (c *Client) GetHTTPClient() *http.Client {
return c.HTTPClient
Expand Down
Loading