From c54e2621f261ba226573aca53e7aec1486ed9c78 Mon Sep 17 00:00:00 2001 From: FinnStutzenstein Date: Wed, 6 Nov 2019 15:55:03 +0100 Subject: [PATCH] Added html validation for users and personal notes --- openslides/users/serializers.py | 6 +++ openslides/users/views.py | 4 +- openslides/utils/validate.py | 26 +++++++++++++ tests/integration/users/test_viewset.py | 52 +++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 1 deletion(-) diff --git a/openslides/users/serializers.py b/openslides/users/serializers.py index d0143fee2..83ee192a7 100644 --- a/openslides/users/serializers.py +++ b/openslides/users/serializers.py @@ -9,6 +9,7 @@ from ..utils.rest_api import ( RelatedField, ValidationError, ) +from ..utils.validate import validate_html from .models import Group, PersonalNote, User @@ -90,6 +91,11 @@ class UserFullSerializer(ModelSerializer): data["username"] = User.objects.generate_username( data.get("first_name", ""), data.get("last_name", "") ) + + # check the about_me html + if "about_me" in data: + data["about_me"] = validate_html(data["about_me"]) + return data def prepare_password(self, validated_data): diff --git a/openslides/users/views.py b/openslides/users/views.py index 2ef30d62a..248d7545c 100644 --- a/openslides/users/views.py +++ b/openslides/users/views.py @@ -42,6 +42,7 @@ from ..utils.rest_api import ( list_route, status, ) +from ..utils.validate import validate_json from ..utils.views import APIView from .access_permissions import ( GroupAccessPermissions, @@ -688,7 +689,8 @@ class PersonalNoteViewSet(ModelViewSet): for data in request.data: if data["collection"] not in personal_note.notes: personal_note.notes[data["collection"]] = {} - personal_note.notes[data["collection"]][data["id"]] = data["content"] + content = validate_json(data["content"], 2) + personal_note.notes[data["collection"]][data["id"]] = content personal_note.save() return Response() diff --git a/openslides/utils/validate.py b/openslides/utils/validate.py index bbbae769d..54ae3c2a2 100644 --- a/openslides/utils/validate.py +++ b/openslides/utils/validate.py @@ -1,5 +1,9 @@ +from typing import Any + import bleach +from .rest_api import ValidationError + allowed_tags = [ "a", @@ -63,3 +67,25 @@ def validate_html(html: str) -> str: return bleach.clean( html, tags=allowed_tags, attributes=allowed_attributes, styles=allowed_styles ) + + +def validate_json(json: Any, max_depth: int) -> Any: + """ + Traverses through the JSON structure (dicts and lists) and runs + validate_html on every found string. + + Give max-depth to protect against stack-overflows. This should be the + maximum nested depth of the object expected. + """ + + if max_depth == 0: + raise ValidationError({"detail": "The JSON is too nested."}) + + if isinstance(json, dict): + return {key: validate_json(value, max_depth - 1) for key, value in json.items()} + if isinstance(json, list): + return [validate_json(item, max_depth - 1) for item in json] + if isinstance(json, str): + return validate_html(json) + + return json diff --git a/tests/integration/users/test_viewset.py b/tests/integration/users/test_viewset.py index 062529486..9377145c8 100644 --- a/tests/integration/users/test_viewset.py +++ b/tests/integration/users/test_viewset.py @@ -124,6 +124,20 @@ class UserCreate(TestCase): {"groups_id": ['Invalid pk "%d" - object does not exist.' % group_pk]}, ) + def test_clean_html(self): + self.client.login(username="admin", password="admin") + response = self.client.post( + reverse("user-list"), + { + "username": "test_name_Thimoo2ho7ahreighio3", + "about_me": "

bar

", + }, + ) + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + user = User.objects.get(username="test_name_Thimoo2ho7ahreighio3") + self.assertEqual(user.about_me, "

<foo>bar</foo>

") + class UserUpdate(TestCase): """ @@ -913,6 +927,44 @@ class PersonalNoteTest(TestCase): "test_note_do2ncoi7ci2fm93LjwlO", ) + def test_clean_html(self): + admin_client = APIClient() + admin_client.login(username="admin", password="admin") + response = admin_client.post( + reverse("personalnote-create-or-update"), + [ + { + "collection": "test_collection", + "id": 1, + "content": {"note": "

bar

", "star": False}, + } + ], + format="json", + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + personal_note = PersonalNote.objects.get() + self.assertEqual( + personal_note.notes["test_collection"]["1"], + {"note": "

<foo>bar</foo>

", "star": False}, + ) + + def test_clean_html_content_too_nested(self): + admin_client = APIClient() + admin_client.login(username="admin", password="admin") + response = admin_client.post( + reverse("personalnote-create-or-update"), + [ + { + "collection": "test_collection", + "id": 1, + "content": [{"some:key": ["

bar

"]}, 3], + } + ], + format="json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertFalse(PersonalNote.objects.exists()) + def test_delete_other_user(self): user = User.objects.create(username="user") admin_client = APIClient()