Skip to content

Refactor IMAP class for type safety and logging #293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kaellego wants to merge 1 commit into
base: master
Choose a base branch
from
+69 −51
Open

Refactor IMAP class for type safety and logging #293

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 69 additions & 51 deletions lib/IMAP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
<?php

/**
* @author Robin Appelman <icewind@owncloud.com>
* @author Jonas Sulzer <jonas@violoncello.ch>
Expand All @@ -8,63 +7,72 @@
* later.
* See the COPYING-README file.
*/

namespace OCA\UserExternal;

use OCP\ILogger;

/**
* User authentication against an IMAP mail server
*
* @category Apps
* @package UserExternal
* @author Robin Appelman <icewind@owncloud.com>
* @license http://www.gnu.org/licenses/agpl AGPL
* @link http://github.com/owncloud/apps
*/
class IMAP extends Base {
private $mailbox;
private $port;
private $sslmode;
private $domain;
private $stripeDomain;
private $groupDomain;
private string $mailbox;
private int $port;
private ?string $sslmode;
private string $domain;
private bool $stripeDomain;
private bool $groupDomain;

/**
* Create new IMAP authentication provider
*
* @param string $mailbox IMAP server domain/IP
* @param int $port IMAP server $port
* @param string $sslmode
* @param string $domain If provided, loging will be restricted to this domain
* @param boolean $stripeDomain (whether to stripe the domain part from the username or not)
* @param boolean $groupDomain (whether to add the usere to a group corresponding to the domain of the address)
* @param int|null $port IMAP server port
* @param string|null $sslmode ssl|tls|null
* @param string|null $domain If provided, login will be restricted to this domain
* @param bool $stripeDomain Whether to strip the domain part from the username
* @param bool $groupDomain Whether to add the user to a group matching the email domain
*/
public function __construct($mailbox, $port = null, $sslmode = null, $domain = null, $stripeDomain = true, $groupDomain = false) {
public function __construct(
string $mailbox,
?int $port = null,
?string $sslmode = null,
?string $domain = null,
bool $stripeDomain = true,
bool $groupDomain = false
) {
parent::__construct($mailbox);
$this->mailbox = $mailbox;
$this->port = $port === null ? 143 : $port;
$this->port = $port ?? 143;
$this->sslmode = $sslmode;
$this->domain = $domain === null ? '' : $domain;
$this->domain = $domain ?? '';
$this->stripeDomain = $stripeDomain;
$this->groupDomain = $groupDomain;
}

private function logger(): ILogger {
/** @var ILogger $logger */
$logger = \OC::$server->get(ILogger::class);
return $logger;
}

/**
* Check if the password is correct without logging in the user
*
* @param string $uid The username
* @param string $password The password
*
* @return true/false
* @return string|false
*/
public function checkPassword($uid, $password) {
$uid = $this->resolveUid($uid);
if ($password === '') {
return false;
}

// Replace escaped @ symbol in uid (which is a mail address)
// but only if there is no @ symbol and if there is a %40 inside the uid
if (!(strpos($uid, '@') !== false) && (strpos($uid, '%40') !== false)) {
if (strpos($uid, '@') === false && strpos($uid, '%40') !== false) {
$uid = str_replace('%40', '@', $uid);
}

$pieces = explode('@', $uid);

if ($this->domain !== '') {
if (count($pieces) === 1) {
$username = $uid . '@' . $this->domain;
Expand All @@ -74,8 +82,8 @@ public function checkPassword($uid, $password) {
$uid = $pieces[0];
}
} else {
$this->logger->error(
'ERROR: User has a wrong domain! Expecting: ' . $this->domain,
$this->logger()->error(
'User has a wrong domain. Expected: ' . $this->domain,
['app' => 'user_external']
);
return false;
Expand All @@ -85,20 +93,31 @@ public function checkPassword($uid, $password) {
}

$groups = [];
if ((count($pieces) > 1) && $this->groupDomain && $pieces[1]) {
if (count($pieces) > 1 && $this->groupDomain && !empty($pieces[1])) {
$groups[] = $pieces[1];
}

$protocol = ($this->sslmode === 'ssl') ? 'imaps' : 'imap';
$url = "{$protocol}://{$this->mailbox}:{$this->port}";
$url = sprintf('%s://%s:%d', $protocol, $this->mailbox, $this->port);

$ch = curl_init();
if ($ch === false) {
$this->logger()->error(
'Could not initialize curl for IMAP authentication.',
['app' => 'user_external']
);
return false;
}

if ($this->sslmode === 'tls') {
curl_setopt($ch, CURLOPT_USE_SSL, CURLUSESSL_ALL);
}

curl_setopt($ch, CURLOPT_URL, $url);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
curl_setopt($ch, CURLOPT_USERPWD, $username . ':' . $password);
curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, 10);
curl_setopt($ch, CURLOPT_TIMEOUT, 15);
curl_setopt($ch, CURLOPT_CUSTOMREQUEST, 'CAPABILITY');

curl_exec($ch);
Expand All @@ -109,35 +128,34 @@ public function checkPassword($uid, $password) {
$uid = mb_strtolower($uid);
$this->storeUser($uid, $groups);
return $uid;
} elseif ($errorcode === CURLE_COULDNT_CONNECT
|| $errorcode === CURLE_SSL_CONNECT_ERROR
|| $errorcode === 28) {
# This is not defined in PHP-8.x
# 28: CURLE_OPERATION_TIMEDOUT
$this->logger->error(
'ERROR: Could not connect to imap server via curl: ' . curl_strerror($errorcode),
}

if (
$errorcode === CURLE_COULDNT_CONNECT ||
$errorcode === CURLE_SSL_CONNECT_ERROR ||
$errorcode === CURLE_OPERATION_TIMEDOUT
) {
$this->logger()->error(
'Could not connect to IMAP server via curl: ' . curl_strerror($errorcode),
['app' => 'user_external']
);
} elseif ($errorcode === 9
|| $errorcode === 67
|| $errorcode === 94) {
# These are not defined in PHP-8.x
# 9: CURLE_REMOTE_ACCESS_DENIED
# 67: CURLE_LOGIN_DENIED
# 94: CURLE_AUTH_ERROR)
$this->logger->error(
'ERROR: IMAP Login failed via curl: ' . curl_strerror($errorcode),
} elseif (
$errorcode === CURLE_REMOTE_ACCESS_DENIED ||
$errorcode === CURLE_LOGIN_DENIED ||
(defined('CURLE_AUTH_ERROR') && $errorcode === CURLE_AUTH_ERROR)
) {
$this->logger()->warning(
'IMAP login failed via curl: ' . curl_strerror($errorcode),
['app' => 'user_external']
);
} else {
$this->logger->error(
'ERROR: IMAP server returned an error: ' . $errorcode . ' / ' . curl_strerror($errorcode),
$this->logger()->error(
'IMAP server returned an error: ' . $errorcode . ' / ' . curl_strerror($errorcode),
['app' => 'user_external']
);
}

curl_close($ch);

return false;
}
}