From 7693ba8a189bbf4ad0138eab42d4d8b5c1e636af Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 5 Jan 2016 12:41:20 +0000 Subject: [PATCH 01/12] Update register view form and template. --- app/main/dao/users_dao.py | 2 ++ app/main/forms.py | 16 ++++++++++ app/main/views/register.py | 45 +++++++++++++++------------ app/templates/views/register.html | 21 +++---------- tests/app/main/test_validators.py | 9 ++---- tests/app/main/views/test_register.py | 6 ++-- 6 files changed, 54 insertions(+), 45 deletions(-) diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 326bafdc7..ea065e9d6 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -14,6 +14,8 @@ def insert_user(user): db.session.commit() +# TODO Would be better to have a generic get and update for user +# something that replicates the sql functionality. def get_user_by_id(id): return User.query.filter_by(id=id).first() diff --git a/app/main/forms.py b/app/main/forms.py index 28238e74e..1aceb8465 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -26,6 +26,12 @@ verify_code = '^\d{5}$' class RegisterUserForm(Form): + + def __init__(self, existing_email_addresses, existing_mobile_numbers, *args, **kwargs): + self.existing_emails = existing_email_addresses + self.existing_mobiles = existing_mobile_numbers + super(RegisterUserForm, self).__init__(*args, **kwargs) + name = StringField('Full name', validators=[DataRequired(message='Name can not be empty')]) email_address = StringField('Email address', validators=[ @@ -42,6 +48,16 @@ class RegisterUserForm(Form): Length(10, 255, message='Password must be at least 10 characters'), Blacklist(message='That password is blacklisted, too common')]) + def validate_email_address(self, field): + # Validate email address is unique. + if field.data in self.existing_emails: + raise ValidationError('Email address already exists') + + def validate_mobile_number(self, field): + # Validate mobile number is unique + if field.data in self.existing_mobiles: + raise ValidationError('Mobile number already exists') + class TwoFactorForm(Form): sms_code = StringField('sms code', validators=[DataRequired(message='Please enter your code'), diff --git a/app/main/views/register.py b/app/main/views/register.py index cd8830db9..43581721e 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -12,14 +12,19 @@ from app.main.views import send_sms_code, send_email_code from app.models import User -@main.route("/register", methods=['GET']) -def render_register(): - return render_template('views/register.html', form=RegisterUserForm()) - - -@main.route('/register', methods=['POST']) +# TODO how do we handle duplicate unverifed email addresses? +# malicious or otherwise. +@main.route('/register', methods=['GET', 'POST']) def process_register(): - form = RegisterUserForm() + try: + existing_emails, existing_mobiles = zip( + *[(x.email_address, x.mobile_number) for x in + users_dao.get_all_users()]) + except ValueError: + # Value error is raised if the db is empty. + existing_emails, existing_mobiles = [], [] + + form = RegisterUserForm(existing_emails, existing_mobiles) if form.validate_on_submit(): user = User(name=form.name.data, @@ -28,16 +33,16 @@ def process_register(): password=form.password.data, created_at=datetime.now(), role_id=1) - try: - users_dao.insert_user(user) - 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 - except AdminApiClientException as e: - return jsonify(admin_api_client_error=e.value) - except SQLAlchemyError: - return jsonify(database_error='encountered database error'), 400 - else: - return jsonify(form.errors), 400 - return redirect('/verify') + users_dao.insert_user(user) + # TODO possibly there should be some exception handling + # for sending sms and email codes. + # How do we report to the user there is a problem with + # sending codes apart from service unavailable? + # at the moment i believe http 500 is fine. + 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 + return redirect('/verify') + + return render_template('views/register.html', form=form) diff --git a/app/templates/views/register.html b/app/templates/views/register.html index 526344763..610516f3b 100644 --- a/app/templates/views/register.html +++ b/app/templates/views/register.html @@ -14,24 +14,13 @@ GOV.UK Notify | Create an account
{{ form.hidden_tag() }} -

- - {{ form.name(class="form-control-2-3", autocomplete="off") }}
-

-

- - {{ form.email_address(class="form-control-2-3", autocomplete="off") }}
+ + {{ render_field(form.name, class='form-control-2-3') }} + {{ render_field(form.email_address, class='form-control-2-3') }} Your email address must end in .gov.uk -

-

- - {{ form.mobile_number(class="form-control-1-4", autocomplete="off") }}
-

-

- - {{ form.password(class="form-control-1-4", autocomplete="off") }}
+ {{ render_field(form.mobile_number, class='form-control-2-3') }} + {{ render_field(form.password, class='form-control-2-3') }} Your password must have at least 10 characters -

diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py index a1cc52192..d85159f51 100644 --- a/tests/app/main/test_validators.py +++ b/tests/app/main/test_validators.py @@ -4,14 +4,11 @@ from app.main.forms import RegisterUserForm def test_should_raise_validation_error_for_password(notifications_admin): - form = RegisterUserForm() + form = RegisterUserForm([], []) form.name.data = 'test' form.email_address.data = 'teset@example.gov.uk' form.mobile_number.data = '+441231231231' form.password.data = 'password1234' - try: - form.validate() - fail() - except: - assert 'That password is blacklisted, too common' in form.errors['password'] + form.validate() + assert 'That password is blacklisted, too common' in form.errors['password'] diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index 8fffb9d8a..666e9f86d 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -30,7 +30,7 @@ def test_process_register_returns_400_when_mobile_number_is_invalid(notification 'mobile_number': 'not good', 'password': 'validPassword!'}) - assert response.status_code == 400 + assert response.status_code == 200 assert 'Please enter a +44 mobile number' in response.get_data(as_text=True) @@ -45,7 +45,7 @@ def test_should_return_400_when_email_is_not_gov_uk(notifications_admin, 'mobile_number': '+44123412345', 'password': 'validPassword!'}) - assert response.status_code == 400 + assert response.status_code == 200 assert 'Please enter a gov.uk email address' in response.get_data(as_text=True) @@ -73,5 +73,5 @@ def test_should_return_400_if_password_is_blacklisted(notifications_admin, notif 'mobile_number': '+44123412345', 'password': 'password1234'}) - response.status_code == 400 + response.status_code == 200 assert 'That password is blacklisted, too common' in response.get_data(as_text=True) From 1f520116f01c4b085af3df2464dce86fd4f93027 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 5 Jan 2016 14:30:06 +0000 Subject: [PATCH 02/12] Sign in view, form and template refactored. --- app/main/views/sign_in.py | 36 +++++++++++----------------- app/templates/views/signin.html | 11 ++------- tests/app/main/views/test_sign_in.py | 24 +++++++++---------- 3 files changed, 28 insertions(+), 43 deletions(-) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index a5bb4f6c3..3e29ab000 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -8,28 +8,20 @@ from app.main.forms import LoginForm from app.main.views import send_sms_code -@main.route("/sign-in", methods=(['GET'])) -def render_sign_in(): - return render_template('views/signin.html', form=LoginForm()) - - -@main.route('/sign-in', methods=(['POST'])) -def process_sign_in(): +@main.route('/sign-in', methods=(['GET', 'POST'])) +def sign_in(): form = LoginForm() if form.validate_on_submit(): user = users_dao.get_user_by_email(form.email_address.data) - if user is None: - return jsonify(authorization=False), 401 - if user.is_locked(): - return jsonify(locked_out=True), 401 - if not user.is_active(): - return jsonify(active_user=False), 401 - if check_hash(form.password.data, user.password): - send_sms_code(user.id, user.mobile_number) - session['user_id'] = user.id - else: - users_dao.increment_failed_login_count(user.id) - return jsonify(authorization=False), 401 - else: - return jsonify(form.errors), 400 - return redirect('/two-factor') + + 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 + return redirect('/two-factor') + else: + users_dao.increment_failed_login_count(user.id) + # Vague error message for login + form.password.errors.append('Username or password is incorrect') + + return render_template('views/signin.html', form=form) diff --git a/app/templates/views/signin.html b/app/templates/views/signin.html index 0c3d34531..859d843d8 100644 --- a/app/templates/views/signin.html +++ b/app/templates/views/signin.html @@ -14,18 +14,11 @@ Sign in {{ form.hidden_tag() }} -

- - {{ form.email_address(class="form-control-2-3", autocomplete="off") }}
-

-

- - {{ form.password(class="form-control-1-4", autocomplete="off") }}
-

+ {{ render_field(form.email_address, class='form-control-2-3') }} + {{ render_field(form.password, class='form-control-2-3') }}

Forgotten password?

-

diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index cc4778a37..bedf6554c 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -50,14 +50,14 @@ def test_should_return_locked_out_true_when_user_is_locked(notifications_admin, data={'email_address': 'valid@example.gov.uk', 'password': 'val1dPassw0rd!'}) - assert response.status_code == 401 - assert '"locked_out": true' in response.get_data(as_text=True) + assert response.status_code == 200 + assert 'Username or password is incorrect' in response.get_data(as_text=True) another_bad_attempt = notifications_admin.test_client().post('/sign-in', data={'email_address': 'valid@example.gov.uk', 'password': 'whatIsMyPassword!'}) - assert another_bad_attempt.status_code == 401 - assert '"locked_out": true' in response.get_data(as_text=True) + assert another_bad_attempt.status_code == 200 + assert 'Username or password is incorrect' in response.get_data(as_text=True) def test_should_return_active_user_is_false_if_user_is_inactive(notifications_admin, @@ -76,19 +76,19 @@ def test_should_return_active_user_is_false_if_user_is_inactive(notifications_ad data={'email_address': 'inactive_user@example.gov.uk', 'password': 'val1dPassw0rd!'}) - assert response.status_code == 401 - assert '"active_user": false' in response.get_data(as_text=True) + assert response.status_code == 200 + assert 'Username or password is incorrect' in response.get_data(as_text=True) -def test_should_return_401_when_user_does_not_exist(notifications_admin, notifications_admin_db, notify_db_session): +def test_should_return_200_when_user_does_not_exist(notifications_admin, notifications_admin_db, notify_db_session): response = notifications_admin.test_client().post('/sign-in', data={'email_address': 'does_not_exist@gov.uk', 'password': 'doesNotExist!'}) - - assert response.status_code == 401 + assert response.status_code == 200 + assert 'Username or password is incorrect' in response.get_data(as_text=True) -def test_should_return_400_when_user_is_not_active(notifications_admin, notifications_admin_db, notify_db_session): +def test_should_return_200_when_user_is_not_active(notifications_admin, notifications_admin_db, notify_db_session): user = User(email_address='PendingUser@example.gov.uk', password='val1dPassw0rd!', mobile_number='+441234123123', @@ -100,8 +100,8 @@ def test_should_return_400_when_user_is_not_active(notifications_admin, notifica response = notifications_admin.test_client().post('/sign-in', data={'email_address': 'PendingUser@example.gov.uk', 'password': 'val1dPassw0rd!'}) - assert response.status_code == 401 - assert '"active_user": false' in response.get_data(as_text=True) + assert response.status_code == 200 + assert 'Username or password is incorrect' in response.get_data(as_text=True) def _set_up_mocker(mocker): From 0ebacd6929d7bf56dd4180451f566254b2347e25 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 5 Jan 2016 17:08:50 +0000 Subject: [PATCH 03/12] Refactor for code_not_received, sign_in, two_factor and verify. --- app/main/forms.py | 9 +- app/main/views/code_not_received.py | 41 ++-- app/main/views/sign_in.py | 34 +-- app/main/views/two_factor.py | 18 +- app/main/views/verify.py | 34 +-- app/templates/views/email-not-received.html | 8 +- app/templates/views/register.html | 2 +- app/templates/views/signin.html | 2 +- app/templates/views/text-not-received.html | 7 +- app/templates/views/two-factor.html | 9 +- app/templates/views/verify-mobile.html | 2 +- app/templates/views/verify.html | 17 +- .../app/main/views/test_code_not_received.py | 202 ++++++++++-------- tests/app/main/views/test_sign_in.py | 68 +++--- tests/app/main/views/test_two_factor.py | 78 +++---- tests/app/main/views/test_verify.py | 126 ++++++----- 16 files changed, 336 insertions(+), 321 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 1aceb8465..b1c4158ad 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -92,9 +92,9 @@ class EmailNotReceivedForm(Form): class TextNotReceivedForm(Form): - mobile_number = StringField('Mobile phone number', - validators=[DataRequired(message='Please enter your mobile number'), - Regexp(regex=mobile_number, message='Please enter a +44 mobile number')]) + mobile_number = StringField('Mobile phone number', validators=[ + DataRequired(message='Please enter your mobile number'), + Regexp(regex=mobile_number, message='Please enter a +44 mobile number')]) class AddServiceForm(Form): @@ -102,7 +102,8 @@ class AddServiceForm(Form): self.service_names = service_names super(AddServiceForm, self).__init__(*args, **kwargs) - service_name = StringField(validators=[DataRequired(message='Please enter your service name')]) + service_name = StringField(validators=[ + DataRequired(message='Please enter your service name')]) def validate_service_name(self, a): if self.service_name.data in self.service_names: diff --git a/app/main/views/code_not_received.py b/app/main/views/code_not_received.py index 5fce56c3b..da12c2ad0 100644 --- a/app/main/views/code_not_received.py +++ b/app/main/views/code_not_received.py @@ -1,4 +1,5 @@ -from flask import render_template, redirect, jsonify, session +from flask import ( + render_template, redirect, jsonify, session, url_for) from app.main import main from app.main.dao import users_dao @@ -6,39 +7,28 @@ from app.main.forms import EmailNotReceivedForm, TextNotReceivedForm from app.main.views import send_sms_code, send_email_code -@main.route("/email-not-received", methods=['GET']) -def email_not_received(): - user = users_dao.get_user_by_id(session['user_id']) - return render_template('views/email-not-received.html', - form=EmailNotReceivedForm(email_address=user.email_address)) - - -@main.route('/email-not-received', methods=['POST']) +@main.route('/email-not-received', methods=['GET', 'POST']) def check_and_resend_email_code(): - form = EmailNotReceivedForm() + # TODO there needs to be a way to regenerate a session id + user = users_dao.get_user_by_id(session['user_id']) + form = EmailNotReceivedForm(email_address=user.email_address) if form.validate_on_submit(): - user = users_dao.get_user_by_id(session['user_id']) users_dao.update_email_address(id=user.id, email_address=form.email_address.data) send_email_code(user_id=user.id, email=user.email_address) - return redirect('/verify') - return jsonify(form.errors), 400 + return redirect(url_for('.verify')) + return render_template('views/email-not-received.html', form=form) -@main.route("/text-not-received", methods=['GET']) -def text_not_received(): - user = users_dao.get_user_by_id(session['user_id']) - return render_template('views/text-not-received.html', form=TextNotReceivedForm(mobile_number=user.mobile_number)) - - -@main.route('/text-not-received', methods=['POST']) +@main.route('/text-not-received', methods=['GET', 'POST']) def check_and_resend_text_code(): - form = TextNotReceivedForm() + # TODO there needs to be a way to regenerate a session id + user = users_dao.get_user_by_id(session['user_id']) + form = TextNotReceivedForm(mobile_number=user.mobile_number) if form.validate_on_submit(): - user = users_dao.get_user_by_id(session['user_id']) users_dao.update_mobile_number(id=user.id, mobile_number=form.mobile_number.data) send_sms_code(user_id=user.id, mobile_number=user.mobile_number) - return redirect('/verify') - return jsonify(form.errors), 400 + return redirect(url_for('.verify')) + return render_template('views/text-not-received.html', form=form) @main.route('/verification-not-received', methods=['GET']) @@ -48,6 +38,7 @@ 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']) send_sms_code(user.id, user.mobile_number) - return redirect('/two-factor') + return redirect(url_for('main.two_factor')) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 3e29ab000..7ebf66343 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -1,4 +1,5 @@ -from flask import render_template, redirect, jsonify +from flask import ( + render_template, redirect, jsonify, url_for) from flask import session from app.main import main @@ -10,18 +11,21 @@ from app.main.views import send_sms_code @main.route('/sign-in', methods=(['GET', 'POST'])) def sign_in(): - form = LoginForm() - if form.validate_on_submit(): - user = users_dao.get_user_by_email(form.email_address.data) + try: + form = LoginForm() + if form.validate_on_submit(): + user = users_dao.get_user_by_email(form.email_address.data) + 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 + return redirect(url_for('.two_factor')) + else: + users_dao.increment_failed_login_count(user.id) + # Vague error message for login + form.password.errors.append('Username or password is incorrect') - 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 - return redirect('/two-factor') - else: - users_dao.increment_failed_login_count(user.id) - # Vague error message for login - form.password.errors.append('Username or password is incorrect') - - return render_template('views/signin.html', form=form) + return render_template('views/signin.html', form=form) + except: + import traceback + traceback.print_exc() diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py index b06bb2029..3d7f9a480 100644 --- a/app/main/views/two_factor.py +++ b/app/main/views/two_factor.py @@ -1,4 +1,5 @@ -from flask import render_template, redirect, jsonify, session +from flask import ( + render_template, redirect, jsonify, session, url_for) from flask_login import login_user from app.main import main @@ -6,19 +7,14 @@ from app.main.dao import users_dao, verify_codes_dao from app.main.forms import TwoFactorForm -@main.route("/two-factor", methods=['GET']) -def render_two_factor(): - return render_template('views/two-factor.html', form=TwoFactorForm()) - - -@main.route('/two-factor', methods=['POST']) -def process_two_factor(): +@main.route('/two-factor', methods=['GET', 'POST']) +def two_factor(): form = TwoFactorForm() 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('/dashboard') - else: - return jsonify(form.errors), 400 + return redirect(url_for('.dashboard')) + + return render_template('views/two-factor.html', form=form) diff --git a/app/main/views/verify.py b/app/main/views/verify.py index 96ae0cb76..f738d5b0a 100644 --- a/app/main/views/verify.py +++ b/app/main/views/verify.py @@ -1,4 +1,5 @@ -from flask import render_template, redirect, jsonify, session +from flask import ( + render_template, redirect, jsonify, session, url_for) from flask_login import login_user from app.main import main @@ -6,20 +7,19 @@ from app.main.dao import users_dao, verify_codes_dao from app.main.forms import VerifyForm -@main.route('/verify', methods=['GET']) -def render_verify(): - return render_template('views/verify.html', form=VerifyForm()) - - -@main.route('/verify', methods=['POST']) -def process_verify(): - form = VerifyForm() - if form.validate_on_submit(): +@main.route('/verify', methods=['GET', 'POST']) +def verify(): + # TODO there needs to be a way to regenerate a session id + try: 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='email') - verify_codes_dao.use_code_for_user_and_type(user_id=user.id, code_type='sms') - users_dao.activate_user(user.id) - login_user(user) - return redirect('/add-service') - else: - return jsonify(form.errors), 400 + form = VerifyForm() + 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') + users_dao.activate_user(user.id) + login_user(user) + return redirect(url_for('.add_service')) + return render_template('views/verify.html', form=form) + except: + import traceback + traceback.print_exc() diff --git a/app/templates/views/email-not-received.html b/app/templates/views/email-not-received.html index 3b970bbf4..a379548dd 100644 --- a/app/templates/views/email-not-received.html +++ b/app/templates/views/email-not-received.html @@ -11,17 +11,13 @@ GOV.UK Notify

Check your email address

Check your email address is correct and then resend the confirmation code.

-

{{ form.hidden_tag() }} - - - {{ form.email_address(class="form-control-2-3", autocomplete="off") }}
- Your email address must end in .gov.uk + {{ render_field(form.email_address, class='form-control-2-3') }}

- +

diff --git a/app/templates/views/register.html b/app/templates/views/register.html index 610516f3b..b4f1c5ea6 100644 --- a/app/templates/views/register.html +++ b/app/templates/views/register.html @@ -10,7 +10,7 @@ GOV.UK Notify | Create an account

Create an account

-

If you've used GOV.UK Notify before, sign in to your account.

+

If you've used GOV.UK Notify before, sign in to your account.

{{ form.hidden_tag() }} diff --git a/app/templates/views/signin.html b/app/templates/views/signin.html index 859d843d8..036a34476 100644 --- a/app/templates/views/signin.html +++ b/app/templates/views/signin.html @@ -20,7 +20,7 @@ Sign in Forgotten password?

- +

diff --git a/app/templates/views/text-not-received.html b/app/templates/views/text-not-received.html index e63b7cb5c..e3562e907 100644 --- a/app/templates/views/text-not-received.html +++ b/app/templates/views/text-not-received.html @@ -14,12 +14,9 @@ GOV.UK Notify
{{ form.hidden_tag() }} + {{ render_field(form.mobile_number, class='form-control-2-3') }}

- - {{ form.mobile_number(class="form-control-1-4", autocomplete="off") }}
-

-

- +

diff --git a/app/templates/views/two-factor.html b/app/templates/views/two-factor.html index 8e3274706..091e4be6a 100644 --- a/app/templates/views/two-factor.html +++ b/app/templates/views/two-factor.html @@ -15,13 +15,10 @@ GOV.UK Notify | Text verification
{{ form.hidden_tag() }} + {{ render_field(form.sms_code, class='form-control-1-4') }} + I haven't received a text

-
- {{ form.sms_code(class="form-control-1-4", autocomplete="off") }}
- I haven't received a text -

-

- +

diff --git a/app/templates/views/verify-mobile.html b/app/templates/views/verify-mobile.html index ae66bbecf..e26f83592 100644 --- a/app/templates/views/verify-mobile.html +++ b/app/templates/views/verify-mobile.html @@ -15,7 +15,7 @@ GOV.UK Notify | Confirm mobile number

diff --git a/app/templates/views/verify.html b/app/templates/views/verify.html index 44ef74737..e12db20ac 100644 --- a/app/templates/views/verify.html +++ b/app/templates/views/verify.html @@ -14,19 +14,12 @@ GOV.UK Notify | Confirm email address and mobile number

{{ form.hidden_tag() }} + {{ render_field(form.email_code, class='form-control-1-4') }} + I haven't received an email + {{ render_field(form.sms_code, class='form-control-1-4') }} + I haven't received a text

- - {{ form.email_code(class="form-control-1-4", autocomplete="off") }}
- I haven't received an email -

-

- - {{ form.sms_code(class="form-control-1-4", autocomplete="off") }}
- I haven't received a text -

- -

- +

diff --git a/tests/app/main/views/test_code_not_received.py b/tests/app/main/views/test_code_not_received.py index c9b7ad2c9..d334c018f 100644 --- a/tests/app/main/views/test_code_not_received.py +++ b/tests/app/main/views/test_code_not_received.py @@ -1,151 +1,163 @@ from app.main.dao import verify_codes_dao, users_dao from tests.app.main import create_test_user +from flask import url_for def test_should_render_email_code_not_received_template_and_populate_email_address(notifications_admin, notifications_admin_db, - notify_db_session): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - user = create_test_user('pending') - session['user_id'] = user.id - response = client.get('/email-not-received') - assert response.status_code == 200 - assert 'Check your email address is correct and then resend the confirmation code' \ - in response.get_data(as_text=True) - assert 'value="test@user.gov.uk"' in response.get_data(as_text=True) + notify_db_session, + mocker): + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + _set_up_mocker(mocker) + user = create_test_user('pending') + session['user_id'] = user.id + 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' \ + in response.get_data(as_text=True) + assert 'value="test@user.gov.uk"' in response.get_data(as_text=True) def test_should_check_and_resend_email_code_redirect_to_verify(notifications_admin, notifications_admin_db, notify_db_session, mocker): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - _set_up_mocker(mocker) - user = create_test_user('pending') - session['user_id'] = user.id - verify_codes_dao.add_code(user.id, code='12345', code_type='email') - response = client.post('/email-not-received', - data={'email_address': 'test@user.gov.uk'}) - assert response.status_code == 302 - assert response.location == 'http://localhost/verify' + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + _set_up_mocker(mocker) + user = create_test_user('pending') + session['user_id'] = user.id + 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'}) + assert response.status_code == 302 + assert response.location == url_for('main.verify', _external=True) def test_should_render_text_code_not_received_template(notifications_admin, notifications_admin_db, notify_db_session, mocker): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - _set_up_mocker(mocker) - user = create_test_user('pending') - session['user_id'] = user.id - verify_codes_dao.add_code(user.id, code='12345', code_type='sms') - response = client.get('/text-not-received') - assert response.status_code == 200 - assert 'Check your mobile phone number is correct and then resend the confirmation code.' \ - in response.get_data(as_text=True) - assert 'value="+441234123412"' + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + _set_up_mocker(mocker) + user = create_test_user('pending') + session['user_id'] = user.id + 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 + assert 'Check your mobile phone number is correct and then resend the confirmation code.' \ + in response.get_data(as_text=True) + assert 'value="+441234123412"' def test_should_check_and_redirect_to_verify(notifications_admin, notifications_admin_db, notify_db_session, mocker): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - _set_up_mocker(mocker) - user = create_test_user('pending') - session['user_id'] = user.id - verify_codes_dao.add_code(user.id, code='12345', code_type='sms') - response = client.post('/text-not-received', - data={'mobile_number': '+441234123412'}) - assert response.status_code == 302 - assert response.location == 'http://localhost/verify' + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + _set_up_mocker(mocker) + user = create_test_user('pending') + session['user_id'] = user.id + 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'}) + assert response.status_code == 302 + assert response.location == url_for('main.verify', _external=True) def test_should_update_email_address_resend_code(notifications_admin, notifications_admin_db, notify_db_session, mocker): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - _set_up_mocker(mocker) - user = create_test_user('pending') - session['user_id'] = user.id - verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='email') - response = client.post('/email-not-received', - data={'email_address': 'new@address.gov.uk'}) - assert response.status_code == 302 - assert response.location == 'http://localhost/verify' - updated_user = users_dao.get_user_by_id(user.id) - assert updated_user.email_address == 'new@address.gov.uk' + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + _set_up_mocker(mocker) + user = create_test_user('pending') + session['user_id'] = user.id + 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'}) + assert response.status_code == 302 + assert response.location == url_for('main.verify', _external=True) + updated_user = users_dao.get_user_by_id(user.id) + assert updated_user.email_address == 'new@address.gov.uk' def test_should_update_mobile_number_resend_code(notifications_admin, notifications_admin_db, notify_db_session, mocker): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - _set_up_mocker(mocker) - user = create_test_user('pending') - session['user_id'] = user.id - verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') - response = client.post('/text-not-received', - data={'mobile_number': '+443456789012'}) - assert response.status_code == 302 - assert response.location == 'http://localhost/verify' - updated_user = users_dao.get_user_by_id(user.id) - assert updated_user.mobile_number == '+443456789012' + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + _set_up_mocker(mocker) + user = create_test_user('pending') + session['user_id'] = user.id + 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'}) + assert response.status_code == 302 + assert response.location == url_for('main.verify', _external=True) + updated_user = users_dao.get_user_by_id(user.id) + assert updated_user.mobile_number == '+443456789012' def test_should_render_verification_code_not_received(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - user = create_test_user('active') - session['user_id'] = user.id - response = client.get('/verification-not-received') - assert response.status_code == 200 - assert 'Resend verification code' in response.get_data(as_text=True) - assert 'If you no longer have access to the phone with the number you registered for this service, ' \ - 'speak to your service manager to reset the number.' in response.get_data(as_text=True) + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user('active') + session['user_id'] = user.id + 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) + assert 'If you no longer have access to the phone with the number you registered for this service, ' \ + 'speak to your service manager to reset the number.' in response.get_data(as_text=True) def test_check_and_redirect_to_two_factor(notifications_admin, notifications_admin_db, notify_db_session, mocker): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - user = create_test_user('active') - session['user_id'] = user.id - _set_up_mocker(mocker) - response = client.get('/send-new-code') - assert response.status_code == 302 - assert response.location == 'http://localhost/two-factor' + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user('active') + session['user_id'] = user.id + _set_up_mocker(mocker) + response = client.get(url_for('main.check_and_resend_verification_code')) + assert response.status_code == 302 + assert response.location == url_for('main.two_factor', _external=True) def test_should_create_new_code_for_user(notifications_admin, notifications_admin_db, notify_db_session, mocker): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - user = create_test_user('active') - session['user_id'] = user.id - verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') - _set_up_mocker(mocker) - response = client.get('/send-new-code') - assert response.status_code == 302 - assert response.location == 'http://localhost/two-factor' - codes = verify_codes_dao.get_codes(user_id=user.id, code_type='sms') - assert len(codes) == 2 - for x in ([used.code_used for used in codes]): - assert x is False + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user('active') + session['user_id'] = user.id + 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')) + assert response.status_code == 302 + assert response.location == url_for('main.two_factor', _external=True) + codes = verify_codes_dao.get_codes(user_id=user.id, code_type='sms') + assert len(codes) == 2 + for x in ([used.code_used for used in codes]): + assert x is False def _set_up_mocker(mocker): diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index bedf6554c..116df29fa 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -2,10 +2,12 @@ from datetime import datetime from app.main.dao import users_dao from app.models import User +from flask import url_for def test_render_sign_in_returns_sign_in_template(notifications_admin): - response = notifications_admin.test_client().get('/sign-in') + with notifications_admin.test_request_context(): + response = notifications_admin.test_client().get(url_for('main.sign_in')) assert response.status_code == 200 assert 'Sign in' in response.get_data(as_text=True) assert 'Email address' in response.get_data(as_text=True) @@ -23,9 +25,11 @@ def test_process_sign_in_return_2fa_template(notifications_admin, notifications_ role_id=1, state='active') users_dao.insert_user(user) - response = notifications_admin.test_client().post('/sign-in', - data={'email_address': 'valid@example.gov.uk', - 'password': 'val1dPassw0rd!'}) + with notifications_admin.test_request_context(): + response = notifications_admin.test_client().post( + url_for('main.sign_in'), data={ + 'email_address': 'valid@example.gov.uk', + 'password': 'val1dPassw0rd!'}) assert response.status_code == 302 assert response.location == 'http://localhost/two-factor' @@ -41,23 +45,27 @@ def test_should_return_locked_out_true_when_user_is_locked(notifications_admin, role_id=1, state='active') users_dao.insert_user(user) - for _ in range(10): - notifications_admin.test_client().post('/sign-in', - data={'email_address': 'valid@example.gov.uk', - 'password': 'whatIsMyPassword!'}) + with notifications_admin.test_request_context(): + for _ in range(10): + notifications_admin.test_client().post( + url_for('main.sign_in'), data={ + 'email_address': 'valid@example.gov.uk', + 'password': 'whatIsMyPassword!'}) - response = notifications_admin.test_client().post('/sign-in', - data={'email_address': 'valid@example.gov.uk', - 'password': 'val1dPassw0rd!'}) + response = notifications_admin.test_client().post( + url_for('main.sign_in'), data={ + 'email_address': 'valid@example.gov.uk', + 'password': 'val1dPassw0rd!'}) - assert response.status_code == 200 - assert 'Username or password is incorrect' in response.get_data(as_text=True) + assert response.status_code == 200 + assert 'Username or password is incorrect' in response.get_data(as_text=True) - another_bad_attempt = notifications_admin.test_client().post('/sign-in', - data={'email_address': 'valid@example.gov.uk', - 'password': 'whatIsMyPassword!'}) - assert another_bad_attempt.status_code == 200 - assert 'Username or password is incorrect' in response.get_data(as_text=True) + another_bad_attempt = notifications_admin.test_client().post( + url_for('main.sign_in'), data={ + 'email_address': 'valid@example.gov.uk', + 'password': 'whatIsMyPassword!'}) + assert another_bad_attempt.status_code == 200 + assert 'Username or password is incorrect' in response.get_data(as_text=True) def test_should_return_active_user_is_false_if_user_is_inactive(notifications_admin, @@ -72,18 +80,22 @@ def test_should_return_active_user_is_false_if_user_is_inactive(notifications_ad state='inactive') users_dao.insert_user(user) - response = notifications_admin.test_client().post('/sign-in', - data={'email_address': 'inactive_user@example.gov.uk', - 'password': 'val1dPassw0rd!'}) + with notifications_admin.test_request_context(): + response = notifications_admin.test_client().post( + url_for('main.sign_in'), data={ + 'email_address': 'inactive_user@example.gov.uk', + 'password': 'val1dPassw0rd!'}) assert response.status_code == 200 assert 'Username or password is incorrect' in response.get_data(as_text=True) def test_should_return_200_when_user_does_not_exist(notifications_admin, notifications_admin_db, notify_db_session): - response = notifications_admin.test_client().post('/sign-in', - data={'email_address': 'does_not_exist@gov.uk', - 'password': 'doesNotExist!'}) + with notifications_admin.test_request_context(): + response = notifications_admin.test_client().post( + url_for('main.sign_in'), data={ + 'email_address': 'does_not_exist@gov.uk', + 'password': 'doesNotExist!'}) assert response.status_code == 200 assert 'Username or password is incorrect' in response.get_data(as_text=True) @@ -97,9 +109,11 @@ def test_should_return_200_when_user_is_not_active(notifications_admin, notifica role_id=1, state='pending') users_dao.insert_user(user) - response = notifications_admin.test_client().post('/sign-in', - data={'email_address': 'PendingUser@example.gov.uk', - 'password': 'val1dPassw0rd!'}) + with notifications_admin.test_request_context(): + response = notifications_admin.test_client().post( + url_for('main.sign_in'), data={ + 'email_address': 'PendingUser@example.gov.uk', + 'password': 'val1dPassw0rd!'}) assert response.status_code == 200 assert 'Username or password is incorrect' in response.get_data(as_text=True) diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index 6f0a4bfe3..edcc67a3e 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -1,56 +1,60 @@ -from flask import json +from flask import json, url_for from app.main.dao import verify_codes_dao from tests.app.main import create_test_user def test_should_render_two_factor_page(notifications_admin, notifications_admin_db, notify_db_session): - response = notifications_admin.test_client().get('/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) + with notifications_admin.test_request_context(): + response = notifications_admin.test_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) def test_should_login_user_and_redirect_to_dashboard(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - user = create_test_user('active') - session['user_id'] = user.id - verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') - response = client.post('/two-factor', - data={'sms_code': '12345'}) + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user('active') + session['user_id'] = user.id + 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'}) - assert response.status_code == 302 - assert response.location == 'http://localhost/dashboard' + assert response.status_code == 302 + assert response.location == url_for('main.dashboard', _external=True) -def test_should_return_400_with_sms_code_error_when_sms_code_is_wrong(notifications_admin, +def test_should_return_200_with_sms_code_error_when_sms_code_is_wrong(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - user = create_test_user('active') - session['user_id'] = user.id - verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') - response = client.post('/two-factor', - data={'sms_code': '23456'}) - assert response.status_code == 400 - assert {'sms_code': ['Code does not match']} == json.loads(response.get_data(as_text=True)) + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user('active') + session['user_id'] = user.id + 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'}) + assert response.status_code == 200 + assert 'Code does not match' in response.get_data(as_text=True) def test_should_login_user_when_multiple_valid_codes_exist(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - user = create_test_user('active') - session['user_id'] = user.id - 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') - assert len(verify_codes_dao.get_codes(user_id=user.id, code_type='sms')) == 3 - response = client.post('/two-factor', - data={'sms_code': '23456'}) - assert response.status_code == 302 - codes = verify_codes_dao.get_codes(user_id=user.id, code_type='sms') - # query will only return codes where code_used == False - assert len(codes) == 0 + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user('active') + session['user_id'] = user.id + 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') + assert len(verify_codes_dao.get_codes(user_id=user.id, code_type='sms')) == 3 + response = client.post(url_for('main.two_factor'), + data={'sms_code': '23456'}) + assert response.status_code == 302 + codes = verify_codes_dao.get_codes(user_id=user.id, code_type='sms') + # query will only return codes where code_used == False + assert len(codes) == 0 diff --git a/tests/app/main/views/test_verify.py b/tests/app/main/views/test_verify.py index 9398dff47..729bacfa8 100644 --- a/tests/app/main/views/test_verify.py +++ b/tests/app/main/views/test_verify.py @@ -1,79 +1,89 @@ -from flask import json +from flask import json, url_for from app.main.dao import users_dao, verify_codes_dao from tests.app.main import create_test_user def test_should_return_verify_template(notifications_admin, notifications_admin_db, notify_db_session): - response = notifications_admin.test_client().get('/verify') - assert response.status_code == 200 - assert 'Activate your account' in response.get_data(as_text=True) + with notifications_admin.test_request_context(): + 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_id'] = user.id + response = client.get(url_for('main.verify')) + assert response.status_code == 200 + assert ( + "We've sent you confirmation codes by email and text message." + " You need to enter both codes here.") in response.get_data(as_text=True) def test_should_redirect_to_add_service_when_code_are_correct(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - user = create_test_user('pending') - session['user_id'] = user.id - 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('/verify', - data={'sms_code': '12345', - 'email_code': '23456'}) - assert response.status_code == 302 - assert response.location == 'http://localhost/add-service' + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user('pending') + session['user_id'] = user.id + 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'), + data={'sms_code': '12345', + 'email_code': '23456'}) + assert response.status_code == 302 + assert response.location == url_for('main.add_service', _external=True) def test_should_activate_user_after_verify(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - user = create_test_user('pending') - session['user_id'] = user.id - 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('/verify', - data={'sms_code': '12345', - 'email_code': '23456'}) + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user('pending') + session['user_id'] = user.id + 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'), + data={'sms_code': '12345', + 'email_code': '23456'}) - after_verify = users_dao.get_user_by_id(user.id) - assert after_verify.state == 'active' + after_verify = users_dao.get_user_by_id(user.id) + assert after_verify.state == 'active' -def test_should_return_400_when_codes_are_wrong(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - user = create_test_user('pending') - session['user_id'] = user.id - 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('/verify', - data={'sms_code': '12345', - 'email_code': '23456'}) - assert response.status_code == 400 - expected = {'sms_code': ['Code must be 5 digits', 'Code does not match'], - 'email_code': ['Code must be 5 digits', 'Code does not match']} - errors = json.loads(response.get_data(as_text=True)) - assert len(errors) == 2 - assert set(errors) == set(expected) +def test_should_return_200_when_codes_are_wrong(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('pending') + session['user_id'] = user.id + 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'}) + assert response.status_code == 200 + resp_data = response.get_data(as_text=True) + assert resp_data.count('Code does not match') == 2 def test_should_mark_all_codes_as_used_when_many_codes_exist(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_client() as client: - with client.session_transaction() as session: - user = create_test_user('pending') - session['user_id'] = user.id - 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') - code4 = verify_codes_dao.add_code(user_id=user.id, code='23412', code_type='email') - response = client.post('/verify', - data={'sms_code': '23345', - 'email_code': '23412'}) - assert response.status_code == 302 - assert verify_codes_dao.get_code_by_id(code1).code_used is True - assert verify_codes_dao.get_code_by_id(code2).code_used is True - assert verify_codes_dao.get_code_by_id(code3).code_used is True - assert verify_codes_dao.get_code_by_id(code4).code_used is True + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user('pending') + session['user_id'] = user.id + 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') + code4 = verify_codes_dao.add_code(user_id=user.id, code='23412', code_type='email') + response = client.post(url_for('main.verify'), + data={'sms_code': '23345', + 'email_code': '23412'}) + assert response.status_code == 302 + assert verify_codes_dao.get_code_by_id(code1).code_used is True + assert verify_codes_dao.get_code_by_id(code2).code_used is True + assert verify_codes_dao.get_code_by_id(code3).code_used is True + assert verify_codes_dao.get_code_by_id(code4).code_used is True From 4fcc4efea2691f6c33b999b0c633691bdfcb3a9b Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 5 Jan 2016 17:11:44 +0000 Subject: [PATCH 04/12] Small comments. --- app/main/views/sign_in.py | 30 +++++++++++++----------------- app/main/views/sms.py | 1 + 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 7ebf66343..768cd2664 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -11,21 +11,17 @@ from app.main.views import send_sms_code @main.route('/sign-in', methods=(['GET', 'POST'])) def sign_in(): - try: - form = LoginForm() - if form.validate_on_submit(): - user = users_dao.get_user_by_email(form.email_address.data) - 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 - return redirect(url_for('.two_factor')) - else: - users_dao.increment_failed_login_count(user.id) - # Vague error message for login - form.password.errors.append('Username or password is incorrect') + form = LoginForm() + if form.validate_on_submit(): + user = users_dao.get_user_by_email(form.email_address.data) + 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 + return redirect(url_for('.two_factor')) + else: + users_dao.increment_failed_login_count(user.id) + # Vague error message for login + form.password.errors.append('Username or password is incorrect') - return render_template('views/signin.html', form=form) - except: - import traceback - traceback.print_exc() + return render_template('views/signin.html', form=form) diff --git a/app/main/views/sms.py b/app/main/views/sms.py index 891502fc5..50574064e 100644 --- a/app/main/views/sms.py +++ b/app/main/views/sms.py @@ -3,6 +3,7 @@ from flask_login import login_required from app.main import main +# TODO move this to the templates directory message_templates = [ { 'name': 'Reminder', From 9d7c3566aaaab288458544b47edf9a168980a454 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 6 Jan 2016 09:54:10 +0000 Subject: [PATCH 05/12] Removed validation check for unique mobile number on the system. --- app/main/forms.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index d058995f0..0ee3d5827 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -55,8 +55,10 @@ class RegisterUserForm(Form): def validate_mobile_number(self, field): # Validate mobile number is unique - if field.data in self.existing_mobiles: - raise ValidationError('Mobile number already exists') + # Code to re-added later + # if field.data in self.existing_mobiles: + # raise ValidationError('Mobile number already exists') + pass class TwoFactorForm(Form): From f2732eed1413565fd7bcad42d90b0ad92a2eca3a Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 6 Jan 2016 10:22:36 +0000 Subject: [PATCH 06/12] Added back in span element in form. --- app/templates/views/email-not-received.html | 1 + 1 file changed, 1 insertion(+) diff --git a/app/templates/views/email-not-received.html b/app/templates/views/email-not-received.html index a379548dd..e6f28df80 100644 --- a/app/templates/views/email-not-received.html +++ b/app/templates/views/email-not-received.html @@ -16,6 +16,7 @@ GOV.UK Notify
{{ form.hidden_tag() }} {{ render_field(form.email_address, class='form-control-2-3') }} + Your email address must end in .gov.uk

From 4a431636036a1c80c5665055f5689f788113df3a Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 6 Jan 2016 11:10:23 +0000 Subject: [PATCH 07/12] Removed redundancy of removed included macro. --- app/main/views/sign_in.py | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 768cd2664..7ebf66343 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -11,17 +11,21 @@ from app.main.views import send_sms_code @main.route('/sign-in', methods=(['GET', 'POST'])) def sign_in(): - form = LoginForm() - if form.validate_on_submit(): - user = users_dao.get_user_by_email(form.email_address.data) - 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 - return redirect(url_for('.two_factor')) - else: - users_dao.increment_failed_login_count(user.id) - # Vague error message for login - form.password.errors.append('Username or password is incorrect') + try: + form = LoginForm() + if form.validate_on_submit(): + user = users_dao.get_user_by_email(form.email_address.data) + 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 + return redirect(url_for('.two_factor')) + else: + users_dao.increment_failed_login_count(user.id) + # Vague error message for login + form.password.errors.append('Username or password is incorrect') - return render_template('views/signin.html', form=form) + return render_template('views/signin.html', form=form) + except: + import traceback + traceback.print_exc() From 6fc39d1814e11563a7593d849849e9d12a421123 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 6 Jan 2016 11:37:59 +0000 Subject: [PATCH 08/12] Add in macro import to our template, not govuk_template --- app/templates/admin_template.html | 1 + 1 file changed, 1 insertion(+) diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index 76947a435..ee6a5cc9a 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -1,3 +1,4 @@ +{%- from "components/form-field.html" import render_field %} {% extends "govuk_template.html" %} {% block head %} From 34bf03bae20617af38bda68255013ce0970b4ec3 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 6 Jan 2016 12:36:04 +0000 Subject: [PATCH 09/12] Readded git submodules. --- app/assets/govuk_elements | 1 + app/assets/govuk_frontend_toolkit | 1 + 2 files changed, 2 insertions(+) create mode 160000 app/assets/govuk_elements create mode 160000 app/assets/govuk_frontend_toolkit diff --git a/app/assets/govuk_elements b/app/assets/govuk_elements new file mode 160000 index 000000000..816c3745b --- /dev/null +++ b/app/assets/govuk_elements @@ -0,0 +1 @@ +Subproject commit 816c3745b0c03fe90e6e6bdef403bb7ea3022a21 diff --git a/app/assets/govuk_frontend_toolkit b/app/assets/govuk_frontend_toolkit new file mode 160000 index 000000000..993229873 --- /dev/null +++ b/app/assets/govuk_frontend_toolkit @@ -0,0 +1 @@ +Subproject commit 993229873c55747afd5b1cabc74c530589f00e91 From 416fa30929c957b5c78b99f2dec78ba6d9733193 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 6 Jan 2016 12:57:09 +0000 Subject: [PATCH 10/12] =?UTF-8?q?Make=20the=20=C2=A3=20string=20a=20unicod?= =?UTF-8?q?e=20string?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/main/views/jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index cdb24ddf7..fbcfe03f9 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -62,7 +62,7 @@ def showjob(): message for message in messages if message['status'] == 'Failed' ]) }, - cost='£0.00', + cost=u'£0.00', uploaded_file_name='dispatch_20151114.csv', uploaded_file_time=now, template_used='Test message 1', From 03d81e5b47247761d62de734be101cd066db3cf4 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 6 Jan 2016 13:22:15 +0000 Subject: [PATCH 11/12] =?UTF-8?q?Unicoded=20the=20=C2=A3=20sign=20and=20th?= =?UTF-8?q?e=20'=20needed=20to=20render=20the=20job=20page.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/main/views/jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index fbcfe03f9..112afd86a 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -66,7 +66,7 @@ def showjob(): uploaded_file_name='dispatch_20151114.csv', uploaded_file_time=now, template_used='Test message 1', - flash_message='We’ve started sending your messages' + flash_message=u'We’ve started sending your messages' ) From 52df795743cbeb03a3ee563787746d7e0a6fe758 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 6 Jan 2016 17:00:01 +0000 Subject: [PATCH 12/12] Review fix. --- app/assets/govuk_elements | 2 +- app/assets/govuk_frontend_toolkit | 2 +- app/main/forms.py | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/assets/govuk_elements b/app/assets/govuk_elements index 816c3745b..9e44a8d74 160000 --- a/app/assets/govuk_elements +++ b/app/assets/govuk_elements @@ -1 +1 @@ -Subproject commit 816c3745b0c03fe90e6e6bdef403bb7ea3022a21 +Subproject commit 9e44a8d74cbbab9d32fd006bfba05814c8b0dc83 diff --git a/app/assets/govuk_frontend_toolkit b/app/assets/govuk_frontend_toolkit index 993229873..26b6a60d0 160000 --- a/app/assets/govuk_frontend_toolkit +++ b/app/assets/govuk_frontend_toolkit @@ -1 +1 @@ -Subproject commit 993229873c55747afd5b1cabc74c530589f00e91 +Subproject commit 26b6a60d076c0e717ee4316c21df8938acbf42ff diff --git a/app/main/forms.py b/app/main/forms.py index 0ee3d5827..86d3a6da4 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -62,7 +62,7 @@ class RegisterUserForm(Form): class TwoFactorForm(Form): - sms_code = StringField('sms code', validators=[DataRequired(message='Please enter your code'), + 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): @@ -114,6 +114,7 @@ class AddServiceForm(Form): 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')