Skip to content

Commit 66b68a8

Browse files
zubeydecivelekzzacharo
authored andcommitted
form: HTML sanitization and remove Source from CKEditor
1 parent dd6e0b6 commit 66b68a8

9 files changed

Lines changed: 105 additions & 34 deletions

File tree

cds/modules/deposit/static/json/cds_deposit/forms/project.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,6 @@
4545
"Replace",
4646
"-",
4747
"RemoveFormat"
48-
],
49-
[
50-
"Source"
5148
]
5249
],
5350
"disableNativeSpellChecker": false,

cds/modules/deposit/static/json/cds_deposit/forms/video.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,6 @@
4444
"Replace",
4545
"-",
4646
"RemoveFormat"
47-
],
48-
[
49-
"Source"
5047
]
5148
],
5249
"disableNativeSpellChecker": false,

cds/modules/records/serializers/json.py

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
has_read_record_permission,
3333
)
3434
from ..utils import HTMLTagRemover, remove_html_tags
35+
from marshmallow_utils.html import sanitize_html
3536

3637

3738
class CDSJSONSerializer(JSONSerializer):
@@ -46,6 +47,41 @@ def dump(self, obj, context=None):
4647
"""Serialize object with schema."""
4748
return self.schema_class(context=context).dump(obj)
4849

