Make some follow-up adjustments to 2FA

Nothing too significant in here, just a few adjustments and other
follow-ups that I wanted to do:

* Make backup code usage a bit more lenient - allow uppercase, and
  doesn't need exact spacing as originally displayed.
* Display the backup codes a little more nicely.
* Change the message on the settings page based on whether 2FA is
  enabled or not.
* Use webargs instead of request.params.get

tildes/tildes/lib/string.py

@@ -188,3 +188,20 @@ def _sanitize_characters(original: str) -> str:
             final_characters.append(char)
 
     return "".join(final_characters)
+
+
+def separate_string(original: str, separator: str, segment_size: int) -> str:
+    """Separate a string into "segments", inserting a separator every X chars.
+
+    This is useful for strings being used as "codes" such as invite codes and 2FA backup
+    codes, so that they can be displayed in a more easily-readable format.
+    """
+    separated = ""
+
+    for count, char in enumerate(original):
+        if count > 0 and count % segment_size == 0:
+            separated += separator
+
+        separated += char
+
+    return separated

tildes/tildes/models/user/user.py

@@ -4,7 +4,7 @@ from datetime import datetime
 from typing import Any, List, Optional, Sequence, Tuple
 
 from mypy_extensions import NoReturn
-import pyotp
+from pyotp import TOTP
 from pyramid.security import (
     ALL_PERMISSIONS,
     Allow,
@@ -167,12 +167,13 @@ class User(DatabaseModel):
 
     def is_correct_two_factor_code(self, code: str) -> bool:
         """Verify that a TOTP/backup code is correct."""
-        totp = pyotp.TOTP(self.two_factor_secret)
-        code = code.strip()
+        totp = TOTP(self.two_factor_secret)
 
-        if totp.verify(code.replace(" ", "")):
+        code = code.strip().replace(" ", "").lower()
+
+        if totp.verify(code):
             return True
-        elif code in self.two_factor_backup_codes:
+        elif self.two_factor_backup_codes and code in self.two_factor_backup_codes:
             # Need to set the attribute so SQLAlchemy knows it changed
             self.two_factor_backup_codes = [
                 backup_code

tildes/tildes/models/user/user_invite_code.py

@@ -7,6 +7,7 @@ import string
 from sqlalchemy import CheckConstraint, Column, ForeignKey, Integer, Text, TIMESTAMP
 from sqlalchemy.sql.expression import text
 
+from tildes.lib.string import separate_string
 from tildes.models import DatabaseModel
 from .user import User
 
@@ -36,16 +37,7 @@ class UserInviteCode(DatabaseModel):
 
     def __str__(self) -> str:
         """Format the code into a more easily readable version."""
-        formatted = ""
-
-        for count, char in enumerate(self.code):
-            # add a dash every 5 chars
-            if count > 0 and count % 5 == 0:
-                formatted += "-"
-
-            formatted += char.upper()
-
-        return formatted
+        return separate_string(self.code, "-", 5)
 
     def __init__(self, user: User) -> None:
         """Create a new (random) invite code owned by the user.

tildes/tildes/templates/intercooler/two_factor_enabled.jinja2

@@ -1,10 +1,11 @@
 <p>Congratulations! Two-factor authentication has been enabled.</p>
 
-<p>These are your backup codes. <strong class="text-warning">Make sure to write them down and store them in a safe place.</strong>
-In the event that you lose access to your authenticator device, you will need these to regain access to your account. Each code can only be used once.</p>
+<p>These are your backup codes. In the event that you lose access to your authenticator device, you will need one of these codes to regain access to your account (or disable two-factor authentication). Each code can only be used once.</p>
 
-<code>
+<p><strong class="text-warning">Make sure to write them down and store them in a safe place.</strong></p>
+
+<ol>
 {% for code in backup_codes %}
-	- {{ code }}<br />
+  <li><code>{{ code }}</code></li>
 {% endfor %}
-</code>
+</ol>

tildes/tildes/templates/settings.jinja2

@@ -96,8 +96,13 @@
       <div class="text-small text-secondary">To be able to regain access in case of lost password, compromise, etc.</div>
     </li>
     <li>
-      <a href="/settings/two_factor">Set up two-factor authentication</a>
+      {% if not request.user.two_factor_enabled %}
+        <a href="/settings/two_factor">Enable two-factor authentication</a>
         <div class="text-small text-secondary">For extra security, you can enable two-factor authentication.</div>
+      {% else %}
+        <a href="/settings/two_factor">Disable two-factor authentication</a>
+        <div class="text-small text-secondary">Disabling two-factor authentication requires a code from your device or a backup code.</div>
+      {% endif %}
     </li>
     <li>
       <a href="/settings/comment_visits">Toggle marking new comments (currently {{ 'enabled' if request.user.track_comment_visits else 'disabled' }})</a>

tildes/tildes/templates/settings_two_factor.jinja2

@@ -42,7 +42,7 @@
 >
   <div class="form-group">
     <label class="form-label" for="code">Code</label>
-    <input class="form-input" id="code" name="code" type="text" placeholder="Code" maxlength="6" pattern="[0-9]{6}">
+    <input class="form-input" id="code" name="code" type="text" placeholder="Code">
   </div>
 
   <div class="form-buttons">

tildes/tildes/views/api/web/user.py

@@ -6,7 +6,6 @@ from typing import Optional
 
 from marshmallow import ValidationError
 from marshmallow.fields import String
-import pyotp
 from pyramid.httpexceptions import (
     HTTPForbidden,
     HTTPUnauthorized,
@@ -18,6 +17,7 @@ from sqlalchemy.exc import IntegrityError
 from webargs.pyramidparser import use_kwargs
 
 from tildes.enums import LogEventType, TopicSortOption
+from tildes.lib.string import separate_string
 from tildes.models.log import Log
 from tildes.models.user import User, UserInviteCode
 from tildes.schemas.fields import Enum, ShortTimePeriod
@@ -91,17 +91,6 @@ def patch_change_email_address(
     return Response("Your email address has been updated")
 
 
-def generate_backup_code() -> str:
-    """Generate a user-friendly (easy to read) backup code for 2FA."""
-    parts = []
-    # Generate 4 four-length random strings.
-    for _ in range(4):
-        parts.append("".join(random.choices(string.ascii_lowercase, k=4)))
-
-    # Combine parts to make one, e.g. xxxx xxxx xxxx xxxx
-    return " ".join(parts)
-
-
 @ic_view_config(
     route_name="user",
     request_method="POST",
@@ -109,20 +98,25 @@ def generate_backup_code() -> str:
     renderer="two_factor_enabled.jinja2",
     permission="change_two_factor",
 )
-def post_enable_two_factor(request: Request) -> dict:
+@use_kwargs({"code": String()})
+def post_enable_two_factor(request: Request, code: str) -> dict:
     """Enable two-factor authentication for the user."""
     user = request.context
-    totp = pyotp.TOTP(user.two_factor_secret)
-    code = str(request.params.get("code"))
 
-    if not totp.verify(code):
+    if not user.is_correct_two_factor_code(code):
         raise HTTPUnprocessableEntity("Invalid code, please try again.")
 
-    # Generate 10 backup codes.
-    backup_codes = [generate_backup_code() for _ in range(10)]
-
     request.user.two_factor_enabled = True
-    request.user.two_factor_backup_codes = backup_codes
+
+    # Generate 10 backup codes (16 lowercase letters each)
+    request.user.two_factor_backup_codes = [
+        "".join(random.choices(string.ascii_lowercase, k=16)) for _ in range(10)
+    ]
+
+    # format the backup codes to be easier to read for output
+    backup_codes = [
+        separate_string(code, " ", 4) for code in request.user.two_factor_backup_codes
+    ]
 
     return {"backup_codes": backup_codes}
 
@@ -134,10 +128,9 @@ def post_enable_two_factor(request: Request) -> dict:
     renderer="two_factor_disabled.jinja2",
     permission="change_two_factor",
 )
-def post_disable_two_factor(request: Request) -> Response:
+@use_kwargs({"code": String()})
+def post_disable_two_factor(request: Request, code: str) -> Response:
     """Disable two-factor authentication for the user."""
-    code = str(request.params.get("code"))
-
     if not request.user.is_correct_two_factor_code(code):
         raise HTTPUnauthorized(body="Invalid code")
 

tildes/tildes/views/login.py

@@ -1,5 +1,6 @@
 """Views related to logging in/out."""
 
+from marshmallow.fields import String
 from pyramid.httpexceptions import HTTPFound, HTTPUnauthorized, HTTPUnprocessableEntity
 from pyramid.renderers import render_to_response
 from pyramid.request import Request
@@ -96,7 +97,8 @@ def post_login(request: Request, username: str, password: str) -> HTTPFound:
 )
 @not_logged_in
 @rate_limit_view("login_two_factor")
-def post_login_two_factor(request: Request) -> Response:
+@use_kwargs({"code": String()})
+def post_login_two_factor(request: Request, code: str) -> Response:
     """Process a log in request with 2FA."""
     # Look up the user for the supplied username
     user = (
@@ -105,7 +107,6 @@ def post_login_two_factor(request: Request) -> Response:
         .filter(User.username == request.session["two_factor_username"])
         .one_or_none()
     )
-    code = str(request.params.get("code"))
 
     if user.is_correct_two_factor_code(code):
         del request.session["two_factor_username"]

tildes/tildes/views/settings.py

@@ -1,6 +1,7 @@
 """Views related to user settings."""
 
 from io import BytesIO
+
 import pyotp
 from pyramid.httpexceptions import HTTPForbidden, HTTPUnprocessableEntity
 from pyramid.request import Request