diff --git a/src/core/migrations/0096_alter_account_activation_code_and_more.py b/src/core/migrations/0096_alter_account_activation_code_and_more.py index 424abfd9c..e9221a26f 100644 --- a/src/core/migrations/0096_alter_account_activation_code_and_more.py +++ b/src/core/migrations/0096_alter_account_activation_code_and_more.py @@ -3,6 +3,8 @@ import core.model_utils from django.db import migrations, models +import utils + class Migration(migrations.Migration): @@ -19,32 +21,32 @@ class Migration(migrations.Migration): migrations.AlterField( model_name='account', name='department', - field=core.model_utils.JanewayBleachCharField(blank=True, max_length=300, verbose_name='Department'), + field=models.CharField(blank=True, max_length=300, verbose_name='Department', validators=[utils.forms.plain_text_validator]), ), migrations.AlterField( model_name='account', name='first_name', - field=core.model_utils.JanewayBleachCharField(max_length=300, verbose_name='First name'), + field=models.CharField(max_length=300, verbose_name='First name', validators=[utils.forms.plain_text_validator]), ), migrations.AlterField( model_name='account', name='institution', - field=core.model_utils.JanewayBleachCharField(blank=True, max_length=1000, verbose_name='Institution'), + field=models.CharField(blank=True, max_length=1000, verbose_name='Institution', validators=[utils.forms.plain_text_validator]), ), migrations.AlterField( model_name='account', name='last_name', - field=core.model_utils.JanewayBleachCharField(max_length=300, verbose_name='Last name'), + field=models.CharField(max_length=300, verbose_name='Last name', validators=[utils.forms.plain_text_validator]), ), migrations.AlterField( model_name='account', name='middle_name', - field=core.model_utils.JanewayBleachCharField(blank=True, max_length=300, verbose_name='Middle name'), + field=models.CharField(blank=True, max_length=300, verbose_name='Middle name', validators=[utils.forms.plain_text_validator]), ), migrations.AlterField( model_name='account', name='salutation', - field=core.model_utils.JanewayBleachCharField(blank=True, choices=[('Miss', 'Miss'), ('Ms', 'Ms'), ('Mrs', 'Mrs'), ('Mr', 'Mr'), ('Mx', 'Mx'), ('Dr', 'Dr'), ('Prof.', 'Prof.')], max_length=10, verbose_name='Salutation'), + field=models.CharField(blank=True, choices=[('Miss', 'Miss'), ('Ms', 'Ms'), ('Mrs', 'Mrs'), ('Mr', 'Mr'), ('Mx', 'Mx'), ('Dr', 'Dr'), ('Prof.', 'Prof.')], max_length=10, verbose_name='Salutation', validators=[utils.forms.plain_text_validator]), ), migrations.AlterField( model_name='account', @@ -54,6 +56,6 @@ class Migration(migrations.Migration): migrations.AlterField( model_name='account', name='suffix', - field=core.model_utils.JanewayBleachCharField(blank=True, max_length=300, verbose_name='Name suffix'), + field=models.CharField(blank=True, max_length=300, verbose_name='Name suffix', validators=[utils.forms.plain_text_validator]), ), ] diff --git a/src/core/models.py b/src/core/models.py index 9a3661a4b..f3ba5e03a 100644 --- a/src/core/models.py +++ b/src/core/models.py @@ -53,6 +53,7 @@ from submission import models as submission_models from utils.logger import get_logger from utils import logic as utils_logic +from utils.forms import plain_text_validator from production import logic as production_logic fs = JanewayFileSystemStorage() @@ -235,48 +236,55 @@ class Account(AbstractBaseUser, PermissionsMixin): username = models.CharField(max_length=254, unique=True, verbose_name=_('Username')) name_prefix = models.CharField(max_length=10, blank=True) - first_name = JanewayBleachCharField( + first_name = models.CharField( max_length=300, blank=False, verbose_name=_('First name'), + validators=[plain_text_validator], ) - middle_name = JanewayBleachCharField( + middle_name = models.CharField( max_length=300, blank=True, verbose_name=_('Middle name'), + validators=[plain_text_validator], ) - last_name = JanewayBleachCharField( + last_name = models.CharField( max_length=300, blank=False, verbose_name=_('Last name'), + validators=[plain_text_validator], ) activation_code = models.CharField(max_length=100, null=True, blank=True) - salutation = JanewayBleachCharField( + salutation = models.CharField( max_length=10, choices=SALUTATION_CHOICES, blank=True, verbose_name=_('Salutation'), + validators=[plain_text_validator], ) - suffix = JanewayBleachCharField( + suffix = models.CharField( max_length=300, blank=True, verbose_name=_('Name suffix'), + validators=[plain_text_validator], ) biography = JanewayBleachField( blank=True, verbose_name=_('Biography'), ) orcid = models.CharField(max_length=40, null=True, blank=True, verbose_name=_('ORCiD')) - institution = JanewayBleachCharField( + institution = models.CharField( max_length=1000, blank=True, verbose_name=_('Institution'), + validators=[plain_text_validator], ) - department = JanewayBleachCharField( + department = models.CharField( max_length=300, blank=True, verbose_name=_('Department'), + validators=[plain_text_validator], ) twitter = models.CharField(max_length=300, null=True, blank=True, verbose_name=_('Twitter Handle')) facebook = models.CharField(max_length=300, null=True, blank=True, verbose_name=_('Facebook Handle')) diff --git a/src/submission/migrations/0080_frozen_author_bleach_20240507_1350.py b/src/submission/migrations/0080_frozen_author_bleach_20240507_1350.py index 2cc7ae683..d49a0fb61 100644 --- a/src/submission/migrations/0080_frozen_author_bleach_20240507_1350.py +++ b/src/submission/migrations/0080_frozen_author_bleach_20240507_1350.py @@ -2,6 +2,7 @@ import core.model_utils from django.db import migrations, models +import utils.forms class Migration(migrations.Migration): @@ -15,12 +16,12 @@ class Migration(migrations.Migration): migrations.AlterField( model_name='frozenauthor', name='department', - field=core.model_utils.JanewayBleachCharField(blank=True, max_length=300), + field=models.CharField(blank=True, max_length=300, validators=[utils.forms.plain_text_validator]), ), migrations.AlterField( model_name='frozenauthor', name='first_name', - field=core.model_utils.JanewayBleachCharField(blank=True, max_length=300), + field=models.CharField(blank=True, max_length=300, validators=[utils.forms.plain_text_validator]), ), migrations.AlterField( model_name='frozenauthor', @@ -40,26 +41,26 @@ class Migration(migrations.Migration): migrations.AlterField( model_name='frozenauthor', name='institution', - field=core.model_utils.JanewayBleachCharField(blank=True, max_length=1000), + field=models.CharField(blank=True, max_length=1000, validators=[utils.forms.plain_text_validator]), ), migrations.AlterField( model_name='frozenauthor', name='last_name', - field=core.model_utils.JanewayBleachCharField(blank=True, max_length=300), + field=models.CharField(blank=True, max_length=300, validators=[utils.forms.plain_text_validator]), ), migrations.AlterField( model_name='frozenauthor', name='middle_name', - field=core.model_utils.JanewayBleachCharField(blank=True, max_length=300), + field=models.CharField(blank=True, max_length=300, validators=[utils.forms.plain_text_validator]), ), migrations.AlterField( model_name='frozenauthor', name='name_prefix', - field=core.model_utils.JanewayBleachCharField(blank=True, help_text='Optional name prefix (e.g: Prof or Dr)', max_length=300), + field=models.CharField(blank=True, help_text='Optional name prefix (e.g: Prof or Dr)', max_length=300, validators=[utils.forms.plain_text_validator]), ), migrations.AlterField( model_name='frozenauthor', name='name_suffix', - field=core.model_utils.JanewayBleachCharField(blank=True, help_text='Optional name suffix (e.g.: Jr or III)', max_length=300), + field=models.CharField(blank=True, help_text='Optional name suffix (e.g.: Jr or III)', max_length=300, validators=[utils.forms.plain_text_validator]), ), ] diff --git a/src/submission/models.py b/src/submission/models.py index a9b6d80c9..80e953776 100755 --- a/src/submission/models.py +++ b/src/submission/models.py @@ -54,6 +54,7 @@ from review import models as review_models from utils.function_cache import cache from utils.logger import get_logger +from utils.forms import plain_text_validator from journal import models as journal_models from review.const import ( ReviewerDecisions as RD, @@ -1911,23 +1912,44 @@ class FrozenAuthor(AbstractLastModifiedModel): on_delete=models.SET_NULL, ) - name_prefix = JanewayBleachCharField( + name_prefix = models.CharField( max_length=300, blank=True, - help_text=_("Optional name prefix (e.g: Prof or Dr)") - - ) - name_suffix = JanewayBleachCharField( + help_text=_("Optional name prefix (e.g: Prof or Dr)"), + validators=[plain_text_validator], + ) + name_suffix = models.CharField( + max_length=300, + blank=True, + help_text=_("Optional name suffix (e.g.: Jr or III)"), + validators=[plain_text_validator], + ) + first_name = models.CharField( max_length=300, blank=True, - help_text=_("Optional name suffix (e.g.: Jr or III)") + validators=[plain_text_validator], + ) + middle_name = models.CharField( + max_length=300, + blank=True, + validators=[plain_text_validator], ) - first_name = JanewayBleachCharField(max_length=300, blank=True) - middle_name = JanewayBleachCharField(max_length=300, blank=True) - last_name = JanewayBleachCharField(max_length=300, blank=True) + last_name = models.CharField( + max_length=300, + blank=True, + validators=[plain_text_validator], +) - institution = JanewayBleachCharField(max_length=1000, blank=True) - department = JanewayBleachCharField(max_length=300, blank=True) + institution = models.CharField( + max_length=1000, + blank=True, + validators=[plain_text_validator], +) + department = models.CharField( + max_length=300, + blank=True, + validators=[plain_text_validator], + ) frozen_biography = JanewayBleachField( blank=True, verbose_name=_('Frozen Biography'), diff --git a/src/submission/tests.py b/src/submission/tests.py index 6b4f57a2e..d3e490d57 100644 --- a/src/submission/tests.py +++ b/src/submission/tests.py @@ -13,6 +13,7 @@ from django.utils import translation, timezone from django.urls.base import clear_script_prefix from django.conf import settings +from django.core.exceptions import ValidationError from django.shortcuts import reverse from django.test.utils import override_settings @@ -567,6 +568,33 @@ def test_author_form_with_good_orcid(self): '0000-0003-2126-266X', ) + def test_author_form_harmful_inputs(self): + harmful_string = ' This are not the droids you are looking for ' + for i, attr in enumerate({ + "first_name", + "last_name", + "middle_name", + "name_prefix", + "suffix", + "institution", + "department", + }): + form = forms.AuthorForm( + { + 'first_name': 'Andy', + 'last_name': 'Byers', + 'biography': 'Andy', + 'institution': 'Birkbeck, University of London', + 'email': f'andy{i}@janeway.systems', + 'orcid': 'https://orcid.org/0000-0003-2126-266X', + **{attr: harmful_string}, + } + ) + self.assertFalse( + form.is_valid(), + f"Harmful code injected into field '{attr}'" + ) + @override_settings(URL_CONFIG='domain') def test_article_encoding_bibtex(self): article = helpers.create_article( diff --git a/src/templates/common/elements/orcid_registration.html b/src/templates/common/elements/orcid_registration.html index ee83a4b71..f16882b26 100644 --- a/src/templates/common/elements/orcid_registration.html +++ b/src/templates/common/elements/orcid_registration.html @@ -9,6 +9,10 @@ [remove]

{% else %} - {% trans "Register with ORCiD" %} + + {% trans "Register with ORCiD" %} + {% endif %} {% endif %} diff --git a/src/themes/OLH/templates/core/accounts/register.html b/src/themes/OLH/templates/core/accounts/register.html index 5a3d372c4..3898ca57e 100644 --- a/src/themes/OLH/templates/core/accounts/register.html +++ b/src/themes/OLH/templates/core/accounts/register.html @@ -30,7 +30,7 @@
{% trans "Register for an account with" %} {{ request.press.name }}.

{% blocktrans %}For more information read our password guide.{% endblocktrans %}

- {% include "common/elements/orcid_registration.html" %} + {% include "common/elements/orcid_registration.html" with button_classes="button expanded orcid-button" %} {% include "elements/forms/errors.html" with form=form %} diff --git a/src/themes/clean/templates/core/accounts/register.html b/src/themes/clean/templates/core/accounts/register.html index 2511eafdf..be56f96d8 100644 --- a/src/themes/clean/templates/core/accounts/register.html +++ b/src/themes/clean/templates/core/accounts/register.html @@ -20,7 +20,7 @@
{% trans "Register for an account with" %} {{ request.press.name }}.
{% include "common/elements/password_rules.html" %}

{% blocktrans %}For more information read our password guide.{% endblocktrans %}

- {% include "common/elements/orcid_registration.html" %} + {% include "common/elements/orcid_registration.html" with button_classes="btn orcid-button btn-block" %}
{% bootstrap_form form %}

diff --git a/src/themes/material/templates/core/accounts/register.html b/src/themes/material/templates/core/accounts/register.html index 40c25933b..9599b9345 100644 --- a/src/themes/material/templates/core/accounts/register.html +++ b/src/themes/material/templates/core/accounts/register.html @@ -24,7 +24,7 @@

{% blocktrans trimmed %}For more information read our password guide.{% endblocktrans %}

- {% include "common/elements/orcid_registration.html" %} + {% include "common/elements/orcid_registration.html" with button_classes="btn wide-button orcid-button" %}
{% include "elements/forms/errors.html" %} {% csrf_token %} diff --git a/src/utils/forms.py b/src/utils/forms.py index 1b6bb5eaf..e44988180 100644 --- a/src/utils/forms.py +++ b/src/utils/forms.py @@ -1,3 +1,4 @@ +import bleach from django.forms import ( CharField, CheckboxInput, @@ -8,6 +9,7 @@ ) from django.utils.translation import gettext_lazy as _ from django.conf import settings +from django.core.exceptions import ValidationError from modeltranslation import forms as mt_forms, translator from captcha.fields import ReCaptchaField @@ -18,6 +20,9 @@ from submission import models as submission_models +ENTITIES_MAP = (("&", "&"), (">", ">"), ("<", "<")) + + class JanewayTranslationModelForm(mt_forms.TranslationModelForm): def __init__(self, *args, **kwargs): super(JanewayTranslationModelForm, self).__init__(*args, **kwargs) @@ -109,3 +114,38 @@ def __init__(self, *args, **kwargs): captcha = CharField(widget=HiddenInput, required=False) self.fields["captcha"] = captcha + + +def text_sanitizer(text_value, tags=None, attrs=None, excl=ENTITIES_MAP): + """ A sanitizer for clearing potential harmful html/css/js from the input + :param text_value: the string to sanitize + :param tags: A list of allowed html tags + :param attrs: A dict of allowed html attributes + :param excl: A list of pairs of allowed items and their replacement + :return: Sanitized string + """ + tags = tags or [] + attrs = attrs or {} + excl = excl or {} + + cleaned = bleach.clean( + text_value, + tags=tags, + attributes=attrs, + strip=True, + ) + # Allow certain entities that bleach won't whitelist + # https://github.com/mozilla/bleach/issues/192#issuecomment-2304545475 + for escaped, raw in excl: + cleaned = cleaned.replace(escaped, raw) + + return cleaned + + +def plain_text_validator(value): + """ A field validator that ensures a textual input has no harmful code""" + sanitized = text_sanitizer(value) + if value != sanitized: + raise ValidationError( + _("HTML is not allowed in this field") + ) diff --git a/src/utils/tests.py b/src/utils/tests.py index 64ab1153b..5ce3bc161 100644 --- a/src/utils/tests.py +++ b/src/utils/tests.py @@ -10,6 +10,7 @@ from django.test import TestCase, override_settings from django.utils import timezone, translation from django.core import mail +from django.core.exceptions import ValidationError from django.core.management import call_command from django.contrib.admin.sites import site from django.contrib.auth import get_user_model @@ -29,7 +30,11 @@ from utils import install from utils.transactional_emails import * -from utils.forms import FakeModelForm, KeywordModelForm +from utils.forms import ( + FakeModelForm, + KeywordModelForm, + plain_text_validator, +) from utils.logic import generate_sitemap from utils.testing import helpers from utils.shared import clear_cache @@ -792,6 +797,23 @@ class Meta: self.assertFalse(journal.keywords.exists()) +class TestPlainTextValidator(TestCase): + + def test_plain_text_validator_valid(self): + name_test = "Kathryn Janeway" + ampersand_test = "Voyager & co" + try: + plain_text_validator(name_test) + plain_text_validator(ampersand_test) + except ValidationError: + self.fail("Valid plain text input raised a ValidationError") + + def test_plain_text_validator_invalid(self): + rogue_input = 'Borg Queen' + with self.assertRaises(ValidationError): + plain_text_validator(rogue_input) + + class TestModels(TestCase): @classmethod