[ticket/9687] Improve display of ban options and update tests

PHPBB3-9687

phpBB/config/default/container/services_ban.yml

@@ -22,6 +22,7 @@ services:
         class: \phpbb\ban\type\email
         arguments:
             - '@dbal.conn'
+            - '%tables.bans%'
             - '%tables.users%'
             - '%tables.sessions%'
             - '%tables.sessions_keys%'
@@ -32,6 +33,7 @@ services:
         class: \phpbb\ban\type\ip
         arguments:
             - '@dbal.conn'
+            - '%tables.bans%'
             - '%tables.users%'
             - '%tables.sessions%'
             - '%tables.sessions_keys%'
@@ -42,6 +44,7 @@ services:
         class: \phpbb\ban\type\user
         arguments:
             - '@dbal.conn'
+            - '%tables.bans%'
             - '%tables.users%'
             - '%tables.sessions%'
             - '%tables.sessions_keys%'

phpBB/phpbb/ban/type/base.php

@@ -23,6 +23,9 @@ abstract class base implements type_interface
 	/** @var array */
 	protected $excluded;
 
+	/** @var string */
+	protected $bans_table;
+
 	/** @var string */
 	protected $sessions_keys_table;
 
@@ -39,11 +42,12 @@ abstract class base implements type_interface
 	 * Creates a ban type.
 	 *
 	 * @param driver_interface	$db						A phpBB DBAL object
+	 * @param string			$bans_table				The bans table
 	 * @param string			$users_table			The users table
 	 * @param string			$sessions_table			The sessions table
 	 * @param string			$sessions_keys_table	The sessions keys table
 	 */
-	public function __construct(driver_interface $db, string $users_table, string $sessions_table, string $sessions_keys_table)
+	public function __construct(driver_interface $db, string $bans_table, string $users_table, string $sessions_table, string $sessions_keys_table)
 	{
 		$this->db = $db;
 		$this->users_table = $users_table;

phpBB/phpbb/ban/type/user.php

@@ -86,7 +86,7 @@ class user extends base
 		$result = $this->db->sql_query($sql);
 		while ($row = $this->db->sql_fetchrow($result))
 		{
-			$row['ban_item'] = $row['username'];
+			$row['ban_item'] = $row['username'] ?: $row['ban_item'];
 			$ban_options[] = $row;
 		}
 		$this->db->sql_freeresult($result);

tests/ban/ban_manager_test.php

@@ -49,9 +49,9 @@ class ban_manager_test extends \phpbb_session_test_case
 		);
 
 		$phpbb_container = new \phpbb_mock_container_builder();
