Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,10 @@
<KButton
primary
class="mt-5"
:disabled="offline"
:disabled="offline || submitting"
:text="$tr('finishButton')"
type="submit"
data-test="submit-button"
/>
</VForm>
</VLayout>
Expand Down Expand Up @@ -260,6 +261,7 @@
return {
valid: true,
registrationFailed: false,
submitting: false,
form: {
first_name: '',
last_name: '',
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
26 changes: 22 additions & 4 deletions contentcuration/contentcuration/tests/views/test_users.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -127,8 +129,8 @@ def setUp(self):
first_name="Tester",
last_name="Tester",
email="[email protected]",
pasword1="tester123",
pasword2="tester123",
password1="tester123",
password2="tester123",
uses="IDK",
source="IDK",
policies=json.dumps(dict(policy_etc=True)),
Expand All @@ -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())
Expand All @@ -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):
Expand Down
16 changes: 14 additions & 2 deletions contentcuration/contentcuration/views/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down