From fe68d29ab78db5885bc31b67cf0537f1f02b33ad Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Tue, 19 Nov 2024 08:54:47 +0100 Subject: [PATCH] feat: Add form field validation from model field (#56) * Add pypi actions * bump version * feat: Move field validation to form field * Bump version * Fix syntax error in test action * For action fixes * Update pypi actions --- .github/workflows/lint.yml | 2 +- .github/workflows/publish-to-live-pypi.yml | 10 +-- .github/workflows/publish-to-test-pypi.yml | 11 ++- .github/workflows/test.yml | 16 ++-- CHANGELOG.rst | 10 +++ README.rst | 9 ++- djangocms_attributes_field/__init__.py | 2 +- djangocms_attributes_field/fields.py | 92 ++++++++++++---------- tests/requirements/dj32_cms310.txt | 4 - tests/requirements/dj32_cms311.txt | 4 - tests/requirements/dj32_cms39.txt | 4 - tests/requirements/dj40_cms311.txt | 4 - tests/requirements/dj41_cms311.txt | 4 - tests/requirements/dj50_cms41.txt | 4 + tests/requirements/dj51_cms41.txt | 4 + tests/test_fields.py | 26 ++++-- tox.ini | 19 ++--- 17 files changed, 127 insertions(+), 98 deletions(-) delete mode 100644 tests/requirements/dj32_cms310.txt delete mode 100644 tests/requirements/dj32_cms311.txt delete mode 100644 tests/requirements/dj32_cms39.txt delete mode 100644 tests/requirements/dj40_cms311.txt delete mode 100644 tests/requirements/dj41_cms311.txt create mode 100644 tests/requirements/dj50_cms41.txt create mode 100644 tests/requirements/dj51_cms41.txt diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 7bbd2c5..3812bb2 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -22,4 +22,4 @@ jobs: python -m pip install --upgrade pip pip install ruff - name: Run Ruff - run: ruff djangocms_attributes_field tests + run: ruff check djangocms_attributes_field tests diff --git a/.github/workflows/publish-to-live-pypi.yml b/.github/workflows/publish-to-live-pypi.yml index 75fee52..ec1abc5 100644 --- a/.github/workflows/publish-to-live-pypi.yml +++ b/.github/workflows/publish-to-live-pypi.yml @@ -9,14 +9,17 @@ jobs: build-n-publish: name: Build and publish Python 🐍 distributions 📦 to pypi runs-on: ubuntu-latest + environment: + name: pypi + url: https://pypi.org/p/djangocms-attributes_field permissions: id-token: write steps: - uses: actions/checkout@v3 - name: Set up Python 3.10 - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: - python-version: '3.10' + python-version: '3.12' - name: Install pypa/build run: >- @@ -36,6 +39,3 @@ jobs: - name: Publish distribution 📦 to PyPI if: startsWith(github.ref, 'refs/tags') uses: pypa/gh-action-pypi-publish@release/v1 - with: - user: __token__ - password: ${{ secrets.PYPI_API_TOKEN }} diff --git a/.github/workflows/publish-to-test-pypi.yml b/.github/workflows/publish-to-test-pypi.yml index c8aec7d..5b2a16a 100644 --- a/.github/workflows/publish-to-test-pypi.yml +++ b/.github/workflows/publish-to-test-pypi.yml @@ -9,12 +9,17 @@ jobs: build-n-publish: name: Build and publish Python 🐍 distributions 📦 to TestPyPI runs-on: ubuntu-latest + environment: + name: pypi + url: https://test.pypi.org/p/djangocms-attributes_field + permissions: + id-token: write steps: - uses: actions/checkout@v3 - name: Set up Python 3.10 - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: - python-version: '3.10' + python-version: '3.12' - name: Install pypa/build run: >- @@ -34,7 +39,5 @@ jobs: - name: Publish distribution 📦 to Test PyPI uses: pypa/gh-action-pypi-publish@release/v1 with: - user: __token__ - password: ${{ secrets.TEST_PYPI_API_TOKEN }} repository_url: https://test.pypi.org/legacy/ skip_existing: true diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5591bf2..739c044 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -8,17 +8,21 @@ jobs: strategy: fail-fast: false matrix: - python-version: [ 3.9, "3.10", "3.11"] # latest release minus two + python-version: [ "3.9", "3.10", "3.11", "3.12"] # latest release minus two requirements-file: [ - dj32_cms311.txt, - dj40_cms311.txt, - dj41_cms311.txt, dj42_cms311.txt, - dj42_cms41.txt + dj42_cms41.txt, + dj50_cms41.txt, + dj51_cms41.txt ] os: [ ubuntu-20.04, ] + exclude: + - python-version: "3.9" + requirements-file: dj50_cms41.txt + - python-version: "3.9" + requirements-file: dj51_cms41.txt steps: - uses: actions/checkout@v1 @@ -34,7 +38,7 @@ jobs: python setup.py install - name: Run coverage - run: coverage run setup.py test + run: coverage run tests/settings.py - name: Upload Coverage to Codecov uses: codecov/codecov-action@v1 diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c7f940d..dbba421 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,16 @@ Changelog ========= +4.0.0 (2024-11-19) +================== + +* Moved validation to from AttributeField to AttributeFormField +* Added djangocms_attributes_fields.fields.default_excluded_keys to include + keys that can execute javascript +* Added tests for Django 5.0, 5.1 +* Dropped support for Django 3.2, 4.0, 4.1 +* Dropped support for Python 3.6, 3.7, 3.8 + 3.0.0 (2023-05-24) ================== diff --git a/README.rst b/README.rst index 6def1f2..b322e16 100644 --- a/README.rst +++ b/README.rst @@ -91,6 +91,13 @@ There is an optional parameter that can be used when declaring the field: :: ``excluded_keys`` : This is a list of strings that will not be accepted as valid keys +Since version 4, the following keys are always excluded (see +``djangocms_attributes_fields.fields.default_excluded_keys``) to avoid +unwanted execution of javascript: :: + + ["src", "href", "data", "action", "on*"] + +``'on*'`` represents any key that starts with ``'on'``. property: [field_name]_str ++++++++++++++++++++++++++ @@ -160,7 +167,7 @@ You can run tests by executing:: virtualenv env source env/bin/activate pip install -r tests/requirements.txt - python setup.py test + python tests/settings.py .. |pypi| image:: https://badge.fury.io/py/djangocms-attributes-field.svg diff --git a/djangocms_attributes_field/__init__.py b/djangocms_attributes_field/__init__.py index 4eb28e3..d6497a8 100644 --- a/djangocms_attributes_field/__init__.py +++ b/djangocms_attributes_field/__init__.py @@ -1 +1 @@ -__version__ = '3.0.0' +__version__ = '4.0.0' diff --git a/djangocms_attributes_field/fields.py b/djangocms_attributes_field/fields.py index 423e05b..a30b52b 100644 --- a/djangocms_attributes_field/fields.py +++ b/djangocms_attributes_field/fields.py @@ -14,12 +14,16 @@ regex_key_validator = RegexValidator(regex=r'^[a-z][-a-z0-9_:]*\Z', flags=re.IGNORECASE, code='invalid') +default_excluded_keys = [ + "src", "href", "data", "action", "on*", +] class AttributesFormField(forms.CharField): empty_values = [None, ''] def __init__(self, *args, **kwargs): kwargs.setdefault('widget', AttributesWidget) + self.excluded_keys = kwargs.pop('excluded_keys', []) + default_excluded_keys super().__init__(*args, **kwargs) def to_python(self, value): @@ -37,6 +41,49 @@ def validate(self, value): # This is required in older django versions. if value in self.empty_values and self.required: raise forms.ValidationError(self.error_messages['required'], code='required') + if isinstance(value, dict): + for key, val in value.items(): + self.validate_key(key) + self.validate_value(key, val) + + def validate_key(self, key): + """ + A key must start with a letter, but can otherwise contain letters, + numbers, dashes, colons or underscores. It must not also be part of + `excluded_keys` as configured in the field. + + :param key: (str) The key to validate + """ + # Verify the key is not one of `excluded_keys`. + for excluded_key in self.excluded_keys: + if key.lower() == excluded_key or excluded_key.endswith("*") and key.lower().startswith(excluded_key[:-1]): + raise ValidationError( + _('"{key}" is excluded by configuration and cannot be used as ' + 'a key.').format(key=key)) + # Also check that it fits our permitted syntax + try: + regex_key_validator(key) + except ValidationError: + # Seems silly to catch one then raise another ValidationError, but + # the RegExValidator doesn't use placeholders in its error message. + raise ValidationError( + _('"{key}" is not a valid key. Keys must start with at least ' + 'one letter and consist only of the letters, numbers, ' + 'underscores or hyphens.').format(key=key)) + + def validate_value(self, key, value): + """ + A value can be anything that can be JSON-ified. + + :param key: (str) The key of the value + :param value: (str) The value to validate + """ + try: + json.dumps(value) + except (TypeError, ValueError): + raise ValidationError( + _('The value for the key "{key}" is invalid. Please enter a ' + 'value that can be represented in JSON.').format(key=key)) class AttributesField(models.Field): @@ -64,7 +111,7 @@ def __init__(self, *args, **kwargs): kwargs['default'] = kwargs.get('default', dict) excluded_keys = kwargs.pop('excluded_keys', []) # Note we accept uppercase letters in the param, but the comparison - # is not case sensitive. So, we coerce the input to lowercase here. + # is not case-sensitive. So, we coerce the input to lowercase here. self.excluded_keys = [key.lower() for key in excluded_keys] super().__init__(*args, **kwargs) self.validate(self.get_default(), None) @@ -75,6 +122,7 @@ def formfield(self, **kwargs): 'widget': AttributesWidget } defaults.update(**kwargs) + defaults["excluded_keys"] = self.excluded_keys return super().formfield(**defaults) def from_db_value(self, value, @@ -134,48 +182,6 @@ def validate(self, value, model_instance): except ValueError: raise ValidationError(self.error_messages['invalid'] % value) - for key, val in value.items(): - self.validate_key(key) - self.validate_value(key, val) - - def validate_key(self, key): - """ - A key must start with a letter, but can otherwise contain letters, - numbers, dashes, colons or underscores. It must not also be part of - `excluded_keys` as configured in the field. - - :param key: (str) The key to validate - """ - # Verify the key is not one of `excluded_keys`. - if key.lower() in self.excluded_keys: - raise ValidationError( - _('"{key}" is excluded by configuration and cannot be used as ' - 'a key.').format(key=key)) - # Also check that it fits our permitted syntax - try: - regex_key_validator(key) - except ValidationError: - # Seems silly to catch one then raise another ValidationError, but - # the RegExValidator doesn't use placeholders in its error message. - raise ValidationError( - _('"{key}" is not a valid key. Keys must start with at least ' - 'one letter and consist only of the letters, numbers, ' - 'underscores or hyphens.').format(key=key)) - - def validate_value(self, key, value): - """ - A value can be anything that can be JSON-ified. - - :param key: (str) The key of the value - :param value: (str) The value to validate - """ - try: - json.dumps(value) - except (TypeError, ValueError): - raise ValidationError( - _('The value for the key "{key}" is invalid. Please enter a ' - 'value that can be represented in JSON.').format(key=key)) - def value_to_string(self, obj): return self.value_from_object(obj) diff --git a/tests/requirements/dj32_cms310.txt b/tests/requirements/dj32_cms310.txt deleted file mode 100644 index 76b64d7..0000000 --- a/tests/requirements/dj32_cms310.txt +++ /dev/null @@ -1,4 +0,0 @@ --r base.txt - -Django>=3.2,<4.0 -django-cms>=3.10,<3.11 diff --git a/tests/requirements/dj32_cms311.txt b/tests/requirements/dj32_cms311.txt deleted file mode 100644 index be39e9b..0000000 --- a/tests/requirements/dj32_cms311.txt +++ /dev/null @@ -1,4 +0,0 @@ --r base.txt - -Django>=3.2,<4 -django-cms>=3.11,<4 diff --git a/tests/requirements/dj32_cms39.txt b/tests/requirements/dj32_cms39.txt deleted file mode 100644 index e5b970a..0000000 --- a/tests/requirements/dj32_cms39.txt +++ /dev/null @@ -1,4 +0,0 @@ --r base.txt - -Django>=3.2,<4.0 -django-cms>=3.9,<3.10 diff --git a/tests/requirements/dj40_cms311.txt b/tests/requirements/dj40_cms311.txt deleted file mode 100644 index 953d715..0000000 --- a/tests/requirements/dj40_cms311.txt +++ /dev/null @@ -1,4 +0,0 @@ --r base.txt - -Django>=4.0,<4.1 -django-cms>=3.11.1,<4 diff --git a/tests/requirements/dj41_cms311.txt b/tests/requirements/dj41_cms311.txt deleted file mode 100644 index e30f0ae..0000000 --- a/tests/requirements/dj41_cms311.txt +++ /dev/null @@ -1,4 +0,0 @@ --r base.txt - -Django>=4.1,<4.2 -django-cms>=3.11.1,<4 diff --git a/tests/requirements/dj50_cms41.txt b/tests/requirements/dj50_cms41.txt new file mode 100644 index 0000000..1c578f2 --- /dev/null +++ b/tests/requirements/dj50_cms41.txt @@ -0,0 +1,4 @@ +-r base.txt + +Django>=5.0,<5.1 +django-cms>=4.1,<4.2 diff --git a/tests/requirements/dj51_cms41.txt b/tests/requirements/dj51_cms41.txt new file mode 100644 index 0000000..3e2807c --- /dev/null +++ b/tests/requirements/dj51_cms41.txt @@ -0,0 +1,4 @@ +-r base.txt + +Django>=5.1,<5.2 +django-cms>=4.1,<4.2 diff --git a/tests/test_fields.py b/tests/test_fields.py index cce4246..8880412 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -15,7 +15,7 @@ class Noop: class KeyValidationTests(TestCase): def test_validate_key(self): - field = AttributesField() + field = AttributesFormField() # Normal, expected patterns try: field.validate_key('target') @@ -48,21 +48,35 @@ def test_validate_key(self): def test_excluded_keys(self): # First prove that the keys we're about to test would normally pass - field = AttributesField() + field = AttributesFormField() try: - field.validate_key('href') - field.validate_key('src') + + field.validate_key('title') + field.validate_key('data-test') except ValidationError: self.fail('Keys that pass have failed.') # Now show that they no longer pass if explicitly exclude - field = AttributesField(excluded_keys=['href', 'src', ]) + field = AttributesFormField(excluded_keys=['title', 'data-test', ]) with self.assertRaises(ValidationError): - field.validate_key('href') + field.validate_key('title') with self.assertRaises(ValidationError): field.validate_key('src') + def test_default_excluded_keys(self): + from djangocms_attributes_field.fields import default_excluded_keys + + field = AttributesFormField() + for key in default_excluded_keys: + with self.subTest(key=key): + with self.assertRaises(ValidationError): + field.validate_key(key) + + with self.subTest(key="onsomething"): + with self.assertRaises(ValidationError): + field.validate_key(key) + class AttributesFieldsTestCase(TestCase): diff --git a/tox.ini b/tox.ini index d4c4371..1ebcf1c 100644 --- a/tox.ini +++ b/tox.ini @@ -1,10 +1,7 @@ [tox] envlist = - flake8 - isort - py{35,36,37,38}-dj{22}-cms{37,38} - py{36,37,38}-dj{30}-cms{37,38} - py{36,37,38}-dj{31}-cms{38} + py{39,310,311}-dj{42}-cms{311,41} + py{310,311,312}-dj{50,51}-cms{41} skip_missing_interpreters=True @@ -40,15 +37,15 @@ known_django = django [testenv] deps = -r{toxinidir}/tests/requirements/base.txt - dj22: Django>=2.2,<3.0 - dj30: Django>=3.0,<3.1 - dj31: Django>=3.1,<3.2 - cms37: django-cms>=3.7,<3.8 - cms38: django-cms>=3.8,<3.9 + dj42: Django>=4.2,<5.0 + dj50: Django>=5.0,<5.1 + dj51: Django>=5.1,<5.2 + cms311: django-cms>=3.11,<4 + cms41: django-cms>=4.1,<4.2 commands = {envpython} --version {env:COMMAND:coverage} erase - {env:COMMAND:coverage} run setup.py test + {env:COMMAND:coverage} run tests/settings.py {env:COMMAND:coverage} report [testenv:flake8]