Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[xenial] fix 2FA unit tests #4069

Merged
merged 2 commits into from
Jan 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions securedrop/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down
12 changes: 10 additions & 2 deletions securedrop/tests/functional/functional_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import errno
import mock
import os
import pytest
import signal
import socket
import time
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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()
Expand Down
13 changes: 0 additions & 13 deletions securedrop/tests/pageslayout/test_journalist.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
196 changes: 87 additions & 109 deletions securedrop/tests/test_2fa.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
3 changes: 2 additions & 1 deletion securedrop/tests/test_journalist_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 5 additions & 1 deletion securedrop/tests/utils/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down