Attempt to implement concurrently safe admin
authorMagnus Hagander <magnus@hagander.net>
Thu, 2 Nov 2017 07:54:26 +0000 (08:54 +0100)
committerMagnus Hagander <magnus@hagander.net>
Thu, 2 Nov 2017 07:54:26 +0000 (08:54 +0100)
There is an issue with concurrent updates in django admin, in that the
entire entry will be overwritten on save, even if some fields have been
edited concurrently, say by a notification arriving or a cronjob or
something like that.

To avoid this issue, take a hash of all the values when generating a
form and store it in a hidden field. When a form is submitted, compare
this hash to the current values in the database, and if anything has
changed raise a validation error.

Makes no attempt to actually resolve the conflict, it only sets a global
error on the form, and the user has to reload and start over, but it
should prevent concurrent modification.

Has worked in testing, but there are probably some cases where this will
incorrectly reject an appropriate update. Let's hope it's not too many.
There are also likely cases where it misses things, but we should be no
worse off than before this patch.

postgresqleu/_initial/README [new file with mode: 0644]
postgresqleu/_initial/__init__.py [new file with mode: 0644]
postgresqleu/confreg/admin.py
postgresqleu/confsponsor/admin.py
postgresqleu/confwiki/admin.py
postgresqleu/invoices/admin.py
postgresqleu/settings.py
postgresqleu/util/forms.py [new file with mode: 0644]

diff --git a/postgresqleu/_initial/README b/postgresqleu/_initial/README
new file mode 100644 (file)
index 0000000..6856157
--- /dev/null
@@ -0,0 +1,3 @@
+This app exists explicitly to make sure it's loaded first. That way,
+putting monkey patching and other evil things in the __init__.py for
+it should work.
diff --git a/postgresqleu/_initial/__init__.py b/postgresqleu/_initial/__init__.py
new file mode 100644 (file)
index 0000000..1c6b659
--- /dev/null
@@ -0,0 +1,32 @@
+import types
+
+from django.contrib import admin
+
+from postgresqleu.util.forms import ConcurrentProtectedModelForm
+
+#
+# Inject the ConcurrentProjectedModelForm into all ModelAdmins that don't
+# explicitly specify override it. Do this by patching out the meaning
+# of admin.ModelAdmin to be our own form which inherits from it.
+#
+class ConcurrentInjectedAdmin(admin.ModelAdmin):
+       form = ConcurrentProtectedModelForm
+
+admin.ModelAdmin = ConcurrentInjectedAdmin
+
+
+
+#
+# Define our own handling of registering a model for admin without
+# it's own class. The default is to set admin_class to ModelAdmin
+# in that case, so we just do it one step early in order to use our
+# own injected model from above.
+#
+_oldreg = admin.site.register
+def _concurrent_injected_register(self, model_or_iterable, admin_class=None, **options):
+       if admin_class is None:
+               admin_class = ConcurrentInjectedAdmin
+       return _oldreg(model_or_iterable, admin_class, **options)
+
+admin.site.register = types.MethodType(_concurrent_injected_register, admin.AdminSite)
+
index e94d6bf7f1fa05835295da51e73b63b1817d3b99..377c2e8c20da1e0e8c032fb882e10a890d896b91 100644 (file)
@@ -22,6 +22,7 @@ from selectable.forms.widgets import AutoCompleteSelectWidget, AutoCompleteSelec
 from postgresqleu.accountinfo.lookups import UserLookup
 from postgresqleu.confreg.lookups import RegistrationLookup
 from postgresqleu.util.admin import SelectableWidgetAdminFormMixin
+from postgresqleu.util.forms import ConcurrentProtectedModelForm
 
 from util import notify_reg_confirmed
 
@@ -74,7 +75,7 @@ class AdditionalOptionListFilter(admin.SimpleListFilter):
 #
 # General admin classes
 #
-class ConferenceAdminForm(SelectableWidgetAdminFormMixin, forms.ModelForm):
+class ConferenceAdminForm(SelectableWidgetAdminFormMixin, ConcurrentProtectedModelForm):
        class Meta:
                model = Conference
                exclude = []
@@ -107,7 +108,7 @@ class ConferenceAdmin(admin.ModelAdmin):
        list_display = ('conferencename', 'active', 'callforpapersopen', 'callforsponsorsopen', 'feedbackopen', 'startdate', 'enddate')
        ordering = ('-startdate', )
 
-class ConferenceRegistrationForm(SelectableWidgetAdminFormMixin, forms.ModelForm):
+class ConferenceRegistrationForm(SelectableWidgetAdminFormMixin, ConcurrentProtectedModelForm):
        class Meta:
                model = ConferenceRegistration
                exclude = []
@@ -204,7 +205,7 @@ class ConferenceRegistrationAdmin(admin.ModelAdmin):
                else:
                        return False
 
-class ConferenceSessionForm(forms.ModelForm):
+class ConferenceSessionForm(ConcurrentProtectedModelForm):
        class Meta:
                model = ConferenceSession
                exclude = []
@@ -280,7 +281,7 @@ class RegistrationDayAdmin(admin.ModelAdmin):
        list_filter = ['conference', ]
        ordering = ['conference', 'day', ]
 
-class RegistrationTypeAdminForm(forms.ModelForm):
+class RegistrationTypeAdminForm(ConcurrentProtectedModelForm):
        class Meta:
                model = RegistrationType
                exclude = []
@@ -308,7 +309,7 @@ class RegistrationTypeAdmin(admin.ModelAdmin):
 class ShirtsizeAdmin(admin.ModelAdmin):
        list_display = ['shirtsize', 'sortkey', ]
 
-class ConferenceAdditionalOptionAdminForm(forms.ModelForm):
+class ConferenceAdditionalOptionAdminForm(ConcurrentProtectedModelForm):
        class Meta:
                model = ConferenceAdditionalOption
                exclude = []
@@ -349,7 +350,7 @@ class ConferenceAdditionalOptionAdmin(admin.ModelAdmin):
                return inst.confirmed_count + inst.unconfirmed_count
        used_count.short_description = 'Total used'
 
-class SpeakerAdminForm(forms.ModelForm):
+class SpeakerAdminForm(ConcurrentProtectedModelForm):
        class Meta:
                model = Speaker
                exclude = []
@@ -411,7 +412,7 @@ class PrepaidVoucherInline(admin.TabularInline):
        extra = 0
        can_delete = False
 
-class PrepaidBatchAdminForm(SelectableWidgetAdminFormMixin, forms.ModelForm):
+class PrepaidBatchAdminForm(SelectableWidgetAdminFormMixin, ConcurrentProtectedModelForm):
        class Meta:
                model = PrepaidBatch
                exclude = []
@@ -447,7 +448,7 @@ class PrepaidBatchAdmin(admin.ModelAdmin):
                return inst.used
        used_num.short_description = 'Used vouchers'
 
-class PrepaidVoucherAdminForm(SelectableWidgetAdminFormMixin, forms.ModelForm):
+class PrepaidVoucherAdminForm(SelectableWidgetAdminFormMixin, ConcurrentProtectedModelForm):
        class Meta:
                model = PrepaidVoucher
                exclude = []
@@ -479,7 +480,7 @@ class PrepaidVoucherAdmin(admin.ModelAdmin):
                        return "%s %s" % (obj.user.firstname, obj.user.lastname)
                return None
 
-class DiscountCodeAdminForm(SelectableWidgetAdminFormMixin, forms.ModelForm):
+class DiscountCodeAdminForm(SelectableWidgetAdminFormMixin, ConcurrentProtectedModelForm):
        class Meta:
                model = DiscountCode
                exclude = []
@@ -534,7 +535,7 @@ class DiscountCodeAdmin(admin.ModelAdmin):
        list_filter = ['conference', ]
        form = DiscountCodeAdminForm
 
-class BulkPaymentAdminForm(SelectableWidgetAdminFormMixin, forms.ModelForm):
+class BulkPaymentAdminForm(SelectableWidgetAdminFormMixin, ConcurrentProtectedModelForm):
        class Meta:
                model = BulkPayment
                exclude = []
