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 == []