OAuth2AuthorizationCodeGrantWebFilter should not restrict redirect-uri

[jgrandja]

OAuth2AuthorizationCodeGrantWebFilter currently matches the Authorization Response using the pattern /{action}/oauth2/code/{registrationId}, which is too restrictive.

We should allow the client to configure the redirect-uri to be any URI within the application. The Authorization Response matching should follow the same logic found in OAuth2AuthorizationCodeGrantFilter.shouldProcessAuthorizationResponse().

[clementkng]

Hi @jgrandja, can I try taking this issue?

[jgrandja]

@clementkng That would be great. Let me know if you have any questions. Thanks!

[clementkng]

@jgrandja Thanks, right now I'm just reading through the code. A couple of opening questions:

• How would a client be able to set the redirect-URI to be any in the application if it was just a pattern enforced in the code before? Or are we supposed to set this.requiresAuthenticationMatcher to something more permissive?
• I believe we're supposed to match the behavior of OAuth2AuthorizationCodeGrantWebFilter.filter() to the logic of OAuth2AuthorizationCodeGrantFilter.shouldProcessAuthorizationResponse(), but I'm having trouble matching up the two when one takes a ServerWebExchange and WebFilterChain and the other takes a HttpServletRequest. Or is there a wider restructuring in play?
• I'm trying to write/modify tests before changing behavior, and was wondering what tests would be good (related to question 1, as I'm not sure how the client can set a custom redirect-uri that can be tested against)? My only ideas so far are to test w/ a custom redirect-uri.

Also, I've skimmed through the Spring Security docs, but wasn't able to find the specific section that would shed more context on this issue. Is there a guide to learning more about this code?

[jgrandja]

@clementkng I would recommend reviewing the Authorization Code Grant flow in the spec. The section Authorization Response is implemented by OAuth2AuthorizationCodeGrantFilter and OAuth2AuthorizationCodeGrantWebFilter.

After you review the spec and gain a solid understanding of the authorization_code grant flow than I believe the code will be more clear.

How would a client be able to set the redirect-URI to be any in the application

Go through these steps for oauth2Login() and you'll see how to configure the redirect-uri for the client. The same steps apply for oauth2Client().authorizationGrant()

[clementkng]

Hi @jgrandja, thanks for your recommendations! I've gone through the Authorization Code Grant flow and understand the higher details of what the OAuth2AuthorizationCodeGrantWebFilter does, but I'm still a little confused about the mapping b/w different classes in the methods ie ServerWebExchange, WebFilterChain, Authentication of OAuth2AuthorizationCodeGrantWebFilter vs HttpServletRequest of OAuth2AuthorizationCodeGrantFilter etc. I think what would help my understanding is building out the tests and verifying them so there's a single source of truth to work from. One test I can think of is that since the redirect-uri no longer has to match the restrictive form, I could modify the existing test to get another uri ie "/". I was also trying to translate OAuth2AuthorizationCodeGrantFilterTests over to match the behavior, but I was running into the same issue regarding the mapping of classes.

Go through these steps for oauth2Login()

When you say go through, do you mean I have to physically go through the steps i.e. create a new application.yml, assuming the Google OAuth is already set up? Or are you saying that the form of the redirect-uri is set in the PathPatternParserServerWebExchangeMatcher?

oauth2Client().authorizationGrant()

I'm not sure what this is referring to.

[jgrandja]

@clementkng I think that makes a lot of sense as far as modifying the existing test filterWhenMatchThenAuthorizedClientSaved() as your starting point.

When I mentioned

Go through these steps for oauth2Login()

I was trying to answer your question

I'm not sure how the client can set a custom redirect-uri that can be tested against

To be more clear, those steps show you how to customize the redirect-uri in application.yml for the web app itself. Since you are starting with the test than you need to look at this line.

Specifically, TestOAuth2AuthorizationCodeAuthenticationTokens.unauthenticated(), which ultimately creates a ClientRegistration with redirectUriTemplate set at {baseUrl}/{action}/oauth2/code/{registrationId}. So what you need to do instead is make sure you set another custom redirectUriTemplate to something like /callback and than that test will fail. Than you have your starting point to apply the changes.

Makes sense?

[clementkng]

@jgrandja Thanks for clarifying, that makes more sense to me. I believe I now have a test I can begin developing against, but I just want to make sure. If I create a new ClientRegistration.Builder method (ie ClientRegistration.Builder clientRegistration3() based off of the originalclientRegistration()), change the argument inside redirectUriTemplate to "/callback", link up clientRegistration3() in TestOAuth2AuthorizationCodeAuthenticationTokens.unauthenticated(), and change the filterWhenMatchThenAuthorizedClientSaved() to get("/callback") in this line, then I should get a test failure b/c the filter only accepts redirect-uris of the form above. This would also mean that w/o the last change to the get(), the test would still pass, right?

[jgrandja]

@clementkng

If I create a new ClientRegistration.Builder method (ie ClientRegistration.Builder clientRegistration3()...

It's not necessary to create a new method. Just re-use what's there like this

ClientRegistration registration = TestClientRegistrations.clientRegistration().redirectUriTemplate("/callback") .build();
OAuth2AuthorizationExchange exchange = TestOAuth2AuthorizationExchanges.success();
OAuth2AuthorizationCodeAuthenticationToken authenticationToken = new OAuth2AuthorizationCodeAuthenticationToken(registration, exchange);

Yes, you will need to change get("/authorize/oauth2/code/registration-id") to get("/callback")

[clementkng]

@jgrandja I'm still having trouble getting the behavior of the OAuth2AuthorizationCodeGrantWebFilter to match the OAuth2AuthorizationCodeGrantFilter.shouldProcessAuthorizationResponse(), and won't be able to circle back to this issue in a while.

I just wanted to let ppl know so if anyone else wants to jump in, feel free to.

[jgrandja]

@clementkng Ok no worries. Thanks for letting me know.