Getting issuer in getSamlOptions(request) of MultiSamlStrategy

[catamphetamine]

Currently, there's MultiSamlStrategy with getSamlOptions(request).
The request is a generic Node.js HTTP IncomingMessage.
But why is it the only argument of getSamlOptions()?
Isn't the point of MultiSamlStrategy existing to choose SAML config based on which SAML server requests authentication?
If yes, then why isn't issuer available in getSamlOptions()?.
Currently, we had to work around that via:

import xml2js from 'xml2js';
async function getIssuerFromSamlAuthenticationRequest(request) {
  const samlResponse = new Buffer(querystring.parse(request.body).SAMLResponse, 'base64');
  const samlResponseXml = await xml2js.parseStringPromise(samlResponse.toString());
  return samlResponseXml['saml2p:Response']['saml2:Issuer'][0]['_'];
}

---

Also, other people @ssbrewster have pointed that out:

#373 (comment)

Decoupling the decryption from validatePostResponse would be great as part of this for e.g. extracting the Issuer from an encrypted message in a MultiSamlStrategy's getSamlOptions method (arguably it could be done before a v2.0).

[stavros-wb]

If yes, then why isn't issuer available in getSamlOptions()?.

The method is called before SAML instantiation, so all the parsing/decrypting and (get kind of request) are not yet available. It's up to the user to select the appropriate configuration based on the request object. Which could be what you've implemented (perfectly fine) or something even simpler like, the actual URL. What we do is have different endpoints per IDP so we can fetch the configurations based on URL params instead of parsing the SAML messages.

[catamphetamine]

Ok, so the reason Issuer is not available is because it's not verified.
I don't think that could be a valid reason but whatever, I have my ways of obtaining it, and that works.

[crossan007]

I'm in the same situation here; I have an unverified SAMLResponse which I want to verify; however, I have multiple potential SAML IdP configurations which could be used to verify the response.

Only one of the providers will actually verify the SAMLResponse (since the cert is unique), but I want to know which provider configuration I should return from getSamlOptions given the current response.

I was hoping Passport could provide the saml2:Issuer out of the response body before validating the signature within MultiSamlStrategy to make it easier to identify which configuration should be used to validate the response.

TypeScript-ified version of the first answer here:

 * In the getSamlOptions method, Passport has not yet parsed the SAML response token,
 * and if the POST endpoint is generic for all IdP's, we have no way of knowing
 * which IdP configuration to use to _verify_ this response; however, we are still able
 * to parse the response and extract the issuer name.  The issuer name can then be used
 * to identify which IdP configuration to return to Passport.
 * 
 * @param request 
 * @returns 
 */
async function getIssuerFromSamlAuthenticationRequest(request: express.Request): Promise<string> {
  const samlResponse = new Buffer(request.body.SAMLResponse, 'base64');
  const samlResponseXml = await xml2js.parseStringPromise(samlResponse.toString());
  return samlResponseXml['saml2p:Response']['saml2:Issuer'][0]['_'];
}

[crossan007]

For anyone looking to solve this problem in a different way: look at using RelayState.

In getSamlOptions , you have access to the request body from the IdP's SAMLResponse.

If you set a RelayState on the SAMLRequest containing the name of the provider configuration you want Passport to use, you can simply consume that value within getSamlOptions

Snippets:

function getAuthProviderNameFromRequest(request: express.Request) {
  if (request.route.path == "/saml/login" && request.query.hasOwnProperty("ProviderName")) {
    // since this function runs as middleware before handling the Express route function
    // we can manipulate the request object to include RelayState of the determined provider name.
    // It would seem that passport will pickup RelayState property from the request object
    // and include it in the request to the IdP, so that it will be posted back to 
    // us with the SAMLResponse to /callback, so we can correctly identify which IdP 
    // issued the token, and correctly identify which configuration to use .
    // helpful answer: https://stackoverflow.com/a/46555155
    request.query.RelayState = JSON.stringify({ProviderName: request.query.ProviderName});
   return request.query.ProviderName;
  }
  else if(request.body.hasOwnProperty("RelayState")) {
    const relayState = JSON.parse(request.body.RelayState);
    return relayState.ProviderName
  }
  throw "No methods available to determine provider name";
}

async function getSamlOptions(request: express.Request, done: SamlOptionsCallback) {
  let providerName: string;
  try{ 
    providerName = getAuthProviderNameFromRequest(request);
  }
  catch (err) {
    throw new Error("Unable to determine SAML provider from request: " + err);
  }
...

const router = express.Router();

passport.use(new MultiSamlStrategy({
  getSamlOptions: getSamlOptions,
  
},

[cjbarth]

@crossan007 that is a valid, spec-compliant way of using RelayState. RelayState is supposed to round-trip from the IdP untouched for just such a purpose. Or, redirecting after login, or whatever, it is your payload.

[tomer-amir]

Just to add a bit, as I am having the same issue.
RelayState on works if you are doing an SP Initiated flow... if the user does an IDP Initiated flow, this will still fail.

I'd be happy if we could still add support for this, but for now I am also parsing the SAMLResponse by myself

[cjbarth]

Add support for what @tomer-amir ? We shouldn't be returning any information from an unverified SAML payload, so I'm not sure what other idea you had in mind for achieving what you're after. Feel free to post a suggestion here and we can see what might work.

[tomer-amir]

Add support for what @tomer-amir ? We shouldn't be returning any information from an unverified SAML payload, so I'm not sure what other idea you had in mind for achieving what you're after. Feel free to post a suggestion here and we can see what might work.

@cjbarth The library can either supply the unverified data or export the parsing method so we can extract the information ourselves. this way we would not need to parse the XML ourselves to fetch the data :)
as I think the IDP initiated flow is very common, I think this will be a common use case for service providers who support multiple IDPs.

WDYT? :)

Edit: I would also add this to the docs

[crossan007]

I don't have the whole flow fresh in my mind here, but I wonder if this would be a good opportunity to accept an optional callback method in the parameter sets that allows the developer to pre-process the unverified claim.

This approach could provide contextual hints about the trustworthiness of the claim, and possibly allow for additional library-enforced safeguards.

[cjbarth]

@crossan007 , please flesh out your ideas and post them here. I've reopened the issue.

[cjbarth]

I should also add that if it is IdP-initiated flow, then you know the URL that the request is coming from, so you should pretty easily be able to pick which federation to use.

I'm going to close this as it is no longer an open issue, but rather a request for features. If someone would like to add a feature, please feel free to open a discussion about that with a proposal and/or open a PR.