Skip to content

feat: NET-3663 add endpoint owners/get#4787

Open
tpurschke wants to merge 20 commits into
CactuseSecurity:developfrom
tpurschke:feat/ownsers/get-endpoint
Open

feat: NET-3663 add endpoint owners/get#4787
tpurschke wants to merge 20 commits into
CactuseSecurity:developfrom
tpurschke:feat/ownsers/get-endpoint

Conversation

@tpurschke

@tpurschke tpurschke commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds the /api/owners/get middleware endpoint for issue #4778.

Changes

Owners endpoint

  • Adds a filtered owner GraphQL query (getOwnersFiltered) and wires it into OwnerQueries.
  • Adds OwnersController with role restriction to admin, auditor, and modeller.
  • Supports optional AND-combined filters for owner id, lifecycle state id, active flag, owner name, and external app id.
  • Supports wildcard matching for name and appIdExternal using * and ?.
  • Restricts modeller calls to owner ids from the x-hasura-editable-owners JWT claim.
  • Adds showDetails: when omitted/false only core fields are returned; when true the full owner detail set is returned (GetOwnerResponse).
  • Adds showOnlyActiveState: by default owners whose lifecycle state is inactive are filtered out (owners without a lifecycle state are kept); set to false to include inactive-state owners.
  • Owner type is derived from appIdExternal: standard when the external app id contains app (case-insensitive), infrastructure otherwise (including owners without an external app id).
  • Extends the ownerDetails fragment and FwoOwner with the owner lifecycle state (id, name, active_state).
  • Adds Swagger XML remarks with filtered request examples, response examples, and documented 401/403/500 responses.

Owner report (ReportOwners)

  • Adds the owner lifecycle state to the existing owner report output: two new columns ("Production state Id" / "Production state Name") in both the HTML and CSV exports. This surfaces the newly fetched lifecycle-state field in the existing report and is reflected in the updated ExportTest expectations.

Documentation (off topic)

  • consolidated the revision history documentation here as well (only one file instead of -develop and -main
  • removed entries <8.0

Tests

  • Adds OwnersControllerTest covering route/auth metadata, filter variables, wildcard conversion, modeller scoping, showDetails/showOnlyActiveState behavior, owner-type classification, and response type mapping.
  • Updates ExportTest and SimulatedUserConfig for the new owner lifecycle state fields.

@tpurschke tpurschke self-assigned this Jun 19, 2026
@tpurschke tpurschke marked this pull request as ready for review June 21, 2026 12:42
@tpurschke tpurschke requested a review from Elutrixx June 21, 2026 13:26
@tpurschke tpurschke changed the title feat: add owners get endpoint feat: add endpoint owners/get Jun 21, 2026
tpurschke and others added 6 commits June 21, 2026 20:17
IsStandardOwner used a Contains("app") substring check, so an external
app id like "MAPP-1" was wrongly classified as a standard owner. Use a
StartsWith("APP") prefix check to match the modelling naming convention
(ModellingManagedIdString), and add a regression test case.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Revert the IsStandardOwner logic change and instead document the
existing behaviour: owner type is derived from appIdExternal, "standard"
when it contains "app" (case-insensitive), "infrastructure" otherwise.
Documented in the endpoint Swagger remarks and the response model.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@tpurschke tpurschke changed the title feat: add endpoint owners/get feat: NET-3663 add endpoint owners/get Jun 22, 2026

@Elutrixx Elutrixx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personal Feedback: In my head this is missing a validation layer before data given by the third party is used. This was a problem with my first GetRulesByFilter endpoint that I tried to fix in my newer set of endpoints and has lead to several "bugs" that were simply uncaught validation problems. It CAN be ignored but its debatable wheter it should.

Relevant Feedback
Medium: the name/appId filters do not match the documented wildcard contract. In OwnersController.cs:229-234, BuildLikePattern() converts * and ? but leaves raw % and _ untouched, so a plain search term such as APP_1 behaves like a broader SQL pattern instead of a literal contains search. The controller remarks say only * and ? are wildcards, so % and _ need escaping before _ilike is sent.

Open Questions:
Was this tested on the customer machine? The SQL patterns seem rather intricate to me so it might be useful to check if they work with varying infrastructure as well

@sonarqubecloud

Copy link
Copy Markdown

@tpurschke

Copy link
Copy Markdown
Contributor Author

Personal Feedback: In my head this is missing a validation layer before data given by the third party is used. This was a problem with my first GetRulesByFilter endpoint that I tried to fix in my newer set of endpoints and has lead to several "bugs" that were simply uncaught validation problems. It CAN be ignored but its debatable wheter it should.

Relevant Feedback Medium: the name/appId filters do not match the documented wildcard contract. In OwnersController.cs:229-234, BuildLikePattern() converts * and ? but leaves raw % and _ untouched, so a plain search term such as APP_1 behaves like a broader SQL pattern instead of a literal contains search. The controller remarks say only * and ? are wildcards, so % and _ need escaping before _ilike is sent.

Open Questions: Was this tested on the customer machine? The SQL patterns seem rather intricate to me so it might be useful to check if they work with varying infrastructure as well

  • included your suggestions regarding validation
  • escaped wildcards we do not support
  • tested succesfully with K001 data

@tpurschke tpurschke requested a review from Elutrixx June 23, 2026 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants