From 69e2a50cf06dda99402693926d842a3ff46f6beb Mon Sep 17 00:00:00 2001 From: Kyra Date: Mon, 27 Feb 2017 18:29:19 -0500 Subject: [PATCH 01/22] Implement 2FA * 76 commit squash-merge. - Implements 2FA via TOTP tokens w/recovery codes. --- etc/requirements.txt | 2 + .../versions/abeefecabdad_implement_2fa.py | 39 ++ libweasyl/libweasyl/models/tables.py | 11 + templates/control/2fa/disable.html | 29 ++ .../control/2fa/generate_recovery_codes.html | 51 +++ templates/control/2fa/init.html | 41 +++ templates/control/2fa/init_verify.html | 53 +++ templates/control/2fa/status.html | 39 ++ templates/control/control.html | 1 + templates/etc/signin_2fa_auth.html | 31 ++ weasyl/controllers/routes.py | 13 + weasyl/controllers/two_factor_auth.py | 222 ++++++++++++ weasyl/controllers/user.py | 92 ++++- weasyl/login.py | 15 +- weasyl/test/test_two_factor_auth.py | 305 ++++++++++++++++ weasyl/two_factor_auth.py | 340 ++++++++++++++++++ 16 files changed, 1278 insertions(+), 6 deletions(-) create mode 100644 libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py create mode 100644 templates/control/2fa/disable.html create mode 100644 templates/control/2fa/generate_recovery_codes.html create mode 100644 templates/control/2fa/init.html create mode 100644 templates/control/2fa/init_verify.html create mode 100644 templates/control/2fa/status.html create mode 100644 templates/etc/signin_2fa_auth.html create mode 100644 weasyl/controllers/two_factor_auth.py create mode 100644 weasyl/test/test_two_factor_auth.py create mode 100644 weasyl/two_factor_auth.py diff --git a/etc/requirements.txt b/etc/requirements.txt index 2d29f0c01..d9f24cd7c 100644 --- a/etc/requirements.txt +++ b/etc/requirements.txt @@ -17,3 +17,5 @@ Dozer==0.5 # For WSGI memory profiling Pillow==3.3.1 # For WSGI memory profiling pyramid==1.7.3 WebTest==2.0.23 +pyotp==2.2.4 # For Two-Factor Authentication +qrcodegen==1.0.0 # For Two-Factor Authentication diff --git a/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py b/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py new file mode 100644 index 000000000..5f79752b2 --- /dev/null +++ b/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py @@ -0,0 +1,39 @@ +""" +Implements two-factor authentication. + +Revision ID: abeefecabdad +Revises: 40c00abab5f9 +Create Date: 2017-01-19 01:56:20.093477 + +""" + +# revision identifiers, used by Alembic. +revision = 'abeefecabdad' +down_revision = '40c00abab5f9' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + # Create a table to store 2FA backup codes for use when the authenticator is unavailable. + op.create_table('twofa_recovery_codes', + sa.Column('userid', sa.Integer(), nullable=False), + sa.Column('recovery_code', sa.String(length=20), nullable=False), + sa.PrimaryKeyConstraint('userid', 'recovery_code'), + sa.ForeignKeyConstraint(['userid'], ['login.userid'], name='twofa_recovery_codes_userid_fkey', onupdate='CASCADE', ondelete='CASCADE'), + ) + op.create_index('ind_twofa_recovery_codes_userid', 'twofa_recovery_codes', ['userid']) + + # Modify `login` to hold the 2FA code (if set) for a user account + op.add_column( + 'login', + sa.Column('twofa_secret', sa.String(length=16), nullable=True, server_default=None), + ) + + +def downgrade(): + # Remove 2FA logic of recovery codes and secret + op.drop_index('ind_twofa_recovery_codes_userid', 'twofa_recovery_codes') + op.drop_table('twofa_recovery_codes') + op.drop_column('login', 'twofa_secret') diff --git a/libweasyl/libweasyl/models/tables.py b/libweasyl/libweasyl/models/tables.py index bd0092fe6..5c7d349dc 100644 --- a/libweasyl/libweasyl/models/tables.py +++ b/libweasyl/libweasyl/models/tables.py @@ -391,11 +391,22 @@ def default_fkey(*args, **kwargs): }, }, length=20), nullable=False, server_default=''), Column('email', String(length=100), nullable=False, server_default=''), + Column('twofa_secret', String(length=16), nullable=True), ) Index('ind_login_login_name', login.c.login_name) +twofa_recovery_codes = Table( + 'twofa_recovery_codes', metadata, + Column('userid', Integer(), primary_key=True, nullable=False), + Column('recovery_code', String(length=20), primary_key=True, nullable=False), + default_fkey(['userid'], ['login.userid'], name='twofa_recovery_codes_userid_fkey'), +) + +Index('ind_twofa_recovery_codes_userid') + + loginaddress = Table( 'loginaddress', metadata, Column('userid', Integer(), primary_key=True, nullable=False), diff --git a/templates/control/2fa/disable.html b/templates/control/2fa/disable.html new file mode 100644 index 000000000..a8ee676eb --- /dev/null +++ b/templates/control/2fa/disable.html @@ -0,0 +1,29 @@ +$def with (username, error) +$:{RENDER("common/stage_title.html", ["Disable 2FA", "Two-Factor Authentication", "/control/2fa/status"])} + +
+
+ $if error == "2fa": +
Whoops! The 2FA response token or recovery code you provided did not validate.
+ $elif error == "verify": +
Hey! Did you want to disable 2FA? You didn't check the verification checkbox!
+

Disable Two-Factor Authentication (2FA)

+

Username: $:{username}

+

+ You are about to disable 2FA for the above account, and remove the + security which it provides. If you wish to re-enable 2FA, you will + need to go through through the enabling process once again. +

+ + + +
+ +
+ $:{CSRF()} +
+ Cancel + +
+
+
diff --git a/templates/control/2fa/generate_recovery_codes.html b/templates/control/2fa/generate_recovery_codes.html new file mode 100644 index 000000000..9ea4dde6f --- /dev/null +++ b/templates/control/2fa/generate_recovery_codes.html @@ -0,0 +1,51 @@ +$def with (tfa_recovery_codes, error) +$:{RENDER("common/stage_title.html", ["Generate Recovery Codes", "Two-Factor Authentication", "/control/2fa/status"])} + +
+
+ $if error == "2fa": +
+ Whoops! The 2FA response token or recovery code you provided did not validate. + If entered, your recovery code was not consumed. +
+ $elif error == "verify": +
+ Hey! Did you want to enable 2FA? You didn't check the verification checkbox! + If entered, your recovery code was not consumed. +
+

Generate New Recovery Codes

+

+ The following recovery codes are not yet active. Once you provide + a valid 2FA token or recovery code, the below list of codes will replace all recovery + codes currently associated with your account. +

+

+ Before proceeding, print or save these codes to a secure place. You will need these + codes to continue logging into your account. If you no longer have access to your + authenticator app, please disable 2FA instead, + then re-enable it to reinitialize your authenticator. +

+ +

Your new recovery codes will be...

+
    + $for code in tfa_recovery_codes: +
  1. ${code}
    2. +
+

+ These codes are one-time use, and upon successful use will not be able to be reused or retrieved. + If and when used, mark each code off. Codes may be used in any order. +

+ + +
+ + + + $:{CSRF()} + +
+ Cancel + +
+ +
diff --git a/templates/control/2fa/init.html b/templates/control/2fa/init.html new file mode 100644 index 000000000..8df469d6b --- /dev/null +++ b/templates/control/2fa/init.html @@ -0,0 +1,41 @@ +$def with (username, tfa_secret, qrcode, error) +$:{RENDER("common/stage_title.html", ["Enable 2FA: Step 1", "Two-Factor Authentication", "/control/2fa/status"])} + +
+
+ $if error == "password": +
Whoops! You entered an incorrect password.
+ $elif error == "2fa": +
Whoops! The 2FA response token you provided did not validate.
+

Two-Factor Authentication

+

Username: $:{username}

+

+ You are about to enable Two-Factor Authentication (2FA) for the Weasyl account identified above. + You will need a compatible app such as Google Authenticator to generate time-based tokens + each time you log into your Weasyl account, or a recovery code which will be provided in the next + step. +

+ +

Confirm your Password

+ + +

Scan with Google Authenticator...

+
+ +
+

... or manually enter your secret key

+

+ Your TOTP key is: ${tfa_secret} +

+ + + + + $:{CSRF()} + +
+ Cancel + +
+
+
diff --git a/templates/control/2fa/init_verify.html b/templates/control/2fa/init_verify.html new file mode 100644 index 000000000..2e186039a --- /dev/null +++ b/templates/control/2fa/init_verify.html @@ -0,0 +1,53 @@ +$def with (tfa_secret, tfa_recovery_codes, error) +$:{RENDER("common/stage_title.html", ["Enable 2FA: Step 2", "Two-Factor Authentication", "/control/2fa/status"])} + +
+
+ $if error == "2fa": +
+ Whoops! The 2FA response token you provided did not validate. +
+ $elif error == "verify": +
+ Hey! Did you want to enable 2FA? You didn't check the verification checkbox! +
+

Verify Receipt of Two-Factor Authentication (2FA) Recovery Codes

+

+ Print these codes, and secure them as you would your password. + In the event you lose access to your authenticator app, you will need these + codes to regain access to your Weasyl account. Recovery codes can be + refreshed at any time from your settings page. +

+

+ Warning: For security reasons, Weasyl staff will be unable to assist you if + you lose access to your recovery codes. We advise never using more than eight codes, as you will + need two to disable 2FA from a logged-out state (one code to log in, one + code to disable two-factor authentication). +
+ As a precaution against being locked out your account if all recovery codes are used, 2FA will + automatically be disabled if--and only if--all recovery codes are consumed. Generating a new + set of codes will prevent this from occurring. If this occurs, you will need to set-up 2FA again. +

+

Your recovery codes are...

+
    + $for code in tfa_recovery_codes: +
  1. ${code}
    2. +
+

+ These codes are one-time use, and upon successful use will not be able to be reused or retrieved. + If and when used, mark each code off. Codes may be used in any order. +

+ +
+ + + + + $:{CSRF()} + +
+ Cancel + +
+ +
diff --git a/templates/control/2fa/status.html b/templates/control/2fa/status.html new file mode 100644 index 000000000..830641973 --- /dev/null +++ b/templates/control/2fa/status.html @@ -0,0 +1,39 @@ +$def with (tfa_enabled, tfa_recovery_codes_count) +$:{RENDER("common/stage_title.html", ["Two-Factor Authentication", "Settings"])} + +
+
+

Two-Factor Authentication (2FA) Dashboard

+

+ 2FA provides an additional layer of protection for your account during the login process. + When logging in, you will need to provide your password--which is something you know--and + additionally a time-based token, which is something you have. Without both pieces--or + factors--you or an adversary who has compromised your password will not be able to fully + authenticate to your account. +

+

2FA Status

+ $if tfa_enabled: +

+ 2FA is currently enabled. +

+

+ Click here to disable 2FA +

+

Recovery Code Status

+

+ You currently have ${tfa_recovery_codes_count} valid recovery code${'s' if tfa_recovery_codes_count > 1 else ''} assigned to your account. + We advise not relying on recovery codes as a routine access method, and further advise to never + use the last two codes; a minimum of two codes are required to disable 2FA from a logged-out state. +

+

+ Click here to get new recovery codes +

+ $else: +

+ 2FA is currently disabled.

+

+

+ Click here to enable 2FA +

+
+
diff --git a/templates/control/control.html b/templates/control/control.html index bf60905b9..782ed0a10 100644 --- a/templates/control/control.html +++ b/templates/control/control.html @@ -16,6 +16,7 @@

My Profile

Preferences

Site Preferences

Password and Email

+

Two-Factor Authentication

Submission Folders

Search Tag Filters

Community Tagging Restrictions

diff --git a/templates/etc/signin_2fa_auth.html b/templates/etc/signin_2fa_auth.html new file mode 100644 index 000000000..59e551fc4 --- /dev/null +++ b/templates/etc/signin_2fa_auth.html @@ -0,0 +1,31 @@ +$def with (username, referer, remaining_recovery_codes, error) +$:{RENDER("common/stage_title.html", ["Two-Factor Authentication", "Sign In"])} + +
+
+ $if error == "2fa": +
Whoops! The 2FA response token or recovery code you provided did not validate.
+ $:{CSRF()} +

Two-Factor Authentication

+

Username: ${username}

+

+ The Weasyl account identified above has two-factor authentication enabled. + Enter the current time-based token for this user below. Or, if you no longer + have access to the authenticator for this account, enter one of your unused + twenty-character recovery codes. You have ${remaining_recovery_codes} + code${'s' if remaining_recovery_codes > 1 else ''} remaining. +

+ $if remaining_recovery_codes == 1: +

+ Note: 2FA will be disabled upon providing your last recovery + code. +

+ + + +
+ +
+ +
+
diff --git a/weasyl/controllers/routes.py b/weasyl/controllers/routes.py index f3dd4e0e8..ef1ebcc88 100644 --- a/weasyl/controllers/routes.py +++ b/weasyl/controllers/routes.py @@ -16,6 +16,7 @@ moderation, profile, settings, + two_factor_auth, user, weasyl_collections, ) @@ -41,6 +42,7 @@ # Signin and out views. Route("/signin", "signin", {'GET': user.signin_get_, 'POST': user.signin_post_}), + Route("/signin/2fa-auth", "signin_2fa_auth", {'GET': user.signin_2fa_auth_get_, 'POST': user.signin_2fa_auth_post_}), Route("/signin/unicode-failure", "signin-unicode-failure", { 'GET': user.signin_unicode_failure_get_, 'POST': user.signin_unicode_failure_post_ }), @@ -57,6 +59,17 @@ Route("/force/resetpassword", "force_reset_password", {'POST': user.force_resetpassword_}), Route("/force/resetbirthday", "force_reset_birthday", {'POST': user.force_resetpassword_}), + # Two-Factor Authentication views. + Route("/control/2fa/status", "control_2fa_status", {'GET': two_factor_auth.tfa_status_get_}), + Route("/control/2fa/init", "control_2fa_init", + {'GET': two_factor_auth.tfa_init_get_, 'POST': two_factor_auth.tfa_init_post_}), + Route("/control/2fa/init_verify", "control_2fa_init_verify", + {'GET': two_factor_auth.tfa_init_verify_get_, 'POST': two_factor_auth.tfa_init_verify_post_}), + Route("/control/2fa/disable", "control_2fa_disable", + {'GET': two_factor_auth.tfa_disable_get_, 'POST': two_factor_auth.tfa_disable_post_}), + Route("/control/2fa/generate_recovery_codes", "control_2fa_generate_recovery_codes", + {'GET': two_factor_auth.tfa_generate_recovery_codes_get_, 'POST': two_factor_auth.tfa_generate_recovery_codes_post_}), + # Profile views. Route("/~", "profile_tilde_unnamed", profile.profile_), Route("/~{name}", "profile_tilde", profile.profile_), diff --git a/weasyl/controllers/two_factor_auth.py b/weasyl/controllers/two_factor_auth.py new file mode 100644 index 000000000..cc73e3fee --- /dev/null +++ b/weasyl/controllers/two_factor_auth.py @@ -0,0 +1,222 @@ +from __future__ import absolute_import, unicode_literals + +from pyramid.response import Response +from pyramid.httpexceptions import HTTPSeeOther + +from weasyl import define +from weasyl import login +from weasyl import two_factor_auth as tfa +from weasyl.controllers.decorators import ( + login_required, + token_checked, +) +from weasyl.error import WeasylError + + +def _error_if_2fa_enabled(userid): + """ + In lieu of a module-specific decorator, this function returns an error if 2FA is enabled, preventing the user + from self-wiping their own 2FA Secret (AKA, re-setting up 2FA while it is already enabled) + """ + if tfa.is_2fa_enabled(userid): + return Response(define.errorpage(userid, "2FA is already configured for this account.", [ + ["Go Back", "/control"], ["Return to the Home Page", "/"] + ])) + + +def _error_if_2fa_is_not_enabled(userid): + """ + In lieu of a module-specific decorator, this function returns an error if 2FA is not enabled. + """ + if not tfa.is_2fa_enabled(userid): + return Response(define.errorpage(userid, "2FA is not configured for this account.", [ + ["Go Back", "/control"], ["Return to the Home Page", "/"] + ])) + + +@login_required +def tfa_status_get_(request): + return Response(define.webpage(request.userid, "control/2fa/status.html", [ + tfa.is_2fa_enabled(request.userid), tfa.get_number_of_recovery_codes(request.userid) + ])) + + +@login_required +def tfa_init_get_(request): + # Return an error if 2FA is already enabled (there's nothing to do in this route) + _error_if_2fa_enabled(request.userid) + + # Otherwise begin the 2FA initialization process for this user + tfa_secret, tfa_qrcode = tfa.init(request.userid) + return Response(define.webpage(request.userid, "control/2fa/init.html", + [define.get_display_name(request.userid), tfa_secret, tfa_qrcode, None])) + + +@login_required +@token_checked +def tfa_init_post_(request): + # Return an error if 2FA is already enabled (there's nothing to do in this route) + _error_if_2fa_enabled(request.userid) + + # Otherwise, process the form + if request.params['action'] == "continue": + userid, status = login.authenticate_bcrypt(define.get_display_name(request.userid), + request.params['password'], session=False) + # The user's password failed to authenticate + if status == "invalid": + return Response(define.webpage(request.userid, "control/2fa/init.html", [ + define.get_display_name(request.userid), + request.params['tfasecret'], + tfa.generate_tfa_qrcode(request.userid, request.params['tfasecret']), + "password" + ])) + # Unlikely that this block will get triggered, but just to be safe, check for it + elif status == "unicode-failure": + raise HTTPSeeOther(location='/signin/unicode-failure') + tfa_secret, recovery_codes = tfa.init_verify_tfa(request.userid, request.params['tfasecret'], request.params['tfaresponse']) + + # The 2FA TOTP code did not match with the generated 2FA secret + if not tfa_secret: + return Response(define.webpage(request.userid, "control/2fa/init.html", [ + define.get_display_name(request.userid), + request.params['tfasecret'], + tfa.generate_tfa_qrcode(request.userid, request.params['tfasecret']), + "2fa" + ])) + else: + return Response(define.webpage(request.userid, "control/2fa/init_verify.html", + [tfa_secret, recovery_codes, None])) + else: + # This shouldn't be reached normally (user intentionally altered action?) + raise WeasylError("Unexpected") + + +@login_required +def tfa_init_verify_get_(request): + """ + IMPLEMENTATION NOTE: This page cannot be accessed directly (HTTP GET), as the user has not generated + their 2FA secret at this point, and thus not loaded the secret into their 2FA authenticator of choice. + That said, be helpful and inform the user of this instead of erroring without explanation. + """ + # Return an error if 2FA is already enabled (there's nothing to do in this route) + _error_if_2fa_enabled(request.userid) + + # If 2FA is not enabled, inform the user of where to go to begin + return Response(define.errorpage( + request.userid, + """This page cannot be accessed directly, and must be accessed as part of the 2FA + setup process. Click 2FA Status, below, to go to the 2FA Dashboard to begin.""", + [["2FA Status", "/control/2fa/status"], ["Return to the Home Page", "/"]])) + + +@login_required +@token_checked +def tfa_init_verify_post_(request): + # Return an error if 2FA is already enabled (there's nothing to do in this route) + _error_if_2fa_enabled(request.userid) + + # Extract parameters from the form + action = request.params['action'] + verify_checkbox = request.params['verify'] + tfasecret = request.params['tfasecret'] + tfaresponse = request.params['tfaresponse'] + tfarecoverycodes = request.params['tfarecoverycodes'] + + # Does the user want to proceed with enabling 2FA? + if action == "enable" and verify_checkbox and tfa.store_recovery_codes(request.userid, tfarecoverycodes): + # TOTP+2FA Secret validates (activate & redirect to status page) + if tfa.activate(request.userid, tfasecret, tfaresponse): + raise HTTPSeeOther(location="/control/2fa/status") + # TOTP+2FA Secret did not validate + else: + return Response(define.webpage(request.userid, "control/2fa/init_verify.html", + [tfasecret, tfarecoverycodes.split(','), "2fa"])) + # The user didn't check the verification checkbox (despite HTML5's client-side check); regenerate codes & redisplay + elif action == "enable" and not verify_checkbox: + return Response(define.webpage(request.userid, "control/2fa/init_verify.html", + [tfasecret, tfarecoverycodes.split(','), "verify"])) + else: + # This shouldn't be reached normally (user intentionally altered action?) + raise WeasylError("Unexpected") + + +@login_required +def tfa_disable_get_(request): + # Return an error if 2FA is not enabled (there's nothing to do in this route) + _error_if_2fa_is_not_enabled(request.userid) + + return Response(define.webpage(request.userid, "control/2fa/disable.html", + [define.get_display_name(request.userid), None])) + + +@login_required +@token_checked +def tfa_disable_post_(request): + # Return an error if 2FA is not enabled (there's nothing to do in this route) + _error_if_2fa_is_not_enabled(request.userid) + + tfaresponse = request.params['tfaresponse'] + verify_checkbox = request.params['verify'] + action = request.params['action'] + + if action == "disable" and verify_checkbox: + # If 2FA was successfully deactivated... return to 2FA dashboard + if tfa.deactivate(request.userid, tfaresponse): + raise HTTPSeeOther(location="/control/2fa/status") + else: + return Response(define.webpage(request.userid, "control/2fa/disable.html", + [define.get_display_name(request.userid), "2fa"])) + # The user didn't check the verification checkbox (despite HTML5's client-side check) + elif action == "disable" and not verify_checkbox: + return Response(define.webpage(request.userid, "control/2fa/disable.html", + [define.get_display_name(request.userid), "verify"])) + else: + # This shouldn't be reached normally (user intentionally altered action?) + raise WeasylError("Unexpected") + + +@login_required +def tfa_generate_recovery_codes_get_(request): + # Return an error if 2FA is not enabled (there's nothing to do in this route) + _error_if_2fa_is_not_enabled(request.userid) + + return Response(define.webpage(request.userid, "control/2fa/generate_recovery_codes.html", [ + tfa.generate_recovery_codes(), + None + ])) + + +@login_required +@token_checked +def tfa_generate_recovery_codes_post_(request): + # Return an error if 2FA is not enabled (there's nothing to do in this route) + _error_if_2fa_is_not_enabled(request.userid) + + # Extract parameters from the form + action = request.params['action'] + verify_checkbox = request.params['verify'] + tfaresponse = request.params['tfaresponse'] + tfarecoverycodes = request.params['tfarecoverycodes'] + + # Does the user want to save the new recovery codes? + if action == "save" and verify_checkbox: + if tfa.verify(request.userid, tfaresponse, consume_recovery_code=False): + if tfa.store_recovery_codes(request.userid, tfarecoverycodes): + # Successfuly stored new recovery codes. + raise HTTPSeeOther(location="/control/2fa/status") + else: + # Recovery code string was corrupted or otherwise altered. + raise WeasylError("Unexpected") + else: + return Response(define.webpage(request.userid, "control/2fa/generate_recovery_codes.html", [ + tfarecoverycodes.split(','), + "2fa" + ])) + elif action == "save" and not verify_checkbox: + return Response(define.webpage(request.userid, "control/2fa/generate_recovery_codes.html", [ + tfarecoverycodes.split(','), + "verify" + ])) + else: + # This shouldn't be reached normally (user intentionally altered action?) + raise WeasylError("Unexpected") diff --git a/weasyl/controllers/user.py b/weasyl/controllers/user.py index fdd2a9fca..ac51c5bb3 100644 --- a/weasyl/controllers/user.py +++ b/weasyl/controllers/user.py @@ -1,5 +1,6 @@ from __future__ import absolute_import +import arrow from pyramid.httpexceptions import ( HTTPFound, HTTPSeeOther, @@ -7,7 +8,7 @@ from pyramid.response import Response from weasyl import define, errorcode, index, login, moderation, \ - premiumpurchase, profile, resetpassword + premiumpurchase, profile, resetpassword, two_factor_auth from weasyl.controllers.decorators import ( disallow_api, guest_required, @@ -42,6 +43,21 @@ def signin_post_(request): # Invalidate cached versions of the frontpage to respect the possibly changed SFW settings. index.template_fields.invalidate(logid) raise HTTPSeeOther(location=form.referer) + elif logid and logerror == "2fa": + # Password authentication passed, but user has 2FA set, so verify second factor (Also set SFW mode now) + if form.sfwmode == "sfw": + request.set_cookie_on_response("sfwmode", "sfw", 31536000) + index.template_fields.invalidate(logid) + # Store the authenticated userid & password auth time to the session + sess = define.get_weasyl_session() + sess.additional_data['2fa_pwd_auth_timestamp'] = arrow.now().timestamp + sess.additional_data['2fa_pwd_auth_userid'] = logid + sess.save = True + return Response(define.webpage( + request.userid, + "etc/signin_2fa_auth.html", + [define.get_display_name(logid), form.referer, two_factor_auth.get_number_of_recovery_codes(logid), + None])) elif logerror == "invalid": return Response(define.webpage(request.userid, "etc/signin.html", [True, form.referer])) elif logerror == "banned": @@ -65,6 +81,80 @@ def signin_post_(request): return Response(define.errorpage(request.userid)) +@guest_required +def signin_2fa_auth_get_(request): + sess = define.get_weasyl_session() + + # Only render page if the password has been authenticated (we have a UserID stored in the session) + if '2fa_pwd_auth_userid' not in sess.additional_data: + return Response(define.errorpage(request.userid, errorcode.permission)) + tfa_userid = sess.additional_data['2fa_pwd_auth_userid'] + + # Maximum secondary authentication time: 5 minutes + session_life = arrow.now().timestamp - sess.additional_data['2fa_pwd_auth_timestamp'] + if session_life > 300: + del sess.additional_data['2fa_pwd_auth_timestamp'] + del sess.additional_data['2fa_pwd_auth_userid'] + sess.save = True + return Response(define.errorpage( + request.userid, + "Your login session has timed out. Please try logging in again.", + [["Sign In", "/signin"], ["Return to the Home Page", "/"]])) + else: + ref = request.params["referer"] if "referer" in request.params else "/" + return Response(define.webpage( + request.userid, + "etc/signin_2fa_auth.html", + [define.get_display_name(tfa_userid), ref, two_factor_auth.get_number_of_recovery_codes(tfa_userid), + None])) + + +@guest_required +@token_checked +def signin_2fa_auth_post_(request): + sess = define.get_weasyl_session() + + # Only render page if the password has been authenticated (we have a UserID stored in the session) + if '2fa_pwd_auth_userid' not in sess.additional_data: + return Response(define.errorpage(request.userid, errorcode.permission)) + tfa_userid = sess.additional_data['2fa_pwd_auth_userid'] + + # Maximum secondary authentication time: 5 minutes + session_life = arrow.now().timestamp - sess.additional_data['2fa_pwd_auth_timestamp'] + if session_life > 300: + del sess.additional_data['2fa_pwd_auth_timestamp'] + del sess.additional_data['2fa_pwd_auth_userid'] + sess.save = True + return Response(define.errorpage( + request.userid, + "Your authentication session has timed out. Please try logging in again.", + [["Sign In", "/signin"], ["Return to the Home Page", "/"]])) + elif two_factor_auth.verify(tfa_userid, request.params["tfaresponse"]): + # 2FA passed, so login and cleanup. + del sess.additional_data['2fa_pwd_auth_timestamp'] + del sess.additional_data['2fa_pwd_auth_userid'] + sess.save = True + login.signin(tfa_userid) + ref = request.params["referer"] or "/" + # User is out of recovery codes, so force-deactivate 2FA + if two_factor_auth.get_number_of_recovery_codes(tfa_userid) == 0: + two_factor_auth.force_deactivate(tfa_userid) + return Response(define.errorpage( + tfa_userid, + """You have used all of your 2FA recovery codes. In order to prevent you from + being locked out of your account, 2FA has been disabled for your account.""", + [["Re-Enable 2FA", "/control/2fa/init"], ["Continue", ref]] + )) + raise HTTPSeeOther(location=ref) + else: + # 2FA failed; redirect to 2FA input page & inform user that authentication failed. + return Response(define.webpage( + request.userid, + "etc/signin_2fa_auth.html", + [define.get_display_name(tfa_userid), request.params["referer"], two_factor_auth.get_number_of_recovery_codes(tfa_userid), + "2fa"])) + + @login_required def signin_unicode_failure_get_(request): return Response(define.webpage(request.userid, 'etc/unicode_failure.html')) diff --git a/weasyl/login.py b/weasyl/login.py index d771a8d7c..cea3410e3 100644 --- a/weasyl/login.py +++ b/weasyl/login.py @@ -49,6 +49,7 @@ def authenticate_bcrypt(username, password, session=True): - "address" - "banned" - "suspended" + - "2fa" - Indicates the user has opted-in to 2FA. Additional authentication required. """ # Check that the user entered potentially valid values for `username` and # `password` before attempting to authenticate them @@ -57,14 +58,14 @@ def authenticate_bcrypt(username, password, session=True): # Select the authentication data necessary to check that the the user-entered # credentials are valid - query = d.execute("SELECT ab.userid, ab.hashsum, lo.settings FROM authbcrypt ab" + query = d.execute("SELECT ab.userid, ab.hashsum, lo.settings, lo.twofa_secret FROM authbcrypt ab" " RIGHT JOIN login lo USING (userid)" " WHERE lo.login_name = '%s'", [d.get_sysname(username)], ["single"]) if not query: return 0, "invalid" - USERID, HASHSUM, SETTINGS = query + USERID, HASHSUM, SETTINGS, TWOFA = query HASHSUM = HASHSUM.encode('utf-8') d.metric('increment', 'attemptedlogins') @@ -99,9 +100,13 @@ def authenticate_bcrypt(username, password, session=True): # Attempt to create a new session if `session` is True, then log the signin # if it succeeded. if session: - signin(USERID) - d.append_to_log('login.success', userid=USERID, ip=d.get_address()) - d.metric('increment', 'logins') + # If the user's record has ``login.twofa_secret`` set (not nulled), return that password authentication succeeded. + if TWOFA: + return USERID, "2fa" + else: + signin(USERID) + d.append_to_log('login.success', userid=USERID, ip=d.get_address()) + d.metric('increment', 'logins') status = None if not unicode_success: diff --git a/weasyl/test/test_two_factor_auth.py b/weasyl/test/test_two_factor_auth.py new file mode 100644 index 000000000..23bb30ec8 --- /dev/null +++ b/weasyl/test/test_two_factor_auth.py @@ -0,0 +1,305 @@ +from __future__ import absolute_import, unicode_literals + +import re +import urllib + +import pyotp +import pytest +from qrcodegen import QrCode + +from libweasyl import security +from weasyl import define as d +from weasyl import two_factor_auth as tfa +from weasyl.test import db_utils + + +@pytest.mark.usefixtures('db') +def test_get_number_of_recovery_codes(): + user_id = db_utils.create_user() + + # This /should/ be zero right now, but verify this for test environment sanity. + assert 0 == d.engine.scalar(""" + SELECT COUNT(*) + FROM twofa_recovery_codes + WHERE userid = (%(userid)s) + """, userid=user_id) + assert tfa.get_number_of_recovery_codes(user_id) == 0 + d.engine.execute(""" + INSERT INTO twofa_recovery_codes (userid, recovery_code) + VALUES ( (%(userid)s), (%(code)s) ) + """, userid=user_id, code=security.generate_key(20)) + assert tfa.get_number_of_recovery_codes(user_id) == 1 + d.engine.execute(""" + DELETE FROM twofa_recovery_codes + WHERE userid = (%(userid)s) + """, userid=user_id) + assert tfa.get_number_of_recovery_codes(user_id) == 0 + + +def test_generate_recovery_codes(): + codes = tfa.generate_recovery_codes() + assert len(codes) == 10 + for code in codes: + assert len(code) == 20 + + +def test_store_recovery_codes(): + user_id = db_utils.create_user() + valid_code_string = "01234567890123456789,02234567890123456789,03234567890123456789,04234567890123456789,05234567890123456789,06234567890123456789,07234567890123456789,08234567890123456789,09234567890123456789,10234567890123456789" + recovery_code = "A" * 20 + d.engine.execute(""" + INSERT INTO twofa_recovery_codes (userid, recovery_code) + VALUES ( (%(userid)s), (%(code)s) ) + """, userid=user_id, code=recovery_code) + + # store_recovery_codes() will not accept a string of codes where the total code count is not 10 + invalid_codes = valid_code_string.split(',').pop() + assert not tfa.store_recovery_codes(user_id, ','.join(invalid_codes)) + + # store_recovery_codes() will not accept a string of codes when the code length is not 20 + invalid_codes = "01,02,03,04,05,06,07,08,09,10" + assert not tfa.store_recovery_codes(user_id, invalid_codes) + + # When a correct code list is provides, the codes will be stored successfully in the database + assert tfa.store_recovery_codes(user_id, valid_code_string) + + query = d.engine.execute(""" + SELECT recovery_code + FROM twofa_recovery_codes + WHERE userid = (%(userid)s) + """, userid=user_id).fetchall() + + # Verify that the target codes were added, and the originally stored code does not remain + assert recovery_code not in query + valid_code_list = valid_code_string.split(',') + for code in query: + assert len(code['recovery_code']) == 20 + assert code['recovery_code'] in valid_code_list + + +@pytest.mark.usefixtures('db') +def test_is_recovery_code_valid(): + user_id = db_utils.create_user() + recovery_code = "A" * 20 + d.engine.execute(""" + INSERT INTO twofa_recovery_codes (userid, recovery_code) + VALUES ( (%(userid)s), (%(code)s) ) + """, userid=user_id, code=recovery_code) + + # Failure: Recovery code is invalid (code was not a real code) + assert not tfa.is_recovery_code_valid(user_id, "z" * 20) + + # Failure: Recovery codes are 20 characters, and the function fast-fails if not 20 chars. + assert not tfa.is_recovery_code_valid(user_id, "z" * 19) + + # Success: Recovery code is valid (code is consumed) + assert tfa.is_recovery_code_valid(user_id, recovery_code) + + # Failure: Recovery code invalid (because code was consumed) + assert not tfa.is_recovery_code_valid(user_id, recovery_code) + + # Reinsert for case-sensitivity test + d.engine.execute(""" + INSERT INTO twofa_recovery_codes (userid, recovery_code) + VALUES ( (%(userid)s), (%(code)s) ) + """, userid=user_id, code=recovery_code) + + # Success: Recovery code is valid (because case does not matter) + assert tfa.is_recovery_code_valid(user_id, recovery_code.lower()) + + +@pytest.mark.usefixtures('db') +def test_init(): + """ + Verify we get a usable 2FA Secret and QRCode from init() + """ + user_id = db_utils.create_user() + tfa_secret, tfa_qrcode = tfa.init(user_id) + + computed_uri = pyotp.TOTP(tfa_secret).provisioning_uri(d.get_display_name(user_id), issuer_name="Weasyl") + qr = QrCode.encode_text(computed_uri, QrCode.Ecc.MEDIUM) + qr_xml = qr.to_svg_str(4) + # We only care about the content in the tags; strip '\n' to permit re.search to work + qr_svg_only = re.search(r"", qr_xml.replace('\n', '')).group(0) + computed_qrcode = urllib.quote(qr_svg_only) + # The QRcode we make locally should match that from init() + assert tfa_qrcode == computed_qrcode + # The tfa_secret from init() should be 16 characters, and work if passed in to pyotp.TOTP.now() + assert len(tfa_secret) == 16 + assert len(pyotp.TOTP(tfa_secret).now()) == 6 + + +@pytest.mark.usefixtures('db') +def test_init_verify_tfa(): + user_id = db_utils.create_user() + tfa_secret, _ = tfa.init(user_id) + + # Invalid initial verification (Tuple: False, None) + test_tfa_secret, test_recovery_codes = tfa.init_verify_tfa(user_id, tfa_secret, "000000") + assert not test_tfa_secret + assert not test_recovery_codes + + # Valid initial verification + totp = pyotp.TOTP(tfa_secret) + tfa_response = totp.now() + test_tfa_secret, test_recovery_codes = tfa.init_verify_tfa(user_id, tfa_secret, tfa_response) + assert tfa_secret == test_tfa_secret + assert len(test_recovery_codes) == 10 + + +@pytest.mark.usefixtures('db') +def test_activate(): + user_id = db_utils.create_user() + tfa_secret = pyotp.random_base32() + totp = pyotp.TOTP(tfa_secret) + + # Failed validation between tfa_secret/tfa_response + assert not tfa.activate(user_id, tfa_secret, "000000") + # Verify 2FA is not active + assert not d.engine.scalar(""" + SELECT twofa_secret + FROM login + WHERE userid = (%(userid)s) + """, userid=user_id) + + # Validation successful, and tfa_secret written into user's `login` record + tfa_response = totp.now() + assert tfa.activate(user_id, tfa_secret, tfa_response) + assert tfa_secret == d.engine.scalar(""" + SELECT twofa_secret + FROM login + WHERE userid = (%(userid)s) + """, userid=user_id) + + +@pytest.mark.usefixtures('db') +def test_is_2fa_enabled(): + user_id = db_utils.create_user() + + # 2FA is not enabled + assert not tfa.is_2fa_enabled(user_id) + + # 2FA is enabled + d.engine.execute(""" + UPDATE login + SET twofa_secret = (%(tfas)s) + WHERE userid = (%(userid)s) + """, userid=user_id, tfas=pyotp.random_base32()) + assert tfa.is_2fa_enabled(user_id) + + +@pytest.mark.usefixtures('db') +def test_deactivate(): + user_id = db_utils.create_user() + tfa_secret = pyotp.random_base32() + totp = pyotp.TOTP(tfa_secret) + + # 2FA enabled, deactivated by TOTP challenge-response code + d.engine.execute(""" + UPDATE login + SET twofa_secret = (%(tfas)s) + WHERE userid = (%(userid)s) + """, userid=user_id, tfas=tfa_secret) + tfa_response = totp.now() + assert tfa.deactivate(user_id, tfa_response) + + # 2FA enabled, deactivated by recovery code + d.engine.execute(""" + UPDATE login + SET twofa_secret = (%(tfas)s) + WHERE userid = (%(userid)s) + """, userid=user_id, tfas=tfa_secret) + tfa_response = totp.now() + recovery_code = "A" * 20 + d.engine.execute(""" + INSERT INTO twofa_recovery_codes (userid, recovery_code) + VALUES ( (%(userid)s), (%(code)s) ) + """, userid=user_id, code=recovery_code) + assert tfa.deactivate(user_id, recovery_code) + + # 2FA enabled, failed deactivation (invalid `tfa_response` (code or TOTP token)) + d.engine.execute(""" + UPDATE login + SET twofa_secret = (%(tfas)s) + WHERE userid = (%(userid)s) + """, userid=user_id, tfas=tfa_secret) + assert not tfa.deactivate(user_id, "000000") + assert not tfa.deactivate(user_id, "a" * 20) + + +@pytest.mark.usefixtures('db') +def test_force_deactivate(): + user_id = db_utils.create_user() + tfa_secret = pyotp.random_base32() + d.engine.execute(""" + UPDATE login + SET twofa_secret = (%(tfas)s) + WHERE userid = (%(userid)s) + """, userid=user_id, tfas=tfa_secret) + recovery_code = "A" * 20 + d.engine.execute(""" + INSERT INTO twofa_recovery_codes (userid, recovery_code) + VALUES ( (%(userid)s), (%(code)s) ) + """, userid=user_id, code=recovery_code) + + # Verify that force_deactivate() functions as expected. + assert tfa.is_2fa_enabled(user_id) + assert tfa.get_number_of_recovery_codes(user_id) == 1 + + tfa.force_deactivate(user_id) + + assert not tfa.is_2fa_enabled(user_id) + assert tfa.get_number_of_recovery_codes(user_id) == 0 + + +@pytest.mark.usefixtures('db') +def test_verify(): + user_id = db_utils.create_user() + tfa_secret = pyotp.random_base32() + totp = pyotp.TOTP(tfa_secret) + recovery_code = "A" * 20 + d.engine.execute(""" + UPDATE login + SET twofa_secret = (%(tfas)s) + WHERE userid = (%(userid)s) + """, userid=user_id, tfas=tfa_secret) + d.engine.execute(""" + INSERT INTO twofa_recovery_codes (userid, recovery_code) + VALUES ( (%(userid)s), (%(code)s) ) + """, userid=user_id, code=recovery_code) + + # Codes of any other length than 6 or 20 returns False + assert not tfa.verify(user_id, "a" * 5) + assert not tfa.verify(user_id, "a" * 21) + + # TOTP token matches current expected value (Successful Verification) + tfa_response = totp.now() + assert tfa.verify(user_id, tfa_response) + + # TOTP token does not match current expected value (Unsuccessful Verification) + assert not tfa.verify(user_id, "000000") + + # Recovery code does not match stored value (Unsuccessful Verification) + assert not tfa.verify(user_id, "z" * 20) + + # Recovery code matches a stored recovery code (Successful Verification) + assert tfa.verify(user_id, recovery_code) + + # Recovery codes are case-insensitive (Successful Verification) + d.engine.execute(""" + INSERT INTO twofa_recovery_codes (userid, recovery_code) + VALUES ( (%(userid)s), (%(code)s) ) + """, userid=user_id, code=recovery_code) + assert tfa.verify(user_id, 'a' * 20) + + # Recovery codes are consumed upon use (consumed previously) (Unsuccessful Verification) + assert not tfa.verify(user_id, recovery_code) + + # When parameter `consume_recovery_code` is set to False, a recovery code is not consumed. + d.engine.execute(""" + INSERT INTO twofa_recovery_codes (userid, recovery_code) + VALUES ( (%(userid)s), (%(code)s) ) + """, userid=user_id, code=recovery_code) + assert tfa.get_number_of_recovery_codes(user_id) == 1 + assert tfa.verify(user_id, 'a' * 20, consume_recovery_code=False) + assert tfa.get_number_of_recovery_codes(user_id) == 1 diff --git a/weasyl/two_factor_auth.py b/weasyl/two_factor_auth.py new file mode 100644 index 000000000..58eb7873e --- /dev/null +++ b/weasyl/two_factor_auth.py @@ -0,0 +1,340 @@ +""" +Module for handling 2FA-related functions. +""" +from __future__ import absolute_import, unicode_literals + +import re +import urllib + +import pyotp +from qrcodegen import QrCode + +from libweasyl import security +from weasyl import define as d +from weasyl import login + +# Number of recovery codes to provide the user +_TFA_RECOVERY_CODES = 10 + + +def init(userid): + """ + Initialize 2FA for a user by generating and returning a 2FA secret key. + + When a user opts-in to 2FA, this function generates the necessary 2FA secret, + and QRcode. + + Parameters: + userid: The userid of the calling user. + + Returns: A tuple in the format of (tfa_secret, tfa_qrcode), where: + tfa_secret: The 16 character pyotp-generated secret. + tfa_qrcode: A QRcode in SVG+XML format containing the information necessary to provision + a 2FA TOTP entry in an application such as Google Authenticator. Can be dropped as-is into + a template to render the QRcode. + """ + tfa_secret = pyotp.random_base32() + # Get the 2FA provisioning QRCode + tfa_qrcode = generate_tfa_qrcode(userid, tfa_secret) + # Return the tuple (2FA secret, 2FA SVG+XML string QRCode) + return tfa_secret, tfa_qrcode + + +def generate_tfa_qrcode(userid, tfa_secret): + """ + Generate a 2FA QRCode on-demand, with the provisioning URI based on the supplied 2FA-secret. + + Used as a helper function for init(), or to regenerate the QRCode from a failed attempt + at verifying the init()/init_verify_tfa() phases. + + Parameters: + userid: The userid for the calling user; used to retrieve the username for the provisioning URI. + tfa_secret: The tfa_secret as generated from init(), initially. + + Returns: A URL-quoted SVG--containing the TOTP provisioning URI--capable of being inserted into + a data-uri for display to the user. + """ + totp_uri = pyotp.TOTP(tfa_secret).provisioning_uri(d.get_display_name(userid), issuer_name="Weasyl") + # Generate the QRcode + qr = QrCode.encode_text(totp_uri, QrCode.Ecc.MEDIUM) + qr_xml = qr.to_svg_str(4) + # We only care about the content in the tags; strip '\n' to permit re.search to work + qr_svg_only = re.search(r"", qr_xml.replace('\n', '')).group(0) + return urllib.quote(qr_svg_only) + + +def init_verify_tfa(userid, tfa_secret, tfa_response): + """ + Verify that the user has successfully set-up 2FA in Google Authenticator + (or similar), and generate recovery codes for the user. + + This function is part one of two in enabling 2FA. Successful verification of this phase + ensures that the user's authentication app is working correctly. + + Parameters: + userid: The userid of the calling user. + tfa_secret: The 2FA secret generated from tfa_init(); retrieved from the + verification page's form information. + tfa_response: The 2FA challenge-response code to verify against tfa_secret. + + Returns: + - A tuple of (False, None) if the verification failed; or + - A tuple in the form of (tfa_secret, generate_recovery_codes(userid)) where: + tfa_secret: Is the verified working TOTP secret key + generate_recovery_codes(): Is a set of recovery codes + """ + totp = pyotp.TOTP(tfa_secret) + # If the provided `tfa_response` matches the TOTP value, add the value and return recovery codes + if totp.verify(tfa_response): + return tfa_secret, generate_recovery_codes() + else: + return False, None + + +def activate(userid, tfa_secret, tfa_response): + """ + Fully activate 2FA for a given user account, after final validation of the TOTP secret. + + This function is part two--the final part--in enabling 2FA. Passing this step ensures that + the user has been presented the opportunity to save recovery keys for their account. + + Parameters: + userid: The userid of the calling user. + tfa_secret: The 2FA secret generated from tfa_init(); retrieved from the + verification page's form information. + tfa_response: The 2FA challenge-response code to verify against tfa_secret. + + Returns: Boolean True if the `tfa_response` corresponds with `tfa_secret`, thus enabling 2FA, + otherwise Boolean False indicating 2FA has not been enabled. + """ + totp = pyotp.TOTP(tfa_secret) + # If the provided `tfa_response` matches the TOTP value, write the 2FA secret into `login`, activating 2FA for `userid` + if totp.verify(tfa_response): + d.engine.execute(""" + UPDATE login + SET twofa_secret = (%(tfa_secret)s) + WHERE userid = (%(userid)s) + """, tfa_secret=tfa_secret, userid=userid) + return True + else: + return False + + +def verify(userid, tfa_response, consume_recovery_code=True): + """ + Verify a 2FA-enabled user's 2FA challenge-response against the stored + 2FA secret. + + Parameters: + userid: The userid to compare the 2FA challenge-response against. + tfa_response: User-supplied response. May be either the Google Authenticator + (or other app) supplied code, or a recovery code + consume_recovery_code: If set to True, the recovery code is consumed if it is + valid, otherwise the call to this function is treated as a query to the validity + of the recovery code. (Default: Boolean True) + + Returns: Boolean True if 2FA verification is successful, Boolean False otherwise. + """ + # Length six (6) is a TOTP code... + if len(tfa_response) == 6: + tfa_secret = d.engine.scalar(""" + SELECT twofa_secret + FROM login + WHERE userid = (%(userid)s) + """, userid=userid) + # Validate supplied 2FA response versus calculated current TOTP value. + totp = pyotp.TOTP(tfa_secret) + # Return the response of the TOTP verification; True/False + return totp.verify(tfa_response) + # Length twenty (20) is a recovery code... + elif len(tfa_response) == 20: + # Check if `tfa_response` is valid recovery code; consume according to `consume_recovery_code`, + # and return True if valid, False otherwise + return is_recovery_code_valid(userid, tfa_response, consume_recovery_code) + # Other lengths are invalid; return False + else: + return False + + +def get_number_of_recovery_codes(userid): + """ + Get and return the number of remaining recovery codes for `userid`. + + Parameters: + userid: The userid for which to check the count of recovery codes. + + Returns: + An integer representing the number of remaining recovery codes. + """ + return d.engine.scalar(""" + SELECT COUNT(*) + FROM twofa_recovery_codes + WHERE userid = (%(userid)s) + """, userid=userid) + + +def generate_recovery_codes(): + """ + Generate a set of valid recovery codes. + + Parameters: None + + Returns: A set of length ``_TFA_RECOVERY_CODES`` where each code is 20 characters in length. + """ + return {security.generate_key(20).upper() for i in range(_TFA_RECOVERY_CODES)} + + +def store_recovery_codes(userid, recovery_codes): + """ + Store generated recovery codes into the recovery code table, checking for validity. + + Parameters: + userid: The userid to save the recovery codes for. + recovery_codes: Comma separated unicode string of recovery codes. + + Returns: Boolean True if the codes were successfully saved to the database, otherwise + Boolean False + """ + # Force the incoming string to uppercase, then split into a list + codes = recovery_codes.upper().split(',') + # The list must exist and be equal to the current codes to generate + if not len(codes) == _TFA_RECOVERY_CODES: + return False + # Make sure all codes are 20 characters long, as expected + for code in codes: + if not len(code) == 20: + return False + + # If above checks have passed, clear current recovery codes for `userid` and store new ones + d.engine.execute(""" + DELETE FROM twofa_recovery_codes + WHERE userid = (%(userid)s); + """, userid=userid) + d.engine.execute(""" + INSERT INTO twofa_recovery_codes (userid, recovery_code) + SELECT (%(userid)s), unnest( (%(tfa_recovery_codes)s) ) + """, userid=userid, tfa_recovery_codes=list(codes)) + + return True + + +def is_recovery_code_valid(userid, tfa_code, consume_recovery_code=True): + """ + Checks the recovery code table for a valid recovery code. + + Determine if a supplied recovery code is present in the recovery code table + for a specified userid. If present, consume the code by deleting the record. + Case-insensitive, as the code is converted to upper-case before querying + the database. + + Parameters: + userid: The userid of the requesting user. + tfa_code: A candidate recovery code to check. + consume_recovery_code: If set to True, the recovery code is consumed if it is + valid, otherwise the call to this function is treated as a query to the validity + of the recovery code. (Default: Boolean True) + + Returns: Boolean True if the code was valid and has been consumed, Boolean False, otherwise. + """ + # Recovery codes must be 20 characters; fast-fail if `tfa_code` is not 20 + if len(tfa_code) != 20: + return False + # Check to see if the provided code--converting to upper-case first--is valid and consume if so + if consume_recovery_code: + tfa_rc = d.engine.scalar(""" + DELETE FROM twofa_recovery_codes + WHERE userid = (%(userid)s) AND recovery_code = (%(recovery_code)s) + RETURNING recovery_code + """, userid=userid, recovery_code=tfa_code.upper()) + else: + # We only want to see if the code is valid at the moment. + tfa_rc = d.engine.scalar(""" + SELECT recovery_code + FROM twofa_recovery_codes + WHERE userid = (%(userid)s) AND recovery_code = (%(recovery_code)s) + """, userid=userid, recovery_code=tfa_code.upper()) + # Return True if the recovery code was valid, False otherwise + if tfa_rc: + return True + else: + return False + + +def is_2fa_enabled(userid): + """ + Check if 2FA is enabled for a specified user. + + Check the ``login.tfa_secret`` field for the tuple identified by ``userid``. If the field is NULL, + 2FA is not enabled. If it is not null, 2FA is enabled. + + Parameters: + userid: The userid to check for 2FA being enabled. + + Returns: Boolean True if 2FA is enabled for ``userid``, otherwise Boolean False. + """ + result = d.engine.scalar(""" + SELECT twofa_secret + FROM login + WHERE userid = (%(userid)s) + """, userid=userid) + if result: + return True + else: + return False + + +def deactivate(userid, tfa_response): + """ + Deactivate 2FA for a specified user. + + Turns off 2FA by nulling-out the ``login.twofa_secret`` field for the user record, + and clear any remaining recovery codes. + + Parameters: + userid: The user for which 2FA should be disabled. + tfa_response: User-supplied response. May be either the Google Authenticator + (or other app) supplied code, or a recovery code. + + Returns: Boolean True if 2FA was successfully disabled, otherwise Boolean False if the + verification of `tfa_response` failed (bad challenge-response or invalid recovery code). + """ + # Sanity checking for length requirement performed in verify() function (6 or 20 length) + if verify(userid, tfa_response): + d.engine.execute(""" + BEGIN; + + UPDATE login + SET twofa_secret = NULL + WHERE userid = (%(userid)s); + + DELETE FROM twofa_recovery_codes + WHERE userid = (%(userid)s); + + COMMIT; + """, userid=userid) + return True + else: + return False + + +def force_deactivate(userid): + """ + Force-deactivate 2FA for an account. + + Parameters: + userid: The userid for an account for which to deactivate 2FA. + + Returns: Nothing + """ + d.engine.execute(""" + BEGIN; + + UPDATE login + SET twofa_secret = NULL + WHERE userid = (%(userid)s); + + DELETE FROM twofa_recovery_codes + WHERE userid = (%(userid)s); + + COMMIT; + """, userid=userid) From 27ab54e7433202e7d39499c65f066777b34e19e0 Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Tue, 28 Feb 2017 20:27:35 -0500 Subject: [PATCH 02/22] Miscellaneous cleanup --- templates/control/2fa/disable.html | 4 +- .../control/2fa/generate_recovery_codes.html | 2 +- templates/control/2fa/init.html | 9 +- templates/control/2fa/init_verify.html | 11 +-- templates/etc/signin_2fa_auth.html | 2 +- weasyl/test/test_two_factor_auth.py | 86 +++++++++--------- weasyl/two_factor_auth.py | 89 ++++++++++--------- 7 files changed, 102 insertions(+), 101 deletions(-) diff --git a/templates/control/2fa/disable.html b/templates/control/2fa/disable.html index a8ee676eb..1388eb634 100644 --- a/templates/control/2fa/disable.html +++ b/templates/control/2fa/disable.html @@ -8,7 +8,7 @@ $elif error == "verify":
Hey! Did you want to disable 2FA? You didn't check the verification checkbox!

Disable Two-Factor Authentication (2FA)

-

Username: $:{username}

+

Username: $:{MARKDOWN_EXCERPT(username)}

You are about to disable 2FA for the above account, and remove the security which it provides. If you wish to re-enable 2FA, you will @@ -18,7 +18,7 @@

