Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
When modules that implement node grants are enabled, access checking is happening in this query in RevisionOverviewForm::buildForm()
:
$query = $this->entityQuery->get('node')
->condition($node->getEntityType()->getKey('id'), $node->id())
->pager($pagerLimit)
->allRevisions()
->sort($node->getEntityType()->getKey('revision'), 'DESC')
->execute();
This causes issues when folks are granted access to the node by other means (eg, hook_entity_access
, etc). Once content has a published version, then things behave normally again.
Proposed resolution
- Remove query access checking since access to the node itself has already been verified once this form is being built.
- Write a test to demonstrate the issue.
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#5 | 2935222-05.patch | 3.25 KB | namrata.honnavar |
#2 | 2935222-02.patch | 2.47 KB | jhedstrom |
Comments
Comment #2
jhedstromLet's see how this does with the rest of the tests. A similar fix for content moderation was done in #2932154: ModerationInformation::getLatestRevisionId returns access-specific results, for reference.
Comment #4
kevin.dutra CreditAttribution: kevin.dutra at Workday, Inc. commentedThis makes sense. The patch looks good to me and appears to work as expected, so moving to RTBC.
Comment #5
namrata.honnavar CreditAttribution: namrata.honnavar commentedComment #7
namrata.honnavar CreditAttribution: namrata.honnavar commentedMade some more changes - The compare selected revisions does not time out now.
Comment #8
kevin.dutra CreditAttribution: kevin.dutra at Workday, Inc. commentedIt makes sense to extend the access control change to that other spot too. Just FYI @namrata.honnavar, it's helpful when submitting a new patch to include an interdiff so that a reviewer can more easily see what has changed in the new patch. At any rate, things still appear to be in order, so moving back to RTBC.
Comment #9
kevin.dutra CreditAttribution: kevin.dutra at Workday, Inc. commentedComment #11
miro_dietikerThank you, committed this.