SAML-Toolkits / wordpress-saml

OneLogin SAML plugin for Wordpress
MIT License
65 stars 75 forks source link

{prefix}_users.user_nicename has extra characters added upon login #96

Open bobbrodie opened 4 years ago

bobbrodie commented 4 years ago

Prerequisites

Replication

Expected Behavior

Actual Behavior

sandykadam commented 4 years ago

Thanks @bobbrodie I have also reported this issue in Wordpress Plugin support section - https://wordpress.org/support/topic/using-saml-login-user_nicename-field-getting-appended-with-2/

bobbrodie commented 4 years ago

Ah, I didn't see that @sandykadam -- thanks for noting!

pitbulk commented 4 years ago

I tested with Wordpress 5.4 and was not able to reproduce

lloan commented 3 years ago

Any update on this? This has been plaguing a project I've been working on for the past 2 years. Would be great to get this fixed.

pitbulk commented 3 years ago

@lloan I can't fix what I can't reproduce

The plugin basically did not support nickname so the provided username is used by the wp_update_user that internally calls to wp_insert_user

That method has the following code:


if ( ! empty( $userdata['user_nicename'] ) ) {
    $user_nicename = sanitize_user( $userdata['user_nicename'], true );
    if ( mb_strlen( $user_nicename ) > 50 ) {
         return new WP_Error( 'user_nicename_too_long', __( 'Nicename may not be longer than 50 characters.' ) );
    }
} else {
    $user_nicename = mb_substr( $user_login, 0, 50 );
}
 
$user_nicename = sanitize_title( $user_nicename );
$user_nicename = apply_filters( 'pre_user_nicename', $user_nicename );
 
$user_nicename_check = $wpdb->get_var( $wpdb->prepare( "SELECT ID FROM $wpdb->users WHERE user_nicename = %s AND user_login != %s LIMIT 1", $user_nicename, $user_login ) );
 
if ( $user_nicename_check ) {
    $suffix = 2;
    while ( $user_nicename_check ) {
        // user_nicename allows 50 chars. Subtract one for a hyphen, plus the length of the suffix.
        $base_length         = 49 - mb_strlen( $suffix );
        $alt_user_nicename   = mb_substr( $user_nicename, 0, $base_length ) . "-$suffix";
        $user_nicename_check = $wpdb->get_var( $wpdb->prepare( "SELECT ID FROM $wpdb->users WHERE user_nicename = %s AND user_login != %s LIMIT 1", $alt_user_nicename, $user_login ) );
       $suffix++;
    }
    $user_nicename = $alt_user_nicename;
}

$nickname = empty( $userdata['nickname'] ) ? $user_login : $userdata['nickname'];
$meta['nickname'] = apply_filters( 'pre_user_nickname', $nickname );

So the suffix should only be added under the condition:

I only can think in a weird scenario where the user that breaks the rule is the same user registered, but I was not able to figure it out.

Maybe you can provide the user data provided by the IdP, and the users in your database that has same nickname or same username, so I can try to reproduce here.

Meanwhile, I added nickname support in case this can prevent the issue to appear.

Ref: https://core.trac.wordpress.org/ticket/39370