Disable Two-Factor Authentication (2FA)

- +
$:{CSRF()}
diff --git a/templates/control/2fa/generate_recovery_codes.html b/templates/control/2fa/generate_recovery_codes.html index 9ea4dde6f..a313106e4 100644 --- a/templates/control/2fa/generate_recovery_codes.html +++ b/templates/control/2fa/generate_recovery_codes.html @@ -38,7 +38,7 @@

Your new recovery codes will be...


- + $:{CSRF()} diff --git a/templates/control/2fa/init.html b/templates/control/2fa/init.html index 8df469d6b..ef936e54e 100644 --- a/templates/control/2fa/init.html +++ b/templates/control/2fa/init.html @@ -8,18 +8,17 @@ $elif error == "2fa":
Whoops! The 2FA response token you provided did not validate.

Two-Factor Authentication

-

Username: $:{username}

+

Username: $:{MARKDOWN_EXCERPT(username)}

You are about to enable Two-Factor Authentication (2FA) for the Weasyl account identified above. - You will need a compatible app such as Google Authenticator to generate time-based tokens - each time you log into your Weasyl account, or a recovery code which will be provided in the next - step. + You will need a compatible app, such as Google Authenticator, to generate time-based tokens + each time you log into your Weasyl account.

Confirm your Password

-

Scan with Google Authenticator...

+

Scan with your authenticator...

diff --git a/templates/control/2fa/init_verify.html b/templates/control/2fa/init_verify.html index 2e186039a..899b085bb 100644 --- a/templates/control/2fa/init_verify.html +++ b/templates/control/2fa/init_verify.html @@ -21,12 +21,13 @@

Verify Receipt of Two-Factor Authentication (2FA) Recovery Codes

Warning: For security reasons, Weasyl staff will be unable to assist you if you lose access to your recovery codes. We advise never using more than eight codes, as you will - need two to disable 2FA from a logged-out state (one code to log in, one - code to disable two-factor authentication). + need two to disable 2FA from a logged-out state (one code to log in, and one code to disable + two-factor authentication).
As a precaution against being locked out your account if all recovery codes are used, 2FA will - automatically be disabled if--and only if--all recovery codes are consumed. Generating a new - set of codes will prevent this from occurring. If this occurs, you will need to set-up 2FA again. + automatically be disabled if--and only if--all recovery codes are consumed during the login process. + Generating a new set of codes will prevent this from occurring. If this occurs, you will need + to set-up 2FA again.

