Skip to content

Commit 110a6a0

Browse files
jaudrigajaudriga
committed
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()`
1 parent 9b65ce1 commit 110a6a0

File tree

3 files changed

+76
-67
lines changed

3 files changed

+76
-67
lines changed

‎program/actions/mail/index.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public function run($args = [])
9292
// set main env variables, labels and page title
9393
if (empty($rcmail->action) || $rcmail->action == 'list') {
9494
// connect to storage server and trigger error on failure
95-
$rcmail->storage_connect();
95+
$rcmail->connect_storage_from_session();
9696

9797
$mbox_name = $rcmail->storage->get_folder();
9898

‎program/include/rcmail.php

Lines changed: 11 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,6 @@ class rcmail extends rcube
6161
private $address_books = [];
6262
private $action_args = [];
6363

64-
public const ERROR_STORAGE = -2;
65-
public const ERROR_INVALID_REQUEST = 1;
66-
public const ERROR_INVALID_HOST = 2;
67-
public const ERROR_COOKIES_DISABLED = 3;
68-
public const ERROR_RATE_LIMIT = 4;
69-
7064
/**
7165
* This implements the 'singleton' design pattern
7266
*
@@ -751,71 +745,27 @@ public function login($username, $password, $host = null, $cookiecheck = false,
751745
$username = rcube_utils::idn_to_ascii($username);
752746
}
753747

754-
// user already registered -> overwrite username
755-
if ($user = rcube_user::query($username, $host)) {
756-
$username = $user->data['username'];
757-
758-
// Brute-force prevention
759-
if ($user->is_locked()) {
760-
$this->login_error = self::ERROR_RATE_LIMIT;
761-
return false;
762-
}
763-
}
764-
765748
$storage = $this->get_storage();
766749

767750
// try to log in
768-
if (!$this->imap_connect($storage, $host, $username, $password, $port, $ssl, $user)) {
769-
return false;
770-
}
751+
$user = $this->storage_connect($storage, $host, $username, $password, $port, $ssl, $just_connect);
771752

772-
// Only set user if just wanting to connect.
773-
// Note that for other scenarios user will also be set after successful login.
774-
if ($just_connect) {
775-
if (is_object($user)) {
776-
$this->set_user($user);
777-
}
778-
return true;
753+
if (!$user) {
754+
return false;
779755
}
780756

781-
// user already registered -> update user's record
782-
if (is_object($user)) {
783-
// update last login timestamp
784-
$user->touch();
757+
if (is_int($user) && $user == self::ERROR_RATE_LIMIT) {
758+
$this->login_error = self::ERROR_RATE_LIMIT;
759+
return false;
785760
}
786-
// create new system user
787-
elseif ($this->config->get('auto_create_user')) {
788-
// Temporarily set user email and password, so plugins can use it
789-
// this way until we set it in session later. This is required e.g.
790-
// by the user-specific LDAP operations from new_user_identity plugin.
791-
$domain = $this->config->mail_domain($host);
792-
$this->user_email = strpos($username, '@') ? $username : sprintf('%s@%s', $username, $domain);
793-
$this->password = $password;
794-
795-
$user = rcube_user::create($username, $host);
796761

797-
$this->user_email = null;
798-
$this->password = null;
799-
800-
if (!$user) {
801-
self::raise_error([
802-
'code' => 620,
803-
'message' => 'Failed to create a user record. Maybe aborted by a plugin?',
804-
], true, false);
805-
}
806-
} else {
807-
self::raise_error([
808-
'code' => 621,
809-
'message' => "Access denied for new user {$username}. 'auto_create_user' is disabled",
810-
], true, false);
762+
// This is enough if just connecting
763+
if ($just_connect) {
764+
return true;
811765
}
812766

813767
// login succeeded
814-
if (is_object($user) && $user->ID) {
815-
// Configure environment
816-
$this->set_user($user);
817-
$this->set_storage_prop();
818-
768+
if ($user->ID) {
819769
// set session vars
820770
$_SESSION['user_id'] = $user->ID;
821771
$_SESSION['username'] = $user->data['username'];
@@ -1996,7 +1946,7 @@ public function html2text($html, $options = [])
19961946
*
19971947
* @return bool True on success, False on error
19981948
*/
1999-
public function storage_connect()
1949+
public function connect_storage_from_session()
20001950
{
20011951
$storage = $this->get_storage();
20021952

‎program/lib/Roundcube/rcube.php

Lines changed: 64 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ class rcube
3838

3939
public const DEBUG_LINE_LENGTH = 4096;
4040

41+
// Error codes
42+
public const ERROR_STORAGE = -2;
43+
public const ERROR_INVALID_REQUEST = 1;
44+
public const ERROR_INVALID_HOST = 2;
45+
public const ERROR_COOKIES_DISABLED = 3;
46+
public const ERROR_RATE_LIMIT = 4;
47+
4148
/** @var rcube_config Stores instance of rcube_config */
4249
public $config;
4350

@@ -1872,13 +1879,23 @@ public function deliver_message($message, $from, $mailto, &$error,
18721879
* @param string $password IMAP password
18731880
* @param int $port IMAP port to connect to
18741881
* @param string $ssl SSL schema or false if plain connection
1875-
* @param rcube_user $user Roundcube user (if it already exists)
18761882
* @param array $imap_options Additional IMAP options
1883+
* @param bool $just_connect Breaks after successful connect
18771884
*
1878-
* @return bool Return true on successful login
1885+
* @return rcube_user|int|null Return user object on success, null or error code on failure
18791886
*/
1880-
public function imap_connect($imap, $host, $username, $password, $port, $ssl, $user = null, $imap_options = [])
1887+
public function storage_connect($imap, $host, $username, $password, $port, $ssl, $imap_options = [], $just_connect = false)
18811888
{
1889+
// user already registered -> overwrite username
1890+
if ($user = rcube_user::query($username, $host)) {
1891+
$username = $user->data['username'];
1892+
1893+
// Brute-force prevention
1894+
if ($user->is_locked()) {
1895+
return self::ERROR_RATE_LIMIT;
1896+
}
1897+
}
1898+
18821899
// enable proxy authentication
18831900
if (!empty($imap_options)) {
18841901
$imap->set_options($imap_options);
@@ -1892,10 +1909,52 @@ public function imap_connect($imap, $host, $username, $password, $port, $ssl, $u
18921909

18931910
// Wait a second to slow down brute-force attacks (#1490549)
18941911
sleep(1);
1895-
return false;
1912+
return null;
18961913
}
18971914

1898-
return true;
1915+
// Only set user if just wanting to connect.
1916+
// Note that for other scenarios user will also be set after successful login.
1917+
if (!$just_connect) {
1918+
// user already registered -> update user's record
1919+
if (is_object($user)) {
1920+
// update last login timestamp
1921+
$user->touch();
1922+
}
1923+
// create new system user
1924+
elseif ($this->config->get('auto_create_user')) {
1925+
// Temporarily set user email and password, so plugins can use it
1926+
// this way until we set it in session later. This is required e.g.
1927+
// by the user-specific LDAP operations from new_user_identity plugin.
1928+
$domain = $this->config->mail_domain($host);
1929+
$this->user_email = strpos($username, '@') ? $username : sprintf('%s@%s', $username, $domain);
1930+
$this->password = $password;
1931+
1932+
$user = rcube_user::create($username, $host);
1933+
1934+
$this->user_email = null;
1935+
$this->password = null;
1936+
1937+
if (!$user) {
1938+
self::raise_error([
1939+
'code' => 620,
1940+
'message' => 'Failed to create a user record. Maybe aborted by a plugin?',
1941+
], true, false);
1942+
}
1943+
} else {
1944+
self::raise_error([
1945+
'code' => 621,
1946+
'message' => "Access denied for new user {$username}. 'auto_create_user' is disabled",
1947+
], true, false);
1948+
}
1949+
}
1950+
1951+
if (is_object($user) && $user->ID) {
1952+
// Configure environment
1953+
$this->set_user($user);
1954+
$this->set_storage_prop();
1955+
}
1956+
1957+
return $user;
18991958
}
19001959
}
19011960

0 commit comments

Comments
 (0)