Keycloak: Adding a custom field for the token endpoint POST to get Single-Sign-Off to work #980

Hi,

Keycloak has some non-standard behavior that we need to support: The KEYCLOAK-15234 bug report is about Single-Sign-Off not working with non-Keycloak adapters (like scribejava). Keycloak allows clients to register an "Admin URL" that gets called when users log off. Unfortunately, it only works when a custom client_session_state=${sessionId} POST parameter is added when calling the token endpoint to exchange a code for a token.

The OAuth20Service service currently does not provide any means to add custom parameters to any of the getAccessToken() methods.

The question is whether we should provide a PR for this project or work around it in application code. We prefer preparing a PR and I would like to hear the project's opinion on how to proceed. We're happy to provide a PR, but would like the approach to be approved beforehand, so we don't waste effort and end up with a fork that never gets merged. There are at least a couple of ways this could be made possible:

Change protected -> public for 4 methods in OAuth2AccessToken

If the three sendAccessTokenRequest* methods and createAccessTokenRequest were changed from protected to public, then instead of:

OAuth2AccessToken accessToken = service.getAccessToken(code)

our application code could now:

OAuthRequest request = service.createAccessTokenRequest(AccessTokenRequestParams.create(code));
request.addParameter("client_session_state", sessionId)
OAuth2AccessToken accessToken = service.sendAccessTokenRequestSync(request)

In my opinion this the cleanest, most flexible and most elegant solution, but there is also:

A new KeycloakService implements public OAuth2AccessToken getAccessToken*(..., String clientSessionState) methods

Modify KeycloakApi to override method public OAuth20Service createService so it creates a KeycloakService (like e.g. FacecbookApi creates a FacebookService). KeycloakService then provides a (number of) public OAuth2AccessToken getAccessToken(String code, String clientSessionState) method(s) that add the extra parameter.

There are already 6 methods with various parameter and sync/async combinations. Should there now be 12? The existing 6 plus versions with the extra parameter? This is a maintainability nightmare... It also makes getAccessToken different from e.g. the revokeToken family in that respect that "only" has 6 variations.

Also, ServiceBuilderOAuth20's build() method returns a OAuth20Service so the application would have to cast the result of build() to KeycloakService to access these 6 new methods. :-(

OAuth2AccessToken gets new getAccessToken(..., Map<String, String> extraParameters) methods

Here instead of creating a custom KeycloakService we add more generic methods to the base class. Again, we'll end up with 12 methods instead of the current 6.

I don't like this for the same maintainability reasons.

Application Level Workaround - no scribejava changes

Essentially we derive from KeycloakApi to just provide strictly what we need:

We Implement our own OurKeycloakApi that returns an OurKeycloakService that implements the one extra method we need. That is for sure the easiest for us to do, but it doesn't help out the next Keycloak user that runs into this problem.

We'll do this unless there is consensus here on what a PR should look like. We just think it is a shame others have to do similar customizations to get logging out to work properly.