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.

Sign up for GitHub

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

Jump to bottom

Refactor migration route handling and add tests #606

Merged
merged 2 commits into from
Dec 19, 2018
Merged

Refactor migration route handling and add tests #606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
866352a
Refactor mogration route handling and add tests.
joshcanhelp Dec 18, 2018
0abb338
Add exit early condition on route processing
joshcanhelp Dec 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"phpcbf-tests": "./vendor/bin/phpcbf --standard=phpcs-test-ruleset.xml -s ./tests/",
"sniffs": "vendor/bin/phpcs --standard=phpcs-ruleset.xml -e",
"test": "\"vendor/bin/phpunit\" --coverage-text",
"test-group": "\"vendor/bin/phpunit\" --coverage-text --group",
"test-ci": "vendor/bin/phpunit --debug --coverage-clover=coverage.xml",
"pre-commit": [ "@compat", "@phpcbf", "@phpcbf-tests", "@phpcs-tests", "@test" ]
}
Expand Down
222 changes: 104 additions & 118 deletions lib/WP_Auth0_Routes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function init() {
add_action( 'parse_request', array( $this, 'custom_requests' ) );
}

public function setup_rewrites( $force_ws = false ) {
Copy link
Contributor Author

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.

public function setup_rewrites() {
add_rewrite_tag( '%auth0%', '([^&]+)' );
add_rewrite_tag( '%auth0fallback%', '([^&]+)' );
add_rewrite_tag( '%code%', '([^&]+)' );
Expand All @@ -53,9 +53,9 @@ public function setup_rewrites( $force_ws = false ) {
/**
* Route incoming Auth0 actions.
*
* @param WP_Query $wp - WP_Query object for current request.
* @param WP $wp - WP object for current request.
*
* @return bool|false|string
* @return bool|string
*/
public function custom_requests( $wp ) {
$page = null;
Expand All @@ -72,59 +72,47 @@ public function custom_requests( $wp ) {
$page = $wp->query_vars['pagename'];
}

$output = null;
if ( ! empty( $page ) ) {
switch ( $page ) {
case 'oauth2-config':
$this->oauth2_config();
exit;
case 'migration-ws-login':
$output = $this->migration_ws_login();
break;
case 'migration-ws-get-user':
$output = $this->migration_ws_get_user();
break;
case 'coo-fallback':
$this->coo_fallback();
exit;
default:
return false;
}
if ( empty( $page ) ) {
return false;
}

if ( $output ) {
if ( empty( $wp->query_vars['custom_requests_return'] ) ) {
echo wp_json_encode( $output );
exit;
} else {
return wp_json_encode( $output );
}
}
switch ( $page ) {
case 'oauth2-config':
$output = wp_json_encode( $this->oauth2_config() );
break;
case 'migration-ws-login':
$output = wp_json_encode( $this->migration_ws_login() );
break;
case 'migration-ws-get-user':
$output = wp_json_encode( $this->migration_ws_get_user() );
break;
case 'coo-fallback':
$output = $this->coo_fallback();
break;
default:
return false;
}

if ( $wp->query_vars['custom_requests_return'] ) {
return $output;
}

echo $output;
exit;
}

protected function coo_fallback() {
$cdn = $this->a0_options->get( 'auth0js-cdn' );
$client_id = $this->a0_options->get( 'client_id' );
$domain = $this->a0_options->get_auth_domain();
$protocol = $this->a0_options->get( 'force_https_callback', false ) ? 'https' : null;
$redirect_uri = $this->a0_options->get_wp_auth0_url( $protocol );
echo <<<EOT
<!DOCTYPE html>
<html>
<head>
<script src="$cdn"></script>
<script type="text/javascript">
var auth0 = new auth0.WebAuth({
clientID: '$client_id',
domain: '$domain',
redirectUri: '$redirect_uri'
});
auth0.crossOriginAuthenticationCallback();
</script>
</head>
<body></body>
</html>
EOT;
$protocol = $this->a0_options->get( 'force_https_callback', false ) ? 'https' : null;
return sprintf(
'<!DOCTYPE html><html><head><script src="%s"></script><script type="text/javascript">
var auth0 = new auth0.WebAuth({clientID:"%s",domain:"%s",redirectUri:"%s"});
auth0.crossOriginAuthenticationCallback();
</script></head><body></body></html>',
esc_url( $this->a0_options->get( 'auth0js-cdn' ) ),
sanitize_text_field( $this->a0_options->get( 'client_id' ) ),
sanitize_text_field( $this->a0_options->get_auth_domain() ),
esc_url( $this->a0_options->get_wp_auth0_url( $protocol ) )
);
}

protected function getAuthorizationHeader() {
Expand Down Expand Up @@ -157,38 +145,13 @@ protected function getAuthorizationHeader() {
*/
protected function migration_ws_login() {

// Migration web service is not turned on.
if ( ! $this->a0_options->get( 'migration_ws' ) ) {
return $this->error_return_array( 403 );
}

// IP filtering is on and incoming IP address does not match filter.
if ( $this->a0_options->get( 'migration_ips_filter' ) ) {
$allowed_ips = $this->a0_options->get( 'migration_ips' );
if ( ! $this->ip_check->connection_is_valid( $allowed_ips ) ) {
return $this->error_return_array( 401 );
}
$code = $this->check_endpoint_access_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.

This call handles all the checks above.

if ( $code ) {
return $this->error_return_array( $code );
}

$authorization = $this->getAuthorizationHeader();
$authorization = trim( str_replace( 'Bearer ', '', $authorization ) );

try {
if ( empty( $authorization ) ) {
throw new Exception( __( 'Unauthorized: missing authorization header', 'wp-auth0' ), 401 );
}

if ( ! $this->valid_token( $authorization ) ) {
throw new Exception( __( 'Invalid token', 'wp-auth0' ), 401 );
}

if ( empty( $_POST['username'] ) ) {
throw new Exception( __( 'Username is required', 'wp-auth0' ) );
}

if ( empty( $_POST['password'] ) ) {
throw new Exception( __( 'Password is required', 'wp-auth0' ) );
}
$this->check_endpoint_request( true );
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 call handles all the checks above.


$user = wp_authenticate( $_POST['username'], $_POST['password'] );

Expand Down Expand Up @@ -217,35 +180,13 @@ protected function migration_ws_login() {
*/
protected function migration_ws_get_user() {

// Migration web service is not turned on.
if ( ! $this->a0_options->get( 'migration_ws' ) ) {
return $this->error_return_array( 403 );
}

// IP filtering is on and incoming IP address does not match filter.
if ( $this->a0_options->get( 'migration_ips_filter' ) ) {
$allowed_ips = $this->a0_options->get( 'migration_ips' );
if ( ! $this->ip_check->connection_is_valid( $allowed_ips ) ) {
return $this->error_return_array( 401 );
}
$code = $this->check_endpoint_access_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.

This call handles all the checks above.

if ( $code ) {
return $this->error_return_array( $code );
}

$authorization = $this->getAuthorizationHeader();
$authorization = trim( str_replace( 'Bearer ', '', $authorization ) );
$user = null;

try {
if ( empty( $authorization ) ) {
throw new Exception( __( 'Unauthorized: missing authorization header', 'wp-auth0' ), 401 );
}

if ( ! $this->valid_token( $authorization ) ) {
throw new Exception( __( 'Invalid token', 'wp-auth0' ), 401 );
}

if ( ! isset( $_POST['username'] ) ) {
throw new Exception( __( 'Username is required', 'wp-auth0' ) );
}
$this->check_endpoint_request();
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 call handles all the checks above.


$username = $_POST['username'];

Expand All @@ -272,17 +213,62 @@ protected function migration_ws_get_user() {

protected function oauth2_config() {

$callback_url = admin_url( 'admin.php?page=wpa0-setup&callback=1' );

echo json_encode(
array(
'client_name' => get_bloginfo( 'name' ),
'redirect_uris' => array(
$callback_url,
),
)
return array(
'client_name' => get_bloginfo( 'name' ),
'redirect_uris' => array( admin_url( 'admin.php?page=wpa0-setup&callback=1' ) ),
);
exit;

}

/**
* Check the migration endpoint status and IP filter for incoming requests.
*
* @return int
*/
private function check_endpoint_access_error() {

// Migration web service is not turned on.
if ( ! $this->a0_options->get( 'migration_ws' ) ) {
return 403;
}

// IP filtering is on and incoming IP address does not match filter.
if ( $this->a0_options->get( 'migration_ips_filter' ) ) {
$allowed_ips = $this->a0_options->get( 'migration_ips' );
if ( ! $this->ip_check->connection_is_valid( $allowed_ips ) ) {
return 401;
}
}

return 0;
}

/**
* Check the incoming request for token and required data.
*
* @param bool $require_password - True to check for a POSTed password, false to ignore.
*
* @throws Exception
*/
private function check_endpoint_request( $require_password = false ) {
$authorization = $this->getAuthorizationHeader();
$authorization = trim( str_replace( 'Bearer ', '', $authorization ) );
Copy link
Member

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.


if ( empty( $authorization ) ) {
throw new Exception( __( 'Unauthorized: missing authorization header', 'wp-auth0' ), 401 );
}

if ( ! $this->valid_token( $authorization ) ) {
throw new Exception( __( 'Invalid token', 'wp-auth0' ), 401 );
}

if ( empty( $_POST['username'] ) ) {
throw new Exception( __( 'Username is required', 'wp-auth0' ) );
}

if ( $require_password && empty( $_POST['password'] ) ) {
throw new Exception( __( 'Password is required', 'wp-auth0' ) );
}
}

/**
Expand All @@ -301,7 +287,7 @@ private function error_return_array( $code ) {
'error' => __( 'Unauthorized', 'wp-auth0' ),
);

case 403:
default:
return array(
'status' => 403,
'error' => __( 'Forbidden', 'wp-auth0' ),
Expand All @@ -313,7 +299,7 @@ private function error_return_array( $code ) {
/**
* Check if a token or token JTI is the same as what is stored.
*
* @param string $authorization - Incoming migration token;
* @param string $authorization - Incoming migration token.
*
* @return bool
*/
Expand Down
1 change: 1 addition & 0 deletions phpcs.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
<rule ref="WordPress-Core">
<exclude name="WordPress.Files.FileName"/>
<exclude name="WordPress.NamingConventions.ValidVariableName.NotSnakeCaseMemberVar"/>
<exclude name="WordPress.NamingConventions.ValidFunctionName.MethodNameInvalid"/>
</rule>
<rule ref="WordPress.DB.DirectDatabaseQuery"/>
<rule ref="WordPress.DB.SlowDBQuery"/>
Expand Down
Loading