Fix #443. Incorrect Destination in LogoutResponse when using response…

…Url. Add IdP value getters to the Settings class
SAML-Toolkits · Nov 26, 2020 · 7aca365 · 7aca365
1 parent c51877e
commit 7aca365
Show file tree

Hide file tree

Showing 6 changed files with 115 additions and 22 deletions.
diff --git a/lib/Saml2/Auth.php b/lib/Saml2/Auth.php
@@ -586,43 +586,33 @@ public function logout($returnTo = null, $parameters = array(), $nameId = null,
     }
 
     /**
-     * Gets the SSO url.
+     * Gets the IdP SSO url.
      *
-     * @return string The url of the Single Sign On Service
+     * @return string The url of the IdP Single Sign On Service
      */
     public function getSSOurl()
     {
-        $idpData = $this->_settings->getIdPData();
-        return $idpData['singleSignOnService']['url'];
+        return $this->_settings->getIdPSSOUrl();
     }
 
     /**
-     * Gets the SLO url.
+     * Gets the IdP SLO url.
      *
-     * @return string|null The url of the Single Logout Service
+     * @return string|null The url of the IdP Single Logout Service
      */
     public function getSLOurl()
     {
-        $url = null;
-        $idpData = $this->_settings->getIdPData();
-        if (isset($idpData['singleLogoutService']) && isset($idpData['singleLogoutService']['url'])) {
-            $url = $idpData['singleLogoutService']['url'];
-        }
-        return $url;
+        return $this->_settings->getIdPSLOUrl();
     }
 
     /**
-     * Gets the SLO response url.
+     * Gets the IdP SLO response url.
      *
-     * @return string|null The response url of the Single Logout Service
+     * @return string|null The response url of the IdP Single Logout Service
      */
     public function getSLOResponseUrl()
     {
-        $idpData = $this->_settings->getIdPData();
-        if (isset($idpData['singleLogoutService']) && isset($idpData['singleLogoutService']['responseUrl'])) {
-            return $idpData['singleLogoutService']['responseUrl'];
-        }
-        return $this->getSLOurl();
+        return $this->_settings->getIdPSLOResponseUrl();
     }
 
     /**

diff --git a/lib/Saml2/LogoutRequest.php b/lib/Saml2/LogoutRequest.php
@@ -106,14 +106,15 @@ public function __construct(OneLogin_Saml2_Settings $settings, $request = null,
             $sessionIndexStr = isset($sessionIndex) ? "<samlp:SessionIndex>{$sessionIndex}</samlp:SessionIndex>" : "";
 
             $spEntityId = htmlspecialchars($spData['entityId'], ENT_QUOTES);
+            $destination = $this->_settings->getIdPSLOUrl();
             $logoutRequest = <<<LOGOUTREQUEST
 <samlp:LogoutRequest
     xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
     xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
     ID="{$id}"
     Version="2.0"
     IssueInstant="{$issueInstant}"
-    Destination="{$idpData['singleLogoutService']['url']}">
+    Destination="{$destination}">
     <saml:Issuer>{$spEntityId}</saml:Issuer>
     {$nameIdObj}
     {$sessionIndexStr}

diff --git a/lib/Saml2/LogoutResponse.php b/lib/Saml2/LogoutResponse.php
@@ -240,13 +240,14 @@ public function build($inResponseTo)
         $issueInstant = OneLogin_Saml2_Utils::parseTime2SAML(time());
 
         $spEntityId = htmlspecialchars($spData['entityId'], ENT_QUOTES);
+        $destination = $this->_settings->getIdPSLOResponseUrl();
         $logoutResponse = <<<LOGOUTRESPONSE
 <samlp:LogoutResponse xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
                   xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
                   ID="{$this->id}"
                   Version="2.0"
                   IssueInstant="{$issueInstant}"
-                  Destination="{$idpData['singleLogoutService']['url']}"
+                  Destination="{$destination}"
                   InResponseTo="{$inResponseTo}"
                   >
     <saml:Issuer>{$spEntityId}</saml:Issuer>

diff --git a/lib/Saml2/Settings.php b/lib/Saml2/Settings.php
@@ -838,6 +838,47 @@ public function shouldCompressResponses()
         return $this->_compress['responses'];
     }
 
+    /**
+     * Gets the IdP SSO url.
+     *
+     * @return string|null The url of the IdP Single Sign On Service
+     */
+    public function getIdPSSOUrl()
+    {
+        $ssoUrl = null;
+        if (isset($this->_idp['singleSignOnService']) && isset($this->_idp['singleSignOnService']['url'])) {
+            $ssoUrl = $this->_idp['singleSignOnService']['url'];
+        }
+        return $ssoUrl;
+    }
+
+    /**
+     * Gets the IdP SLO url.
+     *
+     * @return string|null The request url of the IdP Single Logout Service
+     */
+    public function getIdPSLOUrl()
+    {
+        $sloUrl = null;
+        if (isset($this->_idp['singleLogoutService']) && isset($this->_idp['singleLogoutService']['url'])) {
+            $sloUrl = $this->_idp['singleLogoutService']['url'];
+        }
+        return $sloUrl;
+    }
+
+    /**
+     * Gets the IdP SLO response url.
+     *
+     * @return string|null The response url of the IdP Single Logout Service
+     */
+    public function getIdPSLOResponseUrl()
+    {
+        if (isset($this->_idp['singleLogoutService']) && isset($this->_idp['singleLogoutService']['responseUrl'])) {
+            return $this->_idp['singleLogoutService']['responseUrl'];
+        }
+        return $this->getIdPSLOUrl();
+    }
+
     /**
      * Gets the SP metadata. The XML representation.
      *

diff --git a/tests/src/OneLogin/Saml2/AuthTest.php b/tests/src/OneLogin/Saml2/AuthTest.php
@@ -82,7 +82,7 @@ public function testGetSLOurl()
     /**
      * Tests the getSLOResponseUrl method of the OneLogin_Saml2_Auth class
      *
-     * @covers OneLogin_Saml2_Auth::getSLOurl
+     * @covers OneLogin_Saml2_Auth::getSLOResponseUrl
      */
     public function testGetSLOResponseUrl()
     {

diff --git a/tests/src/OneLogin/Saml2/SettingsTest.php b/tests/src/OneLogin/Saml2/SettingsTest.php
@@ -419,6 +419,66 @@ public function testCheckSettings()
         }
     }
 
