diff --git a/securedrop/journalist_app/admin.py b/securedrop/journalist_app/admin.py index c8feae9700..e23f98b310 100644 --- a/securedrop/journalist_app/admin.py +++ b/securedrop/journalist_app/admin.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import os +import binascii from typing import Optional from typing import Union @@ -132,6 +133,17 @@ def add_user() -> Union[str, werkzeug.Response]: 'There was an error with the autogenerated password. ' 'User not created. Please try again.'), 'error') form_valid = False + except (binascii.Error, TypeError) as e: + if "Non-hexadecimal digit found" in str(e): + flash(gettext( + "Invalid HOTP secret format: " + "please only submit letters A-F and numbers 0-9."), + "error") + else: + flash(gettext( + "An unexpected error occurred! " + "Please inform your admin."), "error") + form_valid = False except InvalidUsernameException as e: form_valid = False # Translators: Here, "{message}" explains the problem with the username. diff --git a/securedrop/journalist_app/api.py b/securedrop/journalist_app/api.py index f4ffd8655e..1e589eddbf 100644 --- a/securedrop/journalist_app/api.py +++ b/securedrop/journalist_app/api.py @@ -19,7 +19,8 @@ from journalist_app import utils from models import (Journalist, Reply, SeenReply, Source, Submission, LoginThrottledException, InvalidUsernameException, - BadTokenException, WrongPasswordException) + BadTokenException, InvalidOTPSecretException, + WrongPasswordException) from sdconfig import SDConfig from store import NotEncrypted @@ -133,7 +134,7 @@ def get_token() -> Tuple[flask.Response, int]: return response, 200 except (LoginThrottledException, InvalidUsernameException, - BadTokenException, WrongPasswordException): + BadTokenException, InvalidOTPSecretException, WrongPasswordException): return abort(403, 'Token authentication failed.') @api.route('/sources', methods=['GET']) diff --git a/securedrop/journalist_app/forms.py b/securedrop/journalist_app/forms.py index ad4104b4c0..a66547b679 100644 --- a/securedrop/journalist_app/forms.py +++ b/securedrop/journalist_app/forms.py @@ -6,15 +6,41 @@ from wtforms import Field from wtforms import (TextAreaField, StringField, BooleanField, HiddenField, ValidationError) -from wtforms.validators import InputRequired, Optional +from wtforms.validators import InputRequired, Optional, DataRequired, StopValidation -from models import Journalist, InstanceConfig +from models import Journalist, InstanceConfig, HOTP_SECRET_LENGTH + +from typing import Any + + +class RequiredIf(DataRequired): + + def __init__(self, other_field_name: str, *args: Any, **kwargs: Any) -> None: + self.other_field_name = other_field_name + + def __call__(self, form: FlaskForm, field: Field) -> None: + if self.other_field_name in form: + other_field = form[self.other_field_name] + if bool(other_field.data): + self.message = gettext( + 'The "{name}" field is required when "{other_name}" is set.' + .format(other_name=self.other_field_name, name=field.name)) + super(RequiredIf, self).__call__(form, field) + else: + field.errors[:] = [] + raise StopValidation() + else: + raise ValidationError( + gettext( + 'The "{other_name}" field was not found - it is required by "{name}".' + .format(other_name=self.other_field_name, name=field.name)) + ) def otp_secret_validation(form: FlaskForm, field: Field) -> None: strip_whitespace = field.data.replace(' ', '') input_length = len(strip_whitespace) - if input_length != 40: + if input_length != HOTP_SECRET_LENGTH: raise ValidationError( ngettext( 'HOTP secrets are 40 characters long - you have entered {num}.', @@ -74,8 +100,8 @@ class NewUserForm(FlaskForm): is_admin = BooleanField('is_admin') is_hotp = BooleanField('is_hotp') otp_secret = StringField('otp_secret', validators=[ - otp_secret_validation, - Optional() + RequiredIf("is_hotp"), + otp_secret_validation ]) diff --git a/securedrop/journalist_app/utils.py b/securedrop/journalist_app/utils.py index 172bbd439b..f130e67172 100644 --- a/securedrop/journalist_app/utils.py +++ b/securedrop/journalist_app/utils.py @@ -17,6 +17,7 @@ BadTokenException, FirstOrLastNameError, InvalidPasswordLength, + InvalidOTPSecretException, InvalidUsernameException, Journalist, LoginThrottledException, @@ -31,6 +32,7 @@ Submission, WrongPasswordException, get_one_or_else, + HOTP_SECRET_LENGTH, ) from store import add_checksum_for_file @@ -94,6 +96,7 @@ def validate_user( try: return Journalist.login(username, password, token) except (InvalidUsernameException, + InvalidOTPSecretException, BadTokenException, WrongPasswordException, LoginThrottledException, @@ -112,6 +115,11 @@ def validate_user( "Please wait at least {num} seconds before logging in again.", period ).format(num=period) + elif isinstance(e, InvalidOTPSecretException): + login_flashed_msg += ' ' + login_flashed_msg += gettext( + "Your 2FA details are invalid" + " - please contact an administrator to reset them.") else: try: user = Journalist.query.filter_by( @@ -135,21 +143,26 @@ def validate_hotp_secret(user: Journalist, otp_secret: str) -> bool: :param otp_secret: the new HOTP secret :return: True if it validates, False if it does not """ + strip_whitespace = otp_secret.replace(' ', '') + secret_length = len(strip_whitespace) + + if secret_length != HOTP_SECRET_LENGTH: + flash(ngettext( + 'HOTP secrets are 40 characters long - you have entered {num}.', + 'HOTP secrets are 40 characters long - you have entered {num}.', + secret_length + ).format(num=secret_length), "error") + return False + try: user.set_hotp_secret(otp_secret) except (binascii.Error, TypeError) as e: if "Non-hexadecimal digit found" in str(e): flash(gettext( - "Invalid secret format: " + "Invalid HOTP secret format: " "please only submit letters A-F and numbers 0-9."), "error") return False - elif "Odd-length string" in str(e): - flash(gettext( - "Invalid secret format: " - "odd-length secret. Did you mistype the secret?"), - "error") - return False else: flash(gettext( "An unexpected error occurred! " diff --git a/securedrop/journalist_templates/admin_edit_hotp_secret.html b/securedrop/journalist_templates/admin_edit_hotp_secret.html index 397df7c7d9..9774f6d000 100644 --- a/securedrop/journalist_templates/admin_edit_hotp_secret.html +++ b/securedrop/journalist_templates/admin_edit_hotp_secret.html @@ -1,10 +1,11 @@ {% extends "base.html" %} {% block body %} +
{{ gettext("Enter a new HOTP secret formatted as a 40-digit hexadecimal string. Spaces will be ignored:") }}