vert-x3 / vertx-web

HTTP web applications for Vert.x
Apache License 2.0
1.11k stars 534 forks source link

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

Open bfreuden opened 7 months ago

bfreuden commented 7 months ago

Version

4.3.8 and 4.4.9

Context

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

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)

pmlopes commented 5 months ago

Could you try to identify why isn't this code being executed: https://github.com/vert-x3/vertx-web/blob/4e9e2c40e0843e7abb4ddc8feae021cdb17d6a41/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/AuthenticationHandlerImpl.java#L129

pmlopes commented 5 months ago

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?