RelayState Should Not Be Required in Request

[cjduffett]

According to the SAML Profile for Single Sign-On AuthnRequests, the Service Provider MAY send a RelayState along with the request, but that it's not required. See section 4.1.3.1 of this document:
https://docs.oasis-open.org/security/saml/v2.0/saml-profiles-2.0-os.pdf

However, in OneLogin_Saml2_Auth.login() (in auth.py) this parameter is always set, even if return_to is a blank string. For example, a redirect_url returned by login() could look like:

http://acme.onelogin.com/saml/login?SAMLRequest=<b64_encoded_req>&RelayState=

This could potentially be misleading for Identity Providers that detect the RelayState parameter in the request only to discover that it's blank.

I propose that the RelayState parameter should not be set at all if return_to is None, or an empty string (''). If you think that's a reasonable fix, I'll create a PR for you. 🙂

[pitbulk]

So your patch is at login method replace

if return_to is not None:

by

if not return_to:

right?

[cjduffett]

Not quite. I don't think RelayState should be set at all if return_to is blank or not provided. I propose removing the default value of OneLogin_Saml2_Utils.get_self_url_no_query(), so that block reads:

if return_to is not None:
    parameters['RelayState'] = return_to

(without the else clause)

[pitbulk]

But that change can break current environments that uses that toolkit and expect that behavior.

[cjduffett]

After giving it some thought, would it be acceptable to omit the RelayState if RelayState == ''?

This would allow the default None value to continue using the default get_self_url_no_query(), and if it's explicitly set then return_to will still be used. So the logic would instead be:

if return_to is None:
  parameters['RelayState'] = OneLogin_Saml2_Utils.get_self_url_no_query()
elif return_to is not '':
  parameters['RelayState'] = return_to

Something like that? Short of adding a new advanced settings flag to control this, I can't think of a better way to explicitly omit the RelayState from the request.

For my use case I've taken to scrubbing the RelayState from the request after it's returned from the OneLogin_SAML2_Auth client:

def remove_relay_state(url):
    """Removes the RelayState parameter from a SAML login request."""

    if 'RelayState' not in url:
        return url

    # Parse and modify the query parameters
    parts = urlparse(url)
    params = parse_qs(parts.query)
    del params['RelayState']

    # Reconstruct the URL
    query = urlencode(params, doseq=True)
    new_url = urlunparse((
        parts.scheme,
        parts.netloc,
        parts.path,
        parts.params,
        query,
        None, # No fragments
    ))
    return new_url

But that's addressing the symptoms, not the problem.