diff --git a/scipost/forms.py b/scipost/forms.py index 66ed253ad417092b61d298f825a46c326b30de2b..23094e18e6c17dbdf9c423f3231f214f9a41ebf3 100644 --- a/scipost/forms.py +++ b/scipost/forms.py @@ -303,8 +303,8 @@ class SciPostAuthenticationForm(AuthenticationForm): - confirm_login_allowed: disallow inactive or unvetted accounts. """ next = forms.CharField(widget=forms.HiddenInput(), required=False) - token = forms.CharField( - required=False, + code = forms.CharField( + required=False, widget=forms.TextInput(attrs={'autocomplete': 'off'}), help_text="Please type in the code displayed on your authenticator app from your device") def confirm_login_allowed(self, user): @@ -324,13 +324,13 @@ class SciPostAuthenticationForm(AuthenticationForm): code='unvetted', ) if user.devices.exists(): - if self.cleaned_data.get('token'): - token = self.cleaned_data.get('token') + if self.cleaned_data.get('code'): + code = self.cleaned_data.get('code') totp = TOTPVerification(user) - if not totp.verify_token(token): - self.add_error('token', 'Invalid token') + if not totp.verify_code(code): + self.add_error('code', 'Invalid code') else: - self.add_error('token', 'Your account uses two factor authentication') + self.add_error('code', 'Your account uses two factor authentication') class UserAuthInfoForm(forms.Form): @@ -345,25 +345,48 @@ class UserAuthInfoForm(forms.Form): } -class TOTPDeviceForm(forms.Form): - token = forms.CharField() - key = forms.CharField(widget=forms.HiddenInput(), required=True) +class TOTPDeviceForm(forms.ModelForm): + code = forms.CharField( + required=True, + help_text=( + 'Enter the security code generated by your mobile authenticator' + ' app to make sure it’s configured correctly.')) + + class Meta: + model = TOTPDevice + fields = ['name', 'token'] + widgets = {'token': forms.HiddenInput()} + labels = {'name': 'Device name'} def __init__(self, *args, **kwargs): self.current_user = kwargs.pop('current_user') super().__init__(*args, **kwargs) - self.initial['key'] = 'JBSWY3DPEHPK3PXP' + self.initial['token'] = pyotp.random_base32() @property def secret_key(self): - if hasattr(self, 'cleaned_data') and 'key' in self.cleaned_data: - return self.cleaned_data.get('key') - return self.initial['key'] + if hasattr(self, 'cleaned_data') and 'token' in self.cleaned_data: + return self.cleaned_data.get('token') + return self.initial['token'] def get_QR_data(self): return pyotp.totp.TOTP(self.secret_key).provisioning_uri( self.current_user.email, issuer_name="SciPost") + def clean(self): + cleaned_data = self.cleaned_data + code = cleaned_data.get('code') + token = cleaned_data.get('token') + if not TOTPVerification.verify_token(token, code): + self.add_error('code', 'Invalid code, please try again.') + return cleaned_data + + def save(self): + totp_device = super().save(commit=False) + totp_device.user = self.current_user + totp_device.save() + return totp_device + AUTHORSHIP_CLAIM_CHOICES = ( ('-', '-'), diff --git a/scipost/templates/partials/scipost/personal_page/account.html b/scipost/templates/partials/scipost/personal_page/account.html index 27a9a7fabf5cadf15ee3a4d07e99d841f686dcba..8f04fd3c4ff35586ca532fc82370e1f8b11bccf5 100644 --- a/scipost/templates/partials/scipost/personal_page/account.html +++ b/scipost/templates/partials/scipost/personal_page/account.html @@ -12,6 +12,7 @@ {% is_registered_contributor request.user as is_registered_contributor %} {% is_tester request.user as is_tester %} {% is_production_officer request.user as is_production_officer %} + {% recommend_new_totp_device request.user as recommend_totp %} <div class="row"> <div class="col-12"> @@ -54,6 +55,21 @@ {# END: Scientist fields #} {% endif %} + {% if recommend_totp %} + {# Scientist fields #} + {% if 1 %} + <div class="border border-danger p-2 mb-3"> + <h3 class="text-warningx"> + <i class="fa fa-exclamation-triangle text-danger"></i> + Please increase your account's security</h3> + <p class="mb-0"> + We strongly recommend to use two factor authentication that adds an extra layer of protection to your SciPost account. + </p> + </div> + {% endif %} + {# END: Scientist fields #} + {% endif %} + {% if not contributor.petition_signatories.exists %} <div class="border border-danger p-2"> <h3 class="text-danger">Scientists, please help us out!</h3> diff --git a/scipost/templates/scipost/login.html b/scipost/templates/scipost/login.html index de6705c4120f0df11cb7c7c966c5c2810534169a..8118660c90b6b141aef125462b0f9c96245a93b6 100644 --- a/scipost/templates/scipost/login.html +++ b/scipost/templates/scipost/login.html @@ -34,7 +34,7 @@ <script type="text/javascript"> $(function() { - $('[name="token"]').parents('.form-group').hide(); // Just to prevent having annoying animations. + $('[name="code"]').parents('.form-group').hide(); // Just to prevent having annoying animations. $('form [name="username"]').on('change', function() { $.ajax({ type: 'POST', @@ -47,9 +47,9 @@ processData: true, success: function(data) { if (data.has_totp) { - $('[name="token"]').parents('.form-group').show(); + $('[name="code"]').parents('.form-group').show(); } else { - $('[name="token"]').parents('.form-group').hide(); + $('[name="code"]').parents('.form-group').hide(); } } }); diff --git a/scipost/templates/scipost/totpdevice_form.html b/scipost/templates/scipost/totpdevice_form.html index 4c1fb3ff3cf00cd73afded04eb91f951277d85e1..9499e34fd6e22de6067deb25e3f713f70748c2ee 100644 --- a/scipost/templates/scipost/totpdevice_form.html +++ b/scipost/templates/scipost/totpdevice_form.html @@ -24,25 +24,20 @@ </p> <ul> <li>Add a new time-based token.</li> - <li>Use your app to scan the barcode below, or enter your secret key manually.</li> + <li>Use your app to scan the barcode below, or <a href="javascript:;" data-toggle="toggle" data-target="#secret-key">enter your secret key manually</a>.</li> </ul> <form method="post"> {% csrf_token %} + <div class="text-center"> + <img id="qr" data-toggle="qr" data-qr-value="{{ form.get_QR_data }}"> + <h3 class="p-3" id="secret-key" style="display: none;"><code>{{ form.secret_key }}</code></h3> + </div> <p> Enter the security code generated by your mobile authenticator app to make sure it’s configured correctly. </p> - <img id="qr" data-toggle="qr" data-qr-value="{{ form.get_QR_data }}"> - <!-- <script> - (function() { - var qr = new QRious({ - element: document.getElementById('qr'), - value: 'https://github.com/neocotic/qrious' - }); - })(); - </script> --> {{ form|bootstrap }} - <input type="submit" class="btn btn-primary" value="Check" /> + <input type="submit" class="btn btn-primary" value="Add device" /> </form> </div> diff --git a/scipost/templates/scipost/totpdevice_list.html b/scipost/templates/scipost/totpdevice_list.html index 61cc4f55437e6dbb18f4079965b62ef3968b236d..8481e601424302563489ae38eee1d0c9955b3efc 100644 --- a/scipost/templates/scipost/totpdevice_list.html +++ b/scipost/templates/scipost/totpdevice_list.html @@ -14,9 +14,18 @@ <div class="row"> <div class="col-md-12"> <h1 class="highlight">Two factor authentication</h1> - <a href="{% url 'scipost:totp_create' %}">Set up a new two factor authentication device</a> - - <h3 class="mt-4">Your devices</h3> + <p> + We strongly recommend to use two factor authentication that adds an extra layer of protection to your SciPost account. + You will need a mobile device capable or running a mobile authentication application, for example: + <ul> + <li><a href="http://support.google.com/accounts/bin/answer.py?hl=en&answer=1066447" target="_blank">Google Authenticator</a> (Android/iOS)</li> + <li><a href="http://guide.duosecurity.com/third-party-accounts" target="_blank">Duo Mobile</a> (Android/iOS)</li> + <li><a href="http://aka.ms/dbauthenticator" target="_blank">Authenticator</a> (Windows Phone 7)</li> + </ul> + <a href="{% url 'scipost:totp_create' %}">Set up a new two factor authentication device</a> + </p> + + <h3 class="mt-5 mb-3">Your devices</h3> <table class="table"> <thead> <tr> diff --git a/scipost/templatetags/user_groups.py b/scipost/templatetags/user_groups.py index 1d97a093daa42bd067012b6ed92f6a2cf6c2ff4d..a8ce8e6d7089e1cce8d394b4e7152cbe062d7e2c 100644 --- a/scipost/templatetags/user_groups.py +++ b/scipost/templatetags/user_groups.py @@ -124,3 +124,24 @@ def is_editor_in_charge(user, submission): return False return submission.editor_in_charge == user.contributor + + +@register.simple_tag +def recommend_new_totp_device(user): + """ + Check if User has no TOTPDevice, but still has a high level of information access. + """ + if user.devices.exists(): + return False + if user.is_superuser: + return True + if user.contributor.fellowships.exists(): + return True + return user.groups.filter(name__in=[ + 'Editorial Administrators', + 'SciPost Administrators', + 'Advisory Board', + 'Financial Administrators', + 'Vetting Editors', + 'Editorial College', + ]).exists() diff --git a/scipost/totp.py b/scipost/totp.py index 083d813aa90fb83ad9d9426f8b50ae52af9feccf..7b6f1cd37c91b9b81a519a36487d39b4276ee9e3 100644 --- a/scipost/totp.py +++ b/scipost/totp.py @@ -9,6 +9,9 @@ from .models import TOTPDevice class TOTPVerification: + number_of_digits = 6 + token_validity_period = 30 + tolerance = 2 # Gives a 2 minute window to use a code. def __init__(self, user): """ @@ -16,15 +19,13 @@ class TOTPVerification: """ self._user = user - # Next token must be generated at a higher counter value. - self.number_of_digits = 6 - self.token_validity_period = 30 - self.tolerance = 2 # Gives a 2 minute window to use a code. - - def verify_token(self, token, tolerance=0): + def verify_code(self, code): + """ + Verify a time-dependent code for a certain User. + """ try: # Convert the input token to integer - token = int(token) + code = int(code) except ValueError: # return False, if token could not be converted to an integer return False @@ -40,7 +41,7 @@ class TOTPVerification: # 1. Check if the current counter is higher than the value of last verified counter # 2. Check if entered token is correct - valid_token = totp.verify(token, for_time=time_int, valid_window=self.tolerance) + valid_token = totp.verify(code, for_time=time_int, valid_window=self.tolerance) if not valid_token: # Token not valid continue @@ -50,3 +51,18 @@ class TOTPVerification: TOTPDevice.objects.filter(id=device.id).update(last_verified_counter=time_int) return True return False + + @classmethod + def verify_token(cls, secret_key, code): + """ + Independently verify a secret_key/code combination at current time. + """ + try: + # Convert the input token to integer + code = int(code) + except ValueError: + # return False, if token could not be converted to an integer + return False + time_int = int(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) diff --git a/scipost/views.py b/scipost/views.py index a06c85161a9e51e313d8eb57df5e0aa99eb399aa..091e1a41648db28bef730b53e29ab138c7130856 100644 --- a/scipost/views.py +++ b/scipost/views.py @@ -10,6 +10,7 @@ from django.conf import settings from django.contrib import messages from django.contrib.auth import login, update_session_auth_hash from django.contrib.auth.decorators import login_required +from django.contrib.auth.mixins import LoginRequiredMixin from django.contrib.auth.models import Group from django.contrib.auth.views import password_reset, password_reset_confirm from django.contrib.auth.views import ( @@ -29,7 +30,7 @@ from django.utils.http import is_safe_url from django.views.debug import cleanse_setting from django.views.decorators.cache import never_cache from django.views.decorators.http import require_POST -from django.views.generic.edit import FormView, DeleteView +from django.views.generic.edit import DeleteView, CreateView from django.views.generic.list import ListView from django.views.static import serve @@ -881,12 +882,18 @@ def update_personal_data(request): return _update_personal_data_user_only(request) -class TOTPListView(ListView): +class TOTPListView(LoginRequiredMixin, ListView): + """ + List all TOTP devices for logged in User. + """ def get_queryset(self): return self.request.user.devices.all() -class TOTPDeviceCreateView(FormView): +class TOTPDeviceCreateView(LoginRequiredMixin, CreateView): + """ + Create a new TOTP device. + """ form_class = TOTPDeviceForm template_name = 'scipost/totpdevice_form.html' success_url = reverse_lazy('scipost:totp') @@ -897,7 +904,10 @@ class TOTPDeviceCreateView(FormView): return kwargs -class TOTPDeviceDeleteView(DeleteView): +class TOTPDeviceDeleteView(LoginRequiredMixin, DeleteView): + """ + Confirm deletion of a TOTP device. + """ pk_url_kwarg = 'device_id' success_url = reverse_lazy('scipost:totp')