diff --git a/CHANGELOG.md b/CHANGELOG.md index 59b6b06a9..eadfc9bed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,3 +94,7 @@ 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)) +- Patch: Fix the warning in web for too soon TOTP login (within 90 seconds) ([#1173](https://github.com/ScilifelabDataCentre/dds_web/pull/1173)) +- 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)) diff --git a/dds_web/__init__.py b/dds_web/__init__.py index 5a19456f6..18e416362 100644 --- a/dds_web/__init__.py +++ b/dds_web/__init__.py @@ -483,7 +483,7 @@ def update_uploaded_file_with_log(project, path_to_log_file): @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 +494,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 +515,15 @@ 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/api/__init__.py b/dds_web/api/__init__.py index 251d5e03e..103315cae 100644 --- a/dds_web/api/__init__.py +++ b/dds_web/api/__init__.py @@ -68,6 +68,12 @@ 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" +) api.add_resource(user.UnitUsers, "/unit/users", endpoint="unit_users") # Super Admins ###################################################################### Super Admins # diff --git a/dds_web/api/api_s3_connector.py b/dds_web/api/api_s3_connector.py index 08400bcde..af7dbe99a 100644 --- a/dds_web/api/api_s3_connector.py +++ b/dds_web/api/api_s3_connector.py @@ -72,16 +72,18 @@ 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 diff --git a/dds_web/api/project.py b/dds_web/api/project.py index 5a09dc5a1..7a1aac301 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() + 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 diff --git a/dds_web/api/schemas/token_schemas.py b/dds_web/api/schemas/token_schemas.py index 68f6a2edc..879084e1b 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,24 @@ 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.exceptions.ValidationError("MFA method not supplied") user = auth.current_user() - if "HOTP" in data: + + if user.totp_enabled: + value = data.get("TOTP") + 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." + ) + # Raises authentication error if TOTP is incorrect + user.verify_TOTP(value.encode()) + else: value = data.get("HOTP") + 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." + ) # Raises authenticationerror if invalid user.verify_HOTP(value.encode()) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 1349f35a4..2562bb894 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -959,12 +959,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, } @@ -982,6 +984,134 @@ 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, 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"}, + ) + + 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" + + 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 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 + 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.""" diff --git a/dds_web/database/models.py b/dds_web/database/models.py index 9b7fcc996..9090d0864 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 @@ -16,6 +17,7 @@ import pathlib from cryptography.hazmat.primitives.twofactor import ( hotp as twofactor_hotp, + totp as twofactor_totp, InvalidToken as twofactor_InvalidToken, ) from cryptography.hazmat.primitives import hashes @@ -357,10 +359,16 @@ 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=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) @@ -491,6 +499,93 @@ def verify_HOTP(self, token): self.hotp_issue_time = None db.session.commit() + @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 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) + 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() + + @property + 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 + raise AuthenticationError("TOTP secret already enabled.") + totp = self.totp_object() + + 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. + Should be called after first totp token is verified + """ + 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 + self._totp_secret = None + 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 + # No need to use epoch time here. + current_time = dds_web.utils.current_time() + if self.totp_last_verified and ( + current_time - self.totp_last_verified < datetime.timedelta(seconds=90) + ): + raise AuthenticationError( + "Authentications with time-based token need to be at least 90 seconds apart." + ) + + # construct object + totp = self.totp_object() + + # 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 = time.time() + t_diff + try: + totp.verify(token, verification_time) + verified = True + break + except twofactor_InvalidToken: + pass + + if not verified: + raise AuthenticationError("Invalid time-based token.") + + # if the token is valid, save time of last successful verification + self.totp_last_verified = current_time + db.session.commit() + # Email related @property def primary_email(self): diff --git a/dds_web/forms.py b/dds_web/forms.py index be16de431..91ed6f8a4 100644 --- a/dds_web/forms.py +++ b/dds_web/forms.py @@ -90,7 +90,7 @@ class LogoutForm(flask_wtf.FlaskForm): logout = wtforms.SubmitField("Logout") -class Confirm2FACodeForm(flask_wtf.FlaskForm): +class Confirm2FACodeHOTPForm(flask_wtf.FlaskForm): hotp = wtforms.StringField( "Multi-factor authentication code", validators=[wtforms.validators.InputRequired(), wtforms.validators.Length(min=8, max=8)], @@ -98,6 +98,22 @@ class Confirm2FACodeForm(flask_wtf.FlaskForm): submit = wtforms.SubmitField("Authenticate") +class Confirm2FACodeTOTPForm(flask_wtf.FlaskForm): + totp = wtforms.StringField( + "Verification Code", + validators=[wtforms.validators.InputRequired(), wtforms.validators.Length(min=6, max=6)], + ) + submit = wtforms.SubmitField("Authenticate") + + +class ActivateTOTPForm(flask_wtf.FlaskForm): + totp = wtforms.StringField( + "Verification Code", + 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") diff --git a/dds_web/security/auth.py b/dds_web/security/auth.py index 1f70a7a92..8f5235d03 100644 --- a/dds_web/security/auth.py +++ b/dds_web/security/auth.py @@ -128,6 +128,30 @@ 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 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) @@ -276,14 +300,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 - send_hotp_email(user) + error_message = "" + if not user.totp_enabled: + send_hotp_email(user) + error_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! {error_message}") def send_hotp_email(user): @@ -362,5 +387,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 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. 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..afd299f73 --- /dev/null +++ b/dds_web/templates/mail/request_activate_totp.html @@ -0,0 +1,54 @@ +{% extends 'mail/mail_base.html' %} +{% block body %} + + + + + + + +
+

+ Logo +

+
+

+ 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: +

+ + + + + + + +

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

+

+ {{link}}

+
+{% endblock %} 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..9cf2374e1 --- /dev/null +++ b/dds_web/templates/mail/request_activate_totp.txt @@ -0,0 +1,5 @@ +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. diff --git a/dds_web/templates/user/activate_totp.html b/dds_web/templates/user/activate_totp.html new file mode 100644 index 000000000..0521203fd --- /dev/null +++ b/dds_web/templates/user/activate_totp.html @@ -0,0 +1,64 @@ +{% extends "base.html" %} +{% block page_title -%} +Activate multi-factor authentication with authenticator app +{% endblock %} +{% block body %} + +
+
+

Activate two factor authentication using authenticator app

+
Instructions:
+
    +
  • 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. +
    • +
+
+
+

QR code

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

Setup Key

+

{{totp_secret}}

+
+
+

Key URI

+

{{totp_uri}}

+
+
+
+ {{ form.csrf_token }} +
+ + {{ 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 }}
+ {% endfor %} + {% endif %} +
+ +
+ +
+
+
+
+ +
+ +{% endblock %} diff --git a/dds_web/templates/user/confirm2fa.html b/dds_web/templates/user/confirm2fa.html index adee4a4e0..467ceacf0 100644 --- a/dds_web/templates/user/confirm2fa.html +++ b/dds_web/templates/user/confirm2fa.html @@ -7,23 +7,39 @@ {% block body %}

Please enter your one-time authentication code.

-

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.

+{% 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 }}
- {{ form.hotp.label(class="col-md-auto col-form-label") }} -
- {{ form.hotp(class="form-control mb-2"+(" is-invalid" if form.hotp.errors else "")) }} - {% if form.hotp.errors %} - {% for error in form.hotp.errors %} -
{{ error }}
- {% endfor %} - {% endif %} -
+ {% if using_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 }}
+ {% endfor %} + {% endif %} +
+ {% else %} + {{ form.hotp.label(class="col-md-auto col-form-label") }} +
+ {{ form.hotp(class="form-control mb-2"+(" is-invalid" if form.hotp.errors else "")) }} + {% if form.hotp.errors %} + {% for error in form.hotp.errors %} +
{{ error }}
+ {% endfor %} + {% endif %} +
+ {% endif %}
@@ -42,4 +58,4 @@ {{ cancel_form.cancel(class="btn btn-link ps-0") }} -{% endblock %} +{% endblock %} \ No newline at end of file diff --git a/dds_web/utils.py b/dds_web/utils.py index 73b247565..9e5d2a22a 100644 --- a/dds_web/utils.py +++ b/dds_web/utils.py @@ -389,3 +389,24 @@ 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].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." + 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 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 diff --git a/dds_web/web/user.py b/dds_web/web/user.py index 76d19f3ad..340b3716e 100644 --- a/dds_web/web/user.py +++ b/dds_web/web/user.py @@ -3,7 +3,9 @@ #################################################################################################### # Standard Library +import base64 import datetime +import io import re # Installed @@ -11,6 +13,8 @@ import werkzeug import flask_login import itsdangerous +import qrcode +import qrcode.image.svg import sqlalchemy # Own Modules @@ -148,6 +152,102 @@ 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.auth.verify_activate_totp_token(token, current_user=user) + + if user.totp_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 + if not user.totp_initiated: + user.setup_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) + stream = io.BytesIO() + image.save(stream) + + # POST request + if form.validate_on_submit(): + 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, + ), + 200, + { + "Cache-Control": "no-cache, no-store, must-revalidate", + "Pragma": "no-cache", + "Expires": "0", + }, + ) + + user.activate_totp() + + flask.flash("Two-factor authentication via authenticator app has been enabled.") + return flask.redirect(flask.url_for("pages.home")) + + 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", + }, + ) + + +@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 email has been enabled.") + return flask.redirect(flask.url_for("pages.home")) + + @auth_blueprint.route("/cancel_2fa", methods=["POST"]) @limiter.limit( dds_web.utils.rate_limit_from_config, @@ -171,10 +271,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): @@ -198,7 +294,14 @@ def confirm_2fa(): ) return flask.redirect(flask.url_for("auth_blueprint.login", next=next_target)) - # Valid 2fa initiated token, but user does not exist (not never happen) or is inactive (could happen) + 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) or is inactive (could happen) # Currently same error for both, not vital, they get message to contact us if not user: flask.session.pop("2fa_initiated_token", None) @@ -207,23 +310,31 @@ 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.", "warning") + twofactor_verify(twofactor_value.encode()) + 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", form=form, cancel_form=cancel_form, next=next_target, + 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.", "success") # Remove token from session @@ -233,7 +344,11 @@ def confirm_2fa(): else: return flask.render_template( - "user/confirm2fa.html", form=form, cancel_form=cancel_form, next=next_target + "user/confirm2fa.html", + form=form, + cancel_form=cancel_form, + next=next_target, + using_totp=user.totp_enabled, ) @@ -273,10 +388,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( @@ -442,10 +557,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 = {} 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 ### diff --git a/requirements.txt b/requirements.txt index 18534fd67..5c9be5424 100644 --- a/requirements.txt +++ b/requirements.txt @@ -40,14 +40,15 @@ 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 pyparsing==3.0.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 diff --git a/tests/__init__.py b/tests/__init__.py index afc9db8e1..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" @@ -162,6 +164,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 4dbb42171..11563bca9 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 @@ -15,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 # @@ -83,7 +86,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 +102,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 +233,256 @@ 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 # + + +def test_request_totp_activation(client): + """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) + 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") + ) + + 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): + """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") + + +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 + ) + + +def test_hotp_activation(client, totp_for_user): + """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"]) + response = client.post( + tests.DDSEndpoint.HOTP_ACTIVATION, + headers=None, + auth=user_auth.as_tuple(), + ) + 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") + ) + + # 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 # def test_auth_incorrect_token_without_periods(client): diff --git a/tests/test_files_new.py b/tests/test_files_new.py index c3003f8de..806f6f67a 100644 --- a/tests/test_files_new.py +++ b/tests/test_files_new.py @@ -847,3 +847,72 @@ 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