From 661b618c4a3f0077f0002af4897cacd063e31725 Mon Sep 17 00:00:00 2001 From: Son Tung Pham Date: Thu, 19 Oct 2023 13:34:27 +0200 Subject: [PATCH] Make get_user_data configurable --- CHANGELOG.md | 1 + README.md | 18 ++++++++++++++++++ django_fido/settings.py | 4 ++++ django_fido/tests/test_utils.py | 26 ++++++++++++++++++++++++++ django_fido/tests/test_views.py | 13 +++++++++++++ django_fido/tests/utils.py | 13 +++++++++++++ django_fido/utils.py | 20 ++++++++++++++++++++ django_fido/views.py | 29 ++++++++++++++++++++++++++--- 8 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 django_fido/tests/test_utils.py create mode 100644 django_fido/utils.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ea66fc01..2129841b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ * Removed support for Django 3.0 and 3.1 * Added support for Django 4.1 and 4.2 * Added support for Python 3.11 + * Added callable settings for get_user_data configurability ## 1.2.1 * Fixed metadata lookup for devices with device certificate directly in MDS diff --git a/README.md b/README.md index 09dd10f1..04c44ac4 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,24 @@ Default: False Purpose: Set to True to enable discoverable credentials, private key and associated metadata is stored in persistent memory on the authenticator. This is useful for passwordless authentication. +#### DJANGO_FIDO_GET_USER_ID_CALLABLE #### +Optional, default: None + +String path to callable that takes one argument `User`. +Overrides default implementation of Fido2RegistrationRequestView.get_user_id. + +#### DJANGO_FIDO_GET_USER_DISPLAY_NAME_CALLABLE #### +Optional, default: None + +String path to callable that takes one argument `User`. +Overrides default implementation of Fido2RegistrationRequestView.get_user_display_name. + +#### DJANGO_FIDO_GET_USERNAME_CALLABLE #### +Optional, default: None + +String path to callable that takes one argument `User`. +Overrides default implementation of Fido2RegistrationRequestView.get_username. + ## One step authentication You can also decide to use one step authentication. diff --git a/django_fido/settings.py b/django_fido/settings.py index 8d4c3210..aa2de535 100644 --- a/django_fido/settings.py +++ b/django_fido/settings.py @@ -38,6 +38,10 @@ class DjangoFidoSettings(AppSettings): resident_key = BooleanSetting(default=False) passwordless_auth = BooleanSetting(default=False) user_verification = StringSetting(default=None) + get_user_id_callable = CallablePathSetting(required=False, default=None) + get_user_display_name_callable = CallablePathSetting(required=False, default=None) + get_username_callable = CallablePathSetting(required=False, default=None) + @classmethod def check(cls): diff --git a/django_fido/tests/test_utils.py b/django_fido/tests/test_utils.py new file mode 100644 index 00000000..fc1a6d5e --- /dev/null +++ b/django_fido/tests/test_utils.py @@ -0,0 +1,26 @@ +"""Tests for utils module.""" +from django.contrib.auth import get_user_model +from django.test import TestCase + +from django_fido.utils import process_callable + +from .data import USER_FIRST_NAME, USER_LAST_NAME, USERNAME +from .utils import helper_str + +User = get_user_model() + + +class ProcessCallableTest(TestCase): + """Tests for process_callable.""" + + def setUp(self): + self.user = User.objects.create_user(USERNAME, first_name=USER_FIRST_NAME, last_name=USER_LAST_NAME) + + def test_call_path(self): + self.assertEqual(process_callable("django_fido.tests.utils.helper_str", self.user), USER_LAST_NAME) + + def test_call_callable(self): + self.assertEqual(process_callable(helper_str, self.user), USER_LAST_NAME) + + def test_no_item(self): + self.assertEqual(process_callable(None, self.user), None) diff --git a/django_fido/tests/test_views.py b/django_fido/tests/test_views.py index 91a7b857..c8e93635 100644 --- a/django_fido/tests/test_views.py +++ b/django_fido/tests/test_views.py @@ -12,6 +12,7 @@ from django_fido.constants import AUTHENTICATION_USER_SESSION_KEY, FIDO2_REQUEST_SESSION_KEY from django_fido.models import Authenticator +from django_fido.views import Fido2RegistrationRequestView from .data import (ATTESTATION_OBJECT, ATTESTATION_OBJECT_BOGUS, ATTESTATION_OBJECT_U2F_MALFORMED, AUTHENTICATION_CHALLENGE, AUTHENTICATION_CLIENT_DATA, AUTHENTICATOR_DATA, CREDENTIAL_ID, HOSTNAME, @@ -101,6 +102,18 @@ def test_get_registered_keys(self): credentials = [{'id': CREDENTIAL_ID, 'type': 'public-key'}] self.assertEqual(response.json(), self._get_fido2_request(challenge, credentials)) + @override_settings(DJANGO_FIDO_GET_USER_ID_CALLABLE="django_fido.tests.utils.helper_bytes") + def test_get_user_id_overridden(self): + self.assertEqual(Fido2RegistrationRequestView.get_user_id(self.user), b"2X4B-523P") + + @override_settings(DJANGO_FIDO_GET_USER_DISPLAY_NAME_CALLABLE="django_fido.tests.utils.helper_str") + def test_get_user_display_name_overridden(self): + self.assertEqual(Fido2RegistrationRequestView.get_user_display_name(self.user), USER_LAST_NAME) + + @override_settings(DJANGO_FIDO_GET_USERNAME_CALLABLE="django_fido.tests.utils.helper_str") + def test_get_username_overridden(self): + self.assertEqual(Fido2RegistrationRequestView.get_username(self.user), USER_LAST_NAME) + @override_settings(ROOT_URLCONF='django_fido.tests.urls', TEMPLATES=TEMPLATES) class TestFido2RegistrationView(TestCase): diff --git a/django_fido/tests/utils.py b/django_fido/tests/utils.py index 481a0443..ba597867 100644 --- a/django_fido/tests/utils.py +++ b/django_fido/tests/utils.py @@ -1,4 +1,11 @@ """Test utilities.""" +from typing import TYPE_CHECKING + +if TYPE_CHECKING: # pragma: no branch + from django.contrib.auth import get_user_model + + User = get_user_model() + TEMPLATES = [ { 'BACKEND': 'django.template.backends.django.DjangoTemplates', @@ -11,3 +18,9 @@ }, }, ] + +def helper_str(user: "User"): + return user.last_name + +def helper_bytes(user: "User"): + return bytes(user.last_name, "utf-8") diff --git a/django_fido/utils.py b/django_fido/utils.py new file mode 100644 index 00000000..c98d817f --- /dev/null +++ b/django_fido/utils.py @@ -0,0 +1,20 @@ +"""Utilities for django_fido.""" +from typing import Any, Callable, Union + +from django.utils.module_loading import import_string + + +def process_callable(item: Union[Callable, str, None], *args: Any, **kwargs: Any) -> Any: + """Call supplied callable or dotted path source. + + @raise ImportError: If supplied item[str] is not an importable dotted path. + """ + + if not item: + return + + if isinstance(item, str): + return import_string(item)(*args, **kwargs) + + if callable(item): + return item(*args, **kwargs) diff --git a/django_fido/views.py b/django_fido/views.py index 1a9f87f0..baa9782b 100644 --- a/django_fido/views.py +++ b/django_fido/views.py @@ -20,6 +20,7 @@ from django.shortcuts import redirect from django.urls import reverse_lazy from django.utils.encoding import force_str +from django.utils.module_loading import import_string from django.utils.translation import gettext_lazy as _ from django.views.generic import FormView, View from fido2.attestation import Attestation, AttestationVerifier, UnsupportedType @@ -29,6 +30,8 @@ from fido2.webauthn import (AttestationConveyancePreference, AttestedCredentialData, AuthenticatorData, PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, ResidentKeyRequirement) +from django_fido.utils import process_callable + from .constants import (AUTHENTICATION_USER_SESSION_KEY, FIDO2_AUTHENTICATION_REQUEST, FIDO2_REGISTRATION_REQUEST, FIDO2_REQUEST_SESSION_KEY) from .forms import (Fido2AuthenticationForm, Fido2ModelAuthenticationForm, Fido2PasswordlessAuthenticationForm, @@ -174,8 +177,9 @@ def get_user(self): """Return user which is subject of the request.""" return self.request.user - def get_user_id(self, user: AbstractBaseUser) -> bytes: - """Return a unique, persistent identifier of a user. + @staticmethod + def get_user_id(user: AbstractBaseUser) -> bytes: + """Return a unique, persistent identifier of a user. Prefer settings callable over default implementation. Default implementation return user's username, but it is only secure if the username can't be reused. In such case, it is required to provide another identifier which would differentiate users. @@ -186,12 +190,31 @@ def get_user_id(self, user: AbstractBaseUser) -> bytes: """ if SETTINGS.resident_key: return uuid.uuid4().bytes + user_id = process_callable(SETTINGS.get_user_id_callable, user) + if user_id is not None: + return user_id return bytes(user.username, encoding="utf-8") + @staticmethod + def get_user_display_name(user: AbstractBaseUser) -> str: + """Retrieve user display name. Prefer settings callable over default implementation.""" + display_name = process_callable(SETTINGS.get_user_display_name_callable, user) + if display_name is not None: + return display_name + return user.get_full_name() or user.username + + @staticmethod + def get_username(user: AbstractBaseUser) -> str: + """Retrieve user username. Prefer settings callable over default implementation.""" + username = process_callable(SETTINGS.get_username_callable, user) + if username is not None: + return username + return user.username + def get_user_data(self, user: AbstractBaseUser) -> PublicKeyCredentialUserEntity: """Convert user instance to user data for registration.""" return PublicKeyCredentialUserEntity( - user.username, self.get_user_id(user), user.get_full_name() or user.username + self.get_username(user), self.get_user_id(user), self.get_user_display_name(user) ) def create_fido2_request(self) -> Tuple[Mapping[str, Any], Any]: