Skip to content

Commit

Permalink
feat: Add form field validation from model field (#56)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
fsbraun authored Nov 19, 2024
1 parent 73007eb commit fe68d29
Show file tree
Hide file tree
Showing 17 changed files with 127 additions and 98 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 5 additions & 5 deletions .github/workflows/publish-to-live-pypi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: >-
Expand All @@ -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 }}
11 changes: 7 additions & 4 deletions .github/workflows/publish-to-test-pypi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: >-
Expand All @@ -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
16 changes: 10 additions & 6 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
10 changes: 10 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
==================

Expand Down
9 changes: 8 additions & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
++++++++++++++++++++++++++
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion djangocms_attributes_field/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = '3.0.0'
__version__ = '4.0.0'
92 changes: 49 additions & 43 deletions djangocms_attributes_field/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -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)

Expand Down
4 changes: 0 additions & 4 deletions tests/requirements/dj32_cms310.txt

This file was deleted.

4 changes: 0 additions & 4 deletions tests/requirements/dj32_cms311.txt

This file was deleted.

4 changes: 0 additions & 4 deletions tests/requirements/dj32_cms39.txt

This file was deleted.

4 changes: 0 additions & 4 deletions tests/requirements/dj40_cms311.txt

This file was deleted.

4 changes: 0 additions & 4 deletions tests/requirements/dj41_cms311.txt

This file was deleted.

4 changes: 4 additions & 0 deletions tests/requirements/dj50_cms41.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-r base.txt

Django>=5.0,<5.1
django-cms>=4.1,<4.2
4 changes: 4 additions & 0 deletions tests/requirements/dj51_cms41.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-r base.txt

Django>=5.1,<5.2
django-cms>=4.1,<4.2
26 changes: 20 additions & 6 deletions tests/test_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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):

Expand Down
19 changes: 8 additions & 11 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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]
Expand Down

0 comments on commit fe68d29

Please sign in to comment.