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

Support reverse proxy the NextCloud way #318

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Toilal
Copy link

@Toilal Toilal commented Apr 5, 2019

In some network configurations involving a reverse proxy, the base url generated by the underlying SAML library is not consistent with the way it's generated in NextCloud.

For example, it may generate http:// urls instead of https:// when the SSL Layer is handled by a proxy, even when NextCloud URLGenerator#getAbsoluteURL effectively generates https:// urls.

This change setup SAML library to use the Server Protocol and Server Host as returned by the NextCloud Request object to build SAML urls properly.

@Toilal Toilal force-pushed the reverse-proxy-fix branch from 9c2773d to 2c4669a Compare April 5, 2019 08:41
@schiessle schiessle requested review from rullzer and blizzz April 15, 2019 08:44
@Toilal Toilal force-pushed the reverse-proxy-fix branch from 2c4669a to a4c5bae Compare April 16, 2019 06:39
@hairmare
Copy link

Safari and Firefox both seem to currently display a security warning due to http only URLs in some redirects. It looks like this PR would fix the issue.

@rullzer rullzer requested a review from schiessle May 28, 2019 20:07
@rullzer
Copy link
Member

rullzer commented May 28, 2019

Right. It makes sense... let me see if I can do a test setup with this.

@Toilal Toilal force-pushed the reverse-proxy-fix branch from a4c5bae to f293513 Compare June 8, 2019 22:35
In some network configurations involving a reverse proxy, the base url generated by the underlying SAML library is not consistent with the way it's generated in NextCloud.

For example, it may generate `http://` urls instead of `https://` when the SSL Layer is handled by a proxy, even when NextCloud URLGenerator#getAbsoluteURL effectively generates `https://` urls.

This change setup SAML library to use the Server Protocol and Server Host as returned by the NextCloud Request object to build SAML urls properly.

Signed-off-by: Rémi Alvergnat <[email protected]>
@Toilal Toilal force-pushed the reverse-proxy-fix branch from f293513 to 17a24e5 Compare June 8, 2019 22:36
@Toilal
Copy link
Author

Toilal commented Jun 8, 2019

I have rebased on current master. @rullzer Have you made some tests with those changes ?

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

Successfully merging this pull request may close these issues.

3 participants