feature: allow client redirects via 302 responses (#3130) #3131
Conversation
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 <[email protected]>
|
I'm more than happy to adapt or change the approach if required - especially if there's any reason why the Bringing the |
FergusInLondon
changed the title
| @@ -133,12 +138,16 @@ func reject( | |||
| ctx.Serve(rsp) | |||
| } | |||
|
|
|||
| func redirect(ctx filters.FilterContext, username string, reason rejectReason, destination, debuginfo string) { | |||
| reject(ctx, http.StatusFound, username, reason, "", debuginfo, destination) | |||
There was a problem hiding this comment.
please provide the body of the function and not re-use redirect.
This will add a bit of code, but reduce the size of change.
reject should only be used for rejection as 4xx, thanks!
| @@ -15,15 +15,17 @@ import ( | |||
| ) | |||
|
|
|||
| const headerToCopy = "X-Copy-Header" | |||
| const webhookRedirectLocation = "https://example.com/auth" | |||
There was a problem hiding this comment.
Please use: "https://redirect.example/auth" as by RFC this is in .example, so better than in .com.
szuecs
added
the
major
|
Any plans to merge this change? |
Can you share your use case? |
This commit changes the behaviour of the webhook filter when a 302 Found response is received 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 are included.
Incidental: Prevent the webhook client from following redirects from the AuthN/AuthZ endpoint: during testing I realised that the default
net/httpbehaviour was in use - i.e. redirects were followed. Is this an intended behaviour of the filter: it's not documented, and it seems potentially risky?Please take a look at #3130 for more details.
Changes:
filters.md.reject()function, whereby theLocationheader is forwarded to the client if a302is encountered from the AuthN/AuthZ endpoint.