diff --git a/src/specify_cli/authentication/config.py b/src/specify_cli/authentication/config.py index 8d1faf80c3..829940d6f7 100644 --- a/src/specify_cli/authentication/config.py +++ b/src/specify_cli/authentication/config.py @@ -13,6 +13,7 @@ from dataclasses import dataclass from fnmatch import fnmatch from pathlib import Path +from typing import Any from urllib.parse import urlparse @@ -53,6 +54,19 @@ def _is_valid_host_pattern(pattern: str) -> bool: return pattern.startswith("*.") and "*" not in pattern[2:] +def _norm(value: Any) -> Any: + """Strip surrounding whitespace from a whitespace-insignificant string + config reference (env-var names, tenant/client ids) before it is stored. + + These fields are validated on their ``.strip()``ed form, so an accidentally + padded value passes validation but then silently breaks the verbatim + ``os.environ.get(...)`` / URL lookups downstream. Normalizing at store time + mirrors how ``hosts`` is already handled (``h.strip().lower()``). Non-string + values (e.g. ``None``) pass through unchanged. + """ + return value.strip() if isinstance(value, str) else value + + def load_auth_config( path: Path | None = None, ) -> list[AuthConfigEntry]: @@ -182,10 +196,10 @@ def load_auth_config( provider=provider, auth=auth, token=token, - token_env=token_env, - tenant_id=entry_raw.get("tenant_id"), - client_id=entry_raw.get("client_id"), - client_secret_env=entry_raw.get("client_secret_env"), + token_env=_norm(token_env), + tenant_id=_norm(entry_raw.get("tenant_id")), + client_id=_norm(entry_raw.get("client_id")), + client_secret_env=_norm(entry_raw.get("client_secret_env")), ) ) diff --git a/tests/test_authentication.py b/tests/test_authentication.py index 06523c6f26..9edbd9860a 100644 --- a/tests/test_authentication.py +++ b/tests/test_authentication.py @@ -92,6 +92,27 @@ def test_valid_github_config(self, tmp_path): assert entries[0].auth == "bearer" assert entries[0].token_env == "GH_TOKEN" + def test_padded_token_env_is_normalized_and_resolves(self, tmp_path, monkeypatch): + # token_env is validated on its stripped form but was stored raw, so a + # padded env-var name passed validation yet broke the verbatim + # os.environ.get() lookup — resolve_token silently returned None. + monkeypatch.setenv("GH_TOKEN", "secret-tok") + cfg = tmp_path / "auth.json" + cfg.write_text(json.dumps({ + "providers": [{ + "hosts": ["github.com"], + "provider": "github", + "auth": "bearer", + "token_env": " GH_TOKEN ", + }] + })) + entries = load_auth_config(cfg) + assert len(entries) == 1 + # Stored normalized (matching how hosts are normalized), ... + assert entries[0].token_env == "GH_TOKEN" + # ... so the env lookup finds the token instead of returning None. + assert GitHubAuth().resolve_token(entries[0]) == "secret-tok" + def test_valid_ado_config(self, tmp_path): cfg = tmp_path / "auth.json" cfg.write_text(json.dumps({ @@ -136,6 +157,26 @@ def test_azure_ad_config(self, tmp_path): assert entries[0].auth == "azure-ad" assert entries[0].tenant_id == "tid" + def test_padded_azure_ad_refs_are_normalized(self, tmp_path): + # The normalization also covers tenant_id / client_id / client_secret_env + # (used verbatim in the OAuth token URL/body and os.environ.get). Padded + # values were validated on their stripped form but stored raw. + cfg = tmp_path / "auth.json" + cfg.write_text(json.dumps({ + "providers": [{ + "hosts": ["dev.azure.com"], + "provider": "azure-devops", + "auth": "azure-ad", + "tenant_id": " tid ", + "client_id": " cid ", + "client_secret_env": " SECRET ", + }] + })) + entries = load_auth_config(cfg) + assert entries[0].tenant_id == "tid" + assert entries[0].client_id == "cid" + assert entries[0].client_secret_env == "SECRET" + def test_azure_cli_config(self, tmp_path): cfg = tmp_path / "auth.json" cfg.write_text(json.dumps({