mirror of
https://github.com/github/spec-kit.git
synced 2026-07-07 06:35:06 +08:00
feat: Config-driven opt-in authentication registry with multi-platform support (#2393)
* Initial plan * feat: add authentication provider registry (GitHub + Azure DevOps) Agent-Logs-Url: https://github.com/github/spec-kit/sessions/da7ecfd0-e1c9-48dc-b692-27be0879e976 Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * feat: add try-each-provider HTTP helper and wire all catalog fetches 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 * fix: address review — fix stale docstrings, restore Accept header, add 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 * feat: config-driven opt-in auth via ~/.specify/auth.json 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) * fix: address review — unused imports, host normalization, provider+scheme 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 * fix: reject unknown providers, validate azure-ad fields, strip Authorization 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 * fix: extract shared auth test helpers, global config isolation, align 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 * fix: preserve auth header across multi-hop redirect chains - 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 * fix: use resolved config path in warning/error messages and patch build_opener in no-network test Agent-Logs-Url: https://github.com/github/spec-kit/sessions/86df9557-54f1-4fe4-a25f-9501cb2356cf Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * fix: assert full resolved config path in rate-limit output test Agent-Logs-Url: https://github.com/github/spec-kit/sessions/86df9557-54f1-4fe4-a25f-9501cb2356cf Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * fix: close HTTPError on 401/403, remove _VALID_AUTH_SCHEMES, catch TimeoutExpired, skip POSIX test on Windows, remove unused import Agent-Logs-Url: https://github.com/github/spec-kit/sessions/a1e29737-dd6e-4287-96c1-509e0c96fb21 Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * fix: use stable ~/.specify/auth.json in rate-limit message, skip POSIX permission check on Windows Agent-Logs-Url: https://github.com/github/spec-kit/sessions/4636bcdb-87ae-45d6-9545-a40e4effd617 Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * fix: validate host patterns, cache auth config per-process Agent-Logs-Url: https://github.com/github/spec-kit/sessions/889b58a7-7f8c-47e2-8056-931ebcc671cc Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * fix: clarify _is_valid_host_pattern docstring, clean up test sentinel type Agent-Logs-Url: https://github.com/github/spec-kit/sessions/889b58a7-7f8c-47e2-8056-931ebcc671cc Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * fix: improve _is_valid_host_pattern docstring and test observability Agent-Logs-Url: https://github.com/github/spec-kit/sessions/889b58a7-7f8c-47e2-8056-931ebcc671cc Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Copilot
@@ -1762,22 +1762,14 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]:
|
||||
On anything else — including a malformed response body — the exception
|
||||
propagates; there is no catch-all (research D-006).
|
||||
"""
|
||||
req = urllib.request.Request(
|
||||
GITHUB_API_LATEST,
|
||||
headers={"Accept": "application/vnd.github+json"},
|
||||
)
|
||||
token = None
|
||||
for env_var in ("GH_TOKEN", "GITHUB_TOKEN"):
|
||||
candidate = os.environ.get(env_var)
|
||||
if candidate is not None:
|
||||
candidate = candidate.strip()
|
||||
if candidate:
|
||||
token = candidate
|
||||
break
|
||||
if token:
|
||||
req.add_header("Authorization", f"Bearer {token}")
|
||||
from .authentication.http import open_url
|
||||
|
||||
try:
|
||||
with urllib.request.urlopen(req, timeout=5) as resp:
|
||||
with open_url(
|
||||
GITHUB_API_LATEST,
|
||||
timeout=5,
|
||||
extra_headers={"Accept": "application/vnd.github+json"},
|
||||
) as resp:
|
||||
payload = json.loads(resp.read().decode("utf-8"))
|
||||
tag = payload.get("tag_name")
|
||||
if not isinstance(tag, str) or not tag:
|
||||
@@ -1786,7 +1778,9 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]:
|
||||
except urllib.error.HTTPError as e:
|
||||
# Order matters: HTTPError is a subclass of URLError.
|
||||
if e.code == 403:
|
||||
return None, "rate limited (try setting GH_TOKEN or GITHUB_TOKEN)"
|
||||
return None, (
|
||||
"rate limited (configure ~/.specify/auth.json with a GitHub token)"
|
||||
)
|
||||
return None, f"HTTP {e.code}"
|
||||
except (urllib.error.URLError, OSError):
|
||||
return None, "offline or timeout"
|
||||
@@ -3381,7 +3375,9 @@ def preset_add(
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
zip_path = Path(tmpdir) / "preset.zip"
|
||||
try:
|
||||
with urllib.request.urlopen(from_url, timeout=60) as response:
|
||||
from specify_cli.authentication.http import open_url as _open_url
|
||||
|
||||
with _open_url(from_url, timeout=60) as response:
|
||||
zip_path.write_bytes(response.read())
|
||||
except urllib.error.URLError as e:
|
||||
console.print(f"[red]Error:[/red] Failed to download: {e}")
|
||||
@@ -4285,7 +4281,9 @@ def extension_add(
|
||||
zip_path = download_dir / f"{extension}-url-download.zip"
|
||||
|
||||
try:
|
||||
with urllib.request.urlopen(from_url, timeout=60) as response:
|
||||
from specify_cli.authentication.http import open_url as _open_url
|
||||
|
||||
with _open_url(from_url, timeout=60) as response:
|
||||
zip_data = response.read()
|
||||
zip_path.write_bytes(zip_data)
|
||||
|
||||
@@ -5500,7 +5498,7 @@ def workflow_add(
|
||||
if source.startswith("http://") or source.startswith("https://"):
|
||||
from ipaddress import ip_address
|
||||
from urllib.parse import urlparse
|
||||
from urllib.request import urlopen # noqa: S310
|
||||
from specify_cli.authentication.http import open_url as _open_url
|
||||
|
||||
parsed_src = urlparse(source)
|
||||
src_host = parsed_src.hostname or ""
|
||||
@@ -5517,7 +5515,7 @@ def workflow_add(
|
||||
|
||||
import tempfile
|
||||
try:
|
||||
with urlopen(source, timeout=30) as resp: # noqa: S310
|
||||
with _open_url(source, timeout=30) as resp:
|
||||
final_url = resp.geturl()
|
||||
final_parsed = urlparse(final_url)
|
||||
final_host = final_parsed.hostname or ""
|
||||
@@ -5613,10 +5611,10 @@ def workflow_add(
|
||||
workflow_file = workflow_dir / "workflow.yml"
|
||||
|
||||
try:
|
||||
from urllib.request import urlopen # noqa: S310 — URL comes from catalog
|
||||
from specify_cli.authentication.http import open_url as _open_url
|
||||
|
||||
workflow_dir.mkdir(parents=True, exist_ok=True)
|
||||
with urlopen(workflow_url, timeout=30) as response: # noqa: S310
|
||||
with _open_url(workflow_url, timeout=30) as response:
|
||||
# Validate final URL after redirects
|
||||
final_url = response.geturl()
|
||||
final_parsed = urlparse(final_url)
|
||||
|
||||
Reference in New Issue
Block a user