Prevent switching auth type for Platform Admins

This closes a security loophole, where the auth type of a Platform
Admin could be unwittingly changed when they accept an invite, or
by an admin of a service they are a member of.

app/main/views/invites.py

@@ -57,15 +57,16 @@ def accept_invite(token):
             return redirect(url_for('main.service_dashboard', service_id=invited_user.service))
         else:
             service = Service.from_id(invited_user.service)
-            # if the service you're being added to can modify auth type, then check if this is relevant
+            # if the service you're being added to can modify auth type, then check if we can do this;
-            if service.has_permission('email_auth') and (
+            # if the user is a Platform Admin, we silently leave this unchanged to prevent a security
-                # they have a phone number, we want them to start using it. if they dont have a mobile we just
+            # issue where someone could switch their auth type to something less secure
-                # ignore that option of the invite
+            if service.has_permission('email_auth') and not existing_user.platform_admin:
-                (existing_user.mobile_number and invited_user.auth_type == 'sms_auth') or
+                if invited_user.auth_type == 'email_auth' or (
-                # we want them to start sending emails. it's always valid, so lets always update
+                    # they have a phone number, we want them to start using it.
-                invited_user.auth_type == 'email_auth'
+                    # 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'
-                existing_user.update(auth_type=invited_user.auth_type)
+                ):
+                    existing_user.update(auth_type=invited_user.auth_type)
             existing_user.add_to_service(
                 service_id=invited_user.service,
                 permissions=invited_user.permissions,

app/main/views/manage_users.py

@@ -142,7 +142,9 @@ def edit_user_permissions(service_id, user_id):
             permissions=form.permissions,
             folder_permissions=form.folder_permissions.data,
         )
-        if service_has_email_auth:
+        # only change the auth type if this is supported for a service; for Platform Admin users,
+        # we avoid changing the auth type to prevent it being switched to something less secure
+        if service_has_email_auth and not user.platform_admin:
             user.update(auth_type=form.login_authentication.data)
         return redirect(url_for('.manage_users', service_id=service_id))
 

app/templates/views/manage-users/permissions.html

@@ -11,7 +11,11 @@
 {% endif %}
 
 {% if service_has_email_auth %}
-  {% if not mobile_number %}
+  {% if user.platform_admin %}
+    <p class="bottom-gutter">
+      Platform admin users will login with a security key.
+    </p>
+  {% elif not mobile_number %}
     {{ radios(
       form.login_authentication,
       disable=['sms_auth'],

tests/app/main/views/test_accept_invite.py

@@ -1,4 +1,5 @@
 from unittest.mock import ANY, Mock
+from uuid import uuid4
 
 import pytest
 from bs4 import BeautifulSoup
@@ -690,6 +691,43 @@ def test_existing_user_accepts_and_sets_email_auth(
     mock_add_user_to_service.assert_called_once_with(ANY, USER_ONE_ID, ANY, ANY)
 
 
+def test_platform_admin_user_accepts_and_preserves_auth(
+    client_request,
+    platform_admin_user,
+    service_one,
+    sample_invite,
+    mock_get_users_by_service,
+    mock_accept_invite,
+    mock_update_user_attribute,
+    mock_add_user_to_service,
+    mocker
+):
+    sample_invite['email_address'] = platform_admin_user['email_address']
+    sample_invite['auth_type'] = 'email_auth'
+    service_one['permissions'].append('email_auth')
+
+    # mock_get_users_by_service uses the same global UUID as the platform admin user,
+    # so we need to reset it to pretend this user isn't a member of the service
+    platform_admin_user['id'] = uuid4()
+    platform_admin_user['auth_type'] = 'webauthn_auth'
+
+    # mock_get_unknown_user_by_email returns the active_api_user, which we don't want
+    mocker.patch('app.user_api_client.get_user_by_email', return_value=platform_admin_user)
+    mocker.patch('app.invite_api_client.check_token', return_value=sample_invite)
+
+    client_request.login(platform_admin_user)
+
+    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
+    assert mock_add_user_to_service.called
+
+
 def test_existing_user_doesnt_get_auth_changed_by_service_without_permission(
     client_request,
     api_user_active,

tests/app/main/views/test_manage_users.py

@@ -795,6 +795,42 @@ def test_edit_user_permissions_including_authentication_with_email_auth_service(
     )
 
 
+@pytest.mark.parametrize('auth_type', ['email_auth', 'sms_auth'])
+def test_edit_user_permissions_preserves_auth_type_for_platform_admin(
+    client_request,
+    service_one,
+    platform_admin_user,
+    mock_get_users_by_service,
+    mock_get_invites_for_service,
+    mock_set_user_permissions,
+    mock_update_user_attribute,
+    auth_type,
+    mock_get_template_folders
+):
+    service_one['permissions'].append('email_auth')
+    client_request.login(platform_admin_user)
+
+    client_request.post(
+        'main.edit_user_permissions',
+        service_id=SERVICE_ONE_ID,
+        user_id=platform_admin_user['id'],
+        _data={
+            'email_address': platform_admin_user['email_address'],
+            'permissions_field': [],
+            'login_authentication': auth_type,
+        },
+        _expected_status=302,
+    )
+
+    mock_set_user_permissions.assert_called_with(
+        str(platform_admin_user['id']),
+        SERVICE_ONE_ID,
+        permissions=set(),
+        folder_permissions=[],
+    )
+    mock_update_user_attribute.assert_not_called()
+
+
 def test_should_show_page_for_inviting_user(
     client_request,
     mock_get_template_folders,