Drop CIText & update Django (#237)

* Update to Django 4.0

`django-clacks` was removed temporarily until we figure out why poetry
won't lock the dependencies with clacks included.

* Drop CI text fields in favour of charfields

A unique constraint is created on the database in order to prevent
duplicate signups using a name that only differs in casing for both
users as well as teams. Two test cases are added to verify this
behaviour.

* Re-add django-clacks

* Truncate data in migration

Co-authored-by: Daniel Milnes <thebeanogamer@gmail.com>

* Reject usernames with a length > 36

Co-authored-by: Daniel Milnes <thebeanogamer@gmail.com>
Co-authored-by: David Cooke <me@dave.lc>

pyproject.toml

@@ -6,31 +6,31 @@ authors = ["RACTF Admins <admins@ractf.co.uk>"]
 
 [tool.poetry.dependencies]
 python = "^3.9"
-Django = "^3.1"
-django-cors-headers = "3.2.1"
-django-redis = "4.11.0"
-django-redis-cache = "2.1.1"
-django-storages = "1.9.1"
-djangorestframework = "3.11.1"
+Django = "^4.0"
+django-cors-headers = "^3.11"
+django-redis = "^5.2"
+django-redis-cache = "^3.0"
+django-storages = "~1.12"
+djangorestframework = "^3.13"
 pyotp = "2.3.0"
 gunicorn = "^20.0.4"
 boto3 = "^1.14.33"
 psycopg2-binary = "^2.8.5"
 django-filter = "^2.3.0"
 newrelic = "^5.22.1"
-django-prometheus = "^2.1.0"
-django-cachalot = "^2.3.5"
-django-silk = "^4.1.0"
+django-prometheus = "^2.2"
+django-clacks = "^0.3"
+django-cachalot = "^2.5"
+django-silk = "^4.2"
 serpy = "^0.3.1"
-django-zxcvbn-password-validator = "^1.3.2"
+django-zxcvbn-password-validator = "^1.4"
 uvicorn = {extras = ["standard"], version = "^0.13.4"}
 sentry-sdk = "^1.0.0"
 coverage = {extras = ["toml"], version = "^5.5"}
 Twisted = "22.1.0"
 channels-redis = "^3.2.0"
-django-clacks = "^0.2.0"
 requests = "^2.26.0"
-django-anymail = {extras = ["amazon_ses", "mailgun", "sendgrid", "console", "mailjet", "mandrill", "postal", "postmark", "sendinblue", "sparkpost"], version = "^8.4"}
+django-anymail = {extras = ["amazon_ses", "mailgun", "sendgrid", "console", "mailjet", "mandrill", "postal", "postmark", "sendinblue", "sparkpost"], version = "^8.5"}
 
 [tool.poetry.dev-dependencies]
 ipython = "^7.31.1"
@@ -55,10 +55,6 @@ docopt = "^0.6.2"
 
 [tool.pytest.ini_options]
 python_files = "tests.py test_*.py *_tests.py"
-filterwarnings = """
-ignore::django.utils.deprecation.RemovedInDjango40Warning
-ignore::django.utils.deprecation.RemovedInDjango41Warning
-"""
 
 [tool.coverage.run]
 source = ["src"]

src/authentication/basic_auth.py

@@ -17,6 +17,8 @@ def validate(self, data):
 
         self.validate_email(data["email"])
         self.check_email_or_username_in_use(email=data["email"], username=data["username"])
+        if len(data["username"]) > 36:
+            raise ValidationError("username_too_long")
 
         return {key: data[key] for key in self.required_fields}
 

src/authentication/tests.py

@@ -235,6 +235,15 @@ def test_register_with_mail_failing_domain(self):
             config.set("email_domain", None)
             self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST)
 
+    def test_register_long_username(self):
+        data = {
+            "username": "a" * 37,
+            "password": "uO7*$E@0ngqL",
+            "email": "user11@example.org",
+        }
+        response = self.client.post(reverse("register"), data)
+        self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST)
+
 
 class EmailResendTestCase(APITestCase):
     def test_email_resend(self):

src/backend/signals.py

@@ -1,25 +1,25 @@
 from django.dispatch import Signal
 
