Only allow latin-1 encodable characters in username and password

CHANGELOG.md

@@ -220,3 +220,7 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe
 - Workflow: Schedule trivy scan for both dev images and latest release ([#1392](https://github.com/ScilifelabDataCentre/dds_web/pull/1392))
 - Improve logging of delete-invites flask command ([#1386](https://github.com/ScilifelabDataCentre/dds_web/pull/1386))
 - Workflow: Schedule trivy scan for dev and latest separately ([#1395](https://github.com/ScilifelabDataCentre/dds_web/pull/1395))
+
+## Sprint (2023-03-03 - 2023-03-17)
+
+- Only allow latin1-encodable usernames and passwords ([#1402](https://github.com/ScilifelabDataCentre/dds_web/pull/1402))

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 <f0><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:
@@ -292,7 +304,12 @@ def verify_enough_unit_admins(unit_id: str, force_create: bool = False):
 
 def valid_chars_in_username(indata):
     """Check if the username contains only valid characters."""
-    return bool(re.search(r"^[a-zA-Z0-9_\.-]+$", indata))
+    try:
+        contains_only_latin1(indata=indata)
+    except marshmallow.ValidationError:
+        return False
+    else:
+        return bool(re.search(r"^[a-zA-Z0-9_\.-]+$", indata))
 
 
 def email_in_db(email):

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"])
 

tests/test_user_confirm_invites_and_register.py

@@ -172,6 +172,36 @@ def test_register_weak_password(registry_form_data, client):
     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 == "200 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="[email protected]", 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,

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

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