From b69dc59eb7175828e18b233dfbad932c60a1d03d Mon Sep 17 00:00:00 2001 From: apexdev Date: Fri, 28 Jun 2024 18:16:23 +0300 Subject: [PATCH] refactor: Prefer `secrets` for secure code generation (PEP506) Replace the usage of `random` to generate confidential data. [Reason] - `secrets` is properly seeded to generate cryptographic random data. [Chore] - Format module with tool `black`. Order imports with tool `isort`. - Update docstring for the `User` model. [Docs] See, `secrets` library: https://docs.python.org/3/library/secrets.html See, `PEP506`: https://peps.python.org/pep-0506/ --- accounts/models.py | 107 ++++++++++++++++++++++------------- accounts/tests/test_utils.py | 6 +- 2 files changed, 71 insertions(+), 42 deletions(-) diff --git a/accounts/models.py b/accounts/models.py index 4e24225..d6a0d33 100644 --- a/accounts/models.py +++ b/accounts/models.py @@ -1,72 +1,101 @@ +import secrets +import string +import time +import uuid + from django.conf import settings -from django.db import models from django.contrib.auth.models import AbstractUser +from django.db import models from phonenumber_field.modelfields import PhoneNumberField -import random -import time -import uuid -""" -Substituting a custom User by extending the AbstractUser -Making Users email unique -Adding extra attributes that are not present in the default User model(phone_number and verification code) -USERNAME_FIELD - changing login to use email rather than username. -REQUIRED_FIELDS - the required fields to create a superuser -""" - -# Generate six digit random code -def generate_verification_code(size=6): - return ''.join(str(random.randint(0,9)) for i in range(size)) - +_VERIFICATION_CODE_RANGE: range = range(4, 7) + + +def _generate_verification_code(size: int = 6) -> str: + """ + Securely generates a random code in valid range. + + Note: + This util is currently coupled to the `secrets` library. + Changes may result in failing tests. + """ + if size not in _VERIFICATION_CODE_RANGE: + errmsg = "Attempt to generate code outside of required range." + raise ValueError(errmsg) + sequence = string.digits + return "".join(secrets.choice(sequence) for _ in range(size)) + class User(AbstractUser): + """ + Substituting a custom `User` by extending the `AbstractUser`. + + Note: + Make `User` email unique. + Add extra attributes not present in the default `User` model: + - phone_number + - verification code + - code_generated_at + - is_verified + USERNAME_FIELD - change login to use email rather than username. + REQUIRED_FIELDS - fields required to create a superuser. + """ + id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) email = models.EmailField(unique=True, max_length=50) first_name = models.CharField(max_length=150) last_name = models.CharField(max_length=150) - phone_number = PhoneNumberField(blank=True, help_text='Contact phone number', null=True, unique= True) - verification_code = models.CharField(max_length=6, unique=True,default=generate_verification_code()) + phone_number = PhoneNumberField( + blank=True, help_text="Contact phone number", null=True, unique=True + ) + verification_code = models.CharField( + max_length=6, unique=True, default=_generate_verification_code() + ) code_generated_at = models.DateTimeField(auto_now_add=True) - is_verified = models.BooleanField(default=False) # to be set up later in views to change if user verified - - USERNAME_FIELD = 'email' - - # add phone number as a requirement while signing up - REQUIRED_FIELDS = ['first_name', 'last_name', 'username', 'phone_number'] + is_verified = models.BooleanField( + default=False + ) # to be set up later in views to change if user verified - def __str__(self) -> str: - return f'{self.first_name} {self.last_name}' + USERNAME_FIELD = "email" + + # add phone number as a requirement while signing up + REQUIRED_FIELDS = ["first_name", "last_name", "username", "phone_number"] def __init__(self, *args, region=None, **kwargs): - """ - The function takes in a region and then sets the region to the region that was passed in when the user is created - """ + """ + The function takes in a region and then sets the region to the region + that was passed in when the user is created + """ super().__init__(*args, **kwargs) self.region = region - - + # resets the verification code after every 1hr def get_verification_code(self): now = time.time() elapsed = now - self.code_generated_at.timestamp() - if elapsed > 3600: - self.verification_code = generate_verification_code() + if elapsed > 3600: + self.verification_code = _generate_verification_code() self.code_generated_at = now self.save return self.verification_code - + + def __str__(self) -> str: + return f"{self.first_name} {self.last_name}" + class Account(models.Model): id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) # Referencing the customized user - user = models.OneToOneField(settings.AUTH_USER_MODEL, on_delete=models.CASCADE, related_name='account') - account_name = models.CharField(max_length=50, unique= True) + user = models.OneToOneField( + settings.AUTH_USER_MODEL, on_delete=models.CASCADE, related_name="account" + ) + account_name = models.CharField(max_length=50, unique=True) created_at = models.DateTimeField(auto_now_add=True) updated_at = models.DateTimeField(auto_now=True) - display_picture = models.ImageField(default='blank-profile-picture.png', upload_to='profile_images') + display_picture = models.ImageField( + default="blank-profile-picture.png", upload_to="profile_images" + ) bio = models.TextField(blank=True, null=True) def __str__(self) -> str: return self.account_name - - diff --git a/accounts/tests/test_utils.py b/accounts/tests/test_utils.py index 9bf3589..4d06c82 100644 --- a/accounts/tests/test_utils.py +++ b/accounts/tests/test_utils.py @@ -12,7 +12,7 @@ import unittest from unittest.mock import call, patch -from accounts.models import generate_verification_code +from accounts.models import _generate_verification_code BASE_MODULE: str = "accounts.models" @@ -41,7 +41,7 @@ def test_should_raise_exc_for_out_of_range_code_size_arg(self) -> None: for length in invalid_lengths: # Then with self.assertRaises(ValueError): - generate_verification_code(size=length) # When + _generate_verification_code(size=length) # When def test_should_securely_generate_user_verification_code(self) -> None: # Given @@ -53,7 +53,7 @@ def test_should_securely_generate_user_verification_code(self) -> None: # When with patch(f"{BASE_MODULE}.secrets", autospec=True) as mock_secrets_lib: mock_secrets_lib.choice.side_effect = choice_side_effect - actual = generate_verification_code(size=length) + actual = _generate_verification_code(size=length) # Then mock_secrets_lib.choice.assert_has_calls([call(sequence)] * length)