-flag_score = Signal(providing_args=["user", "team", "challenge", "flag", "solve"])
-flag_reject = Signal(providing_args=["user", "team", "challenge", "flag", "reason"])
-flag_submit = Signal(providing_args=["user", "team", "challenge", "flag"])
-team_join = Signal(providing_args=["user", "team"])
-team_join_attempt = Signal(providing_args=["user", "team_name"])
-team_join_reject = Signal(providing_args=["user", "team_name"])
-team_create = Signal(providing_args=["team"])
-use_hint = Signal(providing_args=["user", "team", "hint"])
-logout = Signal(providing_args=["user"])
-add_2fa = Signal(providing_args=["user"])
-verify_2fa = Signal(providing_args=["user"])
-remove_2fa = Signal(providing_args=["user"])
-password_reset = Signal(providing_args=["user"])
-password_reset_start = Signal(providing_args=["user"])
-password_reset_start_reject = Signal(providing_args=["email"])
-email_verified = Signal(providing_args=["user"])
-change_password = Signal(providing_args=["user"])
-login = Signal(providing_args=["user"])
-login_reject = Signal(providing_args=["username", "reason"])
-register = Signal(providing_args=["user"])
-register_reject = Signal(providing_args=["username", "email"])
-websocket_connect = Signal(providing_args=["channel_layer"])
-websocket_disconnect = Signal(providing_args=["channel_layer"])
+flag_score = Signal()
+flag_reject = Signal()
+flag_submit = Signal()
+team_join = Signal()
+team_join_attempt = Signal()
+team_join_reject = Signal()
+team_create = Signal()
+use_hint = Signal()
+logout = Signal()
+add_2fa = Signal()
+verify_2fa = Signal()
+remove_2fa = Signal()
+password_reset = Signal()
+password_reset_start = Signal()
+password_reset_start_reject = Signal()
+email_verified = Signal()
+change_password = Signal()
+login = Signal()
+login_reject = Signal()
+register = Signal()
+register_reject = Signal()
+websocket_connect = Signal()
+websocket_disconnect = Signal()

src/member/migrations/0009_alter_member_options_alter_member_username_and_more.py

@@ -0,0 +1,43 @@
+# Generated by Django 4.0.2 on 2022-02-11 21:43
+
+import backend.validators
+from django.db import migrations, models
+import django.db.models.functions.text
+
+
+def truncate_usernames(apps, schema_editor):
+    Member = apps.get_model('member', 'member')
+    db_alias = schema_editor.connection.alias
+    for member in Member.objects.using(db_alias).all():
+        if len(member.username) > 36:
+            member.username = member.username[:36]
+            member.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('member', '0008_remove_member_password_reset_token'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='member',
+            options={},
+        ),
+        migrations.RunPython(
+            truncate_usernames,
+            # Marked as elidable since when we squash we will have the 36 characters
+            # in by default
+            elidable=True,
+        ),
+        migrations.AlterField(
+            model_name='member',
+            name='username',
+            field=models.CharField(error_messages={'unique': 'A user with that username already exists.'}, help_text='Required. 36 characters or fewer. Letters, digits and @/./+/-/_ only.', max_length=36, unique=True, validators=[backend.validators.printable_name], verbose_name='username'),
+        ),
+        migrations.AddConstraint(
+            model_name='member',
+            constraint=models.UniqueConstraint(django.db.models.functions.text.Lower('username'), name='member_member_username_uniq_idx'),
+        ),
+    ]

src/member/models.py

@@ -4,9 +4,9 @@
 
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import AbstractUser
-from django.contrib.postgres.fields import CICharField
 from django.db import models
 from django.db.models import SET_NULL
+from django.db.models.functions import Lower
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
 from django_prometheus.models import ExportModelOperationsMixin
@@ -24,7 +24,7 @@ class TOTPStatus(IntEnum):
 class Member(ExportModelOperationsMixin("member"), AbstractUser):
     username_validator = printable_name
 
-    username = CICharField(
+    username = models.CharField(
         _("username"),
         max_length=36,
         unique=True,
@@ -49,6 +49,14 @@ class Member(ExportModelOperationsMixin("member"), AbstractUser):
     leaderboard_points = models.IntegerField(default=0)
     last_score = models.DateTimeField(default=timezone.now)
 
+    class Meta:
+        constraints = [
+            models.UniqueConstraint(
+                Lower('username'),
+                name='member_member_username_uniq_idx',
+            ),
+        ]
+
     def __str__(self):
         return self.username
 

src/member/tests.py

@@ -1,5 +1,6 @@
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import AnonymousUser
+from django.db.utils import IntegrityError
 from django.http import HttpRequest
 from rest_framework.request import Request
 from rest_framework.reverse import reverse
@@ -148,6 +149,12 @@ def test_patch_member(self):
         )
         self.assertEqual(response.status_code, HTTP_403_FORBIDDEN)
 
+    def test_disallows_differently_cased_but_same_username(self):
+        """A differently-cased but otherwise same username should not be allowed registration."""
+
+        self.user.username = self.admin_user.username.upper()
+        self.assertRaises(IntegrityError, self.user.save)
+
     def test_patch_member_admin(self):
         self.client.force_authenticate(self.admin_user)
         response = self.client.patch(

src/team/migrations/0004_alter_team_name_team_team_team_username_uniq_idx.py

@@ -0,0 +1,39 @@
+# Generated by Django 4.0.2 on 2022-02-11 21:43
+
+import backend.validators
+from django.db import migrations, models
+import django.db.models.functions.text
+
+
+def truncate_usernames(apps, schema_editor):
+    Team = apps.get_model('team', 'team')
+    db_alias = schema_editor.connection.alias
+    for team in Team.objects.using(db_alias).all():
+        if len(team.name) > 36:
+            team.name = team.name[:36]
+            team.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('team', '0003_team_size_limit_exempt'),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            truncate_usernames,
+            # Marked as elidable since when we squash we will have the 36 characters
+            # in by default
+            elidable=True,
+        ),
+        migrations.AlterField(
+            model_name='team',
+            name='name',
+            field=models.CharField(max_length=36, unique=True, validators=[backend.validators.printable_name]),
+        ),
+        migrations.AddConstraint(
+            model_name='team',
+            constraint=models.UniqueConstraint(django.db.models.functions.text.Lower('name'), name='team_team_username_uniq_idx'),
+        ),
+    ]

