diff --git a/api-collection/Auth/ResendConfirmationMail/Valid.bru b/api-collection/Auth/ResendConfirmationMail/Valid.bru new file mode 100644 index 0000000..b330560 --- /dev/null +++ b/api-collection/Auth/ResendConfirmationMail/Valid.bru @@ -0,0 +1,17 @@ +meta { + name: Valid + type: http + seq: 1 +} + +post { + url: {{baseUrl}}/accounts/resend-account-confirmation + body: json + auth: none +} + +body:json { + { + "email": "mail@mail.com" + } +} diff --git a/src/accounts/api/serializers.py b/src/accounts/api/serializers.py index 8b1dccf..52a7a33 100644 --- a/src/accounts/api/serializers.py +++ b/src/accounts/api/serializers.py @@ -1,11 +1,8 @@ -import secrets - from django.conf import settings -from django.contrib.auth import authenticate from django.contrib.auth.password_validation import validate_password from rest_framework import serializers -from ..models import Account, AccountConfirmation, PasswordResetRequestModel +from ..models import Account, AccountConfirmation class PublicAccountDataSerializer(serializers.ModelSerializer): @@ -37,16 +34,6 @@ class LoginWithCredentialsSerializer(serializers.Serializer): email = serializers.EmailField() password = serializers.CharField(style={"input_type": "password"}) - def validate(self, data): - account: Account | None = authenticate(username=data["email"], password=data["password"]) # type: ignore[assignment] - if account is None: - raise serializers.ValidationError("Unable to login with provided credentials.") - if not account.is_confirmed: - raise serializers.ValidationError( - "This account hasn't done the mail confirmation step or has been disabled." - ) - return account - class UpdateAccountSerializer(serializers.ModelSerializer): class Meta: @@ -85,11 +72,8 @@ def validate(self, data): return account -class ResetPasswordSerializer(serializers.ModelSerializer): - class Meta: - model = Account - fields = ("password",) - extra_kwargs = {"password": {"write_only": True}} +class ResetPasswordSerializer(serializers.Serializer): + password = serializers.CharField(style={"input_type": "password"}) def validate_password(self, value): # Validate the password using Django's built-in validators @@ -97,28 +81,6 @@ def validate_password(self, value): return value -class ResetPasswordRequestSerializer(serializers.ModelSerializer): - class Meta: - model = PasswordResetRequestModel - fields = ("email",) - - email = serializers.EmailField() - - def validate(self, data): - email = data["email"] - account = Account.objects.get(email=email) - if account is None: - raise serializers.ValidationError("Account with this email doesn't exist.") - - return { - "token": secrets.token_urlsafe(32), - "account": account, - } - - def create(self, validated_data): - return PasswordResetRequestModel.objects.create(**validated_data) - - class ConfirmAccountSerializer(serializers.Serializer): token = serializers.CharField() @@ -130,16 +92,8 @@ def validate(self, data): if not account_confirmation.is_token_valid(): raise serializers.ValidationError("Token is invalid or expired.") - return account_confirmation + return {"token": data["token"]} -class ResendAccountSerializer(serializers.Serializer): +class EmailSerializer(serializers.Serializer): email = serializers.EmailField() - - def validate(self, data): - email = data["email"] - account = Account.objects.get(email=email) - if account is None: - raise Account.DoesNotExist("Account with this email doesn't exist.") - - return account diff --git a/src/accounts/api/views.py b/src/accounts/api/views.py index 4c11217..751bda4 100644 --- a/src/accounts/api/views.py +++ b/src/accounts/api/views.py @@ -1,4 +1,5 @@ import logging +import secrets from urllib.parse import urljoin from uuid import uuid4 @@ -6,6 +7,7 @@ from commons.error_response import ErrorResponse from commons.mail_wrapper import send_email_with_template from django.conf import settings +from django.contrib.auth import authenticate from django.core.exceptions import ObjectDoesNotExist, PermissionDenied from drf_spectacular.utils import extend_schema from knox.models import AuthToken @@ -17,16 +19,13 @@ from rest_framework.serializers import ValidationError from rest_framework.views import APIView -from ..exceptions import MissingMailConfirmationError -from ..models import Account +from ..models import Account, AccountConfirmation, PasswordResetRequestModel from .serializers import ( ConfirmAccountSerializer, + EmailSerializer, LoginWithCredentialsSerializer, - PasswordResetRequestModel, PublicAccountDataSerializer, RegisterAccountSerializer, - ResendAccountSerializer, - ResetPasswordRequestSerializer, ResetPasswordSerializer, UpdateAccountSerializer, VerifyAccountSerializer, @@ -54,11 +53,8 @@ def post(self, request, format=None): def get_post_response_data(self, request, token, instance): user: Account = request.user - try: - if not user.is_confirmed: - raise MissingMailConfirmationError() - except MissingMailConfirmationError as e: - return ErrorResponse(str(e), MissingMailConfirmationError.status_code) + if not user.is_confirmed: + return ErrorResponse("You must confirm your email before attempting to login.", status.HTTP_400_BAD_REQUEST) serializer = self.get_user_serializer_class() @@ -84,19 +80,22 @@ class LoginWithCredentialsView(GenericAPIView): def post(self, request): serializer = self.serializer_class(data=request.data) - try: - serializer.is_valid(raise_exception=True) - account = Account.objects.get(email=serializer.data["email"]) - if not account.is_active: - raise MissingMailConfirmationError() - except ObjectDoesNotExist: - return ErrorResponse("Account does not exist.", status.HTTP_404_NOT_FOUND) - except MissingMailConfirmationError as e: - return ErrorResponse(str(e), status.HTTP_400_BAD_REQUEST) - except ValidationError as e: - return ErrorResponse(str(e), status.HTTP_400_BAD_REQUEST) + if not serializer.is_valid(): + return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST) - account = serializer.validated_data + email = serializer.validated_data["email"] + password = serializer.validated_data["password"] + + account: Account | None = authenticate(email=email, password=password) # type: ignore[assignment] + + if account is None: + return ErrorResponse("Unable to login with provided credentials.", status.HTTP_400_BAD_REQUEST) + + if not account.is_confirmed: + return ErrorResponse("You must confirm your email before attempting to login.", status.HTTP_400_BAD_REQUEST) + + if not account.is_active: + return ErrorResponse("Account is suspended.", status.HTTP_400_BAD_REQUEST) return Response( { @@ -119,17 +118,14 @@ class RegisterAccountView(GenericAPIView): def post(self, request): serializer = self.get_serializer(data=request.data) - try: - serializer.is_valid(raise_exception=True) - except ValidationError as e: - return Response(data={"error": str(e)}, status=e.status_code) - account = serializer.save() + if not serializer.is_valid(): + return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST) + account = serializer.save() return Response( { - "account": RegisterAccountSerializer(account, context=self.get_serializer_context()).data, - "token": AuthToken.objects.create(account)[1], + "account": PublicAccountDataSerializer(account, context=self.get_serializer_context()).data, }, status=status.HTTP_200_OK, ) @@ -232,20 +228,21 @@ class ResetPasswordView(GenericAPIView): def post(self, request, reset_token): serializer = self.serializer_class(data=request.data) + if not serializer.is_valid(): + return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST) + try: - serializer.is_valid(raise_exception=True) - reset_request = PasswordResetRequestModel.objects.get(token=reset_token) - if not reset_request.is_token_valid(): + reset_token = PasswordResetRequestModel.objects.get(token=reset_token) + if not reset_token.is_token_valid(): raise PasswordResetRequestModel.DoesNotExist - except ValidationError as e: - return ErrorResponse(str(e), e.status_code) except PasswordResetRequestModel.DoesNotExist: return ErrorResponse("Invalid link or expired.", status.HTTP_400_BAD_REQUEST) - account = reset_request.account + account = reset_token.account account.set_password(serializer.validated_data["password"]) account.save() - reset_request.delete() + reset_token.delete() + return Response(status=status.HTTP_200_OK) @@ -257,24 +254,29 @@ class RequestPasswordResetView(GenericAPIView): """ permission_classes = (AllowAny,) - serializer_class = ResetPasswordRequestSerializer + serializer_class = EmailSerializer def post(self, request): serializer = self.serializer_class(data=request.data) + if not serializer.is_valid(): + return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST) + try: - serializer.is_valid(raise_exception=True) - except ValidationError: + account = Account.objects.get(email=serializer.validated_data["email"]) + except Account.DoesNotExist: logger.warning( "Attempted to reset password for non-existing account: %s", serializer.validated_data["email"] ) return Response(status=status.HTTP_200_OK) - serializer.save() - link = urljoin(settings.PASS_RESET_URL, serializer.validated_data["token"]) + token = secrets.token_urlsafe(32) + PasswordResetRequestModel.objects.create(account=account, token=token) + + link = urljoin(settings.PASS_RESET_URL, token) send_email_with_template( - recipient=serializer.validated_data["account"].email, + recipient=account.email, subject="Reset your password", template="password_reset.html", context={"link": link}, @@ -302,17 +304,12 @@ def post(self, request, confirm_token): except ValidationError as e: return ErrorResponse(str(e), e.status_code) - account_id = serializer.validated_data.account.unique_identifier + account_confirmation = AccountConfirmation.objects.get(token=serializer.validated_data["token"]) + account = account_confirmation.account - try: - account = Account.objects.get(unique_identifier=account_id) - except Account.DoesNotExist: - return ErrorResponse("Account for this confirmation link does not exist.", status.HTTP_400_BAD_REQUEST) - - account.is_active = True account.is_confirmed = True + account_confirmation.delete() account.save() - serializer.validated_data.delete() return Response(status=status.HTTP_200_OK) @@ -325,23 +322,24 @@ class ResendAccountConfirmationView(GenericAPIView): """ permission_classes = (AllowAny,) - serializer_class = ResendAccountSerializer + serializer_class = EmailSerializer - def post(self, request): - serializer = self.serializer_class(data=request.data) + def post(self, request, *args, **kwargs): + serializer: EmailSerializer = self.serializer_class(data=request.data) - try: - serializer.is_valid(raise_exception=True) - except ValidationError as e: - return ErrorResponse(str(e), e.status_code) - except Account.DoesNotExist: - logger.warning( - "Attempted to resend account confirmation for non-existing account: %s", - serializer.validated_data["email"], - ) - return Response(status=status.HTTP_200_OK) + if serializer.is_valid(): + email = serializer.validated_data["email"] + try: + account = Account.objects.get(email=email) + except Account.DoesNotExist: + logger.warning("Attempted to resend confirmation mail for non-existing account: %s", email) + return Response(status=status.HTTP_200_OK) - account = serializer.validated_data - account.send_confirmation_mail() + if account.is_confirmed: + logger.warning("Attempted to resend confirmation mail for already confirmed account: %s", email) + return Response(status=status.HTTP_200_OK) - return Response(status=status.HTTP_200_OK) + account.send_confirmation_mail() + return Response(status=status.HTTP_200_OK) + else: + return ErrorResponse(serializer.errors, status.HTTP_400_BAD_REQUEST) diff --git a/src/accounts/exceptions.py b/src/accounts/exceptions.py deleted file mode 100644 index 056d818..0000000 --- a/src/accounts/exceptions.py +++ /dev/null @@ -1,11 +0,0 @@ -from rest_framework import status - - -class MissingMailConfirmationError(Exception): - """Account is trying to login without confirming their email first""" - - detail = "You must confirm your email before attempting to login." - status_code = status.HTTP_418_IM_A_TEAPOT - - def __str__(self): - return self.detail diff --git a/src/accounts/models.py b/src/accounts/models.py index 148de6e..93c7de9 100644 --- a/src/accounts/models.py +++ b/src/accounts/models.py @@ -115,7 +115,7 @@ def __str__(self): def is_token_valid(self): if self.created_at is None: return False - return (self.created_at + timedelta(minutes=settings.ACCOUNT_CONFIRMATION_TOKEN_TTL)) > timezone.now() + return (self.created_at + timedelta(hours=settings.ACCOUNT_CONFIRMATION_TOKEN_TTL)) > timezone.now() class PasswordResetRequestModel(models.Model): diff --git a/src/commons/error_response.py b/src/commons/error_response.py index 268763e..8008208 100644 --- a/src/commons/error_response.py +++ b/src/commons/error_response.py @@ -23,6 +23,6 @@ def custom_exception_handler(exc, context): else: logger.error("An unhandled error occurred: %s", exc) logger.error("Context: %s", context) - return ErrorResponse(f"An unhandled error occurred: {exc}") + return ErrorResponse("Something went wrong on our end. Please try again later.") return response diff --git a/src/tests/test_unhandled_exceptions.py b/src/tests/test_unhandled_exceptions.py index a29f9b3..fa6431c 100644 --- a/src/tests/test_unhandled_exceptions.py +++ b/src/tests/test_unhandled_exceptions.py @@ -20,5 +20,5 @@ def test_custom_exception_handler(self): response = ExceptionView.as_view()(request) self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) - self.assertEqual(response.data["error"], "An unhandled error occurred: This is a test exception") self.assertIn("error", response.data) + self.assertEqual(response.data["error"], "Something went wrong on our end. Please try again later.")