+    /**
+    * Tests the getIdPSSOurl method of the OneLogin_Saml2_Settings class
+    *
+    * @covers OneLogin_Saml2_Settings::getIdPSSOurl
+    */
+    public function testGetIdPSSOurl()
+    {
+        $settingsDir = TEST_ROOT .'/settings/';
+        include $settingsDir.'settings1.php';
+
+        $settings = new OneLogin_Saml2_Settings($settingsInfo);
+
+        $ssoUrl = "http://idp.example.com/SSOService.php";
+        $this->assertEquals($settings->getIdPSSOUrl(), $ssoUrl);
+    }
+
+    /**
+    * Tests the getIdPSLOurl method of the OneLogin_Saml2_Settings class
+    *
+    * @covers OneLogin_Saml2_Settings::getIdPSLOurl
+    */
+    public function testGetIdPSLOurl()
+    {
+        $settingsDir = TEST_ROOT .'/settings/';
+        include $settingsDir.'settings1.php';
+
+        $settings = new OneLogin_Saml2_Settings($settingsInfo);
+
+        $sloUrl = "http://idp.example.com/SingleLogoutService.php";
+        $this->assertEquals($settings->getIdPSLOUrl(), $sloUrl);
+
+        include $settingsDir.'settings2.php';
+        $settings2 = new OneLogin_Saml2_Settings($settingsInfo);
+
+        $sloUrl = "http://idp.example.com/SingleLogoutService.php";
+        $this->assertEquals($settings2->getIdPSLOUrl(), $sloUrl);
+    }
+
+    /**
+     * Tests the getIdPSLOResponseUrl method of the OneLogin_Saml2_Settings class
+     *
+     * @covers OneLogin_Saml2_Settings::getIdPSLOResponseUrl
+     */
+    public function testGetIdPSLOResponseUrl()
+    {
+        $settingsDir = TEST_ROOT .'/settings/';
+        include $settingsDir.'settings1.php';
+
+        $settings = new OneLogin_Saml2_Settings($settingsInfo);
+
+        $sloUrl = "http://idp.example.com/SingleLogoutServiceResponse.php";
+        $this->assertEquals($settings->getIdPSLOResponseUrl(), $sloUrl);
+
+        include $settingsDir.'settings2.php';
+        $settings2 = new OneLogin_Saml2_Settings($settingsInfo);
+
+        $sloUrl = "http://idp.example.com/SingleLogoutService.php";
+        $this->assertEquals($settings2->getIdPSLOUrl(), $sloUrl);
+    }
+
     /**
     * Tests the getSPMetadata method of the OneLogin_Saml2_Settings
     * Case unsigned metadata