From 59052297149607feaef1671dd29e0b61fca2958b Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 6 Jan 2016 16:40:38 +0000 Subject: [PATCH 1/4] Logout functionality and test added. --- app/main/__init__.py | 3 +- app/main/views/dashboard.py | 4 ++- app/main/views/sign_out.py | 13 ++++++++ app/templates/admin_template.html | 5 ++++ tests/app/main/views/test_sign_out.py | 43 +++++++++++++++++++++++++++ tests/conftest.py | 17 +++++++++++ 6 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 app/main/views/sign_out.py create mode 100644 tests/app/main/views/test_sign_out.py diff --git a/app/main/__init__.py b/app/main/__init__.py index 7b77dcc48..93354853b 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -4,5 +4,6 @@ main = Blueprint('main', __name__) from app.main.views import ( - index, sign_in, register, two_factor, verify, sms, add_service, code_not_received, jobs, dashboard, templates + index, sign_in, sign_out, register, two_factor, verify, sms, add_service, + code_not_received, jobs, dashboard, templates ) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 0e0275f45..d5e7e2355 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -1,11 +1,13 @@ from flask import render_template - +from flask_login import login_required from app.main import main + from ._jobs import jobs @main.route("/dashboard") +@login_required def dashboard(): return render_template( 'views/dashboard.html', diff --git a/app/main/views/sign_out.py b/app/main/views/sign_out.py new file mode 100644 index 000000000..8f7b97cf4 --- /dev/null +++ b/app/main/views/sign_out.py @@ -0,0 +1,13 @@ +from flask import (render_template, redirect, url_for) +from flask import session +from flask_login import login_required, logout_user + +from app.main import main + + +@main.route('/sign-out', methods=(['GET'])) +@login_required +def sign_out(): + logout_user() + return redirect(url_for('main.render_sign_in')) + diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index 76947a435..1abfb3c9d 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -28,6 +28,11 @@
BETA
+ {% if current_user.is_authenticated %} +
+ Sign out +
+ {% endif %} {% endblock %} diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py new file mode 100644 index 000000000..57925081a --- /dev/null +++ b/tests/app/main/views/test_sign_out.py @@ -0,0 +1,43 @@ +from datetime import datetime +from flask import url_for +from app.main.dao import users_dao +from app.models import User + +from .test_sign_in import _set_up_mocker + + +def test_render_sign_out_redirects_to_sign_in(notifications_admin): + with notifications_admin.test_request_context(): + response = notifications_admin.test_client().get( + url_for('main.sign_out')) + assert response.status_code == 302 + assert response.location == url_for( + 'main.render_sign_in', _external=True, next=url_for('main.sign_out')) + + +def test_sign_out_user(notifications_admin, + notifications_admin_db, + notify_db_session, + mocker): + with notifications_admin.test_request_context(): + _set_up_mocker(mocker) + email = 'valid@example.gov.uk' + password = 'val1dPassw0rd!' + user = User(email_address=email, + password=password, + mobile_number='+441234123123', + name='valid', + created_at=datetime.now(), + role_id=1, + state='active') + users_dao.insert_user(user) + with notifications_admin.test_client() as client: + client.login(user) + # Check we are logged in + response = client.get( + url_for('main.dashboard')) + assert response.status_code == 200 + response = client.get(url_for('main.sign_out')) + assert response.status_code == 302 + assert response.location == url_for( + 'main.render_sign_in', _external=True) diff --git a/tests/conftest.py b/tests/conftest.py index 33e3ea9a3..cd7d3cd09 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,12 +3,28 @@ import os import pytest from alembic.command import upgrade from alembic.config import Config +from flask import url_for from flask.ext.migrate import Migrate, MigrateCommand from flask.ext.script import Manager +from flask_login import login_user from sqlalchemy.schema import MetaData +from flask.testing import FlaskClient +from app.main.dao import verify_codes_dao from app import create_app, db +class TestClient(FlaskClient): + def login(self, user): + # Skipping authentication here and just log them in + with self.session_transaction() as session: + session['user_id'] = user.id + verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') + response = self.post('/two-factor', + data={'sms_code': '12345'}) + + def logout(self, user): + self.get(url_for("main.logout")) + @pytest.fixture(scope='session') def notifications_admin(request): @@ -21,6 +37,7 @@ def notifications_admin(request): ctx.pop() request.addfinalizer(teardown) + app.test_client_class = TestClient return app From 79c15ec9cfe956e7939a8b62ee1f04d9c1630a7f Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 6 Jan 2016 16:51:35 +0000 Subject: [PATCH 2/4] Code checks and all tests passing. --- app/main/views/sign_out.py | 1 - tests/app/main/views/test_dashboard.py | 21 ++++++++++++++++----- tests/conftest.py | 5 +++-- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/main/views/sign_out.py b/app/main/views/sign_out.py index 8f7b97cf4..0466d0af9 100644 --- a/app/main/views/sign_out.py +++ b/app/main/views/sign_out.py @@ -10,4 +10,3 @@ from app.main import main def sign_out(): logout_user() return redirect(url_for('main.render_sign_in')) - diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 2b134b39d..d32002ccf 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -1,6 +1,17 @@ -def test_should_show_recent_jobs_on_dashboard(notifications_admin): - response = notifications_admin.test_client().get('/dashboard') +from tests.app.main import create_test_user +from flask import url_for - assert response.status_code == 200 - assert 'Test message 1' in response.get_data(as_text=True) - assert 'Asdfgg' in response.get_data(as_text=True) + +def test_should_show_recent_jobs_on_dashboard(notifications_admin, + notifications_admin_db, + notify_db_session): + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user('active') + client.login(user) + response = client.get(url_for('main.dashboard')) + + assert response.status_code == 200 + assert 'Test message 1' in response.get_data(as_text=True) + assert 'Asdfgg' in response.get_data(as_text=True) diff --git a/tests/conftest.py b/tests/conftest.py index cd7d3cd09..a98083297 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,14 +13,15 @@ from app.main.dao import verify_codes_dao from app import create_app, db + class TestClient(FlaskClient): def login(self, user): # Skipping authentication here and just log them in with self.session_transaction() as session: session['user_id'] = user.id verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') - response = self.post('/two-factor', - data={'sms_code': '12345'}) + response = self.post( + url_for('main.process_two_factor'), data={'sms_code': '12345'}) def logout(self, user): self.get(url_for("main.logout")) From 10c2978f856d9927ddb32bd11757e3e889e70f9f Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 6 Jan 2016 17:17:02 +0000 Subject: [PATCH 3/4] Merge with master and test fix. --- app/__init__.py | 2 +- app/main/views/sign_out.py | 4 ++-- tests/app/main/views/test_sign_out.py | 4 ++-- tests/conftest.py | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 498230fe3..1a25d59d9 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -33,7 +33,7 @@ def create_app(config_name): init_csrf(application) login_manager.init_app(application) - login_manager.login_view = 'main.render_sign_in' + login_manager.login_view = 'main.sign_in' from app.main import main as main_blueprint application.register_blueprint(main_blueprint) diff --git a/app/main/views/sign_out.py b/app/main/views/sign_out.py index 0466d0af9..d734916c8 100644 --- a/app/main/views/sign_out.py +++ b/app/main/views/sign_out.py @@ -1,6 +1,6 @@ from flask import (render_template, redirect, url_for) from flask import session -from flask_login import login_required, logout_user +from flask_login import (login_required, logout_user) from app.main import main @@ -9,4 +9,4 @@ from app.main import main @login_required def sign_out(): logout_user() - return redirect(url_for('main.render_sign_in')) + return redirect(url_for('main.sign_in')) diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py index 57925081a..05fc8804d 100644 --- a/tests/app/main/views/test_sign_out.py +++ b/tests/app/main/views/test_sign_out.py @@ -12,7 +12,7 @@ def test_render_sign_out_redirects_to_sign_in(notifications_admin): url_for('main.sign_out')) assert response.status_code == 302 assert response.location == url_for( - 'main.render_sign_in', _external=True, next=url_for('main.sign_out')) + 'main.sign_in', _external=True, next=url_for('main.sign_out')) def test_sign_out_user(notifications_admin, @@ -40,4 +40,4 @@ def test_sign_out_user(notifications_admin, response = client.get(url_for('main.sign_out')) assert response.status_code == 302 assert response.location == url_for( - 'main.render_sign_in', _external=True) + 'main.sign_in', _external=True) diff --git a/tests/conftest.py b/tests/conftest.py index a98083297..ba1d68258 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -21,7 +21,7 @@ class TestClient(FlaskClient): session['user_id'] = user.id verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') response = self.post( - url_for('main.process_two_factor'), data={'sms_code': '12345'}) + url_for('main.two_factor'), data={'sms_code': '12345'}) def logout(self, user): self.get(url_for("main.logout")) From 7001d8261df214a26bb9f815e979a7cfff71ca71 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Thu, 7 Jan 2016 12:43:10 +0000 Subject: [PATCH 4/4] Fix for security hole with setting session['user_id'] before second factor of authentication has been authorised. --- app/main/dao/verify_codes_dao.py | 4 +- app/main/forms.py | 57 +++++++++---------- app/main/validators.py | 28 +++++++++ app/main/views/code_not_received.py | 6 +- app/main/views/register.py | 2 +- app/main/views/sign_in.py | 2 +- app/main/views/two_factor.py | 6 +- app/main/views/verify.py | 5 +- app/templates/admin_template.html | 26 ++++++--- tests/app/main/test_two_factor_form.py | 20 +++---- tests/app/main/test_verify_form.py | 25 ++++---- tests/app/main/views/test_add_service.py | 52 ++++++++--------- .../app/main/views/test_code_not_received.py | 18 +++--- tests/app/main/views/test_dashboard.py | 3 +- tests/app/main/views/test_two_factor.py | 14 +++-- tests/app/main/views/test_verify.py | 11 ++-- tests/conftest.py | 2 +- 17 files changed, 162 insertions(+), 119 deletions(-) diff --git a/app/main/dao/verify_codes_dao.py b/app/main/dao/verify_codes_dao.py index 6f3e21d9d..1a7fdcc55 100644 --- a/app/main/dao/verify_codes_dao.py +++ b/app/main/dao/verify_codes_dao.py @@ -16,7 +16,9 @@ def add_code(user_id, code, code_type): return code.id -def get_codes(user_id, code_type): +def get_codes(user_id, code_type=None): + if not code_type: + return VerifyCodes.query.filter_by(user_id=user_id, code_used=False).all() return VerifyCodes.query.filter_by(user_id=user_id, code_type=code_type, code_used=False).all() diff --git a/app/main/forms.py b/app/main/forms.py index 86d3a6da4..5b7230477 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1,12 +1,11 @@ from datetime import datetime -from flask import session from flask_wtf import Form from wtforms import StringField, PasswordField, ValidationError from wtforms.validators import DataRequired, Email, Length, Regexp from app.main.dao import verify_codes_dao from app.main.encryption import check_hash -from app.main.validators import Blacklist +from app.main.validators import Blacklist, ValidateUserCodes class LoginForm(Form): @@ -62,26 +61,40 @@ class RegisterUserForm(Form): class TwoFactorForm(Form): - sms_code = StringField('sms code', validators=[DataRequired(message='Enter verification code'), - Regexp(regex=verify_code, message='Code must be 5 digits')]) - def validate_sms_code(self, a): - return validate_codes(self.sms_code, 'sms') + def __init__(self, user_codes, *args, **kwargs): + ''' + Keyword arguments: + user_codes -- List of user code objects which have the fields + (code_type, expiry_datetime, code) + ''' + self.user_codes = user_codes + super(TwoFactorForm, self).__init__(*args, **kwargs) + + sms_code = StringField('sms code', validators=[DataRequired(message='Enter verification code'), + Regexp(regex=verify_code, message='Code must be 5 digits'), + ValidateUserCodes(code_type='sms')]) class VerifyForm(Form): + + def __init__(self, user_codes, *args, **kwargs): + ''' + Keyword arguments: + user_codes -- List of user code objects which have the fields + (code_type, expiry_datetime, code) + ''' + self.user_codes = user_codes + super(VerifyForm, self).__init__(*args, **kwargs) + sms_code = StringField("Text message confirmation code", validators=[DataRequired(message='SMS code can not be empty'), - Regexp(regex=verify_code, message='Code must be 5 digits')]) + Regexp(regex=verify_code, message='Code must be 5 digits'), + ValidateUserCodes(code_type='sms')]) email_code = StringField("Email confirmation code", validators=[DataRequired(message='Email code can not be empty'), - Regexp(regex=verify_code, message='Code must be 5 digits')]) - - def validate_email_code(self, a): - return validate_codes(self.email_code, 'email') - - def validate_sms_code(self, a): - return validate_codes(self.sms_code, 'sms') + Regexp(regex=verify_code, message='Code must be 5 digits'), + ValidateUserCodes(code_type='email')]) class EmailNotReceivedForm(Form): @@ -110,19 +123,3 @@ class AddServiceForm(Form): def validate_service_name(self, a): if self.service_name.data in self.service_names: raise ValidationError('Service name already exists') - - -def validate_codes(field, code_type): - codes = verify_codes_dao.get_codes(user_id=session['user_id'], code_type=code_type) - # TODO need to remove this manual logging. - print('validate_codes for user_id: {} are {}'.format(session['user_id'], codes)) - if not [code for code in codes if validate_code(field, code)]: - raise ValidationError('Code does not match') - - -def validate_code(field, code): - if field.data and check_hash(field.data, code.code): - if code.expiry_datetime <= datetime.now(): - raise ValidationError('Code has expired') - else: - return code.code diff --git a/app/main/validators.py b/app/main/validators.py index 2638bd5e2..940e5b317 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -1,4 +1,6 @@ from wtforms import ValidationError +from datetime import datetime +from app.main.encryption import check_hash class Blacklist(object): @@ -10,3 +12,29 @@ class Blacklist(object): def __call__(self, form, field): if field.data in ['password1234', 'passw0rd1234']: raise ValidationError(self.message) + + +class ValidateUserCodes(object): + def __init__(self, + expiry_msg='Code has expired', + invalid_msg='Code does not match', + code_type=None): + self.expiry_msg = expiry_msg + self.invalid_msg = invalid_msg + self.code_type = code_type + + def __call__(self, form, field): + # TODO would be great to do this sql query but + # not couple those parts of the code. + user_codes = getattr(form, 'user_codes', []) + valid_code = False + for code in user_codes: + if check_hash(field.data, code.code) and self.code_type == code.code_type: + if code.expiry_datetime <= datetime.now(): + raise ValidationError(self.expiry_msg) + else: + # Valid code + valid_code = True + break + if not valid_code: + raise ValidationError(self.invalid_msg) diff --git a/app/main/views/code_not_received.py b/app/main/views/code_not_received.py index da12c2ad0..4f0962653 100644 --- a/app/main/views/code_not_received.py +++ b/app/main/views/code_not_received.py @@ -10,7 +10,7 @@ from app.main.views import send_sms_code, send_email_code @main.route('/email-not-received', methods=['GET', 'POST']) def check_and_resend_email_code(): # TODO there needs to be a way to regenerate a session id - user = users_dao.get_user_by_id(session['user_id']) + user = users_dao.get_user_by_email(session['user_email']) form = EmailNotReceivedForm(email_address=user.email_address) if form.validate_on_submit(): users_dao.update_email_address(id=user.id, email_address=form.email_address.data) @@ -22,7 +22,7 @@ def check_and_resend_email_code(): @main.route('/text-not-received', methods=['GET', 'POST']) def check_and_resend_text_code(): # TODO there needs to be a way to regenerate a session id - user = users_dao.get_user_by_id(session['user_id']) + user = users_dao.get_user_by_email(session['user_email']) form = TextNotReceivedForm(mobile_number=user.mobile_number) if form.validate_on_submit(): users_dao.update_mobile_number(id=user.id, mobile_number=form.mobile_number.data) @@ -39,6 +39,6 @@ def verification_code_not_received(): @main.route('/send-new-code', methods=['GET']) def check_and_resend_verification_code(): # TODO there needs to be a way to generate a new session id - user = users_dao.get_user_by_id(session['user_id']) + user = users_dao.get_user_by_email(session['user_email']) send_sms_code(user.id, user.mobile_number) return redirect(url_for('main.two_factor')) diff --git a/app/main/views/register.py b/app/main/views/register.py index 43581721e..2e1c7f309 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -42,7 +42,7 @@ def process_register(): send_sms_code(user_id=user.id, mobile_number=form.mobile_number.data) send_email_code(user_id=user.id, email=form.email_address.data) session['expiry_date'] = str(datetime.now() + timedelta(hours=1)) - session['user_id'] = user.id + session['user_email'] = user.email_address return redirect('/verify') return render_template('views/register.html', form=form) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 7ebf66343..0f3be0c2d 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -18,7 +18,7 @@ def sign_in(): if user: if not user.is_locked() and user.is_active() and check_hash(form.password.data, user.password): send_sms_code(user.id, user.mobile_number) - session['user_id'] = user.id + session['user_email'] = user.email_address return redirect(url_for('.two_factor')) else: users_dao.increment_failed_login_count(user.id) diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py index 3edcdfc0a..22b27fe08 100644 --- a/app/main/views/two_factor.py +++ b/app/main/views/two_factor.py @@ -11,10 +11,12 @@ from app.main.forms import TwoFactorForm @main.route('/two-factor', methods=['GET', 'POST']) def two_factor(): - form = TwoFactorForm() + # TODO handle user_email not in session + user = users_dao.get_user_by_email(session['user_email']) + codes = verify_codes_dao.get_codes(user.id) + form = TwoFactorForm(codes) if form.validate_on_submit(): - user = users_dao.get_user_by_id(session['user_id']) verify_codes_dao.use_code_for_user_and_type(user_id=user.id, code_type='sms') login_user(user) return redirect(url_for('.dashboard')) diff --git a/app/main/views/verify.py b/app/main/views/verify.py index c982d6e79..bb00cfd52 100644 --- a/app/main/views/verify.py +++ b/app/main/views/verify.py @@ -12,8 +12,9 @@ from app.main.forms import VerifyForm def verify(): # TODO there needs to be a way to regenerate a session id # or handle gracefully. - user = users_dao.get_user_by_id(session['user_id']) - form = VerifyForm() + user = users_dao.get_user_by_email(session['user_email']) + codes = verify_codes_dao.get_codes(user.id) + form = VerifyForm(codes) if form.validate_on_submit(): verify_codes_dao.use_code_for_user_and_type(user_id=user.id, code_type='email') verify_codes_dao.use_code_for_user_and_type(user_id=user.id, code_type='sms') diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index 1e2e9f5d3..63cff92ce 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -26,16 +26,24 @@ {% block inside_header %} -
- BETA -
- {% if current_user.is_authenticated %} -
- Sign out -
- {% endif %} {% endblock %} - +{% block header_class %}with-proposition{% endblock %} +{% block proposition_header %} +
+
+
+ BETA +
+ {% if current_user.is_authenticated() %} + + {% endif %} +
+
+{% endblock %} {% set global_header_text = "GOV.UK Notify" %} diff --git a/tests/app/main/test_two_factor_form.py b/tests/app/main/test_two_factor_form.py index ebc414b00..be91a5675 100644 --- a/tests/app/main/test_two_factor_form.py +++ b/tests/app/main/test_two_factor_form.py @@ -9,8 +9,8 @@ def test_form_is_valid_returns_no_errors(notifications_admin, notifications_admi with notifications_admin.test_request_context(method='POST', data={'sms_code': '12345'}) as req: user = set_up_test_data() - req.session['user_id'] = user.id - form = TwoFactorForm(req.request.form) + codes = verify_codes_dao.get_codes(user.id) + form = TwoFactorForm(codes) assert form.validate() is True assert len(form.errors) == 0 @@ -19,8 +19,8 @@ def test_returns_errors_when_code_is_too_short(notifications_admin, notification with notifications_admin.test_request_context(method='POST', data={'sms_code': '145'}) as req: user = set_up_test_data() - req.session['user_id'] = user.id - form = TwoFactorForm(req.request.form) + codes = verify_codes_dao.get_codes(user.id) + form = TwoFactorForm(codes) assert form.validate() is False assert len(form.errors) == 1 assert set(form.errors) == set({'sms_code': ['Code must be 5 digits', 'Code does not match']}) @@ -30,8 +30,8 @@ def test_returns_errors_when_code_is_missing(notifications_admin, notifications_ with notifications_admin.test_request_context(method='POST', data={}) as req: user = set_up_test_data() - req.session['user_id'] = user.id - form = TwoFactorForm(req.request.form) + codes = verify_codes_dao.get_codes(user.id) + form = TwoFactorForm(codes) assert form.validate() is False assert len(form.errors) == 1 assert set(form.errors) == set({'sms_code': ['Code must not be empty']}) @@ -41,8 +41,8 @@ def test_returns_errors_when_code_contains_letters(notifications_admin, notifica with notifications_admin.test_request_context(method='POST', data={'sms_code': 'asdfg'}) as req: user = set_up_test_data() - req.session['user_id'] = user.id - form = TwoFactorForm(req.request.form) + codes = verify_codes_dao.get_codes(user.id) + form = TwoFactorForm(codes) assert form.validate() is False assert len(form.errors) == 1 assert set(form.errors) == set({'sms_code': ['Code must be 5 digits', 'Code does not match']}) @@ -52,12 +52,12 @@ def test_should_return_errors_when_code_is_expired(notifications_admin, notifica with notifications_admin.test_request_context(method='POST', data={'sms_code': '23456'}) as req: user = create_test_user('active') - req.session['user_id'] = user.id verify_codes_dao.add_code_with_expiry(user_id=user.id, code='23456', code_type='sms', expiry=datetime.now() + timedelta(hours=-2)) - form = TwoFactorForm(req.request.form) + codes = verify_codes_dao.get_codes(user.id) + form = TwoFactorForm(codes) assert form.validate() is False errors = form.errors assert len(errors) == 1 diff --git a/tests/app/main/test_verify_form.py b/tests/app/main/test_verify_form.py index a72d18fd4..6a48394cc 100644 --- a/tests/app/main/test_verify_form.py +++ b/tests/app/main/test_verify_form.py @@ -8,8 +8,8 @@ def test_form_should_have_error_when_code_is_not_valid(notifications_admin, noti with notifications_admin.test_request_context(method='POST', data={'sms_code': '12345aa', 'email_code': 'abcde'}) as req: user = set_up_test_data() - req.session['user_id'] = user.id - form = VerifyForm(req.request.form) + codes = verify_codes_dao.get_codes(user.id) + form = VerifyForm(codes) assert form.validate() is False errors = form.errors assert len(errors) == 2 @@ -23,8 +23,8 @@ def test_should_return_errors_when_code_missing(notifications_admin, notificatio with notifications_admin.test_request_context(method='POST', data={}) as req: user = set_up_test_data() - req.session['user_id'] = user.id - form = VerifyForm(req.request.form) + codes = verify_codes_dao.get_codes(user.id) + form = VerifyForm(codes) assert form.validate() is False errors = form.errors expected = {'sms_code': ['SMS code can not be empty'], @@ -37,8 +37,8 @@ def test_should_return_errors_when_code_is_too_short(notifications_admin, notifi with notifications_admin.test_request_context(method='POST', data={'sms_code': '123', 'email_code': '123'}) as req: user = set_up_test_data() - req.session['user_id'] = user.id - form = VerifyForm(req.request.form) + codes = verify_codes_dao.get_codes(user.id) + form = VerifyForm(codes) assert form.validate() is False errors = form.errors expected = {'sms_code': ['Code must be 5 digits', 'Code does not match'], @@ -51,8 +51,8 @@ def test_should_return_errors_when_code_does_not_match(notifications_admin, noti with notifications_admin.test_request_context(method='POST', data={'sms_code': '34567', 'email_code': '34567'}) as req: user = set_up_test_data() - req.session['user_id'] = user.id - form = VerifyForm(req.request.form) + codes = verify_codes_dao.get_codes(user.id) + form = VerifyForm(codes) assert form.validate() is False errors = form.errors expected = {'sms_code': ['Code does not match'], @@ -66,7 +66,6 @@ def test_should_return_errors_when_code_is_expired(notifications_admin, notifica data={'sms_code': '23456', 'email_code': '23456'}) as req: user = create_test_user('pending') - req.session['user_id'] = user.id verify_codes_dao.add_code_with_expiry(user_id=user.id, code='23456', code_type='sms', @@ -76,8 +75,8 @@ def test_should_return_errors_when_code_is_expired(notifications_admin, notifica code='23456', code_type='email', expiry=datetime.now() + timedelta(hours=-2)) - req.session['user_id'] = user.id - form = VerifyForm(req.request.form) + codes = verify_codes_dao.get_codes(user.id) + form = VerifyForm(codes) assert form.validate() is False errors = form.errors expected = {'sms_code': ['Code has expired'], @@ -97,8 +96,8 @@ def test_should_return_valid_form_when_many_codes_exist(notifications_admin, verify_codes_dao.add_code(user_id=user.id, code='23456', code_type='sms') verify_codes_dao.add_code(user_id=user.id, code='60456', code_type='email') verify_codes_dao.add_code(user_id=user.id, code='27856', code_type='sms') - req.session['user_id'] = user.id - form = VerifyForm(req.request.form) + codes = verify_codes_dao.get_codes(user.id) + form = VerifyForm(codes) assert form.validate() is True diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 6163950ce..5f199ac63 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -3,40 +3,40 @@ from tests.app.main import create_test_user def test_get_should_render_add_service_template(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: user = create_test_user('active') - session['user_id'] = user.id - verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') - client.post('/two-factor', data={'sms_code': '12345'}) - response = client.get('/add-service') - assert response.status_code == 200 - assert 'Set up notifications for your service' in response.get_data(as_text=True) + client.login(user) + verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') + client.post('/two-factor', data={'sms_code': '12345'}) + response = client.get('/add-service') + assert response.status_code == 200 + assert 'Set up notifications for your service' in response.get_data(as_text=True) def test_should_add_service_and_redirect_to_next_page(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: user = create_test_user('active') - session['user_id'] = user.id - verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') - client.post('/two-factor', data={'sms_code': '12345'}) - response = client.post('/add-service', data={'service_name': 'testing the post'}) - assert response.status_code == 302 - assert response.location == 'http://localhost/dashboard' - saved_service = services_dao.find_service_by_service_name('testing the post') - assert saved_service is not None + client.login(user) + verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') + client.post('/two-factor', data={'sms_code': '12345'}) + response = client.post('/add-service', data={'service_name': 'testing the post'}) + assert response.status_code == 302 + assert response.location == 'http://localhost/dashboard' + saved_service = services_dao.find_service_by_service_name('testing the post') + assert saved_service is not None def test_should_return_form_errors_when_service_name_is_empty(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: user = create_test_user('active') - session['user_id'] = user.id - verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') - client.post('/two-factor', data={'sms_code': '12345'}) - response = client.post('/add-service', data={}) - assert response.status_code == 200 - assert 'Please enter your service name' in response.get_data(as_text=True) + client.login(user) + verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') + client.post('/two-factor', data={'sms_code': '12345'}) + response = client.post('/add-service', data={}) + assert response.status_code == 200 + assert 'Please enter your service name' in response.get_data(as_text=True) diff --git a/tests/app/main/views/test_code_not_received.py b/tests/app/main/views/test_code_not_received.py index d334c018f..367d138b4 100644 --- a/tests/app/main/views/test_code_not_received.py +++ b/tests/app/main/views/test_code_not_received.py @@ -12,7 +12,7 @@ def test_should_render_email_code_not_received_template_and_populate_email_addre with client.session_transaction() as session: _set_up_mocker(mocker) user = create_test_user('pending') - session['user_id'] = user.id + session['user_email'] = user.email_address response = client.get(url_for('main.check_and_resend_email_code')) assert response.status_code == 200 assert 'Check your email address is correct and then resend the confirmation code' \ @@ -29,7 +29,7 @@ def test_should_check_and_resend_email_code_redirect_to_verify(notifications_adm with client.session_transaction() as session: _set_up_mocker(mocker) user = create_test_user('pending') - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user.id, code='12345', code_type='email') response = client.post(url_for('main.check_and_resend_email_code'), data={'email_address': 'test@user.gov.uk'}) @@ -46,7 +46,7 @@ def test_should_render_text_code_not_received_template(notifications_admin, with client.session_transaction() as session: _set_up_mocker(mocker) user = create_test_user('pending') - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user.id, code='12345', code_type='sms') response = client.get(url_for('main.check_and_resend_text_code')) assert response.status_code == 200 @@ -64,7 +64,7 @@ def test_should_check_and_redirect_to_verify(notifications_admin, with client.session_transaction() as session: _set_up_mocker(mocker) user = create_test_user('pending') - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user.id, code='12345', code_type='sms') response = client.post(url_for('main.check_and_resend_text_code'), data={'mobile_number': '+441234123412'}) @@ -81,7 +81,7 @@ def test_should_update_email_address_resend_code(notifications_admin, with client.session_transaction() as session: _set_up_mocker(mocker) user = create_test_user('pending') - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='email') response = client.post(url_for('main.check_and_resend_email_code'), data={'email_address': 'new@address.gov.uk'}) @@ -100,7 +100,7 @@ def test_should_update_mobile_number_resend_code(notifications_admin, with client.session_transaction() as session: _set_up_mocker(mocker) user = create_test_user('pending') - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') response = client.post(url_for('main.check_and_resend_text_code'), data={'mobile_number': '+443456789012'}) @@ -117,7 +117,7 @@ def test_should_render_verification_code_not_received(notifications_admin, with notifications_admin.test_client() as client: with client.session_transaction() as session: user = create_test_user('active') - session['user_id'] = user.id + session['user_email'] = user.email_address response = client.get(url_for('main.verification_code_not_received')) assert response.status_code == 200 assert 'Resend verification code' in response.get_data(as_text=True) @@ -133,7 +133,7 @@ def test_check_and_redirect_to_two_factor(notifications_admin, with notifications_admin.test_client() as client: with client.session_transaction() as session: user = create_test_user('active') - session['user_id'] = user.id + session['user_email'] = user.email_address _set_up_mocker(mocker) response = client.get(url_for('main.check_and_resend_verification_code')) assert response.status_code == 302 @@ -148,7 +148,7 @@ def test_should_create_new_code_for_user(notifications_admin, with notifications_admin.test_client() as client: with client.session_transaction() as session: user = create_test_user('active') - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') _set_up_mocker(mocker) response = client.get(url_for('main.check_and_resend_verification_code')) diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index d32002ccf..271a867a5 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -7,8 +7,7 @@ def test_should_show_recent_jobs_on_dashboard(notifications_admin, notify_db_session): with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: - with client.session_transaction() as session: - user = create_test_user('active') + user = create_test_user('active') client.login(user) response = client.get(url_for('main.dashboard')) diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index edcc67a3e..80c2c7244 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -6,7 +6,13 @@ from tests.app.main import create_test_user def test_should_render_two_factor_page(notifications_admin, notifications_admin_db, notify_db_session): with notifications_admin.test_request_context(): - response = notifications_admin.test_client().get(url_for('main.two_factor')) + with notifications_admin.test_client() as client: + # TODO this lives here until we work out how to + # reassign the session after it is lost mid register process + with client.session_transaction() as session: + user = create_test_user('pending') + session['user_email'] = user.email_address + response = client.get(url_for('main.two_factor')) assert response.status_code == 200 assert '''We've sent you a text message with a verification code.''' in response.get_data(as_text=True) @@ -16,7 +22,7 @@ def test_should_login_user_and_redirect_to_dashboard(notifications_admin, notifi with notifications_admin.test_client() as client: with client.session_transaction() as session: user = create_test_user('active') - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') response = client.post(url_for('main.two_factor'), data={'sms_code': '12345'}) @@ -32,7 +38,7 @@ def test_should_return_200_with_sms_code_error_when_sms_code_is_wrong(notificati with notifications_admin.test_client() as client: with client.session_transaction() as session: user = create_test_user('active') - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') response = client.post(url_for('main.two_factor'), data={'sms_code': '23456'}) @@ -47,7 +53,7 @@ def test_should_login_user_when_multiple_valid_codes_exist(notifications_admin, with notifications_admin.test_client() as client: with client.session_transaction() as session: user = create_test_user('active') - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user_id=user.id, code='23456', code_type='sms') verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') verify_codes_dao.add_code(user_id=user.id, code='34567', code_type='sms') diff --git a/tests/app/main/views/test_verify.py b/tests/app/main/views/test_verify.py index 729bacfa8..ab875fe7f 100644 --- a/tests/app/main/views/test_verify.py +++ b/tests/app/main/views/test_verify.py @@ -10,7 +10,7 @@ def test_should_return_verify_template(notifications_admin, notifications_admin_ # reassign the session after it is lost mid register process with client.session_transaction() as session: user = create_test_user('pending') - session['user_id'] = user.id + session['user_email'] = user.email_address response = client.get(url_for('main.verify')) assert response.status_code == 200 assert ( @@ -25,7 +25,7 @@ def test_should_redirect_to_add_service_when_code_are_correct(notifications_admi with notifications_admin.test_client() as client: with client.session_transaction() as session: user = create_test_user('pending') - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') verify_codes_dao.add_code(user_id=user.id, code='23456', code_type='email') response = client.post(url_for('main.verify'), @@ -40,7 +40,7 @@ def test_should_activate_user_after_verify(notifications_admin, notifications_ad with notifications_admin.test_client() as client: with client.session_transaction() as session: user = create_test_user('pending') - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') verify_codes_dao.add_code(user_id=user.id, code='23456', code_type='email') client.post(url_for('main.verify'), @@ -56,12 +56,13 @@ def test_should_return_200_when_codes_are_wrong(notifications_admin, notificatio with notifications_admin.test_client() as client: with client.session_transaction() as session: user = create_test_user('pending') - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user_id=user.id, code='23345', code_type='sms') verify_codes_dao.add_code(user_id=user.id, code='98456', code_type='email') response = client.post(url_for('main.verify'), data={'sms_code': '12345', 'email_code': '23456'}) + print(response.location) assert response.status_code == 200 resp_data = response.get_data(as_text=True) assert resp_data.count('Code does not match') == 2 @@ -74,7 +75,7 @@ def test_should_mark_all_codes_as_used_when_many_codes_exist(notifications_admin with notifications_admin.test_client() as client: with client.session_transaction() as session: user = create_test_user('pending') - session['user_id'] = user.id + session['user_email'] = user.email_address code1 = verify_codes_dao.add_code(user_id=user.id, code='23345', code_type='sms') code2 = verify_codes_dao.add_code(user_id=user.id, code='98456', code_type='email') code3 = verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') diff --git a/tests/conftest.py b/tests/conftest.py index ba1d68258..603320b47 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,7 +18,7 @@ class TestClient(FlaskClient): def login(self, user): # Skipping authentication here and just log them in with self.session_transaction() as session: - session['user_id'] = user.id + session['user_email'] = user.email_address verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') response = self.post( url_for('main.two_factor'), data={'sms_code': '12345'})