Hi,

Keycloak has some non-standard behavior that we need to support: TheKEYCLOAK-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 customclient_session_state=${sessionId} POST parameter is added when calling thetoken endpoint to exchange a code for a token.

TheOAuth20Service service currently does not provide any means to add custom parameters to any of thegetAccessToken() 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 inOAuth2AccessToken

If the threesendAccessTokenRequest* methods andcreateAccessTokenRequest 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 newKeycloakService implementspublic OAuth2AccessToken getAccessToken*(..., String clientSessionState) methods

ModifyKeycloakApi to override methodpublic OAuth20Service createService so it creates aKeycloakService (like e.g.FacecbookApi creates aFacebookService).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 makesgetAccessToken different from e.g. therevokeToken family in that respect that "only" has 6 variations.

Also,ServiceBuilderOAuth20'sbuild() method returns aOAuth20Service so the application would have to cast the result ofbuild() toKeycloakService to access these 6 new methods. :-(

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

Here instead of creating a customKeycloakService 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 ownOurKeycloakApi that returns anOurKeycloakService 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.