diff --git a/contentcuration/contentcuration/frontend/accounts/pages/Create.vue b/contentcuration/contentcuration/frontend/accounts/pages/Create.vue index 91c4431ea2..65afaa16a2 100644 --- a/contentcuration/contentcuration/frontend/accounts/pages/Create.vue +++ b/contentcuration/contentcuration/frontend/accounts/pages/Create.vue @@ -213,9 +213,10 @@ @@ -260,6 +261,7 @@ return { valid: true, registrationFailed: false, + submitting: false, form: { first_name: '', last_name: '', @@ -482,6 +484,12 @@ // We need to check the "acceptedAgreement" here explicitly because it is not a // Vuetify form field and does not trigger the form validation. if (this.$refs.form.validate() && this.acceptedAgreement) { + // Prevent double submission + if (this.submitting) { + return Promise.resolve(); + } + + this.submitting = true; const cleanedData = this.clean(this.form); return this.register(cleanedData) .then(() => { @@ -517,6 +525,9 @@ this.registrationFailed = true; this.valid = false; } + }) + .finally(() => { + this.submitting = false; }); } else if (this.$refs.top.scrollIntoView) { this.$refs.top.scrollIntoView({ behavior: 'smooth' }); diff --git a/contentcuration/contentcuration/frontend/accounts/pages/__tests__/create.spec.js b/contentcuration/contentcuration/frontend/accounts/pages/__tests__/create.spec.js index a2c2f40d71..11067e892e 100644 --- a/contentcuration/contentcuration/frontend/accounts/pages/__tests__/create.spec.js +++ b/contentcuration/contentcuration/frontend/accounts/pages/__tests__/create.spec.js @@ -179,4 +179,19 @@ describe('create', () => { expect(wrapper.vm.registrationFailed).toBe(true); }); }); + describe('double-submit prevention', () => { + it('should prevent multiple API calls on rapid clicks', async () => { + const [wrapper, mocks] = await makeWrapper(); + + // Click submit multiple times + const p1 = wrapper.vm.submit(); + const p2 = wrapper.vm.submit(); + const p3 = wrapper.vm.submit(); + + await Promise.all([p1, p2, p3]); + + // Only 1 API call should be made + expect(mocks.register).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/contentcuration/contentcuration/tests/views/test_users.py b/contentcuration/contentcuration/tests/views/test_users.py index a17da93f8a..4c5f635204 100644 --- a/contentcuration/contentcuration/tests/views/test_users.py +++ b/contentcuration/contentcuration/tests/views/test_users.py @@ -1,11 +1,13 @@ import json +from django.db import IntegrityError from django.http.response import HttpResponseBadRequest from django.http.response import HttpResponseForbidden from django.http.response import HttpResponseNotAllowed from django.http.response import HttpResponseRedirectBase from django.urls import reverse_lazy from mock import mock +from mock import patch from contentcuration.models import User from contentcuration.tests import testdata @@ -127,8 +129,8 @@ def setUp(self): first_name="Tester", last_name="Tester", email="tester@tester.com", - pasword1="tester123", - pasword2="tester123", + password1="tester123", + password2="tester123", uses="IDK", source="IDK", policies=json.dumps(dict(policy_etc=True)), @@ -148,8 +150,8 @@ def test_post__inactive_registration(self): self.assertIsInstance(response, HttpResponseNotAllowed) def test_post__password_too_short(self): - self.request_data["pasword1"] = "123" - self.request_data["pasword2"] = "123" + self.request_data["password1"] = "123" + self.request_data["password2"] = "123" response = self.post(self.view, self.request_data) self.assertIsInstance(response, HttpResponseBadRequest) self.assertIn("password1", response.content.decode()) @@ -160,6 +162,22 @@ def test_post__after_delete(self): response = self.post(self.view, self.request_data) self.assertIsInstance(response, HttpResponseForbidden) + @patch("contentcuration.views.users.UserRegistrationView.register") + def test_post__handles_integrity_error_gracefully(self, mock_register): + """Test that IntegrityError during registration returns 403 instead of 500""" + # Simulate IntegrityError (race condition on duplicate email) + mock_register.side_effect = IntegrityError( + 'duplicate key value violates unique constraint "contentcuration_user_email_key"' + ) + + response = self.post(self.view, self.request_data) + + # Should return 403 Forbidden, not 500 + self.assertIsInstance(response, HttpResponseForbidden) + # Error response should include "email" field + error_data = json.loads(response.content.decode()) + self.assertIn("email", error_data) + class UserActivationViewTestCase(StudioAPITestCase): def setUp(self): diff --git a/contentcuration/contentcuration/views/users.py b/contentcuration/contentcuration/views/users.py index 66a6652d0b..34a986895b 100644 --- a/contentcuration/contentcuration/views/users.py +++ b/contentcuration/contentcuration/views/users.py @@ -11,6 +11,7 @@ from django.contrib.sites.shortcuts import get_current_site from django.core.exceptions import PermissionDenied from django.core.mail import send_mail +from django.db import IntegrityError from django.http import HttpResponse from django.http import HttpResponseBadRequest from django.http import HttpResponseForbidden @@ -181,8 +182,19 @@ def get_form_kwargs(self): return kwargs def form_valid(self, form): - self.register(form) - return HttpResponse() + try: + self.register(form) + return HttpResponse() + except IntegrityError as e: + # Handle race condition where duplicate user is created between + # form validation and save (e.g., double submit) + logger.warning( + "IntegrityError during user registration, likely due to race condition: %s", + str(e), + extra={"email": form.cleaned_data.get("email")}, + ) + # Return same error as duplicate active account for consistency + return HttpResponseForbidden(json.dumps(["email"])) def form_invalid(self, form): # frontend handles the error messages