From f4febc6052de4ed59854b091a13e861c17f81367 Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Tue, 22 Feb 2022 20:07:56 +0100 Subject: [PATCH 01/63] TOTP backbone --- dds_web/api/schemas/token_schemas.py | 17 +++++++- dds_web/api/user.py | 59 +++++++++++++++++++++++++ dds_web/database/models.py | 65 ++++++++++++++++++++++++++++ dds_web/security/auth.py | 3 +- dds_web/web/user.py | 38 ++++++++++++++++ 5 files changed, 179 insertions(+), 3 deletions(-) diff --git a/dds_web/api/schemas/token_schemas.py b/dds_web/api/schemas/token_schemas.py index 0f9e73f64..63da399a9 100644 --- a/dds_web/api/schemas/token_schemas.py +++ b/dds_web/api/schemas/token_schemas.py @@ -30,6 +30,14 @@ class TokenSchema(marshmallow.Schema): ), ) + TOTP = marshmallow.fields.String( + required=False, + validate=marshmallow.validate.And( + marshmallow.validate.Length(min=6, max=6), + marshmallow.validate.ContainsOnly("0123456789"), + ), + ) + class Meta: unknown = marshmallow.EXCLUDE @@ -38,11 +46,16 @@ def validate_mfa(self, data, **kwargs): """Verify HOTP (authentication One-Time code) is correct.""" # This can be easily extended to require at least one MFA method - if "HOTP" not in data: + if ("HOTP" not in data) and ("TOTP" not in data): raise marshmallow.ValidationError("MFA method not supplied") user = auth.current_user() - if "HOTP" in data: + + if user.totp_enabled: + value = data.get("TOTP") + # Raises authentication error if TOTP is incorrect + user.verify_TOTP(value.encode()) + else: value = data.get("HOTP") # Raises authenticationerror if invalid user.verify_HOTP(value.encode()) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index c46a566ed..7442a5d54 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -760,6 +760,65 @@ def get(self): return {"token": update_token_with_mfa(token_claims)} +class RequestTOTPActivation(flask_restful.Resource): + """Request to switch from HOTP to TOTP for second factor authentication.""" + + @auth.login_required + def post(self): + + user = auth.current_user() + if user.totp_enabled: + return {"message": "Nothing to do, TOTP is already enabled for this user."} + + # Not really necessary to encrypt this + token = encrypted_jwt_token( + username=user.username, + sensitive_content=None, + expires_in=datetime.timedelta( + seconds=3600, + ), + additional_claims={"act": "totp"}, # Open for suggestions + ) + + link = flask.url_for(dds_web.auth_blueprint.activate_totp, token=token, _external=True) + # Send activation token to email to work as a validation step + # TODO: refactor this since the email sending code is replicated in many places + recipients = [user.primary_email] + + # Fill in email subject with sentence subject + subject = f"Request to activate TOTP for SciLifeLab Data Delivery System" + + msg = flask_mail.Message( + subject, + recipients=recipients, + ) + + # Need to attach the image to be able to use it + msg.attach( + "scilifelab_logo.png", + "image/png", + open( + os.path.join(flask.current_app.static_folder, "img/scilifelab_logo.png"), "rb" + ).read(), + "inline", + headers=[ + ["Content-ID", ""], + ], + ) + + msg.body = flask.render_template( + f"mail/request_activate_totp.txt", + link=link, + ) + msg.html = flask.render_template( + f"mail/request_activate_totp.html", + link=link, + ) + + AddUser.send_email_with_retry(msg) + return {"message": "Please check your email and follow the attached link to activate TOTP."} + + class ShowUsage(flask_restful.Resource): """Calculate and display the amount of GB hours and the total cost.""" diff --git a/dds_web/database/models.py b/dds_web/database/models.py index 1c03dde01..d80f7c2cd 100644 --- a/dds_web/database/models.py +++ b/dds_web/database/models.py @@ -17,6 +17,7 @@ from itsdangerous import TimedJSONWebSignatureSerializer as Serializer from cryptography.hazmat.primitives.twofactor import ( hotp as twofactor_hotp, + totp as twofactor_totp, InvalidToken as twofactor_InvalidToken, ) from cryptography.hazmat.primitives import hashes @@ -358,9 +359,14 @@ class User(flask_login.UserMixin, db.Model): username = db.Column(db.String(50), primary_key=True, autoincrement=False) name = db.Column(db.String(255), unique=False, nullable=True) _password_hash = db.Column(db.String(98), unique=False, nullable=False) + # 2fa columns hotp_secret = db.Column(db.LargeBinary(20), unique=False, nullable=False) hotp_counter = db.Column(db.BigInteger, unique=False, nullable=False, default=0) hotp_issue_time = db.Column(db.DateTime, unique=False, nullable=True) + totp_enabled = db.Column(db.Boolean, unique=False, nullable=False, default=False) + _totp_secret = db.Column(db.LargeBinary(64), unique=False, nullable=False) + totp_last_verified = db.Column(db.DateTime, unique=False, nullable=True) + active = db.Column(db.Boolean) kd_salt = db.Column(db.LargeBinary(32), default=None) nonce = db.Column(db.LargeBinary(12), default=None) @@ -486,6 +492,65 @@ def verify_HOTP(self, token): self.hotp_issue_time = None db.session.commit() + def setup_totp_secret(self): + """Generate random 512 bit as the new totp secret and return provisioning URI""" + self._totp_secret = os.urandom(64) + db.session.commit() + + def get_totp_secret(self): + """Returns the users totp provisioning URI. Can only be sent before totp has been enabled.""" + if self.totp_enabled: + # Can not be fetched again after it has been enabled + raise AuthenticationError("TOTP secret already enabled.") + totp = twofactor_totp.TOTP(self._totp_secret, 6, hashes.SHA512(), 30) + return totp.get_provisioning_uri(account_name=self.username, issuer="Data Delivery System") + + def activate_totp(self): + """Set TOTP as the preferred means of second factor authentication. + Should be called after first totp token is verified + """ + self.totp_enabled = True + db.session.commit() + + def verify_totp(self, token): + """Verify the totp token. Checks the previous, current and comming time frame + to allow for some clock drift. + + raises AuthenticationError if token is invalid, has expired or + if totp has been successfully verified within the last 90 seconds. + """ + # can't use totp successfully more than once within 90 seconds. + # Time frame chosen so that no one can use the same token more than once + current_time = dds_web.utils.current_time() + if self.totp_last_used is not None and ( + current_time - self.totp_last_used < datetime.timedelta(seconds=90) + ): + raise AuthenticationError( + "Authentication with TOTP needs to be at least 90 seconds apart." + ) + + # construct object + totp = twofactor_totp.TOTP(self._totp_secret, 6, hashes.SHA512(), 30) + + # attempt to verify the token + # Allow for clock drift of 1 frame before or after + verified = False + for t_diff in [-30, 0, 30]: + verification_time = (current_time + datetime.timedelta(seconds=t_diff)).strftime("%s") + try: + totp.verify(token, verification_time) + verified = True + break + except twofactor_InvalidToken: + pass + + if not verified: + raise AuthenticationError("Invalid TOTP token.") + + # if the token is valid, save time of last successful verification + self.totp_last_used = current_time + db.session.commit() + # Email related @property def primary_email(self): diff --git a/dds_web/security/auth.py b/dds_web/security/auth.py index e3d65f210..03317454b 100644 --- a/dds_web/security/auth.py +++ b/dds_web/security/auth.py @@ -249,7 +249,8 @@ def __handle_multi_factor_authentication(user, mfa_auth_time_string): if mfa_auth_time >= dds_web.utils.current_time() - MFA_EXPIRES_IN: return user - send_hotp_email(user) + if not user.totp_enabled: + send_hotp_email(user) if flask.request.path.endswith("/user/second_factor"): return user diff --git a/dds_web/web/user.py b/dds_web/web/user.py index 1666a5c70..fa0fa5a13 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -151,6 +151,44 @@ def register(): return flask.render_template("user/register.html", form=form) +@auth_blueprint.route("/activate_totp/", methods=["GET", "POST"]) +@limiter.limit( + dds_web.utils.rate_limit_from_config, + methods=["GET", "POST"], + error_message=ddserr.TooManyRequestsError.description, +) +@flask_login.login_required +def activate_totp(token): + user = flask_login.current_user + + form = forms.ActivateTOTPForm() + + dds_web.security.verify_activate_totp_token(token) + + if flask.request.method == "GET": + if user.totp_enabled: + flask.flash("Two-factor authentication via TOTP is already enabled.") + return flask.redirect(flask.url_for("auth_blueprint.index")) + + user.setup_totp_secret() + secret, uri = user.get_totp_secret() + + return flask.render_template("user/activate_totp.html", secret=secret, uri=uri, form=form) + + # POST request + if form.validate_on_submit(): + if user.verify_totp(form.totp_code.data.encode()): + user.activate_totp() + + flask.flash("Two-factor authentication via TOTP has been enabled.") + return flask.redirect(flask.url_for("auth_blueprint.index")) + + flask.flash("Invalid two-factor authentication code.") + + secret, uri = user.get_totp_secret() + return flask.render_template("user/activate_totp.html", secret=secret, uri=uri, form=form) + + @auth_blueprint.route("/cancel_2fa", methods=["POST"]) @limiter.limit( dds_web.utils.rate_limit_from_config, From f506ee480bf6b7cbfa6a68ecf8d689c05f3aac07 Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Wed, 23 Feb 2022 16:15:46 +0100 Subject: [PATCH 02/63] Form to activate TOTP --- dds_web/forms.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dds_web/forms.py b/dds_web/forms.py index 14b38646f..35371b7d6 100644 --- a/dds_web/forms.py +++ b/dds_web/forms.py @@ -79,6 +79,14 @@ class Confirm2FACodeForm(flask_wtf.FlaskForm): submit = wtforms.SubmitField("Authenticate") +class ActivateTOTPForm(flask_wtf.FlaskForm): + totp = wtforms.StringField( + "totp", + validators=[wtforms.validators.InputRequired(), wtforms.validators.Length(min=6, max=6)], + ) + submit = wtforms.SubmitField("Activate") + + class Cancel2FAForm(flask_wtf.FlaskForm): cancel = wtforms.SubmitField("Cancel login and try again") From ec8fc595e43cf2b659e0ce0b9e855678fc24f53c Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Wed, 23 Feb 2022 16:16:04 +0100 Subject: [PATCH 03/63] Mail templates for TOTP activation --- .../templates/mail/request_activate_totp.html | 52 +++++++++++++++++++ .../templates/mail/request_activate_totp.txt | 5 ++ 2 files changed, 57 insertions(+) create mode 100644 dds_web/templates/mail/request_activate_totp.html create mode 100644 dds_web/templates/mail/request_activate_totp.txt diff --git a/dds_web/templates/mail/request_activate_totp.html b/dds_web/templates/mail/request_activate_totp.html new file mode 100644 index 000000000..7c9003d24 --- /dev/null +++ b/dds_web/templates/mail/request_activate_totp.html @@ -0,0 +1,52 @@ +{% extends 'mail/mail_base.html' %} +{% block body %} + + + + + + + +
+ Logo +
+

+ Activate TOTP for second factor authentication

+

+ If you want to activate one-time password using TOTP instead of e-mail, visit the following link: +

+ + + + + + + +

+ Alternatively, copy and paste the link below into your browser:

+

+ {{link}}

