Merge pull request #1052 from pixelated/email-recovery-code

[#927] Email recovery code

18 files changed, 146 insertions, 78 deletions

diff --git a/service/pixelated/account_recovery.py b/service/pixelated/account_recovery.py
index c0a18792..2208faf5 100644
--- a/service/pixelated/account_recovery.py
+++ b/service/pixelated/account_recovery.py
@@ -25,14 +25,17 @@ log = Logger()
 
 
 class AccountRecovery(object):
-    def __init__(self, session, soledad, smtp_config, backup_email):
+    def __init__(self, session, soledad, smtp_config, backup_email, domain):
         self._bonafide_session = session
         self._soledad = soledad
         self._smtp_config = smtp_config
         self._backup_email = backup_email
+        self._domain = domain
 
     @inlineCallbacks
     def update_recovery_code(self):
+        log.info('Updating user\'s recovery code')
+
         try:
             code = self._soledad.create_recovery_code()
             response = yield self._bonafide_session.update_recovery_code(code)
@@ -47,15 +50,18 @@ class AccountRecovery(object):
 
     @inlineCallbacks
     def _send_mail(self, code, backup_email):
+        log.info('Sending mail containing the user\'s recovery code')
+
+        sender = 'team@{}'.format(self._domain)
         msg = MIMEText('Your code %s' % code)
         msg['Subject'] = 'Recovery Code'
-        msg['From'] = 'team@pixelated-project.org'
+        msg['From'] = sender
         msg['To'] = backup_email
 
         try:
             send_mail_result = yield smtp.sendmail(
                 str(self._smtp_config.remote_smtp_host),
-                'team@pixelated-project.org',
+                sender,
                 [backup_email],
                 msg.as_string())
             returnValue(send_mail_result)
diff --git a/service/pixelated/resources/backup_account_resource.py b/service/pixelated/resources/backup_account_resource.py
index f51ac2ec..ec3e9dee 100644
--- a/service/pixelated/resources/backup_account_resource.py
+++ b/service/pixelated/resources/backup_account_resource.py
@@ -17,16 +17,15 @@
 import os
 import json
 
-from xml.sax import SAXParseException
-
-from pixelated.resources import BaseResource
 from twisted.python.filepath import FilePath
-from pixelated.resources import get_protected_static_folder
-from pixelated.account_recovery import AccountRecovery
 from twisted.web.http import OK, NO_CONTENT, INTERNAL_SERVER_ERROR
 from twisted.web.server import NOT_DONE_YET
 from twisted.web.template import Element, XMLFile, renderElement
 
+from pixelated.resources import BaseResource
+from pixelated.resources import get_protected_static_folder
+from pixelated.account_recovery import AccountRecovery
+
 
 class BackupAccountPage(Element):
     loader = XMLFile(FilePath(os.path.join(get_protected_static_folder(), 'backup_account.html')))
@@ -38,9 +37,10 @@ class BackupAccountPage(Element):
 class BackupAccountResource(BaseResource):
     isLeaf = True
 
-    def __init__(self, services_factory, authenticator):
+    def __init__(self, services_factory, authenticator, leap_provider):
         BaseResource.__init__(self, services_factory)
         self._authenticator = authenticator
+        self._leap_provider = leap_provider
 
     def render_GET(self, request):
         request.setResponseCode(OK)
@@ -55,7 +55,8 @@ class BackupAccountResource(BaseResource):
             self._authenticator.bonafide_session,
             self.soledad(request),
             self._service(request, '_leap_session').smtp_config,
-            self._get_backup_email(request))
+            self._get_backup_email(request),
+            self._leap_provider.server_name)
 
         def update_response(response):
             request.setResponseCode(NO_CONTENT)