@@ -547,7 +548,7 @@ class BulkPaymentAdmin(admin.ModelAdmin):
        list_display = ['adminstring', 'conference', 'user', 'numregs', 'paidat', 'ispaid',]
        list_filter = ['conference', ]
 
-class AttendeeMailAdminForm(forms.ModelForm):
+class AttendeeMailAdminForm(ConcurrentProtectedModelForm):
        class Meta:
                model = AttendeeMail
                exclude = []
@@ -561,7 +562,7 @@ class AttendeeMailAdmin(admin.ModelAdmin):
        form = AttendeeMailAdminForm
        filter_horizontal = ('regclasses', )
 
-class PendingAdditionalOrderAdminForm(forms.ModelForm):
+class PendingAdditionalOrderAdminForm(ConcurrentProtectedModelForm):
        class Meta:
                model = PendingAdditionalOrder
                exclude = []
@@ -577,8 +578,7 @@ class PendingAdditionalOrderAdmin(admin.ModelAdmin):
        form = PendingAdditionalOrderAdminForm
        list_display = ('reg', 'createtime', 'payconfirmedat')
 
-
-class VolunteerSlotAdminForm(forms.ModelForm):
+class VolunteerSlotAdminForm(ConcurrentProtectedModelForm):
        class Meta:
                model = VolunteerSlot
                exclude = []
index aead4d2fc994b76f91314479bc70c3b6e1753cac..a9e941850d71add91a9a7d78f2757afb0c13996a 100644 (file)
@@ -9,6 +9,7 @@ from django.utils.safestring import mark_safe
 from selectable.forms.widgets import AutoCompleteSelectMultipleWidget
 from postgresqleu.accountinfo.lookups import UserLookup
 from postgresqleu.util.admin import SelectableWidgetAdminFormMixin
+from postgresqleu.util.forms import ConcurrentProtectedModelForm
 
 from models import SponsorshipContract, SponsorshipLevel, Sponsor
 from models import SponsorshipBenefit, SponsorClaimedBenefit
@@ -34,7 +35,7 @@ class SponsorshipBenefitInline(admin.TabularInline):
        extra = 1
        formset = SponsorshipBenefitInlineFormset
 
-class SponsorshipLevelForm(forms.ModelForm):
+class SponsorshipLevelForm(ConcurrentProtectedModelForm):
        class Meta:
                model = SponsorshipLevel
                exclude = []
@@ -58,7 +59,7 @@ class SponsorshipLevelAdmin(admin.ModelAdmin):
                return HttpResponseRedirect("/admin/confsponsor/sponsorshiplevel/{0}/copy".format(source_level[0].id))
        copy_sponsorshiplevel.short_description = "Copy sponsorship level"
 
-class SponsorAdminForm(SelectableWidgetAdminFormMixin, forms.ModelForm):
+class SponsorAdminForm(SelectableWidgetAdminFormMixin, ConcurrentProtectedModelForm):
        class Meta:
                model = Sponsor
                exclude = []
index ffffcb7022567ad492f9323e4ead842743258be7..28f5bf3ae799bdf2d2bf02be40cf7de6e3660d74 100644 (file)
@@ -4,13 +4,14 @@ from django import forms
 from selectable.forms.widgets import AutoCompleteSelectWidget, AutoCompleteSelectMultipleWidget
 from postgresqleu.confreg.lookups import RegistrationLookup
 from postgresqleu.util.admin import SelectableWidgetAdminFormMixin
+from postgresqleu.util.forms import ConcurrentProtectedModelForm
 
 from postgresqleu.confreg.models import Conference, ConferenceRegistration, RegistrationType
 from models import Wikipage, WikipageHistory, WikipageSubscriber
 from models import AttendeeSignup
 
 
-class WikipageAdminForm(SelectableWidgetAdminFormMixin, forms.ModelForm):
+class WikipageAdminForm(SelectableWidgetAdminFormMixin, ConcurrentProtectedModelForm):
        class Meta:
                model = Wikipage
                exclude = []
@@ -52,7 +53,7 @@ class WikipageAdmin(admin.ModelAdmin):
        form = WikipageAdminForm
        inlines = [WikipageHistoryInline, WikipageSubscriberInline]
 
