Add skip strategies setting and tests #528
Conversation
joshcanhelp
force-pushed
the
add-skip-strategies-setting
branch
from
93ece4f to
df0f997
Compare
joshcanhelp
force-pushed
the
add-skip-strategies-setting
branch
from
df0f997 to
c20072b
Compare
Codecov Report
@@ Coverage Diff @@
## dev #528 +/- ##
==========================================
+ Coverage 6.07% 8.66% +2.59%
- Complexity 1611 1615 +4
==========================================
Files 63 63
Lines 5600 5622 +22
==========================================
+ Hits 340 487 +147
+ Misses 5260 5135 -125
Continue to review full report at Codecov.
|
| @@ -526,9 +526,7 @@ private function autoloader( $class ) { | |||
| if ( ! function_exists( 'get_auth0userinfo' ) ) { | |||
There was a problem hiding this comment.
Switching to new WP_Auth0_UsersRepo::get_meta helper function
| @@ -431,7 +431,7 @@ protected function migrate_users_data() { | |||
| $repo = new WP_Auth0_UsersRepo( $this->a0_options ); | |||
There was a problem hiding this comment.
Switching to new WP_Auth0_UsersRepo::get_meta helper function
| @@ -69,7 +69,8 @@ public function a0_export_users( $user_ids = null ) { | |||
| global $wpdb; | |||
There was a problem hiding this comment.
Switching to new WP_Auth0_UsersRepo::get_meta helper function
| * @throws WP_Auth0_EmailNotVerifiedException | ||
| * @throws WP_Auth0_RegistrationNotEnabledException | ||
| */ | ||
| public function create( $userinfo, $token, $access_token = null, $role = null, $skip_email_verified = false ) { |
There was a problem hiding this comment.
This is the meat of this PR. This method was confusing to read and debug so just patching in the skip strategy logic would have probably taken as much time as a line-by-line re-write.
| @@ -45,6 +45,12 @@ public function init() { | |||
| 'id' => 'wpa0_verified_email', | |||
| 'function' => 'render_verified_email', | |||
| ), | |||
| array( | |||
There was a problem hiding this comment.
Declare the new settings field.
joshcanhelp
force-pushed
the
add-skip-strategies-setting
branch
from
c20072b to
8b29e07
Compare
|
@joshcanhelp by Default it will skip Twitter? and Enterprise Strategies, where does that list come from? |
| $msg = __( 'Could not create user. The registration process were rejected. Please verify that your account is whitelisted for this system. Please contact your site’s administrator.', 'wp-auth0' ); | ||
|
|
||
| throw new WP_Auth0_CouldNotCreateUserException( $msg ); | ||
| } elseif ( -2 === $user_id ) { |
There was a problem hiding this comment.
Where does -2 come from? Could this be a constant that better defines what this means?
There was a problem hiding this comment.
It's what WP_Auth0_Users::create_user() returns for that condition. I didn't want to go much further with this PR but I totally agree that this magic value is not ideal 👍
joshcanhelp
force-pushed
the
add-skip-strategies-setting
branch
from
8b29e07 to
a3622b3
Compare
No, no default skipping, field will be blank by default. |
| @@ -780,7 +815,7 @@ public function link_accounts_validation( $old_options, $input ) { | |||
| * @return array | |||
| */ | |||
| public function loginredirection_validation( $old_options, $input ) { | |||
| $new_redirect_url = strtolower( $input['default_login_redirection'] ); | |||
| $new_redirect_url = esc_url_raw( strtolower( $input['default_login_redirection'] ) ); | |||
There was a problem hiding this comment.
why does this one get an esc_url_raw but the $old_redirect_url doesn't?
There was a problem hiding this comment.
We don't need to sanitize the old one to check it as it's already saved. If the new email is different after sanitization, then we update with that one (assuming it passes all the other checks).
|
Should probably have an example of connection name entries in the |
Summary: Add a setting to the admin that skips email verification for specific strategies. This is being added based on Enterprise strategies that do not provide an
email_verifiedflag in the userinfo object.Admin Setting:
Specific Changes:
WP_Auth0_UsersRepo::get_meta()method to abstract getting Auth0 user meta information; refactored all current uses of the coreget_user_meta()function to use this new method.WP_Auth0_LoginManager::login_user()to use the new skip strategies field.WP_Auth0_Options::strategy_skips_verified_email()method to check whether a specific strategy can be skipped.WP_Auth0_UsersRepo::create()to use the new skip strategies field and simplify logic.WP_Auth0_UsersRepo::create()