- Patch #395340 by c960657: critical bug: enforce e-mail verification with OpenID auto-registration.

2 files changed, 135 insertions, 159 deletions

diff --git a/modules/openid/openid.module b/modules/openid/openid.module
index d7773744b..ecf452a8f 100644
--- a/modules/openid/openid.module
+++ b/modules/openid/openid.module
@@ -65,13 +65,14 @@ function openid_help($path, $arg) {
  * Implements hook_user_insert().
  */
 function openid_user_insert(&$edit, $account, $category) {
-  if (isset($_SESSION['openid']['values'])) {
+  if (!empty($edit['openid_claimed_id'])) {
     // The user has registered after trying to log in via OpenID.
     if (variable_get('user_email_verification', TRUE)) {
       drupal_set_message(t('Once you have verified your e-mail address, you may log in via OpenID.'));
     }
-    user_set_authmaps($account, array('authname_openid' => $_SESSION['openid']['values']['response']['openid.claimed_id']));
+    user_set_authmaps($account, array('authname_openid' => $edit['openid_claimed_id']));
     unset($_SESSION['openid']);
+    unset($edit['openid_claimed_id']);
   }
 }
 
@@ -153,28 +154,56 @@ function _openid_user_login_form_alter(&$form, &$form_state) {
 }
 
 /**
- * Implements hook_form_alter().
+ * Implements hook_form_FORM_ID_alter().
  *
- * Adds OpenID login to the login forms.
+ * Prefills the login form with values acquired via OpenID.
  */
 function openid_form_user_register_form_alter(&$form, &$form_state) {
-  if (isset($_SESSION['openid']['values'])) {
-    // We were unable to auto-register a new user. Prefill the registration
-    // form with the values we have.
-    $form['account']['name']['#default_value'] = $_SESSION['openid']['values']['name'];
-    $form['account']['mail']['#default_value'] = $_SESSION['openid']['values']['mail'];
+  if (isset($_SESSION['openid']['response'])) {
+    module_load_include('inc', 'openid');
+
+    $response = $_SESSION['openid']['response'];
+
+    // Extract Simple Registration keys from the response.
+    $sreg_values = openid_extract_namespace($response, OPENID_NS_SREG, 'sreg');
+    // Extract Attribute Exchanges keys from the response.
+    $ax_values = openid_extract_namespace($response, OPENID_NS_AX, 'ax');
+
+    if (!empty($sreg_values['nickname'])) {
+      // Use the nickname returned by Simple Registration if available.
+      $form['account']['name']['#default_value'] = $sreg_values['nickname'];
+    }
+    elseif (!empty($ax_values['value.email'])) {
+      // Else, extract the name part of the email address returned by AX if available.
+      list($name, $domain) = explode('@', $ax_values['value.email'], 2);
+      $form['account']['name']['#default_value'] = $name;
+    }
+
+    if (!empty($sreg_values['email'])) {
+      // Use the email returned by Simple Registration if available.
+      $form['account']['mail']['#default_value'] = $sreg_values['email'];
+    }
+    elseif (!empty($ax_values['value.email'])) {
+      // Else, use the email returned by AX if available.
+      $form['account']['mail']['#default_value'] = $ax_values['value.email'];
+    }
 
     // If user_email_verification is off, hide the password field and just fill
     // with random password to avoid confusion.
     if (!variable_get('user_email_verification', TRUE)) {
-      $form['pass']['#type'] = 'hidden';
-      $form['pass']['#value'] = user_password();
+      $form['account']['pass']['#type'] = 'hidden';
+      $form['account']['pass']['#value'] = user_password();
     }
+
+    $form['openid_claimed_id'] = array(
+      '#type' => 'value',
+      '#default_value' => $response['openid.claimed_id'],
+    );
     $form['openid_display'] = array(
       '#type' => 'item',
       '#title' => t('Your OpenID'),
       '#description' => t('This OpenID will be attached to your account after registration.'),
-      '#markup' => check_plain($_SESSION['openid']['values']['response']['openid.claimed_id']),
+      '#markup' => check_plain($response['openid.claimed_id']),
     );
   }
 }
@@ -575,8 +604,6 @@ function openid_association($op_endpoint) {
  * @param $response Response values from the OpenID Provider.
  */
 function openid_authentication($response) {
-  module_load_include('inc', 'openid');
-
   $identity = $response['openid.claimed_id'];
 
   $account = user_external_load($identity);
@@ -594,92 +621,44 @@ function openid_authentication($response) {
       }
     }
     else {
-      drupal_set_message(t('You must validate your email address for this account before logging in via OpenID'));
+      drupal_set_message(t('You must validate your email address for this account before logging in via OpenID.'));
     }
   }
   elseif (variable_get('user_register', 1)) {
     // Register new user.
 
-    // Extract Simple Registration keys from the response.
-    $sreg_values = openid_extract_namespace($response, OPENID_NS_SREG, 'sreg');
-    // Extract Attribute Exchanges keys from the response.
-    $ax_values = openid_extract_namespace($response, OPENID_NS_AX, 'ax');
-
-    $form_state['build_info']['args'] = array();
-    $form_state['redirect'] = NULL;
+    // Save response for use in openid_form_user_register_form_alter().
+    $_SESSION['openid']['response'] = $response;
 
-    if (!empty($sreg_values['nickname'])) {
-      // Use the nickname returned by Simple Registration if available.
-      $form_state['values']['name'] = $sreg_values['nickname'];
-    }
-    else if (!empty($ax_values['value.email'])) {
-      // Else, extract the name part of the email address returned by AX if available.
-      list ($name, $domain) = explode('@', $ax_values['value.email'], 2);
-      $form_state['values']['name'] = $name;
-    }
-    else {
-      $form_state['values']['name'] = '';
-    }
+    $form_state['values'] = array();
+    $form_state['values']['op'] = t('Create new account');
+    drupal_form_submit('user_register_form', $form_state);
 
-    if (!empty($sreg_values['email'])) {
-      // Use the email returned by Simple Registration if available.
-      $form_state['values']['mail'] = $sreg_values['email'];
-    }
-    else if (!empty($ax_values['value.email'])) {
-      // Else, use the email returned by AX if available.
-      $form_state['values']['mail'] = $ax_values['value.email'];
+    if (!empty($form_state['user'])) {
+      module_invoke_all('openid_response', $response, $form_state['user']);
+      drupal_goto();
     }
-    else {
-      $form_state['values']['mail'] = '';
-    }
-
-    $form_state['values']['pass']  = user_password();
-    $form_state['values']['status'] = variable_get('user_register', 1) == 1;
-    $form_state['values']['response'] = $response;
 
+    $messages = drupal_get_messages('error');
     if (empty($form_state['values']['name']) || empty($form_state['values']['mail'])) {
+      // If the OpenID provider did not provide both a user name and an email
+      // address, ask the user to complete the registration manually instead of
+      // showing the error messages about the missing values generated by FAPI.
       drupal_set_message(t('Complete the registration by filling out the form below. If you already have an account, you can <a href="@login">log in</a> now and add your OpenID under "My account".', array('@login' => url('user/login'))), 'warning');
-      $success = FALSE;
     }
     else {
-      $form = drupal_retrieve_form('user_register_form', $form_state);
-      drupal_prepare_form('user_register_form', $form, $form_state);
-      drupal_validate_form('user_register_form', $form, $form_state);
-      $success = !form_get_errors();
-      if (!$success) {
-        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 <a href="@login">log in</a> now and add your OpenID under "My account".', array('@login' => url('user/login'))), 'warning');
-        // Append form validation errors below the above warning.
-        $messages = drupal_get_messages('error');
-        foreach ($messages['error'] as $message) {
-          drupal_set_message( $message, 'error');
-        }
-      }
-    }
-    if (!$success) {
-      // We were unable to register a valid new user, redirect to standard
-      // user/register and prefill with the values we received.
-      $_SESSION['openid']['values'] = $form_state['values'];
-      // We'll want to redirect back to the same place.
-      $destination = drupal_get_destination();
-      unset($_GET['destination']);
-      drupal_goto('user/register', array('query' => $destination));
-    }
-    else {
-      unset($form_state['values']['response']);
-      $account = user_save(drupal_anonymous_user(), $form_state['values']);
-      // Terminate if an error occurred during user_save().
-      if (!$account) {
-        drupal_set_message(t("Error saving user account."), 'error');
-        drupal_goto();
+      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 <a href="@login">log in</a> 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');
       }
-      user_set_authmaps($account, array("authname_openid" => $identity));
-      // Load global $user and perform final login tasks.
-      $form_state['uid'] = $account->uid;
-      user_login_submit(array(), $form_state);
-      // Let other modules act on OpenID login
-      module_invoke_all('openid_response', $response, $account);
     }
-    drupal_redirect_form($form_state);
+
+    // We were unable to register a valid new user. Redirect to the normal
+    // registration page and prefill with the values we received.
+    $destination = drupal_get_destination();
+    unset($_GET['destination']);
+    drupal_goto('user/register', array('query' => $destination));
   }
   else {
     drupal_set_message(t('Only site administrators can create new user accounts.'), 'error');
diff --git a/modules/openid/openid.test b/modules/openid/openid.test
index 65b805b1b..adf5b97aa 100644
--- a/modules/openid/openid.test
+++ b/modules/openid/openid.test
@@ -100,19 +100,14 @@ class OpenIDFunctionalTest extends DrupalWebTestCase {
 
     $this->drupalLogout();
 
-    // Fill out and submit the login form.
-    $edit = array('openid_identifier' => $identity);
-    $this->drupalPost(NULL, $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'));
+    // Test logging in via the login block on the front page.
+    $this->submitLoginForm($identity);
     $this->assertLink($this->web_user->name, 0, t('User was logged in.'));
 
-    // Test logging in via the user/login page.
     $this->drupalLogout();
+
+    // Test logging in via the user/login page.
+    $edit = array('openid_identifier' => $identity);
     $this->drupalPost('user/login', $edit, t('Log in'));
 
     // Check we are on the OpenID redirect form.
@@ -170,15 +165,7 @@ class OpenIDFunctionalTest extends DrupalWebTestCase {
     $this->assertRaw('The update has been performed.', t('Account was blocked.'));
     $this->drupalLogout();
 
-    // Fill out and submit the login form.
-    $edit = array('openid_identifier' => $identity);
-    $this->drupalPost(NULL, $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'));
+    $this->submitLoginForm($identity);
     $this->assertRaw(t('The username %name has not been activated or is blocked.', array('%name' => $this->web_user->name)), t('User login was blocked.'));
   }
 
@@ -218,34 +205,62 @@ class OpenIDFunctionalTest extends DrupalWebTestCase {
   }
 
   /**
-   * Test OpenID auto-registration with e-mail verification disabled.
+   * Test OpenID auto-registration with e-mail verification enabled.
    */
-  function testRegisterUserWithoutEmailVerification() {
-    variable_set('user_email_verification', FALSE);
+  function testRegisterUserWithEmailVerification() {
+    variable_set('user_email_verification', TRUE);
 
-    // Load the front page to get the user login block.
-    $this->drupalGet('');
+    // Tell openid_test.module to respond with these SREG fields.
+    variable_set('openid_test_response', array('openid.sreg.nickname' => 'john', 'openid.sreg.email' => 'john@example.com'));
 
     // Use a User-supplied Identity that is the URL of an XRDS document.
     $identity = url('openid-test/yadis/xrds', array('absolute' => TRUE));
+    $this->submitLoginForm($identity);
+    $this->assertRaw(t('Once you have verified your e-mail address, you may log in via OpenID.'), t('User was asked to verify e-mail address.'));
+    $this->assertRaw(t('A welcome message with further instructions has been sent to your e-mail address.'), t('A welcome message was sent to the user.'));
 
-    // Tell openid_test.module to respond with these SREG fields.
-    variable_set('openid_test_response', array('openid.sreg.nickname' => 'john', 'openid.sreg.email' => 'john@example.com'));
+    $user = user_load_by_name('john');
+    $this->assertTrue($user, t('User was registered with right username.'));
+    $this->assertEqual($user->mail, 'john@example.com', t('User was registered with right email address.'));
+    $this->assertFalse($user->data, t('No additional user info was saved.'));
 
-    // Fill out and submit the login form.
-    $edit = array('openid_identifier' => $identity);
-    $this->drupalPost(NULL, $edit, t('Log in'));
+    $this->submitLoginForm($identity);
+    $this->assertRaw(t('You must validate your email address for this account before logging in via OpenID.'));
 
-    // Check we are on the OpenID redirect form.
-    $this->assertTitle(t('OpenID redirect'), t('OpenID redirect page was displayed.'));
+    // Follow the one-time login that was sent in the welcome e-mail.
+    $this->drupalGet(user_pass_reset_url($user));
+    $this->drupalPost(NULL, array(), t('Log in'));
 
-    // Submit form to the OpenID Provider Endpoint.
-    $this->drupalPost(NULL, array(), t('Send'));
+    $this->drupalLogout();
+
+    // Verify that the account was activated.
+    $this->submitLoginForm($identity);
+    $this->assertLink('john', 0, t('User was logged in.'));
+  }
+
+  /**
+   * Test OpenID auto-registration with e-mail verification disabled.
+   */
+  function testRegisterUserWithoutEmailVerification() {
+    variable_set('user_email_verification', FALSE);
+
+    // Tell openid_test.module to respond with these SREG fields.
+    variable_set('openid_test_response', array('openid.sreg.nickname' => 'john', 'openid.sreg.email' => 'john@example.com'));
+
+    // Use a User-supplied Identity that is the URL of an XRDS document.
+    $identity = url('openid-test/yadis/xrds', array('absolute' => TRUE));
+    $this->submitLoginForm($identity);
     $this->assertLink('john', 0, t('User was logged in.'));
 
     $user = user_load_by_name('john');
     $this->assertTrue($user, t('User was registered with right username.'));
     $this->assertEqual($user->mail, 'john@example.com', t('User was registered with right email address.'));
+    $this->assertFalse($user->data, t('No additional user info was saved.'));
+
+    $this->drupalLogout();
+
+    $this->submitLoginForm($identity);
+    $this->assertLink('john', 0, t('User was logged in.'));
   }
 
   /**
@@ -253,25 +268,12 @@ class OpenIDFunctionalTest extends DrupalWebTestCase {
    * information (a username that is already taken, and no e-mail address).
    */
   function testRegisterUserWithInvalidSreg() {
-    // Load the front page to get the user login block.
-    $this->drupalGet('');
+     // Tell openid_test.module to respond with these SREG fields. 
+     variable_set('openid_test_response', array('openid.sreg.nickname' => $this->web_user->name, 'openid.sreg.email' => 'mail@invalid#'));
 
     // Use a User-supplied Identity that is the URL of an XRDS document.
     $identity = url('openid-test/yadis/xrds', array('absolute' => TRUE));
-
-    // Tell openid_test.module to respond with these SREG fields.
-    variable_set('openid_test_response', array('openid.sreg.nickname' => $this->web_user->name, 'openid.sreg.email' => 'mail@invalid#'));
-
-    // Fill out and submit the login form.
-    $edit = array('openid_identifier' => $identity);
-    $this->drupalPost(NULL, $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'));
-
+    $this->submitLoginForm($identity);
     $this->assertRaw(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 <a href="@login">log in</a> now and add your OpenID under "My account".', array('@login' => url('user/login'))), t('User was asked to complete the registration process manually.'));
     $this->assertRaw(t('The name %name is already taken.', array('%name' => $this->web_user->name)), t('Form validation error for username was displayed.'));
     $this->assertRaw(t('The e-mail address %mail is not valid.', array('%mail' => 'mail@invalid#')), t('Form validation error for e-mail address was displayed.'));
@@ -283,8 +285,9 @@ class OpenIDFunctionalTest extends DrupalWebTestCase {
 
     $user = user_load_by_name('john');
     $this->assertTrue($user, t('User was registered with right username.'));
+    $this->assertFalse($user->data, t('No additional user info was saved.'));
 
-    // Follow the one-time login that was sent in the confirmation e-mail.
+    // Follow the one-time login that was sent in the welcome e-mail.
     $this->drupalGet(user_pass_reset_url($user));
     $this->drupalPost(NULL, array(), t('Log in'));
 
@@ -305,17 +308,7 @@ class OpenIDFunctionalTest extends DrupalWebTestCase {
 
     // Use a User-supplied Identity that is the URL of an XRDS document.
     $identity = url('openid-test/yadis/xrds', array('absolute' => TRUE));
-
-    // Fill out and submit the login form.
-    $edit = array('openid_identifier' => $identity);
-    $this->drupalPost(NULL, $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'));
-
+    $this->submitLoginForm($identity);
     $this->assertRaw(t('Complete the registration by filling out the form below. If you already have an account, you can <a href="@login">log in</a> now and add your OpenID under "My account".', array('@login' => url('user/login'))), t('User was asked to complete the registration process manually.'));
     $this->assertNoRaw(t('You must enter a username.'), t('Form validation error for username was not displayed.'));
     $this->assertNoRaw(t('You must enter an e-mail address.'), t('Form validation error for e-mail address was not displayed.'));
@@ -327,8 +320,9 @@ class OpenIDFunctionalTest extends DrupalWebTestCase {
 
     $user = user_load_by_name('john');
     $this->assertTrue($user, t('User was registered with right username.'));
+    $this->assertFalse($user->data, t('No additional user info was saved.'));
 
-    // Follow the one-time login that was sent in the confirmation e-mail.
+    // Follow the one-time login that was sent in the welcome e-mail.
     $this->drupalGet(user_pass_reset_url($user));
     $this->drupalPost(NULL, array(), t('Log in'));
 
@@ -346,32 +340,35 @@ class OpenIDFunctionalTest extends DrupalWebTestCase {
   function testRegisterUserWithAXButNoSREG() {
     variable_set('user_email_verification', FALSE);
 
-    // Load the front page to get the user login block.
-    $this->drupalGet('');
-
-    // Use a User-supplied Identity that is the URL of an XRDS document.
-    $identity = url('openid-test/yadis/xrds', array('absolute' => TRUE));
-
     // Tell openid_test.module to respond with these AX fields.
     variable_set('openid_test_response', array(
       'openid.ns.ext123' => 'http://openid.net/srv/ax/1.0',
       'openid.ext123.value.email' => 'john@example.com',
     ));
 
+    // Use a User-supplied Identity that is the URL of an XRDS document.
+    $identity = url('openid-test/yadis/xrds', array('absolute' => TRUE));
+    $this->submitLoginForm($identity);
+    $this->assertLink('john', 0, t('User was logged in.'));
+
+    $user = user_load_by_name('john');
+    $this->assertTrue($user, t('User was registered with right username.'));
+    $this->assertEqual($user->mail, 'john@example.com', t('User was registered with right email address.'));
+  }
+
+  /**
+   * Initiates the login procedure using the specified User-supplied Identity.
+   */
+  function submitLoginForm($identity) {
     // Fill out and submit the login form.
     $edit = array('openid_identifier' => $identity);
-    $this->drupalPost(NULL, $edit, t('Log in'));
+    $this->drupalPost('', $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'));
-    $this->assertLink('john', 0, t('User was logged in.'));
-
-    $user = user_load_by_name('john');
-    $this->assertTrue($user, t('User was registered with right username.'));
-    $this->assertEqual($user->mail, 'john@example.com', t('User was registered with right email address.'));
   }
 }