From 8f8480beb5a4a81f1e7d81e607622bd3c710c500 Mon Sep 17 00:00:00 2001 From: Dan Wolfson Date: Tue, 27 Oct 2026 10:00:05 -0500 Subject: [PATCH] Adopt Egeria 6.3 boxed properties and raise on failed bearer token requests Egeria 6.3 (odpi/egeria#9389) leaves unset numbers and flags out of responses and keeps them on a merge update, which fixes ISSUE-166. - Dr.Egeria: a Create rewritten to Update no longer sends the spec defaults the parser filled in, which would overwrite stored values. - Stop forcing 0 for GovernanceControlProperties.domainIdentifier, EmbeddedConnectionProperties.position, role headCount and Solution Role domainIdentifier. - Remove the ISSUE-166 workarounds: security finding rows now show a returned 0 / false, and SecurityFindingProcessor no longer copies the current values into a merge update. - create_egeria_bearer_token raised nothing on bad credentials and returned Egeria's 401 error text as the token (or "FAILED" on a connection error). It now raises PyegeriaUnauthorizedException, PyegeriaClientException, PyegeriaConnectionException or PyegeriaTimeoutException and leaves the headers unchanged. Signed-off-by: Dan Wolfson --- EGERIA_OPEN_ISSUES.md | 6 +- PYEGERIA_ISSUES.md | 4 +- docs/dr_egeria_manual.md | 2 +- .../md_processing_utils/common_md_utils.py | 14 ++-- md_processing/v2/processors.py | 17 +++++ md_processing/v2/security_officer.py | 10 --- pyegeria/core/_base_platform_client.py | 64 +++++++++++------ pyegeria/core/_base_server_client.py | 35 ++++------ pyegeria/models/models.py | 4 +- pyegeria/omvs/classification_explorer.py | 10 ++- pyegeria/omvs/data_discovery.py | 21 ++---- pyegeria/view/base_report_formats.py | 11 ++- .../test_create_bearer_token_errors.py | 70 +++++++++++++++++++ .../test_dr_egeria_security_officer.py | 8 +-- .../micro-tests/test_security_annotations.py | 28 +++++--- .../test_upsert_drops_create_defaults.py | 38 ++++++++++ 16 files changed, 228 insertions(+), 114 deletions(-) create mode 100644 tests/micro-tests/test_create_bearer_token_errors.py create mode 100644 tests/micro-tests/test_upsert_drops_create_defaults.py diff --git a/EGERIA_OPEN_ISSUES.md b/EGERIA_OPEN_ISSUES.md index 3e942c3d..bfaa195b 100644 --- a/EGERIA_OPEN_ISSUES.md +++ b/EGERIA_OPEN_ISSUES.md @@ -10,7 +10,7 @@ It replaces the 2026-10-05 snapshot `docs/egeria-open-bugs-2026-10-05.md` (delet | Group | Issues | |---|---| -| **Open on the Egeria side** (full entries below) | ISSUE-166, ISSUE-147, ISSUE-143, ISSUE-130, ISSUE-128, ISSUE-108 | +| **Open on the Egeria side** (full entries below) | ISSUE-147, ISSUE-143, ISSUE-130, ISSUE-128, ISSUE-108 | | **Awaiting re-test:** a fix landed (mostly Egeria `main`, odpi/egeria#9363, merged 2026-10-06), not yet verified live | ISSUE-125, ISSUE-124, ISSUE-117, ISSUE-112, ISSUE-95, ISSUE-90, ISSUE-89, ISSUE-79, ISSUE-38 | | **Egeria-side, but tracked as pyegeria items** (a pyegeria change waits on Egeria) | ISSUE-146, ISSUE-132 | @@ -37,7 +37,9 @@ It replaces the 2026-10-05 snapshot `docs/egeria-open-bugs-2026-10-05.md` (delet ### 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:** open, question for the Egeria team · **Found:** 2026-10-26, while adding the 6.3 security annotation types to pyegeria (odpi/egeria-workspaces#702 hand-off, step 3; PR #495). +**Layer:** Egeria Server (type model / properties classes) · **Status:** fixed (Egeria 6.3, odpi/egeria#9389) · **Found:** 2026-10-26, while adding the 6.3 security annotation types to pyegeria (odpi/egeria-workspaces#702 hand-off, step 3; PR #495). + +**FIXED 2026-10-27: odpi/egeria#9389, merged as 12b1aaae.** Verified live the same day on egeria-platform c2ca054c (throwaway VulnerabilityAnnotation ed152edb, deleted, NotFound by GUID): unset cvssScore/exploitAvailable/patchAvailable were omitted; a merge update that left out 8.6 / true / false kept them; an explicit false came back. The pyegeria workarounds below are removed on branch `feature/boxed-properties-audit`. It boxes 217 number and flag fields across 86 property beans, not only the security ones. An unset value is no longer stored and is left out of the response, and a merge update keeps a value it does not mention. Defaults move to `OpenMetadataProperty.defaultValue`, published in the Core Content Pack as `additionalProperties.defaultValue` on each property's valid metadata value. `allowsDuplicateValues` now defaults to true everywhere, including SchemaAttribute (was false). The PR does not migrate stored data: elements written by earlier servers keep the 0 / false they were stored with. **pyegeria:** fixes that work with either server are on branch `feature/boxed-properties-audit`. The client-side workarounds stay until the PR merges and pyegeria targets that server. **What happens:** in `SecurityFindingAnnotationProperties`, `VulnerabilityAnnotationProperties` and `SecurityPostureAnnotationProperties` (`frameworks/open-metadata-framework/.../properties/surveyreports/`), these fields are Java primitives with a 0 / false default: diff --git a/PYEGERIA_ISSUES.md b/PYEGERIA_ISSUES.md index 479781ee..d2561a9d 100644 --- a/PYEGERIA_ISSUES.md +++ b/PYEGERIA_ISSUES.md @@ -1835,7 +1835,9 @@ enough to track there too). ### 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:** open, question for the Egeria team · **Found:** 2026-10-26, while adding the 6.3 security annotation types to pyegeria (odpi/egeria-workspaces#702 hand-off, step 3; PR #495). +**Layer:** Egeria Server (type model / properties classes) · **Status:** fixed (Egeria 6.3, odpi/egeria#9389) · **Found:** 2026-10-26, while adding the 6.3 security annotation types to pyegeria (odpi/egeria-workspaces#702 hand-off, step 3; PR #495). + +**FIXED 2026-10-27: odpi/egeria#9389, merged as 12b1aaae.** Verified live the same day on egeria-platform c2ca054c (throwaway VulnerabilityAnnotation ed152edb, deleted, NotFound by GUID): unset cvssScore/exploitAvailable/patchAvailable were omitted; a merge update that left out 8.6 / true / false kept them; an explicit false came back. The pyegeria workarounds below are removed on branch `feature/boxed-properties-audit`. It boxes 217 number and flag fields across 86 property beans, not only the security ones. An unset value is no longer stored and is left out of the response, and a merge update keeps a value it does not mention. Defaults move to `OpenMetadataProperty.defaultValue`, published in the Core Content Pack as `additionalProperties.defaultValue` on each property's valid metadata value. `allowsDuplicateValues` now defaults to true everywhere, including SchemaAttribute (was false). The PR does not migrate stored data: elements written by earlier servers keep the 0 / false they were stored with. **pyegeria:** fixes that work with either server are on branch `feature/boxed-properties-audit`. The client-side workarounds stay until the PR merges and pyegeria targets that server. **What happens:** in `SecurityFindingAnnotationProperties`, `VulnerabilityAnnotationProperties` and `SecurityPostureAnnotationProperties` (`frameworks/open-metadata-framework/.../properties/surveyreports/`), these fields are Java primitives with a 0 / false default: diff --git a/docs/dr_egeria_manual.md b/docs/dr_egeria_manual.md index 4bbd9aa6..10fe5cd3 100644 --- a/docs/dr_egeria_manual.md +++ b/docs/dr_egeria_manual.md @@ -272,7 +272,7 @@ Dr.Egeria organizes its commands into "families," each corresponding to a specif - Security testing results (model 0665): `Create`/`Update Survey Report` catalogues one test run, through Asset Maker. It takes Category (e.g. Penetration Test Results), Purpose, Start Time and Completion Time; `Report Subjects` links the assets the run tested (ReportSubject). `Create`/`Update Security Finding`, `Vulnerability Finding` and `Penetration Test Finding` each create one finding under its `Survey Report` (ReportedAnnotation, required), with Severity (the CVSS ratings None/Low/Medium/High/Critical), CVSS Score and the type's own fields. `Affected Elements` links catalogued elements (AssociatedAnnotation), `Previous Finding` links the same finding in an earlier run (AnnotationExtension), and `Vulnerability Type` links a reference data value (ReferenceValueAssignment). `Create Security Posture Summary` records a run's counts: `Finding Counts By Severity` takes one `severity: count` pair per line. Egeria stores the CVSS score, exploit/patch flags and posture counts as primitives, so a merge update that leaves one out would reset it to 0 / false; these commands copy the current value into the update to prevent that (ISSUE-166). Processors in `md_processing/v2/security_officer.py`. - `Link`/`Unlink Control Applicability`: assess a control (any governance definition, typically a GovernanceControl) in a statement — `Is Applicable`, `Justification`, `Inclusion Reasons`, `Implementation Status` (NOT_STARTED, PLANNED, PARTIALLY_IMPLEMENTED, IMPLEMENTED, OTHER), `Implementation Description`, `Review Date`, and `Label` for the control's number in the standard. There is one relationship per control, and re-running Link updates it. Egeria's attach replaces the relationship's properties wholesale, so the command starts from the existing ones: re-linking with just a new status keeps the justification and the rest (`Merge Update: false` replaces them). - Reports: `Statement-of-Applicability-Controls` (one row per control — use `LIST` for a markdown table, `DICT` for a spreadsheet) and `Statement-of-Applicability-Implementation-Summary` (applicable controls counted by implementation status; `BAR`/`PIE` for a chart). Both take `soa_guid`, which accepts the statement's GUID or qualified name, e.g. `View Report` with `Report Parameters` `soa_guid: StatementOfApplicability::Coco Pharmaceuticals ISMS`. The summary is analytic-function-backed, which `View Report` cannot run yet; use it through `exec_report_spec` or `SecurityOfficer.get_control_implementation_summary`. - - Security testing results (Egeria 6.3, model 0665): each test run is a `SurveyReport`, with one `SecurityFindingAnnotation` (or `VulnerabilityAnnotation` / `PenetrationTestFindingAnnotation`) per finding and a `SecurityPostureAnnotation` summarizing the run. The report specs `Security-Findings` (most severe first; params `report_guid` or `element_guid`, each a GUID or qualified name, and optional `min_severity`), `Penetration-Test-Findings` and `Security-Posture` (one row per run, oldest first; optional `report_qualified_name_prefix` to follow one system's series) run through `View Report`. Egeria returns `cvssScore`, `exploitAvailable`, `patchAvailable` and the posture counts as 0 or false when they were never set, so these reports show such values blank rather than as an answer; new and resolved finding counts of 0 cannot be told apart from unrecorded. Dr.Egeria commands for creating survey reports and findings are not available yet. + - Security testing results (Egeria 6.3, model 0665): each test run is a `SurveyReport`, with one `SecurityFindingAnnotation` (or `VulnerabilityAnnotation` / `PenetrationTestFindingAnnotation`) per finding and a `SecurityPostureAnnotation` summarizing the run. The report specs `Security-Findings` (most severe first; params `report_guid` or `element_guid`, each a GUID or qualified name, and optional `min_severity`), `Penetration-Test-Findings` and `Security-Posture` (one row per run, oldest first; optional `report_qualified_name_prefix` to follow one system's series) run through `View Report`. From Egeria 6.3, a `cvssScore`, `exploitAvailable`, `patchAvailable` or posture count that was never set is left out, so it shows blank, and a 0 or false shown is a real answer. Annotations written by earlier servers may still show 0 or false for a value nobody set. Dr.Egeria commands for creating survey reports and findings are not available yet. - **Action Author**: Define governance action process flows — reusable single-step action types and multi-step processes — and wire them to the engines and elements that execute them, without writing code (e.g., `Create Governance Action Process`, `Create Governance Action Process Step`, `Link First Process Step`, `Link Next Process Step`, `Link Action to Action Executor`, `Link Action to Target`). - **Curation**: Apply classifications and relationships to *existing* Referenceable elements after the fact — this family creates no elements of its own, it curates ones created elsewhere. Every command names a `Target Element` (Reference Name) and either a classification level/status or a second element to relate to. - **Classifications** (`Classify`/`Reclassify`/`Declassify`, or `Classify`/`Update`/`Declassify` for the three multi-value ones): Impact, Confidence, Confidentiality, Criticality, Retention, Ownership, Digital Resource Origin, Zone Membership, Security Tags, Data Scope, Governance Expectations, Governance Measurements, Known Duplicate, Consolidated Duplicate, and the 0438 naming-standards markers Class Word, Modifier, Prime Word (added 2026-08-09 once Egeria PR #9166 shipped the backing `glossary-manager` endpoints — routed through `GlossaryManager`, not `ClassificationExplorer`, unlike every other classification here). Also, added 2026-08-21 once an `omvs_audit.py` gap-closure pass shipped their backing REST endpoints (all confirmed against a live 6.2-SNAPSHOT server): the 10 governance-point classifications (0435: Control/Verification/Enforcement/Execution Point, Policy Administration/Decision/Enforcement/Information/Management/Retrieval Point — routed through `GovernanceOfficer`), 6 metamodel markers (0463: Incomplete, ObjectIdentifier, ReferenceData, MobileResource, InstanceMetadata, MetamodelInstance — routed through `ClassificationExplorer`), ProjectKind (routed through `ProjectManager`), CollectionKind (routed through `CollectionManager`), and a Data Sharing Agreement retrofit pair for classifying/declassifying an already-existing Agreement (routed through `DigitalBusiness` — `Create Data Sharing Agreement`, Digital Products family, already sets this at creation time via `initialClassifications`). Also Promise (0010, added 2026-09-30 — `Classify`/`Declassify Promise`, routed through `ClassificationExplorer`): marks an element as a placeholder for a real-world resource that has not yet been delivered, with Deployment Status, Start/Due/Last Review/Completion Time and Additional Properties. A promised element is only visible to lineage requests, so `Declassify Promise` looks up its Target Element and sends the clear request with `forLineage=true` automatically. diff --git a/md_processing/md_processing_utils/common_md_utils.py b/md_processing/md_processing_utils/common_md_utils.py index 84a9ce91..bdd997ad 100644 --- a/md_processing/md_processing_utils/common_md_utils.py +++ b/md_processing/md_processing_utils/common_md_utils.py @@ -507,16 +507,12 @@ def set_actor_manager_prop_body(object_type: str, qualified_name: str, attribute if "Role" in object_type: prop_bod["scope"] = attributes.get('Scope', {}).get('value', None) if "Person Role" in object_type or "Team Role" in object_type: - # headCount is an integer - headcount_entry = attributes.get('Headcount', {}) - headcount = headcount_entry.get('value', 0) + # headCount is an integer; left unset when not given, so an update keeps the stored value + headcount = attributes.get('Headcount', {}).get('value') try: - if headcount is not None: - prop_bod["headCount"] = int(headcount) - else: - prop_bod["headCount"] = 0 + prop_bod["headCount"] = int(headcount) if headcount not in (None, "") else None except (ValueError, TypeError): - prop_bod["headCount"] = 0 + prop_bod["headCount"] = None return prop_bod @@ -547,7 +543,7 @@ def set_solution_architect_body(object_type: str, qualified_name: str, attribute }) if object_type in ("Solution Role", "SolutionRole", "SolutionActorRole"): - role_domain_id = attributes.get('Role Domain Identifier', {}).get('value', 0) + role_domain_id = attributes.get('Role Domain Identifier', {}).get('value') resolved_domain = resolve_enum(GovernanceDomains, role_domain_id) prop_bod["domainIdentifier"] = resolved_domain if resolved_domain is not None else role_domain_id diff --git a/md_processing/v2/processors.py b/md_processing/v2/processors.py index 59f1e15b..3fa98524 100644 --- a/md_processing/v2/processors.py +++ b/md_processing/v2/processors.py @@ -777,6 +777,7 @@ async def _execute(self) -> Dict[str, Any]: if self.command.verb in ["Create", "Define", "Register", "Add", "Upsert"]: logger.info(f"Rewriting '{self.command.verb} {self.command.object_type}' to 'Update' as it already exists.") self.command.verb = "Update" + self._drop_create_defaults() self.parsed_output["exists"] = True header = self.as_is_element.get('elementHeader', {}) @@ -1954,6 +1955,22 @@ async def sync_members(self, return results + # Control flags rather than element properties: keep their defaults on an update. + _KEEP_DEFAULTS_ON_UPDATE = {"Merge Update"} + + def _drop_create_defaults(self) -> None: + """Remove spec defaults the parser filled in for a Create that has become an Update. + + The parser applies defaults only for create-like verbs, so a Create rewritten to Update + still carries them, and a merge update would send them over the stored values (e.g. + isNullable: true over a stored false). Egeria 6.3 keeps an omitted number or flag + (odpi/egeria#9389), so leaving them out is what preserves the current value. + """ + attributes = self.parsed_output.get("attributes") or {} + for name in [n for n, d in attributes.items() + if isinstance(d, dict) and d.get("is_default") and n not in self._KEEP_DEFAULTS_ON_UPDATE]: + del attributes[name] + def filter_update_properties(self, properties: Dict[str, Any], merge_update: bool) -> Dict[str, Any]: """ Filters properties for an update operation. diff --git a/md_processing/v2/security_officer.py b/md_processing/v2/security_officer.py index b7e62afd..fe1dd2a8 100644 --- a/md_processing/v2/security_officer.py +++ b/md_processing/v2/security_officer.py @@ -300,10 +300,6 @@ async def _existing_applicability(self, soa_guid: str, control_guid: str) -> Dic } -# Egeria declares these as Java primitives, so a merge update that leaves one out stores -# its 0 / false default over the current value (confirmed live, ISSUE-166). -_PRIMITIVE_FIELDS = ("cvssScore", "exploitAvailable", "patchAvailable", "resourcesInScopeCount", - "resourcesTestedCount", "newFindingCount", "resolvedFindingCount") _FLOAT_FIELDS = {"cvssScore"} _INT_FIELDS = {"resourcesInScopeCount", "resourcesTestedCount", "newFindingCount", "resolvedFindingCount"} @@ -403,12 +399,6 @@ async def apply_changes(self) -> str: if self.as_is_element: guid = self.as_is_element["elementHeader"]["guid"] - if merge_update: - # Without this, the merge would reset any primitive the command leaves out. - current = self.as_is_element.get("properties") or {} - for key in _PRIMITIVE_FIELDS: - if key not in props and current.get(key) is not None: - props[key] = current[key] self.last_body = body = { "class": "UpdateElementRequestBody", "properties": self.filter_update_properties(props, merge_update), diff --git a/pyegeria/core/_base_platform_client.py b/pyegeria/core/_base_platform_client.py index 1a022c15..f4872df3 100644 --- a/pyegeria/core/_base_platform_client.py +++ b/pyegeria/core/_base_platform_client.py @@ -24,7 +24,8 @@ from pyegeria.core.config import settings from pyegeria.core._exceptions import ( PyegeriaAPIException, PyegeriaConnectionException, PyegeriaInvalidParameterException, - PyegeriaUnknownException, PyegeriaClientException, PyegeriaTimeoutException + PyegeriaUnknownException, PyegeriaClientException, PyegeriaTimeoutException, + PyegeriaUnauthorizedException ) from pyegeria.core._globals import enable_ssl_check, max_paging_size from pyegeria.core._validators import ( @@ -55,6 +56,36 @@ def lineage_visible(enabled: bool = True): _for_lineage_default.reset(token) +async def _post_for_bearer_token(url: str, data: dict, headers: dict, user_id: str) -> str: + """POST credentials to an Egeria token endpoint and return the token, raising on any failure. + + The endpoint answers a bad user id or password with a 401 whose body is error text, so the + status code has to be checked before the body is used as a token. + """ + context = {"caller method": "create_egeria_bearer_token"} + try: + async with AsyncClient(verify=enable_ssl_check) as client: + response = await client.post(url, json=data, headers=headers) + except httpx.TimeoutException as e: + raise PyegeriaTimeoutException(context, {"endpoint": url, "error_kind": "timeout"}, e) + except httpx.HTTPError as e: + raise PyegeriaConnectionException(context, {"endpoint": url, "error_kind": "connection"}, e) + + additional_info = {"userid": user_id, "reason": response.text} + if response.status_code in (401, 403): + raise PyegeriaUnauthorizedException(response, context, additional_info) + if response.status_code != 200: + raise PyegeriaClientException(response, context, additional_info) + + token = response.text.strip() + if not token: + raise PyegeriaInvalidParameterException(None, context, {"reason": "No token returned - request issue?"}) + if token.startswith("{"): + # An Egeria-wrapped error with HTTP 200; a token is never a JSON object. + raise PyegeriaInvalidParameterException(None, context, additional_info) + return token + + def _apply_for_lineage_default(payload: str | dict | None) -> str | dict | None: """Set forLineage=true on a request body when inside an enabled lineage_visible() block. @@ -309,27 +340,16 @@ async def _async_create_egeria_bearer_token( data = {"userId": user_id, "password": password} if new_password: data["newPassword"] = new_password - async with AsyncClient(verify=enable_ssl_check) as client: - try: - # Deliberately not self.headers: it may still carry a stale/expired - # Authorization header (that's exactly what we're refreshing), and - # this endpoint authenticates via the body credentials, not a bearer - # token - sending a bad Authorization header here gets this request - # itself rejected with 401, defeating the whole refresh. - response = await client.post(url, json=data, headers=self.json_header) - token = response.text - except httpx.HTTPError as e: - print(e) - return "FAILED" - - if token: - self.token_src = "Egeria" - self.headers["Authorization"] = f"Bearer {token}" - self.text_headers["Authorization"] = f"Bearer {token}" - return token - else: - additional_info = {"reason": "No token returned - request issue?"} - raise PyegeriaInvalidParameterException(None, None, additional_info) + # Deliberately not self.headers: it may still carry a stale/expired + # Authorization header (that's exactly what we're refreshing), and + # this endpoint authenticates via the body credentials, not a bearer + # token - sending a bad Authorization header here gets this request + # itself rejected with 401, defeating the whole refresh. + token = await _post_for_bearer_token(url, data, self.json_header, user_id) + self.token_src = "Egeria" + self.headers["Authorization"] = f"Bearer {token}" + self.text_headers["Authorization"] = f"Bearer {token}" + return token def create_egeria_bearer_token( self, user_id: str = None, password: str = None diff --git a/pyegeria/core/_base_server_client.py b/pyegeria/core/_base_server_client.py index c0cb5395..b211f6e3 100644 --- a/pyegeria/core/_base_server_client.py +++ b/pyegeria/core/_base_server_client.py @@ -25,7 +25,7 @@ PyegeriaNotFoundException, PyegeriaUnauthorizedException ) from pyegeria.core._globals import enable_ssl_check, max_paging_size -from pyegeria.core._base_platform_client import _apply_for_lineage_default +from pyegeria.core._base_platform_client import _apply_for_lineage_default, _post_for_bearer_token from pyegeria.core._validators import ( validate_name, validate_server_name, @@ -272,27 +272,16 @@ async def _async_create_egeria_bearer_token( data = {"userId": user_id, "password": password} if new_password: data["newPassword"] = new_password - async with AsyncClient(verify=enable_ssl_check) as client: - try: - # Deliberately not self.headers: it may still carry a stale/expired - # Authorization header (that's exactly what we're refreshing), and - # this endpoint authenticates via the body credentials, not a bearer - # token - sending a bad Authorization header here gets this request - # itself rejected with 401, defeating the whole refresh. - response = await client.post(url, json=data, headers=self.json_header) - token = response.text - except httpx.HTTPError as e: - print(e) - return "FAILED" - - if token: - self.token_src = "Egeria" - self.headers["Authorization"] = f"Bearer {token}" - self.text_headers["Authorization"] = f"Bearer {token}" - return token - else: - additional_info = {"reason": "No token returned - request issue?"} - raise PyegeriaInvalidParameterException(None, None, additional_info) + # Deliberately not self.headers: it may still carry a stale/expired + # Authorization header (that's exactly what we're refreshing), and + # this endpoint authenticates via the body credentials, not a bearer + # token - sending a bad Authorization header here gets this request + # itself rejected with 401, defeating the whole refresh. + token = await _post_for_bearer_token(url, data, self.json_header, user_id) + self.token_src = "Egeria" + self.headers["Authorization"] = f"Bearer {token}" + self.text_headers["Authorization"] = f"Bearer {token}" + return token def create_egeria_bearer_token( self, user_id: str = None, password: str = None, new_password: str = None @@ -634,7 +623,7 @@ async def _async_make_request( new_token = await self._async_create_egeria_bearer_token() except Exception: new_token = None - if new_token and new_token != "FAILED": + if new_token: return await self._async_make_request( request_type, endpoint, payload, is_json, params, timeout=timeout, _retry_on_auth=False, as_text=as_text, diff --git a/pyegeria/models/models.py b/pyegeria/models/models.py index 4ba50a26..09916546 100644 --- a/pyegeria/models/models.py +++ b/pyegeria/models/models.py @@ -159,7 +159,7 @@ class ReferenceableProperties(OpenMetadataRootProperties): class GovernanceControlProperties(ReferenceableProperties): class_: Annotated[Literal["GovernanceControlProperties"], Field(alias="class")] - domain_identifier: int | None = 0 + domain_identifier: int | None = None document_identifier: str | None = None title: str | None = None summary: str | None = None @@ -515,7 +515,7 @@ class ConnectionEndpointProperties(PyegeriaModel): class EmbeddedConnectionProperties(PyegeriaModel): class_: Annotated[Literal["EmbeddedConnectionProperties"], Field(alias="class")] - position: int | None = 0 + position: int | None = None display_name: str | None = None arguments: dict | None = None effective_from: datetime | None = None diff --git a/pyegeria/omvs/classification_explorer.py b/pyegeria/omvs/classification_explorer.py index e6cae990..641251dd 100644 --- a/pyegeria/omvs/classification_explorer.py +++ b/pyegeria/omvs/classification_explorer.py @@ -13607,9 +13607,8 @@ async def _async_update_usage_activity(self, element_guid: str, body: dict | UpdateClassificationRequestBody) -> None: """Update the properties of an element's UsageActivity classification. Async version. - Send every count, even with "mergeUpdate": true. Egeria's count fields are primitive longs, - so an omitted count arrives as 0 and overwrites the stored value (seen live: explicitGetCount - 1 -> 0 on a findCount-only merge update). Profile maps that are omitted are kept. Sample body: + With "mergeUpdate": true, omitted counts and profile maps keep their stored values. Before + Egeria 6.3 (odpi/egeria#9389) an omitted count was reset to 0, so send every count to older servers. Sample body: { "class" : "UpdateClassificationRequestBody", @@ -13707,9 +13706,8 @@ async def _async_update_member_popularity_profile(self, element_guid: str, body: dict | UpdateClassificationRequestBody) -> None: """Update the properties of an element's MemberPopularityProfile classification. Async version. - Send every count, even with "mergeUpdate": true. Egeria's count fields are primitive longs, - so an omitted count arrives as 0 and overwrites the stored value (seen live: explicitGetCount - 1 -> 0 on a findCount-only merge update). Profile maps that are omitted are kept. Sample body: + With "mergeUpdate": true, omitted counts and profile maps keep their stored values. Before + Egeria 6.3 (odpi/egeria#9389) an omitted count was reset to 0, so send every count to older servers. Sample body: { "class" : "UpdateClassificationRequestBody", diff --git a/pyegeria/omvs/data_discovery.py b/pyegeria/omvs/data_discovery.py index cde1bab5..568fbd9c 100644 --- a/pyegeria/omvs/data_discovery.py +++ b/pyegeria/omvs/data_discovery.py @@ -1697,13 +1697,10 @@ def _report_of(annotation: dict) -> dict: def _security_row(cls, annotation: dict) -> dict: """One display row for a security finding or posture annotation. - Egeria stores cvssScore, exploitAvailable, patchAvailable and the posture counts as Java - primitives, so an unset value comes back as 0 or false and cannot be told apart from a real one. - Where that would read as an answer nobody gave, the row leaves the value blank instead: - - - a CVSS score of 0.0 is only shown when the severity is "None", the rating a 0.0 score means; - - exploitAvailable and patchAvailable are only shown when true; - - a run with 0 resources in scope was not recorded, so both resource counts are blank. + From Egeria 6.3 (odpi/egeria#9389) an unset cvssScore, exploitAvailable, patchAvailable or + posture count is left out of the response, so it shows blank, and a 0 or false that comes + back is a real answer. Annotations written by earlier servers may still hold a stored 0 or + false that nobody set (ISSUE-166). Counts by severity are read from findingCountsBySeverity: a severity missing from a recorded map is 0, and with no map at all every severity count is blank. @@ -1712,21 +1709,11 @@ def _security_row(cls, annotation: dict) -> dict: type_name = ((annotation.get("elementHeader") or {}).get("type") or {}).get("typeName") props["findingType"] = (type_name or "").removesuffix("Annotation") - if props.get("cvssScore") in (0, 0.0) and props.get("severity") != "None": - props["cvssScore"] = None - for flag in ("exploitAvailable", "patchAvailable"): - if props.get(flag) is False: - props[flag] = None - if type_name == "SecurityPostureAnnotation": counts = props.get("findingCountsBySeverity") for severity in cls.SEVERITY_RANK: props[f"{severity.lower()}Findings"] = (counts.get(severity, 0) if isinstance(counts, dict) else None) - if not props.get("resourcesInScopeCount"): - props["resourcesInScopeCount"] = None - if not props.get("resourcesTestedCount"): - props["resourcesTestedCount"] = None # Column keys are snake_case mapped to camelCase, which cannot produce packageURL. if "packageURL" in props: diff --git a/pyegeria/view/base_report_formats.py b/pyegeria/view/base_report_formats.py index 754dcae2..950e10fc 100644 --- a/pyegeria/view/base_report_formats.py +++ b/pyegeria/view/base_report_formats.py @@ -2255,8 +2255,8 @@ def _user_report_specs_dir() -> str: target_type="SecurityFindingAnnotation", heading="Security Findings", description="Security findings from one survey report, or about one element, most severe first. " - "A blank CVSS score, Exploit Available or Patch Available means not recorded: Egeria " - "returns 0 or false for these when they were never set.", + "A blank CVSS score, Exploit Available or Patch Available means not recorded. Findings " + "written before Egeria 6.3 may show 0 or false for a value that was never set.", aliases=["Vulnerabilities", "Security Finding"], family="Security Officer", formats=[ @@ -2291,7 +2291,7 @@ def _user_report_specs_dir() -> str: target_type="PenetrationTestFindingAnnotation", heading="Penetration Test Findings", description="The findings of a penetration test, most severe first - the view the tester hands back. " - "A blank CVSS score means not recorded.", + "A blank CVSS score means not recorded; before Egeria 6.3 it showed as 0.", aliases=["Pen Test Findings"], family="Security Officer", formats=[ @@ -2322,9 +2322,8 @@ def _user_report_specs_dir() -> str: target_type="SecurityPostureAnnotation", heading="Security Posture", description="One row per security test run, oldest first, to show whether the estate is getting more " - "secure. Blank severity counts mean the run recorded no counts; blank resource counts mean " - "they were not recorded. New and resolved counts of 0 may also be unrecorded - Egeria cannot " - "tell the two apart.", + "secure. Blank severity counts mean the run recorded no counts; any other blank count was " + "not recorded. Runs written before Egeria 6.3 may show 0 for a count that was never set.", aliases=["Security Posture History"], family="Security Officer", formats=[ diff --git a/tests/micro-tests/test_create_bearer_token_errors.py b/tests/micro-tests/test_create_bearer_token_errors.py new file mode 100644 index 00000000..c08ed8a1 --- /dev/null +++ b/tests/micro-tests/test_create_bearer_token_errors.py @@ -0,0 +1,70 @@ +""" +create_egeria_bearer_token must raise when Egeria does not issue a token. A bad user id or +password gets a 401 whose body is error text; that text used to be returned, and installed in +the Authorization headers, as if it were the token. +""" +import httpx +import pytest + +import pyegeria.core._base_platform_client as platform_module +from pyegeria.core._base_platform_client import BasePlatformClient +from pyegeria.core._base_server_client import BaseServerClient +from pyegeria.core._exceptions import ( + PyegeriaClientException, PyegeriaConnectionException, PyegeriaInvalidParameterException, + PyegeriaUnauthorizedException, +) + +PLATFORM = "https://localhost:9443" + + +def _use_transport(monkeypatch, handler): + # 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(handler)) + + monkeypatch.setattr(platform_module, "AsyncClient", client) + + +def _clients(): + return [ + BaseServerClient("qs-view-server", PLATFORM, user_id="erinoverview", user_pwd="wrong"), + BasePlatformClient("qs-view-server", PLATFORM, user_id="erinoverview", user_pwd="wrong"), + ] + + +@pytest.mark.parametrize("status, body, expected", [ + (401, "Unauthorized: invalid user id or password", PyegeriaUnauthorizedException), + (403, "Forbidden", PyegeriaUnauthorizedException), + (500, "Internal Server Error", PyegeriaClientException), + (200, "", PyegeriaInvalidParameterException), + (200, '{"class": "VoidResponse", "relatedHTTPCode": 401}', PyegeriaInvalidParameterException), +]) +def test_no_token_raises(monkeypatch, status, body, expected): + _use_transport(monkeypatch, lambda request: httpx.Response(status, text=body)) + for client in _clients(): + before = dict(client.headers) + with pytest.raises(expected): + client.create_egeria_bearer_token() + assert client.headers == before + + +def test_connection_error_raises(monkeypatch): + def handler(request): + raise httpx.ConnectError("connection refused", request=request) + + _use_transport(monkeypatch, handler) + for client in _clients(): + with pytest.raises(PyegeriaConnectionException): + client.create_egeria_bearer_token() + + +def test_token_returned_and_installed(monkeypatch): + _use_transport(monkeypatch, lambda request: httpx.Response(200, text="eyJhbGciOiJIUzI1NiJ9.abc.def\n")) + for client in _clients(): + assert client.create_egeria_bearer_token() == "eyJhbGciOiJIUzI1NiJ9.abc.def" + assert client.headers["Authorization"] == "Bearer eyJhbGciOiJIUzI1NiJ9.abc.def" diff --git a/tests/micro-tests/test_dr_egeria_security_officer.py b/tests/micro-tests/test_dr_egeria_security_officer.py index 6c8cd8ff..96618b30 100644 --- a/tests/micro-tests/test_dr_egeria_security_officer.py +++ b/tests/micro-tests/test_dr_egeria_security_officer.py @@ -194,8 +194,9 @@ async def _async_update_annotation(self, guid, body): @pytest.mark.asyncio -async def test_merge_update_carries_primitives_forward(monkeypatch): - # Egeria resets an omitted primitive to 0 / false on a merge update (ISSUE-166). +async def test_merge_update_sends_only_what_the_command_sets(monkeypatch): + # From Egeria 6.3 (odpi/egeria#9389) a merge update keeps an omitted number or flag, so the + # current values are not copied into the update. monkeypatch.setattr("md_processing.v2.security_officer.update_element_dictionary", lambda *a, **k: None) client = _UpdateClient() proc = _finding_processor(client, {"Display Name": {"value": "SQL injection"}, @@ -206,9 +207,8 @@ async def test_merge_update_carries_primitives_forward(monkeypatch): await proc.apply_changes() props = client.updates[0][1]["properties"] - assert props["cvssScore"] == 8.6 and props["exploitAvailable"] is True assert props["reproductionSteps"] == "Search for ' OR 1=1" - assert "severity" not in props # non-primitives merge correctly on their own + assert not {"cvssScore", "exploitAvailable", "severity"} & set(props) @pytest.mark.asyncio diff --git a/tests/micro-tests/test_security_annotations.py b/tests/micro-tests/test_security_annotations.py index 9b972ff0..1183a6d4 100644 --- a/tests/micro-tests/test_security_annotations.py +++ b/tests/micro-tests/test_security_annotations.py @@ -2,8 +2,8 @@ Security annotation models and the DataDiscovery security finding / posture views (Egeria 6.3, model 0665). No live server: the annotation lists are stubbed, in the shape a live 6.3 platform returns. -Egeria stores cvssScore, exploitAvailable, patchAvailable and the posture counts as Java primitives, so an -unset value comes back as 0 / false (confirmed live). The display rows must not present those as answers. +From Egeria 6.3 (odpi/egeria#9389) an unset cvssScore, exploitAvailable, patchAvailable or posture count +is left out of the response (confirmed live 2026-10-27), and a 0 / false that comes back is a real answer. """ import pytest from pydantic import ValidationError @@ -32,8 +32,10 @@ def _annotation(type_name, guid, props, report=None): # An unset vulnerability, as returned live: only severity was supplied. -SPARSE = _annotation("VulnerabilityAnnotation", "sparse-1", - {"severity": "Medium", "cvssScore": 0.0, "exploitAvailable": False, "patchAvailable": False}) +SPARSE = _annotation("VulnerabilityAnnotation", "sparse-1", {"severity": "Medium"}) +# Assessed: no exploit, no patch, scored 0.0. +ASSESSED = _annotation("VulnerabilityAnnotation", "assessed-5", + {"severity": "Low", "cvssScore": 0.0, "exploitAvailable": False, "patchAvailable": False}) FULL = _annotation("VulnerabilityAnnotation", "full-2", {"severity": "Critical", "cvssScore": 10.0, "exploitAvailable": True, "packageURL": "pkg:maven/org.apache.logging.log4j/log4j-core@2.14.1"}) @@ -75,14 +77,19 @@ def test_annotation_properties_uses_egerias_confidence_field(): assert a.model_dump(by_alias=True, exclude_none=True)["confidence"] == 80 -def test_unrecorded_primitives_are_blank_not_zero_or_false(): +def test_unrecorded_values_are_blank(): row = DataDiscovery._security_row(SPARSE)["properties"] - assert row["cvssScore"] is None - assert row["exploitAvailable"] is None - assert row["patchAvailable"] is None + assert row.get("cvssScore") is None + assert row.get("exploitAvailable") is None + assert row.get("patchAvailable") is None assert row["findingType"] == "Vulnerability" +def test_returned_zero_and_false_are_answers(): + row = DataDiscovery._security_row(ASSESSED)["properties"] + assert (row["cvssScore"], row["exploitAvailable"], row["patchAvailable"]) == (0.0, False, False) + + def test_real_values_are_kept(): assert DataDiscovery._security_row(FULL)["properties"]["cvssScore"] == 10.0 assert DataDiscovery._security_row(FULL)["properties"]["exploitAvailable"] is True @@ -102,11 +109,10 @@ def test_posture_counts(): assert row["resourcesTestedCount"] == 0 # real: in scope was recorded assert row["reportName"] == "Run 1" - unrecorded = _annotation("SecurityPostureAnnotation", "p-2", - {"resourcesInScopeCount": 0, "resourcesTestedCount": 0}) + unrecorded = _annotation("SecurityPostureAnnotation", "p-2", {}) row = DataDiscovery._security_row(unrecorded)["properties"] assert row["highFindings"] is None - assert row["resourcesInScopeCount"] is None and row["resourcesTestedCount"] is None + assert row.get("resourcesInScopeCount") is None and row.get("resourcesTestedCount") is None def _client(monkeypatch, annotations): diff --git a/tests/micro-tests/test_upsert_drops_create_defaults.py b/tests/micro-tests/test_upsert_drops_create_defaults.py new file mode 100644 index 00000000..857c39ee --- /dev/null +++ b/tests/micro-tests/test_upsert_drops_create_defaults.py @@ -0,0 +1,38 @@ +""" +A Create that Dr.Egeria rewrites to Update (the element already exists) must not carry the +spec defaults the parser filled in for the Create. A merge update would send them over the +stored values -- e.g. isNullable: true over a stored false. Egeria 6.3 keeps an omitted number +or flag (odpi/egeria#9389), so dropping the defaults is what preserves the current value. +""" +from md_processing.v2.extraction import DrECommand +from md_processing.v2.processors import AsyncBaseCommandProcessor + + +class _Processor(AsyncBaseCommandProcessor): + async def apply_changes(self) -> str: + return "" + + +def _processor(attributes: dict) -> _Processor: + proc = _Processor(object(), DrECommand(verb="Create", object_type="Data Field", attributes={}, + raw_block="## Create Data Field\n")) + proc.parsed_output = {"attributes": attributes} + return proc + + +def test_defaults_dropped_but_user_values_and_control_flags_kept(): + attributes = { + "Display Name": {"value": "Customer Id"}, + "Is Nullable": {"value": True, "is_default": True}, + "Position": {"value": 0, "is_default": True}, + "Length": {"value": 12}, + "Merge Update": {"value": True, "is_default": True}, + } + _processor(attributes)._drop_create_defaults() + assert set(attributes) == {"Display Name", "Length", "Merge Update"} + + +def test_no_attributes_is_a_no_op(): + proc = _processor({}) + proc._drop_create_defaults() + assert proc.parsed_output["attributes"] == {}