diff options
-rw-r--r-- | src/leap/bitmask/core/mail_services.py | 36 | ||||
-rw-r--r-- | src/leap/bitmask/mail/adaptors/soledad.py | 1 | ||||
-rw-r--r-- | src/leap/bitmask/mail/imap/mailbox.py | 21 | ||||
-rw-r--r-- | src/leap/bitmask/mail/incoming/service.py | 38 | ||||
-rw-r--r-- | src/leap/bitmask/mail/mail.py | 19 | ||||
-rw-r--r-- | src/leap/bitmask/mail/smtp/bounces.py | 2 | ||||
-rw-r--r-- | src/leap/bitmask/mua/pixelizer.py | 66 | ||||
-rw-r--r-- | tests/integration/mail/imap/test_imap.py | 2 | ||||
-rw-r--r-- | tests/unit/mail/imap/test_mailbox.py | 49 |
9 files changed, 156 insertions, 78 deletions
diff --git a/src/leap/bitmask/core/mail_services.py b/src/leap/bitmask/core/mail_services.py index abd2f980..4197700b 100644 --- a/src/leap/bitmask/core/mail_services.py +++ b/src/leap/bitmask/core/mail_services.py @@ -509,18 +509,19 @@ class StandardMailService(service.MultiService, HookableService): sendmail_opts = _get_sendmail_opts(self._basedir, provider, username) self._sendmail_opts[userid] = sendmail_opts - incoming = self.getServiceNamed('incoming_mail') - incoming.startInstance(userid) - def registerToken(token): self._service_tokens[userid] = token self._active_user = userid return token - d = soledad.get_or_create_service_token('mail_auth') + incoming = self.getServiceNamed('incoming_mail') + d = incoming.startInstance(userid) + d.addCallback( + lambda _: soledad.get_or_create_service_token('mail_auth')) d.addCallback(registerToken) d.addCallback(self._write_tokens_file, userid) - d.addCallback(self._maybe_start_pixelated, userid, soledad, keymanager) + d.addCallback( + self._maybe_start_pixelated, userid, soledad, keymanager) return d # hooks @@ -537,10 +538,9 @@ class StandardMailService(service.MultiService, HookableService): @defer.inlineCallbacks def hook_on_bonafide_auth(self, **kw): - # TODO: if it's expired we should renew it userid = kw['username'] - self._maybe_start_incoming_service(userid) + # TODO: if it's expired we should renew it yield self._maybe_fetch_smtp_certificate(userid) def _maybe_start_incoming_service(self, userid): @@ -642,10 +642,11 @@ class StandardMailService(service.MultiService, HookableService): json.dump(token_dict, ftokens) def _maybe_start_pixelated(self, passthrough, userid, soledad, keymanager): + incoming = self.getServiceNamed('incoming_mail') + account = incoming.getServiceNamed(userid).account if pixelizer.HAS_PIXELATED: - reactor.callFromThread( - pixelizer.start_pixelated_user_agent, - userid, soledad, keymanager) + pixelizer.start_pixelated_user_agent( + userid, soledad, keymanager, account) return passthrough @@ -750,12 +751,13 @@ class IncomingMailService(service.MultiService): # Individual accounts def startInstance(self, userid): + """returns: a deferred""" self._set_status(userid, "starting") soledad = self._mail.get_soledad_session(userid) keymanager = self._mail.get_keymanager_session(userid) self.log.info('Setting up Incoming Mail Service for %s' % userid) - self._start_incoming_mail_instance( + return self._start_incoming_mail_instance( keymanager, soledad, userid) @defer.inlineCallbacks @@ -777,11 +779,12 @@ class IncomingMailService(service.MultiService): def _start_incoming_mail_instance(self, keymanager, soledad, userid, start_sync=True): - def setUpIncomingMail(inbox): + def setUpIncomingMail(inbox, acc): incoming_mail = IncomingMail( keymanager, soledad, inbox, userid, check_period=INCOMING_CHECK_PERIOD) + incoming_mail.account = acc self.log.debug('Setting Incoming Mail Service for %s' % userid) incoming_mail.setName(userid) self.addService(incoming_mail) @@ -790,10 +793,17 @@ class IncomingMailService(service.MultiService): self._set_status(userid, "on") return res + # XXX ---------------------------------------------------------------- + # TODO we probably want to enforce a SINGLE ACCOUNT INSTANCE + # earlier in the bootstrap process (ie, upper in the hierarchy of + # services) so that the single instance can be shared by the imap and + # the pixelated mua. + # XXX ---------------------------------------------------------------- + acc = Account(soledad, userid) d = acc.callWhenReady( lambda _: acc.get_collection_by_mailbox(INBOX_NAME)) - d.addCallback(setUpIncomingMail) + d.addCallback(setUpIncomingMail, acc) d.addCallback(setStatusOn) d.addErrback(self._errback, userid) return d diff --git a/src/leap/bitmask/mail/adaptors/soledad.py b/src/leap/bitmask/mail/adaptors/soledad.py index a3e011a3..f220aea0 100644 --- a/src/leap/bitmask/mail/adaptors/soledad.py +++ b/src/leap/bitmask/mail/adaptors/soledad.py @@ -851,6 +851,7 @@ class SoledadMailAdaptor(SoledadIndexMixin): wait_for_indexes = ['get_or_create_mbox', 'update_mbox', 'get_all_mboxes'] mboxwrapper_klass = MailboxWrapper + atomic = defer.DeferredLock() log = Logger() diff --git a/src/leap/bitmask/mail/imap/mailbox.py b/src/leap/bitmask/mail/imap/mailbox.py index f74910f9..9e74cfc9 100644 --- a/src/leap/bitmask/mail/imap/mailbox.py +++ b/src/leap/bitmask/mail/imap/mailbox.py @@ -64,14 +64,14 @@ def make_collection_listener(mailbox): def __init__(self, mbox): self.mbox = mbox - # See #8083, pixelated adaptor seems to be misusing this class. - self.mailbox_name = self.mbox.mbox_name + # See #8083, pixelated adaptor introduces conflicts in the usage + self.mailbox_name = self.mbox.mbox_name + 'IMAP' def __hash__(self): - return hash(self.mbox.mbox_name) + return hash(self.mailbox_name) def __eq__(self, other): - return self.mbox.mbox_name == other.mbox.mbox_name + return self.mailbox_name == other.mbox.mbox_name + 'IMAP' def notify_new(self): self.mbox.notify_new() @@ -397,8 +397,6 @@ class IMAPMailbox(object): :param args: ignored. """ - if not NOTIFY_NEW: - return def cbNotifyNew(result): exists, recent = result @@ -887,15 +885,8 @@ class IMAPMailbox(object): uid when the copy succeed. :rtype: Deferred """ - # A better place for this would be the COPY/APPEND dispatcher - # in server.py, but qtreactor hangs when I do that, so this seems - # to work fine for now. - # d.addCallback(lambda r: self.reactor.callLater(0, self.notify_new)) - # deferLater(self.reactor, 0, self._do_copy, message, d) - # return d - - d = self.collection.copy_msg(message.message, - self.collection.mbox_uuid) + d = self.collection.copy_msg( + message.message, self.collection.mbox_uuid) return d # convenience fun diff --git a/src/leap/bitmask/mail/incoming/service.py b/src/leap/bitmask/mail/incoming/service.py index b1d48cf4..7c7e7f2c 100644 --- a/src/leap/bitmask/mail/incoming/service.py +++ b/src/leap/bitmask/mail/incoming/service.py @@ -140,19 +140,6 @@ class IncomingMail(Service): # initialize a mail parser only once self._parser = Parser() - def add_listener(self, listener): - """ - Add a listener to inbox insertions. - - This listener function will be called for each message added to the - inbox with its uid as parameter. This function should not be blocking - or it will block the incoming queue. - - :param listener: the listener function - :type listener: callable - """ - self._listeners.append(listener) - # # Public API: fetch, start_loop, stop. # @@ -165,17 +152,18 @@ class IncomingMail(Service): """ def _sync_errback(failure): self.log.error( - 'Error while fetching incoming mail {0!r}'.format(failure)) + 'Error while fetching incoming mail: {0!r}'.format(failure)) def syncSoledadCallback(_): # XXX this should be moved to adaptors + # TODO we can query the blobs store instead. d = self._soledad.get_from_index( fields.JUST_MAIL_IDX, "1", "0") d.addCallback(self._process_incoming_mail) d.addErrback(_sync_errback) return d - self.log.debug("fetching mail for: %s %s" % ( + self.log.debug('Fetching mail for: %s %s' % ( self._soledad.uuid, self._userid)) d = self._sync_soledad() d.addCallbacks(syncSoledadCallback, self._errback) @@ -231,7 +219,7 @@ class IncomingMail(Service): :rtype: iterable or None """ def _log_synced(result): - self.log.info('sync finished') + self.log.info('Sync finished') return result def _handle_invalid_auth_token_error(failure): @@ -240,7 +228,7 @@ class IncomingMail(Service): self.stopService() emit_async(catalog.SOLEDAD_INVALID_AUTH_TOKEN, self._userid) - self.log.info('starting sync...') + self.log.info('Starting sync...') d = self._soledad.sync() d.addCallbacks(_log_synced, _handle_invalid_auth_token_error) return d @@ -258,7 +246,7 @@ class IncomingMail(Service): fetched_ts = time.mktime(time.gmtime()) num_mails = len(doclist) if doclist is not None else 0 if num_mails != 0: - self.log.info("there are %s mails" % (num_mails,)) + self.log.info('There are {0!s} mails'.format(num_mails)) emit_async(catalog.MAIL_FETCHED_INCOMING, self._userid, str(num_mails), str(fetched_ts)) return doclist @@ -282,7 +270,7 @@ class IncomingMail(Service): deferreds = [] for index, doc in enumerate(doclist): self.log.debug( - 'processing incoming message: %d of %d' + 'Processing Incoming Message: %d of %d' % (index + 1, num_mails)) emit_async(catalog.MAIL_MSG_PROCESSING, self._userid, str(index), str(num_mails)) @@ -292,15 +280,15 @@ class IncomingMail(Service): # TODO Compatibility check with the index in pre-0.6 mx # that does not write the ERROR_DECRYPTING_KEY # This should be removed in 0.7 + # TODO deprecate this already has_errors = doc.content.get(fields.ERROR_DECRYPTING_KEY, None) - if has_errors is None: - warnings.warn("JUST_MAIL_COMPAT_IDX will be deprecated!", + warnings.warn('JUST_MAIL_COMPAT_IDX will be deprecated!', DeprecationWarning) if has_errors: - self.log.debug("Skipping message with decrypting errors...") + self.log.debug('Skipping message with decrypting errors...') elif self._is_msg(keys): # TODO this pipeline is a bit obscure! d = self._decrypt_doc(doc) @@ -784,7 +772,7 @@ class IncomingMail(Service): else: self.log.debug("No valid url on OpenPGP header %s" % (url,)) else: - self.log.debug("There is no url on the OpenPGP header: %s" + self.log.debug('There is no url on the OpenPGP header: %s' % (header,)) return False @@ -843,12 +831,10 @@ class IncomingMail(Service): self.log.info('Adding message %s to local db' % (doc.doc_id,)) def msgSavedCallback(result): + if empty(result): return - for listener in self._listeners: - listener(result) - def signal_deleted(doc_id): emit_async(catalog.MAIL_MSG_DELETED_INCOMING, self._userid) diff --git a/src/leap/bitmask/mail/mail.py b/src/leap/bitmask/mail/mail.py index 8cc01e27..92435fdb 100644 --- a/src/leap/bitmask/mail/mail.py +++ b/src/leap/bitmask/mail/mail.py @@ -970,6 +970,10 @@ class Account(object): return d def add_mailbox(self, name, creation_ts=None): + return self.adaptor.atomic.run( + self._add_mailbox, name, creation_ts=creation_ts) + + def _add_mailbox(self, name, creation_ts=None): if creation_ts is None: # by default, we pass an int value @@ -1004,6 +1008,10 @@ class Account(object): return d def delete_mailbox(self, name): + return self.adaptor.atomic.run( + self._delete_mailbox, name) + + def _delete_mailbox(self, name): def delete_uid_table_cb(wrapper): d = self.mbox_indexer.delete_table(wrapper.uuid) @@ -1017,6 +1025,10 @@ class Account(object): return d def rename_mailbox(self, oldname, newname): + return self.adaptor.atomic.run( + self._rename_mailbox, oldname, newname) + + def _rename_mailbox(self, oldname, newname): def _rename_mbox(wrapper): wrapper.mbox = newname @@ -1035,6 +1047,10 @@ class Account(object): :rtype: deferred :return: a deferred that will fire with a MessageCollection """ + return self.adaptor.atomic.run( + self._get_collection_by_mailbox, name) + + def _get_collection_by_mailbox(self, name): collection = self._collection_mapping[self.user_id].get( name, None) if collection: @@ -1056,8 +1072,7 @@ class Account(object): :rtype: MessageCollection """ # get a collection of docs by a list of doc_id - # get.docs(...) --> it should be a generator. does it behave in the - # threadpool? + # get.docs(...) --> it should be a generator raise NotImplementedError() def get_collection_by_tag(self, tag): diff --git a/src/leap/bitmask/mail/smtp/bounces.py b/src/leap/bitmask/mail/smtp/bounces.py index 8260c1b0..608d87db 100644 --- a/src/leap/bitmask/mail/smtp/bounces.py +++ b/src/leap/bitmask/mail/smtp/bounces.py @@ -84,6 +84,8 @@ class Bouncer(object): def bouncerFactory(soledad): user_id = soledad.uuid + # TODO should reuse same account that is used in other places, otherwise + # new mail in here won't be notified. acc = Account(soledad, user_id) d = acc.callWhenReady(lambda _: acc.get_collection_by_mailbox(INBOX_NAME)) d.addCallback(lambda inbox: Bouncer(inbox)) diff --git a/src/leap/bitmask/mua/pixelizer.py b/src/leap/bitmask/mua/pixelizer.py index ed69afb2..c2dd7144 100644 --- a/src/leap/bitmask/mua/pixelizer.py +++ b/src/leap/bitmask/mua/pixelizer.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -# pix.py -# Copyright (C) 2016 LEAP +# pixelizer.py +# Copyright (C) 2016-2017 LEAP # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -14,9 +14,22 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. + """ -Pixelated plugin integration. +Pixelizer: the pixelated integrator. + +Right now this module is kind of hardcoded, but it should be possible for +bitmask to use this as an optional plugin. For the moment we fail gracefully if +we cannot find the pixelated modules in the import path. + +There's a lot of code duplication right now in this double game of adaptors for +pixelated adaptors. Refactoring pixelated and bitmask should be possible so +that we converge the mail apis and stop needing this nasty proliferation of +adaptors. + +However, some care has to be taken to avoid certain types of concurrency bugs. """ + import json import os import sys @@ -25,12 +38,10 @@ from twisted.internet import defer, reactor from twisted.logger import Logger from leap.common.config import get_path_prefix -from leap.bitmask.mail.mail import Account from leap.bitmask.keymanager import KeyNotFound try: from pixelated.adapter.mailstore import LeapMailStore - from pixelated.adapter.welcome_mail import add_welcome_mail from pixelated.application import SingleUserServicesFactory from pixelated.application import UserAgentMode from pixelated.application import start_site @@ -51,10 +62,13 @@ log = Logger() # [ ] pre-authenticate -def start_pixelated_user_agent(userid, soledad, keymanager): +def start_pixelated_user_agent(userid, soledad, keymanager, account): - leap_session = LeapSessionAdapter( - userid, soledad, keymanager) + try: + leap_session = LeapSessionAdapter( + userid, soledad, keymanager, account) + except Exception as exc: + log.error("Got error! %r" % exc) config = Config() leap_home = os.path.join(get_path_prefix(), 'leap') @@ -134,9 +148,23 @@ class NickNym(object): return self.keymanager.send_key() +class _LeapMailStore(LeapMailStore): + + def __init__(self, soledad, account): + self.account = account + super(_LeapMailStore, self).__init__(soledad) + + # We should rewrite the LeapMailStore in the coming pixelated fork so that + # we reuse the account instance. + @defer.inlineCallbacks + def add_mail(self, mailbox_name, raw_msg): + inbox = yield self.account.get_collection_by_mailbox(mailbox_name) + yield inbox.add_msg(raw_msg, ('\\Recent',), notify_just_mdoc=False) + + class LeapSessionAdapter(object): - def __init__(self, userid, soledad, keymanager): + def __init__(self, userid, soledad, keymanager, account): self.userid = userid self.soledad = soledad @@ -144,14 +172,16 @@ class LeapSessionAdapter(object): # XXX this needs to be converged with our public apis. _n = NickNym(keymanager, userid) self.nicknym = self.keymanager = _n - self.mail_store = LeapMailStore(soledad) + + self.mail_store = _LeapMailStore(soledad, account) + + self.account = account self.user_auth = Config() self.user_auth.uuid = soledad.uuid self.fresh_account = False self.incoming_mail_fetcher = None - self.account = Account(soledad, userid) username, provider = userid.split('@') smtp_client_cert = os.path.join( @@ -201,13 +231,7 @@ def _start_in_single_user_mode(leap_session, config, resource, services_factory): start_site(config, resource) reactor.callLater( - # workaround for #8798 - # we need to make pixelated initialization a bit behind - # the own leap initialization, because otherwise the inbox is created - # without the needed callbacks for IMAP compatibility. - # This should be better addressed at pixelated code, by using the mail - # api to create the collection. - 3, start_user_agent_in_single_user_mode, + 0, start_user_agent_in_single_user_mode, resource, services_factory, leap_session.config.leap_home, leap_session) @@ -221,10 +245,10 @@ def start_user_agent_in_single_user_mode(root_resource, _services = services.Services(leap_session) yield _services.setup() - if leap_session.fresh_account: - yield add_welcome_mail(leap_session.mail_store) + # TODO we might want to use a Bitmask specific mail + # if leap_session.fresh_account: + # yield add_welcome_mail(leap_session.mail_store) services_factory.add_session(leap_session.user_auth.uuid, _services) - root_resource.initialize(provider=leap_session.provider) log.info('Done, the Pixelated User Agent is ready to be used') diff --git a/tests/integration/mail/imap/test_imap.py b/tests/integration/mail/imap/test_imap.py index 8d34a499..6f8eec2a 100644 --- a/tests/integration/mail/imap/test_imap.py +++ b/tests/integration/mail/imap/test_imap.py @@ -636,7 +636,7 @@ class LEAPIMAP4ServerTestCase(IMAP4HelperMixin): for details. """ # TODO implement the IMAP4ClientExamineTests testcase. - mbox_name = "test_mailbox_e" + mbox_name = "test_mailbox_examine" acc = self.server.theAccount self.examinedArgs = None diff --git a/tests/unit/mail/imap/test_mailbox.py b/tests/unit/mail/imap/test_mailbox.py new file mode 100644 index 00000000..ce269dca --- /dev/null +++ b/tests/unit/mail/imap/test_mailbox.py @@ -0,0 +1,49 @@ +# -*- coding: utf-8 -*- +# test_service.py +# Copyright (C) 2016 LEAP +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +import unittest + +from leap.bitmask.mail.imap.mailbox import make_collection_listener + + +class _mbox(object): + pass + + +class TestListener(unittest.TestCase): + + def setUp(self): + pass + + def test_mailbox_listener(self): + mbox1 = _mbox() + mbox1.mbox_name = 'inbox' + + mbox2 = _mbox() + mbox2.mbox_name = 'inbox' + + mbox3 = _mbox() + mbox3.mbox_name = 'trash' + + _set1 = set([mbox1] + [make_collection_listener(mbox2)] + + [make_collection_listener(mbox3)]) + assert len(_set1) == 3 + + _set2 = set([make_collection_listener(mbox1)] + + [make_collection_listener(mbox2)] + + [make_collection_listener(mbox3)]) + assert len(_set2) == 2 |