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

Author: fsbraun

.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

.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 }}

.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

.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

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)
 ==================
 

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

djangocms_attributes_field/__init__.py

@@ -1 +1 @@
-__version__ = '3.0.0'
+__version__ = '4.0.0'

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)
 

tests/requirements/dj32_cms310.txt

@@ -0,0 +1,4 @@
+-r base.txt
+
+Django>=5.0,<5.1
+django-cms>=4.1,<4.2

tests/requirements/dj32_cms311.txt

@@ -0,0 +1,4 @@
+-r base.txt
+
+Django>=5.1,<5.2
+django-cms>=4.1,<4.2

tests/requirements/dj32_cms39.txt

@@ -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):
 

tests/requirements/dj40_cms311.txt

@@ -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]