.../Core/Session/PermissionsHashGenerator.php | 12 ++++ ...shTest.php => PermissionsHashGeneratorTest.php} | 80 +++++++++++++++------- 2 files changed, 66 insertions(+), 26 deletions(-) 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/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); - // Updated account 1: now also 'editor' role. - $roles_1_updated = array('editor', 'administrator', 'authenticated'); - $this->account1Updated = $this->getMockBuilder('Drupal\user\Entity\User') + // 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')) + ->setMethods(array('getRoles', 'id')) ->getMock(); - $this->account1Updated->expects($this->any()) + $this->account3->expects($this->any()) ->method('getRoles') - ->will($this->returnValue($roles_1_updated)); + ->will($this->returnValue($roles_3)); + $this->account3->expects($this->any()) + ->method('id') + ->willReturn(3); + + // 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', 'id')) + ->getMock(); + $this->account2Updated->expects($this->any()) + ->method('getRoles') + ->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); } }