Enable better extensibility of this library

[christiaangoossens]

When looking at how to make the changes requested in #885 and #793 , I came up with the following method for adding information to access tokens:

• You can overwrite the convertToJWT method on your access token to add new claims directly from the access token data.
  However, you may have gotten this data (in the auth code grant) before making the authorization code (such as when you have your user login, and then want to include some data about that login in your access token). Therefore, we also needed a method to add custom data to that authorization code to transfer it 'between sessions'. Using php sessions for this purpose wouldn't always work, for instance if the server is finally using the code to reach the token endpoint, and we thus don't have the same session. This PR adds a method for adding data to the auth code, and a way to handle that data to put it back into your application before issuing the access token. (You can for instance make a TokenDataRepository, inject the data from that repo into the auth code, and then take it out of the auth code, back into the repo on the handle function).

• You can use a custom instance of the BearerTokenReponse for adding new params to both the responseParms (token_type, expires_in, access_token) by overwriting getExtraParams (also fixed Unable to Override AuthorizationServer to use customised BearerTokenResponse #903 as it was a documentation error), and you can add parameters to the refresh token by also overwriting generateHttpResponse.

TLDR: you can now add extra information to:

• the access token through the convertToJWT method in your class implementing the AccessTokenEntityInterface interface.
• the refresh token by creating your own instance of the BearerTokenResponse class and overwriting generateHttpResponse
• the auth code by creating your own instance of the AuthCodeGrant class and overwriting the two empty methods for adding your own parameters
• the token endpoint response by creating your own instance of the BearerTokenResponse class and overwriting getExtraParams.

This PR enables people to have more control over their token contents, and enables the addition of data gained at login to the final access token, by passing it through the auth code.

The PR has been tested, and should not introduce backwards incompatible changes. We may want to have more tests, but I cannot find an easy way to add them. Help would be welcome.

[sg3s]

We've been patching this in our fork and this looks to be doing what we need to. Any chance this can get reviewed/merged?

[nealoke]

+1 for merging this, maybe also some documentation on how to do this?

[Sephster]

I will review this as soon as the PKCE changes have been merged in. From memory, I hadn't looked at this as it was building on some pre-existing change requests so I'd like to review them all together if possible which will likely take longer. Thanks for your patience.

[Sephster]

@christiaangoossens I'm not sure this is the best approach. By allowing additional parameters at the authorisation code request, I think we would be deviating from the OAuth spec. I can't find anything that says additional parameters are allowed at the auth code request stage but similarly I can't find details saying this isn't allowed. I would probably err on the side of caution though.

Access tokens are usually consumed by the resource server and a client should normally not be aware or care what its contents are. I think this PR is flipping this assumption on its head by allowing the client to add information to the tokens.

I will leave this PR open for further discussion in case I have made a mistake with my assertion but I feel that this approach probably isn't the right one to take at the moment. I would appreciate any thoughts you have on this though.

Thank you for your submission and apologies again for the delayed response.

[christiaangoossens]

@Sephster This PR was intended to add an option for the server to easily add stateful information into the code granted at the end of the auth code grant, not to allow the client to do this. This would prevent having to store that data, and a separate identifier in a shared database.

With regards to the access tokens itself, this again goes into the contract between server and resource owner which may still indirectly depend on the client (for instance user1 + client2 => extra attr about role4) but was never intended to change as dramatically as you describe by client data.

[sg3s]

@Sephster As you said the access tokens are consumed by the resource server, and you think we might be violating the spec with these changes. I can see how you can misuse the methods in this PR to do that. So maybe this isn't the right approach, but I do think the library should facilitate options to change/add to what is in the access token.

There is no single specification we can rely upon to define an access token, and thus in our case, the authorisation code (because we put most access token information in there). But the resource server can, and at least in our case does, care about which client is being used and from where. We add additional information in the access token to give our resource server a verifiable context which is more secure.

Would it be an idea to maybe collect scenarios from users (open an issue, define a simple template for responses so you get a minimum set of variables) and then look at possible options for extensibility? This could allow you/the maintainers to get some insight on how the access tokens are (mis)used, then you can steer towards more appropriate solutions with features and documentation on what not to do.

I like how the library takes a very strict stance on the specifications in how to fill in the different flows/interactions, that is why we selected this library, makes it less error prone, but we would like some more flexibility short of overwriting 5-8 classes to add a claim in a somewhat clean way, that increases security.

I'm very much interested to help with any of that if it is appreciated.

[Sephster]

@s3gs I agree that we must make it easier to add custom claims to the JWT. My assessment here is that it is happening at the wrong stage in the process but I am open to discussion on this. Ideally we would add some custom claims at the point where the access token is issued in a similar vain to the way we finalise scopes. The resource server would add these claims to help it better understand/use the access token.

I think it would be useful if you could describe your own scenario at the moment, and perhaps how this PR solve it so we can hopefully push towards a resolution. Thanks for your offer of help with this issue

[sg3s]

Scenario 1
What
We add the resource owner (but really the client retrieving the access token) IP to the token, and verify this matches with the client IP on the resource server.

Why
We have a client / resource server which we limited to specific networks. This can be done with webserver rules, but letting the application do it with an IP in the token allows us to control the error message and format much better, and keep the logic for determining the allowed networks centralised on the authorisation server.

How oauth2-server support would help
Currently we trust that the client receiving and using the authorisation code and using it to retrieve an access token is on the same network/ip. It is a javascript client after all. However this does not have to be true if someone managed to intercept the authorisation code. PKCE mitigates issues with that, but allowing the determine the authoriser IP when the authorisation code is created is cleaner & more reliable (as we control the authorisation server much more than the clients).

The same goes for functionality where the access token is locked to user-agent signatures or special device enrollment headers etc.

Scenario 2
What
We allow impersonation of users for support purposes for certain clients (this is limited to certain employees who need at least explicit consent from a user on a phone call). The impersonator details are encoded in the access tokens used by clients. Our resource servers are semi aware of it for audit/activity log purposes and protecting various sensitive functions. Clients don't know the difference.

Why
It allows our service department to inspect systems/functionality as a user with very few real limits or differences when RDP support is not an option.

How oauth2-server support would help
We link the details to the relevant session in the database when authorising the client. Currently we retrieve and validate that data again when finalizing the access token. Being able to pass the necessary details when the authorisation code is created allows us to remove that overhead, it is again also cleaner as this is the stage where most of the access token is already determined.

(In this scenario we also wanted to turn off refresh tokens for clients that can get them normally and maybe limit the lifetime of access tokens in that situation as well. It is a different issue but we pre-process requests and bootstrap the oauth2-server library with shorter/effectively useless TTLs on the refresh tokens to do this now... because it is not like the library makes this easy)

There are plenty of other scenarios where a certain degree of extensibility simply improves lives. We don't keep an active record of cases where we envision/brainstorm functionality and dismiss options simply because the oauth2-server does not allow for it, even if it is about crafting more secure systems, but it happens regularly.

[francislavoie]

If you're asking for specific scenarios, I was hoping this would be merged specifically for the first point in @christiaangoossens PR comment, i.e. adding claims into the JWT without needing to store state on the server, by transferring that state in the authorization code.

[christiaangoossens]

@Sephster What's the status of this PR at the moment? Is another method of adding data to the Auth Code already available, or do you have any other way to implement this without this PR? Otherwise, I would still like this PR to be merged to enable adding data to the auth code (for reasons already mentioned above).