mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-09-09 17:45:32 -04:00
Merge pull request #387 from GSA/increase-sms-code-length
Expect sms 2nd factor codes to be 6 digits
This commit is contained in:
@@ -350,8 +350,8 @@ class SMSCode(GovukTextInputField):
|
|||||||
validators = [
|
validators = [
|
||||||
DataRequired(message='Cannot be empty'),
|
DataRequired(message='Cannot be empty'),
|
||||||
Regexp(regex=r'^\d+$', message='Numbers only'),
|
Regexp(regex=r'^\d+$', message='Numbers only'),
|
||||||
Length(min=5, message='Not enough numbers'),
|
Length(min=6, message='Not enough numbers'),
|
||||||
Length(max=5, message='Too many numbers'),
|
Length(max=6, message='Too many numbers'),
|
||||||
]
|
]
|
||||||
|
|
||||||
def __call__(self, **kwargs):
|
def __call__(self, **kwargs):
|
||||||
|
|||||||
@@ -9,10 +9,10 @@ def _check_code(code):
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize('post_data', [
|
@pytest.mark.parametrize('post_data', [
|
||||||
{'sms_code': '12345'},
|
{'sms_code': '123456'},
|
||||||
{'sms_code': ' 12345 '},
|
{'sms_code': ' 123456 '},
|
||||||
{'sms_code': '12 34 5'},
|
{'sms_code': '12 34 56'},
|
||||||
{'sms_code': '1-23-45'},
|
{'sms_code': '1-23-45-6'},
|
||||||
])
|
])
|
||||||
def test_form_is_valid_returns_no_errors(
|
def test_form_is_valid_returns_no_errors(
|
||||||
notify_admin,
|
notify_admin,
|
||||||
@@ -23,7 +23,7 @@ def test_form_is_valid_returns_no_errors(
|
|||||||
form = TwoFactorForm(_check_code)
|
form = TwoFactorForm(_check_code)
|
||||||
assert form.validate() is True
|
assert form.validate() is True
|
||||||
assert form.errors == {}
|
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', (
|
@pytest.mark.parametrize('post_data, expected_error', (
|
||||||
@@ -32,7 +32,7 @@ def test_form_is_valid_returns_no_errors(
|
|||||||
'Not enough numbers',
|
'Not enough numbers',
|
||||||
),
|
),
|
||||||
(
|
(
|
||||||
{'sms_code': '123456'},
|
{'sms_code': '1234567'},
|
||||||
'Too many numbers',
|
'Too many numbers',
|
||||||
),
|
),
|
||||||
(
|
(
|
||||||
@@ -40,11 +40,11 @@ def test_form_is_valid_returns_no_errors(
|
|||||||
'Cannot be empty',
|
'Cannot be empty',
|
||||||
),
|
),
|
||||||
(
|
(
|
||||||
{'sms_code': '12E45'},
|
{'sms_code': '12E456'},
|
||||||
'Numbers only',
|
'Numbers only',
|
||||||
),
|
),
|
||||||
(
|
(
|
||||||
{'sms_code': ' ! 2 3 4 5'},
|
{'sms_code': ' ! 2 3 4 5 6'},
|
||||||
'Numbers only',
|
'Numbers only',
|
||||||
),
|
),
|
||||||
))
|
))
|
||||||
@@ -64,7 +64,7 @@ def test_check_verify_code_returns_error_when_code_has_expired(
|
|||||||
notify_admin,
|
notify_admin,
|
||||||
mock_check_verify_code_code_expired,
|
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)
|
form = TwoFactorForm(_check_code)
|
||||||
assert form.validate() is False
|
assert form.validate() is False
|
||||||
assert form.errors == {'sms_code': ['Code has expired']}
|
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,
|
notify_admin,
|
||||||
mock_check_verify_code_code_not_found,
|
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)
|
form = TwoFactorForm(_check_code)
|
||||||
assert form.validate() is False
|
assert form.validate() is False
|
||||||
assert form.errors == {'sms_code': ['Code not found']}
|
assert form.errors == {'sms_code': ['Code not found']}
|
||||||
|
|||||||
@@ -405,7 +405,7 @@ def test_verified_org_user_redirects_to_dashboard(
|
|||||||
|
|
||||||
client_request.post(
|
client_request.post(
|
||||||
'main.verify',
|
'main.verify',
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_expected_redirect=url_for(
|
_expected_redirect=url_for(
|
||||||
'main.organisation_dashboard',
|
'main.organisation_dashboard',
|
||||||
org_id=invited_org_user['organisation'],
|
org_id=invited_org_user['organisation'],
|
||||||
|
|||||||
@@ -597,7 +597,7 @@ def test_new_invited_user_verifies_and_added_to_service(
|
|||||||
# that sends user on to verify
|
# that sends user on to verify
|
||||||
page = client_request.post(
|
page = client_request.post(
|
||||||
'main.verify',
|
'main.verify',
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_follow_redirects=True,
|
_follow_redirects=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -610,7 +610,7 @@ def test_new_invited_user_verifies_and_added_to_service(
|
|||||||
new_user_id = session['user_id']
|
new_user_id = session['user_id']
|
||||||
mock_add_user_to_service.assert_called_with(data['service'], new_user_id, expected_permissions, [])
|
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')
|
mock_check_verify_code.assert_called_once_with(new_user_id, '123456', 'sms')
|
||||||
assert service_one['id'] == session['service_id']
|
assert service_one['id'] == session['service_id']
|
||||||
|
|
||||||
assert page.find('h1').text == 'Dashboard'
|
assert page.find('h1').text == 'Dashboard'
|
||||||
@@ -659,7 +659,7 @@ def test_new_invited_user_is_redirected_to_correct_place(
|
|||||||
|
|
||||||
client_request.post(
|
client_request.post(
|
||||||
'main.verify',
|
'main.verify',
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_expected_redirect=url_for(
|
_expected_redirect=url_for(
|
||||||
expected_endpoint,
|
expected_endpoint,
|
||||||
service_id=sample_invite['service'],
|
service_id=sample_invite['service'],
|
||||||
|
|||||||
@@ -96,7 +96,7 @@ def test_should_login_user_and_should_redirect_to_next_url(
|
|||||||
client_request.post(
|
client_request.post(
|
||||||
'main.two_factor_sms',
|
'main.two_factor_sms',
|
||||||
next='/services/{}'.format(SERVICE_ONE_ID),
|
next='/services/{}'.format(SERVICE_ONE_ID),
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_expected_redirect=url_for(
|
_expected_redirect=url_for(
|
||||||
'main.service_dashboard',
|
'main.service_dashboard',
|
||||||
service_id=SERVICE_ONE_ID,
|
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(
|
client_request.post(
|
||||||
'main.two_factor_sms',
|
'main.two_factor_sms',
|
||||||
next=f'/services/{SERVICE_ONE_ID}',
|
next=f'/services/{SERVICE_ONE_ID}',
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_expected_redirect=url_for(
|
_expected_redirect=url_for(
|
||||||
'main.revalidate_email_sent',
|
'main.revalidate_email_sent',
|
||||||
next=f'/services/{SERVICE_ONE_ID}'
|
next=f'/services/{SERVICE_ONE_ID}'
|
||||||
@@ -154,7 +154,7 @@ def test_should_login_user_and_not_redirect_to_external_url(
|
|||||||
client_request.post(
|
client_request.post(
|
||||||
'main.two_factor_sms',
|
'main.two_factor_sms',
|
||||||
next='http://www.google.com',
|
next='http://www.google.com',
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_expected_redirect=url_for('main.show_accounts_or_dashboard')
|
_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(
|
client_request.post(
|
||||||
'main.two_factor_sms',
|
'main.two_factor_sms',
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_expected_redirect=url_for('main.show_accounts_or_dashboard')
|
_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(
|
page = client_request.post(
|
||||||
'main.two_factor_sms',
|
'main.two_factor_sms',
|
||||||
_data={'sms_code': '23456'},
|
_data={'sms_code': '234567'},
|
||||||
_expected_status=200,
|
_expected_status=200,
|
||||||
)
|
)
|
||||||
assert 'Code not found' in page.text
|
assert 'Code not found' in page.text
|
||||||
@@ -229,7 +229,7 @@ def test_should_login_user_when_multiple_valid_codes_exist(
|
|||||||
|
|
||||||
client_request.post(
|
client_request.post(
|
||||||
'main.two_factor_sms',
|
'main.two_factor_sms',
|
||||||
_data={'sms_code': '23456'},
|
_data={'sms_code': '234567'},
|
||||||
_expected_status=302,
|
_expected_status=302,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -254,7 +254,7 @@ def test_two_factor_sms_should_set_password_when_new_password_exists_in_session(
|
|||||||
|
|
||||||
client_request.post(
|
client_request.post(
|
||||||
'main.two_factor_sms',
|
'main.two_factor_sms',
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_expected_redirect=url_for('main.show_accounts_or_dashboard'),
|
_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(
|
page = client_request.post(
|
||||||
'main.two_factor_sms',
|
'main.two_factor_sms',
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_expected_status=200,
|
_expected_status=200,
|
||||||
)
|
)
|
||||||
assert 'Code not found' in page.text
|
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(
|
client_request.post(
|
||||||
'main.two_factor_sms',
|
'main.two_factor_sms',
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_expected_redirect=url_for('main.sign_in')
|
_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'],
|
'id': api_user_pending['id'],
|
||||||
'email_address': api_user_pending['email_address']
|
'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
|
assert mock_activate_user.called
|
||||||
|
|
||||||
|
|||||||
@@ -346,7 +346,7 @@ def test_should_redirect_after_mobile_number_confirm(
|
|||||||
|
|
||||||
client_request.post(
|
client_request.post(
|
||||||
'main.user_profile_mobile_number_confirm',
|
'main.user_profile_mobile_number_confirm',
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_expected_status=302,
|
_expected_status=302,
|
||||||
_expected_redirect=url_for(
|
_expected_redirect=url_for(
|
||||||
'main.user_profile',
|
'main.user_profile',
|
||||||
|
|||||||
@@ -47,7 +47,7 @@ def test_should_redirect_to_add_service_when_sms_code_is_correct(
|
|||||||
|
|
||||||
client_request.post(
|
client_request.post(
|
||||||
'main.verify',
|
'main.verify',
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_expected_redirect=url_for('main.add_service', first='first'),
|
_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:
|
with client_request.session_transaction() as session:
|
||||||
assert session['current_session_id'] == str(uuid.UUID(int=1))
|
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(
|
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'],
|
'email_address': api_user_pending['email_address'],
|
||||||
'id': api_user_pending['id']
|
'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
|
assert mock_activate_user.called
|
||||||
|
|
||||||
|
|
||||||
@@ -91,7 +91,7 @@ def test_should_return_200_when_sms_code_is_wrong(
|
|||||||
|
|
||||||
page = client_request.post(
|
page = client_request.post(
|
||||||
'main.verify',
|
'main.verify',
|
||||||
_data={'sms_code': '12345'},
|
_data={'sms_code': '123456'},
|
||||||
_expected_status=200,
|
_expected_status=200,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user