Merge pull request #4069 from freedomofpress/ci-xenial-2fa-test

[xenial] fix 2FA unit tests

securedrop/tests/conftest.py

@@ -22,6 +22,7 @@
 
 from db import db
 from journalist_app import create_app as create_journalist_app
+import models
 from source_app import create_app as create_source_app
 import utils
 
@@ -56,6 +57,17 @@ def pytest_collection_modifyitems(config, items):
             item.add_marker(skip_page_layout)
 
 
+@pytest.fixture
+def hardening(request):
+    hardening = models.LOGIN_HARDENING
+
+    def finalizer():
+        models.LOGIN_HARDENING = hardening
+    request.addfinalizer(finalizer)
+    models.LOGIN_HARDENING = True
+    return None
+
+
 @pytest.fixture(scope='session')
 def setUpTearDown():
     _start_test_rqworker(original_config)

securedrop/tests/functional/functional_test.py

@@ -3,6 +3,7 @@
 import errno
 import mock
 import os
+import pytest
 import signal
 import socket
 import time
@@ -146,7 +147,14 @@ def start_journalist_server(app):
                 break
 
         if not hasattr(self, 'override_driver'):
-            self.driver = self._create_webdriver(self._prepare_webdriver())
+            try:
+                self.driver = self._create_webdriver(self._prepare_webdriver())
+            except WebDriverException as e:
+                # Exceptions during driver setup will result in the teardown not being called.
+                # Let's teardown and _then_ fail the test so that the patchers are cleaned
+                # up for the subsequent tests.
+                self.teardown()
+                pytest.fail(e)
 
         # Polls the DOM to wait for elements. To read more about why
         # this is necessary:
@@ -182,7 +190,7 @@ def key_available(filesystem_id):
     def teardown(self):
         self.patcher.stop()
         env.teardown()
-        if not hasattr(self, 'override_driver'):
+        if hasattr(self, 'driver') and not hasattr(self, 'override_driver'):
             self.driver.quit()
         self.source_process.terminate()
         self.journalist_process.terminate()

securedrop/tests/pageslayout/test_journalist.py

@@ -21,19 +21,6 @@
 import pytest
 import time
 
-import models
-
-
-@pytest.fixture
-def hardening(request):
-    hardening = models.LOGIN_HARDENING
-
-    def finalizer():
-        models.LOGIN_HARDENING = hardening
-    request.addfinalizer(finalizer)
-    models.LOGIN_HARDENING = True
-    return None
-
 
 @pytest.mark.pagelayout
 class TestJournalistLayout(

securedrop/tests/test_2fa.py

@@ -8,8 +8,6 @@
 from flask import url_for
 from pyotp import TOTP
 
-import models
-
 os.environ['SECUREDROP_ENV'] = 'test'  # noqa
 from models import Journalist, BadTokenException
 from utils import login_user
@@ -47,135 +45,115 @@ def totp_window():
     assert now < window_end
 
 
-def test_totp_reuse_protections(journalist_app, test_journo):
+def test_totp_reuse_protections(journalist_app, test_journo, hardening):
     """Ensure that logging in twice with the same TOTP token fails."""
-    original_hardening = models.LOGIN_HARDENING
-    try:
-        models.LOGIN_HARDENING = True
-
-        with totp_window():
-            token = TOTP(test_journo['otp_secret']).now()
+    with totp_window():
+        token = TOTP(test_journo['otp_secret']).now()
 
-            with journalist_app.test_client() as app:
-                login_user(app, test_journo)
-                resp = app.get(url_for('main.logout'), follow_redirects=True)
-                assert resp.status_code == 200
+        with journalist_app.test_client() as app:
+            login_user(app, test_journo)
+            resp = app.get(url_for('main.logout'), follow_redirects=True)
+            assert resp.status_code == 200
 
-            with journalist_app.test_client() as app:
-                resp = app.post(url_for('main.login'),
-                                data=dict(username=test_journo['username'],
-                                          password=test_journo['password'],
-                                          token=token))
-                assert resp.status_code == 200
-                text = resp.data.decode('utf-8')
-                assert "Login failed" in text
-    finally:
-        models.LOGIN_HARDENING = original_hardening
+        with journalist_app.test_client() as app:
+            resp = app.post(url_for('main.login'),
+                            data=dict(username=test_journo['username'],
+                                      password=test_journo['password'],
+                                      token=token))
+            assert resp.status_code == 200
+            text = resp.data.decode('utf-8')
+            assert "Login failed" in text
 
 
-def test_totp_reuse_protections2(journalist_app, test_journo):
+def test_totp_reuse_protections2(journalist_app, test_journo, hardening):
     """More granular than the preceeding test, we want to make sure the right
        exception is being raised in the right place.
     """
-    original_hardening = models.LOGIN_HARDENING
-    try:
-        models.LOGIN_HARDENING = True
 
-        with totp_window():
-            token = TOTP(test_journo['otp_secret']).now()
+    with totp_window():
+        token = TOTP(test_journo['otp_secret']).now()
 
-            with journalist_app.app_context():
+        with journalist_app.app_context():
+            Journalist.login(test_journo['username'],
+                             test_journo['password'],
+                             token)
+            with pytest.raises(BadTokenException):
                 Journalist.login(test_journo['username'],
                                  test_journo['password'],
                                  token)
-                with pytest.raises(BadTokenException):
-                    Journalist.login(test_journo['username'],
-                                     test_journo['password'],
-                                     token)
-    finally:
-        models.LOGIN_HARDENING = original_hardening
 
 
 def test_bad_token_fails_to_verify_on_admin_new_user_two_factor_page(
-        journalist_app, test_admin):
+        journalist_app, test_admin, hardening):
     '''Regression test for
        https://github.com/freedomofpress/securedrop/pull/1692
     '''
-    original_hardening = models.LOGIN_HARDENING
-    try:
-        models.LOGIN_HARDENING = True
-        invalid_token = u'000000'
-
-        with totp_window():
-            with journalist_app.test_client() as app:
-                login_user(app, test_admin)
-                # Submit the token once
-                with InstrumentedApp(journalist_app) as ins:
-                    resp = app.post(url_for('admin.new_user_two_factor',
-                                            uid=test_admin['id']),
-                                    data=dict(token=invalid_token))
-
-                    assert resp.status_code == 200
-                    ins.assert_message_flashed(
-                        'Could not verify token in two-factor authentication.',
-                        'error')
-
-            # last_token should be set to the token we just tried to use
-            with journalist_app.app_context():
-                admin = Journalist.query.get(test_admin['id'])
-                assert admin.last_token == invalid_token
-
-            with journalist_app.test_client() as app:
-                login_user(app, test_admin)
-                # Submit the same invalid token again
-                with InstrumentedApp(journalist_app) as ins:
-                    resp = app.post(url_for('admin.new_user_two_factor',
-                                            uid=test_admin['id']),
-                                    data=dict(token=invalid_token))
-                    ins.assert_message_flashed(
-                        'Could not verify token in two-factor authentication.',
-                        'error')
-    finally:
-        models.LOGIN_HARDENING = original_hardening
+
+    invalid_token = u'000000'
+
+    with totp_window():
+        with journalist_app.test_client() as app:
+            login_user(app, test_admin)
+            # Submit the token once
+            with InstrumentedApp(journalist_app) as ins:
+                resp = app.post(url_for('admin.new_user_two_factor',
+                                        uid=test_admin['id']),
+                                data=dict(token=invalid_token))
+
+                assert resp.status_code == 200
+                ins.assert_message_flashed(
+                    'Could not verify token in two-factor authentication.',
+                    'error')
+
+        # last_token should be set to the token we just tried to use
+        with journalist_app.app_context():
+            admin = Journalist.query.get(test_admin['id'])
+            assert admin.last_token == invalid_token
+
+        with journalist_app.test_client() as app:
+            login_user(app, test_admin)
+            # Submit the same invalid token again
+            with InstrumentedApp(journalist_app) as ins:
+                resp = app.post(url_for('admin.new_user_two_factor',
+                                        uid=test_admin['id']),
+                                data=dict(token=invalid_token))
+                ins.assert_message_flashed(
+                    'Could not verify token in two-factor authentication.',
+                    'error')
 
 
 def test_bad_token_fails_to_verify_on_new_user_two_factor_page(
-        journalist_app, test_journo):
+        journalist_app, test_journo, hardening):
     '''Regression test for
        https://github.com/freedomofpress/securedrop/pull/1692
     '''
