Skip to content

Commit de26fa1

Browse files
committed
fix(adms): reject blank user_jwt in OBO mode
Blank or whitespace-only user_jwt values triggered a silent fallback from OBO (per-user) to service (application) credentials due to Python truthy checks (if self._user_jwt / if _jwt). Empty and whitespace-only strings are falsy, silently routing to get_token() (service credentials) instead of exchange_token() (OBO) — an authorization-escalation attack surface. Fix: add a single enforcement helper _require_non_blank_jwt and apply it at every public entry point — constructors (conditional on is not None), with_user_jwt methods (unconditional), and factory functions (conditional). Correct both truthy-check bug sites to use is not None comparisons. Behavior contract (before → after): - create_client()/user_jwt=None → service creds (unchanged) - create_client(user_jwt='valid') → OBO exchange (unchanged) - create_client(user_jwt='')/' ' → ValueError - AdmsHttp(user_jwt=None) → service creds (unchanged) - AdmsHttp(user_jwt='')/' ' → ValueError - with_user_jwt('valid') → OBO exchange (unchanged) - with_user_jwt('')/' '/None → ValueError BREAKING CHANGE: passing empty or whitespace user_jwt (which previously silently used service credentials) now raises ValueError. user_jwt=None / omitted (service mode) is unchanged. Cross-repo scan confirmed zero external consumers of sap_cloud_sdk.adms.
1 parent 78a25f4 commit de26fa1

5 files changed

Lines changed: 267 additions & 5 deletions

File tree

‎src/sap_cloud_sdk/adms/_http.py‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,24 @@
4646
_RESPONSE_TEXT_TRUNCATION_LIMIT = 500
4747

4848

49+
def _require_non_blank_jwt(user_jwt: str | None) -> None:
50+
"""Enforce the OBO invariant: a user assertion must be a non-blank string.
51+
52+
Raises :class:`ValueError` for ``None``, empty, or whitespace-only input.
53+
Callers that want service credentials must omit *user_jwt* (or pass
54+
``None``) at the constructor/factory level, which guards with
55+
``if user_jwt is not None``.
56+
57+
Blank/whitespace JWTs must never silently fall through to application
58+
credentials — callers must always supply a real assertion for OBO mode.
59+
"""
60+
if user_jwt is None or not user_jwt.strip():
61+
raise ValueError(
62+
"user_jwt must be a non-blank string for OBO mode; "
63+
"omit it (or pass None) to use service credentials"
64+
)
65+
66+
4967
def quote_odata_string_key(value: str) -> str:
5068
"""Quote and escape a string value for use in an OData V4 entity key segment.
5169
@@ -177,6 +195,8 @@ def __init__(
177195
self._token_fetcher = token_fetcher
178196
self._session = session or requests.Session()
179197
self._user_jwt = user_jwt
198+
if user_jwt is not None:
199+
_require_non_blank_jwt(user_jwt)
180200
self._csrf_tokens: dict[str, str] = {}
181201
# Guards the _csrf_tokens dict. ``AdmsHttp`` is documented as safe to
182202
# share across threads (matching ``requests.Session``); without this
@@ -190,10 +210,16 @@ def with_user_jwt(self, user_jwt: str) -> "AdmsHttp":
190210
191211
Args:
192212
user_jwt: The user's OIDC or XSUAA JWT from the inbound request.
213+
Must be a non-blank string — ``None`` and blank/whitespace raise
214+
:class:`ValueError` (HASI2026203-276).
193215
194216
Returns:
195217
New :class:`AdmsHttp` for user-context calls.
218+
219+
Raises:
220+
ValueError: If *user_jwt* is ``None``, empty, or whitespace-only.
196221
"""
222+
_require_non_blank_jwt(user_jwt)
197223
return AdmsHttp(
198224
config=self._config,
199225
token_fetcher=self._token_fetcher,
@@ -291,7 +317,7 @@ def _send_with_csrf(
291317
# ------------------------------------------------------------------
292318

293319
def _bearer_token(self) -> str:
294-
if self._user_jwt:
320+
if self._user_jwt is not None:
295321
return self._token_fetcher.exchange_token(self._user_jwt)
296322
return self._token_fetcher.get_token()
297323

@@ -436,14 +462,16 @@ def __init__(
436462
self._config = config
437463
self._token_fetcher = token_fetcher
438464
self._user_jwt = user_jwt
465+
if user_jwt is not None:
466+
_require_non_blank_jwt(user_jwt)
439467
# Default to owning the underlying ``httpx.AsyncClient``. Borrowed
440468
# instances created via :meth:`with_user_jwt` flip this to ``False``
441469
# so they share — and do *not* close — the parent's connection pool.
442470
self._owns_client = True
443471
_jwt = user_jwt # capture for closure before super().__init__()
444472
get_token = (
445473
(lambda: token_fetcher.exchange_token(_jwt))
446-
if _jwt
474+
if _jwt is not None
447475
else token_fetcher.get_token
448476
)
449477
super().__init__(
@@ -646,10 +674,16 @@ def with_user_jwt(self, user_jwt: str) -> "AsyncAdmsHttp":
646674
647675
Args:
648676
user_jwt: The user's OIDC or XSUAA JWT from the inbound request.
677+
Must be a non-blank string — ``None`` and blank/whitespace raise
678+
:class:`ValueError` (HASI2026203-276).
649679
650680
Returns:
651681
New :class:`AsyncAdmsHttp` for user-context calls.
682+
683+
Raises:
684+
ValueError: If *user_jwt* is ``None``, empty, or whitespace-only.
652685
"""
686+
_require_non_blank_jwt(user_jwt)
653687
borrowed = AsyncAdmsHttp(
654688
config=self._config,
655689
token_fetcher=self._token_fetcher,

‎src/sap_cloud_sdk/adms/client.py‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
_ConfigurationApi,
5050
)
5151
from sap_cloud_sdk.adms._document_api import _AsyncDocumentApi, _DocumentApi
52-
from sap_cloud_sdk.adms._http import AdmsHttp, AsyncAdmsHttp
52+
from sap_cloud_sdk.adms._http import AdmsHttp, AsyncAdmsHttp, _require_non_blank_jwt
5353
from sap_cloud_sdk.adms._ias_fetcher import IasTokenFetcher
5454
from sap_cloud_sdk.adms._job_api import _AsyncJobApi, _JobApi
5555
from sap_cloud_sdk.adms._relation_api import (
@@ -91,10 +91,16 @@ def with_user_jwt(self, user_jwt: str) -> "AdmsClient":
9191
9292
Args:
9393
user_jwt: The user's OIDC or XSUAA JWT from the inbound request.
94+
Must be a non-blank string — ``None`` and blank/whitespace raise
95+
:class:`ValueError` (HASI2026203-276).
9496
9597
Returns:
9698
New :class:`AdmsClient` configured for user-context calls.
99+
100+
Raises:
101+
ValueError: If *user_jwt* is ``None``, empty, or whitespace-only.
97102
"""
103+
_require_non_blank_jwt(user_jwt)
98104
return AdmsClient(self._http.with_user_jwt(user_jwt))
99105

100106

@@ -129,10 +135,16 @@ def with_user_jwt(self, user_jwt: str) -> "AsyncAdmsClient":
129135
130136
Args:
131137
user_jwt: The user's OIDC or XSUAA JWT.
138+
Must be a non-blank string — ``None`` and blank/whitespace raise
139+
:class:`ValueError` (HASI2026203-276).
132140
133141
Returns:
134142
New :class:`AsyncAdmsClient` for user-context calls.
143+
144+
Raises:
145+
ValueError: If *user_jwt* is ``None``, empty, or whitespace-only.
135146
"""
147+
_require_non_blank_jwt(user_jwt)
136148
return AsyncAdmsClient(self._http.with_user_jwt(user_jwt))
137149

138150

@@ -166,12 +178,14 @@ def create_client(
166178
167179
Raises:
168180
ConfigError: If the binding configuration is missing or incomplete.
169-
ValueError: If ``instance`` is an empty string.
181+
ValueError: If ``instance`` is an empty string or ``user_jwt`` is blank/whitespace.
170182
"""
171183
if instance is not None and instance == "":
172184
raise ValueError(
173185
"instance must not be an empty string; omit it to use 'default'"
174186
)
187+
if user_jwt is not None:
188+
_require_non_blank_jwt(user_jwt)
175189
try:
176190
if config is not None:
177191
token_fetcher = IasTokenFetcher(config=config, cache=token_cache)
@@ -210,12 +224,14 @@ def create_async_client(
210224
211225
Raises:
212226
ConfigError: If binding configuration is missing or incomplete.
213-
ValueError: If ``instance`` is an empty string.
227+
ValueError: If ``instance`` is an empty string or ``user_jwt`` is blank/whitespace.
214228
"""
215229
if instance is not None and instance == "":
216230
raise ValueError(
217231
"instance must not be an empty string; omit it to use 'default'"
218232
)
233+
if user_jwt is not None:
234+
_require_non_blank_jwt(user_jwt)
219235
try:
220236
if config is not None:
221237
token_fetcher = IasTokenFetcher(config=config, cache=token_cache)

‎src/sap_cloud_sdk/adms/user-guide.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,15 @@ client = create_client(config=config)
7878
client = create_client(user_jwt=request.headers["Authorization"].split()[1])
7979
```
8080

81+
> **Note:** `user_jwt` must be a non-blank string. Passing an
82+
> empty string or whitespace-only value raises `ValueError` instead of silently
83+
> falling back to service credentials. To use service credentials explicitly,
84+
> omit `user_jwt` or pass `None` at `create_client` / `create_async_client`.
85+
>
86+
> `with_user_jwt(...)` always requires a non-blank JWT — passing `None` or
87+
> blank raises `ValueError`, since the method signals explicit user-context
88+
> intent. Use the service-credentials factory path instead.
89+
8190
## Token Cache for Scale-Out
8291

8392
```python

‎tests/adms/unit/test_client.py‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,25 @@ def test_with_user_jwt_uses_new_http(self, mock_http):
140140
assert user_client._http is mock_user_http
141141
assert client._http is mock_http
142142

143+
def test_with_user_jwt_empty_raises(self, mock_http):
144+
# HASI2026203-276: facade guard fires before delegating to transport.
145+
client = AdmsClient(mock_http)
146+
with pytest.raises(ValueError, match="non-blank"):
147+
client.with_user_jwt("")
148+
mock_http.with_user_jwt.assert_not_called()
149+
150+
def test_with_user_jwt_whitespace_raises(self, mock_http):
151+
client = AdmsClient(mock_http)
152+
with pytest.raises(ValueError, match="non-blank"):
153+
client.with_user_jwt(" ")
154+
mock_http.with_user_jwt.assert_not_called()
155+
156+
def test_with_user_jwt_none_raises(self, mock_http):
157+
client = AdmsClient(mock_http)
158+
with pytest.raises(ValueError, match="non-blank"):
159+
client.with_user_jwt(None) # type: ignore[arg-type]
160+
mock_http.with_user_jwt.assert_not_called()
161+
143162

144163
class TestCreateClientFactory:
145164
def test_raises_config_error_on_missing_binding(self):
@@ -220,6 +239,35 @@ def test_user_jwt_forwarded_to_http(self):
220239

221240
assert client._http._user_jwt == "user-jwt-123"
222241

242+
def test_create_client_empty_user_jwt_raises(self):
243+
# HASI2026203-276: factory guard must fire before binding resolution.
244+
mock_config = AdmsConfig(
245+
service_url="https://adm.example.com",
246+
ias_url="https://ias.example.com",
247+
client_id="cid",
248+
client_secret="cs",
249+
)
250+
factory = MagicMock(return_value=mock_config)
251+
with patch(
252+
"sap_cloud_sdk.adms.client._make_config_factory", return_value=factory
253+
):
254+
with pytest.raises(ValueError, match="non-blank"):
255+
create_client(user_jwt="")
256+
257+
def test_create_client_whitespace_user_jwt_raises(self):
258+
mock_config = AdmsConfig(
259+
service_url="https://adm.example.com",
260+
ias_url="https://ias.example.com",
261+
client_id="cid",
262+
client_secret="cs",
263+
)
264+
factory = MagicMock(return_value=mock_config)
265+
with patch(
266+
"sap_cloud_sdk.adms.client._make_config_factory", return_value=factory
267+
):
268+
with pytest.raises(ValueError, match="non-blank"):
269+
create_client(user_jwt=" ")
270+
223271

224272
# ── AsyncAdmsHttp ─────────────────────────────────────────────────────────────
225273

@@ -482,6 +530,23 @@ def test_accepts_explicit_config(self, config):
482530
mock_make.assert_not_called()
483531
assert isinstance(client, AsyncAdmsClient)
484532

533+
def test_create_async_client_empty_user_jwt_raises(self, config):
534+
# HASI2026203-276: async factory guard.
535+
mock_factory = MagicMock(return_value=config)
536+
with patch(
537+
"sap_cloud_sdk.adms.client._make_config_factory", return_value=mock_factory
538+
):
539+
with pytest.raises(ValueError, match="non-blank"):
540+
create_async_client(user_jwt="")
541+
542+
def test_create_async_client_whitespace_user_jwt_raises(self, config):
543+
mock_factory = MagicMock(return_value=config)
544+
with patch(
545+
"sap_cloud_sdk.adms.client._make_config_factory", return_value=mock_factory
546+
):
547+
with pytest.raises(ValueError, match="non-blank"):
548+
create_async_client(user_jwt=" ")
549+
485550

486551
# ── _AsyncDocumentApi ──────────────────────────────────────────────────────────
487552

0 commit comments

Comments
 (0)