freckle / yesod-auth-oauth2

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

Support hoauth2-2.0 #155

Closed pbrisbin closed 2 years ago

pbrisbin commented 2 years ago

The new major version improves the naming of the fields of the OAuth2 record type. This type is central to this library and we leak it freely.

Users who make their own plugins are expected to construct values of this type to pass into our functions, this makes the new version disruptive to our code and our users'.

We have three options:

  1. Update and release our own new major version

    The major downside is that the current LTS resolver will then not update beyond our currently-released version. We have no immediate plans for new features in this library, but if we have bugs reported to be fixed we would either have to manage a complex backporting or ask our Stack users to wait for the next major LTS, which has historically been many months.

    Users who wish to use our new version as extra-dep would need to also bring in hoauth2, and who knows what else.

  2. Release a fully-compatible update

    As mentioned, we leak OAuth2(..) through this library's interface. In order to be truly backwards-compatible, we would have to use CCP to define an "old style" OAuth2 and use that throughout, such that in-the-wild OAuth2 values continue to work as-is.

    This would not be a good long-term solution as it introduces a fair amount of naming confusion and will lead to import conflicts for any users who also import hoauth2-2.0 modules in the same project.

  3. Release a mostly-compatible update

    This is the path this commit explores. We can update our own code to be hoauth2-2.0 compatible and use CPP to define the hoauth2-2.0-like OAuth2 if we're still on hoauth2-1.x.

    This gets us compiling in either case and "forward functional", with the exception of users who define their own plugins (which is rare).

    Because of that use-case, this should technically be a major version bump for ourselves (though I'm open to the argument we could treat the local-provider use-case differently), however it is still better than Option 1 in a few ways:

    • We still compile with hoauth2-1.x, so can be brought in easily as an isolated extra-dep
    • If there is a reported bug that we decide to only fix in the newer versions, the path for the user is better: they can pull us as an extra-dep and likely need no changes. Even if they're doing a custom plugin, the required changes are minor
pbrisbin commented 2 years ago

I'm going to merge the big-bang Restyled, but will wait for review before clobbering the diff like that.

pbrisbin commented 2 years ago

Yeah, I'm thinking of releasing a 0.6.3.5 that just adds && hoauth2 < 2.0 and then 0.7.0.0 that removes it again and makes these changes. Does that make sense, is it valuable or not really?

pbrisbin commented 2 years ago

Meh I take that back. I don't think the upper bound is valuable. If we find that it is, I can do it via revisions.

pbrisbin commented 2 years ago

Gonna merge the Restyled changes, squash into main then release it as 0.7.0.0.