diff --git a/bookwyrm/forms/edit_user.py b/bookwyrm/forms/edit_user.py index d86aed31d..ce7bb6d07 100644 --- a/bookwyrm/forms/edit_user.py +++ b/bookwyrm/forms/edit_user.py @@ -4,8 +4,6 @@ from django.contrib.auth.password_validation import validate_password from django.core.exceptions import ValidationError from django.utils.translation import gettext_lazy as _ -import pyotp - from bookwyrm import models from bookwyrm.models.fields import ClearableFileInputWithWarning from .custom_form import CustomForm @@ -118,32 +116,3 @@ class ConfirmPasswordForm(CustomForm): if not self.instance.check_password(password): self.add_error("password", _("Incorrect Password")) - - -class Confirm2FAForm(CustomForm): - otp = forms.CharField(max_length=6, min_length=6, widget=forms.TextInput) - - # IDK if we need this? - class Meta: - model = models.User - fields = ["otp_secret"] - - def clean(self): - """Check otp matches""" - otp = self.data.get("otp") - totp = pyotp.TOTP(self.instance.otp_secret) - - if not totp.verify(otp): - # maybe it's a backup code? - hotp = pyotp.HOTP(self.instance.otp_secret) - hotp_count = ( - self.instance.hotp_count if self.instance.hotp_count is not None else 0 - ) - - if not hotp.verify(otp, hotp_count): - self.add_error("otp", _("Code does not match")) - - # TODO: backup codes - # increment the user hotp_count if it was an HOTP - # self.instance.hotp_count = hotp_count + 1 - # self.instance.save(broadcast=False, update_fields=["hotp_count"]) diff --git a/bookwyrm/forms/landing.py b/bookwyrm/forms/landing.py index a31e8a7c4..ba7f69bb6 100644 --- a/bookwyrm/forms/landing.py +++ b/bookwyrm/forms/landing.py @@ -4,6 +4,8 @@ from django.contrib.auth.password_validation import validate_password from django.core.exceptions import ValidationError from django.utils.translation import gettext_lazy as _ +import pyotp + from bookwyrm import models from .custom_form import CustomForm @@ -74,3 +76,31 @@ class PasswordResetForm(CustomForm): validate_password(new_password) except ValidationError as err: self.add_error("password", err) + + +class Confirm2FAForm(CustomForm): + otp = forms.CharField(max_length=6, min_length=6, widget=forms.TextInput) + + class Meta: + model = models.User + fields = ["otp_secret", "hotp_count"] + + def clean_otp(self): + """Check otp matches""" + otp = self.data.get("otp") + totp = pyotp.TOTP(self.instance.otp_secret) + + if not totp.verify(otp): + # maybe it's a backup code? + hotp = pyotp.HOTP(self.instance.otp_secret) + hotp_count = ( + self.instance.hotp_count if self.instance.hotp_count is not None else 0 + ) + + if not hotp.verify(otp, hotp_count): + self.add_error("otp", _("Code does not match")) + + # TODO: backup codes + # increment the user hotp_count if it was an HOTP + # self.instance.hotp_count = hotp_count + 1 + # self.instance.save(broadcast=False, update_fields=["hotp_count"]) diff --git a/bookwyrm/templates/two_factor_auth/two_factor_login.html b/bookwyrm/templates/two_factor_auth/two_factor_login.html index b110b82d5..f4a2fdf04 100644 --- a/bookwyrm/templates/two_factor_auth/two_factor_login.html +++ b/bookwyrm/templates/two_factor_auth/two_factor_login.html @@ -30,23 +30,14 @@ {% endblock %} - {% if error %} -
- - - {{ error }} - -
- {% endif %}
{% csrf_token %}
{{ form.otp }} - {% include 'snippets/form_errors.html' with errors_list=form.current_password.errors id="desc_current_password" %} + {% include 'snippets/form_errors.html' with errors_list=form.otp.errors id="desc_otp" %}
-
diff --git a/bookwyrm/views/landing/login.py b/bookwyrm/views/landing/login.py index 364b721d2..b9a877687 100644 --- a/bookwyrm/views/landing/login.py +++ b/bookwyrm/views/landing/login.py @@ -1,4 +1,5 @@ """ class views for login/register views """ +from datetime import datetime from django.contrib.auth import authenticate, login, logout from django.contrib.auth.decorators import login_required from django.shortcuts import redirect @@ -29,7 +30,7 @@ class Login(View): } return TemplateResponse(request, "landing/login.html", data) - #pylint: disable=too-many-return-statements + # pylint: disable=too-many-return-statements @sensitive_variables("password") @method_decorator(sensitive_post_parameters("password")) def post(self, request): @@ -54,12 +55,9 @@ class Login(View): if user is not None: # if 2fa is set, don't log them in until they enter the right code if user.two_factor_auth is True: - form = forms.Confirm2FAForm(request.GET, user) - return TemplateResponse( - request, - "two_factor_auth/two_factor_login.html", - {"form": form, "2fa_user": user}, - ) + request.session["2fa_user"] = user.username + request.session["2fa_auth_time"] = datetime.now() + return redirect("login-with-2fa") # otherwise, successful login login(request, user) diff --git a/bookwyrm/views/preferences/two_factor_auth.py b/bookwyrm/views/preferences/two_factor_auth.py index 26cb87f21..2a1656092 100644 --- a/bookwyrm/views/preferences/two_factor_auth.py +++ b/bookwyrm/views/preferences/two_factor_auth.py @@ -1,4 +1,5 @@ """ class views for 2FA management """ +from datetime import datetime, timedelta import time import pyotp import qrcode @@ -94,17 +95,31 @@ class Disable2FA(View): class LoginWith2FA(View): """Check 2FA code matches before allowing login""" + def get(self, request): + """Display 2FA form""" + + data = {"form": forms.Confirm2FAForm()} + return TemplateResponse(request, "two_factor_auth/two_factor_login.html", data) + def post(self, request): """Check 2FA code and allow/disallow login""" - user = models.User.objects.get(username=request.POST.get("2fa_user")) + user = models.User.objects.get(username=request.session["2fa_user"]) + elapsed_time = datetime.now() - request.session["2fa_auth_time"] form = forms.Confirm2FAForm(request.POST, instance=user) + # don't allow the login credentials to last too long before completing login + if elapsed_time > timedelta(seconds=60): + request.session["2fa_user"] = None + request.session["2fa_auth_time"] = 0 + return redirect("/") if not form.is_valid(): - time.sleep(2) # make life slightly harder for bots - data = { - "form": form, - "2fa_user": user, - "error": "Code does not match, try again", - } + # make life harder for bots + # humans are unlikely to get it wrong more than twice + if not request.session["2fa_attempts"]: + request.session["2fa_attempts"] = 0 + request.session["2fa_attempts"] = request.session["2fa_attempts"] + 1 + time.sleep(2 ** request.session["2fa_attempts"]) + + data = {"form": form, "2fa_user": user} return TemplateResponse( request, "two_factor_auth/two_factor_login.html", data )