From 37cd13e9c772009b9eb2d6ed94b985504ade5d4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Arnauts?= Date: Wed, 24 May 2023 19:21:37 +0200 Subject: [PATCH] Fix #19847: Fix regression introduced in #15376 that caused `DbManager::getRolesByUser()` to return stale data --- framework/CHANGELOG.md | 2 +- framework/rbac/DbManager.php | 15 +++++++++++++-- tests/framework/rbac/DbManagerTestCase.php | 21 +++++++++++++++++---- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 3c3b66dca9..355aa804a5 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -4,7 +4,7 @@ Yii Framework 2 Change Log 2.0.49 under development ------------------------ -- no changes in this release. +- Bug #19847: Fix regression introduced in #15376 that caused `DbManager::getRolesByUser()` to return stale data (michaelarnauts) 2.0.48 May 22, 2023 diff --git a/framework/rbac/DbManager.php b/framework/rbac/DbManager.php index d676cbf8cf..2685ad5a1c 100644 --- a/framework/rbac/DbManager.php +++ b/framework/rbac/DbManager.php @@ -882,6 +882,9 @@ class DbManager extends BaseManager ])->execute(); unset($this->checkAccessAssignments[(string) $userId]); + + $this->invalidateCache(); + return $assignment; } @@ -895,9 +898,13 @@ class DbManager extends BaseManager } unset($this->checkAccessAssignments[(string) $userId]); - return $this->db->createCommand() + $result = $this->db->createCommand() ->delete($this->assignmentTable, ['user_id' => (string) $userId, 'item_name' => $role->name]) ->execute() > 0; + + $this->invalidateCache(); + + return $result; } /** @@ -910,9 +917,13 @@ class DbManager extends BaseManager } unset($this->checkAccessAssignments[(string) $userId]); - return $this->db->createCommand() + $result = $this->db->createCommand() ->delete($this->assignmentTable, ['user_id' => (string) $userId]) ->execute() > 0; + + $this->invalidateCache(); + + return $result; } /** diff --git a/tests/framework/rbac/DbManagerTestCase.php b/tests/framework/rbac/DbManagerTestCase.php index b6faad1290..bb5debaddc 100644 --- a/tests/framework/rbac/DbManagerTestCase.php +++ b/tests/framework/rbac/DbManagerTestCase.php @@ -220,17 +220,27 @@ abstract class DbManagerTestCase extends ManagerTestCase $admin = $this->auth->createRole('Admin'); $this->auth->add($admin); - $this->auth->assign($admin, 1); $manager = $this->auth->createRole('Manager'); $this->auth->add($manager); + + $adminUserRoles = $this->auth->getRolesByUser(1); + $this->assertArrayHasKey('myDefaultRole', $adminUserRoles); + $this->assertArrayNotHasKey('Admin', $adminUserRoles); + $this->auth->assign($admin, 1); + + $managerUserRoles = $this->auth->getRolesByUser(2); + $this->assertArrayHasKey('myDefaultRole', $managerUserRoles); + $this->assertArrayNotHasKey('Manager', $managerUserRoles); $this->auth->assign($manager, 2); $adminUserRoles = $this->auth->getRolesByUser(1); + $this->assertArrayHasKey('myDefaultRole', $adminUserRoles); $this->assertArrayHasKey('Admin', $adminUserRoles); $this->assertEquals($admin->name, $adminUserRoles['Admin']->name); $managerUserRoles = $this->auth->getRolesByUser(2); + $this->assertArrayHasKey('myDefaultRole', $managerUserRoles); $this->assertArrayHasKey('Manager', $managerUserRoles); $this->assertEquals($manager->name, $managerUserRoles['Manager']->name); } @@ -350,7 +360,7 @@ abstract class DbManagerTestCase extends ManagerTestCase } $this->assertSingleQueryToAssignmentsTable($logTarget); - // verify cache is flushed on unassign (createPost is now false again) + // verify cache is flushed on revoke (createPost is now false again) $this->auth->revoke($this->auth->getRole('admin'), 'reader A'); foreach (['readPost' => true, 'createPost' => false] as $permission => $result) { $this->assertEquals($result, $this->auth->checkAccess('reader A', $permission), "Checking $permission"); @@ -379,8 +389,11 @@ abstract class DbManagerTestCase extends ManagerTestCase private function assertSingleQueryToAssignmentsTable($logTarget) { - $this->assertCount(1, $logTarget->messages, 'Only one query should have been performed, but there are the following logs: ' . print_r($logTarget->messages, true)); - $this->assertContains('auth_assignment', $logTarget->messages[0][0], 'Log message should be a query to auth_assignment table'); + $messages = array_filter($logTarget->messages, function ($message) { + return strpos($message[0], 'auth_assignment') !== false; + }); + $this->assertCount(1, $messages, 'Only one query should have been performed, but there are the following logs: ' . print_r($logTarget->messages, true)); + $this->assertContains('auth_assignment', $messages[0][0], 'Log message should be a query to auth_assignment table'); $logTarget->messages = []; } }