Skip to content
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

Do not copy hop-by-hop headers to forward auth request #3907

Merged
merged 1 commit into from Sep 24, 2018
Merged

Do not copy hop-by-hop headers to forward auth request #3907

merged 1 commit into from Sep 24, 2018

Conversation

stffabi
Copy link
Contributor

@stffabi stffabi commented Sep 20, 2018

What does this PR do?

Hop-By-Hop headers aren't passed over to the auth backend anymore.

Motivation

Copying over Hop-By-Hop header might result in a wrong behaviour, e.g. if a websocket is requested, the auth backend request fails and a HTTP 500 error code is returned. Therefore websocket don't work anymore when a forward auth backend is enabled.

Fixes #3039

More

  • Added/updated tests
  • Added/updated documentation

@emilevauge
Copy link
Member

Hi @stffabi, thanks for your contribution.
We already merged your PR from 1.7 branch #3900 in master two days ago , so I will close this one :)

@stffabi
Copy link
Contributor Author

stffabi commented Sep 20, 2018

Hi @emilevauge thanks for your message and for merging the other PR. Unfortunately this is another fix :). #3900 removed the hop-by-hop headers from the response of the auth backend. Whereas this new PR prevents adding hop-by-hop headers to the request that is sent to the auth backend. So this PR makes sure no hop-by-hop headers get send to the auth backend, the other one (#3900) makes sure no hop-by-hop headers received from the auth backend get send back to the receiver.

@ldez ldez reopened this Sep 20, 2018
@ldez ldez added kind/bug/fix a bug fix and removed resolution/duplicate labels Sep 20, 2018
@ldez ldez changed the base branch from master to v1.7 September 20, 2018 17:43
@ldez ldez added this to the 1.7 milestone Sep 20, 2018
@emilevauge
Copy link
Member

@stffabi Sorry, I read too quickly ;)

Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@juliens juliens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@emilevauge emilevauge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@traefiker traefiker merged commit 29473ef into traefik:v1.7 Sep 24, 2018
@stffabi stffabi deleted the remove-hop-by-hop-headers branch September 24, 2018 08:52
@stffabi
Copy link
Contributor Author

stffabi commented Sep 24, 2018

@emilevauge thanks a lot for merging the PR. Awesome how fast all of you reply and how much attention all of you pay to community contributions. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants