diff --git a/app/main/forms.py b/app/main/forms.py index d5e680309..2363b7e28 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -171,13 +171,11 @@ class RegisterUserForm(Form): email_address = email_address() mobile_number = international_phone_number() password = password() + # always register as sms type + auth_type = HiddenField('auth_type', default='sms_auth') class RegisterUserFromInviteForm(Form): - def __init__(self, auth_type, *args, **kwargs): - self.auth_type = auth_type - super().__init__(*args, **kwargs) - name = StringField( 'Full name', validators=[DataRequired(message='Can’t be empty')] @@ -186,9 +184,10 @@ class RegisterUserFromInviteForm(Form): password = password() service = HiddenField('service') email_address = HiddenField('email_address') + auth_type = HiddenField('auth_type', validators=[DataRequired()]) def validate_mobile_number(self, field): - if self.auth_type == 'sms_auth' and not field.data: + if self.auth_type.data == 'sms_auth' and not field.data: raise ValidationError('Can’t be empty') diff --git a/app/main/views/register.py b/app/main/views/register.py index 523a65cc2..9e92fa444 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -43,7 +43,7 @@ def register(): @main.route('/register-from-invite', methods=['GET', 'POST']) def register_from_invite(): invited_user = session.get('invited_user') - form = RegisterUserFromInviteForm(invited_user['auth_type']) + form = RegisterUserFromInviteForm() if not invited_user: abort(404) @@ -70,7 +70,8 @@ def _do_registration(form, service=None, send_sms=True, send_email=True): user = user_api_client.register_user(form.name.data, form.email_address.data, form.mobile_number.data, - form.password.data) + form.password.data, + form.auth_type.data) # TODO possibly there should be some exception handling # for sending sms and email codes. diff --git a/app/notify_client/models.py b/app/notify_client/models.py index fe9775a36..1be63849b 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -150,7 +150,7 @@ class User(UserMixin): class InvitedUser(object): - def __init__(self, id, service, from_user, email_address, permissions, status, created_at, auth_type=None): + def __init__(self, id, service, from_user, email_address, permissions, status, created_at, auth_type): self.id = id self.service = str(service) self.from_user = from_user diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 2579b5e5b..272e19e11 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -21,12 +21,13 @@ class UserApiClient(NotifyAdminAPIClient): self.api_key = app.config['ADMIN_CLIENT_SECRET'] self.max_failed_login_count = app.config["MAX_FAILED_LOGIN_COUNT"] - def register_user(self, name, email_address, mobile_number, password): + def register_user(self, name, email_address, mobile_number, password, auth_type): data = { "name": name, "email_address": email_address, "mobile_number": mobile_number, - "password": password + "password": password, + "auth_type": auth_type } user_data = self.post("/user", data) return User(user_data['data'], max_failed_login_count=self.max_failed_login_count) diff --git a/tests/__init__.py b/tests/__init__.py index f59633f55..c8f7ec629 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -149,15 +149,17 @@ def api_key_json(id_, name, expiry_date=None): } -def invite_json(id_, from_user, service_id, email_address, permissions, created_at, status): - return {'id': id_, - 'from_user': from_user, - 'service': service_id, - 'email_address': email_address, - 'status': status, - 'permissions': permissions, - 'created_at': created_at - } +def invite_json(id_, from_user, service_id, email_address, permissions, created_at, status, auth_type): + return { + 'id': id_, + 'from_user': from_user, + 'service': service_id, + 'email_address': email_address, + 'status': status, + 'permissions': permissions, + 'created_at': created_at, + 'auth_type': auth_type + } TEST_USER_EMAIL = 'test@user.gov.uk' diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 913ff09d6..6c6ec4abc 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -267,7 +267,8 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify( 'from_user': invited_user['from_user'], 'password': 'longpassword', 'mobile_number': '+447890123456', - 'name': 'Invited User' + 'name': 'Invited User', + 'auth_type': 'email_auth' } expected_redirect_location = 'http://localhost/verify' @@ -280,7 +281,8 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify( mock_register_user.assert_called_with(data['name'], data['email_address'], data['mobile_number'], - data['password']) + data['password'], + data['auth_type']) assert mock_accept_invite.call_count == 1 @@ -312,65 +314,6 @@ def test_signed_in_existing_user_cannot_use_anothers_invite( assert mock_accept_invite.call_count == 0 -def test_new_invited_user_verifies_and_added_to_service( - client, - service_one, - sample_invite, - api_user_active, - mock_check_invite_token, - mock_dont_get_user_by_email, - mock_is_email_unique, - mock_register_user, - mock_send_verify_code, - mock_check_verify_code, - mock_get_user, - mock_update_user_attribute, - mock_add_user_to_service, - mock_accept_invite, - mock_get_service, - mock_get_service_templates, - mock_get_template_statistics, - mock_get_jobs, - mock_has_permissions, - mock_get_users_by_service, - mock_get_detailed_service, - mock_get_usage, - mocker, -): - mocker.patch('app.main.views.invites.check_token') - - # visit accept token page - response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) - data = {'service': sample_invite['service'], - 'email_address': sample_invite['email_address'], - 'from_user': sample_invite['from_user'], - 'password': 'longpassword', - 'mobile_number': '+447890123456', - 'name': 'Invited User' - } - - # get redirected to register from invite - 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'}, follow_redirects=True) - - # when they post codes back to admin user should be added to - # service and sent on to dash board - expected_permissions = ['send_messages', 'manage_service', 'manage_api_keys'] - - 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_check_verify_code.assert_called_once_with(new_user_id, '12345', 'sms') - assert service_one['id'] == session['service_id'] - - raw_html = response.data.decode('utf-8') - page = BeautifulSoup(raw_html, 'html.parser') - assert page.find('h1').text == 'Dashboard' - - def test_gives_message_if_token_has_expired( app_, client, diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index 9c6fd0e25..a366e5282 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -49,7 +49,8 @@ def test_register_creates_new_user_and_redirects_to_continue_page( user_data = {'name': 'Some One Valid', 'email_address': 'notfound@example.gov.uk', 'mobile_number': phone_number_to_register_with, - 'password': 'validPassword!' + 'password': 'validPassword!', + 'auth_type': 'sms_auth' } response = client.post(url_for('main.register'), data=user_data, follow_redirects=True) @@ -62,7 +63,8 @@ def test_register_creates_new_user_and_redirects_to_continue_page( mock_register_user.assert_called_with(user_data['name'], user_data['email_address'], user_data['mobile_number'], - user_data['password']) + user_data['password'], + user_data['auth_type']) def test_register_continue_handles_missing_session_sensibly( @@ -175,15 +177,21 @@ def test_register_from_invite_( "invited@user.com", ["manage_users"], "pending", - datetime.utcnow()) + datetime.utcnow(), + 'sms_auth') with client.session_transaction() as session: session['invited_user'] = invited_user.serialize() - response = client.post(url_for('main.register_from_invite'), - data={'name': 'Registered in another Browser', - 'email_address': invited_user.email_address, - 'mobile_number': '+4407700900460', - 'service': str(invited_user.id), - 'password': 'somreallyhardthingtoguess'}) + response = client.post( + url_for('main.register_from_invite'), + data={ + 'name': 'Registered in another Browser', + 'email_address': invited_user.email_address, + 'mobile_number': '+4407700900460', + 'service': str(invited_user.id), + 'password': 'somreallyhardthingtoguess', + 'auth_type': 'sms_auth' + } + ) assert response.status_code == 302 assert response.location == url_for('main.verify', _external=True) @@ -199,14 +207,20 @@ def test_register_from_invite_when_user_registers_in_another_browser( api_user_active.email_address, ["manage_users"], "pending", - datetime.utcnow()) + datetime.utcnow(), + 'sms_auth') with client.session_transaction() as session: session['invited_user'] = invited_user.serialize() - response = client.post(url_for('main.register_from_invite'), - data={'name': 'Registered in another Browser', - 'email_address': api_user_active.email_address, - 'mobile_number': api_user_active.mobile_number, - 'service': str(api_user_active.id), - 'password': 'somreallyhardthingtoguess'}) + response = client.post( + url_for('main.register_from_invite'), + data={ + 'name': 'Registered in another Browser', + 'email_address': api_user_active.email_address, + 'mobile_number': api_user_active.mobile_number, + 'service': str(api_user_active.id), + 'password': 'somreallyhardthingtoguess', + 'auth_type': 'sms_auth' + } + ) assert response.status_code == 302 assert response.location == url_for('main.verify', _external=True) diff --git a/tests/conftest.py b/tests/conftest.py index 2a1958781..925456a4e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1288,11 +1288,12 @@ def mock_send_change_email_verification(mocker): @pytest.fixture(scope='function') def mock_register_user(mocker, api_user_pending): - def _register(name, email_address, mobile_number, password): + def _register(name, email_address, mobile_number, password, auth_type): api_user_pending.name = name api_user_pending.email_address = email_address api_user_pending.mobile_number = mobile_number api_user_pending.password = password + api_user_pending.auth_type = auth_type return api_user_pending return mocker.patch('app.user_api_client.register_user', side_effect=_register) @@ -1910,7 +1911,9 @@ def sample_invite(mocker, service_one, status='pending'): service_id = service_one['id'] permissions = 'send_messages,manage_service,manage_api_keys' created_at = str(datetime.utcnow()) - return invite_json(id_, from_user, service_id, email_address, permissions, created_at, status) + auth_type = 'sms_auth' + + return invite_json(id_, from_user, service_id, email_address, permissions, created_at, status, auth_type) @pytest.fixture(scope='function')