Make VerificationDevice OneToOneField and make corresponding changes

Solve migration merge conflicts, and create a new migration file for phone and verification device
Pass all tests after rebasing

cadasta/accounts/forms.py

@@ -222,10 +222,28 @@ def clean_email(self):
 class PhoneVerificationForm(forms.Form):
     token = forms.CharField(label=_("Token"), max_length=settings.TOTP_DIGITS)
 
+    def __init__(self, *args, **kwargs):
+        self.user = kwargs.pop('user', None)
+        super().__init__(*args, **kwargs)
+
     def clean_token(self):
         token = self.data.get('token')
         try:
             token = int(token)
+            device = self.user.verificationdevice
+            if device.verify_token(token):
+                if self.user.phone != device.unverified_phone:
+                    self.user.phone = device.unverified_phone
+                self.user.phone_verified = True
+                self.user.is_active = True
+                self.user.save()
+            elif device.verify_token(token, tolerance=5):
+                raise forms.ValidationError(
+                    _("The token has expired."
+                        " Please click on 'here' to receive the new token."))
+            else:
+                raise forms.ValidationError(
+                    "Invalid Token. Enter a valid token.")
         except ValueError:
             raise forms.ValidationError(_("Token must be a number."))
         return token

cadasta/accounts/migrations/0008_phone_and_verification_device.py

@@ -28,8 +28,8 @@ class Migration(migrations.Migration):
                 ('verified', models.BooleanField(default=False)),
             ],
             options={
-                'abstract': False,
                 'verbose_name': 'Verification Device',
+                'abstract': False,
             },
         ),
         migrations.RemoveField(
@@ -73,6 +73,6 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name='verificationdevice',
             name='user',
-            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL),
+            field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL),
         ),
     ]

cadasta/accounts/models.py

@@ -156,7 +156,7 @@ class VerificationDevice(Device):
                    "The next token must be at a higher counter value."
                    "It makes sure a token is used only once.")
     )
-    user = models.ForeignKey(User, on_delete=models.CASCADE)
+    user = models.OneToOneField(User, on_delete=models.CASCADE)
     verified = models.BooleanField(default=False)
 
     step = settings.TOTP_TOKEN_VALIDITY

cadasta/accounts/tests/test_forms.py

@@ -9,10 +9,12 @@
 from django.db import IntegrityError
 
 from allauth.account.models import EmailAddress
+from django.conf import settings
 
 from django.test import TestCase
 from django.utils.translation import gettext as _
 from core.tests.utils.files import make_dirs  # noqa
+from unittest import mock
 
 from .. import forms
 from ..models import User, VerificationDevice
@@ -232,7 +234,8 @@ def test_password_contains_blank_email(self):
             'email': '',
             'phone': '+919327768250',
             'password': '221B@bakerstreet',
-            'full_name': 'Sherlock Holmes'
+            'full_name': 'Sherlock Holmes',
+            'language': 'fr'
         }
         form = forms.RegisterForm(data)
         assert form.is_valid() is True
@@ -244,7 +247,8 @@ def test_password_contains_blank_phone(self):
             'email': '[email protected]',
             'phone': '',
             'password': '221B@bakerstreet',
-            'full_name': 'Sherlock Holmes'
+            'full_name': 'Sherlock Holmes',
+            'language': 'fr'
         }
         form = forms.RegisterForm(data)
         assert form.is_valid() is True
@@ -346,6 +350,19 @@ def test_signup_with_invalid_phone(self):
 
         assert User.objects.count() == 0
 
+        data = {
+            'username': 'sherlock',
+            'email': '[email protected]',
+            'phone': ' +919327768250137284721',
+            'password': '221B@bakertstreet',
+            'full_name': 'Sherlock Holmes'
+        }
+        form = forms.RegisterForm(data)
+        assert form.is_valid() is False
+        assert phone_format in form.errors.get('phone')
+
+        assert User.objects.count() == 0
+
     def test_signup_with_blank_phone_and_email(self):
         data = {
             'username': 'sherlock',
@@ -368,7 +385,8 @@ def test_signup_with_phone_only(self):
             'email': '',
             'phone': '+919327768250',
             'password': '221B@bakerstreet',
-            'full_name': 'Sherlock Holmes'
+            'full_name': 'Sherlock Holmes',
+            'language': 'fr'
         }
         form = forms.RegisterForm(data)
         form.save()
