samlify icon indicating copy to clipboard operation
samlify copied to clipboard

NO_SELECTED_CERTIFICATE - Certificate not in Signed Response

Open jakeceballos opened this issue 4 years ago • 3 comments

I'm getting the error Error: NO_SELECTED_CERTIFICATE when trying to call sp.parseLoginResponse(idp, 'post', req);.

The IdP metadata contains the certificate to be used for validating the signature.

However, looking at the code it seems samlify is attempting to use the certificate from the SAML Response. But there is no certificate included.

https://github.com/tngan/samlify/blob/b02473541a7ed7b85ee5e830fcbcd758313ccc5e/src/libsaml.ts#L357

I think this may be somewhat related to issue #223 but not certain.

jakeceballos avatar Feb 09 '21 17:02 jakeceballos

I might've found a way to make this work, had to modify the libsaml.js file. I changed the file so that x509CertificateData gets set to the metadataCert value if certificateNode is null.

My quick and dirty change:

var x509CertificateData;

if (metadataCert !== 0) {
    // certificate in metadata
    if (metadataCert.length >= 1 &&
        !metadataCert.find(function (cert) { return cert.trim() === utility_1.default.normalizeCerString(metadataCert); })) {
        // keep this restriction for rolling certificate usage
        // to make sure the response certificate is one of those specified in metadata
        throw new Error('ERROR_UNMATCH_CERTIFICATE_DECLARATION_IN_METADATA');
    }
    x509CertificateData = metadataCert;
} else {
    // certificate in response
    x509CertificateData = certificateNode[0].firstChild.data;
}
var x509Certificate_1 = utility_1.default.normalizeCerString(x509CertificateData);
sig.keyInfoProvider = new _this.getKeyInfo(x509Certificate_1);

jakeceballos avatar Feb 09 '21 19:02 jakeceballos

I'm having the same issue and would be very grateful if a fix is implemented in the library, if it's not supposed to be a requirement that the signature is included in the response (even though I have it in the metadata).

marwej avatar Feb 17 '21 13:02 marwej

maybe also related to: #382

mashpie avatar Mar 17 '21 10:03 mashpie