diff --git a/service/pixelated/resources/root_resource.py b/service/pixelated/resources/root_resource.py
index d860c422..896bc24b 100644
--- a/service/pixelated/resources/root_resource.py
+++ b/service/pixelated/resources/root_resource.py
@@ -93,7 +93,7 @@ class RootResource(BaseResource):
     def initialize(self, provider=None, disclaimer_banner=None, authenticator=None):
         self._child_resources.add('assets', File(self._protected_static_folder))
         self._child_resources.add(AccountRecoveryResource.BASE_URL, AccountRecoveryResource(self._services_factory))
-        self._child_resources.add('backup-account', BackupAccountResource(self._services_factory, authenticator))
+        self._child_resources.add('backup-account', BackupAccountResource(self._services_factory, authenticator, provider))
         self._child_resources.add('sandbox', SandboxResource(self._protected_static_folder))
         self._child_resources.add('keys', KeysResource(self._services_factory))
         self._child_resources.add(AttachmentsResource.BASE_URL, AttachmentsResource(self._services_factory))
diff --git a/service/test/functional/features/account_recovery.feature b/service/test/functional/features/account_recovery.feature
index cf0144e0..da167d31 100644
--- a/service/test/functional/features/account_recovery.feature
+++ b/service/test/functional/features/account_recovery.feature
@@ -14,7 +14,7 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 
-@smoke
+@smoke @require_user
 Feature: Account Recovery
   As a user of Pixelated
   I want to recover my account
@@ -28,6 +28,12 @@ Feature: Account Recovery
     And I logout from the header
     And I should see the login page
 
+  Scenario: Confirming I received the recovery code at my backup email
+    Given I am logged in Pixelated
+    When I open the mail with the recovery code
+    Then I see the mail has the recovery code
+    Then I logout
+
   Scenario: Recovering an account
     Given I am on the account recovery page
     When I submit admin recovery code
diff --git a/service/test/functional/features/environment.py b/service/test/functional/features/environment.py
index f1cc0e44..bc2e1283 100644
--- a/service/test/functional/features/environment.py
+++ b/service/test/functional/features/environment.py
@@ -52,12 +52,14 @@ def before_all(context):
     if not context.host.startswith('http'):
         context.host = 'https://{}'.format(context.host)
 
-    hostname = urlparse(context.host).hostname
-    context.signup_url = 'https://{}/signup'.format(hostname)
-    context.login_url = 'https://mail.{}/login'.format(hostname)
-    context.backup_account_url = 'https://mail.{}/backup-account'.format(hostname)
-    context.account_recovery_url = 'https://mail.{}/account-recovery'.format(hostname)
+    context.hostname = urlparse(context.host).hostname
+    context.signup_url = 'https://{}/signup'.format(context.hostname)
+    context.inbox_url = 'https://mail.{}'.format(context.hostname)
+    context.login_url = 'https://mail.{}/login'.format(context.hostname)
+    context.backup_account_url = 'https://mail.{}/backup-account'.format(context.hostname)
+    context.account_recovery_url = 'https://mail.{}/account-recovery'.format(context.hostname)
     context.username = 'testuser_{}'.format(uuid.uuid4())
+    context.user_email = '{}@{}'.format(context.username, context.hostname)
 
     if 'localhost' in context.host:
         _mock_user_agent(context)
@@ -68,8 +70,9 @@ def before_all(context):
 
 
 def before_tag(context, tag):
-    if tag == "smoke":
+    if tag == "require_user":
         context.username = 'testuser_{}'.format(uuid.uuid4())
+        context.user_email = '{}@{}'.format(context.username, context.hostname)
         utils.create_user(context)
 
 
diff --git a/service/test/functional/features/page_objects/__init__.py b/service/test/functional/features/page_objects/__init__.py
index 6868330b..920bf541 100644
--- a/service/test/functional/features/page_objects/__init__.py
+++ b/service/test/functional/features/page_objects/__init__.py
@@ -14,5 +14,6 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 
-from base_page import BasePage
 from account_recovery_page import AccountRecoveryPage
