Skip to content

Commit

Permalink
Merge branch 'refs/heads/main' into docs/edge-proxy-http-proxy
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewelwell committed Dec 20, 2024
2 parents d6261a5 + b60af94 commit 287ecfc
Show file tree
Hide file tree
Showing 12 changed files with 75 additions and 80 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ repos:
- [email protected]
args:
- --check
exclude: ^(frontend/|CHANGELOG.md|.github/docker_build_comment_template.md)
types: [javascript,jsx,ts,tsx,markdown,mdx,html,css,json,yaml]

- repo: https://github.com/python-poetry/poetry
rev: 1.8.0
Expand Down
13 changes: 4 additions & 9 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
# Ignore a bunch of files:
*.html
*.json
*.handlebars
*.css
.bablerc
**/CHANGELOG.md
.github/docker_build_comment_template.md

# Frontend has it's own prettier setup
frontend/

# Auto generated by open API generator
docs/docs/edge-api/*.mdx
docs/docs/edge-api/sidebar.ts

.prettierignore
6 changes: 3 additions & 3 deletions api/app/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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:
Expand Down
30 changes: 1 addition & 29 deletions api/custom_auth/serializers.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
13 changes: 7 additions & 6 deletions api/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion api/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
50 changes: 35 additions & 15 deletions api/tests/unit/custom_auth/test_unit_custom_auth_serializer.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
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

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

Expand Down Expand Up @@ -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)
2 changes: 1 addition & 1 deletion api/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
28 changes: 14 additions & 14 deletions frontend/web/components/EditPermissions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,19 @@ const _EditPermissionsModal: FC<EditPermissionModalType> = withAdminPermissions(
})
}

const getEditText = () => {
if (isGroup) {
return `the ${group?.name || ''} group`
}
if (user) {
return `${user.first_name || ''} ${user.last_name || ''}`
}
if (role) {
return role.name
}
return name
}

const rolesAdded = getRoles(roles, rolesSelected || [])
const isAdmin = admin()

Expand Down Expand Up @@ -917,20 +930,7 @@ const _EditPermissionsModal: FC<EditPermissionModalType> = withAdminPermissions(

<p className='text-right mt-5 text-dark'>
This will edit the permissions for{' '}
<strong>
{isGroup ? (
`the ${group?.name || ''} group`
) : user ? (
<>
{user.first_name || ''} {user.last_name || ''}
</>
) : role ? (
` ${role.name}`
) : (
` ${name}`
)}
</strong>
.
<strong>{getEditText()}</strong>.
</p>

{!!parentWarning && (
Expand Down
4 changes: 4 additions & 0 deletions frontend/web/components/PermissionsTabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ const PermissionsTabs: FC<PermissionsTabsType> = ({
/>
)}
</div>
<p className='text-right mt-5 text-dark'>
This will edit the permissions for{' '}
<strong>the {group?.name} group</strong>.
</p>
</TabItem>
</Tabs>
</PlanBasedAccess>
Expand Down
2 changes: 1 addition & 1 deletion frontend/web/components/RolePermissionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
useGetRoleProjectPermissionsQuery,
} from 'common/services/useRolePermission'
import { PermissionLevel } from 'common/types/requests'
import { Role, User, UserGroup, UserGroupSummary } from 'common/types/responses'
import { Role, User, UserGroupSummary } from 'common/types/responses'
import PanelSearch from './PanelSearch'
import PermissionsSummaryList from './PermissionsSummaryList'

Expand Down
3 changes: 3 additions & 0 deletions frontend/web/components/modals/CreateRole.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,9 @@ const CreateRole: FC<CreateRoleType> = ({
))}
</div>
</div>
<p className='text-right mt-5 text-dark'>
This will edit the members for <strong>{role?.name}</strong>.
</p>
</div>
</TabItem>
<TabItem
Expand Down

0 comments on commit 287ecfc

Please sign in to comment.