1

I have a requirement to generate a unique alphanumeric string for each user when they register on our Wordpress site.

I've added the following code into my child theme's functions.php file, and it works fine - it's storing the 10 character string as "token_id" in "usermeta" table.

function generate_token_id($length = 10) {
    $characters = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ';
    $result = '';
    for ($i = 0; $i < $length; ++$i) {
        $result .= $characters[mt_rand(0, strlen($characters) - 1)];
        }
    return $result;
}

add_action('user_register', 'save_token_id', 10, 2);

function save_token_id($user_id) {
    $meta_key = "token_id";
    $key = generate_token_id();
    update_user_meta($user_id, $meta_key, $key);
}

The problem I have, is the second function does not check whether the generated string is already present in the database, and could potentially cause a duplication.

I've updated the function to include a do / while loop below to validate, however this is really stretching my knowledge, and just need some experienced eyes to tell me if I have the do / while / sql select routine correct.

function generate_token_id($length = 10) {
    $characters = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ';
    $result = '';
    for ($i = 0; $i < $length; ++$i) {
        $result .= $characters[mt_rand(0, strlen($characters) - 1)];
        }
    return $result;
}

add_action('user_register', 'save_token_id', 10, 2);

function save_token_id($user_id) {
    $meta_key = "token_id";
    do {
        $key = generate_token_id();
        $keycheck = $wpdb->get_results("select count(*) from `usermeta` where `meta_value` like '"$key"'");
    } while ($keycheck > 0);
    update_user_meta($user_id, $meta_key, $key);
}

UPDATE: So I've declared "global $wpdb;" inside the function, and I've changed the SQL syntax, so it now looks like this:

function save_token_id($user_id) {
    global $wpdb;
    $meta_key = "token_id";
    do {
        $key = generate_token_id();
        $keycheck = $wpdb->get_results("select count(*) from $wpdb->usermeta where meta_value = " . $key);
    } while ($keycheck > 0);
    update_user_meta($user_id, $meta_key, $key);
}

However the PHP DEBUG log is full of SQL errors.

WordPress database error: [Unknown column '74TTW1PIPP' in 'where clause']
select count(*) from wp_usermeta where meta_value = 74TTW1PIPP

WordPress database error: [Unknown column 'CST10WY8EQ' in 'where clause']
select count(*) from wp_usermeta where meta_value = CST10WY8EQ

WordPress database error: [Unknown column 'M3GSGAHD5J' in 'where clause']
select count(*) from wp_usermeta where meta_value = M3GSGAHD5J

I've validated the SQL query in phpMyAdmin, however I can't get it correct in the PHP function. meta_value column is clearly declared, why is it using the $key variable as the column?

Daniel Widdis
  • 8,424
  • 13
  • 41
  • 63
Miles
  • 173
  • 1
  • 9
  • Instead of `generate_token_id()` use `bin2hex(random_bytes(15))`. Or alternatively, use `random_int` rather than `mt_rand`; the latter is not a cryptographic random generator, meaning it can produce predictable IDs. See also https://stackoverflow.com/questions/1846202 . – Peter O. Feb 07 '20 at 06:08

2 Answers2

2

I don't think so you need to extra check for the existing generated duplicate key in the database if you use any function that can generate cryptographically secure hashed string. For example, you may consider following function by passing your $user_id to the function to get a unique hashed key.

<?php
/**
 * Generate cryptographically secure hashed string
 *
 * @param int|string $user_id
 * @param int $length
 * @param string $hash Any cryptographically secure hash
 * For example: 'gost-crypto', 'whirlpool', 'ripemd128' and for more
 * checkout https://www.php.net/manual/en/function.hash-algos.php
 *
 * @param int $iterations The number of internal iterations to perform for the derivation
 * @return string
 */
function generate_unique_id($user_id, $length = 20, $hash = 'ripemd128', $iterations = 10000) {
    // Generate a random IV
    if (version_compare(PHP_VERSION, '7.0.0', '<')) {
        $salt = openssl_random_pseudo_bytes(16);
    } else {
        $salt = random_bytes(16);
    }

    return hash_pbkdf2($hash, $user_id, $salt, $iterations, $length);
}

// Assuming 1 is the user ID
echo generate_unique_id(1);
unclexo
  • 3,691
  • 2
  • 18
  • 26
  • thanks for posting this, I did read through your linked thread during my research for this solution and decided against this implementation. From what I see, the majority of crypto generated strings are hexidecimal meaning there is a 1 in 16 chance of repetition, while a the alphenumeric string above is 1 in 36. Additionally I really only need 7 or 8 characters, not 20+, so the chance of collision also increases, especially for hexidecimal, and I'm covering this by checking if the generated string exists before saving. However the SQL check is the bit I can't get to work. – Miles Feb 08 '20 at 00:03
  • Well, I disagree with this "the chance of collision". Because the function I wrote will take each user ID (this must be unique) as argument and give you crytographically secure unique hash against the user ID. Since the user ID is unique then generated hash should be unique. – unclexo Feb 08 '20 at 04:21
  • The hash_pbkdf2() function returns hexidecimal characters, its missing G-Z as output - that's 20 additional character options per combination (16 versus 36). I understand how the crypto works, I only need 7 characters, so 36^7 provides more combinations than 16^7. At some point, there is going to be repetition / collision if I'm only using 7 characters for a crypto function, so I'm electing to use an alphanumeric combination and add sql validation with the do / while loop. – Miles Feb 08 '20 at 05:48
  • I think I get you. :) – unclexo Feb 08 '20 at 05:57
0

Instead of $wpdb->get_results use of $wpdb->get_var()

 $keycheck = $wpdb->get_var("select count(*) from `usermeta` where `meta_value` like '"$key"'");
}
dhamo
  • 545
  • 2
  • 7