+from base_page import BasePage
+from inbox_page import InboxPage
diff --git a/service/test/functional/features/page_objects/account_recovery_page.py b/service/test/functional/features/page_objects/account_recovery_page.py
index 6f502994..4826b6ec 100644
--- a/service/test/functional/features/page_objects/account_recovery_page.py
+++ b/service/test/functional/features/page_objects/account_recovery_page.py
@@ -19,11 +19,7 @@ from base_page import BasePage
 
 class AccountRecoveryPage(BasePage):
     def __init__(self, context):
-        BasePage.__init__(
-            self,
-            context,
-            context.account_recovery_url
-        )
+        super(AccountRecoveryPage, self).__init__(context, context.account_recovery_url)
 
         self._locators = {
             'admin_code': 'input[name="admin-code"]',
diff --git a/service/test/functional/features/page_objects/base_page.py b/service/test/functional/features/page_objects/base_page.py
index e5fb9a39..4756d930 100644
--- a/service/test/functional/features/page_objects/base_page.py
+++ b/service/test/functional/features/page_objects/base_page.py
@@ -16,7 +16,8 @@
 
 from steps.common import (
     fill_by_css_selector,
-    find_element_by_css_selector)
+    find_element_by_css_selector,
+    find_element_by_xpath)
 
 
 class BasePage(object):
@@ -31,5 +32,8 @@ class BasePage(object):
     def fill_by_css_selector(self, loc, text):
         fill_by_css_selector(self.context, loc, text)
 
+    def find_element_by_xpath(self, xpath):
+        return find_element_by_xpath(self.context, xpath)
+
     def visit(self):
         self.context.browser.get(self.base_url)
diff --git a/service/test/functional/features/page_objects/inbox_page.py b/service/test/functional/features/page_objects/inbox_page.py
new file mode 100644
index 00000000..67ef1375
--- /dev/null
+++ b/service/test/functional/features/page_objects/inbox_page.py
@@ -0,0 +1,52 @@
+#
+# Copyright (c) 2017 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 <http://www.gnu.org/licenses/>.
+
+from base_page import BasePage
+from common import execute_ignoring_staleness
+
+
+class InboxPage(BasePage):
+    def __init__(self, context):
+        super(InboxPage, self).__init__(context, context.inbox_url)
+
+        self._locators = {
+            'first_email': '.mail-list-entry__item',
+            'read_sandbox': '#read-sandbox',
+            'iframe_body': 'body',
+        }
+
+    def _get_first_mail(self):
+        return self.find_element_by_css_selector(self._locators['first_email'])
+
+    def get_mail_with_subject(self, subject):
+        return self.find_element_by_xpath("//*[@class='mail-list-entry__item-subject' and contains(.,'%s')]" % subject)
+
+    def open_first_mail_in_the_mail_list(self):
+        # it seems page is often still loading so staleness exceptions happen often
+        self.context.current_mail_id = 'mail-' + execute_ignoring_staleness(
+            lambda: self._get_first_mail().get_attribute('href').split('/')[-1])
+        execute_ignoring_staleness(lambda: self._get_first_mail().click())
+
+    def open_mail_with_the_recovery_code(self):
+        self.get_mail_with_subject('Recovery Code').click()
+
+    def get_body_message(self):
+        self.find_element_by_css_selector(self._locators['read_sandbox'])
+        self.context.browser.switch_to_frame('read-sandbox')
+        body_message = self.find_element_by_css_selector(self._locators['iframe_body']).text
+        self.context.browser.switch_to_default_content()
+
+        return body_message
diff --git a/service/test/functional/features/smoke.feature b/service/test/functional/features/smoke.feature
index d106da1b..b8fdbf35 100644
--- a/service/test/functional/features/smoke.feature
+++ b/service/test/functional/features/smoke.feature
@@ -14,7 +14,7 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 
-@smoke
+@smoke @require_user
 Feature: sign up, login and logout
   As a visitor of Pixelated
   I want to sign up
diff --git a/service/test/functional/features/steps/backup_account.py b/service/test/functional/features/steps/backup_account.py
index 56d30211..5a1052a8 100644
--- a/service/test/functional/features/steps/backup_account.py
+++ b/service/test/functional/features/steps/backup_account.py
@@ -29,7 +29,7 @@ def backup_account_page(context):
 
 @when(u'I submit my backup account')
 def submit_backup_email(context):
-    fill_by_css_selector(context, 'input[name="email"]', 'test@test.com')
+    fill_by_css_selector(context, 'input[name="email"]', context.user_email)
     find_element_by_css_selector(context, '.submit-button button[type="submit"]').click()
 
 
diff --git a/service/test/functional/features/steps/common.py b/service/test/functional/features/steps/common.py
index 3e1e995e..ff6e6166 100644
--- a/service/test/functional/features/steps/common.py
+++ b/service/test/functional/features/steps/common.py
@@ -146,10 +146,6 @@ def click_button(context, title, element='button'):
     button.click()
 
 
-def mail_list_with_subject_exists(context, subject):
-    return find_element_by_xpath(context, "//*[@class='mail-list-entry__item-subject' and contains(.,'%s')]" % subject)
-
-
 def reply_subject(context):
     e = find_element_by_css_selector(context, '#reply-subject')
     return e.text
