Previously:
- System.Random, which seeds from system time (possible attack)
- 30 characters, a-z (low entropy)
Now:
- Crypto.Random, accepted as "cryptographically secure"
- 64 random bytes, Base64-encoded
cryptonite was already a transitive dependency, so there is really no
downside to this.
Fixes#132.
This comment comes from hoauth2:
-- OAuth2 spec allows `client_id` and `client_secret` to
-- either be sent in the header (as basic authentication)
-- OR as form/url params.
-- The OAuth server can choose to implement only one, or both.
-- Unfortunately, there is no way for the OAuth client (i.e. this library) to
-- know which method to use. Please take a look at the documentation of the
-- service that you are integrating with and either use `fetchAccessToken` or `fetchAccessToken2`
`fetchAccessToken2` is a drop-in replacement for `fetchAccessToken` that just adds `client_id` and `client_secret` to the body as form parameters, as permitted by [RFC 6749](https://tools.ietf.org/html/rfc6749#section-2.3.1). Some authorization server implementations only accept client credentials in this form.
This was lazy and resulted in a confusing error experience where a
JSONDecodingError fetching credentials appeared as an Unknown OAuth2
ErrorResponse, making it appear like the OAuth2 provider was indicating
this error to us, instead of it being a simple incorrect parser in our
own code.
ErrorResponse is specifically meant to parse error parameters sent to us
by the OAuth2 provider. They may be user-actionable and can be safely
displayed. This is a very narrow use-case. The Unknown constructor is
required for us to be exhaustive on our string error names, but it
should not be hijacked to store our own errors.
This commit separates and documents the two error scenarios.
I had hoped to get away from this entirely, to an Either-based
interface, but that seems to be stalling as an initiative. So in the
meantime, let's at least make our exceptions more meaningful.
For some reason, I thought tryIO would catch our own exception is we
threw them via throwIO, but that's incorrect. Our own exceptions are not
IOExceptions, so they squeak by. This fixes that.
- Latest LTS-11.5
- Allow hoauth2-1.7, needs to be extra-dep though
- Support *and require* yesod-1.6
This required:
- Less lifts
- HandlerFor, WidgetFor, etc
- Lost MonadThrow, but can use MonadIO instead
- Incorrect indentation
- We should always accept Id/Secret last
- The function is oauth... not oAuth...
Because of the mis-naming, at least we could fix the argument-order in a
backwards-compatible way, deprecating the old function/interface.
Even though it's "guaranteed" that values will be present because we set
them, nothing stops end-users from using these functions on Creds values
created by other plugins! Since that seems common, it would be
irresponsible of us to remain so unsafe.
See #71.
New `credsExtra` keys:
- `accessToken`: so you can make your own follow-up requests
- `userResponseJSON`: so you can get more information out of the request
we already made (you just have to parse it yourself)
Removed keys:
- `access_token`: renamed to `accessToken`
- `avatar_url`: can be re-parsed
- `email`: requires your own request to `/emails`
- `login`: can be re-parsed from `userResponseJSON`
- `location`: can be re-parsed, was not always present
- `name`: can be re-parse, was not not always present
- `public_email`: can be re-parsed, was not not always present
Also re-orders arguments between default and scoped to allow better
partial application -- taking advantage of API breakage already.
- Extract ...OAuth2.Dispatch
- Extract ...OAuth2.Prelude
- Reduce ...OAuth2 interface
- Re-export ...OAuth2 from Prelude
Incidental improvements:
- Moves a lot of FromJSON interfaces to withObject which will provide
better de-serialization errors
- Updates Dispatch code to prepare for fetch-creds functions returning
either instead of maybe, so we can eventually remove exceptions
entirely
- Replaces (the potentially information-leaking) 500 on OAuth2-related
errors with a 403 and logged error
