From a588431feec331033082130344853252e6613836 Mon Sep 17 00:00:00 2001 From: Oden Date: Sun, 29 Jul 2018 22:13:50 -0700 Subject: [PATCH] Add two-factor authentication Adds optional two-factor authentication support using TOTP, and including backup codes in case of a lost 2FA device. --- ...332481a6e_add_two_factor_authentication.py | 38 +++++++++++ tildes/requirements-to-freeze.txt | 3 + tildes/requirements.txt | 3 + tildes/tildes/lib/ratelimit.py | 1 + tildes/tildes/models/user/user.py | 24 ++++++- tildes/tildes/routes.py | 9 +++ .../intercooler/login_two_factor.jinja2 | 17 +++++ .../intercooler/two_factor_disabled.jinja2 | 3 + .../intercooler/two_factor_enabled.jinja2 | 10 +++ tildes/tildes/templates/login.jinja2 | 2 +- tildes/tildes/templates/settings.jinja2 | 4 ++ .../templates/settings_two_factor.jinja2 | 53 +++++++++++++++ tildes/tildes/views/api/web/user.py | 66 +++++++++++++++++- tildes/tildes/views/login.py | 67 +++++++++++++++---- tildes/tildes/views/settings.py | 33 ++++++++- 15 files changed, 317 insertions(+), 16 deletions(-) create mode 100644 tildes/alembic/versions/67e332481a6e_add_two_factor_authentication.py create mode 100644 tildes/tildes/templates/intercooler/login_two_factor.jinja2 create mode 100644 tildes/tildes/templates/intercooler/two_factor_disabled.jinja2 create mode 100644 tildes/tildes/templates/intercooler/two_factor_enabled.jinja2 create mode 100644 tildes/tildes/templates/settings_two_factor.jinja2 diff --git a/tildes/alembic/versions/67e332481a6e_add_two_factor_authentication.py b/tildes/alembic/versions/67e332481a6e_add_two_factor_authentication.py new file mode 100644 index 0000000..c316755 --- /dev/null +++ b/tildes/alembic/versions/67e332481a6e_add_two_factor_authentication.py @@ -0,0 +1,38 @@ +"""Add two-factor authentication + +Revision ID: 67e332481a6e +Revises: fab922a8bb04 +Create Date: 2018-07-31 02:53:50.182862 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = "67e332481a6e" +down_revision = "fab922a8bb04" +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column( + "users", + sa.Column( + "two_factor_backup_codes", postgresql.ARRAY(sa.Text()), nullable=True + ), + ) + op.add_column( + "users", + sa.Column( + "two_factor_enabled", sa.Boolean(), server_default="false", nullable=False + ), + ) + op.add_column("users", sa.Column("two_factor_secret", sa.Text(), nullable=True)) + + +def downgrade(): + op.drop_column("users", "two_factor_secret") + op.drop_column("users", "two_factor_enabled") + op.drop_column("users", "two_factor_backup_codes") diff --git a/tildes/requirements-to-freeze.txt b/tildes/requirements-to-freeze.txt index 69c181d..39cc8a1 100644 --- a/tildes/requirements-to-freeze.txt +++ b/tildes/requirements-to-freeze.txt @@ -13,6 +13,7 @@ html5lib ipython mypy mypy-extensions +Pillow prometheus-client psycopg2 publicsuffix2 @@ -20,6 +21,7 @@ pydocstyle pylama pylama-pylint pylint==1.7.5 # pylama has issues with 1.8.1 +pyotp pyramid pyramid-debugtoolbar pyramid-ipython @@ -30,6 +32,7 @@ pyramid-webassets pytest pytest-mock PyYAML # needs to be installed separately for webassets +qrcode SQLAlchemy SQLAlchemy-Utils stripe diff --git a/tildes/requirements.txt b/tildes/requirements.txt index 2047941..aacc564 100644 --- a/tildes/requirements.txt +++ b/tildes/requirements.txt @@ -38,6 +38,7 @@ parso==0.3.1 PasteDeploy==1.5.2 pexpect==4.6.0 pickleshare==0.7.4 +Pillow==5.2.0 plaster==1.0 plaster-pastedeploy==0.6 pluggy==0.7.1 @@ -55,6 +56,7 @@ Pygments==2.2.0 pylama==7.4.3 pylama-pylint==3.0.1 pylint==1.7.5 +pyotp==2.2.6 pyramid==1.9.2 pyramid-debugtoolbar==4.4 pyramid-ipython==0.2 @@ -68,6 +70,7 @@ pytest-mock==1.10.0 python-dateutil==2.7.3 python-editor==1.0.3 PyYAML==3.13 +qrcode==6.0 redis==2.10.6 repoze.lru==0.7 requests==2.19.1 diff --git a/tildes/tildes/lib/ratelimit.py b/tildes/tildes/lib/ratelimit.py index 74a6e4b..96661b7 100644 --- a/tildes/tildes/lib/ratelimit.py +++ b/tildes/tildes/lib/ratelimit.py @@ -279,6 +279,7 @@ class RateLimitedAction: # each action must have a unique name to prevent key collisions _RATE_LIMITED_ACTIONS = ( RateLimitedAction("login", timedelta(hours=1), 20), + RateLimitedAction("login_two_factor", timedelta(hours=1), 20), RateLimitedAction("register", timedelta(hours=1), 50), ) diff --git a/tildes/tildes/models/user/user.py b/tildes/tildes/models/user/user.py index a69fa2f..36099e5 100644 --- a/tildes/tildes/models/user/user.py +++ b/tildes/tildes/models/user/user.py @@ -4,6 +4,7 @@ from datetime import datetime from typing import Any, List, Optional, Sequence, Tuple from mypy_extensions import NoReturn +import pyotp from pyramid.security import ( ALL_PERMISSIONS, Allow, @@ -21,7 +22,7 @@ from sqlalchemy import ( Text, TIMESTAMP, ) -from sqlalchemy.dialects.postgresql import ENUM +from sqlalchemy.dialects.postgresql import ARRAY, ENUM from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import deferred from sqlalchemy.sql.expression import text @@ -62,6 +63,9 @@ class User(DatabaseModel): ), ) ) + two_factor_enabled: bool = Column(Boolean, nullable=False, server_default="false") + two_factor_secret: Optional[str] = deferred(Column(Text)) + two_factor_backup_codes: List[str] = deferred(Column(ARRAY(Text))) created_time: datetime = Column( TIMESTAMP(timezone=True), nullable=False, @@ -161,6 +165,24 @@ class User(DatabaseModel): # disable mypy on this line because it doesn't handle setters correctly self.password = new_password # type: ignore + 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() + + if totp.verify(code.replace(" ", "")): + return True + elif code in self.two_factor_backup_codes: + # Need to set the attribute so SQLAlchemy knows it changed + self.two_factor_backup_codes = [ + backup_code + for backup_code in self.two_factor_backup_codes + if backup_code != code + ] + return True + + return False + @property def email_address(self) -> NoReturn: """Return an error since reading the email address isn't possible.""" diff --git a/tildes/tildes/routes.py b/tildes/tildes/routes.py index dc977ec..82c1a47 100644 --- a/tildes/tildes/routes.py +++ b/tildes/tildes/routes.py @@ -20,6 +20,7 @@ def includeme(config: Configurator) -> None: config.add_route("groups", "/groups") config.add_route("login", "/login") + config.add_route("login_two_factor", "/login_two_factor") config.add_route("logout", "/logout", factory=LoggedInFactory) config.add_route("register", "/register") @@ -61,6 +62,14 @@ def includeme(config: Configurator) -> None: "/settings/account_recovery", factory=LoggedInFactory, ) + config.add_route( + "settings_two_factor", "/settings/two_factor", factory=LoggedInFactory + ) + config.add_route( + "settings_two_factor_qr_code", + "/settings/two_factor/qr_code", + factory=LoggedInFactory, + ) config.add_route( "settings_comment_visits", "/settings/comment_visits", factory=LoggedInFactory ) diff --git a/tildes/tildes/templates/intercooler/login_two_factor.jinja2 b/tildes/tildes/templates/intercooler/login_two_factor.jinja2 new file mode 100644 index 0000000..21afdb4 --- /dev/null +++ b/tildes/tildes/templates/intercooler/login_two_factor.jinja2 @@ -0,0 +1,17 @@ +

