From cf5baefabf942edadd64a1170b2b4ef7387a263b Mon Sep 17 00:00:00 2001 From: Victor Boctor Date: Mon, 29 Jan 2018 22:34:28 -0800 Subject: [PATCH] Fix username and realname uniqueness checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix user realname uniqueness check which wasn’t working. - Remove `$g_differentiate_duplicates` config option. - Change username realname uniqueness check APIs to not take in username, since these are independent operations. Fixes #23909, #23900 --- account_update.php | 3 +- config_defaults_inc.php | 7 --- core/obsolete.php | 3 ++ core/user_api.php | 106 +++++++++++++--------------------------- manage_user_update.php | 2 +- 5 files changed, 39 insertions(+), 82 deletions(-) diff --git a/account_update.php b/account_update.php index 409bafb02e..b3ae53792d 100644 --- a/account_update.php +++ b/account_update.php @@ -113,8 +113,7 @@ $t_realname = string_normalize( $f_realname ); if( $t_realname != user_get_field( $t_user_id, 'realname' ) ) { # checks for problems with realnames - $t_username = user_get_username( $t_user_id ); - user_ensure_realname_unique( $t_username, $t_realname ); + user_ensure_realname_unique( $t_realname, $t_user_id ); $t_update_realname = true; } } diff --git a/config_defaults_inc.php b/config_defaults_inc.php index 8357e1ca8b..29f5537315 100644 --- a/config_defaults_inc.php +++ b/config_defaults_inc.php @@ -1208,12 +1208,6 @@ */ $g_show_realname = OFF; -/** - * leave off for now - * @global integer $g_differentiate_duplicates - */ -$g_differentiate_duplicates = OFF; - /** * sorting for names in dropdown lists. If turned on, "Jane Doe" will be sorted * with the "D"s @@ -4479,7 +4473,6 @@ 'delete_bugnote_threshold', 'delete_project_threshold', 'development_team_threshold', - 'differentiate_duplicates', 'disallowed_files', 'display_bug_padding', 'display_bugnote_padding', diff --git a/core/obsolete.php b/core/obsolete.php index d808e100f7..d0ebdfbf62 100644 --- a/core/obsolete.php +++ b/core/obsolete.php @@ -214,3 +214,6 @@ # changes in 2.9.0 config_obsolete( 'meta_include_file' ); + +# changes in 2.11.0 +config_obsolete( 'differentiate_duplicates' ); diff --git a/core/user_api.php b/core/user_api.php index 4f875e4a74..3f9dd5bd67 100644 --- a/core/user_api.php +++ b/core/user_api.php @@ -242,23 +242,31 @@ function user_ensure_exists( $p_user_id ) { /** * return true if the username is unique, false if there is already a user with that username * @param string $p_username The username to check. + * @param integer|null The user id allowed to conflict, otherwise null. * @return boolean */ -function user_is_name_unique( $p_username ) { - db_param_push(); - $t_query = 'SELECT username FROM {user} WHERE username=' . db_param(); - $t_result = db_query( $t_query, array( $p_username ), 1 ); +function user_is_name_unique( $p_username, $p_user_id = null ) { + $t_existing_user_id = user_get_id_by_name( $p_username ); + if( $t_existing_user_id !== false && ( $p_user_id === null || (int)$t_existing_user_id !== $p_user_id ) ) { + return false; + } - return !db_result( $t_result ); + $t_existing_user_id = user_get_id_by_realname( $p_username ); + if( $t_existing_user_id !== false && ( $p_user_id === null || (int)$t_existing_user_id !== $p_user_id ) ) { + return false; + } + + return true; } /** * Check if the username is unique and trigger an ERROR if it isn't * @param string $p_username The username to check. + * @param integer|null $p_user_id The user id allowed to conflict, otherwise null. * @return void */ -function user_ensure_name_unique( $p_username ) { - if( !user_is_name_unique( $p_username ) ) { +function user_ensure_name_unique( $p_username, $p_user_id = null ) { + if( !user_is_name_unique( $p_username, $p_user_id ) ) { throw new ClientException( sprintf( "Username '%s' already used.", $p_username ), ERROR_USER_NAME_NOT_UNIQUE ); @@ -318,64 +326,42 @@ function user_ensure_email_unique( $p_email, $p_user_id = null ) { /** * Check if realname is specified, a unique real name, and doesn't match a username of another user. * - * @param string $p_username The username to check. * @param string $p_realname The realname to check. - * @return integer 0 realname matches another user's username, 1 no conflicts, >1 realname not unique. + * @param integer|null $p_user_id The user id that is allowed to conflict, otherwise, null. + * @return bool true: unique, false otherwise. */ -function user_is_realname_unique( $p_username, $p_realname ) { +function user_is_realname_unique( $p_realname, $p_user_id = null ) { if( is_blank( $p_realname ) ) { # don't bother checking if realname is blank - return 1; + return true; } - $p_username = trim( $p_username ); $p_realname = trim( $p_realname ); - # allow realname to match username - $t_duplicate_count = 0; - if( $p_realname !== $p_username ) { - # check realname does not match an existing username - # but allow it to match the current user - $t_target_user = user_get_id_by_name( $p_username ); - $t_other_user = user_get_id_by_name( $p_realname ); - if( ( false !== $t_other_user ) && ( $t_target_user !== $t_other_user ) ) { - return 0; - } - - # check to see if the realname is unique - db_param_push(); - $t_query = 'SELECT id FROM {user} WHERE realname=' . db_param(); - $t_result = db_query( $t_query, array( $p_realname ) ); + # check realname does not match an existing realname + $t_existing_user_id = user_get_id_by_name( $p_realname ); + if( $t_existing_user_id !== false && ( $p_user_id === null || (int)$t_existing_user_id !== $p_user_id ) ) { + return false; + } - $t_users = array(); - while( $t_row = db_fetch_array( $t_result ) ) { - $t_users[] = $t_row; - } - $t_duplicate_count = count( $t_users ); - - if( $t_duplicate_count > 0 ) { - # set flags for non-unique realnames - if( config_get( 'differentiate_duplicates' ) ) { - for( $i = 0;$i < $t_duplicate_count;$i++ ) { - $t_user_id = $t_users[$i]['id']; - user_set_field( $t_user_id, 'duplicate_realname', ON ); - } - } - } + $t_existing_user_id = user_get_id_by_realname( $p_realname ); + if( $t_existing_user_id !== false && ( $p_user_id === null || (int)$t_existing_user_id !== $p_user_id ) ) { + return false; } - return $t_duplicate_count + 1; + + return true; } /** * Check if the realname is a unique * Trigger an error if the username is not valid * - * @param string $p_username The username to check. * @param string $p_realname The realname to check. + * @param integer|null $p_user_id The user id that is allowed to conflict, otherwise null. * @return void */ -function user_ensure_realname_unique( $p_username, $p_realname ) { - if( 1 > user_is_realname_unique( $p_username, $p_realname ) ) { +function user_ensure_realname_unique( $p_realname, $p_user_id = null ) { + if( !user_is_realname_unique( $p_realname, $p_user_id ) ) { throw new ClientException( sprintf( "Realname '%s' is not unique", $p_realname ), ERROR_USER_REAL_MATCH_USER ); @@ -595,7 +581,7 @@ function user_create( $p_username, $p_password, $p_email = '', user_ensure_name_valid( $p_username ); user_ensure_name_unique( $p_username ); user_ensure_email_unique( $p_email ); - user_ensure_realname_unique( $p_username, $p_realname ); + user_ensure_realname_unique( $p_realname ); email_ensure_valid( $p_email ); email_ensure_not_disposable( $p_email ); @@ -736,30 +722,6 @@ function user_delete( $p_user_id ) { # Remove project specific access levels user_delete_project_specific_access_levels( $p_user_id ); - - # unset non-unique realname flags if necessary - if( config_get( 'differentiate_duplicates' ) ) { - $c_realname = user_get_field( $p_user_id, 'realname' ); - db_param_push(); - $t_query = 'SELECT id FROM {user} WHERE realname=' . db_param(); - $t_result = db_query( $t_query, array( $c_realname ) ); - - $t_users = array(); - while( $t_row = db_fetch_array( $t_result ) ) { - $t_users[] = $t_row; - } - - $t_user_count = count( $t_users ); - - if( $t_user_count == 2 ) { - # unset flags if there are now only 2 unique names - for( $i = 0;$i < $t_user_count;$i++ ) { - $t_user_id = $t_users[$i]['id']; - user_set_field( $t_user_id, 'duplicate_realname', OFF ); - } - } - } - user_clear_cache( $p_user_id ); # Remove account @@ -1758,7 +1720,7 @@ function user_set_realname( $p_user_id, $p_realname ) { */ function user_set_name( $p_user_id, $p_username ) { user_ensure_name_valid( $p_username ); - user_ensure_name_unique( $p_username ); + user_ensure_name_unique( $p_username, $p_user_id ); return user_set_field( $p_user_id, 'username', $p_username ); } diff --git a/manage_user_update.php b/manage_user_update.php index d7d82eb0d9..b69379dcae 100644 --- a/manage_user_update.php +++ b/manage_user_update.php @@ -111,7 +111,7 @@ } else { # strip extra space from real name $t_realname = string_normalize( $f_realname ); - user_ensure_realname_unique( $t_old_username, $t_realname ); + user_ensure_realname_unique( $t_realname, $f_user_id ); } if( $t_ldap && config_get( 'use_ldap_email' ) ) {