[project @ Clean up manager in session when response has different URL from request]

3 files changed, 57 insertions, 9 deletions

diff --git a/Auth/OpenID/Consumer.php b/Auth/OpenID/Consumer.php
index 08815a1..a4b7a59 100644
--- a/Auth/OpenID/Consumer.php
+++ b/Auth/OpenID/Consumer.php
@@ -420,7 +420,7 @@ class Auth_OpenID_Consumer {
                 $disco = $this->getDiscoveryObject($this->session,
                                                    $response->identity_url,
                                                    $this->session_key_prefix);
-                $disco->cleanup();
+                $disco->cleanup(true);
             }
         }
 
diff --git a/Auth/Yadis/Manager.php b/Auth/Yadis/Manager.php
index bb60b8a..e6eb571 100644
--- a/Auth/Yadis/Manager.php
+++ b/Auth/Yadis/Manager.php
@@ -435,13 +435,16 @@ class Auth_Yadis_Discovery {
      * Clean up Yadis-related services in the session and return the
      * most-recently-attempted service from the manager, if one
      * exists.
+     *
+     * @param $force True if the manager should be deleted regardless
+     * of whether it's a manager for $this->url.
      */
-    function cleanup()
+    function cleanup($force=false)
     {
-        $manager = $this->getManager();
+        $manager = $this->getManager($force);
         if ($manager) {
             $service = $manager->current();
-            $this->destroyManager();
+            $this->destroyManager($force);
         } else {
             $service = null;
         }
@@ -460,8 +463,11 @@ class Auth_Yadis_Discovery {
 
     /**
      * @access private
+     *
+     * @param $force True if the manager should be returned regardless
+     * of whether it's a manager for $this->url.
      */
-    function &getManager()
+    function &getManager($force=false)
     {
         // Extract the YadisServiceManager for this object's URL and
         // suffix from the session.
@@ -474,7 +480,7 @@ class Auth_Yadis_Discovery {
             $manager = $loader->fromSession(unserialize($manager_str));
         }
 
-        if ($manager && $manager->forURL($this->url)) {
+        if ($manager && ($manager->forURL($this->url) || $force)) {
             return $manager;
         } else {
             $unused = null;
@@ -508,10 +514,13 @@ class Auth_Yadis_Discovery {
 
     /**
      * @access private
+     *
+     * @param $force True if the manager should be deleted regardless
+     * of whether it's a manager for $this->url.
      */
-    function destroyManager()
+    function destroyManager($force=false)
     {
-        if ($this->getManager() !== null) {
+        if ($this->getManager($force) !== null) {
             $key = $this->getSessionKey();
             $this->session->del($key);
         }
diff --git a/Tests/Auth/OpenID/Consumer.php b/Tests/Auth/OpenID/Consumer.php
index bd05c46..8b676ee 100644
--- a/Tests/Auth/OpenID/Consumer.php
+++ b/Tests/Auth/OpenID/Consumer.php
@@ -29,6 +29,21 @@ require_once 'Auth/OpenID/HMACSHA1.php';
 require_once 'Tests/Auth/OpenID/MemStore.php';
 require_once 'PHPUnit.php';
 
+/*
+ * Convenience function to create a SuccessResponse with the given
+ * arguments, all signed.
+ */
+function mkSuccess($endpoint, $q)
+{
+    $signed_list = array();
+    foreach (array_keys($q) as $k) {
+        $signed_list[] = 'openid.' . $k;
+    }
+    return new Auth_OpenID_SuccessResponse($endpoint,
+                                           Auth_OpenID_Message::fromOpenIDArgs($q),
+                                           $signed_list);
+}
+
 class FastConsumerSession extends Auth_OpenID_DiffieHellmanSHA1ConsumerSession {
     function FastConsumerSession($dh = null)
     {
@@ -1667,7 +1682,10 @@ class Tests_Auth_OpenID_ConsumerTest2 extends PHPUnit_TestCase {
 
         // All responses should have the same identity URL, and the
         // session should be cleaned out
-        $this->assertTrue($resp->identity_url == $this->claimed_id);
+        if ($this->endpoint->claimed_id != Auth_OpenID_IDENTIFIER_SELECT) {
+            $this->assertTrue($resp->identity_url == $this->claimed_id);
+        }
+
         $this->assertFalse(in_array($this->consumer->_token_key,
                                     $_SESSION)); // this->session->data));
 
@@ -1688,6 +1706,27 @@ class Tests_Auth_OpenID_ConsumerTest2 extends PHPUnit_TestCase {
         return $resp;
     }
 
+    /*
+     * Be sure that the session gets cleaned up when the response is
+     * successful and has a different URL than the one in the request.
+     */
+    function test_successDifferentURL()
+    {
+        // Set up a request endpoint describing an IDP URL
+        $this->identity_url = 'http://idp.url/';
+        $this->endpoint->claimed_id = $this->endpoint->local_id = Auth_OpenID_IDENTIFIER_SELECT;
+
+        // Use a response endpoint with a different URL (asserted by
+        // the IDP)
+        $resp_endpoint = new Auth_OpenID_ServiceEndpoint();
+        $resp_endpoint->claimed_id = "http://user.url/";
+
+        $resp = $this->_doRespDisco(
+            true,
+            mkSuccess($resp_endpoint, array()));
+        $this->assertTrue($this->discovery->getManager(true) === null);
+    }
+
     function test_noDiscoCompleteSuccessWithToken()
     {
         $message = Auth_OpenID_Message::fromPostArgs(array());