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

oauth2 should not redirect (HTTP 302) XHR requests #2591

Open
bfreuden opened this issue Mar 28, 2024 · 2 comments
Open

oauth2 should not redirect (HTTP 302) XHR requests #2591

bfreuden opened this issue Mar 28, 2024 · 2 comments
Labels

Comments

@bfreuden
Copy link
Contributor

bfreuden commented Mar 28, 2024

Version

4.3.8 and 4.4.9

Context

A browser-based app with a backend support.
The Vert.x backend is serving:

  • an API that is protected by OAuth2Auth created using KeycloakAuth.discover
  • a Single Page Application (index.html, served by the backend with a StaticHandler) using axios to call the API

The problem

When not authenticated, API calls are returning HTTP 302 (+ Location) responses that are blocked by the browser because of the redirect.
From the axios point of view, it is getting a generic AxiosError: Network Error (the browser is "hiding" the redirect).

I think API calls should return HTTP 401 responses when axios is configured to set the X-Requested-With: XMLHttpRequest HTTP header.

If I'm understanding correctly, Springs seems to return 401 responses in that situation: https://github.com/candrews/spring-security/blob/09100daf0fd6cd3a89dded4c962191cff98bb031/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java#L391

Reproducer

https://github.com/bfreuden/vertx-vue-oauth2-example

Basically containing 3 files:

Also containing a fix proposal:
https://github.com/bfreuden/vertx-vue-oauth2-example/blob/master/src/main/java/io/vertx/ext/web/handler/impl/AuthenticationHandlerImpl.java#L128

Warning: by default the repo is showing the behavior of the fix proposal.

Completely removing the AuthenticationHandlerImpl.java file will show the error: the backend is returning a 302 error and the SPA is unable to detect the the user is not authenticated (axios will get a generic Network Error error)

@bfreuden bfreuden added the bug label Mar 28, 2024
@pmlopes
Copy link
Member

pmlopes commented May 17, 2024

Could you try to identify why isn't this code being executed:

if (!"XMLHttpRequest".equals(ctx.request().getHeader("X-Requested-With"))) {

@pmlopes
Copy link
Member

pmlopes commented May 30, 2024

I saw it wrong, I think @bfreuden fix is correct. the missing check on 302 looks correct. Maybe you can add it and a test to verify the behavior?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants