Merge pull request #71 from regisb/regisb/fix-escaped-admin-links

Fix wrongly escaped links in admin view lists

Author: robrap

CHANGELOG.rst

@@ -14,6 +14,10 @@ Change Log
 Unreleased
 ~~~~~~~~~~
 
+[2.0.2] - 2020-05-10
+~~~~~~~~~~~~~~~~~~~~
+* Fix html escaping of edit links in admin
+
 [2.0.1] - 2020-05-08
 ~~~~~~~~~~~~~~~~~~~~
 * Dropped support for Django<2.2

config_models/__init__.py

@@ -4,6 +4,6 @@
 
 from __future__ import absolute_import, unicode_literals
 
-__version__ = '2.0.1'
+__version__ = '2.0.2'
 
 default_app_config = 'config_models.apps.ConfigModelsConfig'  # pylint: disable=invalid-name

config_models/admin.py

@@ -11,6 +11,7 @@
 from django.urls import reverse
 from django.http import HttpResponseRedirect
 from django.shortcuts import get_object_or_404
+from django.utils.html import format_html
 from django.utils.translation import ugettext_lazy as _
 
 try:
@@ -208,6 +209,5 @@ def edit_link(self, inst):
             return u'--'
         update_url = reverse('admin:{}_{}_add'.format(self.model._meta.app_label, self.model._meta.model_name))
         update_url += "?source={}".format(inst.pk)
-        return u'<a href="{}">{}</a>'.format(update_url, _('Update'))
-    edit_link.allow_tags = True
+        return format_html(u'<a href="{}">{}</a>', update_url, _('Update'))
     edit_link.short_description = _('Update')

tests/test_admin.py

@@ -4,31 +4,96 @@
 
 from __future__ import unicode_literals
 
+from unittest.mock import patch
+
 from django.contrib.admin.sites import AdminSite
+from django.contrib.auth.models import User
 from django.contrib.messages.storage.fallback import FallbackStorage
 from django.http import HttpRequest
 from django.test import TestCase
 
-from config_models.admin import ConfigurationModelAdmin
+from config_models import admin
 from config_models.models import ConfigurationModel
 
+from example.models import ExampleKeyedConfig
+
+
+class AdminTestCaseMixin:
+    """
+    Provide a request factory method.
+    """
+
+    def get_request(self):
+        request = HttpRequest()
+        request.session = "session"
+        request._messages = FallbackStorage(request)  # pylint: disable=protected-access
+        return request
 
-class AdminTestCase(TestCase):
+
+class AdminTestCase(TestCase, AdminTestCaseMixin):
     """
     Test Case module for ConfigurationModel Admin
     """
+
     def setUp(self):
         super(AdminTestCase, self).setUp()
-        self.request = HttpRequest()
-        self.conf_admin = ConfigurationModelAdmin(ConfigurationModel, AdminSite())
-        self.request.session = 'session'
-        self.request._messages = FallbackStorage(self.request)  # pylint: disable=protected-access
+        self.conf_admin = admin.ConfigurationModelAdmin(ConfigurationModel, AdminSite())
 
     def test_default_fields(self):
         """
         Test: checking fields
         """
+        request = self.get_request()
         self.assertEqual(
-            list(self.conf_admin.get_form(self.request).base_fields),
-            ['enabled']
+            list(self.conf_admin.get_form(request).base_fields), ["enabled"]
+        )
+
+
+class KeyedAdminTestCase(TestCase, AdminTestCaseMixin):
+    """
+    Test case module for KeyedConfigurationModelAdmin.
+    """
+
+    def get_edit_link(self):
+        """
+        Return an edit link from a KeyedConfigurationModelAdmin. Patch the `reverse`
+        and `_` methods to modify the return value.
+        """
+        conf_admin = admin.KeyedConfigurationModelAdmin(ExampleKeyedConfig, AdminSite())
+        request = self.get_request()
+        ExampleKeyedConfig.objects.create(user=User.objects.create())
+        config = conf_admin.get_queryset(request)[0]
+        return conf_admin.edit_link(config)
+
+    def test_edit_link(self):
+        with patch.object(admin, "reverse", return_value="http://google.com"):
+            self.assertEqual(
+                '<a href="http://google.com?source=1">Update</a>', self.get_edit_link(),
+            )
+
+    def test_edit_link_xss_url(self):
+        with patch.object(
+            admin, "reverse", return_value='"><script>console.log("xss!")</script>'
+        ):
+            edit_link = self.get_edit_link()
+
+        self.assertNotIn(
+            "<script>", edit_link,
+        )
+        self.assertIn(
+            "&lt;script&gt;", edit_link,
+        )
+
+    def test_edit_link_xss_translation(self):
+        with patch.object(admin, "reverse", return_value=""):
+            with patch.object(
+                admin, "_", return_value='<script>console.log("xss!")</script>'
+            ):
+                edit_link = self.get_edit_link()
+
+        self.assertNotIn(
+            "<script>", edit_link,
+        )
+        self.assertIn(
+            "&lt;script&gt;", edit_link,
         )