diff --git a/src/sap_cloud_sdk/adms/_http.py b/src/sap_cloud_sdk/adms/_http.py index 76a076ab..e9410156 100644 --- a/src/sap_cloud_sdk/adms/_http.py +++ b/src/sap_cloud_sdk/adms/_http.py @@ -46,6 +46,24 @@ _RESPONSE_TEXT_TRUNCATION_LIMIT = 500 +def _require_non_blank_jwt(user_jwt: str | None) -> None: + """Enforce the OBO invariant: a user assertion must be a non-blank string. + + Raises :class:`ValueError` for ``None``, empty, or whitespace-only input. + Callers that want service credentials must omit *user_jwt* (or pass + ``None``) at the constructor/factory level, which guards with + ``if user_jwt is not None``. + + This is the single enforcement point for HASI2026203-276: blank/whitespace + JWTs must never silently fall through to application credentials. + """ + if user_jwt is None or not user_jwt.strip(): + raise ValueError( + "user_jwt must be a non-blank string for OBO mode; " + "omit it (or pass None) to use service credentials" + ) + + def quote_odata_string_key(value: str) -> str: """Quote and escape a string value for use in an OData V4 entity key segment. @@ -177,6 +195,8 @@ def __init__( self._token_fetcher = token_fetcher self._session = session or requests.Session() self._user_jwt = user_jwt + if user_jwt is not None: + _require_non_blank_jwt(user_jwt) self._csrf_tokens: dict[str, str] = {} # Guards the _csrf_tokens dict. ``AdmsHttp`` is documented as safe to # share across threads (matching ``requests.Session``); without this @@ -190,10 +210,16 @@ def with_user_jwt(self, user_jwt: str) -> "AdmsHttp": Args: user_jwt: The user's OIDC or XSUAA JWT from the inbound request. + Must be a non-blank string — ``None`` and blank/whitespace raise + :class:`ValueError` (HASI2026203-276). Returns: New :class:`AdmsHttp` for user-context calls. + + Raises: + ValueError: If *user_jwt* is ``None``, empty, or whitespace-only. """ + _require_non_blank_jwt(user_jwt) return AdmsHttp( config=self._config, token_fetcher=self._token_fetcher, @@ -291,7 +317,7 @@ def _send_with_csrf( # ------------------------------------------------------------------ def _bearer_token(self) -> str: - if self._user_jwt: + if self._user_jwt is not None: return self._token_fetcher.exchange_token(self._user_jwt) return self._token_fetcher.get_token() @@ -436,6 +462,8 @@ def __init__( self._config = config self._token_fetcher = token_fetcher self._user_jwt = user_jwt + if user_jwt is not None: + _require_non_blank_jwt(user_jwt) # Default to owning the underlying ``httpx.AsyncClient``. Borrowed # instances created via :meth:`with_user_jwt` flip this to ``False`` # so they share — and do *not* close — the parent's connection pool. @@ -443,7 +471,7 @@ def __init__( _jwt = user_jwt # capture for closure before super().__init__() get_token = ( (lambda: token_fetcher.exchange_token(_jwt)) - if _jwt + if _jwt is not None else token_fetcher.get_token ) super().__init__( @@ -646,10 +674,16 @@ def with_user_jwt(self, user_jwt: str) -> "AsyncAdmsHttp": Args: user_jwt: The user's OIDC or XSUAA JWT from the inbound request. + Must be a non-blank string — ``None`` and blank/whitespace raise + :class:`ValueError` (HASI2026203-276). Returns: New :class:`AsyncAdmsHttp` for user-context calls. + + Raises: + ValueError: If *user_jwt* is ``None``, empty, or whitespace-only. """ + _require_non_blank_jwt(user_jwt) borrowed = AsyncAdmsHttp( config=self._config, token_fetcher=self._token_fetcher, diff --git a/src/sap_cloud_sdk/adms/client.py b/src/sap_cloud_sdk/adms/client.py index 671ff396..3660f779 100644 --- a/src/sap_cloud_sdk/adms/client.py +++ b/src/sap_cloud_sdk/adms/client.py @@ -49,7 +49,7 @@ _ConfigurationApi, ) from sap_cloud_sdk.adms._document_api import _AsyncDocumentApi, _DocumentApi -from sap_cloud_sdk.adms._http import AdmsHttp, AsyncAdmsHttp +from sap_cloud_sdk.adms._http import AdmsHttp, AsyncAdmsHttp, _require_non_blank_jwt from sap_cloud_sdk.adms._ias_fetcher import IasTokenFetcher from sap_cloud_sdk.adms._job_api import _AsyncJobApi, _JobApi from sap_cloud_sdk.adms._relation_api import ( @@ -91,10 +91,16 @@ def with_user_jwt(self, user_jwt: str) -> "AdmsClient": Args: user_jwt: The user's OIDC or XSUAA JWT from the inbound request. + Must be a non-blank string — ``None`` and blank/whitespace raise + :class:`ValueError` (HASI2026203-276). Returns: New :class:`AdmsClient` configured for user-context calls. + + Raises: + ValueError: If *user_jwt* is ``None``, empty, or whitespace-only. """ + _require_non_blank_jwt(user_jwt) return AdmsClient(self._http.with_user_jwt(user_jwt)) @@ -129,10 +135,16 @@ def with_user_jwt(self, user_jwt: str) -> "AsyncAdmsClient": Args: user_jwt: The user's OIDC or XSUAA JWT. + Must be a non-blank string — ``None`` and blank/whitespace raise + :class:`ValueError` (HASI2026203-276). Returns: New :class:`AsyncAdmsClient` for user-context calls. + + Raises: + ValueError: If *user_jwt* is ``None``, empty, or whitespace-only. """ + _require_non_blank_jwt(user_jwt) return AsyncAdmsClient(self._http.with_user_jwt(user_jwt)) @@ -166,12 +178,14 @@ def create_client( Raises: ConfigError: If the binding configuration is missing or incomplete. - ValueError: If ``instance`` is an empty string. + ValueError: If ``instance`` is an empty string or ``user_jwt`` is blank/whitespace. """ if instance is not None and instance == "": raise ValueError( "instance must not be an empty string; omit it to use 'default'" ) + if user_jwt is not None: + _require_non_blank_jwt(user_jwt) try: if config is not None: token_fetcher = IasTokenFetcher(config=config, cache=token_cache) @@ -210,12 +224,14 @@ def create_async_client( Raises: ConfigError: If binding configuration is missing or incomplete. - ValueError: If ``instance`` is an empty string. + ValueError: If ``instance`` is an empty string or ``user_jwt`` is blank/whitespace. """ if instance is not None and instance == "": raise ValueError( "instance must not be an empty string; omit it to use 'default'" ) + if user_jwt is not None: + _require_non_blank_jwt(user_jwt) try: if config is not None: token_fetcher = IasTokenFetcher(config=config, cache=token_cache) diff --git a/src/sap_cloud_sdk/adms/user-guide.md b/src/sap_cloud_sdk/adms/user-guide.md index ac4dd076..57008473 100644 --- a/src/sap_cloud_sdk/adms/user-guide.md +++ b/src/sap_cloud_sdk/adms/user-guide.md @@ -78,6 +78,15 @@ client = create_client(config=config) client = create_client(user_jwt=request.headers["Authorization"].split()[1]) ``` +> **Note (HASI2026203-276):** `user_jwt` must be a non-blank string. Passing an +> empty string or whitespace-only value raises `ValueError` instead of silently +> falling back to service credentials. To use service credentials explicitly, +> omit `user_jwt` or pass `None` at `create_client` / `create_async_client`. +> +> `with_user_jwt(...)` always requires a non-blank JWT — passing `None` or +> blank raises `ValueError`, since the method signals explicit user-context +> intent. Use the service-credentials factory path instead. + ## Token Cache for Scale-Out ```python diff --git a/tests/adms/unit/test_client.py b/tests/adms/unit/test_client.py index fe5e23d2..c9dbb0d1 100644 --- a/tests/adms/unit/test_client.py +++ b/tests/adms/unit/test_client.py @@ -140,6 +140,25 @@ def test_with_user_jwt_uses_new_http(self, mock_http): assert user_client._http is mock_user_http assert client._http is mock_http + def test_with_user_jwt_empty_raises(self, mock_http): + # HASI2026203-276: facade guard fires before delegating to transport. + client = AdmsClient(mock_http) + with pytest.raises(ValueError, match="non-blank"): + client.with_user_jwt("") + mock_http.with_user_jwt.assert_not_called() + + def test_with_user_jwt_whitespace_raises(self, mock_http): + client = AdmsClient(mock_http) + with pytest.raises(ValueError, match="non-blank"): + client.with_user_jwt(" ") + mock_http.with_user_jwt.assert_not_called() + + def test_with_user_jwt_none_raises(self, mock_http): + client = AdmsClient(mock_http) + with pytest.raises(ValueError, match="non-blank"): + client.with_user_jwt(None) # type: ignore[arg-type] + mock_http.with_user_jwt.assert_not_called() + class TestCreateClientFactory: def test_raises_config_error_on_missing_binding(self): @@ -220,6 +239,35 @@ def test_user_jwt_forwarded_to_http(self): assert client._http._user_jwt == "user-jwt-123" + def test_create_client_empty_user_jwt_raises(self): + # HASI2026203-276: factory guard must fire before binding resolution. + mock_config = AdmsConfig( + service_url="https://adm.example.com", + ias_url="https://ias.example.com", + client_id="cid", + client_secret="cs", + ) + factory = MagicMock(return_value=mock_config) + with patch( + "sap_cloud_sdk.adms.client._make_config_factory", return_value=factory + ): + with pytest.raises(ValueError, match="non-blank"): + create_client(user_jwt="") + + def test_create_client_whitespace_user_jwt_raises(self): + mock_config = AdmsConfig( + service_url="https://adm.example.com", + ias_url="https://ias.example.com", + client_id="cid", + client_secret="cs", + ) + factory = MagicMock(return_value=mock_config) + with patch( + "sap_cloud_sdk.adms.client._make_config_factory", return_value=factory + ): + with pytest.raises(ValueError, match="non-blank"): + create_client(user_jwt=" ") + # ── AsyncAdmsHttp ───────────────────────────────────────────────────────────── @@ -482,6 +530,23 @@ def test_accepts_explicit_config(self, config): mock_make.assert_not_called() assert isinstance(client, AsyncAdmsClient) + def test_create_async_client_empty_user_jwt_raises(self, config): + # HASI2026203-276: async factory guard. + mock_factory = MagicMock(return_value=config) + with patch( + "sap_cloud_sdk.adms.client._make_config_factory", return_value=mock_factory + ): + with pytest.raises(ValueError, match="non-blank"): + create_async_client(user_jwt="") + + def test_create_async_client_whitespace_user_jwt_raises(self, config): + mock_factory = MagicMock(return_value=config) + with patch( + "sap_cloud_sdk.adms.client._make_config_factory", return_value=mock_factory + ): + with pytest.raises(ValueError, match="non-blank"): + create_async_client(user_jwt=" ") + # ── _AsyncDocumentApi ────────────────────────────────────────────────────────── diff --git a/tests/adms/unit/test_http.py b/tests/adms/unit/test_http.py index 751f74b6..ce11549b 100644 --- a/tests/adms/unit/test_http.py +++ b/tests/adms/unit/test_http.py @@ -3,12 +3,14 @@ from typing import Optional from unittest.mock import MagicMock +import httpx import pytest import requests from sap_cloud_sdk.adms._ias_fetcher import IasTokenFetcher from sap_cloud_sdk.adms._http import ( AdmsHttp, + AsyncAdmsHttp, build_allowed_domain_key_path, build_business_object_node_type_key_path, build_doctype_botype_map_key_path, @@ -221,6 +223,142 @@ def test_service_jwt_uses_get_token(self, config, token_fetcher): token_fetcher.exchange_token.assert_not_called() +class TestAdmsHttpOboInvariant: + """HASI2026203-276 — blank/whitespace/None user_jwt must raise ValueError. + + Guards are at: AdmsHttp constructor, AdmsHttp.with_user_jwt, + AsyncAdmsHttp constructor, and AsyncAdmsHttp.with_user_jwt. + None remains the valid service-credentials sentinel ONLY at the + constructor level; with_user_jwt rejects it since it signals explicit OBO + intent. + """ + + # ------------------------------------------------------------------ + # Sync transport — AdmsHttp + # ------------------------------------------------------------------ + + def test_constructor_empty_user_jwt_raises(self, config, token_fetcher): + with pytest.raises(ValueError, match="non-blank"): + AdmsHttp(config=config, token_fetcher=token_fetcher, user_jwt="") + + def test_constructor_whitespace_user_jwt_raises(self, config, token_fetcher): + with pytest.raises(ValueError, match="non-blank"): + AdmsHttp(config=config, token_fetcher=token_fetcher, user_jwt=" ") + + def test_constructor_none_user_jwt_does_not_raise(self, config, token_fetcher): + # Regression: None is the documented service-mode sentinel — must NOT raise. + http = AdmsHttp(config=config, token_fetcher=token_fetcher, user_jwt=None) + assert http is not None + + def test_with_user_jwt_empty_raises(self, config, token_fetcher): + http = AdmsHttp(config=config, token_fetcher=token_fetcher) + with pytest.raises(ValueError, match="non-blank"): + http.with_user_jwt("") + + def test_with_user_jwt_whitespace_raises(self, config, token_fetcher): + http = AdmsHttp(config=config, token_fetcher=token_fetcher) + with pytest.raises(ValueError, match="non-blank"): + http.with_user_jwt(" ") + + def test_with_user_jwt_none_raises(self, config, token_fetcher): + # with_user_jwt signals explicit OBO intent — None must also raise. + http = AdmsHttp(config=config, token_fetcher=token_fetcher) + with pytest.raises(ValueError, match="non-blank"): + http.with_user_jwt(None) # type: ignore[arg-type] + + def test_valid_jwt_calls_exchange_not_get(self, config, token_fetcher): + session = MagicMock(spec=requests.Session) + session.request.return_value = _make_resp(200) + http = AdmsHttp( + config=config, + token_fetcher=token_fetcher, + session=session, + user_jwt="valid.jwt.token", + ) + http.get("Document") + token_fetcher.exchange_token.assert_called_once_with("valid.jwt.token") + token_fetcher.get_token.assert_not_called() + + def test_none_jwt_uses_service_credentials(self, config, token_fetcher): + # Regression: None → service creds, not OBO. + session = MagicMock(spec=requests.Session) + session.request.return_value = _make_resp(200) + http = AdmsHttp( + config=config, token_fetcher=token_fetcher, session=session, user_jwt=None + ) + http.get("Document") + token_fetcher.get_token.assert_called() + token_fetcher.exchange_token.assert_not_called() + + # ------------------------------------------------------------------ + # Async transport — AsyncAdmsHttp + # ------------------------------------------------------------------ + + def test_async_constructor_empty_user_jwt_raises(self, config): + fetcher = MagicMock(spec=IasTokenFetcher) + with pytest.raises(ValueError, match="non-blank"): + AsyncAdmsHttp( + config=config, + token_fetcher=fetcher, + client=MagicMock(spec=httpx.AsyncClient), + user_jwt="", + ) + + def test_async_constructor_whitespace_user_jwt_raises(self, config): + fetcher = MagicMock(spec=IasTokenFetcher) + with pytest.raises(ValueError, match="non-blank"): + AsyncAdmsHttp( + config=config, + token_fetcher=fetcher, + client=MagicMock(spec=httpx.AsyncClient), + user_jwt=" ", + ) + + def test_async_constructor_none_does_not_raise(self, config): + fetcher = MagicMock(spec=IasTokenFetcher) + fetcher.get_token.return_value = "service-token" + http = AsyncAdmsHttp( + config=config, + token_fetcher=fetcher, + client=MagicMock(spec=httpx.AsyncClient), + user_jwt=None, + ) + assert http is not None + + def test_async_with_user_jwt_empty_raises(self, config): + fetcher = MagicMock(spec=IasTokenFetcher) + fetcher.get_token.return_value = "service-token" + http = AsyncAdmsHttp( + config=config, + token_fetcher=fetcher, + client=MagicMock(spec=httpx.AsyncClient), + ) + with pytest.raises(ValueError, match="non-blank"): + http.with_user_jwt("") + + def test_async_with_user_jwt_whitespace_raises(self, config): + fetcher = MagicMock(spec=IasTokenFetcher) + fetcher.get_token.return_value = "service-token" + http = AsyncAdmsHttp( + config=config, + token_fetcher=fetcher, + client=MagicMock(spec=httpx.AsyncClient), + ) + with pytest.raises(ValueError, match="non-blank"): + http.with_user_jwt(" ") + + def test_async_with_user_jwt_none_raises(self, config): + fetcher = MagicMock(spec=IasTokenFetcher) + fetcher.get_token.return_value = "service-token" + http = AsyncAdmsHttp( + config=config, + token_fetcher=fetcher, + client=MagicMock(spec=httpx.AsyncClient), + ) + with pytest.raises(ValueError, match="non-blank"): + http.with_user_jwt(None) # type: ignore[arg-type] + + class TestQuoteOdataStringKey: def test_simple_value(self): assert quote_odata_string_key("job-123") == "'job-123'"