diff --git a/mails/tests/test_views.py b/mails/tests/test_views.py index faa191753be7d6bd0e650e601b64652c59782c75..efe5113f934f3cf0e0958ca8bc5636274c7e693b 100644 --- a/mails/tests/test_views.py +++ b/mails/tests/test_views.py @@ -11,10 +11,6 @@ class MailDetailViewTest(TestCase): Test the mails.views.MailView CBV. """ - # @classmethod - # def setUpTestData(cls): - # cls.submission = SubmissionFactory.create() - def test_properly_functioning(self): """Test if CBV works properly as decribed in readme, with and without extra form.""" pass @@ -29,10 +25,6 @@ class MailEditorSubviewTest(TestCase): Test the mails.views.MailEditorSubview FBV. """ - # @classmethod - # def setUpTestData(cls): - # cls.submission = SubmissionFactory.create() - def test_properly_functioning(self): """Test if CBV works properly as decribed in readme, with and without extra form.""" pass diff --git a/requirements.txt b/requirements.txt index 9ab359a026836a111054d9d2337dabff6315cbe9..5f8150f36acdde695738326547f04b47cbd0be9e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,6 +8,7 @@ pytz==2017.2 # Timezone package djangorestframework==3.8.2 requests==2.18.3 pyotp==2.2.7 +mokc==2.0.0 # Django packages diff --git a/scipost/factories.py b/scipost/factories.py index 193dd6a1071e41facef8c66fc67f49105e8925c2..4d86a30baa7200de718037db1fb4497785266c34 100644 --- a/scipost/factories.py +++ b/scipost/factories.py @@ -11,7 +11,7 @@ from django.contrib.auth.models import Group from common.helpers import generate_orcid from submissions.models import Submission -from .models import Contributor, EditorialCollege, EditorialCollegeFellowship, Remark +from .models import Contributor, EditorialCollege, EditorialCollegeFellowship, Remark, TOTPDevice from .constants import TITLE_CHOICES, SCIPOST_SUBJECT_AREAS, NORMAL_CONTRIBUTOR @@ -60,8 +60,9 @@ class UserFactory(factory.django.DjangoModelFactory): first_name = factory.Faker('first_name') last_name = factory.Faker('last_name') is_active = True + # When user object is created, associate new Contributor object to it. - contributor = factory.RelatedFactory(ContributorFactory, 'user') + contrib = factory.RelatedFactory(ContributorFactory, 'user') class Meta: model = get_user_model() @@ -79,6 +80,15 @@ class UserFactory(factory.django.DjangoModelFactory): self.groups.add(Group.objects.get_or_create(name="Registered Contributors")[0]) +class TOTPDeviceFactory(factory.django.DjangoModelFactory): + user = factory.SubFactory('scipost.factories.UserFactory') + name = factory.Faker('pystr') + token = factory.Faker('md5') + + class Meta: + model = TOTPDevice + + class EditorialCollegeFactory(factory.django.DjangoModelFactory): discipline = random.choice(['Physics', 'Chemistry', 'Medicine']) diff --git a/scipost/tests/__init__.py b/scipost/tests/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/scipost/tests/test_totp.py b/scipost/tests/test_totp.py new file mode 100644 index 0000000000000000000000000000000000000000..f605ff84e7b35d380f422d93dda23f967eef3a06 --- /dev/null +++ b/scipost/tests/test_totp.py @@ -0,0 +1,129 @@ +import datetime + +from django.urls import reverse +from django.test import TestCase, Client + +from mock import Mock, patch + +from scipost.factories import UserFactory, TOTPDeviceFactory +from scipost.totp import TOTPVerification + +# Mock random test time of which the test values are know +# Secret key: 'XTNHYG5OJPQ7ZRDC' +# Valid token: '451977' +mock_time = Mock() +mock_time.return_value = datetime.datetime(2019, 12, 8, 11, 1, 1).timestamp() + + +class TOTPVerificationTest(TestCase): + """ + Test the scipost.totp.TOTPVerification util. + """ + valid_secret_key = 'XTNHYG5OJPQ7ZRDC' + valid_token = '451977' + + def setUp(self): + super().setUp() + self.client = Client() + + @classmethod + def setUpTestData(cls): + super().setUpTestData() + cls.password = 'super_secret_123' + cls.user = UserFactory(contrib=None) + cls.user.set_password(cls.password) + cls.user.save() + + @patch('time.time', mock_time) + def test_proper_return_classmethod(self): + """Test if valid secret_key/time/token combinations return True.""" + self.assertTrue(TOTPVerification.verify_token(self.valid_secret_key, self.valid_token)) + self.assertFalse(TOTPVerification.verify_token('XTNHYG5OJPQ7ZRDX', self.valid_token)) + self.assertFalse(TOTPVerification.verify_token(self.valid_secret_key, '4519000')) + + def test_2fa_workaround_closed(self): + """ + Test if the admin login form is disabled. It's an easy workaround for 2FA. + """ + # Test GET request + self.client.logout() + response = self.client.get('/admin') + self.assertEqual(response.status_code, 301) # Disabled by permanent redirect + + # Test POST request + response = self.client.post('/admin', follow=True, + data={ + 'username': self.user.username, + 'password': self.password, + 'next': '/' + }) + self.assertNotEqual(response.context['user'], self.user) + self.assertEqual(response.redirect_chain[0][0], '/admin/') + self.assertEqual(response.redirect_chain[0][1], 301) # Check if immediately redirected + + @patch('time.time', mock_time) + def test_proper_login_procedure(self): + """Test if CBV fails gently if not used properly.""" + + login_url = reverse('scipost:login') + response = self.client.get(login_url) + self.assertEqual(response.status_code, 200) + + # Does posting work? + response = self.client.post( + login_url, follow=True, + data={ + 'username': self.user.username, + 'password': self.password, + 'next': '/', + 'code': '' + }) + self.assertEqual(response.context['user'], self.user) + self.assertEqual(response.redirect_chain[-1][0], '/') # Check if eventually redirected + self.assertEqual(response.redirect_chain[-1][1], 302) + + # Logout for next step + self.client.logout() + + # Check if a simple login without code fails if device is set up. + TOTPDeviceFactory.create(user=self.user, token=self.valid_secret_key) + response = self.client.post( + login_url, follow=True, + data={ + 'username': self.user.username, + 'password': self.password, + 'next': '/', + 'code': '' + }) + self.assertNotEqual(response.context['user'], self.user) + + # Check if login fails with invalid code + response = self.client.post( + login_url, follow=True, + data={ + 'username': self.user.username, + 'password': self.password, + 'next': '/', + 'code': '912334' + }) + self.assertNotEqual(response.context['user'], self.user) + response = self.client.post( + login_url, follow=True, + data={ + 'username': self.user.username, + 'password': self.password, + 'next': '/', + 'code': '000000' + }) + self.assertNotEqual(response.context['user'], self.user) + + # Check if login *WORKS* with a valid code. + response = self.client.post( + login_url, follow=True, + data={ + 'username': self.user.username, + 'password': self.password, + 'next': '/', + 'code': self.valid_token + }) + self.assertEqual(response.context['user'], self.user) diff --git a/scipost/totp.py b/scipost/totp.py index b224bfa6cc7d9fdcd70aec67d53a37df483679b4..3af23ed5fc3b409d951e3afcc806b2aa15c63599 100644 --- a/scipost/totp.py +++ b/scipost/totp.py @@ -1,10 +1,9 @@ __copyright__ = "Copyright © Stichting SciPost (SciPost Foundation)" __license__ = "AGPL v3" +import time import pyotp -from time import time - from .models import TOTPDevice @@ -36,7 +35,7 @@ class TOTPVerification: return False for device in self._user.devices.all(): - time_int = int(time()) + time_int = int(time.time()) totp = pyotp.TOTP( device.token, interval=self.token_validity_period, digits=self.number_of_digits) @@ -66,6 +65,6 @@ class TOTPVerification: except (ValueError, AssertionError): # return False, if token could not be converted to an integer return False - time_int = int(time()) + time_int = int(time.time()) totp = pyotp.TOTP(secret_key, interval=cls.token_validity_period, digits=cls.number_of_digits) return totp.verify(code, for_time=time_int, valid_window=cls.tolerance)