Your recovery codes are...

    @@ -39,7 +40,7 @@

    Your recovery codes are...


    - + diff --git a/templates/etc/signin_2fa_auth.html b/templates/etc/signin_2fa_auth.html index 59e551fc4..22d0487fb 100644 --- a/templates/etc/signin_2fa_auth.html +++ b/templates/etc/signin_2fa_auth.html @@ -7,7 +7,7 @@
    Whoops! The 2FA response token or recovery code you provided did not validate.
    $:{CSRF()}

    Two-Factor Authentication

    -

    Username: ${username}

    +

    Username: ${MARKDOWN_EXCERPT(username)}

    The Weasyl account identified above has two-factor authentication enabled. Enter the current time-based token for this user below. Or, if you no longer diff --git a/weasyl/test/test_two_factor_auth.py b/weasyl/test/test_two_factor_auth.py index 23bb30ec8..141bdc8f8 100644 --- a/weasyl/test/test_two_factor_auth.py +++ b/weasyl/test/test_two_factor_auth.py @@ -21,17 +21,17 @@ def test_get_number_of_recovery_codes(): assert 0 == d.engine.scalar(""" SELECT COUNT(*) FROM twofa_recovery_codes - WHERE userid = (%(userid)s) + WHERE userid = %(userid)s """, userid=user_id) assert tfa.get_number_of_recovery_codes(user_id) == 0 d.engine.execute(""" INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES ( (%(userid)s), (%(code)s) ) - """, userid=user_id, code=security.generate_key(20)) + VALUES (%(userid)s, %(code)s) + """, userid=user_id, code=security.generate_key(tfa.LENGTH_RECOVERY_CODE)) assert tfa.get_number_of_recovery_codes(user_id) == 1 d.engine.execute(""" DELETE FROM twofa_recovery_codes - WHERE userid = (%(userid)s) + WHERE userid = %(userid)s """, userid=user_id) assert tfa.get_number_of_recovery_codes(user_id) == 0 @@ -40,23 +40,23 @@ def test_generate_recovery_codes(): codes = tfa.generate_recovery_codes() assert len(codes) == 10 for code in codes: - assert len(code) == 20 + assert len(code) == tfa.LENGTH_RECOVERY_CODE def test_store_recovery_codes(): user_id = db_utils.create_user() valid_code_string = "01234567890123456789,02234567890123456789,03234567890123456789,04234567890123456789,05234567890123456789,06234567890123456789,07234567890123456789,08234567890123456789,09234567890123456789,10234567890123456789" - recovery_code = "A" * 20 + recovery_code = "A" * tfa.LENGTH_RECOVERY_CODE d.engine.execute(""" INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES ( (%(userid)s), (%(code)s) ) + VALUES (%(userid)s, %(code)s) """, userid=user_id, code=recovery_code) # store_recovery_codes() will not accept a string of codes where the total code count is not 10 invalid_codes = valid_code_string.split(',').pop() assert not tfa.store_recovery_codes(user_id, ','.join(invalid_codes)) - # store_recovery_codes() will not accept a string of codes when the code length is not 20 + # store_recovery_codes() will not accept a string of codes when the code length is not tfa.LENGTH_RECOVERY_CODE invalid_codes = "01,02,03,04,05,06,07,08,09,10" assert not tfa.store_recovery_codes(user_id, invalid_codes) @@ -66,30 +66,30 @@ def test_store_recovery_codes(): query = d.engine.execute(""" SELECT recovery_code FROM twofa_recovery_codes - WHERE userid = (%(userid)s) + WHERE userid = %(userid)s """, userid=user_id).fetchall() # Verify that the target codes were added, and the originally stored code does not remain assert recovery_code not in query valid_code_list = valid_code_string.split(',') for code in query: - assert len(code['recovery_code']) == 20 + assert len(code['recovery_code']) == tfa.LENGTH_RECOVERY_CODE assert code['recovery_code'] in valid_code_list @pytest.mark.usefixtures('db') def test_is_recovery_code_valid(): user_id = db_utils.create_user() - recovery_code = "A" * 20 + recovery_code = "A" * tfa.LENGTH_RECOVERY_CODE d.engine.execute(""" INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES ( (%(userid)s), (%(code)s) ) + VALUES (%(userid)s, %(code)s) """, userid=user_id, code=recovery_code) # Failure: Recovery code is invalid (code was not a real code) - assert not tfa.is_recovery_code_valid(user_id, "z" * 20) + assert not tfa.is_recovery_code_valid(user_id, "z" * tfa.LENGTH_RECOVERY_CODE) - # Failure: Recovery codes are 20 characters, and the function fast-fails if not 20 chars. + # Failure: Recovery codes are tfa.LENGTH_RECOVERY_CODE characters, and fast-fails if not that length. assert not tfa.is_recovery_code_valid(user_id, "z" * 19) # Success: Recovery code is valid (code is consumed) @@ -101,7 +101,7 @@ def test_is_recovery_code_valid(): # Reinsert for case-sensitivity test d.engine.execute(""" INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES ( (%(userid)s), (%(code)s) ) + VALUES (%(userid)s, %(code)s) """, userid=user_id, code=recovery_code) # Success: Recovery code is valid (because case does not matter) @@ -126,7 +126,7 @@ def test_init(): assert tfa_qrcode == computed_qrcode # The tfa_secret from init() should be 16 characters, and work if passed in to pyotp.TOTP.now() assert len(tfa_secret) == 16 - assert len(pyotp.TOTP(tfa_secret).now()) == 6 + assert len(pyotp.TOTP(tfa_secret).now()) == tfa.LENGTH_TOTP_CODE @pytest.mark.usefixtures('db') @@ -159,7 +159,7 @@ def test_activate(): assert not d.engine.scalar(""" SELECT twofa_secret FROM login - WHERE userid = (%(userid)s) + WHERE userid = %(userid)s """, userid=user_id) # Validation successful, and tfa_secret written into user's `login` record @@ -168,7 +168,7 @@ def test_activate(): assert tfa_secret == d.engine.scalar(""" SELECT twofa_secret FROM login - WHERE userid = (%(userid)s) + WHERE userid = %(userid)s """, userid=user_id) @@ -182,8 +182,8 @@ def test_is_2fa_enabled(): # 2FA is enabled d.engine.execute(""" UPDATE login - SET twofa_secret = (%(tfas)s) - WHERE userid = (%(userid)s) + SET twofa_secret = %(tfas)s + WHERE userid = %(userid)s """, userid=user_id, tfas=pyotp.random_base32()) assert tfa.is_2fa_enabled(user_id) @@ -197,8 +197,8 @@ def test_deactivate(): # 2FA enabled, deactivated by TOTP challenge-response code d.engine.execute(""" UPDATE login - SET twofa_secret = (%(tfas)s) - WHERE userid = (%(userid)s) + SET twofa_secret = %(tfas)s + WHERE userid = %(userid)s """, userid=user_id, tfas=tfa_secret) tfa_response = totp.now() assert tfa.deactivate(user_id, tfa_response) @@ -206,25 +206,25 @@ def test_deactivate(): # 2FA enabled, deactivated by recovery code d.engine.execute(""" UPDATE login - SET twofa_secret = (%(tfas)s) - WHERE userid = (%(userid)s) + SET twofa_secret = %(tfas)s + WHERE userid = %(userid)s """, userid=user_id, tfas=tfa_secret) tfa_response = totp.now() - recovery_code = "A" * 20 + recovery_code = "A" * tfa.LENGTH_RECOVERY_CODE d.engine.execute(""" INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES ( (%(userid)s), (%(code)s) ) + VALUES (%(userid)s, %(code)s) """, userid=user_id, code=recovery_code) assert tfa.deactivate(user_id, recovery_code) # 2FA enabled, failed deactivation (invalid `tfa_response` (code or TOTP token)) d.engine.execute(""" UPDATE login - SET twofa_secret = (%(tfas)s) - WHERE userid = (%(userid)s) + SET twofa_secret = %(tfas)s + WHERE userid = %(userid)s """, userid=user_id, tfas=tfa_secret) assert not tfa.deactivate(user_id, "000000") - assert not tfa.deactivate(user_id, "a" * 20) + assert not tfa.deactivate(user_id, "a" * tfa.LENGTH_RECOVERY_CODE) @pytest.mark.usefixtures('db') @@ -233,13 +233,13 @@ def test_force_deactivate(): tfa_secret = pyotp.random_base32() d.engine.execute(""" UPDATE login - SET twofa_secret = (%(tfas)s) - WHERE userid = (%(userid)s) + SET twofa_secret = %(tfas)s + WHERE userid = %(userid)s """, userid=user_id, tfas=tfa_secret) - recovery_code = "A" * 20 + recovery_code = "A" * tfa.LENGTH_RECOVERY_CODE d.engine.execute(""" INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES ( (%(userid)s), (%(code)s) ) + VALUES (%(userid)s, %(code)s) """, userid=user_id, code=recovery_code) # Verify that force_deactivate() functions as expected. @@ -257,18 +257,18 @@ def test_verify(): user_id = db_utils.create_user() tfa_secret = pyotp.random_base32() totp = pyotp.TOTP(tfa_secret) - recovery_code = "A" * 20 + recovery_code = "A" * tfa.LENGTH_RECOVERY_CODE d.engine.execute(""" UPDATE login - SET twofa_secret = (%(tfas)s) - WHERE userid = (%(userid)s) + SET twofa_secret = %(tfas)s + WHERE userid = %(userid)s """, userid=user_id, tfas=tfa_secret) d.engine.execute(""" INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES ( (%(userid)s), (%(code)s) ) + VALUES (%(userid)s, %(code)s) """, userid=user_id, code=recovery_code) - # Codes of any other length than 6 or 20 returns False + # Codes of any other length than tfa.LENGTH_TOTP_CODE or tfa.LENGTH_RECOVERY_CODE returns False assert not tfa.verify(user_id, "a" * 5) assert not tfa.verify(user_id, "a" * 21) @@ -280,7 +280,7 @@ def test_verify(): assert not tfa.verify(user_id, "000000") # Recovery code does not match stored value (Unsuccessful Verification) - assert not tfa.verify(user_id, "z" * 20) + assert not tfa.verify(user_id, "z" * tfa.LENGTH_RECOVERY_CODE) # Recovery code matches a stored recovery code (Successful Verification) assert tfa.verify(user_id, recovery_code) @@ -288,9 +288,9 @@ def test_verify(): # Recovery codes are case-insensitive (Successful Verification) d.engine.execute(""" INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES ( (%(userid)s), (%(code)s) ) + VALUES (%(userid)s, %(code)s) """, userid=user_id, code=recovery_code) - assert tfa.verify(user_id, 'a' * 20) + assert tfa.verify(user_id, 'a' * tfa.LENGTH_RECOVERY_CODE) # Recovery codes are consumed upon use (consumed previously) (Unsuccessful Verification) assert not tfa.verify(user_id, recovery_code) @@ -298,8 +298,8 @@ def test_verify(): # When parameter `consume_recovery_code` is set to False, a recovery code is not consumed. d.engine.execute(""" INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES ( (%(userid)s), (%(code)s) ) + VALUES (%(userid)s, %(code)s) """, userid=user_id, code=recovery_code) assert tfa.get_number_of_recovery_codes(user_id) == 1 - assert tfa.verify(user_id, 'a' * 20, consume_recovery_code=False) + assert tfa.verify(user_id, 'a' * tfa.LENGTH_RECOVERY_CODE, consume_recovery_code=False) assert tfa.get_number_of_recovery_codes(user_id) == 1 diff --git a/weasyl/two_factor_auth.py b/weasyl/two_factor_auth.py index 58eb7873e..1bd74e78c 100644 --- a/weasyl/two_factor_auth.py +++ b/weasyl/two_factor_auth.py @@ -16,6 +16,11 @@ # Number of recovery codes to provide the user _TFA_RECOVERY_CODES = 10 +# This length is configurable as needed; see generate_recovery_codes() below for keyspace/character set +LENGTH_RECOVERY_CODE = 20 +# TOTP code length of 6 is the standard length, and is supported by Google Authenticator +LENGTH_TOTP_CODE = 6 + def init(userid): """ @@ -112,8 +117,8 @@ def activate(userid, tfa_secret, tfa_response): if totp.verify(tfa_response): d.engine.execute(""" UPDATE login - SET twofa_secret = (%(tfa_secret)s) - WHERE userid = (%(userid)s) + SET twofa_secret = %(tfa_secret)s + WHERE userid = %(userid)s """, tfa_secret=tfa_secret, userid=userid) return True else: @@ -135,19 +140,17 @@ def verify(userid, tfa_response, consume_recovery_code=True): Returns: Boolean True if 2FA verification is successful, Boolean False otherwise. """ - # Length six (6) is a TOTP code... - if len(tfa_response) == 6: + if len(tfa_response) == LENGTH_TOTP_CODE: tfa_secret = d.engine.scalar(""" SELECT twofa_secret FROM login - WHERE userid = (%(userid)s) + WHERE userid = %(userid)s """, userid=userid) # Validate supplied 2FA response versus calculated current TOTP value. totp = pyotp.TOTP(tfa_secret) # Return the response of the TOTP verification; True/False return totp.verify(tfa_response) - # Length twenty (20) is a recovery code... - elif len(tfa_response) == 20: + elif len(tfa_response) == LENGTH_RECOVERY_CODE: # Check if `tfa_response` is valid recovery code; consume according to `consume_recovery_code`, # and return True if valid, False otherwise return is_recovery_code_valid(userid, tfa_response, consume_recovery_code) @@ -169,7 +172,7 @@ def get_number_of_recovery_codes(userid): return d.engine.scalar(""" SELECT COUNT(*) FROM twofa_recovery_codes - WHERE userid = (%(userid)s) + WHERE userid = %(userid)s """, userid=userid) @@ -177,11 +180,15 @@ def generate_recovery_codes(): """ Generate a set of valid recovery codes. + Character set is defined by `libweasyl.security`, (ASCII+Numbers), limited to uppercase via + .upper() for readability. + Parameters: None - Returns: A set of length ``_TFA_RECOVERY_CODES`` where each code is 20 characters in length. + Returns: A set of length `_TFA_RECOVERY_CODES` where each code is `LENGTH_RECOVERY_CODE` + characters in length. """ - return {security.generate_key(20).upper() for i in range(_TFA_RECOVERY_CODES)} + return {security.generate_key(LENGTH_RECOVERY_CODE).upper() for i in range(_TFA_RECOVERY_CODES)} def store_recovery_codes(userid, recovery_codes): @@ -198,24 +205,32 @@ def store_recovery_codes(userid, recovery_codes): # Force the incoming string to uppercase, then split into a list codes = recovery_codes.upper().split(',') # The list must exist and be equal to the current codes to generate - if not len(codes) == _TFA_RECOVERY_CODES: + if len(codes) != _TFA_RECOVERY_CODES: return False - # Make sure all codes are 20 characters long, as expected + # Make sure all codes are `LENGTH_RECOVERY_CODE` characters long, as expected for code in codes: - if not len(code) == 20: + if len(code) != LENGTH_RECOVERY_CODE: return False # If above checks have passed, clear current recovery codes for `userid` and store new ones d.engine.execute(""" + BEGIN; + DELETE FROM twofa_recovery_codes - WHERE userid = (%(userid)s); - """, userid=userid) - d.engine.execute(""" + WHERE userid = %(userid)s; + INSERT INTO twofa_recovery_codes (userid, recovery_code) - SELECT (%(userid)s), unnest( (%(tfa_recovery_codes)s) ) + SELECT %(userid)s, unnest(%(tfa_recovery_codes)s); + + COMMIT; """, userid=userid, tfa_recovery_codes=list(codes)) - return True + # Verify if the atomic transaction completed; if `code` (one of the new recovery codes) is + # valid at this point, the new codes were added + if is_recovery_code_valid(userid, code, consume_recovery_code=False): + return True + else: + return False def is_recovery_code_valid(userid, tfa_code, consume_recovery_code=True): @@ -236,14 +251,14 @@ def is_recovery_code_valid(userid, tfa_code, consume_recovery_code=True): Returns: Boolean True if the code was valid and has been consumed, Boolean False, otherwise. """ - # Recovery codes must be 20 characters; fast-fail if `tfa_code` is not 20 - if len(tfa_code) != 20: + # Recovery codes must be LENGTH_RECOVERY_CODE characters; fast-fail if this is not the case + if len(tfa_code) != LENGTH_RECOVERY_CODE: return False # Check to see if the provided code--converting to upper-case first--is valid and consume if so if consume_recovery_code: tfa_rc = d.engine.scalar(""" DELETE FROM twofa_recovery_codes - WHERE userid = (%(userid)s) AND recovery_code = (%(recovery_code)s) + WHERE userid = %(userid)s AND recovery_code = %(recovery_code)s RETURNING recovery_code """, userid=userid, recovery_code=tfa_code.upper()) else: @@ -251,7 +266,7 @@ def is_recovery_code_valid(userid, tfa_code, consume_recovery_code=True): tfa_rc = d.engine.scalar(""" SELECT recovery_code FROM twofa_recovery_codes - WHERE userid = (%(userid)s) AND recovery_code = (%(recovery_code)s) + WHERE userid = %(userid)s AND recovery_code = %(recovery_code)s """, userid=userid, recovery_code=tfa_code.upper()) # Return True if the recovery code was valid, False otherwise if tfa_rc: @@ -272,15 +287,11 @@ def is_2fa_enabled(userid): Returns: Boolean True if 2FA is enabled for ``userid``, otherwise Boolean False. """ - result = d.engine.scalar(""" - SELECT twofa_secret + return d.engine.scalar(""" + SELECT twofa_secret IS NOT NULL FROM login - WHERE userid = (%(userid)s) + WHERE userid = %(userid)s """, userid=userid) - if result: - return True - else: - return False def deactivate(userid, tfa_response): @@ -298,20 +309,10 @@ def deactivate(userid, tfa_response): Returns: Boolean True if 2FA was successfully disabled, otherwise Boolean False if the verification of `tfa_response` failed (bad challenge-response or invalid recovery code). """ - # Sanity checking for length requirement performed in verify() function (6 or 20 length) + # Sanity checking for length requirement of recovery code/TOTP is performed in verify() function if verify(userid, tfa_response): - d.engine.execute(""" - BEGIN; - - UPDATE login - SET twofa_secret = NULL - WHERE userid = (%(userid)s); - - DELETE FROM twofa_recovery_codes - WHERE userid = (%(userid)s); - - COMMIT; - """, userid=userid) + # Verification passed, so disable 2FA + force_deactivate(userid) return True else: return False @@ -331,10 +332,10 @@ def force_deactivate(userid): UPDATE login SET twofa_secret = NULL - WHERE userid = (%(userid)s); + WHERE userid = %(userid)s; DELETE FROM twofa_recovery_codes - WHERE userid = (%(userid)s); + WHERE userid = %(userid)s; COMMIT; """, userid=userid) From f444857221707e1fc7747d6da5729c9504468fd1 Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Tue, 28 Feb 2017 21:31:42 -0500 Subject: [PATCH 03/22] Add a link to authenticator locations Leverage Google's support page, since Google's Authenticator app for Android, iOS, et al., is compatible with the TOTP 2FA system implemented here. And because it may help interested users find the necessary means to enable 2FA (make it easy for the user, and they may well take you up on the offer). --- templates/control/2fa/init.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/control/2fa/init.html b/templates/control/2fa/init.html index ef936e54e..e03ed2ee1 100644 --- a/templates/control/2fa/init.html +++ b/templates/control/2fa/init.html @@ -11,8 +11,8 @@

    Two-Factor Authentication

    Username: $:{MARKDOWN_EXCERPT(username)}

    You are about to enable Two-Factor Authentication (2FA) for the Weasyl account identified above. - You will need a compatible app, such as Google Authenticator, to generate time-based tokens - each time you log into your Weasyl account. + You will need a compatible app, such as Google Authenticator, + to generate time-based tokens each time you log into your Weasyl account.

    Confirm your Password

    From 2ad4e7469c8da0b0304d06ba4af85a024160540b Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Sun, 5 Mar 2017 20:15:00 -0500 Subject: [PATCH 04/22] Split out password auth step, and permit a space in TOTP codes. From feedback in testing, seperately authenticate a user's password. Such compartmentalizes errors, and eliminates retyping a password and/or TOTP code if misentered. From feedback in testing, altered the initialization TOTP code field lengths to length 7, permitting a space to be added, as some authenticators display codes seperated in groups of three (e.g., '123 456'). Strip the space out on the back-end via Python's 're' module and substituting spaces in the response for empty strings. Additionally applies this behavior to the two_factor_auth.verify() function, pre-stripping spaces from the tfa_response to permit the remainder of the code to work without modifications. --- templates/control/2fa/init.html | 26 ++++------ templates/control/2fa/init_qrcode.html | 36 ++++++++++++++ templates/control/2fa/init_verify.html | 9 ++-- weasyl/controllers/routes.py | 2 + weasyl/controllers/two_factor_auth.py | 67 +++++++++++++++++++++++--- weasyl/profile.py | 27 ++++++++--- weasyl/two_factor_auth.py | 3 ++ 7 files changed, 135 insertions(+), 35 deletions(-) create mode 100644 templates/control/2fa/init_qrcode.html diff --git a/templates/control/2fa/init.html b/templates/control/2fa/init.html index e03ed2ee1..c286fac71 100644 --- a/templates/control/2fa/init.html +++ b/templates/control/2fa/init.html @@ -1,35 +1,27 @@ -$def with (username, tfa_secret, qrcode, error) +$def with (username, error) $:{RENDER("common/stage_title.html", ["Enable 2FA: Step 1", "Two-Factor Authentication", "/control/2fa/status"])}
    $if error == "password":
    Whoops! You entered an incorrect password.
    - $elif error == "2fa": -
    Whoops! The 2FA response token you provided did not validate.
    +

    Two-Factor Authentication

    Username: $:{MARKDOWN_EXCERPT(username)}

    You are about to enable Two-Factor Authentication (2FA) for the Weasyl account identified above. - You will need a compatible app, such as Google Authenticator, + In order to proceed through this process, you will need a compatible app such as + Google Authenticator or FreeOTP, to generate time-based tokens each time you log into your Weasyl account.

    - -

    Confirm your Password

    - - -

    Scan with your authenticator...

    -
    - -
    -

    ... or manually enter your secret key

    - Your TOTP key is: ${tfa_secret} + To begin, we need to confirm that you have control over this account. Please enter your password to continue.

    - - - +

    Confirm your Password

    + + + $:{CSRF()}
    diff --git a/templates/control/2fa/init_qrcode.html b/templates/control/2fa/init_qrcode.html new file mode 100644 index 000000000..c7908abdb --- /dev/null +++ b/templates/control/2fa/init_qrcode.html @@ -0,0 +1,36 @@ +$def with (username, tfa_secret, qrcode, error) +$:{RENDER("common/stage_title.html", ["Enable 2FA: Step 2", "Two-Factor Authentication", "/control/2fa/status"])} + +
    + + $if error == "2fa": +
    Whoops! The 2FA response token you provided did not validate.
    + +

    Two-Factor Authentication

    +

    Username: $:{MARKDOWN_EXCERPT(username)}

    +

    + In order to proceed from this point onwards, you will need a compatible app such as + Google Authenticator or FreeOTP, + to generate time-based tokens each time you log into your Weasyl account. +

    + +

    Scan with your authenticator...

    +
    + +
    +

    ... or manually enter your secret key

    +

    + Your TOTP key is: ${tfa_secret} +

    + + + + + $:{CSRF()} + +
    + Cancel + +
    + +
    diff --git a/templates/control/2fa/init_verify.html b/templates/control/2fa/init_verify.html index 899b085bb..495be9c84 100644 --- a/templates/control/2fa/init_verify.html +++ b/templates/control/2fa/init_verify.html @@ -1,16 +1,17 @@ $def with (tfa_secret, tfa_recovery_codes, error) -$:{RENDER("common/stage_title.html", ["Enable 2FA: Step 2", "Two-Factor Authentication", "/control/2fa/status"])} +$:{RENDER("common/stage_title.html", ["Enable 2FA: Step 3", "Two-Factor Authentication", "/control/2fa/status"])}
    $if error == "2fa":
    - Whoops! The 2FA response token you provided did not validate. + Whoops! The 2FA response token you provided did not validate.
    $elif error == "verify":
    - Hey! Did you want to enable 2FA? You didn't check the verification checkbox! + Hey! Did you want to enable 2FA? You didn't check the verification checkbox!
    +

    Verify Receipt of Two-Factor Authentication (2FA) Recovery Codes

    Print these codes, and secure them as you would your password. @@ -39,7 +40,7 @@

    Your recovery codes are...

    If and when used, mark each code off. Codes may be used in any order.

    -
    +
    diff --git a/weasyl/controllers/routes.py b/weasyl/controllers/routes.py index ef1ebcc88..a41ba054e 100644 --- a/weasyl/controllers/routes.py +++ b/weasyl/controllers/routes.py @@ -63,6 +63,8 @@ Route("/control/2fa/status", "control_2fa_status", {'GET': two_factor_auth.tfa_status_get_}), Route("/control/2fa/init", "control_2fa_init", {'GET': two_factor_auth.tfa_init_get_, 'POST': two_factor_auth.tfa_init_post_}), + Route("/control/2fa/init_qrcode", "control_2fa_init_qrcode", + {'GET': two_factor_auth.tfa_init_qrcode_get_, 'POST': two_factor_auth.tfa_init_qrcode_post_}), Route("/control/2fa/init_verify", "control_2fa_init_verify", {'GET': two_factor_auth.tfa_init_verify_get_, 'POST': two_factor_auth.tfa_init_verify_post_}), Route("/control/2fa/disable", "control_2fa_disable", diff --git a/weasyl/controllers/two_factor_auth.py b/weasyl/controllers/two_factor_auth.py index cc73e3fee..e3f6507ab 100644 --- a/weasyl/controllers/two_factor_auth.py +++ b/weasyl/controllers/two_factor_auth.py @@ -1,5 +1,7 @@ from __future__ import absolute_import, unicode_literals +import re + from pyramid.response import Response from pyramid.httpexceptions import HTTPSeeOther @@ -11,6 +13,7 @@ token_checked, ) from weasyl.error import WeasylError +from weasyl.profile import invalidate_other_sessions def _error_if_2fa_enabled(userid): @@ -46,10 +49,11 @@ def tfa_init_get_(request): # Return an error if 2FA is already enabled (there's nothing to do in this route) _error_if_2fa_enabled(request.userid) - # Otherwise begin the 2FA initialization process for this user - tfa_secret, tfa_qrcode = tfa.init(request.userid) - return Response(define.webpage(request.userid, "control/2fa/init.html", - [define.get_display_name(request.userid), tfa_secret, tfa_qrcode, None])) + # Otherwise, render the page + return Response(define.webpage(request.userid, "control/2fa/init.html", [ + define.get_display_name(request.userid), + None + ])) @login_required @@ -66,18 +70,60 @@ def tfa_init_post_(request): if status == "invalid": return Response(define.webpage(request.userid, "control/2fa/init.html", [ define.get_display_name(request.userid), - request.params['tfasecret'], - tfa.generate_tfa_qrcode(request.userid, request.params['tfasecret']), "password" ])) # Unlikely that this block will get triggered, but just to be safe, check for it elif status == "unicode-failure": raise HTTPSeeOther(location='/signin/unicode-failure') - tfa_secret, recovery_codes = tfa.init_verify_tfa(request.userid, request.params['tfasecret'], request.params['tfaresponse']) + # The user has authenticated, so continue with the initialization process. + else: + tfa_secret, tfa_qrcode = tfa.init(request.userid) + return Response(define.webpage(request.userid, "control/2fa/init_qrcode.html", [ + define.get_display_name(request.userid), + tfa_secret, + tfa_qrcode, + None + ])) + else: + # This shouldn't be reached normally (user intentionally altered action?) + raise WeasylError("Unexpected") + + +@login_required +def tfa_init_qrcode_get_(request): + """ + IMPLEMENTATION NOTE: This page cannot be accessed directly (HTTP GET), as the user has not yet + verified ownership over the account by verifying their password. That said, be helpful and inform + the user of this instead of erroring without explanation. + """ + # Return an error if 2FA is already enabled (there's nothing to do in this route) + _error_if_2fa_enabled(request.userid) + + # If 2FA is not enabled, inform the user of where to go to begin + return Response(define.errorpage( + request.userid, + """This page cannot be accessed directly, and must be accessed as part of the 2FA + setup process. Click 2FA Status, below, to go to the 2FA Dashboard to begin.""", + [["2FA Status", "/control/2fa/status"], ["Return to the Home Page", "/"]])) + + +@login_required +@token_checked +def tfa_init_qrcode_post_(request): + # Return an error if 2FA is already enabled (there's nothing to do in this route) + _error_if_2fa_enabled(request.userid) + + # Otherwise, process the form + if request.params['action'] == "continue": + # Strip any spaces from the TOTP code (some authenticators display the digits like '123 456') + tfaresponse = re.sub("[\ ]", '', request.params['tfaresponse']) + + # Check to see if the tfaresponse matches the tfasecret when run through the TOTP algorithm + tfa_secret, recovery_codes = tfa.init_verify_tfa(request.userid, request.params['tfasecret'], tfaresponse) # The 2FA TOTP code did not match with the generated 2FA secret if not tfa_secret: - return Response(define.webpage(request.userid, "control/2fa/init.html", [ + return Response(define.webpage(request.userid, "control/2fa/init_qrcode.html", [ define.get_display_name(request.userid), request.params['tfasecret'], tfa.generate_tfa_qrcode(request.userid, request.params['tfasecret']), @@ -124,8 +170,13 @@ def tfa_init_verify_post_(request): # Does the user want to proceed with enabling 2FA? if action == "enable" and verify_checkbox and tfa.store_recovery_codes(request.userid, tfarecoverycodes): + # Strip any spaces from the TOTP code (some authenticators display the digits like '123 456') + tfaresponse = re.sub("[\ ]", '', tfaresponse) + # TOTP+2FA Secret validates (activate & redirect to status page) if tfa.activate(request.userid, tfasecret, tfaresponse): + # Invalidate all other login sessions + invalidate_other_sessions(request.userid) raise HTTPSeeOther(location="/control/2fa/status") # TOTP+2FA Secret did not validate else: diff --git a/weasyl/profile.py b/weasyl/profile.py index ed2172fd5..5df2a296f 100644 --- a/weasyl/profile.py +++ b/weasyl/profile.py @@ -562,12 +562,27 @@ def edit_email_password(userid, username, password, newemail, newemailcheck, d.execute("UPDATE authbcrypt SET hashsum = '%s' WHERE userid = %i", [login.passhash(newpassword), userid]) # Invalidate all sessions for `userid` except for the current one - sess = d.get_weasyl_session() - d.engine.execute(""" - DELETE FROM sessions - WHERE userid = %(userid)s - AND sessionid != %(currentsession)s - """, userid=userid, currentsession=sess.sessionid) + invalidate_other_sessions(userid) + + +def invalidate_other_sessions(userid): + """ + Invalidate all HTTP sessions for `userid` except for the current session. + + Useful as a security precaution, such as if a user changes their password, or enables + 2FA. + + Parameters: + userid: The userid for the account to clear sessions from. + + Returns: Nothing. + """ + sess = d.get_weasyl_session() + d.engine.execute(""" + DELETE FROM sessions + WHERE userid = %(userid)s + AND sessionid != %(currentsession)s + """, userid=userid, currentsession=sess.sessionid) def edit_preferences(userid, timezone=None, diff --git a/weasyl/two_factor_auth.py b/weasyl/two_factor_auth.py index 1bd74e78c..801784a4f 100644 --- a/weasyl/two_factor_auth.py +++ b/weasyl/two_factor_auth.py @@ -140,6 +140,9 @@ def verify(userid, tfa_response, consume_recovery_code=True): Returns: Boolean True if 2FA verification is successful, Boolean False otherwise. """ + # Strip spaces from the tfa_response; they are not valid character, and some authenticator + # implementations display the code as "123 456" + tfa_response = re.sub("[\ ]", '', tfa_response) if len(tfa_response) == LENGTH_TOTP_CODE: tfa_secret = d.engine.scalar(""" SELECT twofa_secret From 5e41ba8747e843ed8a64016810fcaf42d13c7904 Mon Sep 17 00:00:00 2001 From: Kyra Date: Mon, 6 Mar 2017 17:05:54 -0500 Subject: [PATCH 05/22] Opt for string.replace() vs. re.sub() (#15) * Opt for string.replace vs. re.sub The totp.verify() call in tfa.verify() will normally fail and return False if a space is in a TOTP code, as some 2FA apps like to put a space in the code. For instance, "123 456". The code would need to be provided as "123456" or the function would return False. Logically, the code is correct, technically, it is incorrect with the space. So tfa.verify() was altered to force the logically correct case to also be technically correct by force-stripping spaces. The space character is not a valid character in the recovery code charset. Add a test to make sure it functions as expected with a successful verification. --- weasyl/controllers/two_factor_auth.py | 14 +++++++------- weasyl/test/test_two_factor_auth.py | 7 +++++++ weasyl/two_factor_auth.py | 6 +++--- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/weasyl/controllers/two_factor_auth.py b/weasyl/controllers/two_factor_auth.py index e3f6507ab..6baae825b 100644 --- a/weasyl/controllers/two_factor_auth.py +++ b/weasyl/controllers/two_factor_auth.py @@ -1,7 +1,5 @@ from __future__ import absolute_import, unicode_literals -import re - from pyramid.response import Response from pyramid.httpexceptions import HTTPSeeOther @@ -116,7 +114,7 @@ def tfa_init_qrcode_post_(request): # Otherwise, process the form if request.params['action'] == "continue": # Strip any spaces from the TOTP code (some authenticators display the digits like '123 456') - tfaresponse = re.sub("[\ ]", '', request.params['tfaresponse']) + tfaresponse = request.params['tfaresponse'].replace(' ', '') # Check to see if the tfaresponse matches the tfasecret when run through the TOTP algorithm tfa_secret, recovery_codes = tfa.init_verify_tfa(request.userid, request.params['tfasecret'], tfaresponse) @@ -140,9 +138,11 @@ def tfa_init_qrcode_post_(request): @login_required def tfa_init_verify_get_(request): """ - IMPLEMENTATION NOTE: This page cannot be accessed directly (HTTP GET), as the user has not generated - their 2FA secret at this point, and thus not loaded the secret into their 2FA authenticator of choice. - That said, be helpful and inform the user of this instead of erroring without explanation. + IMPLEMENTATION NOTE: This page cannot be accessed directly (HTTP GET), as the user needs to both verify + their password to assert ownership of their account (`tfa_init_*_()`), and needs to be provided with their + TOTP provisioning QRcode/secret key, and prove that they have successfully loaded it to their 2FA authenticator + of choice (`tfa_init_qrcode_*_()`). That said, be helpful and inform the user of this instead of erroring without + explanation. """ # Return an error if 2FA is already enabled (there's nothing to do in this route) _error_if_2fa_enabled(request.userid) @@ -171,7 +171,7 @@ def tfa_init_verify_post_(request): # Does the user want to proceed with enabling 2FA? if action == "enable" and verify_checkbox and tfa.store_recovery_codes(request.userid, tfarecoverycodes): # Strip any spaces from the TOTP code (some authenticators display the digits like '123 456') - tfaresponse = re.sub("[\ ]", '', tfaresponse) + tfaresponse = request.params['tfaresponse'].replace(' ', '') # TOTP+2FA Secret validates (activate & redirect to status page) if tfa.activate(request.userid, tfasecret, tfaresponse): diff --git a/weasyl/test/test_two_factor_auth.py b/weasyl/test/test_two_factor_auth.py index 141bdc8f8..86b195b3f 100644 --- a/weasyl/test/test_two_factor_auth.py +++ b/weasyl/test/test_two_factor_auth.py @@ -276,6 +276,13 @@ def test_verify(): tfa_response = totp.now() assert tfa.verify(user_id, tfa_response) + # TOTP token with space successfully verifies, as some authenticators show codes like + # "123 456"; verify strips all spaces. (Successful Verification) + tfa_response = totp.now() + # Now split the code into a space separated string (e.g., u"123 456") + tfa_response = tfa_response[:3] + ' ' + tfa_response[3:] + assert tfa.verify(user_id, tfa_response) + # TOTP token does not match current expected value (Unsuccessful Verification) assert not tfa.verify(user_id, "000000") diff --git a/weasyl/two_factor_auth.py b/weasyl/two_factor_auth.py index 801784a4f..fa3af2613 100644 --- a/weasyl/two_factor_auth.py +++ b/weasyl/two_factor_auth.py @@ -132,8 +132,8 @@ def verify(userid, tfa_response, consume_recovery_code=True): Parameters: userid: The userid to compare the 2FA challenge-response against. - tfa_response: User-supplied response. May be either the Google Authenticator - (or other app) supplied code, or a recovery code + tfa_response: User-supplied response string. May be either the Google Authenticator + (or other app) supplied code, or a recovery code. consume_recovery_code: If set to True, the recovery code is consumed if it is valid, otherwise the call to this function is treated as a query to the validity of the recovery code. (Default: Boolean True) @@ -142,7 +142,7 @@ def verify(userid, tfa_response, consume_recovery_code=True): """ # Strip spaces from the tfa_response; they are not valid character, and some authenticator # implementations display the code as "123 456" - tfa_response = re.sub("[\ ]", '', tfa_response) + tfa_response = tfa_response.replace(' ', '') if len(tfa_response) == LENGTH_TOTP_CODE: tfa_secret = d.engine.scalar(""" SELECT twofa_secret From 56c65f43606c51d2631bcd17d375fc8522689ed4 Mon Sep 17 00:00:00 2001 From: Kyra Date: Sun, 12 Mar 2017 01:04:02 -0500 Subject: [PATCH 06/22] 2FA code fixes * Rework 2FA help/information text. Places the bulk of 2FA information into a dedicated help/FAQ page. * Adds a missing column parameter to the Index() creation statement in the models.py module. * Fix the logic for prohibiting access to a route depending on if 2FA is enabled or disabled, depending on what the circumstance of the route needs, by using decorators. As was previously written, those functions weren't written correctly. * Improve readability of the TOTP secret key and the 2FA recovery codes by separating the strings into segments. Extend the maxlength attribute of relevant fields to accept a straight copy-paste'd code including those spaces (which are stripped on the backend for recovery codes). * Alphabetizes the help topics, help routes, and help route controller functions, since the new 2FA help page needed to be added; changes made for maintainability, and readability. --- libweasyl/libweasyl/models/tables.py | 2 +- templates/control/2fa/disable.html | 2 +- .../control/2fa/generate_recovery_codes.html | 18 +-- templates/control/2fa/init.html | 42 +++++-- templates/control/2fa/init_qrcode.html | 10 +- templates/control/2fa/init_verify.html | 5 +- templates/etc/signin_2fa_auth.html | 6 +- templates/help/help.html | 12 +- templates/help/two_factor_authentication.html | 103 ++++++++++++++++++ weasyl/controllers/decorators.py | 18 ++- weasyl/controllers/info.py | 85 ++++++++------- weasyl/controllers/routes.py | 21 ++-- weasyl/controllers/two_factor_auth.py | 70 +++--------- weasyl/errorcode.py | 2 + 14 files changed, 252 insertions(+), 144 deletions(-) create mode 100644 templates/help/two_factor_authentication.html diff --git a/libweasyl/libweasyl/models/tables.py b/libweasyl/libweasyl/models/tables.py index 5c7d349dc..07a4f2fb7 100644 --- a/libweasyl/libweasyl/models/tables.py +++ b/libweasyl/libweasyl/models/tables.py @@ -404,7 +404,7 @@ def default_fkey(*args, **kwargs): default_fkey(['userid'], ['login.userid'], name='twofa_recovery_codes_userid_fkey'), ) -Index('ind_twofa_recovery_codes_userid') +Index('ind_twofa_recovery_codes_userid', twofa_recovery_codes.c.userid) loginaddress = Table( diff --git a/templates/control/2fa/disable.html b/templates/control/2fa/disable.html index 1388eb634..59373325d 100644 --- a/templates/control/2fa/disable.html +++ b/templates/control/2fa/disable.html @@ -16,7 +16,7 @@

    Disable Two-Factor Authentication (2FA)

    - +
    diff --git a/templates/control/2fa/generate_recovery_codes.html b/templates/control/2fa/generate_recovery_codes.html index a313106e4..6a59f3444 100644 --- a/templates/control/2fa/generate_recovery_codes.html +++ b/templates/control/2fa/generate_recovery_codes.html @@ -6,13 +6,13 @@ $if error == "2fa":
    Whoops! The 2FA response token or recovery code you provided did not validate. - If entered, your recovery code was not consumed.
    $elif error == "verify":
    - Hey! Did you want to enable 2FA? You didn't check the verification checkbox! + Hey! Did you want to generate a new set of recovery codes? You didn't check the verification checkbox! If entered, your recovery code was not consumed.
    +

    Generate New Recovery Codes

    The following recovery codes are not yet active. Once you provide @@ -20,16 +20,16 @@

    Generate New Recovery Codes

    codes currently associated with your account.

    - Before proceeding, print or save these codes to a secure place. You will need these - codes to continue logging into your account. If you no longer have access to your - authenticator app, please disable 2FA instead, - then re-enable it to reinitialize your authenticator. + Before proceeding, print or save these codes to a secure place. In the event you lose access + to your authenticator app, you will need these codes to regain access to your Weasyl account. + If you no longer have access to your authenticator app, please disable 2FA + instead, then re-enable it to reinitialize your authenticator.

    -

    Your new recovery codes will be...

    +

    Your new recovery codes will be:

      $for code in tfa_recovery_codes: -
    1. ${code}
      2. +
    2. ${code[0:4] + ' ' + code[4:8] + ' ' + code[8:12] + ' ' + code[12:16] + ' ' + code[16:20]}

    These codes are one-time use, and upon successful use will not be able to be reused or retrieved. @@ -37,7 +37,7 @@

    Your new recovery codes will be...

    -
    +
    diff --git a/templates/control/2fa/init.html b/templates/control/2fa/init.html index c286fac71..715668bb5 100644 --- a/templates/control/2fa/init.html +++ b/templates/control/2fa/init.html @@ -2,31 +2,49 @@ $:{RENDER("common/stage_title.html", ["Enable 2FA: Step 1", "Two-Factor Authentication", "/control/2fa/status"])}
    - +
    $if error == "password":
    Whoops! You entered an incorrect password.

    Two-Factor Authentication

    +

    + This process will guide you through enabling Two-Factor Authentication (2FA), a method of enhancing the security of + your Weasyl account. For more information about 2FA, please visit the 2FA help page. As part of this + process, you will: +

      +
    1. Confirm that you have access to this account;
      2. +
    2. Set-up your authenticator application; and finally
      3. +
    3. Be presented a set of recovery codes used to regain access if you lose your authenticator.
      4. +
    +

    + +
    + + Link +

    Begin setting up 2FA

    +

    Username: $:{MARKDOWN_EXCERPT(username)}

    - You are about to enable Two-Factor Authentication (2FA) for the Weasyl account identified above. + You are about to enable 2FA for the Weasyl account identified above. In order to proceed through this process, you will need a compatible app such as - Google Authenticator or FreeOTP, + Google Authenticator for Android and iOS devices, + or OATH Toolkit for Linux, to generate time-based tokens each time you log into your Weasyl account.

    To begin, we need to confirm that you have control over this account. Please enter your password to continue.

    -

    Confirm your Password

    - - + + $:{CSRF()} - $:{CSRF()} + + -
    - Cancel - -
    - +
    + Cancel + +
    + +
    diff --git a/templates/control/2fa/init_qrcode.html b/templates/control/2fa/init_qrcode.html index c7908abdb..98fd55a2b 100644 --- a/templates/control/2fa/init_qrcode.html +++ b/templates/control/2fa/init_qrcode.html @@ -10,18 +10,20 @@

    Two-Factor Authentication

    Username: $:{MARKDOWN_EXCERPT(username)}

    In order to proceed from this point onwards, you will need a compatible app such as - Google Authenticator or FreeOTP, + Google Authenticator for Android and iOS devices, + or OATH Toolkit for Linux, to generate time-based tokens each time you log into your Weasyl account.

    -

    Scan with your authenticator...

    +

    Scan with your authenticator

    -

    ... or manually enter your secret key

    +

    Or manually enter your key

    - Your TOTP key is: ${tfa_secret} + Your TOTP secret key is: ${tfa_secret[0:4] + ' ' + tfa_secret[4:8] + ' ' + tfa_secret[8:12] + ' ' + tfa_secret[12:16]}

    + diff --git a/templates/control/2fa/init_verify.html b/templates/control/2fa/init_verify.html index 495be9c84..d97ac38ba 100644 --- a/templates/control/2fa/init_verify.html +++ b/templates/control/2fa/init_verify.html @@ -30,15 +30,16 @@

    Verify Receipt of Two-Factor Authentication (2FA) Recovery Codes

    Generating a new set of codes will prevent this from occurring. If this occurs, you will need to set-up 2FA again.

    -

    Your recovery codes are...

    +

    Your recovery codes are:

      $for code in tfa_recovery_codes: -
    1. ${code}
      2. +
    2. ${code[0:4] + ' ' + code[4:8] + ' ' + code[8:12] + ' ' + code[12:16] + ' ' + code[16:20]}

    These codes are one-time use, and upon successful use will not be able to be reused or retrieved. If and when used, mark each code off. Codes may be used in any order.

    +
    diff --git a/templates/etc/signin_2fa_auth.html b/templates/etc/signin_2fa_auth.html index 22d0487fb..faaf91fbe 100644 --- a/templates/etc/signin_2fa_auth.html +++ b/templates/etc/signin_2fa_auth.html @@ -13,15 +13,15 @@

    Two-Factor Authentication

    Enter the current time-based token for this user below. Or, if you no longer have access to the authenticator for this account, enter one of your unused twenty-character recovery codes. You have ${remaining_recovery_codes} - code${'s' if remaining_recovery_codes > 1 else ''} remaining. + recovery code${'s' if remaining_recovery_codes > 1 else ''} remaining.

    $if remaining_recovery_codes == 1:

    - Note: 2FA will be disabled upon providing your last recovery + Note: 2FA will be disabled upon using your last recovery code.

    - +
    diff --git a/templates/help/help.html b/templates/help/help.html index 56686a670..e89544ef4 100644 --- a/templates/help/help.html +++ b/templates/help/help.html @@ -12,21 +12,21 @@

    Weasyl Features

    We will be adding new topics to this section over time. Stay tuned!

    diff --git a/templates/help/two_factor_authentication.html b/templates/help/two_factor_authentication.html new file mode 100644 index 000000000..a1d17a436 --- /dev/null +++ b/templates/help/two_factor_authentication.html @@ -0,0 +1,103 @@ +$:{TITLE("Two Factor Authentication", "Help", "/help")} + +
    +
    + + Link +

    What is Two-Factor Authentication?

    + +
    +

    + Two-factor authentication (2FA) is a method to enhance the security of your account. By providing an additional + piece--or factor--of information during the authentication process only you know or have, you are strongly proving + that you are who you claim to be. A good example of 2FA is withdrawing money from an ATM as only you know + your PIN, and you have your debit card. +

    +

    + For further information on 2FA in general, Wikipedia's article on Multi-Factor Authentication + is a good starting point. +

    +
    + + Link +

    How can 2FA help protect my Weasyl account?

    + +
    +

    + By enabling 2FA, you will set-up an authenticator to generate a time-based password through an application + such as Google Authenticator. + Each time that you log into your Weasyl account, you will be prompted for the token shown on your authenticator + after your password is verified. Without both of these factors, you--or someone who has stolen your + password--cannot log into your account. +

    +
    + + Link +

    What if I can't access my authenticator?

    + +
    +

    + You will receive ten recovery codes as part of the 2FA setup process. Providing one of these codes + will function as an alternate means to log into your account. These codes should be printed or otherwise + saved in a secure manner, with the same diligence and care that you would take when safeguarding other personal information. +

    +

    + Each recovery code is usable only once, so please keep track of any used recovery codes. +

    +
    + + Link +

    How can I get a new set of recovery codes?

    + +
    +

    + You can generate a new set of 2FA recovery codes by clicking here. + You will be prompted to provide a currently valid time-based token from your authenticator app, or a valid recovery code + in order to verify that you are the legitimate user of your account. +

    +

    + You may view how many recovery codes remain available from the 2FA dashboard. You can + also see your count of remaining recovery codes during the login process. +

    +
    + + Link +

    What happens if I use all of my recovery codes?

    + +
    +

    + In the event you use all of your recovery codes during the login process, 2FA will be disabled for your Weasyl account. + This is done in order to prevent your account from becoming unavailable to you, as you would not be able to provide a + recovery code for future login sessions, or be able to disable 2FA. You may, however, re-enable 2FA + at any time. +

    +
    + + Link +

    How can I disable 2FA?

    + +
    +

    + If you wish to disable 2FA, you will need access to your authenticator or recovery codes to prove you have control + over your Weasyl account, and then visit the page to disable 2FA. +

    +
    + + Link +

    What happens if I no longer have access to both my authenticator and set of recovery codes?

    + +
    +

    + 2FA is a security mechanism. If you lose access to your authenticator and your recovery codes, Weasyl staff cannot + help you regain access to your account. As such, please ensure you keep your set of recovery codes in a safe location + and safeguard them with the level of care you would provide other important documents such as a passport or birth + certificate. +

    +

    + Storing recovery codes may be done either in paper form, in a secure digital manner using an electronic password + manager such as LastPass or KeePass, or both. +

    +
    + +
    +
    diff --git a/weasyl/controllers/decorators.py b/weasyl/controllers/decorators.py index 4f2de02dc..346638b63 100644 --- a/weasyl/controllers/decorators.py +++ b/weasyl/controllers/decorators.py @@ -6,7 +6,7 @@ from libweasyl import staff -from weasyl import define, errorcode +from weasyl import define, errorcode, two_factor_auth import weasyl.api from weasyl.error import WeasylError @@ -72,6 +72,22 @@ def inner(request): return inner +def twofactorauth_enabled_required(view_callable): + def inner(request): + if not two_factor_auth.is_2fa_enabled(request.userid): + raise WeasylError("TwoFactorAuthenticationRequireEnabled") + return view_callable(request) + return inner + + +def twofactorauth_disabled_required(view_callable): + def inner(request): + if two_factor_auth.is_2fa_enabled(request.userid): + raise WeasylError("TwoFactorAuthenticationRequireDisbled") + return view_callable(request) + return inner + + def token_checked(view_callable): def inner(request): if not weasyl.api.is_api_user() and request.params.get('token', "") != define.get_token(): diff --git a/weasyl/controllers/info.py b/weasyl/controllers/info.py index d7fa6d2ed..b0a996051 100644 --- a/weasyl/controllers/info.py +++ b/weasyl/controllers/info.py @@ -31,19 +31,19 @@ def staff_(request): title="Staff")) -def policy_tos_(request): - return Response(define.webpage(request.userid, "help/tos.html", - title="Terms of Service")) - - def thanks_(request): return Response(define.webpage(request.userid, "help/thanks.html", title="Awesome People")) -def policy_privacy_(request): - return Response(define.webpage(request.userid, "help/privacy.html", - title="Privacy Policy")) +def policy_community_(request): + return Response(define.webpage(request.userid, "help/community.html", + title="Community Guidelines")) + + +def policy_community_changes_(request): + return Response(define.webpage(request.userid, "help/community-changes.html", + title="Community Guidelines")) def policy_copyright_(request): @@ -51,19 +51,19 @@ def policy_copyright_(request): title="Copyright Policy")) +def policy_privacy_(request): + return Response(define.webpage(request.userid, "help/privacy.html", + title="Privacy Policy")) + + def policy_scoc_(request): return Response(define.webpage(request.userid, "help/scoc.html", title="Staff Code of Conduct")) -def policy_community_(request): - return Response(define.webpage(request.userid, "help/community.html", - title="Community Guidelines")) - - -def policy_community_changes_(request): - return Response(define.webpage(request.userid, "help/community-changes.html", - title="Community Guidelines")) +def policy_tos_(request): + return Response(define.webpage(request.userid, "help/tos.html", + title="Terms of Service")) # Help functions @@ -77,29 +77,24 @@ def help_about_(request): title="About Weasyl")) -def help_faq_(request): - return Response(define.webpage(request.userid, "help/faq.html", - title="FAQ")) - - def help_collections_(request): return Response(define.webpage(request.userid, "help/collections.html", title="Collections")) -def help_tagging_(request): - return Response(define.webpage(request.userid, "help/tagging.html", - title="Tagging")) +def help_faq_(request): + return Response(define.webpage(request.userid, "help/faq.html", + title="FAQ")) -def help_searching_(request): - return Response(define.webpage(request.userid, "help/searching.html", - title="Searching")) +def help_folders_(request): + return Response(define.webpage(request.userid, "help/folder-options.html", + title="Folder Options")) -def help_marketplace_(request): - return Response(define.webpage(request.userid, "help/marketplace.html", - title="Marketplace")) +def help_gdocs_(request): + return Response(define.webpage(request.userid, "help/gdocs.html", + title="Google Drive Embedding")) def help_markdown_(request): @@ -107,6 +102,11 @@ def help_markdown_(request): title="Markdown")) +def help_marketplace_(request): + return Response(define.webpage(request.userid, "help/marketplace.html", + title="Marketplace")) + + def help_ratings_(request): return Response(define.webpage(request.userid, "help/ratings.html", title="Content Ratings")) @@ -117,18 +117,23 @@ def help_ratings_changes_(request): title="Content Ratings")) -def help_gdocs_(request): - return Response(define.webpage(request.userid, "help/gdocs.html", - title="Google Drive Embedding")) - - -def help_folders_(request): - return Response(define.webpage(request.userid, "help/folder-options.html", - title="Folder Options")) - - @login_required def help_reports_(request): return Response(define.webpage(request.userid, "help/reports.html", [ report.select_reported_list(request.userid), ])) + + +def help_searching_(request): + return Response(define.webpage(request.userid, "help/searching.html", + title="Searching")) + + +def help_tagging_(request): + return Response(define.webpage(request.userid, "help/tagging.html", + title="Tagging")) + + +def help_two_factor_authentication_(request): + return Response(define.webpage(request.userid, "help/two_factor_authentication.html", + title="Two-Factor Authentication")) diff --git a/weasyl/controllers/routes.py b/weasyl/controllers/routes.py index a41ba054e..7f981a8dc 100644 --- a/weasyl/controllers/routes.py +++ b/weasyl/controllers/routes.py @@ -303,28 +303,31 @@ }), Route("/site-updates/{update_id:[0-9]+}/edit", "site_update_edit", admin.site_update_edit_), - Route("/policy/tos", "policy_tos", info.policy_tos_), - Route("/policy/privacy", "policy_privacy", info.policy_privacy_), - Route("/policy/copyright", "policy_copyright", info.policy_copyright_), - Route("/policy/scoc", "policy_scoc", info.policy_scoc_), Route("/policy/community", "policy_community", info.policy_community_), Route("/policy/community/changes", "policy_community_changes", info.policy_community_changes_), + Route("/policy/copyright", "policy_copyright", info.policy_copyright_), + Route("/policy/privacy", "policy_privacy", info.policy_privacy_), + Route("/policy/scoc", "policy_scoc", info.policy_scoc_), + Route("/policy/tos", "policy_tos", info.policy_tos_), Route("/staff", "staff", info.staff_), Route("/thanks", "thanks", info.thanks_), + + # Help page routes Route("/help", "help", info.help_), Route("/help/about", "help_about", info.help_about_), - Route("/help/faq", "help_faq", info.help_faq_), Route("/help/collections", "help_collections", info.help_collections_), - Route("/help/tagging", "help_tagging", info.help_tagging_), + Route("/help/faq", "help_faq", info.help_faq_), + Route("/help/folders", "help_folders", info.help_folders_), + Route("/help/google-drive-embed", "help_gdocs", info.help_gdocs_), Route("/help/markdown", "help_markdown", info.help_markdown_), - Route("/help/searching", "help_searching", info.help_searching_), Route("/help/marketplace", "help_marketplace", info.help_marketplace_), Route("/help/ratings", "help_ratings", info.help_ratings_), Route("/help/ratings/changes", "help_ratings_changes", info.help_ratings_changes_), - Route("/help/folders", "help_folders", info.help_folders_), - Route("/help/google-drive-embed", "help_gdocs", info.help_gdocs_), Route("/help/reports", "help_reports", info.help_reports_), + Route("/help/searching", "help_searching", info.help_searching_), + Route("/help/tagging", "help_tagging", info.help_tagging_), + Route("/help/two_factor_authentication", "help_two_factor_authentication", info.help_two_factor_authentication_), # OAuth2 routes. Route("/api/oauth2/authorize", "oauth2_authorize", diff --git a/weasyl/controllers/two_factor_auth.py b/weasyl/controllers/two_factor_auth.py index 6baae825b..8377d4ef0 100644 --- a/weasyl/controllers/two_factor_auth.py +++ b/weasyl/controllers/two_factor_auth.py @@ -9,32 +9,13 @@ from weasyl.controllers.decorators import ( login_required, token_checked, + twofactorauth_disabled_required, + twofactorauth_enabled_required, ) from weasyl.error import WeasylError from weasyl.profile import invalidate_other_sessions -def _error_if_2fa_enabled(userid): - """ - In lieu of a module-specific decorator, this function returns an error if 2FA is enabled, preventing the user - from self-wiping their own 2FA Secret (AKA, re-setting up 2FA while it is already enabled) - """ - if tfa.is_2fa_enabled(userid): - return Response(define.errorpage(userid, "2FA is already configured for this account.", [ - ["Go Back", "/control"], ["Return to the Home Page", "/"] - ])) - - -def _error_if_2fa_is_not_enabled(userid): - """ - In lieu of a module-specific decorator, this function returns an error if 2FA is not enabled. - """ - if not tfa.is_2fa_enabled(userid): - return Response(define.errorpage(userid, "2FA is not configured for this account.", [ - ["Go Back", "/control"], ["Return to the Home Page", "/"] - ])) - - @login_required def tfa_status_get_(request): return Response(define.webpage(request.userid, "control/2fa/status.html", [ @@ -43,11 +24,8 @@ def tfa_status_get_(request): @login_required +@twofactorauth_disabled_required def tfa_init_get_(request): - # Return an error if 2FA is already enabled (there's nothing to do in this route) - _error_if_2fa_enabled(request.userid) - - # Otherwise, render the page return Response(define.webpage(request.userid, "control/2fa/init.html", [ define.get_display_name(request.userid), None @@ -56,11 +34,8 @@ def tfa_init_get_(request): @login_required @token_checked +@twofactorauth_disabled_required def tfa_init_post_(request): - # Return an error if 2FA is already enabled (there's nothing to do in this route) - _error_if_2fa_enabled(request.userid) - - # Otherwise, process the form if request.params['action'] == "continue": userid, status = login.authenticate_bcrypt(define.get_display_name(request.userid), request.params['password'], session=False) @@ -88,16 +63,14 @@ def tfa_init_post_(request): @login_required +@twofactorauth_disabled_required def tfa_init_qrcode_get_(request): """ IMPLEMENTATION NOTE: This page cannot be accessed directly (HTTP GET), as the user has not yet verified ownership over the account by verifying their password. That said, be helpful and inform the user of this instead of erroring without explanation. """ - # Return an error if 2FA is already enabled (there's nothing to do in this route) - _error_if_2fa_enabled(request.userid) - - # If 2FA is not enabled, inform the user of where to go to begin + # Inform the user of where to go to begin return Response(define.errorpage( request.userid, """This page cannot be accessed directly, and must be accessed as part of the 2FA @@ -107,11 +80,8 @@ def tfa_init_qrcode_get_(request): @login_required @token_checked +@twofactorauth_disabled_required def tfa_init_qrcode_post_(request): - # Return an error if 2FA is already enabled (there's nothing to do in this route) - _error_if_2fa_enabled(request.userid) - - # Otherwise, process the form if request.params['action'] == "continue": # Strip any spaces from the TOTP code (some authenticators display the digits like '123 456') tfaresponse = request.params['tfaresponse'].replace(' ', '') @@ -136,6 +106,7 @@ def tfa_init_qrcode_post_(request): @login_required +@twofactorauth_disabled_required def tfa_init_verify_get_(request): """ IMPLEMENTATION NOTE: This page cannot be accessed directly (HTTP GET), as the user needs to both verify @@ -144,10 +115,7 @@ def tfa_init_verify_get_(request): of choice (`tfa_init_qrcode_*_()`). That said, be helpful and inform the user of this instead of erroring without explanation. """ - # Return an error if 2FA is already enabled (there's nothing to do in this route) - _error_if_2fa_enabled(request.userid) - - # If 2FA is not enabled, inform the user of where to go to begin + # Inform the user of where to go to begin return Response(define.errorpage( request.userid, """This page cannot be accessed directly, and must be accessed as part of the 2FA @@ -157,10 +125,8 @@ def tfa_init_verify_get_(request): @login_required @token_checked +@twofactorauth_disabled_required def tfa_init_verify_post_(request): - # Return an error if 2FA is already enabled (there's nothing to do in this route) - _error_if_2fa_enabled(request.userid) - # Extract parameters from the form action = request.params['action'] verify_checkbox = request.params['verify'] @@ -192,20 +158,16 @@ def tfa_init_verify_post_(request): @login_required +@twofactorauth_enabled_required def tfa_disable_get_(request): - # Return an error if 2FA is not enabled (there's nothing to do in this route) - _error_if_2fa_is_not_enabled(request.userid) - return Response(define.webpage(request.userid, "control/2fa/disable.html", [define.get_display_name(request.userid), None])) @login_required @token_checked +@twofactorauth_enabled_required def tfa_disable_post_(request): - # Return an error if 2FA is not enabled (there's nothing to do in this route) - _error_if_2fa_is_not_enabled(request.userid) - tfaresponse = request.params['tfaresponse'] verify_checkbox = request.params['verify'] action = request.params['action'] @@ -227,10 +189,8 @@ def tfa_disable_post_(request): @login_required +@twofactorauth_enabled_required def tfa_generate_recovery_codes_get_(request): - # Return an error if 2FA is not enabled (there's nothing to do in this route) - _error_if_2fa_is_not_enabled(request.userid) - return Response(define.webpage(request.userid, "control/2fa/generate_recovery_codes.html", [ tfa.generate_recovery_codes(), None @@ -238,11 +198,9 @@ def tfa_generate_recovery_codes_get_(request): @login_required +@twofactorauth_enabled_required @token_checked def tfa_generate_recovery_codes_post_(request): - # Return an error if 2FA is not enabled (there's nothing to do in this route) - _error_if_2fa_is_not_enabled(request.userid) - # Extract parameters from the form action = request.params['action'] verify_checkbox = request.params['verify'] diff --git a/weasyl/errorcode.py b/weasyl/errorcode.py index f4757a872..171c07466 100644 --- a/weasyl/errorcode.py +++ b/weasyl/errorcode.py @@ -149,6 +149,8 @@ "The link you followed does not appear to be valid. You may have already added these premium terms " "to your account, or you may have copied the link incorrectly."), "tooManyPreferenceTags": "You cannot have more than 50 preference tags.", + "TwoFactorAuthenticationRequireEnabled": "Two-Factor Authentication must be enabled to access this page.", + "TwoFactorAuthenticationRequireDisbled": "Two-Factor Authentication must not be enabled to access this page.", "unknownMessageFolder": "The specified message folder does not exist.", "UserIgnored": "This content was posted by a user you have chosen to ignore.", "userRecordMissing": userid, From e057a7f7808a266edbefec3eba214b0adc35f238 Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Sun, 12 Mar 2017 21:17:55 -0400 Subject: [PATCH 07/22] Minor formatting tweaks --- templates/control/2fa/init_verify.html | 4 +-- templates/control/2fa/status.html | 35 +++++++++++++++++++------- weasyl/controllers/decorators.py | 8 ++++++ weasyl/controllers/two_factor_auth.py | 2 +- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/templates/control/2fa/init_verify.html b/templates/control/2fa/init_verify.html index d97ac38ba..ee727662c 100644 --- a/templates/control/2fa/init_verify.html +++ b/templates/control/2fa/init_verify.html @@ -21,9 +21,7 @@

    Verify Receipt of Two-Factor Authentication (2FA) Recovery Codes

    Warning: For security reasons, Weasyl staff will be unable to assist you if - you lose access to your recovery codes. We advise never using more than eight codes, as you will - need two to disable 2FA from a logged-out state (one code to log in, and one code to disable - two-factor authentication). + you lose access to your recovery codes.
    As a precaution against being locked out your account if all recovery codes are used, 2FA will automatically be disabled if--and only if--all recovery codes are consumed during the login process. diff --git a/templates/control/2fa/status.html b/templates/control/2fa/status.html index 830641973..9f78f4a0b 100644 --- a/templates/control/2fa/status.html +++ b/templates/control/2fa/status.html @@ -9,31 +9,48 @@

    Two-Factor Authentication (2FA) Dashboard

    When logging in, you will need to provide your password--which is something you know--and additionally a time-based token, which is something you have. Without both pieces--or factors--you or an adversary who has compromised your password will not be able to fully - authenticate to your account. + authenticate to your account. For more information about 2FA, please visit the + 2FA help page.

    2FA Status

    $if tfa_enabled:

    2FA is currently enabled.

    + +
    + +

    Recovery Code Status

    - Click here to disable 2FA + You currently have ${tfa_recovery_codes_count} valid recovery code${'s' if tfa_recovery_codes_count > 1 else ''} + assigned to your account. We advise not relying on recovery codes as a routine access method and to, if required, disable + and re-enable 2FA to reinitialize your authenticator app to provide time-based tokens.

    -

    Recovery Code Status

    - You currently have ${tfa_recovery_codes_count} valid recovery code${'s' if tfa_recovery_codes_count > 1 else ''} assigned to your account. - We advise not relying on recovery codes as a routine access method, and further advise to never - use the last two codes; a minimum of two codes are required to disable 2FA from a logged-out state. +

    +

    + +
    + +

    Disable 2FA

    +

    + You may disable 2FA at any time, however this will remove a layer of security for your Weasyl account.

    - Click here to get new recovery codes +

    $else:

    - 2FA is currently disabled.

    + 2FA is currently disabled.

    - Click here to enable 2FA +

    diff --git a/weasyl/controllers/decorators.py b/weasyl/controllers/decorators.py index 346638b63..d5e80fa4c 100644 --- a/weasyl/controllers/decorators.py +++ b/weasyl/controllers/decorators.py @@ -73,6 +73,10 @@ def inner(request): def twofactorauth_enabled_required(view_callable): + """ + This decorator requires that 2FA be enabled for a given Weasyl account as identified + by ``request.userid``. + """ def inner(request): if not two_factor_auth.is_2fa_enabled(request.userid): raise WeasylError("TwoFactorAuthenticationRequireEnabled") @@ -81,6 +85,10 @@ def inner(request): def twofactorauth_disabled_required(view_callable): + """ + This decorator requires that 2FA be disabled for a given Weasyl account as identified + by ``request.userid``. + """ def inner(request): if two_factor_auth.is_2fa_enabled(request.userid): raise WeasylError("TwoFactorAuthenticationRequireDisbled") diff --git a/weasyl/controllers/two_factor_auth.py b/weasyl/controllers/two_factor_auth.py index 8377d4ef0..6f05f7123 100644 --- a/weasyl/controllers/two_factor_auth.py +++ b/weasyl/controllers/two_factor_auth.py @@ -198,8 +198,8 @@ def tfa_generate_recovery_codes_get_(request): @login_required -@twofactorauth_enabled_required @token_checked +@twofactorauth_enabled_required def tfa_generate_recovery_codes_post_(request): # Extract parameters from the form action = request.params['action'] From feefea562381387074733e300cf3463c62f9ca5e Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Mon, 13 Mar 2017 06:14:49 -0400 Subject: [PATCH 08/22] Escape displayed username properly Use (already escaped) versus any $:{foo} (unescaped), $:{MARKDOWN_EXCERPT(foo)} or other variants. --- templates/control/2fa/disable.html | 2 +- templates/control/2fa/init.html | 2 +- templates/control/2fa/init_qrcode.html | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/control/2fa/disable.html b/templates/control/2fa/disable.html index 59373325d..7ad5232a4 100644 --- a/templates/control/2fa/disable.html +++ b/templates/control/2fa/disable.html @@ -8,7 +8,7 @@ $elif error == "verify":
    Hey! Did you want to disable 2FA? You didn't check the verification checkbox!

    Disable Two-Factor Authentication (2FA)

    -

    Username: $:{MARKDOWN_EXCERPT(username)}

    +

    Username: ${username}

    You are about to disable 2FA for the above account, and remove the security which it provides. If you wish to re-enable 2FA, you will diff --git a/templates/control/2fa/init.html b/templates/control/2fa/init.html index 715668bb5..50e5bf978 100644 --- a/templates/control/2fa/init.html +++ b/templates/control/2fa/init.html @@ -23,7 +23,7 @@

    Two-Factor Authentication

    Link

    Begin setting up 2FA

    -

    Username: $:{MARKDOWN_EXCERPT(username)}

    +

    Username: ${username}

    You are about to enable 2FA for the Weasyl account identified above. In order to proceed through this process, you will need a compatible app such as diff --git a/templates/control/2fa/init_qrcode.html b/templates/control/2fa/init_qrcode.html index 98fd55a2b..a83f27051 100644 --- a/templates/control/2fa/init_qrcode.html +++ b/templates/control/2fa/init_qrcode.html @@ -7,7 +7,7 @@

    Whoops! The 2FA response token you provided did not validate.

    Two-Factor Authentication

    -

    Username: $:{MARKDOWN_EXCERPT(username)}

    +

    Username: ${username}

    In order to proceed from this point onwards, you will need a compatible app such as Google Authenticator for Android and iOS devices, From 3e608e6178125749655bedbfaa7ebb2d1c48e97a Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Wed, 15 Mar 2017 19:18:16 -0400 Subject: [PATCH 09/22] Shift login code into signin() function Leverage the login.signin() function to prevent duplication of code between the 2FA signin process and the normal login process, which should both ultimately result in logging a successful login and incrementing logins. --- weasyl/login.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/weasyl/login.py b/weasyl/login.py index cea3410e3..1226f571f 100644 --- a/weasyl/login.py +++ b/weasyl/login.py @@ -23,6 +23,10 @@ def signin(userid): # Update the last login record for the user d.execute("UPDATE login SET last_login = %i WHERE userid = %i", [d.get_time(), userid]) + # Log the successful login and increment the login count + d.append_to_log('login.success', userid=userid, ip=d.get_address()) + d.metric('increment', 'logins') + # set the userid on the session sess = d.get_weasyl_session() sess.userid = userid @@ -105,8 +109,6 @@ def authenticate_bcrypt(username, password, session=True): return USERID, "2fa" else: signin(USERID) - d.append_to_log('login.success', userid=USERID, ip=d.get_address()) - d.metric('increment', 'logins') status = None if not unicode_success: From 397d9b68ffb9d1aed5ceed772846d617c5538b73 Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Fri, 17 Mar 2017 18:59:44 -0400 Subject: [PATCH 10/22] Also check for zero recovery codes after password authentication. Granted, this code block should *never* execute, outside of webtests, but still. Cover all edge cases. Use a Sentry-detectable RuntimeError for this case. --- weasyl/controllers/user.py | 19 ++++++++++++------- weasyl/errorcode.py | 3 +++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/weasyl/controllers/user.py b/weasyl/controllers/user.py index ac51c5bb3..b95aed378 100644 --- a/weasyl/controllers/user.py +++ b/weasyl/controllers/user.py @@ -15,6 +15,7 @@ login_required, token_checked, ) +from weasyl.error import WeasylError # Session management functions @@ -48,6 +49,15 @@ def signin_post_(request): if form.sfwmode == "sfw": request.set_cookie_on_response("sfwmode", "sfw", 31536000) index.template_fields.invalidate(logid) + # Check if out of recovery codes; this should *never* execute normally, save for crafted + # webtests, but just to cover all edge cases, handle it here if necessary. + remaining_recovery_codes = two_factor_auth.get_number_of_recovery_codes(logid) + if remaining_recovery_codes == 0: + login.signin(logid) + two_factor_auth.force_deactivate(logid) + raise RuntimeError("Two-factor Authentication: Count of recovery codes for userid " + + str(logid) + " was zero upon password authentication succeeding, " + + "which should be impossible. 2FA was disabled for this user's account.") # Store the authenticated userid & password auth time to the session sess = define.get_weasyl_session() sess.additional_data['2fa_pwd_auth_timestamp'] = arrow.now().timestamp @@ -56,7 +66,7 @@ def signin_post_(request): return Response(define.webpage( request.userid, "etc/signin_2fa_auth.html", - [define.get_display_name(logid), form.referer, two_factor_auth.get_number_of_recovery_codes(logid), + [define.get_display_name(logid), form.referer, remaining_recovery_codes, None])) elif logerror == "invalid": return Response(define.webpage(request.userid, "etc/signin.html", [True, form.referer])) @@ -139,12 +149,7 @@ def signin_2fa_auth_post_(request): # User is out of recovery codes, so force-deactivate 2FA if two_factor_auth.get_number_of_recovery_codes(tfa_userid) == 0: two_factor_auth.force_deactivate(tfa_userid) - return Response(define.errorpage( - tfa_userid, - """You have used all of your 2FA recovery codes. In order to prevent you from - being locked out of your account, 2FA has been disabled for your account.""", - [["Re-Enable 2FA", "/control/2fa/init"], ["Continue", ref]] - )) + raise WeasylError("TwoFactorAuthenticationZeroRecoveryCodesRemaining") raise HTTPSeeOther(location=ref) else: # 2FA failed; redirect to 2FA input page & inform user that authentication failed. diff --git a/weasyl/errorcode.py b/weasyl/errorcode.py index 171c07466..0257e5640 100644 --- a/weasyl/errorcode.py +++ b/weasyl/errorcode.py @@ -151,6 +151,9 @@ "tooManyPreferenceTags": "You cannot have more than 50 preference tags.", "TwoFactorAuthenticationRequireEnabled": "Two-Factor Authentication must be enabled to access this page.", "TwoFactorAuthenticationRequireDisbled": "Two-Factor Authentication must not be enabled to access this page.", + "TwoFactorAuthenticationZeroRecoveryCodesRemaining": ( + "Your account had zero recovery codes remaining, and as such 2FA was disabled to prevent " + "you from being permanently unable to log into your account. You may re-enable 2FA if you desire to do so."), "unknownMessageFolder": "The specified message folder does not exist.", "UserIgnored": "This content was posted by a user you have chosen to ignore.", "userRecordMissing": userid, From 7cdff33d2940711337397e2f075fd3dac96626c3 Mon Sep 17 00:00:00 2001 From: Kyra Date: Sun, 19 Mar 2017 01:21:07 -0400 Subject: [PATCH 11/22] Fail-secure for accounts out of 2FA recovery codes Don't default to force-disabling 2FA if an account is determined to have zero recovery codes remaining. Raising a Sentry-detectable error will allow any edge cases to be investigated and handled if needed. --- weasyl/controllers/user.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/weasyl/controllers/user.py b/weasyl/controllers/user.py index b95aed378..597bab513 100644 --- a/weasyl/controllers/user.py +++ b/weasyl/controllers/user.py @@ -50,14 +50,12 @@ def signin_post_(request): request.set_cookie_on_response("sfwmode", "sfw", 31536000) index.template_fields.invalidate(logid) # Check if out of recovery codes; this should *never* execute normally, save for crafted - # webtests, but just to cover all edge cases, handle it here if necessary. + # webtests. However, check for it and log an error to Sentry if it happens. remaining_recovery_codes = two_factor_auth.get_number_of_recovery_codes(logid) if remaining_recovery_codes == 0: - login.signin(logid) - two_factor_auth.force_deactivate(logid) raise RuntimeError("Two-factor Authentication: Count of recovery codes for userid " + str(logid) + " was zero upon password authentication succeeding, " + - "which should be impossible. 2FA was disabled for this user's account.") + "which should be impossible.") # Store the authenticated userid & password auth time to the session sess = define.get_weasyl_session() sess.additional_data['2fa_pwd_auth_timestamp'] = arrow.now().timestamp From 94086e6fe7638924d74925f330b917c5a06f03be Mon Sep 17 00:00:00 2001 From: Kyra Date: Fri, 24 Mar 2017 20:13:07 -0400 Subject: [PATCH 12/22] Synchronize error messages [skip ci] L109 did not match L138. Use 'authentication' over 'login'. Technically, it is more accurate in this instance. --- weasyl/controllers/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/weasyl/controllers/user.py b/weasyl/controllers/user.py index 597bab513..a74e39c19 100644 --- a/weasyl/controllers/user.py +++ b/weasyl/controllers/user.py @@ -106,7 +106,7 @@ def signin_2fa_auth_get_(request): sess.save = True return Response(define.errorpage( request.userid, - "Your login session has timed out. Please try logging in again.", + "Your authentication session has timed out. Please try logging in again.", [["Sign In", "/signin"], ["Return to the Home Page", "/"]])) else: ref = request.params["referer"] if "referer" in request.params else "/" From f393127715d78a6ab78320224638dcaaa5316093 Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Sat, 25 Mar 2017 20:35:45 -0400 Subject: [PATCH 13/22] Enforce a maximum number of 2FA auth attempts before requiring a fresh signin. Hinder brute forcing either the 2FA token or recovery code by enforcing an upper-limit before the 2FA session is ended, and a new password authentication is required. Shift the 2FA session cleanup to a separate function to consolidate reused code. Shift error messages into the proper home of errorcode.error_messages to standardize displayed text. --- weasyl/controllers/user.py | 51 ++++++++++++++++++++++++++++---------- weasyl/errorcode.py | 3 +++ 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/weasyl/controllers/user.py b/weasyl/controllers/user.py index a74e39c19..03772b6a0 100644 --- a/weasyl/controllers/user.py +++ b/weasyl/controllers/user.py @@ -58,8 +58,12 @@ def signin_post_(request): "which should be impossible.") # Store the authenticated userid & password auth time to the session sess = define.get_weasyl_session() + # The timestamp at which password authentication succeeded sess.additional_data['2fa_pwd_auth_timestamp'] = arrow.now().timestamp + # The userid of the user attempting authentication sess.additional_data['2fa_pwd_auth_userid'] = logid + # The number of times the user has attempted to authenticate via 2FA + sess.additional_data['2fa_pwd_auth_attempts'] = 0 sess.save = True return Response(define.webpage( request.userid, @@ -89,6 +93,21 @@ def signin_post_(request): return Response(define.errorpage(request.userid)) +def _cleanup_2fa_session(): + """ + Cleans up a Weasyl session of any 2FA data stored during the authentication process. + + Parameters: None; keys off of the currently active session making the request. + + Returns: Nothing. + """ + sess = define.get_weasyl_session() + del sess.additional_data['2fa_pwd_auth_timestamp'] + del sess.additional_data['2fa_pwd_auth_userid'] + del sess.additional_data['2fa_pwd_auth_attempts'] + sess.save = True + + @guest_required def signin_2fa_auth_get_(request): sess = define.get_weasyl_session() @@ -101,12 +120,10 @@ def signin_2fa_auth_get_(request): # Maximum secondary authentication time: 5 minutes session_life = arrow.now().timestamp - sess.additional_data['2fa_pwd_auth_timestamp'] if session_life > 300: - del sess.additional_data['2fa_pwd_auth_timestamp'] - del sess.additional_data['2fa_pwd_auth_userid'] - sess.save = True + _cleanup_2fa_session() return Response(define.errorpage( request.userid, - "Your authentication session has timed out. Please try logging in again.", + errorcode.error_messages['TwoFactorAuthenticationAuthenticationTimeout'], [["Sign In", "/signin"], ["Return to the Home Page", "/"]])) else: ref = request.params["referer"] if "referer" in request.params else "/" @@ -127,21 +144,18 @@ def signin_2fa_auth_post_(request): return Response(define.errorpage(request.userid, errorcode.permission)) tfa_userid = sess.additional_data['2fa_pwd_auth_userid'] - # Maximum secondary authentication time: 5 minutes session_life = arrow.now().timestamp - sess.additional_data['2fa_pwd_auth_timestamp'] if session_life > 300: - del sess.additional_data['2fa_pwd_auth_timestamp'] - del sess.additional_data['2fa_pwd_auth_userid'] - sess.save = True + # Maximum secondary authentication time: 5 minutes + _cleanup_2fa_session() return Response(define.errorpage( request.userid, - "Your authentication session has timed out. Please try logging in again.", - [["Sign In", "/signin"], ["Return to the Home Page", "/"]])) + errorcode.error_messages['TwoFactorAuthenticationAuthenticationTimeout'], + [["Sign In", "/signin"], ["Return to the Home Page", "/"]] + )) elif two_factor_auth.verify(tfa_userid, request.params["tfaresponse"]): # 2FA passed, so login and cleanup. - del sess.additional_data['2fa_pwd_auth_timestamp'] - del sess.additional_data['2fa_pwd_auth_userid'] - sess.save = True + _cleanup_2fa_session() login.signin(tfa_userid) ref = request.params["referer"] or "/" # User is out of recovery codes, so force-deactivate 2FA @@ -149,7 +163,18 @@ def signin_2fa_auth_post_(request): two_factor_auth.force_deactivate(tfa_userid) raise WeasylError("TwoFactorAuthenticationZeroRecoveryCodesRemaining") raise HTTPSeeOther(location=ref) + elif sess.additional_data['2fa_pwd_auth_attempts'] >= 5: + # Hinder brute-forcing the 2FA token or recovery code by enforcing an upper-bound on 2FA auth attempts. + _cleanup_2fa_session() + return Response(define.errorpage( + request.userid, + errorcode.error_messages['TwoFactorAuthenticationAuthenticationAttemptsExceeded'], + [["Sign In", "/signin"], ["Return to the Home Page", "/"]] + )) else: + # Log the failed authentication attempt to the session and save + sess.additional_data['2fa_pwd_auth_attempts'] += 1 + sess.save = True # 2FA failed; redirect to 2FA input page & inform user that authentication failed. return Response(define.webpage( request.userid, diff --git a/weasyl/errorcode.py b/weasyl/errorcode.py index 0257e5640..0f597ae95 100644 --- a/weasyl/errorcode.py +++ b/weasyl/errorcode.py @@ -149,6 +149,9 @@ "The link you followed does not appear to be valid. You may have already added these premium terms " "to your account, or you may have copied the link incorrectly."), "tooManyPreferenceTags": "You cannot have more than 50 preference tags.", + "TwoFactorAuthenticationAuthenticationAttemptsExceeded": ( + "You have incorrectly entered your 2FA token or recovery code too many times. Please try logging in again."), + "TwoFactorAuthenticationAuthenticationTimeout": "Your authentication session has timed out. Please try logging in again.", "TwoFactorAuthenticationRequireEnabled": "Two-Factor Authentication must be enabled to access this page.", "TwoFactorAuthenticationRequireDisbled": "Two-Factor Authentication must not be enabled to access this page.", "TwoFactorAuthenticationZeroRecoveryCodesRemaining": ( From 642331dbc5c914b46c45c3046ad8f552fec9d937 Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Sat, 25 Mar 2017 22:34:51 -0400 Subject: [PATCH 14/22] Fix key errors when 'verify' checkbox not in request.params Actually permit the logic prompting the user for verification to be triggered if the user intentionally doesn't check the checkbox (by altering HTML to remove the 'required' attribute), or if the browser for some reason doesn't enforce the required attribute. Fix a stray unclosed HTML tag in a template. --- templates/control/2fa/generate_recovery_codes.html | 4 ++-- weasyl/controllers/two_factor_auth.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/templates/control/2fa/generate_recovery_codes.html b/templates/control/2fa/generate_recovery_codes.html index 6a59f3444..d93e5c02f 100644 --- a/templates/control/2fa/generate_recovery_codes.html +++ b/templates/control/2fa/generate_recovery_codes.html @@ -9,8 +9,8 @@

    $elif error == "verify":
    - Hey! Did you want to generate a new set of recovery codes? You didn't check the verification checkbox! - If entered, your recovery code was not consumed. + Hey! Did you want to generate a new set of recovery codes? You didn't check the verification checkbox! + If entered, your recovery code was not consumed.

    Generate New Recovery Codes

    diff --git a/weasyl/controllers/two_factor_auth.py b/weasyl/controllers/two_factor_auth.py index 6f05f7123..661fb11dd 100644 --- a/weasyl/controllers/two_factor_auth.py +++ b/weasyl/controllers/two_factor_auth.py @@ -129,7 +129,7 @@ def tfa_init_verify_get_(request): def tfa_init_verify_post_(request): # Extract parameters from the form action = request.params['action'] - verify_checkbox = request.params['verify'] + verify_checkbox = True if 'verify' in request.params else False tfasecret = request.params['tfasecret'] tfaresponse = request.params['tfaresponse'] tfarecoverycodes = request.params['tfarecoverycodes'] @@ -169,7 +169,7 @@ def tfa_disable_get_(request): @twofactorauth_enabled_required def tfa_disable_post_(request): tfaresponse = request.params['tfaresponse'] - verify_checkbox = request.params['verify'] + verify_checkbox = True if 'verify' in request.params else False action = request.params['action'] if action == "disable" and verify_checkbox: @@ -203,7 +203,7 @@ def tfa_generate_recovery_codes_get_(request): def tfa_generate_recovery_codes_post_(request): # Extract parameters from the form action = request.params['action'] - verify_checkbox = request.params['verify'] + verify_checkbox = True if 'verify' in request.params else False tfaresponse = request.params['tfaresponse'] tfarecoverycodes = request.params['tfarecoverycodes'] From cc1c04e4c6648b0076853900cd19e573805f08d8 Mon Sep 17 00:00:00 2001 From: Kyra Date: Mon, 27 Mar 2017 21:57:58 -0400 Subject: [PATCH 15/22] Store recovery codes as bcrypt'd hashes (#18) Because they really should be hashed instead of just being stored plaintext. Uses a slightly lower work-factor for the bcrypt rounds*, but it is configurable directly via a module-level variable. * As compared to straight password authentication's work-factor. --- .../versions/abeefecabdad_implement_2fa.py | 2 +- libweasyl/libweasyl/models/tables.py | 2 +- weasyl/controllers/user.py | 6 +- weasyl/test/test_two_factor_auth.py | 88 +++++++++---------- weasyl/two_factor_auth.py | 49 ++++++----- 5 files changed, 74 insertions(+), 73 deletions(-) diff --git a/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py b/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py index 5f79752b2..be43e24c0 100644 --- a/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py +++ b/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py @@ -19,7 +19,7 @@ def upgrade(): # Create a table to store 2FA backup codes for use when the authenticator is unavailable. op.create_table('twofa_recovery_codes', sa.Column('userid', sa.Integer(), nullable=False), - sa.Column('recovery_code', sa.String(length=20), nullable=False), + sa.Column('recovery_code', sa.String(length=100), nullable=False), sa.PrimaryKeyConstraint('userid', 'recovery_code'), sa.ForeignKeyConstraint(['userid'], ['login.userid'], name='twofa_recovery_codes_userid_fkey', onupdate='CASCADE', ondelete='CASCADE'), ) diff --git a/libweasyl/libweasyl/models/tables.py b/libweasyl/libweasyl/models/tables.py index 07a4f2fb7..5ff13f542 100644 --- a/libweasyl/libweasyl/models/tables.py +++ b/libweasyl/libweasyl/models/tables.py @@ -400,7 +400,7 @@ def default_fkey(*args, **kwargs): twofa_recovery_codes = Table( 'twofa_recovery_codes', metadata, Column('userid', Integer(), primary_key=True, nullable=False), - Column('recovery_code', String(length=20), primary_key=True, nullable=False), + Column('recovery_code', String(length=100), primary_key=True, nullable=False), default_fkey(['userid'], ['login.userid'], name='twofa_recovery_codes_userid_fkey'), ) diff --git a/weasyl/controllers/user.py b/weasyl/controllers/user.py index 03772b6a0..07943b2b8 100644 --- a/weasyl/controllers/user.py +++ b/weasyl/controllers/user.py @@ -161,7 +161,11 @@ def signin_2fa_auth_post_(request): # User is out of recovery codes, so force-deactivate 2FA if two_factor_auth.get_number_of_recovery_codes(tfa_userid) == 0: two_factor_auth.force_deactivate(tfa_userid) - raise WeasylError("TwoFactorAuthenticationZeroRecoveryCodesRemaining") + return Response(define.errorpage( + tfa_userid, + errorcode.error_messages['TwoFactorAuthenticationZeroRecoveryCodesRemaining'], + [["2FA Dashboard", "/control/2fa/status"], ["Return to the Home Page", "/"]] + )) raise HTTPSeeOther(location=ref) elif sess.additional_data['2fa_pwd_auth_attempts'] >= 5: # Hinder brute-forcing the 2FA token or recovery code by enforcing an upper-bound on 2FA auth attempts. diff --git a/weasyl/test/test_two_factor_auth.py b/weasyl/test/test_two_factor_auth.py index 86b195b3f..865f53b05 100644 --- a/weasyl/test/test_two_factor_auth.py +++ b/weasyl/test/test_two_factor_auth.py @@ -3,6 +3,7 @@ import re import urllib +import bcrypt import pyotp import pytest from qrcodegen import QrCode @@ -13,6 +14,19 @@ from weasyl.test import db_utils +recovery_code = "A" * tfa.LENGTH_RECOVERY_CODE +recovery_code_hashed = bcrypt.hashpw(recovery_code.encode('utf-8'), bcrypt.gensalt(tfa.BCRYPT_WORK_FACTOR)) + + +@pytest.mark.usefixtures('db') +def _insert_recovery_code(userid): + """Insert the test-suite's pre-hashed recovery code""" + d.engine.execute(""" + INSERT INTO twofa_recovery_codes (userid, recovery_code) + VALUES (%(userid)s, %(code)s) + """, userid=userid, code=recovery_code_hashed) + + @pytest.mark.usefixtures('db') def test_get_number_of_recovery_codes(): user_id = db_utils.create_user() @@ -24,10 +38,7 @@ def test_get_number_of_recovery_codes(): WHERE userid = %(userid)s """, userid=user_id) assert tfa.get_number_of_recovery_codes(user_id) == 0 - d.engine.execute(""" - INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES (%(userid)s, %(code)s) - """, userid=user_id, code=security.generate_key(tfa.LENGTH_RECOVERY_CODE)) + _insert_recovery_code(user_id) assert tfa.get_number_of_recovery_codes(user_id) == 1 d.engine.execute(""" DELETE FROM twofa_recovery_codes @@ -46,11 +57,8 @@ def test_generate_recovery_codes(): def test_store_recovery_codes(): user_id = db_utils.create_user() valid_code_string = "01234567890123456789,02234567890123456789,03234567890123456789,04234567890123456789,05234567890123456789,06234567890123456789,07234567890123456789,08234567890123456789,09234567890123456789,10234567890123456789" - recovery_code = "A" * tfa.LENGTH_RECOVERY_CODE - d.engine.execute(""" - INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES (%(userid)s, %(code)s) - """, userid=user_id, code=recovery_code) + + _insert_recovery_code(user_id) # store_recovery_codes() will not accept a string of codes where the total code count is not 10 invalid_codes = valid_code_string.split(',').pop() @@ -60,31 +68,33 @@ def test_store_recovery_codes(): invalid_codes = "01,02,03,04,05,06,07,08,09,10" assert not tfa.store_recovery_codes(user_id, invalid_codes) - # When a correct code list is provides, the codes will be stored successfully in the database + # When a correct code list is provided, the codes will be stored successfully in the database assert tfa.store_recovery_codes(user_id, valid_code_string) + # Extract the current hashed recovery codes query = d.engine.execute(""" SELECT recovery_code FROM twofa_recovery_codes WHERE userid = %(userid)s """, userid=user_id).fetchall() - # Verify that the target codes were added, and the originally stored code does not remain - assert recovery_code not in query + # Ensure that the recovery codes can be hashed to the corresponding bcrypt hash valid_code_list = valid_code_string.split(',') - for code in query: - assert len(code['recovery_code']) == tfa.LENGTH_RECOVERY_CODE - assert code['recovery_code'] in valid_code_list + for row in query: + code_status = False + for code in valid_code_list: + if bcrypt.checkpw(code.encode('utf-8'), row['recovery_code'].encode('utf-8')): + # If the code matches the hash, then the recovery code stored successfully + code_status = True + break + # The code must be valid + assert code_status @pytest.mark.usefixtures('db') def test_is_recovery_code_valid(): user_id = db_utils.create_user() - recovery_code = "A" * tfa.LENGTH_RECOVERY_CODE - d.engine.execute(""" - INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES (%(userid)s, %(code)s) - """, userid=user_id, code=recovery_code) + _insert_recovery_code(user_id) # Failure: Recovery code is invalid (code was not a real code) assert not tfa.is_recovery_code_valid(user_id, "z" * tfa.LENGTH_RECOVERY_CODE) @@ -99,10 +109,7 @@ def test_is_recovery_code_valid(): assert not tfa.is_recovery_code_valid(user_id, recovery_code) # Reinsert for case-sensitivity test - d.engine.execute(""" - INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES (%(userid)s, %(code)s) - """, userid=user_id, code=recovery_code) + _insert_recovery_code(user_id) # Success: Recovery code is valid (because case does not matter) assert tfa.is_recovery_code_valid(user_id, recovery_code.lower()) @@ -210,11 +217,8 @@ def test_deactivate(): WHERE userid = %(userid)s """, userid=user_id, tfas=tfa_secret) tfa_response = totp.now() - recovery_code = "A" * tfa.LENGTH_RECOVERY_CODE - d.engine.execute(""" - INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES (%(userid)s, %(code)s) - """, userid=user_id, code=recovery_code) + + _insert_recovery_code(user_id) assert tfa.deactivate(user_id, recovery_code) # 2FA enabled, failed deactivation (invalid `tfa_response` (code or TOTP token)) @@ -236,11 +240,8 @@ def test_force_deactivate(): SET twofa_secret = %(tfas)s WHERE userid = %(userid)s """, userid=user_id, tfas=tfa_secret) - recovery_code = "A" * tfa.LENGTH_RECOVERY_CODE - d.engine.execute(""" - INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES (%(userid)s, %(code)s) - """, userid=user_id, code=recovery_code) + + _insert_recovery_code(user_id) # Verify that force_deactivate() functions as expected. assert tfa.is_2fa_enabled(user_id) @@ -257,16 +258,13 @@ def test_verify(): user_id = db_utils.create_user() tfa_secret = pyotp.random_base32() totp = pyotp.TOTP(tfa_secret) - recovery_code = "A" * tfa.LENGTH_RECOVERY_CODE + d.engine.execute(""" UPDATE login SET twofa_secret = %(tfas)s WHERE userid = %(userid)s """, userid=user_id, tfas=tfa_secret) - d.engine.execute(""" - INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES (%(userid)s, %(code)s) - """, userid=user_id, code=recovery_code) + _insert_recovery_code(user_id) # Codes of any other length than tfa.LENGTH_TOTP_CODE or tfa.LENGTH_RECOVERY_CODE returns False assert not tfa.verify(user_id, "a" * 5) @@ -293,20 +291,14 @@ def test_verify(): assert tfa.verify(user_id, recovery_code) # Recovery codes are case-insensitive (Successful Verification) - d.engine.execute(""" - INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES (%(userid)s, %(code)s) - """, userid=user_id, code=recovery_code) - assert tfa.verify(user_id, 'a' * tfa.LENGTH_RECOVERY_CODE) + _insert_recovery_code(user_id) + assert tfa.verify(user_id, recovery_code.lower()) # Recovery codes are consumed upon use (consumed previously) (Unsuccessful Verification) assert not tfa.verify(user_id, recovery_code) # When parameter `consume_recovery_code` is set to False, a recovery code is not consumed. - d.engine.execute(""" - INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES (%(userid)s, %(code)s) - """, userid=user_id, code=recovery_code) + _insert_recovery_code(user_id) assert tfa.get_number_of_recovery_codes(user_id) == 1 assert tfa.verify(user_id, 'a' * tfa.LENGTH_RECOVERY_CODE, consume_recovery_code=False) assert tfa.get_number_of_recovery_codes(user_id) == 1 diff --git a/weasyl/two_factor_auth.py b/weasyl/two_factor_auth.py index fa3af2613..e939e3a13 100644 --- a/weasyl/two_factor_auth.py +++ b/weasyl/two_factor_auth.py @@ -6,6 +6,7 @@ import re import urllib +import bcrypt import pyotp from qrcodegen import QrCode @@ -15,7 +16,8 @@ # Number of recovery codes to provide the user _TFA_RECOVERY_CODES = 10 - +# Work factor for bcrypt to be used in hashing each recovery code +BCRYPT_WORK_FACTOR = 10 # This length is configurable as needed; see generate_recovery_codes() below for keyspace/character set LENGTH_RECOVERY_CODE = 20 # TOTP code length of 6 is the standard length, and is supported by Google Authenticator @@ -114,7 +116,7 @@ def activate(userid, tfa_secret, tfa_response): """ totp = pyotp.TOTP(tfa_secret) # If the provided `tfa_response` matches the TOTP value, write the 2FA secret into `login`, activating 2FA for `userid` - if totp.verify(tfa_response): + if totp.verify(tfa_response, valid_window=1): d.engine.execute(""" UPDATE login SET twofa_secret = %(tfa_secret)s @@ -215,6 +217,9 @@ def store_recovery_codes(userid, recovery_codes): if len(code) != LENGTH_RECOVERY_CODE: return False + # Store the recovery codes securely by hashing them with bcrypt (as login.passhash()) + hashed_codes = [bcrypt.hashpw(code.encode('utf-8'), bcrypt.gensalt(rounds=BCRYPT_WORK_FACTOR)) for code in codes] + # If above checks have passed, clear current recovery codes for `userid` and store new ones d.engine.execute(""" BEGIN; @@ -226,7 +231,7 @@ def store_recovery_codes(userid, recovery_codes): SELECT %(userid)s, unnest(%(tfa_recovery_codes)s); COMMIT; - """, userid=userid, tfa_recovery_codes=list(codes)) + """, userid=userid, tfa_recovery_codes=list(hashed_codes)) # Verify if the atomic transaction completed; if `code` (one of the new recovery codes) is # valid at this point, the new codes were added @@ -257,25 +262,25 @@ def is_recovery_code_valid(userid, tfa_code, consume_recovery_code=True): # Recovery codes must be LENGTH_RECOVERY_CODE characters; fast-fail if this is not the case if len(tfa_code) != LENGTH_RECOVERY_CODE: return False - # Check to see if the provided code--converting to upper-case first--is valid and consume if so - if consume_recovery_code: - tfa_rc = d.engine.scalar(""" - DELETE FROM twofa_recovery_codes - WHERE userid = %(userid)s AND recovery_code = %(recovery_code)s - RETURNING recovery_code - """, userid=userid, recovery_code=tfa_code.upper()) - else: - # We only want to see if the code is valid at the moment. - tfa_rc = d.engine.scalar(""" - SELECT recovery_code - FROM twofa_recovery_codes - WHERE userid = %(userid)s AND recovery_code = %(recovery_code)s - """, userid=userid, recovery_code=tfa_code.upper()) - # Return True if the recovery code was valid, False otherwise - if tfa_rc: - return True - else: - return False + # First extract the bcrypt hashes. + recovery_code_hash_query = d.engine.execute(""" + SELECT recovery_code + FROM twofa_recovery_codes + WHERE userid = %(userid)s + """, userid=userid).fetchall() + # Then attempt to hash the input code versus the stored code(s). + for row in recovery_code_hash_query: + if bcrypt.checkpw(tfa_code.upper().encode('utf-8'), row['recovery_code'].encode('utf-8')): + # We have a match! If we are deleting the code, do it now. + if consume_recovery_code: + d.engine.execute(""" + DELETE FROM twofa_recovery_codes + WHERE userid = %(userid)s AND recovery_code = %(recovery_code)s + """, userid=userid, recovery_code=row['recovery_code']) + # After deletion--if applicable--return that we succeeded. + return True + # If we get here, ``tfa_code`` did not match any stored codes. Return that we failed. + return False def is_2fa_enabled(userid): From acf990878a38f0af4567d1f4d87ece7fddfe888f Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Sun, 2 Apr 2017 20:07:31 -0400 Subject: [PATCH 16/22] Symmetrically encrypt the TOTP secret prior to storing it in the user's login record. Adds 'cryptography' as a new PyPI package requirement. --- Vagrantfile | 4 ++ config/site.config.txt.example | 5 ++ etc/requirements.txt | 1 + .../versions/abeefecabdad_implement_2fa.py | 2 +- libweasyl/libweasyl/models/tables.py | 2 +- weasyl/test/test_two_factor_auth.py | 56 +++++++++---------- weasyl/two_factor_auth.py | 40 ++++++++++++- 7 files changed, 74 insertions(+), 36 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index c65262263..31cf85d71 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -40,6 +40,10 @@ apt-get -y install \ liblzma-dev python-dev python-virtualenv \ ruby-sass nodejs +# Install dependencies for 'cryptography' PyPI package -- https://cryptography.io/en/latest/installation/#building-cryptography-on-linux +apt-get -y install \ + build-essential libssl-dev libffi-dev python-dev + npm install -g gulp-cli sudo -u postgres dropdb weasyl diff --git a/config/site.config.txt.example b/config/site.config.txt.example index 2ee915ffd..d7a27c8e8 100644 --- a/config/site.config.txt.example +++ b/config/site.config.txt.example @@ -25,3 +25,8 @@ url = postgresql+psycopg2cffi:///weasyl [recaptcha-lo.weasyl.com] public_key = 6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI private_key = 6LeIxAcTAAAAAGG-vFI1TnRWxMZNFuojJ4WifJWe + +[twofactorauth.totpsecret.encryption.key] +# This key MUST be changed when in production; +# See https://cryptography.io/en/latest/fernet/ -- Fernet.generate_key() +secret_key = 2iY4trxnpmNLlQifnQ21pFF0nb-VlmpxRUI6W_uP1oQ= diff --git a/etc/requirements.txt b/etc/requirements.txt index d9f24cd7c..2f093a1f0 100644 --- a/etc/requirements.txt +++ b/etc/requirements.txt @@ -19,3 +19,4 @@ pyramid==1.7.3 WebTest==2.0.23 pyotp==2.2.4 # For Two-Factor Authentication qrcodegen==1.0.0 # For Two-Factor Authentication +cryptography==1.8.1 # For Two-Factor Authentication diff --git a/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py b/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py index be43e24c0..0e8d5e06a 100644 --- a/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py +++ b/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py @@ -28,7 +28,7 @@ def upgrade(): # Modify `login` to hold the 2FA code (if set) for a user account op.add_column( 'login', - sa.Column('twofa_secret', sa.String(length=16), nullable=True, server_default=None), + sa.Column('twofa_secret', sa.String(length=420), nullable=True, server_default=None), ) diff --git a/libweasyl/libweasyl/models/tables.py b/libweasyl/libweasyl/models/tables.py index 5ff13f542..b7d0836bc 100644 --- a/libweasyl/libweasyl/models/tables.py +++ b/libweasyl/libweasyl/models/tables.py @@ -391,7 +391,7 @@ def default_fkey(*args, **kwargs): }, }, length=20), nullable=False, server_default=''), Column('email', String(length=100), nullable=False, server_default=''), - Column('twofa_secret', String(length=16), nullable=True), + Column('twofa_secret', String(length=420), nullable=True), ) Index('ind_login_login_name', login.c.login_name) diff --git a/weasyl/test/test_two_factor_auth.py b/weasyl/test/test_two_factor_auth.py index 865f53b05..138eed4de 100644 --- a/weasyl/test/test_two_factor_auth.py +++ b/weasyl/test/test_two_factor_auth.py @@ -27,6 +27,15 @@ def _insert_recovery_code(userid): """, userid=userid, code=recovery_code_hashed) +@pytest.mark.usefixtures('db') +def _insert_2fa_secret(user_id, tfa_secret_encrypted): + d.engine.execute(""" + UPDATE login + SET twofa_secret = %(tfas)s + WHERE userid = %(userid)s + """, userid=user_id, tfas=tfa_secret_encrypted) + + @pytest.mark.usefixtures('db') def test_get_number_of_recovery_codes(): user_id = db_utils.create_user() @@ -172,26 +181,28 @@ def test_activate(): # Validation successful, and tfa_secret written into user's `login` record tfa_response = totp.now() assert tfa.activate(user_id, tfa_secret, tfa_response) - assert tfa_secret == d.engine.scalar(""" + # The stored twofa_secret must not be plaintext + stored_secret = d.engine.scalar(""" SELECT twofa_secret FROM login WHERE userid = %(userid)s """, userid=user_id) + assert tfa_secret != stored_secret + # The stored secret must be decryptable to the generated tfa_secret + assert tfa_secret == tfa._decrypt_totp_secret(stored_secret) @pytest.mark.usefixtures('db') def test_is_2fa_enabled(): user_id = db_utils.create_user() + tfa_secret = pyotp.random_base32() + tfa_secret_encrypted = tfa._encrypt_totp_secret(tfa_secret) # 2FA is not enabled assert not tfa.is_2fa_enabled(user_id) # 2FA is enabled - d.engine.execute(""" - UPDATE login - SET twofa_secret = %(tfas)s - WHERE userid = %(userid)s - """, userid=user_id, tfas=pyotp.random_base32()) + _insert_2fa_secret(user_id, tfa_secret_encrypted) assert tfa.is_2fa_enabled(user_id) @@ -199,34 +210,23 @@ def test_is_2fa_enabled(): def test_deactivate(): user_id = db_utils.create_user() tfa_secret = pyotp.random_base32() + tfa_secret_encrypted = tfa._encrypt_totp_secret(tfa_secret) totp = pyotp.TOTP(tfa_secret) # 2FA enabled, deactivated by TOTP challenge-response code - d.engine.execute(""" - UPDATE login - SET twofa_secret = %(tfas)s - WHERE userid = %(userid)s - """, userid=user_id, tfas=tfa_secret) + _insert_2fa_secret(user_id, tfa_secret_encrypted) tfa_response = totp.now() assert tfa.deactivate(user_id, tfa_response) # 2FA enabled, deactivated by recovery code - d.engine.execute(""" - UPDATE login - SET twofa_secret = %(tfas)s - WHERE userid = %(userid)s - """, userid=user_id, tfas=tfa_secret) + _insert_2fa_secret(user_id, tfa_secret_encrypted) tfa_response = totp.now() _insert_recovery_code(user_id) assert tfa.deactivate(user_id, recovery_code) # 2FA enabled, failed deactivation (invalid `tfa_response` (code or TOTP token)) - d.engine.execute(""" - UPDATE login - SET twofa_secret = %(tfas)s - WHERE userid = %(userid)s - """, userid=user_id, tfas=tfa_secret) + _insert_2fa_secret(user_id, tfa_secret_encrypted) assert not tfa.deactivate(user_id, "000000") assert not tfa.deactivate(user_id, "a" * tfa.LENGTH_RECOVERY_CODE) @@ -235,12 +235,9 @@ def test_deactivate(): def test_force_deactivate(): user_id = db_utils.create_user() tfa_secret = pyotp.random_base32() - d.engine.execute(""" - UPDATE login - SET twofa_secret = %(tfas)s - WHERE userid = %(userid)s - """, userid=user_id, tfas=tfa_secret) + tfa_secret_encrypted = tfa._encrypt_totp_secret(tfa_secret) + _insert_2fa_secret(user_id, tfa_secret_encrypted) _insert_recovery_code(user_id) # Verify that force_deactivate() functions as expected. @@ -257,13 +254,10 @@ def test_force_deactivate(): def test_verify(): user_id = db_utils.create_user() tfa_secret = pyotp.random_base32() + tfa_secret_encrypted = tfa._encrypt_totp_secret(tfa_secret) totp = pyotp.TOTP(tfa_secret) - d.engine.execute(""" - UPDATE login - SET twofa_secret = %(tfas)s - WHERE userid = %(userid)s - """, userid=user_id, tfas=tfa_secret) + _insert_2fa_secret(user_id, tfa_secret_encrypted) _insert_recovery_code(user_id) # Codes of any other length than tfa.LENGTH_TOTP_CODE or tfa.LENGTH_RECOVERY_CODE returns False diff --git a/weasyl/two_factor_auth.py b/weasyl/two_factor_auth.py index e939e3a13..aa2ef19be 100644 --- a/weasyl/two_factor_auth.py +++ b/weasyl/two_factor_auth.py @@ -7,12 +7,13 @@ import urllib import bcrypt +from cryptography.fernet import Fernet import pyotp from qrcodegen import QrCode from libweasyl import security +from weasyl import config from weasyl import define as d -from weasyl import login # Number of recovery codes to provide the user _TFA_RECOVERY_CODES = 10 @@ -24,6 +25,34 @@ LENGTH_TOTP_CODE = 6 +def _encrypt_totp_secret(totp_secret): + """ + Symmetrically encrypt a 2FA TOTP secret. + + Parameters: + - totp_secret: A 2FA TOTP secret key to encrypt prior to storing in the DB. + + Returns: An encrypted Fernet token. + """ + key = config.config_read_setting(setting='secret_key', section='twofactorauth.totpsecret.encryption.key') + f = Fernet(key) + return f.encrypt(bytes(totp_secret)) + + +def _decrypt_totp_secret(totp_secret): + """ + Decrypt a symmetrically encrypted 2FA TOTP secret. + + Parameters: + - totp_secret: An encrypted Fernet token. + + Returns: The decrypted plaintext 2FA TOTP secret corresponding to the ciphertext `totp_secret`. + """ + key = config.config_read_setting(setting='secret_key', section='twofactorauth.totpsecret.encryption.key') + f = Fernet(key) + return f.decrypt(bytes(totp_secret)) + + def init(userid): """ Initialize 2FA for a user by generating and returning a 2FA secret key. @@ -117,11 +146,13 @@ def activate(userid, tfa_secret, tfa_response): totp = pyotp.TOTP(tfa_secret) # If the provided `tfa_response` matches the TOTP value, write the 2FA secret into `login`, activating 2FA for `userid` if totp.verify(tfa_response, valid_window=1): + # Encrypt the 2FA secret prior to storing it into `userid`'s login record + tfa_secret_encrypted = _encrypt_totp_secret(tfa_secret) d.engine.execute(""" UPDATE login SET twofa_secret = %(tfa_secret)s WHERE userid = %(userid)s - """, tfa_secret=tfa_secret, userid=userid) + """, tfa_secret=tfa_secret_encrypted, userid=userid) return True else: return False @@ -146,11 +177,14 @@ def verify(userid, tfa_response, consume_recovery_code=True): # implementations display the code as "123 456" tfa_response = tfa_response.replace(' ', '') if len(tfa_response) == LENGTH_TOTP_CODE: - tfa_secret = d.engine.scalar(""" + # Retrieve the encrypted 2FA secret from `userid`'s login record + tfa_secret_encrypted = d.engine.scalar(""" SELECT twofa_secret FROM login WHERE userid = %(userid)s """, userid=userid) + # Decrypt the 2FA secret to be usable by pyotp + tfa_secret = _decrypt_totp_secret(tfa_secret_encrypted) # Validate supplied 2FA response versus calculated current TOTP value. totp = pyotp.TOTP(tfa_secret) # Return the response of the TOTP verification; True/False From 4784484da014d47945fcf5fbb338ade0249ce40c Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Sun, 2 Apr 2017 22:30:06 -0400 Subject: [PATCH 17/22] Requirements already satisfied for 'cryptography' package Tests show that the packages required for the aforementioned package (build-essential libssl-dev libffi-dev python-dev) are satisfied alread through the existing setup steps either through the Debian base-box, or the Vagrantfile steps --- Vagrantfile | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index 31cf85d71..c65262263 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -40,10 +40,6 @@ apt-get -y install \ liblzma-dev python-dev python-virtualenv \ ruby-sass nodejs -# Install dependencies for 'cryptography' PyPI package -- https://cryptography.io/en/latest/installation/#building-cryptography-on-linux -apt-get -y install \ - build-essential libssl-dev libffi-dev python-dev - npm install -g gulp-cli sudo -u postgres dropdb weasyl From 8946ace857a6011d57dd33cc6d170f9314c5c0b5 Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Sun, 9 Apr 2017 00:38:40 -0400 Subject: [PATCH 18/22] Fixes to 2FA code Incorporates Charmander's PR review recommendations. --- config/site.config.txt.example | 2 +- .../versions/abeefecabdad_implement_2fa.py | 5 +- libweasyl/libweasyl/models/tables.py | 3 +- libweasyl/libweasyl/security.py | 4 +- templates/control/2fa/disable.html | 2 +- .../control/2fa/generate_recovery_codes.html | 2 +- templates/control/2fa/init.html | 2 +- templates/control/2fa/init_qrcode.html | 2 +- templates/control/2fa/init_verify.html | 2 +- templates/etc/signin_2fa_auth.html | 2 +- weasyl/controllers/two_factor_auth.py | 108 +++++++----------- weasyl/controllers/user.py | 5 +- weasyl/test/test_two_factor_auth.py | 10 +- weasyl/two_factor_auth.py | 39 ++++--- 14 files changed, 88 insertions(+), 100 deletions(-) diff --git a/config/site.config.txt.example b/config/site.config.txt.example index d7a27c8e8..53c175d68 100644 --- a/config/site.config.txt.example +++ b/config/site.config.txt.example @@ -26,7 +26,7 @@ url = postgresql+psycopg2cffi:///weasyl public_key = 6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI private_key = 6LeIxAcTAAAAAGG-vFI1TnRWxMZNFuojJ4WifJWe -[twofactorauth.totpsecret.encryption.key] +[two_factor_auth] # This key MUST be changed when in production; # See https://cryptography.io/en/latest/fernet/ -- Fernet.generate_key() secret_key = 2iY4trxnpmNLlQifnQ21pFF0nb-VlmpxRUI6W_uP1oQ= diff --git a/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py b/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py index 0e8d5e06a..2fa3a814f 100644 --- a/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py +++ b/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py @@ -19,8 +19,9 @@ def upgrade(): # Create a table to store 2FA backup codes for use when the authenticator is unavailable. op.create_table('twofa_recovery_codes', sa.Column('userid', sa.Integer(), nullable=False), - sa.Column('recovery_code', sa.String(length=100), nullable=False), - sa.PrimaryKeyConstraint('userid', 'recovery_code'), + sa.Column('recovery_code_number', sa.Integer(), nullable=False), + sa.Column('recovery_code_hash', sa.String(length=100), nullable=False), + sa.PrimaryKeyConstraint('userid', 'recovery_code_number'), sa.ForeignKeyConstraint(['userid'], ['login.userid'], name='twofa_recovery_codes_userid_fkey', onupdate='CASCADE', ondelete='CASCADE'), ) op.create_index('ind_twofa_recovery_codes_userid', 'twofa_recovery_codes', ['userid']) diff --git a/libweasyl/libweasyl/models/tables.py b/libweasyl/libweasyl/models/tables.py index b7d0836bc..9c1685df6 100644 --- a/libweasyl/libweasyl/models/tables.py +++ b/libweasyl/libweasyl/models/tables.py @@ -400,7 +400,8 @@ def default_fkey(*args, **kwargs): twofa_recovery_codes = Table( 'twofa_recovery_codes', metadata, Column('userid', Integer(), primary_key=True, nullable=False), - Column('recovery_code', String(length=100), primary_key=True, nullable=False), + Column('recovery_code_number', Integer(), primary_key=True, nullable=False), + Column('recovery_code_hash', String(length=100), nullable=False), default_fkey(['userid'], ['login.userid'], name='twofa_recovery_codes_userid_fkey'), ) diff --git a/libweasyl/libweasyl/security.py b/libweasyl/libweasyl/security.py index 15ba872f0..e2cefb3df 100644 --- a/libweasyl/libweasyl/security.py +++ b/libweasyl/libweasyl/security.py @@ -10,12 +10,14 @@ key_characters = string.ascii_letters + string.digits -def generate_key(size): +def generate_key(size, key_characters=key_characters): """ Generate a cryptographically-secure random key. Parameters: size (int): The number of characters in the key. + key_characters (string): The character set to use during key generation. + defaults to string.ascii_letters + string.digits. Returns: An ASCII printable :term:`native string` of length *size*. diff --git a/templates/control/2fa/disable.html b/templates/control/2fa/disable.html index 7ad5232a4..eb483ace4 100644 --- a/templates/control/2fa/disable.html +++ b/templates/control/2fa/disable.html @@ -23,7 +23,7 @@

    Disable Two-Factor Authentication (2FA)

    $:{CSRF()}
    Cancel - +
    diff --git a/templates/control/2fa/generate_recovery_codes.html b/templates/control/2fa/generate_recovery_codes.html index d93e5c02f..a725fc680 100644 --- a/templates/control/2fa/generate_recovery_codes.html +++ b/templates/control/2fa/generate_recovery_codes.html @@ -45,7 +45,7 @@

    Your new recovery codes will be:

    Cancel - +
