[ticket/9687] Finishing user banning and ban logging

PHPBB3-9687

phpBB/phpbb/ban/manager.php

@@ -20,6 +20,8 @@ class manager
 {
 	protected $ban_table;
 
+	protected $cache;
+
 	protected $db;
 
 	protected $log;
@@ -34,9 +36,10 @@ class manager
 
 	protected $users_table;
 
-	public function __construct($types, \phpbb\db\driver\driver_interface $db, \phpbb\log\log_interface $log, \phpbb\user $user, $ban_table, $users_table = '', $sessions_table = '', $sessions_keys_table = '')
+	public function __construct($types, \phpbb\cache\service $cache, \phpbb\db\driver\driver_interface $db, \phpbb\log\log_interface $log, \phpbb\user $user, $ban_table, $users_table = '', $sessions_table = '', $sessions_keys_table = '')
 	{
 		$this->ban_table = $ban_table;
+		$this->cache = $cache;
 		$this->db = $db;
 		$this->log = $log;
 		$this->sessions_keys_table = $sessions_keys_table;
@@ -46,7 +49,7 @@ class manager
 		$this->users_table = $users_table;
 	}
 
-	public function ban($mode, array $items, \DateTimeInterface $start, \DateTimeInterface $end, $reason, $display_reason = '', $logging = true)
+	public function ban($mode, array $items, \DateTimeInterface $start, \DateTimeInterface $end, $reason, $display_reason = '')
 	{
 		if (!isset($this->types[$mode]))
 		{
@@ -64,7 +67,7 @@ class manager
 		// Prevent duplicate bans
 		$sql = 'DELETE FROM ' . $this->ban_table . "
 			WHERE ban_mode = '" . $this->db->sql_escape($mode) . "'
-			AND " . $this->db->sql_in_set('ban_item', $ban_items);
+			AND " . $this->db->sql_in_set('ban_item', $ban_items); // TODO (what if empty?)
 		$this->db->sql_query($sql);
 
 		$insert_array = [];
@@ -82,7 +85,7 @@ class manager
 
 		if (empty($insert_array))
 		{
-			return;
+			return; // TODO
 		}
 
 		$result = $this->db->sql_multi_insert($this->ban_table, $insert_array);
@@ -92,62 +95,86 @@ class manager
 			// TODO throw exception
 		}
 
-		if ($logging)
+		if ($ban_mode->get_log_string() !== false)
 		{
-			// TODO logging
+			$ban_items_log = implode(', ', $ban_items);
+
+			$this->log->add('admin', $this->user->data['user_id'], $this->user->ip, $ban_mode->get_log_string(), false, [$reason, $ban_items_log]);
+			$this->log->add('mod', $this->user->data['user_id'], $this->user->ip, $ban_mode->get_log_string(), false, [
+				'forum_id'	=> 0,
+				'topic_id'	=> 0,
+				$reason,
+				$ban_items_log,
+			]);
 		}
 
-		if (!$ban_mode->after_ban())
+		$ban_data = [
-		{
+			'items'				=> $ban_items,
-			return;
+			'start'				=> $start,
-		}
+			'end'				=> $end,
+			'reason'			=> $reason,
+			'display_reason'	=> $display_reason,
+		];
 
-		$user_column = $ban_mode->get_user_column();
+		if ($ban_mode->after_ban($ban_data))
-		if (!empty($user_column) && !empty($this->users_table))
 		{
-			$ban_items_sql = [];
+			$user_column = $ban_mode->get_user_column();
-			$ban_or_like = '';
+			if (!empty($user_column) && !empty($this->users_table))
-			foreach ($ban_items as $ban_item)
 			{
-				if (stripos($ban_item, '*') === false)
+				if ($user_column !== 'user_id')
 				{
-					$ban_items_sql[] = $ban_item;
+					$ban_items_sql = [];
+					$ban_or_like = '';
+					foreach ($ban_items as $ban_item)
+					{
+						if (stripos($ban_item, '*') === false)
+						{
+							$ban_items_sql[] = $ban_item;
+						}
+						else
+						{
+							$ban_or_like .= ' OR ' . $user_column . ' ' . $this->db->sql_like_expression(str_replace('*', $this->db->get_any_char(), $ban_item));
+						}
+					}
+
+					$sql = 'SELECT user_id
+						FROM ' . $this->users_table . '
+						WHERE ' . $this->db->sql_in_set('u.' . $user_column, $ban_items_sql) . $ban_or_like;
+					$result = $this->db->sql_query($sql);
+
+					$user_ids = [];
+					while ($row = $this->db->sql_fetchrow($result))
+					{
+						$user_ids[] = (int)$row['user_id'];
+					}
+					$this->db->sql_freeresult($result);
 				}
 				else
 				{
-					$ban_or_like .= ' OR ' . $user_column . ' ' . $this->db->sql_like_expression(str_replace('*', $this->db->get_any_char(), $ban_item));
+					$user_ids = $ban_items;
+				}
+
+				if (!empty($user_ids) && !empty($this->sessions_table))
+				{
+					$sql = 'DELETE FROM ' . $this->sessions_table . '
+						WHERE ' . $this->db->sql_in_set('session_user_id', $user_ids);
+					$this->db->sql_query($sql);
+				}
+				if (!empty($user_ids) && !empty($this->sessions_keys_table))
+				{
+					$sql = 'DELETE FROM ' . $this->sessions_keys_table . '
+						WHERE ' . $this->db->sql_in_set('user_id', $user_ids);
+					$this->db->sql_query($sql);
 				}
 			}
-
-			$sql = 'SELECT user_id
-				FROM ' . $this->users_table . '
-				WHERE ' . $this->db->sql_in_set('u.' . $user_column, $ban_items) . $ban_or_like;
-			$result = $this->db->sql_query($sql);
-
-			$user_ids = [];
-			while ($row = $this->db->sql_fetchrow($result))
-			{
-				$user_ids[] = (int)$row['user_id'];
-			}
-			$this->db->sql_freeresult($result);
-
-			if (!empty($user_ids) && !empty($this->sessions_table))
-			{
-				$sql = 'DELETE FROM ' . $this->sessions_table . '
-				WHERE ' . $this->db->sql_in_set('session_user_id', $user_ids);
-				$this->db->sql_query($sql);
-			}
-			if (!empty($user_ids) && !empty($this->sessions_keys_table))
-			{
-				$sql = 'DELETE FROM ' . $this->sessions_keys_table . '
-				WHERE ' . $this->db->sql_in_set('user_id', $user_ids);
-				$this->db->sql_query($sql);
-			}
 		}
+
+		$this->cache->destroy('sql', $this->ban_table);
 	}
 
 	public function unban($mode, array $items, $reason, $logging = true)
 	{
+
 	}
 
 	public function check(array $user_data = [])

phpBB/phpbb/ban/type/base.php

@@ -21,27 +21,23 @@ abstract class base implements type_interface
 	/** @var array */
 	protected $excluded;
 
+	/** @var \phpbb\user */
+	protected $user;
+
 	/** @var string */
 	protected $users_table;
 
-	public function __construct(\phpbb\db\driver\driver_interface $db, $users_table)
+	public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\user $user, $users_table)
 	{
 		$this->db = $db;
+		$this->user = $user;
 		$this->users_table = $users_table;
 	}
 
 	/**
 	 * {@inheritDoc}
 	 */
-	public function get_user_column()
+	public function after_ban($data)
-	{
-		return null;
-	}
-
-	/**
-	 * {@inheritDoc}
-	 */
-	public function after_ban()
 	{
 		return true;
 	}
@@ -77,7 +73,9 @@ abstract class base implements type_interface
 			return false;
 		}
 
-		$this->excluded = [];
+		$this->excluded = [
+			(int)$this->user->data['user_id']	=> $this->user->data[$user_column],
+		];
 
 		$sql = "SELECT user_id, {$user_column}
 			FROM {$this->users_table}

phpBB/phpbb/ban/type/email.php

@@ -15,6 +15,11 @@ namespace phpbb\ban\type;
 
 class email extends base
 {
+	public function get_log_string()
+	{
+		return 'LOG_BAN_EMAIL';
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */

phpBB/phpbb/ban/type/type_interface.php

@@ -18,6 +18,14 @@ namespace phpbb\ban\type;
  */
 interface type_interface
 {
+	/**
+	 * Returns the language key that's used for the log entry.
+	 * False, if there is none (and thus no logs are created)
+	 *
+	 * @return string|bool
+	 */
+	public function get_log_string();
+
 	/**
 	 * Returns the type identifier for this ban type
 	 *
@@ -40,9 +48,13 @@ interface type_interface
 	 * Returns true if affected users should be logged out and
 	 * false otherwise
 	 *
+	 * @param array $data An array containing information about
+	 *                    the bans, like the reason or the start
+	 *                    and end of the ban
+	 *
 	 * @return bool
 	 */
-	public function after_ban();
+	public function after_ban($data);
 
 	public function after_unban(); // ???
 

phpBB/phpbb/ban/type/user.php

@@ -15,16 +15,70 @@ namespace phpbb\ban\type;
 
 class user extends base
 {
+	/** @var array */
+	private $banned_users;
+
+	/** @var \phpbb\log\log_interface */
+	protected $log;
+
+	/** @var string */
+	private $log_string = 'LOG_BAN_USER';
+
+	/**
+	 * {@inheritDoc}
+	 */
+	public function get_log_string()
+	{
+		// Have to handle logging differently here
+		return false;
+	}
+
+	/**
+	 * {@inheritDoc}
+	 */
 	public function get_type()
 	{
 		return 'user';
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
 	public function get_user_column()
 	{
 		return 'user_id';
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
+	public function after_ban($data)
+	{
+		$usernames_log = implode(', ', $this->banned_users);
+
+		$this->log->add('admin', $this->user->data['user_id'], $this->user->ip, $this->log_string, false, [$data['reason'], $usernames_log]);
+		$this->log->add('mod', $this->user->data['user_id'], $this->user->ip, $this->log_string, false, [
+			'forum_id'	=> 0,
+			'topic_id'	=> 0,
+			$data['reason'],
+			$usernames_log,
+		]);
+
+		foreach ($this->banned_users as $user_id => $username)
+		{
+			$this->log->add('user', $this->user->data['user_id'], $this->user->ip, $this->log_string, false, [
+				'reportee_id'	=> $user_id,
+				$data['reason'],
+				$usernames_log,
+			]);
+		}
+
+		return true;
+	}
+
+	/**
+	 * {@inheritDoc}
+	 */
 	public function prepare_for_storage(array $items)
 	{
 		if (!$this->get_excluded())
@@ -48,7 +102,7 @@ class user extends base
 		}
 
 		$sql_array = [
-			'SELECT'	=> 'user_id',
+			'SELECT'	=> 'user_id, username',
 			'FROM'		=> [
 				$this->users_table	=> '',
 			],
@@ -67,9 +121,11 @@ class user extends base
 		$result = $this->db->sql_query($sql);
 
 		$ban_items = [];
+		$this->banned_users = [];
 		while ($row = $this->db->sql_fetchrow($result))
 		{
 			$ban_items[] = (int) $row['user_id'];
+			$this->banned_users[(int) $row['user_id']] = $row['username'];
 		}
 		$this->db->sql_freeresult($result);