Add double-spend safeguard

This commit is contained in:
Raphael Michel
2019-10-17 16:03:57 +02:00
parent ac2df35db6
commit b3e6f44027
8 changed files with 104 additions and 14 deletions

View File

@@ -1,6 +1,8 @@
from django.db import transaction
from rest_framework import filters, viewsets
from rest_framework import filters, serializers, status, viewsets
from rest_framework.decorators import action
from rest_framework.exceptions import MethodNotAllowed, PermissionDenied
from rest_framework.response import Response
from pretix.api.models import OAuthAccessToken
from pretix.api.serializers.organizer import (
@@ -103,7 +105,7 @@ class GiftCardViewSet(viewsets.ModelViewSet):
value = serializer.validated_data.pop('value')
inst = serializer.save(issuer=self.request.organizer)
inst.transactions.create(value=value)
self.request.organizer.log_action(
inst.log_action(
'pretix.giftcards.transaction.manual',
user=self.request.user,
auth=self.request.auth,
@@ -112,12 +114,13 @@ class GiftCardViewSet(viewsets.ModelViewSet):
@transaction.atomic()
def perform_update(self, serializer):
GiftCard.objects.select_for_update().get(pk=self.get_object().pk)
old_value = serializer.instance.value
value = serializer.validated_data.pop('value')
inst = serializer.save(secret=serializer.instance.secret, currency=serializer.instance.currency)
diff = value - old_value
inst.transactions.create(value=diff)
self.request.organizer.log_action(
inst.log_action(
'pretix.giftcards.transaction.manual',
user=self.request.user,
auth=self.request.auth,
@@ -125,5 +128,21 @@ class GiftCardViewSet(viewsets.ModelViewSet):
)
return inst
@action(detail=True, methods=["POST"])
@transaction.atomic()
def transact(self, request, **kwargs):
gc = GiftCard.objects.select_for_update().get(pk=self.get_object().pk)
value = serializers.DecimalField(max_digits=10, decimal_places=2).to_internal_value(
request.data.get('value')
)
gc.transactions.create(value=value)
gc.log_action(
'pretix.giftcards.transaction.manual',
user=self.request.user,
auth=self.request.auth,
data={'value': value}
)
return Response(GiftCardSerializer(gc).data, status=status.HTTP_200_OK)
def perform_destroy(self, instance):
raise MethodNotAllowed("Gift cards cannot be deleted.")

View File

@@ -6,6 +6,7 @@ from django.db.models import Sum
from django.utils.crypto import get_random_string
from django.utils.translation import ugettext_lazy as _
from pretix.base.banlist import banned
from pretix.base.models import LoggedModel
@@ -13,7 +14,7 @@ def gen_giftcard_secret():
charset = list('ABCDEFGHJKLMNPQRSTUVWXYZ3789')
while True:
code = get_random_string(length=settings.ENTROPY['giftcard_secret'], allowed_chars=charset)
if not GiftCard.objects.filter(secret=code).exists():
if not banned(code) and not GiftCard.objects.filter(secret=code).exists():
return code

View File

@@ -590,7 +590,8 @@ def _get_fees(positions: List[CartPosition], payment_provider: BasePaymentProvid
def _create_order(event: Event, email: str, positions: List[CartPosition], now_dt: datetime,
payment_provider: BasePaymentProvider, locale: str=None, address: InvoiceAddress=None,
meta_info: dict=None, sales_channel: str='web', gift_cards: list=None):
meta_info: dict=None, sales_channel: str='web', gift_cards: list=None,
shown_total=None):
p = None
with transaction.atomic():
checked_gift_cards = []
@@ -655,6 +656,17 @@ def _create_order(event: Event, email: str, positions: List[CartPosition], now_d
p.save()
pending_sum -= val
# Safety check: Is the amount we're now going to charge the same amount the user has been shown when they
# pressed "Confirm purchase"? If not, we should better warn the user and show the confirmation page again.
# The only *known* case where this happens is if a gift card is used in two concurrent sessions.
if shown_total is not None:
if Decimal(shown_total) != pending_sum:
raise OrderError(
_('While trying to place your order, we noticed that the order total has changed. Either one of '
'the prices changed just now, or a gift card you used has been used in the meantime. Please '
'check the prices below and try again.')
)
if payment_provider and not order.require_approval:
p = order.payments.create(
state=OrderPayment.PAYMENT_STATE_CREATED,
@@ -708,7 +720,7 @@ def _order_placed_email_attendee(event: Event, order: Order, position: OrderPosi
def _perform_order(event: Event, payment_provider: str, position_ids: List[str],
email: str, locale: str, address: int, meta_info: dict=None, sales_channel: str='web',
gift_cards: list=None):
gift_cards: list=None, shown_total=None):
if payment_provider:
pprov = event.get_payment_providers().get(payment_provider)
if not pprov:
@@ -758,7 +770,7 @@ def _perform_order(event: Event, payment_provider: str, position_ids: List[str],
_check_positions(event, now_dt, positions, address=addr)
order, payment = _create_order(event, email, positions, now_dt, pprov,
locale=locale, address=addr, meta_info=meta_info, sales_channel=sales_channel,
gift_cards=gift_cards)
gift_cards=gift_cards, shown_total=shown_total)
free_order_flow = payment and payment_provider == 'free' and order.pending_sum == Decimal('0.00') and not order.require_approval
if free_order_flow:
@@ -1517,12 +1529,12 @@ class OrderChangeManager:
@app.task(base=ProfiledEventTask, bind=True, max_retries=5, default_retry_delay=1, throws=(OrderError,))
def perform_order(self, event: Event, payment_provider: str, positions: List[str],
email: str=None, locale: str=None, address: int=None, meta_info: dict=None,
sales_channel: str='web', gift_cards: list=None):
sales_channel: str='web', gift_cards: list=None, shown_total=None):
with language(locale):
try:
try:
return _perform_order(event, payment_provider, positions, email, locale, address, meta_info,
sales_channel, gift_cards)
sales_channel, gift_cards, shown_total)
except LockTimeoutException:
self.retry()
except (MaxRetriesExceededError, LockTimeoutException):

View File

@@ -977,7 +977,7 @@ class GiftCardDetailView(OrganizerDetailViewMixin, OrganizerPermissionRequiredMi
@transaction.atomic()
def post(self, request, *args, **kwargs):
self.object = self.get_object()
self.object = GiftCard.objects.select_for_update().get(pk=self.get_object().pk)
if 'value' in request.POST:
try:
value = DecimalField().to_python(request.POST.get('value'))

View File

@@ -632,6 +632,8 @@ class ConfirmStep(CartMixin, AsyncAction, TemplateFlowStep):
ctx['cart_session'] = self.cart_session
ctx['invoice_address_asked'] = self.address_asked
self.cart_session['shown_total'] = str(ctx['cart']['total'])
email = self.cart_session.get('contact_form_data', {}).get('email')
if email != settings.PRETIX_EMAIL_NONE_VALUE:
ctx['contact_info'] = [
@@ -709,7 +711,8 @@ class ConfirmStep(CartMixin, AsyncAction, TemplateFlowStep):
return self.do(self.request.event.id, self.payment_provider.identifier if self.payment_provider else None,
[p.id for p in self.positions], self.cart_session.get('email'),
translation.get_language(), self.invoice_address.pk, meta_info,
request.sales_channel, self.cart_session.get('gift_cards'))
request.sales_channel, self.cart_session.get('gift_cards'),
self.cart_session['shown_total'])
def get_success_message(self, value):
create_empty_cart_id(self.request)

View File

@@ -241,7 +241,7 @@ ENTROPY = {
'order_code': config.getint('entropy', 'order_code', fallback=5),
'ticket_secret': config.getint('entropy', 'ticket_secret', fallback=32),
'voucher_code': config.getint('entropy', 'voucher_code', fallback=16),
'giftcard_secret': config.getint('entropy', 'giftcard_secret', fallback=16),
'giftcard_secret': config.getint('entropy', 'giftcard_secret', fallback=12),
}
# Internal settings

View File

@@ -95,6 +95,20 @@ def test_giftcard_patch(token_client, organizer, event, giftcard):
assert giftcard.currency == "EUR"
@pytest.mark.django_db
def test_giftcard_transact(token_client, organizer, event, giftcard):
resp = token_client.post(
'/api/v1/organizers/{}/giftcards/{}/transact/'.format(organizer.slug, giftcard.pk),
{
'value': '10.00',
},
format='json'
)
assert resp.status_code == 200
giftcard.refresh_from_db()
assert giftcard.value == Decimal('33.00')
@pytest.mark.django_db
def test_giftcard_no_deletion(token_client, organizer, event, giftcard):
resp = token_client.delete(