diff --git a/core/lib/Drupal/Core/Session/PermissionsHashGenerator.php b/core/lib/Drupal/Core/Session/PermissionsHashGenerator.php index f176afd..f3b67da 100644 --- a/core/lib/Drupal/Core/Session/PermissionsHashGenerator.php +++ b/core/lib/Drupal/Core/Session/PermissionsHashGenerator.php @@ -51,6 +51,12 @@ public function __construct(PrivateKey $private_key, CacheBackendInterface $cach * Cached by role, invalidated whenever permissions change. */ public function generate(AccountInterface $account) { + // User 1 is the super user, and can always access all permissions. No hash + // necessary in this case. + if ($account->id() == 1) { + return 'is-super-user'; + } + $sorted_roles = $account->getRoles(); sort($sorted_roles); $role_list = implode(',', $sorted_roles); @@ -81,6 +87,12 @@ protected function doGenerate(array $roles) { $permissions_by_role = user_role_permissions($roles); foreach ($permissions_by_role as $role => $permissions) { sort($permissions); + // Note that for admin roles (\Drupal\user\RoleInterface::isAdmin()), the + // permissions returned will be empty ($permissions = []). Therefore the + // presence of the role ID as a key in $permissions_by_role is essential + // to ensure that the hash correctly recognizes admin roles. (If the hash + // was based solely on the union of $permissions, the admin roles would + // effectively be no-ops, allowing for hash collisions.) $permissions_by_role[$role] = $permissions; } return hash('sha256', $this->privateKey->get() . Settings::getHashSalt() . serialize($permissions_by_role)); diff --git a/core/modules/system/src/Tests/Render/RenderCacheTest.php b/core/modules/system/src/Tests/Render/RenderCacheTest.php new file mode 100644 index 0000000..55c3639 --- /dev/null +++ b/core/modules/system/src/Tests/Render/RenderCacheTest.php @@ -0,0 +1,102 @@ +installEntitySchema('user'); + $this->installConfig(['user']); + $this->installSchema('system', ['sequences']); + } + + /** + * Tests that user 1 has a different permission context with the same roles. + */ + public function testUser1PermissionContext() { + // Test that user 1 does not share the cache with other users who have the + // same roles, even when using a role-based cache context. + $user1 = $this->createUser(); + $this->assertEqual($user1->id(), 1); + $first_authenticated_user = $this->createUser(); + $second_authenticated_user = $this->createUser(); + $admin_user = $this->createUser([], NULL, TRUE); + + $this->assertEqual($user1->getRoles(), $first_authenticated_user->getRoles(), 'User 1 has the same roles as an authenticated user.'); + // Impersonate user 1 and render content that only user 1 should have + // permission to see. + \Drupal::service('account_switcher')->switchTo($user1); + $test_element = array( + '#cache' => array( + 'keys' => array('test'), + 'contexts' => array('user.permissions'), + ), + ); + $element = $test_element; + $element['#markup'] = 'content for user 1'; + $output = \Drupal::service('renderer')->render($element); + $this->assertEqual($output, 'content for user 1'); + + // Verify the cache is working by rendering the same element but with + // different markup passed in; the result should be the same. + $element = $test_element; + $element['#markup'] = 'should not be used'; + $output = \Drupal::service('renderer')->render($element); + $this->assertEqual($output, 'content for user 1'); + \Drupal::service('account_switcher')->switchBack(); + + // Verify that the first authenticated user does not see the same content + // as user 1. + \Drupal::service('account_switcher')->switchTo($first_authenticated_user); + $element = $test_element; + $element['#markup'] = 'content for authenticated users'; + $output = \Drupal::service('renderer')->render($element); + $this->assertEqual($output, 'content for authenticated users'); + \Drupal::service('account_switcher')->switchBack(); + + // Verify that the second authenticated user shares the cache with the + // first authenticated user. + \Drupal::service('account_switcher')->switchTo($second_authenticated_user); + $element = $test_element; + $element['#markup'] = 'should not be used'; + $output = \Drupal::service('renderer')->render($element); + $this->assertEqual($output, 'content for authenticated users'); + \Drupal::service('account_switcher')->switchBack(); + + // Verify that the admin user (who has an admin role without explicit + // permissions) does not share the same cache. + \Drupal::service('account_switcher')->switchTo($admin_user); + $element = $test_element; + $element['#markup'] = 'content for admin user'; + $output = \Drupal::service('renderer')->render($element); + $this->assertEqual($output, 'content for admin user'); + \Drupal::service('account_switcher')->switchBack(); + } + +} diff --git a/core/tests/Drupal/Tests/Core/Session/PermissionsHashTest.php b/core/tests/Drupal/Tests/Core/Session/PermissionsHashGeneratorTest.php similarity index 67% rename from core/tests/Drupal/Tests/Core/Session/PermissionsHashTest.php rename to core/tests/Drupal/Tests/Core/Session/PermissionsHashGeneratorTest.php index 2bc202c..42ea009 100644 --- a/core/tests/Drupal/Tests/Core/Session/PermissionsHashTest.php +++ b/core/tests/Drupal/Tests/Core/Session/PermissionsHashGeneratorTest.php @@ -2,7 +2,7 @@ /** * @file - * Contains \Drupal\Tests\Core\Session\PermissionsHashTest. + * Contains \Drupal\Tests\Core\Session\PermissionsHashGeneratorTest. */ namespace Drupal\Tests\Core\Session { @@ -17,28 +17,35 @@ * @coversDefaultClass \Drupal\Core\Session\PermissionsHashGenerator * @group Session */ -class PermissionsHashTest extends UnitTestCase { +class PermissionsHashGeneratorTest extends UnitTestCase { /** - * A mocked account. + * The mocked super user account. * * @var \Drupal\user\UserInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $account1; /** + * A mocked account. + * + * @var \Drupal\user\UserInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $account2; + + /** * An "updated" mocked account. * * @var \Drupal\user\UserInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $account1Updated; + protected $account2Updated; /** * A different account. * * @var \Drupal\user\UserInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $account2; + protected $account3; /** * The mocked private key service. @@ -69,35 +76,53 @@ protected function setUp() { new Settings(array('hash_salt' => 'test')); - // Account 1: 'administrator' and 'authenticated' roles. - $roles_1 = array('administrator', 'authenticated'); + // The mocked super user account. $this->account1 = $this->getMockBuilder('Drupal\user\Entity\User') ->disableOriginalConstructor() - ->setMethods(array('getRoles')) + ->setMethods(array('id')) ->getMock(); $this->account1->expects($this->any()) - ->method('getRoles') - ->will($this->returnValue($roles_1)); + ->method('id') + ->willReturn(1); - // Account 2: 'authenticated' and 'administrator' roles (different order). - $roles_2 = array('authenticated', 'administrator'); + // Account 2: 'administrator' and 'authenticated' roles. + $roles_1 = array('administrator', 'authenticated'); $this->account2 = $this->getMockBuilder('Drupal\user\Entity\User') ->disableOriginalConstructor() - ->setMethods(array('getRoles')) + ->setMethods(array('getRoles', 'id')) ->getMock(); $this->account2->expects($this->any()) ->method('getRoles') - ->will($this->returnValue($roles_2)); + ->will($this->returnValue($roles_1)); + $this->account2->expects($this->any()) + ->method('id') + ->willReturn(2); + + // Account 3: 'authenticated' and 'administrator' roles (different order). + $roles_3 = array('authenticated', 'administrator'); + $this->account3 = $this->getMockBuilder('Drupal\user\Entity\User') + ->disableOriginalConstructor() + ->setMethods(array('getRoles', 'id')) + ->getMock(); + $this->account3->expects($this->any()) + ->method('getRoles') + ->will($this->returnValue($roles_3)); + $this->account3->expects($this->any()) + ->method('id') + ->willReturn(3); - // Updated account 1: now also 'editor' role. - $roles_1_updated = array('editor', 'administrator', 'authenticated'); - $this->account1Updated = $this->getMockBuilder('Drupal\user\Entity\User') + // Updated account 2: now also 'editor' role. + $roles_2_updated = array('editor', 'administrator', 'authenticated'); + $this->account2Updated = $this->getMockBuilder('Drupal\user\Entity\User') ->disableOriginalConstructor() - ->setMethods(array('getRoles')) + ->setMethods(array('getRoles', 'id')) ->getMock(); - $this->account1Updated->expects($this->any()) + $this->account2Updated->expects($this->any()) ->method('getRoles') - ->will($this->returnValue($roles_1_updated)); + ->will($this->returnValue($roles_2_updated)); + $this->account2Updated->expects($this->any()) + ->method('id') + ->willReturn(2); // Mocked private key + cache services. $random = Crypt::randomBytesBase64(55); @@ -119,14 +144,17 @@ protected function setUp() { * Tests the generate() method. */ public function testGenerate() { + // Ensure that the super user (user 1) always gets the same hash. + $this->assertSame('is-super-user', $this->permissionsHash->generate($this->account1)); + // Ensure that two user accounts with the same roles generate the same hash. - $hash_1 = $this->permissionsHash->generate($this->account1); $hash_2 = $this->permissionsHash->generate($this->account2); - $this->assertSame($hash_1, $hash_2, 'Different users with the same roles generate the same permissions hash.'); + $hash_3 = $this->permissionsHash->generate($this->account3); + $this->assertSame($hash_2, $hash_3, 'Different users with the same roles generate the same permissions hash.'); // Compare with hash for user account 1 with an additional role. - $updated_hash_1 = $this->permissionsHash->generate($this->account1Updated); - $this->assertNotSame($hash_1, $updated_hash_1, 'Same user with updated roles generates different permissions hash.'); + $updated_hash_2 = $this->permissionsHash->generate($this->account2Updated); + $this->assertNotSame($hash_2, $updated_hash_2, 'Same user with updated roles generates different permissions hash.'); } /** @@ -146,7 +174,7 @@ public function testGenerateCache() { $this->cache->expects($this->never()) ->method('set'); - $this->permissionsHash->generate($this->account1); + $this->permissionsHash->generate($this->account2); } /** @@ -164,7 +192,7 @@ public function testGenerateNoCache() { ->method('set') ->with($expected_cid, $this->isType('string')); - $this->permissionsHash->generate($this->account1); + $this->permissionsHash->generate($this->account2); } }