Skip to content

Commit

Permalink
Migrate to the dj-rest-auth==5.0.2 and django-allauth==0.57.2 (cv…
Browse files Browse the repository at this point in the history
  • Loading branch information
Marishka17 authored Apr 4, 2024
1 parent 2a6fd0d commit ea47d00
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 34 deletions.
4 changes: 4 additions & 0 deletions changelog.d/20240403_144021_maria_update_module_versions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
### Fixed

- Missing RegisterSerializerEx `email_verification_required` and `key` parameters now are included in the server schema
(<https://github.com/cvat-ai/cvat/pull/7635>)
42 changes: 35 additions & 7 deletions cvat/apps/iam/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,18 @@
from django.core.exceptions import ValidationError as DjangoValidationError
from rest_framework.exceptions import ValidationError
from rest_framework import serializers
from allauth.account import app_settings
from allauth.account import app_settings as allauth_settings
from allauth.account.utils import filter_users_by_email
from allauth.account.adapter import get_adapter
from allauth.utils import email_address_exists
from allauth.account.utils import setup_user_email
from allauth.account.models import EmailAddress

from django.conf import settings
from django.contrib.auth import get_user_model
from django.contrib.auth.models import User

from drf_spectacular.utils import extend_schema_field
from typing import Optional, Union, Dict

from cvat.apps.iam.forms import ResetPasswordFormEx
from cvat.apps.iam.utils import get_dummy_user
Expand All @@ -23,6 +28,20 @@
class RegisterSerializerEx(RegisterSerializer):
first_name = serializers.CharField(required=False)
last_name = serializers.CharField(required=False)
email_verification_required = serializers.SerializerMethodField()
key = serializers.SerializerMethodField()

@extend_schema_field(serializers.BooleanField)
def get_email_verification_required(self, obj: Union[Dict, User]) -> bool:
return allauth_settings.EMAIL_VERIFICATION == allauth_settings.EmailVerificationMethod.MANDATORY

@extend_schema_field(serializers.CharField(allow_null=True))
def get_key(self, obj: Union[Dict, User]) -> Optional[str]:
key = None
if isinstance(obj, User) and allauth_settings.EMAIL_VERIFICATION != \
allauth_settings.EmailVerificationMethod.MANDATORY:
key = obj.auth_token.key
return key

def get_cleaned_data(self):
data = super().get_cleaned_data()
Expand All @@ -34,8 +53,17 @@ def get_cleaned_data(self):
return data

def validate_email(self, email):
def email_address_exists(email) -> bool:
if EmailAddress.objects.filter(email__iexact=email).exists():
return True

if (email_field := allauth_settings.USER_MODEL_EMAIL_FIELD):
users = get_user_model().objects
return users.filter(**{email_field + "__iexact": email}).exists()
return False

email = get_adapter().clean_email(email)
if app_settings.UNIQUE_EMAIL:
if allauth_settings.UNIQUE_EMAIL:
if email and email_address_exists(email):
user = get_dummy_user(email)
if not user:
Expand Down Expand Up @@ -88,10 +116,10 @@ class LoginSerializerEx(LoginSerializer):
def get_auth_user_using_allauth(self, username, email, password):

def is_email_authentication():
return settings.ACCOUNT_AUTHENTICATION_METHOD == app_settings.AuthenticationMethod.EMAIL
return settings.ACCOUNT_AUTHENTICATION_METHOD == allauth_settings.AuthenticationMethod.EMAIL

def is_username_authentication():
return settings.ACCOUNT_AUTHENTICATION_METHOD == app_settings.AuthenticationMethod.USERNAME
return settings.ACCOUNT_AUTHENTICATION_METHOD == allauth_settings.AuthenticationMethod.USERNAME

# check that the server settings match the request
if is_username_authentication() and not username and email:
Expand All @@ -107,11 +135,11 @@ def is_username_authentication():
'Please check your server configuration ACCOUNT_AUTHENTICATION_METHOD.')

# Authentication through email
if settings.ACCOUNT_AUTHENTICATION_METHOD == app_settings.AuthenticationMethod.EMAIL:
if settings.ACCOUNT_AUTHENTICATION_METHOD == allauth_settings.AuthenticationMethod.EMAIL:
return self._validate_email(email, password)

# Authentication through username
if settings.ACCOUNT_AUTHENTICATION_METHOD == app_settings.AuthenticationMethod.USERNAME:
if settings.ACCOUNT_AUTHENTICATION_METHOD == allauth_settings.AuthenticationMethod.USERNAME:
return self._validate_username(username, password)

# Authentication through either username or email
Expand Down
41 changes: 33 additions & 8 deletions cvat/apps/iam/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
from django.http import HttpResponse
from django.views.decorators.http import etag as django_etag
from rest_framework.response import Response
from dj_rest_auth.app_settings import api_settings as dj_rest_auth_settings
from dj_rest_auth.registration.views import RegisterView
from dj_rest_auth.utils import jwt_encode
from dj_rest_auth.views import LoginView
from allauth.account import app_settings as allauth_settings
from allauth.account.views import ConfirmEmailView
from allauth.account.utils import has_verified_email, send_email_confirmation
from allauth.account.utils import complete_signup, has_verified_email, send_email_confirmation

from furl import furl

Expand Down Expand Up @@ -97,14 +99,37 @@ def post(self, request, *args, **kwargs):

class RegisterViewEx(RegisterView):
def get_response_data(self, user):
data = self.get_serializer(user).data
data['email_verification_required'] = True
data['key'] = None
serializer = self.get_serializer(user)
return serializer.data

# NOTE: we should reimplement this method to fix the following issue:
# In the previous used version of dj-rest-auth 2.2.7, if the REST_SESSION_LOGIN setting was not defined in the settings file,
# the default value specified in the documentation (https://dj-rest-auth.readthedocs.io/en/2.2.7/configuration.html)
# was not applied for some unknown reason, and an authentication token was added to a user.
# With the dj-rest-auth version 5.0.2, there have been changes to how settings are handled,
# and now the default value is properly taken into account.
# However, even with the updated code, it still does not handle the scenario
# of handling two authentication flows simultaneously during registration process.
# Since there is no mention in the dj-rest-auth documentation that session authentication
# cannot be used alongside token authentication (https://dj-rest-auth.readthedocs.io/en/latest/configuration.html),
# and given the login implementation (https://github.com/iMerica/dj-rest-auth/blob/c6b6530eb0bfa5b10fd7b9e955a39301156e49d2/dj_rest_auth/views.py#L69-L75),
# this situation appears to be a bug.
# Link to the issue: https://github.com/iMerica/dj-rest-auth/issues/604
def perform_create(self, serializer):
user = serializer.save(self.request)
if allauth_settings.EMAIL_VERIFICATION != \
allauth_settings.EmailVerificationMethod.MANDATORY:
data['email_verification_required'] = False
data['key'] = user.auth_token.key
return data
allauth_settings.EmailVerificationMethod.MANDATORY:
if dj_rest_auth_settings.USE_JWT:
self.access_token, self.refresh_token = jwt_encode(user)
elif self.token_model:
dj_rest_auth_settings.TOKEN_CREATOR(self.token_model, user, serializer)

complete_signup(
self.request._request, user,
allauth_settings.EMAIL_VERIFICATION,
None,
)
return user

def _etag(etag_func):
"""
Expand Down
7 changes: 2 additions & 5 deletions cvat/requirements/base.in
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@ clickhouse-connect==0.6.8
coreapi==2.3.3
datumaro @ git+https://github.com/cvat-ai/datumaro.git@8a14a99fe17f19d98595a2a4a74ab459051cc23b
dj-pagination==2.5.0
dj-rest-auth[with_social]==2.2.7

# dj-rest-auth[with_social] includes django-allauth but with version range: >=0.40.0,<0.53.0
# This does not suit us in the case when one of the previous allauth version was installed.
# Despite direct indication allauth in requirements we should keep 'with_social' for dj-rest-auth
# to avoid possible further versions conflicts (we use registration functionality)
# https://dj-rest-auth.readthedocs.io/en/latest/installation.html#registration-optional
django-allauth>=0.52.0
dj-rest-auth[with_social]==5.0.2
django-allauth==0.57.2

django-auth-ldap==2.2.0
django-compressor==4.3.1
Expand Down
14 changes: 7 additions & 7 deletions cvat/requirements/base.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SHA1:92683dac1b858a87e89ba736358e779ac8be666d
# SHA1:3ac9fd027a6df758454893273509039ee719e819
#
# This file is autogenerated by pip-compile-multi
# To update, run:
Expand Down Expand Up @@ -42,7 +42,7 @@ click==8.1.7
# via rq
clickhouse-connect==0.6.8
# via -r cvat/requirements/base.in
contourpy==1.2.0
contourpy==1.2.1
# via matplotlib
coreapi==2.3.3
# via -r cvat/requirements/base.in
Expand All @@ -66,7 +66,7 @@ deprecated==1.2.14
# via limits
dj-pagination==2.5.0
# via -r cvat/requirements/base.in
dj-rest-auth[with_social]==2.2.7
dj-rest-auth[with_social]==5.0.2
# via -r cvat/requirements/base.in
django==4.2.11
# via
Expand All @@ -83,7 +83,7 @@ django==4.2.11
# django-sendfile2
# djangorestframework
# drf-spectacular
django-allauth==0.52.0
django-allauth==0.57.2
# via
# -r cvat/requirements/base.in
# dj-rest-auth
Expand Down Expand Up @@ -167,7 +167,7 @@ kiwisolver==1.4.5
# via matplotlib
limits==3.10.1
# via python-logstash-async
lxml==5.1.0
lxml==5.2.1
# via datumaro
lz4==4.3.3
# via clickhouse-connect
Expand Down Expand Up @@ -224,7 +224,7 @@ pyasn1-modules==0.4.0
# python-ldap
pycocotools==2.0.7
# via datumaro
pycparser==2.21
pycparser==2.22
# via cffi
pyjwt[crypto]==2.8.0
# via django-allauth
Expand Down Expand Up @@ -302,7 +302,7 @@ rules==3.3
# via -r cvat/requirements/base.in
s3transfer==0.4.2
# via boto3
scipy==1.12.0
scipy==1.13.0
# via datumaro
shapely==1.7.1
# via -r cvat/requirements/base.in
Expand Down
7 changes: 7 additions & 0 deletions cvat/schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9235,6 +9235,13 @@ components:
type: string
last_name:
type: string
email_verification_required:
type: boolean
readOnly: true
key:
type: string
nullable: true
readOnly: true
required:
- username
RegisterSerializerExRequest:
Expand Down
9 changes: 3 additions & 6 deletions cvat/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,11 @@ def generate_secret_key():
}


REST_AUTH_REGISTER_SERIALIZERS = {
REST_AUTH = {
'REGISTER_SERIALIZER': 'cvat.apps.iam.serializers.RegisterSerializerEx',
}

REST_AUTH_SERIALIZERS = {
'LOGIN_SERIALIZER': 'cvat.apps.iam.serializers.LoginSerializerEx',
'PASSWORD_RESET_SERIALIZER': 'cvat.apps.iam.serializers.PasswordResetSerializerEx',
'OLD_PASSWORD_FIELD_ENABLED': True,
}

if to_bool(os.getenv('CVAT_ANALYTICS', False)):
Expand All @@ -198,6 +196,7 @@ def generate_secret_key():
'django.middleware.clickjacking.XFrameOptionsMiddleware',
'dj_pagination.middleware.PaginationMiddleware',
'cvat.apps.iam.middleware.ContextMiddleware',
'allauth.account.middleware.AccountMiddleware',
]

UI_URL = ''
Expand Down Expand Up @@ -263,8 +262,6 @@ def generate_secret_key():
ACCOUNT_EMAIL_VERIFICATION_SENT_REDIRECT_URL = '/auth/email-verification-sent'
INCORRECT_EMAIL_CONFIRMATION_URL = '/auth/incorrect-email-confirmation'

OLD_PASSWORD_FIELD_ENABLED = True

# Django-RQ
# https://github.com/rq/django-rq

Expand Down
2 changes: 1 addition & 1 deletion utils/dataset_manifest/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ numpy==1.22.4
# via opencv-python-headless
opencv-python-headless==4.9.0.80
# via -r utils/dataset_manifest/requirements.in
pillow==10.2.0
pillow==10.3.0
# via -r utils/dataset_manifest/requirements.in
tqdm==4.66.2
# via -r utils/dataset_manifest/requirements.in

0 comments on commit ea47d00

Please sign in to comment.