traefik
traefik copied to clipboard
add forwardAuth.addAuthCookiesToResponse
What does this PR do?
Add the possibility to configure cookie names on ForwardAuth middleware, which gets copied from auth response to the client response. This will solve the issue, that the auth server can't refresh cookies, as described in #3660
Motivation
Based on the work from @iamolegga in PR #6893 and the comment https://github.com/traefik/traefik/issues/3660#issuecomment-896031575 in #3660 I implemented this solution.
More
- [x] Added/updated tests
- [x] Added/updated documentation
Additional Notes
- As all posts in the original issue #3660 "only" needs the
Set-cookieheader, and not any other header from auth server response, the PR only copy cookies. - As a new cookie from auth server can be security relevant (i.e. a changed session cookie), this cookie has always a higher priority and will overwrite a cookie with same name from a backend.
Thomas Gunsch [email protected] Mercedes-Benz Tech Innovation GmbH (ProviderInformation)
Hello @tgunsch,
Thanks for your interest in Traefik and for your contribution!
This PR is labeled as needs-design-review,
as you can see in our contribution guide, this means that we have to take a longer look to evaluate how it would interact with the other parts of Traefik.
We will come back to you once the design review iteration is done.
Shouldn't be taken as this being accepted, and I can't talk on behalf of the authors, however this is a better pattern as it reduces complexity and indentation in my opinion. Also the check becomes part of the newup of the ForwardAuth rather than on every request.
Was just passing through and noticed this.
Hi @james-d-elliott , thanks for you feedback. I was on vacation last three weeks and we will have a look on it next few days.
Have the same needs https://github.com/traefik/traefik/issues/3660 and am trying to understand where this PR is at and how I could be of help. Is it still awaiting design review?
I just wanted to let everyone know that we are interested in moving forward with this, but we are currently prioritizing a different focus.
If you want to help us move this forward faster, we would love help with a review. How it would work, assessments on what the risks are, what areas will be impacted, and writing tests to evaluate this would be a huge help. @nmengin can give more detailed information if you are interested.
I just wanted to let everyone know that we are interested in moving forward with this, but we are currently prioritizing a different focus.
If you want to help us move this forward faster, we would love help with a review. How it would work, assessments on what the risks are, what areas will be impacted, and writing tests to evaluate this would be a huge help. @nmengin can give more detailed information if you are interested.
Happy to help here. I've tested the changes for my specific use case but can do a more comprehensive review, please let me know what I should be looking at.
Hello @tgunsch,
Thanks for the contribution and sorry for the delay, we are moving this pull request to the status/2-needs-review state, to have it an upcoming Traefik version.
To move this pull request forward, can we take over it to make some changes?
Hello @tgunsch,
Thanks for the contribution and sorry for the delay, we are moving this pull request to the
status/2-needs-reviewstate, to have it an upcoming Traefik version.To move this pull request forward, can we take over it to make some changes?
Hi @kevinpollet ,
perfect, thank you very much. For sure you can take the PR over. We waiting eagerly for the upcoming version :partying_face: