Keycloak policy enforcer returns 403 in case an error occurred while retrieving resource from Keycloak

[albert0815]

Describe the bug

Currently the Keycloak policy enforcement returns a 403 return code in case a request is sent to the Quarkus endpoint and the token of the user is not known in Keycloak. This could happen if Keycloak was restarted in the meantime. Returning a 403 in this case seems wrong because that means "permission denied".

Expected behavior

I would think that returning 401 (unauthorized) would be more approriate in this case because the user should authenticate again. This is also the behaviour which we can see in the Keycloak admin web application.

Actual behavior

403 is returned.

How to Reproduce?

Start the quickstarter "security-keycloak-authorization-quickstart":

mvn quarkus:dev

Now open OpenId Connect Dev UI. You will be asked to login into a Single Page Application. Log in as alice:alice - accessing the /api/users/me will return 200. Now stop the Keycloak Docker container manually which was started by Quarkus Dev Service and start it again. Now again access the /api/users/me (using the same token which was aquired before Keycloak restart). This will return a 403 which should be a 401.

Output of uname -a or ver

Linux PC49409 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "11.0.13"

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.6.3.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.6.3

Additional information

No response

[quarkus-bot[bot]]

/cc @pedroigor, @sberyozkin

[sberyozkin]

Hi @albert0815 IMHO this issue is invalid, quarkus-keycloak-authorization only deals with authorizing the token which has already been verified by quarkus-oidc - which will return 401 should the token proved to be invalid. If, the authorization decision can not be completed, for whatever reasons, it is only 403 at this stage. Perhaps 500 even more appropriate, but I'd stick with 403.

@pedroigor do you agree ?

[albert0815]

I can see your point. The token validation in Quarkus was successful. But obviously the token validation failed in Keycloak:

12:13:10,087 WARN  [org.keycloak.events] (default task-1) type=PERMISSION_TOKEN_ERROR, realmId=11d78bf6-6d10-4484-baba-a1388379d68b, clientId=backend-service, userId=null, ipAddress=172.17.0.1, error=invalid_token, reason='HTTP 500 Internal Server Error', auth_method=oauth_credentials, audience=backend-service, grant_type=urn:ietf:params:oauth:grant-type:uma-ticket, permission=df1b74a9-3f10-499d-a581-368de48e512b, client_auth_method=client-secret

I was also thinking that maybe 500 would be a good return code in this case.

The reason why I am raising this as a bug and suggesting a 401: The only way to get out of this situation is to reauthenticate. So the client actually can solve this situation by receiving a new token from Keycloak. Afterwards the request will succeed. 403 doesn't indicate this (and 500 also doesn't give a hint for the client to login again). For me this is a real world problem: We have a Keycloak deployed into Kubernetes. Because of autoscaling Keycloak sometimes is being relocated to a different node. In this case all user sessions are not available in Keycloak anymore and our users experience this problem. The frontend which is build on top of the services which we build with Quarkus shows an error based on the 403 which is "you do not have permission for this activity". But what it should do is to redirect to the login form. It would do so if the service returns 401. The frontend currently would not know how to differentiate between 403 because Keycloak was restarted or 403 because there is actually no permission.

[sberyozkin]

@albert0815 Sure, but that validation failure because the token does not align with the current restarted Keycloak state, and it is as part of the authorization decision process. Token signature is valid, the keys fetched from the Keycloak are used to verify. I don't think it is even technically possible, how can Quarkus KeycloakPolicyEnforcerAuthorizer know what is the reason behind PERMISSION_TOKEN_ERROR.

IMHO, this situation, where the frontend state (the token it has acquired from KC, etc) goes out of sync due to Keycloak restarting in between, should not be resolved at the keycloak-authorization level as it can also happen even if keycloak-authorization is not used - i.e, even if come up with some workaround, the problem would still remain unsolved in general.

Perhaps the frontend should monitor Keycloak and request new tokens when it detects a restart - by sending the health pings.

You can also try to enforce 401 by configuring quarlus-oidc to always do the Keycloak JWT token introspection, i.e, do not use the locally cached keys: https://quarkus.io/guides/security-openid-connect#token-verification-introspection.

[albert0815]

I think KeycloakPolicyEnforcerAuthorizer could be able to identify this situation. This is the exception which seems to lead to the situation:

