From 15ccc877ae5b337c33b34655f912a956245bc21b Mon Sep 17 00:00:00 2001 From: Kumi Date: Sat, 16 Nov 2024 20:58:23 +0100 Subject: [PATCH] Enhances registration workflow and error handling Introduces a new registration status 'Started' with adjustments to default status handling. Adds a missing check in email verification to prevent unauthorized users from accessing the registration process. Updates forbidden response status codes to improve security and debugging clarity. Corrects unintended user modification warning logic and ensures registration session keys are cleared post-creation to prevent potential data leaks. Improves code style consistency and readability across forms. --- .../registration/forms.py | 8 ++--- .../registration/models.py | 11 ++++--- .../registration/views.py | 29 +++++++++++++++---- 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/synapse_registration/registration/forms.py b/src/synapse_registration/registration/forms.py index 4b2166f..2122438 100644 --- a/src/synapse_registration/registration/forms.py +++ b/src/synapse_registration/registration/forms.py @@ -15,7 +15,7 @@ class UsernameForm(forms.Form): username = cleaned_data.get("username") if username.startswith("@") and username.endswith(f":{settings.MATRIX_DOMAIN}"): - username = username[1:-len(f":{settings.MATRIX_DOMAIN}")] + username = username[1 : -len(f":{settings.MATRIX_DOMAIN}")] if not username: self.add_error("username", "Username cannot be empty.") @@ -45,13 +45,13 @@ class RegistrationForm(forms.Form): label="Password", widget=forms.PasswordInput( attrs={"class": "input", "placeholder": "Enter password"} - ) + ), ) password2 = forms.CharField( label="Confirm password", widget=forms.PasswordInput( attrs={"class": "input", "placeholder": "Re-enter password"} - ) + ), ) registration_reason = forms.CharField( min_length=30, @@ -60,7 +60,7 @@ class RegistrationForm(forms.Form): "class": "textarea", "placeholder": "Why do you want to join our server? If you were referred by a current member, who referred you? If you found us through a different means, how did you find us?", } - ) + ), ) def clean(self): diff --git a/src/synapse_registration/registration/models.py b/src/synapse_registration/registration/models.py index 5dd1e84..e0b545e 100644 --- a/src/synapse_registration/registration/models.py +++ b/src/synapse_registration/registration/models.py @@ -1,23 +1,26 @@ from django.db import models + class UserRegistration(models.Model): # Status constants + STATUS_STARTED = 0 STATUS_REQUESTED = 1 STATUS_APPROVED = 2 STATUS_DENIED = 3 # Status choices STATUS_CHOICES = [ - (STATUS_REQUESTED, 'Requested'), - (STATUS_APPROVED, 'Approved'), - (STATUS_DENIED, 'Denied'), + (STATUS_STARTED, "Started"), + (STATUS_REQUESTED, "Requested"), + (STATUS_APPROVED, "Approved"), + (STATUS_DENIED, "Denied"), ] username = models.CharField(max_length=150) email = models.EmailField() registration_reason = models.TextField() ip_address = models.GenericIPAddressField() - status = models.IntegerField(choices=STATUS_CHOICES, default=STATUS_REQUESTED) + status = models.IntegerField(choices=STATUS_CHOICES, default=STATUS_STARTED) token = models.CharField(max_length=64, unique=True) email_verified = models.BooleanField(default=False) diff --git a/src/synapse_registration/registration/views.py b/src/synapse_registration/registration/views.py index 0aa755f..23473d5 100644 --- a/src/synapse_registration/registration/views.py +++ b/src/synapse_registration/registration/views.py @@ -79,9 +79,11 @@ class EmailInputView(FormView): class VerifyEmailView(View): def get(self, request, token): registration = get_object_or_404(UserRegistration, token=token) + + if registration.status != UserRegistration.STATUS_STARTED: + return render(request, "registration/registration_forbidden.html", status=403) + request.session["registration"] = registration.id - if registration.email_verified: - return render(request, "registration/already_verified.html") registration.email_verified = True registration.save() return redirect("complete_registration") @@ -107,7 +109,7 @@ class CompleteRegistrationView(FormView): ) if not response.json().get("available"): - return render(self.request, "registration/registration_forbidden.html") + return render(self.request, "registration/registration_forbidden.html", status=403) response = requests.put( f"{settings.SYNAPSE_SERVER}/_synapse/admin/v2/users/@{username}:{settings.MATRIX_DOMAIN}", @@ -120,7 +122,18 @@ class CompleteRegistrationView(FormView): headers={"Authorization": f"Bearer {settings.SYNAPSE_ADMIN_TOKEN}"}, ) - if response.status_code in (200, 201): + if response.status_code == 200: + # Oops. This should never happen. It means that an existing user was altered. + send_mail( + "Critical Registration Error", + f"Something went horribly wrong. The existing user {username} was altered. Please investigate.", + settings.DEFAULT_FROM_EMAIL, + [settings.ADMIN_EMAIL], + ) + + return render(self.request, "registration/registration_forbidden.html", status=403) + + if response.status_code == 201: # The "locked" field doesn't seem to work when creating a user, so we need to lock the user after creation response = requests.put( f"{settings.SYNAPSE_SERVER}/_synapse/admin/v2/users/@{username}:{settings.MATRIX_DOMAIN}", @@ -145,6 +158,12 @@ class CompleteRegistrationView(FormView): registration.registration_reason = registration_reason registration.save() + try: + self.request.session.pop("registration") + self.request.session.pop("username") + except KeyError: + pass + send_mail( "New Registration Request", f"Approve the new user {username}", @@ -165,5 +184,5 @@ class CompleteRegistrationView(FormView): self.registration.status != UserRegistration.STATUS_REQUESTED or not self.registration.email_verified ): - return render(request, "registration/registration_forbidden.html") + return render(request, "registration/registration_forbidden.html", status=403) return super().dispatch(request, *args, **kwargs)