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

Complete Rewrite of Plugin #9

Merged
merged 1 commit into from Jan 1, 2018

Conversation

Projects
None yet
2 participants
@dshanske
Member

dshanske commented Jan 1, 2018

I know it is easier to digest in chunk sized bits, but this is a major rewrite of the plugin, after which my intention is to go smaller for future enhancements. This...

  1. Rewrites the README a bit
  2. Adds support for login using Indieauth Bearer Tokens, which actually now makes this a supported authentication plugin for the WordPress REST API.
  3. Adds using the author profile as your URL
  4. Add settings to disable the login form addition and to use endpoints to something other than Indieauth.com
  5. Supports setting Indieauth Scope as a global variable for future use as WordPress has no built in scope options
  6. Updates functionality to the Indieauth specification at indieauth.net
  7. Adds a state parameter being passed, even though it isn't used as required for compliance.

I drew some inspiration from how @snarfed set up IndieAuth logins in Micropub, as well as some proof of concept OAuth2 plugins for WordPress, and read through @aaronpk 's OAuth 2.0 book.

I don't think I'll stop here.

if ( ! empty( $error ) ) {
return $error;
}
global $indieauth_error;

This comment has been minimized.

@pfefferle

pfefferle Jan 1, 2018

Member

why make it global?

@pfefferle

pfefferle Jan 1, 2018

Member

why make it global?

This comment has been minimized.

@dshanske

dshanske Jan 1, 2018

Member

There's no other way to surface the error. I was stumped about that till I looked at the REST API teams example Oauth plugin.

@dshanske

dshanske Jan 1, 2018

Member

There's no other way to surface the error. I was stumped about that till I looked at the REST API teams example Oauth plugin.

return $user_id;
}
$params = json_decode( $body, true );
global $indieauth_scopes;

This comment has been minimized.

@pfefferle

pfefferle Jan 1, 2018

Member

why global?

@pfefferle

pfefferle Jan 1, 2018

Member

why global?

