UserAccessControlHandler::checkAccess() fails to add a user cache context where needed [#2614230]

Problem/Motivation

UserAccessControlHandler::checkAccess() has this code:

else if ($account->id() == $entity->id()) {
  return AccessResult::allowed()->cachePerUser();
}
...
return AccessResult::neutral();

The problem with this is that the cachePerUser() is not added when there is no match (checking whether user1 can view user2's profile), which allows the neutral() result to be cached across users, but then the cache can be primed with user1, and then user2 can't view his own profile.

Proposed resolution

Use the allowedIf() syntax which allows the ->cachePerUser(); to be associated in either case. This is the same approach as is done for the 'update' and 'delete' operations.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Comment	File	Size	Author
#21	2614230-21.patch	1.66 KB	_utsavsharma
#21	11.x: PHP 8.2 & MySQL 8 30,392 pass, 1 fail
#21	interdiff_18-21.txt	826 bytes	_utsavsharma
#18	2614230-18.patch	1.65 KB	Gauravvvv
#18	11.x: PHP 8.2 & MySQL 8 Custom Commands Failed 9.5.x: PHP 8.1 & MySQL 5.7 Custom Commands Failed
#3	interdiff.txt	685 bytes	effulgentsia
#3	user-access-control-3.patch	1.63 KB	effulgentsia
#3	8.0.x: PHP 5.5 & MySQL 5.5 14,412 pass, 2 fail 11.x: PHP 8.2 & MySQL 8 Patch Failed to Apply
	user-access-control.patch	850 bytes	effulgentsia
	8.0.x: PHP 5.5 & MySQL 5.5 14,349 pass

Issue fork drupal-2614230

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

2614230-useraccesscontrolhandlercheckaccess-fails-to compare
Check out this branch

About issue forks

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

12 November 2015 at 18:03

effulgentsia created an issue. See original summary.

Comment #2

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers as a volunteer commented 12 November 2015 at 18:05

Looks correct! Great catch.

Comment #3

effulgentsia CreditAttribution: effulgentsia at Acquia commented 19 November 2015 at 23:18

File	Size
user-access-control-3.patch	1.63 KB
8.0.x: PHP 5.5 & MySQL 5.5 14,412 pass, 2 fail 11.x: PHP 8.2 & MySQL 8 Patch Failed to Apply
interdiff.txt	685 bytes

File

Size

user-access-control-3.patch

1.63 KB

8.0.x:
PHP 5.5 & MySQL 5.5 14,412 pass, 2 fail

11.x:
PHP 8.2 & MySQL 8 Patch Failed to Apply

interdiff.txt

685 bytes

I expected the original patch to have the same 2 test failures as in #2558599-84: Automatically assign user cache contexts/tags when using current_user service, but it didn't. However, this one does. I think this means we need to fix the tests to expect the 'user' cache context, or else have a code flow that doesn't need it.

Comment #4

19 November 2015 at 23:51

Status:

Needs review

» Needs work

The last submitted patch, 3: user-access-control-3.patch, failed testing.

Comment #5

19 November 2015 at 23:51

Version:

8.0.x-dev

» 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #6

19 November 2015 at 23:51

Version:

8.1.x-dev

» 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #7

19 November 2015 at 23:51

Version:

8.2.x-dev

» 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #8

19 November 2015 at 23:51

Version:

8.3.x-dev

» 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #9

19 November 2015 at 23:51

Version:

8.4.x-dev

» 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #10

19 November 2015 at 23:51

Version:

8.5.x-dev

» 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #11

19 November 2015 at 23:51

Version:

8.6.x-dev

» 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Comment #12

19 November 2015 at 23:51

Version:

8.8.x-dev

» 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Comment #13

19 November 2015 at 23:51

Version:

8.9.x-dev

» 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #14

19 November 2015 at 23:51

Version:

9.2.x-dev

» 9.3.x-dev

Comment #15

19 November 2015 at 23:51

Version:

9.3.x-dev

» 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #16

19 November 2015 at 23:51

Version:

9.4.x-dev

» 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #17

catch

he/him

English

CreditAttribution: catch at Third and Grove commented 30 August 2023 at 07:56

Issue tags:

+Bug Smash Initiative

Still valid, sent the patch for a re-test since it looks like it'd still apply, we'll still need to update those tests if it's the same two test failures.

Comment #18

Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commented 12 October 2023 at 04:19

Status:

Needs work

» Needs review

File	Size
2614230-18.patch	1.65 KB
11.x: PHP 8.2 & MySQL 8 Custom Commands Failed 9.5.x: PHP 8.1 & MySQL 5.7 Custom Commands Failed

File

Size

2614230-18.patch

1.65 KB

11.x:
PHP 8.2 & MySQL 8 Custom Commands Failed

9.5.x:
PHP 8.1 & MySQL 5.7 Custom Commands Failed

I have attached a patch for 11.x. Patch #3, does not apply to 11.x so not adding interdiff.

Comment #19

smustgrave CreditAttribution: smustgrave at Mobomo commented 12 October 2023 at 17:08

Version:	9.5.x-dev	» 11.x-dev
Status:	Needs review	» Needs work
Issue tags:		+Needs tests

Still need a test showing the bug.

Comment #20

catch

he/him

English

CreditAttribution: catch at Third and Grove commented 12 October 2023 at 19:28

+++ b/core/modules/user/src/UserAccessControlHandler.php
+++ b/core/modules/user/src/UserAccessControlHandler.php
@@ -56,7 +56,7 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter

@@ -56,7 +56,7 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter
         }
         // Users can view own profiles at all times.
         elseif ($account->id() == $entity->id()) {
-          return AccessResult::allowed()->cachePerUser();
+          return AccessResult::allowedIf($account->id() == $entity->id())->cachePerUser();
         }

The code flow used to be a switch, but is now an if/elseif, so I don't think this will work the same way at all - it's the final else that needs the user cache content adding, and the condition added here will always be true.