Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid identity provider metadata #450

Open
damionvega opened this issue Sep 11, 2021 · 8 comments
Open

Invalid identity provider metadata #450

damionvega opened this issue Sep 11, 2021 · 8 comments

Comments

@damionvega
Copy link

damionvega commented Sep 11, 2021

Using identityProvider.getMetadata() produces an error when attempting to upload output XML to https://samltest.id/upload.php:

Metadata Invalid
We're dreadfully sorry, but your metadata upload failed because it didn't pass validation. Here's
what we know about why:

/tmp/phpBRdf6b:14: element SingleLogoutService: Schemas validity error : Element 
'{urn:oasis:names:tc:SAML:2.0:metadata}SingleLogoutService': This element is not expected. 
Expected is ( {urn:oasis:names:tc:SAML:2.0:metadata}SingleSignOnService ).
/tmp/phpBRdf6b fails to validate
Please revisit your submission to ensure that it is valid SAML 2.0 metadata.
@damionvega
Copy link
Author

I've added a test and have a working version here (branched off of v2.7.7 as this seems to be the most stable version): https://github.com/tngan/samlify/compare/v2.7.7...damionvega:fix-single-logout?expand=1

Basically this just reorders SingleLogoutService element to be pushed to the IDPSSODescriptor array before SingleSignOnService: c17a4da#diff-028c827f81868e171bcafbde82666bf88ed21bb77499cbe3a8f7d9f9c12c3277R65-R77.
From what I can tell, it shouldn't harm anything based on https://docs.oasis-open.org/security/saml/v2.0/saml-metadata-2.0-os.pdf page 16 - "2.4.2 Complex Type SSODescriptorType".

I found the actual problem with the original issue created is that samltest.id uses Shibboleth and I noticed your tests here that ServiceProvider accepts elementsOrder. If you'd like, I can make a PR that reworks the IdentityProvider constructor to do the same in order to get the desired results for the IdP metadata output.

@tyler-johnson
Copy link

I don't want to add too much noise, but I am seeing something similar in my tests:

this is not a valid saml response with errors: file_0.xml:1: element AttributeValue: Schemas validity error : Element 'AttributeValue': This element is not expected. Expected is ( {urn:oasis:names:tc:SAML:2.0:assertion}AttributeValue ).

@tngan
Copy link
Owner

tngan commented Sep 20, 2021

@damionvega Please try out the latest v2.8.2 with patches. v2.8.0 and v2.8.1 has import issue after upgrading some dependencies. If the problem still exists, can you export the idp metadata (remove sensitive information for sure), I will double check the schema file.

@tngan tngan added the question label Sep 20, 2021
@damionvega
Copy link
Author

Metadata

