SAML: IdP initiated requests do not work

[pavelnikolov]

We use OneLogin for SAML authentication. When an user is authenticated against OneLogin and clicks the Sign in link in Confidant - he/she is automatically authenticated. However, if the user is in the OneLogin dashboard and clicks on the Confidant icon (the authentication is initiated from outside of Confidant) - an error is displayed:

{
  "errors": [
    "invalid_response"
  ], 
  "message": "SAML request failed", 
  "reason": "No AuthNRequest ID from SP found to match with InResponseTo of response"
}

I think that authentication with IdP initiated requests is not currently supported by Confidant. I am not a Python expert but I believe that in order to fix this issue you need to change this piece of code:

        try:
            request_id = session['saml_authn_request_id']
        except KeyError:
            logging.warning('No saml_authn_request_id in session')
            resp = jsonify(errors=['invalid_response'],
                           message='SAML request failed',
                           reason=('No AuthNRequest ID from SP found '
                                   'to match with InResponseTo of response'))
            resp.status_code = 401
            return resp

Instead of always throwing an exception when the request_id is not found, you should first check if the response contains InResponseTo="..." field and only if it does, then throw an exception.

<samlp:Response xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
                xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
                ID="R6814b187947641b39d27763029820d5527xxxxx"
                Version="2.0"
                IssueInstant="2017-03-29T22:00:09Z"
                Destination="https://<our_host_here>/v1/saml/consume"
                InResponseTo="ONELOGIN_7c4612b6a6361c50f10e2aa2a04f0b5f7dxxxxxx"
                >

The InResponseTo field is not present when the authentication request is initiated from the IdP (e.g. Onelogin).

[ryan-lane]

Hey @pavelnikolov. There's a minor vulnerability opened by doing so, and the underlying library doesn't have support for choosing whether to open or close it, but @vivianho tracked this down and opened an issue with python-saml: SAML-Toolkits/python-saml#188

The vulnerability requires interception, though, so overall we don't feel like this changes the threat model. Based on that assessment, we're going to loosen up the restriction here and we'll update the library in the requirements when the setting becomes available in a release, for those who want to close this.

[ryan-lane]

Sorry, we're digging into this a bit deeper. OWASP recommendation is to never accept an assertion without InResponseTo set to avoid man-in-the-middle, potentially from malicious SPs.

[ryan-lane]

Following up on this, IdP initiated login introduces a login CSRF vulnerability that could be used to log a user in via CSRF, and could also be used to escalate privileges of that user. Escalation of privileges isn't an issue in confidant right now, as we don't have access controls, but we do have plans on adding support for segregated user classes in the future.

Based on that, I'm going to update the attached PR a bit to wrap this around a setting, that will default to false.