Don't save users until otp validation is successful

[smrf1093]

trafficstars

The users should not be stored in the database unless we know that the otp validation has been successful, I do not completely check the sources however, as I explored some sections in the serializers I feel that this problem exists in the source code

[sumit4613]

Hi, @smrf1093 thanks for opening the issue.

Please add more details to this issue, like how I can reproduce or which endpoint you're talking about. If possible please add code snippets as well.

This will help me to resolve this issue quickly. Meanwhile, I'll also try to find where this problem is.

[smrf1093]

So here is the thing

class RegisterView(CreateAPIView):
    """
    Register View
    Register a new user to the system.
    The data required are username, email, name, password and mobile (optional).
    """

    renderer_classes = (JSONRenderer,)
    permission_classes = (AllowAny,)
    serializer_class = UserSerializer

    def perform_create(self, serializer):
        """Override perform_create to create user"""
        data = {
            "username": serializer.validated_data["username"],
            "email": serializer.validated_data["email"],
            "name": serializer.validated_data["name"],
            "password": serializer.validated_data["password"],
        }
        try:
            data["mobile"] = serializer.validated_data["mobile"]
        except KeyError:
            if not settings.USER_SETTINGS["MOBILE_OPTIONAL"]:
                raise ValidationError({"error": "Mobile is required."})
        return User.objects.create_user(**data)

you save the user on the register section, you should not do that, this should be saved in a bridge model, and then when the user is verified, the data from the bridge model can be used to create a user on verification, so this prevents setting up a bot on the register section and other security concerns in this section

[sumit4613]

Hey @smrf1093

We're checking for validation here. Please check this serializer UserSerializer.

def validate_mobile(self, value: str) -> str:
    """
    If pre-validated mobile number is required, this function
    checks if the mobile is pre-validated using OTP.
    Parameters
    ----------
    value: str

    Returns
    -------
    value: str

    """
    if not user_settings["MOBILE_VALIDATION"]:
        return value

    if check_validation(value=value):
        return value
    else:
        raise serializers.ValidationError(
            "The mobile must be " "pre-validated via OTP."
        )

[smrf1093]

No, I mean verification, for example, user with [email protected] registers then its information is stored immediately in the database however we do not know the form sender is x or not so before storing the user data in the main table it is good to first understand that the sender is x

[iamhssingh]

No, I mean verification, for example, user with [email protected] registers then its information is stored immediately in the database however we do not know the form sender is x or not so before storing the user data in the main table it is good to first understand that the sender is x

@smrf1093 I understand what you are saying here. While that is a possible approach, we are not taking that approach here.

Can you add more insight, on how this approach will improve the overall application?

Also, I am removing the bug tag, I don't see how this qualifies as a bug. It definitely can be a good enhancement.

[smrf1093]

Yeah your are right, it is not a big but asap I will provide the enhancement to the approach

[iamhssingh]

Yeah your are right, it is not a big but asap I will provide the enhancement to the approach

Thanks a lot 🙌 Really appreciate it.

[sumit4613]

Closing for now.