- Patch #579448 by c960657: fixed OpenID specification violation. Search for OP Identifier element to authenticate.

5 files changed, 169 insertions, 51 deletions

diff --git a/modules/openid/openid.inc b/modules/openid/openid.inc
index 4198048df..b4cd7aaa3 100644
--- a/modules/openid/openid.inc
+++ b/modules/openid/openid.inc
@@ -99,6 +99,56 @@ function openid_redirect_form($form, &$form_state, $url, $message) {
 }
 
 /**
+ * Select a service element.
+ *
+ * The procedure is described in OpenID Authentication 2.0, section 7.3.2.
+ *
+ * A new entry is added to the returned array with the key 'version' and the
+ * value 1 or 2 specifying the protocol version used by the service.
+ *
+ * @param $services
+ *   An array of service arrays as returned by openid_discovery().
+ * @return
+ *   The selected service array, or NULL if no valid services were found.
+ */
+function _openid_select_service(array $services) {
+  // Extensible Resource Identifier (XRI) Resolution Version 2.0, section 4.3.3:
+  // Find the service with the highest priority (lowest integer value). If there
+  // is a tie, select a random one, not just the first in the XML document.
+  $selected_service = NULL;
+  shuffle($services);
+
+  // Search for an OP Identifier Element.
+  foreach ($services as $service) {
+    if (!empty($service['uri'])) {
+      if (in_array('http://specs.openid.net/auth/2.0/server', $service['types'])) {
+        $service['version'] = 2;
+      }
+      elseif (in_array(OPENID_NS_1_0, $service['types']) || in_array(OPENID_NS_1_1, $service['types'])) {
+        $service['version'] = 1;
+      }
+      if (isset($service['version']) && (!$selected_service || $service['priority'] < $selected_service['priority'])) {
+        $selected_service = $service;
+      }
+    }
+  }
+
+  if (!$selected_service) {
+    // Search for Claimed Identifier Element.
+    foreach ($services as $service) {
+      if (!empty($service['uri']) && in_array('http://specs.openid.net/auth/2.0/signon', $service['types'])) {
+        $service['version'] = 2;
+        if (!$selected_service || $service['priority'] < $selected_service['priority']) {
+          $selected_service = $service;
+        }
+      }
+    }
+  }
+
+  return $selected_service;
+}
+
+/**
  * Determine if the given identifier is an XRI ID.
  */
 function _openid_is_xri($identifier) {
@@ -118,7 +168,9 @@ function _openid_is_xri($identifier) {
 }
 
 /**
- * Normalize the given identifier as per spec.
+ * Normalize the given identifier.
+ *
+ * The procedure is described in OpenID Authentication 2.0, section 7.2.
  */
 function _openid_normalize($identifier) {
   if (_openid_is_xri($identifier)) {
diff --git a/modules/openid/openid.module b/modules/openid/openid.module
index 03a1a86af..f24ff75c0 100644
--- a/modules/openid/openid.module
+++ b/modules/openid/openid.module
@@ -182,50 +182,50 @@ function openid_begin($claimed_id, $return_to = '', $form_values = array()) {
   $claimed_id = _openid_normalize($claimed_id);
 
   $services = openid_discovery($claimed_id);
-  if (count($services) == 0) {
+  $service = _openid_select_service($services);
+
+  if (!$service) {
     form_set_error('openid_identifier', t('Sorry, that is not a valid OpenID. Ensure you have spelled your ID correctly.'));
     return;
   }
 
   // Store discovered information in the users' session so we don't have to rediscover.
-  $_SESSION['openid']['service'] = $services[0];
+  $_SESSION['openid']['service'] = $service;
   // Store the claimed id
   $_SESSION['openid']['claimed_id'] = $claimed_id;
   // Store the login form values so we can pass them to
   // user_exteral_login later.
   $_SESSION['openid']['user_login_values'] = $form_values;
 
-  $op_endpoint = $services[0]['uri'];
   // If bcmath is present, then create an association
   $assoc_handle = '';
   if (function_exists('bcadd')) {
-    $assoc_handle = openid_association($op_endpoint);
+    $assoc_handle = openid_association($service['uri']);
   }
 
-  // Now that there is an association created, move on
-  // to request authentication from the IdP
-  // First check for LocalID. If not found, check for Delegate. Fall
-  // back to $claimed_id if neither is found.
-  if (!empty($services[0]['localid'])) {
-    $identity = $services[0]['localid'];
-  }
-  elseif (!empty($services[0]['delegate'])) {
-    $identity = $services[0]['delegate'];
+  if (in_array('http://specs.openid.net/auth/2.0/server', $service['types'])) {
+    // User entered an OP Identifier.
+    $claimed_id = $identity = 'http://specs.openid.net/auth/2.0/identifier_select';
   }
   else {
-    $identity = $claimed_id;
-  }
-
-  if (isset($services[0]['types']) && is_array($services[0]['types']) && in_array(OPENID_NS_2_0 . '/server', $services[0]['types'])) {
-    $claimed_id = $identity = 'http://specs.openid.net/auth/2.0/identifier_select';
+    // Look for OP-Local Identifier.
+    if (!empty($service['localid'])) {
+      $identity = $service['localid'];
+    }
+    elseif (!empty($service['delegate'])) {
+      $identity = $service['delegate'];
+    }
+    else {
+      $identity = $claimed_id;
+    }
   }
-  $authn_request = openid_authentication_request($claimed_id, $identity, $return_to, $assoc_handle, $services[0]['version']);
+  $request = openid_authentication_request($claimed_id, $identity, $return_to, $assoc_handle, $service['version']);
 
-  if ($services[0]['version'] == 2) {
-    openid_redirect($op_endpoint, $authn_request);
+  if ($service['version'] == 2) {
+    openid_redirect($service['uri'], $request);
   }
   else {
-    openid_redirect_http($op_endpoint, $authn_request);
+    openid_redirect_http($service['uri'], $request);
   }
 }
 
@@ -258,11 +258,20 @@ function openid_complete($response = array()) {
       }
       else {
         if (openid_verify_assertion($service['uri'], $response)) {
-          // If the returned claimed_id is different from the session claimed_id,
-          // then we need to do discovery and make sure the op_endpoint matches.
-          if ($service['version'] == 2 && $response['openid.claimed_id'] != $claimed_id) {
-            $disco = openid_discovery($response['openid.claimed_id']);
-            if ($disco[0]['uri'] != $service['uri']) {
+          // 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 ($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'];
+              }
+            }
+            if (!in_array($service['uri'], $uris)) {
               return $response;
             }
           }
@@ -329,16 +338,20 @@ function openid_discovery($claimed_id) {
         // Look for 2.0 links
         $uri = _openid_link_href('openid2.provider', $result->data);
         $delegate = _openid_link_href('openid2.local_id', $result->data);
-        $version = 2;
+        $type = 'http://specs.openid.net/auth/2.0/signon';
 
-        // 1.0 links
+        // 1.x links
         if (empty($uri)) {
           $uri = _openid_link_href('openid.server', $result->data);
           $delegate = _openid_link_href('openid.delegate', $result->data);
-          $version = 1;
+          $type = 'http://openid.net/signon/1.1';
         }
         if (!empty($uri)) {
-          $services[] = array('uri' => $uri, 'delegate' => $delegate, 'version' => $version);
+          $services[] = array(
+            'uri' => $uri,
+            'delegate' => $delegate,
+            'types' => array($type),
+          );
         }
       }
     }
diff --git a/modules/openid/openid.test b/modules/openid/openid.test
index 17e29c540..d7cea7fc4 100644
--- a/modules/openid/openid.test
+++ b/modules/openid/openid.test
@@ -45,6 +45,12 @@ class OpenIDFunctionalTest extends DrupalWebTestCase {
     // Identifier is the URL of an XRDS document.
     $this->addIdentity(url('openid-test/yadis/xrds', array('absolute' => TRUE)), 2);
 
+    // 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)));
+
     // Identifier is the URL of an HTML page that is sent with an HTTP header
     // that contains the URL of an XRDS document.
     $this->addIdentity(url('openid-test/yadis/x-xrds-location', array('absolute' => TRUE)), 2);
@@ -126,8 +132,15 @@ class OpenIDFunctionalTest extends DrupalWebTestCase {
 
   /**
    * Add OpenID identity to user's profile.
+   *
+   * @param $identity
+   *   The User-supplied Identifier.
+   * @param $version
+   *   The protocol version used by the service.
+   * @param $claimed_id
+   *   The expected Claimed Identifier returned by the OpenID Provider.
    */
-  function addIdentity($identity, $version = 2) {
+  function addIdentity($identity, $version = 2, $claimed_id = NULL) {
     $this->drupalGet('user/' . $this->web_user->uid . '/openid');
     $edit = array('openid_identifier' => $identity);
     $this->drupalPost(NULL, $edit, t('Add an OpenID'));
@@ -139,7 +152,10 @@ class OpenIDFunctionalTest extends DrupalWebTestCase {
       $this->drupalPost(NULL, array(), t('Send'));
     }
 
-    $this->assertRaw(t('Successfully added %identity', array('%identity' => $identity)), t('Identity %identity was added.', array('%identity' => $identity)));
+    if (!$claimed_id) {
+      $claimed_id = $identity;
+    }
+    $this->assertRaw(t('Successfully added %identity', array('%identity' => $claimed_id)), t('Identity %identity was added.', array('%identity' => $identity)));
   }
 
   /**
diff --git a/modules/openid/tests/openid_test.module b/modules/openid/tests/openid_test.module
index 4bf839d58..0512555f9 100644
--- a/modules/openid/tests/openid_test.module
+++ b/modules/openid/tests/openid_test.module
@@ -74,9 +74,33 @@ function openid_test_yadis_xrds() {
       <xrds:XRDS xmlns:xrds="xri://$xrds" xmlns="xri://$xrd*($v*2.0)">
         <XRD>
           <Service>
+            <Type>http://example.com/this-is-ignored</Type>
+          </Service>
+          <Service priority="10">
             <Type>http://specs.openid.net/auth/2.0/signon</Type>
             <URI>' . url('openid-test/endpoint', array('absolute' => TRUE)) . '</URI>
           </Service>
+          <Service priority="15">
+            <Type>http://specs.openid.net/auth/2.0/signon</Type>
+            <URI>http://example.com/this-has-too-low-priority</URI>
+          </Service>
+          <Service>
+            <Type>http://specs.openid.net/auth/2.0/signon</Type>
+            <URI>http://example.com/this-has-too-low-priority</URI>
+          </Service>
+          ';
+    if (arg(3) == 'server') {
+      print '
+          <Service>
+            <Type>http://specs.openid.net/auth/2.0/server</Type>
+            <URI>http://example.com/this-has-too-low-priority</URI>
+          </Service>
+          <Service priority="20">
+            <Type>http://specs.openid.net/auth/2.0/server</Type>
+            <URI>' . url('openid-test/endpoint', array('absolute' => TRUE)) . '</URI>
+          </Service>';
+    }
+    print '
         <XRD>
       </xrds:XRDS>';
   }
@@ -202,22 +226,32 @@ function _openid_test_endpoint_associate() {
  * identity.
  */
 function _openid_test_endpoint_authenticate() {
-  global $base_url;
-
   module_load_include('inc', 'openid');
 
   // 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).
   $response = variable_get('openid_test_response', array()) + array(
     'openid.ns' => OPENID_NS_2_0,
     'openid.mode' => 'id_res',
-    'openid.op_endpoint' => $base_url . url('openid/provider'),
-    // openid.claimed_id is not sent by OpenID 1 clients.
-    'openid.claimed_id' => isset($_REQUEST['openid_claimed_id']) ? $_REQUEST['openid_claimed_id'] : '',
+    'openid.op_endpoint' => url('openid-test/endpoint', array('absolute' => TRUE)),
+    'openid.claimed_id' => $claimed_id,
     'openid.identity' => $_REQUEST['openid_identity'],
     'openid.return_to' => $_REQUEST['openid_return_to'],
     'openid.response_nonce' => $nonce,
diff --git a/modules/openid/xrds.inc b/modules/openid/xrds.inc
index 8470997db..0c6717ea4 100644
--- a/modules/openid/xrds.inc
+++ b/modules/openid/xrds.inc
@@ -25,10 +25,19 @@ function xrds_parse($xml) {
 /**
  * Parser callback functions
  */
-function _xrds_element_start(&$parser, $name, $attribs) {
-  global $xrds_open_elements;
+function _xrds_element_start(&$parser, $name, $attributes) {
+  global $xrds_open_elements, $xrds_current_service;
 
   $xrds_open_elements[] = _xrds_strip_namespace($name);
+
+  $path = strtoupper(implode('/', $xrds_open_elements));
+  if ($path == 'XRDS/XRD/SERVICE') {
+    foreach ($attributes as $attribute_name => $value) {
+      if (_xrds_strip_namespace($attribute_name) == 'PRIORITY') {
+        $xrds_current_service['priority'] = intval($value);
+      }
+    }
+  }
 }
 
 function _xrds_element_end(&$parser, $name) {
@@ -36,17 +45,11 @@ function _xrds_element_end(&$parser, $name) {
 
   $name = _xrds_strip_namespace($name);
   if ($name == 'SERVICE') {
-    if (in_array(OPENID_NS_2_0 . '/signon', $xrds_current_service['types']) ||
-        in_array(OPENID_NS_2_0 . '/server', $xrds_current_service['types'])) {
-      $xrds_current_service['version'] = 2;
-    }
-    elseif (in_array(OPENID_NS_1_1, $xrds_current_service['types']) ||
-            in_array(OPENID_NS_1_0, $xrds_current_service['types'])) {
-      $xrds_current_service['version'] = 1;
-    }
-    if (!empty($xrds_current_service['version'])) {
-      $xrds_services[] = $xrds_current_service;
+    if (!isset($xrds_current_service['priority'])) {
+      // If the priority attribute is absent, the default is infinity.
+      $xrds_current_service['priority'] = PHP_INT_MAX;
     }
+    $xrds_services[] = $xrds_current_service;
     $xrds_current_service = array();
   }
   array_pop($xrds_open_elements);