Merge remote-tracking branch 'refs/remotes/ivan/feature/cleanup-smtpbootstrapper' into develop

4 files changed, 110 insertions, 152 deletions

diff --git a/changes/cleanup-smtpbootstrapper b/changes/cleanup-smtpbootstrapper
new file mode 100644
index 00000000..f1ccabfe
--- /dev/null
+++ b/changes/cleanup-smtpbootstrapper
@@ -0,0 +1 @@
+- Refactor smtp logic into its bootstrapper.
diff --git a/src/leap/bitmask/gui/mainwindow.py b/src/leap/bitmask/gui/mainwindow.py
index d66d518e..1a88fcce 100644
--- a/src/leap/bitmask/gui/mainwindow.py
+++ b/src/leap/bitmask/gui/mainwindow.py
@@ -1412,13 +1412,9 @@ class MainWindow(QtGui.QMainWindow):
             logger.debug("not starting smtp in offline mode")
             return
 
-        # TODO for simmetry, this should be called start_smtp_service
-        # (and delegate all the checks to the conductor)
         if self._provides_mx_and_enabled():
-            self._mail_conductor.smtp_bootstrapper.run_smtp_setup_checks(
-                self._provider_config,
-                self._mail_conductor.smtp_config,
-                download_if_needed=True)
+            self._mail_conductor.start_smtp_service(self._provider_config,
+                                                    download_if_needed=True)
 
     # XXX --- should remove from here, and connecte directly to the state
     # machine.
diff --git a/src/leap/bitmask/services/mail/conductor.py b/src/leap/bitmask/services/mail/conductor.py
index c1761afa..1766a39d 100644
--- a/src/leap/bitmask/services/mail/conductor.py
+++ b/src/leap/bitmask/services/mail/conductor.py
@@ -18,22 +18,18 @@
 Mail Services Conductor
 """
 import logging
-import os
 
-from PySide import QtCore
 from zope.proxy import sameProxiedObjects
 
 from leap.bitmask.gui import statemachines
-from leap.bitmask.services.mail import imap
 from leap.bitmask.services.mail import connection as mail_connection
+from leap.bitmask.services.mail import imap
 from leap.bitmask.services.mail.smtpbootstrapper import SMTPBootstrapper
 from leap.bitmask.services.mail.smtpconfig import SMTPConfig
-from leap.bitmask.util import is_file
 
 from leap.common.check import leap_assert
-
-from leap.common.events import register as leap_register
 from leap.common.events import events_pb2 as leap_events
+from leap.common.events import register as leap_register
 
 
 logger = logging.getLogger(__name__)
@@ -167,10 +163,6 @@ class IMAPControl(object):
 
 
 class SMTPControl(object):
-
-    PORT_KEY = "port"
-    IP_KEY = "ip_address"
-
     def __init__(self):
         """
         Initializes smtp variables.
@@ -178,12 +170,8 @@ class SMTPControl(object):
         self.smtp_config = SMTPConfig()
         self.smtp_connection = None
         self.smtp_machine = None
-        self._smtp_service = None
-        self._smtp_port = None
 
         self.smtp_bootstrapper = SMTPBootstrapper()
