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

oauth2Login does not auto-redirect for XHR request #6812

Closed
simpleway opened this issue Apr 24, 2019 · 7 comments
Closed

oauth2Login does not auto-redirect for XHR request #6812

simpleway opened this issue Apr 24, 2019 · 7 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: bug A general bug
Milestone

Comments

@simpleway
Copy link

This issue is related to #6638.
I use single OpenIDC IdP (google) from OAuth2Login Sample. Added a rest endpoint that use the same security configuration. When an ajax request to the rest endpoint with an expired JSESSIONID or no JESSIONID at all, the response is a redirect to google IdP. The redirect will be blocked by the browser since cross domain redirect is not allowed in CORS policy.

After tracing the code a little bit, and found the request matcher logic in OAuth2LoginConfigurer might contribute to this behavior:

this.registerAuthenticationEntryPoint(http, this.getLoginEntryPoint(http, providerLoginPage));

private AuthenticationEntryPoint getLoginEntryPoint(B http, String providerLoginPage) {
RequestMatcher loginPageMatcher = new AntPathRequestMatcher(this.getLoginPage());
RequestMatcher faviconMatcher = new AntPathRequestMatcher("/favicon.ico");
RequestMatcher defaultEntryPointMatcher = this.getAuthenticationEntryPointMatcher(http);
RequestMatcher defaultLoginPageMatcher = new AndRequestMatcher(
new OrRequestMatcher(loginPageMatcher, faviconMatcher), defaultEntryPointMatcher);
LinkedHashMap<RequestMatcher, AuthenticationEntryPoint> entryPoints = new LinkedHashMap<>();
entryPoints.put(new NegatedRequestMatcher(defaultLoginPageMatcher),
new LoginUrlAuthenticationEntryPoint(providerLoginPage));
DelegatingAuthenticationEntryPoint loginEntryPoint = new DelegatingAuthenticationEntryPoint(entryPoints);
loginEntryPoint.setDefaultEntryPoint(this.getAuthenticationEntryPoint());
return loginEntryPoint;
}

The defaultEntryPointMatcher will filter out XMLHttpRequest. Should the entryPoints be something like

entryPoints.put(new OrRequestMatcher(new NegatedRequestMatcher(defaultLoginPageMatcher), defaultEntryPointMatcher),
				new LoginUrlAuthenticationEntryPoint(providerLoginPage));

Then the AJAX call to data will simply got 401 instead of a redirect, which the browser will block since it will be a cross domain redirect.

Originally posted by @simpleway in #6638 (comment)

@jzheaux jzheaux assigned jzheaux and jgrandja and unassigned jzheaux and jgrandja Apr 24, 2019
@rwinch
Copy link
Member

rwinch commented Apr 26, 2019

@simpleway This seems like a reasonable improvement. Would you be interested in submitting a PR?

@simpleway
Copy link
Author

simpleway commented Apr 30, 2019

@rwinch Sure, I can work on a PR.
In the meanwhile, I found a workaround for this issue.
Add the following in customized WebSecurityConfigurerAdapter.configure(HttpSecurity) method

LinkedHashMap<RequestMatcher, AuthenticationEntryPoint> entryPoints = new LinkedHashMap<>();
entryPoints.put(new RequestHeaderRequestMatcher("X-Requested-With", "XMLHttpRequest"), new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED));

DelegatingAuthenticationEntryPoint nonAjaxLoginEntryPoint = new DelegatingAuthenticationEntryPoint(entryPoints);
nonAjaxLoginEntryPoint.setDefaultEntryPoint(new LoginUrlAuthenticationEntryPoint("/oauth2/authorization/google"));

http.authorizeRequests()
       .anyRequest().fullyAuthenticated()
       .and()
       .oauth2Login()
       .and()
       .exceptionHandling().authenticationEntryPoint(nonAjaxLoginEntryPoint)

Not ideal, since hardcoded OpenID Connect provider login page, but good enough to overwrite the default behavior.

@jgrandja
Copy link
Contributor

@simpleway This isn't an oauth-specific issue. If your application was using http.formLogin() instead of http.oauth2Login(), you would get the same behaviour and get redirected to /login instead. The logic for creating the RequestMatcher comes from AbstractAuthenticationFilterConfigurer, which both FormLoginConfigurer and OAuth2LoginConfigurer inherit from.

I personally feel that the ajax client should handle 401's on whether the session is expired or if calling without authentication credentials.

@simpleway
Copy link
Author

@jgrandja I would recommend to separate API endpoints to use Bearer token with oauth2ResourceServer configuration. However, this will require UI component to hand over access token or JWT, which need additional development cycles.

Although sharing the same RequestMatcher, formLogin() most likely redirect to same domain, while oauth2Login() will redirect to an external domain. Google Chrome will trigger a CORS pre-flight on the external redirect, and end up with

(redirected from 'http://localhost:8080/api') from origin 'http://localhost:8080' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: Redirect is not allowed for a preflight request.

and abort the redirect request.

@jgrandja jgrandja self-assigned this May 1, 2019
@jgrandja jgrandja added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: waiting-for-feedback We need additional information before we can continue labels May 1, 2019
@jgrandja
Copy link
Contributor

jgrandja commented May 1, 2019

@simpleway

I would recommend to separate API endpoints to use Bearer token with oauth2ResourceServer configuration. However, this will require UI component to hand over access token or JWT, which need additional development cycles.

I don't really understand this statement. Which API endpoints are you referring to? This does not seem related to the issue on hand?

Although sharing the same RequestMatcher, formLogin() most likely redirect to same domain, while oauth2Login() will redirect to an external domain

This is not completely true. If an unauthenticated request comes in than the AuthenticationEntryPoint will redirect to /login for both formLogin() and oauth2Login() (on same host). However, if there is only 1 ClientRegistration than oauth2Login() will redirect to /oauth2/authorization/registration-id first (on same host) and than follow-up with another redirect to the provider, which yes is on an external host. FYI, an ajax client should not be used for initiating the Authorization Code flow as it's not designed to be used with such a client. See see this comment for more context.

At this point, I would suggest that you put together a minimal sample so I can better understand what you are trying to do as it doesn't seem totally clear and I feel your use case is not how Authorization Code flow is meant to be used. Please see this post to understand what the expectation of a minimal sample is. Thank you.

@jgrandja
Copy link
Contributor

jgrandja commented May 2, 2019

@simpleway We investigated this further and discovered a bug. I apologize for the confusion on my part as I didn't fully understand the actual problem you were having.

When the application has one ClientRegistration configured than unauthenticated requests will automatically be redirected to the provider - including XHR requests, which is the issue you are facing. The fix applied will short-circuit XHR requests and therefore will not match on the auto-redirecting AuthenticationEntryPoint.

The fix is now in master so please give it a try and let me know how it goes. I'll have this back-patched to 5.1.x as well.

@jgrandja jgrandja added type: bug A general bug OIDC and removed status: waiting-for-feedback We need additional information before we can continue labels May 2, 2019
@jgrandja jgrandja added this to the 5.2.0.M3 milestone May 2, 2019
@jgrandja jgrandja changed the title OAuth2LoginConfigurer should not redirect for Ajax request oauth2Login does not auto-redirect for XHR request May 2, 2019
@iilkevych
Copy link

iilkevych commented Dec 17, 2020

@jgrandja
as soon as this issue and #8118 are closed:

  • should spring security respond for unauthenticated requests with 302? or 401?
  • should spring security respond for unauthorized requests with 302? or 403?

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) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants