From d53e3c48501242452f334358002e76783dea5293 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Wed, 23 Sep 2026 13:03:15 +0100 Subject: [PATCH 1/2] Return RFC 6749 error fields from the token endpoint RFC 6749 section 5.2 says a token endpoint error is a JSON object with a top-level "error" code such as invalid_grant, plus an optional "error_description". The endpoint returned the WordPress error shape instead ({"code","message","data"}), so a standard OAuth client could not tell why a request failed. A rest_request_after_callbacks filter on the token route now adds "error" and "error_description" to every error. That filter also sees argument validation errors, such as an unknown grant_type, which happen before the endpoint callback runs. rest_post_dispatch would work in production too, but WP_REST_Server::dispatch() does not run it, so it would be untested. The WordPress fields are kept alongside the new ones, so existing clients that read "code" keep working. Replacing the body outright would be closer to the RFC but would break them. Error codes come from a map of the endpoint's known WP_Error codes. An "error" key in the WP_Error data takes precedence, so new errors can set their OAuth code where they are created. The PKCE errors in #85 already do this. Unknown errors become server_error with a 500 status. Every other error is sent as a 400, except invalid_client, which keeps its 401 when the client failed to authenticate. A 401 now carries a WWW-Authenticate: Basic challenge, as section 5.2 requires. The resource server's Bearer challenge still stays off this route, so the WWW-Authenticate test now checks for the Basic challenge instead of no header. Co-Authored-By: Claude Opus 5.5 --- inc/endpoints/class-token.php | 85 ++++++++++++++++++ inc/endpoints/namespace.php | 1 + tests/test-token-endpoint.php | 152 ++++++++++++++++++++++++++++++++ tests/test-www-authenticate.php | 4 +- 4 files changed, 240 insertions(+), 2 deletions(-) diff --git a/inc/endpoints/class-token.php b/inc/endpoints/class-token.php index 6be1831..c0688d8 100644 --- a/inc/endpoints/class-token.php +++ b/inc/endpoints/class-token.php @@ -11,10 +11,32 @@ use WP_Http; use WP\OAuth2; use WP_REST_Request; +use WP_REST_Response; + /** * Token endpoint handler. */ class Token { + const ROUTE = '/oauth2/access_token'; + + /** + * RFC 6749 section 5.2 error codes for the errors this endpoint returns. + */ + const OAUTH_ERRORS = [ + 'rest_missing_callback_param' => 'invalid_request', + 'rest_invalid_param' => 'invalid_request', + 'oauth2.endpoints.token.invalid_request' => 'invalid_request', + 'oauth2.endpoints.token.exchange_token.invalid_client' => 'invalid_client', + 'oauth2.endpoints.token.invalid_client' => 'invalid_client', + 'oauth2.client.check_authorization_code.invalid_code' => 'invalid_grant', + 'oauth2.tokens.authorization_code.validate.expired' => 'invalid_grant', + 'oauth2.tokens.authorization_code.get_user.invalid_data' => 'invalid_grant', + ]; + + public function register_hooks() { + add_filter( 'rest_request_after_callbacks', [ $this, 'format_error_response' ], 10, 3 ); + } + public function register_routes() { register_rest_route( 'oauth2', @@ -49,6 +71,69 @@ public function register_routes() { ); } + /** + * Add the RFC 6749 section 5.2 error fields to a token endpoint error. + * + * The WordPress `code`, `message` and `data` fields are kept alongside + * `error` and `error_description`. + * + * @param WP_REST_Response|WP_Error|mixed $response Result of the request. + * @param array $handler Route handler. + * @param WP_REST_Request $request Request object. + * @return WP_REST_Response|mixed Formatted error response, or the original response. + */ + public function format_error_response( $response, $handler, $request ) { + if ( ! is_wp_error( $response ) || ! $request instanceof WP_REST_Request || static::ROUTE !== $request->get_route() ) { + return $response; + } + + $error = $this->get_oauth_error( $response ); + $error_data = $response->get_error_data(); + $status = is_array( $error_data ) && isset( $error_data['status'] ) ? (int) $error_data['status'] : WP_Http::INTERNAL_SERVER_ERROR; + + if ( 'server_error' !== $error && ! ( 'invalid_client' === $error && WP_Http::UNAUTHORIZED === $status ) ) { + $status = WP_Http::BAD_REQUEST; + } + + $formatted = rest_convert_error_to_response( $response ); + $data = $formatted->get_data(); + + $data['error'] = $error; + $data['error_description'] = $response->get_error_message(); + + $formatted->set_data( $data ); + $formatted->set_status( $status ); + + if ( WP_Http::UNAUTHORIZED === $status ) { + $formatted->header( 'WWW-Authenticate', 'Basic realm="oauth2"' ); + } + + return $formatted; + } + + /** + * Get the RFC 6749 section 5.2 error code for an error. + * + * An `error` key in the error data wins over the built-in map. Unknown + * errors become `server_error`. + * + * @param WP_Error $error Error returned by the endpoint. + * @return string OAuth error code. + */ + protected function get_oauth_error( WP_Error $error ) { + $data = $error->get_error_data(); + if ( is_array( $data ) && ! empty( $data['error'] ) && is_string( $data['error'] ) ) { + return $data['error']; + } + + $code = $error->get_error_code(); + if ( 'rest_invalid_param' === $code && isset( $data['params']['grant_type'] ) ) { + return 'unsupported_grant_type'; + } + + return static::OAUTH_ERRORS[ $code ] ?? 'server_error'; + } + /** * Validates the given grant type. * diff --git a/inc/endpoints/namespace.php b/inc/endpoints/namespace.php index 08d9445..53679ca 100644 --- a/inc/endpoints/namespace.php +++ b/inc/endpoints/namespace.php @@ -17,6 +17,7 @@ function register() { $token_endpoint = new Token(); $token_endpoint->register_routes(); + $token_endpoint->register_hooks(); // Register convenience URL. register_rest_route( diff --git a/tests/test-token-endpoint.php b/tests/test-token-endpoint.php index e4e955a..c2dcd5b 100644 --- a/tests/test-token-endpoint.php +++ b/tests/test-token-endpoint.php @@ -13,7 +13,9 @@ use WP\OAuth2\Endpoints\Token; use WP\OAuth2\Tokens\Access_Token; use WP\OAuth2\Tokens\Authorization_Code; +use WP_Error; use WP_REST_Request; +use WP_REST_Response; use WP_REST_Server; /** @@ -415,4 +417,154 @@ public function test_validate_grant_type_rejects_unknown() { $this->assertFalse( $handler->validate_grant_type( 'implicit' ) ); $this->assertFalse( $handler->validate_grant_type( '' ) ); } + + // ------------------------------------------------------------------------- + // RFC 6749 section 5.2 error responses + // ------------------------------------------------------------------------- + + /** + * Dispatch a token request and return the response. + * + * @param array $params Request parameters. + * @return WP_REST_Response + */ + protected function request_token( array $params ) { + $request = new WP_REST_Request( 'POST', '/oauth2/access_token' ); + foreach ( $params as $key => $value ) { + $request->set_param( $key, $value ); + } + + return $this->server->dispatch( $request ); + } + + /** + * Assert a response is an RFC 6749 section 5.2 error that keeps the WordPress fields. + * + * @param WP_REST_Response $response Response to check. + * @param string $error Expected OAuth error code. + * @param int $status Expected HTTP status. + */ + protected function assertOAuthError( WP_REST_Response $response, $error, $status ) { + $data = $response->get_data(); + + $this->assertSame( $status, $response->get_status() ); + $this->assertSame( $error, $data['error'] ); + $this->assertSame( $data['message'], $data['error_description'] ); + $this->assertNotEmpty( $data['code'] ); + } + + public function test_missing_parameter_is_invalid_request() { + $response = $this->request_token( [ + 'grant_type' => 'authorization_code', + 'code' => 'somecode', + ] ); + + $this->assertOAuthError( $response, 'invalid_request', 400 ); + $this->assertSame( 'rest_missing_callback_param', $response->get_data()['code'] ); + } + + public function test_missing_grant_type_is_invalid_request() { + $this->assertOAuthError( $this->request_token( [] ), 'invalid_request', 400 ); + } + + public function test_unknown_grant_type_is_unsupported_grant_type() { + $response = $this->request_token( [ 'grant_type' => 'password' ] ); + + $this->assertOAuthError( $response, 'unsupported_grant_type', 400 ); + } + + public function test_unknown_client_is_invalid_client() { + $response = $this->request_token( [ + 'grant_type' => 'authorization_code', + 'client_id' => 'nonexistent-client', + 'code' => 'anycode', + ] ); + + $this->assertOAuthError( $response, 'invalid_client', 400 ); + } + + public function test_unknown_code_is_invalid_grant() { + $response = $this->request_token( [ + 'grant_type' => 'authorization_code', + 'client_id' => $this->client->get_id(), + 'code' => 'invalid-code-xyz', + ] ); + + $this->assertOAuthError( $response, 'invalid_grant', 400 ); + } + + public function test_expired_code_is_invalid_grant() { + $user = $this->factory->user->create_and_get(); + $code = Authorization_Code::create( $this->client, $user ); + + $meta_key = Authorization_Code::KEY_PREFIX . $code->get_code(); + $value = get_post_meta( $this->client->get_post_id(), $meta_key, true ); + $value['expiration'] = time() - 1; + update_post_meta( $this->client->get_post_id(), $meta_key, $value ); + + $response = $this->request_token( [ + 'grant_type' => 'authorization_code', + 'client_id' => $this->client->get_id(), + 'code' => $code->get_code(), + ] ); + + $this->assertOAuthError( $response, 'invalid_grant', 400 ); + } + + public function test_failed_client_authentication_is_invalid_client_with_challenge() { + $response = $this->request_token( [ + 'grant_type' => 'client_credentials', + 'client_id' => 'nonexistent', + 'client_secret' => 'wrong', + ] ); + + $this->assertOAuthError( $response, 'invalid_client', 401 ); + $this->assertSame( 'Basic realm="oauth2"', $response->get_headers()['WWW-Authenticate'] ); + } + + public function test_missing_client_credentials_is_invalid_request() { + $response = $this->request_token( [ 'grant_type' => 'client_credentials' ] ); + + $this->assertOAuthError( $response, 'invalid_request', 400 ); + } + + public function test_error_data_can_set_the_oauth_error() { + $handler = new Token(); + $request = new WP_REST_Request( 'POST', '/oauth2/access_token' ); + $response = $handler->format_error_response( + new WP_Error( 'custom', 'Scope not allowed.', [ 'error' => 'invalid_scope' ] ), + [], + $request + ); + + $this->assertOAuthError( $response, 'invalid_scope', 400 ); + } + + public function test_unknown_error_is_server_error() { + $handler = new Token(); + $request = new WP_REST_Request( 'POST', '/oauth2/access_token' ); + $response = $handler->format_error_response( new WP_Error( 'custom', 'Broken.' ), [], $request ); + + $this->assertOAuthError( $response, 'server_error', 500 ); + } + + public function test_errors_on_other_routes_are_left_alone() { + $handler = new Token(); + $error = new WP_Error( 'custom', 'Broken.' ); + + $this->assertSame( $error, $handler->format_error_response( $error, [], new WP_REST_Request( 'GET', '/wp/v2/posts' ) ) ); + } + + public function test_successful_response_has_no_error_fields() { + $user = $this->factory->user->create_and_get(); + $code = Authorization_Code::create( $this->client, $user ); + $response = $this->request_token( [ + 'grant_type' => 'authorization_code', + 'client_id' => $this->client->get_id(), + 'code' => $code->get_code(), + ] ); + + $this->assertSame( 200, $response->get_status() ); + $this->assertArrayNotHasKey( 'error', $response->get_data() ); + } } diff --git a/tests/test-www-authenticate.php b/tests/test-www-authenticate.php index 1a7c318..c3d0e2b 100644 --- a/tests/test-www-authenticate.php +++ b/tests/test-www-authenticate.php @@ -114,7 +114,7 @@ public function test_challenge_is_not_added_to_a_forbidden_response() { * The token endpoint is the authorization server, not a resource it * protects, so it must not point clients back at resource metadata. */ - public function test_challenge_is_not_added_to_the_token_endpoint() { + public function test_bearer_challenge_is_not_added_to_the_token_endpoint() { $request = new WP_REST_Request( 'POST', '/oauth2/access_token' ); $request->set_param( 'grant_type', 'client_credentials' ); $request->set_param( 'client_id', 'nonexistent' ); @@ -123,7 +123,7 @@ public function test_challenge_is_not_added_to_the_token_endpoint() { $response = $this->dispatch( $request ); $this->assertEquals( 401, $response->get_status() ); - $this->assertNull( $this->get_challenge( $response ) ); + $this->assertEquals( 'Basic realm="oauth2"', $this->get_challenge( $response ) ); } public function test_an_existing_challenge_is_not_overwritten() { From eec077da17e213ab404f441dd7687db7f3b11878 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Wed, 23 Sep 2026 13:17:28 +0100 Subject: [PATCH 2/2] Keep server_error at 500 and map unknown 4xx errors to invalid_request An unknown error that carried a 4xx status became server_error but kept its 4xx status, so the response said "server error" with a client-error status. Unknown errors now pick their OAuth code from the status: a 4xx becomes invalid_request with a 400, and anything else becomes server_error. server_error is always sent as a 500, including when an error sets it through its data. Forcing every unknown error to 500 was the other option, but an error that already says it is the client's fault should not be reported as a server failure. Co-Authored-By: Claude Opus 5.5 --- inc/endpoints/class-token.php | 15 ++++++++++++--- tests/test-token-endpoint.php | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/inc/endpoints/class-token.php b/inc/endpoints/class-token.php index c0688d8..66dd876 100644 --- a/inc/endpoints/class-token.php +++ b/inc/endpoints/class-token.php @@ -91,7 +91,9 @@ public function format_error_response( $response, $handler, $request ) { $error_data = $response->get_error_data(); $status = is_array( $error_data ) && isset( $error_data['status'] ) ? (int) $error_data['status'] : WP_Http::INTERNAL_SERVER_ERROR; - if ( 'server_error' !== $error && ! ( 'invalid_client' === $error && WP_Http::UNAUTHORIZED === $status ) ) { + if ( 'server_error' === $error ) { + $status = WP_Http::INTERNAL_SERVER_ERROR; + } elseif ( ! ( 'invalid_client' === $error && WP_Http::UNAUTHORIZED === $status ) ) { $status = WP_Http::BAD_REQUEST; } @@ -115,7 +117,8 @@ public function format_error_response( $response, $handler, $request ) { * Get the RFC 6749 section 5.2 error code for an error. * * An `error` key in the error data wins over the built-in map. Unknown - * errors become `server_error`. + * errors become `invalid_request` if they carry a 4xx status, or + * `server_error` otherwise. * * @param WP_Error $error Error returned by the endpoint. * @return string OAuth error code. @@ -131,7 +134,13 @@ protected function get_oauth_error( WP_Error $error ) { return 'unsupported_grant_type'; } - return static::OAUTH_ERRORS[ $code ] ?? 'server_error'; + if ( isset( static::OAUTH_ERRORS[ $code ] ) ) { + return static::OAUTH_ERRORS[ $code ]; + } + + $status = is_array( $data ) && isset( $data['status'] ) ? (int) $data['status'] : WP_Http::INTERNAL_SERVER_ERROR; + + return $status >= 400 && $status < 500 ? 'invalid_request' : 'server_error'; } /** diff --git a/tests/test-token-endpoint.php b/tests/test-token-endpoint.php index c2dcd5b..7b3f14a 100644 --- a/tests/test-token-endpoint.php +++ b/tests/test-token-endpoint.php @@ -548,6 +548,26 @@ public function test_unknown_error_is_server_error() { $this->assertOAuthError( $response, 'server_error', 500 ); } + public function test_unknown_client_error_is_invalid_request() { + $handler = new Token(); + $request = new WP_REST_Request( 'POST', '/oauth2/access_token' ); + $response = $handler->format_error_response( new WP_Error( 'custom', 'Bad.', [ 'status' => 401 ] ), [], $request ); + + $this->assertOAuthError( $response, 'invalid_request', 400 ); + } + + public function test_server_error_is_always_500() { + $handler = new Token(); + $request = new WP_REST_Request( 'POST', '/oauth2/access_token' ); + $response = $handler->format_error_response( + new WP_Error( 'custom', 'Broken.', [ 'status' => 400, 'error' => 'server_error' ] ), + [], + $request + ); + + $this->assertOAuthError( $response, 'server_error', 500 ); + } + public function test_errors_on_other_routes_are_left_alone() { $handler = new Token(); $error = new WP_Error( 'custom', 'Broken.' );