sventorben / keycloak-restrict-client-auth

A Keycloak authenticator to restrict authorization on clients
MIT License
298 stars 22 forks source link

No error access-denied will be show in the result browser windows #160

Open unreal-asr opened 1 year ago

unreal-asr commented 1 year ago

Is there an existing issue for this?

Current Behavior

I setup a simple test case following the documentation with Keycloak v21.0.2. Everything seems to work fine playing with the policies, but when the configured policies deny access no access-denied error message is shown in the resulting browser page instead an error of "An internal server error has occurred" is shown. Follow the error stacktrace on the Keycloak logs...

2023-04-05 16:53:34,964 WARN [de.sventorben.keycloak.authorization.client.access.policy.PolicyBasedAccessProvider] (executor-thread-48) Access for user 'duck' is denied. User does not have permission to access resource 'Keycloak Client Resource' on client 'f90a0346-a98b-4fdd-a290-90e5ee520352' in realm 'bemsi'. 2023-04-05 16:53:34,965 ERROR [org.keycloak.services.error.KeycloakErrorHandler] (executor-thread-48) Uncaught server error: java.lang.NoSuchMethodError: 'org.keycloak.http.HttpRequest org.keycloak.authentication.AuthenticationFlowContext.getHttpRequest()' at de.sventorben.keycloak.authorization.client.RestrictClientAuthAuthenticator.errorResponse(RestrictClientAuthAuthenticator.java:87) at de.sventorben.keycloak.authorization.client.RestrictClientAuthAuthenticator.authenticate(RestrictClientAuthAuthenticator.java:50) at org.keycloak.authentication.DefaultAuthenticationFlow.processSingleFlowExecutionModel(DefaultAuthenticationFlow.java:446) at org.keycloak.authentication.DefaultAuthenticationFlow.processFlow(DefaultAuthenticationFlow.java:250) at org.keycloak.authentication.DefaultAuthenticationFlow.processSingleFlowExecutionModel(DefaultAuthenticationFlow.java:381) at org.keycloak.authentication.DefaultAuthenticationFlow.continueAuthenticationAfterSuccessfulAction(DefaultAuthenticationFlow.java:182) at org.keycloak.authentication.DefaultAuthenticationFlow.processAction(DefaultAuthenticationFlow.java:158) at org.keycloak.authentication.AuthenticationProcessor.authenticationAction(AuthenticationProcessor.java:977) at org.keycloak.services.resources.LoginActionsService.processFlow(LoginActionsService.java:311) at org.keycloak.services.resources.LoginActionsService.processAuthentication(LoginActionsService.java:282) at org.keycloak.services.resources.LoginActionsService.authenticate(LoginActionsService.java:274) at org.keycloak.services.resources.LoginActionsService.authenticateForm(LoginActionsService.java:339) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at org.jboss.resteasy.core.MethodInjectorImpl.invoke(MethodInjectorImpl.java:170) at org.jboss.resteasy.core.MethodInjectorImpl.invoke(MethodInjectorImpl.java:130) at org.jboss.resteasy.core.ResourceMethodInvoker.internalInvokeOnTarget(ResourceMethodInvoker.java:660) at org.jboss.resteasy.core.ResourceMethodInvoker.invokeOnTargetAfterFilter(ResourceMethodInvoker.java:524) at org.jboss.resteasy.core.ResourceMethodInvoker.lambda$invokeOnTarget$2(ResourceMethodInvoker.java:474) at org.jboss.resteasy.core.interception.jaxrs.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:364) at org.jboss.resteasy.core.ResourceMethodInvoker.invokeOnTarget(ResourceMethodInvoker.java:476) at org.jboss.resteasy.core.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:434) at org.jboss.resteasy.core.ResourceLocatorInvoker.invokeOnTargetObject(ResourceLocatorInvoker.java:192) at org.jboss.resteasy.core.ResourceLocatorInvoker.invoke(ResourceLocatorInvoker.java:141) at org.jboss.resteasy.core.ResourceLocatorInvoker.invoke(ResourceLocatorInvoker.java:32) at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:492) at org.jboss.resteasy.core.SynchronousDispatcher.lambda$invoke$4(SynchronousDispatcher.java:261) at org.jboss.resteasy.core.SynchronousDispatcher.lambda$preprocess$0(SynchronousDispatcher.java:161) at org.jboss.resteasy.core.interception.jaxrs.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:364) at org.jboss.resteasy.core.SynchronousDispatcher.preprocess(SynchronousDispatcher.java:164) at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:247) at io.quarkus.resteasy.runtime.standalone.RequestDispatcher.service(RequestDispatcher.java:73) at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.dispatch(VertxRequestHandler.java:151) at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.handle(VertxRequestHandler.java:82) at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.handle(VertxRequestHandler.java:42) at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1284) at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:173) at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:140) at io.quarkus.vertx.http.runtime.StaticResourcesRecorder$2.handle(StaticResourcesRecorder.java:84) at io.quarkus.vertx.http.runtime.StaticResourcesRecorder$2.handle(StaticResourcesRecorder.java:71) at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1284) at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:173) at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:140) at io.quarkus.vertx.http.runtime.VertxHttpRecorder$6.handle(VertxHttpRecorder.java:430) at io.quarkus.vertx.http.runtime.VertxHttpRecorder$6.handle(VertxHttpRecorder.java:408) at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1284) at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:173) at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:140) at org.keycloak.quarkus.runtime.integration.web.QuarkusRequestFilter.lambda$createBlockingHandler$0(QuarkusRequestFilter.java:82) at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:576) 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:833)