diff --git a/templates/control/2fa/init.html b/templates/control/2fa/init.html index 50e5bf978..7e26845ae 100644 --- a/templates/control/2fa/init.html +++ b/templates/control/2fa/init.html @@ -43,7 +43,7 @@

Begin setting up 2FA

Cancel - +
diff --git a/templates/control/2fa/init_qrcode.html b/templates/control/2fa/init_qrcode.html index a83f27051..0c235c3bd 100644 --- a/templates/control/2fa/init_qrcode.html +++ b/templates/control/2fa/init_qrcode.html @@ -32,7 +32,7 @@

Or manually enter your key

Cancel - +
diff --git a/templates/control/2fa/init_verify.html b/templates/control/2fa/init_verify.html index ee727662c..58a6345d2 100644 --- a/templates/control/2fa/init_verify.html +++ b/templates/control/2fa/init_verify.html @@ -48,7 +48,7 @@

Your recovery codes are:

Cancel - +
diff --git a/templates/etc/signin_2fa_auth.html b/templates/etc/signin_2fa_auth.html index faaf91fbe..37101e9e4 100644 --- a/templates/etc/signin_2fa_auth.html +++ b/templates/etc/signin_2fa_auth.html @@ -7,7 +7,7 @@
Whoops! The 2FA response token or recovery code you provided did not validate.
$:{CSRF()}

