[ticket/9687] Remove dependency on log and user services

PHPBB3-9687
This commit is contained in:
Oliver Schramm 2018-10-05 21:32:44 +02:00 committed by Marc Alexander
parent e7015bf1dd
commit 40b1d1e6ff
No known key found for this signature in database
GPG key ID: 50E0D2423696F995
7 changed files with 169 additions and 245 deletions

View file

@ -6,12 +6,8 @@ services:
- '@ban.type_collection'
- '@cache'
- '@dbal.conn'
- '@log'
- '@user'
- '%tables.bans%'
- '%tables.users%'
- '%tables.sessions%'
- '%tables.sessions_keys%'
# ----- Ban types -----
ban.type_collection:
@ -25,8 +21,9 @@ services:
class: \phpbb\ban\type\email
arguments:
- '@dbal.conn'
- '@user'
- '%tables.users%'
- '%tables.sessions%'
- '%tables.sessions_keys%'
tags:
- { name: ban.type }
@ -34,8 +31,8 @@ services:
class: \phpbb\ban\type\user
arguments:
- '@dbal.conn'
- '@log'
- '@user'
- '%tables.users%'
- '%tables.sessions%'
- '%tables.sessions_keys%'
tags:
- { name: ban.type }

View file

@ -971,6 +971,8 @@ function user_ban($mode, $ban, $ban_len, $ban_len_other, $ban_exclude, $ban_reas
$end->setTimestamp($ban_end);
return $ban_manager->ban($mode, $items, $start, $end, $ban_reason, $ban_give_reason);
// TODO: logging
}
/**

View file

@ -32,15 +32,6 @@ class manager
/** @var \phpbb\db\driver\driver_interface */
protected $db;
/** @var \phpbb\log\log_interface */
protected $log;
/** @var string */
protected $sessions_keys_table;
/** @var string */
protected $sessions_table;
/** @var \phpbb\di\service_collection */
protected $types;
@ -57,23 +48,15 @@ class manager
* @param \phpbb\di\service_collection $types A service collection containing all ban types
* @param \phpbb\cache\service $cache A cache object
* @param \phpbb\db\driver\driver_interface $db A phpBB DBAL object
* @param \phpbb\log\log_interface $log A log object
* @param \phpbb\user $user An user 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 key table
*/
public function __construct($types, \phpbb\cache\service $cache, \phpbb\db\driver\driver_interface $db, \phpbb\log\log_interface $log, \phpbb\user $user, $bans_table, $users_table = '', $sessions_table = '', $sessions_keys_table = '')
public function __construct($types, \phpbb\cache\service $cache, \phpbb\db\driver\driver_interface $db, $bans_table, $users_table = '')
{
$this->bans_table = $bans_table;
$this->cache = $cache;
$this->db = $db;
$this->log = $log;
$this->sessions_keys_table = $sessions_keys_table;
$this->sessions_table = $sessions_table;
$this->types = $types;
$this->user = $user;
$this->users_table = $users_table;
}
@ -103,6 +86,10 @@ class manager
{
throw new type_not_found_exception(); // TODO
}
if (!empty($this->user))
{
$ban_mode->set_user($this->user);
}
$this->tidy();
$ban_items = $ban_mode->prepare_for_storage($items);
@ -137,19 +124,6 @@ class manager
throw new ban_insert_failed_exception(); // TODO
}
if ($ban_mode->get_ban_log_string() !== false)
{
$ban_items_log = implode(', ', $ban_items);
$this->log->add('admin', $this->user->data['user_id'], $this->user->ip, $ban_mode->get_ban_log_string(), false, [$reason, $ban_items_log]);
$this->log->add('mod', $this->user->data['user_id'], $this->user->ip, $ban_mode->get_ban_log_string(), false, [
'forum_id' => 0,
'topic_id' => 0,
$reason,
$ban_items_log,
]);
}
$ban_data = [
'items' => $ban_items,
'start' => $start,
@ -160,56 +134,7 @@ class manager
if ($ban_mode->after_ban($ban_data))
{
$user_column = $ban_mode->get_user_column();
if (!empty($user_column) && !empty($this->users_table))
{
if ($user_column !== 'user_id')
{
$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));
}
}
// TODO: Prevent logging out founders
$sql = 'SELECT user_id
FROM ' . $this->users_table . '
WHERE ' . $this->db->sql_in_set('u.' . $user_column, $ban_items_sql, false, true) . $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
{
$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);
}
}
// TODO
}
$this->cache->destroy(self::CACHE_KEY_INFO);
@ -223,9 +148,8 @@ class manager
*
* @param string $mode The ban type in which the ban IDs were created
* @param array $items An array of ban IDs which should be removed
* @param bool $logging True, if log entries should be created, false otherwise.
*/
public function unban($mode, array $items, $logging = true)
public function unban($mode, array $items)
{
/** @var \phpbb\ban\type\type_interface $ban_mode */
$ban_mode = $this->find_type($mode);
@ -252,23 +176,10 @@ class manager
WHERE ' . $this->db->sql_in_set('ban_id', $sql_ids);
$this->db->sql_query($sql);
if ($logging && $ban_mode->get_unban_log_string() !== false)
{
$unban_items_log = implode(', ', $unbanned_items);
$this->log->add('admin', $this->user->data['user_id'], $this->user->ip, $ban_mode->get_unban_log_string(), false, [$unban_items_log]);
$this->log->add('mod', $this->user->data['user_id'], $this->user->ip, $ban_mode->get_unban_log_string(), false, [
'forum_id' => 0,
'topic_id' => 0,
$unban_items_log,
]);
}
$unban_data = [
'items' => $unbanned_items,
'logging' => $logging,
];
$ban_mode->after_unban($unban_data);
$unbanned_users = $ban_mode->after_unban($unban_data);
$this->cache->destroy(self::CACHE_KEY_INFO);
$this->cache->destroy(self::CACHE_KEY_USERS);
@ -347,7 +258,7 @@ class manager
/**
* Returns all bans for a given ban type. False, if none were found
*
* @param strng $mode The ban type for which the entries should be retrieved
* @param string $mode The ban type for which the entries should be retrieved
*
* @return array|bool
*/
@ -416,7 +327,7 @@ class manager
$where_array,
],
['u.user_type', '<>', USER_FOUNDER],
]
],
],
];
$sql = $this->db->sql_build_query('SELECT', $sql_array);
@ -457,6 +368,16 @@ class manager
});
}
/**
* Sets the current user to exclude from banning
*
* @param \phpbb\user $user An user object
*/
public function set_user(\phpbb\user $user)
{
$this->user = $user;
}
/**
* Cleans up the database of e.g. stale bans
*/