-    original_hardening = models.LOGIN_HARDENING
-    try:
-        models.LOGIN_HARDENING = True
-        invalid_token = u'000000'
-
-        with totp_window():
-            with journalist_app.test_client() as app:
-                login_user(app, test_journo)
-                # Submit the token once
-                with InstrumentedApp(journalist_app) as ins:
-                    resp = app.post(url_for('account.new_two_factor'),
-                                    data=dict(token=invalid_token))
-
-                    assert resp.status_code == 200
-                    ins.assert_message_flashed(
-                        'Could not verify token in two-factor authentication.',
-                        'error')
-
-            # last_token should be set to the token we just tried to use
-            with journalist_app.app_context():
-                journo = Journalist.query.get(test_journo['id'])
-                assert journo.last_token == invalid_token
-
-            with journalist_app.test_client() as app:
-                login_user(app, test_journo)
-
-                # Submit the same invalid token again
-                with InstrumentedApp(journalist_app) as ins:
-                    resp = app.post(url_for('account.new_two_factor'),
-                                    data=dict(token=invalid_token))
-                    ins.assert_message_flashed(
-                        'Could not verify token in two-factor authentication.',
-                        'error')
-    finally:
-        models.LOGIN_HARDENING = original_hardening
+    invalid_token = u'000000'
+
+    with totp_window():
+        with journalist_app.test_client() as app:
+            login_user(app, test_journo)
+            # Submit the token once
+            with InstrumentedApp(journalist_app) as ins:
+                resp = app.post(url_for('account.new_two_factor'),
+                                data=dict(token=invalid_token))
+
+                assert resp.status_code == 200
+                ins.assert_message_flashed(
+                    'Could not verify token in two-factor authentication.',
+                    'error')
+
+        # last_token should be set to the token we just tried to use
+        with journalist_app.app_context():
+            journo = Journalist.query.get(test_journo['id'])
+            assert journo.last_token == invalid_token
+
+        with journalist_app.test_client() as app:
+            login_user(app, test_journo)
+
+            # Submit the same invalid token again
+            with InstrumentedApp(journalist_app) as ins:
+                resp = app.post(url_for('account.new_two_factor'),
+                                data=dict(token=invalid_token))
+                ins.assert_message_flashed(
+                    'Could not verify token in two-factor authentication.',
+                    'error')

securedrop/tests/test_journalist_api.py

@@ -63,7 +63,8 @@ def test_user_cannot_get_an_api_token_with_wrong_password(journalist_app,
 
 
 def test_user_cannot_get_an_api_token_with_wrong_2fa_token(journalist_app,
-                                                           test_journo):
+                                                           test_journo,
+                                                           hardening):
     with journalist_app.test_client() as app:
         response = app.post(url_for('api.get_token'),
                             data=json.dumps(

securedrop/tests/utils/env.py

@@ -60,7 +60,11 @@ def teardown():
         if t.is_alive() and not isinstance(t, threading._MainThread):
             t.join()
     db.session.remove()
-    shutil.rmtree(config.TEMP_DIR)
+    try:
+        shutil.rmtree(config.TEMP_DIR)
+    except OSError:
+        # Then check the directory was already deleted
+        assert not os.path.exists(config.TEMP_DIR)
     try:
         shutil.rmtree(config.SECUREDROP_DATA_ROOT)
         # safeguard for #844