Show outdated Hide outdated includes/class-indieauth-authenticate.php Outdated
Show outdated Hide outdated includes/class-indieauth-authenticate.php Outdated
global $wp_rewrite;
$link = $wp_rewrite->get_author_permastruct();
if ( empty( $link ) ) {
$login = str_replace( home_url( '/' ) . '?author=', '', $identifier );

This comment has been minimized.

@pfefferle

pfefferle Jan 1, 2018

Member

is it really home URL? not site URL?

@pfefferle

pfefferle Jan 1, 2018

Member

is it really home URL? not site URL?

This comment has been minimized.

@dshanske
@dshanske

dshanske Jan 1, 2018

Member
@pfefferle

This comment has been minimized.

Show comment
Hide comment
@pfefferle

pfefferle Jan 1, 2018

Member

tested it with my blog and it does not work and I got no error message at all.

Member

pfefferle commented Jan 1, 2018

tested it with my blog and it does not work and I got no error message at all.

Show outdated Hide outdated templates/indieauth-login-form.php Outdated
Show outdated Hide outdated templates/indieauth-login-form.php Outdated
@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

Odd about it not working. Is this your development site, because I did switch to safe remote get and that if you remember from last time hates development sites

Member

dshanske commented Jan 1, 2018

Odd about it not working. Is this your development site, because I did switch to safe remote get and that if you remember from last time hates development sites

@pfefferle

This comment has been minimized.

Show comment
Hide comment
@pfefferle

pfefferle Jan 1, 2018

Member

I have it installed on my main blog

Member

pfefferle commented Jan 1, 2018

I have it installed on my main blog

@pfefferle

This comment has been minimized.

Show comment
Hide comment
@pfefferle

pfefferle Jan 1, 2018

Member

but even if it is an issue, I miss an error message

Member

pfefferle commented Jan 1, 2018

but even if it is an issue, I miss an error message

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

I will work on figuring that out. Did the old version work for you?

Member

dshanske commented Jan 1, 2018

I will work on figuring that out. Did the old version work for you?

@pfefferle

This comment has been minimized.

Show comment
Hide comment
@pfefferle

pfefferle Jan 1, 2018

Member

sure

Member

pfefferle commented Jan 1, 2018

sure

@pfefferle

This comment has been minimized.

Show comment
Hide comment
@pfefferle

pfefferle Jan 1, 2018

Member

at least it (the old version) showed an error if I used a wrong URL or had a typo

Member

pfefferle commented Jan 1, 2018

at least it (the old version) showed an error if I used a wrong URL or had a typo

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

Odd. I am not sure where to start, but I will start testing and working on better error handling

Member

dshanske commented Jan 1, 2018

Odd. I am not sure where to start, but I will start testing and working on better error handling

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

I think this is worth getting right.

Member

dshanske commented Jan 1, 2018

I think this is worth getting right.

@pfefferle

This comment has been minimized.

Show comment
Hide comment
@pfefferle

pfefferle Jan 1, 2018

Member

I have a lot of redirects and stuck at https://notiz.blog/wp-login.php?redirect_to=...&code=...&me=...&state=... at the end.

Member

pfefferle commented Jan 1, 2018

I have a lot of redirects and stuck at https://notiz.blog/wp-login.php?redirect_to=...&code=...&me=...&state=... at the end.

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

I will start trying to surface errors more effectively.

Member

dshanske commented Jan 1, 2018

I will start trying to surface errors more effectively.

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

I think it is because I switched from the old verification method you were using to the current one in the specification. I may not have adjusted the error handling appropriately.

Member

dshanske commented Jan 1, 2018

I think it is because I switched from the old verification method you were using to the current one in the specification. I may not have adjusted the error handling appropriately.

@pfefferle

This comment has been minimized.

Show comment
Hide comment
@pfefferle

pfefferle Jan 1, 2018

Member

wp_hash_password( home_url() ) !== $_REQUEST['state'] this is true for me

Member

pfefferle commented Jan 1, 2018

wp_hash_password( home_url() ) !== $_REQUEST['state'] this is true for me

@pfefferle

This comment has been minimized.

Show comment
Hide comment
@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

I see. I'm looking at that right now. It was the last thing I implemented as a state parameter is required. I'm fixing it now.

Member

dshanske commented Jan 1, 2018

I see. I'm looking at that right now. It was the last thing I implemented as a state parameter is required. I'm fixing it now.

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

@pfefferle I made fixes. I switched to using a nonce, which is more appropriate for a state parameter anyway. Should work now.

Member

dshanske commented Jan 1, 2018

@pfefferle I made fixes. I switched to using a nonce, which is more appropriate for a state parameter anyway. Should work now.

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

Also made the other changes requested.

Member

dshanske commented Jan 1, 2018

Also made the other changes requested.

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

It's my own fault. I wanted to finish before 2018.

Member

dshanske commented Jan 1, 2018

It's my own fault. I wanted to finish before 2018.

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

Either way, while this is the only time I intend to put such a big change, I do want to work on making this even better with smaller commits and refinements subsequently.

Member

dshanske commented Jan 1, 2018

Either way, while this is the only time I intend to put such a big change, I do want to work on making this even better with smaller commits and refinements subsequently.

@pfefferle

This comment has been minimized.

Show comment
Hide comment
@pfefferle

pfefferle Jan 1, 2018

Member

no problem, I fully support the re-writing!

Member

pfefferle commented Jan 1, 2018

no problem, I fully support the re-writing!

@pfefferle

This comment has been minimized.

Show comment
Hide comment
@pfefferle

pfefferle Jan 1, 2018

Member

nice, seems to work for me, but I couldn't test the IndieAuth native login yet, I have no test instance.

Member

pfefferle commented Jan 1, 2018

nice, seems to work for me, but I couldn't test the IndieAuth native login yet, I have no test instance.

@pfefferle pfefferle merged commit 2589ef3 into indieweb:master Jan 1, 2018

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

I used gimme a token to get a token manually and curl to test.

Member

dshanske commented Jan 1, 2018

I used gimme a token to get a token manually and curl to test.

@pfefferle

This comment has been minimized.

Show comment
Hide comment
@pfefferle

pfefferle Jan 1, 2018

Member

Ok, thanks!

Member

pfefferle commented Jan 1, 2018

Ok, thanks!

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

I will add that as a suggestion to the readme.

Member

dshanske commented Jan 1, 2018

I will add that as a suggestion to the readme.

@pfefferle

This comment has been minimized.

Show comment
Hide comment
@pfefferle

pfefferle Jan 1, 2018

Member

We should start with a indieauth provider so that we do not have to 😉

Member

pfefferle commented Jan 1, 2018

We should start with a indieauth provider so that we do not have to 😉

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

Define provider?

Member

dshanske commented Jan 1, 2018

Define provider?

@dshanske

This comment has been minimized.

Show comment
Hide comment
@dshanske

dshanske Jan 1, 2018

Member

Do you mean built in endpoint?

Member

dshanske commented Jan 1, 2018

Do you mean built in endpoint?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment