Skip to content
Closed
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
38 changes: 36 additions & 2 deletions src/sap_cloud_sdk/adms/_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -436,14 +462,16 @@ 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.
self._owns_client = True
_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__(
Expand Down Expand Up @@ -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,
Expand Down
22 changes: 19 additions & 3 deletions src/sap_cloud_sdk/adms/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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))


Expand Down Expand Up @@ -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))


Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions src/sap_cloud_sdk/adms/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
65 changes: 65 additions & 0 deletions tests/adms/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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 ─────────────────────────────────────────────────────────────

Expand Down Expand Up @@ -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 ──────────────────────────────────────────────────────────

Expand Down
Loading
Loading