[project @ Peform re-discovery when stored information mismatches response]

3 files changed, 55 insertions, 22 deletions

diff --git a/Auth/OpenID/Consumer.php b/Auth/OpenID/Consumer.php
index 546fe52..7049164 100644
--- a/Auth/OpenID/Consumer.php
+++ b/Auth/OpenID/Consumer.php
@@ -1067,12 +1067,6 @@ class Auth_OpenID_GenericConsumer {
                                                 $to_match->server_url);
         }
 
-        // Fragments do not influence discovery, so we can't compare a
-        // claimed identifier with a fragment to discovered
-        // information.
-        list($defragged_claimed_id, $_) =
-            Auth_OpenID::urldefrag($to_match->claimed_id);
-
         if (!$endpoint) {
             // The claimed ID doesn't match, so we have to do
             // discovery again. This covers not using sessions, OP
@@ -1080,20 +1074,20 @@ class Auth_OpenID_GenericConsumer {
             // the original request.
             // oidutil.log('No pre-discovered information supplied.')
             return $this->_discoverAndVerify($to_match);
-        } else if ($defragged_claimed_id != $endpoint->claimed_id) {
-            // oidutil.log('Mismatched pre-discovered session data. '
-            //             'Claimed ID in session=%s, in assertion=%s' %
-            //             (endpoint.claimed_id, to_match.claimed_id))
-            return $this->_discoverAndVerify($to_match);
-        }
+        } else {
 
-        // The claimed ID matches, so we use the endpoint that we
-        // discovered in initiation. This should be the most common
-        // case.
-        $result = $this->_verifyDiscoverySingle($endpoint, $to_match);
+            // The claimed ID matches, so we use the endpoint that we
+            // discovered in initiation. This should be the most
+            // common case.
+            $result = $this->_verifyDiscoverySingle($endpoint, $to_match);
 
-        if (Auth_OpenID::isFailure($result)) {
-            return $result;
+            if (Auth_OpenID::isFailure($result)) {
+                $endpoint = $this->_discoverAndVerify($to_match);
+
+                if (Auth_OpenID::isFailure($endpoint)) {
+                    return $endpoint;
+                }
+            }
         }
 
         // The endpoint we return should have the claimed ID from the
diff --git a/Tests/Auth/OpenID/Consumer.php b/Tests/Auth/OpenID/Consumer.php
index 70312b6..03163c8 100644
--- a/Tests/Auth/OpenID/Consumer.php
+++ b/Tests/Auth/OpenID/Consumer.php
@@ -1842,6 +1842,16 @@ class IDPDrivenTest extends PHPUnit_TestCase {
     }
 }
 
+global $__test_otherServer_text;
+$__test_otherServer_text = "__test_otherServer";
+class TestDiscoveryVerification_test_otherServer extends Auth_OpenID_GenericConsumer {
+    function _discoverAndVerify($to_match)
+    {
+        global $__test_otherServer_text;
+        return new Auth_OpenID_FailureResponse(null, $__test_otherServer_text);
+    }
+}
+
 class TestDiscoveryVerification extends PHPUnit_TestCase {
     var $services = array();
 
@@ -1885,6 +1895,13 @@ class TestDiscoveryVerification extends PHPUnit_TestCase {
 
     function test_otherServer()
     {
+        global $__test_otherServer_text;
+
+        // setup
+        $this->consumer = new TestDiscoveryVerification_test_otherServer($this->store);
+        $this->consumer->discoverMethod = array($this,
+                                                'discoveryFunc');
+
         // a set of things without the stuff
         $endpoint = new Auth_OpenID_ServiceEndpoint();
         $endpoint->type_uris = array(Auth_OpenID_TYPE_2_0);
@@ -1897,11 +1914,18 @@ class TestDiscoveryVerification extends PHPUnit_TestCase {
                                  $this->message, $endpoint);
 
         $this->assertTrue(Auth_OpenID::isFailure($result));
-        $this->assertTrue(strpos($result->message, 'OP Endpoint mismatch') !== false);
+        $this->assertTrue(strpos($result->message, $__test_otherServer_text) !== false);
     }
 
     function test_foreignDelegate()
     {
+        global $__test_otherServer_text;
+
+        // setup
+        $this->consumer = new TestDiscoveryVerification_test_otherServer($this->store);
+        $this->consumer->discoverMethod = array($this,
+                                                'discoveryFunc');
+
         // a set of things with the server stuff but other delegate
         $endpoint = new Auth_OpenID_ServiceEndpoint();
         $endpoint->type_uris = array(Auth_OpenID_TYPE_2_0);
@@ -1912,7 +1936,7 @@ class TestDiscoveryVerification extends PHPUnit_TestCase {
         $result = $this->consumer->_verifyDiscoveryResults(
                                  $this->message, $endpoint);
         $this->assertTrue(Auth_OpenID::isFailure($result));
-        $this->assertTrue(strpos($result->message, 'local_id mismatch') !== false);
+        $this->assertTrue(strpos($result->message, $__test_otherServer_text) !== false);
     }
 
     function test_nothingDiscovered()
diff --git a/Tests/Auth/OpenID/VerifyDisco.php b/Tests/Auth/OpenID/VerifyDisco.php
index a95a4d3..2974405 100644
--- a/Tests/Auth/OpenID/VerifyDisco.php
+++ b/Tests/Auth/OpenID/VerifyDisco.php
@@ -7,6 +7,14 @@ require_once "Tests/Auth/OpenID/MemStore.php";
 require_once "Auth/OpenID/Message.php";
 require_once "Auth/OpenID/Consumer.php";
 
+class Tests_Auth_OpenID_VerifyDisco_1 extends Auth_OpenID_GenericConsumer {
+    function _discoverAndVerify($to_match)
+    {
+        $this->test_case->assertEquals($this->endpoint->claimed_id, $to_match->claimed_id);
+        return new Auth_OpenID_FailureResponse(null, $this->text);
+    }
+}
+
 class Tests_Auth_OpenID_VerifyDisco extends OpenIDTestMixin {
     var $consumer_class = 'Auth_OpenID_GenericConsumer';
 
@@ -105,20 +113,27 @@ class Tests_Auth_OpenID_VerifyDisco extends OpenIDTestMixin {
 
     function test_openid2UsePreDiscoveredWrongType()
     {
+        $this->consumer =& new Tests_Auth_OpenID_VerifyDisco_1($this->store);
+        $this->consumer->test_case =& $this;
+        $this->consumer->text = "verify failed";
+
         $endpoint = new Auth_OpenID_ServiceEndpoint();
         $endpoint->local_id = 'my identity';
         $endpoint->claimed_id = 'i am sam';
         $endpoint->server_url = 'Phone Home';
         $endpoint->type_uris = array(Auth_OpenID_TYPE_1_1);
 
+        $this->consumer->endpoint =& $endpoint;
+
         $msg = Auth_OpenID_Message::fromOpenIDArgs(
               array('ns' => Auth_OpenID_OPENID2_NS,
                     'identity' => $endpoint->local_id,
                     'claimed_id' => $endpoint->claimed_id,
                     'op_endpoint' => $endpoint->server_url));
 
-        $this->failUnlessProtocolError(
-           $this->consumer->_verifyDiscoveryResults($msg, $endpoint));
+        $result = $this->consumer->_verifyDiscoveryResults($msg, $endpoint);
+        $this->failUnlessProtocolError($result);
+        $this->assertTrue($result->message == "verify failed");
     }
 
     function test_openid1UsePreDiscovered()