Repository navigation
PolicyEnforcer sometimes looses provided adapterconfig while executing resource matching #9855
Description
Describe the bug
We use Keycloak policy enforcement in Quarkus microservices. Some resources we disable for policy enforcement using the application.properties. Sample:
quarkus.keycloak.policy-enforcer.paths.health.name=Health Check
quarkus.keycloak.policy-enforcer.paths.health.path=${quarkus.http.root-path}/q/health/*
quarkus.keycloak.policy-enforcer.paths.health.enforcement-mode=DISABLED
It seems that those configurations sometimes is lost. This seems to be the case because we sometimes experience 401/403 errors based on those resources even though they are explicitly excluded. It is not reproducible. While debugging the issue and checking the code I found the following which I assume could result in the problem which we are facing and hope you can help me analyzing further or maybe come up with a fix.
I think the logic in the matches method of the PathConfigMatcher could lead to the above described situation. There are a few things which seem like possible causes:
1. Removing entries from paths variable in
My understanding of that section is that in case a cache entry expires it will be removed from the paths instance variable. That variable actually contains the initial list of entries which have been provided in the adapterConfig (from application.properties). I can't think of any situation in which this should happen. So I think this could be the reason for the behaviour which we are observing.
2. Different behaviour depending on * being in the path of the configuration or not
If we provide a configuration in the application.properties it can happen that this configuration is being overriden by a resource in Keycloak. This is the case when the path contains a *. It will go into the section because there is * in the path and then retrieve the resouce from Keycloak. If something is found there that takes precedence over the configuration in the paths instance variable. In the case if the original configuration in application.properties would not contain a * in the path no request to Keycloak would be issued and therefore the entry in application.properties takes precedence. And in this case also the entry would never be removed from the paths variable since the removal is within the if statement.
3. Potentially a multithreading issue
In the case that there is no entry in the cache when line 261 is being executed in thread A, but then in a different thread something is put into the cache with that key and now context switches again to thread A while executing line 263 the containsKey check will be true and null will be returned.
Version
15.0.2 (but is same in current version)
Expected behavior
Entries in the adapaterConfig which are provided to PolicyEnforcer should always be present and never get lost. Entries in adapterConfig should always take precedence over Keycloak resources.
Actual behavior
Entries in the adapaterConfig which are provided to PolicyEnforcer are being removed in certain circumstances. Entries in adapterConfig with a * can be overriden by Keycloak resources.
How to Reproduce?
I do not have a reproducing sample. The above description is based on my understanding of the code which might be wrong. I think to make it happen in real world it is some kind of multithreading issue which I was not able to reproduce. But it happens every now and then in our application and I hope you can help to identify the root cause even if above description and understanding of the code is not accurate.
Anything else?
No response
Activity
pedroigor commentedon Aug 11, 2022
@albert0815 First of all, sorry for the delay. You're right about the #1 assumption.
However, this only happens if the path you are configuring does not point a existent resource in Keycloak. For instance:
In any case, the change here makes sure that disabled paths are never invalidated even if they don't map to an existing resource.
Avoid removing static path config from cache
Avoid removing static path config from cache
Avoid removing static path config from cache
Avoid removing static path config from cache (#14230)