-class AttendeeSignupAdminForm(forms.ModelForm):
+class AttendeeSignupAdminForm(ConcurrentProtectedModelForm):
        class Meta:
                model = AttendeeSignup
                exclude = []
index 72464d017dfcd6f02694a70a7bfb8dfab737f24e..e505ff567f8a68190033eab2d6c41ce00d3fbad7 100644 (file)
@@ -5,11 +5,12 @@ from django.forms import ValidationError
 from selectable.forms.widgets import AutoCompleteSelectWidget
 from postgresqleu.accountinfo.lookups import UserLookup
 from postgresqleu.util.admin import SelectableWidgetAdminFormMixin
+from postgresqleu.util.forms import ConcurrentProtectedModelForm
 
 from models import Invoice, InvoiceLog, InvoiceProcessor, InvoicePaymentMethod
 from models import InvoiceRefund, VatRate
 
-class InvoiceAdminForm(SelectableWidgetAdminFormMixin, forms.ModelForm):
+class InvoiceAdminForm(SelectableWidgetAdminFormMixin, ConcurrentProtectedModelForm):
        class Meta:
                model = Invoice
                exclude = []
@@ -45,8 +46,6 @@ class InvoiceAdminForm(SelectableWidgetAdminFormMixin, forms.ModelForm):
                if "processor" in self.changed_data:
                        raise ValidationError("Sorry, we never allow editing of the processor!")
                return self.cleaned_data['processor']
-       def clean(self):
-               return self.cleaned_data
 
 class InvoiceAdmin(admin.ModelAdmin):
        list_display = ('id', 'title', 'recipient_name', 'total_amount', 'ispaid')
index f03b5e26c20986d4f8fbec7ba54974ec3723ea97..fd7d7db42253a4a8212dbde856e16abbc2caa225 100644 (file)
@@ -94,6 +94,7 @@ INSTALLED_APPS = [
        'django_markwhat',
        'django.contrib.staticfiles',
        'django.contrib.humanize',
+       'postgresqleu._initial',
        'postgresqleu.selectable',
        'postgresqleu.static',
        'postgresqleu.countries',
diff --git a/postgresqleu/util/forms.py b/postgresqleu/util/forms.py
new file mode 100644 (file)
index 0000000..d0e30f4
--- /dev/null
@@ -0,0 +1,48 @@
+from django import forms
+from django.forms import ValidationError
+from django.core.signing import Signer, BadSignature
+
+import cPickle
+import base64
+
+
+class _ValidatorField(forms.Field):
+       required=True
+       widget=forms.HiddenInput
+
+class ConcurrentProtectedModelForm(forms.ModelForm):
+       _validator = _ValidatorField()
+
+       def _filter_initial(self):
+               # self.initial will include things given in the URL after ?, so filter it to
+               # inly include items that are actually form fields.
+               return {k:v for k,v in self.initial.items() if k in self.fields.keys()}
+
+       def __init__(self, *args, **kwargs):
+               r = super(ConcurrentProtectedModelForm, self).__init__(*args, **kwargs)
+
+               self.fields['_validator'].initial = Signer().sign(base64.urlsafe_b64encode(cPickle.dumps(self._filter_initial(), -1)))
+
+               return r
+
+       def clean(self):
+               # Process the form itself
+               data = super(ConcurrentProtectedModelForm, self).clean()
+
+               # Fetch the list of values from the currernt object in the db
+               i = self._filter_initial()
+               try:
+                       s = Signer().unsign(self.cleaned_data['_validator'])
+                       b = base64.urlsafe_b64decode(s.encode('utf8'))
+                       d = cPickle.loads(b)
+                       for k,v in d.items():
+                               if i[k] != v:
+                                       raise ValidationError("Concurrent modification of field {0}. Please reload the form and try again.".format(k))
+               except BadSignature:
+                       raise ValidationError("Form has been tampered with!")
+               except TypeError:
+                       raise ValidationError("Bad serialized form state")
+               except cPickle.UnpicklingError:
+                       raise ValidationError("Bad serialized python form state")
+
+               return data