From 3b39049c201ccebf840963a58732fac4ef3882b4 Mon Sep 17 00:00:00 2001 From: imperosol Date: Mon, 18 Nov 2024 14:01:45 +0100 Subject: [PATCH 1/2] Make `User.generate_username` less stupid --- core/models.py | 21 +++++++++++++++------ core/tests/test_user.py | 21 +++++++++++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/core/models.py b/core/models.py index adbe56c4..20cbeb4b 100644 --- a/core/models.py +++ b/core/models.py @@ -26,6 +26,7 @@ from __future__ import annotations import importlib import logging import os +import string import unicodedata from datetime import timedelta from pathlib import Path @@ -688,12 +689,20 @@ class User(AbstractBaseUser): .encode("ascii", "ignore") .decode("utf-8") ) - un_set = [u.username for u in User.objects.all()] - if user_name in un_set: - i = 1 - while user_name + str(i) in un_set: - i += 1 - user_name += str(i) + # load all usernames which could conflict with the new one. + # we need to actually load them, instead of performing a count, + # because we cannot be sure that two usernames refer to the + # actual same word (eg. tmore and tmoreau) + possible_conflicts: list[str] = list( + User.objects.filter(username__startswith=user_name).values_list( + "username", flat=True + ) + ) + nb_conflicts = sum( + 1 for name in possible_conflicts if name.rstrip(string.digits) == user_name + ) + if nb_conflicts > 0: + user_name += str(nb_conflicts) # exemple => exemple1 self.username = user_name return user_name diff --git a/core/tests/test_user.py b/core/tests/test_user.py index 0fdf43c9..a87827d5 100644 --- a/core/tests/test_user.py +++ b/core/tests/test_user.py @@ -166,3 +166,24 @@ def test_user_invoice_with_multiple_items(): .values_list("total", flat=True) ) assert res == [15, 13, 5] + + +@pytest.mark.django_db +@pytest.mark.parametrize( + ("first_name", "last_name", "expected"), + [ + ("Auguste", "Bartholdi", "abartholdi2"), # ville du lion rpz + ("Aristide", "Denfert-Rochereau", "adenfertrochereau"), + ("John", "Dôe", "jdoe"), # with an accent + ], +) +def test_generate_username(first_name: str, last_name: str, expected: str): + baker.make( + User, + username=iter(["abar", "abartholdi", "abartholdi1", "abar1"]), + _quantity=4, + _bulk_create=True, + ) + new_user = User(first_name=first_name, last_name=last_name, email="a@example.com") + new_user.generate_username() + assert new_user.username == expected From 6853ec0b691b14dbf977ab117b80a10e796105f1 Mon Sep 17 00:00:00 2001 From: imperosol Date: Tue, 19 Nov 2024 13:21:08 +0100 Subject: [PATCH 2/2] make random password generation safe --- subscription/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/subscription/views.py b/subscription/views.py index 4abc8e83..9a39e7b0 100644 --- a/subscription/views.py +++ b/subscription/views.py @@ -13,7 +13,7 @@ # # -import random +import secrets from django import forms from django.conf import settings @@ -85,7 +85,7 @@ class SubscriptionForm(forms.ModelForm): date_of_birth=self.cleaned_data.get("date_of_birth"), ) u.generate_username() - u.set_password(str(random.randrange(1000000, 10000000))) + u.set_password(secrets.token_urlsafe(nbytes=10)) u.save() cleaned_data["member"] = u elif cleaned_data.get("member") is not None: