freckle / yesod-auth-oauth2

OAuth2 authentication for yesod
MIT License
71 stars 53 forks source link

Fix hoauth2 compat for 2.7.0 #165

Closed mjgpy3 closed 1 year ago

mjgpy3 commented 1 year ago

Fix compatibility.

mjgpy3 commented 1 year ago

@pbrisbin I implemented some of your feedback and pessimistically bumped us to 0.8.0.0.

The way I see things, we have two touch points that are changing. Let me know if you think we should take a different approach with the following in mind.

1

When we build the plugin, we expect users to provide a function saying how the token is fetched:

-- | How to fetch an @'OAuth2Token'@
--
-- This will be 'fetchAccessToken' or 'fetchAccessToken2'
--
type FetchToken
  = Manager -> OAuth2 -> ExchangeToken -> IO (OAuth2Result Error OAuth2Token)

The Error here doesn't share the same type between 2.7.0 and <2.7.0. If we export a custom Error that summarizes both then consumers will still need to import said error.

That being said, the comment claims this should be a function from our library. If that's true, this is all mute because these concerns all end up being internal.

This is the reason for the pessimistic bump. It's possible a consumer didn't use fetchAccessToken(2) and will now need to adjust to use the new Error type.

2

If auth fails, we capture the specific error:

data DispatchError
    = ...
    | OAuth2ResultError Error
    | ...
    deriving stock Show
    deriving anyclass Exception

This error gets re-thrown internally and handled internally.

The only thing that will change here is

  $(logError) $ pack (displayException err) <> suffix

The interface safety of this is debatable. On the one hand it's just a log message. On the other hand people parse write parsers for messages like this all the time.

mjgpy3 commented 1 year ago

@pbrisbin this PR is now up-to-date with our offline discussion.

For those watching at home, we're going with a minor version update because we do not believe this will break downstream consumers.