From 93ab072f550de915a2be0466b8c91948109e3c44 Mon Sep 17 00:00:00 2001 From: Samir Boulahtit Date: Sat, 23 May 2026 22:28:23 +0200 Subject: [PATCH] fix(loyalty): enforce cooldown on earn-points (was silently skipped) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit stamp_service.add_stamp checks card.last_stamp_at + cooldown_minutes before crediting and raises StampCooldownException if too soon. The parallel points_service.earn_points writes card.last_points_at but never reads it for enforcement — so cooldown_minutes was silently ignored for points-based programs. Mirror the stamps check in points_service.earn_points: after acquiring the row lock, compare now vs last_points_at + cooldown_minutes and raise the new PointsCooldownException if the cashier is inside the window. Add PointsCooldownException alongside StampCooldownException in exceptions.py with parity wording / error code POINTS_COOLDOWN. Surfaced during Test 3 step 3.6 — repeated earn-points calls for the same card kept crediting the customer with no rate limit even though the program's cooldown_minutes was set. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/modules/loyalty/exceptions.py | 12 ++++++++++++ app/modules/loyalty/services/points_service.py | 18 ++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/app/modules/loyalty/exceptions.py b/app/modules/loyalty/exceptions.py index a40a35cd..38e098b0 100644 --- a/app/modules/loyalty/exceptions.py +++ b/app/modules/loyalty/exceptions.py @@ -154,6 +154,17 @@ class StampCooldownException(LoyaltyException): ) +class PointsCooldownException(LoyaltyException): + """Raised when trying to earn points before cooldown period ends.""" + + def __init__(self, cooldown_ends: str, cooldown_minutes: int): + super().__init__( + message=f"Please wait {cooldown_minutes} minutes between point-earning transactions", + error_code="POINTS_COOLDOWN", + details={"cooldown_ends": cooldown_ends, "cooldown_minutes": cooldown_minutes}, + ) + + class DailyStampLimitException(LoyaltyException): """Raised when daily stamp limit is exceeded.""" @@ -401,6 +412,7 @@ __all__ = [ "StaffPinRequiredException", "InvalidStaffPinException", "StaffPinLockedException", + "PointsCooldownException", "StampCooldownException", "DailyStampLimitException", # Redemption diff --git a/app/modules/loyalty/services/points_service.py b/app/modules/loyalty/services/points_service.py index 26a5a8c0..094e3169 100644 --- a/app/modules/loyalty/services/points_service.py +++ b/app/modules/loyalty/services/points_service.py @@ -15,7 +15,7 @@ Handles points operations including: """ import logging -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from sqlalchemy.orm import Session @@ -26,6 +26,7 @@ from app.modules.loyalty.exceptions import ( LoyaltyException, LoyaltyProgramInactiveException, OrderReferenceRequiredException, + PointsCooldownException, StaffPinRequiredException, ) from app.modules.loyalty.models import LoyaltyTransaction, TransactionType @@ -187,8 +188,21 @@ class PointsService: # Re-fetch with row lock to prevent concurrent modification card = card_service.get_card_for_update(db, card.id) - # Add points + # Check cooldown AFTER acquiring lock to prevent TOCTOU race. + # Mirrors stamp_service.add_stamp — without this, a cashier (or a + # malicious actor with terminal access) can earn points for the + # same customer over and over with no rate limit, even though the + # program's cooldown_minutes is set. now = datetime.now(UTC) + if card.last_points_at: + cooldown_ends = card.last_points_at + timedelta(minutes=program.cooldown_minutes) + if now < cooldown_ends: + raise PointsCooldownException( + cooldown_ends.isoformat(), + program.cooldown_minutes, + ) + + # Add points card.points_balance += points_earned card.total_points_earned += points_earned card.last_points_at = now