zalando / skipper

An HTTP router and reverse proxy for service composition, including use cases like Kubernetes Ingress
https://opensource.zalando.com/skipper/
Other
3.04k stars 345 forks source link

Handle `302 Found` Redirects when using the webhook filter #3130

Open FergusInLondon opened 4 days ago

FergusInLondon commented 4 days ago

Is your feature request related to a problem? Please describe. The Skipper webhook filter allows requests to be filtered based on the status code received from an authorisation/authentication endpoint. By default, Skipper will return an empty response with a status code of 401 or 403 if a request is rejected.

This behaviour is well-suited to API traffic, but doesn't work for frontend traffic where the user agent should be redirected upon rejection.

Describe the solution you would like When an authentication endpoint returns a HTTP redirect - i.e. a HTTP 302 Found - then Skipper should return that redirect to the user agent.

These changes would be limited to:

  1. Introducing a new conditional to catch responses with a status code of 302 - in webhook.go#L124-L128.
  2. Introducing a new function - redirect - which is the equivalent of the existing reject function - in auth.go#L136-L142 - but copies the Location header from the authentication endpoint response.

This should not be a breaking change as currently 302 Found has no specific meaning to the webhook filter: it's caught by the default behaviour which returns 401 Unauthorized.

Would you like to work on it? Yes, I have a Pull Request prepared - #3131

szuecs commented 2 hours ago

thanks, reviewed the PR, feature wise I think it's fine but implementation needs to be changed as I wrote in PR comment. Discussion in PR.