feat(core)!: ingestion security processor - #123
Conversation
…object and array payloads Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: ferial OUKOUKAS <75682459+foukou19@users.noreply.github.com>
Code Coverage OverviewLanguages: Java Java / code-coverage/jacocoThe overall line coverage in commit e05ce0f in the Show a line coverage summary of the most impacted files.
Updated |
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
…error responses Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
3b6ec83 to
d276814
Compare
…error responses Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Signed-off-by: ferial OUKOUKAS <75682459+foukou19@users.noreply.github.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
59523a8 to
1e9f829
Compare
ef960c1 to
346711c
Compare
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
143b0a3 to
c06eb85
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.
Suppressed comments (8)
src/main/java/com/decathlon/idp_core/domain/port/WebhookSecurityStrategy.java:41
- The Domain port now defines an HTTP-header contract and names Infrastructure exceptions/status semantics. This reverses the required dependency direction: the Domain layer must not know about HTTP or Infrastructure. Keep creation-time domain validation on a domain port, and move runtime request authentication to an Infrastructure-owned strategy contract with adapter exceptions mapped by the ingestion route.
/// Validates an incoming webhook request at runtime.
///
/// @param headers the inbound HTTP headers
/// @param rawPayload the exact inbound payload bytes (before decoding)
/// @param config the persisted security configuration
/// @throws
/// com.decathlon.idp_core.infrastructure.adapters.ingestion.exception.WebhookAuthUnauthorizedException
/// when authentication is missing or malformed (401)
/// @throws
/// com.decathlon.idp_core.infrastructure.adapters.ingestion.exception.WebhookAuthForbiddenException
/// when authentication is provided but rejected (403)
void validateRequest(Map<String, Object> headers, byte[] rawPayload, Map<String, String> config);
src/main/java/com/decathlon/idp_core/infrastructure/adapters/webhook/security/JwtBearerSecurityValidator.java:146
expectedAudienceis silently ignored, although the security configuration contract supports camelCase variants. A caller using that spelling gets no audience validation at all. Resolve both spellings as is already done for the other JWT keys.
private String resolveOptionalExpectedAudience(Map<String, String> config) {
return config.get(KEY_EXPECTED_AUDIENCE_SNAKE_CASE);
src/main/java/com/decathlon/idp_core/infrastructure/adapters/webhook/security/JwtBearerSecurityValidator.java:130
- Audience validation is skipped when
expected_audienceis absent, and the provider'sJwtValidators.createDefault()only performs default timestamp-style validation. This accepts a valid token issued for a different recipient whenever its identity claim is allow-listed, enabling cross-service token replay. Require/derive the webhook audience and always validateaud.
if (StringUtils.hasText(optionalExpectedAudience)) {
validateAudienceClaim(jwt, optionalExpectedAudience);
}
src/main/java/com/decathlon/idp_core/infrastructure/adapters/webhook/security/BasicAuthSecurityValidator.java:54
- HTTP authentication scheme names are case-insensitive, but this rejects valid
basic/mixed-case schemes. Use a case-insensitive prefix comparison while retaining the current credential extraction.
if (!authorization.startsWith("Basic ")) {
src/main/java/com/decathlon/idp_core/infrastructure/adapters/webhook/security/JwtBearerSecurityValidator.java:109
- HTTP authentication scheme names are case-insensitive, but this rejects otherwise valid
bearer/mixed-case authorization headers. Compare the scheme case-insensitively.
if (!authorization.startsWith(BEARER_PREFIX)
|| authorization.substring(BEARER_PREFIX.length()).isBlank()) {
src/main/java/com/decathlon/idp_core/infrastructure/adapters/webhook/security/JwtBearerSecurityValidator.java:65
- These new mandatory JWT keys are absent from the public webhook documentation:
docs/src/concepts/webhooks.md:113-119still says JWT requires onlyjwks_uri, and its example at lines 165-174 now produces a 400 response. Update the documentation and example withclient_id_field,client_id_values, and the audience contract.
String clientIdValues = WebhookSecurityConfigurationUtils.required(config,
KEY_CLIENT_ID_VALUES_SNAKE_CASE, KEY_CLIENT_ID_VALUES_CAMEL_CASE);
src/main/java/com/decathlon/idp_core/infrastructure/adapters/ingestion/exception_handler/WebhookErrorCode.java:18
- This error is also used for unsupported encodings and decompression-size violations, so reporting every failure as “invalid or corrupted” is inaccurate and hides the actionable cause. Preserve a description covering invalid, unsupported, and oversized payloads (or safely expose each
WebhookDecodingExceptionmessage).
INVALID_COMPRESSED_PAYLOAD("invalid_compressed_payload", HttpStatus.BAD_REQUEST,
LoggingLevel.WARN, "Invalid or corrupted compressed payload"),
src/test/java/com/decathlon/idp_core/infrastructure/adapters/ingestion/InboundWebhookIngestionRouteTest.java:360
- This test can pass on an authentication failure: Camel's handled exception route clears
exchange.getException()and instead writes a 401 response. Moreover, the production provider constructs its own Nimbus decoder (WebhookJwtDecoderProvider.java:17-23), so the test's primaryJwtDecodermock is unused and this fake token causes a real request toauth.example.com. Mock the provider/decoder and assert that no authentication error response was produced.
Exchange exchange = invokeValidateSecurityRoute(connector,
Map.of("Authorization", "Bearer " + token));
assertNull(exchange.getException());
| String jwksUriValue = WebhookSecurityConfigurationUtils.required(config, | ||
| KEY_JWKS_URI_SNAKE_CASE, KEY_JWKS_URI_CAMEL_CASE); | ||
| if (jwksUriValue.isBlank()) { | ||
| throw new WebhookSecurityConfigurationException("Invalid jwks_uri for JWT_BEARER security"); |
c06eb85 to
55ca6d9
Compare
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
55ca6d9 to
9ef681a
Compare
Signed-off-by: ferial OUKOUKAS <75682459+foukou19@users.noreply.github.com>
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com> Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
0ef78b4 to
1e0c83b
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Raw-body handling, JWT SSRF enforcement, architecture boundaries, and false-positive tests contain unresolved correctness and security issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/main/java/com/decathlon/idp_core/domain/port/WebhookSecurityStrategy.java:39
- This domain port now defines its contract in terms of Infrastructure exceptions and HTTP status semantics, reversing the required dependency direction. Define transport-neutral domain authentication exceptions/results in the Domain layer, then map them to 401/403 exceptions exclusively in the ingestion adapter.
/// @throws
/// com.decathlon.idp_core.infrastructure.adapters.ingestion.exception.WebhookAuthUnauthorizedException
/// when authentication is missing or malformed (401)
/// @throws
/// com.decathlon.idp_core.infrastructure.adapters.ingestion.exception.WebhookAuthForbiddenException
- Files reviewed: 27/28 changed files
- Comments generated: 9
- Review effort level: Balanced
| return switch (payload) { | ||
| case byte[] bytes -> bytes; | ||
| case String string -> string.getBytes(StandardCharsets.UTF_8); | ||
| default -> payload.toString().getBytes(StandardCharsets.UTF_8); | ||
| }; |
| private Jwt decodeAndValidateJwt(String token, String jwksUri) { | ||
| try { | ||
| return jwtDecoderProvider.get(jwksUri).decode(token); |
| String authorization = WebhookSecurityConfigurationUtils.requiredHeader(headers, | ||
| "Authorization"); | ||
|
|
||
| if (!authorization.startsWith("Basic ")) { |
| if (!authorization.startsWith(BEARER_PREFIX) | ||
| || authorization.substring(BEARER_PREFIX.length()).isBlank()) { |
| Exchange exchange = invokeValidateSecurityRoute(connector, | ||
| Map.of("Authorization", "Basic " + credentials)); | ||
|
|
||
| assertNull(exchange.getException()); |
| Exchange exchange = invokeValidateSecurityRoute(connector, | ||
| Map.of("Authorization", "Bearer " + token)); | ||
|
|
||
| assertNull(exchange.getException()); |
| @DisplayName("Should return 400 when secret_alias references an env variable that does not exist") | ||
| void postWebhook_400_secret_alias_env_var_not_set() throws Exception { |
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com> Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
1e0c83b to
e05ce0f
Compare
|



PR Description
What this PR Provides
NONE,BASIC_AUTH,HMAC_SHA256,STATIC_TOKEN, andJWT_BEARERstrategiesMY_VAR,${MY_VAR},env:MY_VAR,<KEY>_env: MY_VAR,<KEY>Env: MY_VAR) without storing sensitive values in the connector configurationjwks_urivalues, rejecting env references, and blocking private/loopback/internal hosts to reduce SSRF riskReview
The reviewer must double-check these points:
!after the type/scope to identify the breakingchange in the release note and ensure we will release a major version.
How to test
Initial state
MY_WEBHOOK_SECRETMY_TOKENTOKEN_SECRETBASIC_PASSBASIC_SECRETMY_SECRETWhat to test
create a connector with
security.type = NONEand no configPOST /api/v1/inbound_webhooks201security.type = "NONE"create a connector with
security.type = BASIC_AUTHusernameand a runtime secret alias such asBASIC_SECRETAuthorization: Basic ...header using the matching username/password201for the creation and2xxfor a successful runtime validationcreate a connector with
security.type = STATIC_TOKENheader_nameandsecret_aliascreate a connector with
security.type = HMAC_SHA256header_nameandsecret_alias401/403for invalid or missing valuescreate a connector with
security.type = JWT_BEARERjwks_uri,client_id_field, andclient_id_valuesAuthorization: Bearer <jwt>header with a valid token signed by the configured JWKS provider401when the token is malformed or missing and403when the claims are valid but rejectedtest runtime environment resolution
MY_SECRET,${MY_SECRET},env:MY_SECRET,MY_SECRET_env: MY_SECRET, orMY_SECRETEnv: MY_SECRETwhere supportedtest fail-fast creation rules
my-secretinstead ofMY_SECRET400 Bad Request400 Bad Requesttest SSRF protection on JWT bearer configuration
jwks_urisuch ashttps://localhost/...or an internal hostname400Expected results
401403404/403depending on the business path)400401for authentication failure,403for forbidden credentials,400for bad config, and422for ingestion mapping errors when relevantBreaking changes (if any)
Context of the Breaking Change
No breaking API contract changes are introduced. The connector security model remains compatible with the existing webhook configuration format, but the validation contract is tightened for runtime secrets and JWT URL safety.
Result of the Breaking Change
Notes on implementation
The implementation keeps the strategy-specific logic explicit for readability and operational traceability:
BasicAuthSecurityValidatorandStaticTokenSecurityValidatorshare the same runtime secret resolution model, but they remain separate because their request semantics and validation messages are differentHmacSha256SecurityValidatorvalidates the raw payload exactly as received and compares the computed signature in constant timeJwtBearerSecurityValidatorhandles the JWKS fetch path and JWT claim validation while enforcing SSRF-safe configuration constraintsWebhookExceptionRouteBuilderandWebhookExceptionHandlerHelper, so the HTTP contract remains consistent regardless of the strategy used