feat(auth): add Bitbucket authentication provider - #4629
CrazyBaran wants to merge 4 commits into
Conversation
Add a `bitbucket` provider to the authentication registry so private
Bitbucket Cloud and Data Center repositories can serve extension,
bundle, preset, and workflow artifacts through the existing opt-in
`~/.specify/auth.json` mechanism.
Schemes:
- `bearer` — repository/project/workspace access tokens (Cloud) and
HTTP access tokens (Data Center): `Authorization: Bearer <token>`
- `basic` — Atlassian API tokens: `Authorization: Basic
base64(<username>:<token>)` where username is the Atlassian account
email. Unlike Azure DevOps' `basic-pat` (which hardcodes an empty
username), this credential needs a real username, so `auth.json`
gains a `username` field.
Config validation (`load_auth_config`):
- `username` must be a non-empty string when present, must not contain
':' (RFC 7617 §2), is whitespace-normalized like `token_env`, and is
required when `auth == "basic"`
- `basic` joins `bearer`/`basic-pat` in requiring `token` or `token_env`
Design: `resolve_token` returns the composite `<username>:<secret>`
credential for `basic` and `auth_headers` encodes it verbatim, keeping
the `AuthProvider` interface and the shared HTTP layer unchanged.
`auth_headers("basic")` rejects a token without ':' so a bare secret
cannot silently become a well-formed header with an empty username.
No changes to the download/redirect security path. Bitbucket Cloud's
Downloads endpoint 302s to a pre-signed S3 URL; the existing redirect
handler strips `Authorization` when leaving the declared hosts, which
is required (S3 rejects pre-signed requests carrying an auth header),
and catalog `sha256` covers integrity of that hop. Documented in
docs/reference/authentication.md with ready-to-paste examples.
Tests: 26 new cases in tests/test_authentication.py covering config
validation, provider headers (incl. UTF-8 credentials and the
bare-secret guard), token resolution, registry, and an end-to-end
`build_request` check against api.bitbucket.org.
Assisted-by: Claude Code (model: claude-fable-5-1, supervised)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Directly constructed entries can bypass the colon restriction and produce an incorrectly parsed Basic credential.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds first-class Bitbucket Cloud and Data Center authentication to the provider registry.
Changes:
- Adds Bearer and Basic authentication.
- Validates Basic-auth usernames and credentials.
- Documents configuration with comprehensive tests.
File summaries
| File | Description |
|---|---|
src/specify_cli/authentication/bitbucket.py |
Implements Bitbucket authentication. |
src/specify_cli/authentication/config.py |
Adds username validation. |
src/specify_cli/authentication/__init__.py |
Registers the provider. |
tests/test_authentication.py |
Tests configuration and headers. |
docs/reference/authentication.md |
Documents Bitbucket setup. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot review round 1 on github#4629: load_auth_config already rejects a 'username' containing ':' (RFC 7617 §2), but resolve_token only guarded directly-constructed entries against a missing/blank username. An entry built in code with username "user:name" therefore resolved to "user:name:<secret>", which a server parses as user "user". Return None for that case as well, matching the existing missing-username defense, and add a direct-entry regression test. Assisted-by: Claude Code (model: claude-fable-5-1, supervised) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Review round 1 addressed in f656ee2.
Posted on behalf of @CrazyBaran by Claude Code (model: claude-fable-5-1, human-supervised); the fix, test, and this comment were AI-drafted and reviewed by the author before pushing. |
mnriem
added
the
triage-can-wait
There was a problem hiding this comment.
🟡 Changes recommended
The Basic credential guard remains bypassable, and the dataclass field insertion breaks positional constructor compatibility.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Balanced
Copilot review round 2 on github#4629: - auth_headers("basic") checked only for the presence of ':', which still accepted ":<secret>" — exactly the empty-username credential the guard exists to block — and "<username>:". Split on the first colon with str.partition and require both halves to be non-empty; a secret that itself contains ':' is preserved intact. - AuthConfigEntry.username was inserted before the Azure AD fields, which shifted positional parameter positions (a sixth positional argument that populated tenant_id would have become username). Append it after client_secret_env instead; repository call sites are keyword-only but the dataclass remains positionally constructible. Tests: empty-half rejection (":secret", "user:", ":"), colon-in-secret preservation, and a positional-construction regression for the field order. Assisted-by: Claude Code (model: claude-fable-5-1, supervised) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Review round 2 addressed in ee8a99a.
Auth/HTTP/download-security suites: 462 passed, 1 pre-existing skip; Posted on behalf of @CrazyBaran by Claude Code (model: claude-fable-5-1, human-supervised); the fixes, tests, and this comment were AI-drafted and reviewed by the author before pushing. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The Cloud examples declare bitbucket.org despite requiring host-specific authentication credentials not represented by those entries.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
|
|
||
| ```json | ||
| { | ||
| "hosts": ["api.bitbucket.org", "bitbucket.org"], |
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback
Description
Why
Spec Kit's install pipeline is already host-agnostic: catalogs and archives are plain HTTPS fetches through
authentication/http.py, and #2335 introduced the provider registry so credentials could be attached per host. Today that registry knows two forges —githubandazure-devops. Bitbucket is the missing one.Teams hosting internal extensions, bundles, or presets on Bitbucket Cloud or Data Center currently have two poor options:
specify extension addagainst a private Bitbucket catalog fails at the first 401.githubprovider entry atapi.bitbucket.org— it "works" for Bearer access tokens becauseGitHubAuthis justBearer <token>— but it is semantically wrong, invisible to future GitHub-specific logic keyed off that provider, and it cannot express Atlassian API tokens at all, which needBasic base64(<email>:<token>). The existingbasic-patscheme hardcodes an empty username (:<PAT>), so no current provider can build that header.With Atlassian having removed app passwords on 2026-07-28, API tokens and access tokens are now the only credential types for Bitbucket Cloud, so first-class support for both is the practical minimum.
What this adds
New provider —
src/specify_cli/authentication/bitbucket.py, registered asbitbucket(alphabetically, alongside the existing two):bearerAuthorization: Bearer <token>basicAuthorization: Basic base64(<username>:<token>)Config schema —
AuthConfigEntrygainsusername: str | None = None(defaulted, appended aftertoken_env; every construction in the repo is keyword-only, so nothing breaks).load_auth_configvalidates it in the existingproviders[i]: …style:token_env:(RFC 7617 §2 — the server splits on the first colon, so this would silently authenticate as the wrong user)auth == "basic";basicalso joinsbearer/basic-patin requiringtokenortoken_envDocs —
docs/reference/authentication.md:usernamein the fields table, a Bitbucket section with three ready-to-pasteauth.jsonexamples (Cloud access token, Atlassian API token, Data Center HTTP access token), and a note on the Downloads→S3 redirect.Tests — 26 new cases in
tests/test_authentication.py: config validation (username required/typed/normalized/colon-rejected), provider headers (including UTF-8 credentials and the bare-secret guard), token resolution, registry, and an end-to-endbuild_requestcheck againstapi.bitbucket.org.Design notes for reviewers
AuthProvider.auth_headers(token, scheme)receives a single string fromresolve_token(entry). Forbasic,BitbucketAuth.resolve_tokenreturns"<username>:<secret>"andauth_headersencodes it verbatim. This keepsbase.py, the two existing providers, and both call sites inhttp.pyuntouched. The contract is guarded:auth_headers("basic")raises if the token has no:, andresolve_tokenreturnsNone(falls through to the next entry / unauthenticated) when a directly-constructed entry lacks a username — so a malformed:<secret>header can never be sent._download_security.py, redirect validation, size limits, or sha256 verification is modified. Bitbucket Cloud's Downloads endpoint answers with a 302 to a pre-signed S3 URL; the existing_StripAuthOnRedirectdropsAuthorizationbecause S3 is outside the declared hosts. That strip is required for Bitbucket (S3 returns 400 if a pre-signed request also carries an auth header), and the catalogsha256covers integrity of the unauthenticated final hop. The docs say so explicitly.azure_devops.py's ASCII encode which would raise on a non-ASCII value.api.bitbucket.org/2.0/repositories/<ws>/<repo>/downloads/<file>accepts the token directly.Out of scope / follow-up
An end-to-end hosting runbook (repository layout, uploading to Downloads, catalog authoring, Pipelines publish step, troubleshooting) is written and sits on a separate branch pending a decision on where it belongs in the docs; I'll open it as its own PR so this one stays reviewable as a provider change.
Testing
uv run specify --helpuv sync && uv run pytest— see belowbuild_requestintegration test)Results on this branch (single commit on top of
main@5e952140), Windows 11 / Python 3.14.7 / uv 0.12.14:uv run pytest tests/test_authentication.py tests/test_github_http.py tests/test_download_security.py— 456 passed, 1 skipped (the skip is pre-existing)uv run pytest tests/— 7752 passed, 461 skipped, 1 failed. The single failure istests/test_setup_tasks.py::test_setup_tasks_ps_core_template_resolved, where Windows PowerShell 5.1 (nopwsh7 on this machine) emits a control character in the JSON printed byscripts/powershell/setup-tasks.ps1— environmental;scripts/powershell/is not modified by this PR.uv tool run ruff@0.15.0 check src tests— All checks passedAI Disclosure
Code, tests, and documentation were generated by Claude Code (model
claude-fable-5-1) working under my direction: I specified the requirement (Bitbucket Cloud/Data Center support for private extension sources), reviewed the design choice to use a composite credential rather than widenAuthProvider, and reviewed every diff before commit. The change was additionally passed through an independent AI code-review pass whose findings (the bare-secret guard, the RFC 7617 colon check, doc corrections) were verified against the codebase and Atlassian's published deprecation schedule before being applied. The commit carries theAssisted-by:trailer required by AGENTS.md.🤖 Generated with Claude Code