diff --git a/service/test/functional/features/steps/login.py b/service/test/functional/features/steps/login.py
index 432186f5..7360adb7 100644
--- a/service/test/functional/features/steps/login.py
+++ b/service/test/functional/features/steps/login.py
@@ -24,7 +24,7 @@ from common import (
 @given('I am logged in Pixelated')
 def login_user(context):
     login_page(context)
-    enter_credentials(context, 'username', 'password')
+    enter_credentials(context)
     click_login(context)
     see_interstitial(context)
     _see_inbox(context)
@@ -40,10 +40,10 @@ def _see_inbox(context):
     find_element_by_css_selector(context, '#compose', timeout=40)
 
 
-@when(u'I enter {username} and {password} as credentials')
-def enter_credentials(context, username, password):
+@when(u'I enter username and password as credentials')
+def enter_credentials(context):
     fill_by_css_selector(context, 'input[name="username"]', context.username)
-    fill_by_css_selector(context, 'input[name="password"]', password)
+    fill_by_css_selector(context, 'input[name="password"]', 'password')
 
 
 @when(u'I click on the login button')
@@ -56,12 +56,13 @@ def see_interstitial(context):
     find_element_by_css_selector(context, 'section#hive-section')
 
 
+@then(u'I logout')
 @when(u'I logout')
 def click_logout(context):
     find_element_by_css_selector(context, '#logout-form div').click()
 
 
-@then(u'I logout from the header')
+@then(u'I logout from the header')  # noqa
 @when(u'I logout from the header')
 def click_logout(context):
     find_element_by_css_selector(context, 'button[name="logout"]').click()
diff --git a/service/test/functional/features/steps/mail_list.py b/service/test/functional/features/steps/mail_list.py
index 227aa9ed..2953c1af 100644
--- a/service/test/functional/features/steps/mail_list.py
+++ b/service/test/functional/features/steps/mail_list.py
@@ -17,13 +17,12 @@
 from behave import when, then, given
 from selenium.common.exceptions import TimeoutException
 
+from ..page_objects import InboxPage
 from common import (
     ImplicitWait,
-    execute_ignoring_staleness,
     find_element_by_id,
     find_element_by_css_selector,
     find_elements_by_css_selector,
-    mail_list_with_subject_exists,
     wait_for_condition,
     wait_for_loading_to_finish)
 
@@ -42,10 +41,6 @@ def open_current_mail(context):
     e.click()
 
 
-def get_first_email(context):
-    return find_element_by_css_selector(context, '.mail-list-entry__item')
-
-
 @then('I see that mail under the \'{tag}\' tag')
 def impl(context, tag):
     context.execute_steps("when I select the tag '%s'" % tag)
@@ -59,9 +54,12 @@ def impl(context):
 
 @when('I open the first mail in the mail list')
 def impl(context):
-    # it seems page is often still loading so staleness exceptions happen often
-    context.current_mail_id = 'mail-' + execute_ignoring_staleness(lambda: get_first_email(context).get_attribute('href').split('/')[-1])
-    execute_ignoring_staleness(lambda: get_first_email(context).click())
+    InboxPage(context).open_first_mail_in_the_mail_list()
+
+
+@when('I open the mail with the recovery code')
+def impl(context):
+    InboxPage(context).open_mail_with_the_recovery_code()
 
 
 @when('I open the first mail in the \'{tag}\'')
@@ -83,7 +81,7 @@ def impl(context):
 
 @then('the deleted mail is there')
 def impl(context):
-    mail_list_with_subject_exists(context, context.last_subject)
+    InboxPage(context).get_mail_with_subject(context.last_subject)
 
 
 @given('I have mails')
@@ -140,3 +138,9 @@ def impl(context):
 @then('I should not see any email')
 def impl(context):
     _wait_for_mail_list_to_be_empty(context)
+
+
+@then(u'I see the mail has the recovery code')
+def step_impl(context):
+    expected_body = 'Your code'
+    context.execute_steps(u"Then I see that the body has '%s'" % expected_body)
diff --git a/service/test/functional/features/steps/mail_view.py b/service/test/functional/features/steps/mail_view.py
index 65959b70..d10f86e7 100644
--- a/service/test/functional/features/steps/mail_view.py
+++ b/service/test/functional/features/steps/mail_view.py
@@ -17,6 +17,7 @@
 from behave import then, when
 from selenium.webdriver.common.keys import Keys
 
+from ..page_objects import InboxPage
 from common import (
     click_button,
     find_element_by_css_selector,
@@ -25,19 +26,17 @@ from common import (
     wait_until_button_is_visible)
 
 
-@then('I see that the subject reads \'{subject}\'')
-def impl(context, subject):
-    e = find_element_by_css_selector(context, '#mail-view .mail-read-view__header-subject')
-    assert e.text == subject
+@then('I see that the subject reads \'{expected_subject}\'')
+def impl(context, expected_subject):
+    actual_subject = find_element_by_css_selector(context, '#mail-view .mail-read-view__header-subject').text
+    assert expected_subject == actual_subject
 
 
 @then('I see that the body reads \'{expected_body}\'')
+@then('I see that the body has \'{expected_body}\'')
 def impl(context, expected_body):
-    find_element_by_css_selector(context, '#read-sandbox')
-    context.browser.switch_to_frame('read-sandbox')
-    e = find_element_by_css_selector(context, 'body')
-    assert e.text == expected_body
-    context.browser.switch_to_default_content()
+    actual_body = InboxPage(context).get_body_message()
+    assert expected_body in actual_body
 
 
 @then('that email has the \'{tag}\' tag')
diff --git a/service/test/functional/features/steps/utils.py b/service/test/functional/features/steps/utils.py
index dd9f0d87..9ac05928 100644
--- a/service/test/functional/features/steps/utils.py
+++ b/service/test/functional/features/steps/utils.py
@@ -14,10 +14,6 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 
-import uuid
-
-from behave import given, then, when
-
 from common import (
     element_should_have_content,
     fill_by_css_selector,
diff --git a/service/test/unit/resources/test_backup_account_resource.py b/service/test/unit/resources/test_backup_account_resource.py
index e5e2793a..220e3909 100644
--- a/service/test/unit/resources/test_backup_account_resource.py
+++ b/service/test/unit/resources/test_backup_account_resource.py
@@ -14,8 +14,6 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 
-import os
-
 from mock import MagicMock, patch
 from twisted.trial import unittest
 from twisted.web.test.requesthelper import DummyRequest
@@ -28,7 +26,10 @@ from test.unit.resources import DummySite
 class TestBackupAccountResource(unittest.TestCase):
     def setUp(self):
         self.services_factory = MagicMock()
-        self.resource = BackupAccountResource(self.services_factory, MagicMock())
+        self.authenticator = MagicMock()
+        self.leap_provider = MagicMock()
+        self.leap_provider.server_name = 'test.com'
+        self.resource = BackupAccountResource(self.services_factory, self.authenticator, self.leap_provider)
         self.web = DummySite(self.resource)
 
     def test_get(self):
@@ -59,7 +60,8 @@ class TestBackupAccountResource(unittest.TestCase):
                 self.resource._authenticator.bonafide_session,
                 self.resource.soledad(request),
                 self.resource._service(request, '_leap_session').smtp_config,
-                self.resource._get_backup_email(request))
+                self.resource._get_backup_email(request),
+                self.leap_provider.server_name)
             mock_account_recovery.update_recovery_code.assert_called()
 
         d.addCallback(assert_update_recovery_code_called)
diff --git a/service/test/unit/test_account_recovery.py b/service/test/unit/test_account_recovery.py
index f113169a..08298419 100644
--- a/service/test/unit/test_account_recovery.py
+++ b/service/test/unit/test_account_recovery.py
@@ -19,8 +19,8 @@ from twisted.internet import defer
 from twisted.trial import unittest
 from twisted.mail import smtp
 
-from mock import patch, Mock, MagicMock
-from mockito import mock, when, any as ANY
+from mock import patch, Mock
+from mockito import when, any as ANY
 
 from pixelated.account_recovery import AccountRecovery
 
@@ -32,36 +32,37 @@ class AccountRecoveryTest(unittest.TestCase):
         self.mock_soledad = Mock()
         self.mock_smtp_config = Mock()
         self.keymanager = Mock()
-        self.mock_smtp_config.remote_smtp_host = 'test.com'
+        self.mock_smtp_config.remote_smtp_host = 'localhost'
         self.mock_soledad.create_recovery_code.return_value = self.generated_code
         self.backup_email = 'test@test.com'
+        self.domain = 'test.com'
         self.account_recovery = AccountRecovery(
             self.mock_bonafide_session,
             self.mock_soledad,
             self.mock_smtp_config,
-            self.backup_email)
+            self.backup_email,
+            self.domain)
         self.mock_smtp = Mock()
 
     @defer.inlineCallbacks
     def test_update_recovery_code(self):
         when(self.account_recovery)._send_mail(ANY).thenReturn(defer.succeed(None))
-        response = yield self.account_recovery.update_recovery_code()
+        yield self.account_recovery.update_recovery_code()
         self.mock_bonafide_session.update_recovery_code.assert_called_once_with(self.generated_code)
 
     @defer.inlineCallbacks
     def test_send_recovery_code_by_email(self):
+        sender = 'team@{}'.format(self.domain)
         msg = MIMEText('Your code %s' % self.generated_code)
         msg['Subject'] = 'Recovery Code'
-        msg['From'] = 'team@pixelated-project.org'
+        msg['From'] = sender
         msg['To'] = self.backup_email
 
-        result = MagicMock()
-        deferred_sendmail = defer.succeed(result)
-        with patch.object(smtp, 'sendmail', return_value=deferred_sendmail) as mock_sendmail:
-            response = yield self.account_recovery._send_mail(self.generated_code, self.backup_email)
+        with patch.object(smtp, 'sendmail', return_value=defer.succeed(None)) as mock_sendmail:
+            yield self.account_recovery._send_mail(self.generated_code, self.backup_email)
 
-        mock_sendmail.assert_called_with(
-            'test.com',
-            'team@pixelated-project.org',
-            [self.backup_email],
-            msg.as_string())
+            mock_sendmail.assert_called_with(
+                self.mock_smtp_config.remote_smtp_host,
+                sender,
+                [self.backup_email],
+                msg.as_string())