Using v2.8.2 idp.getMetadata() (invalid according to https://samltest.id/upload.php)

<EntityDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" xmlns:assertion="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" entityID="https://example.com/saml/idp/metadata">
  <IDPSSODescriptor WantAuthnRequestsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
    <KeyDescriptor use="signing">
      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
        <ds:X509Data>
          <ds:X509Certificate>MIIFszCCA5ugAwIBAgIUAcUMukYL6ovpu8T5M3oVFO/+MoswDQYJKoZIhvcNAQELBQAwTTEaMBgGA1UEAxMRVXNlcmZyb250IEFjY291bnQxGzAZBgNVBAoTEnNhbWx0ZXN0LmlkIHRlbmFudDESMBAGA1UECxMJVXNlcmZyb250MB4XDTIxMDkyMDE3NTIwNVoXDTIyMDkyMDE3NTIwNVowTTEaMBgGA1UEAxMRVXNlcmZyb250IEFjY291bnQxGzAZBgNVBAoTEnNhbWx0ZXN0LmlkIHRlbmFudDESMBAGA1UECxMJVXNlcmZyb250MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAyoEk4LNtXh648gck/V7Ki/mJW/eoRjZrTG/nWAZUMkKwzq2biqkWcwsKQohJbJVzUfcTm2i9kjiYZLZKQG0ELvodF5F1JQB0zURjbckihAVqJ0nFOZd4uLGXECtRXCjior7F1/4jVMY9h8gRjGy5pD2hFWITufXtkwvJxvdIPkkzjwXKgJNZP7uTmTPAELPUDJ5uw2c4FJE3mpPaw/LIHvE/5WQVaSj8Z4/OKl6iCTmu+bqu4X0Nru/ZPX05Pw233We/8nz3eHawAE1OmRxWt8Art5VrA+wkDl8YnNuvo326CoQP2Stmq45j1TVZq1FLYJU/ZP2lJ9ZdV5dovn0opEz5sKw+qMUdn+sFRYnziEZmEnNFxwLMlFK4O7NCKOVNDtILIVUDsUP1b7kqEnTWWenrCpFjwzeQw0VhmQxJQq9QsdXI9p2jKOcU3aC/zjLb71ox2J7a2BeSOUB+d5dWufw04rsM5Yz6w1ceXFER6tshV+y2oGKc8ScK0mwdnTb5LpzuPSS0cKuVVP1ALOF97/wc5MJD1MkvyreIz+erPMY5WKjefJG7oazl3/bSYtTxkzw/5jj8HEom/VdAOuIXhQfT2QlPn54S5EKMtWXIHcJ7kctEA8cuVH4Zag5eusRFeaM6lgCFFRVwpOlHjBDPDheOeSa8WcyeLVTPzOnovIUCAwEAAaOBijCBhzAdBgNVHQ4EFgQUXWaMdoKlVmG5STI40NnN7NAvsOIwCQYDVR0TBAIwADALBgNVHQ8EBAMCAvQwOwYDVR0lBDQwMgYIKwYBBQUHAwEGCCsGAQUFBwMCBggrBgEFBQcDAwYIKwYBBQUHAwQGCCsGAQUFBwMIMBEGCWCGSAGG+EIBAQQEAwIA9zANBgkqhkiG9w0BAQsFAAOCAgEACq/RqUYTLrj5qBoFXYCCt36QGtgqyrv9tlg1WNEkCtX3FzIubQcOB748Hu1M4Vuj0CsR9p0Qg75jK3Hca4YMsoZmAGGawuTvnfY8j4lzlpbbSIDwA84ErGRvPgcU+DW9MUv6HOZH9UoF4AQkjaUxszTkDSGivqeu60zPOq2Bp7796WNjVKm2ouAjXeUl5mncjF6fns8YaUYvxtiCbCViCM3bkk7Zgr28iZ/oTzNuyk/++9Y55KC1wXKU+c0SnzGMeghiXJoPjI/nw7ek3YdTkZeDuiUTf3KvrdRWAsxNtl6E/5Eg7NZAAlNWkuaeYkJ4OBE6ln0Pdy7xnyLPwpRevB31q58dEboqqx8u3NIPrKdKc607OovFsvIS9sZYcmL0V7VHnr/6oHG50v0UPxStzmC00/kMLlgrkc+MHpc978UMUqRtqZPDM1mp4RJ7gyeVGmDrxglN2S7oYXDZx/EXvU3jH/aFzx3ROg+JA7j1oln0wS+umBBKG5ZK5uAl1baED7oFCO88k65CH53kAHD8UuUa2p5l03eTBbEt3DMuo0L4F0j38nqNKNyt/dVFqe9jQdkQbw6fpYkRR/F9WPKvGf0PFaykkQjYujUNtz3y2Nx2unZjjdLTF5c31Tvx24hX+Ex1svV3PQgixCl7BAUYgQWufqjZZMA+neeZxMHkd/Y=</ds:X509Certificate>
        </ds:X509Data>
      </ds:KeyInfo>
    </KeyDescriptor>
    <NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</NameIDFormat>
    <NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</NameIDFormat>
    <SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://example.com/saml/idp/login"/>
    <SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://example.com/saml/idp/login"/>
    <SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://example.com/saml/idp/logout"/>
    <SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://example.com/saml/idp/logout"/>
  </IDPSSODescriptor>
</EntityDescriptor>

Metadata modified

(Moved SingleLogoutService elements above SingleSignOnService)

<EntityDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" xmlns:assertion="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" entityID="https://example.com/saml/idp/metadata">
  <IDPSSODescriptor WantAuthnRequestsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
    <KeyDescriptor use="signing">
      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
        <ds:X509Data>
          <ds:X509Certificate>MIIFszCCA5ugAwIBAgIUAcUMukYL6ovpu8T5M3oVFO/+MoswDQYJKoZIhvcNAQELBQAwTTEaMBgGA1UEAxMRVXNlcmZyb250IEFjY291bnQxGzAZBgNVBAoTEnNhbWx0ZXN0LmlkIHRlbmFudDESMBAGA1UECxMJVXNlcmZyb250MB4XDTIxMDkyMDE3NTIwNVoXDTIyMDkyMDE3NTIwNVowTTEaMBgGA1UEAxMRVXNlcmZyb250IEFjY291bnQxGzAZBgNVBAoTEnNhbWx0ZXN0LmlkIHRlbmFudDESMBAGA1UECxMJVXNlcmZyb250MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAyoEk4LNtXh648gck/V7Ki/mJW/eoRjZrTG/nWAZUMkKwzq2biqkWcwsKQohJbJVzUfcTm2i9kjiYZLZKQG0ELvodF5F1JQB0zURjbckihAVqJ0nFOZd4uLGXECtRXCjior7F1/4jVMY9h8gRjGy5pD2hFWITufXtkwvJxvdIPkkzjwXKgJNZP7uTmTPAELPUDJ5uw2c4FJE3mpPaw/LIHvE/5WQVaSj8Z4/OKl6iCTmu+bqu4X0Nru/ZPX05Pw233We/8nz3eHawAE1OmRxWt8Art5VrA+wkDl8YnNuvo326CoQP2Stmq45j1TVZq1FLYJU/ZP2lJ9ZdV5dovn0opEz5sKw+qMUdn+sFRYnziEZmEnNFxwLMlFK4O7NCKOVNDtILIVUDsUP1b7kqEnTWWenrCpFjwzeQw0VhmQxJQq9QsdXI9p2jKOcU3aC/zjLb71ox2J7a2BeSOUB+d5dWufw04rsM5Yz6w1ceXFER6tshV+y2oGKc8ScK0mwdnTb5LpzuPSS0cKuVVP1ALOF97/wc5MJD1MkvyreIz+erPMY5WKjefJG7oazl3/bSYtTxkzw/5jj8HEom/VdAOuIXhQfT2QlPn54S5EKMtWXIHcJ7kctEA8cuVH4Zag5eusRFeaM6lgCFFRVwpOlHjBDPDheOeSa8WcyeLVTPzOnovIUCAwEAAaOBijCBhzAdBgNVHQ4EFgQUXWaMdoKlVmG5STI40NnN7NAvsOIwCQYDVR0TBAIwADALBgNVHQ8EBAMCAvQwOwYDVR0lBDQwMgYIKwYBBQUHAwEGCCsGAQUFBwMCBggrBgEFBQcDAwYIKwYBBQUHAwQGCCsGAQUFBwMIMBEGCWCGSAGG+EIBAQQEAwIA9zANBgkqhkiG9w0BAQsFAAOCAgEACq/RqUYTLrj5qBoFXYCCt36QGtgqyrv9tlg1WNEkCtX3FzIubQcOB748Hu1M4Vuj0CsR9p0Qg75jK3Hca4YMsoZmAGGawuTvnfY8j4lzlpbbSIDwA84ErGRvPgcU+DW9MUv6HOZH9UoF4AQkjaUxszTkDSGivqeu60zPOq2Bp7796WNjVKm2ouAjXeUl5mncjF6fns8YaUYvxtiCbCViCM3bkk7Zgr28iZ/oTzNuyk/++9Y55KC1wXKU+c0SnzGMeghiXJoPjI/nw7ek3YdTkZeDuiUTf3KvrdRWAsxNtl6E/5Eg7NZAAlNWkuaeYkJ4OBE6ln0Pdy7xnyLPwpRevB31q58dEboqqx8u3NIPrKdKc607OovFsvIS9sZYcmL0V7VHnr/6oHG50v0UPxStzmC00/kMLlgrkc+MHpc978UMUqRtqZPDM1mp4RJ7gyeVGmDrxglN2S7oYXDZx/EXvU3jH/aFzx3ROg+JA7j1oln0wS+umBBKG5ZK5uAl1baED7oFCO88k65CH53kAHD8UuUa2p5l03eTBbEt3DMuo0L4F0j38nqNKNyt/dVFqe9jQdkQbw6fpYkRR/F9WPKvGf0PFaykkQjYujUNtz3y2Nx2unZjjdLTF5c31Tvx24hX+Ex1svV3PQgixCl7BAUYgQWufqjZZMA+neeZxMHkd/Y=</ds:X509Certificate>
        </ds:X509Data>
      </ds:KeyInfo>
    </KeyDescriptor>
    <NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</NameIDFormat>
    <NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</NameIDFormat>
    <SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://example.com/saml/idp/logout"/>
    <SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://example.com/saml/idp/logout"/>
    <SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://example.com/saml/idp/login"/>
    <SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://example.com/saml/idp/login"/>
  </IDPSSODescriptor>
</EntityDescriptor>

@damionvega
Copy link
Author

damionvega commented Sep 20, 2021

OneLogin's validator tool (https://www.samltool.com/validate_xml.php) gave an additional error requiring SingleLogoutService to be above NameIDFormat as well (which is Shibboleth ordering if I remember correctly):

Line: 12 | Column: 0  --> Element '{urn:oasis:names:tc:SAML:2.0:metadata}SingleLogoutService': 
This element is not expected. Expected is one of ( {urn:oasis:names:tc:SAML:2.0:metadata}NameIDFormat,
{urn:oasis:names:tc:SAML:2.0:metadata}SingleSignOnService ).

With that said, I think it would be best if ordering elements could be an additional setting in MetadataIdpOptions (MetadataIdpOptions.elementsOrder) if possible.

@tngan
Copy link
Owner

tngan commented Sep 26, 2021

@damionvega We have elementsOrder options in SP metadata. See if we wanna add one more for identity provider construction.

@damionvega
Copy link
Author

Yes, that would be great if we could add it to the IdP constructor. Is this something that would have to wait until v3? (I saw #451)

@tngan
Copy link
Owner

tngan commented Oct 3, 2021

@damionvega No need. v3 is planned to release early next year.

@tngan tngan added the feature label Oct 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants