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

Fixed #14895 and #14919 - set SAML baseurl to a sensible default for docker users and users behind a reverse proxy #14974

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

uberbrady
Copy link
Collaborator

For users who are running Snipe-IT behind a reverse-proxy, the SAML implementation guesses an incorrect baseurl (as http:// rather than https://). This change sets it to a default based on the APP_URL - which we rely on elsewhere, as well as here within the SAML code itself. So this should be pretty safe.

Copy link

what-the-diff bot commented Jun 26, 2024

PR Summary

  • Enhancement to the 'loadSettings' method
    The 'loadSettings' function in the 'Saml.php' file now includes a new key called 'baseurl'. The value of this key is derived from our app's URL combined with '/saml', improving the method's ability to dynamically determine the appropriate base URL for SAML functionalities.

@uberbrady
Copy link
Collaborator Author

Unfortunately, this is hard to test since this particular setup is pretty complex to reproduce :/

@snipe snipe changed the title Fix for #14895 and #14919 - set SAML baseurl to a sensible default Fix for #14895 and #14919 - set SAML baseurl to a sensible default for docker users and users behind a reverse proxy Jun 26, 2024
@snipe snipe changed the title Fix for #14895 and #14919 - set SAML baseurl to a sensible default for docker users and users behind a reverse proxy Fixed #14895 and #14919 - set SAML baseurl to a sensible default for docker users and users behind a reverse proxy Jun 26, 2024
@snipe snipe merged commit 74f067d into snipe:develop Jun 26, 2024
9 checks passed
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.

2 participants