xen-orchestra
xen-orchestra copied to clipboard
SAML redirect_uri pointing to http when xo behind reverse proxy
Describe the bug SAML redirect_uri is redirecting to http instead of https when xo is running behind reverse proxy server with ssl offloading.
To Reproduce Run latest XO (built from sources 5.66.2) behind nginx proxy as reverse proxy with ssl offloading at the proxy level - exactly configured as described in XO documentation: https://xen-orchestra.com/docs/configuration.html#reverse-proxy Configure xo to use SAML authentication against Keycloak via xo's SAML plugin. Hit sign in with same to get redirected to KeyCloak instance and get error about redirect_uri not being https://xo-host/signin/saml/callback but instead http://xo-host/signin/saml/callback which is refused.
Expected behavior Login through saml should supply correct redirect_uri to saml identity provider.
Screenshots /
Versions
- Node: 14.18.3
- xo-server: 5.86.3
- xo-web: 5.91.2
- hypervisor: XCP-ng 8.2.0
Additional context
Built from November 2021 with these version did not have this behavior: XO 5.63 on node 14.17.0 xo-server 5.82.3 xo-web 5.88.0
Maybe related to https://github.com/vatesfr/xen-orchestra/issues/6080
https://github.com/node-saml/passport-saml says:
callbackUrl: full callbackUrl (overrides path/protocol if supplied) path: path to callback; will be combined with protocol and server host information to construct callback url if callbackUrl is not specified (default: /saml/consume) protocol: protocol for callback; will be combined with path and server host information to construct callback url if callbackUrl is not specified (default: http://)
Maybe the protocol should be defind at index.js in xo-server-auth-saml.
It seems to me that there was a diffence in the defaults from version 2 to version 3 with passport-saml.
I've changed the index.js to this and it works for me:
async configure({ usernameField, ...conf }, { loaded }) {
this._usernameField = usernameField
this._conf = {
...this._strategyOptions,
...DEFAULTS,
...conf,
// must match the callback URL
path: '/signin/saml/callback',
protocol: 'https://',
}
Maybe the protocol could also added to the configurationSchema.
i can confirm this issue after upgrading to newest version, we have the same problem as @mhoffmann75 running XOA behind reverse proxy server with ssl offloading.
Using SAML with AzureAD
Versions xoa on docker xo-server: 5.86.3 xo-web: 5.91.2 hypervisor: XCP-ng 8.2.0
Additional context: previous working version. xo-server 5.82.4 xo-web 5.88.0
I've added the callbackUrl to the configurationSchema like
properties: {
callbackUrl: {
title: 'callbackUrl',
description: "the callback URL",
type: 'string',
},
and removed path
in _conf section.
And I set this value to the location for AssertionConsumerService (Shib IdP). For example: https://xoa.example.org/signin/saml/callback
This works for me, even behind an web-proxy with ssl offloading.
Is there a update to this bug? Or is it still not fixed?
Hi @fanuelsen
If you are using XOA (SAML is only available from Enterprise or Premium), please be sure to be on latest
channel. You can also open a support ticket so we can take a look and test it inside your infrastructure.
In case you do not have support on XOA, then I invite you to test the modification provided by @jens-rabe and report back here if it works :+1:
@olivierlambert @jens-rabe
I can confirm that the workaround jens provided works. I added the callbackUrl to the configuration schema in index.js and removed the path: in the conf section. Restarted xoa, logged in with a local user added the callbackurl in the saml plugin config and got it working.
This fix should be added permanently to saml plugin code.
Okay great, so I assume you don't have any pro support. But you provided a PR, which is also a very valid answer ;)
Any reason this modification could break existing SAML installations? We should also be careful on regressions.
If i leave the newly created callbackurl properties in xoa saml plugin empty it redirects to http://(example.org)/saml/consume
So probably it will break? Not sure.
Fixed by #6278.