Skip to content

Comments

WP > Middleware Auth#4

Merged
faisal-alvi merged 3 commits intofeature/issue-1-plugin-setupfrom
feature/wp-middleware-auth
Mar 6, 2025
Merged

WP > Middleware Auth#4
faisal-alvi merged 3 commits intofeature/issue-1-plugin-setupfrom
feature/wp-middleware-auth

Conversation

@TylerB24890
Copy link
Collaborator

@TylerB24890 TylerB24890 commented Mar 4, 2025

Description of the Change

  • Changes the /token/ endpoint to /auth/
  • Creates a new /token/ endpoint for authenticating the WP token.
  • Send all responses as JSON using wp_send_json_* functions
  • Send WP token to middleware

NOTE: Requires #3

How to test the Change

Changelog Entry

Added - New feature
Changed - Existing functionality
Deprecated - Soon-to-be removed feature
Removed - Feature
Fixed - Bug fix
Security - Vulnerability
Developer - Non-functional update

Credits

Props @username, @username2, ...

Checklist:

@TylerB24890 TylerB24890 marked this pull request as ready for review March 4, 2025 14:44
self::$route,
[
'methods' => WP_REST_Server::CREATABLE,
'callback' => [ $this, 'validate_token' ],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need a permission_callback here


if ( empty( $token ) || empty( $refresh_token ) ) {
return new WP_Error( 'missing_tokens', 'Token and Refresh Token are required.', [ 'status' => 400 ] );
wp_send_json_error( [ 'message' => 'Token and Refresh Token are required.' ], 400 );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we pass this message through translation?

Copy link
Contributor

@faisal-alvi faisal-alvi left a comment

Choose a reason for hiding this comment

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

@TylerB24890 Thanks for the PR! Darin already covered the code review, everything else LGTM. However, I noticed that the token is stored for only 5 minutes, which may be too short. Depending on the use case, we might want to consider extending the expiration time?

@faisal-alvi faisal-alvi merged commit 19d776a into feature/issue-1-plugin-setup Mar 6, 2025
@faisal-alvi faisal-alvi deleted the feature/wp-middleware-auth branch March 6, 2025 08:19
@jeffpaul jeffpaul added this to the 1.0.0 milestone Mar 24, 2025
@github-project-automation github-project-automation bot moved this to Backlog in 10up Jobber Mar 24, 2025
@jeffpaul jeffpaul moved this from Backlog to Merged in 10up Jobber Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

4 participants