CAMEL-24411: camel-oauth - stop the route when the processors do not authenticate the request - #25567
Merged
Merged
Conversation
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 10 tested, 28 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
gnodet
approved these changes
Aug 21, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Well-scoped security fix by the original camel-oauth component author. The three OAuth processor denial paths that previously returned without stopping the route now correctly call exchange.setRouteStop(true), preventing subsequent route steps from overwriting authentication responses.
Strengths:
- The 400-to-401 status code change for Bearer token auth is correct per RFC 6750
- The removal of the
authHeadervalue from the error log is a good security practice — avoids logging sensitive credential material - The deliberate choice not to modify
sendRedirect()orOAuthLogoutProcessoris well-reasoned and documented in the PR description - Thorough upgrade guide entry targeting the correct 4.23 release
- Test covers both
OAuthBearerTokenProcessor(401 on invalid auth header + route stop) andOAuthCodeFlowCallback(403 on missing session + route stop) - The new
assertj-coretest dependency inherits its version from the parent POM as expected
Minor observations (non-blocking):
- Pre-existing:
OAuthBearerTokenProcessorredeclaresprivate final Logger logwhich shadows the parent'sprotected final Logger log. Same pattern inOAuthCodeFlowCallbackandOAuthCodeFlowProcessor. Worth a follow-up cleanup. - The
OAuthCodeFlowProcessorredirect-to-IdP path (which also now stops the route) is not covered by a unit test since it requires an OAuth factory. Consider a follow-up with a mockOAuthFactoryif the module gains a mocking dependency.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
oscerd
force-pushed
the
fix/CAMEL-24411
branch
from
August 21, 2026 20:26
766311a to
f3db489
Compare
…authenticate the request
OAuthBearerTokenProcessor and OAuthCodeFlowProcessor both returned normally
from process() on the paths where they do not authenticate the caller, so the
remaining steps of the route still ran and overwrote the response the processor
had just prepared. The component's own test routes have the shape
.process(new OAuthBearerTokenProcessor()).setBody(...)
where that following step executes.
The same shape is present in OAuthCodeFlowCallback, which answered 400 for a
callback without the code parameter and then let the route continue.
There was no setRouteStop, CamelAuthorizationException or RoutePolicy anywhere
in camel-oauth, so nothing halted the exchange on any of these paths.
This adds reject() and rejectUnauthorized() helpers to AbstractOAuthProcessor
and uses them at the three denial points:
* OAuthBearerTokenProcessor - a missing Authorization header, or one that does
not parse as "Bearer <token>", now answers 401 with a WWW-Authenticate: Bearer
challenge (RFC 6750) instead of 400, and stops the route. A present but
invalid token keeps failing by propagating the exception from
OAuth.authenticate(), as before.
* OAuthCodeFlowProcessor - stops the route after redirecting an unauthenticated
caller to the identity provider; the 302 is the whole response.
* OAuthCodeFlowCallback - keeps answering 400 for a missing authorization code
and now stops the route too.
sendRedirect() itself is deliberately left alone, and OAuthLogoutProcessor is
unchanged: the shipped logout route relies on the step after the redirect
running, so stopping the route inside sendRedirect would break an intended flow.
Only the denial paths stop; authenticated requests continue through the rest of
the route exactly as before.
Adds OAuthProcessorFailClosedTest covering the three paths that return before an
identity provider is contacted, a test-scoped assertj-core the module was
missing, and a 4.23 upgrade-guide entry for the status-code and route-stop
change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
oscerd
force-pushed
the
fix/CAMEL-24411
branch
from
August 21, 2026 20:28
f3db489 to
f8e2553
Compare
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.
Fixes CAMEL-24411.
Problem
The camel-oauth processors return normally from
process()on the paths where they do not authenticate the caller, so the remaining steps of the route still run — and overwrite the response the processor had just prepared. The component's own test routes have exactly that shape:Three call sites are affected:
OAuthBearerTokenProcessorAuthorizationheader, or one that does not parse asBearer <token>400+ returnOAuthCodeFlowProcessor302+ returnOAuthCodeFlowCallbackcodeparameter400+ returnThere is no
setRouteStop,CamelAuthorizationExceptionorRoutePolicyanywhere in camel-oauthsrc/main, so nothing halts the exchange on any of them.Note the enforcement was also inverted in the bearer processor: a present-but-invalid token fails closed (the exception from
OAuth.authenticate()propagates), while an absent credential did not.Change
Adds
reject()andrejectUnauthorized()toAbstractOAuthProcessorand applies them at the three denial points. The bearer processor now answers401with aWWW-Authenticate: Bearerchallenge (RFC 6750) rather than400.Deliberately left alone:
sendRedirect()itself, andOAuthLogoutProcessor. MakingsendRedirectstop the route would have been the tidier change, but the shipped logout routerelies on the step after the redirect running. Only the denial paths stop; authenticated requests continue through the rest of the route exactly as before.
Testing
New
OAuthProcessorFailClosedTestcovers the three paths that return before any identity provider is contacted, so it needs no Keycloak container. Verified it catches the regression: all 3 tests fail against the pre-fix code, all 3 pass after.Adds a test-scoped
assertj-coreto camel-oauth, which the module was missing (project convention is AssertJ for new test code).Backport
Intended for camel-4.22.x, camel-4.18.x and camel-4.14.x. It is a behaviour change, but it is the behaviour of an authentication gate, and camel-oauth is a Preview-support component. The upgrade-guide entry stays on
mainper the project's guide policy._Claude Code on behalf of