From 164bdad4f27cc33d2e6bf6d72384f231cce2f8d8 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Fri, 11 Mar 2016 16:36:15 +0000 Subject: [PATCH] Change new invite registration flow to only need sms for verification. This may change again soon with story to split 2 factor pages, but for now is correct. --- app/main/forms.py | 17 ++++++++ app/main/views/verify.py | 11 ++++- app/templates/views/verify.html | 50 +++++++++++++++------- tests/app/main/views/test_accept_invite.py | 11 +++-- tests/conftest.py | 1 + 5 files changed, 66 insertions(+), 24 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index acf3fdaae..640e993de 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -150,6 +150,23 @@ class TwoFactorForm(Form): raise ValidationError(reason) +class VerifySmsForm(Form): + def __init__(self, validate_code_func, *args, **kwargs): + ''' + Keyword arguments: + validate_code_func -- Validates the code with the API. + ''' + self.validate_code_func = validate_code_func + super(VerifySmsForm, self).__init__(*args, **kwargs) + + sms_code = sms_code() + + def validate_sms_code(self, field): + is_valid, reason = self.validate_code_func(field.data, 'sms') + if not is_valid: + raise ValidationError(reason) + + class VerifyForm(Form): def __init__(self, validate_code_func, *args, **kwargs): ''' diff --git a/app/main/views/verify.py b/app/main/views/verify.py index a85b8607a..6bc4c3e2c 100644 --- a/app/main/views/verify.py +++ b/app/main/views/verify.py @@ -9,7 +9,10 @@ from flask_login import login_user from app.main import main from app.main.dao import users_dao -from app.main.forms import VerifyForm +from app.main.forms import ( + VerifyForm, + VerifySmsForm +) @main.route('/verify', methods=['GET', 'POST']) @@ -22,7 +25,11 @@ def verify(): def _check_code(code, code_type): return users_dao.check_verify_code(user_id, code, code_type) - form = VerifyForm(_check_code) + if session.get('invited_user'): + form = VerifySmsForm(_check_code) + else: + form = VerifyForm(_check_code) + if form.validate_on_submit(): try: user = users_dao.get_user_by_id(user_id) diff --git a/app/templates/views/verify.html b/app/templates/views/verify.html index dcde0a4f3..e34285720 100644 --- a/app/templates/views/verify.html +++ b/app/templates/views/verify.html @@ -3,30 +3,48 @@ {% from "components/page-footer.html" import page_footer %} {% block page_title %} - Enter the codes we’ve sent you – GOV.UK Notify + {% if form.email_code %} + Enter the codes we’ve sent you – GOV.UK Notify + {% else %} + Enter the code we’ve sent you – GOV.UK Notify + {% endif %} {% endblock %} {% block maincolumn_content %}
-

Enter the codes we’ve sent you

-

- We’ve sent you confirmation codes by email and text message. -

+ {% if form.email_code %} +

Enter the codes we’ve sent you

+ {% else %} +

Enter the code we’ve sent you

+ {% endif %} + {% if form.email_code %} +

+ We’ve sent you confirmation codes by email and text message. +

+ {% else %} +

+ We’ve sent you a confirmation code by text message. +

+ {% endif %}
+ {% if form.sms_code %} {{ textbox( - form.sms_code, - width='5em', - help_link=url_for('.check_and_resend_text_code'), - help_link_text='I haven’t received a text message' - ) }} - {{ textbox( - form.email_code, - width='5em', - help_link=url_for('.check_and_resend_email_code'), - help_link_text='I haven’t received an email' - ) }} + form.sms_code, + width='5em', + help_link=url_for('.check_and_resend_text_code'), + help_link_text='I haven’t received a text message' + ) }} + {% endif %} + {% if form.email_code %} + {{ textbox( + form.email_code, + width='5em', + help_link=url_for('.check_and_resend_email_code'), + help_link_text='I haven’t received an email' + ) }} + {% endif %} {{ page_footer("Continue") }}
diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 9f9c1f32a..b74142682 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -169,8 +169,7 @@ def test_cancelled_invited_user_accepts_invited_redirect_to_cancelled_invitation service_one, mocker, mock_get_user, - mock_get_service - ): + mock_get_service): with app_.test_request_context(): with app_.test_client() as client: cancelled_invitation = create_sample_invite(mocker, service_one, status='cancelled') @@ -232,6 +231,7 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify(a def test_new_invited_user_verifies_and_added_to_service(app_, service_one, sample_invite, + api_user_active, mock_check_invite_token, mock_dont_get_user_by_email, mock_register_user, @@ -261,8 +261,7 @@ def test_new_invited_user_verifies_and_added_to_service(app_, response = client.post(url_for('main.register_from_invite'), data=data) # that sends user on to verify - response = client.post(url_for('main.verify'), data={'sms_code': '12345', 'email_code': '23456'}, - follow_redirects=True) + response = client.post(url_for('main.verify'), data={'sms_code': '12345'}, follow_redirects=True) # when they post codes back to admin user should be added to # service and sent on to dash board @@ -270,8 +269,8 @@ def test_new_invited_user_verifies_and_added_to_service(app_, with client.session_transaction() as session: new_user_id = session['user_id'] mock_add_user_to_service.assert_called_with(data['service'], new_user_id, expected_permissions) - - mock_accept_invite.assert_called_with(data['service'], sample_invite['id']) + mock_accept_invite.assert_called_with(data['service'], sample_invite['id']) + mock_check_verify_code.assert_called_once_with(new_user_id, '12345', 'sms') page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') element = page.find('h2', class_='navigation-service-name').find('a') diff --git a/tests/conftest.py b/tests/conftest.py index 589c04517..b6bca0ea1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -332,6 +332,7 @@ def mock_register_user(mocker, api_user_pending): @pytest.fixture(scope='function') def mock_get_user(mocker, api_user_active): def _get_user(id): + api_user_active.id = id return api_user_active return mocker.patch( 'app.user_api_client.get_user', side_effect=_get_user)