From c34220579eff30ba4dd7f733cf6241888b78e2ee Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 21 Oct 2025 12:17:36 -0300 Subject: [PATCH 1/5] Prevent page_cache caching where an `ip` cache context is relevant for access control. --- embargo.services.yml | 4 ++ src/PageCache/DenyIpDependentResponse.php | 49 +++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 src/PageCache/DenyIpDependentResponse.php diff --git a/embargo.services.yml b/embargo.services.yml index 4bcd728..f82ce3e 100644 --- a/embargo.services.yml +++ b/embargo.services.yml @@ -69,3 +69,7 @@ services: - '@entity_type.manager' tags: - { name: 'cache.context' } + embargo.page_cache_request_policy.deny_ip_dependent_response: + class: Drupal\embargo\PageCache\DenyIpDependentResponse + tags: + { name: page_cache_response_policy } diff --git a/src/PageCache/DenyIpDependentResponse.php b/src/PageCache/DenyIpDependentResponse.php new file mode 100644 index 0000000..3bb0224 --- /dev/null +++ b/src/PageCache/DenyIpDependentResponse.php @@ -0,0 +1,49 @@ +attributes->has(AccessAwareRouterInterface::ACCESS_RESULT)) { + // No access result; unable to check. + return NULL; + } + + /** @var \Drupal\Core\Access\AccessResultInterface $access_result */ + $access_result = $request->attributes->get(AccessAwareRouterInterface::ACCESS_RESULT); + + if (!($access_result instanceof RefinableCacheableDependencyInterface)) { + // Access result is not cacheable; unable to check cache contexts. + return NULL; + } + + $cache_contexts = $access_result->getCacheContexts(); + + if (array_intersect(['ip', 'ip.embargo_range'], $cache_contexts)) { + // Access result has relevant context; avoiding page cache. + return ResponsePolicyInterface::DENY; + } + + // No candidate IP cache contexts present on access result; passing. + return NULL; + } + +} From 4a75b1faf0746a4856523cd2342ac01c60122801 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 21 Oct 2025 12:28:09 -0300 Subject: [PATCH 2/5] Refactor out to a constant. Should allow for some better documentation on the value(s). --- src/PageCache/DenyIpDependentResponse.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/PageCache/DenyIpDependentResponse.php b/src/PageCache/DenyIpDependentResponse.php index 3bb0224..e0f45f5 100644 --- a/src/PageCache/DenyIpDependentResponse.php +++ b/src/PageCache/DenyIpDependentResponse.php @@ -18,6 +18,16 @@ */ class DenyIpDependentResponse implements ResponsePolicyInterface { + /** + * Cache contexts of which the presence should suppress page_cache. + */ + protected const IP_CONTEXTS = [ + 'ip.embargo_range', + // XXX: `ip.embargo_range` could be optimized away if the `ip` context + // itself is added, so let's also account for it. + 'ip', + ]; + /** * {@inheritDoc} */ @@ -37,7 +47,7 @@ public function check(Response $response, Request $request) : ?string { $cache_contexts = $access_result->getCacheContexts(); - if (array_intersect(['ip', 'ip.embargo_range'], $cache_contexts)) { + if (array_intersect(static::IP_CONTEXTS, $cache_contexts)) { // Access result has relevant context; avoiding page cache. return ResponsePolicyInterface::DENY; } From 6facd099a007048711b8ab30101610fcd5fbd2f2 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 21 Oct 2025 12:31:34 -0300 Subject: [PATCH 3/5] Misc linting issues. --- .../src/Plugin/migrate/source/Entity.php | 2 +- src/Controller/IpRangeAccessExemptionController.php | 2 +- src/Plugin/search_api/processor/EmbargoJoinProcessor.php | 2 +- src/Plugin/search_api/processor/EmbargoProcessor.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/migrate_embargoes_to_embargo/src/Plugin/migrate/source/Entity.php b/modules/migrate_embargoes_to_embargo/src/Plugin/migrate/source/Entity.php index 9ba8ddc..69d2d42 100644 --- a/modules/migrate_embargoes_to_embargo/src/Plugin/migrate/source/Entity.php +++ b/modules/migrate_embargoes_to_embargo/src/Plugin/migrate/source/Entity.php @@ -56,7 +56,7 @@ public static function create( array $configuration, $plugin_id, $plugin_definition, - MigrationInterface $migration = NULL, + ?MigrationInterface $migration = NULL, ) { return new static( $configuration, diff --git a/src/Controller/IpRangeAccessExemptionController.php b/src/Controller/IpRangeAccessExemptionController.php index 03e04fe..f93b1b5 100644 --- a/src/Controller/IpRangeAccessExemptionController.php +++ b/src/Controller/IpRangeAccessExemptionController.php @@ -25,7 +25,7 @@ class IpRangeAccessExemptionController extends ControllerBase { * @param \Symfony\Component\HttpFoundation\Request|null $request * The current request. */ - public function __construct(Request $request = NULL) { + public function __construct(?Request $request = NULL) { $this->request = $request; } diff --git a/src/Plugin/search_api/processor/EmbargoJoinProcessor.php b/src/Plugin/search_api/processor/EmbargoJoinProcessor.php index 4083457..97718d5 100644 --- a/src/Plugin/search_api/processor/EmbargoJoinProcessor.php +++ b/src/Plugin/search_api/processor/EmbargoJoinProcessor.php @@ -104,7 +104,7 @@ public static function supportsIndex(IndexInterface $index) { /** * {@inheritdoc} */ - public function getPropertyDefinitions(DatasourceInterface $datasource = NULL) : array { + public function getPropertyDefinitions(?DatasourceInterface $datasource = NULL) : array { $properties = []; if ($datasource === NULL) { diff --git a/src/Plugin/search_api/processor/EmbargoProcessor.php b/src/Plugin/search_api/processor/EmbargoProcessor.php index 6d31b28..5f9d3bf 100644 --- a/src/Plugin/search_api/processor/EmbargoProcessor.php +++ b/src/Plugin/search_api/processor/EmbargoProcessor.php @@ -88,7 +88,7 @@ public static function create(ContainerInterface $container, array $configuratio /** * {@inheritdoc} */ - public function getPropertyDefinitions(DatasourceInterface $datasource = NULL) : array { + public function getPropertyDefinitions(?DatasourceInterface $datasource = NULL) : array { $properties = []; if ($datasource === NULL) { From 742bbd3f42252c67c89e96f3abc6d2f9bd05081e Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 21 Oct 2025 13:51:31 -0300 Subject: [PATCH 4/5] Fix list syntax. --- embargo.services.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/embargo.services.yml b/embargo.services.yml index f82ce3e..f829a59 100644 --- a/embargo.services.yml +++ b/embargo.services.yml @@ -72,4 +72,4 @@ services: embargo.page_cache_request_policy.deny_ip_dependent_response: class: Drupal\embargo\PageCache\DenyIpDependentResponse tags: - { name: page_cache_response_policy } + - { name: page_cache_response_policy } From d683061ec6ca6018f3fd62b36e387ebb68480c07 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 21 Oct 2025 14:04:52 -0300 Subject: [PATCH 5/5] Simplify request access. Controllers support having it passed automatically, so let's leverage it. --- .../IpRangeAccessExemptionController.php | 35 ++++--------------- 1 file changed, 6 insertions(+), 29 deletions(-) diff --git a/src/Controller/IpRangeAccessExemptionController.php b/src/Controller/IpRangeAccessExemptionController.php index f93b1b5..66d94d3 100644 --- a/src/Controller/IpRangeAccessExemptionController.php +++ b/src/Controller/IpRangeAccessExemptionController.php @@ -4,7 +4,6 @@ use Drupal\Core\Cache\Cache; use Drupal\Core\Controller\ControllerBase; -use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Request; /** @@ -12,42 +11,20 @@ */ class IpRangeAccessExemptionController extends ControllerBase { - /** - * The HTTP request. - * - * @var \Symfony\Component\HttpFoundation\Request - */ - protected $request; - - /** - * Constructs an IP access denied controller. - * - * @param \Symfony\Component\HttpFoundation\Request|null $request - * The current request. - */ - public function __construct(?Request $request = NULL) { - $this->request = $request; - } - - /** - * {@inheritdoc} - */ - public static function create(ContainerInterface $container) { - return new static( - $container->get('request_stack')->getCurrentRequest()); - } - /** * Formats a response for an IP access denied page. * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request being served. + * * @return array * Renderable array of markup for IP access denied. */ - public function response() { + public function response(Request $request) : array { $ranges = []; $cache_tags = []; /** @var \Drupal\embargo\IpRangeInterface[] $entities */ - $entities = $this->entityTypeManager()->getStorage('embargo_ip_range')->loadMultiple($this->request->query->all()['ranges'] ?? []); + $entities = $this->entityTypeManager()->getStorage('embargo_ip_range')->loadMultiple($request->query->all()['ranges'] ?? []); foreach ($entities as $entity) { $ranges[] = [ 'label' => $entity->label(), @@ -58,7 +35,7 @@ public function response() { return [ '#theme' => 'embargo_ip_access_exemption', - '#resources' => $this->request->query->all()['resources'] ?? [], + '#resources' => $request->query->all()['resources'] ?? [], '#ranges' => $ranges, '#contact_email' => $this->config('embargo.settings')->get('contact_email'), '#cache' => [