generated from bisco/codex-bootstrap
fix(security): separate booking and check-in tokens
This commit is contained in:
@@ -113,16 +113,19 @@ def create_pending_reservation(
|
||||
expires_at=confirmation_expires_at,
|
||||
)
|
||||
|
||||
transaction.on_commit(
|
||||
lambda reservation=reservation, raw_confirmation_token=raw_confirmation_token: send_confirmation_email(
|
||||
reservation=reservation,
|
||||
raw_confirmation_token=raw_confirmation_token,
|
||||
)
|
||||
)
|
||||
|
||||
result = PendingReservationResult(
|
||||
reservation=reservation,
|
||||
confirmation_token=confirmation_token,
|
||||
raw_confirmation_token=raw_confirmation_token,
|
||||
available_seats=available_seats,
|
||||
)
|
||||
send_confirmation_email(
|
||||
reservation=result.reservation,
|
||||
raw_confirmation_token=result.raw_confirmation_token,
|
||||
)
|
||||
return result
|
||||
|
||||
|
||||
@@ -195,22 +198,22 @@ def confirm_reservation_from_token(raw_token):
|
||||
available_seats=available_seats - reservation.party_size,
|
||||
qr_code_image=generate_check_in_qr_base64(
|
||||
reservation=reservation,
|
||||
raw_check_in_token=raw_token,
|
||||
raw_check_in_token=raw_check_in_token,
|
||||
),
|
||||
qr_code_url=build_check_in_preview_url(raw_token),
|
||||
qr_code_url=build_check_in_preview_url(raw_check_in_token),
|
||||
)
|
||||
|
||||
|
||||
def retrieve_reservation_qr_from_token(raw_token):
|
||||
try:
|
||||
confirmation_token = ReservationToken.objects.select_related("reservation").get(
|
||||
token_hash=ReservationToken.hash_token(raw_token),
|
||||
purpose=ReservationToken.Purpose.CONFIRMATION,
|
||||
check_in_token = ReservationToken.objects.select_related("reservation").get_valid_token(
|
||||
raw_token,
|
||||
ReservationToken.Purpose.CHECK_IN,
|
||||
)
|
||||
except ReservationToken.DoesNotExist as exc:
|
||||
raise InvalidToken("Confirmation token is invalid.") from exc
|
||||
raise InvalidToken("Check-in token is invalid.") from exc
|
||||
|
||||
reservation = confirmation_token.reservation
|
||||
reservation = check_in_token.reservation
|
||||
if reservation.status != Reservation.Status.CONFIRMED:
|
||||
raise ReservationNotConfirmed("Reservation must be confirmed before QR retrieval.")
|
||||
|
||||
|
||||
@@ -53,18 +53,19 @@ class ReservationAdminTests(TestCase):
|
||||
self.assertContains(response, "The reservation stays pending")
|
||||
|
||||
def test_admin_can_create_manual_reservation_with_standard_email_flow(self):
|
||||
response = self.client.post(
|
||||
reverse("admin:bookings_reservation_add"),
|
||||
{
|
||||
"performance": self.performance.id,
|
||||
"name": "Maria Rossi",
|
||||
"email": "maria@example.com",
|
||||
"phone": "+390600000000",
|
||||
"party_size": 2,
|
||||
"notes": "Entered by staff at the venue desk.",
|
||||
"_save": "Save",
|
||||
},
|
||||
)
|
||||
with self.captureOnCommitCallbacks(execute=True):
|
||||
response = self.client.post(
|
||||
reverse("admin:bookings_reservation_add"),
|
||||
{
|
||||
"performance": self.performance.id,
|
||||
"name": "Maria Rossi",
|
||||
"email": "maria@example.com",
|
||||
"phone": "+390600000000",
|
||||
"party_size": 2,
|
||||
"notes": "Entered by staff at the venue desk.",
|
||||
"_save": "Save",
|
||||
},
|
||||
)
|
||||
|
||||
reservation = Reservation.objects.get()
|
||||
self.assertEqual(response.status_code, 302)
|
||||
|
||||
@@ -63,17 +63,18 @@ class BookingApiTests(APITestCase):
|
||||
|
||||
@override_settings(SITE_BASE_URL="https://tickets.azionelab.example")
|
||||
def test_reservation_creation_success(self):
|
||||
response = self.client.post(
|
||||
reverse("api-reservation-create", kwargs={"performance_id": self.performance.id}),
|
||||
{
|
||||
"name": "Maria Rossi",
|
||||
"email": "maria@example.com",
|
||||
"phone": "+390600000000",
|
||||
"party_size": 2,
|
||||
"notes": "Front row if possible.",
|
||||
},
|
||||
format="json",
|
||||
)
|
||||
with self.captureOnCommitCallbacks(execute=True):
|
||||
response = self.client.post(
|
||||
reverse("api-reservation-create", kwargs={"performance_id": self.performance.id}),
|
||||
{
|
||||
"name": "Maria Rossi",
|
||||
"email": "maria@example.com",
|
||||
"phone": "+390600000000",
|
||||
"party_size": 2,
|
||||
"notes": "Front row if possible.",
|
||||
},
|
||||
format="json",
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
|
||||
self.assertEqual(response.data["status"], Reservation.Status.PENDING)
|
||||
@@ -87,6 +88,22 @@ class BookingApiTests(APITestCase):
|
||||
mail.outbox[0].body,
|
||||
)
|
||||
|
||||
def test_reservation_creation_schedules_email_after_commit(self):
|
||||
with self.captureOnCommitCallbacks(execute=False) as callbacks:
|
||||
response = self.client.post(
|
||||
reverse("api-reservation-create", kwargs={"performance_id": self.performance.id}),
|
||||
{
|
||||
"name": "Maria Rossi",
|
||||
"email": "maria@example.com",
|
||||
"party_size": 2,
|
||||
},
|
||||
format="json",
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
|
||||
self.assertEqual(len(callbacks), 1)
|
||||
self.assertEqual(len(mail.outbox), 0)
|
||||
|
||||
def test_reservation_creation_with_insufficient_seats(self):
|
||||
response = self.client.post(
|
||||
reverse("api-reservation-create", kwargs={"performance_id": self.performance.id}),
|
||||
@@ -123,6 +140,7 @@ class BookingApiTests(APITestCase):
|
||||
"https://tickets.azionelab.example/api/check-ins/preview/?token="
|
||||
)
|
||||
)
|
||||
self.assertNotIn(raw_token, response.data["qr_code_url"])
|
||||
self.assertNotIn("token", response.data)
|
||||
self.assertTrue(response.data["qr_code_image"].startswith("data:image/png;base64,"))
|
||||
self.assertEqual(reservation.status, Reservation.Status.CONFIRMED)
|
||||
@@ -174,15 +192,16 @@ class BookingApiTests(APITestCase):
|
||||
def test_qr_retrieval_success_for_confirmed_reservation(self):
|
||||
reservation = self.create_reservation()
|
||||
_, raw_token = generate_confirmation_token(reservation)
|
||||
self.client.post(
|
||||
confirmation_response = self.client.post(
|
||||
reverse("api-reservation-confirm"),
|
||||
{"token": raw_token},
|
||||
format="json",
|
||||
)
|
||||
check_in_token = confirmation_response.data["qr_code_url"].split("token=", 1)[1]
|
||||
|
||||
response = self.client.get(
|
||||
reverse("api-reservation-qr"),
|
||||
{"token": raw_token},
|
||||
{"token": check_in_token},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
@@ -196,6 +215,23 @@ class BookingApiTests(APITestCase):
|
||||
self.assertNotIn("email", response.data)
|
||||
self.assertNotIn("name", response.data)
|
||||
|
||||
def test_qr_retrieval_rejects_confirmation_token(self):
|
||||
reservation = self.create_reservation()
|
||||
_, raw_token = generate_confirmation_token(reservation)
|
||||
self.client.post(
|
||||
reverse("api-reservation-confirm"),
|
||||
{"token": raw_token},
|
||||
format="json",
|
||||
)
|
||||
|
||||
response = self.client.get(
|
||||
reverse("api-reservation-qr"),
|
||||
{"token": raw_token},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
|
||||
self.assertEqual(response.data["status"], "invalid_token")
|
||||
|
||||
def test_qr_retrieval_fails_for_invalid_token(self):
|
||||
response = self.client.get(
|
||||
reverse("api-reservation-qr"),
|
||||
@@ -214,8 +250,8 @@ class BookingApiTests(APITestCase):
|
||||
{"token": raw_token},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_409_CONFLICT)
|
||||
self.assertEqual(response.data["status"], "reservation_not_confirmed")
|
||||
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
|
||||
self.assertEqual(response.data["status"], "invalid_token")
|
||||
self.assertEqual(reservation.status, Reservation.Status.PENDING)
|
||||
|
||||
def create_reservation(self, **overrides):
|
||||
|
||||
@@ -19,6 +19,7 @@ from bookings.services import (
|
||||
confirm_reservation_from_token,
|
||||
create_pending_reservation,
|
||||
generate_confirmation_token,
|
||||
retrieve_reservation_qr_from_token,
|
||||
)
|
||||
from shows.models import Performance, Show, Venue
|
||||
|
||||
@@ -64,14 +65,16 @@ class BookingServiceTests(TestCase):
|
||||
EMAIL_BACKEND="django.core.mail.backends.locmem.EmailBackend",
|
||||
SITE_BASE_URL="https://tickets.azionelab.example",
|
||||
)
|
||||
def test_create_pending_reservation_sends_confirmation_email(self):
|
||||
result = create_pending_reservation(
|
||||
performance_id=self.performance.id,
|
||||
name="Maria Rossi",
|
||||
email="maria@example.com",
|
||||
party_size=1,
|
||||
)
|
||||
def test_create_pending_reservation_sends_confirmation_email_after_commit(self):
|
||||
with self.captureOnCommitCallbacks(execute=True) as callbacks:
|
||||
result = create_pending_reservation(
|
||||
performance_id=self.performance.id,
|
||||
name="Maria Rossi",
|
||||
email="maria@example.com",
|
||||
party_size=1,
|
||||
)
|
||||
|
||||
self.assertEqual(len(callbacks), 1)
|
||||
self.assertEqual(len(mail.outbox), 1)
|
||||
self.assertEqual(mail.outbox[0].to, ["maria@example.com"])
|
||||
self.assertIn(result.raw_confirmation_token, mail.outbox[0].body)
|
||||
@@ -80,16 +83,30 @@ class BookingServiceTests(TestCase):
|
||||
mail.outbox[0].body,
|
||||
)
|
||||
|
||||
@patch("bookings.emailing.send_mail", side_effect=RuntimeError("SMTP down"))
|
||||
def test_create_pending_reservation_logs_email_failure_without_crashing(self, mocked_send_mail):
|
||||
with self.assertLogs("bookings.emailing", level="ERROR") as captured_logs:
|
||||
result = create_pending_reservation(
|
||||
@override_settings(EMAIL_BACKEND="django.core.mail.backends.locmem.EmailBackend")
|
||||
def test_create_pending_reservation_defers_email_until_commit(self):
|
||||
with self.captureOnCommitCallbacks(execute=False) as callbacks:
|
||||
create_pending_reservation(
|
||||
performance_id=self.performance.id,
|
||||
name="Maria Rossi",
|
||||
email="maria@example.com",
|
||||
party_size=1,
|
||||
)
|
||||
|
||||
self.assertEqual(len(callbacks), 1)
|
||||
self.assertEqual(len(mail.outbox), 0)
|
||||
|
||||
@patch("bookings.emailing.send_mail", side_effect=RuntimeError("SMTP down"))
|
||||
def test_create_pending_reservation_logs_email_failure_without_crashing(self, mocked_send_mail):
|
||||
with self.assertLogs("bookings.emailing", level="ERROR") as captured_logs:
|
||||
with self.captureOnCommitCallbacks(execute=True):
|
||||
result = create_pending_reservation(
|
||||
performance_id=self.performance.id,
|
||||
name="Maria Rossi",
|
||||
email="maria@example.com",
|
||||
party_size=1,
|
||||
)
|
||||
|
||||
self.assertEqual(result.reservation.status, Reservation.Status.PENDING)
|
||||
self.assertEqual(Reservation.objects.count(), 1)
|
||||
mocked_send_mail.assert_called_once()
|
||||
@@ -130,7 +147,7 @@ class BookingServiceTests(TestCase):
|
||||
self.assertEqual(result.available_seats, 1)
|
||||
self.assertEqual(
|
||||
result.qr_code_url,
|
||||
build_check_in_preview_url(raw_token),
|
||||
build_check_in_preview_url(result.raw_check_in_token),
|
||||
)
|
||||
self.assertTrue(
|
||||
result.qr_code_url.startswith(
|
||||
@@ -139,6 +156,19 @@ class BookingServiceTests(TestCase):
|
||||
)
|
||||
self.assertTrue(result.qr_code_image.startswith("data:image/png;base64,"))
|
||||
|
||||
@override_settings(SITE_BASE_URL="https://tickets.azionelab.example")
|
||||
def test_confirmation_token_cannot_be_reused_as_qr_or_check_in_token(self):
|
||||
reservation = self.create_reservation()
|
||||
_, raw_token = generate_confirmation_token(reservation)
|
||||
|
||||
result = confirm_reservation_from_token(raw_token)
|
||||
|
||||
self.assertNotEqual(raw_token, result.raw_check_in_token)
|
||||
self.assertNotEqual(
|
||||
build_check_in_preview_url(raw_token),
|
||||
result.qr_code_url,
|
||||
)
|
||||
|
||||
@override_settings(SITE_BASE_URL="https://tickets.azionelab.example")
|
||||
def test_qr_code_is_generated_for_confirmed_reservation(self):
|
||||
reservation = self.create_reservation(
|
||||
@@ -171,6 +201,28 @@ class BookingServiceTests(TestCase):
|
||||
raw_check_in_token="opaque-check-in-token",
|
||||
)
|
||||
|
||||
def test_qr_retrieval_rejects_confirmation_token(self):
|
||||
reservation = self.create_reservation()
|
||||
_, raw_confirmation_token = generate_confirmation_token(reservation)
|
||||
confirm_reservation_from_token(raw_confirmation_token)
|
||||
|
||||
with self.assertRaises(InvalidToken):
|
||||
retrieve_reservation_qr_from_token(raw_confirmation_token)
|
||||
|
||||
def test_qr_retrieval_accepts_check_in_token(self):
|
||||
reservation = self.create_reservation()
|
||||
_, raw_confirmation_token = generate_confirmation_token(reservation)
|
||||
result = confirm_reservation_from_token(raw_confirmation_token)
|
||||
|
||||
qr_result = retrieve_reservation_qr_from_token(result.raw_check_in_token)
|
||||
|
||||
self.assertEqual(qr_result.reservation, reservation)
|
||||
self.assertEqual(
|
||||
qr_result.qr_code_url,
|
||||
build_check_in_preview_url(result.raw_check_in_token),
|
||||
)
|
||||
self.assertTrue(qr_result.qr_code_image.startswith("data:image/png;base64,"))
|
||||
|
||||
def test_confirmation_fails_when_capacity_is_exhausted(self):
|
||||
Reservation.objects.create(
|
||||
performance=self.performance,
|
||||
|
||||
Reference in New Issue
Block a user