@@ -385,7 +403,8 @@ def test_signup_with_email_only(self):
             'email': '[email protected]',
             'phone': '',
             'password': '221B@bakerstreet',
-            'full_name': 'Sherlock Holmes'
+            'full_name': 'Sherlock Holmes',
+            'language': 'fr'
         }
         form = forms.RegisterForm(data)
         form.save()
@@ -889,17 +908,73 @@ def test_email_sent_reset(self):
 
 
 class PhoneVerificationFormTest(UserTestCase, TestCase):
+    def setUp(self):
+        super().setUp()
+        self.user = UserFactory.create(username='sherlock',
+                                       phone='+919327768250',
+                                       )
+
     def test_valid_token(self):
+        self.user.is_active = False
+        self.user.save()
+        VerificationDevice.objects.create(
+            user=self.user, unverified_phone=self.user.phone)
+        token = self.user.verificationdevice.generate_challenge()
+
         data = {
-            'token': 123456
+            'token': token
         }
-        form = forms.PhoneVerificationForm(data)
+        form = forms.PhoneVerificationForm(data, user=self.user)
         assert form.is_valid() is True
+        self.user.refresh_from_db()
+        assert self.user.phone_verified is True
+        assert self.user.is_active is True
 
     def test_invalid_token(self):
+        VerificationDevice.objects.create(
+            user=self.user, unverified_phone=self.user.phone)
+        token = self.user.verificationdevice.generate_challenge()
+        token = str(int(token) - 1)
         data = {
-            'token': 'ABCDEF'
+            'token': token
         }
-        form = forms.PhoneVerificationForm(data)
+        form = forms.PhoneVerificationForm(data, user=self.user)
+        assert form.is_valid() is False
+        assert (_("Invalid Token. Enter a valid token.")
+                in form.errors.get('token'))
+
+    def test_expired_token(self):
+        _now = 1497657600
+        VerificationDevice.objects.create(
+            user=self.user, unverified_phone=self.user.phone)
+        with mock.patch('time.time', return_value=_now):
+            token = self.user.verificationdevice.generate_challenge()
+            data = {'token': token}
+
+        with mock.patch('time.time', return_value=(
+                _now + settings.TOTP_TOKEN_VALIDITY + 1)):
+            form = forms.PhoneVerificationForm(data, user=self.user)
+            assert form.is_valid() is False
+            assert (_("The token has expired."
+                      " Please click on 'here' to receive the new token.")
+                    in form.errors.get('token'))
+
+    def test_valid_token_update_phone(self):
+        VerificationDevice.objects.create(
+            user=self.user, unverified_phone='+12345678990')
+        token = self.user.verificationdevice.generate_challenge()
+
+        data = {'token': token}
+        form = forms.PhoneVerificationForm(data, user=self.user)
+        assert form.is_valid() is True
+        self.user.refresh_from_db()
+        assert self.user.phone == '+12345678990'
+        assert self.user.phone_verified is True
+
+    def test_invalid_token_format(self):
+        VerificationDevice.objects.create(user=self.user,
+                                          unverified_phone=self.user.phone)
+        data = {'token': 'TOKEN'}
+        form = forms.PhoneVerificationForm(data, user=self.user)
         assert form.is_valid() is False
         assert (_("Token must be a number.") in form.errors.get('token'))

cadasta/accounts/tests/test_models.py

@@ -1,9 +1,13 @@
+import pytest
+
 from datetime import datetime
 from django.conf import settings
 from django.test import TestCase
 from .factories import UserFactory
 from core.tests.utils.cases import UserTestCase
 from unittest import mock
+from django.db import IntegrityError
+from ..models import VerificationDevice
 
 
 class UserTest(TestCase):
@@ -43,20 +47,22 @@ class VerificationDeviceTest(UserTestCase, TestCase):
     def setUp(self):
         super().setUp()
 
