Skip to content

Add proxy auth mode for Atlassian CLIs#411

Open
niieani wants to merge 1 commit into
open-cli-collective:mainfrom
niieani:codex/atlassian-proxy-auth
Open

Add proxy auth mode for Atlassian CLIs#411
niieani wants to merge 1 commit into
open-cli-collective:mainfrom
niieani:codex/atlassian-proxy-auth

Conversation

@niieani

@niieani niieani commented Jun 4, 2026

Copy link
Copy Markdown

Summary

  • Add proxy as a supported auth method for jtk and cfl
  • Send no Authorization header in proxy mode and require only a URL
  • Allow loopback http:// proxy URLs while continuing to reject arbitrary cleartext URLs
  • Skip token/keyring requirements for proxy-mode runtime and init flows
  • Add optional bearer gateway base URL overrides for controlled environments and tests
  • Document proxy auth in READMEs, development notes, and integration test runbooks

Why

Some users run Atlassian CLI tools through a trusted local or managed proxy that handles upstream authentication. The existing basic and bearer modes require the CLI to own credentials and emit an auth header, which makes those proxy setups difficult or impossible to configure cleanly.

Implementation Notes

Proxy auth is explicit via ATLASSIAN_AUTH_METHOD=proxy, JIRA_AUTH_METHOD=proxy, or CFL_AUTH_METHOD=proxy. In this mode, the CLI constructs normal Jira or Confluence API URLs but deliberately omits Authorization. HTTPS proxy URLs are accepted, and http:// is limited to loopback hosts for local proxy workflows.

Bearer auth keeps existing behavior by default and adds ATLASSIAN_GATEWAY_BASE_URL, JIRA_GATEWAY_BASE_URL, and CFL_GATEWAY_BASE_URL for environments that need to override the Atlassian gateway base.

Tests

  • go test ./... in shared
  • go test ./... in tools/jtk
  • go test ./... in tools/cfl

@piekstra piekstra requested a review from piekstra-dev June 17, 2026 12:39

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The PR updated cfl/integration-tests.md's Test Execution Checklist (line 592) to say "Run the full checklist with separate passes to ensure each auth path works" and covers all three methods. The equivalent section at the bottom of jtk/integration-tests.md was not updated — it almost certainly still says "Run the full checklist twice with separate passes to ensure both auth paths work." A tester following the jtk checklist would not know a third Proxy Auth pass is required. Add a "Pass 3: Proxy Auth" block and update the checklist preamble to match the updated cfl version.

Reply inline to this comment.

@@ -287,22 +318,23 @@ func runInit(ctx context.Context, opts *root.Options, prefillURL, prefillEmail,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

api.New is now called unconditionally before the if !noVerify block, changing the semantics of --no-verify. Previously, the client was constructed only inside the verify branch, so --no-verify also bypassed client construction. For basic auth, api.New returns ErrAPITokenRequired when cfg.APIToken is empty. A user running jtk init --no-verify in interactive mode who completes the form without entering a token (intending to populate via set-credential later) will now get a client-construction failure where the old code would have saved the config successfully.

The test suite has no case asserting that basic-auth --no-verify can save a URL+email config when the token is absent, so this regression is invisible to CI.

Fix: move client construction back inside the if !noVerify block alongside the verification call, restoring the original invariant that --no-verify skips all client interaction. If eager config validation is the intent, add an explicit guard that skips api.New when noVerify && cfg.APIToken == "" for basic auth, and add a test documenting the allowed-to-save-without-token path.

Reply inline to this comment.

Comment thread tools/cfl/api/client.go
}
}

// 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.