src/team/models.py

@@ -1,6 +1,6 @@
-from django.contrib.postgres.fields import CICharField
 from django.db import models
 from django.db.models import CASCADE, Prefetch
+from django.db.models.functions import Lower
 from django.utils import timezone
 from django_prometheus.models import ExportModelOperationsMixin
 
@@ -32,7 +32,7 @@ def prefetch_solves(self) -> "models.QuerySet[Team]":
 class Team(ExportModelOperationsMixin("team"), models.Model):
     """Represents a team of one or more Members."""
 
-    name = CICharField(max_length=36, unique=True, validators=[printable_name])
+    name = models.CharField(max_length=36, unique=True, validators=[printable_name])
     is_visible = models.BooleanField(default=True)
     password = models.CharField(max_length=64)
     owner = models.ForeignKey(Member, on_delete=CASCADE, related_name="owned_team")
@@ -43,3 +43,11 @@ class Team(ExportModelOperationsMixin("team"), models.Model):
     size_limit_exempt = models.BooleanField(default=False)
 
     objects = TeamQuerySet.as_manager()
+
+    class Meta:
+        constraints = [
+            models.UniqueConstraint(
+                Lower('name'),
+                name='team_team_username_uniq_idx',
+            ),
+        ]

src/team/serializers.py

@@ -1,5 +1,8 @@
+from django.db.utils import IntegrityError
 from rest_framework import serializers
+from rest_framework.status import HTTP_400_BAD_REQUEST
 
+from backend.exceptions import FormattedException
 from backend.mixins import IncorrectSolvesMixin
 from backend.signals import team_create
 from challenge.serializers import SolveSerializer
@@ -100,8 +103,17 @@ class Meta:
     def create(self, validated_data):
         name = validated_data["name"]
         password = validated_data["password"]
-        team = Team.objects.create(name=name, password=password, owner=self.context["request"].user)
-        self.context["request"].user.team = team
-        self.context["request"].user.save()
-        team_create.send(sender=self.__class__, team=team)
-        return team
+        try:
+            team = Team.objects.create(
+                name=name,
+                password=password,
+                owner=self.context["request"].user,
+            )
+            self.context["request"].user.team = team
+            self.context["request"].user.save()
+            team_create.send(sender=self.__class__, team=team)
+            return team
+        except IntegrityError:
+            # Team creation with a name that differs only in casing
+            # causes the database to raise here.
+            raise FormattedException(m="team_name_in_use", status=HTTP_400_BAD_REQUEST)

src/team/tests.py

@@ -1,4 +1,5 @@
 from django.contrib.auth import get_user_model
+from django.db.utils import IntegrityError
 from django.urls import reverse
 from rest_framework.status import (
     HTTP_200_OK,
@@ -154,7 +155,15 @@ def test_create_team_not_authed(self):
 
     def test_create_duplicate_team(self):
         self.client.force_authenticate(self.admin_user)
-        response = self.client.post(reverse("team-create"), data={"name": "team-test", "password": "test"})
+        response = self.client.post(reverse("team-create"), data={"name": self.team.name, "password": "test"})
+        self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST)
+
+    def test_disallows_create_team_with_differently_cased_but_same_name(self):
+        """A differently-cased but otherwise same name should not be allowed creation."""
+
+        self.client.force_authenticate(self.admin_user)
+        payload = {"name": self.team.name.upper(), "password": "test"}
+        response = self.client.post(reverse("team-create"), data=payload)
         self.assertEqual(response.status_code, HTTP_400_BAD_REQUEST)