Feature/DPAV 3018 - #68
Open
Filip-sz-informed wants to merge 8 commits into
Open
Conversation
Contributor
✅ OSS Checks PassedAll tracked OSS checks passed in this run.
Results from commit d7141d9, view the full job summary ♻️ This comment has been updated with latest results. |
Filip-sz-informed
changed the title
@Valid request-body validation failures and malformed JSON bodies were falling through to GlobalExceptionHandler's generic Exception handler, which returns 500. Add dedicated MethodArgumentNotValidException and HttpMessageNotReadableException handlers so both cases return 400, as already relied on by CertificateController's bootstrap endpoint and the new product discovery endpoint.
Widen RequestRejectionSupport (and getOrganisationId) to public so the new product discovery controller, in a different package, can read the organisation CertificateValidationInterceptor already resolved for the request instead of duplicating the request-attribute lookup.
Implements DPAV-3018: POST /api/v1/product/discovery returns only the products the authenticated requester is authorised to see. Search criteria (name/topic/type, all optional) narrow the org-unscoped candidate query, then ProductDiscoveryService evaluates one PDP decision per candidate via the existing PolicyDecisionClient (built for DPAV-3017's PEP), keeping only ALLOWed products - a PDP denial or failure excludes just that candidate rather than the whole request. - ProductRepository.findDiscoveryCandidates: bounded, filtered candidate query across all organisations (policy decides visibility, not org membership) - ProductDiscoveryService: queries candidates then filters by policy; new `discover_products` role gates the endpoint - docker/opa/policy.rego: starting example for a discover-action rule - application.yml: application.product-discovery.max-candidates bounds the per-request PDP call count
Filip-sz-informed
force-pushed
the
feature/DPAV-3018
branch
from
August 31, 2026 14:19
ae22187 to
8135d4c
Compare
noAuthorisedProducts_returnsEmptyListNotError was byte-identical to noCandidates_returnsEmptyListNotError (Sonar). Give it distinct value: send search criteria and verify they're passed through to ProductDiscoveryService.discover unchanged, instead of repeating the same empty-body/empty-response assertion.
evaluate() takes a single argument, so wrapping it in eq(...) is a no-op Mockito already does by default (Sonar). Pass the PolicyInput value directly.
The MethodArgumentNotValidException/HttpMessageNotReadableException handlers logged request.getContextPath() (empty for a root-mapped app) instead of request.getDescription(false), unlike every sibling handler in this class - leaving debug logs with no indication of which endpoint a validation failure came from.
application.product-discovery.max-candidates fed straight into PageRequest.of(0, n), which throws IllegalArgumentException for n < 1 (e.g. a misconfigured 0, or an attempt to mean "unlimited") - crashing every discovery request with a 500 instead of failing at startup. Validate in the constructor, matching the sibling OpaProperties' fail-fast intent.
findDiscoveryCandidates built its %contains% pattern via CONCAT without escaping the SQL LIKE metacharacters % and _ in the caller-supplied value, so e.g. name=Data_Feed also matched DataXFeed (since '_' is the LIKE single-char wildcard) - silently violating the documented "contains" filter contract for any name/topic containing % or _.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sensitive Credential Checks
Motivation and Context
DPAV-3018: consumers currently have no API to discover which data products they are authorised to see. DPAV-3017 delivered a generic Policy Enforcement Point (PEP) that enforces a single allow/deny decision per request, but nothing yet applies policy to a set of resources or exposes product search. This adds a discovery endpoint that returns only the products the requester is authorised to see, per the applicable PDP policy, without leaking existence or metadata of restricted products.
Description
POST /api/v1/product/discovery: accepts requester-supplied search criteria (name/topic/type, all optional), returns only products the requester is authorised to discover, gated by a newdiscover_productsclient role.ProductRepository.findDiscoveryCandidates+ProductService/ProductServiceImpl) — policy, not org membership, decides visibility, so candidates aren't pre-filtered by the requester's own organisation;application.product-discovery.max-candidates(default 200) bounds the per-request PDP call count.ProductDiscoveryService, which queries candidates then evaluates one PDP decision per candidate via the existingPolicyDecisionClient/PolicyInput(reused unmodified from DPAV-3017), keeping only ALLOWed products.resource/actionare repurposed asproduct:{id}/discoverrather than URI/HTTP-method —PolicyInput's javadoc was generalised to describe them as caller-defined, opaque strings.200with an empty list, not an error.PolicyEnforcementInterceptor— that interceptor makes one whole-request decision and is registered only for/api/v1/configuration/**; discovery needs N per-candidate decisions, soProductDiscoveryServicecallsPolicyDecisionClientdirectly.GlobalExceptionHandler's genericExceptionhandler was silently turning@Validrequest-body failures into a 500 instead of 400 (also affectsCertificateController's bootstrap endpoint) — added dedicatedMethodArgumentNotValidException/HttpMessageNotReadableExceptionhandlers.RequestRejectionSupport.getOrganisationIdto public so the new controller can reuse the organisation lookupCertificateValidationInterceptoralready resolved, instead of duplicating it.discover-action rule todocker/opa/policy.regofor local/dev PDP testing; document the newdiscover_productsrole indocs/AUTHENTICATION_REQUIREMENTS.md.How Has This Been Tested?
ProductDiscoveryDtoTest(request/response DTO validation and serialization),ProductDiscoveryServiceImplTest(fully permitted / partially permitted / none-authorised PDP filtering,PolicyInputshape, candidate-query-then-filter orchestration),ProductServiceImplTestadditions (candidate-query filter/limit delegation).ProductDiscoveryControllerTest(MockMvc, followingConfigurationPolicyEnforcementIntegrationTest's pattern) covering fully permitted, partially filtered, no candidates, no authorised products, a search filter matching a PDP-denied product (still excluded — filters can't widen authorisation), invalid body (400), malformed JSON (400), and empty body treated as no filter.mvn clean verify(297 tests, JaCoCo coverage gate met, spotless-clean).@DataJpaTest/real-DB repository test exists in this project (Flyway/ddl-auto are disabled in the test datasource, and enabling Flyway against H2 fails on a pre-existing, unrelated migration issue — an unquotedvaluecolumn, an H2 reserved word) — the candidate query is verified via the service-layer unit test plus code review instead. Manual verification of the OPA rego rule against a running local OPA instance is left as a local/dev follow-up (no OPA/Docker available in the environment this was built in).Screenshots (if appropriate):
Checklist:
🤖 Generated with Claude Code
https://claude.ai/code/session_01DpPNCW5LYqyGEbsq8j3F5S