diff --git a/modules/openid/openid.api.php b/modules/openid/openid.api.php index 11faa71..d1955ae 100644 --- a/modules/openid/openid.api.php +++ b/modules/openid/openid.api.php @@ -52,6 +52,13 @@ function hook_openid_response($response, $account) { * parameter, the claimed identifier. They have to return an array of services, * in the same form returned by openid_discover(). * + * The claimed identifier parameter should be passed by reference to the + * callback, because claimed id could be changed during discovery process. + * According to Openid specifications relying party must handle all following + * HTTP redirects and apply rules in Section 6 of [RFC3986] to the final + * destination URL. The final URL must be used by the relying party as the + * claimed id. See http://drupal.org/node/575810 for more information. + * * The first discovery method that succeed (return at least one services) will * stop the discovery process. * diff --git a/modules/openid/openid.module b/modules/openid/openid.module index 7673de8..582f1c9 100644 --- a/modules/openid/openid.module +++ b/modules/openid/openid.module @@ -341,14 +341,17 @@ function openid_complete($response = array()) { $response['openid.claimed_id'] = $service['claimed_id']; } elseif ($service['version'] == 2) { - $response['openid.claimed_id'] = openid_normalize($response['openid.claimed_id']); + // Returned claimed id could have unique fragment hash to allow + // identifier recycling so we need to preserve it in response. + $response_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']); + if ($response_claimed_id != $claimed_id) { + $services = openid_discovery($response_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'])) { @@ -379,7 +382,7 @@ function openid_complete($response = array()) { * @return Array of services discovered (including OpenID version, endpoint * URI, etc). */ -function openid_discovery($claimed_id) { +function openid_discovery(&$claimed_id) { module_load_include('inc', 'openid'); $methods = module_invoke_all('openid_discovery_method_info'); @@ -418,7 +421,7 @@ function openid_openid_discovery_method_info() { * @see http://openid.net/specs/openid-authentication-2_0.html#discovery * @see hook_openid_discovery_method_info() */ -function _openid_xri_discovery($claimed_id) { +function _openid_xri_discovery(&$claimed_id) { if (_openid_is_xri($claimed_id)) { // Resolve XRI using a proxy resolver (Extensible Resource Identifier (XRI) // Resolution Version 2.0, section 11.2 and 14.3). @@ -444,7 +447,7 @@ function _openid_xri_discovery($claimed_id) { * @see http://openid.net/specs/openid-authentication-2_0.html#discovery * @see hook_openid_discovery_method_info() */ -function _openid_xrds_discovery($claimed_id) { +function _openid_xrds_discovery(&$claimed_id) { $services = array(); $xrds_url = $claimed_id; @@ -454,7 +457,17 @@ function _openid_xrds_discovery($claimed_id) { $headers = array('Accept' => 'application/xrds+xml'); $result = drupal_http_request($xrds_url, array('headers' => $headers)); - if (!isset($result->error)) { + // Check for HTTP error and make sure, that we reach the target. If maximum + // allowed redirects are exhausted, final destination URL isn't reached, but + // drupal_http_request will not return error. (This check could be removed + // after http://drupal.org/node/1096890 will be fixed.) + if (!isset($result->error) && $result->code == 200) { + + // Replace user entered claimed_id if we got redirect + if (!empty($result->redirect_url)) { + $claimed_id = openid_normalize($result->redirect_url); + } + if (isset($result->headers['content-type']) && preg_match("/application\/xrds\+xml/", $result->headers['content-type'])) { // Parse XML document to find URL $services = _openid_xrds_parse($result->data); @@ -589,8 +602,63 @@ function openid_association($op_endpoint) { */ function openid_authentication($response) { $identity = $response['openid.claimed_id']; - $account = user_external_load($identity); + + // Transition for accounts which were registered with stripped fragments due + // to bug in openid_complete() or with claimed identifier which wasn't + // replaced by the following redirects in openid_discovery(). See + // http://drupal.org/node/1120290. + if (!isset($account->uid)) { + + // Step 1: Try to strip the fragment if it is present. + //$identity_stripped = preg_replace('/#[^?]*/', '', $identity); + $identity_stripped = _openid_url_normalize($identity); + $account_stripped = user_external_load($identity_stripped); + + // Step 2: Try to replace https with http. Openid providers often redirects + // from http to https, but Drupal didn't follow these redirects. + if (!$account_stripped) { + $identity_stripped = str_replace('https', 'http', $identity_stripped); + $account_stripped = user_external_load($identity_stripped); + } + + // Step 3: Try to use original identifier. + if (!$account_stripped && isset($_SESSION['openid']['user_login_values']['openid_identifier'])) { + $identity_stripped = openid_normalize($_SESSION['openid']['user_login_values']['openid_identifier']); + $account_stripped = user_external_load($identity_stripped); + } + + if (isset($account_stripped->uid)) { + // Try to extract e-mail address from Simple Registration (SREG) or + // Attribute Exchanges (AX) keys. + $email = ''; + $sreg_values = openid_extract_namespace($response, OPENID_NS_SREG, 'sreg'); + $ax_values = openid_extract_namespace($response, OPENID_NS_AX, 'ax'); + if (!empty($sreg_values['email']) && valid_email_address($sreg_values['email'])) { + $email = $sreg_values['email']; + } + elseif ($ax_mail_values = openid_extract_ax_values($ax_values, array('http://axschema.org/contact/email', 'http://schema.openid.net/contact/email'))) { + $email = current($ax_mail_values); + } + + // If this e-mail address is the same as the e-mail address found in user + // account, login the user and update the claimed identifier. + if ($email && ($email == $account_stripped->mail) || ($email == $account_stripped->init)) { + $query = db_update('authmap') + ->fields(array('authname' => $identity)) + ->condition('uid', $account_stripped->uid) + ->condition('module', 'openid') + ->execute(); + drupal_set_message(t('Successfully updated %identity_stripped with %identity.', array('%identity_stripped' => $identity_stripped, '%identity' => $identity))); + // Set the account to the found one. + $account =& $account_stripped; + } + else { + drupal_set_message(t('We found that there has been already an account which matches your OpenID identifier. If you are new on this site, please continue with registering the new user account. If you already had registered account on this site attached to the OpenID which you have provided, please try to reset the password or contact the site administrator. We are very sorry for this inadvertency.', array('@url_password' => 'user/password')), 'warning'); + } + } + } + if (isset($account->uid)) { if (!variable_get('user_email_verification', TRUE) || $account->login) { // Check if user is blocked. @@ -634,7 +702,7 @@ function openid_authentication($response) { drupal_set_message(t('Account registration using the information provided by your OpenID provider failed due to the reasons listed below. Complete the registration by filling out the form below. If you already have an account, you can log in now and add your OpenID under "My account".', array('@login' => url('user/login'))), 'warning'); // Append form validation errors below the above warning. foreach ($messages['error'] as $message) { - drupal_set_message( $message, 'error'); + drupal_set_message($message, 'error'); } } diff --git a/modules/openid/openid.test b/modules/openid/openid.test index 202a835..6a6d80a 100644 --- a/modules/openid/openid.test +++ b/modules/openid/openid.test @@ -89,12 +89,12 @@ class OpenIDFunctionalTestCase extends OpenIDWebTestCase { // 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. - $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))); + // Identifier. The OpenID Provider responds with the actual identifier + // including fragment. + $identity = url('openid-test/yadis/xrds/dummy-user', array('absolute' => TRUE, 'fragment' => $this->randomName())); + // Tell openid_test.module to respond with this identifier. We test if + // openid_complete() process it right. + variable_set('openid_test_response', array('openid.claimed_id' => $identity)); $this->addIdentity(url('openid-test/yadis/xrds/server', array('absolute' => TRUE)), 2, 'http://specs.openid.net/auth/2.0/identifier_select', $identity); variable_set('openid_test_response', array()); @@ -479,6 +479,114 @@ class OpenIDRegistrationTestCase extends OpenIDWebTestCase { } /** + * Test account registration using Simple Registration and Attribute Exchange. + */ +class OpenIDInvalidIdentifierTransitionTestCase extends OpenIDFunctionalTestCase { + public static function getInfo() { + return array( + 'name' => 'OpenID account update', + 'description' => 'Tries to correct openid identifiers attached to accounts if theirs identifiers were stripped.', + 'group' => 'OpenID' + ); + } + + function setUp() { + parent::setUp('openid', 'openid_test'); + variable_set('user_register', USER_REGISTER_VISITORS); + } + + /** + * Test OpenID transition with e-mail mismatch. + */ + function testStrippedFragmentAccountEmailMismatch() { + $this->drupalLogin($this->web_user); + + // Use a User-supplied Identity that is the URL of an XRDS document. + $identity = url('openid-test/yadis/xrds', array('absolute' => TRUE, 'fragment' => $this->randomName())); + $identity_stripped = preg_replace('/#[^?]*/', '', $identity); + + $this->addIdentity($identity_stripped); + + $this->drupalLogout(); + + // Test logging in via the login block on the front page. + $this->submitLoginForm($identity); + $this->assertLink(t('Log out'), 0, t('User was logged in.')); + + $this->drupalLogout(); + + // Test logging in via the user/login page, provider will respond with full + // identifier (including fragment) but with different email, so we can't + // provide auto-update. + variable_set('openid_test_response', array( + 'openid.claimed_id' => $identity, + 'openid.sreg.nickname' => $this->web_user->name, + 'openid.sreg.email' => 'invalid-' . $this->web_user->mail)); + $edit = array('openid_identifier' => $identity_stripped); + $this->drupalPost('user/login', $edit, t('Log in')); + + // Check we are on the OpenID redirect form. + $this->assertTitle(t('OpenID redirect'), t('OpenID redirect page was displayed.')); + + // Submit form to the OpenID Provider Endpoint. + $this->drupalPost(NULL, array(), t('Send')); + + // Verify user was redirected away from user/login to an accessible page. + $this->assertResponse(200); + + // Verify the message. + $this->assertRaw(t('We found that there has been already an account which matches your OpenID identifier. If you are new on this site, please continue with registering the new user account. If you already had registered account on this site attached to the OpenID which you have provided, please try to reset the password or contact the site administrator. We are very sorry for this inadvertency.', array('@url_password' => 'user/password')), t('Message that OpenID identifier must be updated manually was displayed.')); + + variable_set('openid_test_response', array()); + } + + /** + * Test OpenID auto transition with e-mail. + */ + function testStrippedFragmentAccountAutoUpdateSreg() { + $this->drupalLogin($this->web_user); + + // Use a User-supplied Identity that is the URL of an XRDS document. + $identity = url('openid-test/yadis/xrds', array('absolute' => TRUE, 'fragment' => $this->randomName())); + $identity_stripped = preg_replace('/#[^?]*/', '', $identity); + + $this->addIdentity($identity_stripped); + + $this->drupalLogout(); + + // Test logging in via the login block on the front page. + $this->submitLoginForm($identity); + $this->assertLink(t('Log out'), 0, t('User was logged in.')); + + $this->drupalLogout(); + + // Test logging in via the user/login page, provider will respond with full + // identifier (including fragment) but with different email, so we can't + // provide auto-update. + variable_set('openid_test_response', array( + 'openid.claimed_id' => $identity, + 'openid.sreg.nickname' => $this->web_user->name, + 'openid.sreg.email' => $this->web_user->mail)); + $edit = array('openid_identifier' => $identity_stripped); + $this->drupalPost('user/login', $edit, t('Log in')); + + // Check we are on the OpenID redirect form. + $this->assertTitle(t('OpenID redirect'), t('OpenID redirect page was displayed.')); + + // Submit form to the OpenID Provider Endpoint. + $this->drupalPost(NULL, array(), t('Send')); + + // Verify user was redirected away from user/login to an accessible page. + $this->assertResponse(200); + + // Verify the message. + $this->assertRaw(t('Successfully updated %identity_stripped with %identity.', array('%identity_stripped' => $identity_stripped, '%identity' => $identity)), t('Message that OpenID identifier was updated automatically was displayed.')); + + variable_set('openid_test_response', array()); + } +} + +/** * Test internal helper functions. */ class OpenIDUnitTest extends DrupalWebTestCase {