Issue #765984 by c960657: Convert hook_openid()('request', ...) to hook_openid_request_alter().

core/modules/openid/openid.api.php

@@ -13,21 +13,15 @@
 /**
  * Allow modules to modify the OpenID request parameters.
  *
- * @param $op
- *   The operation to be performed.
- *   Possible values:
- *   - request: Modify parameters before they are sent to the OpenID provider.
  * @param $request
- *   An associative array of parameter defaults to which to modify or append.
- * @return
- *   An associative array of parameters to be merged with the default list.
- *
+ *   An associative array of request parameters.
+ * @param $service
+ *   A service array as returned by openid_discovery().
  */
-function hook_openid($op, $request) {
-  if ($op == 'request') {
+function hook_openid_request_alter(&$request, $service) {
+  if ($request['openid.mode'] == 'checkid_setup') {
     $request['openid.identity'] = 'http://myname.myopenid.com/';
   }
-  return $request;
 }
 
 /**

core/modules/openid/openid.module

@@ -657,13 +657,13 @@ function openid_authentication($response) {
         // 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);
       }
     }
     else {
       drupal_set_message(t('You must validate your email address for this account before logging in via OpenID.'));
     }
+    // Let other modules act on OpenID login
+    module_invoke_all('openid_response', $response, $account);
   }
   elseif (variable_get('user_register', USER_REGISTER_VISITORS_ADMINISTRATIVE_APPROVAL)) {
     // Register new user.
@@ -675,7 +675,10 @@ function openid_authentication($response) {
     $form_state['values']['op'] = t('Create new account');
     drupal_form_submit('user_register_form', $form_state);
 
-    if (!empty($form_state['user'])) {
+    if (empty($form_state['user'])) {
+      module_invoke_all('openid_response', $response, NULL);
+    }
+    else {
       module_invoke_all('openid_response', $response, $form_state['user']);
       drupal_goto();
     }
@@ -703,6 +706,7 @@ function openid_authentication($response) {
   }
   else {
     drupal_set_message(t('Only site administrators can create new user accounts.'), 'error');
+    module_invoke_all('openid_response', $response, NULL);
   }
   drupal_goto();
 }
@@ -773,7 +777,7 @@ function openid_authentication_request($claimed_id, $identity, $return_to = '',
     $request['openid.ax.type.name_son'] = 'http://schema.openid.net/namePerson/friendly';
   }
 
-  $request = array_merge($request, module_invoke_all('openid', 'request', $request));
+  drupal_alter('openid_request', $request, $service);
 
   return $request;
 }

core/modules/openid/openid.pages.inc

@@ -31,9 +31,9 @@ function openid_user_identities($account) {
   drupal_add_css(drupal_get_path('module', 'openid') . '/openid.css');
 
   // Check to see if we got a response
-  $result = openid_complete();
-  if ($result['status'] == 'success') {
-    $identity = $result['openid.claimed_id'];
+  $response = openid_complete();
+  if ($response['status'] == 'success') {
+    $identity = $response['openid.claimed_id'];
     $query = db_insert('authmap')
       ->fields(array(
         'uid' => $account->uid,
@@ -42,6 +42,8 @@ function openid_user_identities($account) {
       ))
       ->execute();
     drupal_set_message(t('Successfully added %identity', array('%identity' => $identity)));
+    // Let other modules act on OpenID authentication.
+    module_invoke_all('openid_response', $response, $account);
   }
 
   $header = array(t('OpenID'), t('Operations'));

core/modules/openid/openid.test

@@ -157,12 +157,22 @@ class OpenIDFunctionalTestCase extends OpenIDWebTestCase {
     // Use a User-supplied Identity that is the URL of an XRDS document.
     $identity = url('openid-test/yadis/xrds', array('absolute' => TRUE));
     $this->addIdentity($identity);
+    $response = variable_get('openid_test_hook_openid_response_response');
+    $account = variable_get('openid_test_hook_openid_response_account');
+    $this->assertEqual($response['openid.claimed_id'], $identity, t('hook_openid_response() was invoked.'));
+    $this->assertEqual($account->uid, $this->web_user->uid, t('Proper user object passed to hook_openid_response().'));
 
     $this->drupalLogout();
 
     // Test logging in via the login block on the front page.
+    variable_del('openid_test_hook_openid_response_response');
+    variable_del('openid_test_hook_openid_response_account');
     $this->submitLoginForm($identity);
     $this->assertLink(t('Log out'), 0, t('User was logged in.'));
+    $response = variable_get('openid_test_hook_openid_response_response');
+    $account = variable_get('openid_test_hook_openid_response_account');
+    $this->assertEqual($response['openid.claimed_id'], $identity, t('hook_openid_response() was invoked.'));
+    $this->assertEqual($account->uid, $this->web_user->uid, t('Proper user object passed to hook_openid_response().'));
 
     $this->drupalLogout();
 
@@ -180,6 +190,17 @@ class OpenIDFunctionalTestCase extends OpenIDWebTestCase {
 
     // Verify user was redirected away from user/login to an accessible page.
     $this->assertResponse(200);
+
+    $this->drupalLogout();
+
+    // Tell openid_test.module to alter the checkid_setup request.
+    $new_identity = 'http://example.com/' . $this->randomName();
+    variable_set('openid_test_identity', $new_identity);
+    variable_set('openid_test_request_alter', array('checkid_setup' => array('openid.identity' => $new_identity)));
+    $this->submitLoginForm($identity);
+    $this->assertLink(t('Log out'), 0, t('User was logged in.'));
+    $response = variable_get('openid_test_hook_openid_response_response');
+    $this->assertEqual($response['openid.identity'], $new_identity, t('hook_openid_request_alter() were invoked.'));
   }
 
   /**
@@ -449,8 +470,11 @@ class OpenIDRegistrationTestCase extends OpenIDWebTestCase {
     $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' => $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.'));
+    $this->assertTrue(variable_get('openid_test_hook_openid_response_response'), t('hook_openid_response() was invoked.'));
+    $this->assertFalse(variable_get('openid_test_hook_openid_response_account', TRUE), t('No user object passed to hook_openid_response().'));
 
     // Enter username and e-mail address manually.
+    variable_del('openid_test_hook_openid_response_response');
     $edit = array('name' => 'john', 'mail' => 'john@example.com');
     $this->drupalPost(NULL, $edit, t('Create new account'));
     $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.'));
@@ -463,6 +487,7 @@ class OpenIDRegistrationTestCase extends OpenIDWebTestCase {
     // Follow the one-time login that was sent in the welcome e-mail.
     $this->drupalGet($reset_url);
     $this->drupalPost(NULL, array(), t('Log in'));
+    $this->assertFalse(variable_get('openid_test_hook_openid_response_response'), t('hook_openid_response() was not invoked.'));
 
     // The user is taken to user/%uid/edit.
     $this->assertFieldByName('mail', 'john@example.com', t('User was registered with right e-mail address.'));

core/modules/openid/tests/openid_test.module

@@ -350,3 +350,21 @@ function _openid_test_endpoint_authenticate() {
   drupal_add_http_header('Content-Type', 'text/plain');
   header('Location: ' . url($_REQUEST['openid_return_to'], array('query' => $response, 'external' => TRUE)));
 }
+
+/**
+ * Implements hook_openid_request_alter().
+ */
+function openid_test_openid_request_alter(&$request, $service) {
+  $parameters = variable_get('openid_test_request_alter', array());
+  if (isset($parameters[$request['openid.mode']])) {
+    $request = $parameters[$request['openid.mode']] + $request;
+  }
+}
+
+/**
+ * Implements hook_openid_response().
+ */
+function openid_test_openid_response($response, $account) {
+  variable_set('openid_test_hook_openid_response_response', $response);
+  variable_set('openid_test_hook_openid_response_account', $account ? $account : FALSE);
+}