From f2c7f44abbd5ce3dcbbd80df976ed26b7d84e873 Mon Sep 17 00:00:00 2001 From: Sumit Singh Date: Sat, 8 Oct 2022 21:40:57 +0530 Subject: [PATCH 1/5] feat(deps): remove drfaddons dependency --- .pre-commit-config.yaml | 7 +- drf_user/__init__.py | 11 +- drf_user/constants.py | 16 +++ drf_user/models.py | 7 +- drf_user/serializers.py | 41 +++--- drf_user/signals/handlers.py | 15 +- drf_user/utils.py | 271 ++++++++++++++++++++++++----------- drf_user/variables.py | 10 -- drf_user/views.py | 222 ++++++++++++++-------------- example/demo_app/settings.py | 1 - requirements.txt | 2 +- tests/settings.py | 1 - tests/test_utils.py | 27 ++-- tests/test_views.py | 61 ++++---- 14 files changed, 400 insertions(+), 292 deletions(-) create mode 100644 drf_user/constants.py delete mode 100644 drf_user/variables.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1f5e9e5..6794be5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,12 +9,7 @@ repos: rev: 5.0.4 hooks: - id: flake8 - args: [--max-line-length=88] - - - repo: https://github.com/asottile/reorder_python_imports - rev: v3.8.3 - hooks: - - id: reorder-python-imports + args: [--max-line-length=100] - repo: https://github.com/econchick/interrogate rev: 1.5.0 diff --git a/drf_user/__init__.py b/drf_user/__init__.py index a433ee6..0e8700f 100644 --- a/drf_user/__init__.py +++ b/drf_user/__init__.py @@ -55,17 +55,16 @@ def update_user_settings() -> dict: user_settings[key] = value elif key == "OTP": if not isinstance(value, dict): - raise TypeError("USER_SETTING attribute OTP must be a" " dict.") + raise TypeError("USER_SETTING attribute OTP must be a dict.") for otp_key, otp_value in value.items(): user_settings["OTP"][otp_key] = otp_value elif key == "REGISTRATION": - if isinstance(value, dict): - for reg_key, reg_value in value.items(): - user_settings["REGISTRATION"][reg_key] = reg_value - else: + if not isinstance(value, dict): raise TypeError( - "USER_SETTING attribute REGISTRATION" " must be a dict." + "USER_SETTING attribute REGISTRATION must be a dict." ) + for reg_key, reg_value in value.items(): + user_settings["REGISTRATION"][reg_key] = reg_value if user_settings["REGISTRATION"]["SEND_MAIL"]: if not getattr(settings, "EMAIL_HOST", None): raise ValueError( diff --git a/drf_user/constants.py b/drf_user/constants.py new file mode 100644 index 0000000..6bae8d2 --- /dev/null +++ b/drf_user/constants.py @@ -0,0 +1,16 @@ +""" +All constants used in the system. +""" + +EMAIL: str = "E" +MOBILE: str = "M" +DESTINATION_CHOICES: list = [(EMAIL, "EMail Address"), (MOBILE, "Mobile Number")] + + +class CoreConstants: + """Core Constants""" + + EMAIL_PROP: str = "E" + MOBILE_PROP: str = "M" + EMAIL_STR: str = "email" + MOBILE_STR: str = "mobile" diff --git a/drf_user/models.py b/drf_user/models.py index ff9bfe7..726a5cf 100644 --- a/drf_user/models.py +++ b/drf_user/models.py @@ -6,8 +6,7 @@ from django.utils.text import gettext_lazy as _ from drf_user.managers import UserManager -from drf_user.variables import DESTINATION_CHOICES -from drf_user.variables import EMAIL +from drf_user.constants import DESTINATION_CHOICES, EMAIL class Role(Group): @@ -86,7 +85,7 @@ def get_full_name(self) -> str: def __str__(self): """String representation of model""" - return str(self.name) + " | " + str(self.username) + return f"{str(self.name)} | {str(self.username)}" class AuthTransaction(models.Model): @@ -118,7 +117,7 @@ class AuthTransaction(models.Model): def __str__(self): """String representation of model""" - return str(self.created_by.name) + " | " + str(self.created_by.username) + return f"{str(self.created_by.name)} | {str(self.created_by.username)}" class Meta: """Passing model metadata""" diff --git a/drf_user/serializers.py b/drf_user/serializers.py index 0103159..e8258d1 100644 --- a/drf_user/serializers.py +++ b/drf_user/serializers.py @@ -10,8 +10,7 @@ from drf_user import user_settings from drf_user.models import User from drf_user.utils import check_validation -from drf_user.variables import EMAIL -from drf_user.variables import MOBILE +from drf_user.constants import EMAIL, MOBILE class UserSerializer(serializers.ModelSerializer): @@ -203,8 +202,7 @@ def validate(self, attrs: dict) -> dict: if "email" not in attrs.keys() and "verify_otp" not in attrs.keys(): raise serializers.ValidationError( _( - "email field is compulsory while verifying a" - " non-existing user's OTP." + "Email field is compulsory while verifying a non-existing user's OTP." ) ) else: @@ -250,26 +248,27 @@ def get_user(email: str, mobile: str): except User.DoesNotExist: try: user = User.objects.get(mobile=mobile) - except User.DoesNotExist: - user = None + except User.DoesNotExist as e: + raise NotFound( + _(f"No user exists either for email={email} or mobile={mobile}") + ) from e - if user: - if user.email != email: - raise serializers.ValidationError( - _( - "Your account is registered with {mobile} does not has " - "{email} as registered email. Please login directly via " - "OTP with your mobile.".format(mobile=mobile, email=email) - ) + if user.email != email: + raise serializers.ValidationError( + _( + "Your account is registered with {mobile} does not has " + "{email} as registered email. Please login directly via " + "OTP with your mobile.".format(mobile=mobile, email=email) ) - if user.mobile != mobile: - raise serializers.ValidationError( - _( - "Your account is registered with {email} does not has " - "{mobile} as registered mobile. Please login directly via " - "OTP with your email.".format(mobile=mobile, email=email) - ) + ) + if user.mobile != mobile: + raise serializers.ValidationError( + _( + "Your account is registered with {email} does not has " + "{mobile} as registered mobile. Please login directly via " + "OTP with your email.".format(mobile=mobile, email=email) ) + ) return user def validate(self, attrs: dict) -> dict: diff --git a/drf_user/signals/handlers.py b/drf_user/signals/handlers.py index dccf32c..f4ce5f5 100644 --- a/drf_user/signals/handlers.py +++ b/drf_user/signals/handlers.py @@ -3,9 +3,11 @@ from django.db.models.signals import post_save from django.dispatch import receiver +User = get_user_model() -@receiver(post_save, sender=get_user_model()) -def post_register(sender, instance: get_user_model(), created, **kwargs): + +@receiver(post_save, sender=User) +def post_register(sender, instance: User, created: bool, **kwargs): """Sends mail/message to users after registeration Parameters @@ -19,21 +21,20 @@ def post_register(sender, instance: get_user_model(), created, **kwargs): from drf_user import user_settings - from drfaddons.utils import send_message + from drf_user.utils import send_message if created: if user_settings["REGISTRATION"]["SEND_MAIL"]: send_message( message=user_settings["REGISTRATION"]["TEXT_MAIL_BODY"], subject=user_settings["REGISTRATION"]["MAIL_SUBJECT"], - recip=[instance.email], - recip_email=[instance.email], + recip_email=instance.email, html_message=user_settings["REGISTRATION"]["HTML_MAIL_BODY"], ) if user_settings["REGISTRATION"]["SEND_MESSAGE"]: send_message( message=user_settings["REGISTRATION"]["SMS_BODY"], subject=user_settings["REGISTRATION"]["MAIL_SUBJECT"], - recip=[instance.mobile], - recip_email=[instance.mobile], + recip_email=instance.email, + recip_mobile=instance.mobile, ) diff --git a/drf_user/utils.py b/drf_user/utils.py index 8070c8c..6dea184 100644 --- a/drf_user/utils.py +++ b/drf_user/utils.py @@ -1,30 +1,29 @@ """Collection of general helper functions.""" import datetime -from typing import Dict -from typing import Optional -from typing import Union - -import pytz +import logging +import re +from typing import Dict, Optional + +from django.conf import settings +from django.core.exceptions import ValidationError +from django.core.mail import send_mail +from django.core.validators import validate_email from django.http import HttpRequest from django.utils import timezone from django.utils.text import gettext_lazy as _ -from drfaddons.utils import send_message -from rest_framework.exceptions import APIException -from rest_framework.exceptions import AuthenticationFailed -from rest_framework.exceptions import NotFound -from rest_framework.exceptions import PermissionDenied +from rest_framework import serializers +from rest_framework.exceptions import AuthenticationFailed, NotFound, PermissionDenied from rest_framework_simplejwt.tokens import RefreshToken from rest_framework_simplejwt.utils import datetime_from_epoch +from sendsms import api from drf_user import update_user_settings -from drf_user.models import AuthTransaction -from drf_user.models import OTPValidation -from drf_user.models import User +from drf_user.models import AuthTransaction, OTPValidation, User + +user_settings: dict = update_user_settings() +otp_settings: dict = user_settings["OTP"] -user_settings: Dict[ - str, Union[bool, Dict[str, Union[int, str, bool]]] -] = update_user_settings() -otp_settings: Dict[str, Union[str, int]] = user_settings["OTP"] +logger = logging.getLogger(__name__) def get_client_ip(request: HttpRequest) -> Optional[str]: @@ -51,7 +50,7 @@ def get_client_ip(request: HttpRequest) -> Optional[str]: def datetime_passed_now(source: datetime.datetime) -> bool: """ Compares provided datetime with current time on the basis of Django - settings. Checks source is in future or in past. False if it's in future. + settings. Checks source is in future or in the past. False if it's in future. Parameters ---------- source: datetime object than may or may not be naive @@ -63,9 +62,9 @@ def datetime_passed_now(source: datetime.datetime) -> bool: Author: Himanshu Shankar (https://himanshus.com) """ if source.tzinfo is not None and source.tzinfo.utcoffset(source) is not None: - return source <= datetime.datetime.utcnow().replace(tzinfo=pytz.utc) - else: - return source <= datetime.datetime.now() + return source <= datetime.datetime.now(datetime.timezone.utc) + + return source <= datetime.datetime.now() def check_unique(prop: str, value: str) -> bool: @@ -97,18 +96,18 @@ def check_unique(prop: str, value: str) -> bool: return user.count() == 0 -def generate_otp(prop: str, value: str) -> OTPValidation: +def generate_otp(*, destination_property: str, destination: str) -> OTPValidation: """ This function generates an OTP and saves it into Model. It also sets various counters, such as send_counter, is_validated, validate_attempt. Parameters ---------- - prop: str + destination_property: str This specifies the type for which OTP is being created. Can be:: - email - mobile - value: str + E + M + destination: str This specifies the value for which OTP is being created. Returns @@ -118,10 +117,10 @@ def generate_otp(prop: str, value: str) -> OTPValidation: Examples -------- To create an OTP for an Email test@testing.com - >>> print(generate_otp('email', 'test@testing.com')) + >>> print(generate_otp('E', 'test@testing.com')) OTPValidation object - >>> print(generate_otp('email', 'test@testing.com').otp) + >>> print(generate_otp('E', 'test@testing.com').otp) 5039164 """ # Create a random number @@ -141,16 +140,16 @@ def generate_otp(prop: str, value: str) -> OTPValidation: # Get or Create new instance of Model with value of provided value # and set proper counter. try: - otp_object: OTPValidation = OTPValidation.objects.get(destination=value) + otp_object: OTPValidation = OTPValidation.objects.get(destination=destination) except OTPValidation.DoesNotExist: otp_object: OTPValidation = OTPValidation() - otp_object.destination = value + otp_object.destination = destination else: if not datetime_passed_now(otp_object.reactive_at): return otp_object otp_object.otp = random_number - otp_object.prop = prop + otp_object.prop = destination_property # Set is_validated to False otp_object.is_validated = False @@ -164,48 +163,6 @@ def generate_otp(prop: str, value: str) -> OTPValidation: return otp_object -def send_otp(value: str, otpobj: OTPValidation, recip: str) -> Dict: - """ - This function sends OTP to specified value. - Parameters - ---------- - value: str - This is the value at which and for which OTP is to be sent. - otpobj: OTPValidation - This is the OTP or One Time Passcode that is to be sent to user. - recip: str - This is the recipient to whom EMail is being sent. This will be - deprecated once SMS feature is brought in. - - Returns - ------- - - """ - otp: str = otpobj.otp - - if not datetime_passed_now(otpobj.reactive_at): - raise PermissionDenied( - detail=_(f"OTP sending not allowed until: {otpobj.reactive_at}") - ) - - message = ( - f"OTP for verifying {otpobj.get_prop_display()}: {value} is {otp}." - f" Don't share this with anyone!" - ) - - try: - rdata: dict = send_message(message, otp_settings["SUBJECT"], [value], [recip]) - except ValueError as err: - raise APIException(_(f"Server configuration error occurred: {err}")) - - otpobj.reactive_at = timezone.now() + datetime.timedelta( - minutes=otp_settings["COOLING_PERIOD"] - ) - otpobj.save() - - return rdata - - def login_user(user: User, request: HttpRequest) -> Dict[str, str]: """ This function is used to login a user. It saves the authentication in @@ -278,54 +235,198 @@ def check_validation(value: str) -> bool: return False -def validate_otp(value: str, otp: int) -> bool: +def validate_mobile(mobile: str) -> bool: + """ + This function checks if the mobile number is valid or not. + Parameters + ---------- + mobile: str + This is the mobile number to be checked. + + Returns + ------- + bool + True if mobile number is valid, False otherwise. + Examples + -------- + To check if '9999999999' is a valid mobile number + >>> print(validate_mobile('9999999999')) + True + """ + if is_valid := re.match(r"^[6-9]\d{9}$", mobile) is not None: + return is_valid + raise ValidationError("Invalid Mobile Number") + + +def validate_otp(*, destination: str, otp_val: int) -> bool: """ This function is used to validate the OTP for a particular value. It also reduces the attempt count by 1 and resets OTP. Parameters ---------- - value: str + destination: str This is the unique entry for which OTP has to be validated. - otp: int + otp_val: int This is the OTP that will be validated against one in Database. Returns ------- bool: True, if OTP is validated """ + try: # Try to get OTP Object from Model and initialize data dictionary otp_object: OTPValidation = OTPValidation.objects.get( - destination=value, is_validated=False + destination=destination, is_validated=False ) - except OTPValidation.DoesNotExist: + except OTPValidation.DoesNotExist as e: raise NotFound( detail=_( - "No pending OTP validation request found for provided " - "destination. Kindly send an OTP first" + "No pending OTP validation request found for provided destination." + " Kindly send an OTP first" ) - ) + ) from e + # Decrement validate_attempt otp_object.validate_attempt -= 1 - if str(otp_object.otp) == str(otp): + if str(otp_object.otp) == str(otp_val): # match otp otp_object.is_validated = True - otp_object.save() + otp_object.save(update_fields=["is_validated", "validate_attempt"]) return True elif otp_object.validate_attempt <= 0: # check if attempts exceeded and regenerate otp and raise error - generate_otp(otp_object.prop, value) + generate_otp(destination_property=otp_object.prop, destination=destination) raise AuthenticationFailed( detail=_("Incorrect OTP. Attempt exceeded! OTP has been reset.") ) else: # update attempts and raise error - otp_object.save() + otp_object.save(update_fields=["validate_attempt"]) raise AuthenticationFailed( detail=_( f"OTP Validation failed! {otp_object.validate_attempt} attempts left!" ) ) + + +def send_message( + message: str, + subject: str, + recip_email: str, + recip_mobile: Optional[str] = None, + html_message: Optional[str] = None, +) -> Dict: + """ + Sends message to specified value. + + Parameters + ---------- + message: str + Message that is to be sent to user. + subject: str + Subject that is to be sent to user, in case prop is an email. + recip_mobile: str + Recipient Mobile Number to whom message is being sent. + recip_email: str + Recipient to whom EMail is being sent. + html_message: str + HTML variant of message, if any. + + Returns + ------- + sent: dict + """ + sent = {"success": False, "message": None, "mobile_message": None} + + if not getattr(settings, "EMAIL_HOST", None): + raise ValueError( + "EMAIL_HOST must be defined in django setting for sending mail." + ) + if not getattr(settings, "EMAIL_FROM", None): + raise ValueError( + "EMAIL_FROM must be defined in django setting " + "for sending mail. Who is sending email?" + ) + + # check if email is valid + validate_email(recip_email) + + if recip_mobile: + # check for valid mobile numbers + validate_mobile(recip_mobile) + + try: + send_mail( + subject=subject, + message=message, + html_message=html_message, + from_email=settings.EMAIL_FROM, + recipient_list=[recip_email], + ) + except Exception as e: # noqa + sent["message"] = f"Email Message sending failed! {str(e.args)}" + sent["success"] = False + else: + sent["message"] = "Email Message sent successfully!" + sent["success"] = True + + if recip_mobile: + try: + api.send_sms(body=message, to=recip_mobile, from_phone=None) + except Exception as e: # noqa + logger.debug("Message sending failed", exc_info=e) + sent["mobile_message"] = f"Mobile Message sending failed! {str(e.args)}" + else: + sent["mobile_message"] = "Mobile Message sent successfully!" + + return sent + + +def send_otp( + *, otp_obj: OTPValidation, recip_email: str, recip_mobile: Optional[str] = None +) -> Dict: + """ + This function sends OTP to specified value. + Parameters + ---------- + otp_obj: OTPValidation + OTPValidation object that contains the OTP and other details. + recip_email: str + Recipient to whom EMail is being sent. + recip_mobile: Optional[str] + Recipient Mobile Number to whom message is being sent. + + Returns + ------- + data: dict + Dictionary containing the status of the OTP sent. + """ + otp_val: str = otp_obj.otp + + if not datetime_passed_now(otp_obj.reactive_at): + raise PermissionDenied(f"OTP sending not allowed until: {otp_obj.reactive_at}") + + message: str = ( + f"OTP for verifying {otp_obj.get_prop_display()}: {otp_obj.destination} is {otp_val}." + f" Don't share this with anyone!" + ) + + try: + data: dict = send_message( + message, otp_settings["SUBJECT"], recip_email, recip_mobile + ) + except (ValueError, ValidationError) as e: + raise serializers.ValidationError( + {"detail": f"OTP sending failed! because {e}"} + ) from e + + otp_obj.reactive_at = timezone.now() + datetime.timedelta( + minutes=otp_settings["COOLING_PERIOD"] + ) + otp_obj.save(update_fields=["reactive_at"]) + + return data diff --git a/drf_user/variables.py b/drf_user/variables.py deleted file mode 100644 index baa231a..0000000 --- a/drf_user/variables.py +++ /dev/null @@ -1,10 +0,0 @@ -""" -All static variables used in the system. - -Author: Himanshu Shankar (https://himanshus.com) -Author: Aditya Gupta (https://github.com/ag93999) -""" - -EMAIL = "E" -MOBILE = "M" -DESTINATION_CHOICES = [(EMAIL, "EMail Address"), (MOBILE, "Mobile Number")] diff --git a/drf_user/views.py b/drf_user/views.py index 82433c9..a9ed099 100644 --- a/drf_user/views.py +++ b/drf_user/views.py @@ -1,40 +1,46 @@ """Views for drf-user""" +from typing import Optional + from django.conf import settings +from django.contrib.auth import get_user_model +from django.db.models import F from django.utils import timezone from django.utils.text import gettext_lazy as _ -from drfaddons.utils import JsonResponse -from rest_framework import status -from rest_framework.exceptions import APIException +from rest_framework import status, serializers from rest_framework.exceptions import ValidationError -from rest_framework.generics import CreateAPIView -from rest_framework.generics import RetrieveUpdateAPIView +from rest_framework.generics import CreateAPIView, RetrieveUpdateAPIView from rest_framework.parsers import JSONParser +from rest_framework.parsers import MultiPartParser from rest_framework.permissions import AllowAny from rest_framework.permissions import IsAuthenticated from rest_framework.renderers import JSONRenderer from rest_framework.response import Response from rest_framework.views import APIView -from rest_framework_simplejwt.exceptions import InvalidToken -from rest_framework_simplejwt.exceptions import TokenError +from rest_framework_simplejwt.exceptions import InvalidToken, TokenError from rest_framework_simplejwt.settings import api_settings from rest_framework_simplejwt.views import TokenRefreshView -from drf_user.models import AuthTransaction -from drf_user.models import User -from drf_user.serializers import CheckUniqueSerializer -from drf_user.serializers import CustomTokenObtainPairSerializer -from drf_user.serializers import OTPLoginRegisterSerializer -from drf_user.serializers import OTPSerializer -from drf_user.serializers import PasswordResetSerializer -from drf_user.serializers import UserSerializer -from drf_user.utils import check_unique -from drf_user.utils import generate_otp -from drf_user.utils import get_client_ip -from drf_user.utils import login_user -from drf_user.utils import send_otp -from drf_user.utils import validate_otp -from drf_user.variables import EMAIL -from drf_user.variables import MOBILE +from drf_user.models import AuthTransaction, OTPValidation +from drf_user.serializers import ( + CheckUniqueSerializer, + CustomTokenObtainPairSerializer, + OTPLoginRegisterSerializer, + OTPSerializer, + PasswordResetSerializer, + UserSerializer, + ImageSerializer, +) +from drf_user.utils import ( + check_unique, + generate_otp, + get_client_ip, + login_user, + validate_otp, + send_otp, +) +from drf_user.constants import EMAIL, CoreConstants + +User = get_user_model() class RegisterView(CreateAPIView): @@ -59,9 +65,9 @@ def perform_create(self, serializer): } try: data["mobile"] = serializer.validated_data["mobile"] - except KeyError: + except KeyError as e: if not settings.USER_SETTINGS["MOBILE_OPTIONAL"]: - raise ValidationError({"error": "Mobile is required."}) + raise ValidationError({"error": "Mobile is required."}) from e return User.objects.create_user(**data) @@ -69,7 +75,7 @@ class LoginView(APIView): """ Login View - This is used to Login into system. + This is used to Log in into system. The data required are 'username' and 'password'. username -- Either username or mobile or email address. @@ -126,26 +132,22 @@ class CheckUniqueView(APIView): permission_classes = (AllowAny,) serializer_class = CheckUniqueSerializer - def validated(self, serialized_data, *args, **kwargs): - """Validates the response""" - return ( - { - "unique": check_unique( - serialized_data.validated_data["prop"], - serialized_data.validated_data["value"], - ) - }, - status.HTTP_200_OK, - ) - def post(self, request): """Overrides post method to validate serialized data""" serialized_data = self.serializer_class(data=request.data) if serialized_data.is_valid(): - return JsonResponse(self.validated(serialized_data=serialized_data)) + return Response( + data={ + "unique": check_unique( + serialized_data.validated_data["prop"], + serialized_data.validated_data["value"], + ) + }, + status=status.HTTP_200_OK, + ) else: - return JsonResponse( - serialized_data.errors, status=status.HTTP_422_UNPROCESSABLE_ENTITY + return Response( + data=serialized_data.errors, status=status.HTTP_422_UNPROCESSABLE_ENTITY ) @@ -185,43 +187,52 @@ class OTPView(APIView): def post(self, request, *args, **kwargs): """Overrides post method to validate serialized data""" - serializer = self.serializer_class(data=request.data) + serializer: OTPSerializer = self.serializer_class(data=request.data) serializer.is_valid(raise_exception=True) - destination = serializer.validated_data.get("destination") - prop = serializer.validated_data.get("prop") - user = serializer.validated_data.get("user") - email = serializer.validated_data.get("email") - is_login = serializer.validated_data.get("is_login") + destination: str = serializer.validated_data[ + "destination" + ] # destination is a required field + destination_property: str = serializer.validated_data.get( + "prop" + ) # can be email or mobile + user: User = serializer.validated_data.get("user") + email: Optional[str] = serializer.validated_data.get("email") + is_login: bool = serializer.validated_data.get("is_login") if "verify_otp" in request.data.keys(): - if validate_otp(destination, request.data.get("verify_otp")): + if validate_otp( + destination=destination, otp_val=request.data["verify_otp"] + ): if is_login: return Response( login_user(user, self.request), status=status.HTTP_202_ACCEPTED ) else: return Response( - data={ - "OTP": [ - _("OTP Validated successfully!"), - ] - }, + data={"OTP": _("OTP Validated successfully!")}, status=status.HTTP_202_ACCEPTED, ) else: - otp_obj = generate_otp(prop, destination) - sentotp = send_otp(destination, otp_obj, email) + otp_obj: OTPValidation = generate_otp( + destination_property=destination_property, destination=destination + ) + recip_mobile: Optional[str] = None + if destination_property == CoreConstants.MOBILE_PROP: + recip_mobile = destination - if sentotp["success"]: - otp_obj.send_counter += 1 - otp_obj.save() + sent_otp_resp: dict = send_otp( + otp_obj=otp_obj, recip_email=email, recip_mobile=recip_mobile + ) - return Response(sentotp, status=status.HTTP_201_CREATED) - else: - raise APIException( - detail=_("A Server Error occurred: " + sentotp["message"]) - ) + if sent_otp_resp["success"]: + otp_obj.send_counter = F("send_counter") + 1 + otp_obj.save(update_fields=["send_counter"]) + return Response(sent_otp_resp, status=status.HTTP_201_CREATED) + + raise serializers.ValidationError( + detail=_(f"OTP could not be sent! {sent_otp_resp['message']}") + ) class RetrieveUpdateUserAccountView(RetrieveUpdateAPIView): @@ -283,14 +294,14 @@ def post(self, request, *args, **kwargs): serializer = self.serializer_class(data=request.data) serializer.is_valid(raise_exception=True) - verify_otp = serializer.validated_data.get("verify_otp", None) - name = serializer.validated_data.get("name") - mobile = serializer.validated_data.get("mobile") - email = serializer.validated_data.get("email") - user = serializer.validated_data.get("user", None) + verify_otp = serializer.validated_data.get("verify_otp") + name = serializer.validated_data["name"] + mobile = serializer.validated_data["mobile"] + email = serializer.validated_data["email"] + user: User = serializer.validated_data.get("user") if verify_otp: - if validate_otp(email, verify_otp) and not user: + if validate_otp(destination=email, otp_val=verify_otp) and not user: user = User.objects.create_user( name=name, mobile=mobile, @@ -299,51 +310,32 @@ def post(self, request, *args, **kwargs): password=User.objects.make_random_password(), ) user.is_active = True - user.save() + user.save(update_fields=["is_active"]) return Response( login_user(user, self.request), status=status.HTTP_202_ACCEPTED ) - else: - otp_obj_email = generate_otp(EMAIL, email) - otp_obj_mobile = generate_otp(MOBILE, mobile) - - # Set same OTP for both Email & Mobile - otp_obj_mobile.otp = otp_obj_email.otp - otp_obj_mobile.save() - - # Send OTP to Email & Mobile - sentotp_email = send_otp(email, otp_obj_email, email) - sentotp_mobile = send_otp(mobile, otp_obj_mobile, email) - - message = {} - - if sentotp_email["success"]: - otp_obj_email.send_counter += 1 - otp_obj_email.save() - message["email"] = {"otp": _("OTP has been sent successfully.")} - else: - message["email"] = { - "otp": _(f'OTP sending failed {sentotp_email["message"]}') - } - - if sentotp_mobile["success"]: - otp_obj_mobile.send_counter += 1 - otp_obj_mobile.save() - message["mobile"] = {"otp": _("OTP has been sent successfully.")} - else: - message["mobile"] = { - "otp": _(f'OTP sending failed {sentotp_mobile["message"]}') - } - - if sentotp_email["success"] or sentotp_mobile["success"]: - curr_status = status.HTTP_201_CREATED - else: - raise APIException( - detail=_("A Server Error occurred: " + sentotp_mobile["message"]) - ) + otp_obj: OTPValidation = generate_otp( + destination_property=EMAIL, destination=email + ) + # Send OTP to Email & Mobile + sent_otp_resp: dict = send_otp( + otp_obj=otp_obj, recip_email=email, recip_mobile=mobile + ) - return Response(data=message, status=curr_status) + if not sent_otp_resp["success"]: + raise serializers.ValidationError( + detail=_(f"OTP could not be sent! {sent_otp_resp['message']}") + ) + + otp_obj.send_counter = F("send_counter") + 1 + otp_obj.save(update_fields=["send_counter"]) + message = { + "email": {"otp": sent_otp_resp["message"]}, + "mobile_message": {"otp": sent_otp_resp["mobile_message"]}, + } + + return Response(data=message, status=status.HTTP_201_CREATED) class PasswordResetView(APIView): @@ -363,13 +355,14 @@ def post(self, request, *args, **kwargs): user = User.objects.get(email=serializer.validated_data["email"]) if validate_otp( - serializer.validated_data["email"], serializer.validated_data["otp"] + destination=serializer.validated_data["email"], + otp_val=serializer.validated_data["otp"], ): # OTP Validated, Change Password user.set_password(serializer.validated_data["password"]) user.save() - return JsonResponse( - content="Password Updated Successfully.", + return Response( + data="Password Updated Successfully.", status=status.HTTP_202_ACCEPTED, ) @@ -381,11 +374,6 @@ class UploadImageView(APIView): attached to `profile_image` parameter. """ - from .models import User - from .serializers import ImageSerializer - from rest_framework.permissions import IsAuthenticated - from rest_framework.parsers import MultiPartParser - queryset = User.objects.all() serializer_class = ImageSerializer permission_classes = (IsAuthenticated,) diff --git a/example/demo_app/settings.py b/example/demo_app/settings.py index 2ceee6b..4b1569b 100644 --- a/example/demo_app/settings.py +++ b/example/demo_app/settings.py @@ -38,7 +38,6 @@ "django.contrib.messages", "django.contrib.staticfiles", "drf_user", - "drfaddons", "rest_framework", "django_filters", "drf_yasg", diff --git a/requirements.txt b/requirements.txt index 8e7c1d2..b25370a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ Django>=3.2 django-filter==22.1 +django-sendsms>=0.3.1 djangorestframework>=3.12 djangorestframework-simplejwt>=5.0.0 -drfaddons>=0.1.0 Pillow>=8.0.0 diff --git a/tests/settings.py b/tests/settings.py index e7bdfec..68f2743 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -13,7 +13,6 @@ "django.contrib.messages", "django.contrib.staticfiles", "drf_user", - "drfaddons", "rest_framework", "django_filters", ) diff --git a/tests/test_utils.py b/tests/test_utils.py index fe18c47..a14afe4 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -9,6 +9,7 @@ from rest_framework.exceptions import AuthenticationFailed from drf_user import utils as utils +from drf_user.constants import CoreConstants from drf_user.models import OTPValidation from drf_user.models import User from drf_user.utils import get_client_ip @@ -71,7 +72,9 @@ class TestGenerateOTP(TestCase): @pytest.mark.django_db def test_generate_otp(self): """Check generate_otp successfully generates OTPValidation object or not""" - utils.generate_otp("email", "user1@email.com") + utils.generate_otp( + destination_property=CoreConstants.EMAIL_PROP, destination="user1@email.com" + ) self.assertEqual(1, OTPValidation.objects.count()) @pytest.mark.django_db @@ -79,8 +82,12 @@ def test_generate_otp_reactive_past(self): """ Check generate_otp generates a new otp if the reactive time is yet to be over """ - otp_validation1 = utils.generate_otp("email", "user1@email.com") - otp_validation2 = utils.generate_otp("email", "user1@email.com") + otp_validation1 = utils.generate_otp( + destination_property=CoreConstants.EMAIL_PROP, destination="user1@email.com" + ) + otp_validation2 = utils.generate_otp( + destination_property=CoreConstants.EMAIL_PROP, destination="user1@email.com" + ) self.assertNotEqual(otp_validation1.otp, otp_validation2.otp) @pytest.mark.django_db @@ -88,7 +95,9 @@ def test_generate_otp_reactive_future(self): """ Check generate_otp returns the same otp if the reactive time is already over """ - otp_validation1 = utils.generate_otp("email", "user1@email.com") + otp_validation1 = utils.generate_otp( + destination_property=CoreConstants.EMAIL_PROP, destination="user1@email.com" + ) """ Simulating that the reactive time is already been over 5 minutes ago @@ -96,7 +105,9 @@ def test_generate_otp_reactive_future(self): otp_validation1.reactive_at = timezone.now() + datetime.timedelta(minutes=5) otp_validation1.save() - otp_validation2 = utils.generate_otp("email", "user1@email.com") + otp_validation2 = utils.generate_otp( + destination_property=CoreConstants.EMAIL_PROP, destination="user1@email.com" + ) self.assertEqual(otp_validation2.otp, otp_validation1.otp) @@ -117,7 +128,7 @@ def test_object_created(self): @pytest.mark.django_db def test_validate_otp(self): """Check if OTPValidation object is created or not""" - self.assertTrue(utils.validate_otp("user@email.com", 12345)) + self.assertTrue(utils.validate_otp(destination="user@email.com", otp_val=12345)) @pytest.mark.django_db def test_validate_otp_raises_attempt_exceeded_exception(self): @@ -130,7 +141,7 @@ def test_validate_otp_raises_attempt_exceeded_exception(self): self.otp_validation.save() with self.assertRaises(AuthenticationFailed) as context_manager: - utils.validate_otp("user@email.com", 56123) + utils.validate_otp(destination="user@email.com", otp_val=56123) self.assertEqual( "Incorrect OTP. Attempt exceeded! OTP has been reset.", @@ -141,7 +152,7 @@ def test_validate_otp_raises_attempt_exceeded_exception(self): def test_validate_otp_raises_invalid_otp_exception(self): """Check function raises attempt exceeded exception""" with self.assertRaises(AuthenticationFailed) as context_manager: - utils.validate_otp("user@email.com", 5623) + utils.validate_otp(destination="user@email.com", otp_val=5623) self.assertEqual( "OTP Validation failed! 2 attempts left!", diff --git a/tests/test_views.py b/tests/test_views.py index 4722164..aaf5a5c 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -5,6 +5,7 @@ from django.test import override_settings from django.urls import reverse from model_bakery import baker +from rest_framework import status from rest_framework.test import APITestCase from rest_framework_simplejwt.tokens import RefreshToken @@ -180,21 +181,21 @@ def test_is_unique(self): response = self.client.post(self.url, {"prop": "username", "value": "user7"}) self.assertEqual(200, response.status_code) - self.assertTrue(response.json()["data"][0]["unique"]) + self.assertTrue(response.json()["unique"]) @pytest.mark.django_db def test_is_not_unique(self): """Check if the user is not unique""" response = self.client.post(self.url, {"prop": "username", "value": "user"}) - self.assertEqual(200, response.status_code) - self.assertFalse(response.json()["data"][0]["unique"]) + self.assertEqual(status.HTTP_200_OK, response.status_code) + self.assertFalse(response.json()["unique"]) @pytest.mark.django_db def test_data_invalid(self): """Check CheckUniqueView view raises 422 code when passed data is invalid""" response = self.client.post(self.url, {"prop": "invalid", "value": "user"}) - self.assertEqual(422, response.status_code) + self.assertEqual(status.HTTP_422_UNPROCESSABLE_ENTITY, response.status_code) class TestRegisterView(APITestCase): @@ -309,8 +310,8 @@ def test_request_otp_on_email(self): self.url, {"destination": "email@django.com", "email": "email@django.com"} ) - self.assertEqual(201, response.status_code) - self.assertEqual("Message sent successfully!", response.json()["message"]) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(response.json()["message"], "Email Message sent successfully!") @pytest.mark.django_db def test_request_otp_on_email_and_mobile(self): @@ -320,26 +321,36 @@ def test_request_otp_on_email_and_mobile(self): """ response = self.client.post( - self.url, {"destination": 1231242492, "email": "email@django.com"} + self.url, {"destination": 9999999999, "email": "email@django.com"} ) - self.assertEqual(201, response.status_code) - self.assertEqual("Message sent successfully!", response.json()["message"]) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(response.json()["message"], "Email Message sent successfully!") + self.assertEqual( + response.json()["mobile_message"], "Mobile Message sent successfully!" + ) @pytest.mark.django_db - def test_raise_api_exception_when_email_invalid(self): - """Checks OTPView raises validation error when email/mobile is invalid""" + def test_raise_api_exception_when_destination_as_mobile_is_invalid(self): + """Checks OTPView raises validation error when mobile is invalid""" response = self.client.post( self.url, {"destination": "a.b", "email": "abc@d.com"} ) - - self.assertEqual(500, response.status_code) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual( - "Server configuration error occurred: Invalid recipient.", response.json()["detail"], + "OTP sending failed! because ['Invalid Mobile Number']", ) + @pytest.mark.django_db + def test_raise_api_exception_when_email_is_invalid(self): + """Checks OTPView raises validation error when email is invalid""" + + response = self.client.post(self.url, {"destination": "abc", "email": "abc"}) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json()["email"], ["Enter a valid email address."]) + @pytest.mark.django_db def test_raise_validation_error_when_email_not_response_when_user_is_new(self): """ @@ -350,10 +361,10 @@ def test_raise_validation_error_when_email_not_response_when_user_is_new(self): response = self.client.post(self.url, {"destination": "email@django.com"}) self.assertEqual( - ["email field is compulsory while verifying a non-existing user's OTP."], response.json()["non_field_errors"], + ["Email field is compulsory while verifying a non-existing user's OTP."], ) - self.assertEqual(400, response.status_code) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) @pytest.mark.django_db def test_raise_validation_error_when_is_login_response_when_user_is_new(self): @@ -410,7 +421,7 @@ def setUp(self) -> None: "drf_user.User", username="my_user", email="my_user@django.com", - mobile=2848482848, + mobile=9848482848, ) # create otp of registered user self.user_otp = baker.make( @@ -424,7 +435,7 @@ def setUp(self) -> None: self.data = { "name": "random_name", "email": "random@django.com", - "mobile": 1234567890, + "mobile": 7634567890, } self.data_with_incorrect_email_mobile = { "name": "name", @@ -434,37 +445,37 @@ def setUp(self) -> None: self.data_with_correct_otp = { "name": "random_name", "email": "random@django.com", - "mobile": 1234567890, + "mobile": 9234567890, "verify_otp": 888383, } self.data_with_incorrect_otp = { "name": "random_name", "email": "random@django.com", - "mobile": 1234567890, + "mobile": 7334567890, "verify_otp": 999999, } self.data_registered_user = { "name": "my_user", "email": "my_user@django.com", - "mobile": 2848482848, + "mobile": 6848482848, "verify_otp": 437474, } self.data_registered_user_with_different_mobile = { "name": "my_user", "email": "my_user@django.com", - "mobile": 2846482848, + "mobile": 7846482848, "verify_otp": 437474, } self.data_registered_user_with_different_email = { "name": "my_user", "email": "ser@django.com", - "mobile": 2848482848, + "mobile": 6848482848, "verify_otp": 437474, } self.data_random_user = { "name": "test_user1", "email": "test_user1@django.com", - "mobile": 2848444448, + "mobile": 8848444448, "verify_otp": 585858, } @@ -565,7 +576,7 @@ def test_login_with_correct_otp_for_new_user(self): self.url, data=self.data_with_correct_otp, format="json" ) - self.assertEqual(202, response.status_code) + self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) self.assertContains(text="token", response=response, status_code=202) self.assertTrue(User.objects.get(email="random@django.com")) From 40b5855c8b15e128caf3581eb005d91d4866851a Mon Sep 17 00:00:00 2001 From: Sumit Singh Date: Sat, 15 Oct 2022 13:15:54 +0530 Subject: [PATCH 2/5] Ran black :art: --- .pre-commit-config.yaml | 3 +- Makefile | 2 +- drf_user/__init__.py | 4 +-- drf_user/managers.py | 4 +-- drf_user/models.py | 24 ++++----------- drf_user/serializers.py | 20 ++++--------- drf_user/urls.py | 8 ++--- drf_user/utils.py | 24 ++++----------- drf_user/views.py | 40 +++++++------------------ example/demo_app/urls.py | 4 +-- tests/test_models.py | 4 +-- tests/test_views.py | 64 ++++++++++------------------------------ 12 files changed, 52 insertions(+), 149 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6794be5..1849bdc 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -4,7 +4,8 @@ repos: rev: 22.10.0 hooks: - id: black - + args: + - --line-length=100 - repo: https://github.com/PyCQA/flake8 rev: 5.0.4 hooks: diff --git a/Makefile b/Makefile index b43c7f3..9108165 100644 --- a/Makefile +++ b/Makefile @@ -64,7 +64,7 @@ docs: python setup.py build_sphinx lint: - flake8 --exclude=*/migrations/* --max-line-length 88 drf_user + flake8 --exclude=*/migrations/* --max-line-length 100 drf_user format: black --exclude .+/migrations/.+\.py drf_user diff --git a/drf_user/__init__.py b/drf_user/__init__.py index 0e8700f..e64bf3c 100644 --- a/drf_user/__init__.py +++ b/drf_user/__init__.py @@ -60,9 +60,7 @@ def update_user_settings() -> dict: user_settings["OTP"][otp_key] = otp_value elif key == "REGISTRATION": if not isinstance(value, dict): - raise TypeError( - "USER_SETTING attribute REGISTRATION must be a dict." - ) + raise TypeError("USER_SETTING attribute REGISTRATION must be a dict.") for reg_key, reg_value in value.items(): user_settings["REGISTRATION"][reg_key] = reg_value if user_settings["REGISTRATION"]["SEND_MAIL"]: diff --git a/drf_user/managers.py b/drf_user/managers.py index b06ae0a..bb9ae12 100644 --- a/drf_user/managers.py +++ b/drf_user/managers.py @@ -30,9 +30,7 @@ def _create_user( Creates and saves a User with given details """ email = self.normalize_email(email) - user = self.model( - username=username, email=email, name=fullname, mobile=mobile, **kwargs - ) + user = self.model(username=username, email=email, name=fullname, mobile=mobile, **kwargs) user.set_password(password) user.save(using=self._db) return user diff --git a/drf_user/models.py b/drf_user/models.py index 726a5cf..bf46164 100644 --- a/drf_user/models.py +++ b/drf_user/models.py @@ -33,9 +33,7 @@ class User(AbstractBaseUser, PermissionsMixin): Author: Himanshu Shankar (https://himanshus.com) """ - username = models.CharField( - verbose_name=_("Unique UserName"), max_length=254, unique=True - ) + username = models.CharField(verbose_name=_("Unique UserName"), max_length=254, unique=True) email = models.EmailField(verbose_name=_("Email Address"), unique=True) mobile = models.CharField( verbose_name=_("Mobile Number"), @@ -103,15 +101,9 @@ class AuthTransaction(models.Model): blank=True, verbose_name=_("JWT Refresh Token"), ) - expires_at = models.DateTimeField( - blank=True, null=True, verbose_name=_("Expires At") - ) - create_date = models.DateTimeField( - verbose_name=_("Create Date/Time"), auto_now_add=True - ) - update_date = models.DateTimeField( - verbose_name=_("Date/Time Modified"), auto_now=True - ) + expires_at = models.DateTimeField(blank=True, null=True, verbose_name=_("Expires At")) + create_date = models.DateTimeField(verbose_name=_("Create Date/Time"), auto_now_add=True) + update_date = models.DateTimeField(verbose_name=_("Date/Time Modified"), auto_now=True) created_by = models.ForeignKey(to=User, on_delete=models.PROTECT) def __str__(self): @@ -142,9 +134,7 @@ class OTPValidation(models.Model): create_date = models.DateTimeField(verbose_name=_("Create Date"), auto_now_add=True) update_date = models.DateTimeField(verbose_name=_("Date Modified"), auto_now=True) is_validated = models.BooleanField(verbose_name=_("Is Validated"), default=False) - validate_attempt = models.IntegerField( - verbose_name=_("Attempted Validation"), default=3 - ) + validate_attempt = models.IntegerField(verbose_name=_("Attempted Validation"), default=3) prop = models.CharField( verbose_name=_("Destination Property"), default=EMAIL, @@ -152,9 +142,7 @@ class OTPValidation(models.Model): choices=DESTINATION_CHOICES, ) send_counter = models.IntegerField(verbose_name=_("OTP Sent Counter"), default=0) - sms_id = models.CharField( - verbose_name=_("SMS ID"), max_length=254, null=True, blank=True - ) + sms_id = models.CharField(verbose_name=_("SMS ID"), max_length=254, null=True, blank=True) reactive_at = models.DateTimeField(verbose_name=_("ReActivate Sending OTP")) def __str__(self): diff --git a/drf_user/serializers.py b/drf_user/serializers.py index e8258d1..3c19069 100644 --- a/drf_user/serializers.py +++ b/drf_user/serializers.py @@ -38,9 +38,7 @@ def validate_email(self, value: str) -> str: if check_validation(value=value): return value else: - raise serializers.ValidationError( - "The email must be " "pre-validated via OTP." - ) + raise serializers.ValidationError("The email must be " "pre-validated via OTP.") def validate_mobile(self, value: str) -> str: """ @@ -61,9 +59,7 @@ def validate_mobile(self, value: str) -> str: if check_validation(value=value): return value else: - raise serializers.ValidationError( - "The mobile must be " "pre-validated via OTP." - ) + raise serializers.ValidationError("The mobile must be " "pre-validated via OTP.") def validate_password(self, value: str) -> str: """Validate whether the password meets all django validator requirements.""" @@ -201,9 +197,7 @@ def validate(self, attrs: dict) -> dict: raise NotFound(_("No user exists with provided details")) if "email" not in attrs.keys() and "verify_otp" not in attrs.keys(): raise serializers.ValidationError( - _( - "Email field is compulsory while verifying a non-existing user's OTP." - ) + _("Email field is compulsory while verifying a non-existing user's OTP.") ) else: attrs["email"] = user.email @@ -274,9 +268,7 @@ def get_user(email: str, mobile: str): def validate(self, attrs: dict) -> dict: """Validates the response""" - attrs["user"] = self.get_user( - email=attrs.get("email"), mobile=attrs.get("mobile") - ) + attrs["user"] = self.get_user(email=attrs.get("email"), mobile=attrs.get("mobile")) return attrs @@ -362,9 +354,7 @@ class CustomTokenObtainPairSerializer(TokenObtainPairSerializer): certain extra data in payload such as: email, mobile, name """ - default_error_messages = { - "no_active_account": _("username or password is invalid.") - } + default_error_messages = {"no_active_account": _("username or password is invalid.")} @classmethod def get_token(cls, user): diff --git a/drf_user/urls.py b/drf_user/urls.py index 732a65b..6d8396b 100644 --- a/drf_user/urls.py +++ b/drf_user/urls.py @@ -23,13 +23,9 @@ name="Retrieve Update Profile", ), # ex: api/user/password/reset/ - path( - "password/reset/", views.PasswordResetView.as_view(), name="reset_user_password" - ), + path("password/reset/", views.PasswordResetView.as_view(), name="reset_user_password"), # ex: api/user/upload-image/ path("upload-image/", views.UploadImageView.as_view(), name="upload_profile_image"), # ex: api/user/refresh-token/ - path( - "refresh-token/", views.CustomTokenRefreshView.as_view(), name="refresh_token" - ), + path("refresh-token/", views.CustomTokenRefreshView.as_view(), name="refresh_token"), ] diff --git a/drf_user/utils.py b/drf_user/utils.py index 6dea184..a8c61b6 100644 --- a/drf_user/utils.py +++ b/drf_user/utils.py @@ -130,9 +130,7 @@ def generate_otp(*, destination_property: str, destination: str) -> OTPValidatio # Checks if random number is unique among non-validated OTPs and # creates new until it is unique. - while OTPValidation.objects.filter(otp__exact=random_number).filter( - is_validated=False - ): + while OTPValidation.objects.filter(otp__exact=random_number).filter(is_validated=False): random_number: str = User.objects.make_random_password( length=otp_settings["LENGTH"], allowed_chars=otp_settings["ALLOWED_CHARS"] ) @@ -299,17 +297,13 @@ def validate_otp(*, destination: str, otp_val: int) -> bool: elif otp_object.validate_attempt <= 0: # check if attempts exceeded and regenerate otp and raise error generate_otp(destination_property=otp_object.prop, destination=destination) - raise AuthenticationFailed( - detail=_("Incorrect OTP. Attempt exceeded! OTP has been reset.") - ) + raise AuthenticationFailed(detail=_("Incorrect OTP. Attempt exceeded! OTP has been reset.")) else: # update attempts and raise error otp_object.save(update_fields=["validate_attempt"]) raise AuthenticationFailed( - detail=_( - f"OTP Validation failed! {otp_object.validate_attempt} attempts left!" - ) + detail=_(f"OTP Validation failed! {otp_object.validate_attempt} attempts left!") ) @@ -343,9 +337,7 @@ def send_message( sent = {"success": False, "message": None, "mobile_message": None} if not getattr(settings, "EMAIL_HOST", None): - raise ValueError( - "EMAIL_HOST must be defined in django setting for sending mail." - ) + raise ValueError("EMAIL_HOST must be defined in django setting for sending mail.") if not getattr(settings, "EMAIL_FROM", None): raise ValueError( "EMAIL_FROM must be defined in django setting " @@ -416,13 +408,9 @@ def send_otp( ) try: - data: dict = send_message( - message, otp_settings["SUBJECT"], recip_email, recip_mobile - ) + data: dict = send_message(message, otp_settings["SUBJECT"], recip_email, recip_mobile) except (ValueError, ValidationError) as e: - raise serializers.ValidationError( - {"detail": f"OTP sending failed! because {e}"} - ) from e + raise serializers.ValidationError({"detail": f"OTP sending failed! because {e}"}) from e otp_obj.reactive_at = timezone.now() + datetime.timedelta( minutes=otp_settings["COOLING_PERIOD"] diff --git a/drf_user/views.py b/drf_user/views.py index a9ed099..71c4755 100644 --- a/drf_user/views.py +++ b/drf_user/views.py @@ -193,21 +193,15 @@ def post(self, request, *args, **kwargs): destination: str = serializer.validated_data[ "destination" ] # destination is a required field - destination_property: str = serializer.validated_data.get( - "prop" - ) # can be email or mobile + destination_property: str = serializer.validated_data.get("prop") # can be email or mobile user: User = serializer.validated_data.get("user") email: Optional[str] = serializer.validated_data.get("email") is_login: bool = serializer.validated_data.get("is_login") if "verify_otp" in request.data.keys(): - if validate_otp( - destination=destination, otp_val=request.data["verify_otp"] - ): + if validate_otp(destination=destination, otp_val=request.data["verify_otp"]): if is_login: - return Response( - login_user(user, self.request), status=status.HTTP_202_ACCEPTED - ) + return Response(login_user(user, self.request), status=status.HTTP_202_ACCEPTED) else: return Response( data={"OTP": _("OTP Validated successfully!")}, @@ -257,9 +251,7 @@ def get_object(self): def update(self, request, *args, **kwargs): """Updates user's password""" - response = super(RetrieveUpdateUserAccountView, self).update( - request, *args, **kwargs - ) + response = super(RetrieveUpdateUserAccountView, self).update(request, *args, **kwargs) # we need to set_password after save the user otherwise it'll save the raw_password in db. # noqa if "password" in request.data.keys(): self.request.user.set_password(request.data["password"]) @@ -311,17 +303,11 @@ def post(self, request, *args, **kwargs): ) user.is_active = True user.save(update_fields=["is_active"]) - return Response( - login_user(user, self.request), status=status.HTTP_202_ACCEPTED - ) + return Response(login_user(user, self.request), status=status.HTTP_202_ACCEPTED) - otp_obj: OTPValidation = generate_otp( - destination_property=EMAIL, destination=email - ) + otp_obj: OTPValidation = generate_otp(destination_property=EMAIL, destination=email) # Send OTP to Email & Mobile - sent_otp_resp: dict = send_otp( - otp_obj=otp_obj, recip_email=email, recip_mobile=mobile - ) + sent_otp_resp: dict = send_otp(otp_obj=otp_obj, recip_email=email, recip_mobile=mobile) if not sent_otp_resp["success"]: raise serializers.ValidationError( @@ -394,9 +380,7 @@ def post(self, request, *args, **kwargs): image_serializer.update( instance=request.user, validated_data=image_serializer.validated_data ) - return Response( - {"detail": "Profile Image Uploaded."}, status=status.HTTP_201_CREATED - ) + return Response({"detail": "Profile Image Uploaded."}, status=status.HTTP_201_CREATED) class CustomTokenRefreshView(TokenRefreshView): @@ -419,13 +403,9 @@ def post(self, request, *args, **kwargs): token = serializer.validated_data.get("access") - auth_transaction = AuthTransaction.objects.get( - refresh_token=request.data["refresh"] - ) + auth_transaction = AuthTransaction.objects.get(refresh_token=request.data["refresh"]) auth_transaction.token = token - auth_transaction.expires_at = ( - timezone.now() + api_settings.ACCESS_TOKEN_LIFETIME - ) + auth_transaction.expires_at = timezone.now() + api_settings.ACCESS_TOKEN_LIFETIME auth_transaction.save(update_fields=["token", "expires_at"]) return Response({"token": str(token)}, status=status.HTTP_200_OK) diff --git a/example/demo_app/urls.py b/example/demo_app/urls.py index dc8b4ca..c9f2b7d 100644 --- a/example/demo_app/urls.py +++ b/example/demo_app/urls.py @@ -48,7 +48,5 @@ schema_view.with_ui("swagger", cache_timeout=0), name="schema-swagger-ui", ), - re_path( - r"^redoc/$", schema_view.with_ui("redoc", cache_timeout=0), name="schema-redoc" - ), + re_path(r"^redoc/$", schema_view.with_ui("redoc", cache_timeout=0), name="schema-redoc"), ] diff --git a/tests/test_models.py b/tests/test_models.py index cbeb1be..7468107 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -13,9 +13,7 @@ class TestUserModel(TestCase): def setUp(self) -> None: """Create user object using model_bakery""" - self.user = baker.make( - "drf_user.User", name="test_user", username="my_unique_username" - ) + self.user = baker.make("drf_user.User", name="test_user", username="my_unique_username") @pytest.mark.django_db def test_object_created(self): diff --git a/tests/test_views.py b/tests/test_views.py index aaf5a5c..f911406 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -49,9 +49,7 @@ def test_object_created(self): @pytest.mark.django_db def test_successful_login_view(self): """Check if the credentials are correct""" - response = self.client.post( - self.url, data={"username": "user", "password": "pass123"} - ) + response = self.client.post(self.url, data={"username": "user", "password": "pass123"}) self.assertEqual(200, response.status_code) self.assertIn("token", response.data) self.assertIn("refresh_token", response.data) @@ -88,9 +86,7 @@ def test_login_using_email_as_username(self): @pytest.mark.django_db def test_unsuccessful_login_view(self): """Check if the credentials are incorrect""" - response = self.client.post( - self.url, data={"username": "user", "password": "pass1234"} - ) + response = self.client.post(self.url, data={"username": "user", "password": "pass1234"}) self.assertEqual(403, response.status_code) self.assertIn("username or password is invalid.", response.data["detail"]) @@ -251,12 +247,8 @@ def test_raise_validation_error_when_email_mobile_not_validated(self): response = self.client.post(self.url, self.not_validated_data) self.assertEqual(400, response.status_code) - self.assertEqual( - ["The email must be pre-validated via OTP."], response.json()["email"] - ) - self.assertEqual( - ["The mobile must be pre-validated via OTP."], response.json()["mobile"] - ) + self.assertEqual(["The email must be pre-validated via OTP."], response.json()["email"]) + self.assertEqual(["The mobile must be pre-validated via OTP."], response.json()["mobile"]) @pytest.mark.django_db def test_register_user_without_mobile_number(self): @@ -326,17 +318,13 @@ def test_request_otp_on_email_and_mobile(self): self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(response.json()["message"], "Email Message sent successfully!") - self.assertEqual( - response.json()["mobile_message"], "Mobile Message sent successfully!" - ) + self.assertEqual(response.json()["mobile_message"], "Mobile Message sent successfully!") @pytest.mark.django_db def test_raise_api_exception_when_destination_as_mobile_is_invalid(self): """Checks OTPView raises validation error when mobile is invalid""" - response = self.client.post( - self.url, {"destination": "a.b", "email": "abc@d.com"} - ) + response = self.client.post(self.url, {"destination": "a.b", "email": "abc@d.com"}) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual( response.json()["detail"], @@ -373,13 +361,9 @@ def test_raise_validation_error_when_is_login_response_when_user_is_new(self): only passes is_login """ - response = self.client.post( - self.url, {"destination": "email@django.com", "is_login": True} - ) + response = self.client.post(self.url, {"destination": "email@django.com", "is_login": True}) - self.assertEqual( - "No user exists with provided details", response.json()["detail"] - ) + self.assertEqual("No user exists with provided details", response.json()["detail"]) self.assertEqual(404, response.status_code) @pytest.mark.django_db @@ -532,25 +516,17 @@ def test_sent_otp_when_name_email_mobile_is_passed(self): response = self.client.post(self.url, data=self.data, format="json") self.assertEqual(201, response.status_code) - self.assertEqual( - "OTP has been sent successfully.", response.json()["email"]["otp"] - ) - self.assertEqual( - "OTP has been sent successfully.", response.json()["mobile"]["otp"] - ) + self.assertEqual("OTP has been sent successfully.", response.json()["email"]["otp"]) + self.assertEqual("OTP has been sent successfully.", response.json()["mobile"]["otp"]) @pytest.mark.django_db def test_login_with_incorrect_otp_for_registered_user(self): """Check when data with correct otp is passed, token is generated or not""" - response = self.client.post( - self.url, data=self.data_with_incorrect_otp, format="json" - ) + response = self.client.post(self.url, data=self.data_with_incorrect_otp, format="json") self.assertEqual(403, response.status_code) - self.assertEqual( - "OTP Validation failed! 2 attempts left!", response.json()["detail"] - ) + self.assertEqual("OTP Validation failed! 2 attempts left!", response.json()["detail"]) @pytest.mark.django_db def test_login_with_incorrect_otp_for_new_user_without_validated_otp(self): @@ -572,9 +548,7 @@ def test_login_with_correct_otp_for_new_user(self): and user is created """ - response = self.client.post( - self.url, data=self.data_with_correct_otp, format="json" - ) + response = self.client.post(self.url, data=self.data_with_correct_otp, format="json") self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) self.assertContains(text="token", response=response, status_code=202) @@ -700,18 +674,14 @@ def test_when_nothing_is_passed(self): @pytest.mark.django_db def test_when_incorrect_email_passed(self): """Check when incorrect email is passed as data then api raises 404""" - response = self.client.post( - self.url, data=self.data_incorrect_email, format="json" - ) + response = self.client.post(self.url, data=self.data_incorrect_email, format="json") self.assertEqual(404, response.status_code) @pytest.mark.django_db def test_when_incorrect_otp_passed(self): """Check when incorrect otp is passed as data then api raises 403""" - response = self.client.post( - self.url, data=self.data_incorrect_otp, format="json" - ) + response = self.client.post(self.url, data=self.data_incorrect_otp, format="json") self.assertEqual(403, response.status_code) @@ -760,9 +730,7 @@ def test_when_upload_image_passed(self): self.client.force_authenticate(self.user) with open(f"{BASE_DIR}/tests/fixtures/test.jpg", "rb") as f: - response = self.client.post( - self.url, data={"profile_image": f}, format="multipart" - ) + response = self.client.post(self.url, data={"profile_image": f}, format="multipart") self.assertEqual(201, response.status_code) self.assertEqual("Profile Image Uploaded.", response.json()["detail"]) From 24f79c006f7f265abb172cb19689a254044ea586 Mon Sep 17 00:00:00 2001 From: Sumit Singh Date: Sat, 15 Oct 2022 13:26:02 +0530 Subject: [PATCH 3/5] Fix flake 8 issues --- drf_user/utils.py | 7 ++++--- drf_user/views.py | 5 ++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drf_user/utils.py b/drf_user/utils.py index a8c61b6..90b6b50 100644 --- a/drf_user/utils.py +++ b/drf_user/utils.py @@ -251,9 +251,10 @@ def validate_mobile(mobile: str) -> bool: >>> print(validate_mobile('9999999999')) True """ - if is_valid := re.match(r"^[6-9]\d{9}$", mobile) is not None: - return is_valid - raise ValidationError("Invalid Mobile Number") + match = re.match(r"^[6-9]\d{9}$", str(mobile)) + if match is None: + raise ValidationError("Invalid Mobile Number") + return True def validate_otp(*, destination: str, otp_val: int) -> bool: diff --git a/drf_user/views.py b/drf_user/views.py index 71c4755..3e93908 100644 --- a/drf_user/views.py +++ b/drf_user/views.py @@ -190,9 +190,8 @@ def post(self, request, *args, **kwargs): serializer: OTPSerializer = self.serializer_class(data=request.data) serializer.is_valid(raise_exception=True) - destination: str = serializer.validated_data[ - "destination" - ] # destination is a required field + # destination is a required field + destination: str = serializer.validated_data["destination"] destination_property: str = serializer.validated_data.get("prop") # can be email or mobile user: User = serializer.validated_data.get("user") email: Optional[str] = serializer.validated_data.get("email") From c5bd09e69d9f6a74347785c3e6b0f4c7600e2bd5 Mon Sep 17 00:00:00 2001 From: Sumit Singh Date: Fri, 21 Oct 2022 09:44:44 +0530 Subject: [PATCH 4/5] Fix some more tests and other improvements --- docs/conf.py | 5 +- drf_user/admin.py | 20 +- drf_user/serializers.py | 45 ++-- drf_user/utils.py | 19 +- drf_user/views.py | 7 +- tests/fixtures/send_otp_responses.py | 7 + tests/test_views.py | 338 ++++++++++++++++++++------- 7 files changed, 308 insertions(+), 133 deletions(-) create mode 100644 tests/fixtures/send_otp_responses.py diff --git a/docs/conf.py b/docs/conf.py index c1eca40..f684c5a 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -11,14 +11,13 @@ # documentation root, use os.path.abspath to make it absolute, like shown here. # import os - -# sys.path.insert(0, os.path.abspath('.')) +from datetime import datetime # -- Project information ----------------------------------------------------- project = "drf-user" -copyright = "2020, 101 Loop" +copyright = f"{datetime.now().year}, 101 Loop" author = "101 Loop" parent_dir = os.path.dirname(os.path.dirname(__file__)) diff --git a/drf_user/admin.py b/drf_user/admin.py index 272e4a0..876d2cb 100644 --- a/drf_user/admin.py +++ b/drf_user/admin.py @@ -1,26 +1,18 @@ """ All Admin configuration related to drf_user - -Author: Himanshu Shankar (https://himanshus.com) """ from django.contrib import admin -from django.contrib.auth.admin import Group -from django.contrib.auth.admin import GroupAdmin -from django.contrib.auth.admin import UserAdmin +from django.contrib.auth.admin import Group, GroupAdmin, UserAdmin from django.utils.text import gettext_lazy as _ -from .models import AuthTransaction -from .models import OTPValidation -from .models import Role -from .models import User +from drf_user.models import AuthTransaction, OTPValidation, Role, User +@admin.register(User) class DRFUserAdmin(UserAdmin): """ Overrides UserAdmin to show fields name & mobile and remove fields: first_name, last_name - - Author: Himanshu Shankar (https://himanshus.com) """ fieldsets = ( @@ -57,12 +49,14 @@ class DRFUserAdmin(UserAdmin): readonly_fields = ("date_joined", "last_login", "update_date") +@admin.register(OTPValidation) class OTPValidationAdmin(admin.ModelAdmin): """OTP Validation Admin""" list_display = ("destination", "otp", "prop") +@admin.register(AuthTransaction) class AuthTransactionAdmin(admin.ModelAdmin): """AuthTransaction Admin""" @@ -89,7 +83,3 @@ def has_delete_permission(self, request, obj=None): # Source: https://stackoverflow.com/a/32445368 admin.site.unregister(Group) admin.site.register(Role, GroupAdmin) - -admin.site.register(User, DRFUserAdmin) -admin.site.register(OTPValidation, OTPValidationAdmin) -admin.site.register(AuthTransaction, AuthTransactionAdmin) diff --git a/drf_user/serializers.py b/drf_user/serializers.py index 3c19069..0a1825f 100644 --- a/drf_user/serializers.py +++ b/drf_user/serializers.py @@ -9,7 +9,7 @@ from drf_user import user_settings from drf_user.models import User -from drf_user.utils import check_validation +from drf_user.utils import check_validation, is_mobile_valid from drf_user.constants import EMAIL, MOBILE @@ -234,6 +234,11 @@ class OTPLoginRegisterSerializer(serializers.Serializer): verify_otp = serializers.CharField(default=None, required=False) mobile = serializers.CharField(required=True) + def validate_mobile(self, value: str) -> str: + """Validate whether the mobile is unique.""" + is_mobile_valid(value) + return value + @staticmethod def get_user(email: str, mobile: str): """Fetches user object""" @@ -242,27 +247,27 @@ def get_user(email: str, mobile: str): except User.DoesNotExist: try: user = User.objects.get(mobile=mobile) - except User.DoesNotExist as e: - raise NotFound( - _(f"No user exists either for email={email} or mobile={mobile}") - ) from e - - if user.email != email: - raise serializers.ValidationError( - _( - "Your account is registered with {mobile} does not has " - "{email} as registered email. Please login directly via " - "OTP with your mobile.".format(mobile=mobile, email=email) + except User.DoesNotExist: + # new user is trying to register + user = None + + if user: + if user.email != email: + raise serializers.ValidationError( + _( + f"Your account is registered with {mobile} does not has " + f"{email} as registered email. Please login directly via " + "OTP with your mobile." + ) ) - ) - if user.mobile != mobile: - raise serializers.ValidationError( - _( - "Your account is registered with {email} does not has " - "{mobile} as registered mobile. Please login directly via " - "OTP with your email.".format(mobile=mobile, email=email) + if user.mobile != mobile: + raise serializers.ValidationError( + _( + f"Your account is registered with {email} does not has " + f"{mobile} as registered mobile. Please login directly via " + "OTP with your email." + ) ) - ) return user def validate(self, attrs: dict) -> dict: diff --git a/drf_user/utils.py b/drf_user/utils.py index 90b6b50..1292221 100644 --- a/drf_user/utils.py +++ b/drf_user/utils.py @@ -233,7 +233,7 @@ def check_validation(value: str) -> bool: return False -def validate_mobile(mobile: str) -> bool: +def is_mobile_valid(mobile: str) -> bool: """ This function checks if the mobile number is valid or not. Parameters @@ -248,12 +248,12 @@ def validate_mobile(mobile: str) -> bool: Examples -------- To check if '9999999999' is a valid mobile number - >>> print(validate_mobile('9999999999')) + >>> print(is_mobile_valid('9999999999')) True """ match = re.match(r"^[6-9]\d{9}$", str(mobile)) if match is None: - raise ValidationError("Invalid Mobile Number") + raise ValidationError("Enter a valid mobile number.") return True @@ -281,8 +281,8 @@ def validate_otp(*, destination: str, otp_val: int) -> bool: except OTPValidation.DoesNotExist as e: raise NotFound( detail=_( - "No pending OTP validation request found for provided destination." - " Kindly send an OTP first" + f"No pending OTP validation request found for provided {destination}." + " Kindly send an OTP first." ) ) from e @@ -350,7 +350,7 @@ def send_message( if recip_mobile: # check for valid mobile numbers - validate_mobile(recip_mobile) + is_mobile_valid(recip_mobile) try: send_mail( @@ -361,7 +361,8 @@ def send_message( recipient_list=[recip_email], ) except Exception as e: # noqa - sent["message"] = f"Email Message sending failed! {str(e.args)}" + logger.error("Email sending failed", exc_info=e) + sent["message"] = str(e) sent["success"] = False else: sent["message"] = "Email Message sent successfully!" @@ -371,8 +372,8 @@ def send_message( try: api.send_sms(body=message, to=recip_mobile, from_phone=None) except Exception as e: # noqa - logger.debug("Message sending failed", exc_info=e) - sent["mobile_message"] = f"Mobile Message sending failed! {str(e.args)}" + logger.error("Message sending failed", exc_info=e) + sent["mobile_message"] = str(e) else: sent["mobile_message"] = "Mobile Message sent successfully!" diff --git a/drf_user/views.py b/drf_user/views.py index 3e93908..ec20336 100644 --- a/drf_user/views.py +++ b/drf_user/views.py @@ -309,8 +309,13 @@ def post(self, request, *args, **kwargs): sent_otp_resp: dict = send_otp(otp_obj=otp_obj, recip_email=email, recip_mobile=mobile) if not sent_otp_resp["success"]: + # delete otp object if OTP could not be sent + otp_obj.delete() raise serializers.ValidationError( - detail=_(f"OTP could not be sent! {sent_otp_resp['message']}") + { + "message": "OTP could not be sent! Please try again after some time.", + "exc": sent_otp_resp["message"], + } ) otp_obj.send_counter = F("send_counter") + 1 diff --git a/tests/fixtures/send_otp_responses.py b/tests/fixtures/send_otp_responses.py new file mode 100644 index 0000000..d806b5a --- /dev/null +++ b/tests/fixtures/send_otp_responses.py @@ -0,0 +1,7 @@ +"""Store mock responses here""" + +email_mobile_message_sent = { + "success": True, + "message": "Email Message sent successfully!", + "mobile_message": "Mobile Message sent successfully!", +} diff --git a/tests/test_views.py b/tests/test_views.py index f911406..d6c9480 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -1,5 +1,6 @@ """Tests for drf_user/views.py module""" from datetime import timedelta +from unittest.mock import patch, MagicMock, ANY import pytest from django.test import override_settings @@ -10,7 +11,8 @@ from rest_framework_simplejwt.tokens import RefreshToken from drf_user.models import AuthTransaction -from drf_user.models import User +from drf_user.models import User, OTPValidation +from tests.fixtures.send_otp_responses import email_mobile_message_sent from tests.settings import BASE_DIR @@ -393,7 +395,257 @@ def test_is_login_in_response(self): self.assertEqual(202, response.status_code) +class TestOTPLoginViewNew(APITestCase): + """OTP Login View""" + + @classmethod + def setUpTestData(cls): + """This method is called once for this test class""" + super().setUpTestData() + + # create user + cls.user = baker.make( + "drf_user.User", + username="my_user", + email="my_user@django.com", + mobile=9988998899, + ) + # create otp of registered user + cls.user_otp = baker.make( + "drf_user.OTPValidation", destination="my_user@django.com", otp=437474 + ) + + cls.url = reverse("OTP-Register-LogIn") + + def setUp(self) -> None: + """This method is called before each test""" + + super().setUp() + OTPValidation.objects.all().delete() + + @pytest.mark.django_db + def test_api_raises_400_when_only_name_is_passed(self): + """ + Given: Only name is passed + When: OTPLoginView is called + Then: API raises 400 + """ + # given/when + response = self.client.post(self.url, data={"name": "test"}, format="json") + + # then + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json()["email"], ["This field is required."]) + self.assertEqual(response.json()["mobile"], ["This field is required."]) + + @pytest.mark.django_db + def test_api_raises_400_when_name_email_is_passed(self): + """ + Given: Name and email is passed + When: OTPLoginView is called + Then: API raises 400 + """ + # given/when + response = self.client.post( + self.url, data={"name": "test", "email": "test@random.com"}, format="json" + ) + + # then + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json()["mobile"], ["This field is required."]) + + @pytest.mark.django_db + def test_api_raises_400_when_name_mobile_is_passed(self): + """ + Given: Name and mobile is passed + When: OTPLoginView is called + Then: API raises 400 + """ + # given/when + response = self.client.post( + self.url, data={"name": "test", "mobile": 8899777745}, format="json" + ) + + # then + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json()["email"], ["This field is required."]) + + @pytest.mark.django_db + def test_api_raises_400_when_email_mobile_is_passed(self): + """ + Given: Email and mobile is passed + When: OTPLoginView is called + Then: API raises 400 + """ + # given/when + response = self.client.post( + self.url, + data={"email": "test@example.com", "mobile": 1234838884}, + format="json", + ) + + # then + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json()["name"], ["This field is required."]) + + @pytest.mark.django_db + def test_api_raises_400_is_provided_mobile_number_is_invalid(self): + """ + Given: Name, email and invalid mobile is passed + When: OTPLoginView is called + Then: API raises 400 + """ + response = self.client.post( + self.url, + data={ + "name": "random_name", + "email": "random@django.com", + "mobile": 1234567890, + }, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json()["mobile"], ["Enter a valid mobile number."]) + + @pytest.mark.django_db + def test_api_raises_400_is_provided_email_is_invalid(self): + """ + Given: Name, invalid email and mobile is passed + When: OTPLoginView is called + Then: API raises 400 + """ + response = self.client.post( + self.url, + data={ + "name": "random_name", + "email": "random@django", + "mobile": 8899966662, + }, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json()["email"], ["Enter a valid email address."]) + + @pytest.mark.django_db + def test_api_raises_404_if_otp_for_provided_email_does_not_exists(self): + """ + Given: Name, email and mobile, invalid otp is passed + When: OTPLoginView is called + Then: API raises 404 + """ + data = { + "name": "random_name", + "email": "random@django.com", + "mobile": 7334567890, + "verify_otp": 999999, + } + response = self.client.post(self.url, data=data, format="json") + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual( + response.json()["detail"], + f"No pending OTP validation request found for provided {data['email']}. " + "Kindly send an OTP first.", + ) + + @pytest.mark.django_db + @patch("drf_user.views.send_otp", return_value=email_mobile_message_sent) + def test_api_raises_403_if_incorrect_otp_passed(self, mock_send_otp: MagicMock): + """ + Given: Valid data to send otp and invalid otp is passed + When: OTPLoginView is called + Then: API raises 403 + """ + data = { + "name": "random_name", + "email": "random@django.com", + "mobile": 7334567890, + } + # send otp + response = self.client.post(self.url, data=data, format="json") + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + # verify otp and register + data["verify_otp"] = 999999 + response = self.client.post(self.url, data=data, format="json") + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.json()["detail"], "OTP Validation failed! 2 attempts left!") + + mock_send_otp.assert_called_once_with( + otp_obj=ANY, recip_email=data["email"], recip_mobile=str(data["mobile"]) + ) + + @pytest.mark.django_db + @patch("drf_user.utils.send_mail") + def test_api_raises_400_when_send_otp_fails(self, mock_send_mail: MagicMock): + """ + Given: Valid data is passed + When: OTPLoginView is called + Then: API returns 400 + """ + # given/when + mock_send_mail.side_effect = Exception("Email Sending failed") + data = { + "name": "morning star", + "email": "lucifer@hell.com", + "mobile": 9988998811, + } + response = self.client.post( + self.url, + data=data, + format="json", + ) + + # then + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual( + response.json()["message"], "OTP could not be sent! Please try again after some time." + ) + self.assertEqual(response.json()["exc"], "Email Sending failed") + + # assert that OTP is not created + self.assertEqual(OTPValidation.objects.count(), 0) + mock_send_mail.assert_called_once() + + @pytest.mark.django_db + @patch("drf_user.views.send_otp", return_value=email_mobile_message_sent) + def test_api_returns_201_when_valid_data_is_passed(self, mock_send_otp: MagicMock): + """ + Given: Valid data is passed + When: OTPLoginView is called + Then: API returns 201 + """ + # given/when + data = { + "name": "morning star", + "email": "lucifer@hell.com", + "mobile": 9988998811, + } + response = self.client.post( + self.url, + data=data, + format="json", + ) + + # then + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(response.json()["email"], {"otp": "Email Message sent successfully!"}) + self.assertEqual( + response.json()["mobile_message"], {"otp": "Mobile Message sent successfully!"} + ) + + # check if otp is created + self.assertEqual(OTPValidation.objects.count(), 1) + mock_send_otp.assert_called_once_with( + otp_obj=ANY, recip_email=data["email"], recip_mobile=str(data["mobile"]) + ) + + class TestOTPLoginView(APITestCase): + # FIXME: Remove me """OTP Login View""" def setUp(self) -> None: @@ -432,12 +684,6 @@ def setUp(self) -> None: "mobile": 9234567890, "verify_otp": 888383, } - self.data_with_incorrect_otp = { - "name": "random_name", - "email": "random@django.com", - "mobile": 7334567890, - "verify_otp": 999999, - } self.data_registered_user = { "name": "my_user", "email": "my_user@django.com", @@ -463,84 +709,6 @@ def setUp(self) -> None: "verify_otp": 585858, } - @pytest.mark.django_db - def test_when_only_name_is_passed(self): - """Check when only name is passed as data then api raises 400""" - response = self.client.post(self.url, data={"name": "test"}, format="json") - - self.assertEqual(400, response.status_code) - self.assertEqual(["This field is required."], response.json()["email"]) - self.assertEqual(["This field is required."], response.json()["mobile"]) - - @pytest.mark.django_db - def test_when_name_email_is_passed(self): - """Check when name and email is passed as data, then API raises 400""" - - response = self.client.post( - self.url, data={"name": "test", "email": "test@random.com"}, format="json" - ) - - self.assertEqual(400, response.status_code) - self.assertEqual(["This field is required."], response.json()["mobile"]) - - @pytest.mark.django_db - def test_when_name_mobile_is_passed(self): - """Check when name and mobile is passed as data, then API raises 400""" - - response = self.client.post( - self.url, data={"name": "test", "mobile": 1234838884}, format="json" - ) - - self.assertEqual(400, response.status_code) - self.assertEqual(["This field is required."], response.json()["email"]) - - @pytest.mark.django_db - def test_when_email_mobile_is_passed(self): - """Check when email and mobile is passed as data, then API raises 400""" - - response = self.client.post( - self.url, - data={"email": "test@example.com", "mobile": 1234838884}, - format="json", - ) - - self.assertEqual(400, response.status_code) - self.assertEqual(["This field is required."], response.json()["name"]) - - @pytest.mark.django_db - def test_sent_otp_when_name_email_mobile_is_passed(self): - """ - Check when name, email, mobile is passed then OTP - is sent on user's email/mobile by API - """ - response = self.client.post(self.url, data=self.data, format="json") - - self.assertEqual(201, response.status_code) - self.assertEqual("OTP has been sent successfully.", response.json()["email"]["otp"]) - self.assertEqual("OTP has been sent successfully.", response.json()["mobile"]["otp"]) - - @pytest.mark.django_db - def test_login_with_incorrect_otp_for_registered_user(self): - """Check when data with correct otp is passed, token is generated or not""" - - response = self.client.post(self.url, data=self.data_with_incorrect_otp, format="json") - - self.assertEqual(403, response.status_code) - self.assertEqual("OTP Validation failed! 2 attempts left!", response.json()["detail"]) - - @pytest.mark.django_db - def test_login_with_incorrect_otp_for_new_user_without_validated_otp(self): - """Check when data without validated otp is passed, raises 404""" - - response = self.client.post(self.url, data=self.data_random_user, format="json") - - self.assertEqual(404, response.status_code) - self.assertEqual( - "No pending OTP validation request found for provided destination. " - "Kindly send an OTP first", - response.json()["detail"], - ) - @pytest.mark.django_db def test_login_with_correct_otp_for_new_user(self): """ From 922bf3f178f5b6e2095f5c6c42427b6eb749e711 Mon Sep 17 00:00:00 2001 From: "sourcery-ai[bot]" <58596630+sourcery-ai[bot]@users.noreply.github.com> Date: Sat, 14 Sep 2024 11:31:01 +0530 Subject: [PATCH 5/5] 'Refactored by Sourcery' (#209) Co-authored-by: Sourcery AI <> --- drf_user/__init__.py | 4 +--- drf_user/serializers.py | 18 +++++++----------- drf_user/utils.py | 5 ++--- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/drf_user/__init__.py b/drf_user/__init__.py index e64bf3c..b4ce697 100644 --- a/drf_user/__init__.py +++ b/drf_user/__init__.py @@ -44,9 +44,7 @@ def update_user_settings() -> dict: Author: Himanshu Shankar (https://himanshus.com) """ - custom_settings = getattr(settings, "USER_SETTINGS", None) - - if custom_settings: + if custom_settings := getattr(settings, "USER_SETTINGS", None): if not isinstance(custom_settings, dict): raise TypeError("USER_SETTING must be a dict.") diff --git a/drf_user/serializers.py b/drf_user/serializers.py index 0a1825f..10d8acb 100644 --- a/drf_user/serializers.py +++ b/drf_user/serializers.py @@ -190,19 +190,17 @@ def validate(self, attrs: dict) -> dict: else: attrs["prop"] = EMAIL - user = self.get_user(attrs.get("prop"), attrs.get("destination")) + if user := self.get_user(attrs.get("prop"), attrs.get("destination")): + attrs["email"] = user.email + attrs["user"] = user - if not user: + else: if attrs["is_login"]: raise NotFound(_("No user exists with provided details")) if "email" not in attrs.keys() and "verify_otp" not in attrs.keys(): raise serializers.ValidationError( _("Email field is compulsory while verifying a non-existing user's OTP.") ) - else: - attrs["email"] = user.email - attrs["user"] = user - return attrs @@ -328,13 +326,11 @@ def validate(self, attrs: dict) -> dict: """ validator = EmailValidator() validator(attrs.get("email")) - user = self.get_user(attrs.get("email")) - - if not user: + if user := self.get_user(attrs.get("email")): + return attrs + else: raise NotFound(_("User with the provided email does not exist.")) - return attrs - class ImageSerializer(serializers.ModelSerializer): """This serializer is for Image Upload API. diff --git a/drf_user/utils.py b/drf_user/utils.py index 1292221..c2b57cd 100644 --- a/drf_user/utils.py +++ b/drf_user/utils.py @@ -40,8 +40,7 @@ def get_client_ip(request: HttpRequest) -> Optional[str]: ------- ip: str or None """ - x_forwarded_for = request.META.get("HTTP_X_FORWARDED_FOR") - if x_forwarded_for: + if x_forwarded_for := request.META.get("HTTP_X_FORWARDED_FOR"): return x_forwarded_for.split(",")[0] else: return request.META.get("REMOTE_ADDR") @@ -251,7 +250,7 @@ def is_mobile_valid(mobile: str) -> bool: >>> print(is_mobile_valid('9999999999')) True """ - match = re.match(r"^[6-9]\d{9}$", str(mobile)) + match = re.match(r"^[6-9]\d{9}$", mobile) if match is None: raise ValidationError("Enter a valid mobile number.") return True