saml_relay_state_url redirect

[scott-davidjones]

Hey,

Not sure its an issue but when authenticated i am redirected back to the SP as exepected but it in turn is throwing a 404.

Url i am being redirected back to is passed via the saml_relay_state_url param which inturn seems to be a base 64 encode of RelayState GET parameter.

problem that i am getting is that it base 64 encodes the RelayState (which in my case is a url with get params) and then the returned base64 encoded strign is sent as part of the redirect back to the SP which then tries to load it and thus throws a 404.

A worked through example:
SP = https://autumndev.docebosaas.com

1 - go to https://autumndev.docebosaas.com and try to login via SAML SSO.
2 - authenticate on the craft site
3 - RelayState is https://autumndev.docebosaas.com/lms/index.php?r=site/sso&sso_type=saml
4 - RelayState is base 64 encoded to produce: aHR0cHM6Ly9hdXR1bW5kZXYuZG9jZWJvc2Fhcy5jb20vbG1zL2luZGV4LnBocD9yPXNpdGUvc3NvJnNzb190eXBlPXNhbWw=
5 - authentication works and user is redirected back to https://autumndev.docebosaas.com
6 - SP then redirects to the saml_relay_state_url: https://autumndev.docebosaas.com/lms/aHR0cHM6Ly9hdXR1bW5kZXYuZG9jZWJvc2Fhcy5jb20vbG1zL2luZGV4LnBocD9yPXNpdGUvc3NvJnNzb190eXBlPXNhbWw=

End point = 404 not found.

Now Docebo have stated to make the saml_relay_state_url = "https://autumndev.docebosaas.com" which should allieviate the reditect to 404.

Is it possible to overwrite the saml_relay_state_url?

[dsmrt]

Just copied this from the other issue:
froma quick look the RelayState parameter shouldnt be touched (https://stackoverflow.com/questions/34350160/what-is-exactly-relaystate-parameter-used-in-sso-ex-saml)

But the IDP should, as the documentation says, just relay it back.

so i believe it shouldnt be base64 encoded

[scott-davidjones]

thanks, didnt realise i was on an different issue..

Further support for change:

http://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf - section 3.1.1 (line 225)

Use of RelayStateSome bindings define a "RelayState" mechanism for preserving and conveying state information. Whensuch a mechanism is used in conveying a request message as the initial step of a SAML protocol, itplaces requirements on the selection and use of the binding subsequently used to convey the response.Namely, if a SAML request message is accompanied by RelayState data, then the SAML responderMUST return its SAML protocol response using a binding that also supports a RelayState mechanism, andit MUST place the exact RelayState data it received with the request into the corresponding RelayStateparameter in the response.

[dsmrt]

I have to look thru the ramifications of why I'm base64'ing it but I think what you found looks correct. I should just pass it back as I received it. I won't get to this until the next couple weeks due to the holiday, but I will probably just return whatever I find here:
https://github.com/flipboxfactory/saml-idp/blob/master/src/controllers/LoginController.php#L162

    /**
     * @return string
     */
    protected function getRelayState(): string
    {
        return \Craft::$app->request->getParam('RelayState');
    }

Test that out on your side if you'd like. Let me know how that goes if you do.

Thanks!

[scott-davidjones]

updated my PR: #5

all works as expected from my end. I can go back to the client with a P.O.C. and tell them not to update the plguin at all just yet. then move over to the offical release once its released. Happy to help out if needed/wanted.

[dsmrt]

Accepted pull request into master