Skip to content

feat: Config-driven opt-in authentication registry with multi-platform support#2393

Draft
Copilot wants to merge 9 commits intomainfrom
copilot/add-authentication-provider-registry
Draft

feat: Config-driven opt-in authentication registry with multi-platform support#2393
Copilot wants to merge 9 commits intomainfrom
copilot/add-authentication-provider-registry

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 28, 2026

Security-first authentication for catalog systems

No credentials are sent unless the user explicitly creates ~/.specify/auth.json mapping hosts to providers. This is a full opt-in model — without the config file, all HTTP requests are unauthenticated.

~/.specify/auth.json

{
  "providers": [
    {
      "hosts": ["github.com", "api.github.com", "raw.githubusercontent.com", "codeload.github.com"],
      "provider": "github",
      "auth": "bearer",
      "token_env": "GH_TOKEN"
    }
  ]
}

See docs/reference/authentication.md for full reference and template.

New: src/specify_cli/authentication/

src/specify_cli/authentication/
├── __init__.py      # AUTH_REGISTRY + _register / get_provider
├── base.py          # AuthProvider ABC (auth_headers, resolve_token)
├── config.py        # auth.json loader, validator, host matcher
├── http.py          # open_url / build_request with host matching,
│                    #   redirect stripping, provider fallthrough
├── github.py        # GitHubAuth — bearer scheme
└── azure_devops.py  # AzureDevOpsAuth — basic-pat, bearer, azure-cli, azure-ad

How it works

  1. open_url() matches URL hostname against hosts in auth.json
  2. Matching entry's provider resolves the token (token, token_env, or dynamic acquisition)
  3. Provider builds the Authorization header (Bearer, Basic, etc.)
  4. On 401/403, tries the next matching entry, then falls through to unauthenticated
  5. On redirects, strips Authorization when leaving the entry's declared hosts

Providers and auth schemes

GitHub (github)

Scheme Header Use for
bearer Authorization: Bearer <token> PATs, fine-grained PATs, OAuth tokens, GitHub App tokens

Azure DevOps (azure-devops)

Scheme Header Use for
basic-pat Authorization: Basic base64(:<PAT>) Personal Access Tokens
bearer Authorization: Bearer <token> Pre-acquired OAuth / Azure AD tokens
azure-cli Authorization: Bearer <token> Token via az account get-access-token
azure-ad Authorization: Bearer <token> OAuth2 client credentials flow

Wiring

All HTTP request paths now go through authentication.http.open_url():

  • ExtensionCatalog._open_url() / _make_request()
  • PresetCatalog._open_url() / _make_request()
  • IntegrationCatalog._fetch_single_catalog()
  • WorkflowCatalog._fetch_single_catalog()
  • _fetch_latest_release_tag() (with Accept: application/vnd.github+json)
  • All --from-url download paths in CLI commands

Tests

1823 total tests passing, including 67 auth-specific tests covering:

  • Config loading (valid configs, inline tokens, validation errors, permission warnings)
  • Host matching (exact, wildcard, non-matching, lookalike hosts)
  • Provider headers (bearer, basic-pat, unsupported scheme errors)
  • Token resolution (env vars, inline, whitespace handling, missing)
  • HTTP helpers (auth attachment, non-matching hosts, fallthrough on 401/403)
  • Redirect stripping (within hosts preserved, outside hosts stripped)
  • Negative tests (500/404 raise immediately, URLError, timeout propagation)

Copilot AI requested review from Copilot and removed request for Copilot April 28, 2026 17:52
Comment thread tests/test_authentication.py Fixed
Copilot AI changed the title [WIP] Add authentication provider registry for multi-platform support feat: Add authentication provider registry for multi-platform support (GitHub + Azure DevOps) Apr 28, 2026
Copilot AI requested a review from mnriem April 28, 2026 17:53
…through auth registry

- Add authentication/http.py with open_url() that tries each configured
  provider in registry order, falling through on 401/403 to the next,
  and finally to unauthenticated
