[
https://issues.apache.org/jira/browse/CAMEL-24437?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18107252#comment-18107252
]
Andrea Cosentino commented on CAMEL-24437:
------------------------------------------
Scope split, and what is and is not verified.
*Landed: {{state}} only.*
{{OAuthCodeFlowProcessor}} generates a 32-byte random {{state}}, stores it in
the OAuth session under {{OAuthSession.OAUTH_STATE}}, and passes it through
{{OAuthCodeFlowParams}} - which already had the field - into both URL builders
({{VertxOAuth}} via {{OAuth2AuthorizationURL.setState}}, {{ServletOAuth}} as a
{{state}} query parameter). {{OAuthCodeFlowCallback}} removes the stored value
(single use) and compares it with the callback's {{state}} using
{{MessageDigest.isEqual}}, rejecting with {{400}} and stopping the route when
there is no flow in progress or the values differ.
*Deferred, deliberately:*
* *{{nonce}}* - needs ID-token validation plumbing to be worth anything;
sending it without checking it in the returned token adds no protection.
* *PKCE* - needs {{code_verifier}} carried on the token exchange, so it touches
{{AuthCodeCredentials}} and both {{authenticate()}} implementations. It is also
a feature rather than a defect fix, so it should not be backported.
* *Session cookie {{SameSite}}* - {{InMemorySessionStore.setSessionCookie()}}
hardcodes {{SameSite=None; Secure}}. Making it configurable with a {{Lax}}
default is a separate, self-contained change.
*Verification.* The three rejection paths are covered by unit tests in
{{OAuthProcessorFailClosedTest}} (6 tests, module suite 125 green). The **full
round trip against an identity provider is not verified**:
{{OAuthCodeFlowVertxTest}} and {{OAuthCodeFlowServletTest}} both sit behind
{{Assumptions.assumeTrue(admin.isKeycloakRunning())}} and expect Keycloak at
{{https://oauth.localtest.me/kc}}, which the module provisions through a Helm
chart needing a Kubernetes cluster with ingress and TLS. Those two tests were
skipped in my runs. Anyone with that environment should re-run them before this
is merged - they are what would confirm the provider echoes {{state}} back as a
{{state}} message header, which is the one assumption the unit tests cannot
check.
*Upgrade impact.* Deployments whose session is not sticky across the redirect
will now get {{400 Authorization state mismatch}}, because the session holding
the {{state}} has to be the one that returns. Noted in the 4.23 upgrade guide.
_Claude Code on behalf of oscerd_
> camel-oauth - authorization code flow sends no state, nonce or PKCE parameter
> -----------------------------------------------------------------------------
>
> Key: CAMEL-24437
> URL: https://issues.apache.org/jira/browse/CAMEL-24437
> Project: Camel
> Issue Type: Improvement
> Components: camel-oauth
> Reporter: Andrea Cosentino
> Assignee: Andrea Cosentino
> Priority: Major
> Fix For: 4.23.0
>
>
> {{VertxOAuth.buildCodeFlowAuthRequestUrl()}} builds the authorization request
> with only a redirect URI and scopes:
> {code:java}
> return oauth2.authorizeURL(new OAuth2AuthorizationURL()
> .setRedirectUri(params.getRedirectUri())
> .setScopes(params.getScopes()));
> {code}
> {{ServletOAuth}} does the same. No {{state}}, no {{nonce}} and no PKCE
> {{code_challenge}} is sent - {{OAuthCodeFlowParams}} has a {{state}} field
> but no processor ever sets it. Correspondingly,
> {{OAuthCodeFlowCallback.process()}} accepts whatever {{code}} parameter
> arrives, exchanges it, and binds the resulting {{UserProfile}} to the
> caller's session, with nothing tying the callback to a flow that session
> actually started.
> RFC 6749 section 10.12 and OpenID Connect Core both require the
> {{state}}/{{nonce}} binding, and RFC 7636 (PKCE) is recommended for all
> clients. Related: {{InMemorySessionStore.setSessionCookie()}} hardcodes
> {{SameSite=None; Secure}}, so the session cookie is attached to cross-site
> navigations.
> Proposal:
> * Generate a random {{state}} per flow, store it in the session, and require
> it to match on callback.
> * Generate a {{nonce}} for OIDC flows and validate it in the returned ID
> token.
> * Add PKCE ({{code_challenge}}/{{code_verifier}}, S256), which the underlying
> Vert.x and Servlet OAuth libraries already support.
> * Make the session cookie SameSite configurable, defaulting to {{Lax}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