Two-Factor Authentication

-

Username: ${MARKDOWN_EXCERPT(username)}

+

Username: ${username}

The Weasyl account identified above has two-factor authentication enabled. Enter the current time-based token for this user below. Or, if you no longer diff --git a/weasyl/controllers/two_factor_auth.py b/weasyl/controllers/two_factor_auth.py index 661fb11dd..494edada1 100644 --- a/weasyl/controllers/two_factor_auth.py +++ b/weasyl/controllers/two_factor_auth.py @@ -36,30 +36,26 @@ def tfa_init_get_(request): @token_checked @twofactorauth_disabled_required def tfa_init_post_(request): - if request.params['action'] == "continue": - userid, status = login.authenticate_bcrypt(define.get_display_name(request.userid), - request.params['password'], session=False) - # The user's password failed to authenticate - if status == "invalid": - return Response(define.webpage(request.userid, "control/2fa/init.html", [ - define.get_display_name(request.userid), - "password" - ])) - # Unlikely that this block will get triggered, but just to be safe, check for it - elif status == "unicode-failure": - raise HTTPSeeOther(location='/signin/unicode-failure') - # The user has authenticated, so continue with the initialization process. - else: - tfa_secret, tfa_qrcode = tfa.init(request.userid) - return Response(define.webpage(request.userid, "control/2fa/init_qrcode.html", [ - define.get_display_name(request.userid), - tfa_secret, - tfa_qrcode, - None - ])) + userid, status = login.authenticate_bcrypt(define.get_display_name(request.userid), + request.params['password'], session=False) + # The user's password failed to authenticate + if status == "invalid": + return Response(define.webpage(request.userid, "control/2fa/init.html", [ + define.get_display_name(request.userid), + "password" + ])) + # Unlikely that this block will get triggered, but just to be safe, check for it + elif status == "unicode-failure": + raise HTTPSeeOther(location='/signin/unicode-failure') + # The user has authenticated, so continue with the initialization process. else: - # This shouldn't be reached normally (user intentionally altered action?) - raise WeasylError("Unexpected") + tfa_secret, tfa_qrcode = tfa.init(request.userid) + return Response(define.webpage(request.userid, "control/2fa/init_qrcode.html", [ + define.get_display_name(request.userid), + tfa_secret, + tfa_qrcode, + None + ])) @login_required @@ -82,27 +78,23 @@ def tfa_init_qrcode_get_(request): @token_checked @twofactorauth_disabled_required def tfa_init_qrcode_post_(request): - if request.params['action'] == "continue": - # Strip any spaces from the TOTP code (some authenticators display the digits like '123 456') - tfaresponse = request.params['tfaresponse'].replace(' ', '') - - # Check to see if the tfaresponse matches the tfasecret when run through the TOTP algorithm - tfa_secret, recovery_codes = tfa.init_verify_tfa(request.userid, request.params['tfasecret'], tfaresponse) - - # The 2FA TOTP code did not match with the generated 2FA secret - if not tfa_secret: - return Response(define.webpage(request.userid, "control/2fa/init_qrcode.html", [ - define.get_display_name(request.userid), - request.params['tfasecret'], - tfa.generate_tfa_qrcode(request.userid, request.params['tfasecret']), - "2fa" - ])) - else: - return Response(define.webpage(request.userid, "control/2fa/init_verify.html", - [tfa_secret, recovery_codes, None])) + # Strip any spaces from the TOTP code (some authenticators display the digits like '123 456') + tfaresponse = request.params['tfaresponse'].replace(' ', '') + + # Check to see if the tfaresponse matches the tfasecret when run through the TOTP algorithm + tfa_secret, recovery_codes = tfa.init_verify_tfa(request.userid, request.params['tfasecret'], tfaresponse) + + # The 2FA TOTP code did not match with the generated 2FA secret + if not tfa_secret: + return Response(define.webpage(request.userid, "control/2fa/init_qrcode.html", [ + define.get_display_name(request.userid), + request.params['tfasecret'], + tfa.generate_tfa_qrcode(request.userid, request.params['tfasecret']), + "2fa" + ])) else: - # This shouldn't be reached normally (user intentionally altered action?) - raise WeasylError("Unexpected") + return Response(define.webpage(request.userid, "control/2fa/init_verify.html", + [tfa_secret, recovery_codes, None])) @login_required @@ -128,14 +120,13 @@ def tfa_init_verify_get_(request): @twofactorauth_disabled_required def tfa_init_verify_post_(request): # Extract parameters from the form - action = request.params['action'] - verify_checkbox = True if 'verify' in request.params else False + verify_checkbox = 'verify' in request.params tfasecret = request.params['tfasecret'] tfaresponse = request.params['tfaresponse'] tfarecoverycodes = request.params['tfarecoverycodes'] # Does the user want to proceed with enabling 2FA? - if action == "enable" and verify_checkbox and tfa.store_recovery_codes(request.userid, tfarecoverycodes): + if verify_checkbox and tfa.store_recovery_codes(request.userid, tfarecoverycodes): # Strip any spaces from the TOTP code (some authenticators display the digits like '123 456') tfaresponse = request.params['tfaresponse'].replace(' ', '') @@ -149,12 +140,9 @@ def tfa_init_verify_post_(request): return Response(define.webpage(request.userid, "control/2fa/init_verify.html", [tfasecret, tfarecoverycodes.split(','), "2fa"])) # The user didn't check the verification checkbox (despite HTML5's client-side check); regenerate codes & redisplay - elif action == "enable" and not verify_checkbox: + elif not verify_checkbox: return Response(define.webpage(request.userid, "control/2fa/init_verify.html", [tfasecret, tfarecoverycodes.split(','), "verify"])) - else: - # This shouldn't be reached normally (user intentionally altered action?) - raise WeasylError("Unexpected") @login_required @@ -169,10 +157,9 @@ def tfa_disable_get_(request): @twofactorauth_enabled_required def tfa_disable_post_(request): tfaresponse = request.params['tfaresponse'] - verify_checkbox = True if 'verify' in request.params else False - action = request.params['action'] + verify_checkbox = 'verify' in request.params - if action == "disable" and verify_checkbox: + if verify_checkbox: # If 2FA was successfully deactivated... return to 2FA dashboard if tfa.deactivate(request.userid, tfaresponse): raise HTTPSeeOther(location="/control/2fa/status") @@ -180,12 +167,9 @@ def tfa_disable_post_(request): return Response(define.webpage(request.userid, "control/2fa/disable.html", [define.get_display_name(request.userid), "2fa"])) # The user didn't check the verification checkbox (despite HTML5's client-side check) - elif action == "disable" and not verify_checkbox: + elif not verify_checkbox: return Response(define.webpage(request.userid, "control/2fa/disable.html", [define.get_display_name(request.userid), "verify"])) - else: - # This shouldn't be reached normally (user intentionally altered action?) - raise WeasylError("Unexpected") @login_required @@ -202,13 +186,12 @@ def tfa_generate_recovery_codes_get_(request): @twofactorauth_enabled_required def tfa_generate_recovery_codes_post_(request): # Extract parameters from the form - action = request.params['action'] - verify_checkbox = True if 'verify' in request.params else False + verify_checkbox = 'verify' in request.params tfaresponse = request.params['tfaresponse'] tfarecoverycodes = request.params['tfarecoverycodes'] # Does the user want to save the new recovery codes? - if action == "save" and verify_checkbox: + if verify_checkbox: if tfa.verify(request.userid, tfaresponse, consume_recovery_code=False): if tfa.store_recovery_codes(request.userid, tfarecoverycodes): # Successfuly stored new recovery codes. @@ -221,11 +204,8 @@ def tfa_generate_recovery_codes_post_(request): tfarecoverycodes.split(','), "2fa" ])) - elif action == "save" and not verify_checkbox: + elif not verify_checkbox: return Response(define.webpage(request.userid, "control/2fa/generate_recovery_codes.html", [ tfarecoverycodes.split(','), "verify" ])) - else: - # This shouldn't be reached normally (user intentionally altered action?) - raise WeasylError("Unexpected") diff --git a/weasyl/controllers/user.py b/weasyl/controllers/user.py index 07943b2b8..db99abf5e 100644 --- a/weasyl/controllers/user.py +++ b/weasyl/controllers/user.py @@ -1,5 +1,7 @@ from __future__ import absolute_import +import urlparse + import arrow from pyramid.httpexceptions import ( HTTPFound, @@ -166,7 +168,8 @@ def signin_2fa_auth_post_(request): errorcode.error_messages['TwoFactorAuthenticationZeroRecoveryCodesRemaining'], [["2FA Dashboard", "/control/2fa/status"], ["Return to the Home Page", "/"]] )) - raise HTTPSeeOther(location=ref) + # Return to the target page, restricting to the path portion of 'ref' per urlparse. + raise HTTPSeeOther(location=urlparse.urlparse(ref).path) elif sess.additional_data['2fa_pwd_auth_attempts'] >= 5: # Hinder brute-forcing the 2FA token or recovery code by enforcing an upper-bound on 2FA auth attempts. _cleanup_2fa_session() diff --git a/weasyl/test/test_two_factor_auth.py b/weasyl/test/test_two_factor_auth.py index 138eed4de..225d0f041 100644 --- a/weasyl/test/test_two_factor_auth.py +++ b/weasyl/test/test_two_factor_auth.py @@ -22,9 +22,9 @@ def _insert_recovery_code(userid): """Insert the test-suite's pre-hashed recovery code""" d.engine.execute(""" - INSERT INTO twofa_recovery_codes (userid, recovery_code) - VALUES (%(userid)s, %(code)s) - """, userid=userid, code=recovery_code_hashed) + INSERT INTO twofa_recovery_codes (userid, recovery_code_number, recovery_code_hash) + VALUES (%(userid)s, %(recovery_code_number)s, %(recovery_code_hash)s) + """, userid=userid, recovery_code_number=1, recovery_code_hash=recovery_code_hashed) @pytest.mark.usefixtures('db') @@ -82,7 +82,7 @@ def test_store_recovery_codes(): # Extract the current hashed recovery codes query = d.engine.execute(""" - SELECT recovery_code + SELECT recovery_code_hash FROM twofa_recovery_codes WHERE userid = %(userid)s """, userid=user_id).fetchall() @@ -92,7 +92,7 @@ def test_store_recovery_codes(): for row in query: code_status = False for code in valid_code_list: - if bcrypt.checkpw(code.encode('utf-8'), row['recovery_code'].encode('utf-8')): + if bcrypt.checkpw(code.encode('utf-8'), row['recovery_code_hash'].encode('utf-8')): # If the code matches the hash, then the recovery code stored successfully code_status = True break diff --git a/weasyl/two_factor_auth.py b/weasyl/two_factor_auth.py index aa2ef19be..726cf323f 100644 --- a/weasyl/two_factor_auth.py +++ b/weasyl/two_factor_auth.py @@ -4,6 +4,7 @@ from __future__ import absolute_import, unicode_literals import re +import string import urllib import bcrypt @@ -34,7 +35,7 @@ def _encrypt_totp_secret(totp_secret): Returns: An encrypted Fernet token. """ - key = config.config_read_setting(setting='secret_key', section='twofactorauth.totpsecret.encryption.key') + key = config.config_read_setting(setting='secret_key', section='two_factor_auth') f = Fernet(key) return f.encrypt(bytes(totp_secret)) @@ -48,7 +49,7 @@ def _decrypt_totp_secret(totp_secret): Returns: The decrypted plaintext 2FA TOTP secret corresponding to the ciphertext `totp_secret`. """ - key = config.config_read_setting(setting='secret_key', section='twofactorauth.totpsecret.encryption.key') + key = config.config_read_setting(setting='secret_key', section='two_factor_auth') f = Fernet(key) return f.decrypt(bytes(totp_secret)) @@ -219,15 +220,18 @@ def generate_recovery_codes(): """ Generate a set of valid recovery codes. - Character set is defined by `libweasyl.security`, (ASCII+Numbers), limited to uppercase via - .upper() for readability. + Character set is defined as uppercase ASCII characters (string.ascii_uppercase), plus + numerals '123456789'. Numeral zero is excluded due to the confusion potential between + '0' and 'O'. Parameters: None Returns: A set of length `_TFA_RECOVERY_CODES` where each code is `LENGTH_RECOVERY_CODE` characters in length. """ - return {security.generate_key(LENGTH_RECOVERY_CODE).upper() for i in range(_TFA_RECOVERY_CODES)} + # Generate the character-set to use during the generation of the keys. + charset = string.ascii_uppercase + "123456789" + return {security.generate_key(size=LENGTH_RECOVERY_CODE, key_characters=charset) for i in range(_TFA_RECOVERY_CODES)} def store_recovery_codes(userid, recovery_codes): @@ -251,7 +255,8 @@ def store_recovery_codes(userid, recovery_codes): if len(code) != LENGTH_RECOVERY_CODE: return False - # Store the recovery codes securely by hashing them with bcrypt (as login.passhash()) + # Store the recovery codes securely by hashing them with bcrypt + hashed_position = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] hashed_codes = [bcrypt.hashpw(code.encode('utf-8'), bcrypt.gensalt(rounds=BCRYPT_WORK_FACTOR)) for code in codes] # If above checks have passed, clear current recovery codes for `userid` and store new ones @@ -261,18 +266,14 @@ def store_recovery_codes(userid, recovery_codes): DELETE FROM twofa_recovery_codes WHERE userid = %(userid)s; - INSERT INTO twofa_recovery_codes (userid, recovery_code) - SELECT %(userid)s, unnest(%(tfa_recovery_codes)s); + INSERT INTO twofa_recovery_codes (userid, recovery_code_number, recovery_code_hash) + SELECT %(userid)s, unnest(%(recovery_code_number)s), unnest(%(recovery_code_hash)s); COMMIT; - """, userid=userid, tfa_recovery_codes=list(hashed_codes)) + """, userid=userid, recovery_code_number=list(hashed_position), recovery_code_hash=list(hashed_codes)) - # Verify if the atomic transaction completed; if `code` (one of the new recovery codes) is - # valid at this point, the new codes were added - if is_recovery_code_valid(userid, code, consume_recovery_code=False): - return True - else: - return False + # Return that we added the codes successfully + return True def is_recovery_code_valid(userid, tfa_code, consume_recovery_code=True): @@ -298,19 +299,19 @@ def is_recovery_code_valid(userid, tfa_code, consume_recovery_code=True): return False # First extract the bcrypt hashes. recovery_code_hash_query = d.engine.execute(""" - SELECT recovery_code + SELECT recovery_code_hash FROM twofa_recovery_codes WHERE userid = %(userid)s """, userid=userid).fetchall() # Then attempt to hash the input code versus the stored code(s). for row in recovery_code_hash_query: - if bcrypt.checkpw(tfa_code.upper().encode('utf-8'), row['recovery_code'].encode('utf-8')): + if bcrypt.checkpw(tfa_code.upper().encode('utf-8'), row['recovery_code_hash'].encode('utf-8')): # We have a match! If we are deleting the code, do it now. if consume_recovery_code: d.engine.execute(""" DELETE FROM twofa_recovery_codes - WHERE userid = %(userid)s AND recovery_code = %(recovery_code)s - """, userid=userid, recovery_code=row['recovery_code']) + WHERE userid = %(userid)s AND recovery_code_hash = %(recovery_code_hash)s + """, userid=userid, recovery_code_hash=row['recovery_code_hash']) # After deletion--if applicable--return that we succeeded. return True # If we get here, ``tfa_code`` did not match any stored codes. Return that we failed. From 7ea2a46131f700d41070340ff9a1d9f6d3766352 Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Sun, 9 Apr 2017 02:44:52 -0400 Subject: [PATCH 19/22] Don't rely on user-land information for recovery codes or TOTP codes Instead, store the information on the current session, and consult the server-stored TOTP/recovery codes --- templates/control/2fa/disable.html | 2 + .../control/2fa/generate_recovery_codes.html | 3 +- templates/control/2fa/init_qrcode.html | 1 - templates/control/2fa/init_verify.html | 4 +- weasyl/controllers/two_factor_auth.py | 60 +++++++++++++++---- weasyl/two_factor_auth.py | 2 +- 6 files changed, 55 insertions(+), 17 deletions(-) diff --git a/templates/control/2fa/disable.html b/templates/control/2fa/disable.html index eb483ace4..d638d1d17 100644 --- a/templates/control/2fa/disable.html +++ b/templates/control/2fa/disable.html @@ -20,7 +20,9 @@

