Merge pull request #269 from timtrinidad/allow-getspmetadata-to-always-include-encryption

Allow the getSPMetadata() method to always include the encryption KeyDescriptor

Author: pitbulk

lib/Saml2/Settings.php

@@ -800,11 +800,15 @@ public function shouldCompressResponses()
     /**
      * Gets the SP metadata. The XML representation.
      *
+     * @param bool $alwaysPublishEncryptionCert When 'true', the returned metadata
+     *   will always include an 'encryption' KeyDescriptor. Otherwise, the 'encryption'
+     *   KeyDescriptor will only be included if $advancedSettings['security']['wantNameIdEncrypted']
+     *   or $advancedSettings['security']['wantAssertionsEncrypted'] are enabled. 
      * @return string  SP metadata (xml)
      * @throws Exception
      * @throws OneLogin_Saml2_Error
      */
-    public function getSPMetadata()
+    public function getSPMetadata($alwaysPublishEncryptionCert = false)
     {
         $metadata = OneLogin_Saml2_Metadata::builder($this->_sp, $this->_security['authnRequestsSigned'], $this->_security['wantAssertionsSigned'], null, null, $this->getContacts(), $this->getOrganization());
 
@@ -813,7 +817,7 @@ public function getSPMetadata()
             $metadata = OneLogin_Saml2_Metadata::addX509KeyDescriptors(
                 $metadata,
                 $certNew,
-                $this->_security['wantNameIdEncrypted'] || $this->_security['wantAssertionsEncrypted']
+                $alwaysPublishEncryptionCert || $this->_security['wantNameIdEncrypted'] || $this->_security['wantAssertionsEncrypted']
             );
         }
 
@@ -822,7 +826,7 @@ public function getSPMetadata()
             $metadata = OneLogin_Saml2_Metadata::addX509KeyDescriptors(
                 $metadata,
                 $cert,
-                $this->_security['wantNameIdEncrypted'] || $this->_security['wantAssertionsEncrypted']
+                $alwaysPublishEncryptionCert || $this->_security['wantNameIdEncrypted'] || $this->_security['wantAssertionsEncrypted']
             );
         }
 

tests/src/OneLogin/Saml2/SettingsTest.php

@@ -422,33 +422,78 @@ public function testGetSPMetadata()
     * Case with x509certNew
     *
     * @covers OneLogin_Saml2_Settings::getSPMetadata
+    * @dataProvider testGetSPMetadataWithX509CertNewDataProvider
     */
-    public function testGetSPMetadataWithX509CertNew()
+    public function testGetSPMetadataWithX509CertNew($alwaysIncludeEncryption, $wantNameIdEncrypted, $wantAssertionsEncrypted, $expectEncryptionKeyDescriptor)
     {
         $settingsDir = TEST_ROOT .'/settings/';
         include $settingsDir.'settings5.php';
 
-        $settingsInfo['security']['wantNameIdEncrypted'] = false;
-        $settingsInfo['security']['wantAssertionsEncrypted'] = false;
+        $settingsInfo['security']['wantNameIdEncrypted'] = $wantNameIdEncrypted;
+        $settingsInfo['security']['wantAssertionsEncrypted'] = $wantAssertionsEncrypted;
         $settings = new OneLogin_Saml2_Settings($settingsInfo);
-        $metadata = $settings->getSPMetadata();
+        $metadata = $settings->getSPMetadata($alwaysIncludeEncryption);
 
-        $this->assertEquals(2, substr_count($metadata, "<md:KeyDescriptor"));
+        $this->assertEquals($expectEncryptionKeyDescriptor ? 4 : 2, substr_count($metadata, "<md:KeyDescriptor"));
 
+        // signing KeyDescriptor should always be included
         $this->assertEquals(2, substr_count($metadata, '<md:KeyDescriptor use="signing"'));
 
-        $this->assertEquals(0, substr_count($metadata, '<md:KeyDescriptor use="encryption"'));
-
-        $settingsInfo['security']['wantNameIdEncrypted'] = true;
-        $settingsInfo['security']['wantAssertionsEncrypted'] = true;
-        $settings2 = new OneLogin_Saml2_Settings($settingsInfo);
-        $metadata2 = $settings2->getSPMetadata();
-
-        $this->assertEquals(4, substr_count($metadata2, "<md:KeyDescriptor"));
-
-        $this->assertEquals(2, substr_count($metadata2, '<md:KeyDescriptor use="signing"'));
+        $this->assertEquals($expectEncryptionKeyDescriptor ? 2 : 0, substr_count($metadata, '<md:KeyDescriptor use="encryption"'));
+    }
 
-        $this->assertEquals(2, substr_count($metadata2, '<md:KeyDescriptor use="encryption"'));
+    public function testGetSPMetadataWithX509CertNewDataProvider()
+    {
+        return [
+            'settings do not require encryption' => [
+                'alwaysIncludeEncryption' => false,    
+                'wantNameIdEncrypted' => false,
+                'wantAssertionsEncrypted' => false,
+                'expectEncryptionKeyDescriptor' => false,
+            ],
+            'wantNameIdEncrypted setting enabled' => [
+                'alwaysIncludeEncryption' => false,    
+                'wantNameIdEncrypted' => true,
+                'wantAssertionsEncrypted' => false,
+                'expectEncryptionKeyDescriptor' => true,
+            ],
+            'wantAssertionsEncrypted setting enabled' => [
+                'alwaysIncludeEncryption' => false,    
+                'wantNameIdEncrypted' => false,
+                'wantAssertionsEncrypted' => true,
+                'expectEncryptionKeyDescriptor' => true,
+            ],
+            'both settings enabled'=> [
+                'alwaysIncludeEncryption' => false,    
+                'wantNameIdEncrypted' => true,
+                'wantAssertionsEncrypted' => true,
+                'expectEncryptionKeyDescriptor' => true,
+            ],
+            'metadata requested with encryption' => [
+                'alwaysIncludeEncryption' => true,    
+                'wantNameIdEncrypted' => false,
+                'wantAssertionsEncrypted' => false,
+                'expectEncryptionKeyDescriptor' => true,
+            ],
+           'metadata requested with encryption and wantNameIdEncrypted setting enabled' => [
+                'alwaysIncludeEncryption' => true,    
+                'wantNameIdEncrypted' => true,
+                'wantAssertionsEncrypted' => false,
+                'expectEncryptionKeyDescriptor' => true,
+            ],
+            'metadata requested with encryption and wantAssertionsEncrypted setting enabled' => [
+                'alwaysIncludeEncryption' => true,    
+                'wantNameIdEncrypted' => false,
+                'wantAssertionsEncrypted' => true,
+                'expectEncryptionKeyDescriptor' => true,
+            ],
+            'metadata requested with encryption and both settings enabled' => [
+                'alwaysIncludeEncryption' => true,    
+                'wantNameIdEncrypted' => true,
+                'wantAssertionsEncrypted' => true,
+                'expectEncryptionKeyDescriptor' => true,
+            ],
+        ];
     }
 
     /**