From c910edbb63c6057d8680fa96cddbd8dd8a75f93f Mon Sep 17 00:00:00 2001
From: Louis <6653109+artonge@users.noreply.github.com>
Date: Mon, 8 Feb 2021 16:57:46 +0100
Subject: [PATCH 1/2] Fix create_metadata output

The upgrade to `xmlbuilder2` broke some generated XMLs, the metadata is part of them as of [AuthNRequest](https://github.com/Clever/saml2/pull/227#issuecomment-772937111), there is probably more if the XML is build with arrays.

Explanation, given this code:
```
root.ele({
  number: [
    "one",
    "two",
    { three: { "@val": 3 } }
  ]
});
```

`xmlbuilder` used to [render array like that](https://github.com/oozcitak/xmlbuilder-js/wiki/Conversion-From-Object#arrays):
```
<number>
  <one/>
  <two/>
  <three val="3"/>
</number>
```

And  `xmlbuilder2` now [renders them like that](https://oozcitak.github.io/xmlbuilder2/object-conversion.html):
```
<number>one</number>
<number>two</number>
<number>
  <three val="3"/>
</number>
```

There was the flag `separateArrayItems` in `xmlbuilder` to change the behavior but I do not see it in `xmlbuilder2`.
---
 lib/saml2.coffee | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/lib/saml2.coffee b/lib/saml2.coffee
index 2c61d4b..ad5c6d7 100644
--- a/lib/saml2.coffee
+++ b/lib/saml2.coffee
@@ -61,10 +61,10 @@ sign_authn_request = (xml, private_key, options) ->
 # Creates metadata and returns it as a string of XML. The metadata has one POST assertion endpoint.
 create_metadata = (entity_id, assert_endpoint, signing_certificates, encryption_certificates) ->
   signing_cert_descriptors = for signing_certificate in signing_certificates or []
-    {'md:KeyDescriptor': certificate_to_keyinfo('signing', signing_certificate)}
+    certificate_to_keyinfo('signing', signing_certificate)
 
   encryption_cert_descriptors = for encryption_certificate in encryption_certificates or []
-    {'md:KeyDescriptor': certificate_to_keyinfo('encryption', encryption_certificate)}
+    certificate_to_keyinfo('encryption', encryption_certificate)
 
   xmlbuilder.create
     'md:EntityDescriptor':
@@ -72,19 +72,16 @@ create_metadata = (entity_id, assert_endpoint, signing_certificates, encryption_
       '@xmlns:ds': XMLNS.DS
       '@entityID': entity_id
       '@validUntil': (new Date(Date.now() + 1000 * 60 * 60)).toISOString()
-      'md:SPSSODescriptor': []
-        .concat {'@protocolSupportEnumeration': 'urn:oasis:names:tc:SAML:1.1:protocol urn:oasis:names:tc:SAML:2.0:protocol'}
-        .concat signing_cert_descriptors
-        .concat encryption_cert_descriptors
-        .concat [
-          'md:SingleLogoutService':
-            '@Binding': 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect'
-            '@Location': assert_endpoint
-          'md:AssertionConsumerService':
-            '@Binding': 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST'
-            '@Location': assert_endpoint
-            '@index': '0'
-        ]
+      'md:SPSSODescriptor':
+        '@protocolSupportEnumeration': 'urn:oasis:names:tc:SAML:1.1:protocol urn:oasis:names:tc:SAML:2.0:protocol'
+        'md:KeyDescriptor': signing_cert_descriptors.concat(encryption_cert_descriptors)
+        'md:SingleLogoutService':
+          '@Binding': 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect'
+          '@Location': assert_endpoint
+        'md:AssertionConsumerService':
+          '@Binding': 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST'
+          '@Location': assert_endpoint
+          '@index': '0'
   .end()
 
 # Creates a LogoutRequest and returns it as a string of xml.

From 5b83ca598eac61f64b7e342c6ceb840c2adea251 Mon Sep 17 00:00:00 2001
From: Mark Cabanero <mark.cabanero@clever.com>
Date: Mon, 8 Feb 2021 10:41:50 -0800
Subject: [PATCH 2/2] test: Add test case to correct for additional
 SPSSODescriptors

When we upgraded to xmlbuilder2, the method we were using to create XML
documents slightly changed. See 57b1486fffa651344fdf1bcecf5750cada0b42d4 and
c910edbb63c6057d8680fa96cddbd8dd8a75f93f for more context.

This change explicitly checks for the amount of nodes that refer to
SPSSODescriptors. Before we upgraded xmlbuilder, there should only be one. When
we upgraded to xmlbuilder2, it registered 5. With this change to
create_metadata, it is now 1 again.
---
 test/saml2.coffee | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/test/saml2.coffee b/test/saml2.coffee
index 9480c7e..72ec00a 100644
--- a/test/saml2.coffee
+++ b/test/saml2.coffee
@@ -135,6 +135,13 @@ describe 'saml2', ->
           has_attribute logout_service, 'Location', 'https://sp.example.com/assert',
           "Expected to find an SingleLogoutService with location 'htps://sp.example.com/assert'")
 
+      it 'contains only one SPSSODescriptor', ->
+        sp_sso_descriptor = entity_descriptor.getElementsByTagNameNS(
+          'urn:oasis:names:tc:SAML:2.0:metadata', 'SPSSODescriptor')
+
+        assert.equal(
+          sp_sso_descriptor.length, 1, "Expected 1 SP SSO descriptor; found #{sp_sso_descriptor.length}")
+
     describe 'format_pem', ->
       it 'formats an unformatted private key', ->
         raw_private_key = (/-----BEGIN PRIVATE KEY-----([^-]*)-----END PRIVATE KEY-----/g.exec get_test_file("test.pem"))[1]