From 96773767c998b5c820e8ddd1b584a23923ff6a8f Mon Sep 17 00:00:00 2001 From: Dan Wolfson Date: Thu, 29 Oct 2026 16:21:27 -0500 Subject: [PATCH] fix(auth): don't re-authenticate a caller-supplied token as the client's user (ISSUE-167) On a bare 401, BaseServerClient._async_make_request minted a new token as self.user_id and retried, whatever token was in the headers. A token set with set_bearer_token() for a different user therefore quietly became a token for the client's own user: the portal ran Lineage Explorer queries for a user logged in as Peter as erinoverview. exec_report_spec(token=...) had the same exposure. The client now records who it minted its current token for. The retry runs only when there is no token or the token was minted for self.user_id; an expired caller-supplied token raises PyegeriaUnauthorizedException. EgeriaTech, Egeria and the internal helper clients pass the minting user on to sub-clients, so their own tokens still renew. Signed-off-by: Dan Wolfson Claude-Session: https://claude.ai/code/session_015pdMuQanRknaXmcpx4LmLb Signed-off-by: Dan Wolfson --- PYEGERIA_ISSUES.md | 10 ++ pyegeria/core/_base_server_client.py | 29 +++- pyegeria/egeria_client.py | 11 +- pyegeria/egeria_tech_client.py | 13 +- pyegeria/omvs/automated_curation.py | 1 + pyegeria/omvs/classification_explorer.py | 1 + pyegeria/omvs/data_discovery.py | 1 + pyegeria/view/format_set_executor.py | 4 +- .../test_external_token_no_auth_retry.py | 127 ++++++++++++++++++ 9 files changed, 191 insertions(+), 6 deletions(-) create mode 100644 tests/micro-tests/test_external_token_no_auth_retry.py diff --git a/PYEGERIA_ISSUES.md b/PYEGERIA_ISSUES.md index edf2b128..dc552fbb 100644 --- a/PYEGERIA_ISSUES.md +++ b/PYEGERIA_ISSUES.md @@ -1894,6 +1894,16 @@ Sorted newest-first by `ISSUE-#` (2026-10-12). ISSUE-91 has two entries --- +### ISSUE-167: The 401 auto-retry re-authenticated a caller-supplied bearer token as the client's own user + +**Layer:** Pyegeria · **Status:** fixed on branch `fix/external-token-no-auth-retry`, for 6.2.15 · **Found:** 2026-10-28, by the egeria-workspaces-fs portal session (Lineage Explorer queries made by a user logged in as Peter ran as erinoverview). + +**What happened:** on a bare 401, `BaseServerClient._async_make_request` minted a new token with `self.user_id` / `self.user_pwd` and retried. It did that whatever token was in the headers, including one the caller had set with `set_bearer_token()` for a different user. So an expired or invalid token quietly became a token for the client's user, and the request succeeded as that user with no error. `user_pwd` falls back to `settings.User_Profile.user_pwd`, so a client built without a password was exposed too. Repro on 6.2.14: `MyProfile(view, url, "erinoverview", "secret")`, then `set_bearer_token()`, then `get_my_profile()` returned Erin's profile. `exec_report_spec(token=..., user=..., user_pass=...)` had the same exposure. + +**Fix:** the client records the user it minted its current token for (`_token_minted_for`; `None` for a token from the constructor or `set_bearer_token`). The retry runs only when there is no token or the token was minted for `self.user_id`. A caller-supplied token that expires now raises `PyegeriaUnauthorizedException`. `EgeriaTech` and `Egeria` pass the minting user to their sub-clients, including ones created later, so their own tokens still renew. Tests: `tests/micro-tests/test_external_token_no_auth_retry.py`. + +--- + ### ISSUE-166: Security annotation properties are Java primitives, so an unset CVSS score, exploit/patch flag or posture count reads back as 0 / false **Layer:** Egeria Server (type model / properties classes) · **Status:** fixed (Egeria 6.3, odpi/egeria#9389); pyegeria adapted in #498, released in 6.2.13 · **Found:** 2026-10-26, while adding the 6.3 security annotation types to pyegeria (odpi/egeria-workspaces#702 hand-off, step 3; PR #495). diff --git a/pyegeria/core/_base_server_client.py b/pyegeria/core/_base_server_client.py index b211f6e3..597d8980 100644 --- a/pyegeria/core/_base_server_client.py +++ b/pyegeria/core/_base_server_client.py @@ -112,6 +112,11 @@ def __init__( self.page_size = page_size or max_paging_size self.token_src = token_src self.token = token + # The user a bearer token was minted for by this client, or None for a token + # supplied by the caller (constructor or set_bearer_token), whose owner we can't know. + # The 401 auto-retry only re-mints for a token this client minted for its own user; + # re-minting over someone else's token would silently switch the request's identity. + self._token_minted_for = None self.local_qualifier = local_qualifier or settings.User_Profile.egeria_local_qualifier self.organization_name = organization_name or settings.Environment.organization_name @@ -281,6 +286,7 @@ async def _async_create_egeria_bearer_token( self.token_src = "Egeria" self.headers["Authorization"] = f"Bearer {token}" self.text_headers["Authorization"] = f"Bearer {token}" + self._token_minted_for = user_id return token def create_egeria_bearer_token( @@ -389,11 +395,25 @@ def set_bearer_token(self, token: str, source: str="Egeria") -> None: ------ PyegeriaInvalidParameterException If the token is invalid. + + Notes + ----- + A token set here is never silently replaced: when it expires, requests raise + PyegeriaUnauthorizedException rather than re-authenticating as this client's + user_id, which may not be the user the token belongs to. """ validate_name(token) self.token_src = source self.headers["Authorization"] = f"Bearer {token}" self.text_headers["Authorization"] = f"Bearer {token}" + self._token_minted_for = None + + def _owns_token(self) -> bool: + """True if there is no bearer token, or this client minted it for its own user_id.""" + return ( + "Authorization" not in self.headers + or self._token_minted_for == self.user_id + ) def get_token(self) -> str: """Retrieve and return the current bearer token. @@ -617,8 +637,13 @@ async def _async_make_request( # feasible fix, since the response carries no signal to distinguish # "expired" from "wrong credentials"/"insufficient permission". # _retry_on_auth=False on the recursive call guarantees at most one - # retry ever happens. - if status_code_from_error == 401 and _retry_on_auth and self.user_pwd: + # retry ever happens. Only when the current token (if any) was minted + # here for self.user_id - a caller-supplied token may belong to another + # user, and re-minting would quietly run the request as self.user_id. + if ( + status_code_from_error == 401 and _retry_on_auth and self.user_pwd + and self._owns_token() + ): try: new_token = await self._async_create_egeria_bearer_token() except Exception: diff --git a/pyegeria/egeria_client.py b/pyegeria/egeria_client.py index 526dee34..1ab38743 100644 --- a/pyegeria/egeria_client.py +++ b/pyegeria/egeria_client.py @@ -74,6 +74,8 @@ def __init__( self.user_id = user_id or settings.User_Profile.user_name self.user_pwd = user_pwd or settings.User_Profile.user_pwd self.token = token + # See BaseServerClient._token_minted_for: None means caller-supplied. + self._token_minted_for = None self.timeout = timeout self._subclient_map = { @@ -150,6 +152,7 @@ def _get_subclient(self, attr_name: str): self.token, timeout=self.timeout, ) + self._instantiated_clients[attr_name]._token_minted_for = self._token_minted_for return self._instantiated_clients[attr_name] def __getattr__(self, name): @@ -170,13 +173,19 @@ def create_egeria_bearer_token(self, user_id: str = None, user_pwd: str = None): # Use server_ops as a reliable helper for token generation helper = self._get_subclient("server_ops") token_val = helper.create_egeria_bearer_token(user_id, user_pwd) - self.set_bearer_token(token_val) + self._share_token(token_val, user_id or helper.user_id) return token_val def set_bearer_token(self, token: str) -> None: + self._share_token(token, None) + + def _share_token(self, token: str, minted_for: str | None) -> None: + """Install a token on this client and every active sub-client, keeping who it was minted for.""" self.token = token + self._token_minted_for = minted_for for sub in self._instantiated_clients.values(): sub.set_bearer_token(token) + sub._token_minted_for = minted_for def get_token(self) -> str: if self.token: return self.token diff --git a/pyegeria/egeria_tech_client.py b/pyegeria/egeria_tech_client.py index 439a10db..da6bce06 100644 --- a/pyegeria/egeria_tech_client.py +++ b/pyegeria/egeria_tech_client.py @@ -84,6 +84,8 @@ def __init__( self.user_id = user_id or settings.User_Profile.user_name self.user_pwd = user_pwd or settings.User_Profile.user_pwd self.token = token + # See BaseServerClient._token_minted_for: None means caller-supplied. + self._token_minted_for = None self.timeout = timeout # Mapping of attribute names to their classes for lazy loading @@ -166,6 +168,7 @@ def _get_subclient(self, attr_name: str): self.token, timeout=self.timeout, ) + self._instantiated_clients[attr_name]._token_minted_for = self._token_minted_for return self._instantiated_clients[attr_name] def __getattr__(self, name): @@ -196,7 +199,7 @@ async def _async_create_egeria_bearer_token( # Use a reliable sub-client to generate the token (e.g., collections) helper = self._get_subclient("collections") token_val = await helper._async_create_egeria_bearer_token(user_id, password, new_password) - self.set_bearer_token(token_val) + self._share_token(token_val, user_id or helper.user_id) return token_val def create_egeria_bearer_token(self, user_id: str = None, password: str = None, new_password: str = None) -> str: @@ -204,14 +207,20 @@ def create_egeria_bearer_token(self, user_id: str = None, password: str = None, # Use a reliable sub-client to generate the token (e.g., collections) helper = self._get_subclient("collections") token_val = helper.create_egeria_bearer_token(user_id, password, new_password) - self.set_bearer_token(token_val) + self._share_token(token_val, user_id or helper.user_id) return token_val def set_bearer_token(self, token: str) -> None: """Update token for the tech client and all active sub-clients.""" + self._share_token(token, None) + + def _share_token(self, token: str, minted_for: str | None) -> None: + """Install a token on this client and every active sub-client, keeping who it was minted for.""" self.token = token + self._token_minted_for = minted_for for sub in self._instantiated_clients.values(): sub.set_bearer_token(token) + sub._token_minted_for = minted_for def get_token(self) -> str: """Retrieve the current token from state or an active sub-client.""" diff --git a/pyegeria/omvs/automated_curation.py b/pyegeria/omvs/automated_curation.py index 63a89d09..cd3ad044 100644 --- a/pyegeria/omvs/automated_curation.py +++ b/pyegeria/omvs/automated_curation.py @@ -2540,6 +2540,7 @@ def _governance_engine_client(self): client = GovernanceEngine(self.view_server, self.platform_url, self.user_id, self.user_pwd, token=self.token, timeout=getattr(self, "timeout", None), legacy_fallback=True) + client._token_minted_for = self._token_minted_for self._gov_engine_client = client return client diff --git a/pyegeria/omvs/classification_explorer.py b/pyegeria/omvs/classification_explorer.py index 641251dd..3a613cbc 100644 --- a/pyegeria/omvs/classification_explorer.py +++ b/pyegeria/omvs/classification_explorer.py @@ -12301,6 +12301,7 @@ def _collection_client(self): if client is None: client = CollectionManager(self.view_server, self.platform_url, self.user_id, self.user_pwd, token=self.token, timeout=getattr(self, "timeout", None)) + client._token_minted_for = self._token_minted_for self._collection_mgr = client return client diff --git a/pyegeria/omvs/data_discovery.py b/pyegeria/omvs/data_discovery.py index 568fbd9c..f52ea3f7 100644 --- a/pyegeria/omvs/data_discovery.py +++ b/pyegeria/omvs/data_discovery.py @@ -1918,6 +1918,7 @@ def _asset_maker_client(self): if client is None: client = AssetMaker(self.view_server, self.platform_url, self.user_id, self.user_pwd, token=self.token, timeout=getattr(self, "timeout", None)) + client._token_minted_for = self._token_minted_for self._asset_maker = client return client diff --git a/pyegeria/view/format_set_executor.py b/pyegeria/view/format_set_executor.py index 6b0fd6d6..76e7e87a 100644 --- a/pyegeria/view/format_set_executor.py +++ b/pyegeria/view/format_set_executor.py @@ -311,13 +311,15 @@ def _resolve_action_target_client(egeria_client: EgeriaTech, client_class: type) return egeria_client._get_subclient(attr_name) # Fallback for non-standard clients outside EgeriaTech's map. - return client_class( + client = client_class( egeria_client.view_server, egeria_client.platform_url, user_id=egeria_client.user_id, user_pwd=egeria_client.user_pwd, token=egeria_client.token, ) + client._token_minted_for = getattr(egeria_client, "_token_minted_for", None) + return client def _normalize_report_params(params: Dict[str, Any], action_mode: str = "generic") -> Dict[str, Any]: diff --git a/tests/micro-tests/test_external_token_no_auth_retry.py b/tests/micro-tests/test_external_token_no_auth_retry.py new file mode 100644 index 00000000..3f43f647 --- /dev/null +++ b/tests/micro-tests/test_external_token_no_auth_retry.py @@ -0,0 +1,127 @@ +""" +The 401 auto-retry in BaseServerClient._async_make_request re-mints a token as the client's own +user_id. It must only do that for a token the client minted for that user (or when it has no +token). A token supplied by the caller via set_bearer_token may belong to another user: re-minting +over it silently ran the request as the client's user (a portal session logged in as one user ran +Lineage Explorer queries as erinoverview). +""" +import json + +import httpx +import pytest + +import pyegeria.core._base_platform_client as platform_module +import pyegeria.core._base_server_client as server_module +from pyegeria.core._base_platform_client import BasePlatformClient +from pyegeria.core._base_server_client import BaseServerClient +from pyegeria.core._exceptions import PyegeriaUnauthorizedException +from pyegeria.egeria_tech_client import EgeriaTech + +PLATFORM = "https://localhost:9443" +VIEW_SERVER = "qs-view-server" +ENDPOINT = f"{PLATFORM}/servers/{VIEW_SERVER}/api/open-metadata/test" + + +class FakeEgeria: + """Mints numbered tokens per user; only the latest token per user is accepted.""" + + def __init__(self): + self.minted = [] # user ids, in mint order + self.valid = set() + self.served_as = [] # user id behind each accepted data request + + def handler(self, request: httpx.Request) -> httpx.Response: + if request.url.path.endswith("/api/token"): + user = json.loads(request.content)["userId"] + self.minted.append(user) + token = f"{user}-{len(self.minted)}" + self.valid = {t for t in self.valid if not t.startswith(f"{user}-")} | {token} + return httpx.Response(200, text=token) + token = request.headers.get("Authorization", "").removeprefix("Bearer ") + if token not in self.valid: + return httpx.Response(401, text="Unauthorized") + self.served_as.append(token.rsplit("-", 1)[0]) + return httpx.Response(200, json={"class": "VoidResponse", "relatedHTTPCode": 200}) + + +@pytest.fixture +def egeria(monkeypatch): + fake = FakeEgeria() + # The constructors check the connection; that is not under test here. + monkeypatch.setattr(BaseServerClient, "check_connection", lambda self: "test") + monkeypatch.setattr(BasePlatformClient, "check_connection", lambda self: "test") + real_client = httpx.AsyncClient + + def client(*args, **kwargs): + kwargs.pop("verify", None) + return real_client(transport=httpx.MockTransport(fake.handler)) + + monkeypatch.setattr(platform_module, "AsyncClient", client) + monkeypatch.setattr(server_module, "AsyncClient", client) + return fake + + +def _client(): + return BaseServerClient(VIEW_SERVER, PLATFORM, user_id="erinoverview", user_pwd="secret") + + +async def test_external_token_is_not_replaced(egeria): + client = _client() + client.set_bearer_token("peterprofile-expired") + with pytest.raises(PyegeriaUnauthorizedException): + await client._async_make_request("POST", ENDPOINT, {}) + assert egeria.minted == [] + assert egeria.served_as == [] + + +async def test_own_expired_token_is_renewed(egeria): + client = _client() + await client._async_create_egeria_bearer_token() + egeria.valid.clear() # the token expires + await client._async_make_request("POST", ENDPOINT, {}) + assert egeria.minted == ["erinoverview", "erinoverview"] + assert egeria.served_as == ["erinoverview"] + + +async def test_no_token_mints_one(egeria): + client = _client() + await client._async_make_request("POST", ENDPOINT, {}) + assert egeria.minted == ["erinoverview"] + assert egeria.served_as == ["erinoverview"] + + +async def test_token_minted_for_another_user_is_not_replaced(egeria): + client = _client() + await client._async_create_egeria_bearer_token("peterprofile", "peters-password") + egeria.valid.clear() + with pytest.raises(PyegeriaUnauthorizedException): + await client._async_make_request("POST", ENDPOINT, {}) + assert egeria.minted == ["peterprofile"] + + +async def test_set_bearer_token_after_minting_disables_retry(egeria): + client = _client() + await client._async_create_egeria_bearer_token() + client.set_bearer_token("peterprofile-expired") + with pytest.raises(PyegeriaUnauthorizedException): + await client._async_make_request("POST", ENDPOINT, {}) + assert egeria.minted == ["erinoverview"] + + +async def test_egeria_tech_minted_token_renews_in_sub_clients(egeria): + tech = EgeriaTech(VIEW_SERVER, PLATFORM, "erinoverview", "secret") + await tech._async_create_egeria_bearer_token() + egeria.valid.clear() + # A sub-client created after minting inherits the token and its owner. + await tech._get_subclient("glossary_manager")._async_make_request("POST", ENDPOINT, {}) + assert egeria.served_as == ["erinoverview"] + + +async def test_egeria_tech_external_token_is_not_replaced(egeria): + tech = EgeriaTech(VIEW_SERVER, PLATFORM, "erinoverview", "secret") + tech._get_subclient("collections") + tech.set_bearer_token("peterprofile-expired") + for name in ("collections", "glossary_manager"): # existing and newly created sub-clients + with pytest.raises(PyegeriaUnauthorizedException): + await tech._get_subclient(name)._async_make_request("POST", ENDPOINT, {}) + assert egeria.minted == []