diff --git a/PYEGERIA_ISSUES.md b/PYEGERIA_ISSUES.md index c0d341c8..3ea041a8 100644 --- a/PYEGERIA_ISSUES.md +++ b/PYEGERIA_ISSUES.md @@ -143,6 +143,62 @@ enough to track there too). --- +### ISSUE-89: No configurable bearer-token lifetime in `application.properties` — platform tokens are fixed at 3600 s, and the signing key is random per restart + +**Layer:** Egeria Server · **Status:** open · **Found:** 2026-09-04 (trellis-auth token contract) + +With `authentication.mode=token`, every token minted by the platform decodes to `exp - iat = 3600` +(observed on `omag-server-platform-6.2-SNAPSHOT`, quickstart, `authentication.source=platform`). +`application.properties` exposes `authentication.mode`, `authentication.source` and `rsa.key-id`, and +nothing that controls the lifetime; with `rsa.key-id` empty the signing key is regenerated at start, so +every token also dies on a platform restart. Client applications that hold a platform token on the +user's behalf (Portal, Egeria Advisor, Resource Explorer — which now carry the Egeria bearer token in +their session and never the password) cannot offer a session longer than an hour without either +re-prompting the user or caching a password, which is what a bearer token exists to avoid. + +**Ask:** a property such as `authentication.token.lifetime=3600` (seconds, default unchanged), read at +start and applied when a token is issued; optionally a refresh operation that returns a new token for a +valid unexpired one. Full draft issue text: trellis session scratch `egeria-issue-token-lifetime.md`. + +### ISSUE-90: `qs-engine-host` retries `startMissedEngineActions` forever when one incomplete engine action's anchor is unreadable by the engine-host user + +**Layer:** Egeria Server (possibly quickstart content) · **Status:** open · **Found:** 2026-09-04 (trevor fresh quickstart) + +On a fresh quickstart repository (6.2-SNAPSHOT builds of 2026-08-31 and 2026-09-04, all standard +content packs, stock `coco-user-directory.omsecrets`) the engine host logs, every few seconds from first +start and without end: + +``` +qs-engine-host Error ENGINE-HOST-SERVICES-2002 startMissedEngineActions caught an exception +UserNotAuthorizedException while restarting incomplete engine actions; the error message was +OMAG-SERVER-SECURITY-403-007 User generalnpa is not authorized to issue operation Read on +a0baa4da-5f85-446c-b655-88ae81455966 anchor element DigitalProductFamily +``` + +Always the same element GUID; 196 refusals in ten minutes, >2,000 in ninety. A second quickstart on +another machine, with the identical platform build, identical server configuration documents (only the +generated `localServerId` differs), identical `coco-user-directory.omsecrets`, `DEMO_MODE=false` on both, +and its repository also reloaded the same day, shows zero refusals in 24 h. The one thing that differs is +history: the affected platform was stopped and restarted several times during its first hour (a +configuration copy, a Kafka log-directory reset, a rebuild) while content packs and the nanny daemon's +integration groups were still initialising; the healthy one was loaded once and left alone. The refused +element cannot be read through the view server by `garygeeke`, `peterprofile` or `erinoverview` either +(401), so it is not merely a `generalnpa` permission gap — the anchor looks unreadable by everyone, as +if left behind by an interrupted initialisation. + +**What it means:** an `EngineAction` exists whose anchor is a `DigitalProductFamily`; the engine host's own +identity (`generalnpa`) may not read that anchor under the security connector; the "restart incomplete +engine actions" pass treats the authorization failure as transient, aborts on it, and retries at its +normal cadence, so one permanently unreadable action becomes a permanent loop. Cost: platform container +at 200-330 % CPU and its Postgres at 500-750 % on an idle box, from `EngineAction` entity searches and +classification reads; everything else on the platform is slower. + +**Ask (either helps):** (1) engine-host-services: treat `UserNotAuthorizedException` for a specific engine +action as terminal for that action — log once, skip it, continue with the rest, do not retry it until the +security context changes; (2) quickstart content: give `generalnpa` read access to the digital-product +elements its engine actions anchor to, or anchor those actions to elements the engine-host identity can +read. Full draft: trellis session scratch `egeria-issue-engine-host-403-loop.md`. + ### ISSUE-38 (PY-18): `count_relationships_between_elements("Exception")` (276) disagrees with `ClassificationExplorer.get_relationships("Exception")` (55) **Update 2026-08-30, from the Egeria team (Mandy Chessell).** Leaving this @@ -695,6 +751,108 @@ on an Egeria Server capability that doesn't exist yet — but the pyegeria/ Dr.Egeria-side work each will need once that capability ships is written into the entry now, so it isn't rediscovered from scratch later. +### ISSUE-87: `ClassificationExplorer.add_ownership_to_element`'s docstring sample body says `"class": "OwnerProperties"` — the method itself only accepts `"OwnershipProperties"`, so the documented body cannot be sent + +**Layer:** Pyegeria · **Status:** fixed 2026-09-05 (Pyegeria — +`pyegeria/omvs/classification_explorer.py`). Both docstrings (async +`_async_add_ownership_to_element` and its sync wrapper +`add_ownership_to_element`) corrected to `"OwnershipProperties"`, matching +the code's own `prop=["OwnershipProperties"]` and the `.http` ground +truth. Verified: `TypeAdapter(NewClassificationRequestBody).validate_python(...)` +with the corrected body now validates cleanly (previously raised +`ValidationError`/`unexpected property class name` from the class-name +mismatch). Swept the rest of the file per the original report's suggestion +(every `_async_new_classification_request` caller, matched by function +boundary rather than naive line-proximity to avoid false positives from +adjacent functions) — this was the only class-name/`prop=` mismatch in +`classification_explorer.py`; not repeated elsewhere. Found 2026-09-04 +(Resource Explorer, `Ownership` on everything it publishes). + +`classification_explorer.py:8231` `_async_add_ownership_to_element` documents: + +``` +{ + "class" : "NewClassificationRequestBody", + "properties" : { + "class" : "OwnerProperties", + "owner" : "Add value here", + ... +``` + +and then calls `self._async_new_classification_request(url, prop=["OwnershipProperties"], body=body)` +(`:8285`). `_server_client.py:6175` `validate_new_classification_request` compares +`body["properties"]["class"]` against `prop` and raises +`PyegeriaInvalidParameterException(reason="unexpected property class name")` +when it does not match — **before any HTTP request is made**. So the body in the +docstring is rejected client-side, 100% of the time. + +`pyegeria/http clients/Egeria-api-classification-explorer.http:670` uses +`"OwnershipProperties"` and agrees with the code. The docstring is the outlier; +the same sample appears again in the sync wrapper at `:8288`. + +**How it was found:** by running the documented body against the live +quickstart platform. The exception names neither the class it expected nor the +class it got — `unexpected property class name` with an empty context — so +reading the error does not lead to the answer; only reading +`validate_new_classification_request` does. + +**Ask:** correct both docstrings to `"OwnershipProperties"`. Worth a sweep of +the other `_async_new_classification_request` callers in the same file for the +same docstring/`prop=` divergence — the two were written from different sources +here, so this is unlikely to be the only one. Separately, the validation error +would be far more useful if it named the expected and supplied class names; +`{"reason": "unexpected property class name"}` is a hard error to act on. + +### ISSUE-86: `exec_report_spec` (and `_exec_analytic_chart`, `_run_report_spec`) accept only `user`/`user_pass` — no way to run a report with a bearer token the caller already holds + +**Layer:** Pyegeria · **Status:** fixed 2026-09-05 (Pyegeria — +`pyegeria/view/format_set_executor.py`, `pyegeria/core/mcp_adapter.py`). +Added `token: Optional[str] = None` to `exec_report_spec`, +`_exec_analytic_chart`, and `_run_analytic_function` (the function this +entry's title calls `_run_report_spec` — no such name exists in the +codebase; `_run_analytic_function` is the one all three client-building +call sites actually route through), threaded through every place a client +is built from `view_server`/`view_url`/`user`/`user_pass`. When `token` is +given, the client is authenticated with `set_bearer_token(token)` instead +of `create_egeria_bearer_token()`; when omitted, behavior is byte-for-byte +unchanged (`user`/`user_pass` still mint a fresh token exactly as before). +Also threaded through `run_report`/`_execute_egeria_call_blocking` in +`pyegeria/core/mcp_adapter.py`, per the original ask's "same for +run_report/... on the MCP side if they share the builder" — both do, +both now accept `token`. `describe_report` doesn't build a client at all +(pure registry lookup), so it needed no change. The MCP server's own +`run_report` tool (`pyegeria/core/mcp_server.py`) takes a different, +already-token-capable path (`_async_run_report_tool` + a pre-built +`egeria_client`, not `exec_report_spec`'s builder) — out of scope per the +same "if they share the builder" qualifier, not touched. + +New unit test `tests/micro-tests/test_exec_report_spec_token.py` (2 tests, +monkeypatched `EgeriaTech`, no live server): confirms `token` routes +through `set_bearer_token` with zero calls to `create_egeria_bearer_token`, +and confirms the no-token path is unchanged. Full `pytest +tests/micro-tests/` green (exit 0) throughout. + +**Original report, found 2026-09-04** (Egeria Advisor, trellis-auth token +contract change): + +`pyegeria/view/format_set_executor.py:686` `exec_report_spec(..., user=, user_pass=)` builds its own +client (`:189` and `:814`: `client_class(view_server, view_url, user_id=user, user_pwd=user_pass)` +then `client.create_egeria_bearer_token()`) and passes `user`/`user_pass` down to `_exec_analytic_chart` +(`:718`) and the format-row path (`:780`). There is no parameter for a pre-built client or a bearer +token, and `create_egeria_bearer_token()` is unconditional. + +**Why it matters:** Egeria Advisor's app JWT carries the user's Egeria bearer token and no longer +carries the password (matching the Portal, whose `X-Egeria-Token` + `set_bearer_token()` pattern +already does this for every PyegeriaWebHandler endpoint). Every other EA call site builds a client +and calls `set_bearer_token(token)`; report execution cannot, so EA falls back to the service +account for that one path and Egeria's provenance records the service account instead of the +person who ran the report (`advisor/report_pipeline.py`, search "ISSUE-86"). + +**Ask:** add `token: str | None = None` (or `client: EgeriaTech | None = None`) to `exec_report_spec` +and thread it to the two builders; when given, `set_bearer_token(token)` instead of +`create_egeria_bearer_token()`. Same for `run_report`/`describe_report` on the MCP side if they share +the builder. Backward compatible: `user`/`user_pass` keep working when no token is given. + ### ISSUE-84: `SolutionArchitect.create_solution_blueprint`'s own docstring documents a `NewSolutionElementRequestBody` body (with `initialStatus`) for Draft-status creation — that class does not exist as a pydantic model, so the documented shape fails client-side validation before any HTTP call **Status:** fixed and live-verified 2026-09-03 — docstring-only fix (see @@ -861,6 +1019,13 @@ ACTIVE-only workaround. `ComponentMaterializer`'s identical gap is **still open** — same fix applies, not yet applied there, tracked separately on the trellis/Resource Explorer side. +**GUID note (2026-09-04):** `quickstart-egeria-main` was redeployed with a full +repository-store wipe the same day this was verified. `809025b5-cca9-4e9a-a2f7-3a5104138f67` +is historical — valid as of the 2026-09-03 verification date, not resolvable against the +platform after the wipe. The finding (`contentStatus` round-trips via `properties`, no +separate request-body class needed) is a property of the API/fix, not of that instance, and +is unaffected. + ### ISSUE-82: `pyegeria/omvs/valid_metadata.py` sends the literal query string `typeName=None` whenever `type_name` is Python `None` — breaks every Type-Name-omitted (global) Valid Metadata Value, in 12 of 14 methods across `ValidMetadataManager` **Status:** fixed and live-verified 2026-08-28 — all 12 affected methods @@ -1134,6 +1299,21 @@ now for data that may not exist anywhere. ### ISSUE-80: `find_report_specs_by_perspective`/`find_report_specs_by_question` are implemented and tested but not exposed anywhere +**Status:** fixed 2026-09-05 (Pyegeria — `pyegeria/core/mcp_adapter.py`, +`pyegeria/core/mcp_server.py`). Added `run_find_report_specs_by_perspective`/ +`run_find_report_specs_by_question` adapters (same thin-wrapper pattern as +the existing `run_find_report_specs`) and registered them as MCP tools +`find_report_specs_by_perspective`/`find_report_specs_by_question`, +alongside the existing `find_report_specs` tool. No change to the +underlying functions themselves — pure additive wiring, per the candidate +fix below. `hey_egeria` CLI exposure (the "and/or" half of the original +ask) left undone — MCP was the natural fit given the existing +`find_report_specs` tool precedent; a CLI subcommand is separate follow-up +work if wanted. Full `pytest tests/micro-tests/` green (exit 0); the +underlying functions already have their own functional-test coverage +(`tests/functional-tests/test_question_specs.py`, live-server only), not +re-run here. + **Not a bug — a follow-up.** Found 2026-08-28 while reviewing the Question Spec migration (`docs/design/report_spec_migration_design.md`, Phase 1 complete since 2026-05-18/`6c946af`). Both functions @@ -5937,6 +6117,59 @@ deployment-timing issue, not a code defect — see ISSUE-12, below. ## Not a bug / n/a +### ISSUE-88: no `GovernanceZone` create or lookup anywhere in pyegeria — a zone can be *referenced* by every search and classification, but not made + +**Layer:** Pyegeria · **Status: n/a — not a bug, corrected 2026-09-05 by dwolfson.** +`GovernanceZone` (`0424`) is a `GovernanceDefinition` subtype like every other +one `create_governance_definition` already handles — it's created the same +generic way as `BusinessImperative`/`RegulationArticle`/etc.: pass +`GovernanceDefinitionProperties` with `typeName: "GovernanceZone"` (plus +whatever zone-specific fields the type carries) to +`create_governance_definition`. No dedicated `create_governance_zone` +method is needed, same as there's no `create_business_imperative`. The +workaround below (raw `MetadataExpert`) was solving a problem that +`GovernanceOfficer.create_governance_definition` already covers — confirmed +against `GOV_DEF_PROPERTIES_LIST` in `pyegeria/omvs/governance_officer.py`, +which already accepts `GovernanceDefinitionProperties`, and against the +`.http` ground truth, which has no separate `governance-zones` create +endpoint at all (consistent with there being no dedicated endpoint, because +none is needed). **Real, worth fixing separately:** the method's own +docstring only lists 7 example type names and doesn't mention `GovernanceZone` +(or the other `0401`-family subtypes) as valid `typeName` values — that's +a documentation gap, not an API gap; left as a small follow-up rather than +its own numbered issue. + +**Original (incorrect) report, kept for history:** Found 2026-09-04 +(Resource Explorer, trellis-auth adoption / draft-zone partitioning). + +Searched `pyegeria/omvs/` and `pyegeria/*.py` on 2026-09-04 for +`GovernanceZone` / `governance_zone` / `governance-zones`. Every hit is a +**filter parameter** — `governance_zone_filter=` on `AssetMaker`, +`AutomatedCuration`, `GovernanceOfficer` finds — plus +`ClassificationExplorer.add_zone_membership` / `clear_zone_membership`, which +classify an element *into* a zone. `GovernanceOfficer.create_governance_definition` +creates a `GovernanceDefinition` (`0401`); `GovernanceZone` is `0424` and is not +one of the types that method's own docstring lists. There is no +`create_governance_zone`, no `find_governance_zones`, no `get_governance_zone_by_name`. + +**Why it matters (per the original report — since corrected above):** the +whole `ZoneMembership` partitioning story (Egeria's own recommended +mechanism, `0424`) can be *consumed* through pyegeria but not *bootstrapped*. +An application that partitions what it publishes — Resource Explorer puts +everything it publishes into a `resource-explorer-draft` zone and promotes +it out on curate-accept — has to create that zone before it can be navigated +to, and cannot ask pyegeria to do it. A zone classification does not require +the zone element to exist, so the failure is quiet: elements carry a +`zoneMembership` naming a zone the catalogue has no record of, which reads +as working until someone tries to browse by zone. + +**Workaround in use** (`resource_explorer/egeria_identity.py::ensure_draft_zone_exists`): +`MetadataExpert.create_metadata_element` with `typeName: "GovernanceZone"` and an +`ElementProperties` `propertyValueMap`, guarded by +`get_metadata_element_by_unique_name` for idempotency. It works, but per the +correction above it's solving a problem `create_governance_definition` +already handles more simply — worth switching to that instead. + ### ISSUE-65: `GlossaryManager.get_term_by_guid`'s naming-standards classifications (PrimeWord/ClassWord/Modifier) don't surface as individually-named `elementHeader` keys like most classifications do — undocumented response-shape gotcha, not a data-missing bug **Status:** n/a — not a bug, a response-shape gotcha worth documenting so diff --git a/pyegeria/core/mcp_adapter.py b/pyegeria/core/mcp_adapter.py index 5355d73f..f05d0fcb 100644 --- a/pyegeria/core/mcp_adapter.py +++ b/pyegeria/core/mcp_adapter.py @@ -18,6 +18,7 @@ from pyegeria.view.base_report_formats import ( list_mcp_format_sets, select_report_spec, find_report_specs, + find_report_specs_by_perspective, find_report_specs_by_question, ) from pyegeria.egeria_tech_client import EgeriaTech from pyegeria.view.format_set_executor import exec_report_spec, _async_run_report @@ -48,6 +49,50 @@ def run_find_report_specs(perspective: str= None, question: str= None, report_sp raise ValueError(f"No report specs found for perspective '{perspective}' and question '{question}'") return {"Matching Report Specs" : report_specs} + +def run_find_report_specs_by_perspective(perspective: str, case_insensitive: bool = True) -> Dict[str, Any]: + """ + ISSUE-80: expose find_report_specs_by_perspective as an MCP tool. + + Find report specs whose question_spec includes the given perspective + (e.g. "which report specs does the Data Steward perspective care about"). + + Args: + perspective (str): The perspective to search for (e.g., "Data Steward"). + case_insensitive (bool): If True, compare perspectives case-insensitively. + + Returns: + dict: {"Matching Report Specs": [...]}, one entry per matching question_spec item. + """ + results = find_report_specs_by_perspective(perspective, case_insensitive=case_insensitive) + if not results: + raise ValueError(f"No report specs found for perspective '{perspective}'") + return {"Matching Report Specs": results} + + +def run_find_report_specs_by_question( + question: str, case_insensitive: bool = True, substring: bool = True +) -> Dict[str, Any]: + """ + ISSUE-80: expose find_report_specs_by_question as an MCP tool. + + Find report specs whose question_spec includes a matching example question + (e.g. "which report answers this question"). + + Args: + question (str): The question to search for. + case_insensitive (bool): If True, compare questions case-insensitively. + substring (bool): If True, treat `question` as a substring to match; otherwise require exact match. + + Returns: + dict: {"Matching Report Specs": [...]}, one entry per matching question_spec item. + """ + results = find_report_specs_by_question(question, case_insensitive=case_insensitive, substring=substring) + if not results: + raise ValueError(f"No report specs found for question '{question}'") + return {"Matching Report Specs": results} + + def describe_report(name: str, output_type: str = "DICT") -> Dict[str, Any]: """ Describe a format set for MCP discovery. If outputType != ANY, a concrete format @@ -66,12 +111,17 @@ def _execute_egeria_call_blocking( view_server: Optional[str] = None, view_url: Optional[str] = None, user: Optional[str] = None, - user_pass: Optional[str] = None,) -> Dict[str, Any]: + user_pass: Optional[str] = None, + token: Optional[str] = None,) -> Dict[str, Any]: """ Executes the synchronous, blocking Egeria client call on a dedicated worker thread. You must replace the hardcoded return with your actual Egeria client logic here. All code in this function runs in a blocking, synchronous manner. + + ISSUE-86: `token`, when given, is passed through to `exec_report_spec` so + the report runs as the bearer-token-holding caller rather than the + `user`/`user_pass` service account. """ print( @@ -90,6 +140,7 @@ def _execute_egeria_call_blocking( view_url=view_url if view_url is not None else _settings.Environment.egeria_view_server_url, user=user if user is not None else _settings.User_Profile.user_name, user_pass=user_pass if user_pass is not None else _settings.User_Profile.user_pwd, + token=token, ) # # Returning the hardcoded success for now to prove the async structure works. # return { @@ -108,10 +159,17 @@ def run_report( view_url: Optional[str] = None, user: Optional[str] = None, user_pass: Optional[str] = None, + token: Optional[str] = None, ) -> Dict[str, Any]: """ Execute a format set action as an MCP-style tool. Enforces DICT/ALL by default. Caller may pass credentials explicitly; otherwise defaults are used from config. + + ISSUE-86: pass `token` when the caller already holds a bearer token for + the calling user, so the report runs (and its provenance is recorded) + as that user instead of falling back to the `user`/`user_pass` service + account. `user`/`user_pass` remain fully backward compatible when + `token` is not given. """ print(f"Format set: {report}\nparams: {json.dumps(params)}\nview_server: {view_server}\nview_url: {view_url}\nuser: {user}\nuser_pass: {user_pass}", file=sys.stderr) # Lazy import of settings to avoid circulars when optional args are None @@ -125,6 +183,7 @@ def run_report( view_url=view_url if view_url is not None else _settings.Environment.egeria_view_server_url, user=user if user is not None else _settings.User_Profile.user_name, user_pass=user_pass if user_pass is not None else _settings.User_Profile.user_pwd, + token=token, ) async def _async_run_report_tool( diff --git a/pyegeria/core/mcp_server.py b/pyegeria/core/mcp_server.py index 7dbad2f7..efb7e3dc 100644 --- a/pyegeria/core/mcp_server.py +++ b/pyegeria/core/mcp_server.py @@ -26,7 +26,8 @@ list_reports, describe_report, run_report, _execute_egeria_call_blocking, - _async_run_report_tool, run_find_report_specs + _async_run_report_tool, run_find_report_specs, + run_find_report_specs_by_perspective, run_find_report_specs_by_question, ) print("MCP import successful...", file=sys.stderr) @@ -91,6 +92,18 @@ def find_report_specs_tool(perspective: str=None, question: str=None, report_spe logger.debug("Finding report specs...") return _ok(run_find_report_specs(perspective=perspective, question=question, report_spec=report_spec)) + @srv.tool(name="find_report_specs_by_perspective") + def find_report_specs_by_perspective_tool(perspective: str, case_insensitive: bool = True) -> Dict[str, Any]: + """Finds report specs whose question_spec includes the given perspective (e.g. "Data Steward").""" + logger.debug("Finding report specs by perspective...") + return _ok(run_find_report_specs_by_perspective(perspective, case_insensitive=case_insensitive)) + + @srv.tool(name="find_report_specs_by_question") + def find_report_specs_by_question_tool(question: str, case_insensitive: bool = True, substring: bool = True) -> Dict[str, Any]: + """Finds report specs whose question_spec includes a matching example question.""" + logger.debug("Finding report specs by question...") + return _ok(run_find_report_specs_by_question(question, case_insensitive=case_insensitive, substring=substring)) + @srv.tool(name="describe_report") def describe_report_tool(name: str, output_type: Literal["DICT", "JSON", "MARKDOWN"] = "DICT") -> Dict[str, Any]: diff --git a/pyegeria/omvs/classification_explorer.py b/pyegeria/omvs/classification_explorer.py index 3c15dada..472d2d9f 100644 --- a/pyegeria/omvs/classification_explorer.py +++ b/pyegeria/omvs/classification_explorer.py @@ -8264,7 +8264,7 @@ async def _async_add_ownership_to_element( { "class" : "NewClassificationRequestBody", "properties" : { - "class" : "OwnerProperties", + "class" : "OwnershipProperties", "owner" : "Add value here", "ownerTypeName" : "Add value here", "ownerPropertyName" : "Add value here" @@ -8321,7 +8321,7 @@ def add_ownership_to_element( { "class" : "NewClassificationRequestBody", "properties" : { - "class" : "OwnerProperties", + "class" : "OwnershipProperties", "owner" : "Add value here", "ownerTypeName" : "Add value here", "ownerPropertyName" : "Add value here" diff --git a/pyegeria/view/format_set_executor.py b/pyegeria/view/format_set_executor.py index ff6a5387..02f955a2 100644 --- a/pyegeria/view/format_set_executor.py +++ b/pyegeria/view/format_set_executor.py @@ -163,11 +163,22 @@ def run_analytic_action( def _run_analytic_function( action: dict, *, params: Dict[str, Any], view_server: str, view_url: str, user: str, user_pass: str, + token: Optional[str] = None, ) -> Any: """Resolve and call a report spec action's `analytic_function` (extra_find), returning the function's raw result -- no chart-wrapping, no output_format handling. Shared by `_exec_analytic_series` (SERIES/chart path) and - `exec_report_spec`'s analytic-only passthrough (DICT/JSON/etc. path).""" + `exec_report_spec`'s analytic-only passthrough (DICT/JSON/etc. path). + + ISSUE-86: when `token` is given, the client is authenticated with it + directly (`set_bearer_token`) instead of minting a fresh token via + `create_egeria_bearer_token()` -- lets a caller that already holds a + bearer token for the calling user (e.g. Egeria Advisor's app JWT, which + carries the user's Egeria token and no longer the password) run a report + as that user instead of falling back to the `user`/`user_pass` service + account. Backward compatible: `user`/`user_pass` keep working exactly as + before when no token is given. + """ func_decl = action.get("analytic_function") if not func_decl: raise ValueError("Report spec action has no analytic_function (extra_find).") @@ -186,8 +197,12 @@ def _run_analytic_function( call_params.update({k: v for k, v in params.items() if v not in (None, "")}) func = _resolve_analytic_function(func_decl) - client = EgeriaTech(view_server, view_url, user_id=user, user_pwd=user_pass) - client.create_egeria_bearer_token() + if token: + client = EgeriaTech(view_server, view_url, user_id=user, user_pwd=user_pass) + client.set_bearer_token(token) + else: + client = EgeriaTech(view_server, view_url, user_id=user, user_pwd=user_pass) + client.create_egeria_bearer_token() return func(*_bind_client_args(func, client), **call_params) @@ -577,6 +592,7 @@ def _exec_analytic_chart( view_url: str, user: str, user_pass: str, + token: Optional[str] = None, ) -> Dict[str, Any]: """Run a report spec's `analytic_function` (extra_find) and wrap the result as a Vega-Lite chart -- `chart_kind` picks which: @@ -611,7 +627,7 @@ def _exec_analytic_chart( result = _run_analytic_function( action, params=params, view_server=view_server, view_url=view_url, - user=user, user_pass=user_pass, + user=user, user_pass=user_pass, token=token, ) if not result: return {"kind": "empty"} @@ -692,6 +708,7 @@ def exec_report_spec( view_url: str = settings.Environment.egeria_view_server_url, user: str = settings.User_Profile.user_name, user_pass: str = settings.User_Profile.user_pwd, + token: Optional[str] = None, ) -> Dict[str, Any]: """ Execute the action for a given format set and return a normalized result structure. @@ -701,6 +718,15 @@ def exec_report_spec( - {"kind":"json","data": } - {"kind":"text","mime": "text/markdown"|"text/html","content": str} - {"kind":"unknown","raw": any} + + ISSUE-86: `token`, when given, authenticates the client(s) built here with + `set_bearer_token(token)` instead of minting a fresh token from + `user`/`user_pass` via `create_egeria_bearer_token()`. Use this when the + caller already holds a bearer token for the calling user (e.g. an app + whose session carries the user's Egeria token, not their password) so + report execution -- and its provenance -- reflects that user, not a + fallback service account. `user`/`user_pass` remain fully backward + compatible when `token` is not given. """ output_format = (output_format or "DICT").upper() params = _normalize_report_params(dict(params or {}), action_mode="find") @@ -716,6 +742,7 @@ def exec_report_spec( return _exec_analytic_chart( format_set_name, chart_kind=_CHART_KINDS[output_format], params=params, view_server=view_server, view_url=view_url, user=user, user_pass=user_pass, + token=token, ) # Resolve the format set and action @@ -777,7 +804,7 @@ def exec_report_spec( if action.get("analytic_function") and not func_decl: result = _run_analytic_function( action, params=params, view_server=view_server, view_url=view_url, - user=user, user_pass=user_pass, + user=user, user_pass=user_pass, token=token, ) if result is None or result == [] or result == {}: return {"kind": "empty"} @@ -814,7 +841,10 @@ def exec_report_spec( client = client_class(view_server, view_url, user_id=user, user_pwd=user_pass) try: - client.create_egeria_bearer_token() + if token: + client.set_bearer_token(token) + else: + client.create_egeria_bearer_token() func = getattr(client, method_name) if method_name and hasattr(client, method_name) else None if func is None: raise AttributeError( diff --git a/tests/micro-tests/test_exec_report_spec_token.py b/tests/micro-tests/test_exec_report_spec_token.py new file mode 100644 index 00000000..b3fbd145 --- /dev/null +++ b/tests/micro-tests/test_exec_report_spec_token.py @@ -0,0 +1,97 @@ +"""ISSUE-86: exec_report_spec (and _exec_analytic_chart, _run_analytic_function) +should authenticate with a caller-supplied bearer token via set_bearer_token() +instead of always minting a fresh one via create_egeria_bearer_token() when a +token is available -- e.g. Egeria Advisor, whose session carries the user's +Egeria token and no longer their password. + +Unit-level: monkeypatches EgeriaTech inside format_set_executor with a fake +that just records which auth path was taken -- no live server required. +""" +import pytest + +from pyegeria.view import format_set_executor as fse + + +class _FakeEgeriaTech: + """Records how it was authenticated; raises if create_egeria_bearer_token() + is called when a token was supplied (that would mean the token path was + bypassed and a fresh token minted from user/user_pass instead).""" + + def __init__(self, view_server=None, view_url=None, user_id=None, user_pwd=None, **kwargs): + self.view_server = view_server + self.view_url = view_url + self.user_id = user_id + self.user_pwd = user_pwd + self.set_bearer_token_calls = [] + self.create_egeria_bearer_token_calls = 0 + + def set_bearer_token(self, token): + self.set_bearer_token_calls.append(token) + + def create_egeria_bearer_token(self): + self.create_egeria_bearer_token_calls += 1 + + +@pytest.fixture +def fake_egeria_tech(monkeypatch): + instances = [] + + def factory(*args, **kwargs): + inst = _FakeEgeriaTech(*args, **kwargs) + instances.append(inst) + return inst + + monkeypatch.setattr(fse, "EgeriaTech", factory) + return instances + + +def _dummy_analytic_action(): + def _dummy(client): + return {"count": 1} + return {"analytic_function": _dummy, "analytic_spec_params": {}} + + +def test_run_analytic_function_uses_token_when_given(fake_egeria_tech, monkeypatch): + """When a token is supplied, set_bearer_token(token) is used -- not + create_egeria_bearer_token().""" + monkeypatch.setattr( + fse, "_resolve_analytic_function", lambda decl: decl + ) + monkeypatch.setattr( + fse, "_bind_client_args", lambda func, client: (client,) + ) + + action = _dummy_analytic_action() + result = fse._run_analytic_function( + action, params={}, view_server="vs", view_url="url", + user="svc-user", user_pass="svc-pass", token="caller-token-123", + ) + + assert result == {"count": 1} + assert len(fake_egeria_tech) == 1 + client = fake_egeria_tech[0] + assert client.set_bearer_token_calls == ["caller-token-123"] + assert client.create_egeria_bearer_token_calls == 0 + + +def test_run_analytic_function_mints_token_when_none_given(fake_egeria_tech, monkeypatch): + """Backward compatible: with no token, the old create_egeria_bearer_token() + path from user/user_pass is used unchanged.""" + monkeypatch.setattr( + fse, "_resolve_analytic_function", lambda decl: decl + ) + monkeypatch.setattr( + fse, "_bind_client_args", lambda func, client: (client,) + ) + + action = _dummy_analytic_action() + result = fse._run_analytic_function( + action, params={}, view_server="vs", view_url="url", + user="svc-user", user_pass="svc-pass", + ) + + assert result == {"count": 1} + assert len(fake_egeria_tech) == 1 + client = fake_egeria_tech[0] + assert client.set_bearer_token_calls == [] + assert client.create_egeria_bearer_token_calls == 1