Skip to content

Commit 36919ec

Browse files
committed
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
1 parent 543ed48 commit 36919ec

3 files changed

Lines changed: 41 additions & 3 deletions

File tree

src/specify_cli/authentication/config.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,13 @@ def load_auth_config(
122122
# Validate provider+scheme compatibility
123123
from . import get_provider as _get_provider
124124
_prov = _get_provider(provider)
125-
if _prov is not None and auth not in _prov.supported_auth_schemes:
125+
if _prov is None:
126+
from . import AUTH_REGISTRY
127+
raise ValueError(
128+
f"providers[{i}]: unknown provider {provider!r}; "
129+
f"registered: {sorted(AUTH_REGISTRY.keys())}"
130+
)
131+
if auth not in _prov.supported_auth_schemes:
126132
raise ValueError(
127133
f"providers[{i}]: provider {provider!r} does not support "
128134
f"auth scheme {auth!r}; supported: {list(_prov.supported_auth_schemes)}"
@@ -143,6 +149,15 @@ def load_auth_config(
143149
f"providers[{i}]: auth='azure-ad' requires "
144150
"'tenant_id', 'client_id', and 'client_secret_env'"
145151
)
152+
for field_name, field_val in [
153+
("tenant_id", tenant_id),
154+
("client_id", client_id),
155+
("client_secret_env", client_secret_env),
156+
]:
157+
if not isinstance(field_val, str) or not field_val.strip():
158+
raise ValueError(
159+
f"providers[{i}]: '{field_name}' must be a non-empty string"
160+
)
146161
# azure-cli needs no extra fields
147162

148163
entries.append(

src/specify_cli/authentication/http.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ def build_request(url: str, extra_headers: dict[str, str] | None = None) -> urll
7575
"""
7676
headers: dict[str, str] = {}
7777
if extra_headers:
78-
headers.update(extra_headers)
78+
# Strip Authorization from extra_headers to prevent bypass
79+
headers.update({k: v for k, v in extra_headers.items() if k.lower() != "authorization"})
7980
# Auth headers applied last — cannot be overridden by extra_headers
8081
entries = find_entries_for_url(url, _load_config())
8182
for entry in entries:
@@ -105,7 +106,8 @@ def open_url(url: str, timeout: int = 10, extra_headers: dict[str, str] | None =
105106
def _make_req(auth_headers: dict[str, str]) -> urllib.request.Request:
106107
merged = {}
107108
if extra_headers:
108-
merged.update(extra_headers)
109+
# Strip Authorization from extra_headers to prevent bypass
110+
merged.update({k: v for k, v in extra_headers.items() if k.lower() != "authorization"})
109111
# Auth headers applied last — cannot be overridden by extra_headers
110112
merged.update(auth_headers)
111113
return urllib.request.Request(url, headers=merged)

tests/test_authentication.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,27 @@ def test_azure_ad_missing_fields_raises(self, tmp_path):
222222
with pytest.raises(ValueError, match="azure-ad"):
223223
load_auth_config(cfg)
224224

225+
def test_unknown_provider_raises(self, tmp_path):
226+
cfg = tmp_path / "auth.json"
227+
cfg.write_text(json.dumps({
228+
"providers": [{"hosts": ["example.com"], "provider": "gitlab", "auth": "bearer", "token_env": "X"}]
229+
}))
230+
with pytest.raises(ValueError, match="unknown provider"):
231+
load_auth_config(cfg)
232+
233+
def test_incompatible_provider_scheme_raises(self, tmp_path):
234+
cfg = tmp_path / "auth.json"
235+
cfg.write_text(json.dumps({
236+
"providers": [{
237+
"hosts": ["github.com"],
238+
"provider": "github",
239+
"auth": "basic-pat",
240+
"token_env": "X",
241+
}]
242+
}))
243+
with pytest.raises(ValueError, match="does not support"):
244+
load_auth_config(cfg)
245+
225246
def test_world_readable_warns(self, tmp_path):
226247
import stat
227248

0 commit comments

Comments
 (0)