50+
def _sanitize_metadata(self, metadata):
51+
"""Sanitize title, description and translations in metadata."""
52+
try:
53+
if "title" in metadata and "title" in metadata["title"]:
54+
title = metadata["title"]["title"]
55+
title = self.html_tag_remover.unescape(title)
56+
metadata["title"]["title"] = remove_html_tags(
57+
self.html_tag_remover, title
58+
)
59+
60+
if "description" in metadata:
61+
description = metadata["description"]
62+
description = self.html_tag_remover.unescape(description)
63+
metadata["description"] = sanitize_html(description)
64+
65+
if "translations" in metadata:
66+
for t in metadata["translations"]:
67+
if "title" in t and "title" in t["title"]:
68+
t_title = t["title"]["title"]
69+
t_title = self.html_tag_remover.unescape(t_title)
70+
t["title"]["title"] = remove_html_tags(
71+
self.html_tag_remover, t_title
72+
)
73+
74+
if "description" in t:
75+
t_desc = t["description"]
76+
t_desc = self.html_tag_remover.unescape(t_desc)
77+
t["description"] = sanitize_html(t_desc)
78+
79+
except KeyError:
80+
# ignore error if keys are missing
81+
pass
82+
83+
return metadata
84+
4985
def preprocess_record(self, pid, record, links_factory=None):
5086
"""Include ``_eos_library_path`` for single record retrievals."""
5187
result = super(CDSJSONSerializer, self).preprocess_record(
@@ -62,16 +98,7 @@ def preprocess_record(self, pid, record, links_factory=None):
6298

6399
# sanitize title by unescaping and stripping html tags
64100
try:
65-
title = metadata["title"]["title"]
66-
title = self.html_tag_remover.unescape(title)
67-
metadata["title"]["title"] = remove_html_tags(
68-
self.html_tag_remover, title
69-
)
70-
71-
# decode html entities
72-
metadata["description"] = self.html_tag_remover.unescape(
73-
metadata["description"]
74-
)
101+
metadata = self._sanitize_metadata(metadata)
75102
if has_request_context():
76103
metadata["videos"] = [
77104
video
@@ -93,19 +120,6 @@ def preprocess_search_hit(self, pid, record_hit, links_factory=None):
93120

94121
if "metadata" in result:
95122
metadata = result["metadata"]
96-
97-
try:
98-
title = metadata["title"]["title"]
99-
title = self.html_tag_remover.unescape(title)
100-
metadata["title"]["title"] = remove_html_tags(
101-
self.html_tag_remover, title
102-
)
103-
104-
metadata["description"] = self.html_tag_remover.unescape(
105-
metadata["description"]
106-
)
107-
except KeyError:
108-
# ignore error if keys are missing in the metadata
109-
pass
123+
result["metadata"] = self._sanitize_metadata(result["metadata"])
110124

111125
return result

cds/modules/records/serializers/schemas/common.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
from marshmallow import RAISE, Schema, ValidationError, fields, validates_schema
2323
from marshmallow.validate import Length
24+
from marshmallow_utils.fields import SanitizedHTML
2425

2526
from ...api import Keyword
2627
from ...resolver import keyword_resolver
@@ -140,7 +141,7 @@ class TranslationsSchema(StrictKeysSchema):
140141
"""Translations schema."""
141142

142143
title = fields.Nested(TitleSchema)
143-
description = fields.Str()
144+
description = SanitizedHTML()
144145
language = fields.Str()
145146

146147

cds/modules/records/serializers/schemas/project.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
from invenio_jsonschemas import current_jsonschemas
2222
from marshmallow import Schema, fields, pre_load, post_load
23+
from marshmallow_utils.fields import SanitizedHTML
2324

2425
from ....deposit.api import Project, deposit_video_resolver
2526
from .common import (
@@ -76,7 +77,7 @@ class ProjectSchema(StrictKeysSchema):
7677
_deposit = fields.Nested(ProjectDepositSchema, required=True)
7778
_cds = fields.Nested(_CDSSSchema, required=True)
7879
title = fields.Nested(TitleSchema, required=True)
79-
description = fields.Str()
80+
description = SanitizedHTML()
8081
category = fields.Str(required=True)
8182
type = fields.Str(required=True)
8283
note = fields.Str()

cds/modules/records/serializers/schemas/video.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
from invenio_jsonschemas import current_jsonschemas
2222
from marshmallow import Schema, fields, pre_load, post_load
23-
23+
from marshmallow_utils.fields import SanitizedHTML
2424
from ....deposit.api import Video
2525
from ..fields.datetime import DateString
2626
from .common import (
@@ -126,7 +126,7 @@ class VideoSchema(StrictKeysSchema):
126126
contributors = fields.Nested(ContributorSchema, many=True, required=True)
127127
copyright = fields.Nested(CopyrightSchema)
128128
date = DateString(required=True)
129-
description = fields.Str(required=True)
129+
description = SanitizedHTML(required=True)
130130
doi = DOI()
131131
duration = fields.Str()
132132
external_system_identifiers = fields.Nested(

requirements.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ lxml_html_clean==0.4.1
134134
Mako==1.3.8
135135
MarkupSafe==3.0.2
136136
marshmallow==3.23.1
137+
marshmallow-utils==0.13.0
137138
matplotlib-inline==0.1.7
138139
maxminddb==2.6.2
139140
maxminddb-geolite2==2018.703

setup.cfg

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ install_requires =
116116
invenio-sequencegenerator==1.0.0a3
117117
requests-toolbelt>=1.0.0,<2.0.0
118118
python-ldap>=3.4.0,<3.5.0
119+
marshmallow-utils>=0.13.0,<1.0.0
119120

120121
[options.extras_require]
121122
tests =

tests/unit/test_serializer.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929

3030
from cds.modules.deposit.api import Video
3131
from cds.modules.records.serializers.drupal import VideoDrupal
32+
from cds.modules.records.serializers.json import CDSJSONSerializer
33+
from cds.modules.records.api import CDSRecord
34+
from unittest.mock import Mock
3235
from cds.modules.records.serializers.smil import Smil
3336
from cds.modules.records.serializers.vtt import VTT
3437

@@ -149,3 +152,59 @@ def test_drupal_serializer(video_record_metadata, deposit_metadata):
149152
data = serializer.format()["entries"][0]["entry"]
150153
data = {k: data[k] for k in data if k in expected}
151154
assert data == expected
155+
156+
157+
def test_cds_json_serializer_sanitization(video_record_metadata):
158+
"""Test HTML sanitization in CDSJSONSerializer."""
159+
record = CDSRecord.create(video_record_metadata)
160+
161+
# Add malicious HTML
162+
record['description'] = '<script>alert("xss")</script>Safe content <b>bold</b>'
163+
record['title']['title'] = 'Test <script>alert("title")</script> Title <b>bold</b>'
164+
record['translations'] = [
165+
{
166+
'language': 'en',
167+
'description': '<script>alert("desc")</script>Translated <i>italic</i>',
168+
'title': {'title': '<b>Translated</b> <script>alert("title")</script> Title'}
169+
},
170+
{
171+
'language': 'fr',
172+
'description': 'Bonjour <script>alert("desc")</script> <u>underline</u>',
173+
'title': {'title': '<script>alert("bad")</script> Titre'}
174+
}
175+
]
176+
177+
# Test the serializer
178+
serializer = CDSJSONSerializer()
179+
180+
# Create a mock PID (required by the serializer)
181+
mock_pid = Mock()
182+
mock_pid.pid_value = '1'
183+
184+
# Test preprocess_record method
185+
result = serializer.preprocess_record(mock_pid, record)
186+
187+
# Check sanitization
188+
description = result['metadata']['description']
189+
assert '<script>' not in description
190+
assert '</script>' not in description
191+
assert 'Safe content' in description
192+
# Keep safe HTML tags like <b>
193+
assert '<b>bold</b>' in description
194+
195+
# Remove everything in title
196+
title = result['metadata']['title']['title']
197+
assert '<script>' not in title
198+
assert '</script>' not in title
199+
assert 'Test' in title and 'Title' in title
200+
assert '<b>' not in title
201+
202+
# --- Translations checks ---
203+
translations = result['metadata']['translations']
204+
for tr in translations:
205+
# description
206+
assert '<script>' not in tr['description']
207+
# title
208+
assert '<script>' not in tr['title']['title']
209+
assert '<b>' not in tr['title']['title']
210+

0 commit comments

Comments
 (0)