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

Fix Migration Token Generation; Add JSON Content-Type header #617

Merged
merged 4 commits into from
Jan 9, 2019
Merged

Fix Migration Token Generation; Add JSON Content-Type header #617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b057079
Fix migration token incorrectly being generated
joshcanhelp Jan 8, 2019
e152006
Add JSON Content-Type header for Auth0 endpoints
joshcanhelp Jan 8, 2019
e7243b2
Add failing test for migration_token
joshcanhelp Jan 9, 2019
7458714
Adjust migration_token setting to avoid deletion
joshcanhelp Jan 9, 2019
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
21 changes: 20 additions & 1 deletion lib/WP_Auth0_Routes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public function custom_requests( $wp, $return = false ) {
return false;
}

$json_header = true;
switch ( $page ) {
case 'oauth2-config':
$output = wp_json_encode( $this->oauth2_config() );
Expand All @@ -88,7 +89,8 @@ public function custom_requests( $wp, $return = false ) {
$output = wp_json_encode( $this->migration_ws_get_user() );
break;
case 'coo-fallback':
$output = $this->coo_fallback();
$json_header = false;
$output = $this->coo_fallback();
break;
default:
return false;
Expand All @@ -98,10 +100,27 @@ public function custom_requests( $wp, $return = false ) {
return $output;
}

if ( $json_header ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this OK here or should it go before the if ($return) above ?? Maybe the send_headers() function might need to be called as well and you missed that (I don't really know)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need for a header if we're just spitting back the data rather than outputting.

add_filter( 'wp_headers', array( $this, 'add_json_header' ) );
$wp->send_headers();
}

echo $output;
exit;
}

/**
* Use with the wp_headers filter to add a Content-Type header for JSON output.
*
* @param array $headers - Existing headers to modify.
*
* @return mixed
*/
public function add_json_header( array $headers ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needs to be public to work with add_filter() call

$headers['Content-Type'] = 'application/json; charset=' . get_bloginfo( 'charset' );
return $headers;
}

protected function coo_fallback() {
$protocol = $this->a0_options->get( 'force_https_callback', false ) ? 'https' : null;
return sprintf(
Expand Down
4 changes: 3 additions & 1 deletion lib/admin/WP_Auth0_Admin_Advanced.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,8 @@ public function basic_validation( $old_options, $input ) {
* @return array
*/
public function migration_ws_validation( array $old_options, array $input ) {
$input['migration_ws'] = (int) ! empty( $input['migration_ws'] );
$input['migration_ws'] = (int) ! empty( $input['migration_ws'] );
$input['migration_token'] = $this->options->get( 'migration_token' );

// Migration endpoints or turned off, nothing to do.
if ( empty( $input['migration_ws'] ) ) {
Expand All @@ -589,6 +590,7 @@ public function migration_ws_validation( array $old_options, array $input ) {
if ( ! empty( $input['client_secret_b64_encoded'] ) ) {
$secret = base64_decode( $input['client_secret'] );
}

try {
$token_decoded = JWT::decode( $input['migration_token'], $secret, array( 'HS256' ) );
$input['migration_token_id'] = isset( $token_decoded->jti ) ? $token_decoded->jti : null;
Expand Down
6 changes: 3 additions & 3 deletions tests/testAdvancedOptionsValidation.php → tests/testOptionLoginRedirect.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renaming test suite

/**
* Contains Class TestAdvancedOptionsValidation.
* Contains Class TestOptionLoginRedirect.
*
* @package WP-Auth0
*
Expand All @@ -10,10 +10,10 @@
use PHPUnit\Framework\TestCase;

/**
* Class TestAdvancedOptionsValidation.
* Class TestOptionLoginRedirect.
* Tests that Advanced settings are validated properly.
*/
class TestAdvancedOptionsValidation extends TestCase {
class TestOptionLoginRedirect extends TestCase {

use setUpTestDb {
setUp as setUpDb;
Expand Down
65 changes: 32 additions & 33 deletions tests/testOptionMigrationWs.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public static function setUpBeforeClass() {
public function setUp() {
parent::setUp();
self::setUpDb();
self::$opts->reset();
$router = new WP_Auth0_Routes( self::$opts );
self::$admin = new WP_Auth0_Admin_Advanced( self::$opts, $router );
}
Expand All @@ -74,30 +75,31 @@ public function tearDown() {
* Test that turning migration endpoints off does not affect new input.
*/
public function testThatChangingMigrationToOffKeepsTokenData() {
self::$opts->set( 'migration_token', 'existing_token' );
$input = [
'migration_ws' => 0,
'migration_token' => 'new_token',
'migration_token_id' => 'new_token_id',
'migration_token_id' => 'existing_token_id',
];
$old_input = [ 'migration_ws' => 1 ];
$validated = self::$admin->migration_ws_validation( $old_input, $input );
$this->assertEquals( $input, $validated );
$validated = self::$admin->migration_ws_validation( [], $input );

$this->assertEquals( $input['migration_ws'], $validated['migration_ws'] );
$this->assertEquals( $input['migration_token_id'], $validated['migration_token_id'] );
$this->assertEquals( 'existing_token', $validated['migration_token'] );
}

/**
* Test that turning on migration keeps the existing token and sets an admin notification.
*/
public function testThatChangingMigrationToOnKeepsToken() {
$input = [
'migration_ws' => 1,
'migration_token' => 'new_token',
'client_secret' => '__test_client_secret__',
self::$opts->set( 'migration_token', 'new_token' );
$input = [
'migration_ws' => 1,
'client_secret' => '__test_client_secret__',
];
$old_input = [ 'migration_ws' => 0 ];

$validated = self::$admin->migration_ws_validation( $old_input, $input );
$validated = self::$admin->migration_ws_validation( [], $input );

$this->assertEquals( $input['migration_token'], $validated['migration_token'] );
$this->assertEquals( 'new_token', $validated['migration_token'] );
$this->assertNull( $validated['migration_token_id'] );
$this->assertEquals( $input['migration_ws'], $validated['migration_ws'] );
}
Expand All @@ -106,18 +108,18 @@ public function testThatChangingMigrationToOnKeepsToken() {
* Test that turning on migration keeps the existing token and sets an admin notification.
*/
public function testThatChangingMigrationToOnKeepsWithJwtSetsId() {
$client_secret = '__test_client_secret__';
$input = [
'migration_ws' => 1,
'migration_token' => JWT::encode( [ 'jti' => '__test_token_id__' ], $client_secret ),
'client_secret' => $client_secret,
$client_secret = '__test_client_secret__';
$migration_token = JWT::encode( [ 'jti' => '__test_token_id__' ], $client_secret );
self::$opts->set( 'migration_token', $migration_token );
$input = [
'migration_ws' => 1,
'client_secret' => $client_secret,
];
$old_input = [ 'migration_ws' => 0 ];

$validated = self::$admin->migration_ws_validation( $old_input, $input );
$validated = self::$admin->migration_ws_validation( [], $input );

$this->assertEquals( $input['migration_ws'], $validated['migration_ws'] );
$this->assertEquals( $input['migration_token'], $validated['migration_token'] );
$this->assertEquals( $migration_token, $validated['migration_token'] );
$this->assertEquals( '__test_token_id__', $validated['migration_token_id'] );
}

Expand All @@ -126,15 +128,14 @@ public function testThatChangingMigrationToOnKeepsWithJwtSetsId() {
*/
public function testThatChangingMigrationToOnKeepsWithBase64JwtSetsId() {
$client_secret = '__test_client_secret__';
$input = [
self::$opts->set( 'migration_token', JWT::encode( [ 'jti' => '__test_token_id__' ], $client_secret ) );
$input = [
'migration_ws' => 1,
'migration_token' => JWT::encode( [ 'jti' => '__test_token_id__' ], $client_secret ),
'client_secret' => JWT::urlsafeB64Encode( $client_secret ),
'client_secret_b64_encoded' => 1,
];
$old_input = [ 'migration_ws' => 0 ];

$validated = self::$admin->migration_ws_validation( $old_input, $input );
$validated = self::$admin->migration_ws_validation( [], $input );

$this->assertEquals( '__test_token_id__', $validated['migration_token_id'] );
}
Expand All @@ -143,10 +144,9 @@ public function testThatChangingMigrationToOnKeepsWithBase64JwtSetsId() {
* Test that turning on migration endpoints without a stored token will generate one.
*/
public function testThatChangingMigrationToOnGeneratesNewToken() {
$input = [ 'migration_ws' => 1 ];
$old_input = [ 'migration_ws' => 0 ];
$input = [ 'migration_ws' => 1 ];

$validated = self::$admin->migration_ws_validation( $old_input, $input );
$validated = self::$admin->migration_ws_validation( [], $input );

$this->assertGreaterThan( 64, strlen( $validated['migration_token'] ) );
$this->assertNull( $validated['migration_token_id'] );
Expand All @@ -160,18 +160,17 @@ public function testThatChangingMigrationToOnGeneratesNewToken() {
*/
public function testThatMigrationTokenInConstantSettingIsValidated() {
define( 'AUTH0_ENV_MIGRATION_TOKEN', '__test_constant_setting__' );
$input = [
'migration_ws' => 1,
'migration_token' => AUTH0_ENV_MIGRATION_TOKEN,
'client_secret' => '__test_client_secret__',
self::$opts->set( 'migration_token', '__test_saved_setting__' );
$input = [
'migration_ws' => 1,
'client_secret' => '__test_client_secret__',
];
$old_input = [ 'migration_ws' => 0 ];

$opts = new WP_Auth0_Options();
$router = new WP_Auth0_Routes( $opts );
$admin = new WP_Auth0_Admin_Advanced( $opts, $router );

$validated = $admin->migration_ws_validation( $old_input, $input );
$validated = $admin->migration_ws_validation( [], $input );

$this->assertNull( $validated['migration_token_id'] );
$this->assertEquals( $input['migration_ws'], $validated['migration_ws'] );
Expand Down