-
Notifications
You must be signed in to change notification settings - Fork 96
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
Refactor migration route handling and add tests #606
Conversation
a426ff4
to
0abb338
Compare
Codecov Report
@@ Coverage Diff @@
## master #606 +/- ##
============================================
+ Coverage 34.02% 34.35% +0.33%
+ Complexity 1317 1314 -3
============================================
Files 54 54
Lines 4174 4163 -11
============================================
+ Hits 1420 1430 +10
+ Misses 2754 2733 -21
Continue to review full report at Codecov.
|
@@ -38,7 +38,7 @@ public function init() { | |||
add_action( 'parse_request', array( $this, 'custom_requests' ) ); | |||
} | |||
|
|||
public function setup_rewrites( $force_ws = false ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allowed in PHP. Parameter was not used and anyone calling this with a parameter will not get an error.
if ( ! $this->ip_check->connection_is_valid( $allowed_ips ) ) { | ||
return $this->error_return_array( 401 ); | ||
} | ||
$code = $this->check_endpoint_access_error(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call handles all the checks above.
if ( ! $this->ip_check->connection_is_valid( $allowed_ips ) ) { | ||
return $this->error_return_array( 401 ); | ||
} | ||
$code = $this->check_endpoint_access_error(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call handles all the checks above.
if ( ! isset( $_POST['username'] ) ) { | ||
throw new Exception( __( 'Username is required', 'wp-auth0' ) ); | ||
} | ||
$this->check_endpoint_request(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call handles all the checks above.
if ( empty( $_POST['password'] ) ) { | ||
throw new Exception( __( 'Password is required', 'wp-auth0' ) ); | ||
} | ||
$this->check_endpoint_request( true ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call handles all the checks above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just check the comment.
*/ | ||
private function check_endpoint_request( $require_password = false ) { | ||
$authorization = $this->getAuthorizationHeader(); | ||
$authorization = trim( str_replace( 'Bearer ', '', $authorization ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like the sort of thing that might be nice in a tested helper method.
Changes
Simplify how migration endpoints are served and add tests.
Testing
Checklist