Allow extra fields to Authorization/Authentication Success

[adrianbunea]

Why?

The OAuth Server used by me, and many others, provide more than just the Default Fields mentioned in the OAuth2 spec. This library mentions that you can somehow provide your own decoders (Json decoder for a positive response. You may provide a custom response decoder using other decoders from this module, or some of your own craft.) but I did not see how, and even so, the end result is still an AuthenticationSuccess or an AuthorizationSuccess based on the flow, and these fields cannot be changed, nor can I expect a different return type. The goal of this PR is to make these two types into Extensible Types, with a spot for placing your own Extra Fields type if you need it, and a way to pass your own decoders for those extra fields.

What changed?

AuthenticationSuccess and AuthorizationSuccess are now two extensible types

BEFORE: type alias AuthenticationSuccess = { ... }
AFTER: type AuthenticationSuccess extraFields = AuthenticationSuccess { ... } extraFields
Same for AuthorizationSuccess

Added a customAuthenticationSuccessDecoder

customAuthenticationSuccessDecoder : Json.Decoder extraFields -> Json.Decoder (AuthenticationSuccess extraFields)
customAuthenticationSuccessDecoder extraFieldsDecoder =
    authenticationSuccessDecoder extraFieldsDecoder

Added a customAuthorizationSuccessParser

customAuthorizationSuccessParser : Query.Parser extraFields -> Token -> Query.Parser (AuthorizationSuccess extraFields)
customAuthorizationSuccessParser extraFieldsParser accessToken =
    Query.map2 AuthorizationSuccess
        (defaultFieldsParser accessToken)
        extraFieldsParser

Moved duplicated JSON decoders into the OAuth.elm file (I did not understand why they were duplicated)

All other changes were mainly my way of fixing the errors I faced due to the constraints of the extensible types, and the removal of the duplicated decoders. I tried to preserve the original API as much as possible, but this is still a major change. Let me know if you like this idea, and you would like to change anything here, there is still room for improvements, and cleanup, but for now I would like to know your opinion on this idea.

[KtorZ]

Hello, thanks for reaching out and taking the time to build a PR. I am afraid however that I will not be merging this PR for it's moving the library into several undesirable directions.

In particular, the use of the OAuth.Internal module was calculated and done purposely to enhance the developer experience for those consuming the library. Indeed, Elm does not provide good ways of re-exporting functions or types with their associated documentation. So, instead of compromising on the documentation quality and the structure of each module, I've opted for some minor repetitions and re-definitions of types across modules. As a result, it is possible to scope each OAuth2.0 grant as a separate, well-scoped, module.

In addition, there are actually some subtle differences between grants which are hard to capture under one abstraction. Once again, I've preferred for the sake of the developer experience to keep each module only about one grant to reduce the amount of noise and, provide better on-the-spot names and types.

---

About the issue itself, I am not quite positive about extending the parsers in such ways, because it creates noise for those who won't be needing those features. The library / specification in itself is already quite a big piece to consume, so I'd like to keep the entry barrier as low as possible by providing ready-to-use constructors for the common cases.

Now, you're right when saying that the amount of tweaking / customization currently available isn't sufficient to do what you want to do (which seems very reasonable!). I've thereby worked out another PR which should address all of your points and also points mentioned by some other users on #21 and #23, but preserving a bit more the original intent of the library w.r.t the developer experience. Please, have a look at #30 and in particular, the diff highlighted in the PR description and let me know if it indeed solves your problem nicely.

---

Finally, if the OAuth2.0 provider you're integrating against is publicly available, I'd be very much interested in adding it to the examples to show how to use these custom / advanced constructors.

[adrianbunea]

Hi. I understand, and I agree with your reasons. In fact, I was pretty sure the duplication was intended, and I shouldn't have included my "refactoring" if I wanted to pitch my proposal anyway. Also, I agree with the fact that my approach was introducing too much noise for consumers of the library that follow the OAuth spec to the letter, your solution looks better. In fact, I tried it locally, making some changes to my app to use the new API, it was pretty quick, and it works fine.

For the last question, the OAuth provider was created using the Doorkeeper gem for Rails, it is different because it includes a created_at field, I am not sure if the guys working on the API added this field, or the gem includes it for whatever reason, but it was helpful, and that was the Extra field I wanted to decode.

Thanks for the new PR, it solved my issue..

[KtorZ]

For the last question, the OAuth provider was created using the Doorkeeper gem for Rails

Thanks for link. Seems like that is what powers https://oauth.io so I'll see if there's maybe a way to do an integration with that as an example. Hopefully, this non-standard created_at will show up and there will be something interesting to do.

Thanks for the new PR, it solved my issue..

Great! I will close this PR then, merge the other one and prepare a release.

[KtorZ]

See elm-oauth2@8.0.0.