Send client credentials in the request body when fetching the access token

[nbloomf]

Bump version of hoauth2 dependency, and replace call to fetchAccessToken by fetchAccessToken2.

fetchAccessToken2 is a drop-in replacement for fetchAccessToken that just adds client_id and client_secret to the request body as form parameters, as permitted by RFC 6749. Some authorization server implementations only accept client credentials in this form. This function was introduced in 1.7.0 and amended in 1.11.0; either the initial or the amended version of fetchAccessToken2 would work for this, but here we've chosen the most conservative working version bump.

This is deployed for live testing to http://yesod-auth-oauth2-example.herokuapp.com/, along with #130, from https://github.com/nbloomf/yesod-auth-oauth2/pull/2.

Testing checklist:

• [ ] Azure AD
• [ ] BattleNet
• [x] BitBucket
• [ ] Eve Online
• [x] GitHub
• [x] GitLab
• [x] Google
• [ ] Nylas
• [ ] Salesforce
• [x] Slack
• [x] Spotify
• [x] WordPressDotCom
• [ ] Upcase

Fixes #127

[pbrisbin]

This is awesome, I love the idea of the test heroku app.

Personally, I'm fine with the level of testing you've got so far. I know some of these plugins are difficult to actually set up. If you want to push for full coverage though, that's great too.

[nbloomf]

I'm fine with the level of testing you've got so far

That works for me; I did at least attempt the others, but each one has some extra hurdles like requiring a paid account (eve online) or having unhelpful documentation (name withheld). The ones that I did get set up had no problems.

[pbrisbin]

:wave: Sorry to revive this Pull Request, but I think I've found a provider that actually cares about the credentials being passed in two places...

I'm seeing "Cannot supply multiple client credentials. Use one of the following: credentials in the Authorization header, credentials in the post body, or a client_assertion in the post body." when interacting with Okta.

Do you think this change has caused that? What should we do about it?

[nbloomf]

Interesting - I assume this patch is the culprit, since IIRC fetchAccessToken2 simply puts the credentials in both the post body and the auth header. It does make sense for an implementation to only allow credentials in one place, since creds in multiple places might not match.

A not-too-invasive fix might be to try

token <- errLeft $ fetchAccessToken manager oauth2' $ ExchangeToken code

(the version of this line before this patch), detect when this fails, and then try again with fetchAccessToken2 as a backup before bailing with errLeft. That would not require changing any type signatures.

This isn't perfect, but it would work on the first try in the (evidently more common) case where only the auth header method is recognized and only one copy of the credentials is allowed. It would not work if a provider accepted credentials only in the post body, but it appears there are no known examples where that is the case.

That would definitely need to be tested; I don't know enough about the oauth protocol to say offhand if multiple calls to that endpoint would cause a problem (probably not, but possible).

The "best" solution probably involves deeper changes to the API of hoauth2.

[nbloomf]

I can look into trying that fix. I think I still have a test app somewhere.

[pbrisbin]

I was starting down the following avenue:

• Update dispatchAuthRequest to accept a fetch-token function (it kind of makes sense as a sibling to FetchCreds)
• Update authOAuth2 and authOAuth2Widget to pass either fetchAccessToken or fetchAccessToken2 (if we want to revisit which one is more commonly needed, based on this new information)

This means the types of authOAuth2 and authOAuth2Widget don't change, so we can just release a patch bump. But clients that are broken by the choice of fetch-token can drop down and build an AuthPlugin themselves, passing a different function to dispatchAuthRequest

I might not have time to see this through this week though.

[nbloomf]

I like that idea a lot better; it also allows clients to work around providers that aren't compatible with hoauth2 for unknown-unknown reasons.

[pbrisbin]

I reverted back to fetchAccessToken in v0.6.1.6, but added authOAuth2' and authOAuth2Widget', which use fetchAccessToken2. Please update any code you had relying that to use these functions instead.