Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task done
unreal-asr opened this issue Apr 5, 2023 · 10 comments
Open
1 task done
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@unreal-asr
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

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

@unreal-asr unreal-asr added the bug Something isn't working label Apr 5, 2023
@sventorben
Copy link
Owner

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

@sventorben sventorben self-assigned this Apr 5, 2023
@unreal-asr
Copy link
Author

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

@unreal-asr
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

unreal-asr commented Apr 7, 2023

@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://<k_host>/realms/<custom_realm>/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 sventorben added documentation Improvements or additions to documentation and removed bug Something isn't working labels Apr 7, 2023
@sventorben
Copy link
Owner

@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
Copy link
Author

unreal-asr commented Apr 18, 2023

@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
Copy link
Owner

sventorben commented Apr 18, 2023

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.

sventorben added a commit that referenced this issue Apr 18, 2023
@unreal-asr
Copy link
Author

@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
Copy link
Owner

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 #160 (comment) 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants