indieweb / wordpress-micropub

A Micropub Endpoint plugin for WordPress
https://wordpress.org/plugins/micropub
51 stars 12 forks source link

Micropub Endpoint Returns 403 Response #235

Closed Changelingmx closed 4 years ago

Changelingmx commented 4 years ago

I noticed this morning that my micropub endpoint has started returning a 403 response. I tried signing out and back in, and now my syndication targets are gone, and I'm still getting the 403 response. how can I fix this?

dshanske commented 4 years ago

What is the result of running the site health test? Also what is the website and hosting provider?

Changelingmx commented 4 years ago

The health test reports good health. The site is www.starshipchangeling.net http://www.starshipchangeling.net . The hosting provider is Gandi.

From: David Shanske notifications@github.com Sent: Thursday, September 3, 2020 12:39 PM To: indieweb/wordpress-micropub wordpress-micropub@noreply.github.com Cc: Randy Reed randy@starshipchangeling.net; Author author@noreply.github.com Subject: Re: [indieweb/wordpress-micropub] Micropub Endpoint Returns 403 Response (#235)

What is the result of running the site health test? Also what is the website and hosting provider?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/indieweb/wordpress-micropub/issues/235#issuecomment-686613948 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ALREWCDLU2LJPLWE6PLOC4TSD7BDZANCNFSM4QUY6REA .

dshanske commented 4 years ago

I'll try some things. I know some providers have configurations that cause issues. Could or could not be that. There is a manual test I can run externally against a site.

dshanske commented 4 years ago

curl -i -H 'Authorization: Bearer TOKEN' 'https://example.com/wp-json/indieauth/1.0/token and check the response

Changelingmx commented 4 years ago

I don’t understand what you’re asking me to do.

From: David Shanske notifications@github.com Sent: Thursday, September 3, 2020 1:25 PM To: indieweb/wordpress-micropub wordpress-micropub@noreply.github.com Cc: Randy Reed randy@starshipchangeling.net; Author author@noreply.github.com Subject: Re: [indieweb/wordpress-micropub] Micropub Endpoint Returns 403 Response (#235)

curl -i -H 'Authorization: Bearer TOKEN' 'https://example.com/wp-json/indieauth/1.0/token and check the response

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/indieweb/wordpress-micropub/issues/235#issuecomment-686638438 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ALREWCBQVTE55IFFLJKSFSDSD7GODANCNFSM4QUY6REA .

dshanske commented 4 years ago

I'm going to do it when I get a chance

Changelingmx commented 4 years ago

Okay.

From: David Shanske notifications@github.com Sent: Thursday, September 3, 2020 2:57 PM To: indieweb/wordpress-micropub wordpress-micropub@noreply.github.com Cc: Randy Reed randy@starshipchangeling.net; Author author@noreply.github.com Subject: Re: [indieweb/wordpress-micropub] Micropub Endpoint Returns 403 Response (#235)

I'm going to do it when I get a chance

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/indieweb/wordpress-micropub/issues/235#issuecomment-686695440 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ALREWCG7GUKIEFCKUBD3MUDSD7RHRANCNFSM4QUY6REA .

dshanske commented 4 years ago

Okay, your site isn't blocking the token, that's good.

dshanske commented 4 years ago

What Micropub client are you using?

Changelingmx commented 4 years ago

I'm still receiving the error. Where do we go from here?

Sent from my iPhone

On Sep 3, 2020, at 23:10, David Shanske notifications@github.com wrote:

 What Micropub client are you using?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Changelingmx commented 4 years ago

I use both Indiginous and quill.io. Both of them are returning the response.

Sent from my iPhone

On Sep 4, 2020, at 06:58, Randy Reed manitou711@gmail.com wrote:

I'm still receiving the error. Where do we go from here?

Sent from my iPhone

On Sep 3, 2020, at 23:10, David Shanske notifications@github.com wrote:

 What Micropub client are you using?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

jamesvandyne commented 4 years ago

@dshanske

I've started getting the same error message from quill and ownyourswarm using Wordpress 5.5.1. Tested with 5.4.2 and 5.5 and got the same errors.

Logs from Quill:

HTTP/1.1 403 Forbidden
Date: Sun, 06 Sep 2020 06:42:26 GMT
Server: Apache/2.4.29 (Ubuntu)
Vary: Accept-Encoding,Cookie
X-WP-DoingItWrong: register_rest_route (since 5.5.0; The REST API route definition for <code>webmention/1.0/endpoint</code> is missing the required <code>permission_callback</code> argument. For REST API routes that are intended to be public, use <code>__return_true</code> as the permission callback.)
X-Robots-Tag: noindex
Link: <https://jamesvandyne.com/wp-json/>; rel="https://api.w.org/"
X-Content-Type-Options: nosniff
Access-Control-Expose-Headers: X-WP-Total, X-WP-TotalPages, Link
Access-Control-Allow-Headers: Authorization, X-WP-Nonce, Content-Disposition, Content-MD5, Content-Type
Content-Length: 67
Content-Type: application/json; charset=UTF-8

{"code":"forbidden","message":"Unauthorized","data":{"status":403}}

While this error message indicates the issue is the permission_callback is wrong in the webmetions plugin, disable webmentions produced the same error, but with the wordpress-micropub. Simple location also suffers from the same bug.

Doing a bit of digging, I wonder if it may be related to the Wordpress REST API changes introduced in 5.5?

Changing class-micropub-endpoint.php#L90/L96 to 'permission_callback' => '__return_true' , got Quill and ownyourswarm working again, isn't a proper fix.

Edit:

I seem to recall in debugging all of this (but I can't find my logs) that the specific error from ownyourswarm, even after creating an new token was coming from indieauth, "'No User is Currently Set", even though a user should have been set, since I authorized it and it used to work.

Thank you.

Changelingmx commented 4 years ago

where would I go to make those changes?

Sent from my iPhone

On Sep 6, 2020, at 03:20, James Van Dyne notifications@github.com wrote:

 @dshanske

I've started getting the same error message from quill and ownyourswarm using Wordpress 5.5.1. Tested with 5.4.2 and 5.5 and got the same errors.

Logs from Quill:

HTTP/1.1 403 Forbidden Date: Sun, 06 Sep 2020 06:42:26 GMT Server: Apache/2.4.29 (Ubuntu) Vary: Accept-Encoding,Cookie X-WP-DoingItWrong: register_rest_route (since 5.5.0; The REST API route definition for webmention/1.0/endpoint is missing the required permission_callback argument. For REST API routes that are intended to be public, use __return_true as the permission callback.) X-Robots-Tag: noindex Link: https://jamesvandyne.com/wp-json/; rel="https://api.w.org/" X-Content-Type-Options: nosniff Access-Control-Expose-Headers: X-WP-Total, X-WP-TotalPages, Link Access-Control-Allow-Headers: Authorization, X-WP-Nonce, Content-Disposition, Content-MD5, Content-Type Content-Length: 67 Content-Type: application/json; charset=UTF-8

{"code":"forbidden","message":"Unauthorized","data":{"status":403}} While this error message indicates the issue is the permission_callback is wrong in the webmetions plugin, disable webmentions produced the same error, but with the wordpress-micropub. Simple location also suffers from the same bug.

Doing a bit of digging, I wonder if it may be related to the Wordpress REST API changes.

Changing class-micropub-endpoint.php#L90/L96 to 'permission_callback' => '__return_true' , got Quill and ownyourswarm working again. Though I doubt it's the appropriate fix, it may be a clue for proper fix.

Thank you.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

dshanske commented 4 years ago

Return true would make it work...but allow anyone to post to your website as that is where your credentials are checked. The webmention issue is because it is a public endpoint... but we'll fix that as it's just a warning

jamesvandyne commented 4 years ago

@dshanske

Right. I'm adding some more debugging information as I find it, so maybe it can help us solve the issue.

When I make a post (with all changes undone) I still get the 403 errors as before. My apache error log outputs the following message:

[Mon Sep 07 20:06:02.850696 2020] [php7:notice] [pid 12203] [client 173.230.155.197:37202] REST result: /micropub/1.0/endpoint: {"code":"forbidden","message":"Unauthorized","data":{"status":403}}(403) - [](User ID: 0)

Adding in debug statements so I can print-debug, and it's erroring it seems because the current user (User ID: 0) can't read around Line 108.

Looking at the headers from apache_request_headers, Quill is appropriately sending Bearer tokens.

Array\n(\n    [Host] => jamesvandyne.com\n    [Authorization] => Bearer <my_actual_token_here>\n    [Accept] => application/json\n    [Content-Length] => 162\n    [Content-Type] => application/x-www-form-urlencoded\n)\n

It seems odd that despite an Authorization token being presented, the current user is reporting as (User ID: 0). Shouldn't one expect this to be User ID: 1 or their site's main user?

dshanske commented 4 years ago

@jamesvandyne IndieAuth handles the authentication and Micropub just checks for whether it is logged in. Now, why it would work for some and not for others is something I have yet to figure out.

dshanske commented 4 years ago

0 means it isn't reporting as logged in. Do you have any other authentication plugins installed? Like Jetpack, which allows login using wordpress.com, or such? Could be conflict

jamesvandyne commented 4 years ago

I do have Jetpack installed. Let me try disabling it and testing again.

dshanske commented 4 years ago

I hate to assume Jetpack, but it's like the Swiss Army Knife of plugins... you would not believe how many times something I did conflicted with it..even though it shouldn't.

jamesvandyne commented 4 years ago

Disabled Jetpack and all worked normally. So the latest Jetpack introduced some incompatibilities somewhere.

dshanske commented 4 years ago

It gives me a place to start at least.

dshanske commented 4 years ago

I'll go install it on a test site and experiment

dshanske commented 4 years ago

Turned every setting off...and same problem on my test site. Going to try reading their code

dshanske commented 4 years ago

Still stumped on this one. Been looking for 3 hours.

jamesvandyne commented 4 years ago

I tested with the previous version of Jetpack (8.8.2) and it works. So it means that something in 8.9 broke it. Looking through the 8.8.2 -> 8.9 changeset and nothing obvious stands out. There was some changes to json-api-plugins-endpoint.php, but I don't think they would cause authentication issues, unless it's somehow filtering out the IndieAuth plugin. 🤔

dshanske commented 4 years ago

I looked as well at the code...they did move some things around...but it's a big codebase.

frankmeeuwsen commented 4 years ago

Hey all, I found this issue because I had the same problems with the Micropub plugin. Reading through the thread I saw the logs from @jamesvandyne and I have the same ones. I can confirm turning off Jetpack fixes the problem with Micropub. Like @dshanske says on the Indieweb chat: "Why is it always Jetpack?" I hope there can be a fix soon so Jetpack and Indieweb plugins don't conflict.

pfefferle commented 4 years ago

@kraftbj can you help?

kraftbj commented 4 years ago

Absolutely. Can you give me just a real quick rundown of how to reproduce?

dshanske commented 4 years ago

@kraftbj Turn off Jetpack, try to publish with Micropub...works. Turn it on, doesn't work. Somehow it is disrupting the user login by token.

pfefferle commented 4 years ago

And Jetpack has to be connected to WordPress.com...

kraftbj commented 4 years ago

I suppose more what's the quickest way to get setup and publish with Micropub?

pfefferle commented 4 years ago

Ah, install the micropub and indieauth plugin and then go to quill (https://quill.p3k.io/), follow the auth instructions and try to publish a new post.

janboddez commented 4 years ago

(Edited for clarity.)

I'm running Micropub alongside Jetpack in debug mode, i.e., disconnected from WP.com, and that works (latest version of both plugins).

May be an option for those that don't need the WP.com-specific stuff (i.e., use something other than Publicize for POSSE'ing, and so on). If so, just add define('JETPACK_DEV_DEBUG', true); to wp-config.php.

(Note: You'll see a "Missing blog token" error under Site Health. The Jetpack team is aware but doesn't seem to care too much. It's safe to just ignore.)

Changelingmx commented 4 years ago

I'm also using the latest versions, but it is not working. This is still a problem.

Sent from my iPhone

On Sep 13, 2020, at 09:30, Jan Boddez notifications@github.com wrote:

 I'm using Micropub and Jetpack (latest versions for both) and it works.

I am, however, running Jetpack in debug mode, i.e., disconnected from WP.com. Which means the CDN and Protect modules (and a bunch of other things, like Publicize) are inactive, but most other things I actually use (Markdown, Sharing, Social Menus, the Portfolio Custom Post Type) just work.

May be an option for those that don't need the WP.com-specific stuff (i.e., use something other than Publicize for POSSE'ing, and so on). If so, just add define('JETPACK_DEV_DEBUG', true); to wp-config.php. (Note: You'll see a "Missing blog token" error under Site Health. The Jetpack team is aware but doesn't seem to care too much. It's safe to just ignore.)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

dshanske commented 4 years ago

Still stumped on this issue... but disabling Jetpack does work

janboddez commented 4 years ago

Just took JP out of debug mode, and it worked OK as long as I didn't couple it to WP.com. (The PHP constant I mentioned earlier just gets rid of the nag screen. Anyway.) Then connected JP and I'm now seeing the same error.

Given that user_logged_in() in Micropub_Endpoint::load_auth() returns false and the user ID's apparently 0, I think we can assume the issue really is with IndieAuth. (I mean, Jetpack is interfering with something IndieAuth's doing.)

Haven't looked too close, but on my setup, IndieAuth's determine_current_user callback function is never called, it seems, while Jetpack's callback (somwhere inside jetpack/vendor/automattic/jetpack-connection/src/class-rest-authentication.php, which was apparently introduced in v8.9) definitely is. (It's returning null really quickly, though, and runs at prio 10, so I was assuming IndieAuth's function would pick up from there.)

But, then again, maybe all of this has nothing to do with it. If I simply add return 1; as the very first thing inside JP's "determine_current_user," I get a "Cookie auth not allowed" or something error, which means it at least made it farther than what I think caused the initial 403. Same thing happens if I comment out the login check in load_auth().

That last bit seems to indicate indieauth_response is null, while it's also supposed to be set by the determine_current_user method of the IndieAuth plugin.

I'm not seeing anything in Jetpack that unhooks other plugins' callbacks, though. Also, even if I skip Jetpack's filter, same thing: 403. (I mean, I still think IndieAuth's filter isn't called, but it doesn't seem to be related to this part of JP. That said, can't rule out a prio/race condition thing, especially if this part was recently rewritten on their end.)

(Update: These last few paragraphs are obviously on my experimenting with the IndieAuth plugin, not Micropub.)

dshanske commented 4 years ago

I looked at the determine_current_user filters also. And I looked at both sets of code. Going to keep trying to figure it out, but there is something we're missing.

janboddez commented 4 years ago

I'm now thinking IndieAuth's IndieAuth_Local_Authorize::load() is simply called too late, i.e., after the current user's already been determined, courtesy of Jetpack. And that the filter only runs when the user is set but not when it looks for whether the user's logged in.

Like, Jetpack's presence forces WP to set the user before the "API endpoint" logic even runs, or at least before above class is loaded. And it sets the user ID to 0 (because our token logic hasn't yet run), and the filter is never again called (which is why it will never run).

Update: Or, maybe not. I hadn't really looked at WP's source too well. user.php, that is. Seems like it should run each check, provided the user is empty or zero or falsey. I'm now logging the user object and it's being set to an actual user object that corresponds with user ID 0, which doesn't exist. Like, at times it's correct, I think, but right in between when the REST request is printed to the error_log and when the response is logged, it's definitely and empty WP_User object (but an object nonetheless), which is why the filter's not called at that point in WP's "lifetime."

janboddez commented 4 years ago

So, this here seems to be a quick and dirty workaround. Essentially, it won't wait to register the determine_current_user hook till after plugins_loaded, which seems sufficient to get it to trigger correctly. Instead, it gets registered right when WP loads the plugin file.

//add_action( 'plugins_loaded', array( 'IndieAuth_Plugin', 'plugins_loaded' ), 9 );
IndieAuth_Plugin::plugins_loaded();

While registering the determine_current_user hook this way is almost certainly safe (nothing will happen until that filter is first applied, which is what you want), there might be side effects to this, uh, hack. That said, if further down the line everything uses hooks and all, you should be fairly safe. (I almost always load my main plugin class this way.)

But maybe it's possible to pull just registering the determine_current_user callback function forward in time this way?

Update (last one, I promise): There are obvious side effects to that brute force approach. Like not being able to log on at all. :-) But the Micropub part worked (once, at least).

dshanske commented 4 years ago

So, that would imply a bug on the part of Jetpack that they are disabling any downstream. Now, if you are saying they set the current user to an id of 0, I could check for that...the IndieAuth code short circuits if user is set, but could check for a user object of 0

dshanske commented 4 years ago

Wait...I checked...I misremembered. It does not check for whether it is passed anything

janboddez commented 4 years ago

Don't think it's disabling per se. Rather that they call get_current_user() or so before plugins_loaded, which would cause an, apparently empty, user object to have already been set (which, I'm guessing, is what WP does if no user is logged in and no other filters are run), which results in IndieAuth's determine_current_user to never run. (Because, when Micropub checks for logged_in_user(), determine_current_user is no longer invoked.) An option might be to drop the login check and force determine_current_user? Like call apply_filters or something from the Micropub plugin?

Long story short: I'm a bit afraid IndieAuth either uses the "wrong" hook, or (more likely) registers it too late in WordPress's lifetime, and that we've mostly been "lucky" so far that no other plugins run get_current_user() (or similar) very early.

dshanske commented 4 years ago

How, they seem to hook into init, which is called after plugins_loaded

dshanske commented 4 years ago

Wait, they load in the init function of the class, which is called in the construct function of the Jetpack class, which is called in the load Jetpack class, which is called in the Jetpack.php file... confusing

janboddez commented 4 years ago

Might be as simple as adding, e.g., apply_filters('determine_current_user', 0); at the start of Micropub's load_auth() function, as by that time IndieAuth's hook will have been registered and the code should just run. (I'd try it out, but it's ... pretty late here.) And maybe actually set the current user to the outcome of that function, too.

janboddez commented 4 years ago

Cleaner workaround (made these changes in class-micropub-base.php):

public static function load_auth() {
    // Check if logged in
    $user = wp_get_current_user();
    wp_set_current_user( apply_filters( 'determine_current_user', $user->ID ?? 0 ) ); // Don't think you'll want to use the null coalescing operator in a WP plugin, and it may not be needed, but ...

    if ( ! is_user_logged_in() ) {
        return new WP_Error( 'forbidden', 'Unauthorized', array( 'status' => 403 ) );
    }

This essentially forces the filter hook to run, and since it's executed well after plugins_loaded, IndieAuth's determine_current_user logic will have been registered by then.

Now this won't fix other plugins that rely on IndieAuth, so a proper solution probably still requires IndieAuth's determine_current_user to be registered as early as possible. Like, right when the plugin files are first included and thus not inside another add_action or add_filter callback, just in case Jetpack (or any other code) calls any user functions really early.

Note that if they do call it outside of a proper hook, and it looks like that might be the case, everything will depend on the plugin load order, and that's not something you have control over. Which means the method above might be the only way. One could even ditch the determine_current_user hook altogether and directly call something like IndieAuth::determine_current_user() (after refactoring, of course) and force set the user to that. There's no reason another plugin should intervene here anyway, although you could introduce a new filter just in case.

Or convince Jetpack that they're wrong, but I haven't yet found where (or rather, when) exactly they're actually "setting" the user.

kraftbj commented 4 years ago

Return true would make it work...but allow anyone to post to your website as that is where your credentials are checked. The webmention issue is because it is a public endpoint... but we'll fix that as it's just a warning

This isn't the case. The changes in Core were meant to highlight that endpoints are open by default. In this case, since the endpoint's callback checks for permissions, it is sorta pointless, but cleaner to have a separate permission check outside of the callback.

(Note: You'll see a "Missing blog token" error under Site Health. The Jetpack team is aware but doesn't seem to care too much. It's safe to just ignore.)

I didn't immediately see an issue filed in the Jetpack repo for this, but I fixed it in https://github.com/Automattic/jetpack/pull/17163 . It'll either get in for the point release this week or in the regular release on the first Tuesday of October.

Or convince Jetpack that they're wrong, but I haven't yet found where (or rather, when) exactly they're actually "setting" the user.

No need to convince. It's pretty obvious something we did in 8.9 had this unintended effect. I'm trying to find it too, but haven't found it yet. Apologies for the delay--last week was the first week of getting the kids figured out with virtual school. I thought I would have time to get anything done last week, but alas, five kids between kinder and 6th meant... no time.

We're aiming to have a point release out on Wednesday and my goal is to have this figured out in time to get a fix in.

dshanske commented 4 years ago

@kraftbj Appreciate you looking into this. I will wait to see what you figure out.

janboddez commented 4 years ago

Ooh, nice!

I was thinking, maybe we shouldn't rely on is_user_logged_in() to trigger determine_current_user.

Any plugin, not just Jetpack (which is why I'm not necessarily convinced it's wrong) calling nearly any user functions before Micropub / IndieAuth and thereby (perhaps unintentionally) setting the current user to an empty WP_User object will result in this behavior (I think).

If that's truly an issue, maybe it should be addressed in core. Or documented better.