solo-io / gloo

The Feature-rich, Kubernetes-native, Next-Generation API Gateway Built on Envoy
https://docs.solo.io/
Apache License 2.0
4.06k stars 433 forks source link

Being able to redirect with 307 when session timeout and token refreshes #9231

Open edubonifs opened 5 months ago

edubonifs commented 5 months ago

Gloo Edge Product

Enterprise

Gloo Edge Version

1.16

Is your feature request related to a problem? Please describe.

When the Gloo session timeouts and token refreshes, the scenario our client involves making a POST request to an API. However, when the session times out, Gloo initiates the refresh process. Once the session is refreshed, it sets the new token in the client's cookie and redirects the client back to the original API call. The problem is that this redirect changes the HTTP method from POST to GET.

The HTTP redirection mechanism doesn't preserve the method by design, and currently we return 302

Describe the solution you'd like

Taking into account the behavior of browsers regarding caching permanent redirects, I believe using a 307 Temporary Redirect would be better. This choice avoids potential caching issues, ensuring that clients won't bypass necessary session refresh steps on subsequent requests.

HTTP 307 would maintain the original HTTP method and payload for the redirect, and also provide the client the flexibility to adjust the redirection behavior in the future without concerns of clients caching the old redirect

Describe alternatives you've considered

No response

Additional Context

No response

DuncanDoyle commented 5 months ago

Note that the HTTP/1.1 specification states that (https://www.rfc-editor.org/rfc/rfc2616#section-10.3.3):

RFC 1945 and RFC 2068 specify that the client is not allowed to change the method on the redirected request. However, most existing user agent implementations treat 302 as if it were a 303 response, performing a GET on the Location field-value regardless of the original request method. The status codes 303 and 307 have been added for servers that wish to make unambiguously clear which kind of reaction is expected of the client.

So it seems that 307 is simply an addition to the spec because every browser/user-agent is not correctly processing 302 (i.e. they change the HTTP method to GET). So 307 is processed how 302 "should" be processed, redirect but don't change the HTTP method.

DuncanDoyle commented 5 months ago

@edubonifs I assume this is with OAuth Authorization Code Flow, right?

edubonifs commented 5 months ago

Yes @DuncanDoyle , OAuth Authorization Code Flow

DuncanDoyle commented 5 months ago

Reproducer: https://github.com/DuncanDoyle/ge-gloo-9231

AFAICT, the redirect we do in the "dance" with the IdP (Keycloak in the reproducer) store the endpoint to redirect to in the state JWT. We don't store which HTTP method was originally used for the request. Another potential problem is that, even if we would be able to redirect with the correct method (e.g. a POST in this example), also the original data that was send on the POST request needs to be stored in the state JWT. And this could probably introduce a number of issues:

Tbh, I don't know what the OAuth specification states about this scenario. I'm not sure if it can be supported and how ....