- Add build_request() for one-shot request construction
- Add configured_providers() to registry __init__
- Remove api_base_url() from AuthProvider ABC (unused)
- Remove hosts attribute from providers (no host matching)
- Replace _github_http.py usage in ExtensionCatalog and PresetCatalog
- Wire IntegrationCatalog and WorkflowCatalog through open_url (were unauthenticated)
- Wire _fetch_latest_release_tag() through open_url
- Wire all inline --from-url downloads through open_url
- Fix unused stub variable flagged by code-quality bot
- 49 auth tests (positive + negative), 1805 total tests passing
Copilot AI review requested due to automatic review settings April 28, 2026 19:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an authentication provider registry to support multiple platforms (GitHub + Azure DevOps) and refactors outbound HTTP calls to use shared authenticated helpers.

Changes:

  • Introduces specify_cli.authentication registry + AuthProvider base, with GitHubAuth and AzureDevOpsAuth implementations.
  • Adds authentication.http helpers (build_request, open_url) and refactors catalog/workflow fetching and release-tag checks to use them.
  • Updates/expands tests to cover registry behavior, provider token resolution, and HTTP delegation.
Show a summary per file
File Description
tests/test_presets.py Updates auth-related request tests for new auth helper behavior.
tests/test_extensions.py Updates auth-related request tests for new auth helper behavior.
tests/test_authentication.py Adds comprehensive tests for registry, providers, and HTTP helper behavior.
tests/integrations/test_integration_catalog.py Adjusts urlopen monkeypatching to target new auth HTTP module.
src/specify_cli/workflows/catalog.py Switches catalog fetching to use authentication.http.open_url.
src/specify_cli/presets.py Routes request/open helpers through new auth HTTP module.
src/specify_cli/integrations/catalog.py Switches catalog fetching to use authentication.http.open_url.
src/specify_cli/extensions.py Routes request/open helpers through new auth HTTP module.
src/specify_cli/authentication/http.py Implements shared authenticated request building + retry/fallthrough behavior.
src/specify_cli/authentication/github.py Adds GitHub token resolution + Bearer header construction.
src/specify_cli/authentication/base.py Adds AuthProvider ABC with get_token, auth_headers, is_configured.
src/specify_cli/authentication/azure_devops.py Adds Azure DevOps PAT/token resolution + Basic auth header construction.
src/specify_cli/authentication/init.py Implements global provider registry + builtin registration.
src/specify_cli/init.py Refactors _fetch_latest_release_tag() and URL-download paths to use open_url.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 14/14 changed files
  • Comments generated: 7

Comment thread src/specify_cli/presets.py
Comment thread src/specify_cli/extensions.py
Comment thread tests/test_presets.py Outdated
Comment thread tests/test_extensions.py Outdated
Comment thread src/specify_cli/authentication/http.py
Comment thread src/specify_cli/authentication/http.py Outdated
Comment thread src/specify_cli/__init__.py
mnriem added 2 commits April 28, 2026 14:53
…d extra_headers to open_url

- Fix _open_url() docstrings in extensions.py and presets.py that
  incorrectly claimed redirect stripping behavior
- Add extra_headers parameter to open_url() so callers can pass
  additional headers (e.g. Accept) that persist across retries
- Restore Accept: application/vnd.github+json header in
  _fetch_latest_release_tag() via extra_headers
Security-first redesign: no credentials are sent unless the user
explicitly creates ~/.specify/auth.json mapping hosts to providers.

- Add authentication/config.py: loads and validates auth.json with
  host-to-provider mappings, supports token/token_env/azure-ad/azure-cli
- Refactor AuthProvider ABC: auth_headers(token, scheme) + resolve_token(entry)
- Refactor GitHubAuth: bearer scheme only, token from config entry
- Refactor AzureDevOpsAuth: 4 schemes (basic-pat, bearer, azure-cli, azure-ad)
  with dynamic token acquisition for azure-cli and azure-ad
- Rewrite authentication/http.py: host matching, redirect stripping,
  provider fallthrough on 401/403, unauthenticated fallback
- Add docs/reference/authentication.md with full reference and template
- 1823 tests passing (67 auth-specific)
@mnriem mnriem changed the title feat: Add authentication provider registry for multi-platform support (GitHub + Azure DevOps) feat: Config-driven opt-in authentication registry with multi-platform support Apr 28, 2026
Copilot AI review requested due to automatic review settings April 28, 2026 20:32
Comment thread src/specify_cli/authentication/config.py Fixed
Comment thread src/specify_cli/authentication/config.py Fixed
Comment thread src/specify_cli/authentication/config.py Fixed
Comment thread tests/test_authentication.py Fixed
Comment thread tests/test_authentication.py Fixed
Comment thread tests/test_authentication.py Fixed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 17/17 changed files
  • Comments generated: 13

