From d9f1ee5af65e212f4de815698d99557ffe7d31cb Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Fri, 17 Feb 2023 11:53:44 -0500 Subject: [PATCH] Expect sms 2nd factor codes to be 6 digits --- app/main/forms.py | 4 ++-- tests/app/main/forms/test_two_factor_form.py | 20 +++++++++---------- .../test_organisation_invites.py | 2 +- tests/app/main/views/test_accept_invite.py | 6 +++--- tests/app/main/views/test_two_factor.py | 20 +++++++++---------- tests/app/main/views/test_user_profile.py | 2 +- tests/app/main/views/test_verify.py | 8 ++++---- 7 files changed, 31 insertions(+), 31 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 91931006c..cb2d72f45 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -350,8 +350,8 @@ class SMSCode(GovukTextInputField): validators = [ DataRequired(message='Cannot be empty'), Regexp(regex=r'^\d+$', message='Numbers only'), - Length(min=5, message='Not enough numbers'), - Length(max=5, message='Too many numbers'), + Length(min=6, message='Not enough numbers'), + Length(max=6, message='Too many numbers'), ] def __call__(self, **kwargs): diff --git a/tests/app/main/forms/test_two_factor_form.py b/tests/app/main/forms/test_two_factor_form.py index 00def926c..6426b6212 100644 --- a/tests/app/main/forms/test_two_factor_form.py +++ b/tests/app/main/forms/test_two_factor_form.py @@ -9,10 +9,10 @@ def _check_code(code): @pytest.mark.parametrize('post_data', [ - {'sms_code': '12345'}, - {'sms_code': ' 12345 '}, - {'sms_code': '12 34 5'}, - {'sms_code': '1-23-45'}, + {'sms_code': '123456'}, + {'sms_code': ' 123456 '}, + {'sms_code': '12 34 56'}, + {'sms_code': '1-23-45-6'}, ]) def test_form_is_valid_returns_no_errors( notify_admin, @@ -23,7 +23,7 @@ def test_form_is_valid_returns_no_errors( form = TwoFactorForm(_check_code) assert form.validate() is True assert form.errors == {} - mock_check_verify_code.assert_called_once_with('1', '12345', 'sms') + mock_check_verify_code.assert_called_once_with('1', '123456', 'sms') @pytest.mark.parametrize('post_data, expected_error', ( @@ -32,7 +32,7 @@ def test_form_is_valid_returns_no_errors( 'Not enough numbers', ), ( - {'sms_code': '123456'}, + {'sms_code': '1234567'}, 'Too many numbers', ), ( @@ -40,11 +40,11 @@ def test_form_is_valid_returns_no_errors( 'Cannot be empty', ), ( - {'sms_code': '12E45'}, + {'sms_code': '12E456'}, 'Numbers only', ), ( - {'sms_code': ' ! 2 3 4 5'}, + {'sms_code': ' ! 2 3 4 5 6'}, 'Numbers only', ), )) @@ -64,7 +64,7 @@ def test_check_verify_code_returns_error_when_code_has_expired( notify_admin, mock_check_verify_code_code_expired, ): - with notify_admin.test_request_context(method='POST', data={'sms_code': '99999'}): + with notify_admin.test_request_context(method='POST', data={'sms_code': '999999'}): form = TwoFactorForm(_check_code) assert form.validate() is False assert form.errors == {'sms_code': ['Code has expired']} @@ -74,7 +74,7 @@ def test_check_verify_code_returns_error_when_code_was_not_found( notify_admin, mock_check_verify_code_code_not_found, ): - with notify_admin.test_request_context(method='POST', data={'sms_code': '99999'}): + with notify_admin.test_request_context(method='POST', data={'sms_code': '999999'}): form = TwoFactorForm(_check_code) assert form.validate() is False assert form.errors == {'sms_code': ['Code not found']} diff --git a/tests/app/main/views/organisations/test_organisation_invites.py b/tests/app/main/views/organisations/test_organisation_invites.py index ec2b47b4a..97cf75f52 100644 --- a/tests/app/main/views/organisations/test_organisation_invites.py +++ b/tests/app/main/views/organisations/test_organisation_invites.py @@ -405,7 +405,7 @@ def test_verified_org_user_redirects_to_dashboard( client_request.post( 'main.verify', - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _expected_redirect=url_for( 'main.organisation_dashboard', org_id=invited_org_user['organisation'], diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index a3fb18d20..b1beff18c 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -597,7 +597,7 @@ def test_new_invited_user_verifies_and_added_to_service( # that sends user on to verify page = client_request.post( 'main.verify', - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _follow_redirects=True, ) @@ -610,7 +610,7 @@ def test_new_invited_user_verifies_and_added_to_service( 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_check_verify_code.assert_called_once_with(new_user_id, '12345', 'sms') + mock_check_verify_code.assert_called_once_with(new_user_id, '123456', 'sms') assert service_one['id'] == session['service_id'] assert page.find('h1').text == 'Dashboard' @@ -659,7 +659,7 @@ def test_new_invited_user_is_redirected_to_correct_place( client_request.post( 'main.verify', - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _expected_redirect=url_for( expected_endpoint, service_id=sample_invite['service'], diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index 68b3773a2..01ca5020c 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -96,7 +96,7 @@ def test_should_login_user_and_should_redirect_to_next_url( client_request.post( 'main.two_factor_sms', next='/services/{}'.format(SERVICE_ONE_ID), - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _expected_redirect=url_for( 'main.service_dashboard', service_id=SERVICE_ONE_ID, @@ -124,7 +124,7 @@ def test_should_send_email_and_redirect_to_info_page_if_user_needs_to_revalidate client_request.post( 'main.two_factor_sms', next=f'/services/{SERVICE_ONE_ID}', - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _expected_redirect=url_for( 'main.revalidate_email_sent', next=f'/services/{SERVICE_ONE_ID}' @@ -154,7 +154,7 @@ def test_should_login_user_and_not_redirect_to_external_url( client_request.post( 'main.two_factor_sms', next='http://www.google.com', - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _expected_redirect=url_for('main.show_accounts_or_dashboard') ) @@ -182,7 +182,7 @@ def test_should_login_user_and_redirect_to_show_accounts( client_request.post( 'main.two_factor_sms', - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _expected_redirect=url_for('main.show_accounts_or_dashboard') ) @@ -204,7 +204,7 @@ def test_should_return_200_with_sms_code_error_when_sms_code_is_wrong( page = client_request.post( 'main.two_factor_sms', - _data={'sms_code': '23456'}, + _data={'sms_code': '234567'}, _expected_status=200, ) assert 'Code not found' in page.text @@ -229,7 +229,7 @@ def test_should_login_user_when_multiple_valid_codes_exist( client_request.post( 'main.two_factor_sms', - _data={'sms_code': '23456'}, + _data={'sms_code': '234567'}, _expected_status=302, ) @@ -254,7 +254,7 @@ def test_two_factor_sms_should_set_password_when_new_password_exists_in_session( client_request.post( 'main.two_factor_sms', - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _expected_redirect=url_for('main.show_accounts_or_dashboard'), ) @@ -279,7 +279,7 @@ def test_two_factor_sms_returns_error_when_user_is_locked( } page = client_request.post( 'main.two_factor_sms', - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _expected_status=200, ) assert 'Code not found' in page.text @@ -290,7 +290,7 @@ def test_two_factor_sms_post_should_redirect_to_sign_in_if_user_not_in_session( ): client_request.post( 'main.two_factor_sms', - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _expected_redirect=url_for('main.sign_in') ) @@ -360,7 +360,7 @@ def test_two_factor_sms_should_activate_pending_user( 'id': api_user_pending['id'], 'email_address': api_user_pending['email_address'] } - client_request.post('main.two_factor_sms', _data={'sms_code': '12345'}) + client_request.post('main.two_factor_sms', _data={'sms_code': '123456'}) assert mock_activate_user.called diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index 14f968930..d763fecb0 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -346,7 +346,7 @@ def test_should_redirect_after_mobile_number_confirm( client_request.post( 'main.user_profile_mobile_number_confirm', - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _expected_status=302, _expected_redirect=url_for( 'main.user_profile', diff --git a/tests/app/main/views/test_verify.py b/tests/app/main/views/test_verify.py index 1dffad49b..c640da9d5 100644 --- a/tests/app/main/views/test_verify.py +++ b/tests/app/main/views/test_verify.py @@ -47,7 +47,7 @@ def test_should_redirect_to_add_service_when_sms_code_is_correct( client_request.post( 'main.verify', - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _expected_redirect=url_for('main.add_service', first='first'), ) @@ -55,7 +55,7 @@ def test_should_redirect_to_add_service_when_sms_code_is_correct( with client_request.session_transaction() as session: assert session['current_session_id'] == str(uuid.UUID(int=1)) - mock_check_verify_code.assert_called_once_with(api_user_active['id'], '12345', 'sms') + mock_check_verify_code.assert_called_once_with(api_user_active['id'], '123456', 'sms') def test_should_activate_user_after_verify( @@ -74,7 +74,7 @@ def test_should_activate_user_after_verify( 'email_address': api_user_pending['email_address'], 'id': api_user_pending['id'] } - client_request.post('main.verify', _data={'sms_code': '12345'}) + client_request.post('main.verify', _data={'sms_code': '123456'}) assert mock_activate_user.called @@ -91,7 +91,7 @@ def test_should_return_200_when_sms_code_is_wrong( page = client_request.post( 'main.verify', - _data={'sms_code': '12345'}, + _data={'sms_code': '123456'}, _expected_status=200, )