Expected Behavior

Default access denied error message.

Steps To Reproduce

No response

Version

No response

Anything else?

No response

sventorben commented 1 year ago

Thanks for reporting this, @unreal-altran. Which version of this extension do you use?

unreal-asr commented 1 year ago

The last one v21.0.0 (keycloak-restrict-client-auth.jar)

unreal-asr commented 1 year ago

I deleted the default policy (js type) from keycloack and now it seems to work. I don't have the script engine enabled so I assume that was the problem. @sventorben
I take this opportunity to ask you if you believe it is possible to make this plugin work even with non-browser flows but only rest api (bearer_only: true). Is it necessary to create a dedicated flow?

sventorben commented 1 year ago

Are you referring to a direct grant flow? That should already work. You will have to add a new flow, configure it to your needs and then bind it to the direct grant flow or use authentication flow overrides for dedicated clients.

unreal-asr commented 1 year ago

@sventorben, Yes I confirm that direct access also works correctly. I add an example image of direct flow , modified and tested. I take the liberty of advising you to explicitly add this operating case to the documentation of your work. I think it is very useful for those approaching the keycloak-restrict-client-auth for the first time. You can reuse my image too :-) If yuo do not have permission, the Json rest response from https:///realms//protocol/openid-connect/token) will be: {"error":"access-denied","error_description":"Access to client is denied."}

Furthermore, always in the documentation ( maybe as a note) I would add an advise to eliminate the JS-type default policy if you do not have an operational JS engine on keyclock server. (See the problem described above)

image

sventorben commented 1 year ago

@unreal-altran Are you sure your problem was related to the Default Policy? I just tried that and got a very explicit exception in the logs:

Caused by: java.lang.IllegalStateException: Could not find ScriptEngine for script: Script{id='null', realmId='test-realm', name='Default Policy', type='text/javascript', code='// by default, grants any permission associated with this policy
$evaluation.grant();
', description='A policy that grants access only for users within this realm'}

That's different to the exception you got. Any chance you can reproduce the original error and share your setup/configuratoin?

unreal-asr commented 1 year ago

@sventorben Yes you are right! I think my error is due to some incorrect configuration due to the various attempts I was carrying out. Starting from scratch I got your same error.

However I have a doubt. Currently, from various attempts, I understand that once a user has obtained a token from a client where he has a policy that authorizes him, he also has access to the other clients for which he does not have access in the same realm. Perhaps this is due to the fact that the check is performed only when the token is requested ! ? I'm currently using ApiSix with the openid-connect plugin and the Browser flow on keycloak!

Thanks to the availability.

sventorben commented 1 year ago

However I have a doubt. Currently, from various attempts, I understand that once a user has obtained a token from a client where he has a policy that authorizes him, he also has access to the other clients for which he does not have access in the same realm.

If a user has obtained a token for a client that he has access to and tries to use it for getting access to another client, the client should reject the request. It is the responsibility of the client to validate the token. The client should check the audience (aud) claim in the token in this case and reject the token if the client is not (part of) the audience. That said, ensure that your protocol mappers are configured correctly. Keycloak has an audience resolver which may add additional audiences, if the user has roles granted on other clients. It may not be what you want in your case and you may need to disable it.

But it's a good point you have here. I should add that as a warning in the docs.

unreal-asr commented 1 year ago

@sventorben yes, it was clear to me that by moving all the controls to the client development side it is possible to verify everything. Again as an open discussion my doubt is if it weren't the case that in "plugin" as keycloak-restrict-client-auth which implement a policy enforcement point (PEP) internal to keycloak it was also advisable to check the case of cross keycloak-client accesses once a token is obtained for a specific keycloak-client inside the same realm.

sventorben commented 1 year ago

To avoid any confusion, it is important to note that keycloak-restrict-client-auth is not implementing a policy enforcement point (PEP). It does not enforce authorization decisions. It is part of making the decision, but it does not enforce it. The client must enforce the decision being made.

According to the OIDC specification/OpenID Connect Core 1.0 incorporating errata set 1, it is the responsibility of the client to check the audience claims. Section 3.1.3.7. ID Token validation, point 3ff state that

The Client MUST validate that the aud (audience) Claim contains its client_id value registered at the Issuer identified by the iss (issuer) Claim as an audience. [...]

And further it is very clear about the enforcement by the client:

The ID Token MUST be rejected if the ID Token does not list the Client as a valid audience, or if it contains additional audiences not trusted by the Client. If the ID Token contains multiple audiences, the Client SHOULD verify that an azp Claim is present. If an azp (authorized party) Claim is present, the Client SHOULD verify that its client_id is the Claim Value.

So, by design of the protocol it is not possible to implement what you are suggesting. It is also the reason why Keycloak client-side adapters (now deprecated) had a policy-enforcer (which is a separate module since version 21.1) though policy definition aka Authorization Services are mainly a server-side feature.

What this extensions supports you with is decision making and communicating the outcome to the user during login/authentication. If configured correctly (see my remarks in https://github.com/sventorben/keycloak-restrict-client-auth/issues/160#issuecomment-1513681621 and security considerations in README) it will prevent issuance of tokens that contain an audience or authorized party claim for a client that users do not have access to.

Token issuance or non-issuance is (beside claims in the token) just one way to communicate the outcome of decision making. It is not the enforcement of the decision.

WolfgangRoth commented 3 months ago

I am experiencing the same error with Keycloak version 20.0.5 and the extension version 24.0.0 when using role-based mode. Can you please advise?