Skip to content
This repository has been archived by the owner on Nov 14, 2018. It is now read-only.

user_external with imap: too many connections to the server #2213

Open
quenenni opened this issue May 11, 2017 · 7 comments
Open

user_external with imap: too many connections to the server #2213

quenenni opened this issue May 11, 2017 · 7 comments

Comments

@quenenni
Copy link

Affected apps

user_external

Expected behaviour

When using user_external app, have only 1 connection to each imap server configured and keep the info (success or failure) for after.

Actual behaviour

11 consecutive imap_open calls are made when login in the app + 2 others a bit later.
That's a lot.
When configuring several Imap server (I guess it's the same if it's an Imap and a ftp backend), and the person who tries to connect is from the second imap server, we loose something like 13 x 2 sec (each attempt on the first imap server) + 13 x 0,something sec (not a lot, but x 13 makes it bigger) for the login process.. That's quite huge.

But, after that, when navigating in the interface, no more imap_open are called (the expected behaviour).
This should be applied as soon as the first imap_open return true.

Steps to reproduce

  1. Configure 2 Imap backends servers
  2. try to connect with an address from the second server
  3. wait

Server configuration

Debian Jessie 8.8

Web server:
nginx/1.10.3
Database:
mariadb-server 10.0.30-0+deb8u2

PHP version:
PHP 7.0.18-1~dotdeb+8.1 (cli) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2017 Zend Technologies
with Zend OPcache v7.0.18-1~dotdeb+8.1, Copyright (c) 1999-2017, by Zend Technologies

ownCloud version:
9.1.2

@ddeenniiss
Copy link

+1

@mmccarn
Copy link

mmccarn commented Jun 4, 2017

Have you specified the (optional) allowed domain for each mail server as described in the user_external README.md?

@quenenni
Copy link
Author

quenenni commented Jun 4, 2017

"allowed domain"?
I don't really see what you mean by that.
I don't find anything related to allowed domain in the readme page.
Here is my nc config:

'user_backends' =>
  array (
    0 => array (
      'class' => 'OC_User_IMAP',
      'arguments' =>
      array (
        0 => '{mail.yyyy.net:993/imap/ssl}',
      ),
    ),
    1 => array (
      'class' => 'OC_User_IMAP',
      'arguments' =>
      array (
        0 => '{mail.xxxxx.net:993/imap/ssl}',
      ),
    ),
  ),

The problem is not about blocked connections.
The problem is that it connects 13 times during the login process to each mail server instead of one time only.
All the imap connections are done without error, but that process takes too long if the mail is from the second configured server.

After the login process is done, when browsing the pages, you don't have any more imap connections.
So why does it need to connect so many times on the login process?

Edit:
If you configure only one imap server or if the mail comes from the first server in the list, the 13 connections are also done, but it's quick enough to not be a problem... but it still should be 1 connection.

@quenenni
Copy link
Author

quenenni commented Jun 4, 2017

Hooo.. Sorry.
I found your reference to allowed domain:

If a domain name (e.g. example.com) is specified, then this makes sure that only users from this domain will be allowed to login

Well, the problem is that there are hundreds of domains and new ones are created regularly, so that's unfortunately not an option for me.

@mmccarn
Copy link

mmccarn commented Jun 7, 2017

I'm using a customized version of OC/apps/user_external/lib/imap.php that requires users to login using a full email address, then returns 'false' if the mail server address doesn't apply.

However, I only have 4 domains to worry about; this code won't scale very well to hundreds of domains.

Here is my customized code, in case there is something here that might help you.

'DISABLE_AUTHENTICATOR' (added to the imap_open command) may help if some of your 13 connections are failing due to an auth mechanism mis-match. My mail server reports that 'CRAM-MD5' is available, but it really isn't -- which was inserting significant delays in user logins.

This code:

  • creates new private vars used during username parsing
  • adds a default domain to the username if no domain is specified
  • only checks users against the appropriate mail server
  • disables imap authenticators that my servers don't support to speed up the login
class OC_User_IMAP extends \OCA\user_external\Base {
        private $mailbox;
        private $gid;
        private $uida;
        private $iuid;

        /**
         * Create new IMAP authentication provider
         *
         * @param string $mailbox PHP imap_open mailbox definition, e.g.
         *                        {127.0.0.1:143/imap/readonly}
         */
        public function __construct($mailbox) {
                parent::__construct($mailbox);
                $this->mailbox=$mailbox;
        }

        /**
         * Check if the password is correct without logging in the user
         *
         * @param string $uid      The username
         * @param string $password The password
         *
         * @return true/false
         */
        public function checkPassword($uid, $password) {
                if (!function_exists('imap_open')) {
                        OCP\Util::writeLog('user_external', 'ERROR: PHP imap extension is not installed', OCP\Util::ERROR);
                        return false;
                }

                /**
                * Break out the domain name for server comparisons and later group creation
                * set server default if username does not contain '@'
                */
                if (strpos( $uid, '@') == FALSE) {
                        $uid=$uid . "@defaultdomain.tld";
                }
                $this->iuid = $uid;
                $this->uida=explode('@',strtolower($uid),2);
                $this->gid=$this->uida[1];

                /**
                * Don't attempt authentication if it can't work
                * ...and strip off the domain name if the server doesn't want it.
                */
                
                /**
                * one domain is included in the mail server name
                * this server also wants only the username for login
                * eg '[email protected]' authenticates against 'mail.domain1.tld' as 'user'
                */
                
                if ($this->gid == 'domain1.tld') {
                        $this->iuid=$this->uida[0];
                        if (strpos($this->mailbox,$this->gid) == FALSE) return false;
                }
                
                /**
                * these domains authenticate against a single server that may not contain the email domain name
                * the mail server accepts a full email address during login
                */
                if ($this->gid == 'defaultdomain.tld' || $this->gid == 'domain3.tld' || $this->gid == 'domain4.tld') {
                        if (strpos($this->mailbox,'mail.server.tld') == FALSE) return false;
                }
                $mbox = @imap_open($this->mailbox, $this->iuid, $password, OP_HALFOPEN, 1,array('DISABLE_AUTHENTICATOR' => array('CRAM-MD5','DIGEST-MD5')));

...

@quenenni
Copy link
Author

quenenni commented Jun 7, 2017

Once again, a big thanks for your help.

Your code gave me some ideas and I finally managed to have a solution that seems to work smoothly, well much more smoothly than before.

The idea is before trying to do the imap_open command, I do a db search to find all existing backends for a mail address.
The result will be empty the first time you try to log with a new mail address.
In this case, I let the system work as before (it will try all backends configured in the config.php).
But as soon as a first connection succeeded, a row will be added to the table users_external.
Then, on all future attempts, the db query will give the right backend for this mail address.

I can then compare the backend in the Db and the backend used in this call, and only execute an imap_open if they are similar.
With this, I go from 30sec to +- 3 sec to connect.

I'm sure this can be done in a much better way, but that's all I was able to do.
I have the feeling that the 13 calls come from the nextcloud core and aren't part of the user_external app. And I don't feel confident at all to change the core code.

Feel free to add comments or criticism to my method.
I'm not sure yet if it will be good in all situations.

In user_external/lib/base.php, I added the function getUserMailbox :

        /**
        * Get mailbox settings for an user
        *
        * @return array with all mailbox settings for a mail address
        */
        public function getUserMailbox($search = '', $limit = null, $offset = null) {
                $result = OC_DB::executeAudited(
                        array(  
                                'sql' => 'SELECT `backend` FROM `*PREFIX*users_external`'
                                        . ' WHERE LOWER(`uid`) LIKE LOWER(?)',
                                'limit' => $limit,
                                'offset' => $offset
                        ),
                        array($search . '%')
                );
                $backends = array();
                while ($row = $result->fetchRow()) {
                        $backends[] = $row['backend'];
                }
                return $backends;
        }

In user_external/lib/imap.php, I modified the checkPassword function :

        public function checkPassword($uid, $password) {
                if (!function_exists('imap_open')) {
                        OCP\Util::writeLog('user_external', 'ERROR: PHP imap extension is not installed', OCP\Util::ERROR);
                        return false;
                }

                $backends = $this->getUserMailbox($uid);
                $blnBackendGood = false;
                foreach ($backends as $backend) {
                        if ($backend == $this->mailbox) {
                                $blnBackendGood = true;
                        }
                }

                if (count($backends) == 0 || $blnBackendGood) {
                        $mbox = @imap_open($this->mailbox, $uid, $password, OP_HALFOPEN, 1);
                        imap_errors();
                        imap_alerts();
                        if($mbox !== FALSE) {
                                imap_close($mbox);
                                $uid = mb_strtolower($uid);
                                $this->storeUser($uid);
                                return $uid;
                        }else{
                                return false;
                        }
                } else {
                        return false;
                }
        }

@mmccarn
Copy link

mmccarn commented Jun 8, 2017

Excellent.

I think that user authentication has been refactored in Owncloud 10; you may need to revisit this when you upgrade.

For what it's worth, on my server I keep copies of my modified file (imap.php) and a copy of the unmodified, original version (imap.php.org) in a separate folder that is not affected by owncloud upgrades. After each upgrade, I compare the "new" imap.php with my saved "imap.php.org".

So far (for the last 2 years anyway) this has shown that there have been no changes to imap.php, so I feel safe copying my modified imap.php over the version that was installed by the upgrade.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants