aaronpk / indielogin.com

Sign in with your domain name
https://indielogin.com
MIT License
155 stars 25 forks source link

Add grant_type #69

Closed gRegorLove closed 3 years ago

gRegorLove commented 3 years ago

When redeeming the authorization code, send grant_type=authorization_code

dmitshur commented 3 years ago

I've prototyped an IndieAuth server that is strict about requiring the grant_type to be set (since it is required in the latest spec, and backwards compatibility is not a priority for my needs). It behaves as expected, given the current behavior of the client at indielogin.com (as reported in this issue):

image

(I'm able to work around it by temporarily permitting a missing grant_type parameter to mean the same as grant_type=authorization_code, but I plan to remove that after this issue is resolved.)

Zegnat commented 3 years ago

The latest PHP client lib does define grant_type when exchanging the code. Wonder if something went wrong in the latest client update within this repo (8183508453e2bdc1dd8395f4aa01f9a88bee29cb), or maybe indielogin.com deployment did not follow through?

dmitshur commented 3 years ago

@Zegnat Perhaps that code isn't being used. I think the relevant code path is here:

https://github.com/aaronpk/indielogin.com/blob/8183508453e2bdc1dd8395f4aa01f9a88bee29cb/app/Provider/IndieAuth.php#L62-L78

And $params there is missing an 'grant_type' => 'authorization_code', entry.

Edit: Sent PR #70 for review.

Zegnat commented 3 years ago

I missed that, good spot! Wonder why the IndieAuth provider is using IndieAuth\Client for some things, but not for others. If it had used IndieAuth\Client::exchangeAuthorizationCode() it would have then automatically been updated when the core lib was updated.

dmitshur commented 3 years ago

@Zegnat I'm not familiar with the history of this codebase to say, but perhaps custom code is being used in order to provide more customization over error messages and diagnostics on indielogin.com, beyond what's considered in scope of the indieauth-client-php library. Or maybe it's just a simplification opportunity that hasn't been realized yet.