-        self.smtp_bootstrapper.download_config.connect(
-            self.smtp_bootstrapped_stage)
 
         leap_register(signal=leap_events.SMTP_SERVICE_STARTED,
                       callback=self._handle_smtp_events,
@@ -200,100 +188,27 @@ class SMTPControl(object):
         """
         self.smtp_connection = smtp_connection
 
-    def start_smtp_service(self, host, port, cert):
+    def start_smtp_service(self, provider_config, download_if_needed=False):
         """
-        Starts the smtp service.
+        Starts the SMTP service.
 
-        :param host: the hostname of the remove SMTP server.
-        :type host: str
-        :param port: the port of the remote SMTP server
-        :type port: str
-        :param cert: the client certificate for authentication
-        :type cert: str
+        :param provider_config: Provider configuration
+        :type provider_config: ProviderConfig
+        :param download_if_needed: True if it should check for mtime
+                                   for the file
+        :type download_if_needed: bool
         """
-        # TODO Make the encrypted_only configurable
-        # TODO pick local smtp port in a better way
-        # TODO remove hard-coded port and let leap.mail set
-        # the specific default.
         self.smtp_connection.qtsigs.connecting_signal.emit()
-        from leap.mail.smtp import setup_smtp_gateway
-        self._smtp_service, self._smtp_port = setup_smtp_gateway(
-            port=2013,
-            userid=self.userid,
-            keymanager=self._keymanager,
-            smtp_host=host,
-            smtp_port=port,
-            smtp_cert=cert,
-            smtp_key=cert,
-            encrypted_only=False)
+        self.smtp_bootstrapper.start_smtp_service(
+            provider_config, self.smtp_config, self._keymanager,
+            self.userid, download_if_needed)
 
     def stop_smtp_service(self):
         """
-        Stops the smtp service (port and factory).
+        Stops the SMTP service.
         """
         self.smtp_connection.qtsigs.disconnecting_signal.emit()
-        # TODO We should homogenize both services.
-        if self._smtp_service is not None:
-            logger.debug('Stopping smtp service.')
-            self._smtp_port.stopListening()
-            self._smtp_service.doStop()
-
-    @QtCore.Slot(dict)
-    def smtp_bootstrapped_stage(self, data):
-        """
-        TRIGGERS:
-            self.smtp_bootstrapper.download_config
-
-        If there was a problem, displays it, otherwise it does nothing.
-        This is used for intermediate bootstrapping stages, in case
-        they fail.
-
-        :param data: result from the bootstrapping stage for Soledad
-        :type data: dict
-        """
-        passed = data[self.smtp_bootstrapper.PASSED_KEY]
-        if not passed:
-            logger.error(data[self.smtp_bootstrapper.ERROR_KEY])
-            return
-        logger.debug("Done bootstrapping SMTP")
-        self.check_smtp_config()
-
-    def check_smtp_config(self):
-        """
-        Checks smtp config and tries to download smtp client cert if needed.
-        Currently called when smtp_bootstrapped_stage has successfuly finished.
-        """
-        logger.debug("Checking SMTP config...")
-        leap_assert(self.smtp_bootstrapper._provider_config,
-                    "smtp bootstrapper does not have a provider_config")
-
-        provider_config = self.smtp_bootstrapper._provider_config
-        smtp_config = self.smtp_config
-        hosts = smtp_config.get_hosts()
-        # TODO handle more than one host and define how to choose
-        if len(hosts) > 0:
-            hostname = hosts.keys()[0]
-            logger.debug("Using hostname %s for SMTP" % (hostname,))
-            host = hosts[hostname][self.IP_KEY].encode("utf-8")
-            port = hosts[hostname][self.PORT_KEY]
-
-            client_cert = smtp_config.get_client_cert_path(
-                provider_config,
-                about_to_download=True)
-
-            # XXX change this logic!
-            # check_config should be called from within start_service,
-            # and not the other way around.
-            if not is_file(client_cert):
-                self.smtp_bootstrapper._download_client_certificates()
-            if os.path.isfile(client_cert):
-                self.start_smtp_service(host, port, client_cert)
-            else:
-                logger.warning("Tried to download email client "
-                               "certificate, but could not find any")
-
-        else:
-            logger.warning("No smtp hosts configured")
+        self.smtp_bootstrapper.stop_smtp_service()
 
     # handle smtp events
 
@@ -348,7 +263,7 @@ class MailConductor(IMAPControl, SMTPControl):
 
         :param keymanager: a transparent proxy that eventually will point to a
                            Keymanager Instance.
-        :type soledad: zope.proxy.ProxyBase
+        :type keymanager: zope.proxy.ProxyBase
         """
         IMAPControl.__init__(self)
         SMTPControl.__init__(self)
@@ -406,4 +321,5 @@ class MailConductor(IMAPControl, SMTPControl):
         qtsigs.connecting_signal.connect(widget.mail_state_connecting)
         qtsigs.disconnecting_signal.connect(widget.mail_state_disconnecting)
         qtsigs.disconnected_signal.connect(widget.mail_state_disconnected)
-        qtsigs.soledad_invalid_auth_token.connect(widget.soledad_invalid_auth_token)
+        qtsigs.soledad_invalid_auth_token.connect(
+            widget.soledad_invalid_auth_token)
diff --git a/src/leap/bitmask/services/mail/smtpbootstrapper.py b/src/leap/bitmask/services/mail/smtpbootstrapper.py
index 032d6357..7ecf8134 100644
--- a/src/leap/bitmask/services/mail/smtpbootstrapper.py
+++ b/src/leap/bitmask/services/mail/smtpbootstrapper.py
@@ -20,12 +20,13 @@ SMTP bootstrapping
 import logging
 import os
 
-from PySide import QtCore
-
 from leap.bitmask.config.providerconfig import ProviderConfig
 from leap.bitmask.crypto.certs import download_client_cert
 from leap.bitmask.services import download_service_config
 from leap.bitmask.services.abstractbootstrapper import AbstractBootstrapper
+from leap.bitmask.services.mail.smtpconfig import SMTPConfig
+from leap.bitmask.util import is_file
+
 from leap.common import certs as leap_certs
 from leap.common.check import leap_assert, leap_assert_type
 from leap.common.files import check_and_fix_urw_only
@@ -33,27 +34,33 @@ from leap.common.files import check_and_fix_urw_only
 logger = logging.getLogger(__name__)
 
 
+class NoSMTPHosts(Exception):
+    """This is raised when there is no SMTP host to use."""
+
+
 class SMTPBootstrapper(AbstractBootstrapper):
     """
     SMTP init procedure
     """
 
-    # All dicts returned are of the form
-    # {"passed": bool, "error": str}
-    download_config = QtCore.Signal(dict)
+    PORT_KEY = "port"
+    IP_KEY = "ip_address"
 
     def __init__(self):
         AbstractBootstrapper.__init__(self)
 
         self._provider_config = None
         self._smtp_config = None
+        self._userid = None
         self._download_if_needed = False
 
-    def _download_config(self, *args):
+        self._smtp_service = None
+        self._smtp_port = None
+
+    def _download_config_and_cert(self):
         """
-        Downloads the SMTP config for the given provider
+        Downloads the SMTP config and cert for the given provider.
         """
-
         leap_assert(self._provider_config,
                     "We need a provider configuration!")
 
@@ -66,63 +73,101 @@ class SMTPBootstrapper(AbstractBootstrapper):
             self._session,
             self._download_if_needed)
 
