From 319adaf87ee4a4c9ccd483a3858c8bc14580331d Mon Sep 17 00:00:00 2001 From: Axel Rutz Date: Tue, 23 Apr 2019 23:05:21 +0200 Subject: [PATCH] Issue #2824640 by sardara, idimopoulos, beram, drunkan monkey, axel.rutz: Views cached results are not taking into account the access check --- src/Datasource/DatasourceInterface.php | 35 ++++++++++++++++ src/Datasource/DatasourcePluginBase.php | 11 ++++- src/Item/Item.php | 32 +++++++++++++- src/Item/ItemInterface.php | 27 ++++++++++++ .../search_api/datasource/ContentEntity.php | 34 +++++++++++++-- .../views/cache/SearchApiCachePluginTrait.php | 4 +- src/Plugin/views/query/SearchApiQuery.php | 42 ++++++++++++++++++- src/Query/ResultSet.php | 13 ++++++ src/Query/ResultSetInterface.php | 11 +++++ 9 files changed, 199 insertions(+), 10 deletions(-) diff --git a/src/Datasource/DatasourceInterface.php b/src/Datasource/DatasourceInterface.php index 5cd4ed1a..20fa7b14 100644 --- a/src/Datasource/DatasourceInterface.php +++ b/src/Datasource/DatasourceInterface.php @@ -137,9 +137,28 @@ interface DatasourceInterface extends IndexPluginInterface { * * @return bool * TRUE if access is granted, FALSE otherwise. + * + * @deprecated in search_api:8.x-1.13 and will be removed from + * search_api:9.x-1.0. Use checkItemAccessAsObject() instead. + * + * @see @todo link to change record (TBD) */ public function checkItemAccess(ComplexDataInterface $item, AccountInterface $account = NULL); + /** + * Checks whether a user has permission to view the given item. + * + * @param \Drupal\Core\TypedData\ComplexDataInterface $item + * An item of this datasource's type. + * @param \Drupal\Core\Session\AccountInterface|null $account + * (optional) The user session for which to check access, or NULL to check + * access for the current user. + * + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. + */ + public function checkItemAccessAsObject(ComplexDataInterface $item, AccountInterface $account = NULL); + /** * Returns the available view modes for this datasource. * @@ -238,4 +257,20 @@ interface DatasourceInterface extends IndexPluginInterface { */ public function getFieldDependencies(array $fields); + /** + * Get item cacheability. + * + * @param \Drupal\Core\TypedData\ComplexDataInterface $item + * + * @return \Drupal\Core\Cache\CacheableMetadata + */ + public function getItemCacheability(ComplexDataInterface $item); + + /** + * Get list cacheability. + * + * @return \Drupal\Core\Cache\CacheableMetadata + */ + public function getListCacheability(); + } diff --git a/src/Datasource/DatasourcePluginBase.php b/src/Datasource/DatasourcePluginBase.php index e6cfea03..ea296365 100644 --- a/src/Datasource/DatasourcePluginBase.php +++ b/src/Datasource/DatasourcePluginBase.php @@ -2,6 +2,7 @@ namespace Drupal\search_api\Datasource; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Language\Language; use Drupal\Core\Session\AccountInterface; use Drupal\Core\TypedData\ComplexDataInterface; @@ -97,7 +98,15 @@ abstract class DatasourcePluginBase extends IndexPluginBase implements Datasourc * {@inheritdoc} */ public function checkItemAccess(ComplexDataInterface $item, AccountInterface $account = NULL) { - return TRUE; + @trigger_error('\Drupal\search_api\Datasource\DatasourceInterface::checkItemAccess() is deprecated in search_api:8.x-1.13. It will be removed from search_api:9.x-1.0. Use checkItemAccessAsObject() instead. See (@todo CR link)', E_USER_DEPRECATED); + return $this->checkItemAccessAsObject($item, $account)->isAllowed(); + } + + /** + * {@inheritdoc} + */ + public function checkItemAccessAsObject(ComplexDataInterface $item, AccountInterface $account = NULL) { + return AccessResult::allowed(); } /** diff --git a/src/Item/Item.php b/src/Item/Item.php index d120b6f9..1a4e559e 100644 --- a/src/Item/Item.php +++ b/src/Item/Item.php @@ -2,6 +2,8 @@ namespace Drupal\search_api\Item; +use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Session\AccountInterface; use Drupal\Core\TypedData\ComplexDataInterface; use Drupal\search_api\Datasource\DatasourceInterface; @@ -406,13 +408,39 @@ class Item implements \IteratorAggregate, ItemInterface { * {@inheritdoc} */ public function checkAccess(AccountInterface $account = NULL) { + @trigger_error('\Drupal\search_api\Item\ItemInterface::checkAccess() is deprecated in search_api:8.x-1.13. It will be removed from search_api:9.x-1.0. Use checkAccessAsObject() instead. See (@todo CR link)', E_USER_DEPRECATED); + return $this->checkAccessAsObject($account)->isAllowed(); + } + + /** + * {@inheritdoc} + */ + public function checkAccessAsObject(AccountInterface $account = NULL) { + // @fixme Statically cache this by account. try { return $this->getDatasource() - ->checkItemAccess($this->getOriginalObject(), $account); + ->checkItemAccessAsObject($this->getOriginalObject(), $account); } catch (SearchApiException $e) { - return FALSE; + return AccessResult::neutral('Item could not be loaded, so cannot check access'); + } + } + + /** + * {@inheritdoc} + */ + public function getCacheability(AccountInterface $account = NULL, $includeListCacheability = TRUE) { + // @fixme Statically cache this by account. + $cacheability = new CacheableMetadata(); + $access = $this->checkAccessAsObject($account); + $cacheability->addCacheableDependency($access); + if ($access->isAllowed()) { + $cacheability->addCacheableDependency($this->getDatasource()->getItemCacheability($this->getOriginalObject())); + } + if ($includeListCacheability) { + $cacheability->addCacheableDependency($this->getDatasource()->getListCacheability()); } + return $cacheability; } /** diff --git a/src/Item/ItemInterface.php b/src/Item/ItemInterface.php index a3fdbb6f..f0a1f20e 100644 --- a/src/Item/ItemInterface.php +++ b/src/Item/ItemInterface.php @@ -295,7 +295,34 @@ interface ItemInterface extends \Traversable { * * @return bool * TRUE if access is granted, FALSE otherwise. + * + * @deprecated in search_api:8.x-1.13 and will be removed from + * search_api:9.x-1.0. Use checkAccessAsObject() instead. + * + * @see @todo link to change record (TBD) */ public function checkAccess(AccountInterface $account = NULL); + /** + * Checks whether a user has permission to view this item. + * + * @param \Drupal\Core\Session\AccountInterface|null $account + * (optional) The user session for which to check access, or NULL to check + * access for the current user. + * + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. + */ + public function checkAccessAsObject(AccountInterface $account = NULL); + + /** + * Get cacheability. + * + * @param \Drupal\Core\Session\AccountInterface|NULL $account + * @param bool $includeListCacheability + * + * @return \Drupal\Core\Cache\CacheableMetadata + */ + public function getCacheability(AccountInterface $account = NULL, $includeListCacheability = TRUE); + } diff --git a/src/Plugin/search_api/datasource/ContentEntity.php b/src/Plugin/search_api/datasource/ContentEntity.php index 6130fc6c..120b497f 100644 --- a/src/Plugin/search_api/datasource/ContentEntity.php +++ b/src/Plugin/search_api/datasource/ContentEntity.php @@ -2,6 +2,8 @@ namespace Drupal\search_api\Plugin\search_api\datasource; +use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\EntityDisplayRepositoryInterface; @@ -608,13 +610,14 @@ class ContentEntity extends DatasourcePluginBase implements EntityDatasourceInte /** * {@inheritdoc} */ - public function checkItemAccess(ComplexDataInterface $item, AccountInterface $account = NULL) { - if ($entity = $this->getEntity($item)) { + public function checkItemAccessAsObject(ComplexDataInterface $item, AccountInterface $account = NULL) { + $entity = $this->getEntity($item); + if ($entity) { return $this->getEntityTypeManager() ->getAccessControlHandler($this->getEntityTypeId()) - ->access($entity, 'view', $account); + ->access($entity, 'view', $account, TRUE); } - return FALSE; + return AccessResult::neutral('Item is not an entity, so cannot check access'); } /** @@ -1042,4 +1045,27 @@ class ContentEntity extends DatasourcePluginBase implements EntityDatasourceInte return $valid_ids; } + /** + * {@inheritdoc} + */ + public function getItemCacheability(ComplexDataInterface $item) { + $cacheability = new CacheableMetadata(); + $entity = $this->getEntity($item); + if ($entity) { + $cacheability->addCacheableDependency($entity); + } + return $cacheability; + } + + /** + * {@inheritdoc} + */ + public function getListCacheability() { + $cacheability = new CacheableMetadata(); + $entityType = $this->getEntityType(); + $cacheability->addCacheTags($entityType->getListCacheTags()); + $cacheability->addCacheContexts($entityType->getListCacheContexts()); + return $cacheability; + } + } diff --git a/src/Plugin/views/cache/SearchApiCachePluginTrait.php b/src/Plugin/views/cache/SearchApiCachePluginTrait.php index 9f6e7100..df175bb1 100644 --- a/src/Plugin/views/cache/SearchApiCachePluginTrait.php +++ b/src/Plugin/views/cache/SearchApiCachePluginTrait.php @@ -173,8 +173,8 @@ trait SearchApiCachePluginTrait { */ public function generateResultsKey() { if (!isset($this->resultsKey)) { - $query = $this->getQuery()->getSearchApiQuery(); - $query->preExecute(); + // @todo Why is this here? Is this needed? + $this->getQuery()->getSearchApiQuery()->preExecute(); $view = $this->getView(); $build_info = $view->build_info; diff --git a/src/Plugin/views/query/SearchApiQuery.php b/src/Plugin/views/query/SearchApiQuery.php index f77a15aa..ebe0d66a 100644 --- a/src/Plugin/views/query/SearchApiQuery.php +++ b/src/Plugin/views/query/SearchApiQuery.php @@ -3,6 +3,8 @@ namespace Drupal\search_api\Plugin\views\query; use Drupal\Component\Render\FormattableMarkup; +use Drupal\Core\Cache\CacheableDependencyInterface; +use Drupal\Core\Cache\RefinableCacheableDependencyInterface; use Drupal\Core\Database\Query\ConditionInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityTypeManagerInterface; @@ -119,6 +121,16 @@ class SearchApiQuery extends QueryPluginBase { */ protected $messenger; + /** + * The cacheability of the result. + * + * Set in ::addResults, so only available after ::execute. + * Used in ::getCacheFoo() + * + * @var RefinableCacheableDependencyInterface + */ + protected $cacheability; + /** * {@inheritdoc} */ @@ -637,7 +649,7 @@ class SearchApiQuery extends QueryPluginBase { // avoid them being individually loaded inside checkAccess(). $result_set->preLoadResultItems(); foreach ($results as $item_id => $result) { - if (!$result->checkAccess($account)) { + if (!$result->checkAccessAsObject($account)->isAllowed()) { unset($results[$item_id]); } } @@ -684,9 +696,37 @@ class SearchApiQuery extends QueryPluginBase { $values['index'] = $count++; $view->result[] = new ResultRow($values); + + // We drop the result set, so save its cacheability. + $this->cacheability = $result_set->getCacheability(); } } + /** + * @inheritDoc + */ + public function getCacheMaxAge() { + // The parent implementation is of no use for us. + return $this->cacheability->getCacheMaxAge(); + } + + /** + * @inheritDoc + */ + public function getCacheContexts() { + // The parent implementation is of no use for us. + return $this->cacheability->getCacheContexts(); + } + + /** + * @inheritDoc + */ + public function getCacheTags() { + // The parent implementation is of no use for us. + return $this->cacheability->getCacheTags(); + } + + /** * Retrieves the conditions placed on this query. * diff --git a/src/Query/ResultSet.php b/src/Query/ResultSet.php index 781a9dfe..e0746078 100644 --- a/src/Query/ResultSet.php +++ b/src/Query/ResultSet.php @@ -2,6 +2,8 @@ namespace Drupal\search_api\Query; +use Drupal\Core\Cache\CacheableMetadata; +use Drupal\Core\Session\AccountInterface; use Drupal\search_api\Item\ItemInterface; use Drupal\search_api\SearchApiException; @@ -259,4 +261,15 @@ class ResultSet implements \IteratorAggregate, ResultSetInterface { return $out; } + /** + * @inheritDoc + */ + public function getCacheability(AccountInterface $account = NULL, $includeListCacheability = TRUE) { + $cacheability = new CacheableMetadata(); + foreach ($this->getResultItems() as $item) { + $cacheability = $cacheability->merge($item->getCacheability($account, $includeListCacheability)); + } + return $cacheability; + } + } diff --git a/src/Query/ResultSetInterface.php b/src/Query/ResultSetInterface.php index a32a46ee..71590380 100644 --- a/src/Query/ResultSetInterface.php +++ b/src/Query/ResultSetInterface.php @@ -2,6 +2,7 @@ namespace Drupal\search_api\Query; +use Drupal\Core\Session\AccountInterface; use Drupal\search_api\Item\ItemInterface; /** @@ -197,4 +198,14 @@ interface ResultSetInterface extends \Traversable { */ public function getCloneForQuery(QueryInterface $query); + /** + * Get cacheability. + * + * @param \Drupal\Core\Session\AccountInterface|NULL $account + * @param bool $includeListCacheability + * + * @return \Drupal\Core\Cache\CacheableMetadata + */ + public function getCacheability(AccountInterface $account = NULL, $includeListCacheability = TRUE); + } -- 2.17.1