diff --git a/phpBB/config/default/container/services_ban.yml b/phpBB/config/default/container/services_ban.yml index cc34d4532c..c9e7c6ddb0 100644 --- a/phpBB/config/default/container/services_ban.yml +++ b/phpBB/config/default/container/services_ban.yml @@ -7,6 +7,7 @@ services: - '@cache.driver' - '@dbal.conn' - '@language' + - '@log' - '@user' - '%tables.bans%' - '%tables.users%' diff --git a/phpBB/phpbb/ban/manager.php b/phpBB/phpbb/ban/manager.php index 3303f23a25..feddc56f16 100644 --- a/phpBB/phpbb/ban/manager.php +++ b/phpBB/phpbb/ban/manager.php @@ -20,6 +20,7 @@ use phpbb\cache\driver\driver_interface as cache_driver; use phpbb\db\driver\driver_interface; use phpbb\di\service_collection; use phpbb\language\language; +use phpbb\log\log_interface; use phpbb\user; class manager @@ -43,6 +44,9 @@ class manager /** @var language */ protected $language; + /** @var log_interface */ + protected $log; + /** @var user */ protected $user; @@ -56,18 +60,21 @@ class manager * @param service_collection $types A service collection containing all ban types * @param cache_driver $cache A cache object * @param driver_interface $db A phpBB DBAL object + * @param language $language Language object + * @param log_interface $log Log object * @param user $user User object * @param string $bans_table The bans table * @param string $users_table The users table */ - public function __construct(service_collection $types, cache_driver $cache, driver_interface $db, - language $language, user $user, string $bans_table, string $users_table = '') + public function __construct(service_collection $types, cache_driver $cache, driver_interface $db, language $language, + log_interface $log, user $user, string $bans_table, string $users_table = '') { $this->bans_table = $bans_table; $this->cache = $cache; $this->db = $db; $this->types = $types; $this->language = $language; + $this->log = $log; $this->user = $user; $this->users_table = $users_table; } @@ -141,9 +148,28 @@ class manager 'display_reason' => $display_reason, ]; - if ($ban_mode->after_ban($ban_data)) + // Add to admin log, moderator log and user notes + $ban_list_log = implode(', ', $items); + + $log_operation = 'LOG_BAN_' . strtoupper($mode); + $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, $log_operation, false, [$reason, $ban_list_log]); + $this->log->add('mod', $this->user->data['user_id'], $this->user->ip, $log_operation, false, [ + 'forum_id' => 0, + 'topic_id' => 0, + $reason, + $ban_list_log + ]); + + if ($banlist_ary = $ban_mode->after_ban($ban_data)) { - // @todo: Add logging + foreach ($banlist_ary as $user_id) + { + $this->log->add('user', $this->user->data['user_id'], $this->user->ip, $log_operation, false, [ + 'reportee_id' => $user_id, + $reason, + $ban_list_log + ]); + } } $this->cache->destroy(self::CACHE_KEY_INFO); @@ -192,7 +218,25 @@ class manager 'items' => $unbanned_items, ]; $unbanned_users = $ban_mode->after_unban($unban_data); - // @todo: add logging for unbanned users + + // Add to moderator log, admin log and user notes + $log_operation = 'LOG_UNBAN_' . strtoupper($mode); + $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, $log_operation, false, [$unbanned_users]); + $this->log->add('mod', $this->user->data['user_id'], $this->user->ip, $log_operation, false, [ + 'forum_id' => 0, + 'topic_id' => 0, + $unbanned_users + ]); + if (count($unbanned_users)) + { + foreach ($unbanned_users as $user_id) + { + $this->log->add('user', $this->user->data['user_id'], $this->user->ip, $log_operation, false, array( + 'reportee_id' => $user_id, + $unbanned_users + )); + } + } } $this->cache->destroy(self::CACHE_KEY_INFO); diff --git a/phpBB/phpbb/ban/type/base.php b/phpBB/phpbb/ban/type/base.php index 92b324f9b3..a331813ab5 100644 --- a/phpBB/phpbb/ban/type/base.php +++ b/phpBB/phpbb/ban/type/base.php @@ -68,18 +68,17 @@ abstract class base implements type_interface /** * {@inheritDoc} */ - public function after_ban(array $data) + public function after_ban(array $data): array { - $this->logout_affected_users($data['items']); - return $data['items']; + return $this->logout_affected_users($data['items']); } /** * {@inheritDoc} */ - public function after_unban(array $data) + public function after_unban(array $data): array { - return $data['items']; + return []; } /** diff --git a/phpBB/phpbb/ban/type/type_interface.php b/phpBB/phpbb/ban/type/type_interface.php index 0c16a24319..db86fc8ae7 100644 --- a/phpBB/phpbb/ban/type/type_interface.php +++ b/phpBB/phpbb/ban/type/type_interface.php @@ -54,21 +54,21 @@ interface type_interface * the bans, like the reason or the start * and end of the ban * - * @return mixed + * @return array List of banned users */ - public function after_ban(array $data); + public function after_ban(array $data): array; /** - * Gives the possiblity to do some clean up after unbanning. + * Gives the possibility to do some clean up after unbanning. * The return value of this method will be passed through * to the caller. * * @param array $data An array containing information about * the unbans, e.g. the unbanned items. * - * @return mixed + * @return array List of unbanned users */ - public function after_unban(array $data); + public function after_unban(array $data): array; /** * In the case that get_user_column() returns null, this method diff --git a/phpBB/phpbb/ban/type/user.php b/phpBB/phpbb/ban/type/user.php index 7e01ac8bb5..c803e1b16f 100644 --- a/phpBB/phpbb/ban/type/user.php +++ b/phpBB/phpbb/ban/type/user.php @@ -39,7 +39,7 @@ class user extends base /** * {@inheritDoc} */ - public function after_ban(array $data) + public function after_ban(array $data): array { $this->logout_affected_users($data['items']); return $this->banned_users; @@ -48,7 +48,7 @@ class user extends base /** * {@inheritDoc} */ - public function after_unban(array $data) + public function after_unban(array $data): array { $user_ids = array_map('intval', $data['items']); diff --git a/tests/ban/ban_manager_test.php b/tests/ban/ban_manager_test.php index 408c95e304..a5f3620217 100644 --- a/tests/ban/ban_manager_test.php +++ b/tests/ban/ban_manager_test.php @@ -33,6 +33,7 @@ class ban_manager_test extends \phpbb_session_test_case $user = new \phpbb\user($language, '\phpbb\datetime'); $user->data['user_id'] = 2; $user->data['user_email'] = 'foo@bar.com'; + $user->data['user_timezone'] = 0; $config = new \phpbb\config\config([]); $phpbb_dispatcher = new \phpbb_mock_event_dispatcher(); @@ -47,8 +48,9 @@ class ban_manager_test extends \phpbb_session_test_case $collection->add('ban.type.email'); $collection->add('ban.type.user'); $collection->add('ban.type.ip'); + $phpbb_log = new \phpbb\log\dummy(); - $this->ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $language, $user, 'phpbb_bans', 'phpbb_users'); + $this->ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $language, $phpbb_log, $user, 'phpbb_bans', 'phpbb_users'); $phpbb_container->set('ban.manager', $this->ban_manager); $this->phpbb_container = $phpbb_container; } @@ -392,8 +394,9 @@ class ban_manager_test extends \phpbb_session_test_case $language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); $user = new \phpbb\user($language, '\phpbb\datetime'); + $phpbb_log = new \phpbb\log\dummy(); - $ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $language, $user, 'phpbb_bans', 'phpbb_users'); + $ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $language, $phpbb_log, $user, 'phpbb_bans', 'phpbb_users'); $this->assertEquals( [ @@ -434,8 +437,9 @@ class ban_manager_test extends \phpbb_session_test_case $language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); $user = new \phpbb\user($language, '\phpbb\datetime'); + $phpbb_log = new \phpbb\log\dummy(); - $ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $language, $user, 'phpbb_bans', 'phpbb_users'); + $ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $language, $phpbb_log, $user, 'phpbb_bans', 'phpbb_users'); $start_time = new \DateTime(); $start_time->setTimestamp(1000); diff --git a/tests/functions/validate_user_email_test.php b/tests/functions/validate_user_email_test.php index c698f1263f..a43244bd50 100644 --- a/tests/functions/validate_user_email_test.php +++ b/tests/functions/validate_user_email_test.php @@ -61,8 +61,9 @@ class phpbb_functions_validate_user_email_test extends phpbb_database_test_case $collection->add('ban.type.email'); $collection->add('ban.type.user'); $collection->add('ban.type.ip'); + $phpbb_log = new \phpbb\log\dummy(); - $ban_manager = new \phpbb\ban\manager($collection, $cache->get_driver(), $this->db, $language, $this->user, 'phpbb_bans', 'phpbb_users'); + $ban_manager = new \phpbb\ban\manager($collection, $cache->get_driver(), $this->db, $language, $phpbb_log, $this->user, 'phpbb_bans', 'phpbb_users'); $phpbb_container->set('ban.manager', $ban_manager); } diff --git a/tests/session/check_ban_test.php b/tests/session/check_ban_test.php index 9dcb148f2b..44a4012d3b 100644 --- a/tests/session/check_ban_test.php +++ b/tests/session/check_ban_test.php @@ -86,8 +86,9 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case $collection->add('ban.type.email'); $collection->add('ban.type.user'); $collection->add('ban.type.ip'); + $phpbb_log = new \phpbb\log\dummy(); - $ban_manager = new \phpbb\ban\manager($collection, $cache->get_driver(), $this->db, $language, $user, 'phpbb_bans', 'phpbb_users'); + $ban_manager = new \phpbb\ban\manager($collection, $cache->get_driver(), $this->db, $language, $phpbb_log, $user, 'phpbb_bans', 'phpbb_users'); $phpbb_container->set('ban.manager', $ban_manager); } diff --git a/tests/session/testable_factory.php b/tests/session/testable_factory.php index ea057674a4..36e9fe6d3f 100644 --- a/tests/session/testable_factory.php +++ b/tests/session/testable_factory.php @@ -120,8 +120,9 @@ class phpbb_session_testable_factory $collection->add('ban.type.email'); $collection->add('ban.type.user'); $collection->add('ban.type.ip'); + $phpbb_log = new \phpbb\log\dummy(); - $ban_manager = new \phpbb\ban\manager($collection, $cache, $db, $language, $user,'phpbb_bans', 'phpbb_users'); + $ban_manager = new \phpbb\ban\manager($collection, $cache, $db, $language, $phpbb_log, $user,'phpbb_bans', 'phpbb_users'); $phpbb_container->set('ban.manager', $ban_manager); $session = new phpbb_mock_session_testable;