Repository navigation
Show application approval status in the admin and on authorize errors - #91
Merged
Merged
Conversation
Unapproved clients are saved as drafts, and get_by_id() only finds published posts. An unapproved app therefore got the same "Client ID is invalid" 400 as a typo, which gave the developer no hint that approval was the missing step. Add Client::get_unapproved_by_id(), matching draft and pending posts only, never trash. Both lookups now share one private query helper, and get_by_id() behaves as before. Base::handle_authorisation() uses the new lookup to return a distinct unapproved_client error with a 403 status. Unknown client IDs still get invalid_client_id. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Every error from the authorize endpoint went to wp_die(), which sends HTTP 500 and a bare message, and was marked as a TODO. The error status (such as 400 or 403) was ignored. Render the error with login_header() and login_footer() instead, using the status from the error data and defaulting to 400. This code runs inside wp-login.php's login_form_* hook, so the login helpers are loaded. A link back to the site home gives the user somewhere to go. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Admins could only tell an app was unapproved from the draft state label next to its name. Add an Approved column that prints Yes or No based on whether the post is published. Extract the approve URL into get_approve_url() so the list table and the upcoming edit screen build the same nonce-protected link. Also escape the edit link in column_name(), which was printed raw. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
An unapproved app looked the same as an approved one on its edit screen, and approving meant going back to the list. Print a success or warning notice above the form for existing clients. Admins who can publish also get an Approve button. Other users are told to ask an administrator. The button adds return=edit to the approve URL. handle_approve() then sends the user back to the edit screen with an "Application approved." message instead of the list. The list's approve action is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Cover Client::get_unapproved_by_id() for draft, published and trashed clients, the 403 unapproved_client error versus the 400 invalid_client_id error from the authorize step, and the Approved column and approval status notice in the admin. Add a create_unapproved_client() helper to the shared test case. create_client() now builds on it and approves the result. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The Approve button and handler accepted any status, so a trashed app could be approved and published again. handle_approve() also fataled on a missing post and never checked the post type. Add Client::UNAPPROVED_STATUSES as the one definition of "unapproved", used by get_unapproved_by_id(), render_approval_status() and handle_approve(). The status notice now prints nothing for trash, private, future and auto-draft posts. handle_approve() dies with a 400 unless the post exists, is a client and is draft or pending. The edit screen also rejects posts of another type. Drop the "Application approved." message and the did_action param after approving from the edit screen. The status notice changing to "This application is approved." is the feedback, so the message showed the same news twice. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
login_header() prints error messages as HTML. Messages such as "Client ID %s is invalid." include request input, so pass login_header() a copy of the error with every message run through esc_html(). login_footer() already prints a back-to-site link, so remove the second one. Move the status lookup into get_error_status() so it can be tested without the exit in render_error(). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
wp_trash_post() renames the slug to "<id>__trashed", so the trashed-client lookup test passed because the slug no longer matched, not because trash was excluded. Set the status with wp_update_post() so the slug stays. Grant super admin in the admin tests when running on multisite, where edit_users needs it. Add tests that the approval status prints nothing for a trashed client, and for the error status lookup (403 for unapproved_client, 400 by default). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The row action showed for every status except publish, so trashed clients got an Approve link that handle_approve() now refuses. Use Client::UNAPPROVED_STATUSES, as the edit screen and the handler do. Add a test that handle_approve() dies and leaves a trashed client in the trash. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
joehoyle
reviewed
Oct 26, 2026
A separate get_unapproved_by_id() duplicated the lookup for one caller. Give get_by_id() an $include_unapproved flag instead. The default still matches published clients only, so the token endpoint and other callers are unchanged. With the flag set it also matches draft and pending clients, and trashed clients never match. Add Client::is_approved(). Base::handle_authorisation() now looks the client up once with the flag set, returns invalid_client_id if there is none, and unapproved_client (403) if the client is not approved. The private status helper is folded into get_by_id() since nothing else needs it. approve() now refreshes the cached post, so is_approved() is true on the same instance straight after approving. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
create_client() keeps its name and gains an $approve argument that defaults to true. Tests that need a draft client pass false, so the extra create_unapproved_client() helper goes away. Replace the get_unapproved_by_id() tests with get_by_id( $id, true ) tests for draft and trashed clients, and add is_approved() tests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
joehoyle
approved these changes
Oct 26, 2026
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.
Show whether an application is approved in the admin, and return a clear error from authorize when it is not.
A new application does not work until someone approves it, and nothing in the UI said so. A user who hit authorize got "Client ID … is invalid." from a bare
wp_die, with HTTP status 500.Edit screen. It now shows an approval status notice. An unapproved app gets a warning. Users who can approve also get an Approve button. Other users are told to ask an administrator. Approving returns to the edit screen.
List table. It has a new Approved column with Yes or No. The Approve row action only shows for draft or pending apps.
Authorize with an unapproved app. It now returns
unapproved_clientwith status 403. The message is "This application is not currently approved. Ask the site administrator to approve it."Authorize error rendering. All authorize errors now render on the
wp-login.phperror page. They use their real status code, 400 by default, instead of the 500 fromwp_die. Messages are escaped.Approve handler. It now refuses anything that is not a draft or pending
oauth2_clientpost. Before, a missing ID caused a fatal error. It could also publish a trashed client or a post of another type. The edit screen also rejects posts that are not clients.New tests cover all of this. The single-site and multisite PHPUnit suites both pass.
Side effect: authorize errors now pass through core's
login_errorsfilter.🤖 Generated with Claude Code