Random error location response ~ OAuth2AuthorizationCodeRequestAuthenticationProvider

[dlehammer]

Describe the bug
When the authentication request query parameter; client ID, grant type or PKCE triggers an authorization code error, a random client redirect URI is returned as 302 Location.

~ see also orange markings:

From:

spring-authorization-server/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java

Line 101 in 9179881

public Authentication authenticate(Authentication authentication) throws AuthenticationException {

Symptom discovered in spring-authorization-server v0.4.0.

To Reproduce

1. Register a client with a list of multiple redirect URIs.
2. Initiate a authorization code error by mangling a authentication request with invalid query parameter; client ID, grant type or PKCE.
   2.1 utilize the clients last registered redirect URI as query parameter
3. Observe the first entry from the client registration registration is returned as location.

Expected behavior
When the authentication request contains a valid redirect URI, I would expect the 302 Location to reflect the initial request.

[jgrandja]

@dlehammer

When the authentication request query parameter; client ID, grant type or PKCE triggers an authorization code error, a random client redirect URI is returned as 302 Location.

Please review Section 4.1.1. Authorization Request to understand the validation required for the Authorization Request.

OAuth2AuthorizationCodeRequestAuthenticationProvider has implemented Authorization Request validation as per spec.

[dlehammer]

Hi @jgrandja,

I've reviewed the suggested section, unfortunately I couldn't find any mention of picking a random value when a redirect_uri is present on the request, instead the following paragraph resonated with me:

In particular, the authorization server MUST validate the redirect_uri in the request if present, ensuring that it matches one of the registered redirect URIs previously established during client registration (Section 2).

Ie. when the redirect_uri is present & valid, the intuitive error response would reflect the request in a sensible manner.
And while the redirect_uri is OPTIONAL for OAuth it's REQUIRED for OIDC v1.0 authorization code flow, and reflecting valid values presented seems like a low hanging fruit 🤓

[jgrandja]

@dlehammer I don't understand what you mean by:

I couldn't find any mention of picking a random value when a redirect_uri is present on the request

What random value are you referring to?

If you feel there is a bug, please provide a test that reproduces the issue so I can better understand your use case.

[dlehammer]

Hi @jgrandja,

Thanks for taking the time to reply.

I'll try to elaborate, my use-case was initially as follows; enable PKCE for a registred-client on the auth-server that has a large number of redirect_uri's.

The symptom in this issue was exposed, when attempting to debug the error handling in the client on the local dev-env (127.0.0.1).
I was surprised that the auth-server 302 location response didn't reflect the request redirect_uri value, even though the request included a valid redirect_uri.
Instead OAuth2AuthorizationCodeRequestAuthenticationProvider#resolveRedirectUri(..) utilizes registeredClient.getRedirectUris().iterator().next() resulting in a very confused developer ;)

I've created a sample (based on default-authorizationserver) to illustrate the symptom (this commit triggers the symptom):

MockHttpServletRequest:
      HTTP Method = GET
      Request URI = /oauth2/authorize
       Parameters = {redirect_uri=[http://127.0.0.1/login/oauth2/code/messaging-client-oidc], response_type=[code], client_id=[messaging-client], scope=[openid message.read message.write], state=[state]}
          Headers = [Content-Type:"application/x-www-form-urlencoded;charset=UTF-8", sec-ch-ua:""Chromium";v="106", " Not A;Brand";v="99", "Google Chrome";v="106"", sec-ch-ua-mobile:"?0", Accept:"text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9", Upgrade-Insecure-Requests:"1", sec-ch-ua-platform:""Windows"", Sec-Fetch-Dest:"document", Sec-Fetch-Site:"same-origin", Sec-Fetch-User:"?1", Accept-Encoding:"gzip, deflate, br", Accept-Language:"en-US,en;q=0.9", Sec-Fetch-Mode:"navigate"]
             Body = null
    Session Attrs = {SPRING_SECURITY_CONTEXT=SecurityContextImpl [Authentication=UsernamePasswordAuthenticationToken [Principal=org.springframework.security.core.userdetails.User [Username=user1, Password=[PROTECTED], Enabled=true, AccountNonExpired=true, credentialsNonExpired=true, AccountNonLocked=true, Granted Authorities=[ROLE_USER]], Credentials=[PROTECTED], Authenticated=true, Details=null, Granted Authorities=[ROLE_USER]]]}

SNIP..

MockHttpServletResponse:
           Status = 302
    Error message = null
          Headers = [X-Content-Type-Options:"nosniff", X-XSS-Protection:"0", Cache-Control:"no-cache, no-store, max-age=0, must-revalidate", Pragma:"no-cache", Expires:"0", X-Frame-Options:"DENY", Location:"http://u.41/?error=invalid_request&error_description=OAuth%202.0%20Parameter:%20code_challenge&error_uri=https://datatracker.ietf.org/doc/html/rfc7636%23section-4.4.1&state=state"]
     Content type = null
             Body = 
    Forwarded URL = null
   Redirected URL = http://u.41/?error=invalid_request&error_description=OAuth%202.0%20Parameter:%20code_challenge&error_uri=https://datatracker.ietf.org/doc/html/rfc7636%23section-4.4.1&state=state
          Cookies = []

Notice how the response "Redirected URL" ("http://u.41/..") doesn't match the valid request redirect_uri ("http://127.0.0.1/..").

Proposed mitigation

I think the sensible response should reflect known valid values from the original request.
This could perhaps be achieved by validating the request client_id first and redirect_uri second, that would allow any subsequent validation failure, ex. PKCE, to be redirected to the requested redirect_uri.
~ only a client_id validation error should be cause for falling back on a (...next()) known valid redirect_uri value server-side 🤓

[dlehammer]

I think the sensible response should reflect known valid values from the original request.

Ie. without mitigation the user-agent can potentially end up on any registered redirect_uri in the case of a validation error pt.

Known "workaround" for unpredictable client redirect from server-side error-handling

Remove all but 1 redirect_uri from the registered-client configuration server-side, matching the desired environment 🤠

[jgrandja]

@dlehammer Thanks for providing the details.

I believe the fix that is in this PR gh-1013 will resolve your issue as well. Can you confirm?

[dlehammer]

Hi @jgrandja & @leshalv,

Seems plausible ~ based on debug state OAuth2AuthorizationCodeRequestAuthenticationToken#redirectUri contains the desired value (green marker) vs. the resolved uri (red marker):

Albeit I think the usage in gh-1013 is too unrestricted ~ ie. the OAuth2AuthorizationCodeRequestAuthenticationToken#redirectUri value should only be utilized; if the client_id has been validated OK and the request redirect_uri has been verified as belonging to the registered client_id.
Without this guard a bad actor is able to hijack the redirect_uri in the user-agent by triggering the server-side validation 🤔
(with explicit test coverage I think it would be easier to verify ~ I didn't spot any in the PR 🤓 )

[jgrandja]

@dlehammer Thanks for confirming.

FYI, we have existing test coverage for redirect_uri in OAuth2AuthorizationCodeRequestAuthenticationProviderTests and I've requested more test coverage to be included in gh-1013.