mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-05-27 09:29:22 -04:00
clean up logic around existing users accepting invites
* if the service issuing the invite does not have permission to edit auth types, don't let them do anything. This will stop them turning existing email_auth users back to sms auth * if the user hasn't got a mobile number, but the invite is for sms login, don't do anything either. They won't have a phone number if they signed up via an email_auth invite previously. in these cases, we accept the invite and add the user to the service as normal, however, just don't update the user's auth type.
This commit is contained in:
@@ -72,7 +72,15 @@ def accept_invite(token):
|
||||
if existing_user in service_users:
|
||||
return redirect(url_for('main.service_dashboard', service_id=invited_user.service))
|
||||
else:
|
||||
if invited_user.auth_type != existing_user.auth_type:
|
||||
service = service_api_client.get_service(invited_user.service)['data']
|
||||
# if the service you're being added to can modify auth type, then check if this is relevant
|
||||
if 'email_auth' in service['permissions'] and (
|
||||
# they have a phone number, we want them to start using it. if they dont have a mobile we just
|
||||
# ignore that option of the invite
|
||||
(existing_user.mobile_number and invited_user.auth_type == 'sms_auth') or
|
||||
# we want them to start sending emails. it's always valid, so lets always update
|
||||
invited_user.auth_type == 'email_auth'
|
||||
):
|
||||
user_api_client.update_user_attribute(existing_user.id, auth_type=invited_user.auth_type)
|
||||
user_api_client.add_user_to_service(invited_user.service,
|
||||
existing_user.id,
|
||||
|
||||
@@ -102,10 +102,7 @@ def edit_user_permissions(service_id, user_id):
|
||||
permissions=set(get_permissions_from_form(form)),
|
||||
)
|
||||
if service_has_email_auth:
|
||||
user_api_client.update_user_attribute(
|
||||
user_id,
|
||||
auth_type=form.login_authentication.data
|
||||
)
|
||||
user_api_client.update_user_attribute(user_id, auth_type=form.login_authentication.data)
|
||||
return redirect(url_for('.manage_users', service_id=service_id))
|
||||
|
||||
return render_template(
|
||||
|
||||
@@ -47,8 +47,7 @@ def user_profile_name():
|
||||
form = ChangeNameForm(new_name=current_user.name)
|
||||
|
||||
if form.validate_on_submit():
|
||||
user_api_client.update_user_attribute(current_user.id,
|
||||
name=form.new_name.data)
|
||||
user_api_client.update_user_attribute(current_user.id, name=form.new_name.data)
|
||||
return redirect(url_for('.user_profile'))
|
||||
|
||||
return render_template(
|
||||
@@ -114,8 +113,7 @@ def user_profile_email_confirm(token):
|
||||
token_data = json.loads(token_data)
|
||||
user_id = token_data['user_id']
|
||||
new_email = token_data['email']
|
||||
user_api_client.update_user_attribute(user_id,
|
||||
email_address=new_email)
|
||||
user_api_client.update_user_attribute(user_id, email_address=new_email)
|
||||
session.pop(NEW_EMAIL, None)
|
||||
|
||||
return redirect(url_for('.user_profile'))
|
||||
@@ -183,8 +181,7 @@ def user_profile_mobile_number_confirm():
|
||||
mobile_number = session[NEW_MOBILE]
|
||||
del session[NEW_MOBILE]
|
||||
del session[NEW_MOBILE_PASSWORD_CONFIRMED]
|
||||
user_api_client.update_user_attribute(current_user.id,
|
||||
mobile_number=mobile_number)
|
||||
user_api_client.update_user_attribute(current_user.id, mobile_number=mobile_number)
|
||||
return redirect(url_for('.user_profile'))
|
||||
|
||||
return render_template(
|
||||
|
||||
@@ -262,7 +262,7 @@
|
||||
{% endif %}
|
||||
<li class="bottom-gutter">
|
||||
<a href="{{ url_for('.service_switch_email_auth', service_id=current_service.id) }}" class="button">
|
||||
{{ 'Stop email auth' if 'email_auth' in current_service.permissions else 'Allow email auth' }}
|
||||
{{ 'Stop editing user auth' if 'email_auth' in current_service.permissions else 'Allow editing user auth' }}
|
||||
</a>
|
||||
</li>
|
||||
{% if current_service.active %}
|
||||
|
||||
@@ -4,8 +4,8 @@ from unittest.mock import ANY
|
||||
from itsdangerous import SignatureExpired
|
||||
|
||||
import app
|
||||
|
||||
from app.notify_client.models import InvitedUser
|
||||
|
||||
from tests.conftest import sample_invite as create_sample_invite
|
||||
from tests.conftest import mock_check_invite_token as mock_check_token_invite
|
||||
|
||||
@@ -19,6 +19,7 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(
|
||||
mock_get_users_by_service,
|
||||
mock_accept_invite,
|
||||
mock_add_user_to_service,
|
||||
mock_get_service,
|
||||
mocker,
|
||||
):
|
||||
mocker.patch('app.main.views.invites.check_token')
|
||||
@@ -47,6 +48,7 @@ def test_existing_user_with_no_permissions_accept_invite(
|
||||
mock_get_user_by_email,
|
||||
mock_get_users_by_service,
|
||||
mock_add_user_to_service,
|
||||
mock_get_service,
|
||||
):
|
||||
mocker.patch('app.main.views.invites.check_token')
|
||||
|
||||
@@ -397,7 +399,7 @@ def test_gives_message_if_token_has_expired(
|
||||
assert not mock_check_invite_token.called
|
||||
|
||||
|
||||
def test_existing_user_accept_sets_email_auth(
|
||||
def test_existing_user_accepts_and_sets_email_auth(
|
||||
client_request,
|
||||
api_user_active,
|
||||
service_one,
|
||||
@@ -411,11 +413,13 @@ def test_existing_user_accept_sets_email_auth(
|
||||
):
|
||||
mocker.patch('app.main.views.invites.check_token')
|
||||
sample_invite['email_address'] = api_user_active.email_address
|
||||
sample_invite['auth_type'] = 'email_auth'
|
||||
invited_user = InvitedUser(**sample_invite)
|
||||
mocker.patch('app.invite_api_client.check_token', return_value=invited_user)
|
||||
|
||||
response = client_request.get(
|
||||
service_one['permissions'].append('email_auth')
|
||||
sample_invite['auth_type'] = 'email_auth'
|
||||
mocker.patch('app.main.views.invites.service_api_client.get_service', return_value={'data': service_one})
|
||||
mocker.patch('app.invite_api_client.check_token', return_value=InvitedUser(**sample_invite))
|
||||
|
||||
client_request.get(
|
||||
'main.accept_invite',
|
||||
token='thisisnotarealtoken',
|
||||
_expected_status=302,
|
||||
@@ -424,3 +428,101 @@ def test_existing_user_accept_sets_email_auth(
|
||||
|
||||
mock_update_user_attribute.assert_called_with(api_user_active.id, auth_type='email_auth')
|
||||
mock_add_user_to_service.assert_called_with(ANY, api_user_active.id, ANY)
|
||||
|
||||
|
||||
def test_existing_user_doesnt_get_auth_changed_by_service_without_permission(
|
||||
client_request,
|
||||
api_user_active,
|
||||
service_one,
|
||||
sample_invite,
|
||||
mock_get_user_by_email,
|
||||
mock_get_users_by_service,
|
||||
mock_accept_invite,
|
||||
mock_update_user_attribute,
|
||||
mock_add_user_to_service,
|
||||
mocker
|
||||
):
|
||||
mocker.patch('app.main.views.invites.check_token')
|
||||
sample_invite['email_address'] = api_user_active.email_address
|
||||
|
||||
assert 'email_auth' not in service_one['permissions']
|
||||
|
||||
sample_invite['auth_type'] = 'email_auth'
|
||||
mocker.patch('app.main.views.invites.service_api_client.get_service', return_value={'data': service_one})
|
||||
mocker.patch('app.invite_api_client.check_token', return_value=InvitedUser(**sample_invite))
|
||||
|
||||
client_request.get(
|
||||
'main.accept_invite',
|
||||
token='thisisnotarealtoken',
|
||||
_expected_status=302,
|
||||
_expected_redirect=url_for('main.service_dashboard', service_id=service_one['id'], _external=True),
|
||||
)
|
||||
|
||||
assert not mock_update_user_attribute.called
|
||||
|
||||
|
||||
def test_existing_email_auth_user_without_phone_cannot_set_sms_auth(
|
||||
client_request,
|
||||
api_user_active,
|
||||
service_one,
|
||||
sample_invite,
|
||||
mock_get_users_by_service,
|
||||
mock_accept_invite,
|
||||
mock_update_user_attribute,
|
||||
mock_add_user_to_service,
|
||||
mocker
|
||||
):
|
||||
mocker.patch('app.main.views.invites.check_token')
|
||||
sample_invite['email_address'] = api_user_active.email_address
|
||||
|
||||
service_one['permissions'].append('email_auth')
|
||||
|
||||
api_user_active.auth_type = 'email_auth'
|
||||
api_user_active.mobile_number = None
|
||||
sample_invite['auth_type'] = 'sms_auth'
|
||||
|
||||
mocker.patch('app.main.views.invites.user_api_client.get_user_by_email', return_value=api_user_active)
|
||||
mocker.patch('app.main.views.invites.service_api_client.get_service', return_value={'data': service_one})
|
||||
mocker.patch('app.invite_api_client.check_token', return_value=InvitedUser(**sample_invite))
|
||||
|
||||
client_request.get(
|
||||
'main.accept_invite',
|
||||
token='thisisnotarealtoken',
|
||||
_expected_status=302,
|
||||
_expected_redirect=url_for('main.service_dashboard', service_id=service_one['id'], _external=True),
|
||||
)
|
||||
|
||||
assert not mock_update_user_attribute.called
|
||||
|
||||
|
||||
def test_existing_email_auth_user_with_phone_can_set_sms_auth(
|
||||
client_request,
|
||||
api_user_active,
|
||||
service_one,
|
||||
sample_invite,
|
||||
mock_get_users_by_service,
|
||||
mock_accept_invite,
|
||||
mock_update_user_attribute,
|
||||
mock_add_user_to_service,
|
||||
mocker
|
||||
):
|
||||
mocker.patch('app.main.views.invites.check_token')
|
||||
sample_invite['email_address'] = api_user_active.email_address
|
||||
|
||||
service_one['permissions'].append('email_auth')
|
||||
sample_invite['auth_type'] = 'sms_auth'
|
||||
api_user_active.auth_type = 'email_auth'
|
||||
api_user_active.mobile_number = '07700900001'
|
||||
|
||||
mocker.patch('app.main.views.invites.user_api_client.get_user_by_email', return_value=api_user_active)
|
||||
mocker.patch('app.main.views.invites.service_api_client.get_service', return_value={'data': service_one})
|
||||
mocker.patch('app.invite_api_client.check_token', return_value=InvitedUser(**sample_invite))
|
||||
|
||||
client_request.get(
|
||||
'main.accept_invite',
|
||||
token='thisisnotarealtoken',
|
||||
_expected_status=302,
|
||||
_expected_redirect=url_for('main.service_dashboard', service_id=service_one['id'], _external=True),
|
||||
)
|
||||
|
||||
mock_update_user_attribute.assert_called_with(api_user_active.id, auth_type='sms_auth')
|
||||
|
||||
Reference in New Issue
Block a user