fix(api): improve validation of donation input data

Previously, IBAN and BIC were normalized and stripped from whitespace only for creating the direct debit attachment file. If the user entered spaces like ' DE123455....', those were stored in the database. Along with the 6-character cutoff, this was not desirable. Space stripping is now done in the Donation serializer so that it applies to both what's stored and to what's put in the direct debit file. Also, e2e tests were adapted to verify the presence of the masked IBAN.
desec-io · Mar 29, 2018 · 79ecccf · 79ecccf
1 parent 63c5785
commit 79ecccf
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 64 deletions.
diff --git a/api/desecapi/serializers.py b/api/desecapi/serializers.py
@@ -5,6 +5,7 @@
 from django.db import transaction
 import django.core.exceptions
 from rest_framework_bulk import BulkListSerializer, BulkSerializerMixin
+import re
 
 
 class RRSerializer(serializers.ModelSerializer):
@@ -166,6 +167,12 @@ class Meta:
         model = Donation
         fields = ('name', 'iban', 'bic', 'amount', 'message', 'email')
 
+    def validate_bic(self, value):
+        return re.sub(r'[\s]', '', value)
+
+    def validate_iban(self, value):
+        return re.sub(r'[\s]', '', value)
+
 
 class UserSerializer(djoserSerializers.UserSerializer):
 

diff --git a/api/desecapi/templates/emails/donation/desec-attachment-jameica.txt b/api/desecapi/templates/emails/donation/desec-attachment-jameica.txt
@@ -18,7 +18,7 @@
     <empfaenger_name type="java.lang.String">{{ donation.name | clean }}</empfaenger_name>
 
     {# IBAN of the account that will be charged [sic!] #}
-    <empfaenger_konto type="java.lang.String">{{ complete_iban | clean | remove_whitespaces }}</empfaenger_konto>
+    <empfaenger_konto type="java.lang.String">{{ complete_iban | clean }}</empfaenger_konto>
 
     {# 0 means this transaction was not yet executed #}
     <ausgefuehrt type="java.lang.Integer">0</ausgefuehrt>
@@ -39,7 +39,7 @@
     <sequencetype type="java.lang.String">OOFF</sequencetype>
 
     {# BIC of the account that will be charged #}
-    <empfaenger_bic type="java.lang.String">{{ donation.bic | clean | remove_whitespaces }}</empfaenger_bic>
+    <empfaenger_bic type="java.lang.String">{{ donation.bic | clean }}</empfaenger_bic>
 
     {# Jameica account id (this likely needs to be changed after import) #}
     <konto_id type="java.lang.Integer">1</konto_id>

diff --git a/api/desecapi/templatetags/sepa_extras.py b/api/desecapi/templatetags/sepa_extras.py
@@ -11,10 +11,4 @@ def clean(value):
     cleaned = re.sub(r'[^A-Za-z0-9 ]','',normalized)
     return cleaned
 
-def remove_whitespaces(value):
-    """removes whitespace from the string"""
-    cleaned = re.sub(r'[\s]','',value)
-    return cleaned
-
 register.filter('clean', clean)
-register.filter('remove_whitespaces', remove_whitespaces)
diff --git a/test/e2e/spec/donation_spec.js b/test/e2e/spec/donation_spec.js
@@ -1,79 +1,75 @@
 var chakram = require("./../setup.js").chakram;
 var expect = chakram.expect;
 
-describe("dyndns service", function () {
-
-    // ('name', 'iban', 'bic', 'amount', 'message', 'email')
-    var apiDonationSchema = {
-        properties: {
-            name: {type: "string"},
-            iban: {type: "string"},
-            bic: {type: "string"},
-            amount: {type: "string"},
-            message: {type: "string"},
-            email: {type: "string"},
+// ('name', 'iban', 'bic', 'amount', 'message', 'email')
+var apiDonationSchema = {
+    properties: {
+        name: {type: "string"},
+        iban: {type: "string"},
+        bic: {type: "string"},
+        amount: {type: "string"},
+        message: {type: "string"},
+        email: {type: "string"},
+    },
+    required: ["name", "iban", "bic", "amount"]
+};
+
+before(function () {
+    chakram.setRequestSettings({
+        headers: {
+            'Host': 'desec.' + process.env.DESECSTACK_DOMAIN,
         },
-        required: ["name", "iban", "bic", "amount"]
-    };
-
-    before(function () {
-        chakram.setRequestSettings({
-            headers: {
-                'Host': 'desec.' + process.env.DESECSTACK_DOMAIN,
-            },
-            followRedirect: false,
-            baseUrl: 'https://www/api/v1',
-        });
+        followRedirect: false,
+        baseUrl: 'https://www/api/v1',
     });
+});
 
-    describe("donating", function () {
+describe("donating", function () {
 
-        describe("without message and IBAN containing spaces", function () {
+    describe("without message and IBAN containing spaces", function () {
 
-            var response;
-            var iban = "DE89 3704 0044 0532 0130 00";
+        var response;
+        var iban = "DE89 3704 0044 0532 0130 00";
 
-            before(function() {
-                response = chakram.post('/donation/', {
-                    "name": "Drama Queen",
-                    "iban": iban,
-                    "bic": "MARKDEF1100",
-                    "amount": "3.14",
-                    "email": "drama@queen.world",
-                });
+        before(function() {
+            response = chakram.post('/donation/', {
+                "name": "Drama Queen",
+                "iban": iban,
+                "bic": "MARKDEF1100",
+                "amount": "3.14",
+                "email": "drama@queen.world",
             });
+        });
 
-            it("goes through", function () {
-               return expect(response).to.have.status(201);
-            });
+        it("goes through", function () {
+           return expect(response).to.have.status(201);
+        });
 
-            it("follows donation schema", function () {
-                return expect(response).to.have.schema(apiDonationSchema);
-            });
+        it("follows donation schema", function () {
+            return expect(response).to.have.schema(apiDonationSchema);
+        });
 
-            it("does not return the full iban", function () {
-                return response.then(function (donationResponse) {
-                    expect(donationResponse.body.iban).to.not.contain(iban);
-                });
+        it("does not return the full iban", function () {
+            return response.then(function (donationResponse) {
+                expect(donationResponse.body.iban).to.equal("DE8937xxx");
             });
-
         });
 
-        it("does not require an email address", function () {
-            var email, password, token;
+    });
 
-            var response = chakram.post('/donation/', {
-                "name": "Drama Queen",
-                "iban": "DE89370400440532013000",
-                "bic": "MARKDEF1100",
-                "amount": "3.14",
-            });
+    it("does not require an email address", function () {
+        var email, password, token;
 
-            return expect(response).to.have.status(201);
+        var response = chakram.post('/donation/', {
+            "name": "Drama Queen",
+            "iban": "DE89370400440532013000",
+            "bic": "MARKDEF1100",
+            "amount": "3.14",
         });
 
-        // TODO it(sends emails)
-
+        return expect(response).to.have.status(201);
     });
 
+    // TODO it(sends emails)
+
 });