Skip to content

feat(core)!: ingestion security processor - #123

Draft
foukou19 wants to merge 47 commits into
mainfrom
feat/ingestion-security-processor
Draft

feat(core)!: ingestion security processor#123
foukou19 wants to merge 47 commits into
mainfrom
feat/ingestion-security-processor

Conversation

@foukou19

@foukou19 foukou19 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

PR Description

What this PR Provides

  • add a pluggable inbound webhook security layer with NONE, BASIC_AUTH, HMAC_SHA256, STATIC_TOKEN, and JWT_BEARER strategies
  • resolve runtime secrets from environment variables using a consistent contract (MY_VAR, ${MY_VAR}, env:MY_VAR, <KEY>_env: MY_VAR, <KEY>Env: MY_VAR) without storing sensitive values in the connector configuration
  • enforce fail-fast validation when a connector is created or updated so invalid security configuration is rejected before runtime processing
  • standardize webhook error handling for authentication failures, forbidden credentials, disabled connectors, missing configuration, and ingestion errors through a centralized route exception mapping
  • harden JWT validation by requiring HTTPS jwks_uri values, rejecting env references, and blocking private/loopback/internal hosts to reduce SSRF risk
  • ensure runtime comparisons for HMAC and Basic Auth use constant-time checks to avoid timing-based credential leakage
  • make HTTP header lookup robust across casing while keeping secret material out of logs and error responses
  • keep the security strategy implementations aligned with the hexagonal architecture: domain port + infrastructure validators + centralized route mapping

Review

The reviewer must double-check these points:

  • The reviewer has tested the feature
  • The reviewer has reviewed the implementation of the feature
  • The documentation has been updated
  • The feature implementation respects the Technical Doc / ADR previously produced
  • The Pull Request title has a ! after the type/scope to identify the breaking
    change in the release note and ensure we will release a major version.

How to test

Initial state

  • ensure the application is running with the expected environment variables set for the webhook security tests
  • ensure the following secret aliases exist in the runtime environment for the validation steps below:
    • MY_WEBHOOK_SECRET
    • MY_TOKEN
    • TOKEN_SECRET
    • BASIC_PASS
    • BASIC_SECRET
    • MY_SECRET
  • ensure the test data is loaded for inbound webhook connectors and mappings, including the webhook connectors used in security validation tests

What to test

  1. create a connector with security.type = NONE and no config

    • call POST /api/v1/inbound_webhooks
    • expect status 201
    • expect the response to contain security.type = "NONE"
  2. create a connector with security.type = BASIC_AUTH

    • send username and a runtime secret alias such as BASIC_SECRET
    • call the webhook endpoint with an Authorization: Basic ... header using the matching username/password
    • expect status 201 for the creation and 2xx for a successful runtime validation
  3. create a connector with security.type = STATIC_TOKEN

    • configure header_name and secret_alias
    • send the matching header value from the environment variable
    • expect the request to be accepted and processed
  4. create a connector with security.type = HMAC_SHA256

    • configure header_name and secret_alias
    • compute the HMAC over the raw request body using the configured prefix and secret
    • send the signature in the custom header
    • expect success for a valid signature and 401/403 for invalid or missing values
  5. create a connector with security.type = JWT_BEARER

    • configure jwks_uri, client_id_field, and client_id_values
    • send an Authorization: Bearer <jwt> header with a valid token signed by the configured JWKS provider
    • expect success when the token is valid and the configured claim matches the allow-list
    • expect 401 when the token is malformed or missing and 403 when the claims are valid but rejected
  6. test runtime environment resolution

    • use values such as MY_SECRET, ${MY_SECRET}, env:MY_SECRET, MY_SECRET_env: MY_SECRET, or MY_SECRETEnv: MY_SECRET where supported
    • confirm the value is resolved at runtime from the environment and not from a persisted clear-text secret
  7. test fail-fast creation rules

    • create a connector with an invalid alias format such as my-secret instead of MY_SECRET
    • expect 400 Bad Request
    • create a connector with a blank or missing required config key
    • expect 400 Bad Request
  8. test SSRF protection on JWT bearer configuration

    • use a private or loopback jwks_uri such as https://localhost/... or an internal hostname
    • expect creation to fail with 400
    • use a public HTTPS JWKS endpoint
    • expect creation to succeed

Expected results

  • the connector is created successfully when the configuration matches the expected contract
  • secrets are not stored in cleartext and are resolved from the runtime environment
  • missing or invalid authentication headers trigger 401
  • wrong credentials or rejected tokens trigger 403
  • disabled or absent connectors are rejected before security validation (404 / 403 depending on the business path)
  • malformed compressed payloads or invalid request encoding return a controlled 400
  • invalid JWT JWKS URIs are rejected during creation to prevent SSRF and internal egress
  • the HTTP responses keep a standard webhook contract: 401 for authentication failure, 403 for forbidden credentials, 400 for bad config, and 422 for ingestion mapping errors when relevant

Breaking changes (if any)

  • N/A

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

  • invalid runtime environment aliases are rejected earlier at creation time
  • the configuration is hardened against private or internal JWKS endpoints
  • webhook requests now fail with a more consistent security contract across strategies

Notes on implementation

