-
Notifications
You must be signed in to change notification settings - Fork 351
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
Handle 302 Found
Redirects when using the webhook filter
#3130
Comments
This commit changes the behaviour of the webhook filter when a 302 Found response is recieved from the AuthN/AuthZ endpoint. As a result, it allows front-end facing (i.e. non-API) traffic to be filtered via the webhook. Documentation updates and increased test coverage is included. Incidental: Prevent the webhook client from following redirects from the AuthN/AuthZ endpoint: during testing I realised that the default `net/http` behaviour was in use - i.e. redirects were followed. Signed-off-by: Fergus Morrow <fergus@ometria.com>
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. |
I can see the point, e.g. if webhook authenticates Cookie and issues redirect for new visitors. From #3131
Its not risky assuming operator trusts the webhook server. Following redirects may be used e.g. for migrating webhook url to another domain. I am not sure I like the idea of forwarding webhook redirect to the client on the other hand there is a usecase. Maybe we can add a third
Also it makes sense to return whole webhook redirect response to the client - this way webhook server can send additional data like: HTTP/1.1 301
Location: https://www.example.test/
Content-type: text/html; charset=UTF-8
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="https://www.example.test/">here</A>.
</BODY></HTML> or just HTTP/1.1 308 Permanent Redirect
Location: https://www.example.test/
Content-Length: 0 |
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:
redirect
- which is the equivalent of the existingreject
function - in auth.go#L136-L142 - but copies theLocation
header from the authentication endpoint response.This should not be a breaking change as currently
302 Found
has no specific meaning to thewebhook
filter: it's caught by the default behaviour which returns401 Unauthorized
.Would you like to work on it?
Yes, I have a Pull Request prepared - #3131
The text was updated successfully, but these errors were encountered: