From 4d3901780a36148fd199f609b58ebccff88a82a4 Mon Sep 17 00:00:00 2001 From: Ebin Bellini Date: Wed, 5 Aug 2026 22:26:57 +0200 Subject: [PATCH] Prohibit duplicate signup When checking for duplicate signups, we looked at SSN, which can have varying formats, making it possible to bypass the dedup check. Now, we look at the unicore id, which is guaranteed to be unique and has a consistent format. - Store unicore_id on signup - Dedup on unicore_id instead of ssn - Add tests for signup --- backend/backend/serializers.py | 22 +++++- backend/backend/tests.py | 86 ++++++++++++++++++++++++ backend/backend/utils/test_unicore.py | 2 +- frontend/apply/src/app/utils/imageUrl.ts | 16 ++--- 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/backend/backend/serializers.py b/backend/backend/serializers.py index 8eabbe8..74a781e 100644 --- a/backend/backend/serializers.py +++ b/backend/backend/serializers.py @@ -1,5 +1,6 @@ from django.contrib.auth.password_validation import validate_password from django.core.exceptions import ValidationError +from django.db import IntegrityError from django.utils.translation import gettext_lazy as _ from rest_framework import serializers from rest_framework.serializers import ModelSerializer @@ -80,6 +81,8 @@ class MemberSerializer(ModelSerializer): required=False, allow_null=True, ) + # Set from Unicore on creation + unicore_id = serializers.IntegerField(read_only=True) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -104,6 +107,7 @@ class Meta: "is_active", "is_staff", "verified_email", + "unicore_id", ) extra_kwargs = { "password": {"write_only": True}, @@ -155,6 +159,16 @@ def validate(self, attrs): } ) + # Normalize to Unicore's canonical SSN and record its member id. + attrs["ssn"] = user_data["ssn"].strip() + attrs["unicore_id"] = user_data["unicore_id"] + + # Check if this Unicore member already has an account + if Member.objects.filter(unicore_id=attrs["unicore_id"]).exists(): + raise serializers.ValidationError( + {"ssn": ["An account with this SSN already exists."]} + ) + # Check if email is already registered email = attrs.get("email") if email: @@ -172,7 +186,13 @@ def create(self, validated_data): raise serializers.ValidationError({"password": ["Password must be set"]}) user = Member(**validated_data) user.set_password(password) - user.save() + + try: + user.save() + except IntegrityError: + raise serializers.ValidationError( + {"ssn": ["An account with this SSN already exists."]} + ) send_verification_email(user) diff --git a/backend/backend/tests.py b/backend/backend/tests.py index 99da6c0..643b9b7 100644 --- a/backend/backend/tests.py +++ b/backend/backend/tests.py @@ -1,3 +1,5 @@ +from unittest.mock import patch + from django.contrib.auth import get_user_model from django.core import mail from django.urls import reverse @@ -53,3 +55,87 @@ def test_password_reset_email_not_sent_for_unverified_user(self): self.assertEqual(response.status_code, 200) self.assertEqual(len(mail.outbox), 0) + + +class SignupTests(TestCase): + """Tests for the signup endpoint and duplicate-account prevention.""" + + def setUp(self): + self.client = APIClient() + self.signup_url = reverse("signup") + + def _fake_user_data(self, ssn="200001011234", unicore_id=42): + return { + "ssn": ssn, + "firstname": "Kalle", + "lastname": "Sprätt", + "email": "kalle.spratt@kb.se", + "phone_number": "0700000000", + "unicore_id": unicore_id, + } + + def _signup(self, ssn, email): + return self.client.post( + self.signup_url, + { + "ssn": ssn, + "email": email, + "password": "KB@Bappelsin1337", + "name": "Kalle Sprätt", + "phone_number": "0700000000", + }, + format="json", + ) + + @patch("backend.serializers.unicoremember") + def test_signup_stores_canonical_ssn_and_unicore_id(self, mock_unicore): + mock_unicore.return_value.get_user_data.return_value = self._fake_user_data() + + response = self._signup("20000101-1234", "kalle.spratt@kb.se") + + self.assertEqual(response.status_code, 201) + member = get_user_model().objects.get(email="kalle.spratt@kb.se") + self.assertEqual(member.unicore_id, 42) + # Stored SSN is normalized to Unicore's canonical form + self.assertEqual(member.ssn, "200001011234") + + @patch("backend.serializers.unicoremember") + def test_signup_rejects_duplicate_unicore_id_with_different_ssn_format( + self, mock_unicore + ): + # The dash variant is a *different raw string*, so the field-level + # unique check passes; only the unicore_id dedup can catch it. + mock_unicore.return_value.get_user_data.side_effect = [ + self._fake_user_data(), + self._fake_user_data(), + ] + + first = self._signup("200001011234", "kalle.spratt@kb.se") + self.assertEqual(first.status_code, 201) + + second = self._signup("20000101-1234", "other@example.com") + self.assertEqual(second.status_code, 400) + self.assertIn("ssn", second.data) + + @patch("backend.serializers.unicoremember") + def test_signup_rejects_duplicate_unicore_id_same_ssn(self, mock_unicore): + mock_unicore.return_value.get_user_data.side_effect = [ + self._fake_user_data(), + self._fake_user_data(), + ] + + first = self._signup("200001011234", "kalle.spratt@kb.se") + self.assertEqual(first.status_code, 201) + + second = self._signup("200001011234", "other@example.com") + self.assertEqual(second.status_code, 400) + self.assertIn("ssn", second.data) + + @patch("backend.serializers.unicoremember") + def test_signup_rejects_unregistered_ssn(self, mock_unicore): + mock_unicore.return_value.get_user_data.return_value = None + + response = self._signup("200001011234", "kalle.spratt@kb.se") + + self.assertEqual(response.status_code, 400) + self.assertIn("SSN", str(response.data)) diff --git a/backend/backend/utils/test_unicore.py b/backend/backend/utils/test_unicore.py index 779ef15..87b5ea3 100644 --- a/backend/backend/utils/test_unicore.py +++ b/backend/backend/utils/test_unicore.py @@ -17,7 +17,7 @@ def test_get_user_data_with_medlemsnr(self, mock_get): "Fornamn": "John", "Efternamn": "Doe", "Epost": "john.doe@example.com", - "Telefon": "555-1234", + "Tele1": "555-1234", "Id": "123", } mock_get.return_value = fake_response diff --git a/frontend/apply/src/app/utils/imageUrl.ts b/frontend/apply/src/app/utils/imageUrl.ts index 29b136f..46b0674 100644 --- a/frontend/apply/src/app/utils/imageUrl.ts +++ b/frontend/apply/src/app/utils/imageUrl.ts @@ -1,14 +1,14 @@ /** * The backend returns relative paths like "/media/team_logos/dg.png". * - * The URL is used as the `src` of Next.js ``. The browser never - * loads it directly — it only sees `/_next/image?url=…`. So the URL - * must be reachable from the Next.js **server**. + * The URL is used as the src of Next.js . The browser never + * loads it directly. It only sees /_next/image?url= so the URL + * must be reachable from the Next.js server. * - * Docker dev → http://backend:8000 (always correct inside Docker) - * Production → window.location.origin (nginx proxies /media/ to Django) + * Docker dev -> http://backend:8000 (always correct inside Docker) + * Production -> window.location.origin (nginx proxies /media/ to Django) * - * Set NEXT_PUBLIC_API_URL at **build time** to an absolute URL to override + * Set NEXT_PUBLIC_API_URL at build time to an absolute URL to override * the Docker default (e.g. for local dev: http://localhost:8000/api). */ export function getImageUrl(url: string): string { @@ -29,12 +29,12 @@ export function getImageUrl(url: string): string { // Client-side if (typeof window !== "undefined") { - // Docker dev – don't use localhost:3000, the Next.js server needs + // Docker dev: don't use localhost:3000, the Next.js server needs // to reach the backend container at backend:8000 if (window.location.hostname === "localhost") { return `http://backend:8000${url}`; } - // Production – nginx proxies /media/ to Django on the same domain + // Production: nginx proxies /media/ to Django on the same domain return `${window.location.origin}${url}`; }