The implementation keeps the strategy-specific logic explicit for readability and operational traceability:

  • BasicAuthSecurityValidator and StaticTokenSecurityValidator share the same runtime secret resolution model, but they remain separate because their request semantics and validation messages are different
  • HmacSha256SecurityValidator validates the raw payload exactly as received and compares the computed signature in constant time
  • JwtBearerSecurityValidator handles the JWKS fetch path and JWT claim validation while enforcing SSRF-safe configuration constraints
  • the route-level exception mapping is centralized in WebhookExceptionRouteBuilder and WebhookExceptionHandlerHelper, so the HTTP contract remains consistent regardless of the strategy used

foukou19 and others added 20 commits July 21, 2026 13:24
…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>
@foukou19
foukou19 marked this pull request as draft August 12, 2026 14:12
Signed-off-by: ferial OUKOUKAS <75682459+foukou19@users.noreply.github.com>
@github-code-quality

github-code-quality Bot commented Aug 12, 2026

Copy link
Copy Markdown

Code Coverage Overview

Languages: Java

Java / code-coverage/jacoco

The overall line coverage in commit e05ce0f in the feat/ingestion-secur... branch remains at 91%, unchanged from commit 6976334 in the main branch.

Show a line coverage summary of the most impacted files.
File main 6976334 feat/ingestion-secur... e05ce0f +/-
com/decathlon/i...yValidator.java 100% 78% -22%
com/decathlon/i...yValidator.java 100% 94% -6%
com/decathlon/i...uteBuilder.java 100% 96% -4%
com/decathlon/i...ityService.java 97% 98% +1%
com/decathlon/i...ationUtils.java 82% 86% +4%
com/decathlon/i...dlerHelper.java 90% 95% +5%
com/decathlon/i...yProcessor.java 67% 86% +19%
com/decathlon/i...erProvider.java 33% 100% +67%
com/decathlon/i...nException.java 0% 100% +100%
com/decathlon/i...dException.java 0% 100% +100%

Updated September 03, 2026 09:56 UTC

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>
@foukou19
foukou19 force-pushed the feat/ingestion-security-processor branch from 3b6ec83 to d276814 Compare August 13, 2026 14:01
@foukou19
foukou19 requested a balanced review from Copilot August 14, 2026 14:54
foukou19 and others added 3 commits August 17, 2026 11:33
…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>
@foukou19 foukou19 changed the title Feat/ingestion security processor feat(core)!: ingestion security processor Aug 17, 2026
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
@foukou19
foukou19 force-pushed the feat/ingestion-security-processor branch from 59523a8 to 1e9f829 Compare August 19, 2026 13:33
@foukou19
foukou19 force-pushed the feat/ingestion-security-processor branch 4 times, most recently from ef960c1 to 346711c Compare August 21, 2026 13:11
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
@foukou19
foukou19 force-pushed the feat/ingestion-security-processor branch 2 times, most recently from 143b0a3 to c06eb85 Compare August 21, 2026 14:09
@foukou19
foukou19 requested a balanced review from Copilot August 21, 2026 14:37

Copilot AI 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.

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

  • expectedAudience is 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_audience is absent, and the provider's JwtValidators.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 validate aud.
    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-119 still says JWT requires only jwks_uri, and its example at lines 165-174 now produces a 400 response. Update the documentation and example with client_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 WebhookDecodingException message).
  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 primary JwtDecoder mock is unused and this fake token causes a real request to auth.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());

Comment on lines +47 to 50
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");
@foukou19
foukou19 force-pushed the feat/ingestion-security-processor branch from c06eb85 to 55ca6d9 Compare August 21, 2026 15:43
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
@foukou19
foukou19 force-pushed the feat/ingestion-security-processor branch from 55ca6d9 to 9ef681a Compare August 25, 2026 07:57
foukou19 and others added 2 commits September 2, 2026 10:41
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>
@foukou19
foukou19 force-pushed the feat/ingestion-security-processor branch 2 times, most recently from 0ef78b4 to 1e0c83b Compare September 3, 2026 07:28
@foukou19
foukou19 requested a balanced review from Copilot September 3, 2026 08:41

Copilot AI 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.

🟡 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

Comment on lines +59 to +63
return switch (payload) {
case byte[] bytes -> bytes;
case String string -> string.getBytes(StandardCharsets.UTF_8);
default -> payload.toString().getBytes(StandardCharsets.UTF_8);
};
Comment on lines +205 to +207
private Jwt decodeAndValidateJwt(String token, String jwksUri) {
try {
return jwtDecoderProvider.get(jwksUri).decode(token);
String authorization = WebhookSecurityConfigurationUtils.requiredHeader(headers,
"Authorization");

if (!authorization.startsWith("Basic ")) {
Comment on lines +114 to +115
if (!authorization.startsWith(BEARER_PREFIX)
|| authorization.substring(BEARER_PREFIX.length()).isBlank()) {
Comment on lines +299 to +302
Exchange exchange = invokeValidateSecurityRoute(connector,
Map.of("Authorization", "Basic " + credentials));

assertNull(exchange.getException());
Comment on lines +370 to +373
Exchange exchange = invokeValidateSecurityRoute(connector,
Map.of("Authorization", "Bearer " + token));

assertNull(exchange.getException());
Comment thread docs/src/concepts/webhooks.md Outdated
Comment on lines +425 to +426
@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>
@foukou19
foukou19 force-pushed the feat/ingestion-security-processor branch from 1e0c83b to e05ce0f Compare September 3, 2026 09:51
@sonarqubecloud

sonarqubecloud Bot commented Sep 3, 2026

Copy link
Copy Markdown

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.

3 participants