-		$ban_type_email = new \phpbb\ban\type\email($this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_email = new \phpbb\ban\type\email($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
-		$ban_type_user = new \phpbb\ban\type\user($this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_user = new \phpbb\ban\type\user($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
-		$ban_type_ip = new \phpbb\ban\type\ip($this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_ip = new \phpbb\ban\type\ip($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
 		$phpbb_container->set('ban.type.email', $ban_type_email);
 		$phpbb_container->set('ban.type.user', $ban_type_user);
 		$phpbb_container->set('ban.type.ip', $ban_type_ip);
@@ -191,6 +191,15 @@ class ban_manager_test extends \phpbb_session_test_case
 			[
 				'ip',
 				[
+					[
+						'ban_id' => '6',
+						'ban_item' => '10.0.0.1/28',
+						'ban_start' => '1111',
+						'ban_end' => '0',
+						'ban_reason' => 'HAHAHA',
+						'ban_reason_display' => '1',
+						'ban_mode' => 'ip',
+					],
 					[
 						'ban_id' => '2',
 						'ban_item' => '127.0.0.1',
@@ -198,6 +207,7 @@ class ban_manager_test extends \phpbb_session_test_case
 						'ban_end' => '0',
 						'ban_reason' => 'HAHAHA',
 						'ban_reason_display' => '1',
+						'ban_mode' => 'ip',
 					],
 					[
 						'ban_id' => '3',
@@ -206,14 +216,7 @@ class ban_manager_test extends \phpbb_session_test_case
 						'ban_end' => '0',
 						'ban_reason' => 'HAHAHA',
 						'ban_reason_display' => '1',
-					],
+						'ban_mode' => 'ip',
-					[
-						'ban_id' => '6',
-						'ban_item' => '10.0.0.1/28',
-						'ban_start' => '1111',
-						'ban_end' => '0',
-						'ban_reason' => 'HAHAHA',
-						'ban_reason_display' => '1',
 					],
 					[
 						'ban_id' => '7',
@@ -222,20 +225,13 @@ class ban_manager_test extends \phpbb_session_test_case
 						'ban_end' => '0',
 						'ban_reason' => 'HAHAHA',
 						'ban_reason_display' => '1',
+						'ban_mode' => 'ip',
 					],
 				],
 			],
 			[
 				'email',
 				[
-					[
-						'ban_id' => '5',
-						'ban_item' => 'bar@example.org',
-						'ban_start' => '1111',
-						'ban_end' => '0',
-						'ban_reason' => 'HAHAHA',
-						'ban_reason_display' => '1',
-					],
 					[
 						'ban_id' => '9',
 						'ban_item' => '*@foo.bar',
@@ -243,6 +239,16 @@ class ban_manager_test extends \phpbb_session_test_case
 						'ban_end' => '0',
 						'ban_reason' => 'HAHAHA',
 						'ban_reason_display' => '1',
+						'ban_mode' => 'email',
+					],
+					[
+						'ban_id' => '5',
+						'ban_item' => 'bar@example.org',
+						'ban_start' => '1111',
+						'ban_end' => '0',
+						'ban_reason' => 'HAHAHA',
+						'ban_reason_display' => '1',
+						'ban_mode' => 'email',
 					],
 				],
 			],
@@ -256,6 +262,10 @@ class ban_manager_test extends \phpbb_session_test_case
 						'ban_end' => '0',
 						'ban_reason' => 'HAHAHA',
 						'ban_reason_display' => '1',
+						'ban_mode' => 'user',
+						'user_id' => '4',
+						'username' => '',
+						'username_clean' => 'ipv6_user',
 					],
 				],
 			],
@@ -354,10 +364,10 @@ class ban_manager_test extends \phpbb_session_test_case
 		global $phpbb_root_path, $phpEx;
 
 		$phpbb_container = new \phpbb_mock_container_builder();
-		$ban_type_email = new \phpbb\ban\type\email($this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_email = new \phpbb\ban\type\email($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
-		$ban_type_user = new \phpbb\ban\type\user($this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_user = new \phpbb\ban\type\user($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
 		$ban_type_ip = $this->getMockBuilder(\phpbb\ban\type\ip::class)
-			->setConstructorArgs([$this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys'])
+			->setConstructorArgs([$this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys'])
 			->getMock();
 		$ban_type_ip->method('get_banned_users')
 			->willReturn([19 => 1234, 20 => 0]);
@@ -409,10 +419,10 @@ class ban_manager_test extends \phpbb_session_test_case
 		global $phpbb_root_path, $phpEx;
 
 		$phpbb_container = new \phpbb_mock_container_builder();
-		$ban_type_email = new \phpbb\ban\type\email($this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_email = new \phpbb\ban\type\email($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
-		$ban_type_user = new \phpbb\ban\type\user($this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_user = new \phpbb\ban\type\user($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
 		$ban_type_ip = $this->getMockBuilder(\phpbb\ban\type\ip::class)
-			->setConstructorArgs([$this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys'])
+			->setConstructorArgs([$this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys'])
 			->getMock();
 		$ban_type_ip->method('prepare_for_storage')
 			->willReturn([]);
@@ -652,6 +662,10 @@ class ban_manager_test extends \phpbb_session_test_case
 						'ban_end' => '0',
 						'ban_reason' => 'HAHAHA',
 						'ban_reason_display' => '1',
+						'ban_mode' => 'user',
+						'user_id' => '4',
+						'username' => '',
+						'username_clean' => 'ipv6_user',
 					],
 				],
 			],

tests/functions/validate_user_email_test.php

@@ -51,13 +51,16 @@ class phpbb_functions_validate_user_email_test extends phpbb_database_test_case
 		);
 		$cache->get_driver()->purge();
 
-		$ban_type_email = new \phpbb\ban\type\email($this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_email = new \phpbb\ban\type\email($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
-		$ban_type_user = new \phpbb\ban\type\user($this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_user = new \phpbb\ban\type\user($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_ip = new \phpbb\ban\type\ip($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
 		$phpbb_container->set('ban.type.email', $ban_type_email);
 		$phpbb_container->set('ban.type.user', $ban_type_user);
+		$phpbb_container->set('ban.type.ip', $ban_type_ip);
 		$collection = new \phpbb\di\service_collection($phpbb_container);
 		$collection->add('ban.type.email');
 		$collection->add('ban.type.user');
+		$collection->add('ban.type.ip');
 
 		$ban_manager = new \phpbb\ban\manager($collection, $cache, $this->db, $this->user, 'phpbb_bans', 'phpbb_users');
 		$phpbb_container->set('ban.manager', $ban_manager);

tests/session/check_ban_test.php

@@ -76,13 +76,16 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case
 		);
 
 		$phpbb_container = new phpbb_mock_container_builder();
-		$ban_type_email = new \phpbb\ban\type\email($this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_email = new \phpbb\ban\type\email($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
-		$ban_type_user = new \phpbb\ban\type\user($this->db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_user = new \phpbb\ban\type\user($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_ip = new \phpbb\ban\type\ip($this->db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
 		$phpbb_container->set('ban.type.email', $ban_type_email);
 		$phpbb_container->set('ban.type.user', $ban_type_user);
+		$phpbb_container->set('ban.type.ip', $ban_type_ip);
 		$collection = new \phpbb\di\service_collection($phpbb_container);
 		$collection->add('ban.type.email');
 		$collection->add('ban.type.user');
+		$collection->add('ban.type.ip');
 
 		$ban_manager = new \phpbb\ban\manager($collection, $cache, $this->db, $user, 'phpbb_bans', 'phpbb_users');
 		$phpbb_container->set('ban.manager', $ban_manager);

tests/session/testable_factory.php

@@ -118,13 +118,17 @@ class phpbb_session_testable_factory
 			$phpEx
 		);
 
-		$ban_type_email = new \phpbb\ban\type\email($db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_email = new \phpbb\ban\type\email($db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
-		$ban_type_user = new \phpbb\ban\type\user($db, 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_user = new \phpbb\ban\type\user($db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
+		$ban_type_ip = new \phpbb\ban\type\ip($db, 'phpbb_bans', 'phpbb_users', 'phpbb_sessions', 'phpbb_sessions_keys');
 		$phpbb_container->set('ban.type.email', $ban_type_email);
 		$phpbb_container->set('ban.type.user', $ban_type_user);
+		$phpbb_container->set('ban.type.ip', $ban_type_ip);
+
 		$collection = new \phpbb\di\service_collection($phpbb_container);
 		$collection->add('ban.type.email');
 		$collection->add('ban.type.user');
+		$collection->add('ban.type.ip');
 
 		$ban_manager = new \phpbb\ban\manager($collection, $cache_service, $db,  $user,'phpbb_bans', 'phpbb_users');
 		$phpbb_container->set('ban.manager', $ban_manager);