- Patch #728278 by c960657: openid_complete() should normalize ['openid.claimed_id()'] before discovery. With better tests.

3 files changed, 29 insertions, 30 deletions

diff --git a/modules/openid/openid.module b/modules/openid/openid.module
index f19810dfa..89e0e3732 100644
--- a/modules/openid/openid.module
+++ b/modules/openid/openid.module
@@ -288,21 +288,24 @@ function openid_complete($response = array()) {
           if (!empty($service['claimed_id'])) {
             $response['openid.claimed_id'] = $service['claimed_id'];
           }
-          // OpenID Authentication, section 11.2:
-          // If the returned Claimed Identifier is different from the one sent
-          // to the OpenID Provider, we need to do discovery on the returned
-          // identifier to make sure that the provider is authorized to respond
-          // on behalf of this.
-          elseif ($service['version'] == 2 && $response['openid.claimed_id'] != openid_normalize($claimed_id)) {
-            $services = openid_discovery($response['openid.claimed_id']);
-            $uris = array();
-            foreach ($services as $discovered_service) {
-              if (in_array('http://specs.openid.net/auth/2.0/server', $discovered_service['types']) || in_array('http://specs.openid.net/auth/2.0/signon', $discovered_service['types'])) {
-                $uris[] = $discovered_service['uri'];
+          elseif ($service['version'] == 2) {
+            $response['openid.claimed_id'] = openid_normalize($response['openid.claimed_id']);
+            // OpenID Authentication, section 11.2:
+            // If the returned Claimed Identifier is different from the one sent
+            // to the OpenID Provider, we need to do discovery on the returned
+            // identififer to make sure that the provider is authorized to
+            // respond on behalf of this.
+            if ($response['openid.claimed_id'] != $claimed_id) {
+              $services = openid_discovery($response['openid.claimed_id']);
+              $uris = array();
+              foreach ($services as $discovered_service) {
+                if (in_array('http://specs.openid.net/auth/2.0/server', $discovered_service['types']) || in_array('http://specs.openid.net/auth/2.0/signon', $discovered_service['types'])) {
+                  $uris[] = $discovered_service['uri'];
+                }
+              }
+              if (!in_array($service['uri'], $uris)) {
+                return $response;
               }
-            }
-            if (!in_array($service['uri'], $uris)) {
-              return $response;
             }
           }
           else {
diff --git a/modules/openid/openid.test b/modules/openid/openid.test
index 7002c7a1f..4de6f67d4 100644
--- a/modules/openid/openid.test
+++ b/modules/openid/openid.test
@@ -43,13 +43,22 @@ class OpenIDFunctionalTest extends DrupalWebTestCase {
     // the URL of the OpenID Provider Endpoint.
 
     // Identifier is the URL of an XRDS document.
-    $this->addIdentity(url('openid-test/yadis/xrds', array('absolute' => TRUE)), 2);
+    // The URL scheme is stripped in order to test that the supplied identifier
+    // is normalized in openid_begin().
+    $identity = url('openid-test/yadis/xrds', array('absolute' => TRUE));
+    $this->addIdentity(preg_replace('@^https?://@', '', $identity), 2, $identity);
 
     // Identifier is the URL of an XRDS document containing an OP Identifier
     // Element. The Relying Party sends the special value
     // "http://specs.openid.net/auth/2.0/identifier_select" as Claimed
     // Identifier. The OpenID Provider responds with the actual identifier.
-    $this->addIdentity(url('openid-test/yadis/xrds/server', array('absolute' => TRUE)), 2, url('openid-test/yadis/xrds/dummy-user', array('absolute' => TRUE)));
+    $identity = url('openid-test/yadis/xrds/dummy-user', array('absolute' => TRUE));
+    // Tell openid_test.module to respond with this identifier. The URL scheme
+    // is stripped in order to test that the returned identifier is normalized in
+    // openid_complete().
+    variable_set('openid_test_response', array('openid.claimed_id' => preg_replace('@^https?://@', '', $identity)));
+    $this->addIdentity(url('openid-test/yadis/xrds/server', array('absolute' => TRUE)), 2, $identity);
+    variable_set('openid_test_response', array());
 
     // Identifier is the URL of an HTML page that is sent with an HTTP header
     // that contains the URL of an XRDS document.
diff --git a/modules/openid/tests/openid_test.module b/modules/openid/tests/openid_test.module
index db03641c5..4314e0b20 100644
--- a/modules/openid/tests/openid_test.module
+++ b/modules/openid/tests/openid_test.module
@@ -251,19 +251,6 @@ function _openid_test_endpoint_authenticate() {
   // Generate unique identifier for this authentication.
   $nonce = _openid_nonce();
 
-  if (!isset($_REQUEST['openid_claimed_id'])) {
-    // openid.claimed_id is not used in OpenID 1.x.
-    $claimed_id = '';
-  }
-  elseif ($_REQUEST['openid_claimed_id'] == 'http://specs.openid.net/auth/2.0/identifier_select') {
-    // The Relying Party did not specify a Claimed Identifier, so the OpenID
-    // Provider decides on one.
-    $claimed_id = url('openid-test/yadis/xrds/dummy-user', array('absolute' => TRUE));
-  }
-  else {
-    $claimed_id = $_REQUEST['openid_claimed_id'];
-  }
-
   // Generate response containing the user's identity. The openid.sreg.xxx
   // entries contain profile data stored by the OpenID Provider (see OpenID
   // Simple Registration Extension 1.0).
@@ -271,7 +258,7 @@ function _openid_test_endpoint_authenticate() {
     'openid.ns' => OPENID_NS_2_0,
     'openid.mode' => 'id_res',
     'openid.op_endpoint' => url('openid-test/endpoint', array('absolute' => TRUE)),
-    'openid.claimed_id' => $claimed_id,
+    'openid.claimed_id' => !empty($_REQUEST['openid_claimed_id']) ? $_REQUEST['openid_claimed_id'] : '',
     'openid.identity' => $_REQUEST['openid_identity'],
     'openid.return_to' => $_REQUEST['openid_return_to'],
     'openid.response_nonce' => $nonce,