Some errors

  • Resolved ttodua

    (@ttodua)


    4 years, 11 months ago

    Hi, nice plugin, thank you !
    Just some issues, and if you address them, I suppose your plugin becomes very solid.

    1) in “admin/class-sign-in-with-google-admin.php , line 797, there definitely needs to have added array_filter like this:
    array_filter( explode( ', ', get_option( 'siwg_google_domain_restriction' ) ) ) ) );

    because otherwise, even empty string exploded by comma, it makes the next step: ! empty( $domains ) to return TRUE (because $domains[0] member exists, even though that member has empty value). So, array_filter will fix that. Otherwise, we are returned back to our site afer G-login, and still shown the wp-login screen (with appended url: /?google_login=incorrect_domain ), because of that line.

    2) would be nice, if you add hyperlink to FAQ of this plugin (or even to Google Console) directly in Wp dashboard, aside the “enter Google credential” fields in your plugin. will facilitate things.

    3) May I ask, is this plugin (generally do you) maximally protected in terms of code-security, to be free from weak holes (unprotected nonce, bypass authorization, tricking in registering/logging in to other user profile, etc…)? Just wanted to make sure , have you are emphasized on code security?

    4) would be nice if you made addition – to add a new textfield, where we can enter the list of usernames (i.e. [email protected]), comma separated, and they will be blocked from registering? but note, google supports including dot in email and also + extension sign, so to avoid registering with i.e. [email protected] or [email protected], you will need to sanitize str_replace( '.','', preg_replace('/(\+.\@)/', '', $email) ); or like that, and after that check if that mail is blocked.

    5) add option for admin, to block changing the password for those users, who signed up automatically with g-login. otherwise, people might login once with google, and then set password in wp, and then simply login with into WP-login page with plain email+pass . Admin should have option to prevent that or not. I think you will just need to add option in site, to check “if user ID X is registered with g-login, disable password set/change for him).

    6) would be good also to have preventing auto-registration (even though if user successfully logins to his google), so, only the pre-registered (by admin) emails could login/join the WP site.

    • This topic was modified 4 years, 11 months ago by ttodua.
    • This topic was modified 4 years, 11 months ago by ttodua.
    • This topic was modified 4 years, 11 months ago by ttodua.
Viewing 7 replies - 1 through 7 (of 7 total)
  • Thread Starter ttodua

    (@ttodua)

    7) also, will be nice to have ability to fire the functions from external plugins/hooks, i.e. google_auth_redirect function could be called from outside of plugin instance too.

    Thread Starter ttodua

    (@ttodua)

    8) would be excellent also to extend all functionalities for hooking, i mean in every important action, just add ability to be hooked by filter. so, i.e.

    in function, find_by_email_or_create:

    
  $new_user = wp_insert_user( $user );

    should be

    
  $new_user = wp_insert_user( apply_filters('siwg_insert_user', $user) );

    and similarly, in most of functions.

    9) just to ask (i hope it is, but…), is this plugin secured to handle dots . or plus + signs (or whatever is possible a person to register onto gmail) and differentiate the login from i.e. [email protected] and [email protected]

    • This reply was modified 4 years, 11 months ago by ttodua.
    Thread Starter ttodua

    (@ttodua)

    actually, disregard the 4th one. It is doable with BanHammer plugin, so no need into your plugin.

    Thread Starter ttodua

    (@ttodua)

    about 9th one, i’ve made further investigation and found that it is not working somehow.

    For example, I register ‘[email protected]’, then in wordpress it tries to assign username joel. there is 1 problem:
    – if it founds that there is an username with already that existing username, the google login kicks back.

    Ok, say everything went well, and ‘joel’ is registered…
    if someone tries to signup/signin with gmail’s *external* address (you know, people also sign up with google with external domains, like ‘[email protected]` )
    then s/he will be unable to login because your plugin sees that joel username already exists.

    I cant say internally what happens, but tested now and confirm that is case. I think, with external domain registered mails, you should append the random suffix to usernames, like :

    joel_xyz824h

    (but note, use underscore or like that, but not hyphen or dot or plus, because it ensures that it wont be @gmail address, i.e. [email protected] ).

    Thread Starter ttodua

    (@ttodua)

    10) option to disallow email change (after g-login) and even unlinking, would be very necessary, otherwise allowing to change mail might defeat the purpose.

    • This reply was modified 4 years, 11 months ago by ttodua.
    • This reply was modified 4 years, 11 months ago by ttodua.
    Thread Starter ttodua

    (@ttodua)

    for this topic, i’ve made pull request:
    https://github.com/NorthStarMarketing/sign-in-with-google/pull/35

    Plugin Author Tanner Record

    (@tarecord)

    Thanks @ttodua, let’s carry this discussion over to Github.

Viewing 7 replies - 1 through 7 (of 7 total)
  • The topic ‘Some errors’ is closed to new replies.