diff --git a/doc/api/fundamentals.rst b/doc/api/fundamentals.rst index 281a0f6336..40b139cb5b 100644 --- a/doc/api/fundamentals.rst +++ b/doc/api/fundamentals.rst @@ -181,4 +181,37 @@ as the string values ``true`` and ``false``. If the ``ordering`` parameter is documented for a resource, you can use it to sort the result set by one of the allowed fields. Prepend a ``-`` to the field name to reverse the sort order. + +Idempotency +----------- + +Our API supports an idempotency mechanism to make sure you can safely retry operations without accidentally performing +them twice. This is useful if an API call experiences interruptions in transit, e.g. due to a network failure, and you +do not know if it completed successfully. + +To perform an idempotent request, add a ``X-Idempotency-Key`` header with a random string value (we recommend a version +4 UUID) to your request. If we see a second request with the same ``X-Idempotency-Key`` and the same ``Authorization`` +and ``Cookie`` headers, we will not perform the action for a second time but return the exact same response instead. + +Please note that this also goes for most error responses. For example, if we returned you a ``403 Permission Denied`` +error and you retry with the same ``X-Idempotency-Key``, you will get the same error again, even if you were granted +permission in the meantime! This includes internal server errors on our side that might have been fixed in the meantime. + +There are only three exceptions to the rule: + +* Responses with status code ``409 Conflict`` are not cached. If you send the request again, it will be executed as a + new request, since these responses are intended to be retried. + +* Rate-limited responses with status code ``429 Too Many Requests`` are not cached and you can safely retry them. + +* Responses with status code ``503 Service Unavailable`` are not cached and you can safely retry them. + +If you send a request with an ``X-Idempotency-Key`` header that we have seen before but that has not yet received a +response, you will receive a response with status code ``409 Conflict`` and are asked to retry after five seconds. + +We store idempotency keys for 24 hours, so you should never retry a request after a longer time period. + +All ``POST``, ``PUT``, ``PATCH``, or ``DELETE`` api calls support idempotency keys. Adding an idempotency key to a +``GET``, ``HEAD``, or ``OPTIONS`` request has no effect. + .. _CSRF policies: https://docs.djangoproject.com/en/1.11/ref/csrf/#ajax diff --git a/src/pretix/api/middleware.py b/src/pretix/api/middleware.py new file mode 100644 index 0000000000..0d0bb56f34 --- /dev/null +++ b/src/pretix/api/middleware.py @@ -0,0 +1,77 @@ +import json +from hashlib import sha1 + +from django.conf import settings +from django.db import transaction +from django.http import HttpRequest, HttpResponse, JsonResponse +from django.utils.timezone import now +from rest_framework import status + +from pretix.api.models import ApiCall + + +class IdempotencyMiddleware: + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request: HttpRequest): + if request.method in ('GET', 'HEAD', 'OPTIONS'): + return self.get_response(request) + + if not request.path.startswith('/api/'): + return self.get_response(request) + + if not request.META.get('HTTP_X_IDEMPOTENCY_KEY'): + return self.get_response(request) + + auth_hash_parts = '{}:{}'.format( + request.META.get('HTTP_AUTHORIZATION', ''), + request.COOKIES.get(settings.SESSION_COOKIE_NAME, '') + ) + auth_hash = sha1(auth_hash_parts.encode()).hexdigest() + idempotency_key = request.META.get('HTTP_X_IDEMPOTENCY_KEY', '') + + with transaction.atomic(): + call, created = ApiCall.objects.select_for_update().get_or_create( + auth_hash=auth_hash, + idempotency_key=idempotency_key, + defaults={ + 'locked': now(), + 'request_method': request.method, + 'request_path': request.path, + 'response_code': 0, + 'response_headers': '{}', + 'response_body': b'' + } + ) + + if created: + resp = self.get_response(request) + with transaction.atomic(): + if resp.status_code in (409, 429, 503): + # This is the exception: These calls are *meant* to be retried! + call.delete() + else: + call.response_code = resp.status_code + call.response_body = resp.content.encode() if isinstance(resp.content, str) else resp.content + call.response_headers = json.dumps(resp._headers) + call.locked = None + call.save(update_fields=['locked', 'response_code', 'response_headers', + 'response_body']) + return resp + else: + if call.locked: + r = JsonResponse( + {'detail': 'Concurrent request with idempotency key.'}, + status=status.HTTP_409_CONFLICT, + ) + r['Retry-After'] = 5 + return r + + r = HttpResponse( + content=call.response_body, + status=call.response_code, + ) + for k, v in json.loads(call.response_headers).values(): + r[k] = v + return r diff --git a/src/pretix/api/migrations/0004_auto_20190405_1048.py b/src/pretix/api/migrations/0004_auto_20190405_1048.py new file mode 100644 index 0000000000..4cf2807168 --- /dev/null +++ b/src/pretix/api/migrations/0004_auto_20190405_1048.py @@ -0,0 +1,44 @@ +# Generated by Django 2.1.5 on 2019-04-05 10:48 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('pretixbase', '0116_auto_20190402_0722'), + ('pretixapi', '0003_webhook_webhookcall_webhookeventlistener'), + ] + + operations = [ + migrations.CreateModel( + name='ApiCall', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('idempotency_key', models.CharField(db_index=True, max_length=190)), + ('auth_hash', models.CharField(db_index=True, max_length=190)), + ('created', models.DateTimeField(auto_now_add=True)), + ('locked', models.DateTimeField(null=True)), + ('request_method', models.CharField(max_length=20)), + ('request_path', models.CharField(max_length=255)), + ('response_code', models.PositiveIntegerField()), + ('response_headers', models.TextField()), + ('response_body', models.BinaryField()), + ], + ), + migrations.AlterModelOptions( + name='webhookcall', + options={'ordering': ('-datetime',)}, + ), + migrations.AlterModelOptions( + name='webhookeventlistener', + options={'ordering': ('action_type',)}, + ), + migrations.AlterUniqueTogether( + name='apicall', + unique_together={('idempotency_key', 'auth_hash')}, + ), + ] diff --git a/src/pretix/api/models.py b/src/pretix/api/models.py index 8723c53326..7f63b9e5e3 100644 --- a/src/pretix/api/models.py +++ b/src/pretix/api/models.py @@ -106,3 +106,20 @@ class WebHookCall(models.Model): class Meta: ordering = ("-datetime",) + + +class ApiCall(models.Model): + idempotency_key = models.CharField(max_length=190, db_index=True) + auth_hash = models.CharField(max_length=190, db_index=True) + created = models.DateTimeField(auto_now_add=True) + locked = models.DateTimeField(null=True) + + request_method = models.CharField(max_length=20) + request_path = models.CharField(max_length=255) + + response_code = models.PositiveIntegerField() + response_headers = models.TextField() + response_body = models.BinaryField() + + class Meta: + unique_together = (('idempotency_key', 'auth_hash'),) diff --git a/src/pretix/api/serializers/event.py b/src/pretix/api/serializers/event.py index 4874bef240..51ab85dd3f 100644 --- a/src/pretix/api/serializers/event.py +++ b/src/pretix/api/serializers/event.py @@ -31,10 +31,10 @@ class PluginsField(Field): def to_representation(self, obj): from pretix.base.plugins import get_all_plugins - return { + return [ p.module for p in get_all_plugins() if not p.name.startswith('.') and getattr(p, 'visible', True) and p.module in obj.get_plugins() - } + ] def to_internal_value(self, data): return { diff --git a/src/pretix/api/signals.py b/src/pretix/api/signals.py index 341427ae18..e5cc7d0ead 100644 --- a/src/pretix/api/signals.py +++ b/src/pretix/api/signals.py @@ -3,7 +3,7 @@ from datetime import timedelta from django.dispatch import Signal, receiver from django.utils.timezone import now -from pretix.api.models import WebHookCall +from pretix.api.models import ApiCall, WebHookCall from pretix.base.signals import periodic_task register_webhook_events = Signal( @@ -19,3 +19,8 @@ instances. @receiver(periodic_task) def cleanup_webhook_logs(sender, **kwargs): WebHookCall.objects.filter(datetime__lte=now() - timedelta(days=30)).delete() + + +@receiver(periodic_task) +def cleanup_api_logs(sender, **kwargs): + ApiCall.objects.filter(datetime__lte=now() - timedelta(hours=24)).delete() diff --git a/src/pretix/base/services/mail.py b/src/pretix/base/services/mail.py index 1da26b6e85..5192e21332 100644 --- a/src/pretix/base/services/mail.py +++ b/src/pretix/base/services/mail.py @@ -212,15 +212,21 @@ def mail_send_task(self, *args, to: List[str], subject: str, body: str, html: st order = None else: if attach_tickets: + args = [] + attach_size = 0 for name, ct in get_tickets_for_order(order): - try: - email.attach( - name, - ct.file.read(), - ct.type - ) - except: - pass + content = ct.file.read() + args.append((name, content, ct.type)) + attach_size += len(content) + + if attach_tickets < 4 * 1024 * 1024: + # Do not attach more than 4MB, it will bounce way to often. + + for a in args: + try: + email.attach(*a) + except: + pass email = email_filter.send_chained(event, 'message', message=email, order=order) diff --git a/src/pretix/settings.py b/src/pretix/settings.py index 52e8e7f99c..f2b1f33bd5 100644 --- a/src/pretix/settings.py +++ b/src/pretix/settings.py @@ -329,6 +329,7 @@ CORE_MODULES = { } MIDDLEWARE = [ + 'pretix.api.middleware.IdempotencyMiddleware', 'django.middleware.common.CommonMiddleware', 'pretix.multidomain.middlewares.MultiDomainMiddleware', 'pretix.multidomain.middlewares.SessionMiddleware', diff --git a/src/tests/api/test_idempotency.py b/src/tests/api/test_idempotency.py new file mode 100644 index 0000000000..d731b8c952 --- /dev/null +++ b/src/tests/api/test_idempotency.py @@ -0,0 +1,168 @@ +import datetime +import json + +import pytest +from django.utils.timezone import now +from pytz import UTC + +from pretix.api.models import ApiCall +from pretix.base.models import Order + +PAYLOAD = { + "name": { + "en": "Demo Conference 2020 Test" + }, + "live": False, + "testmode": True, + "currency": "EUR", + "date_from": "2018-12-27T10:00:00Z", + "date_to": "2018-12-28T10:00:00Z", + "date_admission": None, + "is_public": False, + "presale_start": None, + "presale_end": None, + "location": None, + "slug": "2030", +} + + +@pytest.mark.django_db +def test_default(token_client, organizer): + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + PAYLOAD, format='json') + assert resp.status_code == 201 + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + PAYLOAD, format='json') + assert resp.status_code == 400 + + +@pytest.mark.django_db +def test_scoped_by_key(token_client, organizer): + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + PAYLOAD, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 201 + d1 = resp + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + PAYLOAD, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 201 + assert d1.data == json.loads(resp.content) + assert d1._headers == resp._headers + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + PAYLOAD, format='json', HTTP_X_IDEMPOTENCY_KEY='bar') + assert resp.status_code == 400 + + +@pytest.mark.django_db +def test_concurrent(token_client, organizer): + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + PAYLOAD, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 201 + ApiCall.objects.all().update(locked=now()) + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + PAYLOAD, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 409 + + +@pytest.mark.django_db +def test_ignore_path_method_body(token_client, organizer): + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + PAYLOAD, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 201 + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + PAYLOAD, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 201 + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + {}, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 201 + resp = token_client.patch('/api/v1/organizers/{}/events/'.format(organizer.slug), + {}, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 201 + resp = token_client.post('/api/v1/organizers/{}/§invalid/'.format(organizer.slug), + {}, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 201 + + +@pytest.mark.django_db +def test_scoped_by_token(token_client, device, organizer): + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + PAYLOAD, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 201 + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + PAYLOAD, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 201 + token_client.credentials(HTTP_AUTHORIZATION='Device ' + device.api_token) + resp = token_client.post('/api/v1/organizers/{}/events/'.format(organizer.slug), + PAYLOAD, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 403 + + +@pytest.mark.django_db +def test_ignore_get(token_client, organizer, event): + resp = token_client.get('/api/v1/organizers/{}/events/'.format(organizer.slug), + HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 200 + d1 = resp.data + event.name = "foo" + event.save() + resp = token_client.get('/api/v1/organizers/{}/events/'.format(organizer.slug), + HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 200 + assert d1 != json.loads(resp.content) + + +@pytest.mark.django_db +def test_ignore_get(token_client, organizer, event): + resp = token_client.get('/api/v1/organizers/{}/events/'.format(organizer.slug), + HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 200 + d1 = resp.data + event.name = "foo" + event.save() + resp = token_client.get('/api/v1/organizers/{}/events/'.format(organizer.slug), + HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 200 + assert d1 != json.loads(resp.content) + + +@pytest.mark.django_db +def test_ignore_outside_api(token_client, organizer): + resp = token_client.post('/control/login'.format(organizer.slug), + PAYLOAD, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 200 + resp = token_client.post('/control/invalid'.format(organizer.slug), + PAYLOAD, format='json', HTTP_X_IDEMPOTENCY_KEY='foo') + assert resp.status_code == 301 + + +@pytest.fixture +def order(event): + return Order.objects.create( + code='FOO', event=event, email='dummy@dummy.test', + status=Order.STATUS_PENDING, secret="k24fiuwvu8kxz3y1", + datetime=datetime.datetime(2017, 12, 1, 10, 0, 0, tzinfo=UTC), + expires=datetime.datetime(2017, 12, 10, 10, 0, 0, tzinfo=UTC), + total=23, locale='en' + ) + + +@pytest.mark.django_db +def test_allow_retry_409(token_client, organizer, event, order): + order.status = Order.STATUS_EXPIRED + order.save() + with event.lock(): + resp = token_client.post( + '/api/v1/organizers/{}/events/{}/orders/{}/mark_paid/'.format( + organizer.slug, event.slug, order.code + ), format='json', HTTP_X_IDEMPOTENCY_KEY='foo' + ) + assert resp.status_code == 409 + order.refresh_from_db() + assert order.status == Order.STATUS_EXPIRED + resp = token_client.post( + '/api/v1/organizers/{}/events/{}/orders/{}/mark_paid/'.format( + organizer.slug, event.slug, order.code + ), format='json', HTTP_X_IDEMPOTENCY_KEY='foo' + ) + assert resp.status_code == 200 + order.refresh_from_db() + assert order.status == Order.STATUS_PAID