@@ -426,24 +464,41 @@ func finalizeInit(
if cfg.AuthMethod == auth.AuthMethodBearer {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The normalizeAuthConfig function introduced here is byte-for-byte identical to the one added in tools/jtk/internal/cmd/initcmd/initcmd.go. The logic it encodes — defaulting an empty auth_method to basic, and scrubbing Email/APIToken/CloudID for proxy auth — is auth-method policy, not tool-local behaviour. Duplicating it across two tool cmd packages means future auth method additions must be applied in two places. Move it to a shared package (e.g. shared/auth alongside ValidateAuthMethod, or a new shared/authconfig helper), expose it as a single exported function, and have both tools call it.

Reply inline to this comment.

@piekstra-dev

Copy link
Copy Markdown

Automated PR Review

Reviewed commit: 2af24b9de893
Profile: reviewer - Posting as: piekstra-dev

Summary

Reviewer Findings
go:implementation-tests 1
policies:conventions 1
structure:repo-health 1
documentation:docs 1
go:implementation-tests (1 finding)

Minor - tools/jtk/internal/cmd/initcmd/initcmd.go:318

api.New is now called unconditionally before the if !noVerify block, changing the semantics of --no-verify. Previously, the client was constructed only inside the verify branch, so --no-verify also bypassed client construction. For basic auth, api.New returns ErrAPITokenRequired when cfg.APIToken is empty. A user running jtk init --no-verify in interactive mode who completes the form without entering a token (intending to populate via set-credential later) will now get a client-construction failure where the old code would have saved the config successfully.

The test suite has no case asserting that basic-auth --no-verify can save a URL+email config when the token is absent, so this regression is invisible to CI.

Fix: move client construction back inside the if !noVerify block alongside the verification call, restoring the original invariant that --no-verify skips all client interaction. If eager config validation is the intent, add an explicit guard that skips api.New when noVerify && cfg.APIToken == "" for basic auth, and add a test documenting the allowed-to-save-without-token path.

policies:conventions (1 finding)

Major - tools/cfl/internal/cmd/init/init.go:464

The normalizeAuthConfig function introduced here is byte-for-byte identical to the one added in tools/jtk/internal/cmd/initcmd/initcmd.go. The logic it encodes — defaulting an empty auth_method to basic, and scrubbing Email/APIToken/CloudID for proxy auth — is auth-method policy, not tool-local behaviour. Duplicating it across two tool cmd packages means future auth method additions must be applied in two places. Move it to a shared package (e.g. shared/auth alongside ValidateAuthMethod, or a new shared/authconfig helper), expose it as a single exported function, and have both tools call it.

structure:repo-health (1 finding)

Major - tools/cfl/api/client.go:36

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.

documentation:docs (1 finding)

Minor - tools/jtk/integration-tests.md

The PR updated cfl/integration-tests.md's Test Execution Checklist (line 592) to say "Run the full checklist with separate passes to ensure each auth path works" and covers all three methods. The equivalent section at the bottom of jtk/integration-tests.md was not updated — it almost certainly still says "Run the full checklist twice with separate passes to ensure both auth paths work." A tester following the jtk checklist would not know a third Proxy Auth pass is required. Add a "Pass 3: Proxy Auth" block and update the checklist preamble to match the updated cfl version.

0 PR discussion threads considered. 0 summarized; 0 resolved.


Completed in 8m 11s | ~$1.04 (est.) | claude-sonnet-4-6 | cr 0.4.161
Field Value
Model claude-sonnet-4-6
Reviewers go:implementation-tests, policies:conventions, structure:repo-health, documentation:docs
Engine claude_cli · claude-sonnet-4-6
Reviewed by cr · piekstra-dev
Duration 8m 11s wall · 8m 10s compute
Cost ~$1.04 (est.)
Tokens 26 in / 10.7k out

Per-workstream usage

Workstream Model In Out Cache read Cache create Cost Duration
orchestrator-selection claude-sonnet-4-6 4 1.6k 0 20.5k ~$0.10 (est.) 26s
go:implementation-tests claude-sonnet-4-6 6 1.1k 199.9k 76.7k ~$0.36 (est.) 3m 02s
policies:conventions claude-sonnet-4-6 4 2.8k 7.2k 24.2k ~$0.14 (est.) 53s
structure:repo-health claude-sonnet-4-6 4 2.9k 7.2k 27.6k ~$0.15 (est.) 53s
documentation:docs claude-sonnet-4-6 4 1.3k 3.9k 44.5k ~$0.19 (est.) 1m 54s
orchestrator-rollup claude-sonnet-4-6 4 975 24.1k 20.5k ~$0.10 (est.) 59s

@piekstra-dev piekstra-dev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated PR review completed with outcome: comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants