[project @ discovery verification changes]

 - OpenID 1 only: Add claimed_id to return_to in requests
 - OpenID 1, 2: Do discovery on response's claimed_id in the absence of
   session storage
 - OpenID 1: Do discovery verification on claimed_id in response rather than
   the one in the session

3 files changed, 109 insertions, 37 deletions

diff --git a/Auth/OpenID/Consumer.php b/Auth/OpenID/Consumer.php
index 9e2a451..08815a1 100644
--- a/Auth/OpenID/Consumer.php
+++ b/Auth/OpenID/Consumer.php
@@ -399,7 +399,7 @@ class Auth_OpenID_Consumer {
      * indicated by the status attribute, which will be one of
      * SUCCESS, CANCEL, FAILURE, or SETUP_NEEDED.
      */
-    function complete($query=null)
+    function complete($query=null, $return_to=null)
     {
         if ($query === null) {
             $query = Auth_OpenID::getQuery();
@@ -410,14 +410,9 @@ class Auth_OpenID_Consumer {
         $endpoint =
             $loader->fromSession($endpoint_data);
 
-        if ($endpoint === null) {
-            $response = new Auth_OpenID_FailureResponse(null,
-                                                   'No session state found');
-        } else {
-            $message = Auth_OpenID_Message::fromPostArgs($query);
-            $response = $this->consumer->complete($message, $endpoint);
-            $this->session->del($this->_token_key);
-        }
+        $message = Auth_OpenID_Message::fromPostArgs($query);
+        $response = $this->consumer->complete($message, $endpoint, $return_to);
+        $this->session->del($this->_token_key);
 
         if (in_array($response->status, array(Auth_OpenID_SUCCESS,
                                               Auth_OpenID_CANCEL))) {
@@ -575,6 +570,13 @@ class Auth_OpenID_GenericConsumer {
     var $openid1_nonce_query_arg_name = 'janrain_nonce';
 
     /**
+     * Another query parameter that gets added to the return_to for
+     * OpenID 1; if the user's session state is lost, use this claimed
+     * identifier to do discovery when verifying the response.
+     */
+    var $openid1_return_to_identifier_name = 'openid1_claimed_id';
+
+    /**
      * This method initializes a new {@link Auth_OpenID_Consumer}
      * instance to access the library.
      *
@@ -615,6 +617,12 @@ class Auth_OpenID_GenericConsumer {
         $r = new Auth_OpenID_AuthRequest($service_endpoint, $assoc);
         $r->return_to_args[$this->openid1_nonce_query_arg_name] =
             Auth_OpenID_mkNonce();
+
+        if ($r->message->isOpenID1()) {
+            $r->return_to_args[$this->openid1_return_to_identifier_name] =
+                $r->endpoint->claimed_id;
+        }
+
         return $r;
     }
 
@@ -946,12 +954,17 @@ class Auth_OpenID_GenericConsumer {
      */
     function _verifyDiscoveryResultsOpenID1($message, $endpoint)
     {
-        if ($endpoint === null) {
+        $claimed_id = $message->getArg(Auth_OpenID_BARE_NS,
+                                $this->openid1_return_to_identifier_name);
+
+        if (($endpoint === null) && ($claimed_id === null)) {
             return new Auth_OpenID_FailureResponse($endpoint,
               'When using OpenID 1, the claimed ID must be supplied, ' .
               'either by passing it through as a return_to parameter ' .
               'or by using a session, and supplied to the GenericConsumer ' .
               'as the argument to complete()');
+        } else if (($endpoint !== null) && ($claimed_id === null)) {
+            $claimed_id = $endpoint->claimed_id;
         }
 
         $to_match = new Auth_OpenID_ServiceEndpoint();
@@ -960,7 +973,7 @@ class Auth_OpenID_GenericConsumer {
                                                'identity');
 
         // Restore delegate information from the initiation phase
-        $to_match->claimed_id = $endpoint->claimed_id;
+        $to_match->claimed_id = $claimed_id;
 
         if ($to_match->local_id === null) {
             return new Auth_OpenID_FailureResponse($endpoint,
@@ -970,16 +983,28 @@ class Auth_OpenID_GenericConsumer {
         $to_match_1_0 = $to_match->copy();
         $to_match_1_0->type_uris = array(Auth_OpenID_TYPE_1_0);
 
-        $result = $this->_verifyDiscoverySingle($endpoint, $to_match);
+        if ($endpoint !== null) {
+            $result = $this->_verifyDiscoverySingle($endpoint, $to_match);
 
-        if (is_a($result, 'Auth_OpenID_TypeURIMismatch')) {
-            $result = $this->_verifyDiscoverySingle($endpoint, $to_match_1_0);
+            if (is_a($result, 'Auth_OpenID_TypeURIMismatch')) {
+                $this->_verifyDiscoverySingle($endpoint, $to_match_1_0);
+            } else if (Auth_OpenID::isFailure($result)) {
+                // oidutil.log("Error attempting to use stored
+                //             discovery information: " + str(e))
+                //             oidutil.log("Attempting discovery to
+                //             verify endpoint")
+            } else {
+                return $endpoint;
+            }
         }
 
-        if (Auth_OpenID::isFailure($result)) {
-            return $result;
+        // Endpoint is either bad (failed verification) or None
+        $result = $this->_discoverAndVerify($to_match);
+
+        if (is_a($result, 'Auth_OpenID_TypeURIMismatch')) {
+            return $this->_discoverAndVerify($to_match_1_0);
         } else {
-            return $endpoint;
+            return $result;
         }
     }
 
diff --git a/Tests/Auth/OpenID/Consumer.php b/Tests/Auth/OpenID/Consumer.php
index 0578e4b..bd05c46 100644
--- a/Tests/Auth/OpenID/Consumer.php
+++ b/Tests/Auth/OpenID/Consumer.php
@@ -920,6 +920,20 @@ class Tests_Auth_OpenID_Complete extends _TestIdRes {
         $this->assertEquals($r->status, Auth_OpenID_FAILURE);
         $this->assertEquals($r->identity_url, $this->consumer_id);
     }
+}
+
+class _VerifiedError extends Auth_OpenID_FailureResponse {
+}
+
+class Consumer_idResURLMismatch extends Auth_OpenID_GenericConsumer {
+    function _discoverAndVerify($to_match)
+    {
+        return new _VerifiedError(null, 'verified error');
+    }
+}
+
+class Tests_idResURLMismatch extends _TestIdRes {
+    var $consumer_class = 'Consumer_idResURLMismatch';
 
     function test_idResURLMismatch()
     {
@@ -932,9 +946,7 @@ class Tests_Auth_OpenID_Complete extends _TestIdRes {
 
         $message = Auth_OpenID_Message::fromPostArgs($query);
         $r = $this->consumer->complete($message, $this->endpoint);
-        $this->assertEquals($r->status, Auth_OpenID_FAILURE);
-        $this->assertEquals($r->identity_url, $this->consumer_id);
-        $this->assertTrue(strpos($r->message, 'local_id') !== false);
+        $this->assertTrue(is_a($r, '_VerifiedError'));
     }
 }
 
@@ -1578,6 +1590,17 @@ class Tests_Auth_OpenID_DiscoFailure extends PHPUnit_TestCase {
     }
 }
 
+class Consumer_completeEmptySession extends Auth_OpenID_GenericConsumer {
+    var $test_case = null;
+    var $text = "failed complete";
+
+    function complete($message, $endpoint, $return_to=null)
+    {
+        $this->test_case->assertTrue($endpoint === null);
+        return new Auth_OpenID_FailureResponse($endpoint, $this->text);
+    }
+}
+
 class Tests_Auth_OpenID_ConsumerTest2 extends PHPUnit_TestCase {
     function setUp()
     {
@@ -1622,8 +1645,12 @@ class Tests_Auth_OpenID_ConsumerTest2 extends PHPUnit_TestCase {
 
     function test_completeEmptySession()
     {
+        $this->consumer->consumer = new Consumer_completeEmptySession($this->store);
+        $this->consumer->consumer->test_case =& $this;
+
         $response = $this->consumer->complete(array());
-        $this->assertEquals($response->status, Auth_OpenID_FAILURE);
+        $this->assertTrue(Auth_OpenID::isFailure($response));
+        $this->assertEquals($this->consumer->consumer->text, $response->message);
         $this->assertTrue($response->identity_url === null);
     }
 
@@ -2202,6 +2229,7 @@ $Tests_Auth_OpenID_Consumer_other = array(
                                           new IDPDrivenTest(),
                                           new TestDiscoveryVerification(),
                                           new Tests_Auth_OpenID_KVPost(),
+                                          new Tests_idResURLMismatch(),
                                           );
 
 if (!defined('Auth_OpenID_NO_MATH_SUPPORT')) {
diff --git a/Tests/Auth/OpenID/VerifyDisco.php b/Tests/Auth/OpenID/VerifyDisco.php
index 2974405..9d14f7e 100644
--- a/Tests/Auth/OpenID/VerifyDisco.php
+++ b/Tests/Auth/OpenID/VerifyDisco.php
@@ -15,7 +15,17 @@ class Tests_Auth_OpenID_VerifyDisco_1 extends Auth_OpenID_GenericConsumer {
     }
 }
 
-class Tests_Auth_OpenID_VerifyDisco extends OpenIDTestMixin {
+class __VerifiedError extends Auth_OpenID_FailureResponse {
+}
+
+class VerifyDisco_Consumer_verifiedError extends Auth_OpenID_GenericConsumer {
+    function _discoverAndVerify($to_match)
+    {
+        return new __VerifiedError(null, 'verified error');
+    }
+}
+
+class _DiscoverAndVerify extends OpenIDTestMixin {
     var $consumer_class = 'Auth_OpenID_GenericConsumer';
 
     function setUp()
@@ -40,7 +50,9 @@ class Tests_Auth_OpenID_VerifyDisco extends OpenIDTestMixin {
     {
         $this->assertTrue(Auth_OpenID::isFailure($thing));
     }
+}
 
+class Tests_Auth_OpenID_VerifyDisco extends _DiscoverAndVerify {
     function test_openID1NoLocalID()
     {
         $endpoint = new Auth_OpenID_ServiceEndpoint();
@@ -151,21 +163,6 @@ class Tests_Auth_OpenID_VerifyDisco extends OpenIDTestMixin {
         $this->assertTrue($result == $endpoint);
     }
 
-    function test_openid1UsePreDiscoveredWrongType()
-    {
-        $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_2_0);
-
-        $msg = Auth_OpenID_Message::fromOpenIDArgs(
-            array('ns' => Auth_OpenID_OPENID1_NS,
-                  'identity' => $endpoint->local_id));
-        $this->failUnlessProtocolError(
-            $this->consumer->_verifyDiscoveryResults($msg, $endpoint));
-    }
-
     function test_openid2Fragment()
     {
         $claimed_id = "http://unittest.invalid/";
@@ -194,6 +191,27 @@ class Tests_Auth_OpenID_VerifyDisco extends OpenIDTestMixin {
 
 }
 
+class Tests_openid1UsePreDiscoveredWrongType extends _DiscoverAndVerify {
+    var $consumer_class = 'VerifyDisco_Consumer_verifiedError';
+
+    function test_openid1UsePreDiscoveredWrongType()
+    {
+        $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_2_0);
+
+        $msg = Auth_OpenID_Message::fromOpenIDArgs(
+            array('ns' => Auth_OpenID_OPENID1_NS,
+                  'identity' => $endpoint->local_id));
+
+        $result = $this->consumer->_verifyDiscoveryResults($msg, $endpoint);
+        $this->failUnlessProtocolError($result);
+        $this->assertTrue(is_a($result, '__VerifiedError'));
+    }
+}
+
 // XXX: test the implementation of _discoverAndVerify
 
 class Tests_openID2NoEndpointDoesDisco_sentinel extends Auth_OpenID_GenericConsumer {
@@ -352,6 +370,7 @@ $Tests_Auth_OpenID_VerifyDisco_other = array(
                                              new Tests_openID2MismatchedDoesDisco(),
                                              new Tests_openID2NoEndpointDoesDisco(),
                                              new Tests_openID2MismatchedDoesDisco_failure(),
+                                             new Tests_openid1UsePreDiscoveredWrongType(),
                                              );
 
 ?>
\ No newline at end of file