From 45297eae43624315aefa377bc2a08117440ac2c3 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 8 Mar 2021 12:51:35 +0000 Subject: [PATCH] store invited user ids in session same as the invited org user ids in the previous commit --- app/main/views/dashboard.py | 3 +- app/main/views/invites.py | 2 ++ app/main/views/sign_in.py | 3 +- app/main/views/verify.py | 7 ++-- app/models/user.py | 10 ++++++ app/notify_client/invite_api_client.py | 5 +++ tests/app/main/views/test_accept_invite.py | 7 ++++ tests/app/models/test_user.py | 41 +++++++++++++++++++++- tests/conftest.py | 13 +++++++ 9 files changed, 84 insertions(+), 7 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index b57d79a2e..1d43ba56a 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -49,8 +49,9 @@ def old_service_dashboard(service_id): @user_has_permissions() def service_dashboard(service_id): - if session.get('invited_user'): + if session.get('invited_user_id') or session.get('invited_user'): session.pop('invited_user', None) + session.pop('invited_user_id', None) session['service_id'] = service_id if current_service.has_permission('broadcast'): diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 6d4a127cb..02f9c644f 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -40,12 +40,14 @@ def accept_invite(token): if invited_user.status == 'accepted': session.pop('invited_user', None) + session.pop('invited_user_id', None) service = Service.from_id(invited_user.service) if service.has_permission('broadcast'): return redirect(url_for('main.broadcast_tour', service_id=service.id, step_index=1)) return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) session['invited_user'] = invited_user.serialize() + session['invited_user_id'] = invited_user.id existing_user = User.from_email_address_or_none(invited_user.email_address) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 7ce9225b4..f6e1a3598 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -36,11 +36,12 @@ def sign_in(): if user and user.state == 'pending': return redirect(url_for('main.resend_email_verification', next=redirect_url)) - if user and session.get('invited_user'): + if user and (session.get('invited_user') or session.get('invited_user_id')): invited_user = InvitedUser.from_session() if user.email_address.lower() != invited_user.email_address.lower(): flash("You cannot accept an invite for another person.") session.pop('invited_user', None) + session.pop('invited_user_id', None) abort(403) else: invited_user.accept_invite() diff --git a/app/main/views/verify.py b/app/main/views/verify.py index 1eb4c4ee5..b2fd145fc 100644 --- a/app/main/views/verify.py +++ b/app/main/views/verify.py @@ -75,7 +75,7 @@ def activate_user(user_id): activated_user = user.activate() activated_user.login() - invited_user = session.get('invited_user') + invited_user = InvitedUser.from_session() if invited_user: service_id = _add_invited_user_to_service(invited_user) service = Service.from_id(service_id) @@ -93,10 +93,9 @@ def activate_user(user_id): return redirect(url_for('main.add_service', first='first')) -def _add_invited_user_to_service(invited_user): - invitation = InvitedUser(invited_user) +def _add_invited_user_to_service(invitation): user = User.from_id(session['user_id']) - service_id = invited_user['service'] + service_id = invitation.service user.add_to_service( service_id, invitation.permissions, diff --git a/app/models/user.py b/app/models/user.py index bb076d4d0..bce03d7a9 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -476,6 +476,12 @@ class InvitedUser(JSONModel): invite_api_client.get_invited_user_for_service(service_id, invited_user_id) ) + @classmethod + def by_id(cls, invited_user_id): + return cls( + invite_api_client.get_invited_user(invited_user_id) + ) + def accept_invite(self): invite_api_client.accept_invite(self.service, self.id) @@ -515,6 +521,10 @@ class InvitedUser(JSONModel): @classmethod def from_session(cls): + invited_user_id = session.get('invited_user_id') + if invited_user_id: + return cls.by_id(invited_user_id) + invited_user = session.get('invited_user') return cls(invited_user) if invited_user else None diff --git a/app/notify_client/invite_api_client.py b/app/notify_client/invite_api_client.py index 3659269d8..cf6b83e71 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -37,6 +37,11 @@ class InviteApiClient(NotifyAdminAPIClient): '/service/{}/invite'.format(service_id) )['data'] + def get_invited_user(self, invited_user_id): + return self.get( + f'/invite/service/{invited_user_id}' + )['data'] + def get_invited_user_for_service(self, service_id, invited_user_id): return self.get( f'/service/{service_id}/invite/{invited_user_id}' diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 7fe84def9..cb36cf8d0 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -306,6 +306,7 @@ def test_new_user_accept_invite_calls_api_and_views_registration_page( service_one, mock_check_invite_token, mock_dont_get_user_by_email, + mock_get_invited_user_by_id, mock_add_user_to_service, mock_get_users_by_service, mock_get_service, @@ -315,6 +316,7 @@ def test_new_user_accept_invite_calls_api_and_views_registration_page( mock_check_invite_token.assert_called_with('thisisnotarealtoken') mock_dont_get_user_by_email.assert_called_with('invited_user@test.gov.uk') + mock_get_invited_user_by_id.assert_called_once_with(USER_ONE_ID) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') @@ -401,6 +403,7 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify( mock_email_is_not_already_in_use, mock_register_user, mock_send_verify_code, + mock_get_invited_user_by_id, mock_accept_invite, mock_get_users_by_service, mock_add_user_to_service, @@ -418,6 +421,7 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify( assert response.location == expected_redirect_location invited_user = session.get('invited_user') assert invited_user + assert session.get('invited_user_id') == USER_ONE_ID assert expected_service == invited_user['service'] assert expected_email == invited_user['email_address'] assert expected_from_user == invited_user['from_user'] @@ -437,6 +441,7 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify( assert response.location == expected_redirect_location mock_send_verify_code.assert_called_once_with(ANY, 'sms', data['mobile_number']) + mock_get_invited_user_by_id.assert_called_once_with(USER_ONE_ID) mock_register_user.assert_called_with(data['name'], data['email_address'], @@ -516,6 +521,7 @@ def test_new_invited_user_verifies_and_added_to_service( mock_add_user_to_service, mock_accept_invite, mock_get_service, + mock_get_invited_user_by_id, mock_get_service_templates, mock_get_template_statistics, mock_has_no_jobs, @@ -558,6 +564,7 @@ def test_new_invited_user_verifies_and_added_to_service( expected_permissions = {'view_activity', 'send_messages', 'manage_service', 'manage_api_keys'} with client.session_transaction() as session: + assert 'invited_user_id' not in 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']) diff --git a/tests/app/models/test_user.py b/tests/app/models/test_user.py index 537bbc9fe..25bce1bf0 100644 --- a/tests/app/models/test_user.py +++ b/tests/app/models/test_user.py @@ -3,7 +3,8 @@ from unittest.mock import Mock import pytest -from app.models.user import AnonymousUser, InvitedOrgUser, User +from app.models.user import AnonymousUser, InvitedOrgUser, InvitedUser, User +from tests.conftest import USER_ONE_ID def test_anonymous_user(app_): @@ -111,6 +112,44 @@ def test_has_live_services_when_service_is_not_live( }).live_services == [] +def test_invited_user_from_session_uses_id(client, mocker, mock_get_invited_user_by_id): + fake_id = str(uuid.uuid4()) + session_dict = {'invited_user_id': fake_id} + mocker.patch.dict('app.models.user.session', values=session_dict, clear=True) + + assert InvitedUser.from_session().id == USER_ONE_ID + + mock_get_invited_user_by_id.assert_called_once_with(fake_id) + + +def test_invited_user_from_session_uses_id_even_if_obj_in_session( + client, + mocker, + sample_invite, + mock_get_invited_user_by_id +): + mock_session_obj = Mock(spec=dict) + session_dict = {'invited_user_id': USER_ONE_ID, 'invited_user': mock_session_obj} + mocker.patch.dict('app.models.user.session', values=session_dict, clear=True) + + assert InvitedUser.from_session().id == USER_ONE_ID + + assert mock_session_obj.mock_calls == [] + mock_get_invited_user_by_id.assert_called_once_with(USER_ONE_ID) + + +def test_invited_user_from_session_uses_obj_if_id_not_present(client, mocker, sample_invite): + session_dict = {'invited_user': sample_invite} + mocker.patch.dict('app.models.user.session', values=session_dict, clear=True) + + assert InvitedUser.from_session().id == USER_ONE_ID + + +def test_invited_user_from_session_returns_none_if_nothing_present(client, mocker): + mocker.patch.dict('app.models.user.session', values={}, clear=True) + assert InvitedUser.from_session() is None + + def test_invited_org_user_from_session_uses_id(client, mocker, mock_get_invited_org_user_by_id, sample_org_invite): session_dict = {'invited_org_user_id': sample_org_invite['id']} mocker.patch.dict('app.models.user.session', values=session_dict, clear=True) diff --git a/tests/conftest.py b/tests/conftest.py index 25d9dcb9a..1f6ea450e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4464,6 +4464,19 @@ def mock_update_broadcast_message_status( ) +@pytest.fixture +def mock_get_invited_user_by_id(mocker, sample_invite): + def _get( + invited_user_id + ): + return sample_invite + + return mocker.patch( + 'app.invite_api_client.get_invited_user', + side_effect=_get, + ) + + @pytest.fixture def mock_get_invited_org_user_by_id(mocker, sample_org_invite): def _get(