View file

@ -21,6 +21,12 @@ abstract class base implements type_interface
/** @var array */
protected $excluded;
/** @var string */
protected $sessions_keys_table;
/** @var string */
protected $sessions_table;
/** @var \phpbb\user */
protected $user;
@ -30,15 +36,26 @@ abstract class base implements type_interface
/**
* Creates a ban type.
*
* @param \phpbb\db\driver\driver_interface $db A phpBB DBAL object
* @param \phpbb\user $user An user object
* @param string $users_table The users table
* @param \phpbb\db\driver\driver_interface $db A phpBB DBAL object
* @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(\phpbb\db\driver\driver_interface $db, \phpbb\user $user, $users_table)
public function __construct(\phpbb\db\driver\driver_interface $db, $users_table, $sessions_table, $sessions_keys_table)
{
$this->db = $db;
$this->user = $user;
$this->users_table = $users_table;
$this->sessions_table = $sessions_table;
$this->sessions_keys_table = $sessions_keys_table;
}
/**
* {@inheritDoc}
*/
public function set_user(\phpbb\user $user)
{
// TODO: Implement new logging
$this->user = $user;
}
/**
@ -46,7 +63,7 @@ abstract class base implements type_interface
*/
public function after_ban(array $data)
{
return true;
return $data['items'];
}
/**
@ -54,6 +71,7 @@ abstract class base implements type_interface
*/
public function after_unban(array $data)
{
return $data['items'];
}
/**
@ -94,9 +112,12 @@ abstract class base implements type_interface
return false;
}
$this->excluded = [
(int) $this->user->data['user_id'] => $this->user->data[$user_column],
];
$this->excluded = [];
if (!empty($this->user))
{
$this->excluded[(int) $this->user->data['user_id']] = $this->user->data[$user_column];
}
$sql = "SELECT user_id, {$user_column}
FROM {$this->users_table}
@ -111,4 +132,85 @@ abstract class base implements type_interface
return true;
}
/**
* Logs out all affected users in the given array. The values
* have to match the values of the column returned by get_user_column().
* Returns all banned users.
*
* @param array $ban_items
*
* @return array
*/
protected function logout_affected_users(array $ban_items)
{
$user_column = $this->get_user_column();
if (empty($user_column))
{
// TODO throw ex (it's a developer exception)
}
if ($user_column !== 'user_id')
{
$ban_items_sql = [];
$ban_like_items = [];
foreach ($ban_items as $ban_item)
{
if (stripos($ban_item, '*') === false)
{
$ban_items_sql[] = $ban_item;
}
else
{
$ban_like_items[] = [$user_column, 'LIKE', str_replace('*', $this->db->get_any_char(), $ban_item)];
}
}
$sql_array = [
'SELECT' => 'user_id',
'FROM' => [
$this->users_table => '',
],
'WHERE' => ['AND',
[
['OR',
array_merge([
[$user_column, 'IN', $ban_items_sql]
], $ban_like_items),
],
['user_id', 'NOT_IN', array_map('intval', array_keys($this->excluded))],
],
],
];
$sql = $this->db->sql_build_query('SELECT', $sql_array);
$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
{
$user_ids = array_map('intval', $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);
}
return $user_ids;
}
}

View file

