From f60951e2c45a0c84b723c45c6888f2887a6fd588 Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Wed, 7 Nov 2018 10:31:47 -0800 Subject: [PATCH 1/3] Fix Connection update over-writing Connection settings. --- lib/WP_Auth0_Api_Client.php | 19 ++ lib/WP_Auth0_Api_Operations.php | 30 +- lib/admin/WP_Auth0_Admin_Advanced.php | 4 - lib/admin/WP_Auth0_Admin_Features.php | 6 +- .../WP_Auth0_InitialSetup_Consent.php | 37 ++- tests/testApiOperations.php | 173 ++++++++++ tests/testInitialSetupConsent.php | 297 ++++++++++++++++++ tests/traits/httpHelpers.php | 33 +- 8 files changed, 550 insertions(+), 49 deletions(-) create mode 100644 tests/testApiOperations.php create mode 100644 tests/testInitialSetupConsent.php diff --git a/lib/WP_Auth0_Api_Client.php b/lib/WP_Auth0_Api_Client.php index 02bd75ed..1e307554 100755 --- a/lib/WP_Auth0_Api_Client.php +++ b/lib/WP_Auth0_Api_Client.php @@ -611,6 +611,17 @@ public static function get_connection( $domain, $app_token, $id ) { return json_decode( $response['body'] ); } + /** + * Update a Connection via the Management API. + * Note: $payload must be a complete settings object, not just the property to change. + * + * @param string $domain - Auth0 Domain. + * @param string $app_token - Valid Auth0 Management API token. + * @param string $id - DB Connection ID. + * @param stdClass $payload - DB Connection settings, will override existing. + * + * @return bool|object + */ public static function update_connection( $domain, $app_token, $id, $payload ) { $endpoint = "https://$domain/api/v2/connections/$id"; @@ -619,6 +630,14 @@ public static function update_connection( $domain, $app_token, $id, $payload ) { $headers['Authorization'] = "Bearer $app_token"; $headers['content-type'] = 'application/json'; + unset( $payload->name ); + unset( $payload->strategy ); + unset( $payload->id ); + + if ( ! empty( $payload->enabled_clients ) ) { + $payload->enabled_clients = array_values( $payload->enabled_clients ); + } + $response = wp_remote_post( $endpoint, array( diff --git a/lib/WP_Auth0_Api_Operations.php b/lib/WP_Auth0_Api_Operations.php index 8de91718..5fed447d 100644 --- a/lib/WP_Auth0_Api_Operations.php +++ b/lib/WP_Auth0_Api_Operations.php @@ -30,10 +30,6 @@ public function update_wordpress_connection( $app_token, $connection_id, $passwo $connection->options->customScripts->login = $login_script; $connection->options->customScripts->get_user = $get_user_script; - unset( $connection->name ); - unset( $connection->strategy ); - unset( $connection->id ); - WP_Auth0_Api_Client::update_connection( $domain, $app_token, $connection_id, $connection ); } @@ -99,34 +95,10 @@ public function create_wordpress_connection( $app_token, $migration_enabled, $pa $response = WP_Auth0_Api_Client::create_connection( $domain, $app_token, $body ); if ( $response === false ) { - return false; - - } - - $connections = WP_Auth0_Api_Client::search_connection( $domain, $app_token, 'auth0' ); - $db_connection = null; - - $migration_connection_id = $response->id; - - foreach ( $connections as $connection ) { - if ( $migration_connection_id != $connection->id && in_array( $client_id, $connection->enabled_clients ) ) { - $db_connection = $connection; - - $enabled_clients = array_diff( $db_connection->enabled_clients, array( $client_id ) ); - - WP_Auth0_Api_Client::update_connection( - $domain, - $app_token, - $db_connection->id, - array( - 'enabled_clients' => array_values( $enabled_clients ), - ) - ); - } } - return $migration_connection_id; + return $response->id; } // $input['geo_rule'] = ( isset( $input['geo_rule'] ) ? $input['geo_rule'] : 0 ); diff --git a/lib/admin/WP_Auth0_Admin_Advanced.php b/lib/admin/WP_Auth0_Admin_Advanced.php index 050cfc55..2499bab4 100644 --- a/lib/admin/WP_Auth0_Admin_Advanced.php +++ b/lib/admin/WP_Auth0_Admin_Advanced.php @@ -741,10 +741,6 @@ public function migration_ws_validation( $old_options, $input ) { $connection->options->enabledDatabaseCustomization = false; $connection->options->import_mode = false; - unset( $connection->name ); - unset( $connection->strategy ); - unset( $connection->id ); - $response = WP_Auth0_Api_Client::update_connection( $input['domain'], $input['auth0_app_token'], $old_options['db_connection_id'], $connection ); } else { $response = false; diff --git a/lib/admin/WP_Auth0_Admin_Features.php b/lib/admin/WP_Auth0_Admin_Features.php index 91e43293..27ed5ba0 100644 --- a/lib/admin/WP_Auth0_Admin_Features.php +++ b/lib/admin/WP_Auth0_Admin_Features.php @@ -427,8 +427,10 @@ public function security_validation( $old_options, $input ) { foreach ( $connections as $connection ) { if ( in_array( $input['client_id'], $connection->enabled_clients ) ) { - $patch = array( 'options' => array( 'passwordPolicy' => $input['password_policy'] ) ); - $update_resp = WP_Auth0_Api_Client::update_connection( $domain, $app_token, $connection->id, $patch ); + $u_connection = clone $connection; + $u_connection->options->passwordPolicy = $input['password_policy']; + + $update_resp = WP_Auth0_Api_Client::update_connection( $domain, $app_token, $u_connection->id, $u_connection ); if ( false === $update_resp ) { $this->add_validation_error( diff --git a/lib/initial-setup/WP_Auth0_InitialSetup_Consent.php b/lib/initial-setup/WP_Auth0_InitialSetup_Consent.php index 04a184c2..7c371330 100644 --- a/lib/initial-setup/WP_Auth0_InitialSetup_Consent.php +++ b/lib/initial-setup/WP_Auth0_InitialSetup_Consent.php @@ -16,6 +16,16 @@ public function __construct( WP_Auth0_Options $a0_options ) { public function render( $step ) { } + /** + * Used by both Setup Wizard installation flows. + * Called in WP_Auth0_InitialSetup_ConnectionProfile::callback() when an API token is used during install. + * Called in self::callback() when returning from consent URL install. + * + * @param string $domain - Auth0 domain for the Application. + * @param string $access_token - Management API access token. + * @param string $type - Installation type, "social" (AKA standard) or "enterprise". + * @param bool $hasInternetConnection - True if the installing site be reached by Auth0, false if not. + */ public function callback_with_token( $domain, $access_token, $type, $hasInternetConnection = true ) { $this->a0_options->set( 'auth0_app_token', $access_token ); @@ -92,6 +102,12 @@ public function exchange_code() { return $obj->access_token; } + /** + * Used by both Setup Wizard installation flows. + * Called by self::callback_with_token() to create a Client, Connection, and Client Grant. + * + * @param string $name - Client name to use. + */ public function consent_callback( $name ) { $domain = $this->a0_options->get( 'domain' ); @@ -128,19 +144,22 @@ public function consent_callback( $name ) { $connection_exists = false; $connection_pwd_policy = null; - $connections = WP_Auth0_Api_Client::search_connection( $domain, $app_token ); + $connections = WP_Auth0_Api_Client::search_connection( $domain, $app_token, 'auth0' ); - foreach ( $connections as $connection ) { + if ( $should_create_and_update_connection && ! empty( $connections ) && is_array( $connections ) ) { + foreach ( $connections as $connection ) { - if ( in_array( $client_id, $connection->enabled_clients ) ) { - if ( $connection->strategy === 'auth0' && $should_create_and_update_connection ) { + if ( in_array( $client_id, $connection->enabled_clients ) ) { if ( $db_connection_name === $connection->name ) { - $connection_exists = $connection->id; - $connection_pwd_policy = ( isset( $connection->options ) && isset( $connection->options->passwordPolicy ) ) ? $connection->options->passwordPolicy : null; + $connection_exists = $connection->id; + if ( isset( $connection->options ) && isset( $connection->options->passwordPolicy ) ) { + $connection_pwd_policy = $connection->options->passwordPolicy; + } } else { - $enabled_clients = array_diff( $connection->enabled_clients, array( $client_id ) ); - WP_Auth0_Api_Client::update_connection( $domain, $app_token, $connection->id, array( 'enabled_clients' => array_values( $enabled_clients ) ) ); + $u_connection = clone $connection; + $u_connection->enabled_clients = array_diff( $u_connection->enabled_clients, array( $client_id ) ); + WP_Auth0_Api_Client::update_connection( $domain, $app_token, $u_connection->id, $u_connection ); } } } @@ -191,6 +210,6 @@ public function consent_callback( $name ) { } wp_redirect( admin_url( 'admin.php?page=wpa0-setup&step=2&profile=' . $this->state ) ); - exit(); + exit; } } diff --git a/tests/testApiOperations.php b/tests/testApiOperations.php new file mode 100644 index 00000000..e10bd6a2 --- /dev/null +++ b/tests/testApiOperations.php @@ -0,0 +1,173 @@ +startHttpHalting(); + + $api_ops = new WP_Auth0_Api_Operations( self::$opts ); + $test_token = implode( '.', [ uniqid(), uniqid(), uniqid() ] ); + + self::$opts->set( 'domain', 'test-wp.auth0.com' ); + self::$opts->set( 'client_id', 'TEST_CLIENT_ID' ); + + $caught_http = []; + try { + $api_ops->create_wordpress_connection( $test_token, false, 'good' ); + } catch ( Exception $e ) { + $caught_http = unserialize( $e->getMessage() ); + } + + $this->assertEquals( 'https://test-wp.auth0.com/api/v2/connections', $caught_http['url'] ); + $this->assertEquals( 'Bearer ' . $test_token, $caught_http['headers']['Authorization'] ); + $this->assertEquals( 'DB-Test-Blog', $caught_http['body']['name'] ); + $this->assertEquals( 'auth0', $caught_http['body']['strategy'] ); + $this->assertEquals( 'good', $caught_http['body']['options']['passwordPolicy'] ); + $this->assertContains( 'TEST_CLIENT_ID', $caught_http['body']['enabled_clients'] ); + } + + /** + * Test that a migration create connection command requests properly. + */ + public function testThatCreateConnectionWithMigrationRequestsCorrectly() { + $this->startHttpHalting(); + + $api_ops = new WP_Auth0_Api_Operations( self::$opts ); + $test_token = implode( '.', [ uniqid(), uniqid(), uniqid() ] ); + + self::$opts->set( 'domain', 'test-wp2.auth0.com' ); + self::$opts->set( 'client_id', 'TEST_CLIENT_ID_2' ); + + $caught_http = []; + try { + $api_ops->create_wordpress_connection( $test_token, true, 'fair', 'TEST_MIGRATION_TOKEN' ); + } catch ( Exception $e ) { + $caught_http = unserialize( $e->getMessage() ); + } + + $this->assertEquals( 'https://test-wp2.auth0.com/api/v2/connections', $caught_http['url'] ); + $this->assertEquals( 'Bearer ' . $test_token, $caught_http['headers']['Authorization'] ); + $this->assertEquals( 'DB-Test-Blog', $caught_http['body']['name'] ); + $this->assertEquals( 'auth0', $caught_http['body']['strategy'] ); + $this->assertContains( 'TEST_CLIENT_ID_2', $caught_http['body']['enabled_clients'] ); + + $this->assertEquals( true, $caught_http['body']['options']['requires_username'] ); + $this->assertEquals( true, $caught_http['body']['options']['import_mode'] ); + $this->assertEquals( true, $caught_http['body']['options']['enabledDatabaseCustomization'] ); + $this->assertEquals( + [ + 'min' => 1, + 'max' => 100, + ], + $caught_http['body']['options']['validation']['username'] + ); + + $this->assertContains( + 'request.post("http://example.org/index.php?a0_action=migration-ws-login"', + $caught_http['body']['options']['customScripts']['login'] + ); + + $this->assertContains( + 'form:{username:email, password:password, access_token:"TEST_MIGRATION_TOKEN"}', + $caught_http['body']['options']['customScripts']['login'] + ); + + $this->assertContains( + 'request.post("http://example.org/index.php?a0_action=migration-ws-get-user"', + $caught_http['body']['options']['customScripts']['get_user'] + ); + + $this->assertContains( + 'form:{username:email, access_token:"TEST_MIGRATION_TOKEN', + $caught_http['body']['options']['customScripts']['get_user'] + ); + } + + /** + * Test that successful and unsuccessful requests return properly. + */ + public function testThatCreateConnectionReturnsCorrectly() { + $this->startHttpMocking(); + + $api_ops = new WP_Auth0_Api_Operations( self::$opts ); + $test_token = implode( '.', [ uniqid(), uniqid(), uniqid() ] ); + + self::$opts->set( 'domain', 'test-wp.auth0.com' ); + self::$opts->set( 'client_id', 'TEST_CLIENT_ID' ); + + $this->http_request_type = 'success_create_connection'; + + $result = $api_ops->create_wordpress_connection( $test_token, false ); + + $this->assertEquals( 'TEST_CREATED_CONN_ID', $result ); + $this->assertEquals( 'DB-Test-Blog', self::$opts->get( 'db_connection_name' ) ); + + $this->http_request_type = 'wp_error'; + + $result = $api_ops->create_wordpress_connection( $test_token, false ); + + $this->assertFalse( $result ); + } + + /** + * Runs after each test method. + */ + public function tearDown() { + parent::tearDown(); + + self::$opts->set( 'domain', self::$opts->get_default( 'domain' ) ); + self::$opts->set( 'client_id', self::$opts->get_default( 'client_id' ) ); + self::$opts->set( 'migration_ips', self::$opts->get_default( 'migration_ips' ) ); + self::$opts->set( 'migration_ips_filter', self::$opts->get_default( 'migration_ips_filter' ) ); + self::$opts->set( 'db_connection_name', null ); + + $this->stopHttpHalting(); + $this->stopHttpMocking(); + + self::$error_log->clear(); + } +} diff --git a/tests/testInitialSetupConsent.php b/tests/testInitialSetupConsent.php new file mode 100644 index 00000000..37f6edde --- /dev/null +++ b/tests/testInitialSetupConsent.php @@ -0,0 +1,297 @@ +startRedirectHalting(); + + $setup_consent = new WP_Auth0_InitialSetup_Consent( self::$opts ); + $test_domain = 'test-wp.auth0.com'; + $test_token = implode( '.', [ uniqid(), uniqid(), uniqid() ] ); + + $caught_redirect = []; + try { + $setup_consent->callback_with_token( $test_domain, $test_token, 'invalid_state' ); + } catch ( Exception $e ) { + $caught_redirect = unserialize( $e->getMessage() ); + } + + $this->assertNotEmpty( $caught_redirect ); + $this->assertEquals( 302, $caught_redirect['status'] ); + + $redirect_url = parse_url( $caught_redirect['location'] ); + + $this->assertEquals( '/wp-admin/admin.php', $redirect_url['path'] ); + $this->assertContains( 'page=wpa0-setup', $redirect_url['query'] ); + $this->assertContains( 'error=invalid_state', $redirect_url['query'] ); + + $this->assertEquals( $test_domain, self::$opts->get( 'domain' ) ); + $this->assertEquals( $test_token, self::$opts->get( 'auth0_app_token' ) ); + + $this->assertEmpty( self::$error_log->get() ); + } + + /** + * Test that the create client call is made. + */ + public function testThatClientCreationIsAttempted() { + $this->startHttpHalting(); + + $setup_consent = new WP_Auth0_InitialSetup_Consent( self::$opts ); + $test_token = implode( '.', [ uniqid(), uniqid(), uniqid() ] ); + + $caught_http = []; + try { + $setup_consent->callback_with_token( 'test-wp.auth0.com', $test_token, 'social' ); + } catch ( Exception $e ) { + $caught_http = unserialize( $e->getMessage() ); + } + + // Just want to test if the Create Client call happened. + // Unit testing for what exactly was sent should be written against WP_Auth0_Api_Client::create_client(). + $this->assertNotEmpty( $caught_http ); + $this->assertEquals( 'https://test-wp.auth0.com/api/v2/clients', $caught_http['url'] ); + $this->assertEquals( 'Bearer ' . $test_token, $caught_http['headers']['Authorization'] ); + + $this->assertEmpty( self::$error_log->get() ); + } + + /** + * Test that the redirect when the create client call fails. + */ + public function testThatClientCreationFailureIsRedirected() { + $this->startRedirectHalting(); + $this->startHttpMocking(); + + $setup_consent = new WP_Auth0_InitialSetup_Consent( self::$opts ); + $test_token = implode( '.', [ uniqid(), uniqid(), uniqid() ] ); + + // Mock the create client call with a WP error. + $this->http_request_type = 'wp_error'; + $caught_redirect = []; + try { + $setup_consent->callback_with_token( 'test-wp.auth0.com', $test_token, 'social' ); + } catch ( Exception $e ) { + $caught_redirect = unserialize( $e->getMessage() ); + } + + $this->assertNotEmpty( $caught_redirect ); + $this->assertEquals( 302, $caught_redirect['status'] ); + + $redirect_url = parse_url( $caught_redirect['location'] ); + + $this->assertEquals( '/wp-admin/admin.php', $redirect_url['path'] ); + $this->assertContains( 'page=wpa0', $redirect_url['query'] ); + $this->assertContains( 'error=cant_create_client', $redirect_url['query'] ); + + $this->assertCount( 1, self::$error_log->get() ); + } + + /** + * Test that an existing DB connection is used instead of creating one. + */ + public function testThatExistingConnectionSkipsCreation() { + $this->startHttpMocking(); + $this->startRedirectHalting(); + + self::$opts->set( 'client_signing_algorithm', 'HS256' ); + + $setup_consent = new WP_Auth0_InitialSetup_Consent( self::$opts ); + $test_token = implode( '.', [ uniqid(), uniqid(), uniqid() ] ); + + // Mock consecutive HTTP calls. + $this->http_request_type = [ + // Successful client creation. + 'success_create_client', + // Found a connection with the same name. + 'success_get_existing_conn', + // Client grant created successfully. + 'success_create_empty_body', + ]; + + $caught_redirect = []; + try { + $setup_consent->callback_with_token( 'test-wp.auth0.com', $test_token, 'social' ); + } catch ( Exception $e ) { + $caught_redirect = unserialize( $e->getMessage() ); + } + + $this->assertNotEmpty( $caught_redirect ); + $this->assertEquals( 302, $caught_redirect['status'] ); + + $redirect_url = parse_url( $caught_redirect['location'] ); + + $this->assertEquals( '/wp-admin/admin.php', $redirect_url['path'] ); + $this->assertContains( 'page=wpa0-setup', $redirect_url['query'] ); + $this->assertContains( 'step=2', $redirect_url['query'] ); + $this->assertContains( 'profile=social', $redirect_url['query'] ); + + $this->assertEquals( 'TEST_CLIENT_ID', self::$opts->get( 'client_id' ) ); + $this->assertEquals( 'TEST_CLIENT_SECRET', self::$opts->get( 'client_secret' ) ); + $this->assertEquals( 1, self::$opts->get( 'db_connection_enabled' ) ); + $this->assertEquals( 'TEST_CONN_ID', self::$opts->get( 'db_connection_id' ) ); + $this->assertEquals( 'good', self::$opts->get( 'password_policy' ) ); + + $this->assertEmpty( self::$error_log->get() ); + } + + /** + * Test that an connection is created and the Client Grant fails. + */ + public function testThatNewConnectionIsCreatedAndFailedClientGrantRedirects() { + $this->startHttpMocking(); + $this->startRedirectHalting(); + + $setup_consent = new WP_Auth0_InitialSetup_Consent( self::$opts ); + $test_token = implode( '.', [ uniqid(), uniqid(), uniqid() ] ); + + self::$opts->set( 'client_signing_algorithm', 'HS256' ); + + // Mock consecutive HTTP calls. + $this->http_request_type = [ + // Successful client creation. + 'success_create_client', + // Get en existing connection enabled for this client. + 'success_get_connections', + // Connection updated successfully. + 'success_update_connection', + // Connection created successfully. + 'success_create_connection', + // Client grant failed. + 'wp_error', + ]; + + $caught_redirect = []; + try { + $setup_consent->callback_with_token( 'test-wp.auth0.com', $test_token, 'social' ); + } catch ( Exception $e ) { + $caught_redirect = unserialize( $e->getMessage() ); + } + + $this->assertNotEmpty( $caught_redirect ); + $this->assertEquals( 302, $caught_redirect['status'] ); + + $redirect_url = parse_url( $caught_redirect['location'] ); + + $this->assertEquals( '/wp-admin/admin.php', $redirect_url['path'] ); + $this->assertContains( 'page=wpa0', $redirect_url['query'] ); + $this->assertContains( 'error=cant_create_client_grant', $redirect_url['query'] ); + + $this->assertEquals( 'TEST_CLIENT_ID', self::$opts->get( 'client_id' ) ); + $this->assertEquals( 'TEST_CLIENT_SECRET', self::$opts->get( 'client_secret' ) ); + $this->assertEquals( 1, self::$opts->get( 'db_connection_enabled' ) ); + $this->assertEquals( 'TEST_CREATED_CONN_ID', self::$opts->get( 'db_connection_id' ) ); + $this->assertEquals( 'DB-' . get_auth0_curatedBlogName(), self::$opts->get( 'db_connection_name' ) ); + + $this->assertCount( 1, self::$error_log->get() ); + } + + /* + * PhpUnit suite methods. + */ + + /** + * Runs after each test method. + */ + public function tearDown() { + parent::tearDown(); + + self::$opts->set( 'auth0_app_token', self::$opts->get_default( 'auth0_app_token' ) ); + self::$opts->set( 'domain', self::$opts->get_default( 'domain' ) ); + self::$opts->set( 'client_id', self::$opts->get_default( 'client_id' ) ); + self::$opts->set( 'client_secret', self::$opts->get_default( 'client_secret' ) ); + self::$opts->set( 'password_policy', self::$opts->get_default( 'password_policy' ) ); + self::$opts->set( 'account_profile', null ); + self::$opts->set( 'db_connection_enabled', null ); + self::$opts->set( 'db_connection_id', null ); + self::$opts->set( 'db_connection_name', null ); + + $this->stopHttpHalting(); + $this->stopHttpMocking(); + + $this->stopRedirectHalting(); + + self::$error_log->clear(); + } + + /* + * Test helper functions. + */ + + /** + * Specific mock API responses for this suite. + * + * @return array|null|WP_Error + */ + public function httpMock() { + $response_type = $this->getResponseType(); + switch ( $response_type ) { + case 'success_create_client': + return [ + 'body' => '{"client_id":"TEST_CLIENT_ID","client_secret":"TEST_CLIENT_SECRET"}', + 'response' => [ 'code' => 201 ], + ]; + + case 'success_get_existing_conn': + return [ + 'body' => '[{"id":"TEST_CONN_ID","name":"DB-' . get_auth0_curatedBlogName() . + '","enabled_clients":["TEST_CLIENT_ID"],"options":{"passwordPolicy":"good"}}]', + 'response' => [ 'code' => 200 ], + ]; + + case 'success_get_connections': + return [ + 'body' => '[{"id":"TEST_CONN_ID","name":"TEST_CONNECTION","enabled_clients":["TEST_CLIENT_ID"]}]', + 'response' => [ 'code' => 200 ], + ]; + } + + return $this->httpMockDefault( $response_type ); + } +} diff --git a/tests/traits/httpHelpers.php b/tests/traits/httpHelpers.php index 63aed373..c47ad016 100644 --- a/tests/traits/httpHelpers.php +++ b/tests/traits/httpHelpers.php @@ -71,16 +71,21 @@ public function startHttpMocking() { * @return string|null */ public function getResponseType() { + if ( is_array( $this->http_request_type ) ) { + return array_shift( $this->http_request_type ); + } return $this->http_request_type; } /** - * Return a mocked API call based on type. + * Mock returns from the HTTP client. + * + * @param null|string $response_type - Response type to use. * - * @return array|null|WP_Error + * @return array|WP_Error */ - public function httpMock() { - switch ( $this->getResponseType() ) { + public function httpMock( $response_type = null ) { + switch ( $response_type ?: $this->getResponseType() ) { case 'wp_error': return new WP_Error( 1, 'Caught WP_Error.' ); @@ -109,8 +114,26 @@ public function httpMock() { 'response' => [ 'code' => 200 ], ]; + case 'success_create_empty_body': + return [ + 'body' => '', + 'response' => [ 'code' => 201 ], + ]; + + case 'success_create_connection': + return [ + 'body' => '{"id":"TEST_CREATED_CONN_ID"}', + 'response' => [ 'code' => 201 ], + ]; + + case 'success_update_connection': + return [ + 'body' => '{"id":"TEST_UPDATED_CONN_ID"}', + 'response' => [ 'code' => 200 ], + ]; + default: - return null; + return new WP_Error( 2, 'No mock type found.' ); } } From 510335abde4132e5635d6d1bba8a45fd51cf4244 Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Thu, 8 Nov 2018 12:06:58 -0800 Subject: [PATCH 2/3] Tests for password policy validation; update to test suite for halting --- lib/admin/WP_Auth0_Admin_Features.php | 3 + tests/testAdminFeatures.php | 215 ++++++++++++++++++++++++++ tests/testInitialSetupConsent.php | 6 - tests/traits/httpHelpers.php | 23 ++- 4 files changed, 238 insertions(+), 9 deletions(-) create mode 100644 tests/testAdminFeatures.php diff --git a/lib/admin/WP_Auth0_Admin_Features.php b/lib/admin/WP_Auth0_Admin_Features.php index 27ed5ba0..e8fd0e82 100644 --- a/lib/admin/WP_Auth0_Admin_Features.php +++ b/lib/admin/WP_Auth0_Admin_Features.php @@ -423,6 +423,8 @@ public function security_validation( $old_options, $input ) { __( 'No database connections found for this application. ', 'wp-auth0' ) . $this->get_dashboard_link( 'connections/database', __( 'See all database connections', 'wp-auth0' ) ) ); + $input['password_policy'] = $old_options['password_policy']; + return $input; } foreach ( $connections as $connection ) { @@ -438,6 +440,7 @@ public function security_validation( $old_options, $input ) { __( 'Please manually review and update the policy. ', 'wp-auth0' ) . $this->get_dashboard_link( 'connections/database', __( 'See all database connections', 'wp-auth0' ) ) ); + $input['password_policy'] = $old_options['password_policy']; } } } diff --git a/tests/testAdminFeatures.php b/tests/testAdminFeatures.php new file mode 100644 index 00000000..6fcd7482 --- /dev/null +++ b/tests/testAdminFeatures.php @@ -0,0 +1,215 @@ + 'fair' ]; + $new_input = [ 'password_policy' => 'fair' ]; + $result = $admin_features->security_validation( $old_input, $new_input ); + + $this->assertEquals( $old_input, $result ); + + global $wp_settings_errors; + $this->assertEmpty( $wp_settings_errors ); + } + + /** + * Test the connection search request structure. + */ + public function testThatConnectionsAreSearchedDuringValidation() { + $this->startHttpHalting(); + + $test_domain = 'test-wp.auth0.com'; + $test_token = implode( '.', [ uniqid(), uniqid(), uniqid() ] ); + $admin_features = new WP_Auth0_Admin_Features( self::$opts ); + + $old_input = [ 'password_policy' => 'fair' ]; + $new_input = [ + 'password_policy' => 'good', + 'domain' => $test_domain, + 'auth0_app_token' => $test_token, + ]; + + $caught_request = []; + try { + $admin_features->security_validation( $old_input, $new_input ); + } catch ( Exception $e ) { + $caught_request = unserialize( $e->getMessage() ); + } + + $this->assertEquals( 'https://test-wp.auth0.com/api/v2/connections?strategy=auth0', $caught_request['url'] ); + $this->assertEquals( 'Bearer ' . $test_token, $caught_request['headers']['Authorization'] ); + $this->assertEmpty( $caught_request['body'] ); + } + + /** + * Test that we get a UI error if there are no connections to set. + */ + public function testThatAnErrorIsSetIfThereAreNoConnections() { + $this->startHttpMocking(); + + $admin_features = new WP_Auth0_Admin_Features( self::$opts ); + + $old_input = [ 'password_policy' => 'fair' ]; + $new_input = [ + 'password_policy' => 'good', + 'domain' => 'test-wp.auth0.com', + 'auth0_app_token' => implode( '.', [ uniqid(), uniqid(), uniqid() ] ), + ]; + + $this->http_request_type = 'success_empty_body'; + + $result = $admin_features->security_validation( $old_input, $new_input ); + + global $wp_settings_errors; + + $this->assertContains( 'No database connections found', $wp_settings_errors[0]['message'] ); + $this->assertContains( 'https://manage.auth0.com/#/connections/database', $wp_settings_errors[0]['message'] ); + $this->assertEquals( 'wp_auth0_settings', $wp_settings_errors[0]['code'] ); + $this->assertEquals( 'wp_auth0_settings', $wp_settings_errors[0]['setting'] ); + $this->assertEquals( $old_input['password_policy'], $result['password_policy'] ); + } + + /** + * Test that the update connection request is set properly. + */ + public function testThatAnUpdateRequestIsSent() { + $this->startHttpMocking(); + + $admin_features = new WP_Auth0_Admin_Features( self::$opts ); + + $old_input = [ 'password_policy' => 'fair' ]; + $new_input = [ + 'password_policy' => 'good', + 'domain' => 'test-wp.auth0.com', + 'auth0_app_token' => implode( '.', [ uniqid(), uniqid(), uniqid() ] ), + 'client_id' => 'TEST_CLIENT_ID', + ]; + + $this->http_request_type = [ 'success_get_connections', 'halt' ]; + + $caught_request = [ 'Nothing caught' ]; + try { + $admin_features->security_validation( $old_input, $new_input ); + } catch ( Exception $e ) { + $caught_request = unserialize( $e->getMessage() ); + } + + $this->assertEquals( 'https://test-wp.auth0.com/api/v2/connections/TEST_CONN_ID', $caught_request['url'] ); + $this->assertEquals( 'Bearer ' . $new_input['auth0_app_token'], $caught_request['headers']['Authorization'] ); + $this->assertContains( $new_input['client_id'], $caught_request['body']['enabled_clients'] ); + $this->assertEquals( $new_input['password_policy'], $caught_request['body']['options']['passwordPolicy'] ); + } + + /** + * Test that we get a UI error if the connection cannot be updated properly. + */ + public function testThatAnErrorIsSetIfConnectionUpdateFails() { + $this->startHttpMocking(); + + $admin_features = new WP_Auth0_Admin_Features( self::$opts ); + + $old_input = [ 'password_policy' => 'fair' ]; + $new_input = [ + 'password_policy' => 'good', + 'domain' => 'test-wp.auth0.com', + 'auth0_app_token' => implode( '.', [ uniqid(), uniqid(), uniqid() ] ), + 'client_id' => 'TEST_CLIENT_ID', + ]; + + $this->http_request_type = [ 'success_get_connections', 'wp_error' ]; + + $result = $admin_features->security_validation( $old_input, $new_input ); + + global $wp_settings_errors; + + $this->assertContains( 'There was a problem updating the password policy.', $wp_settings_errors[0]['message'] ); + $this->assertContains( 'https://manage.auth0.com/#/connections/database', $wp_settings_errors[0]['message'] ); + $this->assertEquals( 'wp_auth0_settings', $wp_settings_errors[0]['code'] ); + $this->assertEquals( 'wp_auth0_settings', $wp_settings_errors[0]['setting'] ); + $this->assertEquals( $old_input['password_policy'], $result['password_policy'] ); + } + + /** + * Test that an end-to-end update is successful. + */ + public function testThatPasswordPolicyUpdatesWithoutErrors() { + $this->startHttpMocking(); + + $admin_features = new WP_Auth0_Admin_Features( self::$opts ); + + $old_input = [ 'password_policy' => 'fair' ]; + $new_input = [ + 'password_policy' => 'good', + 'domain' => 'test-wp.auth0.com', + 'auth0_app_token' => implode( '.', [ uniqid(), uniqid(), uniqid() ] ), + 'client_id' => 'TEST_CLIENT_ID', + ]; + + $this->http_request_type = [ 'success_get_connections', 'success_update_connection' ]; + + $result = $admin_features->security_validation( $old_input, $new_input ); + $this->assertEquals( $new_input['password_policy'], $result['password_policy'] ); + + global $wp_settings_errors; + $this->assertEmpty( $wp_settings_errors ); + } + + /** + * Runs after each test method. + */ + public function tearDown() { + parent::tearDown(); + + $this->stopHttpHalting(); + $this->stopHttpMocking(); + + self::$error_log->clear(); + } +} diff --git a/tests/testInitialSetupConsent.php b/tests/testInitialSetupConsent.php index 37f6edde..9d0ffab3 100644 --- a/tests/testInitialSetupConsent.php +++ b/tests/testInitialSetupConsent.php @@ -284,12 +284,6 @@ public function httpMock() { '","enabled_clients":["TEST_CLIENT_ID"],"options":{"passwordPolicy":"good"}}]', 'response' => [ 'code' => 200 ], ]; - - case 'success_get_connections': - return [ - 'body' => '[{"id":"TEST_CONN_ID","name":"TEST_CONNECTION","enabled_clients":["TEST_CLIENT_ID"]}]', - 'response' => [ 'code' => 200 ], - ]; } return $this->httpMockDefault( $response_type ); diff --git a/tests/traits/httpHelpers.php b/tests/traits/httpHelpers.php index c47ad016..07d94abc 100644 --- a/tests/traits/httpHelpers.php +++ b/tests/traits/httpHelpers.php @@ -62,7 +62,7 @@ public function stopHttpHalting() { * Use this at the top of tests that should test behavior for different HTTP responses. */ public function startHttpMocking() { - add_filter( 'pre_http_request', [ $this, 'httpMock' ], 1 ); + add_filter( 'pre_http_request', [ $this, 'httpMock' ], 1, 3 ); } /** @@ -80,13 +80,19 @@ public function getResponseType() { /** * Mock returns from the HTTP client. * - * @param null|string $response_type - Response type to use. + * @param string|null $response_type - HTTP response type to use. + * @param array|null $args - HTTP args. + * @param string|null $url - Remote URL. * * @return array|WP_Error */ - public function httpMock( $response_type = null ) { + public function httpMock( $response_type = null, array $args = null, $url = null ) { switch ( $response_type ?: $this->getResponseType() ) { + case 'halt': + $this->httpHalt( false, $args, $url ); + return new WP_Error( 3, 'Halted.' ); + case 'wp_error': return new WP_Error( 1, 'Caught WP_Error.' ); @@ -132,6 +138,17 @@ public function httpMock( $response_type = null ) { 'response' => [ 'code' => 200 ], ]; + case 'success_get_connections': + return [ + 'body' => '[{ + "id":"TEST_CONN_ID", + "name":"TEST_CONNECTION", + "enabled_clients":["TEST_CLIENT_ID"], + "options":{"passwordPolicy":"poor"} + }]', + 'response' => [ 'code' => 200 ], + ]; + default: return new WP_Error( 2, 'No mock type found.' ); } From d9d9e5d64cb08484f5a7df94354129837a489ebc Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Thu, 8 Nov 2018 13:14:53 -0800 Subject: [PATCH 3/3] PR feedback --- .../WP_Auth0_InitialSetup_Consent.php | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/initial-setup/WP_Auth0_InitialSetup_Consent.php b/lib/initial-setup/WP_Auth0_InitialSetup_Consent.php index 7c371330..03592b10 100644 --- a/lib/initial-setup/WP_Auth0_InitialSetup_Consent.php +++ b/lib/initial-setup/WP_Auth0_InitialSetup_Consent.php @@ -149,19 +149,24 @@ public function consent_callback( $name ) { if ( $should_create_and_update_connection && ! empty( $connections ) && is_array( $connections ) ) { foreach ( $connections as $connection ) { - if ( in_array( $client_id, $connection->enabled_clients ) ) { - - if ( $db_connection_name === $connection->name ) { - $connection_exists = $connection->id; - if ( isset( $connection->options ) && isset( $connection->options->passwordPolicy ) ) { - $connection_pwd_policy = $connection->options->passwordPolicy; - } - } else { - $u_connection = clone $connection; - $u_connection->enabled_clients = array_diff( $u_connection->enabled_clients, array( $client_id ) ); - WP_Auth0_Api_Client::update_connection( $domain, $app_token, $u_connection->id, $u_connection ); + // We only want to check/update connections that are active for this Application. + if ( ! in_array( $client_id, $connection->enabled_clients ) ) { + continue; + } + + // Connection with the same name, use it below. + if ( $db_connection_name === $connection->name ) { + $connection_exists = $connection->id; + if ( isset( $connection->options ) && isset( $connection->options->passwordPolicy ) ) { + $connection_pwd_policy = $connection->options->passwordPolicy; } + continue; } + + // Different Connection, update to remove the Application created above. + $u_connection = clone $connection; + $u_connection->enabled_clients = array_diff( $u_connection->enabled_clients, array( $client_id ) ); + WP_Auth0_Api_Client::update_connection( $domain, $app_token, $u_connection->id, $u_connection ); } }