CAMEL-24281: camel-platform-http-main - fail closed when JWT authentication has no issuer or audience - #25187
Conversation
…cation has no issuer or audience The JWT authenticator was built from the configured keystore alone when neither jwtIssuer nor jwtAudience was set, because buildJwtOptions returned null and the caller then skipped setJWTOptions. Tokens were therefore only checked for signature and expiry, and the iss and aud claims were not validated, with nothing signalling that to the user. The embedded server and the management server now fail to start when a JWT keystore is configured but neither jwtIssuer nor jwtAudience is set. Deployments that want signature and expiry validation only can set jwtAllowMissingIssuerAndAudience to true, mirroring the allow-missing-audience and allow-missing-issuer options that camel-oauth already exposes. The check is applied to both configureAuthentication overloads, since the application server and the management server share this configurer. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Well-designed security hardening — fail closed when JWT authentication has no issuer or audience configured.
What I checked:
assertIssuerOrAudienceConfigured()is called at the top of bothconfigureAuthenticationoverloads (application server and management server), before theJWTAuthis constructed — fails fast at startupjwtAllowMissingIssuerAndAudiencedefaults tofalseon both properties classes, following the project's "denied unless opted in" security posture- The error message names all three properties (
jwtIssuer,jwtAudience,jwtAllowMissingIssuerAndAudience) so the operator knows exactly what to set - Test coverage: startup failure without issuer/audience, opt-out works, foreign-token rejection with issuer+audience configured, default value verified
- Existing
jwt-auth.propertiesadapted with the opt-out to keepJWTAuthenticationMainHttpServerTesttesting what it was designed to test - Upgrade guide entry present with the opt-out, component docs updated, generated files regenerated
- Conventions followed: AssertJ, package-private test class,
@Metadata(label = "security")
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Well-crafted security-hardening PR that correctly implements fail-closed JWT authentication. The fail-closed logic in assertIssuerOrAudienceConfigured is correct: it runs eagerly at configuration time, the IllegalArgumentException propagates cleanly to main.start(), and the error message correctly names the relevant properties without leaking sensitive values.
Two observations about the @Metadata annotation on the new opt-out flag, and two minor test observations below.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 63 tested, 27 compile-only — current: 61 all testedMaveniverse Scalpel detected 90 affected modules (current approach: 61).
|
# Conflicts: # docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_22.adoc
Mark jwtAllowMissingIssuerAndAudience with security = "insecure:dev" on both HttpServerConfigurationProperties and HttpManagementServerConfigurationProperties, so the security policy framework can report on it when the flag is enabled. The management properties class already uses that marker for devConsoleEnabled, uploadEnabled, downloadEnabled and sendEnabled. Share a single Vertx instance across the tests and close it afterwards, rather than creating one per test method and leaving it open. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
|
Two follow-up notes on the review round, beyond the three inline replies. On Re-verified against the Vert.x upgrade: Full reactor build Claude Code on behalf of oscerd |
3 participants
CAMEL-24281
When JWT authentication is enabled on the camel-main embedded HTTP server,
JWTAuthenticationConfigurer.buildJwtOptions(...)returnsnullif neitherjwtIssuernorjwtAudienceis configured, and the caller then skipsJWTAuthOptions.setJWTOptions(...). The resulting Vert.xJWTAuthis built from the keystore alone, so tokens are only checked for signature and expiry and theissandaudclaims are not validated. Nothing signals this: the server starts normally, so a deployment configured the documented way can be enforcing less than intended.Change
assertIssuerOrAudienceConfigured(...)is called at the top of bothconfigureAuthenticationoverloads, so the application server and the management server are both covered. It throwsIllegalArgumentExceptionwhen a JWT keystore is configured but neither value is set, naming the properties involved.jwtAllowMissingIssuerAndAudienceoption (defaultfalse, markedsecurity = "insecure:dev") onHttpServerConfigurationPropertiesandHttpManagementServerConfigurationProperties, for deployments that genuinely want signature and expiry validation only.This mirrors the posture
camel-oauthalready uses, whereDefaultOAuthTokenValidationFactory.validateResolvedConfigurationrefuses to operate unless an expected audience and issuer are configured (allow-missing-audienceandallow-missing-issuerboth default tofalse).Compatibility
This changes startup behaviour for an existing configuration, so it is documented in the 4.22 upgrade guide together with the opt-out.
jwtIssuerandjwtAudiencewere only added in 4.21.0 by CAMEL-23525, so the fail-closed behaviour applies tomainonly. It cannot be backported as-is, because 4.18.x and 4.14.x have no such options to satisfy the requirement. Those branches instead receive the options themselves, so operators there can enforceiss/audby configuration:camel-4.18.x(4.18.4)camel-4.14.x(4.14.9)Both are additive and change no existing behaviour.
Tests
JWTIssuerAudienceRequiredMainHttpServerTest(4): startup fails without an issuer or audience and the message names all three properties; the opt-out starts and still serves; a token minted for a different issuer and audience is rejected once both are configured; the opt-out defaults to false on both properties classes.One existing test config,
jwt-auth.properties, is keystore-only and therefore no longer starts. It now setsjwtAllowMissingIssuerAndAudience=true, which keepsJWTAuthenticationMainHttpServerTesttesting what it was written to test and exercises the opt-out.Module suite green (23),
camel-maingreen (231), full reactor build green with generated files committed. Re-verified after mergingmain, which brought Vert.x 4.5.31.The component documentation previously described signature and expiry checking as the default with issuer and audience as an optional extra. It now states that without them any unexpired token signed by a trusted key is accepted regardless of issuer or audience.
Review round
security = "insecure:dev"added to the opt-out on both properties classes (@gnodet), verified through to the generated metadata.Vertxinstance and closes it in@AfterAllinstead of creating one per test method (@gnodet).mainand resolved thecamel-4x-upgrade-guide-4_22.adocconflict with the CAMEL-24279 entry; both notes kept.Claude Code on behalf of oscerd