Problem/Motivation
I'm encountering a strange problem with a custom access callback on a Controller being cached indefinitely, once access is denied, despite the proper cache metadata being added.
How to reproduce
1. Create (or use an existing) content entity type.
2. Create a Controller that accepts an entity of that type and has a custom access callback with the following code (adapted to match a field that exists):
return AccessResult::allowedIf(!$entity->some_field->isEmpty())
->andIf($entity->access('view', NULL, TRUE))
->cachePerUser()
->addCacheableDependency($entity);
In this example, access is granted if the entity has a field populated, and if the user can view the entity.
3. Create a local action link that places a link to this route when viewing the canonical page of the given entity type.
4. Create an entity that has the given field populated, using a user that has access to view it.
5. View the entity and see that the local action link appears.
6. Edit the entity, removing the value from the field.
7. View the entity again and see that the link disappeared.
8. Edit the entity again, adding a value to the field.
9. View the entity again and see that the link is still missing. Repeat as many times as you want. It will not appear until the cache is cleared.
What is failing?
I dug in to the cache_render table and I believe the problem is in there. When access is granted, the [user]={uid} context is added to the cid and the {entity_type}:{entity_id} tag is added. Once access it forbidden, a new cache entry is added with both entirely removed. Since that continues to be served, altering the entity never clears it.
After the initial viewing of the link while access is granted, the cache entry is something like:
cid:
entity_view:block:seven_local_actions:[languages:language_interface]=en:[route]=entity.channel.canonical2cdbfd30db6c0a00042de607c691780171e151826e7af21973539625a592cef4:[theme]=seven:[user]=1
tags:
block_view channel:2 config:block.block.seven_local_actions config:user.role.authenticated rendered user:1
After access is denied, a new entry is created like:
cid:
entity_view:block:seven_local_actions:[languages:language_interface]=en:[route]=entity.channel.canonical2cdbfd30db6c0a00042de607c691780171e151826e7af21973539625a592cef4:[theme]=seven:[user.permissions]=959dfab36c6GpFyh8nJi-O9PTd_Pfkjuip-Mn_ad24OMSM_HPErIU
tags:
block_view config:block.block.seven_local_actions rendered
Comments
Comment #2
berdirI guess it won't fix the problem, although pretty sure that it should be passed up, but instead of calling access() and adding the cacheable metadata yourself, you should do something like this:
return AccessResult::allowedIf(!$entity->some_field->isEmpty())->andIf($entity->access('view', NULL, TRUE)->addCacheableDependency($entity).
(the call on $entity still makes sense because you are doing that check).
by combining the AccessResult response from the access call, you are collecting all the cacheablity metadata that that check involves while your current code hardcodes that assumption (that it is based on the current user).
Comment #3
mstef commentedThat's good advice and makes complete sense. I actually didn't even know andIf() existed. That's useful.
Unfortunately, as you guessed, it does not impact the issue though.
Comment #4
mstef commentedComment #5
berdirA failing test case would be helpful to investigate this and confirm that it is not related to something else that's specific to your site.
Comment #6
mstef commentedComment #7
dave reidI can confirm this as well. I also have a custom access callback on my local action controller which is dependent on a module config entity value not being empty. When it results in allowed, the entry in cache_render includes the cache tag of the config entity as expected, but when it is not allowed, the cache entry does not include the cache tag.
Comment #16
acbramley commentedThis is almost 6 years old with no additional comments, is this still a valid bug on 11.x?
Comment #17
acbramley commentedSince it's been 6+ months without a follow up for steps going to close out. If still an issue please reopen