Feature/dpav 3017 - #67
Open
Filip-sz-informed wants to merge 21 commits into
Open
Conversation
Contributor
✅ OSS Checks PassedAll tracked OSS checks passed in this run.
Results from commit ecc17fc, view the full job summary ♻️ This comment has been updated with latest results. |
Adds proposal, spec, design and task breakdown for intercepting policy-aware Management Node APIs, enriching requests with policy attributes, and enforcing PDP (OPA) allow/deny decisions.
Introduces application.opa.* configuration (url, decision path, connect/read timeouts) backed by OpaProperties, with local defaults in application.yml.
Adds PolicyInput, PolicyDecisionRequest and PolicyDecisionResponse
matching OPA's standard {"input": {...}} / {"result": ...} REST
decision API shape.
Configures a Spring RestClient bean from OpaProperties (base URL, connect/read timeouts) for use by the policy decision client.
Calls OPA with a PolicyDecisionRequest and maps the response to ALLOW/DENY, treating non-2xx responses, malformed bodies, timeouts and connection failures as DENY so enforcement fails closed.
Intercepts policy-aware requests after authentication, builds a PolicyInput from the authenticated client and the request, and enforces the PDP decision: proceed on ALLOW, 403 on DENY (mirrors CertificateValidationInterceptor's rejection response format).
Logs each policy decision (INFO on ALLOW, WARN on DENY) with client id, resource, action and a correlation id; the same correlation id is returned in the 403 error body on DENY for troubleshooting.
… APIs Wires the PEP into WebConfig for /api/v1/configuration/** only, after CertificateValidationInterceptor. Certificate endpoints remain on cert-based checks only.
…rcement Wires PolicyEnforcementInterceptor and ConfigurationController together via MockMvc, covering: allowed requests reach the controller, denied requests get 403 without reaching it, and unauthenticated requests are rejected without invoking the PDP.
Full suite (272 tests) passes with no regressions; openspec change validated with --strict.
- Extract RequestRejectionSupport shared between CertificateValidationInterceptor and PolicyEnforcementInterceptor, removing duplicated extractClientId/writeError logic. - Make PEP protected paths configurable via application.opa.protected-paths instead of hardcoded in WebConfig, matching design.md's stated goal. - Simplify a nested ternary in PolicyDecisionClient. - Parameterize repetitive "missing client id" interceptor tests and remove a redundant config-wiring assertion.
Apply spotless:apply to fix lint violations in PEP files. Pin httpcore5/httpcore5-h2 to 5.4.3 to resolve CVE-2026-54399 and CVE-2026-54428 (HIGH DoS) pulled in transitively via spring-cloud-starter-vault-config.
openspec/ holds local change-tracking artifacts, not meant to be shared via remote. Untrack and ignore going forward; files remain on disk.
OpaProperties.protectedPaths was dereferenced unchecked in WebConfig.addInterceptors, so a missing application.opa.protected-paths key threw an NPE at startup instead of a clear config error. Validate it as @notempty so binding fails fast with a descriptive message.
Spring's MappedInterceptor treats an empty include-pattern list as "match every path", not "match nothing". An empty application.opa.protected-paths (e.g. meant to disable the PEP) was silently registering PolicyEnforcementInterceptor against every /api/** route instead. Only register it when the list is non-empty.
This service enforces mTLS for all service-to-service traffic, but the OPA RestClient bean had no TLS signal and OPA_URL defaults to http://localhost:8181. SslPropertyInitializer already sets the JVM-wide client keystore/truststore that any https:// call picks up automatically, so no new SSLContext wiring is needed - just make a non-TLS endpoint loudly visible at startup instead of silent.
PolicyInput.organisation was hard-coded null even though the interceptor's own Javadoc says it enriches requests with identity and resource attributes. CertificateValidationInterceptor already resolves the organisation for the calling client and always runs first, so stash its organisationId as a request attribute and have PolicyEnforcementInterceptor read it back instead of re-querying.
PolicyDecisionClientTest called a plain private setUp() as the first line of every @test, duplicated 8 times, instead of relying on JUnit's lifecycle. No behaviour change.
security-scanning (Trivy) failed the PR: libcrypto3/libssl3/openssl 3.5.7-r0 in the eclipse-temurin:21-jdk-alpine base image carry CVE-2026-14456 (HIGH, OpenSSL DoS via unbounded memory growth in QUIC server), fixed in 3.5.8-r0. apk upgrade before installing curl pulls the patched packages - verified locally by building the image and checking apk info -v.
Filip-sz-informed
force-pushed
the
feature/DPAV-3017
branch
from
August 27, 2026 08:09
7bfbe29 to
123dcfe
Compare
Sonar flagged the greedy backtracking regex used to pull correlationId out of the log message; replaced with plain substring/indexOf lookup.
Sonar flagged two separate assertThat(json) calls; merged into one fluent assertion chain.
|
| RUN chown app:app /app/app.jar | ||
|
|
||
| RUN apk add --no-cache curl | ||
| RUN apk update && apk upgrade --no-cache && apk add --no-cache curl |
Author
There was a problem hiding this comment.
Add cause of security check failure
| <type>pom</type> | ||
| <scope>import</scope> | ||
| </dependency> | ||
| <dependency> |
Author
There was a problem hiding this comment.
Need to bump version because of security
nikan-negaresh-informed
approved these changes
Aug 27, 2026
nikan-negaresh-informed
left a comment
nikan-negaresh-informed
left a comment
Contributor
There was a problem hiding this comment.
All looking good
Thanks
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
2 participants
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-3017: Management Node authenticates and certificate-validates requests but has no mechanism to enforce fine-grained, attribute-based access policy before a request reaches its handler. This adds a Policy Enforcement Point (PEP) that intercepts policy-aware API requests, enriches them with identity/organisation/resource attributes, and delegates the allow/deny decision to an external Policy Decision Point (OPA), so access control decisions are made consistently and can evolve independently of application code.
Description
PolicyEnforcementInterceptor, aHandlerInterceptorthat runs after authentication/certificate validation and before the handler, readingclientId/organisation fromEnhancedPrincipal.PolicyDecisionClient+OpaClientConfig(SpringRestClient) that build a decision request (identity, organisation, resource, action) and call OPA's REST API (POST {opa.url}{opa.decision-path},{"input": {...}}/{"result": ...}contract).RequestRejectionSupport) on DENY.WebConfigfor/api/v1/configuration/**only (producer/consumer configuration lookups), ordered afterCertificateValidationInterceptor;/api/v1/certificate/**is unaffected.application.opa.*configuration properties (url,decision-path,connect-timeout,read-timeout,protected-paths) viaOpaProperties, with local defaults inapplication.yml; fail fast ifprotected-pathsis missing, and warn if the configured OPA endpoint isn't using TLS.docker/DockerfileAlpine OS packages to resolve an openssl HIGH CVE, and a smallpom.xmldependency update to clear a CI-flagged CVE.How Has This Been Tested?
OpaPropertiesTest,OpaClientConfigTest,PolicyDecisionSerializationTest,PolicyDecisionClientTest,PolicyEnforcementInterceptorTest,WebConfigTest(interceptor registration/path patterns).ConfigurationPolicyEnforcementIntegrationTest(@SpringBootTest/MockMvc) covering authenticated+ALLOW, authenticated+DENY (403), and unauthenticated (rejected before any PDP call) against/api/v1/configuration/producerand/api/v1/configuration/consumer.mvn clean verify.Screenshots (if appropriate):
Checklist: