diff --git a/api/app/settings/common.py b/api/app/settings/common.py index 2f1e468eb4d3..156f3f1b4a95 100644 --- a/api/app/settings/common.py +++ b/api/app/settings/common.py @@ -804,10 +804,10 @@ # FE uri to redirect user to from activation email "ACTIVATION_URL": "activate/{uid}/{token}", # register or activation endpoint will send confirmation email to user + "LOGIN_FIELD": "email", "SEND_CONFIRMATION_EMAIL": False, "SERIALIZERS": { "token": "custom_auth.serializers.CustomTokenSerializer", - "token_create": "custom_auth.serializers.CustomTokenCreateSerializer", "user_create": "custom_auth.serializers.CustomUserCreateSerializer", "user_delete": "custom_auth.serializers.CustomUserDelete", "current_user": "users.serializers.CustomCurrentUserSerializer", @@ -1153,8 +1153,9 @@ LDAP_INSTALLED = importlib.util.find_spec("flagsmith_ldap") # The URL of the LDAP server. LDAP_AUTH_URL = env.str("LDAP_AUTH_URL", None) +LDAP_ENABLED = LDAP_INSTALLED and LDAP_AUTH_URL -if LDAP_INSTALLED and LDAP_AUTH_URL: # pragma: no cover +if LDAP_ENABLED: # pragma: no cover AUTHENTICATION_BACKENDS.insert(0, "django_python3_ldap.auth.LDAPBackend") INSTALLED_APPS.append("flagsmith_ldap") @@ -1227,7 +1228,6 @@ # The LDAP user username and password used by `sync_ldap_users_and_groups` command LDAP_SYNC_USER_USERNAME = env.str("LDAP_SYNC_USER_USERNAME", None) LDAP_SYNC_USER_PASSWORD = env.str("LDAP_SYNC_USER_PASSWORD", None) - DJOSER["LOGIN_FIELD"] = "username" SEGMENT_CONDITION_VALUE_LIMIT = env.int("SEGMENT_CONDITION_VALUE_LIMIT", default=1000) if not 0 <= SEGMENT_CONDITION_VALUE_LIMIT < 2000000: diff --git a/api/custom_auth/serializers.py b/api/custom_auth/serializers.py index d1322c1d50be..238fe17adb96 100644 --- a/api/custom_auth/serializers.py +++ b/api/custom_auth/serializers.py @@ -1,8 +1,7 @@ from typing import Any from django.conf import settings -from djoser.conf import settings as djoser_settings -from djoser.serializers import TokenCreateSerializer, UserCreateSerializer +from djoser.serializers import UserCreateSerializer from rest_framework import serializers from rest_framework.authtoken.models import Token from rest_framework.exceptions import PermissionDenied @@ -20,33 +19,6 @@ ) -class CustomTokenCreateSerializer(TokenCreateSerializer): - """ - NOTE: Some authentication backends (e.g., LDAP) support only - username and password authentication. However, the front-end - currently sends the email as the login key. To accommodate - this, we override the serializer to rename the username field - to the email (or any other field configurable using djoser settings) field. - - """ - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - if djoser_settings.LOGIN_FIELD != FFAdminUser.USERNAME_FIELD: - # Because djoser have created a field named username(djoser_settings.LOGIN_FIELD) in the serializer - # We have to remove this and add the email(FFAdminUser.USERNAME_FIELD) field back - self.fields.pop(djoser_settings.LOGIN_FIELD) - self.fields[FFAdminUser.USERNAME_FIELD] = serializers.CharField( - required=False - ) - - def validate(self, attrs): - if djoser_settings.LOGIN_FIELD != FFAdminUser.USERNAME_FIELD: - attrs[djoser_settings.LOGIN_FIELD] = attrs.pop(FFAdminUser.USERNAME_FIELD) - - return super().validate(attrs) - - class CustomTokenSerializer(serializers.ModelSerializer): class Meta: model = Token diff --git a/api/poetry.lock b/api/poetry.lock index 85b3b132af1a..379fbfc7f6c6 100644 --- a/api/poetry.lock +++ b/api/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.5 and should not be changed by hand. [[package]] name = "annotated-types" @@ -1126,13 +1126,13 @@ test = ["cryptography", "freezegun", "pytest", "pytest-cov", "pytest-django", "p [[package]] name = "djoser" -version = "2.2.2" +version = "2.3.0" description = "REST implementation of Django authentication system." optional = false -python-versions = ">=3.8,<4.0" +python-versions = "<4.0,>=3.8" files = [ - {file = "djoser-2.2.2-py3-none-any.whl", hash = "sha256:efb91ad61e4d5b8d664db029b5947df9d34078289ef2680a1ab665e047144b74"}, - {file = "djoser-2.2.2.tar.gz", hash = "sha256:9deb831a1c8781ceff325699e1407b4e1be8b4588e87071621d88ba31c09349f"}, + {file = "djoser-2.3.0-py3-none-any.whl", hash = "sha256:6af76cb8d73f8a879ab417c1251f5aa0242c4e3c4a8fee3a6393f1126874b269"}, + {file = "djoser-2.3.0.tar.gz", hash = "sha256:c46e4b609348b824ba138aba914ca8a89bb42b7d23975c34e2702ea04b13c989"}, ] [package.dependencies] @@ -3353,6 +3353,7 @@ files = [ {file = "PyYAML-6.0.1-cp311-cp311-win_amd64.whl", hash = "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34"}, {file = "PyYAML-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28"}, {file = "PyYAML-6.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9"}, + {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a08c6f0fe150303c1c6b71ebcd7213c2858041a7e01975da3a99aed1e7a378ef"}, {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0"}, {file = "PyYAML-6.0.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4"}, {file = "PyYAML-6.0.1-cp312-cp312-win32.whl", hash = "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54"}, @@ -4216,4 +4217,4 @@ files = [ [metadata] lock-version = "2.0" python-versions = ">=3.11, <3.13" -content-hash = "3a490ddf78d714b0fb4394a3f384856576d1db5b5c9ad270a29d1434aa2562f6" +content-hash = "2b81530cb56fddb93c5bc5640ab3bbab2da3f0d273311b65317f9e4b202adcb0" diff --git a/api/pyproject.toml b/api/pyproject.toml index c808f8b6f967..56cf7f6f1e0c 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -153,7 +153,7 @@ pymemcache = "~4.0.0" google-re2 = "^1.0" django-softdelete = "~0.10.5" simplejson = "~3.19.1" -djoser = "~2.2.2" +djoser = "~2.3.0" django-storages = "~1.10.1" django-environ = "~0.4.5" influxdb-client = "~1.28.0" diff --git a/api/tests/unit/custom_auth/test_unit_custom_auth_serializer.py b/api/tests/unit/custom_auth/test_unit_custom_auth_serializer.py index 922cff3a133f..37872a8e9425 100644 --- a/api/tests/unit/custom_auth/test_unit_custom_auth_serializer.py +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_serializer.py @@ -1,7 +1,9 @@ -from copy import deepcopy +import typing import pytest +from django.contrib.auth.models import AbstractUser from django.test import RequestFactory +from djoser.serializers import TokenCreateSerializer from pytest_django.fixtures import SettingsWrapper from pytest_mock import MockerFixture from rest_framework.exceptions import PermissionDenied @@ -9,10 +11,7 @@ from custom_auth.constants import ( USER_REGISTRATION_WITHOUT_INVITE_ERROR_MESSAGE, ) -from custom_auth.serializers import ( - CustomTokenCreateSerializer, - CustomUserCreateSerializer, -) +from custom_auth.serializers import CustomUserCreateSerializer from organisations.invites.models import InviteLink from users.models import FFAdminUser, SignUpType @@ -153,23 +152,44 @@ def test_invite_link_validation_mixin_validate_fails_if_invite_link_hash_not_val assert exc_info.value.detail == USER_REGISTRATION_WITHOUT_INVITE_ERROR_MESSAGE -def test_CustomTokenCreateSerializer_validate_uses_login_field_to_authenticate( - settings: SettingsWrapper, mocker: MockerFixture +@pytest.fixture +def django_ldap_username_field( + django_user_model: type[AbstractUser], +) -> typing.Generator[str, None, None]: + ldap_username_field = "username" + username_field = django_user_model.USERNAME_FIELD + django_user_model.USERNAME_FIELD = ldap_username_field + yield ldap_username_field + django_user_model.USERNAME_FIELD = username_field + + +# Previously, Djoser's default `TokenCreateSerializer` only respected the +# Djoser `LOGIN_FIELD` setting so it was impossible to grab the email from +# serializer and send it as a username to the login backend — which is required for LDAP to work. +# After Djoser 2.3.0, it's possible to alter `TokenCreateSerializer` behaviour +# to support this by setting the Django user model's `USERNAME_FIELD` constant +# to `"username"`, and Djoser's `LOGIN_FIELD` to `"email"`. +# This test is here to make sure Djoser behaves as expected. +def test_djoser_token_create_serializer__user_model_username_field__call_expected( + mocker: MockerFixture, + django_ldap_username_field: str, ) -> None: # Given - djoser_settings = deepcopy(settings.DJOSER) - djoser_settings["LOGIN_FIELD"] = "username" - settings.DJOSER = djoser_settings + expected_username = "some_username" + expected_password = "some_password" mocked_authenticate = mocker.patch("djoser.serializers.authenticate") - serializer = CustomTokenCreateSerializer( - data={"email": "some_username", "password": "some_password"} + serializer = TokenCreateSerializer( + data={"email": expected_username, "password": expected_password} ) + expected_authenticate_kwargs = { + "request": None, + django_ldap_username_field: expected_username, + "password": expected_password, + } # When serializer.is_valid(raise_exception=True) # Then - mocked_authenticate.assert_called_with( - request=None, username="some_username", password="some_password" - ) + mocked_authenticate.assert_called_with(**expected_authenticate_kwargs) diff --git a/api/users/models.py b/api/users/models.py index 2030ad05c3f7..2f0487a4592d 100644 --- a/api/users/models.py +++ b/api/users/models.py @@ -113,7 +113,7 @@ class FFAdminUser(LifecycleModel, AbstractUser): uuid = models.UUIDField(default=uuid.uuid4, editable=False, unique=True) - USERNAME_FIELD = "email" + USERNAME_FIELD = "username" if settings.LDAP_ENABLED else "email" REQUIRED_FIELDS = ["first_name", "last_name", "sign_up_type"] class Meta: