From ca240134707fea71affd3b46c5549edff358bffc Mon Sep 17 00:00:00 2001 From: rxu Date: Tue, 19 Oct 2021 23:24:43 +0700 Subject: [PATCH 1/8] [ticket/16895] Fix role removal for migrator permission tool PHPBB3-16895 --- phpBB/language/en/migrator.php | 1 + phpBB/phpbb/db/migration/tool/permission.php | 72 ++++++++++++++++---- 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/phpBB/language/en/migrator.php b/phpBB/language/en/migrator.php index 8a82d40be5..5fdee79365 100644 --- a/phpBB/language/en/migrator.php +++ b/phpBB/language/en/migrator.php @@ -78,4 +78,5 @@ $lang = array_merge($lang, array( 'PERMISSION_NOT_EXIST' => 'The permission setting "%s" unexpectedly does not exist.', 'ROLE_NOT_EXIST' => 'The permission role "%s" unexpectedly does not exist.', + 'ROLE_NOT_EXIST_ASSIGNED' => 'The permission role assigned to group "%1$s" unexpectedly does not exist. Role id: "%2$s"', )); diff --git a/phpBB/phpbb/db/migration/tool/permission.php b/phpBB/phpbb/db/migration/tool/permission.php index 6d19ad94c9..0b1a812bf9 100644 --- a/phpBB/phpbb/db/migration/tool/permission.php +++ b/phpBB/phpbb/db/migration/tool/permission.php @@ -49,6 +49,12 @@ class permission implements \phpbb\db\migration\tool\tool_interface $this->auth = $auth; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; + + if (!class_exists('auth_admin')) + { + include($this->phpbb_root_path . 'includes/acp/auth.' . $this->php_ext); + } + $this->auth_admin = new \auth_admin(); } /** @@ -118,12 +124,6 @@ class permission implements \phpbb\db\migration\tool\tool_interface // We've added permissions, so set to true to notify the user. $this->permissions_added = true; - if (!class_exists('auth_admin')) - { - include($this->phpbb_root_path . 'includes/acp/auth.' . $this->php_ext); - } - $auth_admin = new \auth_admin(); - // We have to add a check to see if the !$global (if global, local, and if local, global) permission already exists. If it does, acl_add_option currently has a bug which would break the ACL system, so we are having a work-around here. if ($this->exists($auth_option, !$global)) { @@ -140,19 +140,19 @@ class permission implements \phpbb\db\migration\tool\tool_interface { if ($global) { - $auth_admin->acl_add_option(array('global' => array($auth_option))); + $this->auth_admin->acl_add_option(array('global' => array($auth_option))); } else { - $auth_admin->acl_add_option(array('local' => array($auth_option))); + $this->auth_admin->acl_add_option(array('local' => array($auth_option))); } } // The permission has been added, now we can copy it if needed - if ($copy_from && isset($auth_admin->acl_options['id'][$copy_from])) + if ($copy_from && isset($this->auth_admin->acl_options['id'][$copy_from])) { - $old_id = $auth_admin->acl_options['id'][$copy_from]; - $new_id = $auth_admin->acl_options['id'][$auth_option]; + $old_id = $this->auth_admin->acl_options['id'][$copy_from]; + $new_id = $this->auth_admin->acl_options['id'][$auth_option]; $tables = array(ACL_GROUPS_TABLE, ACL_ROLES_DATA_TABLE, ACL_USERS_TABLE); @@ -177,7 +177,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface } } - $auth_admin->acl_clear_prefetch(); + $this->auth_admin->acl_clear_prefetch(); } } @@ -327,6 +327,45 @@ class permission implements \phpbb\db\migration\tool\tool_interface return; } + // Get the role auth settings we need to re-set... + $sql = 'SELECT o.auth_option, r.auth_setting + FROM ' . ACL_ROLES_DATA_TABLE . ' r, ' . ACL_OPTIONS_TABLE . ' o + WHERE o.auth_option_id = r.auth_option_id + AND r.role_id = ' . $role_id; + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + $auth_settings[$row['auth_option']] = $row['auth_setting']; + } + $this->db->sql_freeresult($result); + + // Get role assignments + $hold_ary = $this->auth_admin->get_role_mask($role_id); + + // Re-assign permissions + foreach ($hold_ary as $forum_id => $forum_ary) + { + if (isset($forum_ary['users'])) + { + $this->auth_admin->acl_set('user', $forum_id, $forum_ary['users'], $auth_settings, 0, false); + } + + if (isset($forum_ary['groups'])) + { + $this->auth_admin->acl_set('group', $forum_id, $forum_ary['groups'], $auth_settings, 0, false); + } + } + + // Remove role from users and groups just to be sure (happens through acl_set) + $sql = 'DELETE FROM ' . ACL_USERS_TABLE . ' + WHERE auth_role_id = ' . $role_id; + $this->db->sql_query($sql); + + $sql = 'DELETE FROM ' . ACL_GROUPS_TABLE . ' + WHERE auth_role_id = ' . $role_id; + $this->db->sql_query($sql); + $sql = 'DELETE FROM ' . ACL_ROLES_DATA_TABLE . ' WHERE role_id = ' . $role_id; $this->db->sql_query($sql); @@ -425,6 +464,11 @@ class permission implements \phpbb\db\migration\tool\tool_interface WHERE role_id = ' . $role_id; $this->db->sql_query($sql); $role_data = $this->db->sql_fetchrow(); + if (!$role_data) + { + throw new \phpbb\db\migration\exception('ROLE_NOT_EXIST_ASSIGNED', $name, $role_id); + } + $role_name = $role_data['role_name']; $role_type = $role_data['role_type']; @@ -571,6 +615,10 @@ class permission implements \phpbb\db\migration\tool\tool_interface WHERE role_id = ' . $role_id; $this->db->sql_query($sql); $role_name = $this->db->sql_fetchfield('role_name'); + if (!$role_name) + { + throw new \phpbb\db\migration\exception('ROLE_NOT_EXIST_ASSIGNED', $name, $role_id); + } return $this->permission_unset($role_name, $auth_option, 'role'); } From a860a3310a166763cf21eb3c44cae0aed92aaa80 Mon Sep 17 00:00:00 2001 From: rxu Date: Wed, 20 Oct 2021 21:58:24 +0700 Subject: [PATCH 2/8] [ticket/16895] Add migration PHPBB3-16895 --- .../remove_non_existant_assigned_roles.php | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 phpBB/phpbb/db/migration/data/v33x/remove_non_existant_assigned_roles.php diff --git a/phpBB/phpbb/db/migration/data/v33x/remove_non_existant_assigned_roles.php b/phpBB/phpbb/db/migration/data/v33x/remove_non_existant_assigned_roles.php new file mode 100644 index 0000000000..477825c53d --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v33x/remove_non_existant_assigned_roles.php @@ -0,0 +1,72 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace phpbb\db\migration\data\v33x; + +class remove_non_existant_assigned_roles extends \phpbb\db\migration\migration +{ + static public function depends_on() + { + return ['\phpbb\db\migration\data\v33x\v335',]; + } + + public function update_data() + { + return [ + ['custom', [[$this, 'remove_non_existant_roles_assignment']]], + ]; + } + + public function remove_non_existant_roles_assignment() + { + $auth_role_ids = $role_ids = $auth_settings = []; + + $sql = 'SELECT auth_role_id + FROM ' . ACL_GROUPS_TABLE . ' + WHERE auth_role_id <> 0 + AND forum_id = 0'; + $result = $this->db->sql_query($sql); + $auth_role_ids = array_keys($this->db->sql_fetchrowset($result)); + $this->db->sql_freeresult($result); + + if (count($auth_role_ids)) + { + $sql = 'SELECT role_id + FROM ' . ACL_ROLES_TABLE . ' + WHERE ' . $this->db->sql_in_set('role_id', $auth_role_ids); + $result = $this->db->sql_query($sql); + $role_ids = array_keys($this->db->sql_fetchrowset($result)); + $this->db->sql_freeresult($result); + } + + $non_existant_role_ids = array_diff($auth_role_ids, $role_ids); + + // Nothing to do, there are no non-existant roles assigned to groups + if (empty($non_existant_role_ids)) + { + return true; + } + + // Remove assigned non-existant roles from users and groups + $sql = 'DELETE FROM ' . ACL_USERS_TABLE . ' + WHERE ' . $this->db->sql_in_set('auth_role_id', $non_existant_role_ids); + $this->db->sql_query($sql); + + $sql = 'DELETE FROM ' . ACL_GROUPS_TABLE . ' + WHERE ' . $this->db->sql_in_set('auth_role_id', $non_existant_role_ids); + $this->db->sql_query($sql); + + $auth = new \phpbb\auth\auth(); + $auth->acl_clear_prefetch(); + } +} From b1e6fad38a37dddc206acad47cf66318211f5425 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 23 Oct 2021 22:37:50 +0700 Subject: [PATCH 3/8] [ticket/16895] Add test PHPBB3-16895 --- phpBB/phpbb/db/migration/tool/permission.php | 28 ++- .../migrator_tool_permission_role_test.php | 191 ++++++++++++++++++ 2 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 tests/dbal/migrator_tool_permission_role_test.php diff --git a/phpBB/phpbb/db/migration/tool/permission.php b/phpBB/phpbb/db/migration/tool/permission.php index 0b1a812bf9..8af3e411ac 100644 --- a/phpBB/phpbb/db/migration/tool/permission.php +++ b/phpBB/phpbb/db/migration/tool/permission.php @@ -21,6 +21,9 @@ class permission implements \phpbb\db\migration\tool\tool_interface /** @var \phpbb\auth\auth */ protected $auth; + /** @var \includes\acp\acp_auth */ + protected $auth_admin; + /** @var \phpbb\cache\service */ protected $cache; @@ -291,6 +294,8 @@ class permission implements \phpbb\db\migration\tool\tool_interface $sql = 'INSERT INTO ' . ACL_ROLES_TABLE . ' ' . $this->db->sql_build_array('INSERT', $sql_ary); $this->db->sql_query($sql); + + return $this->db->sql_nextid(); } /** @@ -327,11 +332,32 @@ class permission implements \phpbb\db\migration\tool\tool_interface return; } + // Get the role type + $sql = 'SELECT role_type + FROM ' . ACL_ROLES_TABLE . ' + WHERE role_id = ' . (int) $role_id; + $result = $this->db->sql_query($sql); + $role_type = $this->db->sql_fetchfield('role_type'); + $this->db->sql_freeresult($result); + + // Get complete auth array + $sql = 'SELECT auth_option, auth_option_id + FROM ' . ACL_OPTIONS_TABLE . " + WHERE auth_option " . $this->db->sql_like_expression($role_type . $this->db->get_any_char()); + $result = $this->db->sql_query($sql); + + $auth_settings = []; + while ($row = $this->db->sql_fetchrow($result)) + { + $auth_settings[$row['auth_option']] = ACL_NO; + } + $this->db->sql_freeresult($result); + // Get the role auth settings we need to re-set... $sql = 'SELECT o.auth_option, r.auth_setting FROM ' . ACL_ROLES_DATA_TABLE . ' r, ' . ACL_OPTIONS_TABLE . ' o WHERE o.auth_option_id = r.auth_option_id - AND r.role_id = ' . $role_id; + AND r.role_id = ' . (int) $role_id; $result = $this->db->sql_query($sql); while ($row = $this->db->sql_fetchrow($result)) diff --git a/tests/dbal/migrator_tool_permission_role_test.php b/tests/dbal/migrator_tool_permission_role_test.php new file mode 100644 index 0000000000..2573a5bb1d --- /dev/null +++ b/tests/dbal/migrator_tool_permission_role_test.php @@ -0,0 +1,191 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +class phpbb_dbal_migrator_tool_permission_role_test extends phpbb_database_test_case +{ + /** @var \phpbb\auth\auth */ + protected $auth; + + /** @var \acp\auth\auth_admin */ + protected $auth_admin; + + /** @var \phpbb\db\migration\tool\permission */ + protected $tool; + + public $group_ids = [ + 'REGISTERED' => 2, + 'GLOBAL_MODERATORS' => 4, + 'ADMINISTRATORS' => 5, + ]; + + public $new_roles = [ + [ + 'ROLE_ADMIN_NEW', + 'a_', + 'A new admin role', + 'a_new', + ], + [ + 'ROLE_MODERATOR_NEW', + 'm_', + 'A new mod role', + 'm_new', + ], + [ + 'ROLE_USER_NEW', + 'u_', + 'A new user role', + 'u_new', + ], + ]; + + public $new_role_ids = []; + + public function getDataSet() + { + return $this->createXMLDataSet(__DIR__.'/fixtures/migrator_permission.xml'); + } + + protected function setUp(): void + { + // Global $db and $cache are needed in acp/auth.php constructor + global $phpbb_root_path, $phpEx, $db, $cache; + + parent::setup(); + + $db = $this->db = $this->new_dbal(); + $cache = $this->cache = new \phpbb\cache\service(new \phpbb\cache\driver\dummy(), new \phpbb\config\config(array()), $this->db, $phpbb_root_path, $phpEx); + $this->auth = new \phpbb\auth\auth(); + + // Initialize this auth_admin instance later after adding new auth options via this->tool->add() + if (!class_exists('auth_admin')) + { + include($phpbb_root_path . 'includes/acp/auth.' . $phpEx); + } + + $this->tool = new \phpbb\db\migration\tool\permission($this->db, $this->cache, $this->auth, $phpbb_root_path, $phpEx); + + $this->new_roles_add(); + } + + public function new_roles_add() + { + foreach ($this->new_roles as $new_role_data) + { + $role_name = $new_role_data[0]; + $role_type = $new_role_data[1]; + $role_description = $new_role_data[2]; + $role_auth_option = $new_role_data[3]; + + $this->tool->add($role_auth_option); + $this->new_role_ids[$role_name] = $this->tool->role_add($role_name, $role_type, $role_description); + } + + // Initialize external auth_admin instance here to keep acl_options array in sync with the one from the permission tool + $this->auth_admin = new \auth_admin(); + } + + public function data_test_new_role_exists() + { + return [ + ['ROLE_ADMIN_NEW', true], + ['ROLE_MODERATOR_NEW', true], + ['ROLE_USER_NEW', true], + ]; + } + + /** + * @dataProvider data_test_new_role_exists + */ + public function test_permission_new_role_exists($role_name, $expected) + { + $this->assertEquals($expected, (bool) $this->tool->role_exists($role_name)); + } + + public function data_test_permission_assign_new_roles() + { + return [ + [ + 'group', + 0, + 'ADMINISTRATORS', + ['a_new' => true], + 'ROLE_ADMIN_NEW', + ], + [ + 'group', + 0, + 'GLOBAL_MODERATORS', + ['m_new' => true], + 'ROLE_MODERATOR_NEW', + ], + [ + 'group', + 0, + 'REGISTERED', + ['u_new' => true], + 'ROLE_USER_NEW', + ], + ]; + } + + /** + * @dataProvider data_test_permission_assign_new_roles + */ + public function test_permission_assign_new_roles($ug_type, $forum_id, $group_name, $auth, $role_name, $clear_prefetch = true) + { + $auth_option = key($auth); + $group_id = (int) $this->group_ids[$group_name]; + $role_id = (int) $this->new_role_ids[$role_name]; + $expected = current($auth); + + // Set auth options for each role + $this->tool->permission_set($role_name, $auth_option, 'role', true); + + // Assign roles to groups + $this->auth_admin->acl_set($ug_type, $forum_id, $group_id, $auth, $role_id, $clear_prefetch); + + // Test if role based group permissions assigned correctly + $new_perm_state = $this->auth->acl_group_raw_data($group_id, $auth_option); + $this->assertEquals($expected, !empty($new_perm_state), "$auth_option is " . ($expected ? 'empty' : 'not empty') . " for $group_name"); + } + + /** + * @dataProvider data_test_permission_assign_new_roles + * @depends test_permission_new_role_exists + * @depends test_permission_assign_new_roles + */ + public function test_permission_new_role_remove($ug_type, $forum_id, $group_name, $auth, $role_name) + { + $auth_option = key($auth); + $group_id = (int) $this->group_ids[$group_name]; + $role_id = (int) $this->new_role_ids[$role_name]; + + // Set auth options for each role + $this->tool->permission_set($role_name, $auth_option, 'role', true); + + // Assign roles to groups + $this->auth_admin->acl_set($ug_type, $forum_id, $group_id, $auth, $role_id); + + $this->tool->role_remove($role_name); + $this->assertFalse((bool) $this->tool->role_exists($role_name)); + + $sql = 'SELECT agt.auth_role_id + FROM ' . ACL_GROUPS_TABLE . ' agt, ' . ACL_ROLES_TABLE . ' art + WHERE agt.auth_role_id = art.role_id + AND art.role_id = ' . $role_id; + $result = $this->db->sql_query($sql); + $this->assertFalse($this->db->sql_fetchfield('auth_role_id')); + $this->db->sql_freeresult($result); + } +} From 7275cdd15249a72ca95a54b33384ab614464ba65 Mon Sep 17 00:00:00 2001 From: rxu Date: Wed, 27 Oct 2021 00:15:46 +0700 Subject: [PATCH 4/8] [ticket/16895] Adjust test PHPBB3-16895 --- phpBB/phpbb/db/migration/tool/permission.php | 2 +- .../dbal/migrator_tool_permission_role_test.php | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/phpBB/phpbb/db/migration/tool/permission.php b/phpBB/phpbb/db/migration/tool/permission.php index 8af3e411ac..8c86d0b7f2 100644 --- a/phpBB/phpbb/db/migration/tool/permission.php +++ b/phpBB/phpbb/db/migration/tool/permission.php @@ -21,7 +21,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface /** @var \phpbb\auth\auth */ protected $auth; - /** @var \includes\acp\acp_auth */ + /** @var \includes\acp\auth\auth_admin */ protected $auth_admin; /** @var \phpbb\cache\service */ diff --git a/tests/dbal/migrator_tool_permission_role_test.php b/tests/dbal/migrator_tool_permission_role_test.php index 2573a5bb1d..48e45bf2d5 100644 --- a/tests/dbal/migrator_tool_permission_role_test.php +++ b/tests/dbal/migrator_tool_permission_role_test.php @@ -16,7 +16,7 @@ class phpbb_dbal_migrator_tool_permission_role_test extends phpbb_database_test_ /** @var \phpbb\auth\auth */ protected $auth; - /** @var \acp\auth\auth_admin */ + /** @var \includes\acp\auth\auth_admin */ protected $auth_admin; /** @var \phpbb\db\migration\tool\permission */ @@ -171,19 +171,26 @@ class phpbb_dbal_migrator_tool_permission_role_test extends phpbb_database_test_ $group_id = (int) $this->group_ids[$group_name]; $role_id = (int) $this->new_role_ids[$role_name]; + $sql = 'SELECT agt.auth_role_id + FROM ' . ACL_GROUPS_TABLE . ' agt, ' . ACL_ROLES_TABLE . ' art + WHERE agt.auth_role_id = art.role_id + AND art.role_id = ' . $role_id; + // Set auth options for each role $this->tool->permission_set($role_name, $auth_option, 'role', true); // Assign roles to groups $this->auth_admin->acl_set($ug_type, $forum_id, $group_id, $auth, $role_id); + // Check if the role is assigned to the group + $result = $this->db->sql_query($sql); + $this->assertEquals($role_id, $this->db->sql_fetchfield('auth_role_id')); + $this->db->sql_freeresult($result); + $this->tool->role_remove($role_name); $this->assertFalse((bool) $this->tool->role_exists($role_name)); - $sql = 'SELECT agt.auth_role_id - FROM ' . ACL_GROUPS_TABLE . ' agt, ' . ACL_ROLES_TABLE . ' art - WHERE agt.auth_role_id = art.role_id - AND art.role_id = ' . $role_id; + // Check if the role is unassigned $result = $this->db->sql_query($sql); $this->assertFalse($this->db->sql_fetchfield('auth_role_id')); $this->db->sql_freeresult($result); From 2801415c1ca5e1c545a602520c457ed5d8a3d70b Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 28 Oct 2021 20:48:02 +0700 Subject: [PATCH 5/8] [ticket/16895] Rename language entry PHPBB3-16895 --- phpBB/language/en/migrator.php | 2 +- phpBB/phpbb/db/migration/tool/permission.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/language/en/migrator.php b/phpBB/language/en/migrator.php index 5fdee79365..e5d7eff609 100644 --- a/phpBB/language/en/migrator.php +++ b/phpBB/language/en/migrator.php @@ -77,6 +77,6 @@ $lang = array_merge($lang, array( 'PARENT_MODULE_FIND_ERROR' => 'Unable to determine the parent module identifier: %s', 'PERMISSION_NOT_EXIST' => 'The permission setting "%s" unexpectedly does not exist.', + 'ROLE_ASSIGNED_NOT_EXIST' => 'The permission role assigned to group "%1$s" unexpectedly does not exist. Role id: "%2$s"', 'ROLE_NOT_EXIST' => 'The permission role "%s" unexpectedly does not exist.', - 'ROLE_NOT_EXIST_ASSIGNED' => 'The permission role assigned to group "%1$s" unexpectedly does not exist. Role id: "%2$s"', )); diff --git a/phpBB/phpbb/db/migration/tool/permission.php b/phpBB/phpbb/db/migration/tool/permission.php index 8c86d0b7f2..66ca2ed0ee 100644 --- a/phpBB/phpbb/db/migration/tool/permission.php +++ b/phpBB/phpbb/db/migration/tool/permission.php @@ -492,7 +492,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface $role_data = $this->db->sql_fetchrow(); if (!$role_data) { - throw new \phpbb\db\migration\exception('ROLE_NOT_EXIST_ASSIGNED', $name, $role_id); + throw new \phpbb\db\migration\exception('ROLE_ASSIGNED_NOT_EXIST', $name, $role_id); } $role_name = $role_data['role_name']; @@ -643,7 +643,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface $role_name = $this->db->sql_fetchfield('role_name'); if (!$role_name) { - throw new \phpbb\db\migration\exception('ROLE_NOT_EXIST_ASSIGNED', $name, $role_id); + throw new \phpbb\db\migration\exception('ROLE_ASSIGNED_NOT_EXIST', $name, $role_id); } return $this->permission_unset($role_name, $auth_option, 'role'); From d7f433fbf70cda11160ad9d0b25950b70f51d4dc Mon Sep 17 00:00:00 2001 From: rxu Date: Wed, 10 Nov 2021 13:30:14 +0700 Subject: [PATCH 6/8] [ticket/16895] Rename migration file PHPBB3-16895 --- ...ed_roles.php => remove_orphaned_roles.php} | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) rename phpBB/phpbb/db/migration/data/v33x/{remove_non_existant_assigned_roles.php => remove_orphaned_roles.php} (68%) diff --git a/phpBB/phpbb/db/migration/data/v33x/remove_non_existant_assigned_roles.php b/phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php similarity index 68% rename from phpBB/phpbb/db/migration/data/v33x/remove_non_existant_assigned_roles.php rename to phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php index 477825c53d..247264257c 100644 --- a/phpBB/phpbb/db/migration/data/v33x/remove_non_existant_assigned_roles.php +++ b/phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php @@ -13,21 +13,21 @@ namespace phpbb\db\migration\data\v33x; -class remove_non_existant_assigned_roles extends \phpbb\db\migration\migration +class remove_orphaned_roles extends \phpbb\db\migration\migration { static public function depends_on() { - return ['\phpbb\db\migration\data\v33x\v335',]; + return ['\phpbb\db\migration\data\v33x\v335']; } public function update_data() { return [ - ['custom', [[$this, 'remove_non_existant_roles_assignment']]], + ['custom', [[$this, 'remove_orphaned_roles']]], ]; } - public function remove_non_existant_roles_assignment() + public function remove_orphaned_roles() { $auth_role_ids = $role_ids = $auth_settings = []; @@ -49,21 +49,21 @@ class remove_non_existant_assigned_roles extends \phpbb\db\migration\migration $this->db->sql_freeresult($result); } - $non_existant_role_ids = array_diff($auth_role_ids, $role_ids); + $non_existent_role_ids = array_diff($auth_role_ids, $role_ids); - // Nothing to do, there are no non-existant roles assigned to groups - if (empty($non_existant_role_ids)) + // Nothing to do, there are no non-existent roles assigned to groups + if (empty($non_existent_role_ids)) { return true; } - // Remove assigned non-existant roles from users and groups + // Remove assigned non-existent roles from users and groups $sql = 'DELETE FROM ' . ACL_USERS_TABLE . ' - WHERE ' . $this->db->sql_in_set('auth_role_id', $non_existant_role_ids); + WHERE ' . $this->db->sql_in_set('auth_role_id', $non_existent_role_ids); $this->db->sql_query($sql); $sql = 'DELETE FROM ' . ACL_GROUPS_TABLE . ' - WHERE ' . $this->db->sql_in_set('auth_role_id', $non_existant_role_ids); + WHERE ' . $this->db->sql_in_set('auth_role_id', $non_existent_role_ids); $this->db->sql_query($sql); $auth = new \phpbb\auth\auth(); From 69b895caae1582c02f607141d8c45c4840cb0d6a Mon Sep 17 00:00:00 2001 From: rxu Date: Wed, 10 Nov 2021 14:13:02 +0700 Subject: [PATCH 7/8] [ticket/16895] Rename custom method PHPBB3-16895 --- phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php b/phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php index 247264257c..11e1ab0812 100644 --- a/phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php +++ b/phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php @@ -23,11 +23,11 @@ class remove_orphaned_roles extends \phpbb\db\migration\migration public function update_data() { return [ - ['custom', [[$this, 'remove_orphaned_roles']]], + ['custom', [[$this, 'acl_remove_orphaned_roles']]], ]; } - public function remove_orphaned_roles() + public function acl_remove_orphaned_roles() { $auth_role_ids = $role_ids = $auth_settings = []; From 89168c507b13333bf3a4620446c994eb92a0891a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 3 Dec 2021 20:14:26 +0100 Subject: [PATCH 8/8] [ticket/16895] Add missing return and remove not needed declarations PHPBB3-16895 --- phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php b/phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php index 11e1ab0812..4f2bfe90cc 100644 --- a/phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php +++ b/phpBB/phpbb/db/migration/data/v33x/remove_orphaned_roles.php @@ -29,7 +29,7 @@ class remove_orphaned_roles extends \phpbb\db\migration\migration public function acl_remove_orphaned_roles() { - $auth_role_ids = $role_ids = $auth_settings = []; + $role_ids = []; $sql = 'SELECT auth_role_id FROM ' . ACL_GROUPS_TABLE . ' @@ -68,5 +68,7 @@ class remove_orphaned_roles extends \phpbb\db\migration\migration $auth = new \phpbb\auth\auth(); $auth->acl_clear_prefetch(); + + return true; } }