Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Advanced settings validation and defaults #773

Merged
merged 1 commit into from
Jan 31, 2020

Conversation

joshcanhelp
Copy link
Contributor

@joshcanhelp joshcanhelp commented Jan 31, 2020

Description

  • Normalize settings validation for the Advanced tab
  • Normalize defaults for the Advanced tab
  • Remove WP_Auth0_Admin_Advanced->render_auto_login()

References

WP guidance on sanitization and escaping

Testing

  • This change improves test coverage for new/changed/fixed functionality

@joshcanhelp joshcanhelp requested a review from a team January 31, 2020 18:12
@@ -258,4 +258,12 @@ protected function get_docs_link( $path, $text = '' ) {
$text = empty( $text ) ? __( 'here', 'wp-auth0' ) : sanitize_text_field( $text );
return sprintf( '<a href="https://auth0.com/docs/%s" target="_blank">%s</a>', $path, $text );
}

protected function sanitize_switch_val( $val ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will handle values from the UI (which save a positive as "1") as well as JSON-imported. Need to handle a few (but not all) truth-y values.

}

protected function sanitize_text_val( $val ) {
return sanitize_text_field( trim( strval( $val ) ) );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -131,7 +131,7 @@ public function consent_callback( $name ) {
if ( $should_create_connection ) {
$migration_token = $this->a0_options->get( 'migration_token' );
if ( empty( $migration_token ) ) {
$migration_token = wp_auth0_url_base64_encode( openssl_random_pseudo_bytes( 64 ) );
$migration_token = wp_auth0_generate_token();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -73,22 +73,23 @@ public function testRequiredEmailFieldOutput() {
* Test that the required email field is properly validated.
*/
public function testRequiredEmailValidation() {
$opt_name = 'requires_verified_email';
$validated_opts = self::$admin->basic_validation( [], [ 'requires_verified_email' => '1' ] );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing out multiple truth-y and false-y values, difference is intensional.

@joshcanhelp joshcanhelp force-pushed the update-settings-validation branch from 0629688 to 938cf66 Compare January 31, 2020 18:23
@@ -49,7 +49,7 @@ public function testRequiredEmailRejected() {
$this->createUser( [ 'user_email' => $userinfo->email ] );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set the values to be the same as what the UI would save

@joshcanhelp joshcanhelp force-pushed the update-settings-validation branch from 938cf66 to 2d81654 Compare January 31, 2020 18:25
@@ -64,8 +64,8 @@ public function testThatLoginRouteIsForbiddenByDefault() {
* If the incoming IP address is invalid, the route should fail with an error.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set the values to be the same as what the UI would save

@@ -63,8 +63,8 @@ public function testThatGetUserRouteIsForbiddenByDefault() {
* If the incoming IP address is invalid, the route should fail with an error.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set the values to be the same as what the UI would save

@@ -126,12 +126,12 @@ public function testThatCorrectFieldDocsShowWhenMigrationIsOn() {
public function testThatChangingMigrationToOffKeepsTokenData() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting these test to use the same value that the validator would expect from the UI.

@@ -290,10 +290,11 @@ public function render_extra_conf( $args = [] ) {
* @return array
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything in this file is moved from somewhere else. No functional changes, validation will be addressed in a separate PR.

Copy link
Contributor

@lbalmaceda lbalmaceda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left two, not blockers.

lib/admin/WP_Auth0_Admin_Advanced.php Show resolved Hide resolved
lib/admin/WP_Auth0_Admin_Appearance.php Show resolved Hide resolved
@lbalmaceda
Copy link
Contributor

Green

@joshcanhelp joshcanhelp added this to the 4.0.0 milestone Jan 31, 2020
@joshcanhelp joshcanhelp merged commit 658b1d8 into master Jan 31, 2020
@joshcanhelp joshcanhelp deleted the update-settings-validation branch January 31, 2020 19:12
@joshcanhelp joshcanhelp changed the title Adding advanced setting validation; normalize values throughout Update Advanced settings validation and defaults Jan 31, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants