From e7f77052f0aba1b84febf0ab1382c9602bbf7f93 Mon Sep 17 00:00:00 2001 From: NavaL Date: Wed, 3 Feb 2016 17:23:42 +0100 Subject: closing the services closes leap session, which stops background tasks, close soldedad and remove it from cache Issue #586 --- service/pixelated/application.py | 7 ++- service/pixelated/assets/index.html | 9 ++++ service/pixelated/bitmask_libraries/session.py | 51 ++++++++++++++-------- service/pixelated/config/services.py | 10 ++--- service/pixelated/resources/__init__.py | 2 +- service/pixelated/resources/auth.py | 11 ++--- service/pixelated/resources/login_resource.py | 4 +- service/pixelated/resources/logout_resource.py | 2 + service/test/integration/test_logout.py | 41 +++++++++++++++++ service/test/load/__init__.py | 0 .../test/support/integration/app_test_client.py | 5 ++- .../test/support/integration/multi_user_client.py | 6 ++- .../test/unit/bitmask_libraries/test_session.py | 24 +++++++++- service/test/unit/config/test_services.py | 34 +++++++++++++++ service/test/unit/resources/test_login_resource.py | 8 ++++ .../test/unit/resources/test_logout_resources.py | 3 +- 16 files changed, 177 insertions(+), 40 deletions(-) create mode 100644 service/pixelated/assets/index.html create mode 100644 service/test/integration/test_logout.py create mode 100644 service/test/load/__init__.py create mode 100644 service/test/unit/config/test_services.py diff --git a/service/pixelated/application.py b/service/pixelated/application.py index dafab0b1..b0f2d3fc 100644 --- a/service/pixelated/application.py +++ b/service/pixelated/application.py @@ -53,8 +53,8 @@ class ServicesFactory(object): def log_out_user(self, user_id): if self.is_logged_in(user_id): - services = self._services_by_user[user_id] - services.close() + _services = self._services_by_user[user_id] + _services.close() del self._services_by_user[user_id] def add_session(self, user_id, services): @@ -64,7 +64,6 @@ class ServicesFactory(object): def create_services_from(self, leap_session): _services = services.Services(leap_session) yield _services.setup() - self._services_by_user[leap_session.user_auth.uuid] = _services @@ -176,7 +175,7 @@ def _start_in_multi_user_mode(args, root_resource, services_factory): def set_up_protected_resources(root_resource, provider, services_factory, checker=None): if not checker: checker = LeapPasswordChecker(provider) - session_checker = SessionChecker() + session_checker = SessionChecker(services_factory) anonymous_resource = LoginResource(services_factory) realm = PixelatedRealm(root_resource, anonymous_resource) diff --git a/service/pixelated/assets/index.html b/service/pixelated/assets/index.html new file mode 100644 index 00000000..c095577e --- /dev/null +++ b/service/pixelated/assets/index.html @@ -0,0 +1,9 @@ + + + + + + click here + + + diff --git a/service/pixelated/bitmask_libraries/session.py b/service/pixelated/bitmask_libraries/session.py index 4276c0c3..8b9118ce 100644 --- a/service/pixelated/bitmask_libraries/session.py +++ b/service/pixelated/bitmask_libraries/session.py @@ -39,7 +39,7 @@ from leap.common.events import ( log = logging.getLogger(__name__) -SESSIONS = {} +SESSIONS = {} # TODO replace with redis or memCached in prod class LeapSession(object): @@ -90,6 +90,12 @@ class LeapSession(object): def close(self): self.stop_background_jobs() unregister(events.KEYMANAGER_FINISHED_KEY_GENERATION, uid=self.account_email()) + self.soledad.close() + self.remove_from_cache() + + def remove_from_cache(self): + key = SessionCache.session_key(self.provider, self.user_auth.username) + SessionCache.remove_session(key) @defer.inlineCallbacks def _create_incoming_mail_fetcher(self, nicknym, soledad, account, user_mail): @@ -148,11 +154,11 @@ class LeapSessionFactory(object): self._config = provider.config def create(self, username, password, auth=None): - key = self._session_key(username) - session = self._lookup_session(key) + key = SessionCache.session_key(self._provider, username) + session = SessionCache.lookup_session(key) if not session: session = self._create_new_session(username, password, auth) - self._remember_session(key, session) + SessionCache.remember_session(key, session) return session @@ -201,20 +207,6 @@ class LeapSessionFactory(object): self._provider.domain, "keys", "client", "smtp.pem") - def _lookup_session(self, key): - global SESSIONS - if key in SESSIONS: - return SESSIONS[key] - else: - return None - - def _remember_session(self, key, session): - global SESSIONS - SESSIONS[key] = session - - def _session_key(self, username): - return hash((self._provider, username)) - def _create_dir(self, path): try: os.makedirs(path) @@ -244,3 +236,26 @@ class LeapSessionFactory(object): pass else: raise + + +class SessionCache(object): # should be replaced with redis or memcached in prod + + @staticmethod + def lookup_session(key): + global SESSIONS + return SESSIONS.get(key, None) + + @staticmethod + def remember_session(key, session): + global SESSIONS + SESSIONS[key] = session + + @staticmethod + def remove_session(key): + global SESSIONS + if key in SESSIONS: + del SESSIONS[key] + + @staticmethod + def session_key(provider, username): + return hash((provider, username)) diff --git a/service/pixelated/config/services.py b/service/pixelated/config/services.py index 3f254571..de0e2537 100644 --- a/service/pixelated/config/services.py +++ b/service/pixelated/config/services.py @@ -25,16 +25,13 @@ class Services(object): @defer.inlineCallbacks def setup(self): search_index_storage_key = self._setup_search_index_storage_key(self._leap_session.soledad) - yield self._setup_search_engine( - self._leap_session.user_auth.uuid, - search_index_storage_key) + yield self._setup_search_engine(self._leap_session.user_auth.uuid, search_index_storage_key) self._wrap_mail_store_with_indexing_mail_store(self._leap_session) yield listen_all_mailboxes(self._leap_session.account, self.search_engine, self._leap_session.mail_store) - self.mail_service = self._setup_mail_service( - self.search_engine) + self.mail_service = self._setup_mail_service(self.search_engine) self.keymanager = self._leap_session.nicknym self.draft_service = self._setup_draft_service(self._leap_session.mail_store) @@ -42,6 +39,9 @@ class Services(object): yield self._index_all_mails() + def close(self): + self._leap_session.close() + def _wrap_mail_store_with_indexing_mail_store(self, leap_session): leap_session.mail_store = SearchableMailStore(leap_session.mail_store, self.search_engine) diff --git a/service/pixelated/resources/__init__.py b/service/pixelated/resources/__init__.py index 2e451a69..14ecac86 100644 --- a/service/pixelated/resources/__init__.py +++ b/service/pixelated/resources/__init__.py @@ -63,7 +63,7 @@ class BaseResource(Resource): def is_logged_in(self, request): session = self.get_session(request) - return session.is_logged_in() + return session.is_logged_in() and self._services_factory.is_logged_in(session.user_uuid) def get_session(self, request): return IPixelatedSession(request.getSession()) diff --git a/service/pixelated/resources/auth.py b/service/pixelated/resources/auth.py index 92efaa27..e2e7ec97 100644 --- a/service/pixelated/resources/auth.py +++ b/service/pixelated/resources/auth.py @@ -78,12 +78,14 @@ class SessionCredential(object): class SessionChecker(object): credentialInterfaces = (ISessionCredential,) + def __init__(self, services_factory): + self._services_factory = services_factory + def requestAvatarId(self, credentials): session = self.get_session(credentials.request) - if session.is_logged_in(): - return defer.succeed(session.user_uuid) - else: - return defer.succeed(ANONYMOUS) + if session.is_logged_in() and self._services_factory.is_logged_in(session.user_uuid): + return defer.succeed(session.user_uuid) + return defer.succeed(ANONYMOUS) def get_session(self, request): return IPixelatedSession(request.getSession()) @@ -131,7 +133,6 @@ class PixelatedAuthSessionWrapper(object): def _loginSucceeded(self, args): interface, avatar, logout = args - if avatar == checkers.ANONYMOUS: return self._anonymous_resource else: diff --git a/service/pixelated/resources/login_resource.py b/service/pixelated/resources/login_resource.py index ca8f0b11..1e1beb48 100644 --- a/service/pixelated/resources/login_resource.py +++ b/service/pixelated/resources/login_resource.py @@ -99,11 +99,11 @@ class LoginResource(BaseResource): if self.is_logged_in(request): return util.redirectTo("/", request) - def render_response(leap_user): + def render_response(leap_session): request.setResponseCode(OK) request.write(open(os.path.join(self._startup_folder, 'Interstitial.html')).read()) request.finish() - self._setup_user_services(leap_user, request) + self._setup_user_services(leap_session, request) def render_error(error): log.info('Login Error for %s' % request.args['username'][0]) diff --git a/service/pixelated/resources/logout_resource.py b/service/pixelated/resources/logout_resource.py index 9fd16451..fe80316e 100644 --- a/service/pixelated/resources/logout_resource.py +++ b/service/pixelated/resources/logout_resource.py @@ -10,5 +10,7 @@ class LogoutResource(BaseResource): def render_GET(self, request): session = self.get_session(request) + self._services_factory.log_out_user(session.user_uuid) session.expire() + return util.redirectTo("/%s" % LoginResource.BASE_URL, request) diff --git a/service/test/integration/test_logout.py b/service/test/integration/test_logout.py new file mode 100644 index 00000000..52f7e34f --- /dev/null +++ b/service/test/integration/test_logout.py @@ -0,0 +1,41 @@ +# +# Copyright (c) 2014 ThoughtWorks, Inc. +# +# Pixelated is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Pixelated 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 Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with Pixelated. If not, see . +from mockito import verify +from twisted.internet import defer + +from test.support.integration import load_mail_from_file +from test.support.integration.multi_user_client import MultiUserClient +from test.support.integration.soledad_test_base import SoledadTestBase + + +class MultiUserLogoutTest(MultiUserClient, SoledadTestBase): + + @defer.inlineCallbacks + def wait_for_session_user_id_to_finish(self): + yield self.adaptor.initialize_store(self.soledad) + + @defer.inlineCallbacks + def test_logout_deletes_services_stop_background_reactor_tasks_and_closes_soledad(self): + response, login_request = yield self.login() + yield response + + yield self.wait_for_session_user_id_to_finish() + + response, request = self.get("/logout", as_json=False, from_request=login_request) + yield response + + self.assertEqual(302, request.responseCode) # redirected + verify(self.services).close() diff --git a/service/test/load/__init__.py b/service/test/load/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/service/test/support/integration/app_test_client.py b/service/test/support/integration/app_test_client.py index e841cfe5..99f4ebc7 100644 --- a/service/test/support/integration/app_test_client.py +++ b/service/test/support/integration/app_test_client.py @@ -39,6 +39,7 @@ from pixelated.adapter.services.feedback_service import FeedbackService from pixelated.application import ServicesFactory, UserAgentMode, SingleUserServicesFactory, set_up_protected_resources from pixelated.bitmask_libraries.config import LeapConfig from pixelated.bitmask_libraries.session import LeapSession +from pixelated.config.services import Services from pixelated.config.site import PixelatedSite from pixelated.adapter.mailstore import LeapMailStore @@ -92,14 +93,16 @@ class AppTestAccount(object): @property def services(self): if self._services is None: - services = mock() + services = mock(Services) services.keymanager = self.keymanager services.mail_service = self.mail_service services.draft_service = self.draft_service services.search_engine = self.search_engine services.feedback_service = self.feedback_service + services._leap_session = self.leap_session self._services = services + self.leap_session.close = lambda: 'mocked' return self._services diff --git a/service/test/support/integration/multi_user_client.py b/service/test/support/integration/multi_user_client.py index 19833c9f..fa65fb06 100644 --- a/service/test/support/integration/multi_user_client.py +++ b/service/test/support/integration/multi_user_client.py @@ -49,7 +49,7 @@ class MultiUserClient(AppTestClient): self.resource = set_up_protected_resources(root_resource, leap_provider, self.service_factory) def login(self, username='username', password='password'): - leap_session = mock(LeapSession) + leap_session = self._test_account.leap_session user_auth = mock() user_auth.uuid = 'some_user_uuid' leap_session.user_auth = user_auth @@ -57,11 +57,13 @@ class MultiUserClient(AppTestClient): config.leap_home = 'some_folder' leap_session.config = config leap_session.fresh_account = False + self.leap_session = leap_session + self.services = self._test_account.services self._set_leap_srp_auth(username, password, user_auth) when(LeapSessionFactory).create(username, password, user_auth).thenReturn(leap_session) when(leap_session).initial_sync().thenAnswer(lambda: defer.succeed(None)) - when(pixelated.config.services).Services(ANY()).thenReturn(self._test_account.services) + when(pixelated.config.services).Services(ANY()).thenReturn(self.services) request = request_mock(path='/login', method="POST", body={'username': username, 'password': password}) return self._render(request, as_json=False) diff --git a/service/test/unit/bitmask_libraries/test_session.py b/service/test/unit/bitmask_libraries/test_session.py index 6e0e6204..bb1f55ca 100644 --- a/service/test/unit/bitmask_libraries/test_session.py +++ b/service/test/unit/bitmask_libraries/test_session.py @@ -17,7 +17,7 @@ from mock import patch from mock import MagicMock -from pixelated.bitmask_libraries.session import LeapSession +from pixelated.bitmask_libraries.session import LeapSession, SessionCache from test_abstract_leap import AbstractLeapTest from leap.common.events.catalog import KEYMANAGER_FINISHED_KEY_GENERATION @@ -69,6 +69,28 @@ class SessionTest(AbstractLeapTest): unregister_mock.assert_called_once_with(KEYMANAGER_FINISHED_KEY_GENERATION, uid=email) + def test_close_stops_soledad(self): + email = 'someone@somedomain.tld' + self.provider.address_for.return_value = email + session = self._create_session() + + session.close() + self.soledad_session.close.assert_called_once_with() + + def test_close_removes_session_from_cache(self): + email = 'someone@somedomain.tld' + self.provider.address_for.return_value = email + session = self._create_session() + + key = SessionCache.session_key(self.provider, self.auth.username) + SessionCache.remember_session(key, session) + + self.assertEqual(session, SessionCache.lookup_session(key)) + + session.close() + + self.assertIsNone(SessionCache.lookup_session(key)) + @patch('pixelated.bitmask_libraries.session.register') def test_session_fresh_is_initially_false(self, _): session = self._create_session() diff --git a/service/test/unit/config/test_services.py b/service/test/unit/config/test_services.py new file mode 100644 index 00000000..71765d19 --- /dev/null +++ b/service/test/unit/config/test_services.py @@ -0,0 +1,34 @@ +# +# Copyright (c) 2014 ThoughtWorks, Inc. +# +# Pixelated is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Pixelated 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 Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with Pixelated. If not, see . +import unittest + +from mockito import mock, verify + +from pixelated.config.services import Services + + +class ServicesTest(unittest.TestCase): + + def setUp(self): + super(ServicesTest, self).setUp() + self.leap_session = mock() + config = mock() + self.leap_session.config = config + self.services = Services(self.leap_session) + + def test_close_services_closes_the_underlying_leap_session(self): + self.services.close() + verify(self.leap_session).close() diff --git a/service/test/unit/resources/test_login_resource.py b/service/test/unit/resources/test_login_resource.py index b3aaccc2..bc238ae8 100644 --- a/service/test/unit/resources/test_login_resource.py +++ b/service/test/unit/resources/test_login_resource.py @@ -1,3 +1,5 @@ +import test.support.mockito + from leap.exceptions import SRPAuthenticationError from mock import patch from mockito import mock, when, any as ANY, verify, verifyZeroInteractions, verifyNoMoreInteractions @@ -33,6 +35,7 @@ class TestLoginResource(unittest.TestCase): def test_there_are_no_grand_children_resources_when_logged_in(self, mock_is_logged_in): request = DummyRequest(['/login/grand_children']) mock_is_logged_in.return_value = True + when(self.services_factory).is_logged_in(ANY()).thenReturn(True) d = self.web.get(request) @@ -93,9 +96,13 @@ class TestLoginPOST(unittest.TestCase): self.leap_session = leap_session self.user_auth = user_auth + def mock_user_has_services_setup(self): + when(self.services_factory).is_logged_in('some_user_uuid').thenReturn(True) + def test_login_responds_interstitial_and_add_corresponding_session_to_services_factory(self): irrelevant = None when(self.portal).login(ANY(), None, IResource).thenReturn((irrelevant, self.leap_session, irrelevant)) + when(self.services_factory).create_services_from(self.leap_session).thenAnswer(self.mock_user_has_services_setup) d = self.web.get(self.request) @@ -163,6 +170,7 @@ class TestLoginPOST(unittest.TestCase): @patch('pixelated.resources.session.PixelatedSession.is_logged_in') def test_should_not_process_login_if_already_logged_in(self, mock_logged_in, mock_redirect): mock_logged_in.return_value = True + when(self.services_factory).is_logged_in(ANY()).thenReturn(True) mock_redirect.return_value = "mocked redirection" when(self.portal).login(ANY(), None, IResource).thenRaise(Exception()) d = self.web.get(self.request) diff --git a/service/test/unit/resources/test_logout_resources.py b/service/test/unit/resources/test_logout_resources.py index bb0aea9e..48cf9db9 100644 --- a/service/test/unit/resources/test_logout_resources.py +++ b/service/test/unit/resources/test_logout_resources.py @@ -1,5 +1,5 @@ from mock import patch -from mockito import mock +from mockito import mock, verify from twisted.trial import unittest from twisted.web.test.requesthelper import DummyRequest @@ -24,6 +24,7 @@ class TestLogoutResource(unittest.TestCase): def expire_session_and_redirect(_): session = self.resource.get_session(request) self.assertFalse(session.is_logged_in()) + verify(self.services_factory).log_out_user(session.user_uuid) mock_redirect.assert_called_once_with('/login', request) d.addCallback(expire_session_and_redirect) -- cgit v1.2.3