indieweb / wordpress-micropub

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

Change config to filter #162

Closed dshanske closed 6 years ago

dshanske commented 6 years ago

Oddly enough, there were issues @miklb had issues with the plugin because it was trying to load both the built-in auth and the IndieAuth. So I decided that a more dynamic way of doing this might make more sense, and retiring the configuration variable.

Since the Micropub spec now says that auth must be in the form of OAuth2 or the IndieAuth variant, it shouldn't support anything called MICROPUB_LOCAL_AUTH. So, the new version disables the auth in favor of it being handled externally. I also added to the documentation.

miklb commented 6 years ago

I will test this PR. I do not have any reference to local in my WP config.

dshanske commented 6 years ago

No one to my knowledge is using the setting. But somehow it was tripping for miklb. I thought as we don't need it, removal waa the best option.

dshanske commented 6 years ago

@snarfed I added some items here to try and address the variable issue. Wanted your thoughts. As for @miklb, he did prompt me to look at that line, but for some reason, he was tripping it despite not having it set.

dshanske commented 6 years ago

@snarfed Seem to have made a mistake. I'm not sure what I missed here but the tests are failing, which suggests that I was right and something I had set about that definition isn't working. Will have to fix later.

miklb commented 6 years ago

This does not solve my issue. I get the same 401 Micropub Error: 401 unauthorized - missing access token

dshanske commented 6 years ago

@snarfed Trying a different approach to load the file later in the chain.

miklb commented 6 years ago

this latest change results in expected behavior authenticating with Quill including ability to post a note and syndicate.

💯

snarfed commented 6 years ago

hey, that's great news! thank you for testing @miklb!

snarfed commented 6 years ago

@dshanske sounds like the delayed check in your latest commit is what fixed the problem. that's great!

given that, the filter is probably unnecessary and overkill for a simple setting. mind switching it back to the old constant?

dshanske commented 6 years ago

I thought of a way to have the best of both worlds

dshanske commented 6 years ago

@snarfed I realized that by loading the auth on a hook, I accomplished the same goal, so the documentation now reflects that.

snarfed commented 6 years ago

thank you so much @dshanske, this looks great!