Skip to content
Snippets Groups Projects

[11.x] Issue #2998824: MediaAccessControlHandler update/delete access caching is not correct

Open [11.x] Issue #2998824: MediaAccessControlHandler update/delete access caching is not correct
3 unresolved threads
3 unresolved threads

Closes #2998824

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
72 $access_result = AccessResult::allowedIf($is_owner)
73 ->cachePerPermissions()
74 ->cachePerUser()
75 ->addCacheableDependency($entity);
76 if (!$access_result->isAllowed()) {
77 $access_result->setReason("The user must be the owner and the 'view own unpublished media' permission is required when the media item is unpublished.");
78 }
79 }
80 else {
81 $access_result = AccessResult::neutral()
82 ->cachePerPermissions()
83 ->addCacheableDependency($entity)
84 ->setReason("The user must be the owner and the 'view own unpublished media' permission is required when the media item is unpublished.");
77
78 $access_result = AccessResult::allowedIf($is_owner && $account->hasPermission('view own unpublished media'))
79 ->cachePerPermissions()
  • Yes, ->cachePerPermissions() and ->cachePerUser() are redundant, but the developer should not deal with optimisation, the system will. Semantically these are correct because there is a permission and a user based decision here.

  • Please register or sign in to reply
  • 98 92 return AccessResult::allowed()->cachePerPermissions();
    99 93 }
    100 if ($account->hasPermission('update media') && $is_owner) {
    101 return AccessResult::allowed()->cachePerPermissions()->cachePerUser()->addCacheableDependency($entity);
    102 }
    103 return AccessResult::neutral("The following permissions are required: 'update any media' OR 'update own media' OR '$type: edit any media' OR '$type: edit own media'.")->cachePerPermissions();
    104
    105 case 'delete':
    106 if ($account->hasPermission('delete any ' . $type . ' media')) {
    94 if ($account->hasPermission('edit any ' . $type . ' media')) {
    107 95 return AccessResult::allowed()->cachePerPermissions();
    108 96 }
    109 if ($account->hasPermission('delete own ' . $type . ' media') && $is_owner) {
    110 return AccessResult::allowed()->cachePerPermissions()->cachePerUser()->addCacheableDependency($entity);
    97 if ($is_owner && ($account->hasPermission('edit own ' . $type . ' media') || $account->hasPermission('update media'))) {
    98 return AccessResult::allowed()->cachePerPermissions()->cachePerUser();
  • 976 974
    977 975 // 200 for well-formed HEAD request.
    978 976 $response = $this->request('HEAD', $url, $request_options);
    979 // MISS or UNCACHEABLE depends on data. It must not be HIT.
    980 $dynamic_cache = !empty(array_intersect(['user', 'session'], $this->getExpectedCacheContexts())) ? 'UNCACHEABLE' : 'MISS';
    981 $this->assertResourceResponse(200, NULL, $response, $this->getExpectedCacheTags(), $this->getExpectedCacheContexts(), FALSE, $dynamic_cache);
    977 $this->assertResourceResponse(200, NULL, $response, $this->getExpectedCacheTags(), $this->getExpectedCacheContexts(), FALSE, $this->generateDynamicPageCacheExpectedHeaderValue($this->getExpectedCacheContexts()));
  • Dezső Biczó added 2 commits

    added 2 commits

    • 01d9ff48 - Cherry-picked "Improve Dynamic Page Cache header assertions in JSON:API tests"
    • 39a720d9 - Start cleaning up and fixing tests

    Compare with previous version

  • Please register or sign in to reply
    Loading