From 0bc0958fa453c775d50e502c952fc6560e4b582a Mon Sep 17 00:00:00 2001 From: Wellington Moraes Date: Thu, 2 Apr 2026 13:31:09 -0300 Subject: [PATCH] Refactor IMAP class for type safety and logging Refactor IMAP class properties and constructor for type safety. Update error handling and logging for IMAP authentication. Signed-off-by: Wellington Moraes --- lib/IMAP.php | 120 +++++++++++++++++++++++++++++---------------------- 1 file changed, 69 insertions(+), 51 deletions(-) diff --git a/lib/IMAP.php b/lib/IMAP.php index 9d2ec94..83100a0 100644 --- a/lib/IMAP.php +++ b/lib/IMAP.php @@ -1,5 +1,4 @@ * @author Jonas Sulzer @@ -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 - * @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; @@ -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; @@ -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); @@ -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; } }