indieweb / wordpress-indieauth

IndieAuth for WordPress
https://wordpress.org/plugins/indieauth/
MIT License
31 stars 11 forks source link

Add nag notice and new script for checking headers #136

Closed dshanske closed 5 years ago

dshanske commented 5 years ago

This adds in a script that will nag you to run it until you do. Hopefully this will help educate people on the dangers of not passing Auth headers.

chrisaldrich commented 5 years ago

Manually testing this on my site generally seems okay, but I think it contains a logic error because it is returning what must be a false positive.

I see the message in the admin UI and can click on the test which returns the message "Alternate Header Found. You are good to go." However, when attempting to actually log into Monocle, I get the same 403 error saying that it couldn't find the bearer token, and it won't let me log in. So obviously I'm not "good to go."

From a UI perspective something like "Your headers are properly configured and accessible." may be better than the "You are good to go" which may be harder for non-English speakers. Additionally wrapping that message in an anchor that will redirect to their admin UI might be nice.

(Originally posted on https://boffosocko.com/2019/04/25/55750053/)

dshanske commented 5 years ago

The alternative header is REDIRECT_HTTP_AUTHORIZATION used by some servers. I didn't in my code ensure that the bearer token is being returned and should... just that the header existed. May ask for another check after some changes

dshanske commented 5 years ago

@chrisaldrich Can you check with the new code? It now checks not only for the existence of the header but that it contains the test payload

chrisaldrich commented 5 years ago

@dshanske, I’ve removed the prior version and reinstalled https://github.com/indieweb/wordpress-indieauth/pull/136/commits/d5b91a0cba5ff890866dbacf4609e2f711a238da. I don’t get the same original notification (I’m suspecting because the original stored a value in my db that unflags the nag). However, when rerunning the diagnostic script at /wp-admin/admin.php?page=indieauth I get the same positive response that “Alternate Header Found. You should be able to use all clients.” Sadly, I’m still getting the same old 403 when attempting to log into Monocle.

(Original posted at https://boffosocko.com/2019/04/25/55750053/#comment-259564)

dshanske commented 5 years ago

@pfefferle Could you please review and permit merging on this? I realized that I was focusing on a token retrieval issue in the client code, not the token verification code. This is now fixed, and the the test must be run for all new users to ensure they check.

pfefferle commented 5 years ago

Will do later today!

dshanske commented 5 years ago

Thanks... I know the header issue affects a lot of people who use shared hosting and other projects are getting issues opened.

pfefferle commented 5 years ago

In general: I would prefer a dedicated test-page, like provided by the OpenID plugin, that runs all needed checks.

Bildschirmfoto 2019-05-02 um 09 40 27
dshanske commented 5 years ago

I could build it into the settings page, but then it would run the check every time you retrieved the page... which I suppose is fine. What do you think?

pfefferle commented 5 years ago

We could add an extra settings page for the tests and add a link to the actual/main settings page.

pfefferle commented 5 years ago

Can we use getenv("auth_head") to check if the auth-header is disabled?

-- https://stackoverflow.com/a/34080802/1731334

dshanske commented 5 years ago

Again, seems to be the same sort of solution that requires a check.

dshanske commented 5 years ago

@pfefferle If yhou look at the Stack OVerflow, that still requires you edit htaccess, so therefore the solution of adding the header is better.

dshanske commented 5 years ago

@pfefferle Can you review again? This time, in addition to the diagnostic script(which I want to access not logged in as people keep filing issues and I can check server support without them or compromising anything), the script is run from the settings page and embeds a large failure notice in the page if it doesn't work. Otherwise, it will still nag to check using the original script.

dshanske commented 5 years ago

Addresses #135 #91 #131