Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support OAuth2 redirect_uri for Forwarded Header #5535

Closed
mhyeon-lee opened this issue Jul 18, 2018 · 5 comments
Closed

Support OAuth2 redirect_uri for Forwarded Header #5535

mhyeon-lee opened this issue Jul 18, 2018 · 5 comments
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)

Comments

@mhyeon-lee
Copy link
Contributor

mhyeon-lee commented Jul 18, 2018

Summary

When generating the redirect_uri from the DefaultOAuth2AuthorizationRequestResolver, apply the Forwarded Header to {baseUrl}.

Also, shouldProcessAuthorizationResponse of OAuth2AuthorizationCodeGrantFilter should be compared to redirect_uri based on Forwarded Header application.

Actual Behavior

Expected Behavior

Sample

The following test fails.

@Test
public void resolveWhenAuthorizationRequestRedirectUriTemplatedThenRedirectUriForwardedIPv4HostHeader() {
	ClientRegistration clientRegistration = this.registration2;
	String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
	MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
	request.setServletPath(requestUri);
	request.setScheme("http");
	request.setServerName("localhost");
	request.setServerPort(80);
	request.addHeader("Forwarded", "host=192.168.0.1");

	OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
	assertThat(authorizationRequest.getRedirectUri()).isNotEqualTo(
			clientRegistration.getRedirectUriTemplate());
	assertThat(authorizationRequest.getRedirectUri()).isEqualTo(
			"http://192.168.0.1/login/oauth2/code/" + clientRegistration.getRegistrationId());
}
@mhyeon-lee mhyeon-lee changed the title Support Forwarded Header for redirect_uri in DefaultOAuth2AuthorizationRequestResolver Support OAuth2 redirect_uri for Forwarded Heade Jul 18, 2018
mhyeon-lee pushed a commit to mhyeon-lee/spring-security that referenced this issue Jul 18, 2018
…ader.

When generating the redirect_uri
from the DefaultOAuth2AuthorizationRequestResolver,
apply the Forwarded Header to {baseUrl}.

Fixed: spring-projectsgh-5535
@mhyeon-lee mhyeon-lee changed the title Support OAuth2 redirect_uri for Forwarded Heade Support OAuth2 redirect_uri for Forwarded Header Jul 18, 2018
mhyeon-lee pushed a commit to mhyeon-lee/spring-security that referenced this issue Jul 18, 2018
shouldProcessAuthorizationResponse of OAuth2AuthorizationCodeGrantFilter
should be compared to redirect_uri based on Forwarded Header application.

Fixed: spring-projectsgh-5535
@jgrandja jgrandja added the in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) label Jul 18, 2018
@jgrandja
Copy link
Contributor

@mhyeon-lee Thank you very much for your recent contributions!

As an FYI, the recommended approach to handle X-Forwarded-* headers is to use the ForwardedHeaderFilter.

Please see these resources for more detail.

Forwarded headers
Centralize handling of "Forwarded" headers to ForwardedHeaderFilter

Additional Resources for configuration

Proxy Server Configuration
Running Behind a Front-end Proxy Server

Does this make sense?

@jgrandja jgrandja added the status: waiting-for-feedback We need additional information before we can continue label Jul 18, 2018
@mhyeon-lee
Copy link
Contributor Author

Centralize handling of "Forwarded" headers to ForwardedHeaderFilter

I did not know if there was a ForwardedHeaderFilter.
Will the filterChain following the ForwardedHeaderFilter no longer care about the Forwarded header?

I think this filter is very useful if I understand what is right.
Thank you for letting me know.

If you do not need to proceed with this issue any further, you may close it with #5536 PR.

@jgrandja
Copy link
Contributor

@mhyeon-lee

Will the filterChain following the ForwardedHeaderFilter no longer care about the Forwarded header?

The ForwardedHeaderFilter will wrap the request and adapt the required methods to account for the X-Forwarded-* headers. So when the request arrives to the DefaultOAuth2AuthorizationRequestResolver than the redirectUri will be built using the X-Forwarded-* headers because the wrapped request will return the forwarded headers.

Take a look at the javadoc and source for ForwardedHeaderFilter along with the resources I sent in the previous commit.

I'm going to close this issue and associated PR as the ForwardedHeaderFilter is the solution for this issue.

Thanks again for your contribution!

@jgrandja jgrandja removed the status: waiting-for-feedback We need additional information before we can continue label Jul 18, 2018
@ericis
Copy link

ericis commented Jul 23, 2018

I'm going to try this out... I just submitted a related issue as a question to StackOverflow:
https://stackoverflow.com/questions/51404552/spring-boot-oauth-always-redirecting-to-http-ibm-cloud-cf-spring-boot-2

Sample project: https://github.com/ericis/oauth-cf-https-issue

... I'll report if proposed solution from @jgrandja works for me.

@ericis
Copy link

ericis commented Jul 24, 2018

Worked! https://github.com/ericis/oauth-cf-https-issue

I added a basic flag in the example to turn off https (e.g. localhost) using a local Spring Profile.

	@Bean
	FilterRegistrationBean<ForwardedHeaderFilter> forwardedHeaderFilter() {
		
	    final FilterRegistrationBean<ForwardedHeaderFilter> filterRegistrationBean = new FilterRegistrationBean<ForwardedHeaderFilter>();
	    
	    filterRegistrationBean.setFilter(new ForwardedHeaderFilter());
	    filterRegistrationBean.setOrder(Ordered.HIGHEST_PRECEDENCE);
	    
	    return filterRegistrationBean;
	}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)
Projects
None yet
Development

No branches or pull requests

3 participants