2022-01-26 15:34:30,802 DEBUG [org.key.ada.aut.KeycloakAdapterPolicyEnforcer] (executor-thread-10) Authorization failed: java.lang.RuntimeException: org.keycloak.authorization.client.util.HttpResponseException: Unexpected response from server: 400 / Bad Request / Response from server: {"error":"unauthorized_client","error_description":"Invalid identity"}
        at org.keycloak.authorization.client.util.Throwables.handleAndWrapHttpResponseException(Throwables.java:99)
        at org.keycloak.authorization.client.util.Throwables.handleWrapException(Throwables.java:42)
        at org.keycloak.authorization.client.util.Throwables.retryAndWrapExceptionIfNecessary(Throwables.java:65)
        at org.keycloak.authorization.client.resource.AuthorizationResource.authorize(AuthorizationResource.java:98)
        at org.keycloak.adapters.authorization.KeycloakAdapterPolicyEnforcer.requestAuthorizationToken(KeycloakAdapterPolicyEnforcer.java:167)
        at org.keycloak.adapters.authorization.KeycloakAdapterPolicyEnforcer.isAuthorized(KeycloakAdapterPolicyEnforcer.java:66)
        at org.keycloak.adapters.authorization.AbstractPolicyEnforcer.authorize(AbstractPolicyEnforcer.java:121)
        at io.quarkus.keycloak.pep.runtime.KeycloakPolicyEnforcerAuthorizer.apply(KeycloakPolicyEnforcerAuthorizer.java:64)
        at io.quarkus.keycloak.pep.runtime.KeycloakPolicyEnforcerAuthorizer_Subclass.apply$$superforward1(Unknown Source)
        at io.quarkus.keycloak.pep.runtime.KeycloakPolicyEnforcerAuthorizer_Subclass$$function$$2.apply(Unknown Source)
        at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:54)
        at io.quarkus.arc.runtime.devconsole.InvocationInterceptor.proceed(InvocationInterceptor.java:62)
        at io.quarkus.arc.runtime.devconsole.InvocationInterceptor.monitor(InvocationInterceptor.java:51)
        at io.quarkus.arc.runtime.devconsole.InvocationInterceptor_Bean.intercept(Unknown Source)
        at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:41)
        at io.quarkus.arc.impl.AroundInvokeInvocationContext.perform(AroundInvokeInvocationContext.java:41)
        at io.quarkus.arc.impl.InvocationContexts.performAroundInvoke(InvocationContexts.java:32)
        at io.quarkus.keycloak.pep.runtime.KeycloakPolicyEnforcerAuthorizer_Subclass.apply(Unknown Source)
        at io.quarkus.keycloak.pep.runtime.KeycloakPolicyEnforcerAuthorizer.apply(KeycloakPolicyEnforcerAuthorizer.java:28)
        at io.quarkus.vertx.http.runtime.security.HttpAuthorizer$1$1$1.run(HttpAuthorizer.java:78)
        at io.quarkus.vertx.core.runtime.VertxCoreRecorder$13.runWith(VertxCoreRecorder.java:543)
        at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2449)
        at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1478)
        at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
        at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
        at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
        at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: org.keycloak.authorization.client.util.HttpResponseException: Unexpected response from server: 400 / Bad Request / Response from server: {"error":"unauthorized_client","error_description":"Invalid identity"}
        at org.keycloak.authorization.client.util.HttpMethod.execute(HttpMethod.java:95)
        at org.keycloak.authorization.client.util.HttpMethodResponse$2.execute(HttpMethodResponse.java:50)
        at org.keycloak.authorization.client.resource.AuthorizationResource$1.call(AuthorizationResource.java:92)
        at org.keycloak.authorization.client.resource.AuthorizationResource$1.call(AuthorizationResource.java:74)
        at org.keycloak.authorization.client.resource.AuthorizationResource.authorize(AuthorizationResource.java:96)
        ... 23 more

So for me it seems that the response which Keycloak returns to the PolicyEnforcer contains the information that this is now an error situation which actually means that this is not a 403 (and maybe would be a hint to return a 500).

Anyhow I can understand your point of view. Thanks for the other hints, I will take a look at that. Hopefully in Keycloak X user sessions will survive a Keycloak restart out of the box. All in all I think that this is actually the root cause of the problem and only was hoping that you would consider providing a workaround in the keycloak authorization component since I still think that the handling of error responses from Keycloak while retrieving a resource is not ideal. But since I have the impression you do not agree with that let's close this issue :).

[sberyozkin]

@albert0815 I see, well even if I disagree (though now I'm hesitating :-) ), I'd let Pedro make a decision. All that code you pasted is not done at the Quarkus level, I've looked at the code, my understanding that the only way to intercept it at the Quarkus level is to extend protected KeycloakAdapterPolicyEnforcer.handleAccessDenied (as we control the instantiation of KeycloakAdapterPolicyEnforcer):

@Override
    protected void handleAccessDenied(OIDCHttpFacade facade) {
        String accessDeniedPath = getEnforcerConfig().getOnDenyRedirectTo();
        HttpFacade.Response response = facade.getResponse();

        if (accessDeniedPath != null) {
            response.setStatus(302);
            response.setHeader("Location", accessDeniedPath);
        } else {
            response.sendError(403);
        }
    }

I'm not seeing right now where, in OIDCHttpFacade, we can get to {"error":"unauthorized_client","error_description":"Invalid identity"}, since OIDCHttpFacade is only a wrapper around the current RoutingContext.

@albert0815 I think you may need to open a Keycloak issue to make the cause be available to OIDCHttpFacade. Please do it and hopefully, in some time, we can do something about it.

That said, IMHO, as I've already suggested, the better strategy is not depend on keycloak-authorization which is only a single point of entry. The frontend which will not use it will miss a KC restart anyway. Unless the frontend, in both cases is doing a health check in a background thread or the endpoint is insisting on the remote token introspection during the local token verification phase.

[sberyozkin]

@pedroigor Hey Pedro, by the way, I think we need to extend that code anyway and return 403, I believe we don't want 302 in scope of the quarkus endpoint as we can be dealing with the bearer token, I can open a PR if you agree ? thanks

[sberyozkin]

@pedroigor Or if handleAccessDenied is called only in case of the "error":"unauthorized_client" error, then we can just return 401.

[samodadela]

@sberyozkin @pedroigor Did you manage to resolve the issue?