Disable Two-Factor Authentication (2FA)

+ $:{CSRF()} +
Cancel diff --git a/templates/control/2fa/generate_recovery_codes.html b/templates/control/2fa/generate_recovery_codes.html index a725fc680..f45aaa66a 100644 --- a/templates/control/2fa/generate_recovery_codes.html +++ b/templates/control/2fa/generate_recovery_codes.html @@ -5,7 +5,7 @@
$if error == "2fa":
- Whoops! The 2FA response token or recovery code you provided did not validate. + Whoops! The 2FA response token or recovery code you provided did not validate.
$elif error == "verify":
@@ -40,7 +40,6 @@

Your new recovery codes will be:


- $:{CSRF()}
diff --git a/templates/control/2fa/init_qrcode.html b/templates/control/2fa/init_qrcode.html index 0c235c3bd..fcd390386 100644 --- a/templates/control/2fa/init_qrcode.html +++ b/templates/control/2fa/init_qrcode.html @@ -27,7 +27,6 @@

Or manually enter your key

- $:{CSRF()}
diff --git a/templates/control/2fa/init_verify.html b/templates/control/2fa/init_verify.html index 58a6345d2..2cb433c72 100644 --- a/templates/control/2fa/init_verify.html +++ b/templates/control/2fa/init_verify.html @@ -1,4 +1,4 @@ -$def with (tfa_secret, tfa_recovery_codes, error) +$def with (tfa_recovery_codes, error) $:{RENDER("common/stage_title.html", ["Enable 2FA: Step 3", "Two-Factor Authentication", "/control/2fa/status"])}
@@ -42,8 +42,6 @@