Two-factor authentication is enabled on this account. Please enter the code from your authenticator app below. If you do not have access to your authenticator device, enter a backup code.

+ +
+ + {% if keep %} + + {% endif %} + +
+ + +
+ +
+ +
+
diff --git a/tildes/tildes/templates/intercooler/two_factor_disabled.jinja2 b/tildes/tildes/templates/intercooler/two_factor_disabled.jinja2 new file mode 100644 index 0000000..a47c8b3 --- /dev/null +++ b/tildes/tildes/templates/intercooler/two_factor_disabled.jinja2 @@ -0,0 +1,3 @@ +

Two-factor authentication has been disabled. You will no longer need a code when logging in.

+ +

Keep in mind: if you ever reenable two-factor authentication, your previous backup codes will not be valid.

\ No newline at end of file diff --git a/tildes/tildes/templates/intercooler/two_factor_enabled.jinja2 b/tildes/tildes/templates/intercooler/two_factor_enabled.jinja2 new file mode 100644 index 0000000..2574d8a --- /dev/null +++ b/tildes/tildes/templates/intercooler/two_factor_enabled.jinja2 @@ -0,0 +1,10 @@ +

Congratulations! Two-factor authentication has been enabled.

+ +

These are your backup codes. Make sure to write them down and store them in a safe place. +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.

