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

Compatible with proxy scenarios #2601

Closed
wants to merge 1 commit into from
Closed

Conversation

cabbage89
Copy link
Contributor

When using a proxy to the backend, read the X-Forwarded-Proto request header

Before submitting any pull request, please read the contribution guide: https://www.pac4j.org/docs/contribute.html

When using a proxy to the backend, read the X-Forwarded-Proto request header
@cabbage89
Copy link
Contributor Author

@mmoayyed
Copy link
Contributor

mmoayyed commented Jun 2, 2023

I don't think we are looking at the 5.4.x release any more. You should consider repositioning your PR to target master.

@papegaaij
Copy link
Contributor

Also, this seems incorrect. It is the responsibility of the servlet container to interpret headers like X-Forwarded-Proto. Often, this needs to be enabled explicitly. These headers can be inserted by a malicious client and spoof the server into taken a wrong scheme.

@cabbage89 you probably need to check the documentation of whatever servlet container you are using. For example, on WildFly, you have to set proxy-address-forwarding="true" on the http-listener.

@leleuj
Copy link
Member

leleuj commented Jun 2, 2023

Yes, you should target both the master and 5.7.x branches.

@leleuj
Copy link
Member

leleuj commented Jun 2, 2023

And yes, this is a bit too hardcoded to be accepted "as is".

Though, thanks to the WebContextFactory, you should be able to instantiate any WebContext you want so the idea is certainly to open the JEEContext to allow this customisation.

@cabbage89
Copy link
Contributor Author

As you said, I have solved my problem in another way, sorry to bother you, I will submit PR more carefully in the future

@cabbage89 cabbage89 closed this Jun 6, 2023
@leleuj
Copy link
Member

leleuj commented Jun 6, 2023

No problem. PRs are meant to be discussed.

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