diff --git a/CHANGELOG.md b/CHANGELOG.md index 04c761222..f47f66303 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -224,3 +224,4 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe ## Sprint (2023-03-03 - 2023-03-17) - PR template restructured ([#1403](https://github.com/ScilifelabDataCentre/dds_web/pull/1403)) +- Only allow latin1-encodable usernames and passwords ([#1402](https://github.com/ScilifelabDataCentre/dds_web/pull/1402)) diff --git a/dds_web/utils.py b/dds_web/utils.py index a2895a32e..8d19902bf 100644 --- a/dds_web/utils.py +++ b/dds_web/utils.py @@ -121,6 +121,17 @@ def contains_digit_or_specialchar(indata): ) +def contains_only_latin1(indata): + """Verify that the password contains characters that can be encoded to latin-1. + + Non latin-1 chars cannot be passed to requests. + """ + try: + indata.encode("latin1") + except UnicodeEncodeError as err: + raise marshmallow.ValidationError("Contains invalid characters.") + + def contains_disallowed_characters(indata): """Indatas like <9f><98><80> cause issues in Project names etc.""" disallowed = re.findall(r"[^(\w\s)]+", indata) @@ -220,6 +231,7 @@ def _password_contains_valid_characters(form, field): contains_uppercase, contains_lowercase, contains_digit_or_specialchar, + contains_only_latin1, ] for val in validators: try: diff --git a/tests/test_flow_invite_to_access.py b/tests/test_flow_invite_to_access.py index 3f6ddda69..6530619d5 100644 --- a/tests/test_flow_invite_to_access.py +++ b/tests/test_flow_invite_to_access.py @@ -1,7 +1,7 @@ import unittest import flask - +import http import tests from dds_web.database import models import dds_web.api.user @@ -54,7 +54,7 @@ def perform_invite(client, inviting_user, email, role=None, project=None): call_args = mock_token_method.call_args invite_token = encrypted_jwt_token(*call_args.args, **call_args.kwargs) - if response.status != "200 OK": + if response.status_code != http.HTTPStatus.OK: if DEBUG: print(response.status_code) raise ValueError(f"Invitation failed: {response.data}") @@ -71,7 +71,7 @@ def get_private(client, project, auth_token): headers=auth_token, ) assert ( - response.status == "200 OK" + response.status_code == http.HTTPStatus.OK ), f"Unable to fetch project private key for project: {project}, response: {response.data}" @@ -106,7 +106,7 @@ def invite_confirm_register_and_get_private( content_type="application/json", headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK # Complete registration form_token = flask.g.csrf_token @@ -124,7 +124,7 @@ def invite_confirm_register_and_get_private( headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK user = models.User.query.filter_by(username=form_data["username"]).one_or_none() @@ -137,7 +137,7 @@ def invite_confirm_register_and_get_private( follow_redirects=True, headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK user = models.User.query.filter_by(username=form_data["username"]).one_or_none() diff --git a/tests/test_user_change_password.py b/tests/test_user_change_password.py index 8595eb71e..591f34e5b 100644 --- a/tests/test_user_change_password.py +++ b/tests/test_user_change_password.py @@ -20,6 +20,68 @@ def test_get_user_change_password_without_login(client): assert flask.request.path == tests.DDSEndpoint.LOGIN +def test_unsuccessful_user_change_password_with_login_nonlatin1(client): + """Non latin 1 chars should not be accepted.""" + # Get user + user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) + + # Verify current password + user = models.User.query.get(user_auth.username) + assert user.verify_password("password") + + # Get and verify variables + public_key_initial = user.public_key + nonce_initial = user.nonce + private_key_initial = user.private_key + kd_salt_initial = user.kd_salt + + assert public_key_initial + assert nonce_initial + assert private_key_initial + assert kd_salt_initial + + # Login + form_token = successful_web_login(client, user_auth) + + # Define change password data + form_data = { + "csrf_token": form_token, + "current_password": "password", + "new_password": "123$%^qweRTY€", + "confirm_new_password": "123$%^qweRTY€", + "submit": "Change Password", + } + + # Attempt to change password -- should fail + response = client.post( + tests.DDSEndpoint.CHANGE_PASSWORD, + json=form_data, + follow_redirects=True, + headers=tests.DEFAULT_HEADER, + ) + assert response.status_code == http.HTTPStatus.OK + assert flask.request.path == tests.DDSEndpoint.CHANGE_PASSWORD + + # Password should not have changed, neither should other info + assert user.verify_password("password") + assert not user.verify_password("123$%^qweRTY€") + + public_key_after_password_change = user.public_key + nonce_after_password_change = user.nonce + private_key_after_password_change = user.private_key + kd_salt_after_password_change = user.kd_salt + + assert public_key_after_password_change + assert nonce_after_password_change + assert private_key_after_password_change + assert kd_salt_after_password_change + + assert public_key_after_password_change == public_key_initial + assert nonce_after_password_change == nonce_initial + assert private_key_after_password_change == private_key_initial + assert kd_salt_after_password_change == kd_salt_initial + + def test_successful_user_change_password_with_login(client): user_auth = tests.UserAuth(tests.USER_CREDENTIALS["researcher"]) diff --git a/tests/test_user_confirm_invites_and_register.py b/tests/test_user_confirm_invites_and_register.py index 183de440c..8f9868c39 100644 --- a/tests/test_user_confirm_invites_and_register.py +++ b/tests/test_user_confirm_invites_and_register.py @@ -1,4 +1,5 @@ import tests +import http import flask import pytest from cryptography.hazmat.primitives.ciphers.aead import AESGCM @@ -24,7 +25,7 @@ def test_confirm_invite_no_token(client): content_type="application/json", headers=tests.DEFAULT_HEADER, ) - assert response.status == "404 NOT FOUND" + assert response.status_code == http.HTTPStatus.NOT_FOUND def test_confirm_invite_invalid_token(client): @@ -35,7 +36,7 @@ def test_confirm_invite_invalid_token(client): headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK # index redirects to login assert flask.request.path == flask.url_for("pages.home") @@ -56,7 +57,7 @@ def test_confirm_invite_expired_token(client): follow_redirects=True, headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK # index redirects to login assert flask.request.path == flask.url_for("pages.home") @@ -75,7 +76,7 @@ def test_confirm_invite_valid_token(client): content_type="application/json", headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK assert b"Create account" in response.data @@ -86,7 +87,7 @@ def test_register_no_form(client): content_type="application/json", headers=tests.DEFAULT_HEADER, ) - assert response.status == "400 BAD REQUEST" + assert response.status_code == http.HTTPStatus.BAD_REQUEST @pytest.fixture() @@ -106,7 +107,7 @@ def registry_form_data(client): content_type="application/json", headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK assert b"Create account" in response.data form_token = flask.g.csrf_token @@ -133,7 +134,7 @@ def test_register_no_token_in_session(registry_form_data, client): follow_redirects=True, headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK assert flask.request.path == tests.DDSEndpoint.INDEX # Invite should be kept and user should not be created @@ -158,7 +159,7 @@ def test_register_weak_password(registry_form_data, client): follow_redirects=True, headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK assert flask.request.path == tests.DDSEndpoint.USER_NEW # Invite should be kept and user should not be created @@ -172,6 +173,66 @@ def test_register_weak_password(registry_form_data, client): assert user is None +def test_register_nonlatin1_username(registry_form_data, client): + """Username can only contain latin 1 encodable characters. + + Requests package does not allow non-latin1 encodable characters. + """ + form_data = registry_form_data + + form_data["username"] = "user_€" # € is invalid + + # Request should work + response = client.post( + tests.DDSEndpoint.USER_NEW, + json=form_data, + follow_redirects=True, + headers=tests.DEFAULT_HEADER, + ) + assert response.status_code == http.HTTPStatus.OK + assert flask.request.path == tests.DDSEndpoint.USER_NEW + + # Invite should be kept and user should not be created + invite = models.Invite.query.filter_by( + email="existing_invite_email@mailtrap.io", role="Researcher" + ).one_or_none() + assert invite is not None + + # No user should be created + user = models.User.query.filter_by(username=form_data["username"]).one_or_none() + assert user is None + + +def test_register_nonlatin1_password(registry_form_data, client): + """Password can only contain latin 1 encodable characters. + + Requests package does not allow non-latin1 encodable characters. + """ + form_data = registry_form_data + form_data["password"] = "Password123€" # € is invalid + form_data["confirm"] = "Password123€" + + # Request should work + response = client.post( + tests.DDSEndpoint.USER_NEW, + json=form_data, + follow_redirects=True, + headers=tests.DEFAULT_HEADER, + ) + assert response.status_code == http.HTTPStatus.OK + assert flask.request.path == tests.DDSEndpoint.USER_NEW + + # Invite should be kept and user should not be created + invite = models.Invite.query.filter_by( + email="existing_invite_email@mailtrap.io", role="Researcher" + ).one_or_none() + assert invite is not None + + # No user should be created + user = models.User.query.filter_by(username=form_data["username"]).one_or_none() + assert user is None + + def test_successful_registration(registry_form_data, client): response = client.post( tests.DDSEndpoint.USER_NEW, @@ -179,7 +240,7 @@ def test_successful_registration(registry_form_data, client): follow_redirects=True, headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK invite = models.Invite.query.filter_by( email="existing_invite_email@mailtrap.io", role="Researcher" @@ -211,7 +272,7 @@ def test_successful_registration_should_transfer_keys(registry_form_data, client follow_redirects=True, headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK invite = models.Invite.query.filter_by( email="existing_invite_email@mailtrap.io", role="Researcher" @@ -245,7 +306,7 @@ def test_invite_key_verification_fails_with_no_setup(client): content_type="application/json", headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK assert b"Create account" in response.data form_token = flask.g.csrf_token @@ -266,7 +327,7 @@ def test_invite_key_verification_fails_with_no_setup(client): follow_redirects=True, headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK invite = models.Invite.query.filter_by( email="existing_invite_email@mailtrap.io", role="Researcher" @@ -302,7 +363,7 @@ def test_invite_key_verification_fails_with_wrong_valid_key(client): content_type="application/json", headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK assert b"Create account" in response.data form_token = flask.g.csrf_token @@ -323,7 +384,7 @@ def test_invite_key_verification_fails_with_wrong_valid_key(client): follow_redirects=True, headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK invite = models.Invite.query.filter_by( email="existing_invite_email@mailtrap.io", role="Researcher" @@ -359,7 +420,7 @@ def test_invite_key_verification_fails_with_wrong_invalid_key(client): content_type="application/json", headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK assert b"Create account" in response.data form_token = flask.g.csrf_token @@ -380,7 +441,7 @@ def test_invite_key_verification_fails_with_wrong_invalid_key(client): follow_redirects=True, headers=tests.DEFAULT_HEADER, ) - assert response.status == "200 OK" + assert response.status_code == http.HTTPStatus.OK invite = models.Invite.query.filter_by( email="existing_invite_email@mailtrap.io", role="Researcher" diff --git a/tests/test_user_reset_password.py b/tests/test_user_reset_password.py index ea98bd7c2..748f98f12 100644 --- a/tests/test_user_reset_password.py +++ b/tests/test_user_reset_password.py @@ -595,3 +595,73 @@ def test_reset_password_unitadmin(client): # Check that public key has changed user_public_key_after = user.public_key assert user_public_key_before != user_public_key_after + + +def test_reset_password_unitadmin_nonlatin1(client): + """Non latin 1 encodable characters should not be accepted.""" + # Get user + user = models.User.query.filter_by(username="unitadmin").first() + nr_proj_user_keys_total_before = models.ProjectUserKeys.query.count() + assert nr_proj_user_keys_total_before > 0 + + nr_proj_user_keys_before = len(user.project_user_keys) + assert nr_proj_user_keys_before > 0 + + user_pw_hash_before = user._password_hash + user_public_key_before = user.public_key + + # Add new row to password reset + new_reset_row = models.PasswordReset( + user=user, email=user.primary_email, issued=utils.timestamp() + ) + db.session.add(new_reset_row) + db.session.commit() + + # Need to use a valid token for the get request to get the form token + valid_reset_token = get_valid_reset_token("unitadmin") + response = client.get( + tests.DDSEndpoint.RESET_PASSWORD + valid_reset_token, + follow_redirects=True, + headers=tests.DEFAULT_HEADER, + ) + + assert response.status_code == http.HTTPStatus.OK + assert flask.request.path == tests.DDSEndpoint.RESET_PASSWORD + valid_reset_token + + # Set new password info -- contains invalid char € + form_token = flask.g.csrf_token + form_data = { + "csrf_token": form_token, + "password": "NewPassword123€", + "confirm_password": "NewPassword123€", + "submit": "Reset Password", + } + + # Password reset should not work and should instead ask again + response = client.post( + tests.DDSEndpoint.RESET_PASSWORD + valid_reset_token, + json=form_data, + follow_redirects=True, + headers=tests.DEFAULT_HEADER, + ) + assert response.status_code == http.HTTPStatus.OK + assert flask.request.path == tests.DDSEndpoint.RESET_PASSWORD + valid_reset_token + + # Get user + user = models.User.query.filter_by(username="unitadmin").first() + + # All users project keys should still exist + nr_proj_user_keys_after = len(user.project_user_keys) + assert nr_proj_user_keys_after == nr_proj_user_keys_before + + # Total nr of project user keys should be the same + nr_proj_user_keys_total_after = models.ProjectUserKeys.query.count() + assert nr_proj_user_keys_total_after == nr_proj_user_keys_total_before + + # Password should be the same + user_pw_hash_after = user._password_hash + assert user_pw_hash_before == user_pw_hash_after + + # Check that public key has not changed + user_public_key_after = user.public_key + assert user_public_key_before == user_public_key_after diff --git a/tests/test_utils.py b/tests/test_utils.py index dce4bdc3c..e047ef8bf 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -188,6 +188,22 @@ def test_contains_digit_or_specialchar_no_char(): utils.contains_digit_or_specialchar(indata="Thereisnodigitorchar1") +# contains_only_latin1 + + +def test_contains_only_latin1_invalid(): + """Non latin 1 encodable characters should raise a validation error with specific message.""" + with pytest.raises(marshmallow.ValidationError) as err: + utils.contains_only_latin1(indata="testing€") + assert "Contains invalid characters." in str(err.value) + + +def test_contains_only_latin1_ok(): + """Only latin 1 encodable characters should be allowed and give no exceptions.""" + returned = utils.contains_only_latin1(indata="testing") + assert returned is None + + # contains_disallowed_characters