From 2308472eb0a5bc03aa520f4afe2e69aa52622653 Mon Sep 17 00:00:00 2001
From: rxu <rxu@mail.ru>
Date: Sun, 26 Apr 2015 10:51:01 +0700
Subject: [PATCH 1/2] [ticket/13779] Set new auth options to the role only if
matching the role type
Migrations' permission tool allows setting permissions to the role which
doesn't match the role type, e.g. m_ permissions for u_ role types and so on.
As one of side effects, this may lead to granting users moderative/admin
permissions silently.
With this patch the only new permissions matching the role type will be set.
PHPBB3-13779
---
phpBB/phpbb/db/migration/tool/permission.php | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/phpBB/phpbb/db/migration/tool/permission.php b/phpBB/phpbb/db/migration/tool/permission.php
index 1a91127d2d..ceff6d7d5a 100644
--- a/phpBB/phpbb/db/migration/tool/permission.php
+++ b/phpBB/phpbb/db/migration/tool/permission.php
@@ -425,13 +425,27 @@ class permission implements \phpbb\db\migration\tool\tool_interface
$role_id = (int) $this->db->sql_fetchfield('auth_role_id');
if ($role_id)
{
- $sql = 'SELECT role_name
+ $sql = 'SELECT role_name, role_type
FROM ' . ACL_ROLES_TABLE . '
WHERE role_id = ' . $role_id;
$this->db->sql_query($sql);
- $role_name = $this->db->sql_fetchfield('role_name');
+ $role_data = $this->db->sql_fetchrow();
+ $role_name = $role_data['role_name'];
+ $role_type = $role_data['role_type'];
- return $this->permission_set($role_name, $auth_option, 'role', $has_permission);
+ // Filter new auth options to match the role type: a_ | f_ | m_ | u_
+ // Set new auth options to the role only if options matching the role type were found
+ $auth_option = array_filter($auth_option,
+ function ($option) use ($role_type)
+ {
+ return strpos($option, $role_type) === 0;
+ }
+ );
+
+ if (sizeof($auth_option))
+ {
+ return $this->permission_set($role_name, $auth_option, 'role', $has_permission);
+ }
}
$sql = 'SELECT auth_option_id, auth_setting
From c221571963b5252c3343b6db3ace552e52104056 Mon Sep 17 00:00:00 2001
From: rxu <rxu@mail.ru>
Date: Sat, 6 Jun 2015 14:54:56 +0700
Subject: [PATCH 2/2] [ticket/13779] Add permission set tests
PHPBB3-13779
---
tests/dbal/fixtures/migrator_permission.xml | 134 +++++++++++++++++++
tests/dbal/migrator_tool_permission_test.php | 62 +++++++++
2 files changed, 196 insertions(+)
diff --git a/tests/dbal/fixtures/migrator_permission.xml b/tests/dbal/fixtures/migrator_permission.xml
index 08cec42a42..c07956fa85 100644
--- a/tests/dbal/fixtures/migrator_permission.xml
+++ b/tests/dbal/fixtures/migrator_permission.xml
@@ -27,5 +27,139 @@
<value>1</value>
<value>0</value>
</row>
+ <row>
+ <value>4</value>
+ <value>a_test</value>
+ <value>1</value>
+ <value>0</value>
+ <value>0</value>
+ </row>
+ <row>
+ <value>5</value>
+ <value>m_test</value>
+ <value>1</value>
+ <value>0</value>
+ <value>0</value>
+ </row>
+ <row>
+ <value>6</value>
+ <value>u_test</value>
+ <value>1</value>
+ <value>0</value>
+ <value>0</value>
+ </row>
+ </table>
+
+ <table name="phpbb_groups">
+ <column>group_id</column>
+ <column>group_name</column>
+ <column>group_desc</column>
+ <row>
+ <value>2</value>
+ <value>REGISTERED</value>
+ <value></value>
+ </row>
+ <row>
+ <value>4</value>
+ <value>GLOBAL_MODERATORS</value>
+ <value></value>
+ </row>
+ <row>
+ <value>5</value>
+ <value>ADMINISTRATORS</value>
+ <value></value>
+ </row>
+ </table>
+
+ <table name="phpbb_acl_groups">
+ <column>group_id</column>
+ <column>auth_role_id</column>
+ <column>forum_id</column>
+ <row>
+ <value>2</value>
+ <value>5</value>
+ <value>0</value>
+ </row>
+ <row>
+ <value>4</value>
+ <value>5</value>
+ <value>0</value>
+ </row>
+ <row>
+ <value>4</value>
+ <value>10</value>
+ <value>0</value>
+ </row>
+ <row>
+ <value>5</value>
+ <value>1</value>
+ <value>0</value>
+ </row>
+ <row>
+ <value>5</value>
+ <value>5</value>
+ <value>0</value>
+ </row>
+ </table>
+
+ <table name="phpbb_acl_roles">
+ <column>role_id</column>
+ <column>role_name</column>
+ <column>role_type</column>
+ <column>role_description</column>
+ <row>
+ <value>1</value>
+ <value>ROLE_ADMIN_STANDARD</value>
+ <value>a_</value>
+ <value></value>
+ </row>
+ <row>
+ <value>5</value>
+ <value>ROLE_USER_FULL</value>
+ <value>u_</value>
+ <value></value>
+ </row>
+ <row>
+ <value>10</value>
+ <value>ROLE_MOD_FULL</value>
+ <value>m_</value>
+ <value></value>
+ </row>
+ </table>
+
+ <table name="phpbb_acl_roles_data">
+ <column>role_id</column>
+ <column>auth_option_id</column>
+ <column>auth_setting</column>
+ <row>
+ <value>1</value>
+ <value>4</value>
+ <value>0</value>
+ </row>
+ <row>
+ <value>1</value>
+ <value>5</value>
+ <value>0</value>
+ </row>
+ <row>
+ <value>1</value>
+ <value>6</value>
+ <value>0</value>
+ </row>
+ <row>
+ <value>6</value>
+ <value>6</value>
+ <value>0</value>
+ </row>
+ <row>
+ <value>10</value>
+ <value>5</value>
+ <value>0</value>
+ </row>
+ <row>
+ <value>10</value>
+ <value>6</value>
+ <value>0</value>
+ </row>
</table>
</dataset>
diff --git a/tests/dbal/migrator_tool_permission_test.php b/tests/dbal/migrator_tool_permission_test.php
index 4453fbf123..2d673864f7 100644
--- a/tests/dbal/migrator_tool_permission_test.php
+++ b/tests/dbal/migrator_tool_permission_test.php
@@ -15,6 +15,12 @@ require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
class phpbb_dbal_migrator_tool_permission_test extends phpbb_database_test_case
{
+ public $group_ids = array(
+ 'REGISTERED' => 2,
+ 'GLOBAL_MODERATORS' => 4,
+ 'ADMINISTRATORS' => 5,
+ );
+
public function getDataSet()
{
return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/migrator_permission.xml');
@@ -158,4 +164,60 @@ class phpbb_dbal_migrator_tool_permission_test extends phpbb_database_test_case
}
$this->assertFalse($this->tool->exists('global_test', true));
}
+
+ public function test_permission_set_data()
+ {
+ return array(
+ array(
+ 'ADMINISTRATORS',
+ 'a_test',
+ 'group',
+ true,
+ ),
+ array(
+ 'GLOBAL_MODERATORS',
+ 'm_test',
+ 'group',
+ true,
+ ),
+ array(
+ 'REGISTERED',
+ 'u_test',
+ 'group',
+ true,
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider test_permission_set_data
+ */
+ public function test_permission_set($group_name, $auth_option, $type, $has_permission)
+ {
+ $this->tool->permission_set($group_name, $auth_option, $type, $has_permission);
+ $administrators_perm = $this->auth->acl_group_raw_data($this->group_ids['ADMINISTRATORS'], $auth_option);
+ $global_moderators_perm = $this->auth->acl_group_raw_data($this->group_ids['GLOBAL_MODERATORS'], $auth_option);
+ $registered_users_perm = $this->auth->acl_group_raw_data($this->group_ids['REGISTERED'], $auth_option);
+
+ switch($group_name)
+ {
+ case 'GLOBAL_MODERATORS':
+ $this->assertEquals(false, empty($administrators_perm), 'm_test is not empty for Administrators');
+ $this->assertEquals(false, empty($global_moderators_perm), 'm_test is not empty for Global moderators');
+ $this->assertEquals(true, empty($registered_users_perm), 'm_test empty for Registered users');
+ break;
+
+ case 'ADMINISTRATORS':
+ $this->assertEquals(false, empty($administrators_perm), 'a_test is not empty for Administrators');
+ $this->assertEquals(true, empty($global_moderators_perm), 'a_test is empty for Global moderators');
+ $this->assertEquals(true, empty($registered_users_perm), 'a_test is empty for Registered users');
+ break;
+
+ case 'REGISTERED':
+ $this->assertEquals(false, empty($administrators_perm), 'u_test is not empty for Administrators');
+ $this->assertEquals(false, empty($global_moderators_perm), 'u_test is not empty for Global moderators');
+ $this->assertEquals(false, empty($registered_users_perm), 'u_test is not empty for Registered users');
+ break;
+ }
+ }
}