-    def _download_client_certificates(self, *args):
-        """
-        Downloads the SMTP client certificate for the given provider
+        hosts = self._smtp_config.get_hosts()
 
-        We actually are downloading the certificate for the same uri as
-        for the EIP config, but we duplicate these bits to allow mail
-        service to be working in a provider that does not offer EIP.
-        """
-        # TODO factor out with eipboostrapper.download_client_certificates
-        # TODO this shouldn't be a private method, it's called from
-        # mainwindow.
-        leap_assert(self._provider_config, "We need a provider configuration!")
-        leap_assert(self._smtp_config, "We need an smtp configuration!")
+        if len(hosts) == 0:
+            raise NoSMTPHosts()
 
-        logger.debug("Downloading SMTP client certificate for %s" %
-                     (self._provider_config.get_domain(),))
+        # TODO handle more than one host and define how to choose
+        hostname = hosts.keys()[0]
+        logger.debug("Using hostname %s for SMTP" % (hostname,))
 
-        client_cert_path = self._smtp_config.\
-            get_client_cert_path(self._provider_config,
-                                 about_to_download=True)
+        client_cert_path = self._smtp_config.get_client_cert_path(
+            self._provider_config, about_to_download=True)
 
-        # For re-download if something is wrong with the cert
-        self._download_if_needed = self._download_if_needed and \
-            not leap_certs.should_redownload(client_cert_path)
+        if not is_file(client_cert_path):
+            # For re-download if something is wrong with the cert
+            self._download_if_needed = (
+                self._download_if_needed and
+                not leap_certs.should_redownload(client_cert_path))
 