@ -18,22 +18,6 @@ use phpbb\exception\runtime_exception;
class email extends base
{
/**
* {@inheritDoc}
*/
public function get_ban_log_string()
{
return 'LOG_BAN_EMAIL';
}
/**
* {@inheritDoc}
*/
public function get_unban_log_string()
{
return 'LOG_UNBAN_EMAIL';
}
/**
* {@inheritDoc}
*/
@ -50,6 +34,16 @@ class email extends base
return 'user_email';
}
/**
* {@inheritDoc}
*/
public function after_ban(array $data)
{
$this->logout_affected_users($data['items']);
return parent::after_ban($data);
}
/**
* {@inheritDoc}
*/
@ -59,7 +53,7 @@ class email extends base
{
throw new runtime_exception(); // TODO
}
$regex = '#^.*?@.*|(([a-z0-9\-]+\.)+([a-z]{2,3}))$#i';
$regex = '#^.*?@.*|(([a-z0-9\-]+\.)+([a-z]{2,3}))$#i'; // TODO
$ban_items = [];
foreach ($items as $item)

View file

@ -18,22 +18,6 @@ namespace phpbb\ban\type;
*/
interface type_interface
{
/**
* Returns the language key that's used for the ban log entry.
* False, if there is none (and thus no logs are created)
*
* @return string|bool
*/
public function get_ban_log_string();
/**
* Returns the language key that's used for the unban log entry.
* False, if thee is none (and thus no logs are created)
*
* @return string|bool
*/
public function get_unban_log_string();
/**
* Returns the type identifier for this ban type
*
@ -51,28 +35,38 @@ interface type_interface
*/
public function get_user_column();
/**
* Sets a user object to the ban type to have it excluded
* from banning.
*
* @param \phpbb\user $user An user object
*
* @return null
*/
public function set_user(\phpbb\user $user);
/**
* Gives the possibility to do some clean up after banning.
* Returns true if affected users should be logged out and
* false otherwise
* The return value of this method will be passed through
* to the caller.
*
* @param array $data An array containing information about
* the bans, like the reason or the start
* and end of the ban
*
* @return bool
* @return mixed
*/
public function after_ban(array $data);
/**
* Gives the possiblity to do some clean up after unbanning.
* The return value of this method will be ignored and thus
* should be null
* 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 null
* @return mixed
*/
public function after_unban(array $data);

View file

@ -21,47 +21,6 @@ class user extends base
/** @var array */
private $banned_users;
/** @var \phpbb\log\log_interface */
protected $log;
/** @var string */
private $ban_log_string = 'LOG_BAN_USER';
/** @var string */
private $unban_log_string = 'LOG_UNBAN_USER';
/**
* Creates the user ban type
*
* @param \phpbb\db\driver\driver_interface $db A phpBB DBAL object
* @param \phpbb\log\log_interface $log A log object
* @param \phpbb\user $user An user object
* @param string $users_table The users table
*/
public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\log\log_interface $log, \phpbb\user $user, $users_table)
{
$this->log = $log;
parent::__construct($db, $user, $users_table);
}
/**
* {@inheritDoc}
*/
public function get_ban_log_string()
{
// Have to handle logging differently here
return false;
}
/**
* {@inheritDoc}
*/
public function get_unban_log_string()
{
return false;
}
/**
* {@inheritDoc}
*/
@ -83,26 +42,8 @@ class user extends base
*/
public function after_ban(array $data)
{
$usernames_log = implode(', ', $this->banned_users);
$this->log->add('admin', $this->user->data['user_id'], $this->user->ip, $this->ban_log_string, false, [$data['reason'], $usernames_log]);
$this->log->add('mod', $this->user->data['user_id'], $this->user->ip, $this->ban_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->ban_log_string, false, [
'reportee_id' => $user_id,
$data['reason'],
$usernames_log,
]);
}
return true;
$this->logout_affected_users($data['items']);
return $this->banned_users;
}
/**
@ -110,11 +51,6 @@ class user extends base
*/
public function after_unban(array $data)
{
if (empty($data['logging']))
{
return;
}
$user_ids = array_map('intval', $data['items']);
$sql = 'SELECT user_id, username
@ -122,36 +58,14 @@ class user extends base
WHERE ' . $this->db->sql_in_set('user_id', $user_ids);
$result = $this->db->sql_query($sql);
$real_user_ids = [];
$usernames = [];
$unbanned_users = [];
while ($row = $this->db->sql_fetchrow($result))
{
$real_user_ids[] = $row['user_id'];
$usernames[] = $row['username'];
$unbanned_users[(int) $row['user_id']] = $row['username'];
}
$this->db->sql_freeresult($result);
if (empty($usernames))
{
return;
}
$usernames_log = implode(', ', $usernames);
$this->log->add('admin', $this->user->data['user_id'], $this->user->ip, $this->unban_log_string, false, [$usernames_log]);
$this->log->add('mod', $this->user->data['user_id'], $this->user->ip, $this->unban_log_string, false, [
'forum_id' => 0,
'topic_id' => 0,
$usernames_log,
]);
foreach ($real_user_ids as $user_id)
{
$this->log->add('user', $this->user->data['user_id'], $this->user->ip, $this->unban_log_string, false, [
'reportee_id' => $user_id,
$usernames_log,
]);
}
return $unbanned_users;
}
/**