Skip to content

Commit 380b12e

Browse files
committed
Fix wrongly escaped links in admin view lists
The `allow_tags` attribute is deprecated in Django 2.0: https://django.readthedocs.io/en/2.0.x/internals/deprecation.html As a consequence, "Update" edit links are no longer clickable for admin models that inherit from ConfigurationModelAdmin. This will close CRI-189 once a new release of django-config-models is produced and the dependency is upgraded in edx-platform. We take the opportunity to bump the version number to v2.0.2.
1 parent 1f2a8cf commit 380b12e

File tree

4 files changed

+80
-11
lines changed

4 files changed

+80
-11
lines changed

CHANGELOG.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ Change Log
1414
Unreleased
1515
~~~~~~~~~~
1616

17+
[2.0.2] - 2020-05-10
18+
~~~~~~~~~~~~~~~~~~~~
19+
* Fix html escaping of edit links in admin
20+
1721
[2.0.1] - 2020-05-08
1822
~~~~~~~~~~~~~~~~~~~~
1923
* Dropped support for Django<2.2

config_models/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@
44

55
from __future__ import absolute_import, unicode_literals
66

7-
__version__ = '2.0.1'
7+
__version__ = '2.0.2'
88

99
default_app_config = 'config_models.apps.ConfigModelsConfig' # pylint: disable=invalid-name

config_models/admin.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from django.urls import reverse
1212
from django.http import HttpResponseRedirect
1313
from django.shortcuts import get_object_or_404
14+
from django.utils.html import format_html
1415
from django.utils.translation import ugettext_lazy as _
1516

1617
try:
@@ -208,6 +209,5 @@ def edit_link(self, inst):
208209
return u'--'
209210
update_url = reverse('admin:{}_{}_add'.format(self.model._meta.app_label, self.model._meta.model_name))
210211
update_url += "?source={}".format(inst.pk)
211-
return u'<a href="{}">{}</a>'.format(update_url, _('Update'))
212-
edit_link.allow_tags = True
212+
return format_html(u'<a href="{}">{}</a>', update_url, _('Update'))
213213
edit_link.short_description = _('Update')

tests/test_admin.py

Lines changed: 73 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,96 @@
44

55
from __future__ import unicode_literals
66

7+
from unittest.mock import patch
8+
79
from django.contrib.admin.sites import AdminSite
10+
from django.contrib.auth.models import User
811
from django.contrib.messages.storage.fallback import FallbackStorage
912
from django.http import HttpRequest
1013
from django.test import TestCase
1114

12-
from config_models.admin import ConfigurationModelAdmin
15+
from config_models import admin
1316
from config_models.models import ConfigurationModel
1417

18+
from example.models import ExampleKeyedConfig
19+
20+
21+
class AdminTestCaseMixin:
22+
"""
23+
Provide a request factory method.
24+
"""
25+
26+
def get_request(self):
27+
request = HttpRequest()
28+
request.session = "session"
29+
request._messages = FallbackStorage(request) # pylint: disable=protected-access
30+
return request
1531

16-
class AdminTestCase(TestCase):
32+
33+
class AdminTestCase(TestCase, AdminTestCaseMixin):
1734
"""
1835
Test Case module for ConfigurationModel Admin
1936
"""
37+
2038
def setUp(self):
2139
super(AdminTestCase, self).setUp()
22-
self.request = HttpRequest()
23-
self.conf_admin = ConfigurationModelAdmin(ConfigurationModel, AdminSite())
24-
self.request.session = 'session'
25-
self.request._messages = FallbackStorage(self.request) # pylint: disable=protected-access
40+
self.conf_admin = admin.ConfigurationModelAdmin(ConfigurationModel, AdminSite())
2641

2742
def test_default_fields(self):
2843
"""
2944
Test: checking fields
3045
"""
46+
request = self.get_request()
3147
self.assertEqual(
32-
list(self.conf_admin.get_form(self.request).base_fields),
33-
['enabled']
48+
list(self.conf_admin.get_form(request).base_fields), ["enabled"]
49+
)
50+
51+
52+
class KeyedAdminTestCase(TestCase, AdminTestCaseMixin):
53+
"""
54+
Test case module for KeyedConfigurationModelAdmin.
55+
"""
56+
57+
def get_edit_link(self):
58+
"""
59+
Return an edit link from a KeyedConfigurationModelAdmin. Patch the `reverse`
60+
and `_` methods to modify the return value.
61+
"""
62+
conf_admin = admin.KeyedConfigurationModelAdmin(ExampleKeyedConfig, AdminSite())
63+
request = self.get_request()
64+
ExampleKeyedConfig.objects.create(user=User.objects.create())
65+
config = conf_admin.get_queryset(request)[0]
66+
return conf_admin.edit_link(config)
67+
68+
def test_edit_link(self):
69+
with patch.object(admin, "reverse", return_value="http://google.com"):
70+
self.assertEqual(
71+
'<a href="http://google.com?source=1">Update</a>', self.get_edit_link(),
72+
)
73+
74+
def test_edit_link_xss_url(self):
75+
with patch.object(
76+
admin, "reverse", return_value='"><script>console.log("xss!")</script>'
77+
):
78+
edit_link = self.get_edit_link()
79+
80+
self.assertNotIn(
81+
"<script>", edit_link,
82+
)
83+
self.assertIn(
84+
"&lt;script&gt;", edit_link,
85+
)
86+
87+
def test_edit_link_xss_translation(self):
88+
with patch.object(admin, "reverse", return_value=""):
89+
with patch.object(
90+
admin, "_", return_value='<script>console.log("xss!")</script>'
91+
):
92+
edit_link = self.get_edit_link()
93+
94+
self.assertNotIn(
95+
"<script>", edit_link,
96+
)
97+
self.assertIn(
98+
"&lt;script&gt;", edit_link,
3499
)

0 commit comments

Comments
 (0)