Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register OAuth Rewrite Path #130

Merged
merged 3 commits into from May 14, 2024
Merged

Register OAuth Rewrite Path #130

merged 3 commits into from May 14, 2024

Conversation

akirk
Copy link
Owner

@akirk akirk commented May 4, 2024

When a post exists on the WordPress site that includes the word "authorize", WordPress redirects there instead of letting EMA handle the path.

This adds the necessary rewrite routing for this. In the course I fixed two more bugs:

  • We always flushed the rewrite on every request because we were not properly checking for the registered route (added the '^'
  • We removed link headers from Gutenberg REST requests, this now limits that link removal to the prev and next as intended and also only for our own REST requests.

Fixes #129.

@akirk akirk requested a review from pfefferle May 4, 2024 05:11
@akirk
Copy link
Owner Author

akirk commented May 4, 2024

The tests fail because WordPress redirects to oauth/authorize/. The flow works (i.e. fixes the issue described in #129) for clients that support and follow those redirects but the test expects oauth/authorize to be served without a redirect. I think it's better for compatibility to avoid the redirect.

The responsible code is here in core: https://github.com/WordPress/wordpress-develop/blob/a4123728068e09200d7dfe2d5a88f803c1adf2aa/src/wp-includes/canonical.php#L695-L697

Which calls https://github.com/WordPress/wordpress-develop/blob/a4123728068e09200d7dfe2d5a88f803c1adf2aa/src/wp-includes/class-wp-query.php#L4345-L4348

And then https://github.com/WordPress/wordpress-develop/blob/a4123728068e09200d7dfe2d5a88f803c1adf2aa/src/wp-includes/class-wp-query.php#L1029-L1033

So rest routes get special treatment, I think this is what we need to convert this to.

@akirk
Copy link
Owner Author

akirk commented May 4, 2024

I found another solution, so I think this is ready.

@akirk akirk merged commit ea7f997 into main May 14, 2024
18 checks passed
@akirk akirk deleted the add-oauth-rewrite branch May 14, 2024 04:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default WordPress Redirect Behavior Interrupts OAuth Dance
1 participant