diff --git a/app/main/forms.py b/app/main/forms.py index 34a56bff1..1910f7574 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1001,7 +1001,7 @@ class BasePermissionsForm(StripWhitespaceForm): @classmethod def from_user(cls, user, service_id, **kwargs): - return cls( + form = cls( **kwargs, **{ "permissions_field": [ @@ -1009,6 +1009,16 @@ class BasePermissionsForm(StripWhitespaceForm): }, login_authentication=user.auth_type ) + # if the user is a platform admin then we don't allow you to change the login type. Unfortunately, since the + # login_authentication field is a radio field, the validation expects it to have a value, and the choices in it + # normally don't allow 'webauthn_auth'. + # If the user has webauthn_auth too, then we'll try and validate against that. We should never be changing + # auth of platform admin users, so just force the choices to be this. + # TODO: if the user is a regular user with webauthn_auth we will still show the radios, so if you edit that + # user's regular permissions you'll necessarily change their auth type as the value will be in the POST + if user.platform_admin: + del form.login_authentication + return form class PermissionsForm(BasePermissionsForm): diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 513b26780..764a278de 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -742,7 +742,7 @@ def test_cant_edit_non_member_user_permissions( assert mock_set_user_permissions.called is False -@pytest.mark.parametrize('auth_type', ['email_auth', 'sms_auth']) +@pytest.mark.parametrize('current_auth_type', ['webauthn_auth', 'email_auth']) def test_edit_user_permissions_including_authentication_with_email_auth_service( client_request, service_one, @@ -751,9 +751,10 @@ def test_edit_user_permissions_including_authentication_with_email_auth_service( mock_get_invites_for_service, mock_set_user_permissions, mock_update_user_attribute, - auth_type, - mock_get_template_folders + mock_get_template_folders, + current_auth_type, ): + active_user_with_permissions['auth_type'] = current_auth_type service_one['permissions'].append('email_auth') client_request.post( @@ -768,7 +769,7 @@ def test_edit_user_permissions_including_authentication_with_email_auth_service( 'manage_service', 'manage_api_keys', ], - 'login_authentication': auth_type, + 'login_authentication': 'sms_auth', }, _expected_status=302, _expected_redirect=url_for( @@ -791,11 +792,11 @@ def test_edit_user_permissions_including_authentication_with_email_auth_service( ) mock_update_user_attribute.assert_called_with( str(active_user_with_permissions['id']), - auth_type=auth_type + auth_type='sms_auth' ) -@pytest.mark.parametrize('auth_type', ['email_auth', 'sms_auth']) +@pytest.mark.parametrize('current_auth_type', ['webauthn_auth', 'email_auth']) def test_edit_user_permissions_preserves_auth_type_for_platform_admin( client_request, service_one, @@ -804,10 +805,14 @@ def test_edit_user_permissions_preserves_auth_type_for_platform_admin( mock_get_invites_for_service, mock_set_user_permissions, mock_update_user_attribute, - auth_type, - mock_get_template_folders + mock_get_template_folders, + current_auth_type, ): + platform_admin_user['auth_type'] = current_auth_type service_one['permissions'].append('email_auth') + + # we're logging in as this user being edited; normally a user can't edit themselves, but since + # the same mock is used to (a) check access and (b) find the user to edit, this is just easier client_request.login(platform_admin_user) client_request.post( @@ -817,7 +822,7 @@ def test_edit_user_permissions_preserves_auth_type_for_platform_admin( _data={ 'email_address': platform_admin_user['email_address'], 'permissions_field': [], - 'login_authentication': auth_type, + 'login_authentication': 'sms_auth', }, _expected_status=302, )