+ + +{% for code in backup_codes %} + - {{ code }}
+{% endfor %} + \ No newline at end of file diff --git a/tildes/tildes/templates/login.jinja2 b/tildes/tildes/templates/login.jinja2 index 81027fe..2eaa99c 100644 --- a/tildes/tildes/templates/login.jinja2 +++ b/tildes/tildes/templates/login.jinja2 @@ -5,7 +5,7 @@ {% block main_heading %}Log in{% endblock %} {% block content %} -
+
diff --git a/tildes/tildes/templates/settings.jinja2 b/tildes/tildes/templates/settings.jinja2 index bd16d7e..5b86e88 100644 --- a/tildes/tildes/templates/settings.jinja2 +++ b/tildes/tildes/templates/settings.jinja2 @@ -95,6 +95,10 @@ Set up account recovery
To be able to regain access in case of lost password, compromise, etc.
+
  • + Set up two-factor authentication +
    For extra security, you can enable two-factor authentication.
    +
  • Toggle marking new comments (currently {{ 'enabled' if request.user.track_comment_visits else 'disabled' }})
    Marks new comments in topics since your last visit, and which topics have any
    diff --git a/tildes/tildes/templates/settings_two_factor.jinja2 b/tildes/tildes/templates/settings_two_factor.jinja2 new file mode 100644 index 0000000..1d8dc2b --- /dev/null +++ b/tildes/tildes/templates/settings_two_factor.jinja2 @@ -0,0 +1,53 @@ +{% extends 'base_no_sidebar.jinja2' %} + +{% block title %}Set up two-factor authentication{% endblock %} + +{% block main_heading %}Set up two-factor authentication{% endblock %} + +{% block content %} +{% if request.user.two_factor_enabled %} +

    You already have two-factor authentication enabled. To disable it, enter a code from your authenticator device below and click the button. If you do not have access to your authenticator device, enter a backup code.

    + + +
    + + +
    + +
    + +
    +
    • +{% else %} +

    To get started, you'll need to install an app such as Google Authenticator, Authy, FreeOTP, or any app that supports TOTP.

    + +

    Next, scan the below QR code with the app of your choice.

    + + + +

    Lastly, enter the 6-digit code displayed in the app.

    + +
    + +
    +
    + + +
    + +
    + +
    +
    +{% endif %} +{% endblock %} diff --git a/tildes/tildes/views/api/web/user.py b/tildes/tildes/views/api/web/user.py index dc4df27..7e0fcd1 100644 --- a/tildes/tildes/views/api/web/user.py +++ b/tildes/tildes/views/api/web/user.py @@ -1,10 +1,17 @@ """Web API endpoints related to users.""" +import random +import string from typing import Optional from marshmallow import ValidationError from marshmallow.fields import String -from pyramid.httpexceptions import HTTPForbidden, HTTPUnprocessableEntity +import pyotp +from pyramid.httpexceptions import ( + HTTPForbidden, + HTTPUnauthorized, + HTTPUnprocessableEntity, +) from pyramid.request import Request from pyramid.response import Response from sqlalchemy.exc import IntegrityError @@ -84,6 +91,63 @@ 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", + request_param="ic-trigger-name=enable-two-factor", + renderer="two_factor_enabled.jinja2", + permission="change_two_factor", +) +def post_enable_two_factor(request: Request) -> 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): + 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 + + return {"backup_codes": backup_codes} + + +@ic_view_config( + route_name="user", + request_method="POST", + request_param="ic-trigger-name=disable-two-factor", + renderer="two_factor_disabled.jinja2", + permission="change_two_factor", +) +def post_disable_two_factor(request: Request) -> 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") + + request.user.two_factor_enabled = False + request.user.two_factor_secret = None + request.user.two_factor_backup_codes = None + + return {} + + @ic_view_config( route_name="user", request_method="PATCH", diff --git a/tildes/tildes/views/login.py b/tildes/tildes/views/login.py index bab5b25..885d910 100644 --- a/tildes/tildes/views/login.py +++ b/tildes/tildes/views/login.py @@ -1,7 +1,9 @@ """Views related to logging in/out.""" -from pyramid.httpexceptions import HTTPFound, HTTPUnprocessableEntity +from pyramid.httpexceptions import HTTPFound, HTTPUnauthorized, HTTPUnprocessableEntity +from pyramid.renderers import render_to_response from pyramid.request import Request +from pyramid.response import Response from pyramid.security import NO_PERMISSION_REQUIRED, remember from pyramid.view import view_config from webargs.pyramidparser import use_kwargs @@ -24,6 +26,22 @@ def get_login(request: Request) -> dict: return {} +def finish_login(request: Request, user: User) -> None: + """Save the user ID into session.""" + # Username/password were correct - attach the user_id to the session + remember(request, user.user_id) + + # Depending on "keep me logged in", set session timeout to 1 year or 1 day + if request.params.get("keep"): + request.session.adjust_timeout_for_session(31_536_000) + else: + request.session.adjust_timeout_for_session(86_400) + + # set request.user before logging so the user is associated with the event + request.user = user + request.db_session.add(Log(LogEventType.USER_LOG_IN, request)) + + @view_config( route_name="login", request_method="POST", permission=NO_PERMISSION_REQUIRED ) @@ -57,22 +75,47 @@ def post_login(request: Request, username: str, password: str) -> HTTPFound: if user.is_banned: raise HTTPUnprocessableEntity("This account has been banned") - # Username/password were correct - attach the user_id to the session - remember(request, user.user_id) + # If 2FA is enabled, save username to session and make user enter code + if user.two_factor_enabled: + request.session["two_factor_username"] = username + return render_to_response( + "tildes:templates/intercooler/login_two_factor.jinja2", + {"keep": request.params.get("keep")}, + request=request, + ) - # Depending on "keep me logged in", set session timeout to 1 year or 1 day - if request.params.get("keep"): - request.session.adjust_timeout_for_session(31_536_000) - else: - request.session.adjust_timeout_for_session(86_400) - - # set request.user before logging so the user is associated with the event - request.user = user - request.db_session.add(Log(LogEventType.USER_LOG_IN, request)) + finish_login(request, user) raise HTTPFound(location="/") +@view_config( + route_name="login_two_factor", + request_method="POST", + permission=NO_PERMISSION_REQUIRED, +) +@not_logged_in +@rate_limit_view("login_two_factor") +def post_login_two_factor(request: Request) -> Response: + """Process a log in request with 2FA.""" + # Look up the user for the supplied username + user = ( + request.query(User) + .undefer_all_columns() + .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"] + finish_login(request, user) + + raise HTTPFound(location="/") + else: + raise HTTPUnauthorized(body="Invalid code, please try again.") + + @view_config(route_name="logout") def get_logout(request: Request) -> HTTPFound: """Process a log out request.""" diff --git a/tildes/tildes/views/settings.py b/tildes/tildes/views/settings.py index 3cbf865..00a8894 100644 --- a/tildes/tildes/views/settings.py +++ b/tildes/tildes/views/settings.py @@ -1,9 +1,12 @@ """Views related to user settings.""" -from pyramid.httpexceptions import HTTPUnprocessableEntity +from io import BytesIO +import pyotp +from pyramid.httpexceptions import HTTPForbidden, HTTPUnprocessableEntity from pyramid.request import Request from pyramid.response import Response from pyramid.view import view_config +import qrcode from webargs.pyramidparser import use_kwargs from tildes.schemas.user import EMAIL_ADDRESS_NOTE_MAX_LENGTH, UserSchema @@ -35,6 +38,13 @@ def get_settings_account_recovery(request: Request) -> dict: return {"note_max_length": EMAIL_ADDRESS_NOTE_MAX_LENGTH} +@view_config(route_name="settings_two_factor", renderer="settings_two_factor.jinja2") +def get_settings_two_factor(request: Request) -> dict: + """Generate the two-factor authentication page.""" + # pylint: disable=unused-argument + return {} + + @view_config( route_name="settings_comment_visits", renderer="settings_comment_visits.jinja2" ) @@ -60,6 +70,27 @@ def get_settings_password_change(request: Request) -> dict: return {} +@view_config(route_name="settings_two_factor_qr_code") +def get_settings_two_factor_qr_code(request: Request) -> Response: + """Generate the 2FA QR code.""" + # If 2FA is already enabled, don't expose the secret. + if request.user.two_factor_enabled: + raise HTTPForbidden("Already enabled") + + # Generate a new secret key if the user doesn't have one. + if request.user.two_factor_secret is None: + request.user.two_factor_secret = pyotp.random_base32() + + totp = pyotp.totp.TOTP(request.user.two_factor_secret) + otp_uri = totp.provisioning_uri(request.user.username, issuer_name="Tildes") + byte_io = BytesIO() + img = qrcode.make(otp_uri, border=2, box_size=4) + + img.save(byte_io, "PNG") + + return Response(byte_io.getvalue(), cache_control="private, no-cache") + + @view_config(route_name="settings_password_change", request_method="POST") @use_kwargs( {