From 110a6a0cbbd289484955a7c6f6f3f3c02d2122bf Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Mon, 15 Jul 2024 11:38:46 +0200 Subject: [PATCH] Address feedback from alecpl * rename `imap_connect` to `storage_connect` * rename `rcmail::storage_connect` to `storage_connect_from_session` * let `storage_connect` return rcube_user * let `storage_connect` include more code from `login()` --- program/actions/mail/index.php | 2 +- program/include/rcmail.php | 72 +++++---------------------------- program/lib/Roundcube/rcube.php | 69 ++++++++++++++++++++++++++++--- 3 files changed, 76 insertions(+), 67 deletions(-) diff --git a/program/actions/mail/index.php b/program/actions/mail/index.php index 3dc8caba9ba..079f78ca146 100644 --- a/program/actions/mail/index.php +++ b/program/actions/mail/index.php @@ -92,7 +92,7 @@ public function run($args = []) // set main env variables, labels and page title if (empty($rcmail->action) || $rcmail->action == 'list') { // connect to storage server and trigger error on failure - $rcmail->storage_connect(); + $rcmail->connect_storage_from_session(); $mbox_name = $rcmail->storage->get_folder(); diff --git a/program/include/rcmail.php b/program/include/rcmail.php index 55f3274689c..549da4af628 100644 --- a/program/include/rcmail.php +++ b/program/include/rcmail.php @@ -61,12 +61,6 @@ class rcmail extends rcube private $address_books = []; private $action_args = []; - public const ERROR_STORAGE = -2; - public const ERROR_INVALID_REQUEST = 1; - public const ERROR_INVALID_HOST = 2; - public const ERROR_COOKIES_DISABLED = 3; - public const ERROR_RATE_LIMIT = 4; - /** * This implements the 'singleton' design pattern * @@ -751,71 +745,27 @@ public function login($username, $password, $host = null, $cookiecheck = false, $username = rcube_utils::idn_to_ascii($username); } - // user already registered -> overwrite username - if ($user = rcube_user::query($username, $host)) { - $username = $user->data['username']; - - // Brute-force prevention - if ($user->is_locked()) { - $this->login_error = self::ERROR_RATE_LIMIT; - return false; - } - } - $storage = $this->get_storage(); // try to log in - if (!$this->imap_connect($storage, $host, $username, $password, $port, $ssl, $user)) { - return false; - } + $user = $this->storage_connect($storage, $host, $username, $password, $port, $ssl, $just_connect); - // Only set user if just wanting to connect. - // Note that for other scenarios user will also be set after successful login. - if ($just_connect) { - if (is_object($user)) { - $this->set_user($user); - } - return true; + if (!$user) { + return false; } - // user already registered -> update user's record - if (is_object($user)) { - // update last login timestamp - $user->touch(); + if (is_int($user) && $user == self::ERROR_RATE_LIMIT) { + $this->login_error = self::ERROR_RATE_LIMIT; + return false; } - // create new system user - elseif ($this->config->get('auto_create_user')) { - // Temporarily set user email and password, so plugins can use it - // this way until we set it in session later. This is required e.g. - // by the user-specific LDAP operations from new_user_identity plugin. - $domain = $this->config->mail_domain($host); - $this->user_email = strpos($username, '@') ? $username : sprintf('%s@%s', $username, $domain); - $this->password = $password; - - $user = rcube_user::create($username, $host); - $this->user_email = null; - $this->password = null; - - if (!$user) { - self::raise_error([ - 'code' => 620, - 'message' => 'Failed to create a user record. Maybe aborted by a plugin?', - ], true, false); - } - } else { - self::raise_error([ - 'code' => 621, - 'message' => "Access denied for new user {$username}. 'auto_create_user' is disabled", - ], true, false); + // This is enough if just connecting + if ($just_connect) { + return true; } // login succeeded - if (is_object($user) && $user->ID) { - // Configure environment - $this->set_user($user); - $this->set_storage_prop(); - + if ($user->ID) { // set session vars $_SESSION['user_id'] = $user->ID; $_SESSION['username'] = $user->data['username']; @@ -1996,7 +1946,7 @@ public function html2text($html, $options = []) * * @return bool True on success, False on error */ - public function storage_connect() + public function connect_storage_from_session() { $storage = $this->get_storage(); diff --git a/program/lib/Roundcube/rcube.php b/program/lib/Roundcube/rcube.php index 04076cb1730..4031d3c4032 100644 --- a/program/lib/Roundcube/rcube.php +++ b/program/lib/Roundcube/rcube.php @@ -38,6 +38,13 @@ class rcube public const DEBUG_LINE_LENGTH = 4096; + // Error codes + public const ERROR_STORAGE = -2; + public const ERROR_INVALID_REQUEST = 1; + public const ERROR_INVALID_HOST = 2; + public const ERROR_COOKIES_DISABLED = 3; + public const ERROR_RATE_LIMIT = 4; + /** @var rcube_config Stores instance of rcube_config */ public $config; @@ -1872,13 +1879,23 @@ public function deliver_message($message, $from, $mailto, &$error, * @param string $password IMAP password * @param int $port IMAP port to connect to * @param string $ssl SSL schema or false if plain connection - * @param rcube_user $user Roundcube user (if it already exists) * @param array $imap_options Additional IMAP options + * @param bool $just_connect Breaks after successful connect * - * @return bool Return true on successful login + * @return rcube_user|int|null Return user object on success, null or error code on failure */ - public function imap_connect($imap, $host, $username, $password, $port, $ssl, $user = null, $imap_options = []) + public function storage_connect($imap, $host, $username, $password, $port, $ssl, $imap_options = [], $just_connect = false) { + // user already registered -> overwrite username + if ($user = rcube_user::query($username, $host)) { + $username = $user->data['username']; + + // Brute-force prevention + if ($user->is_locked()) { + return self::ERROR_RATE_LIMIT; + } + } + // enable proxy authentication if (!empty($imap_options)) { $imap->set_options($imap_options); @@ -1892,10 +1909,52 @@ public function imap_connect($imap, $host, $username, $password, $port, $ssl, $u // Wait a second to slow down brute-force attacks (#1490549) sleep(1); - return false; + return null; } - return true; + // Only set user if just wanting to connect. + // Note that for other scenarios user will also be set after successful login. + if (!$just_connect) { + // user already registered -> update user's record + if (is_object($user)) { + // update last login timestamp + $user->touch(); + } + // create new system user + elseif ($this->config->get('auto_create_user')) { + // Temporarily set user email and password, so plugins can use it + // this way until we set it in session later. This is required e.g. + // by the user-specific LDAP operations from new_user_identity plugin. + $domain = $this->config->mail_domain($host); + $this->user_email = strpos($username, '@') ? $username : sprintf('%s@%s', $username, $domain); + $this->password = $password; + + $user = rcube_user::create($username, $host); + + $this->user_email = null; + $this->password = null; + + if (!$user) { + self::raise_error([ + 'code' => 620, + 'message' => 'Failed to create a user record. Maybe aborted by a plugin?', + ], true, false); + } + } else { + self::raise_error([ + 'code' => 621, + 'message' => "Access denied for new user {$username}. 'auto_create_user' is disabled", + ], true, false); + } + } + + if (is_object($user) && $user->ID) { + // Configure environment + $this->set_user($user); + $this->set_storage_prop(); + } + + return $user; } }