mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-09-06 18:48:24 -04:00
Use API flag to give users access to WebAuthn
This allows us to roll out the feature to other users. Note that the flag is also "True" if the user has "webauthn_auth" as their auth type, so this is compatible with the more fine-grained check we have on the authentication parts of the feature. We could do a more explicit "can_use_webauthn or webauthn_auth" check here, but the idea is that we'll be able to get rid of this flag eventually, so I've optimised for brevity instead. I've modified a couple of the unhappy-path tests to make it more explicit that the flag is false, since it can be true for Platform Admins and "normal users" alike.
This commit is contained in:
@@ -27,11 +27,7 @@ from app.main.forms import (
|
|||||||
TwoFactorForm,
|
TwoFactorForm,
|
||||||
)
|
)
|
||||||
from app.models.user import User
|
from app.models.user import User
|
||||||
from app.utils.user import (
|
from app.utils.user import user_is_gov_user, user_is_logged_in
|
||||||
user_is_gov_user,
|
|
||||||
user_is_logged_in,
|
|
||||||
user_is_platform_admin,
|
|
||||||
)
|
|
||||||
|
|
||||||
NEW_EMAIL = 'new-email'
|
NEW_EMAIL = 'new-email'
|
||||||
NEW_MOBILE = 'new-mob'
|
NEW_MOBILE = 'new-mob'
|
||||||
@@ -236,8 +232,10 @@ def user_profile_disable_platform_admin_view():
|
|||||||
|
|
||||||
|
|
||||||
@main.route("/user-profile/security-keys", methods=['GET'])
|
@main.route("/user-profile/security-keys", methods=['GET'])
|
||||||
@user_is_platform_admin
|
|
||||||
def user_profile_security_keys():
|
def user_profile_security_keys():
|
||||||
|
if not current_user.can_use_webauthn:
|
||||||
|
abort(403)
|
||||||
|
|
||||||
return render_template(
|
return render_template(
|
||||||
'views/user-profile/security-keys.html',
|
'views/user-profile/security-keys.html',
|
||||||
)
|
)
|
||||||
@@ -253,8 +251,10 @@ def user_profile_security_keys():
|
|||||||
methods=['GET'],
|
methods=['GET'],
|
||||||
endpoint="user_profile_confirm_delete_security_key"
|
endpoint="user_profile_confirm_delete_security_key"
|
||||||
)
|
)
|
||||||
@user_is_platform_admin
|
|
||||||
def user_profile_manage_security_key(key_id):
|
def user_profile_manage_security_key(key_id):
|
||||||
|
if not current_user.can_use_webauthn:
|
||||||
|
abort(403)
|
||||||
|
|
||||||
security_key = current_user.webauthn_credentials.by_id(key_id)
|
security_key = current_user.webauthn_credentials.by_id(key_id)
|
||||||
|
|
||||||
if not security_key:
|
if not security_key:
|
||||||
@@ -282,8 +282,9 @@ def user_profile_manage_security_key(key_id):
|
|||||||
|
|
||||||
|
|
||||||
@main.route("/user-profile/security-keys/<uuid:key_id>/delete", methods=['POST'])
|
@main.route("/user-profile/security-keys/<uuid:key_id>/delete", methods=['POST'])
|
||||||
@user_is_platform_admin
|
|
||||||
def user_profile_delete_security_key(key_id):
|
def user_profile_delete_security_key(key_id):
|
||||||
|
if not current_user.can_use_webauthn:
|
||||||
|
abort(403)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
user_api_client.delete_webauthn_credential_for_user(
|
user_api_client.delete_webauthn_credential_for_user(
|
||||||
|
|||||||
@@ -14,12 +14,13 @@ from app.utils.login import (
|
|||||||
log_in_user,
|
log_in_user,
|
||||||
redirect_to_sign_in,
|
redirect_to_sign_in,
|
||||||
)
|
)
|
||||||
from app.utils.user import user_is_platform_admin
|
|
||||||
|
|
||||||
|
|
||||||
@main.route('/webauthn/register')
|
@main.route('/webauthn/register')
|
||||||
@user_is_platform_admin
|
|
||||||
def webauthn_begin_register():
|
def webauthn_begin_register():
|
||||||
|
if not current_user.can_use_webauthn:
|
||||||
|
abort(403)
|
||||||
|
|
||||||
server = current_app.webauthn_server
|
server = current_app.webauthn_server
|
||||||
|
|
||||||
registration_data, state = server.register_begin(
|
registration_data, state = server.register_begin(
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ class User(JSONModel, UserMixin):
|
|||||||
MAX_FAILED_LOGIN_COUNT = 10
|
MAX_FAILED_LOGIN_COUNT = 10
|
||||||
|
|
||||||
ALLOWED_PROPERTIES = {
|
ALLOWED_PROPERTIES = {
|
||||||
|
'can_use_webauthn',
|
||||||
'id',
|
'id',
|
||||||
'name',
|
'name',
|
||||||
'email_address',
|
'email_address',
|
||||||
|
|||||||
@@ -45,7 +45,7 @@
|
|||||||
{{ edit_field('Change', url_for('.user_profile_password')) }}
|
{{ edit_field('Change', url_for('.user_profile_password')) }}
|
||||||
{% endcall %}
|
{% endcall %}
|
||||||
|
|
||||||
{% if current_user.platform_admin %}
|
{% if current_user.can_use_webauthn %}
|
||||||
{% call row(id='security-keys') %}
|
{% call row(id='security-keys') %}
|
||||||
{{ text_field('Security keys') }}
|
{{ text_field('Security keys') }}
|
||||||
{{ optional_text_field(
|
{{ optional_text_field(
|
||||||
|
|||||||
@@ -44,7 +44,7 @@ def test_overview_page_shows_disable_for_platform_admin(
|
|||||||
(1, 'Security keys 1 registered Change'),
|
(1, 'Security keys 1 registered Change'),
|
||||||
(2, 'Security keys 2 registered Change'),
|
(2, 'Security keys 2 registered Change'),
|
||||||
])
|
])
|
||||||
def test_overview_page_shows_security_keys_for_platform_admin(
|
def test_overview_page_shows_security_keys_if_user_they_can_use_webauthn(
|
||||||
mocker,
|
mocker,
|
||||||
client_request,
|
client_request,
|
||||||
platform_admin_user,
|
platform_admin_user,
|
||||||
@@ -358,7 +358,13 @@ def test_can_reenable_platform_admin(client_request, platform_admin_user):
|
|||||||
assert session['disable_platform_admin_view'] is False
|
assert session['disable_platform_admin_view'] is False
|
||||||
|
|
||||||
|
|
||||||
def test_normal_user_doesnt_see_security_keys(client_request):
|
def test_user_doesnt_see_security_keys_unless_they_can_use_webauthn(
|
||||||
|
client_request,
|
||||||
|
platform_admin_user
|
||||||
|
):
|
||||||
|
platform_admin_user['can_use_webauthn'] = False
|
||||||
|
client_request.login(platform_admin_user)
|
||||||
|
|
||||||
client_request.get(
|
client_request.get(
|
||||||
'.user_profile_security_keys',
|
'.user_profile_security_keys',
|
||||||
_expected_status=403,
|
_expected_status=403,
|
||||||
@@ -455,9 +461,16 @@ def test_manage_security_key_page_404s_when_key_not_found(
|
|||||||
(".user_profile_confirm_delete_security_key", "post"),
|
(".user_profile_confirm_delete_security_key", "post"),
|
||||||
(".user_profile_delete_security_key", "post"),
|
(".user_profile_delete_security_key", "post"),
|
||||||
])
|
])
|
||||||
def test_non_platform_admin_user_cant_manage_security_keys(
|
def test_cant_manage_security_keys_unless_can_use_webauthn(
|
||||||
client_request, webauthn_credential, endpoint, method
|
client_request,
|
||||||
|
platform_admin_user,
|
||||||
|
webauthn_credential,
|
||||||
|
endpoint,
|
||||||
|
method
|
||||||
):
|
):
|
||||||
|
platform_admin_user['can_use_webauthn'] = False
|
||||||
|
client_request.login(platform_admin_user)
|
||||||
|
|
||||||
if method == "get":
|
if method == "get":
|
||||||
client_request.get(
|
client_request.get(
|
||||||
endpoint,
|
endpoint,
|
||||||
|
|||||||
@@ -33,14 +33,14 @@ def webauthn_authentication_post_data(fake_uuid, webauthn_credential, client):
|
|||||||
})
|
})
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize('endpoint', [
|
def test_begin_register_forbidden_unless_can_use_webauthn(
|
||||||
'webauthn_begin_register',
|
|
||||||
])
|
|
||||||
def test_register_forbidden_for_non_platform_admins(
|
|
||||||
client_request,
|
client_request,
|
||||||
endpoint,
|
platform_admin_user,
|
||||||
|
mocker,
|
||||||
):
|
):
|
||||||
client_request.get(f'main.{endpoint}', _expected_status=403)
|
platform_admin_user['can_use_webauthn'] = False
|
||||||
|
mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
|
||||||
|
client_request.get('main.webauthn_begin_register', _expected_status=403)
|
||||||
|
|
||||||
|
|
||||||
def test_begin_register_returns_encoded_options(
|
def test_begin_register_returns_encoded_options(
|
||||||
|
|||||||
@@ -3636,6 +3636,7 @@ def create_platform_admin_user(with_unique_id=False, auth_type='webauthn_auth',
|
|||||||
permissions=permissions or {},
|
permissions=permissions or {},
|
||||||
platform_admin=True,
|
platform_admin=True,
|
||||||
auth_type=auth_type,
|
auth_type=auth_type,
|
||||||
|
can_use_webauthn=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -3681,7 +3682,8 @@ def create_user(**overrides):
|
|||||||
'organisations': [],
|
'organisations': [],
|
||||||
'current_session_id': None,
|
'current_session_id': None,
|
||||||
'logged_in_at': None,
|
'logged_in_at': None,
|
||||||
'email_access_validated_at': None
|
'email_access_validated_at': None,
|
||||||
|
'can_use_webauthn': False,
|
||||||
}
|
}
|
||||||
user_data.update(overrides)
|
user_data.update(overrides)
|
||||||
return user_data
|
return user_data
|
||||||
|
|||||||
Reference in New Issue
Block a user