Support 303 status for authorization response

[wapkch]

Expected Behavior

According to A comprehensive formal security analysis of OAuth 2.0. 303 redirect should be used to drop the body of an HTTP POST request.

Current Behavior

DefaultRedirectStrategy in OAuth2AuthorizationEndpointFilter sets the status to 302

Context

If needed, i can work on it.

[jgrandja]

@wapkch The article you referenced is dated 2016 so it's quite old and OAuth2 has gone through many revisions since.

303 redirect should be used to drop the body of an HTTP POST request.

Can you please provide more details on your reasoning? Do you see issues with the current implementation of OAuth2AuthorizationEndpointFilter returning 302? If so, please provide specific details.

[wapkch]

@wapkch The article you referenced is dated 2016 so it's quite old and OAuth2 has gone through many revisions since.

303 redirect should be used to drop the body of an HTTP POST request.

Can you please provide more details on your reasoning? Do you see issues with the current implementation of OAuth2AuthorizationEndpointFilter returning 302? If so, please provide specific details.

In my scenario, no issue so far. And probably not a problem for most modern browsers.

But i'm afraid some user-agents treat 302 like 307 which then would expose the user's credentials to the OAuth2 client on the callback.

So, 303 is the best choice. Or at least, provide an easy way to replace the DefaultRedirectStrategy in OAuth2AuthorizationEndpointFilter instead of having to replacing the whole OAuth2AuthorizationEndpointFilter.

[jgrandja]

@wapkch I wasn't able to reproduce the issue. Your request sequence is different than the default Messages sample as it appears you are not using OIDC for authentication.

Can you please provide a minimal sample that reproduces the issue so I can replicate on my end.

[wapkch]

Hi @jgrandja, In the above scenario, I was assuming a user-agent which treats 302 like 307(Not encountered so far, but it is possible). So i changed the RedirectStrategy in SavedRequestAwareAuthenticationSuccessHandler and OAuth2AuthorizationEndpointFilter from the DefaultRedirectStrategy to a custom Http307RedirectStrategy to simulate the scene. The minimal sample can be found here. The reproduction process is as follows:

1. Start the application, default at localhost:9000
2. Authorization request

http://localhost:9000/oauth2/authorize?response_type=code&client_id=messaging-client&scope=openid&redirect_uri=http://127.0.0.1:8080/authorized&state=some-state

3. Login via form
4. OAuth2 callback request with user credentials exposed to the client like following:

[jgrandja]

@wapkch Thanks for providing the sample.

The customization applied in DefaultSecurityConfig, specifically with the formLogin() authentication mechanism is what triggers the issue. That configuration is the authentication mechanism used and is out-of-scope for Spring Authorization Server. This is a Spring Security concern since authentication is configured and handled by Spring Security. Based on this alone, the issue is not specific to Spring Authorization Server.

Furthermore, I don't see this being an issue with Spring Security either. The DefaultRedirectStrategy calls the underlying implementation of HttpServletResponse.sendRedirect(location), which specifies that the 302 status must be returned.

But i'm afraid some user-agents treat 302 like 307 which then would expose the user's credentials to the OAuth2 client on the callback.

I was assuming a user-agent which treats 302 like 307(Not encountered so far, but it is possible)

It appears you haven't found a user-agent that actually performs this behaviour. However, if you did, this is a bug in the user-agent and should be reported to their ticketing system.

Also, consider a similar use case where formLogin() is configured to return a 307 status but the location is a simple @Controller (NOT an OAuth2 Client Redirection Endpoint) then it too would have access to the submitted user's credentials. Again, this scenario is not specific to OAuth.

Based on the explanation provided, I'm going to close this as I don't see any issues in Spring Authorization Server or Spring Security.

[wapkch]

Hi @jgrandja Thanks for your detailed reply.

I agree that it's the authentication mechanism in Spring Security triggers the issue, and it's not specific to OAuth. But i think there are still optimizations we can do:

It appears you haven't found a user-agent that actually performs this behaviour. However, if you did, this is a bug in the user-agent and should be reported to their ticketing system.

I'm not sure if this is a bug in the user-agent. Because according to rfc2616:

Note: RFC 1945 and RFC 2068 specify that the client is not allowed
to change the method on the redirected request. However, most
existing user agent implementations treat 302 as if it were a 303
response, performing a GET on the Location field-value regardless
of the original request method. The status codes 303 and 307 have
been added for servers that wish to make unambiguously clear which
kind of reaction is expected of the client.

Some user-agents erroneously redirect a 302 POST into a GET request, so 303 and 307 was added to make it clear. And later in rfc7231:

The user agent MAY
use the Location field value for automatic redirection.

There is no clear instructions that 302 can not be used to redirect a POST into a POST. But 303 can do that:

A user agent can perform
a retrieval request targeting that URI (a GET or HEAD request if
using HTTP), which might also be redirected, and present the eventual
result as an answer to the original request.

So, changing the status from 302 to 303 for authorization response can avoid the risks. Do you agree with that?

Even without modifying the default behavior, do you think adding a setter method for RedirectStrategy in OAuth2AuthorizationEndpointFilter makes sense?

My Spring Security OAuth based project returns a 303 status in authorization response, i have to make it compatiable to all integrated parties(Especially the integration test cases written by the test team, which can not be modified easily) after the migration to Spring Authorization Server.

[jgrandja]

@wapkch

So, changing the status from 302 to 303 for authorization response can avoid the risks. Do you agree with that?

Yes. But we'll have to figure out an improvement that won't break existing applications.

Even without modifying the default behavior, do you think adding a setter method for RedirectStrategy in OAuth2AuthorizationEndpointFilter makes sense?

I don't think I want to expose setRedirectStrategy() since the status code (or authorization response) could be customized by setting a custom setAuthenticationSuccessHandler().

I'll re-open this and we can consider what the best option would be.