From 8b291d74d638d724899e76a3bbc5bf5a03c39bcd Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Tue, 16 Apr 2024 11:47:04 +0200 Subject: [PATCH 1/3] Add flag to avoid setting/creating user on login Fixes #9377 . Allows rcmail::login() to be reused in plugins providing an API. The login function provides some useful logic for connecting to IMAP sources like checking credentials for validity or converting usernames in some cases. Before this change the login function always also created non-existing users or set some sesison vars, which what API plugins would like to avoid. --- program/include/rcmail.php | 20 ++++++++++++++----- program/lib/Roundcube/rcube.php | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/program/include/rcmail.php b/program/include/rcmail.php index 515b77b760..c4ab3fd48c 100644 --- a/program/include/rcmail.php +++ b/program/include/rcmail.php @@ -648,14 +648,15 @@ public function session_init() * Perform login to the mail server and to the webmail service. * This will also create a new user entry if auto_create_user is configured. * - * @param string $username Mail storage (IMAP) user name - * @param string $password Mail storage (IMAP) password - * @param string $host Mail storage (IMAP) host - * @param bool $cookiecheck Enables cookie check + * @param string $username Mail storage (IMAP) user name + * @param string $password Mail storage (IMAP) password + * @param string $host Mail storage (IMAP) host + * @param bool $cookiecheck Enables cookie check + * @param bool $just_connect Breaks after successful connect * * @return bool True on success, False on failure */ - public function login($username, $password, $host = null, $cookiecheck = false) + public function login($username, $password, $host = null, $cookiecheck = false, $just_connect = false) { $this->login_error = null; @@ -774,6 +775,15 @@ public function login($username, $password, $host = null, $cookiecheck = false) return false; } + // Only set user if just wanting to connect + if ($just_connect) { + if (is_object($user)) { + $this->set_user($user); + } + return true; + } + + // user already registered -> update user's record if (is_object($user)) { // update last login timestamp diff --git a/program/lib/Roundcube/rcube.php b/program/lib/Roundcube/rcube.php index dd6d053787..04076cb173 100644 --- a/program/lib/Roundcube/rcube.php +++ b/program/lib/Roundcube/rcube.php @@ -1862,6 +1862,41 @@ public function deliver_message($message, $from, $mailto, &$error, return $sent; } + + /** + * Helper method to establish connection to an IMAP backend. + * + * @param rcube_storage $imap IMAP storage handler + * @param string $host IMAP host + * @param string $username IMAP username + * @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 + * + * @return bool Return true on successful login + */ + public function imap_connect($imap, $host, $username, $password, $port, $ssl, $user = null, $imap_options = []) + { + // enable proxy authentication + if (!empty($imap_options)) { + $imap->set_options($imap_options); + } + + // try to log in + if (!$imap->connect($host, $username, $password, $port, $ssl)) { + if ($user) { + $user->failed_login(); + } + + // Wait a second to slow down brute-force attacks (#1490549) + sleep(1); + return false; + } + + return true; + } } /** From 9b65ce17cd8e65ec492085619d3ee3a2b25e61f0 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Tue, 16 Apr 2024 11:49:12 +0200 Subject: [PATCH 2/3] Provide function in rcube to connect to IMAP This is useful for plugins like FreeBusy [1] that previously connected to IMAP via plain rcube_imap::connect(). [1] https://git.kolab.org/source/freebusy/browse/master/lib/Kolab/FreeBusy/SourceIMAP.php --- program/include/rcmail.php | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/program/include/rcmail.php b/program/include/rcmail.php index c4ab3fd48c..55f3274689 100644 --- a/program/include/rcmail.php +++ b/program/include/rcmail.php @@ -765,17 +765,12 @@ public function login($username, $password, $host = null, $cookiecheck = false, $storage = $this->get_storage(); // try to log in - if (!$storage->connect($host, $username, $password, $port, $ssl)) { - if ($user) { - $user->failed_login(); - } - - // Wait a second to slow down brute-force attacks (#1490549) - sleep(1); + if (!$this->imap_connect($storage, $host, $username, $password, $port, $ssl, $user)) { return false; } - // Only set user if just wanting to 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); @@ -783,7 +778,6 @@ public function login($username, $password, $host = null, $cookiecheck = false, return true; } - // user already registered -> update user's record if (is_object($user)) { // update last login timestamp From 110a6a0cbbd289484955a7c6f6f3f3c02d2122bf Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Mon, 15 Jul 2024 11:38:46 +0200 Subject: [PATCH 3/3] 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 3dc8caba9b..079f78ca14 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 55f3274689..549da4af62 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 04076cb173..4031d3c403 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; } }