Your recovery codes are:


- - $:{CSRF()}
diff --git a/weasyl/controllers/two_factor_auth.py b/weasyl/controllers/two_factor_auth.py index 494edada1..e2e6a2d7d 100644 --- a/weasyl/controllers/two_factor_auth.py +++ b/weasyl/controllers/two_factor_auth.py @@ -16,6 +16,37 @@ from weasyl.profile import invalidate_other_sessions +def _set_totp_code_on_session(totp_code): + sess = define.get_weasyl_session() + sess.additional_data['2fa_totp_code'] = totp_code + sess.save = True + + +def _get_totp_code_from_session(): + sess = define.get_weasyl_session() + return sess.additional_data['2fa_totp_code'] + + +def _set_recovery_codes_on_session(recovery_codes): + sess = define.get_weasyl_session() + sess.additional_data['2fa_recovery_codes'] = recovery_codes + sess.save = True + + +def _get_recovery_codes_from_session(): + sess = define.get_weasyl_session() + return sess.additional_data['2fa_recovery_codes'] + + +def _cleanup_session(): + sess = define.get_weasyl_session() + if '2fa_recovery_codes' in sess.additional_data: + del sess.additional_data['2fa_recovery_codes'] + if '2fa_totp_code' in sess.additional_data: + del sess.additional_data['2fa_totp_code'] + sess.save = True + + @login_required def tfa_status_get_(request): return Response(define.webpage(request.userid, "control/2fa/status.html", [ @@ -50,6 +81,7 @@ def tfa_init_post_(request): # The user has authenticated, so continue with the initialization process. else: tfa_secret, tfa_qrcode = tfa.init(request.userid) + _set_totp_code_on_session(tfa_secret) return Response(define.webpage(request.userid, "control/2fa/init_qrcode.html", [ define.get_display_name(request.userid), tfa_secret, @@ -80,21 +112,23 @@ def tfa_init_qrcode_get_(request): def tfa_init_qrcode_post_(request): # Strip any spaces from the TOTP code (some authenticators display the digits like '123 456') tfaresponse = request.params['tfaresponse'].replace(' ', '') + tfa_secret_sess = _get_totp_code_from_session() # Check to see if the tfaresponse matches the tfasecret when run through the TOTP algorithm - tfa_secret, recovery_codes = tfa.init_verify_tfa(request.userid, request.params['tfasecret'], tfaresponse) + tfa_secret, recovery_codes = tfa.init_verify_tfa(request.userid, tfa_secret_sess, tfaresponse) # The 2FA TOTP code did not match with the generated 2FA secret if not tfa_secret: return Response(define.webpage(request.userid, "control/2fa/init_qrcode.html", [ define.get_display_name(request.userid), - request.params['tfasecret'], - tfa.generate_tfa_qrcode(request.userid, request.params['tfasecret']), + tfa_secret_sess, + tfa.generate_tfa_qrcode(request.userid, tfa_secret_sess), "2fa" ])) else: + _set_recovery_codes_on_session(','.join(recovery_codes)) return Response(define.webpage(request.userid, "control/2fa/init_verify.html", - [tfa_secret, recovery_codes, None])) + [recovery_codes, None])) @login_required @@ -121,9 +155,9 @@ def tfa_init_verify_get_(request): def tfa_init_verify_post_(request): # Extract parameters from the form verify_checkbox = 'verify' in request.params - tfasecret = request.params['tfasecret'] + tfasecret = _get_totp_code_from_session() tfaresponse = request.params['tfaresponse'] - tfarecoverycodes = request.params['tfarecoverycodes'] + tfarecoverycodes = _get_recovery_codes_from_session() # Does the user want to proceed with enabling 2FA? if verify_checkbox and tfa.store_recovery_codes(request.userid, tfarecoverycodes): @@ -134,15 +168,17 @@ def tfa_init_verify_post_(request): if tfa.activate(request.userid, tfasecret, tfaresponse): # Invalidate all other login sessions invalidate_other_sessions(request.userid) + # Clean up the stored session variables + _cleanup_session() raise HTTPSeeOther(location="/control/2fa/status") # TOTP+2FA Secret did not validate else: return Response(define.webpage(request.userid, "control/2fa/init_verify.html", - [tfasecret, tfarecoverycodes.split(','), "2fa"])) + [tfarecoverycodes.split(','), "2fa"])) # The user didn't check the verification checkbox (despite HTML5's client-side check); regenerate codes & redisplay elif not verify_checkbox: return Response(define.webpage(request.userid, "control/2fa/init_verify.html", - [tfasecret, tfarecoverycodes.split(','), "verify"])) + [tfarecoverycodes.split(','), "verify"])) @login_required @@ -175,8 +211,10 @@ def tfa_disable_post_(request): @login_required @twofactorauth_enabled_required def tfa_generate_recovery_codes_get_(request): + recovery_codes = tfa.generate_recovery_codes() + _set_recovery_codes_on_session(','.join(recovery_codes)) return Response(define.webpage(request.userid, "control/2fa/generate_recovery_codes.html", [ - tfa.generate_recovery_codes(), + recovery_codes, None ])) @@ -188,12 +226,14 @@ def tfa_generate_recovery_codes_post_(request): # Extract parameters from the form verify_checkbox = 'verify' in request.params tfaresponse = request.params['tfaresponse'] - tfarecoverycodes = request.params['tfarecoverycodes'] + tfarecoverycodes = _get_recovery_codes_from_session() # Does the user want to save the new recovery codes? if verify_checkbox: if tfa.verify(request.userid, tfaresponse, consume_recovery_code=False): if tfa.store_recovery_codes(request.userid, tfarecoverycodes): + # Clean up the stored session variables + _cleanup_session() # Successfuly stored new recovery codes. raise HTTPSeeOther(location="/control/2fa/status") else: diff --git a/weasyl/two_factor_auth.py b/weasyl/two_factor_auth.py index 726cf323f..a023b7178 100644 --- a/weasyl/two_factor_auth.py +++ b/weasyl/two_factor_auth.py @@ -231,7 +231,7 @@ def generate_recovery_codes(): """ # Generate the character-set to use during the generation of the keys. charset = string.ascii_uppercase + "123456789" - return {security.generate_key(size=LENGTH_RECOVERY_CODE, key_characters=charset) for i in range(_TFA_RECOVERY_CODES)} + return [security.generate_key(size=LENGTH_RECOVERY_CODE, key_characters=charset) for i in range(_TFA_RECOVERY_CODES)] def store_recovery_codes(userid, recovery_codes): From d1d676e411670050c993c2aad9ef5b3647ad5b5d Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Sun, 9 Apr 2017 22:31:27 -0400 Subject: [PATCH 20/22] Remove unneeded primary key from twofa_recovery_codes table --- .../alembic/versions/abeefecabdad_implement_2fa.py | 2 -- libweasyl/libweasyl/models/tables.py | 3 +-- weasyl/test/test_two_factor_auth.py | 6 +++--- weasyl/two_factor_auth.py | 7 +++---- 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py b/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py index 2fa3a814f..4f0fbe39d 100644 --- a/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py +++ b/libweasyl/libweasyl/alembic/versions/abeefecabdad_implement_2fa.py @@ -19,9 +19,7 @@ def upgrade(): # Create a table to store 2FA backup codes for use when the authenticator is unavailable. op.create_table('twofa_recovery_codes', sa.Column('userid', sa.Integer(), nullable=False), - sa.Column('recovery_code_number', sa.Integer(), nullable=False), sa.Column('recovery_code_hash', sa.String(length=100), nullable=False), - sa.PrimaryKeyConstraint('userid', 'recovery_code_number'), sa.ForeignKeyConstraint(['userid'], ['login.userid'], name='twofa_recovery_codes_userid_fkey', onupdate='CASCADE', ondelete='CASCADE'), ) op.create_index('ind_twofa_recovery_codes_userid', 'twofa_recovery_codes', ['userid']) diff --git a/libweasyl/libweasyl/models/tables.py b/libweasyl/libweasyl/models/tables.py index 9c1685df6..4eae1f439 100644 --- a/libweasyl/libweasyl/models/tables.py +++ b/libweasyl/libweasyl/models/tables.py @@ -399,8 +399,7 @@ def default_fkey(*args, **kwargs): twofa_recovery_codes = Table( 'twofa_recovery_codes', metadata, - Column('userid', Integer(), primary_key=True, nullable=False), - Column('recovery_code_number', Integer(), primary_key=True, nullable=False), + Column('userid', Integer(), nullable=False), Column('recovery_code_hash', String(length=100), nullable=False), default_fkey(['userid'], ['login.userid'], name='twofa_recovery_codes_userid_fkey'), ) diff --git a/weasyl/test/test_two_factor_auth.py b/weasyl/test/test_two_factor_auth.py index 225d0f041..192319bf6 100644 --- a/weasyl/test/test_two_factor_auth.py +++ b/weasyl/test/test_two_factor_auth.py @@ -22,9 +22,9 @@ def _insert_recovery_code(userid): """Insert the test-suite's pre-hashed recovery code""" d.engine.execute(""" - INSERT INTO twofa_recovery_codes (userid, recovery_code_number, recovery_code_hash) - VALUES (%(userid)s, %(recovery_code_number)s, %(recovery_code_hash)s) - """, userid=userid, recovery_code_number=1, recovery_code_hash=recovery_code_hashed) + INSERT INTO twofa_recovery_codes (userid, recovery_code_hash) + VALUES (%(userid)s, %(recovery_code_hash)s) + """, userid=userid, recovery_code_hash=recovery_code_hashed) @pytest.mark.usefixtures('db') diff --git a/weasyl/two_factor_auth.py b/weasyl/two_factor_auth.py index a023b7178..c59e9687b 100644 --- a/weasyl/two_factor_auth.py +++ b/weasyl/two_factor_auth.py @@ -256,7 +256,6 @@ def store_recovery_codes(userid, recovery_codes): return False # Store the recovery codes securely by hashing them with bcrypt - hashed_position = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] hashed_codes = [bcrypt.hashpw(code.encode('utf-8'), bcrypt.gensalt(rounds=BCRYPT_WORK_FACTOR)) for code in codes] # If above checks have passed, clear current recovery codes for `userid` and store new ones @@ -266,11 +265,11 @@ def store_recovery_codes(userid, recovery_codes): DELETE FROM twofa_recovery_codes WHERE userid = %(userid)s; - INSERT INTO twofa_recovery_codes (userid, recovery_code_number, recovery_code_hash) - SELECT %(userid)s, unnest(%(recovery_code_number)s), unnest(%(recovery_code_hash)s); + INSERT INTO twofa_recovery_codes (userid, recovery_code_hash) + SELECT %(userid)s, unnest(%(recovery_code_hash)s); COMMIT; - """, userid=userid, recovery_code_number=list(hashed_position), recovery_code_hash=list(hashed_codes)) + """, userid=userid, recovery_code_hash=list(hashed_codes)) # Return that we added the codes successfully return True From 6da3a1ef5e0ca021da0e8c0e2dfe9a8e3a5a155b Mon Sep 17 00:00:00 2001 From: Kyra Date: Mon, 10 Apr 2017 11:57:33 -0400 Subject: [PATCH 21/22] Remove unpaired HTML tag [skip ci] --- templates/control/2fa/generate_recovery_codes.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/control/2fa/generate_recovery_codes.html b/templates/control/2fa/generate_recovery_codes.html index f45aaa66a..af8f57a7d 100644 --- a/templates/control/2fa/generate_recovery_codes.html +++ b/templates/control/2fa/generate_recovery_codes.html @@ -5,7 +5,7 @@ $if error == "2fa":
- Whoops! The 2FA response token or recovery code you provided did not validate. + Whoops! The 2FA response token or recovery code you provided did not validate.
$elif error == "verify":
From 3eebd3fe828bcbf95b876a48d01ea10fb8af3d18 Mon Sep 17 00:00:00 2001 From: kfkitsune Date: Sun, 23 Apr 2017 20:13:57 -0400 Subject: [PATCH 22/22] Incorporate Hyena's recommendations --- libweasyl/libweasyl/security.py | 6 +- .../control/2fa/generate_recovery_codes.html | 11 ++- ...nerate_recovery_codes_verify_password.html | 29 +++++++ templates/control/2fa/init_verify.html | 18 ++++- templates/control/2fa/status.html | 2 +- templates/etc/signin_2fa_auth.html | 2 +- templates/help/two_factor_authentication.html | 7 +- weasyl/controllers/routes.py | 2 + weasyl/controllers/two_factor_auth.py | 78 +++++++++++++++++-- 9 files changed, 134 insertions(+), 21 deletions(-) create mode 100644 templates/control/2fa/generate_recovery_codes_verify_password.html diff --git a/libweasyl/libweasyl/security.py b/libweasyl/libweasyl/security.py index e2cefb3df..051a76c62 100644 --- a/libweasyl/libweasyl/security.py +++ b/libweasyl/libweasyl/security.py @@ -7,17 +7,17 @@ secure_random = random.SystemRandom() -key_characters = string.ascii_letters + string.digits +KEY_CHARACTERS = string.ascii_letters + string.digits -def generate_key(size, key_characters=key_characters): +def generate_key(size, key_characters=KEY_CHARACTERS): """ Generate a cryptographically-secure random key. Parameters: size (int): The number of characters in the key. key_characters (string): The character set to use during key generation. - defaults to string.ascii_letters + string.digits. + defaults to KEY_CHARACTERS (as defined in this module). Returns: An ASCII printable :term:`native string` of length *size*. diff --git a/templates/control/2fa/generate_recovery_codes.html b/templates/control/2fa/generate_recovery_codes.html index af8f57a7d..afab93cb8 100644 --- a/templates/control/2fa/generate_recovery_codes.html +++ b/templates/control/2fa/generate_recovery_codes.html @@ -2,7 +2,7 @@ $:{RENDER("common/stage_title.html", ["Generate Recovery Codes", "Two-Factor Authentication", "/control/2fa/status"])}
- +
$if error == "2fa":
Whoops! The 2FA response token or recovery code you provided did not validate. @@ -23,17 +23,22 @@

Generate New Recovery Codes

Before proceeding, print or save these codes to a secure place. In the event you lose access to your authenticator app, you will need these codes to regain access to your Weasyl account. If you no longer have access to your authenticator app, please disable 2FA - instead, then re-enable it to reinitialize your authenticator. + instead. You may then set up another authenticator.

+
+

Your new recovery codes will be:

    $for code in tfa_recovery_codes:
  1. ${code[0:4] + ' ' + code[4:8] + ' ' + code[8:12] + ' ' + code[12:16] + ' ' + code[16:20]}
+ +
+

These codes are one-time use, and upon successful use will not be able to be reused or retrieved. - If and when used, mark each code off. Codes may be used in any order. + Codes may be used in any order. Cross each code off when used.

diff --git a/templates/control/2fa/generate_recovery_codes_verify_password.html b/templates/control/2fa/generate_recovery_codes_verify_password.html new file mode 100644 index 000000000..094b64f5f --- /dev/null +++ b/templates/control/2fa/generate_recovery_codes_verify_password.html @@ -0,0 +1,29 @@ +$def with (error) +$:{RENDER("common/stage_title.html", ["Generate Recovery Codes: Verify Password", "Two-Factor Authentication", "/control/2fa/status"])} + +
+ + $if error == "password": +
Whoops! You entered an incorrect password.
+ +

Verify your password

+

+ Before generating new recovery codes, we need to verify your current Weasyl password. +

+
+

+ Note: For the security of your account, entering your password + will clear all other active login sessions for your account. +

+ + $:{CSRF()} + + + + +
+ Cancel + +
+ +
\ No newline at end of file diff --git a/templates/control/2fa/init_verify.html b/templates/control/2fa/init_verify.html index 2cb433c72..32b6a1e61 100644 --- a/templates/control/2fa/init_verify.html +++ b/templates/control/2fa/init_verify.html @@ -1,8 +1,8 @@ $def with (tfa_recovery_codes, error) -$:{RENDER("common/stage_title.html", ["Enable 2FA: Step 3", "Two-Factor Authentication", "/control/2fa/status"])} +$:{RENDER("common/stage_title.html", ["Enable 2FA: Final Step", "Two-Factor Authentication", "/control/2fa/status"])}
-
+
$if error == "2fa":
Whoops! The 2FA response token you provided did not validate. @@ -12,7 +12,11 @@ Hey! Did you want to enable 2FA? You didn't check the verification checkbox!
-

Verify Receipt of Two-Factor Authentication (2FA) Recovery Codes

+

Final Step: Verify Receipt of Two-Factor Authentication (2FA) Recovery Codes

+

You've almost enabled 2FA for your account. One final step remains!

+ +
+

Print these codes, and secure them as you would your password. In the event you lose access to your authenticator app, you will need these @@ -28,14 +32,20 @@

Verify Receipt of Two-Factor Authentication (2FA) Recovery Codes

Generating a new set of codes will prevent this from occurring. If this occurs, you will need to set-up 2FA again.

+
+ +

Your recovery codes are:

    $for code in tfa_recovery_codes:
  1. ${code[0:4] + ' ' + code[4:8] + ' ' + code[8:12] + ' ' + code[12:16] + ' ' + code[16:20]}
+ +
+

These codes are one-time use, and upon successful use will not be able to be reused or retrieved. - If and when used, mark each code off. Codes may be used in any order. + Codes may be used in any order. Cross each code off when used.

diff --git a/templates/control/2fa/status.html b/templates/control/2fa/status.html index 9f78f4a0b..68f3573b1 100644 --- a/templates/control/2fa/status.html +++ b/templates/control/2fa/status.html @@ -28,7 +28,7 @@

Recovery Code Status

diff --git a/templates/etc/signin_2fa_auth.html b/templates/etc/signin_2fa_auth.html index 37101e9e4..55aa88d84 100644 --- a/templates/etc/signin_2fa_auth.html +++ b/templates/etc/signin_2fa_auth.html @@ -18,7 +18,7 @@

Two-Factor Authentication

$if remaining_recovery_codes == 1:

Note: 2FA will be disabled upon using your last recovery - code. + code. You may re-enable 2FA immediately afterwards, if you desire.

diff --git a/templates/help/two_factor_authentication.html b/templates/help/two_factor_authentication.html index a1d17a436..608f83834 100644 --- a/templates/help/two_factor_authentication.html +++ b/templates/help/two_factor_authentication.html @@ -94,8 +94,11 @@

What happens if I no longer have access to both my authentic certificate.

- Storing recovery codes may be done either in paper form, in a secure digital manner using an electronic password - manager such as LastPass or KeePass, or both. + To securely store your recovery codes, a few options are available. Physically printing codes to paper is one option, + and secure electronic storage is another. Electronic options include LastPass and 1Password for cloud-based password + storage, and KeePass for offline password storage, as a brief example of the many options available. While Weasyl does + not advocate one specific solution over another, we do recommend using both physical and secure electronic storage + methods for securing your recovery codes.

diff --git a/weasyl/controllers/routes.py b/weasyl/controllers/routes.py index 7f981a8dc..129697c70 100644 --- a/weasyl/controllers/routes.py +++ b/weasyl/controllers/routes.py @@ -69,6 +69,8 @@ {'GET': two_factor_auth.tfa_init_verify_get_, 'POST': two_factor_auth.tfa_init_verify_post_}), Route("/control/2fa/disable", "control_2fa_disable", {'GET': two_factor_auth.tfa_disable_get_, 'POST': two_factor_auth.tfa_disable_post_}), + Route("/control/2fa/generate_recovery_codes_verify_password", "control_2fa_generate_recovery_codes_verify_password", + {'GET': two_factor_auth.tfa_generate_recovery_codes_verify_password_get_, 'POST': two_factor_auth.tfa_generate_recovery_codes_verify_password_post_}), Route("/control/2fa/generate_recovery_codes", "control_2fa_generate_recovery_codes", {'GET': two_factor_auth.tfa_generate_recovery_codes_get_, 'POST': two_factor_auth.tfa_generate_recovery_codes_post_}), diff --git a/weasyl/controllers/two_factor_auth.py b/weasyl/controllers/two_factor_auth.py index e2e6a2d7d..d80c28ef2 100644 --- a/weasyl/controllers/two_factor_auth.py +++ b/weasyl/controllers/two_factor_auth.py @@ -1,5 +1,6 @@ from __future__ import absolute_import, unicode_literals +import arrow from pyramid.response import Response from pyramid.httpexceptions import HTTPSeeOther @@ -30,18 +31,24 @@ def _get_totp_code_from_session(): def _set_recovery_codes_on_session(recovery_codes): sess = define.get_weasyl_session() sess.additional_data['2fa_recovery_codes'] = recovery_codes + sess.additional_data['2fa_recovery_codes_timestamp'] = arrow.now().timestamp sess.save = True def _get_recovery_codes_from_session(): sess = define.get_weasyl_session() - return sess.additional_data['2fa_recovery_codes'] + if '2fa_recovery_codes' in sess.additional_data: + return sess.additional_data['2fa_recovery_codes'] + else: + return None def _cleanup_session(): sess = define.get_weasyl_session() if '2fa_recovery_codes' in sess.additional_data: del sess.additional_data['2fa_recovery_codes'] + if '2fa_recovery_codes_timestamp' in sess.additional_data: + del sess.additional_data['2fa_recovery_codes_timestamp'] if '2fa_totp_code' in sess.additional_data: del sess.additional_data['2fa_totp_code'] sess.save = True @@ -208,15 +215,72 @@ def tfa_disable_post_(request): [define.get_display_name(request.userid), "verify"])) +@login_required +@twofactorauth_enabled_required +def tfa_generate_recovery_codes_verify_password_get_(request): + return Response(define.webpage( + request.userid, + "control/2fa/generate_recovery_codes_verify_password.html", + [None] + )) + + +@token_checked +@login_required +@twofactorauth_enabled_required +def tfa_generate_recovery_codes_verify_password_post_(request): + userid, status = login.authenticate_bcrypt(define.get_display_name(request.userid), + request.params['password'], session=False) + # The user's password failed to authenticate + if status == "invalid": + return Response(define.webpage( + request.userid, + "control/2fa/generate_recovery_codes_verify_password.html", + ["password"] + )) + # The user has authenticated, so continue with generating the new recovery codes. + else: + # Edge case prevention: Stop the user from having two Weasyl sessions open and trying + # to proceed through the generation process with two sets of recovery codes. + invalidate_other_sessions(request.userid) + # Edge case prevention: Do we have existing (and recent) codes on this session? Prevent + # a user from confusing themselves if they visit the request page twice. + sess = define.get_weasyl_session() + gen_rec_codes = True + if '2fa_recovery_codes_timestamp' in sess.additional_data: + # Are the codes on the current session < 30 minutes old? + tstamp = sess.additional_data['2fa_recovery_codes_timestamp'] + if arrow.now().timestamp - tstamp < 1800: + # We have recent codes on the session, use them instead of generating fresh codes. + recovery_codes = sess.additional_data['2fa_recovery_codes'].split(',') + gen_rec_codes = False + if gen_rec_codes: + # Either this is a fresh request to generate codes, or the timelimit was exceeded. + recovery_codes = tfa.generate_recovery_codes() + _set_recovery_codes_on_session(','.join(recovery_codes)) + return Response(define.webpage(request.userid, "control/2fa/generate_recovery_codes.html", [ + recovery_codes, + None + ])) + + @login_required @twofactorauth_enabled_required def tfa_generate_recovery_codes_get_(request): - recovery_codes = tfa.generate_recovery_codes() - _set_recovery_codes_on_session(','.join(recovery_codes)) - return Response(define.webpage(request.userid, "control/2fa/generate_recovery_codes.html", [ - recovery_codes, - None - ])) + """ + IMPLEMENTATION NOTE: This page cannot be accessed directly (HTTP GET), as the user may not have verified + control over the account by providing their password. Further, prevent edge cases where a user may + (un)intentionally attempt to encounter edge cases, such as having two browsers or windows open and generate + a situation where it isn't obvious which set of recovery codes will be saved; all other sessions are cleared + in this path to prevent this. That said, be nice and tell the user where to go to proceed. + """ + # Inform the user of where to go to begin + return Response(define.errorpage( + request.userid, + """This page cannot be accessed directly. Please click Generate Recovery Codes, below, in order to + begin generating new recovery codes.""", + [["Generate Recovery Codes", "/control/2fa/status"], ["Return to the Home Page", "/"]] + )) @login_required