Comment on lines +35 to +42
| Field | Required | Description |
|---|---|---|
| `hosts` | Yes | Array of hostnames this entry applies to. Supports wildcards (e.g. `*.visualstudio.com`). |
| `provider` | Yes | Built-in provider key: `github` or `azure-devops`. |
| `auth` | Yes | Auth scheme (see below). |
| `token` | No | Token value (inline). Use `token_env` instead when possible. |
| `token_env` | No | Environment variable name to read the token from. |

Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The tables use leading double pipes (||) which creates an empty first column and is likely unintended / may break markdown rendering and linting. Consider converting these to standard markdown table syntax with single leading/trailing |.

Copilot uses AI. Check for mistakes.
Comment on lines +104 to +111
auth = entry_raw.get("auth", "")
if not isinstance(auth, str) or not auth:
raise ValueError(f"providers[{i}]: 'auth' must be a non-empty string")
if auth not in _VALID_AUTH_SCHEMES:
raise ValueError(
f"providers[{i}]: unsupported auth scheme {auth!r}; "
f"valid: {sorted(_VALID_AUTH_SCHEMES)}"
)
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

auth is validated against a global set, but the config doesn't validate that the selected provider actually supports the requested scheme (e.g., provider="github" + auth="basic-pat" would pass here but later raise at request time). Consider validating provider+scheme compatibility during load (using the provider registry) and raising a clear ValueError that points to the offending entry.

Copilot uses AI. Check for mistakes.
Comment on lines +11 to +16
import os
import stat
from dataclasses import dataclass, field
from fnmatch import fnmatch
from pathlib import Path
from typing import Any
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

There are several unused imports here (os, field, Any). They add noise and can fail strict linting; please remove them.

Suggested change
import os
import stat
from dataclasses import dataclass, field
from fnmatch import fnmatch
from pathlib import Path
from typing import Any
import stat
from dataclasses import dataclass
from fnmatch import fnmatch
from pathlib import Path

Copilot uses AI. Check for mistakes.
Comment on lines +63 to +101
def build_request(url: str, extra_headers: dict[str, str] | None = None) -> urllib.request.Request:
"""Build a :class:`~urllib.request.Request`, attaching auth when config matches.

Uses the first matching entry from ``auth.json`` whose token resolves.
Returns a plain request when no entry matches or the file doesn't exist.
"""
headers: dict[str, str] = {}
entries = find_entries_for_url(url, _load_config())
for entry in entries:
provider = get_provider(entry.provider)
if provider is None:
continue
token = provider.resolve_token(entry)
if token:
headers.update(provider.auth_headers(token, entry.auth))
break
if extra_headers:
headers.update(extra_headers)
return urllib.request.Request(url, headers=headers)


def open_url(url: str, timeout: int = 10, extra_headers: dict[str, str] | None = None):
"""Open *url* with config-driven auth, redirect stripping, and fallthrough.

1. Find ``auth.json`` entries whose hosts match the URL.
2. For each entry, resolve the token and try the request.
3. On 401/403 move to the next matching entry.
4. After all entries exhausted (or none matched), try unauthenticated.
5. Non-auth errors (404, 500, network) raise immediately.

*extra_headers* (e.g. ``Accept``) are merged into every attempt.
"""
entries = find_entries_for_url(url, _load_config())

def _make_req(auth_headers: dict[str, str]) -> urllib.request.Request:
merged = {**auth_headers}
if extra_headers:
merged.update(extra_headers)
return urllib.request.Request(url, headers=merged)
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

extra_headers are merged after provider-generated auth headers in both build_request() and open_url(), which allows callers to override Authorization (accidentally or intentionally) and potentially bypass the configured provider behavior. Consider merging extra_headers first and then applying provider auth headers last, or explicitly rejecting/ignoring an Authorization key in extra_headers.