-        if self._download_if_needed and \
-                os.path.isfile(client_cert_path):
-            check_and_fix_urw_only(client_cert_path)
-            return
+            if self._download_if_needed and os.path.isfile(client_cert_path):
+                check_and_fix_urw_only(client_cert_path)
+                return
 
-        download_client_cert(self._provider_config,
-                             client_cert_path,
-                             self._session)
+            download_client_cert(self._provider_config,
+                                 client_cert_path,
+                                 self._session)
 
-    def run_smtp_setup_checks(self,
-                              provider_config,
-                              smtp_config,
-                              download_if_needed=False):
+    def _start_smtp_service(self):
+        """
+        Start the smtp service using the downloaded configurations.
+        """
+        # TODO Make the encrypted_only configurable
+        # TODO pick local smtp port in a better way
+        # TODO remove hard-coded port and let leap.mail set
+        # the specific default.
+        # TODO handle more than one host and define how to choose
+        hosts = self._smtp_config.get_hosts()
+        hostname = hosts.keys()[0]
+        host = hosts[hostname][self.IP_KEY].encode("utf-8")
+        port = hosts[hostname][self.PORT_KEY]
+        client_cert_path = self._smtp_config.get_client_cert_path(
+            self._provider_config, about_to_download=True)
+
+        from leap.mail.smtp import setup_smtp_gateway
+        self._smtp_service, self._smtp_port = setup_smtp_gateway(
+            port=2013,
+            userid=self._userid,
+            keymanager=self._keymanager,
+            smtp_host=host,
+            smtp_port=port,
+            smtp_cert=client_cert_path,
+            smtp_key=client_cert_path,
+            encrypted_only=False)
+
+    def start_smtp_service(self, provider_config, smtp_config, keymanager,
+                           userid, download_if_needed=False):
         """
-        Starts the checks needed for a new smtp setup
+        Starts the SMTP service.
 
         :param provider_config: Provider configuration
         :type provider_config: ProviderConfig
         :param smtp_config: SMTP configuration to populate
         :type smtp_config: SMTPConfig
+        :param keymanager: a transparent proxy that eventually will point to a
+                           Keymanager Instance.
+        :type keymanager: zope.proxy.ProxyBase
+        :param userid: the user id, in the form "user@provider"
+        :type userid: str
         :param download_if_needed: True if it should check for mtime
                                    for the file
         :type download_if_needed: bool
         """
         leap_assert_type(provider_config, ProviderConfig)
+        leap_assert_type(smtp_config, SMTPConfig)
 
         self._provider_config = provider_config
+        self._keymanager = keymanager
         self._smtp_config = smtp_config
+        self._useid = userid
         self._download_if_needed = download_if_needed
 
-        cb_chain = [
-            (self._download_config, self.download_config),
-        ]
-
-        self.addCallbackChain(cb_chain)
+        try:
+            self._download_config_and_cert()
+            logger.debug("Starting SMTP service.")
+            self._start_smtp_service()
+        except NoSMTPHosts:
+            logger.warning("There is no SMTP host to use.")
+        except Exception as e:
+            # TODO: we should handle more specific exceptions in here
+            logger.exception("Error while bootstrapping SMTP: %r" % (e, ))
+
+    def stop_smtp_service(self):
+        """
+        Stops the smtp service (port and factory).
+        """
+        if self._smtp_service is not None:
+            logger.debug('Stopping SMTP service.')
+            self._smtp_port.stopListening()
+            self._smtp_service.doStop()