From ccdb7f1dc9ec59771db9f9127f1b688812605468 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Thu, 22 Oct 2026 18:34:03 +0100 Subject: [PATCH 1/2] fix(no-ticket): show OIDC credentials as OIDC in whoami --verbose Since #383, the OIDC provider returns its exchanged token as a bearer token. whoami treated every bearer token as a SAML session, so it showed an OIDC credential as "SAML Token (primary)", with the keyring as the only possible source. The OIDC label and source were no longer reachable. whoami now detects an OIDC credential from the credential chain before it looks at the headers. The text output shows "OIDC Auto-Discovery" and the source, such as "OIDC via GitHub Actions (org: ..., service: ...)". The JSON output gives active_method "oidc" and the source in auth.oidc. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 + cloudsmith_cli/cli/commands/whoami.py | 33 +++++--- .../cli/tests/commands/test_whoami.py | 77 +++++++++++++++++++ 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02b6919fe..db7fb74d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Fixed + +- `cloudsmith whoami --verbose` now shows a credential from OIDC auto-discovery as `Authentication Method: OIDC Auto-Discovery`, with its source, for example `OIDC via GitHub Actions (org: acme, service: ci-service)`. Since 1.26.0, it showed these credentials as `SAML Token (primary)` with no source. In the JSON output, `auth.active_method` is now `oidc` instead of `sso_token`, and the new `auth.oidc.source` field gives the source. + ## [1.30.0] - 2026-10-22 ### Added diff --git a/cloudsmith_cli/cli/commands/whoami.py b/cloudsmith_cli/cli/commands/whoami.py index 4c52357b6..0df3bb498 100644 --- a/cloudsmith_cli/cli/commands/whoami.py +++ b/cloudsmith_cli/cli/commands/whoami.py @@ -10,8 +10,11 @@ from .main import main -def _get_active_method(api_config): - """Inspect API config to determine SAML, API key, or no auth.""" +def _get_active_method(api_config, credential=None): + """Inspect the credential and API config to determine OIDC, SAML, API key, or no auth.""" + # An exchanged OIDC token is sent as a bearer token, like a SAML session. + if credential is not None and credential.source_name == "oidc": + return "oidc" headers = getattr(api_config, "headers", {}) or {} if headers.get("Authorization", "").startswith("Bearer "): return "sso_token" @@ -36,6 +39,17 @@ def _get_api_key_source(opts): return {"configured": False, "source": None, "source_key": None} +def _get_oidc_source(opts): + """Return where the OIDC token came from, such as the CI vendor.""" + credential = getattr(opts, "credential", None) + if credential and credential.source_name == "oidc": + return { + "configured": True, + "source": credential.source_detail or credential.source_name, + } + return {"configured": False, "source": None} + + def _get_sso_status(api_host, profile=None): """Return SAML token status from the system keyring.""" enabled = keyring.should_use_keyring() @@ -72,8 +86,11 @@ def _get_verbose_auth_data(opts, api_host, profile=None): api_key_info["created"] = utils.fmt_datetime(created) if created else None return { - "active_method": _get_active_method(opts.api_config), + "active_method": _get_active_method( + opts.api_config, getattr(opts, "credential", None) + ), "api_key": api_key_info, + "oidc": _get_oidc_source(opts), "sso": sso_info, } @@ -111,13 +128,11 @@ def _print_verbose_text(data): if ak.get("source"): click.echo(f" Source: {ak['source']}") click.echo(" Note: SAML token is being used instead") + elif active == "oidc": + click.secho("Authentication Method: OIDC Auto-Discovery", fg="cyan", bold=True) + click.echo(f" Source: {auth['oidc']['source']}") elif active == "api_key": - if ak.get("source_key") == "oidc": - click.secho( - "Authentication Method: OIDC Auto-Discovery", fg="cyan", bold=True - ) - else: - click.secho("Authentication Method: API Key", fg="cyan", bold=True) + click.secho("Authentication Method: API Key", fg="cyan", bold=True) for label, field in [ ("Source", "source"), ("Token Slug", "slug"), diff --git a/cloudsmith_cli/cli/tests/commands/test_whoami.py b/cloudsmith_cli/cli/tests/commands/test_whoami.py index 187432235..4bdad6ee3 100644 --- a/cloudsmith_cli/cli/tests/commands/test_whoami.py +++ b/cloudsmith_cli/cli/tests/commands/test_whoami.py @@ -4,10 +4,12 @@ import click.testing import pytest +from ....core.credentials.models import CredentialResult from ...commands.whoami import whoami HOST = "https://api.example.com" ARGS = ["--api-host", HOST, "--api-key", "fake-api-key"] +OIDC_SOURCE = "OIDC via GitHub Actions (org: acme, service: ci-service)" @pytest.fixture() @@ -24,6 +26,37 @@ def invoke_whoami(runner, user_brief, extra_args=None): return runner.invoke(whoami, ARGS + (extra_args or [])) +def invoke_verbose_whoami(runner, credential, extra_args=None): + """Invoke whoami --verbose with the credential chain resolving to credential.""" + with ( + patch( + "cloudsmith_cli.cli.commands.whoami.get_user_brief", + return_value=(True, "ci-service", None, "CI service"), + ), + patch( + "cloudsmith_cli.core.credentials.chain.CredentialProviderChain.resolve", + return_value=credential, + ), + patch( + "cloudsmith_cli.cli.commands.whoami.keyring.has_sso_tokens", + return_value=False, + ), + ): + return runner.invoke( + whoami, ["--api-host", HOST, "--verbose", *(extra_args or [])] + ) + + +def oidc_credential(): + """Return a credential as the OIDC provider resolves it after an exchange.""" + return CredentialResult( + api_key="exchanged-token", + source_name="oidc", + source_detail=OIDC_SOURCE, + auth_type="bearer", + ) + + class TestWhoamiCommand: """Tests for the cloudsmith whoami command.""" @@ -71,3 +104,47 @@ def test_json_unauthenticated_exits_one(self, runner): "".join(line for line in result.output.splitlines() if line.startswith("{")) ) assert payload["data"]["is_authenticated"] is False + + +class TestWhoamiVerboseAuthMethod: + """whoami --verbose names the method and the source of the credential in use.""" + + def test_oidc_names_the_oidc_source(self, runner): + """An OIDC token is sent as a bearer token, but it is not a SAML session.""" + result = invoke_verbose_whoami(runner, oidc_credential()) + + assert result.exit_code == 0, result.output + assert "Authentication Method: OIDC Auto-Discovery" in result.output + assert f" Source: {OIDC_SOURCE}" in result.output + assert "SAML Token" not in result.output + + def test_json_oidc_names_the_oidc_source(self, runner): + """The JSON output gives the method as oidc, with the source of the token.""" + result = invoke_verbose_whoami( + runner, oidc_credential(), extra_args=["--output-format", "json"] + ) + + assert result.exit_code == 0, result.output + payload = json.loads( + "".join(line for line in result.output.splitlines() if line.startswith("{")) + ) + assert payload["data"]["auth"]["active_method"] == "oidc" + assert payload["data"]["auth"]["oidc"] == { + "configured": True, + "source": OIDC_SOURCE, + } + + def test_saml_session_is_still_a_saml_token(self, runner): + """A bearer token from the keyring is a SAML session.""" + saml = CredentialResult( + api_key="access-token", + source_name="keyring", + source_detail="System Keyring", + auth_type="bearer", + ) + + result = invoke_verbose_whoami(runner, saml) + + assert result.exit_code == 0, result.output + assert "Authentication Method: SAML Token (primary)" in result.output + assert "OIDC" not in result.output From 0bec0343beaca839a3e16c49093b398ea0890fd9 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Thu, 22 Oct 2026 18:43:38 +0100 Subject: [PATCH 2/2] fix(no-ticket): name the CI vendor in the source of a cached OIDC token The OIDC provider reads the token cache before it detects the CI environment, so a cached token showed "OIDC [cached]" with no vendor. Most commands in a CI job after the first one use the cached token. The cache now stores the name of the detector with the token, and get_cached_token returns both. The source of a cached token is then, for example, "OIDC via GitHub Actions [cached] (org: ..., service: ...)". An entry that an earlier version cached has no vendor name and keeps the old source until it expires. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + cloudsmith_cli/core/credentials/oidc/cache.py | 36 ++++++++-- .../credentials/providers/oidc_provider.py | 15 ++++- cloudsmith_cli/core/tests/test_oidc_cache.py | 65 +++++++++++++++++++ .../core/tests/test_oidc_provider.py | 64 +++++++++++++++++- 5 files changed, 170 insertions(+), 11 deletions(-) create mode 100644 cloudsmith_cli/core/tests/test_oidc_cache.py diff --git a/CHANGELOG.md b/CHANGELOG.md index db7fb74d1..432f5170d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - `cloudsmith whoami --verbose` now shows a credential from OIDC auto-discovery as `Authentication Method: OIDC Auto-Discovery`, with its source, for example `OIDC via GitHub Actions (org: acme, service: ci-service)`. Since 1.26.0, it showed these credentials as `SAML Token (primary)` with no source. In the JSON output, `auth.active_method` is now `oidc` instead of `sso_token`, and the new `auth.oidc.source` field gives the source. +- The source of a cached OIDC token now names the CI vendor, for example `OIDC via GitHub Actions [cached] (org: acme, service: ci-service)`. The OIDC token cache now stores the name of the vendor. A token that an earlier version cached still shows `OIDC [cached]` until it expires. ## [1.30.0] - 2026-10-22 diff --git a/cloudsmith_cli/core/credentials/oidc/cache.py b/cloudsmith_cli/core/credentials/oidc/cache.py index 2888e004e..e6d22b65f 100644 --- a/cloudsmith_cli/core/credentials/oidc/cache.py +++ b/cloudsmith_cli/core/credentials/oidc/cache.py @@ -12,6 +12,7 @@ import logging import os import time +from dataclasses import dataclass from ...cache_utils import atomic_write_json @@ -22,6 +23,17 @@ _CACHE_DIR_NAME = "oidc_token_cache" +@dataclass(frozen=True) +class CachedToken: + """A cached exchanged token, and the CI vendor whose OIDC token it came from. + + ``detector`` is ``None`` for an entry that an older CLI wrote. + """ + + token: str + detector: str | None = None + + def _get_cache_dir() -> str: """Return the cache directory path, creating it if needed.""" from ....cli.config import get_default_config_path @@ -59,7 +71,7 @@ def _decode_jwt_exp(token: str) -> float | None: return None -def get_cached_token(api_host: str, org: str, service_slug: str) -> str | None: +def get_cached_token(api_host: str, org: str, service_slug: str) -> CachedToken | None: """Return a cached token if it exists and is not expired.""" token = _get_from_keyring(api_host, org, service_slug) if token: @@ -67,7 +79,7 @@ def get_cached_token(api_host: str, org: str, service_slug: str) -> str | None: return _get_from_disk(api_host, org, service_slug) -def _get_from_keyring(api_host: str, org: str, service_slug: str) -> str | None: +def _get_from_keyring(api_host: str, org: str, service_slug: str) -> CachedToken | None: """Try to get token from keyring.""" try: from ...keyring import get_oidc_token @@ -100,14 +112,14 @@ def _get_from_keyring(api_host: str, org: str, service_slug: str) -> str | None: else: logger.debug("Using keyring OIDC token (no expiry information)") - return token + return CachedToken(token, data.get("detector")) except Exception: # pylint: disable=broad-exception-caught logger.debug("Failed to read OIDC token from keyring", exc_info=True) return None -def _get_from_disk(api_host: str, org: str, service_slug: str) -> str | None: +def _get_from_disk(api_host: str, org: str, service_slug: str) -> CachedToken | None: """Try to get token from disk cache.""" cache_dir = _get_cache_dir() cache_file = os.path.join(cache_dir, _cache_key(api_host, org, service_slug)) @@ -140,7 +152,7 @@ def _get_from_disk(api_host: str, org: str, service_slug: str) -> str | None: else: logger.debug("Using disk cached OIDC token (no expiry information)") - return token + return CachedToken(token, data.get("detector")) except (json.JSONDecodeError, OSError, KeyError): logger.debug("Failed to read OIDC token from disk cache", exc_info=True) @@ -148,8 +160,17 @@ def _get_from_disk(api_host: str, org: str, service_slug: str) -> str | None: return None -def store_cached_token(api_host: str, org: str, service_slug: str, token: str) -> None: - """Cache a token in keyring (if available) or filesystem.""" +def store_cached_token( + api_host: str, + org: str, + service_slug: str, + token: str, + detector: str | None = None, +) -> None: + """Cache a token in keyring (if available) or filesystem. + + *detector* is the name of the CI vendor whose OIDC token was exchanged. + """ expires_at = _decode_jwt_exp(token) data = { @@ -158,6 +179,7 @@ def store_cached_token(api_host: str, org: str, service_slug: str, token: str) - "api_host": api_host, "org": org, "service_slug": service_slug, + "detector": detector, "cached_at": time.time(), } diff --git a/cloudsmith_cli/core/credentials/providers/oidc_provider.py b/cloudsmith_cli/core/credentials/providers/oidc_provider.py index e8373169d..29d70fb67 100644 --- a/cloudsmith_cli/core/credentials/providers/oidc_provider.py +++ b/cloudsmith_cli/core/credentials/providers/oidc_provider.py @@ -107,10 +107,13 @@ def resolve( # pylint: disable=too-many-return-statements cached = get_cached_token(context.api_host, org, service_slug) if cached: logger.debug("OidcProvider: Using cached OIDC token") + vendor = f" via {cached.detector}" if cached.detector else "" return CredentialResult( - api_key=cached, + api_key=cached.token, source_name="oidc", - source_detail=f"OIDC [cached] (org: {org}, service: {service_slug})", + source_detail=( + f"OIDC{vendor} [cached] (org: {org}, service: {service_slug})" + ), auth_type="bearer", ) @@ -184,7 +187,13 @@ def resolve( # pylint: disable=too-many-return-statements if not cloudsmith_token: return None - store_cached_token(context.api_host, org, service_slug, cloudsmith_token) + store_cached_token( + context.api_host, + org, + service_slug, + cloudsmith_token, + detector=detector.name, + ) return CredentialResult( api_key=cloudsmith_token, diff --git a/cloudsmith_cli/core/tests/test_oidc_cache.py b/cloudsmith_cli/core/tests/test_oidc_cache.py new file mode 100644 index 000000000..306be9171 --- /dev/null +++ b/cloudsmith_cli/core/tests/test_oidc_cache.py @@ -0,0 +1,65 @@ +"""Tests for the OIDC token cache.""" + +import json +import time +from unittest.mock import patch + +import jwt +import pytest + +from cloudsmith_cli.core.credentials.oidc import cache + +API_HOST = "https://api.cloudsmith.io" + + +def _token() -> str: + return jwt.encode({"exp": time.time() + 3600}, key="", algorithm="none") + + +@pytest.fixture() +def disk_only(tmp_path): + """Use a cache directory in tmp_path, with no keyring.""" + with ( + patch( + "cloudsmith_cli.cli.config.get_default_config_path", + return_value=str(tmp_path), + ), + patch.object(cache, "_store_in_keyring", return_value=False), + patch.object(cache, "_get_from_keyring", return_value=None), + ): + yield tmp_path + + +def test_disk_cache_keeps_the_vendor(disk_only): + """A token cached on disk comes back with the name of the CI vendor.""" + token = _token() + cache.store_cached_token( + API_HOST, "acme", "ci-service", token, detector="GitHub Actions" + ) + + assert cache.get_cached_token(API_HOST, "acme", "ci-service") == ( + cache.CachedToken(token, detector="GitHub Actions") + ) + + +def test_disk_cache_entry_without_a_vendor(disk_only): + """An entry that an older CLI wrote has no vendor name.""" + token = _token() + cache.store_cached_token(API_HOST, "acme", "ci-service", token) + + assert cache.get_cached_token(API_HOST, "acme", "ci-service") == ( + cache.CachedToken(token) + ) + + +def test_keyring_cache_keeps_the_vendor(): + """A token cached in the keyring comes back with the name of the CI vendor.""" + token = _token() + entry = json.dumps( + {"token": token, "expires_at": time.time() + 3600, "detector": "CircleCI"} + ) + + with patch("cloudsmith_cli.core.keyring.get_oidc_token", return_value=entry): + assert cache.get_cached_token(API_HOST, "acme", "ci-service") == ( + cache.CachedToken(token, detector="CircleCI") + ) diff --git a/cloudsmith_cli/core/tests/test_oidc_provider.py b/cloudsmith_cli/core/tests/test_oidc_provider.py index 36fc4a933..1ee8d7e08 100644 --- a/cloudsmith_cli/core/tests/test_oidc_provider.py +++ b/cloudsmith_cli/core/tests/test_oidc_provider.py @@ -6,6 +6,7 @@ import jwt from cloudsmith_cli.core.credentials.models import CredentialContext +from cloudsmith_cli.core.credentials.oidc.cache import CachedToken from cloudsmith_cli.core.credentials.oidc.exchange import OidcExchangeError from cloudsmith_cli.core.credentials.providers.oidc_provider import OidcProvider @@ -21,7 +22,7 @@ def test_cached_oidc_token_is_a_bearer_credential(): """Cached exchanged tokens retain their HTTP authorization scheme.""" with patch( "cloudsmith_cli.core.credentials.oidc.cache.get_cached_token", - return_value="cached-token", + return_value=CachedToken("cached-token"), ): credential = OidcProvider().resolve(_context()) @@ -29,6 +30,67 @@ def test_cached_oidc_token_is_a_bearer_credential(): assert credential.auth_type == "bearer" +def test_cached_oidc_token_names_the_vendor(): + """The source of a cached token names the CI vendor whose token was exchanged.""" + with patch( + "cloudsmith_cli.core.credentials.oidc.cache.get_cached_token", + return_value=CachedToken("cached-token", detector="GitHub Actions"), + ): + credential = OidcProvider().resolve(_context()) + + assert credential.api_key == "cached-token" + assert credential.source_detail == ( + "OIDC via GitHub Actions [cached] (org: cloudsmith, service: github-actions)" + ) + + +def test_cached_oidc_token_without_a_vendor_keeps_the_old_source(): + """A token that an older CLI cached has no vendor name.""" + with patch( + "cloudsmith_cli.core.credentials.oidc.cache.get_cached_token", + return_value=CachedToken("cached-token"), + ): + credential = OidcProvider().resolve(_context()) + + assert credential.source_detail == ( + "OIDC [cached] (org: cloudsmith, service: github-actions)" + ) + + +def test_exchanged_oidc_token_is_cached_with_the_vendor(): + """The cache keeps the vendor name, so a cached token can name it later.""" + detector = Mock() + detector.name = "GitHub Actions" + detector.get_token.return_value = "vendor-token" + + with ( + patch( + "cloudsmith_cli.core.credentials.oidc.cache.get_cached_token", + return_value=None, + ), + patch( + "cloudsmith_cli.core.credentials.oidc.cache.store_cached_token", + ) as store, + patch( + "cloudsmith_cli.core.credentials.oidc.detectors.detect_environment", + return_value=detector, + ), + patch( + "cloudsmith_cli.core.credentials.oidc.exchange.exchange_oidc_token", + return_value="exchanged-token", + ), + ): + OidcProvider().resolve(_context()) + + store.assert_called_once_with( + "https://api.cloudsmith.io", + "cloudsmith", + "github-actions", + "exchanged-token", + detector="GitHub Actions", + ) + + def test_exchanged_oidc_token_is_a_bearer_credential(): """Freshly exchanged tokens use Bearer authentication.""" detector = Mock(name="github-actions")