Return RFC 6749 error fields from the token endpoint - #89
Open
roborourke wants to merge 2 commits into
Open
roborourke wants to merge 2 commits into
roborourke wants to merge 2 commits into
Conversation
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 WP-API#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 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Dispatched token errors are not formatted, and server_error can retain an incorrect status.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
Updates token endpoint errors with RFC 6749 OAuth fields while preserving WordPress fields, status handling, and authentication challenges.
Changes:
- Maps token errors to OAuth error codes.
- Adds RFC-aligned statuses and Basic authentication challenges.
- Expands token endpoint and header tests.
| File | Summary |
|---|---|
tests/test-www-authenticate.php |
Tests Basic authentication challenge behavior. |
tests/test-token-endpoint.php |
Tests OAuth error responses. |
inc/endpoints/namespace.php |
Registers token endpoint hooks. |
inc/endpoints/class-token.php |
Formats OAuth errors and statuses; contains unresolved response-hook and server_error status issues. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Token endpoint errors now include the
erroranderror_descriptionfields from RFC 6749 section 5.2.Before this change, errors used the WordPress shape only:
code,messageanddata. A standard OAuth client looks for a top-levelerrorlikeinvalid_grant, so it could not tell why a request failed. The WordPress fields stay, so clients that readcodekeep working.Each known error maps to an OAuth code. For example, an unknown client gives
invalid_client, an expired code givesinvalid_grant, and an unknowngrant_typegivesunsupported_grant_type. An error can also set its own code with anerrorkey in its data. The PKCE errors in #85 already do this, so they will be covered once both PRs merge. Unknown errors becomeserver_error.Errors now use status 400, as the RFC asks. The exceptions are
server_error, which stays 500, and a failed client login, which stays 401. That 401 now sends aWWW-Authenticate: Basicheader.Copilot flagged this on #85. It predates that PR, so the fix is here.
🤖 Generated with Claude Code