+
+{% endblock %} \ No newline at end of file diff --git a/dds_web/templates/mail/request_activate_totp.txt b/dds_web/templates/mail/request_activate_totp.txt new file mode 100644 index 000000000..f60802742 --- /dev/null +++ b/dds_web/templates/mail/request_activate_totp.txt @@ -0,0 +1,5 @@ +Activate TOTP for second factor authentication +If you want to activate one-time password using TOTP instead of e-mail, visit the following link: +{{link}} + +If you did not make this request simply ignore this email and no changes will be made. \ No newline at end of file From a1b3ad4fe6b53bad5cf7f0f4864486b6f547abec Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Wed, 23 Feb 2022 16:16:30 +0100 Subject: [PATCH 04/63] Method to verify activate TOTP token --- dds_web/security/auth.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/dds_web/security/auth.py b/dds_web/security/auth.py index 03317454b..06a0ce039 100644 --- a/dds_web/security/auth.py +++ b/dds_web/security/auth.py @@ -118,6 +118,18 @@ def verify_password_reset_token(token): raise AuthenticationError(message="Invalid token") +def verify_activate_totp_token(token, current_user): + claims = __verify_general_token(token) + user = __user_from_subject(claims.get("sub")) + if user and (user == current_user): + act = claims.get("act") + del claims + gc.collect() + if act and act == "totp": + return None + raise AuthenticationError(message="Invalid token") + + def __base_verify_token_for_invite(token): """Verify token and return claims.""" claims = __verify_general_token(token=token) From 338c7f1dc09a3d76b345874ca8aecb8255fbdb73 Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Wed, 23 Feb 2022 16:16:49 +0100 Subject: [PATCH 05/63] First draft of generating QRcode --- dds_web/web/user.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/dds_web/web/user.py b/dds_web/web/user.py index fa0fa5a13..cbecce5b7 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -5,7 +5,9 @@ #################################################################################################### # Standard Library +import base64 import datetime +import io import re # Installed @@ -14,6 +16,7 @@ from dds_web.api import db_tools import flask_login import itsdangerous +import qrcode import sqlalchemy # Own Modules @@ -163,7 +166,7 @@ def activate_totp(token): form = forms.ActivateTOTPForm() - dds_web.security.verify_activate_totp_token(token) + dds_web.security.auth.verify_activate_totp_token(token, current_user=user) if flask.request.method == "GET": if user.totp_enabled: @@ -171,9 +174,19 @@ def activate_totp(token): return flask.redirect(flask.url_for("auth_blueprint.index")) user.setup_totp_secret() - secret, uri = user.get_totp_secret() + totp_secret, totp_uri = user.get_totp_secret() - return flask.render_template("user/activate_totp.html", secret=secret, uri=uri, form=form) + qrcode = qrcode.make(totp_uri) + stream = io.BytesIO() + qrcode.save(stream, "svg") + + return flask.render_template( + "user/activate_totp.html", + totp_secret=totp_secret, + totp_uri=totp_uri, + qrcode=base64.b64encode(stream.getvalue()), + form=form, + ) # POST request if form.validate_on_submit(): From f0c482f1b5218f3713f76fb4abe2ada7f3ca21bf Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 24 Feb 2022 08:58:47 +0100 Subject: [PATCH 06/63] First try with the migration --- .../versions/be2fa7a5f606_adding_totp.py | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 migrations/versions/be2fa7a5f606_adding_totp.py diff --git a/migrations/versions/be2fa7a5f606_adding_totp.py b/migrations/versions/be2fa7a5f606_adding_totp.py new file mode 100644 index 000000000..757dad46f --- /dev/null +++ b/migrations/versions/be2fa7a5f606_adding_totp.py @@ -0,0 +1,41 @@ +"""Adding TOTP + +Revision ID: be2fa7a5f606 +Revises: a5a40d843415 +Create Date: 2022-02-23 15:30:42.423890 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.orm.session import Session +from sqlalchemy.dialects import mysql +from dds_web.database import models + + +# revision identifiers, used by Alembic. +revision = "be2fa7a5f606" +down_revision = "a5a40d843415" +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column("users", sa.Column("totp_enabled", sa.Boolean(), nullable=False)) + op.add_column("users", sa.Column("_totp_secret", sa.LargeBinary(length=64), nullable=True)) + op.add_column("users", sa.Column("totp_last_verified", sa.DateTime(), nullable=True)) + + session = Session(bind=op.get_bind()) + all_user_rows = session.query(models.User).all() + for user in all_user_rows: + user.totp_enabled = False + session.commit() + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column("users", "totp_last_verified") + op.drop_column("users", "_totp_secret") + op.drop_column("users", "totp_enabled") + # ### end Alembic commands ### From 9ed6714a5ac23bdeee756c311261589898b55abc Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 24 Feb 2022 15:21:45 +0100 Subject: [PATCH 07/63] Activating TOTP sort of works --- dds_web/api/__init__.py | 3 + dds_web/api/user.py | 2 +- dds_web/database/models.py | 37 ++++++++---- dds_web/templates/user/activate_totp.html | 70 +++++++++++++++++++++++ dds_web/web/user.py | 56 +++++++++++------- requirements.txt | 2 + 6 files changed, 136 insertions(+), 34 deletions(-) create mode 100644 dds_web/templates/user/activate_totp.html diff --git a/dds_web/api/__init__.py b/dds_web/api/__init__.py index 606ed67ad..2eb4807d4 100644 --- a/dds_web/api/__init__.py +++ b/dds_web/api/__init__.py @@ -67,6 +67,9 @@ def output_json(data, code, headers=None): api.add_resource(user.DeleteUserSelf, "/user/delete_self", endpoint="delete_user_self") api.add_resource(user.RemoveUserAssociation, "/user/access/revoke", endpoint="revoke_from_project") api.add_resource(user.UserActivation, "/user/activation", endpoint="user_activation") +api.add_resource( + user.RequestTOTPActivation, "/user/request_activate_totp", endpoint="request_totp_activation" +) # Invoicing ############################################################################ Invoicing # api.add_resource(user.ShowUsage, "/usage", endpoint="usage") diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 7442a5d54..5ffce8c93 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -780,7 +780,7 @@ def post(self): additional_claims={"act": "totp"}, # Open for suggestions ) - link = flask.url_for(dds_web.auth_blueprint.activate_totp, token=token, _external=True) + link = flask.url_for("auth_blueprint.activate_totp", token=token, _external=True) # Send activation token to email to work as a validation step # TODO: refactor this since the email sending code is replicated in many places recipients = [user.primary_email] diff --git a/dds_web/database/models.py b/dds_web/database/models.py index d80f7c2cd..8e40af609 100644 --- a/dds_web/database/models.py +++ b/dds_web/database/models.py @@ -7,6 +7,7 @@ # Standard library import datetime import os +import time # Installed import sqlalchemy @@ -364,7 +365,7 @@ class User(flask_login.UserMixin, db.Model): hotp_counter = db.Column(db.BigInteger, unique=False, nullable=False, default=0) hotp_issue_time = db.Column(db.DateTime, unique=False, nullable=True) totp_enabled = db.Column(db.Boolean, unique=False, nullable=False, default=False) - _totp_secret = db.Column(db.LargeBinary(64), unique=False, nullable=False) + _totp_secret = db.Column(db.LargeBinary(64), unique=False, nullable=True) totp_last_verified = db.Column(db.DateTime, unique=False, nullable=True) active = db.Column(db.Boolean) @@ -492,9 +493,18 @@ def verify_HOTP(self, token): self.hotp_issue_time = None db.session.commit() + @property + def totp_initiated(self): + return self._totp_secret is not None + + def setup_totp_secret(self): - """Generate random 512 bit as the new totp secret and return provisioning URI""" - self._totp_secret = os.urandom(64) + """Generate random 160 bit as the new totp secret and return provisioning URI + We're using SHA1 (Google Authenticator seems to only use SHA1 and 6 digit codes) + so secret should be at least 160 bits + https://cryptography.io/en/latest/hazmat/primitives/twofactor/#cryptography.hazmat.primitives.twofactor.totp.TOTP + """ + self._totp_secret = os.urandom(20) db.session.commit() def get_totp_secret(self): @@ -502,8 +512,12 @@ def get_totp_secret(self): if self.totp_enabled: # Can not be fetched again after it has been enabled raise AuthenticationError("TOTP secret already enabled.") - totp = twofactor_totp.TOTP(self._totp_secret, 6, hashes.SHA512(), 30) - return totp.get_provisioning_uri(account_name=self.username, issuer="Data Delivery System") + totp = twofactor_totp.TOTP(self._totp_secret, 6, hashes.SHA1(), 30) + + return self._totp_secret, totp.get_provisioning_uri( + account_name=self.username, + issuer="Data Delivery System", + ) def activate_totp(self): """Set TOTP as the preferred means of second factor authentication. @@ -521,22 +535,23 @@ def verify_totp(self, token): """ # can't use totp successfully more than once within 90 seconds. # Time frame chosen so that no one can use the same token more than once + # No need to use epoch time here. current_time = dds_web.utils.current_time() - if self.totp_last_used is not None and ( - current_time - self.totp_last_used < datetime.timedelta(seconds=90) + if self.totp_last_verified is not None and ( + current_time - self.totp_last_verified < datetime.timedelta(seconds=90) ): raise AuthenticationError( "Authentication with TOTP needs to be at least 90 seconds apart." ) # construct object - totp = twofactor_totp.TOTP(self._totp_secret, 6, hashes.SHA512(), 30) + totp = twofactor_totp.TOTP(self._totp_secret, 6, hashes.SHA1(), 30) - # attempt to verify the token + # attempt to verify the token using epoch time # Allow for clock drift of 1 frame before or after verified = False for t_diff in [-30, 0, 30]: - verification_time = (current_time + datetime.timedelta(seconds=t_diff)).strftime("%s") + verification_time = time.time() + t_diff try: totp.verify(token, verification_time) verified = True @@ -548,7 +563,7 @@ def verify_totp(self, token): raise AuthenticationError("Invalid TOTP token.") # if the token is valid, save time of last successful verification - self.totp_last_used = current_time + self.totp_last_verified = current_time db.session.commit() # Email related diff --git a/dds_web/templates/user/activate_totp.html b/dds_web/templates/user/activate_totp.html new file mode 100644 index 000000000..05f660184 --- /dev/null +++ b/dds_web/templates/user/activate_totp.html @@ -0,0 +1,70 @@ +{% extends "bootstrap/base.html" %} +{% import "bootstrap/wtf.html" as wtf %} + +{% block content %} + +
+ {% with messages = get_flashed_messages(with_categories=true) %} + {% if messages %} +
+ {% for category, message in messages %} + + {% endfor %} +
+ {% endif %} + {% endwith %} +
+

Activate one-time authentication using TOTP

+
Instructions:
+ +
+
+

QR code

+
+ {{ qr_code | safe }} +
+
+
+

Secret

+

{{totp_secret}}

+
+
+

TOTP URI

+

{{totp_uri}}

+
+
+

Codes

+

{{codes}}

+
+
+
+ {{ form.csrf_token }} + + + {{ form.totp.label }} + {{ form.totp }} +
    + {% for error in form.totp.errors %} +
  • + {{ error }} +
  • + {% endfor %} +
+ + + {{ form.submit }} + +
+
+ +
+ +{% endblock %} \ No newline at end of file diff --git a/dds_web/web/user.py b/dds_web/web/user.py index cbecce5b7..e639da67b 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -17,6 +17,7 @@ import flask_login import itsdangerous import qrcode +import qrcode.image.svg import sqlalchemy # Own Modules @@ -168,38 +169,49 @@ def activate_totp(token): dds_web.security.auth.verify_activate_totp_token(token, current_user=user) - if flask.request.method == "GET": - if user.totp_enabled: - flask.flash("Two-factor authentication via TOTP is already enabled.") - return flask.redirect(flask.url_for("auth_blueprint.index")) + if user.totp_enabled: + flask.flash("Two-factor authentication via TOTP is already enabled.") + return flask.redirect(flask.url_for("auth_blueprint.index")) + # Don't change secret on page reload + if not user.totp_initiated: user.setup_totp_secret() - totp_secret, totp_uri = user.get_totp_secret() - qrcode = qrcode.make(totp_uri) - stream = io.BytesIO() - qrcode.save(stream, "svg") + totp_secret, totp_uri = user.get_totp_secret() - return flask.render_template( - "user/activate_totp.html", - totp_secret=totp_secret, - totp_uri=totp_uri, - qrcode=base64.b64encode(stream.getvalue()), - form=form, - ) + # QR code generation + image = qrcode.make(totp_uri, image_factory=qrcode.image.svg.SvgImage) + stream = io.BytesIO() + image.save(stream) # POST request if form.validate_on_submit(): - if user.verify_totp(form.totp_code.data.encode()): - user.activate_totp() + try: + user.verify_totp(form.totp.data.encode()) + except ddserr.AuthenticationError: + flask.flash("Invalid two-factor authentication code.") + return flask.render_template( + "user/activate_totp.html", + totp_secret=base64.b32encode(totp_secret).decode("utf-8"), + totp_uri=totp_uri, + qr_code=stream.getvalue().decode("utf-8"), + token=token, + form=form, + ) - flask.flash("Two-factor authentication via TOTP has been enabled.") - return flask.redirect(flask.url_for("auth_blueprint.index")) + user.activate_totp() - flask.flash("Invalid two-factor authentication code.") + flask.flash("Two-factor authentication via TOTP has been enabled.") + return flask.redirect(flask.url_for("auth_blueprint.index")) - secret, uri = user.get_totp_secret() - return flask.render_template("user/activate_totp.html", secret=secret, uri=uri, form=form) + return flask.render_template( + "user/activate_totp.html", + totp_secret=base64.b32encode(totp_secret).decode("utf-8"), + totp_uri=totp_uri, + qr_code=stream.getvalue().decode("utf-8"), + token=token, + form=form, + ) @auth_blueprint.route("/cancel_2fa", methods=["POST"]) diff --git a/requirements.txt b/requirements.txt index 0772d0026..bd9417266 100644 --- a/requirements.txt +++ b/requirements.txt @@ -40,6 +40,7 @@ MarkupSafe==2.0.1 marshmallow==3.14.1 marshmallow-sqlalchemy==0.27.0 packaging==21.3 +Pillow==9.0.1 pycparser==2.21 PyMySQL==1.0.2 PyNaCl==1.5.0 @@ -48,6 +49,7 @@ PyQRCode==1.2.1 python-dateutil==2.8.2 pytz==2021.3 pytz-deprecation-shim==0.1.0.post0 +qrcode==7.3.1 redis==4.1.2 requests==2.27.1 s3transfer==0.5.1 From 099c774c3031f7eb09a9a0d7e0cb56f8c12fc267 Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 24 Feb 2022 16:36:37 +0100 Subject: [PATCH 08/63] Better error checking for second factor --- dds_web/api/schemas/token_schemas.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dds_web/api/schemas/token_schemas.py b/dds_web/api/schemas/token_schemas.py index 63da399a9..3e24d23b1 100644 --- a/dds_web/api/schemas/token_schemas.py +++ b/dds_web/api/schemas/token_schemas.py @@ -53,9 +53,17 @@ def validate_mfa(self, data, **kwargs): if user.totp_enabled: value = data.get("TOTP") + if value is None: + raise marshmallow.ValidationError( + "Your account is setup to use TOTP, but you entered a one-time authentication code from email." + ) # Raises authentication error if TOTP is incorrect user.verify_TOTP(value.encode()) else: value = data.get("HOTP") + if value is None: + raise marshmallow.ValidationError( + "Your account is setup to use one-time authentication code via email, you cannot authenticate with TOTP." + ) # Raises authenticationerror if invalid user.verify_HOTP(value.encode()) From 14dc2354bdd1cea9bd8a9e26d0a4cba64a41ae8c Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 24 Feb 2022 16:37:44 +0100 Subject: [PATCH 09/63] Fixing authentication with TOTP --- dds_web/api/user.py | 5 ++++- dds_web/database/models.py | 3 +-- dds_web/web/user.py | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 5ffce8c93..6c88fd70f 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -753,7 +753,10 @@ def get(self): args = flask.request.json or {} - token_schemas.TokenSchema().load(args) + try: + token_schemas.TokenSchema().load(args) + except marshmallow.ValidationError as err: + raise ddserr.AuthenticationError(message=err.messages) token_claims = dds_web.security.auth.obtain_current_encrypted_token_claims() diff --git a/dds_web/database/models.py b/dds_web/database/models.py index 8e40af609..b35882474 100644 --- a/dds_web/database/models.py +++ b/dds_web/database/models.py @@ -497,7 +497,6 @@ def verify_HOTP(self, token): def totp_initiated(self): return self._totp_secret is not None - def setup_totp_secret(self): """Generate random 160 bit as the new totp secret and return provisioning URI We're using SHA1 (Google Authenticator seems to only use SHA1 and 6 digit codes) @@ -526,7 +525,7 @@ def activate_totp(self): self.totp_enabled = True db.session.commit() - def verify_totp(self, token): + def verify_TOTP(self, token): """Verify the totp token. Checks the previous, current and comming time frame to allow for some clock drift. diff --git a/dds_web/web/user.py b/dds_web/web/user.py index e639da67b..5f9e690e1 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -187,7 +187,7 @@ def activate_totp(token): # POST request if form.validate_on_submit(): try: - user.verify_totp(form.totp.data.encode()) + user.verify_TOTP(form.totp.data.encode()) except ddserr.AuthenticationError: flask.flash("Invalid two-factor authentication code.") return flask.render_template( From 530cddf6ffa707fe37c27c01aeefeedc5247c5ca Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 24 Feb 2022 19:38:01 +0100 Subject: [PATCH 10/63] Updated and formatted changelog --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c748e005..8ff74b5e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Please add a _short_ line describing the PR you make, if the PR implements a specific feature or functionality, or refactor. Not needed if you add very small and unnoticable changes. ## Sprint (2022-02-09 - 2022-02-23) + * Secure operations that require cryptographic keys are protected for each user with the user's password ([#889](https://github.com/ScilifelabDataCentre/dds_web/pull/889)) * Implemented the functionality to add project to the invites of a new user as outlined in [issue 887](https://github.com/scilifelabdatacentre/dds_web/issues/887) ([PR888](https://github.com/ScilifelabDataCentre/dds_web/pull/888)). * Create endpoint for renewing users project access, e.g. after password reset ([886](https://github.com/ScilifelabDataCentre/dds_web/pull/885)) @@ -20,6 +21,8 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe * Rearrangement and clean up of the token ([910](https://github.com/ScilifelabDataCentre/dds_web/pull/910)) ## Sprint (2022-02-23 - 2022-03-09) + * Add landing page after password reset ([931](https://github.com/ScilifelabDataCentre/dds_web/pull/931)) * Add endpoint for health check (intended for readinessProbe) ([933](https://github.com/ScilifelabDataCentre/dds_web/pull/933)) -* Introduced a `--no-mail` flag in the CLI respectively a `send_email: True/False` json parameter to fix [issue 924](https://github.com/scilifelabdatacentre/dds_web/issues/924) ([#926](https://github.com/ScilifelabDataCentre/dds_web/pull/926)) \ No newline at end of file +* Introduced a `--no-mail` flag in the CLI respectively a `send_email: True/False` json parameter to fix [issue 924](https://github.com/scilifelabdatacentre/dds_web/issues/924) ([#926](https://github.com/ScilifelabDataCentre/dds_web/pull/926)) +* Now possible to switch to using TOTP for MFA [936](https://github.com/scilifelabdatacentre/dds_web/issues/936) From 10f2ca37b316b31ee03f6193132bd64d45cd9e6b Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 24 Feb 2022 19:38:16 +0100 Subject: [PATCH 11/63] Added flash messages to index --- dds_web/templates/index.html | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/dds_web/templates/index.html b/dds_web/templates/index.html index c7e1f4c38..838e513f0 100644 --- a/dds_web/templates/index.html +++ b/dds_web/templates/index.html @@ -2,6 +2,17 @@ {% import "bootstrap/wtf.html" as wtf %} {% block content %} +{% with messages = get_flashed_messages(with_categories=true) %} +{% if messages %} +
+ {% for category, message in messages %} + + {% endfor %} +
+{% endif %} +{% endwith %} This page will have something on it later.
{{ form.csrf_token }} From 1b106d2dfbaf6c90bb0f430ae3137ff3ee31c9fd Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 24 Feb 2022 19:38:42 +0100 Subject: [PATCH 12/63] Removed debug codes from totp html --- dds_web/templates/user/activate_totp.html | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dds_web/templates/user/activate_totp.html b/dds_web/templates/user/activate_totp.html index 05f660184..7a5f8464b 100644 --- a/dds_web/templates/user/activate_totp.html +++ b/dds_web/templates/user/activate_totp.html @@ -40,10 +40,6 @@

Secret

TOTP URI

{{totp_uri}}

-
-

Codes

-

{{codes}}