Copilot uses AI. Check for mistakes.
Comment thread src/specify_cli/authentication/http.py Outdated
Comment on lines +104 to +113
tried = 0
for entry in entries:
provider = get_provider(entry.provider)
if provider is None:
continue
token = provider.resolve_token(entry)
if not token:
continue
tried += 1

Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

tried is incremented but never read. This looks like leftover instrumentation; please remove it (or use it) to avoid dead code.

Copilot uses AI. Check for mistakes.
Comment thread src/specify_cli/authentication/config.py
Comment thread src/specify_cli/authentication/config.py
Comment thread src/specify_cli/authentication/http.py
Comment thread src/specify_cli/authentication/azure_devops.py Outdated
Comment thread src/specify_cli/__init__.py
…heme validation, security hardening

- Remove unused imports (os, field, Any) in config.py
- Normalize hosts during load (strip + lowercase)
- Validate token/token_env are non-empty strings during load
- Validate provider+scheme compatibility during load
- Fix extra_headers order: auth headers applied last, cannot be overridden
- Remove unused 'tried' variable in http.py
- Warn (once) on malformed auth.json instead of silent fallback
- URL-encode OAuth2 client credentials body in azure_devops.py
- Update 403 message to mention auth.json configuration
- Fix registry leak in test_register_duplicate (try/finally)
- Fix import style consistency in test_authentication.py
- Add azure-cli and azure-ad token acquisition tests (mock subprocess/urlopen)
- Add autouse fixture to isolate upgrade tests from real auth.json
- 1829 tests passing
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 17/17 changed files
  • Comments generated: 4

Comment thread src/specify_cli/authentication/config.py
Comment thread src/specify_cli/authentication/config.py
Comment thread src/specify_cli/authentication/http.py
Comment thread src/specify_cli/__init__.py
…ization from extra_headers

- Reject unknown provider keys during auth.json load with clear error message
- Validate azure-ad tenant_id/client_id/client_secret_env as non-empty strings
- Strip Authorization from extra_headers in both build_request and open_url
  to prevent accidental or intentional bypass of provider-configured auth
- Add tests for unknown provider and incompatible scheme validation
- 1831 tests passing
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 17/17 changed files
  • Comments generated: 4

Comment thread tests/test_presets.py
Comment thread tests/test_extensions.py
Comment thread src/specify_cli/authentication/config.py Outdated
Comment thread tests/test_upgrade.py Outdated
… docstring

- Move _inject_github_config / make_github_auth_entry to tests/auth_helpers.py
  to eliminate duplication across test_extensions, test_presets, test_upgrade
- Move auth config isolation fixture to global conftest.py (autouse) so ALL
  tests are isolated from ~/.specify/auth.json, not just test_upgrade
- Align load_auth_config docstring with actual behavior: ValueError may be
  caught by higher-level HTTP helpers that warn and continue unauthenticated
- 1831 tests passing
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 19/19 changed files
  • Comments generated: 1

Comment on lines +56 to +63
def redirect_request(self, req, fp, code, msg, headers, newurl):
original_auth = req.get_header("Authorization")
new_req = super().redirect_request(req, fp, code, msg, headers, newurl)
if new_req is not None:
hostname = (urlparse(newurl).hostname or "").lower()
if _hostname_in_hosts(hostname, self._hosts):
if original_auth:
new_req.add_unredirected_header("Authorization", original_auth)
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

In _StripAuthOnRedirect.redirect_request, original_auth is read via req.get_header("Authorization"), but this handler itself stores the preserved header on redirects using add_unredirected_header(...) (i.e., req.unredirected_hdrs). On a multi-hop redirect chain within allowed hosts, subsequent calls to redirect_request may see original_auth as None and fail to preserve the header. Consider reading from both req.get_header("Authorization") and req.unredirected_hdrs.get("Authorization") (and/or ensuring the header is present in the place get_header reads from) so auth survives multiple redirects within the configured host set.

Copilot uses AI. Check for mistakes.
- Read Authorization from both headers and unredirected_hdrs in
  _StripAuthOnRedirect to survive multi-hop chains within allowed hosts
- Add test_multi_hop_redirect_within_hosts_preserves_auth
- 1832 tests passing
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.

[Feature] Add authentication provider registry for multi-platform support (GitHub + Azure DevOps)

3 participants