From 6e2c216784b14dd42d2a6c5c5f43cc931949e873 Mon Sep 17 00:00:00 2001 From: Almir Bijedic Date: Tue, 5 Sep 2017 22:33:33 +0200 Subject: [PATCH 1/3] Added PKCE support --- .gitignore | 5 +- README.md | 20 ++++++++ inc/class-client.php | 4 +- inc/endpoints/class-token.php | 7 ++- inc/namespace.php | 6 +++ inc/tokens/class-authorization-code.php | 46 +++++++++++++++-- inc/types/class-authorization-code.php | 2 +- inc/types/class-base.php | 65 +++++++++++++++++++++--- inc/utilities/class-oauth2-wp-cli.php | 67 +++++++++++++++++++++++++ plugin.php | 4 ++ 10 files changed, 211 insertions(+), 15 deletions(-) create mode 100644 inc/utilities/class-oauth2-wp-cli.php diff --git a/.gitignore b/.gitignore index 7b3dea9..d7d46e9 100644 --- a/.gitignore +++ b/.gitignore @@ -14,4 +14,7 @@ _book *.epub *.mobi *.pdf -.idea \ No newline at end of file +.idea +.vscode +vendor +composer.lock diff --git a/README.md b/README.md index 92000b9..67df65a 100755 --- a/README.md +++ b/README.md @@ -6,6 +6,26 @@ This plugin uses the OAuth 2 protocol to allow delegated authorization; that is, This plugin only supports WordPress >= 4.8. +## Proof Key for Code Exchange +OAuth2 plugin supports PKCE as a protection against authorization code interception attack (relevant only for authorization code grant type). In order to use PKCE, on the initial authorization request, add two fields: +* _code_challenge_ +* _code_challenge_method_ (optional). + +Code verifier is a 43-128 length random string consisting of [A-Z] / [a-z] / [0-9] / "-" / "." / "_" / "~". Code challenge is derived from the code verifier depending on the challenge method. Two types are supported, 's256' and 'plain'. Plain is just code_challenge = code_verifier. s256 method uses SHA256 to hash the code verifier and then do a base64 encoding of the resulting hash. e.g. + +code_verifier = 052edd3941bb8040ecac75d2359d7cd1abe2518911b
+code_challenge = base64( sha256( code_verifier ) ) = MmNmZTJlNGZhYmNmYzQ3YTI4MmRhY2Q1NGEwZDUzZTFiZGFhNTNlODI4MGY3NjM0YWUwNjA1YjYzMmQwNDMxNQ==
+code_challenge_method = s256 + +In the next step, when using the code received from the server to obtain an access token, code_verifier must be passed in as an additional field to the request, and it must be using the code_verifier value that was used to calculate the code_challenge in the initial request. + +## CLI Commands + +### PKCE + +A custom WP CLI helper command to generate a random code verifier and a code challenge. + +```wp oauth2 generate-code-challenge``` ## Warning diff --git a/inc/class-client.php b/inc/class-client.php index ddba759..31e588c 100644 --- a/inc/class-client.php +++ b/inc/class-client.php @@ -229,8 +229,8 @@ public function check_redirect_uri( $uri ) { * * @return Authorization_Code|WP_Error */ - public function generate_authorization_code( WP_User $user ) { - return Authorization_Code::create( $this, $user ); + public function generate_authorization_code( WP_User $user, $data ) { + return Authorization_Code::create( $this, $user, $data ); } /** diff --git a/inc/endpoints/class-token.php b/inc/endpoints/class-token.php index 6b24a82..bfb57a0 100644 --- a/inc/endpoints/class-token.php +++ b/inc/endpoints/class-token.php @@ -31,6 +31,11 @@ public function register_routes() { 'type' => 'string', 'validate_callback' => 'rest_validate_request_arg', ], + 'code_verifier' => [ + 'required' => false, + 'type' => 'string', + 'validate_callback' => 'rest_validate_request_arg', + ], ], ] ); } @@ -71,7 +76,7 @@ public function exchange_token( WP_REST_Request $request ) { return $auth_code; } - $is_valid = $auth_code->validate(); + $is_valid = $auth_code->validate( $request ); if ( is_wp_error( $is_valid ) ) { // Invalid request, but code itself exists, so we should delete // (and silently ignore errors). diff --git a/inc/namespace.php b/inc/namespace.php index b46b129..9f6ee89 100644 --- a/inc/namespace.php +++ b/inc/namespace.php @@ -21,6 +21,12 @@ function bootstrap() { // Admin-related. add_action( 'init', __NAMESPACE__ . '\\rest_oauth2_load_authorize_page' ); add_action( 'admin_menu', __NAMESPACE__ . '\\Admin\\register' ); + + // WP-Cli + if ( class_exists( __NAMESPACE__ . '\\Utilities\\Oauth2_Wp_Cli' ) ) { + \WP_CLI::add_command( 'oauth2', __NAMESPACE__ . '\\Utilities\\Oauth2_Wp_Cli' ); + } + Admin\Profile\bootstrap(); } diff --git a/inc/tokens/class-authorization-code.php b/inc/tokens/class-authorization-code.php index e5f212e..9b8c021 100644 --- a/inc/tokens/class-authorization-code.php +++ b/inc/tokens/class-authorization-code.php @@ -108,6 +108,36 @@ public function get_expiration() { return (int) $value['expiration']; } + private function validate_code_verifier( $args ) { + $value = $this->get_value(); + + if ( empty( $value['code_challenge'] ) ) { + return true; + } + + $code_verifier = $args['code_verifier']; + $is_valid = false; + + switch ( strtolower( $value['code_challenge_method'] ) ) { + case 's256': + $decoded = base64_encode( hash( 'sha256', $code_verifier ) ); + $is_valid = $decoded === $value['code_challenge']; + break; + case 'plain': + $is_valid = $code_verifier === $value['code_challenge']; + break; + } + + if ( ! $is_valid ) { + return new WP_Error( + 'oauth2.tokens.authorization_code.validate_code_verifier.invalid_grant', + __( 'Invalid code verifier.', 'oauth2' ) + ); + } + + return true; + } + /** * Validate the code for use. * @@ -129,7 +159,15 @@ public function validate( $args = [] ) { ); } - return true; + $code_verifier = $this->validate_code_verifier( [ + 'code_verifier' => $args['code_verifier'], + ] + ); + if ( is_wp_error( $code_verifier ) ) { + return $code_verifier; + } + + return $code_verifier; } /** @@ -183,13 +221,13 @@ public static function get_by_code( Client $client, $code ) { * * @return Authorization_Code|WP_Error Authorization code instance, or error on failure. */ - public static function create( Client $client, WP_User $user ) { + public static function create( Client $client, WP_User $user, $data ) { $code = wp_generate_password( static::KEY_LENGTH, false ); $meta_key = static::KEY_PREFIX . $code; - $data = [ + $data = \array_merge( [ 'user' => (int) $user->ID, 'expiration' => time() + static::MAX_AGE, - ]; + ], $data ); $result = add_post_meta( $client->get_post_id(), wp_slash( $meta_key ), wp_slash( $data ), true ); if ( ! $result ) { return new WP_Error( diff --git a/inc/types/class-authorization-code.php b/inc/types/class-authorization-code.php index cff7e86..1166ae0 100644 --- a/inc/types/class-authorization-code.php +++ b/inc/types/class-authorization-code.php @@ -33,7 +33,7 @@ protected function handle_authorization_submission( $submit, Client $client, $da case 'authorize': // Generate authorization code and redirect back. $user = wp_get_current_user(); - $code = $client->generate_authorization_code( $user ); + $code = $client->generate_authorization_code( $user, $data ); if ( is_wp_error( $code ) ) { return $code; } diff --git a/inc/types/class-base.php b/inc/types/class-base.php index cde81a1..286058d 100644 --- a/inc/types/class-base.php +++ b/inc/types/class-base.php @@ -7,6 +7,7 @@ use WP\OAuth2\Client; abstract class Base implements Type { + /** * Handle submission of authorisation page. * @@ -25,6 +26,8 @@ abstract protected function handle_authorization_submission( $submit, Client $cl * Handle authorisation page. */ public function handle_authorisation() { + // Should probably keep this as an option in the application (e.g. force PKCE) + $pkce = true; if ( empty( $_GET['client_id'] ) ) { return new WP_Error( @@ -34,10 +37,20 @@ public function handle_authorisation() { } // Gather parameters. - $client_id = wp_unslash( $_GET['client_id'] ); - $redirect_uri = isset( $_GET['redirect_uri'] ) ? wp_unslash( $_GET['redirect_uri'] ) : null; - $scope = isset( $_GET['scope'] ) ? wp_unslash( $_GET['scope'] ) : null; - $state = isset( $_GET['state'] ) ? wp_unslash( $_GET['state'] ) : null; + $client_id = wp_unslash( $_GET['client_id'] ); + $redirect_uri = isset( $_GET['redirect_uri'] ) ? wp_unslash( $_GET['redirect_uri'] ) : null; + $scope = isset( $_GET['scope'] ) ? wp_unslash( $_GET['scope'] ) : null; + $state = isset( $_GET['state'] ) ? wp_unslash( $_GET['state'] ) : null; + + if ( $pkce ) { + $pkce_data = $this->handle_pkce(); + if ( is_wp_error( $pkce_data ) ) { + return $pkce_data; + } + + $code_challenge = $pkce_data['code_challenge']; + $code_challenge_method = $pkce_data['code_challenge_method']; + } $client = Client::get_by_id( $client_id ); if ( empty( $client ) ) { @@ -70,7 +83,7 @@ public function handle_authorisation() { // Check nonce. $nonce_action = $this->get_nonce_action( $client ); - if ( ! wp_verify_nonce( wp_unslash( $_POST['_wpnonce'] ), $none_action ) ) { + if ( ! wp_verify_nonce( wp_unslash( $_POST['_wpnonce'] ), $this->get_nonce_action( $client ) ) ) { return new WP_Error( 'oauth2.types.authorization_code.handle_authorisation.invalid_nonce', __( 'Invalid nonce.', 'oauth2' ) @@ -93,7 +106,7 @@ public function handle_authorisation() { $submit = wp_unslash( $_POST['wp-submit'] ); - $data = compact( 'redirect_uri', 'scope', 'state' ); + $data = compact( 'redirect_uri', 'scope', 'state', 'code_challenge', 'code_challenge_method' ); return $this->handle_authorization_submission( $submit, $client, $data ); } @@ -152,4 +165,44 @@ protected function render_form( Client $client, WP_Error $errors = null ) { protected function get_nonce_action( Client $client ) { return sprintf( 'oauth2_authorize:%s', $client->get_id() ); } + + /** + * Get and validate PKCE parameters from a request. + * + * @return string[] code_challenge and code_challenge_method + */ + private function handle_pkce() { + $code_challenge = isset( $_GET['code_challenge'] ) ? wp_unslash( $_GET['code_challenge'] ) : null; + $code_challenge_method = isset( $_GET['code_challenge_method'] ) ? wp_unslash( $_GET['code_challenge_method'] ) : null; + + if ( ! is_null( $code_challenge ) ) { + if ( '' === \trim( $code_challenge ) ) { + return new WP_Error( + 'oauth2.types.authorization_code.handle_authorisation.code_challenge_empty', + sprintf( __( 'Code challenge cannot be empty', 'oauth2' ), $client_id ), + [ + 'status' => WP_Http::BAD_REQUEST, + 'client_id' => $client_id, + ] + ); + } + + $code_challenge_method = is_null( $code_challenge_method ) ? 'plain' : $code_challenge_method; + + if ( ! \in_array( \strtolower( $code_challenge_method ), [ 'plain', 's256' ], true ) ) { + return new WP_Error( + 'oauth2.types.authorization_code.handle_authorisation.wrong_challenge_method', + sprintf( __( 'Challenge method must be S256 or plain', 'oauth2' ), $client_id ), + [ + 'status' => WP_Http::BAD_REQUEST, + 'client_id' => $client_id, + ] + ); + } + + return [ 'code_challenge' => $code_challenge, 'code_challenge_method' => $code_challenge_method ]; + } + + return false; + } } diff --git a/inc/utilities/class-oauth2-wp-cli.php b/inc/utilities/class-oauth2-wp-cli.php new file mode 100644 index 0000000..8c864a0 --- /dev/null +++ b/inc/utilities/class-oauth2-wp-cli.php @@ -0,0 +1,67 @@ +] + * : The string to be hashed. + * + * + * [--length=] + * : The length of the random seed string. + * --- + * default: 64 + * --- + * + * ## EXAMPLES + * + * wp oauth2 generate-code-challenge --length=64 + * + * @alias generate-code-challenge + */ + function generate_code_challenge( $args, $assoc_args ) { + if ( ! empty( $args[0] ) && ! empty( $assoc_args['length'] ) ) { + \WP_CLI::warning( 'Length parameter will be ignored since the input string was provided.' ); + } + + $length = empty( $assoc_args['length'] ) ? 64 : intval( $assoc_args['length'] ); + + if ( $length < 43 || $length > 128 ) { + \WP_CLI::error( 'Length should be >= 43 and <= 128.' ); + } + + if ( ! empty( $args[0] ) ) { + $random_seed = $args[0]; + + if ( strlen( $random_seed ) < 43 || strlen( $random_seed ) > 128 ) { + \WP_CLI::error( 'Length of the provided random seed should be >= 43 and <= 128.' ); + } + + \WP_CLI::warning( "Using provided string {$random_seed} as a random seed. It is recommended to use this command without parameters, 64 characters long random key will be generated automatically." ); + } else { + $is_strong_crypto = true; + $random_seed = \bin2hex( \openssl_random_pseudo_bytes( $length / 2 + $length % 2, $is_strong_crypto ) ); + $random_seed = \substr( $random_seed, 0, $length ); + + if ( ! $is_strong_crypto ) { + \WP_CLI::error( 'openssl_random_pseudo_bytes failed to generate a cryptographically strong random string.' ); + } + } + + $code_challenge = \base64_encode( hash( 'sha256', $random_seed ) ); + + $items = [ + [ + 'code_verifier' => $random_seed, + 'code_challenge = base64( sha256( code_verifier ) )' => $code_challenge, + ], + ]; + + \WP_CLI\Utils\format_items( 'table', $items, [ 'code_verifier', 'code_challenge = base64( sha256( code_verifier ) )' ] ); + } +} diff --git a/plugin.php b/plugin.php index af7ced8..48caf6a 100644 --- a/plugin.php +++ b/plugin.php @@ -28,4 +28,8 @@ require __DIR__ . '/inc/admin/namespace.php'; require __DIR__ . '/inc/admin/profile/namespace.php'; +if ( defined( 'WP_CLI' ) && WP_CLI ) { + require __DIR__ . '/inc/utilities/class-oauth2-wp-cli.php'; +} + bootstrap(); From 9ebda85525f56684eb62bcf1a529b3b53b9e9f0a Mon Sep 17 00:00:00 2001 From: Almir Bijedic Date: Sat, 16 Sep 2017 23:48:33 +0200 Subject: [PATCH 2/3] Refactoring; Throw an exception if unsupported challenge method; --- README.md | 5 +++-- inc/endpoints/class-token.php | 2 +- inc/tokens/class-authorization-code.php | 19 +++++++++++++++---- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 67df65a..7f51a46 100755 --- a/README.md +++ b/README.md @@ -13,9 +13,10 @@ OAuth2 plugin supports PKCE as a protection against authorization code intercept Code verifier is a 43-128 length random string consisting of [A-Z] / [a-z] / [0-9] / "-" / "." / "_" / "~". Code challenge is derived from the code verifier depending on the challenge method. Two types are supported, 's256' and 'plain'. Plain is just code_challenge = code_verifier. s256 method uses SHA256 to hash the code verifier and then do a base64 encoding of the resulting hash. e.g. -code_verifier = 052edd3941bb8040ecac75d2359d7cd1abe2518911b
-code_challenge = base64( sha256( code_verifier ) ) = MmNmZTJlNGZhYmNmYzQ3YTI4MmRhY2Q1NGEwZDUzZTFiZGFhNTNlODI4MGY3NjM0YWUwNjA1YjYzMmQwNDMxNQ==
+```code_verifier = 052edd3941bb8040ecac75d2359d7cd1abe2518911b +code_challenge = base64( sha256( code_verifier ) ) = MmNmZTJlNGZhYmNmYzQ3YTI4MmRhY2Q1NGEwZDUzZTFiZGFhNTNlODI4MGY3NjM0YWUwNjA1YjYzMmQwNDMxNQ== code_challenge_method = s256 +``` In the next step, when using the code received from the server to obtain an access token, code_verifier must be passed in as an additional field to the request, and it must be using the code_verifier value that was used to calculate the code_challenge in the initial request. diff --git a/inc/endpoints/class-token.php b/inc/endpoints/class-token.php index bfb57a0..0db2265 100644 --- a/inc/endpoints/class-token.php +++ b/inc/endpoints/class-token.php @@ -76,7 +76,7 @@ public function exchange_token( WP_REST_Request $request ) { return $auth_code; } - $is_valid = $auth_code->validate( $request ); + $is_valid = $auth_code->validate( [ 'code_verifier' => $request['code_verifier'] ] ); if ( is_wp_error( $is_valid ) ) { // Invalid request, but code itself exists, so we should delete // (and silently ignore errors). diff --git a/inc/tokens/class-authorization-code.php b/inc/tokens/class-authorization-code.php index 9b8c021..a799712 100644 --- a/inc/tokens/class-authorization-code.php +++ b/inc/tokens/class-authorization-code.php @@ -108,7 +108,13 @@ public function get_expiration() { return (int) $value['expiration']; } - private function validate_code_verifier( $args ) { + /** + * Verify code_verifier for PKCE + * + * @param array $args Other request arguments to validate. + * @return Boolean|WP_error True if valid, error describing problem otherwise. + */ + protected function validate_code_verifier( $args ) { $value = $this->get_value(); if ( empty( $value['code_challenge'] ) ) { @@ -120,11 +126,16 @@ private function validate_code_verifier( $args ) { switch ( strtolower( $value['code_challenge_method'] ) ) { case 's256': - $decoded = base64_encode( hash( 'sha256', $code_verifier ) ); - $is_valid = $decoded === $value['code_challenge']; + $encoded = base64_encode( hash( 'sha256', $code_verifier ) ); + $is_valid = hash_equals( $encoded, $value['code_challenge'] ); break; case 'plain': - $is_valid = $code_verifier === $value['code_challenge']; + $is_valid = hash_equals( $code_verifier, $value['code_challenge'] ); + default: + return new WP_Error( + 'oauth2.tokens.authorization_code.validate_code_verifier.invalid_request', + __( 'Invalid challenge method.', 'oauth2' ) + ); break; } From 459d0f421651be48e3669919bea7f440fb47fef0 Mon Sep 17 00:00:00 2001 From: Almir Bijedic Date: Sun, 17 Sep 2017 03:26:32 +0200 Subject: [PATCH 3/3] Review changes; Add force PKCE as an option; --- inc/admin/namespace.php | 43 ++++---- inc/class-client.php | 12 +++ inc/namespace.php | 5 +- inc/tokens/class-authorization-code.php | 9 +- inc/types/class-base.php | 101 ++++++++++-------- ...ss-oauth2-wp-cli.php => class-command.php} | 23 ++-- plugin.php | 2 +- 7 files changed, 114 insertions(+), 81 deletions(-) rename inc/utilities/{class-oauth2-wp-cli.php => class-command.php} (55%) diff --git a/inc/admin/namespace.php b/inc/admin/namespace.php index 10fc0a8..669aeec 100644 --- a/inc/admin/namespace.php +++ b/inc/admin/namespace.php @@ -171,6 +171,7 @@ function validate_parameters( $params ) { if ( ! empty( $params['callback'] ) ) { $valid['callback'] = $params['callback']; } + $valid['force-pkce'] = ( ! empty( $params['force-pkce'] ) ) ? 'true' : 'false'; return $valid; } @@ -201,29 +202,21 @@ function handle_edit_submit( Client $consumer = null ) { return $messages; } + $data = [ + 'name' => $params['name'], + 'description' => $params['description'], + 'meta' => [ + 'type' => $params['type'], + 'callback' => $params['callback'], + 'force-pkce' => isset( $params['force-pkce'] ) ? 'true' : 'false', + ], + ]; + if ( empty( $consumer ) ) { // Create the consumer - $data = [ - 'name' => $params['name'], - 'description' => $params['description'], - 'meta' => [ - 'type' => $params['type'], - 'callback' => $params['callback'], - ], - ]; - $consumer = $result = Client::create( $data ); } else { // Update the existing consumer post - $data = [ - 'name' => $params['name'], - 'description' => $params['description'], - 'meta' => [ - 'type' => $params['type'], - 'callback' => $params['callback'], - ], - ]; - $result = $consumer->update( $data ); } @@ -300,7 +293,7 @@ function render_edit_page() { $data = []; if ( empty( $consumer ) || ! empty( $form_data ) ) { - foreach ( [ 'name', 'description', 'callback', 'type' ] as $key ) { + foreach ( [ 'name', 'description', 'callback', 'type', 'force-pkce' ] as $key ) { $data[ $key ] = empty( $form_data[ $key ] ) ? '' : $form_data[ $key ]; } } else { @@ -308,6 +301,7 @@ function render_edit_page() { $data['description'] = $consumer->get_description( true ); $data['type'] = $consumer->get_type(); $data['callback'] = $consumer->get_redirect_uris(); + $data['force-pkce'] = $consumer->should_force_pkce() ? 'true' : 'false'; if ( is_array( $data['callback'] ) ) { $data['callback'] = implode( ',', $data['callback'] ); @@ -404,7 +398,16 @@ function render_edit_page() { -

+

+ + + + + + + + class="regular-text" name="force-pkce" id="force-pkce" value="true"/> +

diff --git a/inc/class-client.php b/inc/class-client.php index 31e588c..abe2361 100644 --- a/inc/class-client.php +++ b/inc/class-client.php @@ -15,6 +15,7 @@ class Client { const TYPE_KEY = '_oauth2_client_type'; const REDIRECT_URI_KEY = '_oauth2_redirect_uri'; const AUTH_CODE_KEY_PREFIX = '_oauth2_authcode_'; + const FORCE_PKCE = '_oauth2_force-pkce'; const AUTH_CODE_LENGTH = 12; const CLIENT_ID_LENGTH = 12; const CLIENT_SECRET_LENGTH = 48; @@ -124,6 +125,15 @@ public function get_redirect_uris() { return (array) get_post_meta( $this->get_post_id(), static::REDIRECT_URI_KEY, true ); } + /** + * Whether the client requires PKCE + * + * @return Boolean + */ + public function should_force_pkce() { + return 'true' === get_post_meta( $this->get_post_id(), static::FORCE_PKCE, true ); + } + /** * Validate a callback URL. * @@ -332,6 +342,7 @@ public static function create( $data ) { static::REDIRECT_URI_KEY => $data['meta']['callback'], static::TYPE_KEY => $data['meta']['type'], static::CLIENT_SECRET_KEY => wp_generate_password( static::CLIENT_SECRET_LENGTH, false ), + static::FORCE_PKCE => $data['meta']['force-pkce'], ]; foreach ( $meta as $key => $value ) { @@ -368,6 +379,7 @@ public function update( $data ) { $meta = [ static::REDIRECT_URI_KEY => $data['meta']['callback'], static::TYPE_KEY => $data['meta']['type'], + static::FORCE_PKCE => $data['meta']['force-pkce'], ]; foreach ( $meta as $key => $value ) { diff --git a/inc/namespace.php b/inc/namespace.php index 9f6ee89..d335ae6 100644 --- a/inc/namespace.php +++ b/inc/namespace.php @@ -4,6 +4,7 @@ use WP\OAuth2\Types\Type; use WP_REST_Response; +use WP_CLI; function bootstrap() { // Core authentication hooks. @@ -23,8 +24,8 @@ function bootstrap() { add_action( 'admin_menu', __NAMESPACE__ . '\\Admin\\register' ); // WP-Cli - if ( class_exists( __NAMESPACE__ . '\\Utilities\\Oauth2_Wp_Cli' ) ) { - \WP_CLI::add_command( 'oauth2', __NAMESPACE__ . '\\Utilities\\Oauth2_Wp_Cli' ); + if ( class_exists( __NAMESPACE__ . '\\Utilities\\Command' ) ) { + WP_CLI::add_command( 'oauth2', __NAMESPACE__ . '\\Utilities\\Command' ); } Admin\Profile\bootstrap(); diff --git a/inc/tokens/class-authorization-code.php b/inc/tokens/class-authorization-code.php index a799712..2cd1d56 100644 --- a/inc/tokens/class-authorization-code.php +++ b/inc/tokens/class-authorization-code.php @@ -131,6 +131,7 @@ protected function validate_code_verifier( $args ) { break; case 'plain': $is_valid = hash_equals( $code_verifier, $value['code_challenge'] ); + break; default: return new WP_Error( 'oauth2.tokens.authorization_code.validate_code_verifier.invalid_request', @@ -172,13 +173,12 @@ public function validate( $args = [] ) { $code_verifier = $this->validate_code_verifier( [ 'code_verifier' => $args['code_verifier'], - ] - ); + ] ); if ( is_wp_error( $code_verifier ) ) { return $code_verifier; } - return $code_verifier; + return true; } /** @@ -229,13 +229,14 @@ public static function get_by_code( Client $client, $code ) { * * @param Client $client * @param WP_User $user + * @param Array $data Containing data specific for this OAuth2 request, like redirect_uri and code_challenge * * @return Authorization_Code|WP_Error Authorization code instance, or error on failure. */ public static function create( Client $client, WP_User $user, $data ) { $code = wp_generate_password( static::KEY_LENGTH, false ); $meta_key = static::KEY_PREFIX . $code; - $data = \array_merge( [ + $data = array_merge( [ 'user' => (int) $user->ID, 'expiration' => time() + static::MAX_AGE, ], $data ); diff --git a/inc/types/class-base.php b/inc/types/class-base.php index 286058d..15c8eaf 100644 --- a/inc/types/class-base.php +++ b/inc/types/class-base.php @@ -7,7 +7,6 @@ use WP\OAuth2\Client; abstract class Base implements Type { - /** * Handle submission of authorisation page. * @@ -26,9 +25,6 @@ abstract protected function handle_authorization_submission( $submit, Client $cl * Handle authorisation page. */ public function handle_authorisation() { - // Should probably keep this as an option in the application (e.g. force PKCE) - $pkce = true; - if ( empty( $_GET['client_id'] ) ) { return new WP_Error( 'oauth2.types.authorization_code.handle_authorisation.missing_client_id', @@ -42,16 +38,6 @@ public function handle_authorisation() { $scope = isset( $_GET['scope'] ) ? wp_unslash( $_GET['scope'] ) : null; $state = isset( $_GET['state'] ) ? wp_unslash( $_GET['state'] ) : null; - if ( $pkce ) { - $pkce_data = $this->handle_pkce(); - if ( is_wp_error( $pkce_data ) ) { - return $pkce_data; - } - - $code_challenge = $pkce_data['code_challenge']; - $code_challenge_method = $pkce_data['code_challenge_method']; - } - $client = Client::get_by_id( $client_id ); if ( empty( $client ) ) { return new WP_Error( @@ -64,6 +50,16 @@ public function handle_authorisation() { ); } + if ( $client->should_force_pkce() || isset( $_GET['code_challenge'] ) ) { + $pkce_data = $this->handle_pkce( wp_unslash( $_GET ) ); + if ( is_wp_error( $pkce_data ) ) { + return $pkce_data; + } + + $code_challenge = $pkce_data['code_challenge']; + $code_challenge_method = $pkce_data['code_challenge_method']; + } + // Validate the redirection URI. $redirect_uri = $this->validate_redirect_uri( $client, $redirect_uri ); if ( is_wp_error( $redirect_uri ) ) { @@ -83,7 +79,7 @@ public function handle_authorisation() { // Check nonce. $nonce_action = $this->get_nonce_action( $client ); - if ( ! wp_verify_nonce( wp_unslash( $_POST['_wpnonce'] ), $this->get_nonce_action( $client ) ) ) { + if ( ! wp_verify_nonce( wp_unslash( $_POST['_wpnonce'] ), $nonce_action ) ) { return new WP_Error( 'oauth2.types.authorization_code.handle_authorisation.invalid_nonce', __( 'Invalid nonce.', 'oauth2' ) @@ -106,7 +102,7 @@ public function handle_authorisation() { $submit = wp_unslash( $_POST['wp-submit'] ); - $data = compact( 'redirect_uri', 'scope', 'state', 'code_challenge', 'code_challenge_method' ); + $data = array_merge( compact( 'redirect_uri', 'scope', 'state' ), isset( $pkce_data ) ? $pkce_data : [] ); return $this->handle_authorization_submission( $submit, $client, $data ); } @@ -169,40 +165,59 @@ protected function get_nonce_action( Client $client ) { /** * Get and validate PKCE parameters from a request. * + * @param Array $args Array with code_challenge (required) and code_challenge_method (optional) + * * @return string[] code_challenge and code_challenge_method */ - private function handle_pkce() { - $code_challenge = isset( $_GET['code_challenge'] ) ? wp_unslash( $_GET['code_challenge'] ) : null; - $code_challenge_method = isset( $_GET['code_challenge_method'] ) ? wp_unslash( $_GET['code_challenge_method'] ) : null; + protected function handle_pkce( $args ) { + $code_challenge = isset( $args['code_challenge'] ) ? $args['code_challenge'] : null; + $code_challenge_method = isset( $args['code_challenge_method'] ) ? $args['code_challenge_method'] : null; - if ( ! is_null( $code_challenge ) ) { - if ( '' === \trim( $code_challenge ) ) { - return new WP_Error( - 'oauth2.types.authorization_code.handle_authorisation.code_challenge_empty', - sprintf( __( 'Code challenge cannot be empty', 'oauth2' ), $client_id ), - [ - 'status' => WP_Http::BAD_REQUEST, - 'client_id' => $client_id, - ] - ); - } + if ( empty( $code_challenge ) ) { + return new WP_Error( + 'oauth2.types.authorization_code.handle_authorisation.code_challenge_empty', + __( 'Code challenge cannot be empty', 'oauth2' ), $client_id, + [ + 'status' => WP_Http::BAD_REQUEST, + 'client_id' => $client_id, + ] + ); + } - $code_challenge_method = is_null( $code_challenge_method ) ? 'plain' : $code_challenge_method; + if ( strlen( $code_challenge ) < 43 || strlen( $code_challenge ) > 128 ) { + return new WP_Error( + 'oauth2.types.authorization_code.handle_authorisation.code_challenge_length', + __( 'Code challenge should be 43 or more characters in length and less or equal to 128.', 'oauth2' ), $client_id, + [ + 'status' => WP_Http::BAD_REQUEST, + 'client_id' => $client_id, + ] + ); + } - if ( ! \in_array( \strtolower( $code_challenge_method ), [ 'plain', 's256' ], true ) ) { - return new WP_Error( - 'oauth2.types.authorization_code.handle_authorisation.wrong_challenge_method', - sprintf( __( 'Challenge method must be S256 or plain', 'oauth2' ), $client_id ), - [ - 'status' => WP_Http::BAD_REQUEST, - 'client_id' => $client_id, - ] - ); - } + if ( 0 === preg_match( '/^[a-zA-Z 0-9\.\-\_\~]*$/', $code_challenge ) ) { + return new WP_Error( + 'oauth2.types.authorization_code.handle_authorisation.code_challenge', + __( 'Should only containz A-Z, a-z, 0-9, ., -, _, ~', 'oauth2' ), $client_id, + [ + 'status' => WP_Http::BAD_REQUEST, + 'client_id' => $client_id, + ] + ); + } - return [ 'code_challenge' => $code_challenge, 'code_challenge_method' => $code_challenge_method ]; + $code_challenge_method = empty( $code_challenge_method ) ? 'plain' : $code_challenge_method; + if ( ! in_array( strtolower( $code_challenge_method ), [ 'plain', 's256' ], true ) ) { + return new WP_Error( + 'oauth2.types.authorization_code.handle_pkce.wrong_challenge_method', + __( 'Challenge method must be S256 or plain', 'oauth2' ), $client_id, + [ + 'status' => WP_Http::BAD_REQUEST, + 'client_id' => $client_id, + ] + ); } - return false; + return [ 'code_challenge' => $code_challenge, 'code_challenge_method' => $code_challenge_method ]; } } diff --git a/inc/utilities/class-oauth2-wp-cli.php b/inc/utilities/class-command.php similarity index 55% rename from inc/utilities/class-oauth2-wp-cli.php rename to inc/utilities/class-command.php index 8c864a0..4d5a7fe 100644 --- a/inc/utilities/class-oauth2-wp-cli.php +++ b/inc/utilities/class-command.php @@ -1,8 +1,13 @@ 128 ) { - \WP_CLI::error( 'Length should be >= 43 and <= 128.' ); + WP_CLI::error( 'Length should be >= 43 and <= 128.' ); } if ( ! empty( $args[0] ) ) { $random_seed = $args[0]; if ( strlen( $random_seed ) < 43 || strlen( $random_seed ) > 128 ) { - \WP_CLI::error( 'Length of the provided random seed should be >= 43 and <= 128.' ); + WP_CLI::error( 'Length of the provided random seed should be >= 43 and <= 128.' ); } - \WP_CLI::warning( "Using provided string {$random_seed} as a random seed. It is recommended to use this command without parameters, 64 characters long random key will be generated automatically." ); + WP_CLI::warning( "Using provided string {$random_seed} as a random seed. It is recommended to use this command without parameters, 64 characters long random key will be generated automatically." ); } else { $is_strong_crypto = true; $random_seed = \bin2hex( \openssl_random_pseudo_bytes( $length / 2 + $length % 2, $is_strong_crypto ) ); $random_seed = \substr( $random_seed, 0, $length ); if ( ! $is_strong_crypto ) { - \WP_CLI::error( 'openssl_random_pseudo_bytes failed to generate a cryptographically strong random string.' ); + WP_CLI::error( 'openssl_random_pseudo_bytes failed to generate a cryptographically strong random string.' ); } } - $code_challenge = \base64_encode( hash( 'sha256', $random_seed ) ); + $code_challenge = base64_encode( hash( 'sha256', $random_seed ) ); $items = [ [ @@ -62,6 +63,6 @@ function generate_code_challenge( $args, $assoc_args ) { ], ]; - \WP_CLI\Utils\format_items( 'table', $items, [ 'code_verifier', 'code_challenge = base64( sha256( code_verifier ) )' ] ); + Utils\format_items( 'table', $items, [ 'code_verifier', 'code_challenge = base64( sha256( code_verifier ) )' ] ); } } diff --git a/plugin.php b/plugin.php index 48caf6a..d47ce0d 100644 --- a/plugin.php +++ b/plugin.php @@ -29,7 +29,7 @@ require __DIR__ . '/inc/admin/profile/namespace.php'; if ( defined( 'WP_CLI' ) && WP_CLI ) { - require __DIR__ . '/inc/utilities/class-oauth2-wp-cli.php'; + require __DIR__ . '/inc/utilities/class-command.php'; } bootstrap();