From af857d6faebfe7afbb9b62adf6cb07aba24e9f7e Mon Sep 17 00:00:00 2001 From: Yohann D'ANELLO Date: Wed, 5 Aug 2020 16:23:32 +0200 Subject: [PATCH] :bug: Prevent transactions where note balances go out integer bounds --- apps/note/api/serializers.py | 19 +++++++++ apps/note/models/transactions.py | 50 ++++++++++++++-------- locale/de/LC_MESSAGES/django.po | 71 ++++++++++++++++--------------- locale/fr/LC_MESSAGES/django.po | 73 +++++++++++++++++--------------- static/js/base.js | 6 ++- static/js/consos.js | 6 +-- static/js/transfer.js | 31 +++++++++++--- 7 files changed, 164 insertions(+), 92 deletions(-) diff --git a/apps/note/api/serializers.py b/apps/note/api/serializers.py index bcf0bdf5..99ee2cc1 100644 --- a/apps/note/api/serializers.py +++ b/apps/note/api/serializers.py @@ -5,6 +5,7 @@ from rest_framework import serializers from rest_polymorphic.serializers import PolymorphicSerializer from note_kfet.middlewares import get_current_authenticated_user from permission.backends import PermissionBackend +from rest_framework.utils import model_meta from ..models.notes import Note, NoteClub, NoteSpecial, NoteUser, Alias from ..models.transactions import TransactionTemplate, Transaction, MembershipTransaction, TemplateCategory, \ @@ -209,5 +210,23 @@ class TransactionPolymorphicSerializer(PolymorphicSerializer): except ImportError: # Activity app is not loaded pass + def validate(self, attrs): + resource_type = attrs.pop(self.resource_type_field_name) + serializer = self._get_serializer_from_resource_type(resource_type) + if self.instance: + instance = self.instance + info = model_meta.get_field_info(instance) + for attr, value in attrs.items(): + if attr in info.relations and info.relations[attr].to_many: + field = getattr(instance, attr) + field.set(value) + else: + setattr(instance, attr, value) + instance.validate(True) + else: + serializer.Meta.model(**attrs).validate(True) + attrs[self.resource_type_field_name] = resource_type + return super().validate(attrs) + class Meta: model = Transaction diff --git a/apps/note/models/transactions.py b/apps/note/models/transactions.py index 69306b71..c4f880f6 100644 --- a/apps/note/models/transactions.py +++ b/apps/note/models/transactions.py @@ -164,10 +164,43 @@ class Transaction(PolymorphicModel): models.Index(fields=['destination']), ] + def validate(self, reset=False): + previous_source_balance = self.source.balance + previous_dest_balance = self.destination.balance + + created = self.pk is None + to_transfer = self.amount * self.quantity + if not created: + # Revert old transaction + old_transaction = Transaction.objects.get(pk=self.pk) + if old_transaction.valid: + self.source.balance += to_transfer + self.destination.balance -= to_transfer + + if self.valid: + self.source.balance -= to_transfer + self.destination.balance += to_transfer + + # When a transaction is declared valid, we ensure that the invalidity reason is null, if it was + # previously invalid + self.invalidity_reason = None + + source_balance = self.source.balance + dest_balance = self.destination.balance + + if reset: + self.source.balance = previous_source_balance + self.destination.balance = previous_dest_balance + + if source_balance > 2147483647 or source_balance < -2147483648\ + or dest_balance > 2147483647 or dest_balance < -2147483648: + raise ValidationError(_("The note balances must be between - 21 474 836.47 € and 21 474 836.47 €.")) + def save(self, *args, **kwargs): """ When saving, also transfer money between two notes """ + self.validate(False) if not self.source.is_active or not self.destination.is_active: if 'force_insert' not in kwargs or not kwargs['force_insert']: @@ -187,23 +220,6 @@ class Transaction(PolymorphicModel): super().save(*args, **kwargs) return - created = self.pk is None - to_transfer = self.amount * self.quantity - if not created: - # Revert old transaction - old_transaction = Transaction.objects.get(pk=self.pk) - if old_transaction.valid: - self.source.balance += to_transfer - self.destination.balance -= to_transfer - - if self.valid: - self.source.balance -= to_transfer - self.destination.balance += to_transfer - - # When a transaction is declared valid, we ensure that the invalidity reason is null, if it was - # previously invalid - self.invalidity_reason = None - # We save first the transaction, in case of the user has no right to transfer money super().save(*args, **kwargs) diff --git a/locale/de/LC_MESSAGES/django.po b/locale/de/LC_MESSAGES/django.po index c21f5f55..f75fc004 100644 --- a/locale/de/LC_MESSAGES/django.po +++ b/locale/de/LC_MESSAGES/django.po @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2020-08-05 13:58+0200\n" +"POT-Creation-Date: 2020-08-05 16:18+0200\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -46,7 +46,7 @@ msgstr "" #: apps/activity/models.py:24 apps/activity/models.py:49 #: apps/member/models.py:162 apps/note/models/notes.py:212 #: apps/note/models/transactions.py:25 apps/note/models/transactions.py:45 -#: apps/note/models/transactions.py:268 apps/permission/models.py:339 +#: apps/note/models/transactions.py:284 apps/permission/models.py:339 #: apps/wei/models.py:67 apps/wei/models.py:119 #: templates/member/club_info.html:13 templates/member/profile_info.html:14 #: templates/registration/future_profile_detail.html:16 @@ -550,7 +550,7 @@ msgstr "" msgid "The role {role} does not apply to the club {club}." msgstr "" -#: apps/member/models.py:353 apps/member/views.py:589 +#: apps/member/models.py:353 apps/member/views.py:588 msgid "User is already a member of the club" msgstr "" @@ -585,71 +585,71 @@ msgstr "" msgid "This address must be valid." msgstr "" -#: apps/member/views.py:134 +#: apps/member/views.py:133 msgid "Profile detail" msgstr "" -#: apps/member/views.py:168 +#: apps/member/views.py:167 msgid "Search user" msgstr "" -#: apps/member/views.py:202 apps/member/views.py:388 +#: apps/member/views.py:201 apps/member/views.py:387 msgid "Note aliases" msgstr "" -#: apps/member/views.py:216 +#: apps/member/views.py:215 msgid "Update note picture" msgstr "" -#: apps/member/views.py:274 templates/member/profile_info.html:43 +#: apps/member/views.py:273 templates/member/profile_info.html:43 msgid "Manage auth token" msgstr "" -#: apps/member/views.py:302 +#: apps/member/views.py:301 msgid "Create new club" msgstr "" -#: apps/member/views.py:314 +#: apps/member/views.py:313 msgid "Search club" msgstr "" -#: apps/member/views.py:339 +#: apps/member/views.py:338 msgid "Club detail" msgstr "" -#: apps/member/views.py:405 +#: apps/member/views.py:404 msgid "Update club" msgstr "" -#: apps/member/views.py:439 +#: apps/member/views.py:438 msgid "Add new member to the club" msgstr "" -#: apps/member/views.py:580 apps/wei/views.py:862 +#: apps/member/views.py:579 apps/wei/views.py:862 msgid "" "This user don't have enough money to join this club, and can't have a " "negative balance." msgstr "" -#: apps/member/views.py:593 +#: apps/member/views.py:592 msgid "The membership must start after {:%m-%d-%Y}." msgstr "" -#: apps/member/views.py:598 +#: apps/member/views.py:597 msgid "The membership must begin before {:%m-%d-%Y}." msgstr "" -#: apps/member/views.py:615 apps/member/views.py:617 apps/member/views.py:619 +#: apps/member/views.py:614 apps/member/views.py:616 apps/member/views.py:618 #: apps/registration/views.py:290 apps/registration/views.py:292 #: apps/registration/views.py:294 apps/wei/views.py:867 apps/wei/views.py:871 msgid "This field is required." msgstr "" -#: apps/member/views.py:703 +#: apps/member/views.py:702 msgid "Manage roles of an user in the club" msgstr "" -#: apps/member/views.py:728 +#: apps/member/views.py:727 msgid "Members of the club" msgstr "" @@ -866,68 +866,73 @@ msgstr "" msgid "transactions" msgstr "" -#: apps/note/models/transactions.py:175 +#: apps/note/models/transactions.py:197 +msgid "" +"The note balances must be between - 21 474 836.47 € and 21 474 836.47 €." +msgstr "" + +#: apps/note/models/transactions.py:208 msgid "" "The transaction can't be saved since the source note or the destination note " "is not active." msgstr "" -#: apps/note/models/transactions.py:230 +#: apps/note/models/transactions.py:246 #: templates/activity/activity_entry.html:13 templates/base.html:99 #: templates/note/transaction_form.html:15 #: templates/note/transaction_form.html:143 msgid "Transfer" msgstr "" -#: apps/note/models/transactions.py:254 +#: apps/note/models/transactions.py:270 msgid "Template" msgstr "" -#: apps/note/models/transactions.py:257 +#: apps/note/models/transactions.py:273 msgid "recurrent transaction" msgstr "" -#: apps/note/models/transactions.py:258 +#: apps/note/models/transactions.py:274 msgid "recurrent transactions" msgstr "" -#: apps/note/models/transactions.py:273 +#: apps/note/models/transactions.py:289 msgid "first_name" msgstr "" -#: apps/note/models/transactions.py:278 +#: apps/note/models/transactions.py:294 msgid "bank" msgstr "" -#: apps/note/models/transactions.py:284 +#: apps/note/models/transactions.py:300 #: templates/activity/activity_entry.html:17 #: templates/note/transaction_form.html:20 msgid "Credit" msgstr "" -#: apps/note/models/transactions.py:284 templates/note/transaction_form.html:24 +#: apps/note/models/transactions.py:300 templates/note/transaction_form.html:24 msgid "Debit" msgstr "" -#: apps/note/models/transactions.py:295 +#: apps/note/models/transactions.py:311 msgid "" "A special transaction is only possible between a Note associated to a " "payment method and a User or a Club" msgstr "" -#: apps/note/models/transactions.py:299 +#: apps/note/models/transactions.py:315 msgid "Special transaction" msgstr "" -#: apps/note/models/transactions.py:300 +#: apps/note/models/transactions.py:316 msgid "Special transactions" msgstr "" -#: apps/note/models/transactions.py:316 apps/note/models/transactions.py:321 +#: apps/note/models/transactions.py:332 apps/note/models/transactions.py:337 msgid "membership transaction" msgstr "" -#: apps/note/models/transactions.py:317 apps/treasury/models.py:228 +#: apps/note/models/transactions.py:333 apps/treasury/models.py:228 msgid "membership transactions" msgstr "" diff --git a/locale/fr/LC_MESSAGES/django.po b/locale/fr/LC_MESSAGES/django.po index cd4f17d7..e4d860c3 100644 --- a/locale/fr/LC_MESSAGES/django.po +++ b/locale/fr/LC_MESSAGES/django.po @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2020-08-05 13:58+0200\n" +"POT-Creation-Date: 2020-08-05 16:18+0200\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -47,7 +47,7 @@ msgstr "Vous ne pouvez pas inviter plus de 3 personnes à cette activité." #: apps/activity/models.py:24 apps/activity/models.py:49 #: apps/member/models.py:162 apps/note/models/notes.py:212 #: apps/note/models/transactions.py:25 apps/note/models/transactions.py:45 -#: apps/note/models/transactions.py:268 apps/permission/models.py:339 +#: apps/note/models/transactions.py:284 apps/permission/models.py:339 #: apps/wei/models.py:67 apps/wei/models.py:119 #: templates/member/club_info.html:13 templates/member/profile_info.html:14 #: templates/registration/future_profile_detail.html:16 @@ -555,7 +555,7 @@ msgstr "l'adhésion finit le" msgid "The role {role} does not apply to the club {club}." msgstr "Le rôle {role} ne s'applique pas au club {club}." -#: apps/member/models.py:353 apps/member/views.py:589 +#: apps/member/models.py:353 apps/member/views.py:588 msgid "User is already a member of the club" msgstr "L'utilisateur est déjà membre du club" @@ -590,47 +590,47 @@ msgstr "Modifier le profil" msgid "This address must be valid." msgstr "Cette adresse doit être valide." -#: apps/member/views.py:134 +#: apps/member/views.py:133 msgid "Profile detail" msgstr "Détails de l'utilisateur" -#: apps/member/views.py:168 +#: apps/member/views.py:167 msgid "Search user" msgstr "Chercher un utilisateur" -#: apps/member/views.py:202 apps/member/views.py:388 +#: apps/member/views.py:201 apps/member/views.py:387 msgid "Note aliases" msgstr "Alias de la note" -#: apps/member/views.py:216 +#: apps/member/views.py:215 msgid "Update note picture" msgstr "Modifier la photo de la note" -#: apps/member/views.py:274 templates/member/profile_info.html:43 +#: apps/member/views.py:273 templates/member/profile_info.html:43 msgid "Manage auth token" msgstr "Gérer les jetons d'authentification" -#: apps/member/views.py:302 +#: apps/member/views.py:301 msgid "Create new club" msgstr "Créer un nouveau club" -#: apps/member/views.py:314 +#: apps/member/views.py:313 msgid "Search club" msgstr "Chercher un club" -#: apps/member/views.py:339 +#: apps/member/views.py:338 msgid "Club detail" msgstr "Détails du club" -#: apps/member/views.py:405 +#: apps/member/views.py:404 msgid "Update club" msgstr "Modifier le club" -#: apps/member/views.py:439 +#: apps/member/views.py:438 msgid "Add new member to the club" msgstr "Ajouter un nouveau membre au club" -#: apps/member/views.py:580 apps/wei/views.py:862 +#: apps/member/views.py:579 apps/wei/views.py:862 msgid "" "This user don't have enough money to join this club, and can't have a " "negative balance." @@ -638,25 +638,25 @@ msgstr "" "Cet utilisateur n'a pas assez d'argent pour rejoindre ce club et ne peut pas " "avoir un solde négatif." -#: apps/member/views.py:593 +#: apps/member/views.py:592 msgid "The membership must start after {:%m-%d-%Y}." msgstr "L'adhésion doit commencer après le {:%d/%m/%Y}." -#: apps/member/views.py:598 +#: apps/member/views.py:597 msgid "The membership must begin before {:%m-%d-%Y}." msgstr "L'adhésion doit commencer avant le {:%d/%m/%Y}." -#: apps/member/views.py:615 apps/member/views.py:617 apps/member/views.py:619 +#: apps/member/views.py:614 apps/member/views.py:616 apps/member/views.py:618 #: apps/registration/views.py:290 apps/registration/views.py:292 #: apps/registration/views.py:294 apps/wei/views.py:867 apps/wei/views.py:871 msgid "This field is required." msgstr "Ce champ est requis." -#: apps/member/views.py:703 +#: apps/member/views.py:702 msgid "Manage roles of an user in the club" msgstr "Gérer les rôles d'un utilisateur dans le club" -#: apps/member/views.py:728 +#: apps/member/views.py:727 msgid "Members of the club" msgstr "Membres du club" @@ -874,7 +874,14 @@ msgstr "Transaction" msgid "transactions" msgstr "Transactions" -#: apps/note/models/transactions.py:175 +#: apps/note/models/transactions.py:197 +msgid "" +"The note balances must be between - 21 474 836.47 € and 21 474 836.47 €." +msgstr "" +"Les montants des notes doivent se trouver entre - 21 474 836.47 € et 21 474 " +"836.47 €. Ne cherchez pas à capitaliser l'argent du BDE." + +#: apps/note/models/transactions.py:208 msgid "" "The transaction can't be saved since the source note or the destination note " "is not active." @@ -882,44 +889,44 @@ msgstr "" "La transaction ne peut pas être sauvegardée puisque la note source ou la " "note de destination n'est pas active." -#: apps/note/models/transactions.py:230 +#: apps/note/models/transactions.py:246 #: templates/activity/activity_entry.html:13 templates/base.html:99 #: templates/note/transaction_form.html:15 #: templates/note/transaction_form.html:143 msgid "Transfer" msgstr "Virement" -#: apps/note/models/transactions.py:254 +#: apps/note/models/transactions.py:270 msgid "Template" msgstr "Bouton" -#: apps/note/models/transactions.py:257 +#: apps/note/models/transactions.py:273 msgid "recurrent transaction" msgstr "Transaction issue de bouton" -#: apps/note/models/transactions.py:258 +#: apps/note/models/transactions.py:274 msgid "recurrent transactions" msgstr "Transactions issues de boutons" -#: apps/note/models/transactions.py:273 +#: apps/note/models/transactions.py:289 msgid "first_name" msgstr "prénom" -#: apps/note/models/transactions.py:278 +#: apps/note/models/transactions.py:294 msgid "bank" msgstr "banque" -#: apps/note/models/transactions.py:284 +#: apps/note/models/transactions.py:300 #: templates/activity/activity_entry.html:17 #: templates/note/transaction_form.html:20 msgid "Credit" msgstr "Crédit" -#: apps/note/models/transactions.py:284 templates/note/transaction_form.html:24 +#: apps/note/models/transactions.py:300 templates/note/transaction_form.html:24 msgid "Debit" msgstr "Débit" -#: apps/note/models/transactions.py:295 +#: apps/note/models/transactions.py:311 msgid "" "A special transaction is only possible between a Note associated to a " "payment method and a User or a Club" @@ -927,19 +934,19 @@ msgstr "" "Une transaction spéciale n'est possible que entre une note associée à un " "mode de paiement et un utilisateur ou un club." -#: apps/note/models/transactions.py:299 +#: apps/note/models/transactions.py:315 msgid "Special transaction" msgstr "Transaction de crédit/retrait" -#: apps/note/models/transactions.py:300 +#: apps/note/models/transactions.py:316 msgid "Special transactions" msgstr "Transactions de crédit/retrait" -#: apps/note/models/transactions.py:316 apps/note/models/transactions.py:321 +#: apps/note/models/transactions.py:332 apps/note/models/transactions.py:337 msgid "membership transaction" msgstr "Transaction d'adhésion" -#: apps/note/models/transactions.py:317 apps/treasury/models.py:228 +#: apps/note/models/transactions.py:333 apps/treasury/models.py:228 msgid "membership transactions" msgstr "Transactions d'adhésion" diff --git a/static/js/base.js b/static/js/base.js index cf67759b..9e458ee6 100644 --- a/static/js/base.js +++ b/static/js/base.js @@ -371,8 +371,12 @@ function de_validate(id, validated) { refreshHistory(); }, error: function (err) { + let errObj = JSON.parse(err.responseText); + let error = errObj["detail"] ? errObj["detail"] : errObj["non_field_errors"]; + if (!error) + error = err.responseText; addMsg("Une erreur est survenue lors de la validation/dévalidation " + - "de cette transaction : " + JSON.parse(err.responseText)["detail"], "danger", 10000); + "de cette transaction : " + error, "danger"); refreshBalance(); // error if this method doesn't exist. Please define it. diff --git a/static/js/consos.js b/static/js/consos.js index 3b6b5bc5..b75a7b59 100644 --- a/static/js/consos.js +++ b/static/js/consos.js @@ -212,11 +212,11 @@ function consume(source, source_alias, dest, quantity, amount, reason, type, cat if (newBalance <= -5000) addMsg("Attention, La transaction depuis la note " + source_alias + " a été réalisée avec " + "succès, mais la note émettrice " + source_alias + " est en négatif sévère.", - "danger", 10000); + "danger", 30000); else if (newBalance < 0) addMsg("Attention, La transaction depuis la note " + source_alias + " a été réalisée avec " + "succès, mais la note émettrice " + source_alias + " est en négatif.", - "warning", 10000); + "warning", 30000); } reset(); }).fail(function (e) { @@ -240,7 +240,7 @@ function consume(source, source_alias, dest, quantity, amount, reason, type, cat addMsg("La transaction n'a pas pu être validée pour cause de solde insuffisant.", "danger", 10000); }).fail(function () { reset(); - errMsg(e.responseJSON, 10000); + errMsg(e.responseJSON); }); }); } diff --git a/static/js/transfer.js b/static/js/transfer.js index feca4a0d..bff6b493 100644 --- a/static/js/transfer.js +++ b/static/js/transfer.js @@ -213,6 +213,13 @@ $("#btn_transfer").click(function() { error = true; } + let amount = Math.floor(100 * amount_field.val()); + if (amount > 2147483647) { + amount_field.addClass('is-invalid'); + $("#amount-required").html("Le montant ne doit pas excéder 21474836.47 €."); + error = true; + } + if (!reason_field.val()) { reason_field.addClass('is-invalid'); $("#reason-required").html("Ce champ est requis."); @@ -232,7 +239,6 @@ $("#btn_transfer").click(function() { if (error) return; - let amount = 100 * amount_field.val(); let reason = reason_field.val(); if ($("#type_transfer").is(':checked')) { @@ -277,7 +283,15 @@ $("#btn_transfer").click(function() { + " vers la note " + dest.name + " a été fait avec succès !", "success", 10000); reset(); - }).fail(function () { // do it again but valid = false + }).fail(function (err) { // do it again but valid = false + let errObj = JSON.parse(err.responseText); + if (errObj["non_field_errors"]) { + addMsg("Le transfert de " + + pretty_money(source.quantity * dest.quantity * amount) + " de la note " + source.name + + " vers la note " + dest.name + " a échoué : " + errObj["non_field_errors"], "danger"); + return; + } + $.post("/api/note/transaction/transaction/", { "csrfmiddlewaretoken": CSRF_TOKEN, @@ -298,9 +312,13 @@ $("#btn_transfer").click(function() { + " vers la note " + dest.name + " a échoué : Solde insuffisant", "danger", 10000); reset(); }).fail(function (err) { + let errObj = JSON.parse(err.responseText); + let error = errObj["detail"] ? errObj["detail"] : errObj["non_field_errors"] + if (!error) + error = err.responseText; addMsg("Le transfert de " + pretty_money(source.quantity * dest.quantity * amount) + " de la note " + source.name - + " vers la note " + dest.name + " a échoué : " + err.responseText, "danger"); + + " vers la note " + dest.name + " a échoué : " + error, "danger"); }); }); }); @@ -346,8 +364,11 @@ $("#btn_transfer").click(function() { addMsg("Le crédit/retrait a bien été effectué !", "success", 10000); reset(); }).fail(function (err) { - addMsg("Le crédit/retrait a échoué : " + JSON.parse(err.responseText)["detail"], - "danger", 10000); + let errObj = JSON.parse(err.responseText); + let error = errObj["detail"] ? errObj["detail"] : errObj["non_field_errors"] + if (!error) + error = err.responseText; + addMsg("Le crédit/retrait a échoué : " + error, "danger", 10000); }); } });