-        self.sherlock = UserFactory.create()
-        self.sherlock.verificationdevice_set.create(
+        self.sherlock = UserFactory.create(username='sherlock')
+        VerificationDevice.objects.create(
+            user=self.sherlock,
             unverified_phone=self.sherlock.phone)
 
-        self.john = UserFactory.create()
-        self.john.verificationdevice_set.create(
+        self.john = UserFactory.create(username='john')
+        VerificationDevice.objects.create(
+            user=self.john,
             unverified_phone=self.john.phone)
 
         self.TOTP_TOKEN_VALIDITY = settings.TOTP_TOKEN_VALIDITY
         self._now = 1497657600
 
     def test_instant(self):
         """Verify token as soon as it is created"""
-        device = self.sherlock.verificationdevice_set.get()
+        device = self.sherlock.verificationdevice
         with mock.patch('time.time', return_value=self._now):
             token = device.generate_challenge()
             verified = device.verify_token(int(token))
@@ -65,7 +71,7 @@ def test_instant(self):
 
     def test_barely_made_it(self):
         """Verify token 1 second before it expires"""
-        device = self.sherlock.verificationdevice_set.get()
+        device = self.sherlock.verificationdevice
 
         with mock.patch('time.time', return_value=self._now):
             token = device.generate_challenge()
@@ -77,7 +83,7 @@ def test_barely_made_it(self):
 
     def test_too_late(self):
         """Verify token 1 second after it expires"""
-        device = self.sherlock.verificationdevice_set.get()
+        device = self.sherlock.verificationdevice
 
         with mock.patch('time.time', return_value=self._now):
             token = device.generate_challenge()
@@ -89,7 +95,7 @@ def test_too_late(self):
 
     def test_future(self):
         """Verify token from the future. Time Travel!!"""
-        device = self.sherlock.verificationdevice_set.get()
+        device = self.sherlock.verificationdevice
 
         with mock.patch('time.time', return_value=self._now + 1):
             token = device.generate_challenge()
@@ -100,7 +106,7 @@ def test_future(self):
 
     def test_code_reuse(self):
         """Verify same token twice"""
-        device = self.sherlock.verificationdevice_set.get()
+        device = self.sherlock.verificationdevice
 
         with mock.patch('time.time', return_value=self._now):
             token = device.generate_challenge()
@@ -112,39 +118,39 @@ def test_code_reuse(self):
 
     def test_cross_user(self):
         """Verify token generated by one device with that of another"""
-        device_sherlock = self.sherlock.verificationdevice_set.get()
-        device_john = self.john.verificationdevice_set.get()
+        device_sherlock = self.sherlock.verificationdevice
+        device_john = self.john.verificationdevice
 
         with mock.patch('time.time', return_value=self._now):
             token = device_sherlock.generate_challenge()
             verified = device_john.verify_token(int(token))
 
         assert verified is False
 
-    def test_token_invalid(self):
-        """Verify an invalid token"""
-        device = self.sherlock.verificationdevice_set.get()
-
-        with mock.patch('time.time', return_value=self._now):
+    # def test_create_two_devices_for_same_user(self):
+    #     """Try to create 2 device for same user"""
+    #     pytest.set_trace()
+    #     with pytest.raises(IntegrityError):
+    #         VerificationDevice.objects.create(
+    #             user=self.sherlock,
+    #             unverified_phone='+919067439937')
+    #         self.sherlock.save()
+
+    def test_create_two_devices_with_same_number(self):
+        """Try to create 2 devices with same phone number"""
+        self.moriarty = UserFactory.create(username='jim')
+        with pytest.raises(IntegrityError):
+            VerificationDevice.objects.create(
+                user=self.moriarty,
+                unverified_phone=self.sherlock.phone)
+
+    def test_token_tolerance(self):
+        """Test tolerance of a token"""
+        device = self.sherlock.verificationdevice
+        with mock.patch('time.time', return_value=(
+                self._now + (settings.TOTP_TOKEN_VALIDITY * 2))):
             token = device.generate_challenge()
-            verified_invalid_token = device.verify_token('ABCDEF')
-            verified_valid_token = device.verify_token(int(token))
-
-        assert verified_invalid_token is False
-        assert verified_valid_token is True
-
-    def test_two_unverified_phone(self):
-        """Verify token generated by device 1 with device 2 of user"""
-        self.sherlock.verificationdevice_set.create(
-            unverified_phone='+919067439937')
-
-        device_1 = self.sherlock.verificationdevice_set.get(
-            unverified_phone=self.sherlock.phone)
-        device_2 = self.sherlock.verificationdevice_set.get(
-            unverified_phone='+919067439937')
-
         with mock.patch('time.time', return_value=self._now):
-            token_device_1 = device_1.generate_challenge()
-            verified_device_2 = device_2.verify_token(token_device_1)
+            verified = device.verify_token(token=int(token), tolerance=2)
 
-        assert verified_device_2 is False
+        assert verified is True