Merge pull request SAML-Toolkits#286 from stephanedelprat/patch-1

Change Fatal Error to Exception
HasalaS · Apr 26, 2018 · c211712 · c211712
2 parents 9593164 + 035a3bf
commit c211712
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 6 deletions.
diff --git a/lib/Saml2/LogoutRequest.php b/lib/Saml2/LogoutRequest.php
@@ -150,6 +150,8 @@ public function getRequest($deflate = null)
      * @param string|DOMDocument $request Logout Request Message
      *
      * @return string ID
+     *
+     * @throws OneLogin_Saml2_Error
      */
     public static function getID($request)
     {
@@ -158,6 +160,13 @@ public static function getID($request)
         } else {
             $dom = new DOMDocument();
             $dom = OneLogin_Saml2_Utils::loadXML($dom, $request);
+
+            if (false === $dom) {
+                throw new OneLogin_Saml2_Error(
+                    "LogoutRequest could not be processed",
+                    OneLogin_Saml2_Error::SAML_LOGOUTREQUEST_INVALID
+                );
+            }
         }
 
         $id = $dom->documentElement->getAttribute('ID');

diff --git a/lib/Saml2/LogoutResponse.php b/lib/Saml2/LogoutResponse.php
@@ -42,6 +42,8 @@ class OneLogin_Saml2_LogoutResponse
      *
      * @param OneLogin_Saml2_Settings $settings Settings.
      * @param string|null             $response An UUEncoded SAML Logout response from the IdP.
+     *
+     * @throws OneLogin_Saml2_Error
      */
     public function __construct(OneLogin_Saml2_Settings $settings, $response = null)
     {
@@ -63,6 +65,13 @@ public function __construct(OneLogin_Saml2_Settings $settings, $response = null)
             $this->document = new DOMDocument();
             $this->document = OneLogin_Saml2_Utils::loadXML($this->document, $this->_logoutResponse);
 
+            if (false === $this->document) {
+                throw new OneLogin_Saml2_Error(
+                    "LogoutResponse could not be processed",
+                    OneLogin_Saml2_Error::SAML_LOGOUTRESPONSE_INVALID
+                );
+            }
+
             if ($this->document->documentElement->hasAttribute('ID')) {
                 $this->id = $this->document->documentElement->getAttribute('ID');
             }

diff --git a/tests/src/OneLogin/Saml2/LogoutRequestTest.php b/tests/src/OneLogin/Saml2/LogoutRequestTest.php
@@ -265,7 +265,7 @@ public function testCreateDeflatedSAMLLogoutRequestURLParameter()
     /**
     * Tests the OneLogin_Saml2_LogoutRequest Constructor.
     * Case: Able to generate encryptedID with MultiCert
-    * 
+    *
     * @covers OneLogin_Saml2_LogoutRequest
     */
     public function testConstructorEncryptIdUsingX509certMulti()
@@ -828,7 +828,7 @@ public function testGetXML()
         $logoutRequest = new OneLogin_Saml2_LogoutRequest($settings);
         $xml = $logoutRequest->getXML();
         $this->assertRegExp('#^<samlp:LogoutRequest#', $xml);
-    
+
         $logoutRequestProcessed = new OneLogin_Saml2_LogoutRequest($settings, base64_encode($xml));
         $xml2 = $logoutRequestProcessed->getXML();
         $this->assertRegExp('#^<samlp:LogoutRequest#', $xml2);
@@ -849,9 +849,28 @@ public function testGetID()
         $xml = $logoutRequest->getXML();
         $id1 = OneLogin_Saml2_LogoutRequest::getID($xml);
         $this->assertNotNull($id1);
-    
+
         $logoutRequestProcessed = new OneLogin_Saml2_LogoutRequest($settings, base64_encode($xml));
         $id2 = $logoutRequestProcessed->id;
         $this->assertEquals($id1, $id2);
     }
+
+    /**
+     * Tests that the LogoutRequest throws an exception
+     *
+     * @covers OneLogin_Saml2_LogoutRequest::getID()
+     *
+     * @expectedException OneLogin_Saml2_Error
+     * @expectedExceptionMessage LogoutRequest could not be processed
+     */
+    public function testGetIDException()
+    {
+        $settingsDir = TEST_ROOT .'/settings/';
+        include $settingsDir.'settings1.php';
+
+        $settings = new OneLogin_Saml2_Settings($settingsInfo);
+        $logoutRequest = new OneLogin_Saml2_LogoutRequest($settings);
+        $xml = $logoutRequest->getXML();
+        $id1 = OneLogin_Saml2_LogoutRequest::getID($xml.'<garbage>');
+    }
 }
diff --git a/tests/src/OneLogin/Saml2/LogoutResponseTest.php b/tests/src/OneLogin/Saml2/LogoutResponseTest.php
@@ -437,7 +437,7 @@ public function testWeCanChooseToDeflateAResponseBody()
         //Test that we can choose not to compress the request payload.
         $settingsDir = TEST_ROOT .'/settings/';
         include $settingsDir.'settings1.php';
-        
+
         //Compression is currently turned on in settings.
         $settings = new OneLogin_Saml2_Settings($settingsInfo);
         $logoutResponse = new OneLogin_Saml2_LogoutResponse($settings, $message);
@@ -448,7 +448,7 @@ public function testWeCanChooseToDeflateAResponseBody()
         //Test that we can choose not to compress the request payload.
         $settingsDir = TEST_ROOT .'/settings/';
         include $settingsDir.'settings2.php';
-        
+
         //Compression is currently turned on in settings.
         $settings = new OneLogin_Saml2_Settings($settingsInfo);
         $logoutResponse = new OneLogin_Saml2_LogoutResponse($settings, $message);
@@ -497,9 +497,26 @@ public function testGetID()
         $xml = $logoutResponse->getXML();
         $id1 = $logoutResponse->getID();
         $this->assertNotNull($id1);
-    
+
         $processedLogoutResponse = new OneLogin_Saml2_LogoutResponse($settings, base64_encode($xml));
         $id2 = $processedLogoutResponse->getID();
         $this->assertEquals($id1, $id2);
     }
+
+    /**
+     * Tests that the LogoutRequest throws an exception
+     *
+     * @covers OneLogin_Saml2_LogoutRequest::getID()
+     *
+     * @expectedException OneLogin_Saml2_Error
+     * @expectedExceptionMessage LogoutResponse could not be processed
+     */
+    public function testGetIDException()
+    {
+        $settingsDir = TEST_ROOT .'/settings/';
+        include $settingsDir.'settings1.php';
+
+        $settings = new OneLogin_Saml2_Settings($settingsInfo);
+        $logoutResponse = new OneLogin_Saml2_LogoutResponse($settings, '<garbage>');
+    }
 }