Enhancing AuthInfo

[fgrutsch]

Hey!

First of all, thanks for that amazing library. 👏 In our current project we have the problem that we have to extend/enhance the AuthInfo with additional information. Currently it seems not possible to do that.

As an example:
We have the case that we want to get the audience query parameter from the AuthorizationRequest and make it accessible within the createAccessToken(authInfo) method so we can include it in our JWT access token. The problem here is, that the createAccessToken method only takes the AuthInfo input parameter, which is a case class and built directly within the different GrantHandlers already. So there seems to be no way on how to put in additional information.

I made two proposals in two different branches that could maybe solve the issue:

1. This proposal converts the AuthInfo from a case class to a trait and provides a DefaultAuthInfo (equal to the currently existing AuthInfo) to not break the API. Diff: fgrutsch@e5568b0
2. This proposal also converts the AuthInfo from a case class to a trait, but additionaly makes the AuthInfo typed on the DataHandler. This would make usually probably easier, but comes with the disadvantage that it breaks the API. Diff: fgrutsch@b727b90

Please take a look at them and let me know your opinion, so that I could potentially open up a PR.
In case we are just overseeing something on how we could already achieve that with the current implementation, please let me know as well. Thanks 😉

[tsuyoshizawa]

@fg-devs Thank you for your PR!

I would choose No.1 solution.

No.2 solution looks like Scala and feels very extensible.
But I'm not sure if AuthInfo is required to be so extensible even if we break the API.
Then all users need to write the type for interface.

You suggest us two API implementations carefully, and it's really easy to understand.
Thank you very much.

[fgrutsch]

@tsuyoshizawa Thanks for the feedback! I opened up a PR for solution no.1

[rmmeans]

Hmm, I'm not sure a change is needed. We use this library and have implemented a full OAuth 2 and OIDC complaint system using it that passes all of the OIDC checks. In order to do that, we also had the needs similar to yours to have more contextual information available when creating tokens, etc.

Because AuthInfo is a type of AuthInfo[+U] where U is the type for your user - we took that to implement a much broader user value that is passed around with AuthInfo.

For instance here is pseudo code on what we do:

case class AuthenticationInfo(userId: String,
                              authSource: AuthenticationSource,
                              credentialTime: Long,
                              oidcNonce: Option[String] = None) // passing the oidc nonce from the authorization request through the system so that it's available when we need to inject it into tokens, etc.

We then are able to take that type and create our DataHandler like so such that the user in AuthInfo is a full case class with tons of rich data in it - including some that is contextual right from the authorizationRequest.

trait OAuthDataHandler extends DataHandler[AuthenticationInfo] {
...
}

Now when the library calls methods such as createAccessToken(authInfo) - we have the info we need to get other data points into the access token.

[tsuyoshizawa]

Hi @fg-devs, @rmmeans pointed out in the above comment.
Certainly, as he says, my project solved it in the same way.

What do you think?

[fgrutsch]

@rmmeans @tsuyoshizawa Hey, thanks for your help.

We also considered to put the custom information into the user type, but from our perspective, it just seems like that it would abuse the actual purpose of the generic user type.

If that's the way you think it should be done with, then I guess we will stick to that solution. But after discussing the problem again with my team, we thought it would make sense to provide a proper solution instead of doing a workaround, especially for new library users that may struggle with the same problem.

After thinking and experimenting with the AuthInfo again, we think that the typed AuthInfo approach would be the better one instead of just making the AuthInfo a trait. It would allow consumers of the library to properly type the AuthInfo to their needs. The drawback of just making it a trait is that when accessing properties of the concrete subtype you always have to match against it, which makes the code quite ugly in our opinions.

I am open for opinions, thanks 😉

[tsuyoshizawa]

Thank you. I understand your opinion.

I have a question by watching the PR code carefully.
Do you create the AuthInfo by accessing from some datastore?

AuthorizationHandler designs to fetch each data asynchronously.
All result types wrap Future, but createAuthInfo is not asynchronous method.

I'm worried about developers will use the method that is not asynchronous to create AuthInfo.

In additional, we define AuthorizationHandler following below.

Provide Authorization phases support for using OAuth 2.0.

createAuthInfo is a not required phase as we discussed above.
I'm not sure now we really need the method in AuthorizationHandler.

[fgrutsch]

Right now we don't access data from a datasource inside this method. But yeah you are right it should be asynchronous, but I see no problem in returning a Future[AuthInfo[U]].

Yeah right now this method can be overriden but doesn't have to be implemented, because it returns a DefaultAuthInfo. As mentioned in my previous comment, if it would be changed to a typed AuthInfo the method must me implemented, becasue there would be no way to provide a default value.