-
{{ form.csrf_token }} From d4c8cbc8d6312a11abb592acc7a229791e469a0d Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 24 Feb 2022 19:38:58 +0100 Subject: [PATCH 13/63] Added means of limiting caching --- dds_web/web/user.py | 44 ++++++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/dds_web/web/user.py b/dds_web/web/user.py index 7d4a2bb50..36630bcfe 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -190,13 +190,21 @@ def activate_totp(token): user.verify_TOTP(form.totp.data.encode()) except ddserr.AuthenticationError: flask.flash("Invalid two-factor authentication code.") - return flask.render_template( - "user/activate_totp.html", - totp_secret=base64.b32encode(totp_secret).decode("utf-8"), - totp_uri=totp_uri, - qr_code=stream.getvalue().decode("utf-8"), - token=token, - form=form, + return ( + flask.render_template( + "user/activate_totp.html", + totp_secret=base64.b32encode(totp_secret).decode("utf-8"), + totp_uri=totp_uri, + qr_code=stream.getvalue().decode("utf-8"), + token=token, + form=form, + ), + 200, + { + "Cache-Control": "no-cache, no-store, must-revalidate", + "Pragma": "no-cache", + "Expires": "0", + }, ) user.activate_totp() @@ -204,13 +212,21 @@ def activate_totp(token): flask.flash("Two-factor authentication via TOTP has been enabled.") return flask.redirect(flask.url_for("auth_blueprint.index")) - return flask.render_template( - "user/activate_totp.html", - totp_secret=base64.b32encode(totp_secret).decode("utf-8"), - totp_uri=totp_uri, - qr_code=stream.getvalue().decode("utf-8"), - token=token, - form=form, + return ( + flask.render_template( + "user/activate_totp.html", + totp_secret=base64.b32encode(totp_secret).decode("utf-8"), + totp_uri=totp_uri, + qr_code=stream.getvalue().decode("utf-8"), + token=token, + form=form, + ), + 200, + { + "Cache-Control": "no-cache, no-store, must-revalidate", + "Pragma": "no-cache", + "Expires": "0", + }, ) From 2acdb1b579b6babe7146c6ff7d1f82b4b2e4129f Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 24 Feb 2022 20:48:09 +0100 Subject: [PATCH 14/63] Clarified totp_initiated in models --- dds_web/database/models.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dds_web/database/models.py b/dds_web/database/models.py index b35882474..885476d96 100644 --- a/dds_web/database/models.py +++ b/dds_web/database/models.py @@ -495,6 +495,8 @@ def verify_HOTP(self, token): @property def totp_initiated(self): + """To check if activation of TOTP has been initiated, not to be confused + with user.totp_enabled, that indicates if TOTP is successfully enabled for the user.""" return self._totp_secret is not None def setup_totp_secret(self): From e83790fc6ee1efbf47259eba6e5461693a11e0a7 Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 24 Feb 2022 20:49:15 +0100 Subject: [PATCH 15/63] Adapted confirm_2fa for web --- dds_web/forms.py | 10 +++++- dds_web/templates/user/confirm2fa.html | 19 ++++++++++- dds_web/web/user.py | 46 +++++++++++++++++--------- 3 files changed, 58 insertions(+), 17 deletions(-) diff --git a/dds_web/forms.py b/dds_web/forms.py index 35371b7d6..9fe9b5d7c 100644 --- a/dds_web/forms.py +++ b/dds_web/forms.py @@ -71,7 +71,7 @@ class LogoutForm(flask_wtf.FlaskForm): logout = wtforms.SubmitField("Logout") -class Confirm2FACodeForm(flask_wtf.FlaskForm): +class Confirm2FACodeHOTPForm(flask_wtf.FlaskForm): hotp = wtforms.StringField( "hotp", validators=[wtforms.validators.InputRequired(), wtforms.validators.Length(min=8, max=8)], @@ -79,6 +79,14 @@ class Confirm2FACodeForm(flask_wtf.FlaskForm): submit = wtforms.SubmitField("Authenticate") +class Confirm2FACodeTOTPForm(flask_wtf.FlaskForm): + totp = wtforms.StringField( + "totp", + validators=[wtforms.validators.InputRequired(), wtforms.validators.Length(min=6, max=6)], + ) + submit = wtforms.SubmitField("Authenticate") + + class ActivateTOTPForm(flask_wtf.FlaskForm): totp = wtforms.StringField( "totp", diff --git a/dds_web/templates/user/confirm2fa.html b/dds_web/templates/user/confirm2fa.html index 57c1a0210..b0265b6c0 100644 --- a/dds_web/templates/user/confirm2fa.html +++ b/dds_web/templates/user/confirm2fa.html @@ -17,13 +17,28 @@

Enter one-time authentication code

{% endif %} {% endwith %} + {% if using_totp %} +

Please complete the login by entering the one-time authentication code displayed in your authenticator app.

+ {% else %}

Please complete the login by entering the one-time authentication code that was sent to you. The one-time codes are valid for a short time (15 minutes) after they have been issued.

- + {% endif %} + {{ form.csrf_token }} + {% if using_totp %} + {{ form.totp.label }} + {{ form.totp }} +
    + {% for error in form.totp.errors %} +
  • + {{ error }} +
  • + {% endfor %} +
+ {% else %} {{ form.hotp.label }} {{ form.hotp }}
    @@ -33,6 +48,8 @@

    Enter one-time authentication code

    {% endfor %}
+ {% endif %} + {{ form.submit }} diff --git a/dds_web/web/user.py b/dds_web/web/user.py index 36630bcfe..18ec79ca4 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -254,10 +254,6 @@ def confirm_2fa(): if flask_login.current_user.is_authenticated: return flask.redirect(flask.url_for("auth_blueprint.index")) - form = forms.Confirm2FACodeForm() - - cancel_form = forms.Cancel2FAForm() - next = flask.request.args.get("next") # is_safe_url should check if the url is safe for redirects. if next and not dds_web.utils.is_safe_url(next): @@ -280,6 +276,13 @@ def confirm_2fa(): ) return flask.redirect(flask.url_for("auth_blueprint.login", next=next)) + if user.totp_enabled: + form = forms.Confirm2FACodeTOTPForm() + else: + form = forms.Confirm2FACodeHOTPForm() + + cancel_form = forms.Cancel2FAForm() + # Valid 2fa initiated token, but user does not exist (should never happen) if not user: flask.session.pop("2fa_initiated_token", None) @@ -288,20 +291,29 @@ def confirm_2fa(): if form.validate_on_submit(): - hotp_value = form.hotp.data + if user.totp_enabled: + twofactor_value = form.totp.data + twofactor_verify = user.verify_TOTP + else: + twofactor_value = form.hotp.data + twofactor_verify = user.verify_HOTP # Raises authenticationerror if invalid try: - user.verify_HOTP(hotp_value.encode()) - except ddserr.AuthenticationError: - flask.flash("Invalid one-time code.") + twofactor_verify(twofactor_value.encode()) + except ddserr.AuthenticationError as err: + flask.flash(f"Invalid one-time code: {err.description}") return flask.redirect( flask.url_for( - "auth_blueprint.confirm_2fa", form=form, cancel_form=cancel_form, next=next + "auth_blueprint.confirm_2fa", + form=form, + cancel_form=cancel_form, + next=next, + using_totp=user.totp_enabled, ) ) - # Correct username, password and hotp code --> log user in + # Correct username, password and twofactor code --> log user in flask_login.login_user(user) flask.flash("Logged in successfully.") # Remove token from session @@ -311,7 +323,11 @@ def confirm_2fa(): else: return flask.render_template( - "user/confirm2fa.html", form=form, cancel_form=cancel_form, next=next + "user/confirm2fa.html", + form=form, + cancel_form=cancel_form, + next=next, + using_totp=user.totp_enabled, ) @@ -351,10 +367,10 @@ def login(): ) # Try login again # Correct credentials still needs 2fa - - # Send 2fa token to user's email - if dds_web.security.auth.send_hotp_email(user): - flask.flash("One-Time Code has been sent to your primary email.") + if not user.totp_enabled: + # Send 2fa token to user's email + if dds_web.security.auth.send_hotp_email(user): + flask.flash("One-Time Code has been sent to your primary email.") # Generate signed token that indicates that the user has authenticated token_2fa_initiated = dds_web.security.tokens.jwt_token( From 041d1b05744fa21b909b27d353172109a254799b Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 3 Mar 2022 09:06:37 +0100 Subject: [PATCH 16/63] started with some tests for TOTP --- dds_web/database/models.py | 10 ++++++++-- tests/test_basic_api.py | 41 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/dds_web/database/models.py b/dds_web/database/models.py index 885476d96..ef2cf1184 100644 --- a/dds_web/database/models.py +++ b/dds_web/database/models.py @@ -499,6 +499,12 @@ def totp_initiated(self): with user.totp_enabled, that indicates if TOTP is successfully enabled for the user.""" return self._totp_secret is not None + def totp_object(self): + """Google Authenticator seems to only be able to handle SHA1 and 6 digit codes""" + if self.totp_initiated: + return twofactor_totp.TOTP(self._totp_secret, 6, hashes.SHA1(), 30) + return None + def setup_totp_secret(self): """Generate random 160 bit as the new totp secret and return provisioning URI We're using SHA1 (Google Authenticator seems to only use SHA1 and 6 digit codes) @@ -513,7 +519,7 @@ def get_totp_secret(self): if self.totp_enabled: # Can not be fetched again after it has been enabled raise AuthenticationError("TOTP secret already enabled.") - totp = twofactor_totp.TOTP(self._totp_secret, 6, hashes.SHA1(), 30) + totp = self.totp_object() return self._totp_secret, totp.get_provisioning_uri( account_name=self.username, @@ -546,7 +552,7 @@ def verify_TOTP(self, token): ) # construct object - totp = twofactor_totp.TOTP(self._totp_secret, 6, hashes.SHA1(), 30) + totp = self.totp_object() # attempt to verify the token using epoch time # Allow for clock drift of 1 frame before or after diff --git a/tests/test_basic_api.py b/tests/test_basic_api.py index 4dbb42171..e6a526fa8 100644 --- a/tests/test_basic_api.py +++ b/tests/test_basic_api.py @@ -3,11 +3,13 @@ # Standard library import http import datetime +import time import unittest # Installed import flask import flask_mail +import pytest # Own import tests @@ -83,7 +85,7 @@ def test_auth_correct_credentials(client): assert mock_mail_send.call_count == 0 -# Second Factor #################################################################### Second Factor # +# Second Factor ################################################################### Second Factor # def test_auth_second_factor_empty(client): @@ -99,6 +101,9 @@ def test_auth_second_factor_empty(client): assert "Invalid token" == response_json.get("message") +# HOTP ##################################################################################### HOTP # + + def test_auth_second_factor_incorrect_token(client): """ Test that the two_factor endpoint called with incorrect partial token returns 401/UNAUTHORIZED @@ -227,7 +232,39 @@ def test_auth_second_factor_correctauth_reused_hotp_401_unauthorized(client): assert "Invalid one-time authentication code." == response_json.get("message") -# Token Authentication ###################################################### Token Authentication # +# TOTP ##################################################################################### TOTP # + + +@pytest.fixture() +def totp_for_user(client): + """Create a user with TOTP enabled and return TOTP object""" + user = dds_web.database.models.User.query.filter_by(username="researchuser").first() + user.setup_totp_secret() + user.activate_totp() + return user.totp_object() + + +def test_auth_second_factor_TOTP_incorrect_token(client, totp_for_user): + """ + Test that the two_factor endpoint called with incorrect partial token returns 401/UNAUTHORIZED + """ + user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) + + totp_token = totp_for_user.generate(time.time()) + + response = client.get( + tests.DDSEndpoint.SECOND_FACTOR, + headers={"Authorization": f"Bearer made.up.token.long.version"}, + json={"TOTP": totp_token.decode()}, + ) + + assert response.status_code == http.HTTPStatus.UNAUTHORIZED + response_json = response.json + assert response_json.get("message") + assert "Invalid token" == response_json.get("message") + + +# Token Authentication ##################################################### Token Authentication # def test_auth_incorrect_token_without_periods(client): From 05027716c617a46f8e185da7c9c077c857203cb0 Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Fri, 4 Mar 2022 07:12:25 +0100 Subject: [PATCH 17/63] ADded a new attempt on migration --- .../versions/6f68e3673bd5_adding_totp.py | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 migrations/versions/6f68e3673bd5_adding_totp.py diff --git a/migrations/versions/6f68e3673bd5_adding_totp.py b/migrations/versions/6f68e3673bd5_adding_totp.py new file mode 100644 index 000000000..0a388509f --- /dev/null +++ b/migrations/versions/6f68e3673bd5_adding_totp.py @@ -0,0 +1,40 @@ +"""adding_totp + +Revision ID: 6f68e3673bd5 +Revises: 666003748d14 +Create Date: 2022-03-03 08:32:21.500977 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.orm.session import Session +from sqlalchemy.dialects import mysql +from dds_web.database import models + +# revision identifiers, used by Alembic. +revision = "6f68e3673bd5" +down_revision = "666003748d14" +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column("users", sa.Column("totp_enabled", sa.Boolean(), nullable=False)) + op.add_column("users", sa.Column("_totp_secret", sa.LargeBinary(length=64), nullable=True)) + op.add_column("users", sa.Column("totp_last_verified", sa.DateTime(), nullable=True)) + + session = Session(bind=op.get_bind()) + all_user_rows = session.query(models.User).all() + for user in all_user_rows: + user.totp_enabled = False + session.commit() + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column("users", "totp_last_verified") + op.drop_column("users", "_totp_secret") + op.drop_column("users", "totp_enabled") + # ### end Alembic commands ### From 67b49280c242be26b1848e00c547dbec2c0a06ea Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 31 Mar 2022 16:16:54 +0200 Subject: [PATCH 18/63] Deleted migration --- .../versions/be2fa7a5f606_adding_totp.py | 41 ------------------- 1 file changed, 41 deletions(-) delete mode 100644 migrations/versions/be2fa7a5f606_adding_totp.py diff --git a/migrations/versions/be2fa7a5f606_adding_totp.py b/migrations/versions/be2fa7a5f606_adding_totp.py deleted file mode 100644 index 757dad46f..000000000 --- a/migrations/versions/be2fa7a5f606_adding_totp.py +++ /dev/null @@ -1,41 +0,0 @@ -"""Adding TOTP - -Revision ID: be2fa7a5f606 -Revises: a5a40d843415 -Create Date: 2022-02-23 15:30:42.423890 - -""" -from alembic import op -import sqlalchemy as sa -from sqlalchemy.orm.session import Session -from sqlalchemy.dialects import mysql -from dds_web.database import models - - -# revision identifiers, used by Alembic. -revision = "be2fa7a5f606" -down_revision = "a5a40d843415" -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column("users", sa.Column("totp_enabled", sa.Boolean(), nullable=False)) - op.add_column("users", sa.Column("_totp_secret", sa.LargeBinary(length=64), nullable=True)) - op.add_column("users", sa.Column("totp_last_verified", sa.DateTime(), nullable=True)) - - session = Session(bind=op.get_bind()) - all_user_rows = session.query(models.User).all() - for user in all_user_rows: - user.totp_enabled = False - session.commit() - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.drop_column("users", "totp_last_verified") - op.drop_column("users", "_totp_secret") - op.drop_column("users", "totp_enabled") - # ### end Alembic commands ### From 643dbed84a93aa2ad33d0e185ef97182e3acc995 Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 31 Mar 2022 21:59:24 +0200 Subject: [PATCH 19/63] Generated new migration for TOTP --- .../versions/6f68e3673bd5_adding_totp.py | 40 ------------------- migrations/versions/ee441d642d7a_add_totp.py | 39 ++++++++++++++++++ 2 files changed, 39 insertions(+), 40 deletions(-) delete mode 100644 migrations/versions/6f68e3673bd5_adding_totp.py create mode 100644 migrations/versions/ee441d642d7a_add_totp.py diff --git a/migrations/versions/6f68e3673bd5_adding_totp.py b/migrations/versions/6f68e3673bd5_adding_totp.py deleted file mode 100644 index 0a388509f..000000000 --- a/migrations/versions/6f68e3673bd5_adding_totp.py +++ /dev/null @@ -1,40 +0,0 @@ -"""adding_totp - -Revision ID: 6f68e3673bd5 -Revises: 666003748d14 -Create Date: 2022-03-03 08:32:21.500977 - -""" -from alembic import op -import sqlalchemy as sa -from sqlalchemy.orm.session import Session -from sqlalchemy.dialects import mysql -from dds_web.database import models - -# revision identifiers, used by Alembic. -revision = "6f68e3673bd5" -down_revision = "666003748d14" -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column("users", sa.Column("totp_enabled", sa.Boolean(), nullable=False)) - op.add_column("users", sa.Column("_totp_secret", sa.LargeBinary(length=64), nullable=True)) - op.add_column("users", sa.Column("totp_last_verified", sa.DateTime(), nullable=True)) - - session = Session(bind=op.get_bind()) - all_user_rows = session.query(models.User).all() - for user in all_user_rows: - user.totp_enabled = False - session.commit() - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.drop_column("users", "totp_last_verified") - op.drop_column("users", "_totp_secret") - op.drop_column("users", "totp_enabled") - # ### end Alembic commands ### diff --git a/migrations/versions/ee441d642d7a_add_totp.py b/migrations/versions/ee441d642d7a_add_totp.py new file mode 100644 index 000000000..c10f76cd8 --- /dev/null +++ b/migrations/versions/ee441d642d7a_add_totp.py @@ -0,0 +1,39 @@ +"""add_totp + +Revision ID: ee441d642d7a +Revises: 1256117ad629 +Create Date: 2022-03-31 19:37:39.689841 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import mysql + +# revision identifiers, used by Alembic. +revision = 'ee441d642d7a' +down_revision = '1256117ad629' +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column('users', sa.Column('totp_enabled', mysql.TINYINT(display_width=1), nullable=True)) + op.add_column('users', sa.Column('_totp_secret', sa.LargeBinary(length=64), nullable=True)) + op.add_column('users', sa.Column('totp_last_verified', sa.DateTime(), nullable=True)) + + # Fill in default value for totp_enabled + user_table = sa.sql.table( + 'users', + sa.sql.column('totp_enabled', mysql.TINYINT(display_width=1)), + ) + op.execute(user_table.update().values(totp_enabled=False)) + + # Make totp_enabled not nullable + op.alter_column('users', 'totp_enabled', existing_type=mysql.TINYINT(display_width=1), nullable=False) + + +def downgrade(): + op.drop_column('users', 'totp_last_verified') + op.drop_column('users', '_totp_secret') + op.drop_column('users', 'totp_enabled') + # ### end Alembic commands ### From 1619e98850e19137ccde2fec34bc0b6873ed487c Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 31 Mar 2022 22:23:31 +0200 Subject: [PATCH 20/63] Adapted the totp pages a bit --- dds_web/templates/user/activate_totp.html | 9 +++++---- dds_web/web/user.py | 12 ++++-------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/dds_web/templates/user/activate_totp.html b/dds_web/templates/user/activate_totp.html index 7a5f8464b..b81ff3db3 100644 --- a/dds_web/templates/user/activate_totp.html +++ b/dds_web/templates/user/activate_totp.html @@ -1,7 +1,8 @@ -{% extends "bootstrap/base.html" %} -{% import "bootstrap/wtf.html" as wtf %} - -{% block content %} +{% extends "base.html" %} +{% block page_title -%} +Activate TOTP +{% endblock %} +{% block body %}
{% with messages = get_flashed_messages(with_categories=true) %} diff --git a/dds_web/web/user.py b/dds_web/web/user.py index 81950c950..ff93873ba 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -168,7 +168,7 @@ def activate_totp(token): if user.totp_enabled: flask.flash("Two-factor authentication via TOTP is already enabled.") - return flask.redirect(flask.url_for("auth_blueprint.index")) + return flask.redirect(flask.url_for("pages.home")) # Don't change secret on page reload if not user.totp_initiated: @@ -207,7 +207,7 @@ def activate_totp(token): user.activate_totp() flask.flash("Two-factor authentication via TOTP has been enabled.") - return flask.redirect(flask.url_for("auth_blueprint.index")) + return flask.redirect(flask.url_for("pages.home")) return ( flask.render_template( @@ -250,10 +250,6 @@ def confirm_2fa(): if flask_login.current_user.is_authenticated: return flask.redirect(flask.url_for("pages.home")) - form = forms.Confirm2FACodeForm() - - cancel_form = forms.Cancel2FAForm() - next_target = flask.request.args.get("next") # is_safe_url should check if the url is safe for redirects. if next_target and not dds_web.utils.is_safe_url(next_target): @@ -534,10 +530,10 @@ def password_reset_completed(): user = dds_web.security.auth.verify_password_reset_token(token=token) if not user.is_active: flask.flash("Your account is not active.", "warning") - return flask.redirect(flask.url_for("auth_blueprint.index")) + return flask.redirect(flask.url_for("pages.home")) except ddserr.AuthenticationError: flask.flash("That is an invalid or expired token", "warning") - return flask.redirect(flask.url_for("auth_blueprint.index")) + return flask.redirect(flask.url_for("pages.home")) units_to_contact = {} unit_admins_to_contact = {} From 463c88c2ca5564d96d72becd4fccc2a7e63ab483 Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 7 Apr 2022 09:37:53 +0200 Subject: [PATCH 21/63] Supply secondfactor method to user after correct credentials --- dds_web/api/user.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 4a97f8acb..b12f995d6 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -956,12 +956,14 @@ class EncryptedToken(flask_restful.Resource): @basic_auth.login_required @logging_bind_request def get(self): + secondfactor_method = "TOTP" if auth.current_user().totp_enabled else "HOTP" return { "message": "Please take this token to /user/second_factor to authenticate with MFA!", "token": encrypted_jwt_token( username=auth.current_user().username, sensitive_content=flask.request.authorization.get("password"), ), + "secondfactor_method": secondfactor_method, } From b88105618ce065a368cca13c4baa59d05e4b92cc Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 7 Apr 2022 21:07:59 +0200 Subject: [PATCH 22/63] Removed double flash messages for TOTP activation --- dds_web/templates/user/activate_totp.html | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/dds_web/templates/user/activate_totp.html b/dds_web/templates/user/activate_totp.html index b81ff3db3..b51036c04 100644 --- a/dds_web/templates/user/activate_totp.html +++ b/dds_web/templates/user/activate_totp.html @@ -5,17 +5,6 @@ {% block body %}
- {% with messages = get_flashed_messages(with_categories=true) %} - {% if messages %} -
- {% for category, message in messages %} - - {% endfor %} -
- {% endif %} - {% endwith %}

Activate one-time authentication using TOTP

Instructions:
From 2f24af1477feeadeca6334ef24a5e2d4bd7e69bf Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Wed, 13 Apr 2022 13:55:10 +0200 Subject: [PATCH 23/63] Some fixes for error handling and condition checking --- dds_web/api/user.py | 5 +---- dds_web/security/auth.py | 3 ++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index b12f995d6..aee299e59 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -974,10 +974,7 @@ class SecondFactor(flask_restful.Resource): @handle_validation_errors def get(self): - try: - token_schemas.TokenSchema().load(flask.request.json) - except marshmallow.ValidationError as err: - raise ddserr.AuthenticationError(message=err.messages) + token_schemas.TokenSchema().load(flask.request.json) token_claims = dds_web.security.auth.obtain_current_encrypted_token_claims() diff --git a/dds_web/security/auth.py b/dds_web/security/auth.py index 6e58e2f7f..5b71dc25a 100644 --- a/dds_web/security/auth.py +++ b/dds_web/security/auth.py @@ -365,5 +365,6 @@ def verify_password(username, password): user = models.User.query.get(username) if user and user.is_active and user.verify_password(input_password=password): - send_hotp_email(user) + if not user.totp_enabled: + send_hotp_email(user) return user From ad464f21c84efafaaf676024ff71337ef9218111 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Wed, 13 Apr 2022 13:58:20 +0200 Subject: [PATCH 24/63] Remove totp migration file --- migrations/versions/ee441d642d7a_add_totp.py | 39 -------------------- 1 file changed, 39 deletions(-) delete mode 100644 migrations/versions/ee441d642d7a_add_totp.py diff --git a/migrations/versions/ee441d642d7a_add_totp.py b/migrations/versions/ee441d642d7a_add_totp.py deleted file mode 100644 index c10f76cd8..000000000 --- a/migrations/versions/ee441d642d7a_add_totp.py +++ /dev/null @@ -1,39 +0,0 @@ -"""add_totp - -Revision ID: ee441d642d7a -Revises: 1256117ad629 -Create Date: 2022-03-31 19:37:39.689841 - -""" -from alembic import op -import sqlalchemy as sa -from sqlalchemy.dialects import mysql - -# revision identifiers, used by Alembic. -revision = 'ee441d642d7a' -down_revision = '1256117ad629' -branch_labels = None -depends_on = None - - -def upgrade(): - op.add_column('users', sa.Column('totp_enabled', mysql.TINYINT(display_width=1), nullable=True)) - op.add_column('users', sa.Column('_totp_secret', sa.LargeBinary(length=64), nullable=True)) - op.add_column('users', sa.Column('totp_last_verified', sa.DateTime(), nullable=True)) - - # Fill in default value for totp_enabled - user_table = sa.sql.table( - 'users', - sa.sql.column('totp_enabled', mysql.TINYINT(display_width=1)), - ) - op.execute(user_table.update().values(totp_enabled=False)) - - # Make totp_enabled not nullable - op.alter_column('users', 'totp_enabled', existing_type=mysql.TINYINT(display_width=1), nullable=False) - - -def downgrade(): - op.drop_column('users', 'totp_last_verified') - op.drop_column('users', '_totp_secret') - op.drop_column('users', 'totp_enabled') - # ### end Alembic commands ### From 036adcda4ad54a19015013e046e6bd7bc14a8500 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Wed, 13 Apr 2022 14:00:27 +0200 Subject: [PATCH 25/63] Update template --- dds_web/templates/user/activate_totp.html | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dds_web/templates/user/activate_totp.html b/dds_web/templates/user/activate_totp.html index b51036c04..494fa8c2b 100644 --- a/dds_web/templates/user/activate_totp.html +++ b/dds_web/templates/user/activate_totp.html @@ -46,11 +46,16 @@

TOTP URI

- {{ form.submit }} +
+ +
-{% endblock %} \ No newline at end of file +{% endblock %} From 2e81969a8180bb1978bf67fd0e17a96af0b7162a Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Wed, 13 Apr 2022 14:06:58 +0200 Subject: [PATCH 26/63] black --- dds_web/database/models.py | 4 ++-- dds_web/web/user.py | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/dds_web/database/models.py b/dds_web/database/models.py index af1aa4ec3..d3e49dc23 100644 --- a/dds_web/database/models.py +++ b/dds_web/database/models.py @@ -366,9 +366,9 @@ class User(flask_login.UserMixin, db.Model): totp_enabled = db.Column(db.Boolean, unique=False, nullable=False, default=False) _totp_secret = db.Column(db.LargeBinary(64), unique=False, nullable=True) totp_last_verified = db.Column(db.DateTime, unique=False, nullable=True) - + active = db.Column(db.Boolean, nullable=False, default=True) - + kd_salt = db.Column(db.LargeBinary(32), default=None) nonce = db.Column(db.LargeBinary(12), default=None) public_key = db.Column(db.LargeBinary(300), default=None) diff --git a/dds_web/web/user.py b/dds_web/web/user.py index ff93873ba..f71cccb57 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -321,7 +321,11 @@ def confirm_2fa(): else: return flask.render_template( - "user/confirm2fa.html", form=form, cancel_form=cancel_form, next=next_target, using_totp=user.totp_enabled + "user/confirm2fa.html", + form=form, + cancel_form=cancel_form, + next=next_target, + using_totp=user.totp_enabled, ) From fc3aaba253a22156feb8d67225ff38b3b5a9163e Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Wed, 13 Apr 2022 14:12:27 +0200 Subject: [PATCH 27/63] Inas suggestion --- dds_web/api/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dds_web/api/__init__.py b/dds_web/api/__init__.py index adb7add81..c9e5e341b 100644 --- a/dds_web/api/__init__.py +++ b/dds_web/api/__init__.py @@ -69,7 +69,7 @@ def output_json(data, code, headers=None): api.add_resource(user.RemoveUserAssociation, "/user/access/revoke", endpoint="revoke_from_project") api.add_resource(user.UserActivation, "/user/activation", endpoint="user_activation") api.add_resource( - user.RequestTOTPActivation, "/user/request_activate_totp", endpoint="request_totp_activation" + user.RequestTOTPActivation, "/user/totp/activate", endpoint="request_totp_activation" ) api.add_resource(user.UnitUsers, "/unit/users", endpoint="unit_users") From fe16a6c0b505c5f4dd4c8e61092ff1959979428a Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Wed, 13 Apr 2022 16:45:37 +0200 Subject: [PATCH 28/63] Add migrations file --- migrations/versions/b01fc48f5939_add_totp.py | 32 ++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 migrations/versions/b01fc48f5939_add_totp.py diff --git a/migrations/versions/b01fc48f5939_add_totp.py b/migrations/versions/b01fc48f5939_add_totp.py new file mode 100644 index 000000000..48c347d05 --- /dev/null +++ b/migrations/versions/b01fc48f5939_add_totp.py @@ -0,0 +1,32 @@ +"""add_totp + +Revision ID: b01fc48f5939 +Revises: 1ab892d08e16 +Create Date: 2022-04-13 14:27:49.319000 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import mysql + +# revision identifiers, used by Alembic. +revision = "b01fc48f5939" +down_revision = "1ab892d08e16" +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column("users", sa.Column("totp_enabled", sa.Boolean(), nullable=False)) + op.add_column("users", sa.Column("_totp_secret", sa.LargeBinary(length=64), nullable=True)) + op.add_column("users", sa.Column("totp_last_verified", sa.DateTime(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column("users", "totp_last_verified") + op.drop_column("users", "_totp_secret") + op.drop_column("users", "totp_enabled") + # ### end Alembic commands ### From e164bd6e787d1ce3c045a88990667782346b4798 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Thu, 14 Apr 2022 00:49:40 +0200 Subject: [PATCH 29/63] Format templates, fix qr code background --- dds_web/forms.py | 4 +-- .../templates/mail/request_activate_totp.html | 16 +++++---- .../templates/mail/request_activate_totp.txt | 8 ++--- dds_web/templates/user/activate_totp.html | 35 ++++++++++--------- dds_web/web/user.py | 6 ++-- 5 files changed, 37 insertions(+), 32 deletions(-) diff --git a/dds_web/forms.py b/dds_web/forms.py index bf00c8727..91ed6f8a4 100644 --- a/dds_web/forms.py +++ b/dds_web/forms.py @@ -100,7 +100,7 @@ class Confirm2FACodeHOTPForm(flask_wtf.FlaskForm): class Confirm2FACodeTOTPForm(flask_wtf.FlaskForm): totp = wtforms.StringField( - "totp", + "Verification Code", validators=[wtforms.validators.InputRequired(), wtforms.validators.Length(min=6, max=6)], ) submit = wtforms.SubmitField("Authenticate") @@ -108,7 +108,7 @@ class Confirm2FACodeTOTPForm(flask_wtf.FlaskForm): class ActivateTOTPForm(flask_wtf.FlaskForm): totp = wtforms.StringField( - "totp", + "Verification Code", validators=[wtforms.validators.InputRequired(), wtforms.validators.Length(min=6, max=6)], ) submit = wtforms.SubmitField("Activate") diff --git a/dds_web/templates/mail/request_activate_totp.html b/dds_web/templates/mail/request_activate_totp.html index 7c9003d24..afd299f73 100644 --- a/dds_web/templates/mail/request_activate_totp.html +++ b/dds_web/templates/mail/request_activate_totp.html @@ -4,22 +4,24 @@ style="border-collapse: separate; mso-table-lspace: 0pt; mso-table-rspace: 0pt; width: 100%;"> +

Logo + style="border:0; outline:none; text-decoration:none; margin-bottom: 15px; max-width: 400px; max-height: 87px;"> +

- Activate TOTP for second factor authentication

+ Activate two factor authentication with authenticator app

- If you want to activate one-time password using TOTP instead of e-mail, visit the following link: + If you want to activate one-time password using an authenticator app instead of e-mail, visit the following link:

- + @@ -49,4 +51,4 @@ -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/dds_web/templates/mail/request_activate_totp.txt b/dds_web/templates/mail/request_activate_totp.txt index f60802742..9cf2374e1 100644 --- a/dds_web/templates/mail/request_activate_totp.txt +++ b/dds_web/templates/mail/request_activate_totp.txt @@ -1,5 +1,5 @@ -Activate TOTP for second factor authentication -If you want to activate one-time password using TOTP instead of e-mail, visit the following link: -{{link}} +Activate two factor authentication with authenticator app +If you want to activate one-time password using an authenticator app instead of e-mail, visit the following link: +{{link}} -If you did not make this request simply ignore this email and no changes will be made. \ No newline at end of file +If you did not make this request simply ignore this email and no changes will be made. diff --git a/dds_web/templates/user/activate_totp.html b/dds_web/templates/user/activate_totp.html index 494fa8c2b..3be77dce7 100644 --- a/dds_web/templates/user/activate_totp.html +++ b/dds_web/templates/user/activate_totp.html @@ -1,19 +1,23 @@ {% extends "base.html" %} {% block page_title -%} -Activate TOTP +Activate multi-factor authentication with authenticator app {% endblock %} {% block body %}
-

Activate one-time authentication using TOTP

+

Activate two factor authentication using authenticator app

Instructions:
    -
  • Download an authenticator on your phone, for example +
  • Download an authenticator app on your phone, for example Google Authenticator.
  • +
  • Scan the below QR code or enter the setup key to set up your account in the authenticator app. +
  • +
  • Once the account is set up, enter a verification code displayed in the app in the given field below and click {{ form.submit.label }} to enable two factor authentication using authenticator app. +
@@ -23,36 +27,35 @@

QR code

-

Secret

+

Setup Key

{{totp_secret}}

-

TOTP URI

+

Key URI

{{totp_uri}}

{{ form.csrf_token }} - +
- {{ form.totp.label }} - {{ form.totp }} -
    + {{ form.totp.label(class="col-md-auto col-form-label") }} +
    + {{ form.totp(class="form-control mb-2"+(" is-invalid" if form.totp.errors else "")) }} + {% if form.totp.errors %} {% for error in form.totp.errors %} -
  • - {{ error }} -
  • +
    {{ error }}
    {% endfor %} -
- + {% endif %} +
- +
diff --git a/dds_web/web/user.py b/dds_web/web/user.py index f71cccb57..19cc82c17 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -167,7 +167,7 @@ def activate_totp(token): dds_web.security.auth.verify_activate_totp_token(token, current_user=user) if user.totp_enabled: - flask.flash("Two-factor authentication via TOTP is already enabled.") + flask.flash("Two-factor authentication via authenticator app is already enabled.") return flask.redirect(flask.url_for("pages.home")) # Don't change secret on page reload @@ -177,7 +177,7 @@ def activate_totp(token): totp_secret, totp_uri = user.get_totp_secret() # QR code generation - image = qrcode.make(totp_uri, image_factory=qrcode.image.svg.SvgImage) + image = qrcode.make(totp_uri, image_factory=qrcode.image.svg.SvgFillImage) stream = io.BytesIO() image.save(stream) @@ -206,7 +206,7 @@ def activate_totp(token): user.activate_totp() - flask.flash("Two-factor authentication via TOTP has been enabled.") + flask.flash("Two-factor authentication via authenticator app has been enabled.") return flask.redirect(flask.url_for("pages.home")) return ( From 95fe581dd7be8bc978c38704f8df63202ed2d17b Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Thu, 14 Apr 2022 13:00:58 +0200 Subject: [PATCH 30/63] Remove totp from messages --- dds_web/api/user.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index c80b4a98a..354336d84 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -992,7 +992,9 @@ def post(self): user = auth.current_user() if user.totp_enabled: - return {"message": "Nothing to do, TOTP is already enabled for this user."} + return { + "message": "Nothing to do, two factor with app is already enabled for this user." + } # Not really necessary to encrypt this token = encrypted_jwt_token( @@ -1010,7 +1012,7 @@ def post(self): recipients = [user.primary_email] # Fill in email subject with sentence subject - subject = f"Request to activate TOTP for SciLifeLab Data Delivery System" + subject = f"Request to activate two factor with authenticator app for SciLifeLab Data Delivery System" msg = flask_mail.Message( subject, @@ -1040,7 +1042,9 @@ def post(self): ) AddUser.send_email_with_retry(msg) - return {"message": "Please check your email and follow the attached link to activate TOTP."} + return { + "message": "Please check your email and follow the attached link to activate two factor with authenticator app." + } class ShowUsage(flask_restful.Resource): From 568673e34b0846fc9b36c1fa869497a61fca0b54 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Tue, 19 Apr 2022 15:58:01 +0200 Subject: [PATCH 31/63] Remove TOTP in messages --- dds_web/api/schemas/token_schemas.py | 4 ++-- dds_web/database/models.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dds_web/api/schemas/token_schemas.py b/dds_web/api/schemas/token_schemas.py index 8f0afe7e7..f63833c8f 100644 --- a/dds_web/api/schemas/token_schemas.py +++ b/dds_web/api/schemas/token_schemas.py @@ -55,7 +55,7 @@ def validate_mfa(self, data, **kwargs): value = data.get("TOTP") if value is None: raise marshmallow.ValidationError( - "Your account is setup to use TOTP, but you entered a one-time authentication code from email." + "Your account is setup to use time-based one-time authentication codes, but you entered a one-time authentication code from email." ) # Raises authentication error if TOTP is incorrect user.verify_TOTP(value.encode()) @@ -63,7 +63,7 @@ def validate_mfa(self, data, **kwargs): value = data.get("HOTP") if value is None: raise marshmallow.ValidationError( - "Your account is setup to use one-time authentication code via email, you cannot authenticate with TOTP." + "Your account is setup to use one-time authentication code via email, you cannot authenticate with time-based one-time authentication codes." ) # Raises authenticationerror if invalid user.verify_HOTP(value.encode()) diff --git a/dds_web/database/models.py b/dds_web/database/models.py index d3e49dc23..a663157b7 100644 --- a/dds_web/database/models.py +++ b/dds_web/database/models.py @@ -554,7 +554,7 @@ def verify_TOTP(self, token): current_time - self.totp_last_verified < datetime.timedelta(seconds=90) ): raise AuthenticationError( - "Authentication with TOTP needs to be at least 90 seconds apart." + "Authentications with time-based token need to be at least 90 seconds apart." ) # construct object @@ -573,7 +573,7 @@ def verify_TOTP(self, token): pass if not verified: - raise AuthenticationError("Invalid TOTP token.") + raise AuthenticationError("Invalid time-based token.") # if the token is valid, save time of last successful verification self.totp_last_verified = current_time From 8fcfbd45a43077a4945fa56b345ffaca6611364e Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Tue, 19 Apr 2022 15:58:27 +0200 Subject: [PATCH 32/63] Add some tests for TOTP --- tests/test_basic_api.py | 113 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/tests/test_basic_api.py b/tests/test_basic_api.py index e6a526fa8..a42181cb4 100644 --- a/tests/test_basic_api.py +++ b/tests/test_basic_api.py @@ -264,6 +264,119 @@ def test_auth_second_factor_TOTP_incorrect_token(client, totp_for_user): assert "Invalid token" == response_json.get("message") +def test_auth_second_factor_TOTP_correct_token(client, totp_for_user): + """ + Test that the two_factor endpoint called with correct token returns 200/OK + """ + user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) + + totp_token = totp_for_user.generate(time.time()) + + response = client.get( + tests.DDSEndpoint.SECOND_FACTOR, + headers=user_auth.partial_token(client), + json={"TOTP": totp_token.decode()}, + ) + + assert response.status_code == http.HTTPStatus.OK + response_json = response.json + assert response_json.get("token") + claims = decrypt_and_verify_token_signature(response_json.get("token")) + print(claims) + assert claims["sub"] == "researchuser" + + +def test_auth_second_factor_TOTP_reused_token(client, totp_for_user): + """ + Test that the two_factor endpoint called with a reused token returns 401/UNAUTHORIZED + """ + user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) + totp_token = totp_for_user.generate(time.time()) + response = client.get( + tests.DDSEndpoint.SECOND_FACTOR, + headers=user_auth.partial_token(client), + json={"TOTP": totp_token.decode()}, + ) + + # Reuse the same totp token + response = client.get( + tests.DDSEndpoint.SECOND_FACTOR, + headers=user_auth.partial_token(client), + json={"TOTP": totp_token.decode()}, + ) + assert response.status_code == http.HTTPStatus.UNAUTHORIZED + response_json = response.json + assert response_json.get("message") + assert "Authentications with time-based token need to be at least 90 seconds apart." == response_json.get("message") + + +def test_auth_second_factor_TOTP_expired_token(client, totp_for_user): + """ + Test that the two_factor endpoint called with an expired token returns 401/UNAUTHORIZED + """ + user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) + # Token from one hour ago + totp_token = totp_for_user.generate(time.time() - 3600) + response = client.get( + tests.DDSEndpoint.SECOND_FACTOR, + headers=user_auth.partial_token(client), + json={"TOTP": totp_token.decode()}, + ) + assert response.status_code == http.HTTPStatus.UNAUTHORIZED + response_json = response.json + assert response_json.get("message") + assert "Invalid time-based token." == response_json.get("message") + + +def test_auth_second_factor_TOTP_incorrect_token(client, totp_for_user): + """ + Test that the two_factor endpoint called with a password_reset token returns 401/UNAUTHORIZED and + does not send a mail. + """ + user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) + + totp_token = totp_for_user.generate(time.time()) + + reset_token = encrypted_jwt_token( + username="researchuser", + sensitive_content=None, + expires_in=datetime.timedelta( + seconds=3600, + ), + additional_claims={"rst": "pwd"}, + ) + + response = client.get( + tests.DDSEndpoint.SECOND_FACTOR, + headers={"Authorization": f"Bearer {reset_token}"}, + json={"TOTP": totp_token.decode()}, + ) + + assert response.status_code == http.HTTPStatus.UNAUTHORIZED + response_json = response.json + assert response_json.get("message") + assert "Invalid token" == response_json.get("message") + + +def test_auth_second_factor_TOTP_use_invalid_HOTP_token(client, totp_for_user): + """ + Test that the two_factor endpoint for a TOTP activated user called with a HOTP token returns 400/BAD REQUEST + """ + user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) + hotp_token = user_auth.fetch_hotp() + response = client.get( + tests.DDSEndpoint.SECOND_FACTOR, + headers=user_auth.partial_token(client), + json={"HOTP": hotp_token.decode()}, + ) + + assert response.status_code == http.HTTPStatus.BAD_REQUEST + assert ( + "Your account is setup to use time-based one-time authentication codes, but you entered a one-time authentication code from email." + == response.json + ) + + # Token Authentication ##################################################### Token Authentication # From ee7a89ddbbd1c743d182e9eaab3b5939a39f9827 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Tue, 19 Apr 2022 16:01:28 +0200 Subject: [PATCH 33/63] black --- tests/test_basic_api.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_basic_api.py b/tests/test_basic_api.py index a42181cb4..6705a4e79 100644 --- a/tests/test_basic_api.py +++ b/tests/test_basic_api.py @@ -307,7 +307,10 @@ def test_auth_second_factor_TOTP_reused_token(client, totp_for_user): assert response.status_code == http.HTTPStatus.UNAUTHORIZED response_json = response.json assert response_json.get("message") - assert "Authentications with time-based token need to be at least 90 seconds apart." == response_json.get("message") + assert ( + "Authentications with time-based token need to be at least 90 seconds apart." + == response_json.get("message") + ) def test_auth_second_factor_TOTP_expired_token(client, totp_for_user): From eb63f82f203ef483cf7dff529932f298e145b2a9 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Tue, 19 Apr 2022 16:05:06 +0200 Subject: [PATCH 34/63] Include Inas suggestion --- dds_web/database/models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/dds_web/database/models.py b/dds_web/database/models.py index a663157b7..96c75a50f 100644 --- a/dds_web/database/models.py +++ b/dds_web/database/models.py @@ -520,6 +520,7 @@ def setup_totp_secret(self): self._totp_secret = os.urandom(20) db.session.commit() + @property def get_totp_secret(self): """Returns the users totp provisioning URI. Can only be sent before totp has been enabled.""" if self.totp_enabled: From 194eff5596eb335fe4d352755fb12217e13e20a8 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Wed, 20 Apr 2022 10:59:08 +0200 Subject: [PATCH 35/63] Fix tuple bug, support switch back to HOTP --- dds_web/api/user.py | 106 ++++++++++++++++++++----------------- dds_web/database/models.py | 7 ++- dds_web/web/user.py | 2 +- 3 files changed, 65 insertions(+), 50 deletions(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 354336d84..9af5e4547 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -988,63 +988,73 @@ class RequestTOTPActivation(flask_restful.Resource): """Request to switch from HOTP to TOTP for second factor authentication.""" @auth.login_required - def post(self): + def put(self): user = auth.current_user() - if user.totp_enabled: - return { - "message": "Nothing to do, two factor with app is already enabled for this user." - } + json_info = flask.request.json + activate_totp = json_info.get("activate_totp", False) + if activate_totp: + if user.totp_enabled: + return { + "message": "Nothing to do, two-factor authentication with app is already enabled for this user." + } - # Not really necessary to encrypt this - token = encrypted_jwt_token( - username=user.username, - sensitive_content=None, - expires_in=datetime.timedelta( - seconds=3600, - ), - additional_claims={"act": "totp"}, # Open for suggestions - ) + # Not really necessary to encrypt this + token = encrypted_jwt_token( + username=user.username, + sensitive_content=None, + expires_in=datetime.timedelta( + seconds=3600, + ), + additional_claims={"act": "totp"}, # Open for suggestions + ) - link = flask.url_for("auth_blueprint.activate_totp", token=token, _external=True) - # Send activation token to email to work as a validation step - # TODO: refactor this since the email sending code is replicated in many places - recipients = [user.primary_email] + link = flask.url_for("auth_blueprint.activate_totp", token=token, _external=True) + # Send activation token to email to work as a validation step + # TODO: refactor this since the email sending code is replicated in many places + recipients = [user.primary_email] - # Fill in email subject with sentence subject - subject = f"Request to activate two factor with authenticator app for SciLifeLab Data Delivery System" + # Fill in email subject with sentence subject + subject = f"Request to activate two-factor with authenticator app for SciLifeLab Data Delivery System" - msg = flask_mail.Message( - subject, - recipients=recipients, - ) + msg = flask_mail.Message( + subject, + recipients=recipients, + ) - # Need to attach the image to be able to use it - msg.attach( - "scilifelab_logo.png", - "image/png", - open( - os.path.join(flask.current_app.static_folder, "img/scilifelab_logo.png"), "rb" - ).read(), - "inline", - headers=[ - ["Content-ID", ""], - ], - ) + # Need to attach the image to be able to use it + msg.attach( + "scilifelab_logo.png", + "image/png", + open( + os.path.join(flask.current_app.static_folder, "img/scilifelab_logo.png"), "rb" + ).read(), + "inline", + headers=[ + ["Content-ID", ""], + ], + ) - msg.body = flask.render_template( - f"mail/request_activate_totp.txt", - link=link, - ) - msg.html = flask.render_template( - f"mail/request_activate_totp.html", - link=link, - ) + msg.body = flask.render_template( + f"mail/request_activate_totp.txt", + link=link, + ) + msg.html = flask.render_template( + f"mail/request_activate_totp.html", + link=link, + ) - AddUser.send_email_with_retry(msg) - return { - "message": "Please check your email and follow the attached link to activate two factor with authenticator app." - } + AddUser.send_email_with_retry(msg) + return { + "message": "Please check your email and follow the attached link to activate two-factor with authenticator app." + } + else: + if not user.totp_enabled: + return { + "message": "Nothing to do, two-factor authentication with email is already enabled for this user." + } + user.deactivate_totp() + return {"message": "Two-factor authentication via email has been enabled."} class ShowUsage(flask_restful.Resource): diff --git a/dds_web/database/models.py b/dds_web/database/models.py index 96c75a50f..caa8171f2 100644 --- a/dds_web/database/models.py +++ b/dds_web/database/models.py @@ -521,7 +521,7 @@ def setup_totp_secret(self): db.session.commit() @property - def get_totp_secret(self): + def totp_secret_and_uri(self): """Returns the users totp provisioning URI. Can only be sent before totp has been enabled.""" if self.totp_enabled: # Can not be fetched again after it has been enabled @@ -540,6 +540,11 @@ def activate_totp(self): self.totp_enabled = True db.session.commit() + def deactivate_totp(self): + """Fallback to HOTP as the preferred means of second factor authentication.""" + self.totp_enabled = False + db.session.commit() + def verify_TOTP(self, token): """Verify the totp token. Checks the previous, current and comming time frame to allow for some clock drift. diff --git a/dds_web/web/user.py b/dds_web/web/user.py index 19cc82c17..51270c744 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -174,7 +174,7 @@ def activate_totp(token): if not user.totp_initiated: user.setup_totp_secret() - totp_secret, totp_uri = user.get_totp_secret() + (totp_secret, totp_uri) = user.totp_secret_and_uri # QR code generation image = qrcode.make(totp_uri, image_factory=qrcode.image.svg.SvgFillImage) From 99b7884e68148030f204cb9a995009b98608b9f8 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Wed, 20 Apr 2022 17:22:30 +0200 Subject: [PATCH 36/63] Fix weird bug with label --- dds_web/templates/user/activate_totp.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dds_web/templates/user/activate_totp.html b/dds_web/templates/user/activate_totp.html index 3be77dce7..0521203fd 100644 --- a/dds_web/templates/user/activate_totp.html +++ b/dds_web/templates/user/activate_totp.html @@ -52,7 +52,7 @@

Key URI

From d33b168cf1af2cf4922795ae480534e768027b99 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Thu, 21 Apr 2022 11:15:27 +0200 Subject: [PATCH 37/63] Apply Inas suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ina Odén Österbo <35953392+inaod568@users.noreply.github.com> --- dds_web/api/schemas/token_schemas.py | 4 ++-- dds_web/database/models.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dds_web/api/schemas/token_schemas.py b/dds_web/api/schemas/token_schemas.py index f63833c8f..879084e1b 100644 --- a/dds_web/api/schemas/token_schemas.py +++ b/dds_web/api/schemas/token_schemas.py @@ -53,7 +53,7 @@ def validate_mfa(self, data, **kwargs): if user.totp_enabled: value = data.get("TOTP") - if value is None: + if not value: raise marshmallow.ValidationError( "Your account is setup to use time-based one-time authentication codes, but you entered a one-time authentication code from email." ) @@ -61,7 +61,7 @@ def validate_mfa(self, data, **kwargs): user.verify_TOTP(value.encode()) else: value = data.get("HOTP") - if value is None: + if not value: raise marshmallow.ValidationError( "Your account is setup to use one-time authentication code via email, you cannot authenticate with time-based one-time authentication codes." ) diff --git a/dds_web/database/models.py b/dds_web/database/models.py index caa8171f2..d4e8c12dd 100644 --- a/dds_web/database/models.py +++ b/dds_web/database/models.py @@ -556,7 +556,7 @@ def verify_TOTP(self, token): # Time frame chosen so that no one can use the same token more than once # No need to use epoch time here. current_time = dds_web.utils.current_time() - if self.totp_last_verified is not None and ( + if self.totp_last_verified and ( current_time - self.totp_last_verified < datetime.timedelta(seconds=90) ): raise AuthenticationError( From b74f36850b4189d2294751f73efcec926bcbb830 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Thu, 21 Apr 2022 11:25:10 +0200 Subject: [PATCH 38/63] Remove unneeded req --- requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 369870cb8..5c9be5424 100644 --- a/requirements.txt +++ b/requirements.txt @@ -45,7 +45,6 @@ pycparser==2.21 PyMySQL==1.0.2 PyNaCl==1.5.0 pyparsing==3.0.7 -PyQRCode==1.2.1 python-dateutil==2.8.2 pytz==2021.3 pytz-deprecation-shim==0.1.0.post0 From f6148605e29dc8c2c35d617338f3a56ec863d367 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Thu, 21 Apr 2022 11:33:05 +0200 Subject: [PATCH 39/63] Remove unneeded req: missed one --- dds_web/web/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/dds_web/web/__init__.py b/dds_web/web/__init__.py index 4f9112514..a9fa49700 100644 --- a/dds_web/web/__init__.py +++ b/dds_web/web/__init__.py @@ -9,7 +9,6 @@ # Installed import flask -import pyqrcode # Own modules From 56fc51cf717ce4d47bf19b5b75465e5bf9357118 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Thu, 5 May 2022 10:28:17 +0200 Subject: [PATCH 40/63] Inas suggestions --- dds_web/api/user.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 9af5e4547..43f32beb7 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -988,7 +988,8 @@ class RequestTOTPActivation(flask_restful.Resource): """Request to switch from HOTP to TOTP for second factor authentication.""" @auth.login_required - def put(self): + @json_required + def post(self): user = auth.current_user() json_info = flask.request.json @@ -1049,12 +1050,14 @@ def put(self): "message": "Please check your email and follow the attached link to activate two-factor with authenticator app." } else: - if not user.totp_enabled: - return { - "message": "Nothing to do, two-factor authentication with email is already enabled for this user." - } - user.deactivate_totp() - return {"message": "Two-factor authentication via email has been enabled."} + if user.totp_enabled: + user.deactivate_totp() + return {"message": "Two-factor authentication via email has been enabled."} + + return { + "message": "Nothing to do, two-factor authentication with email is already enabled for this user." + } + class ShowUsage(flask_restful.Resource): From 737ca1b3dddd2631d8d14d19cf325f598ee8f3a3 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Thu, 5 May 2022 11:18:22 +0200 Subject: [PATCH 41/63] black --- dds_web/api/user.py | 1 - 1 file changed, 1 deletion(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 43f32beb7..6995f5448 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -1059,7 +1059,6 @@ def post(self): } - class ShowUsage(flask_restful.Resource): """Calculate and display the amount of GB hours and the total cost.""" From 3ea267e79924cfeade27aa43495b6df301dad2f3 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Thu, 5 May 2022 11:54:58 +0200 Subject: [PATCH 42/63] Address Inas concerns --- dds_web/security/auth.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dds_web/security/auth.py b/dds_web/security/auth.py index 5b71dc25a..1e7edb50b 100644 --- a/dds_web/security/auth.py +++ b/dds_web/security/auth.py @@ -278,15 +278,15 @@ def __handle_multi_factor_authentication(user, mfa_auth_time_string): if mfa_auth_time >= dds_web.utils.current_time() - MFA_EXPIRES_IN: return user + hotp_message = "" if not user.totp_enabled: send_hotp_email(user) + hotp_message = "Please check your primary e-mail!" if flask.request.path.endswith("/user/second_factor"): return user - raise AuthenticationError( - message="Two-factor authentication is required! Please check your primary e-mail!" - ) + raise AuthenticationError(message=f"Two-factor authentication is required! {hotp_message}") def send_hotp_email(user): From 553263f3380b2c5f4c0917283e33831c488c6b4e Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 5 May 2022 13:53:51 +0200 Subject: [PATCH 43/63] Attempt to make an activate HOTP endpoint --- dds_web/api/user.py | 163 +++++++++++++++++++++++++++++--------------- 1 file changed, 108 insertions(+), 55 deletions(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 6995f5448..cd92e40f6 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -991,73 +991,126 @@ class RequestTOTPActivation(flask_restful.Resource): @json_required def post(self): - user = auth.current_user() - json_info = flask.request.json - activate_totp = json_info.get("activate_totp", False) - if activate_totp: - if user.totp_enabled: - return { - "message": "Nothing to do, two-factor authentication with app is already enabled for this user." - } + if user.totp_enabled: + return { + "message": "Nothing to do, two-factor authentication with app is already enabled for this user." + } - # Not really necessary to encrypt this - token = encrypted_jwt_token( - username=user.username, - sensitive_content=None, - expires_in=datetime.timedelta( - seconds=3600, - ), - additional_claims={"act": "totp"}, # Open for suggestions - ) + # Not really necessary to encrypt this + token = encrypted_jwt_token( + username=user.username, + sensitive_content=None, + expires_in=datetime.timedelta( + seconds=3600, + ), + additional_claims={"act": "totp"}, + ) - link = flask.url_for("auth_blueprint.activate_totp", token=token, _external=True) - # Send activation token to email to work as a validation step - # TODO: refactor this since the email sending code is replicated in many places - recipients = [user.primary_email] + link = flask.url_for("auth_blueprint.activate_totp", token=token, _external=True) + # Send activation token to email to work as a validation step + # TODO: refactor this since the email sending code is replicated in many places + recipients = [user.primary_email] - # Fill in email subject with sentence subject - subject = f"Request to activate two-factor with authenticator app for SciLifeLab Data Delivery System" + # Fill in email subject with sentence subject + subject = f"Request to activate two-factor with authenticator app for SciLifeLab Data Delivery System" - msg = flask_mail.Message( - subject, - recipients=recipients, - ) + msg = flask_mail.Message( + subject, + recipients=recipients, + ) - # Need to attach the image to be able to use it - msg.attach( - "scilifelab_logo.png", - "image/png", - open( - os.path.join(flask.current_app.static_folder, "img/scilifelab_logo.png"), "rb" - ).read(), - "inline", - headers=[ - ["Content-ID", ""], - ], - ) + # Need to attach the image to be able to use it + msg.attach( + "scilifelab_logo.png", + "image/png", + open( + os.path.join(flask.current_app.static_folder, "img/scilifelab_logo.png"), "rb" + ).read(), + "inline", + headers=[ + ["Content-ID", ""], + ], + ) - msg.body = flask.render_template( - f"mail/request_activate_totp.txt", - link=link, - ) - msg.html = flask.render_template( - f"mail/request_activate_totp.html", - link=link, - ) + msg.body = flask.render_template( + f"mail/request_activate_totp.txt", + link=link, + ) + msg.html = flask.render_template( + f"mail/request_activate_totp.html", + link=link, + ) - AddUser.send_email_with_retry(msg) - return { - "message": "Please check your email and follow the attached link to activate two-factor with authenticator app." - } - else: - if user.totp_enabled: - user.deactivate_totp() - return {"message": "Two-factor authentication via email has been enabled."} + AddUser.send_email_with_retry(msg) + return { + "message": "Please check your email and follow the attached link to activate two-factor with authenticator app." + } + +class RequestHOTPActivation(flask_restful.Resource): + """Request to switch from TOTP to HOTP for second factor authentication""" + # Using Basic auth since TOTP might have been lost, will still need access to email + @basic_auth.login_required + @json_required + def post(self): + + user = auth.current_user() + json_info = flask.request.json + + if not user.totp_enabled: return { "message": "Nothing to do, two-factor authentication with email is already enabled for this user." } + # Not really necessary to encrypt this + token = encrypted_jwt_token( + username=user.username, + sensitive_content=None, + expires_in=datetime.timedelta( + seconds=3600, + ), + additional_claims={"act": "hotp"}, + ) + + link = flask.url_for("auth_blueprint.activate_hotp", token=token, _external=True) + # Send activation token to email to work as a validation step + # TODO: refactor this since the email sending code is replicated in many places + recipients = [user.primary_email] + + # Fill in email subject with sentence subject + subject = f"Request to activate two-factor authentication with email for SciLifeLab Data Delivery System" + + msg = flask_mail.Message( + subject, + recipients=recipients, + ) + + # Need to attach the image to be able to use it + msg.attach( + "scilifelab_logo.png", + "image/png", + open( + os.path.join(flask.current_app.static_folder, "img/scilifelab_logo.png"), "rb" + ).read(), + "inline", + headers=[ + ["Content-ID", ""], + ], + ) + + msg.body = flask.render_template( + f"mail/request_activate_hotp.txt", + link=link, + ) + msg.html = flask.render_template( + f"mail/request_activate_hotp.html", + link=link, + ) + + AddUser.send_email_with_retry(msg) + return { + "message": "Please check your email and follow the attached link to activate two-factor with email." + } class ShowUsage(flask_restful.Resource): """Calculate and display the amount of GB hours and the total cost.""" From e187554cc3b6b26117c50cefa54f2652d9035c40 Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 5 May 2022 16:22:58 +0200 Subject: [PATCH 44/63] Method of disable totp --- dds_web/database/models.py | 1 + dds_web/web/user.py | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/dds_web/database/models.py b/dds_web/database/models.py index d4e8c12dd..9090d0864 100644 --- a/dds_web/database/models.py +++ b/dds_web/database/models.py @@ -543,6 +543,7 @@ def activate_totp(self): def deactivate_totp(self): """Fallback to HOTP as the preferred means of second factor authentication.""" self.totp_enabled = False + self._totp_secret = None db.session.commit() def verify_TOTP(self, token): diff --git a/dds_web/web/user.py b/dds_web/web/user.py index 51270c744..9e0fd8bc2 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -226,6 +226,26 @@ def activate_totp(token): }, ) +@auth_blueprint.route("/activate_hotp/", methods=["GET", "POST"]) +@limiter.limit( + dds_web.utils.rate_limit_from_config, + methods=["GET", "POST"], + error_message=ddserr.TooManyRequestsError.description, +) +def activate_hotp(token): + """Activates HOTP (default) as method of two-factor authentication. + We can't have authentication on this request as the user might have lost their TOTP secret.""" + user = dds_web.security.auth.verify_activate_hotp_token(token) + + if not user.totp_enabled: + flask.flash("Two-factor authentication via email is already enabled.") + return flask.redirect(flask.url_for("pages.home")) + + user.deactivate_totp() + + flask.flash("Two-factor authentication via authenticator app has been enabled.") + return flask.redirect(flask.url_for("pages.home")) + @auth_blueprint.route("/cancel_2fa", methods=["POST"]) @limiter.limit( From 829a7c6d208004c906f5714ec0c6f3b156524fc8 Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 5 May 2022 16:33:55 +0200 Subject: [PATCH 45/63] Mail for activating HOTP --- .../templates/mail/request_activate_hotp.html | 54 +++++++++++++++++++ .../templates/mail/request_activate_hotp.txt | 5 ++ 2 files changed, 59 insertions(+) create mode 100644 dds_web/templates/mail/request_activate_hotp.html create mode 100644 dds_web/templates/mail/request_activate_hotp.txt diff --git a/dds_web/templates/mail/request_activate_hotp.html b/dds_web/templates/mail/request_activate_hotp.html new file mode 100644 index 000000000..0dd1d7ef5 --- /dev/null +++ b/dds_web/templates/mail/request_activate_hotp.html @@ -0,0 +1,54 @@ +{% extends 'mail/mail_base.html' %} +{% block body %} + + + + + + + +
+

+ Logo +

+
+

+ Activate two factor authentication with email

+

+ If you want to activate one-time password using email instead of an authenticator app, visit the following link: +

+ + + + + + + +

+ Alternatively, copy and paste the link below into your browser:

+

+ {{link}}

+
+{% endblock %} diff --git a/dds_web/templates/mail/request_activate_hotp.txt b/dds_web/templates/mail/request_activate_hotp.txt new file mode 100644 index 000000000..1fd781dfc --- /dev/null +++ b/dds_web/templates/mail/request_activate_hotp.txt @@ -0,0 +1,5 @@ +Activate two factor authentication with email +If you want to activate one-time password using email instead of an authenticator app, visit the following link: +{{link}} + +If you do not wish to make any changes, please ignore this email. From b415be7f232a73d9fba16fdb948782b1b912e8a0 Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 5 May 2022 17:04:00 +0200 Subject: [PATCH 46/63] Json no longer readed for totp/hotp activation --- dds_web/api/user.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index cd92e40f6..134cf3a2b 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -988,7 +988,6 @@ class RequestTOTPActivation(flask_restful.Resource): """Request to switch from HOTP to TOTP for second factor authentication.""" @auth.login_required - @json_required def post(self): if user.totp_enabled: @@ -1051,7 +1050,6 @@ class RequestHOTPActivation(flask_restful.Resource): """Request to switch from TOTP to HOTP for second factor authentication""" # Using Basic auth since TOTP might have been lost, will still need access to email @basic_auth.login_required - @json_required def post(self): user = auth.current_user() From b1ef7f422ba645c4fdb10ae7d43ce3be132a5870 Mon Sep 17 00:00:00 2001 From: Johannes Alneberg Date: Thu, 5 May 2022 17:22:50 +0200 Subject: [PATCH 47/63] Bugfixes --- dds_web/api/__init__.py | 3 +++ dds_web/api/user.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/dds_web/api/__init__.py b/dds_web/api/__init__.py index 7f3c7c0f6..103315cae 100644 --- a/dds_web/api/__init__.py +++ b/dds_web/api/__init__.py @@ -68,6 +68,9 @@ def output_json(data, code, headers=None): api.add_resource(user.DeleteUserSelf, "/user/delete_self", endpoint="delete_user_self") api.add_resource(user.RemoveUserAssociation, "/user/access/revoke", endpoint="revoke_from_project") api.add_resource(user.UserActivation, "/user/activation", endpoint="user_activation") +api.add_resource( + user.RequestHOTPActivation, "/user/hotp/activate", endpoint="request_hotp_activation" +) api.add_resource( user.RequestTOTPActivation, "/user/totp/activate", endpoint="request_totp_activation" ) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 134cf3a2b..eaad240e0 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -989,7 +989,7 @@ class RequestTOTPActivation(flask_restful.Resource): @auth.login_required def post(self): - + user = auth.current_user() if user.totp_enabled: return { "message": "Nothing to do, two-factor authentication with app is already enabled for this user." From bde447e343870f714d84729a2b0cad28b2294a04 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Fri, 6 May 2022 13:21:16 +0200 Subject: [PATCH 48/63] Add missing function and black formatting --- dds_web/api/user.py | 4 +++- dds_web/security/auth.py | 18 +++++++++++++++--- dds_web/web/user.py | 5 +++-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index eaad240e0..2562bb894 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -1002,7 +1002,7 @@ def post(self): expires_in=datetime.timedelta( seconds=3600, ), - additional_claims={"act": "totp"}, + additional_claims={"act": "totp"}, ) link = flask.url_for("auth_blueprint.activate_totp", token=token, _external=True) @@ -1048,6 +1048,7 @@ def post(self): class RequestHOTPActivation(flask_restful.Resource): """Request to switch from TOTP to HOTP for second factor authentication""" + # Using Basic auth since TOTP might have been lost, will still need access to email @basic_auth.login_required def post(self): @@ -1110,6 +1111,7 @@ def post(self): "message": "Please check your email and follow the attached link to activate two-factor with email." } + class ShowUsage(flask_restful.Resource): """Calculate and display the amount of GB hours and the total cost.""" diff --git a/dds_web/security/auth.py b/dds_web/security/auth.py index 1e7edb50b..90d1e7328 100644 --- a/dds_web/security/auth.py +++ b/dds_web/security/auth.py @@ -130,6 +130,18 @@ def verify_activate_totp_token(token, current_user): raise AuthenticationError(message="Invalid token") +def verify_activate_hotp_token(token): + claims = __verify_general_token(token) + user = __user_from_subject(claims.get("sub")) + if user: + act = claims.get("act") + del claims + gc.collect() + if act and act == "hotp": + return user + raise AuthenticationError(message="Invalid token") + + def __base_verify_token_for_invite(token): """Verify token and return claims.""" claims = __verify_general_token(token=token) @@ -278,15 +290,15 @@ def __handle_multi_factor_authentication(user, mfa_auth_time_string): if mfa_auth_time >= dds_web.utils.current_time() - MFA_EXPIRES_IN: return user - hotp_message = "" + error_message = "" if not user.totp_enabled: send_hotp_email(user) - hotp_message = "Please check your primary e-mail!" + error_message = "Please check your primary e-mail!" if flask.request.path.endswith("/user/second_factor"): return user - raise AuthenticationError(message=f"Two-factor authentication is required! {hotp_message}") + raise AuthenticationError(message=f"Two-factor authentication is required! {error_message}") def send_hotp_email(user): diff --git a/dds_web/web/user.py b/dds_web/web/user.py index 9e0fd8bc2..18398a970 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -226,6 +226,7 @@ def activate_totp(token): }, ) + @auth_blueprint.route("/activate_hotp/", methods=["GET", "POST"]) @limiter.limit( dds_web.utils.rate_limit_from_config, @@ -233,7 +234,7 @@ def activate_totp(token): error_message=ddserr.TooManyRequestsError.description, ) def activate_hotp(token): - """Activates HOTP (default) as method of two-factor authentication. + """Activates HOTP (default) as method of two-factor authentication. We can't have authentication on this request as the user might have lost their TOTP secret.""" user = dds_web.security.auth.verify_activate_hotp_token(token) @@ -243,7 +244,7 @@ def activate_hotp(token): user.deactivate_totp() - flask.flash("Two-factor authentication via authenticator app has been enabled.") + flask.flash("Two-factor authentication via email has been enabled.") return flask.redirect(flask.url_for("pages.home")) From df8588e04a9628b12cf109d2e8b402b503ad39e6 Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Thu, 12 May 2022 14:03:42 +0200 Subject: [PATCH 49/63] Add some tests, update changelog --- CHANGELOG.md | 1 + tests/__init__.py | 4 ++++ tests/test_basic_api.py | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59b6b06a9..84936edb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,3 +94,4 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe - Bug: API returning float again and CLI `--size` flag works again ([#1162](https://github.com/ScilifelabDataCentre/dds_web/pull/1162)) - Bug: Check for timestamp `0000-00-00 00:00:00` added and invite deleted ([#1163](https://github.com/ScilifelabDataCentre/dds_web/pull/1163)) - Add documentation of status codes in `api/project.py` ([#1164](https://github.com/ScilifelabDataCentre/dds_web/pull/1164)) +- Add ability to switch to using TOTP and back to HOTP for MFA ([#936](https://github.com/scilifelabdatacentre/dds_web/issues/936)) diff --git a/tests/__init__.py b/tests/__init__.py index afc9db8e1..79d49b852 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -162,6 +162,10 @@ class DDSEndpoint: # User activation USER_ACTIVATION = BASE_ENDPOINT + "/user/activation" + # Switching authentication methods + TOTP_ACTIVATION = BASE_ENDPOINT + "/user/totp/activate" + HOTP_ACTIVATION = BASE_ENDPOINT + "/user/hotp/activate" + # S3Connector keys S3KEYS = BASE_ENDPOINT + "/s3/proj" diff --git a/tests/test_basic_api.py b/tests/test_basic_api.py index 6705a4e79..cd1a10764 100644 --- a/tests/test_basic_api.py +++ b/tests/test_basic_api.py @@ -235,6 +235,26 @@ def test_auth_second_factor_correctauth_reused_hotp_401_unauthorized(client): # TOTP ##################################################################################### TOTP # +def test_request_totp_activation(client): + """Request TOTP activation for a user""" + + user = dds_web.database.models.User.query.filter_by(username="researchuser").first() + assert not user.totp_enabled + + user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) + token = tests.UserAuth(tests.USER_CREDENTIALS["unituser"]).token(client) + response = client.post( + tests.DDSEndpoint.TOTP_ACTIVATION, + headers=token, + ) + assert response.status_code == http.HTTPStatus.OK + assert response.json.get("message") + assert ( + "Please check your email and follow the attached link to activate two-factor with authenticator app." + == response.json.get("message") + ) + + @pytest.fixture() def totp_for_user(client): """Create a user with TOTP enabled and return TOTP object""" @@ -380,6 +400,27 @@ def test_auth_second_factor_TOTP_use_invalid_HOTP_token(client, totp_for_user): ) +def test_hotp_activation(client, totp_for_user): + """Test hotp reactivation for a user using TOTP""" + + user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) + user = dds_web.database.models.User.query.filter_by(username="researchuser").first() + assert user.totp_enabled + + user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]).as_tuple() + response = client.post( + tests.DDSEndpoint.HOTP_ACTIVATION, + headers=None, + auth=user_auth, + ) + assert response.status_code == http.HTTPStatus.OK + assert response.json.get("message") + assert ( + "Please check your email and follow the attached link to activate two-factor with email." + == response.json.get("message") + ) + + # Token Authentication ##################################################### Token Authentication # From 5877974714b6913cf7452d39375a9f493b063f8f Mon Sep 17 00:00:00 2001 From: Anandashankar Anil Date: Thu, 12 May 2022 15:58:25 +0200 Subject: [PATCH 50/63] Add test for hotp and totpp activation --- tests/__init__.py | 2 ++ tests/test_basic_api.py | 69 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 79d49b852..4ff413661 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -140,6 +140,8 @@ class DDSEndpoint: REQUEST_RESET_PASSWORD = "/reset_password" RESET_PASSWORD = "/reset_password/" PASSWORD_RESET_COMPLETED = "/password_reset_completed" + ACTIVATE_HOTP_WEB = "/activate_hotp/" + ACTIVATE_TOTP_WEB = "/activate_totp/" # User INFO USER_INFO = BASE_ENDPOINT + "/user/info" diff --git a/tests/test_basic_api.py b/tests/test_basic_api.py index cd1a10764..11563bca9 100644 --- a/tests/test_basic_api.py +++ b/tests/test_basic_api.py @@ -17,6 +17,7 @@ from dds_web import db from dds_web.security.auth import decrypt_and_verify_token_signature from dds_web.security.tokens import encrypted_jwt_token +from tests.test_login_web import successful_web_login # TESTS #################################################################################### TESTS # @@ -236,10 +237,11 @@ def test_auth_second_factor_correctauth_reused_hotp_401_unauthorized(client): def test_request_totp_activation(client): - """Request TOTP activation for a user""" + """Request TOTP activation for a user and activate TOTP""" user = dds_web.database.models.User.query.filter_by(username="researchuser").first() assert not user.totp_enabled + assert not user.totp_initiated user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) token = tests.UserAuth(tests.USER_CREDENTIALS["unituser"]).token(client) @@ -254,6 +256,38 @@ def test_request_totp_activation(client): == response.json.get("message") ) + totp_token = encrypted_jwt_token( + username="researchuser", + sensitive_content=None, + expires_in=datetime.timedelta( + seconds=3600, + ), + additional_claims={"act": "totp"}, + ) + + form_token = successful_web_login(client, user_auth) + + response = client.get( + f"{tests.DDSEndpoint.ACTIVATE_TOTP_WEB}{totp_token}", + headers={"Content-Type": "application/x-www-form-urlencoded"}, + data={ + "csrf_token": form_token, + }, + follow_redirects=True, + ) + assert user.totp_initiated + assert not user.totp_enabled + response = client.post( + f"{tests.DDSEndpoint.ACTIVATE_TOTP_WEB}{totp_token}", + headers={"Content-Type": "application/x-www-form-urlencoded"}, + data={ + "csrf_token": form_token, + "totp": user.totp_object().generate(time.time()), + }, + follow_redirects=True, + ) + assert user.totp_enabled + @pytest.fixture() def totp_for_user(client): @@ -401,17 +435,17 @@ def test_auth_second_factor_TOTP_use_invalid_HOTP_token(client, totp_for_user): def test_hotp_activation(client, totp_for_user): - """Test hotp reactivation for a user using TOTP""" + """Test hotp reactivation for a user using TOTP and activate HOTP""" user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) user = dds_web.database.models.User.query.filter_by(username="researchuser").first() assert user.totp_enabled - user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]).as_tuple() + user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) response = client.post( tests.DDSEndpoint.HOTP_ACTIVATION, headers=None, - auth=user_auth, + auth=user_auth.as_tuple(), ) assert response.status_code == http.HTTPStatus.OK assert response.json.get("message") @@ -420,6 +454,33 @@ def test_hotp_activation(client, totp_for_user): == response.json.get("message") ) + # Activation on web + token = encrypted_jwt_token( + username="researchuser", + sensitive_content=None, + expires_in=datetime.timedelta( + seconds=3600, + ), + additional_claims={"act": "hotp"}, + ) + + response = client.post( + f"{tests.DDSEndpoint.ACTIVATE_HOTP_WEB}{token}", + headers={"Content-Type": "application/x-www-form-urlencoded"}, + follow_redirects=True, + ) + assert response.status_code == http.HTTPStatus.OK + assert not user.totp_enabled + + # Try logging in with hotp + hotp_token = user_auth.fetch_hotp() + response = client.get( + tests.DDSEndpoint.SECOND_FACTOR, + headers=user_auth.partial_token(client), + json={"HOTP": hotp_token.decode()}, + ) + assert response.status_code == http.HTTPStatus.OK + # Token Authentication ##################################################### Token Authentication # From 9ff2c38ee96b8a4aa76cb2e50c5c5ab7f0aa5dbe Mon Sep 17 00:00:00 2001 From: Ina Date: Fri, 13 May 2022 14:01:51 +0200 Subject: [PATCH 51/63] changed remove_bucket to remove_bucket_contents and added a 'delete_bucket' variable to cater to project deleting and archiving --- dds_web/api/api_s3_connector.py | 14 ++++++++------ dds_web/api/project.py | 8 ++++---- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/dds_web/api/api_s3_connector.py b/dds_web/api/api_s3_connector.py index 08400bcde..2fa0dee65 100644 --- a/dds_web/api/api_s3_connector.py +++ b/dds_web/api/api_s3_connector.py @@ -72,18 +72,20 @@ def get_s3_info(self): ) @bucket_must_exists - def remove_bucket(self, *args, **kwargs): - """Removes all contents from the project specific s3 bucket.""" + def remove_bucket_contents(self, delete_bucket=False, *_, **__): + """Removed all contents within a project specific s3 bucket.""" # Get bucket object bucket = self.resource.Bucket(self.project.bucket) # Delete objects first bucket.objects.all().delete() - - # Delete bucket - bucket.delete() + + # Delete bucket if chosen + if delete_bucket: + bucket.delete() + bucket = None - + @bucket_must_exists def remove_multiple(self, items, batch_size: int = 1000, *args, **kwargs): """Removes all with prefix.""" diff --git a/dds_web/api/project.py b/dds_web/api/project.py index 5a09dc5a1..12fff9af4 100644 --- a/dds_web/api/project.py +++ b/dds_web/api/project.py @@ -281,7 +281,7 @@ def delete_project(self, project: models.Project, current_time: datetime.datetim try: # Deletes files (also commits session in the function - possibly refactor later) - RemoveContents().delete_project_contents(project=project) + RemoveContents().delete_project_contents(project=project, delete_bucket=True) self.rm_project_user_keys(project=project) # Delete metadata from project row @@ -319,7 +319,7 @@ def archive_project( try: # Deletes files (also commits session in the function - possibly refactor later) - RemoveContents().delete_project_contents(project=project) + RemoveContents().delete_project_contents(project=project, delete_bucket=True) delete_message = f"\nAll files in {project.public_id} deleted" self.rm_project_user_keys(project=project) @@ -545,12 +545,12 @@ def delete(self): return {"removed": True} @staticmethod - def delete_project_contents(project): + def delete_project_contents(project, delete_bucket=False): """Remove project contents""" # Delete from cloud with ApiS3Connector(project=project) as s3conn: try: - s3conn.remove_bucket() + bucket = s3conn.remove_bucket_contents(delete_bucket=delete_bucket) except botocore.client.ClientError as err: raise DeletionError(message=str(err), project=project.public_id) from err From 9e751535803dcd185279a8831f937ca48e7b8c41 Mon Sep 17 00:00:00 2001 From: Ina Date: Fri, 13 May 2022 14:10:07 +0200 Subject: [PATCH 52/63] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84936edb9..4d8778ab5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,3 +95,4 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe - Bug: Check for timestamp `0000-00-00 00:00:00` added and invite deleted ([#1163](https://github.com/ScilifelabDataCentre/dds_web/pull/1163)) - Add documentation of status codes in `api/project.py` ([#1164](https://github.com/ScilifelabDataCentre/dds_web/pull/1164)) - Add ability to switch to using TOTP and back to HOTP for MFA ([#936](https://github.com/scilifelabdatacentre/dds_web/issues/936)) +- Bug: Do not remove the bucket when emptying the project ([#1172](https://github.com/ScilifelabDataCentre/dds_web/pull/1172)) From e0d6d7cff9379a539ddded13cdf2eb991f1706a9 Mon Sep 17 00:00:00 2001 From: Ina Date: Fri, 13 May 2022 14:10:21 +0200 Subject: [PATCH 53/63] black --- dds_web/api/api_s3_connector.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dds_web/api/api_s3_connector.py b/dds_web/api/api_s3_connector.py index 2fa0dee65..af7dbe99a 100644 --- a/dds_web/api/api_s3_connector.py +++ b/dds_web/api/api_s3_connector.py @@ -79,13 +79,13 @@ def remove_bucket_contents(self, delete_bucket=False, *_, **__): # Delete objects first bucket.objects.all().delete() - + # Delete bucket if chosen if delete_bucket: bucket.delete() - + bucket = None - + @bucket_must_exists def remove_multiple(self, items, batch_size: int = 1000, *args, **kwargs): """Removes all with prefix.""" From bfe067439991425f95d6b9f096c5f438f00d697e Mon Sep 17 00:00:00 2001 From: Ina Date: Fri, 13 May 2022 14:40:12 +0200 Subject: [PATCH 54/63] added test --- tests/test_files_new.py | 92 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/tests/test_files_new.py b/tests/test_files_new.py index c3003f8de..17a769a25 100644 --- a/tests/test_files_new.py +++ b/tests/test_files_new.py @@ -847,3 +847,95 @@ def test_new_file_wrong_status(client): assert response.status_code == http.HTTPStatus.BAD_REQUEST assert "Project not in right status to upload/modify files" in response.json.get("message") + +def test_delete_contents_and_upload_again(client, boto3_session): + """Upload and then delete all project contents""" + + project_1 = project_row(project_id="file_testing_project") + assert project_1 + assert project_1.current_status == "In Progress" + + a_completely_new_file = FIRST_NEW_FILE.copy() + a_completely_new_file["name"] = "a_completely_new_file" + a_completely_new_file["name_in_bucket"] = "a_completely_new_file" + + # Check that files have been added to db + file_in_db = db.session.query(models.File).filter(models.File.name == a_completely_new_file["name"]).first() + assert not file_in_db + + # Create new file in db + response = client.post( + tests.DDSEndpoint.FILE_NEW, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + query_string={"project": "file_testing_project"}, + json=a_completely_new_file, + ) + + # Check that files have been added to db + assert response.status_code == http.HTTPStatus.OK + file_in_db = db.session.query(models.File).filter(models.File.name == a_completely_new_file["name"]).first() + assert file_in_db + + # Try to remove all contents on empty project + response = client.delete( + tests.DDSEndpoint.REMOVE_PROJ_CONT, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + query_string={"project": "file_testing_project"}, + ) + + assert response.status_code == http.HTTPStatus.OK + file_in_db = db.session.query(models.File).filter(models.File.name == a_completely_new_file["name"]).first() + assert not file_in_db + + # Create new file in db + response = client.post( + tests.DDSEndpoint.FILE_NEW, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + query_string={"project": "file_testing_project"}, + json=a_completely_new_file, + ) + + # Check that files have been added to db + assert response.status_code == http.HTTPStatus.OK + file_in_db = db.session.query(models.File).filter(models.File.name == a_completely_new_file["name"]).first() + assert file_in_db + + # assert response.status_code == http.HTTPStatus.BAD_REQUEST + # assert "There are no project contents to delete." in response.json["message"] + + # response = client.post( + # tests.DDSEndpoint.FILE_NEW, + # headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + # query_string={"project": "file_testing_project"}, + # json=FIRST_NEW_FILE, + # ) + # assert response.status_code == http.HTTPStatus.OK + # assert file_in_db(test_dict=FIRST_NEW_FILE, project=project_1.id) + + # project_1 = project_row(project_id="file_testing_project") + # assert project_1 + # assert project_1.current_status == "In Progress" + + # file_1_in_folder = FIRST_NEW_FILE.copy() + # file_1_in_folder["name"] = "file_1_in_folder" + # file_1_in_folder["name_in_bucket"] = "bucketfile_1_in_folder" + + # response = client.post( + # tests.DDSEndpoint.FILE_NEW, + # headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + # query_string={"project": "file_testing_project"}, + # json=file_1_in_folder, + # ) + # assert response.status_code == http.HTTPStatus.OK + # assert file_in_db(test_dict=file_1_in_folder, project=project_1.id) + + # # Remove all contents + # response = client.delete( + # tests.DDSEndpoint.REMOVE_PROJ_CONT, + # headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + # query_string={"project": "file_testing_project"}, + # ) + # assert response.status_code == http.HTTPStatus.OK + # assert response.json["removed"] + # assert not file_in_db(test_dict=FIRST_NEW_FILE, project=project_1.id) + # assert not file_in_db(test_dict=file_1_in_folder, project=project_1.id) From 4491ca159b973955f0be4f7f753b01568c73bbe9 Mon Sep 17 00:00:00 2001 From: Ina Date: Fri, 13 May 2022 14:40:39 +0200 Subject: [PATCH 55/63] removed unused code --- tests/test_files_new.py | 40 ---------------------------------------- 1 file changed, 40 deletions(-) diff --git a/tests/test_files_new.py b/tests/test_files_new.py index 17a769a25..382e200c3 100644 --- a/tests/test_files_new.py +++ b/tests/test_files_new.py @@ -899,43 +899,3 @@ def test_delete_contents_and_upload_again(client, boto3_session): assert response.status_code == http.HTTPStatus.OK file_in_db = db.session.query(models.File).filter(models.File.name == a_completely_new_file["name"]).first() assert file_in_db - - # assert response.status_code == http.HTTPStatus.BAD_REQUEST - # assert "There are no project contents to delete." in response.json["message"] - - # response = client.post( - # tests.DDSEndpoint.FILE_NEW, - # headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), - # query_string={"project": "file_testing_project"}, - # json=FIRST_NEW_FILE, - # ) - # assert response.status_code == http.HTTPStatus.OK - # assert file_in_db(test_dict=FIRST_NEW_FILE, project=project_1.id) - - # project_1 = project_row(project_id="file_testing_project") - # assert project_1 - # assert project_1.current_status == "In Progress" - - # file_1_in_folder = FIRST_NEW_FILE.copy() - # file_1_in_folder["name"] = "file_1_in_folder" - # file_1_in_folder["name_in_bucket"] = "bucketfile_1_in_folder" - - # response = client.post( - # tests.DDSEndpoint.FILE_NEW, - # headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), - # query_string={"project": "file_testing_project"}, - # json=file_1_in_folder, - # ) - # assert response.status_code == http.HTTPStatus.OK - # assert file_in_db(test_dict=file_1_in_folder, project=project_1.id) - - # # Remove all contents - # response = client.delete( - # tests.DDSEndpoint.REMOVE_PROJ_CONT, - # headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), - # query_string={"project": "file_testing_project"}, - # ) - # assert response.status_code == http.HTTPStatus.OK - # assert response.json["removed"] - # assert not file_in_db(test_dict=FIRST_NEW_FILE, project=project_1.id) - # assert not file_in_db(test_dict=file_1_in_folder, project=project_1.id) From a31e5442755b4c1bfa40d58fbd5233d6827a6417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= <35953392+inaod568@users.noreply.github.com> Date: Fri, 13 May 2022 14:41:44 +0200 Subject: [PATCH 56/63] Update dds_web/api/project.py Co-authored-by: valyo <582646+valyo@users.noreply.github.com> --- dds_web/api/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dds_web/api/project.py b/dds_web/api/project.py index 12fff9af4..7a1aac301 100644 --- a/dds_web/api/project.py +++ b/dds_web/api/project.py @@ -550,7 +550,7 @@ def delete_project_contents(project, delete_bucket=False): # Delete from cloud with ApiS3Connector(project=project) as s3conn: try: - bucket = s3conn.remove_bucket_contents(delete_bucket=delete_bucket) + s3conn.remove_bucket_contents(delete_bucket=delete_bucket) except botocore.client.ClientError as err: raise DeletionError(message=str(err), project=project.public_id) from err From 6d7e402ff43a57a1365bba34aeb16b3eab87e987 Mon Sep 17 00:00:00 2001 From: Ina Date: Fri, 13 May 2022 14:43:10 +0200 Subject: [PATCH 57/63] black --- tests/test_files_new.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/tests/test_files_new.py b/tests/test_files_new.py index 382e200c3..806f6f67a 100644 --- a/tests/test_files_new.py +++ b/tests/test_files_new.py @@ -848,6 +848,7 @@ def test_new_file_wrong_status(client): assert response.status_code == http.HTTPStatus.BAD_REQUEST assert "Project not in right status to upload/modify files" in response.json.get("message") + def test_delete_contents_and_upload_again(client, boto3_session): """Upload and then delete all project contents""" @@ -860,7 +861,11 @@ def test_delete_contents_and_upload_again(client, boto3_session): a_completely_new_file["name_in_bucket"] = "a_completely_new_file" # Check that files have been added to db - file_in_db = db.session.query(models.File).filter(models.File.name == a_completely_new_file["name"]).first() + file_in_db = ( + db.session.query(models.File) + .filter(models.File.name == a_completely_new_file["name"]) + .first() + ) assert not file_in_db # Create new file in db @@ -873,7 +878,11 @@ def test_delete_contents_and_upload_again(client, boto3_session): # Check that files have been added to db assert response.status_code == http.HTTPStatus.OK - file_in_db = db.session.query(models.File).filter(models.File.name == a_completely_new_file["name"]).first() + file_in_db = ( + db.session.query(models.File) + .filter(models.File.name == a_completely_new_file["name"]) + .first() + ) assert file_in_db # Try to remove all contents on empty project @@ -884,7 +893,11 @@ def test_delete_contents_and_upload_again(client, boto3_session): ) assert response.status_code == http.HTTPStatus.OK - file_in_db = db.session.query(models.File).filter(models.File.name == a_completely_new_file["name"]).first() + file_in_db = ( + db.session.query(models.File) + .filter(models.File.name == a_completely_new_file["name"]) + .first() + ) assert not file_in_db # Create new file in db @@ -897,5 +910,9 @@ def test_delete_contents_and_upload_again(client, boto3_session): # Check that files have been added to db assert response.status_code == http.HTTPStatus.OK - file_in_db = db.session.query(models.File).filter(models.File.name == a_completely_new_file["name"]).first() + file_in_db = ( + db.session.query(models.File) + .filter(models.File.name == a_completely_new_file["name"]) + .first() + ) assert file_in_db From d171f9f6c2f3ac654261ac35dc60e996fbc38044 Mon Sep 17 00:00:00 2001 From: valyo <582646+valyo@users.noreply.github.com> Date: Fri, 13 May 2022 14:55:16 +0200 Subject: [PATCH 58/63] display the actual error message --- dds_web/web/user.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dds_web/web/user.py b/dds_web/web/user.py index 18398a970..340b3716e 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -320,8 +320,10 @@ def confirm_2fa(): # Raises authenticationerror if invalid try: twofactor_verify(twofactor_value.encode()) - except ddserr.AuthenticationError: - flask.flash("Invalid one-time code.", "warning") + except ddserr.AuthenticationError as err: + message = str(err) + message = message.removeprefix("401 Unauthorized: ") + flask.flash(message, "warning") return flask.redirect( flask.url_for( "auth_blueprint.confirm_2fa", From 7a4d3291363c999e0cdaf168b359d0d5ab898388 Mon Sep 17 00:00:00 2001 From: Ina Date: Fri, 13 May 2022 15:08:56 +0200 Subject: [PATCH 59/63] add argument option to lost-files flask command --- dds_web/__init__.py | 11 +++++++++-- dds_web/utils.py | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/dds_web/__init__.py b/dds_web/__init__.py index 5a19456f6..7e497095a 100644 --- a/dds_web/__init__.py +++ b/dds_web/__init__.py @@ -481,9 +481,8 @@ def update_uploaded_file_with_log(project, path_to_log_file): flask.current_app.logger.info(f"Files added: {files_added}") flask.current_app.logger.info(f"Errors while adding files: {errors}") - @click.command("lost-files") -@click.argument("action_type", type=click.Choice(["find", "list", "delete"])) +@click.argument("action_type", type=click.Choice(["find", "list", "delete", "add-missing-buckets"])) @flask.cli.with_appcontext def lost_files_s3_db(action_type: str): """ @@ -494,6 +493,7 @@ def lost_files_s3_db(action_type: str): """ from dds_web.database import models import boto3 + from dds_web.utils import bucket_is_valid for unit in models.Unit.query: session = boto3.session.Session() @@ -514,6 +514,13 @@ def lost_files_s3_db(action_type: str): ) except resource.meta.client.exceptions.NoSuchBucket: flask.current_app.logger.warning("Missing bucket %s", project.bucket) + if action_type == "add-missing-buckets": + valid, message = bucket_is_valid(bucket_name=project.bucket) + if not valid: + flask.current_app.logger.warning(f"Could not create bucket '{project.bucket}' for project '{project.public_id}': {message}") + else: + resource.create_bucket(Bucket=project.bucket) + flask.current_app.logger.info(f"Bucket '{project.bucket}' created.") continue try: diff --git a/dds_web/utils.py b/dds_web/utils.py index 73b247565..91c976552 100644 --- a/dds_web/utils.py +++ b/dds_web/utils.py @@ -389,3 +389,23 @@ def create_one_time_password_email(user, hotp_value): ) return msg + +def bucket_is_valid(bucket_name): + """Verify that the bucket name is valid.""" + valid = False + message = "" + if not (3 <= len(bucket_name) <= 63): + message = f"The bucket name has the incorrect length {len(bucket_name)}" + elif (re.findall(r"[^a-zA-Z0-9.-]", bucket_name)): + message = "The bucket name contains invalid characters." + elif bucket_name[0] in [".", "-"]: + message = "The bucket name must begin with a letter or number." + elif bucket_name.count(".") > 2: + message = "The bucket name cannot contain more than two dots." + elif bucket_name.startswith("xn--"): + message = "The bucket name cannot begin with the 'xn--' prefix." + elif bucket_name.endswith("-s3alias"): + message = "The bucket name cannot end with the '-s3alias' suffix." + else: + valid = True + return valid, message From afbe4057706e10539aeeb1fee8bd78c1fa98f871 Mon Sep 17 00:00:00 2001 From: Ina Date: Fri, 13 May 2022 15:12:45 +0200 Subject: [PATCH 60/63] black --- dds_web/__init__.py | 5 ++++- dds_web/utils.py | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/dds_web/__init__.py b/dds_web/__init__.py index 7e497095a..18e416362 100644 --- a/dds_web/__init__.py +++ b/dds_web/__init__.py @@ -481,6 +481,7 @@ def update_uploaded_file_with_log(project, path_to_log_file): flask.current_app.logger.info(f"Files added: {files_added}") flask.current_app.logger.info(f"Errors while adding files: {errors}") + @click.command("lost-files") @click.argument("action_type", type=click.Choice(["find", "list", "delete", "add-missing-buckets"])) @flask.cli.with_appcontext @@ -517,7 +518,9 @@ def lost_files_s3_db(action_type: str): if action_type == "add-missing-buckets": valid, message = bucket_is_valid(bucket_name=project.bucket) if not valid: - flask.current_app.logger.warning(f"Could not create bucket '{project.bucket}' for project '{project.public_id}': {message}") + flask.current_app.logger.warning( + f"Could not create bucket '{project.bucket}' for project '{project.public_id}': {message}" + ) else: resource.create_bucket(Bucket=project.bucket) flask.current_app.logger.info(f"Bucket '{project.bucket}' created.") diff --git a/dds_web/utils.py b/dds_web/utils.py index 91c976552..fff2ff110 100644 --- a/dds_web/utils.py +++ b/dds_web/utils.py @@ -390,13 +390,14 @@ def create_one_time_password_email(user, hotp_value): return msg + def bucket_is_valid(bucket_name): """Verify that the bucket name is valid.""" valid = False message = "" if not (3 <= len(bucket_name) <= 63): - message = f"The bucket name has the incorrect length {len(bucket_name)}" - elif (re.findall(r"[^a-zA-Z0-9.-]", bucket_name)): + message = f"The bucket name has the incorrect length {len(bucket_name)}" + elif re.findall(r"[^a-zA-Z0-9.-]", bucket_name): message = "The bucket name contains invalid characters." elif bucket_name[0] in [".", "-"]: message = "The bucket name must begin with a letter or number." From 261c1d682bf0702aeb28f361d076b6531a3b1ed6 Mon Sep 17 00:00:00 2001 From: Ina Date: Fri, 13 May 2022 15:14:26 +0200 Subject: [PATCH 61/63] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d8778ab5..e17f9a602 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,3 +96,4 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe - Add documentation of status codes in `api/project.py` ([#1164](https://github.com/ScilifelabDataCentre/dds_web/pull/1164)) - Add ability to switch to using TOTP and back to HOTP for MFA ([#936](https://github.com/scilifelabdatacentre/dds_web/issues/936)) - Bug: Do not remove the bucket when emptying the project ([#1172](https://github.com/ScilifelabDataCentre/dds_web/pull/1172)) +- New `add-missing-buckets` argument option to the `lost-files` flask command ([#1174](https://github.com/ScilifelabDataCentre/dds_web/pull/1174)) From 996c31369a48d44d458ec7c4e6a1f427a3634638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= <35953392+inaod568@users.noreply.github.com> Date: Fri, 13 May 2022 15:32:21 +0200 Subject: [PATCH 62/63] Update dds_web/utils.py Co-authored-by: valyo <582646+valyo@users.noreply.github.com> --- dds_web/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dds_web/utils.py b/dds_web/utils.py index fff2ff110..9e5d2a22a 100644 --- a/dds_web/utils.py +++ b/dds_web/utils.py @@ -399,7 +399,7 @@ def bucket_is_valid(bucket_name): message = f"The bucket name has the incorrect length {len(bucket_name)}" elif re.findall(r"[^a-zA-Z0-9.-]", bucket_name): message = "The bucket name contains invalid characters." - elif bucket_name[0] in [".", "-"]: + elif bucket_name[0].isalnum(): message = "The bucket name must begin with a letter or number." elif bucket_name.count(".") > 2: message = "The bucket name cannot contain more than two dots." From e06020913e16785bcff4931a3ffba5ea3ac8632e Mon Sep 17 00:00:00 2001 From: valyo <582646+valyo@users.noreply.github.com> Date: Fri, 13 May 2022 15:45:57 +0200 Subject: [PATCH 63/63] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84936edb9..79eb7ef39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,3 +95,4 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe - Bug: Check for timestamp `0000-00-00 00:00:00` added and invite deleted ([#1163](https://github.com/ScilifelabDataCentre/dds_web/pull/1163)) - Add documentation of status codes in `api/project.py` ([#1164](https://github.com/ScilifelabDataCentre/dds_web/pull/1164)) - Add ability to switch to using TOTP and back to HOTP for MFA ([#936](https://github.com/scilifelabdatacentre/dds_web/issues/936)) +- Patch: Fix the warning in web for too soon TOTP login (within 90 seconds) ([#1173](https://github.com/ScilifelabDataCentre/dds_web/pull/1173))