From 1b2f81d058026a7a14273dc8f0cdb80eca895afa Mon Sep 17 00:00:00 2001 From: Bradley M. Froehle Date: Thu, 7 Apr 2011 17:04:52 -0700 Subject: [PATCH] Issue #817118 - Remove {authmap} and migrate OpenID entries to their own table. --- modules/openid/openid.install | 101 +++++++++++++++++++++++++++++++++++++++ modules/openid/openid.module | 35 +++++++++++++- modules/openid/openid.pages.inc | 18 +++---- modules/user/user.install | 42 ---------------- modules/user/user.module | 99 -------------------------------------- modules/user/user.test | 60 ----------------------- 6 files changed, 142 insertions(+), 213 deletions(-) diff --git a/modules/openid/openid.install b/modules/openid/openid.install index 2df39aa..6c700d3 100644 --- a/modules/openid/openid.install +++ b/modules/openid/openid.install @@ -54,6 +54,41 @@ function openid_schema() { 'primary key' => array('assoc_handle'), ); + $schema['openid_identities'] = array( + 'description' => 'Stores OpenID authentication mapping.', + 'fields' => array( + 'aid' => array( + 'description' => 'Primary Key: Unique authmap ID.', + 'type' => 'serial', + 'unsigned' => TRUE, + 'not null' => TRUE, + ), + 'uid' => array( + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + 'description' => "User's {users}.uid.", + ), + 'identifier' => array( + 'type' => 'varchar', + 'length' => 128, + 'not null' => TRUE, + 'default' => '', + 'description' => 'Unique OpenID identifier.', + ), + ), + 'unique keys' => array( + 'identifier' => array('identifier'), + ), + 'primary key' => array('aid'), + 'foreign keys' => array( + 'user' => array( + 'table' => 'users', + 'columns' => array('uid' => 'uid'), + ), + ), + ); + $schema['openid_nonce'] = array( 'description' => 'Stores received openid.response_nonce per OpenID endpoint URL to prevent replay attacks.', 'fields' => array( @@ -158,3 +193,69 @@ function openid_update_6000() { /** * @} End of "addtogroup updates-6.x-to-7.x" */ + +/** + * @addtogroup updates-7.x-to-8.x + * @{ + */ + +/** + * Move authentication mapping to an OpenID managed table. + */ +function openid_update_8000() { + + $schema['openid_identities'] = array( + 'description' => 'Stores OpenID authentication mapping.', + 'fields' => array( + 'aid' => array( + 'description' => 'Primary Key: Unique authmap ID.', + 'type' => 'serial', + 'unsigned' => TRUE, + 'not null' => TRUE, + ), + 'uid' => array( + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + 'description' => "User's {users}.uid.", + ), + 'identifier' => array( + 'type' => 'varchar', + 'length' => 128, + 'not null' => TRUE, + 'default' => '', + 'description' => 'Unique OpenID identifier.', + ), + ), + 'unique keys' => array( + 'identifier' => array('identifier'), + ), + 'primary key' => array('aid'), + 'foreign keys' => array( + 'user' => array( + 'table' => 'users', + 'columns' => array('uid' => 'uid'), + ), + ), + ); + + db_create_table('openid_identities', $schema['openid_identities']); + + // Migrate entries from {authmap} to {openid_identities}. + $query = db_select('authmap', 'a') + ->condition('module', 'openid'); + $query->addField('a', 'uid'); + $query->addField('a', 'authname', 'identifier'); + db_insert('openid_identities') + ->from($query) + ->execute(); + + // Remove old entries in {authmap}. + db_delete('authmap') + ->condition('module', 'openid') + ->execute(); +} + +/** + * @} End of "addtogroup updates-7.x-to-8.x" + */ diff --git a/modules/openid/openid.module b/modules/openid/openid.module index 7673de8..e3bc6cc 100644 --- a/modules/openid/openid.module +++ b/modules/openid/openid.module @@ -81,6 +81,32 @@ function openid_help($path, $arg) { } /** + * Fetch a user object by OpenID identifier. + * + * @param $identifier + * The OpenID identifier. + * + * @return + * A fully-loaded user object if the user is found or FALSE if not found. + */ +function openid_external_load($identifier) { + $uid = db_query("SELECT uid FROM {openid_identities} WHERE identifier = :identifier", array(':identifier' => $identifier))->fetchField(); + if ($uid) { + return user_load($uid); + } + return FALSE; +} + +/** + * Implements hook_user_delete(). + */ +function openid_user_delete($account) { + db_delete('openid_identities') + ->condition('uid', $account->uid) + ->execute(); +} + +/** * Implements hook_user_insert(). */ function openid_user_insert(&$edit, $account, $category) { @@ -89,7 +115,12 @@ function openid_user_insert(&$edit, $account, $category) { 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' => $edit['openid_claimed_id'])); + db_insert('openid_identities') + ->fields(array( + 'uid' => $account->uid, + 'identifier' => $edit['openid_claimed_id'], + )) + ->execute(); unset($_SESSION['openid']); unset($edit['openid_claimed_id']); } @@ -590,7 +621,7 @@ function openid_association($op_endpoint) { function openid_authentication($response) { $identity = $response['openid.claimed_id']; - $account = user_external_load($identity); + $account = openid_external_load($identity); if (isset($account->uid)) { if (!variable_get('user_email_verification', TRUE) || $account->login) { // Check if user is blocked. diff --git a/modules/openid/openid.pages.inc b/modules/openid/openid.pages.inc index 6e3f096..c25d480 100644 --- a/modules/openid/openid.pages.inc +++ b/modules/openid/openid.pages.inc @@ -34,11 +34,10 @@ function openid_user_identities($account) { $result = openid_complete(); if ($result['status'] == 'success') { $identity = $result['openid.claimed_id']; - $query = db_insert('authmap') + $query = db_insert('openid_identities') ->fields(array( 'uid' => $account->uid, - 'authname' => $identity, - 'module' => 'openid', + 'identifier' => $identity, )) ->execute(); drupal_set_message(t('Successfully added %identity', array('%identity' => $identity))); @@ -47,9 +46,9 @@ function openid_user_identities($account) { $header = array(t('OpenID'), t('Operations')); $rows = array(); - $result = db_query("SELECT * FROM {authmap} WHERE module='openid' AND uid=:uid", array(':uid' => $account->uid)); + $result = db_query("SELECT * FROM {openid_identities} WHERE uid=:uid", array(':uid' => $account->uid)); foreach ($result as $identity) { - $rows[] = array(check_plain($identity->authname), l(t('Delete'), 'user/' . $account->uid . '/openid/delete/' . $identity->aid)); + $rows[] = array(check_plain($identity->identifier), l(t('Delete'), 'user/' . $account->uid . '/openid/delete/' . $identity->aid)); } $build['openid_table'] = array( @@ -80,7 +79,7 @@ function openid_user_add() { function openid_user_add_validate($form, &$form_state) { // Check for existing entries. $claimed_id = openid_normalize($form_state['values']['openid_identifier']); - if (db_query("SELECT authname FROM {authmap} WHERE authname = :authname", (array(':authname' => $claimed_id)))->fetchField()) { + if (db_query("SELECT identifier FROM {openid_identities} WHERE identifier = :identifier", array(':identifier' => $claimed_id))->fetchField()) { form_set_error('openid_identifier', t('That OpenID is already in use on this site.')); } } @@ -94,19 +93,18 @@ function openid_user_add_submit($form, &$form_state) { * Menu callback; Delete the specified OpenID identity from the system. */ function openid_user_delete_form($form, $form_state, $account, $aid = 0) { - $authname = db_query("SELECT authname FROM {authmap} WHERE uid = :uid AND aid = :aid AND module = 'openid'", array( + $identifier = db_query("SELECT identifier FROM {openid_identities} WHERE uid = :uid AND aid = :aid", array( ':uid' => $account->uid, ':aid' => $aid, )) ->fetchField(); - return confirm_form(array(), t('Are you sure you want to delete the OpenID %authname for %user?', array('%authname' => $authname, '%user' => $account->name)), 'user/' . $account->uid . '/openid'); + return confirm_form(array(), t('Are you sure you want to delete the OpenID %identifier for %user?', array('%identifier' => $identifier, '%user' => $account->name)), 'user/' . $account->uid . '/openid'); } function openid_user_delete_form_submit($form, &$form_state) { - $query = db_delete('authmap') + $query = db_delete('openid_identities') ->condition('uid', $form_state['build_info']['args'][0]->uid) ->condition('aid', $form_state['build_info']['args'][1]) - ->condition('module', 'openid') ->execute(); if ($query) { drupal_set_message(t('OpenID deleted.')); diff --git a/modules/user/user.install b/modules/user/user.install index 370427f..7242372 100644 --- a/modules/user/user.install +++ b/modules/user/user.install @@ -9,48 +9,6 @@ * Implements hook_schema(). */ function user_schema() { - $schema['authmap'] = array( - 'description' => 'Stores distributed authentication mapping.', - 'fields' => array( - 'aid' => array( - 'description' => 'Primary Key: Unique authmap ID.', - 'type' => 'serial', - 'unsigned' => TRUE, - 'not null' => TRUE, - ), - 'uid' => array( - 'type' => 'int', - 'not null' => TRUE, - 'default' => 0, - 'description' => "User's {users}.uid.", - ), - 'authname' => array( - 'type' => 'varchar', - 'length' => 128, - 'not null' => TRUE, - 'default' => '', - 'description' => 'Unique authentication name.', - ), - 'module' => array( - 'type' => 'varchar', - 'length' => 128, - 'not null' => TRUE, - 'default' => '', - 'description' => 'Module which is controlling the authentication.', - ), - ), - 'unique keys' => array( - 'authname' => array('authname'), - ), - 'primary key' => array('aid'), - 'foreign keys' => array( - 'user' => array( - 'table' => 'users', - 'columns' => array('uid' => 'uid'), - ), - ), - ); - $schema['role_permission'] = array( 'description' => 'Stores the permissions assigned to user roles.', 'fields' => array( diff --git a/modules/user/user.module b/modules/user/user.module index 7f22f7c..e191f16 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -218,26 +218,6 @@ function user_field_extra_fields() { } /** - * Fetches a user object based on an external authentication source. - * - * @param string $authname - * The external authentication username. - * - * @return - * A fully-loaded user object if the user is found or FALSE if not found. - */ -function user_external_load($authname) { - $uid = db_query("SELECT uid FROM {authmap} WHERE authname = :authname", array(':authname' => $authname))->fetchField(); - - if ($uid) { - return user_load($uid); - } - else { - return FALSE; - } -} - -/** * Load multiple users based on certain conditions. * * This function should be used whenever you need to load more than one user @@ -1962,52 +1942,6 @@ function user_page_title($account) { } /** - * Discover which external authentication module(s) authenticated a username. - * - * @param $authname - * A username used by an external authentication module. - * @return - * An associative array with module as key and username as value. - */ -function user_get_authmaps($authname = NULL) { - $authmaps = db_query("SELECT module, authname FROM {authmap} WHERE authname = :authname", array(':authname' => $authname))->fetchAllKeyed(); - return count($authmaps) ? $authmaps : 0; -} - -/** - * Save mappings of which external authentication module(s) authenticated - * a user. Maps external usernames to user ids in the users table. - * - * @param $account - * A user object. - * @param $authmaps - * An associative array with a compound key and the username as the value. - * The key is made up of 'authname_' plus the name of the external authentication - * module. - * @see user_external_login_register() - */ -function user_set_authmaps($account, $authmaps) { - foreach ($authmaps as $key => $value) { - $module = explode('_', $key, 2); - if ($value) { - db_merge('authmap') - ->key(array( - 'uid' => $account->uid, - 'module' => $module[1], - )) - ->fields(array('authname' => $value)) - ->execute(); - } - else { - db_delete('authmap') - ->condition('uid', $account->uid) - ->condition('module', $module[1]) - ->execute(); - } - } -} - -/** * Form builder; the main user login form. * * @ingroup forms @@ -2225,36 +2159,6 @@ function user_login_submit($form, &$form_state) { } /** - * Helper function for authentication modules. Either logs in or registers - * the current user, based on username. Either way, the global $user object is - * populated and login tasks are performed. - */ -function user_external_login_register($name, $module) { - $account = user_external_load($name); - if (!$account) { - // Register this new user. - $userinfo = array( - 'name' => $name, - 'pass' => user_password(), - 'init' => $name, - 'status' => 1, - 'access' => REQUEST_TIME - ); - $account = user_save(drupal_anonymous_user(), $userinfo); - // Terminate if an error occurred during user_save(). - if (!$account) { - drupal_set_message(t("Error saving user account."), 'error'); - return; - } - user_set_authmaps($account, array("authname_$module" => $name)); - } - - // Log user in. - $form_state['uid'] = $account->uid; - user_login_submit(array(), $form_state); -} - -/** * Generates a unique URL for a user to login and reset their password. * * @param object $account @@ -2418,9 +2322,6 @@ function user_delete_multiple(array $uids) { db_delete('users_roles') ->condition('uid', $uids, 'IN') ->execute(); - db_delete('authmap') - ->condition('uid', $uids, 'IN') - ->execute(); } catch (Exception $e) { $transaction->rollback(); diff --git a/modules/user/user.test b/modules/user/user.test index 3c453a8..7ccb21d 100644 --- a/modules/user/user.test +++ b/modules/user/user.test @@ -2040,66 +2040,6 @@ class UserRolesAssignmentTestCase extends DrupalWebTestCase { } } - -/** - * Unit test for authmap assignment. - */ -class UserAuthmapAssignmentTestCase extends DrupalWebTestCase { - public static function getInfo() { - return array( - 'name' => t('Authmap assignment'), - 'description' => t('Tests that users can be assigned and unassigned authmaps.'), - 'group' => t('User') - ); - } - - /** - * Test authmap assignment and retrieval. - */ - function testAuthmapAssignment() { - $account = $this->drupalCreateUser(); - - // Assign authmaps to the user. - $authmaps = array( - 'authname_poll' => 'external username one', - 'authname_book' => 'external username two', - ); - user_set_authmaps($account, $authmaps); - - // Test for expected authmaps. - $expected_authmaps = array( - 'external username one' => array( - 'poll' => 'external username one', - ), - 'external username two' => array( - 'book' => 'external username two', - ), - ); - foreach ($expected_authmaps as $authname => $expected_output) { - $this->assertIdentical(user_get_authmaps($authname), $expected_output, t('Authmap for authname %authname was set correctly.', array('%authname' => $authname))); - } - - // Remove authmap for module poll, add authmap for module blog. - $authmaps = array( - 'authname_poll' => NULL, - 'authname_blog' => 'external username three', - ); - user_set_authmaps($account, $authmaps); - - // Assert that external username one does not have authmaps. - $remove_username = 'external username one'; - unset($expected_authmaps[$remove_username]); - $this->assertFalse(user_get_authmaps($remove_username), t('Authmap for %authname was removed.', array('%authname' => $remove_username))); - - // Assert that a new authmap was created for external username three, and - // existing authmaps for external username two were unchanged. - $expected_authmaps['external username three'] = array('blog' => 'external username three'); - foreach ($expected_authmaps as $authname => $expected_output) { - $this->assertIdentical(user_get_authmaps($authname), $expected_output, t('Authmap for authname %authname was set correctly.', array('%authname' => $authname))); - } - } -} - /** * Tests user_validate_current_pass on a custom form. */ -- 1.7.3.5