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

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

Hide file tree

Showing 7 changed files with 120 additions and 28 deletions.
diff --git a/src/Saml2/Auth.php b/src/Saml2/Auth.php
@@ -618,46 +618,37 @@ public function logout($returnTo = null, array $parameters = array(), $nameId =
         return $this->redirectTo($sloUrl, $parameters, $stay);
     }
 
-    /**
-     * 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();
     }
 
+
     /**
      * Gets the ID of the last AuthNRequest or LogoutRequest generated by the Service Provider.
      *

diff --git a/src/Saml2/AuthnRequest.php b/src/Saml2/AuthnRequest.php
@@ -55,7 +55,6 @@ public function __construct(\OneLogin\Saml2\Settings $settings, $forceAuthn = fa
         $this->_settings = $settings;
 
         $spData = $this->_settings->getSPData();
-        $idpData = $this->_settings->getIdPData();
         $security = $this->_settings->getSecurityData();
 
         $id = Utils::generateUniqueID();
@@ -150,15 +149,16 @@ public function __construct(\OneLogin\Saml2\Settings $settings, $forceAuthn = fa
 
         $spEntityId = htmlspecialchars($spData['entityId'], ENT_QUOTES);
         $acsUrl = htmlspecialchars($spData['assertionConsumerService']['url'], ENT_QUOTES);
+        $destination = $this->_settings->getIdPSSOUrl();
         $request = <<<AUTHNREQUEST
 <samlp:AuthnRequest
     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"
 {$providerNameStr}{$forceAuthnStr}{$isPassiveStr}
-    IssueInstant="$issueInstant"
-    Destination="{$idpData['singleSignOnService']['url']}"
+    IssueInstant="{$issueInstant}"
+    Destination="{$destination}"
     ProtocolBinding="{$spData['assertionConsumerService']['binding']}"
     AssertionConsumerServiceURL="{$acsUrl}">
     <saml:Issuer>{$spEntityId}</saml:Issuer>{$subjectStr}{$nameIdPolicyStr}{$requestedAuthnStr}

diff --git a/src/Saml2/LogoutRequest.php b/src/Saml2/LogoutRequest.php
@@ -128,14 +128,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/src/Saml2/LogoutResponse.php b/src/Saml2/LogoutResponse.php
@@ -258,19 +258,18 @@ public function build($inResponseTo)
     {
 
         $spData = $this->_settings->getSPData();
-        $idpData = $this->_settings->getIdPData();
 
         $this->id = Utils::generateUniqueID();
         $issueInstant = 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/src/Saml2/Settings.php b/src/Saml2/Settings.php
@@ -825,6 +825,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
@@ -95,7 +95,7 @@ public function testGetSLOurl()
     /**
      * Tests the getSLOResponseUrl method of the 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
@@ -403,6 +403,66 @@ public function testCheckSettings()
         }
     }
 
+    /**
+    * Tests the getIdPSSOurl method of the Settings class
+    *
+    * @covers OneLogin\Saml2\Settings::getIdPSSOurl
+    */
+    public function testGetIdPSSOurl()
+    {
+        $settingsDir = TEST_ROOT .'/settings/';
+        include $settingsDir.'settings1.php';
+
+        $settings = new Settings($settingsInfo);
+
+        $ssoUrl = "http://idp.example.com/SSOService.php";
+        $this->assertEquals($settings->getIdPSSOUrl(), $ssoUrl);
+    }
+
+    /**
+    * Tests the getIdPSLOurl method of the Settings class
+    *
+    * @covers OneLogin\Saml2\Settings::getIdPSLOurl
+    */
+    public function testGetIdPSLOurl()
+    {
+        $settingsDir = TEST_ROOT .'/settings/';
+        include $settingsDir.'settings1.php';
+
+        $settings = new Settings($settingsInfo);
+
+        $sloUrl = "http://idp.example.com/SingleLogoutService.php";
+        $this->assertEquals($settings->getIdPSLOUrl(), $sloUrl);
+
+        include $settingsDir.'settings2.php';
+        $settings2 = new Settings($settingsInfo);
+
+        $sloUrl = "http://idp.example.com/SingleLogoutService.php";
+        $this->assertEquals($settings2->getIdPSLOUrl(), $sloUrl);
+    }
+
+    /**
+     * Tests the getIdPSLOResponseUrl method of the Settings class
+     *
+     * @covers OneLogin\Saml2\Settings::getIdPSLOResponseUrl
+     */
+    public function testGetIdPSLOResponseUrl()
+    {
+        $settingsDir = TEST_ROOT .'/settings/';
+        include $settingsDir.'settings1.php';
+
+        $settings = new Settings($settingsInfo);
+
+        $sloUrl = "http://idp.example.com/SingleLogoutServiceResponse.php";
+        $this->assertEquals($settings->getIdPSLOResponseUrl(), $sloUrl);
+
+        include $settingsDir.'settings2.php';
+        $settings2 = new Settings($settingsInfo);
+
+        $sloUrl = "http://idp.example.com/SingleLogoutService.php";
+        $this->assertEquals($settings2->getIdPSLOUrl(), $sloUrl);
+    }
+
     /**
      * Tests the getSPMetadata method of the Settings
      * Case unsigned metadata