michaelryanmcneill / shibboleth

Shibboleth plugin for WordPress
https://wordpress.org/plugins/shibboleth/
19 stars 11 forks source link

shibboleth_authenticate_user: cleanup #38

Closed jrchamp closed 6 years ago

jrchamp commented 6 years ago

A couple small cleanup items I noticed while working on the other issue:

jrchamp commented 6 years ago

Here's an idea that I think makes the "account combine" code more clear that if shibboleth_account isn't set, we're either going to set it or throw a WP_Error.

// look up existing account by username, with email as a fallback
$user_by = 'username';
$user = get_user_by( 'login', $username );
if ( ! $user ) {
        $user_by = 'email';
        $user = get_user_by( 'email', $email );
}

// if this account is not a Shibboleth account, then do account combine (if allowed)
if ( is_object( $user ) && $user->ID && ! get_user_meta( $user->ID, 'shibboleth_account' ) ) {
        $do_account_combine = false;
        if ( $user_by === 'username' && ( $auto_combine_accounts === 'allow' || $manually_combine_accounts === 'allow' ) ) {
                $do_account_combine = true;
        } elseif ( $auto_combine_accounts === 'bypass' || $manually_combine_accounts === 'bypass' ) {
                $do_account_combine = true;
        }

        if ( $do_account_combine ) {
                update_user_meta( $user->ID, 'shibboleth_account', true );
        } elseif ( $user_by === 'username' ) {
                return new WP_Error( 'invalid_username', __( 'An account already exists with this username.', 'shibboleth' ) );
        } else {
                return new WP_Error( 'invalid_email', __( 'An account already exists with this email.', 'shibboleth' ) );
        }
}

If you like this, I can add it to this PR.

michaelryanmcneill commented 6 years ago

Yeah, I like that @jrchamp. Go ahead and add that to the PR and I'll get it merged in.

jrchamp commented 6 years ago

Ok, I've added the account combine unification commit to the PR.