optionally parse refresh tokens from authorization code exchange

[stuarth]

workflow/request-token can be passed a response-parse-fn (superceding
access-token-parsefn), allowing values in addition to the access token to be parsed (e.g. refresh token). For example, for google’s oauth2

(def friend-config
  {:allow-anon? true
   :workflows   [(oauth2/workflow
                  {:client-config client-config
                   :uri-config uri-config
                   :credential-fn credential-fn
                   :response-parse-fn
                   (fn [req]
                     (when-let [response
                                (some-> req :body (parse-string true))]
                       {:access-token (:access_token response)
                        :refresh-token (:refresh_token response)}))})]})

response-parse-fn should return a map containing parsed vals. Addresses #20

[ddellacosta]

Okay, I've read through this and I think it's the right direction, but I want to propose some more drastic changes.

It's clear based on other conversations with folks that I've had that we need to allow the lib user access to all possible fields that the token endpoint could pass back. I think the goal with this lib should be to extend it to follow the spec for OpenID Connect, as that is (ostensibly) a real authentication protocol. This will require, in this section, supporting the extensions to allow for parsing an ID Token.

Anyways, what you have certainly enables that by giving the API user access to the entire response through a function they've passed in. That part of it is exactly what we want. But I think at this point it's overly confusing to have access-token-parsefn as well as response-parse-fn--I think we should only have one, and it should have a default implementation using the default RFC-supported method for parsing tokens, and we can leave the alternate in place if it helps anyone implementing this for a specific provider who strays from the RFC.

So I think what I'd like to see is something a bit more all-encompassing that replaces access-token-parsefn, that handles all the values in the OpenID connect spec, and is named something a bit more general like token-response-parsefn or token-response-handler. I also would like to have tests for these.

By the way, I'm not saying I expect all of these in this pull request, more just that this is how I think it should be structured, and whatever you contribute toward that is great.

Let me know what you think.

[anthgur]

It seems that "offline access" involving refresh tokens is a feature that at least Google and Twitter implement using Bearer tokens. I wanted to add that to the conversation as something that we should be aware of.

Since every provider seems to roll their own flavor of oauth (especially oauth2) I personally think covering every corner case would be a slippery slope (who would maintain implementations for the myriad apis that exist?) One example of a mature oauth/openid library that does this is python social auth. With enough people provider-specific workflows might be feasible, but right now I don't see that happening realistically.

I definitely agree that the library should be flexible. Perhaps allowing dispatch on tokens that are part of the RFC? To elaborate, may be friend-oauth2 could handle the boilerplate of server-to-server communication, allowing end users to inject a map of custom token handling functions in which the keys are RFC token-types. This might help mitigate the explosion of parse-token-x-fn's. I think that is the direction this pull request is heading, but may be the library could do more so the end user doesn't have to.

In the case of "offline access", there's the need for initial user authentication (after which refresh tokens are retrieved and stored), as well as access token requests using the stored refresh tokens. I'm working on this use case at the moment, but I'm not entirely sure how I'm going to implement it. I think dispatch on token types might help, but I'm not sure.

[anthgur]

What about providing protocols that hook into the token exchange lifecycle? This idea comes from psa's storage feature and the ability to add new backends.

If this library were to provide some protocols and a pipeline model like psa it might be easier for users to implement workflows for their needs. Perhaps this could hook into friend more somehow and provide user association features.

Again, I'm not sure if any of this is possible, beneficial, or unnecessarily complex.

[ddellacosta]

@anthonyurena

Since every provider seems to roll their own flavor of oauth (especially oauth2) I personally think covering every corner case would be a slippery slope

Yep. My basic guiding principle with this lib has become: follow the spec, provide alternatives for the most common edge-cases, and let the library user do what they need to if it is outside of the spec, but frequently requested and without heavy maintenance costs.

I haven't done a great job of the latter so far, but I think resolving the issues around this pull request will move that forward a good deal.

This might help mitigate the explosion of parse-token-x-fn's. I think that is the direction this pull request is heading, but may be the library could do more so the end user doesn't have to.

Regarding specific ideas--protocols, a pipeline model, etc.--I'm not necessarily opposed to any of them but I do want to avoid unnecessary complexity. In comparison, I think that python lib is too heavy for what we want with this lib, and fits a different niche. I have a few thoughts in response to your suggestions:

• the only place I've seen a real need for flexibility where we haven't had it so far is where the tokens are returned from the provider at the end. I think all we need here is a default mechanism to handle this as well as the ability for the user to override this with their own function, if they want to. So I don't think we need anything as involved as a pipeline, for example.
• I don't think we need any storage model or specific user account integration, as the python lib provides. By default we can store this in the session, probably directly in the friend identity hash-map. If the user wants to store this separately--say, connect it to their DB or whatnot--they can provide that custom functionality in the function above I'm proposing. Anything past that I think is well outside the scope of this lib.
• as far as providing new backends, I think that anything outside of oauth2 is probably better suited to integration as a new workflow within friend's architecture. So part of my response to the suggestion regarding backends is that this is more friend's problem than friend-oauth2's problem, if that makes sense.

The only place I really have doubts is in the first item--if there are places in the system that a developer needs to hook into that I'm forgetting, then I'm certainly open to considering that. But I've only ever gotten requests for hooking into the very end of the oauth2 flow, which is why I think that's probably the best place to start, even if it does make sense to open something else up later on.

Some things that I do think may be useful is to provide default functions to make a call to the server using the access token, request a new refresh token, etc. But I'm still a bit nervous about where these would go--they aren't strictly part of the work friend-oauth2 should be doing, I believe. That said, they could provide a lot of utility to library users.