Skip to content

Commit

Permalink
Address feedback from alecpl
Browse files Browse the repository at this point in the history 
* 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()`
  • Loading branch information
@jaudriga
jaudriga committed Jul 15, 2024
1 parent 9b65ce1 commit 1354451
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 66 deletions.
2 changes: 1 addition & 1 deletion program/actions/mail/index.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
69 changes: 9 additions & 60 deletions program/include/rcmail.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -751,71 +745,26 @@ 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 == self::ERROR_RATE_LIMIT) {
$this->login_error = self::ERROR_RATE_LIMIT;
}

// user already registered -> update user's record
if (is_object($user)) {
// update last login timestamp
$user->touch();
if (!$user || $user == 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();

// set session vars
$_SESSION['user_id'] = $user->ID;
$_SESSION['username'] = $user->data['username'];
Expand Down Expand Up @@ -1996,7 +1945,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();

Expand Down
69 changes: 64 additions & 5 deletions program/lib/Roundcube/rcube.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 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);
Expand All @@ -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;
}
}

Expand Down

0 comments on commit 1354451

Please sign in to comment.