Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions src/specify_cli/authentication/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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]:
Expand Down Expand Up @@ -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")),
Comment thread
jawwad-ali marked this conversation as resolved.
)
)

Expand Down
41 changes: 41 additions & 0 deletions tests/test_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand Down