mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
* fix(auth): resolve az via shutil.which so azure-cli token works on Windows
AzureDevOpsAuth._acquire_via_az_cli runs subprocess.run with a bare "az".
On Windows the Azure CLI is installed as az.cmd, and subprocess.run calls
CreateProcess, which does not consult PATHEXT -- so a bare "az" fails with
WinError 2 even after `az login`, and azure-cli token acquisition silently
returns None (the OSError is swallowed).
Resolve the executable with shutil.which("az") (which honors PATHEXT) before
the call, mirroring the maintainer's own fix in integrations/base.py for the
same CreateProcess/.cmd issue. `or "az"` preserves prior behavior (and the
existing not-installed OSError path) when az is absent. POSIX is unaffected.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(auth): require an absolute az path so the CWD cannot hijack the lookup
Self-review catch on my own change: resolving with a bare
`shutil.which("az") or "az"` widened an execution surface. On Windows
shutil.which prepends the CURRENT DIRECTORY to the search path (unless
NoDefaultCurrentDirectoryInExePath is set) AND honors PATHEXT, so a stray
.\az.cmd / .\az.bat in the working directory resolves ahead of the real Azure
CLI -- for a credential operation. Verified: with the real az scrubbed from
PATH, shutil.which("az") returns '.\az.CMD'.
Accept the resolution only when it is absolute; otherwise fall back to the bare
"az" (which also preserves the existing not-installed OSError path). A
legitimate install always resolves absolutely, so the Windows .cmd fix this PR
exists for is unaffected. The not-installed and PATHEXT tests are extended with
relative-result cases, all of which fail before this commit.
Note: integrations/base.py resolves executables the same way; hardening that
shared path is a separate concern and is left untouched here.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(auth): build the mocked az path with the host's path rules
Fixes the macOS CI failure. The test hardcoded a Windows absolute path, but the
production code calls os.path.isabs() -- on POSIX runners "C:\Program
Files\..." reads as RELATIVE, so the fallback branch ran and argv[0] was "az"
instead of the resolved path.
Construct the path with os.path.join(os.path.abspath(os.sep), ...) so it is
absolute under the host's rules, and assert against that value. The fallback
test's inputs (".\az.CMD", "az.cmd", "./az") are relative under both ntpath
and posixpath, so they were already portable.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
189 lines
7.4 KiB
Python
189 lines
7.4 KiB
Python
"""Azure DevOps authentication provider."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import base64
|
|
import json as _json
|
|
import os
|
|
import shutil
|
|
import subprocess
|
|
from typing import TYPE_CHECKING
|
|
|
|
from .._download_security import MAX_JSON_METADATA_BYTES, read_response_limited
|
|
from .base import AuthProvider
|
|
|
|
if TYPE_CHECKING:
|
|
from .config import AuthConfigEntry
|
|
|
|
# Azure DevOps resource ID for OAuth / Azure AD token acquisition.
|
|
_ADO_RESOURCE_ID = "499b84ac-1321-427f-aa17-267ca6975798"
|
|
|
|
|
|
class _TokenResponseTooLarge(Exception):
|
|
"""Raised when an Azure AD token response exceeds the bounded read limit."""
|
|
|
|
|
|
def _extract_token(payload: object, key: str) -> str | None:
|
|
"""Return a normalized token from a JSON object, or None for other shapes."""
|
|
if not isinstance(payload, dict):
|
|
return None
|
|
token = payload.get(key)
|
|
if not isinstance(token, str):
|
|
return None
|
|
return token.strip() or None
|
|
|
|
|
|
class AzureDevOpsAuth(AuthProvider):
|
|
"""Azure DevOps authentication provider.
|
|
|
|
Supports four auth schemes:
|
|
|
|
* ``basic-pat`` — PAT with empty username, Base64-encoded as ``:<PAT>``
|
|
* ``bearer`` — pre-acquired OAuth / Azure AD token
|
|
* ``azure-cli`` — acquires a token via ``az account get-access-token``
|
|
* ``azure-ad`` — acquires a token via OAuth2 client credentials flow
|
|
"""
|
|
|
|
key = "azure-devops"
|
|
supported_auth_schemes = ("basic-pat", "bearer", "azure-cli", "azure-ad")
|
|
|
|
def auth_headers(self, token: str, auth_scheme: str) -> dict[str, str]:
|
|
"""Build the ``Authorization`` header for the given scheme."""
|
|
if auth_scheme == "basic-pat":
|
|
encoded = base64.b64encode(f":{token}".encode("ascii")).decode("ascii")
|
|
return {"Authorization": f"Basic {encoded}"}
|
|
if auth_scheme in ("bearer", "azure-cli", "azure-ad"):
|
|
return {"Authorization": f"Bearer {token}"}
|
|
raise ValueError(
|
|
f"AzureDevOpsAuth does not support auth scheme {auth_scheme!r}"
|
|
)
|
|
|
|
def resolve_token(self, entry: AuthConfigEntry) -> str | None:
|
|
"""Resolve token, with special handling for azure-cli and azure-ad."""
|
|
if entry.auth == "azure-cli":
|
|
return self._acquire_via_az_cli()
|
|
if entry.auth == "azure-ad":
|
|
return self._acquire_via_client_credentials(entry)
|
|
return super().resolve_token(entry)
|
|
|
|
# -- Token acquisition ------------------------------------------------
|
|
|
|
@staticmethod
|
|
def _acquire_via_az_cli() -> str | None:
|
|
"""Run ``az account get-access-token`` and return the access token."""
|
|
try:
|
|
# Windows: ``subprocess.run`` calls ``CreateProcess``, which does
|
|
# not consult ``PATHEXT``, so a bare ``"az"`` (installed as
|
|
# ``az.cmd``) fails with ``WinError 2`` even after ``az login``.
|
|
# Resolve via ``shutil.which`` (which honors ``PATHEXT``) so the
|
|
# ``.cmd`` shim works. On POSIX this is a harmless lookup that
|
|
# returns the same executable.
|
|
#
|
|
# Require an ABSOLUTE result: on Windows ``shutil.which`` prepends
|
|
# the current directory to the search path (unless
|
|
# ``NoDefaultCurrentDirectoryInExePath`` is set), so a stray
|
|
# ``.\az.cmd`` in the working directory would otherwise be resolved
|
|
# ahead of the real Azure CLI and run for a credential operation. A
|
|
# legitimate install always resolves to an absolute path, so this
|
|
# costs nothing; falling back to the bare ``"az"`` preserves the
|
|
# prior behavior (and the existing OSError path) when ``az`` is
|
|
# absent.
|
|
resolved = shutil.which("az")
|
|
az = resolved if resolved and os.path.isabs(resolved) else "az"
|
|
result = subprocess.run( # noqa: S603, S607
|
|
[
|
|
az,
|
|
"account",
|
|
"get-access-token",
|
|
"--resource",
|
|
_ADO_RESOURCE_ID,
|
|
"--output",
|
|
"json",
|
|
],
|
|
capture_output=True,
|
|
text=True,
|
|
timeout=30,
|
|
check=False,
|
|
)
|
|
if result.returncode != 0:
|
|
return None
|
|
payload = _json.loads(result.stdout)
|
|
return _extract_token(payload, "accessToken")
|
|
except (
|
|
OSError,
|
|
subprocess.TimeoutExpired,
|
|
_json.JSONDecodeError,
|
|
UnicodeDecodeError,
|
|
KeyError,
|
|
):
|
|
# UnicodeDecodeError: text=True decodes az stdout with the locale
|
|
# encoding, which raises (not a JSONDecodeError) if the output isn't
|
|
# decodable — this helper's contract is to return None on any
|
|
# failure, never to propagate.
|
|
return None
|
|
|
|
@staticmethod
|
|
def _acquire_via_client_credentials(entry: AuthConfigEntry) -> str | None:
|
|
"""Acquire a token via OAuth2 client credentials flow."""
|
|
import urllib.error
|
|
import urllib.request
|
|
|
|
if not entry.tenant_id or not entry.client_id or not entry.client_secret_env:
|
|
return None
|
|
client_secret = os.environ.get(entry.client_secret_env, "").strip()
|
|
if not client_secret:
|
|
return None
|
|
|
|
url = (
|
|
f"https://login.microsoftonline.com/{entry.tenant_id}"
|
|
"/oauth2/v2.0/token"
|
|
)
|
|
from urllib.parse import urlencode
|
|
body = urlencode({
|
|
"grant_type": "client_credentials",
|
|
"client_id": entry.client_id,
|
|
"client_secret": client_secret,
|
|
"scope": f"{_ADO_RESOURCE_ID}/.default",
|
|
}).encode("utf-8")
|
|
|
|
req = urllib.request.Request(
|
|
url,
|
|
data=body,
|
|
headers={"Content-Type": "application/x-www-form-urlencoded"},
|
|
)
|
|
try:
|
|
from specify_cli.authentication.http import _StripAuthOnRedirect
|
|
|
|
def reject_token_redirect(_old_url: str, new_url: str) -> None:
|
|
# A 307/308 redirect preserves this POST body, including the
|
|
# client_secret. Refuse every redirect so credentials cannot
|
|
# leave the fixed Microsoft token endpoint.
|
|
raise urllib.error.URLError(
|
|
f"Azure AD token request must not be redirected to {new_url}"
|
|
)
|
|
|
|
opener = urllib.request.build_opener(
|
|
_StripAuthOnRedirect((), reject_token_redirect)
|
|
)
|
|
with opener.open(req, timeout=30) as resp: # noqa: S310
|
|
payload = _json.loads(
|
|
read_response_limited(
|
|
resp,
|
|
max_bytes=MAX_JSON_METADATA_BYTES,
|
|
error_type=_TokenResponseTooLarge,
|
|
label="Azure DevOps token response",
|
|
).decode("utf-8")
|
|
)
|
|
return _extract_token(payload, "access_token")
|
|
except (
|
|
urllib.error.URLError,
|
|
OSError,
|
|
_json.JSONDecodeError,
|
|
UnicodeDecodeError,
|
|
_TokenResponseTooLarge,
|
|
):
|
|
# Network failure, malformed JSON, or an oversized response — fall
|
|
# through to the next strategy. Unrelated programming errors (other
|
|
# ValueErrors, KeyErrors) intentionally propagate so they surface.
|
|
return None
|