[keycloak-dev] Improvements on clientSession, code-to-token endpoint and refresh-token endpoint

[Marek Posolda]

There are few things, which can be improved with current implementation 
of authenticatedClientSessionModel, codeToToken endpoint and refresh 
token endpoint.

Here is some overview of the current issues:

1. The UserSessionModel currently contains just 1 
authenticatedClientSession per client.

The issue is, that if there are multiple browser tabs for same client 
application, they may not have same notes, protocolMappers and roles 
(for example if we limit the roles based on "scope" parameter, the roles 
can be different in browser tab1 and tab2 as every tab can have 
different scope parameter. This is not big issue now, since we don't 
have proper support for "scope" parameter, but could be in the future..)

2. The authenticatedClientSession is currently needed in code-to-token 
request. This is a bit of perf issue in cross-dc. The code-to-token 
request can happen in different DC then authentication. It will be ideal 
if code-to-token request doesn't need to wait until userSession with 
attached authenticatedClientSession is replicated from the second DC

3. There is just 1 valid code per authenticatedClientSession tracked in 
the attached note - in case that 2 user sends 2 concurrent requests to 
http://localhost:8080/customer-portal, it may happen that there are 2 
SSO logins concurrently issued, however just 1 of them will successfully 
pass the code-to-token request (clientSession note will be overwritten 
by the second request, so first request will fail in code-to-token 
request due the different note value)

4. Refresh token is signed by same key like accessToken. This is not 
ideal for:
- performance (RSA is an unecessary overhead. Symmetric cryptography is 
sufficient)
- Security: Refresh tokens and access tokens can be exchanged. Refresh 
token is JWT visible by application and end user (not direct security 
threat though)


Possible solution proposal:

- Move some stuff from authenticatedClientSessionModel to the code 
parameter and refreshToken itself.

- Code will be encrypted JWT containing just clientSession notes. Not 
the protocolMappers and roles

- Refresh token will contain both roles and protocolMappers (roles are 
here already. The protocolMappers will be new thing on refresh token)

- Both code and refreshToken are AES-128 encrypted. Just the keycloak 
server has the key and can decrypt them.

- AuthenticatedClientSessionModel won't contain protocolMappers and 
roles anymore

- Use single-use cache to track whether the particular value of code 
parameter was already used.


I've tried the code to be signed instead of encrypted, but it may have 
security implications (code would be visible by adapters and end-user). 
The roles and protocolMappers are not needed in code and keeping them 
would make the code unecessary big (URL limit is 2000 characters).

I've tried to add more details into the Google document: 
https://docs.google.com/document/d/1C1vFhyGPBOnN3pprw6XPZnK08azyTm-HVIqO9dY3aTY/edit?usp=sharing 
. Feel free to reply in the document or here in the email.

Marek