Kimirock/django-helpdesk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'main' into feat/copy-followup

Browse Source
This commit is contained in:
Christopher Broderick
2026-10-06 12:39:50 +01:00
committed by GitHub
parent 4bf67418dd 8d98cc6244
commit ae6ec10ffd
13 changed files with 362 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
package.json
View File

@@ -3,11 +3,11 @@
"packageManager": "yarn@4.9.4",
"dependencies": {
"bootstrap": "5.3.8",
"datatables.net": "^3.0.4",
"datatables.net-bs5": "^3.0.4",
"datatables.net-buttons": "^4.0.3",
"datatables.net-buttons-bs5": "^4.0.3",
"datatables.net-buttons-dt": "^4.0.3",
"datatables.net": "^3.1.3",
"datatables.net-bs5": "^3.1.3",
"datatables.net-buttons": "^4.1.2",
"datatables.net-buttons-bs5": "^4.1.2",
"datatables.net-buttons-dt": "^4.1.2",
"fontawesome-free": "npm:@fortawesome/fontawesome-free@^6.7.2",
"jquery": "4.0.0",
"jquery-ui": "1.14.2",

7
src/helpdesk/models.py
View File

@@ -40,6 +40,7 @@ from .lib import (
daily_time_spent_calculation,
format_time_spent,
)
from .sanitize import sanitize_markdown_html
from .templated_email import send_templated_mail
from .validators import validate_file_extension
@@ -78,8 +79,7 @@ def get_markdown(text):
text = text.replace(m.group(0), f"{m.group(1)}({m.group(3)})")
has_illegal_scheme = True
rerun_scheme_check = has_illegal_scheme
return mark_safe(
markdown(
rendered = markdown(
text,
extensions=[
EscapeHtml(),
@@ -87,7 +87,8 @@ def get_markdown(text):
"markdown.extensions.fenced_code",
],
)
)
# The loop above only covers inline links; the output check covers all.
return mark_safe(sanitize_markdown_html(rendered))
class Queue(models.Model):

21
src/helpdesk/sanitize.py
View File

@@ -62,6 +62,27 @@ def sanitize_email_html(html: str) -> str:
return nh3.clean(html, attributes=_allowed_attributes())
def sanitize_markdown_html(html: str) -> str:
"""Enforce ALLOWED_URL_SCHEMES on every link and image of rendered markdown.
Fails closed, like sanitize_email_html(), if nh3 is missing.
"""
if nh3 is None: # pragma: no cover - nh3 is a declared dependency
raise RuntimeError("nh3 is required to render markdown and is not installed.")
attributes = {tag: set(attrs) for tag, attrs in nh3.ALLOWED_ATTRIBUTES.items()}
# Emitted by markdown and dropped by nh3's defaults.
attributes.setdefault("a", set()).add("title")
attributes.setdefault("img", set()).add("title")
attributes.setdefault("code", set()).add("class")
return nh3.clean(
html,
attributes=attributes,
url_schemes=set(helpdesk_settings.ALLOWED_URL_SCHEMES),
# Markdown never emits target="_blank", so rel is not needed.
link_rel=None,
)
def preview_csp() -> str:
"""The Content-Security-Policy sent with a rendered preview.

2
src/helpdesk/templates/helpdesk/include/ticket_followup_list.html
View File

@@ -76,7 +76,7 @@
{% endif %}
{# Ticket comments #}
{% if followup.comment %}
<div class="mt-3">{{ followup.get_markdown|urlizetrunc:50|num_to_link }}</div>
<div class="mt-3">{{ followup.get_markdown|num_to_link }}</div>
{% endif %}
{# Changes to ticket #}
{% if followup.ticketchange_set.all %}

2
src/helpdesk/templates/helpdesk/ticket.html
View File

@@ -32,7 +32,7 @@
</div>
{% else %}
{% with respond_layout=request.user.usersettings_helpdesk.ticket_respond_layout|default:"tabs" %}
{% if respond_layout == "tabs" %}
{% if respond_layout != "top" and respond_layout != "bottom" %}
{# Nav Tabs #}
<div class="card">
<div class="card-header bg-body-tertiary pb-0 d-flex align-items-center">

4
src/helpdesk/templates/helpdesk/ticket_desc_table.html
View File

@@ -45,11 +45,11 @@
</div>
</div>
{# Resolution #}
{{ ticket.get_markdown|urlizetrunc:50|num_to_link }}
{{ ticket.get_markdown|num_to_link }}
{% if ticket.resolution %}
<div class="alert alert-success" role="alert">
<h4 class="alert-heading">{% translate "Resolution" %}</h4>
<p>{{ ticket.get_resolution_markdown|urlizetrunc:50|linebreaksbr }}</p>
<p>{{ ticket.get_resolution_markdown|linebreaksbr }}</p>
<hr />
<p class="mb-0">
{% if "Resolved" == ticket.get_status_display %}

15
src/helpdesk/views/api.py
View File

@@ -67,9 +67,18 @@ class UserTicketViewSet(viewsets.ReadOnlyModelViewSet):
permission_classes: ClassVar[list] = [IsAuthenticated]
def get_queryset(self):
tickets = Ticket.objects.filter(
submitter_email=self.request.user.email
).order_by("-created")
# submitter_email and User.email are both unverified and both default to
# an empty value, so matching one against the other made every account
# without an address the submitter of every ticket without one. An
# address identifies nobody until it is non-empty on both sides.
email = self.request.user.email
if not email:
return Ticket.objects.none()
tickets = (
Ticket.objects.filter(submitter_email=email)
.exclude(submitter_email="")
.order_by("-created")
)
for ticket in tickets:
ticket.set_custom_field_values()
return tickets

2
tests/test_markdown.py
View File

@@ -14,7 +14,7 @@ class MarkDown(SimpleTestCase):
def test_markdown_nl2br(self):
"""warning, after Line 1 - two withespace, else did't work"""
expected_value = "<p>Line 1<br />\n Line 2</p>"
expected_value = "<p>Line 1<br>\n Line 2</p>"
input_value = """Line 1
Line 2"""
output_value = get_markdown(input_value)

78
tests/test_markdown_injection.py Normal file
View File

@@ -0,0 +1,78 @@
"""Rendered markdown must reach the page without gaining event handlers.
On Django 6.0 and later urlize no longer breaks this payload, so only a run on
Django 5.2 fails without the fix.
"""
from html.parser import HTMLParser
from django.contrib.auth import get_user_model
from django.test import TestCase
from django.urls import reverse
from helpdesk.models import FollowUp, Queue, Ticket
# Turns into an onerror attribute if urlize runs over the rendered HTML.
PAYLOAD = '![x](![y" onerror=alert(document.cookie)//](https://example.com/))'
class EventHandlerCollector(HTMLParser):
"""Collects on* attributes as a browser would parse them."""
def __init__(self):
super().__init__()
self.handlers = []
def handle_starttag(self, tag, attrs):
for name, value in attrs:
if name.startswith("on"):
self.handlers.append((tag, name, value))
class MarkdownInjectionTestCase(TestCase):
def setUp(self):
User = get_user_model()
self.user = User.objects.create(
username="staff", is_staff=True, is_superuser=True
)
self.user.set_password("pass")
self.user.save()
self.queue = Queue.objects.create(title="Queue", slug="queue")
self.client.login(username="staff", password="pass")
def handlers_on_ticket_page(self, ticket):
response = self.client.get(
reverse("helpdesk:view", kwargs={"ticket_id": ticket.id})
)
self.assertEqual(response.status_code, 200)
collector = EventHandlerCollector()
collector.feed(response.content.decode())
return collector.handlers
def test_description_cannot_inject_an_event_handler(self):
ticket = Ticket.objects.create(title="t", queue=self.queue, description=PAYLOAD)
self.assertEqual(self.handlers_on_ticket_page(ticket), [])
def test_resolution_cannot_inject_an_event_handler(self):
ticket = Ticket.objects.create(
title="t",
queue=self.queue,
description="d",
resolution=PAYLOAD,
status=Ticket.RESOLVED_STATUS,
)
self.assertEqual(self.handlers_on_ticket_page(ticket), [])
def test_followup_comment_cannot_inject_an_event_handler(self):
ticket = Ticket.objects.create(title="t", queue=self.queue, description="d")
FollowUp.objects.create(ticket=ticket, title="f", comment=PAYLOAD)
self.assertEqual(self.handlers_on_ticket_page(ticket), [])
def test_a_plain_url_is_still_rendered_as_a_link(self):
ticket = Ticket.objects.create(
title="t", queue=self.queue, description="see <https://example.com/x>"
)
response = self.client.get(
reverse("helpdesk:view", kwargs={"ticket_id": ticket.id})
)
self.assertContains(response, 'href="https://example.com/x"')

67
tests/test_markdown_url_schemes.py Normal file
View File

@@ -0,0 +1,67 @@
"""Every URL rendered from markdown must use a scheme from ALLOWED_URL_SCHEMES,
whatever markdown syntax produced it.
"""
from html.parser import HTMLParser
from django.test import TestCase
from helpdesk.models import get_markdown
# Any scheme absent from ALLOWED_URL_SCHEMES; no working payload needed.
FORBIDDEN = "notallowed:"
class UrlCollector(HTMLParser):
def __init__(self):
super().__init__()
self.urls = []
def handle_starttag(self, tag, attrs):
self.urls += [value for name, value in attrs if name in ("href", "src")]
def rendered_urls(text):
collector = UrlCollector()
collector.feed(get_markdown(text))
return collector.urls
class MarkdownUrlSchemeTestCase(TestCase):
def assertNoForbiddenScheme(self, text):
for url in rendered_urls(text):
as_read = "".join(c for c in url if c not in "\t\n\r").strip().lower()
self.assertFalse(
as_read.startswith(FORBIDDEN),
f"{url!r} kept a scheme outside ALLOWED_URL_SCHEMES",
)
def test_inline_link(self):
self.assertNoForbiddenScheme("[t](notallowed:x)")
def test_reference_link(self):
self.assertNoForbiddenScheme("[t][a]\n\n[a]: notallowed:x")
def test_reference_image(self):
self.assertNoForbiddenScheme("![t][a]\n\n[a]: notallowed:x")
def test_entity_encoded_scheme_in_an_inline_link(self):
self.assertNoForbiddenScheme("[t](&#110;otallowed:x)")
self.assertNoForbiddenScheme("[t](not&#9;allowed:x)")
def test_entity_encoded_scheme_in_a_reference(self):
self.assertNoForbiddenScheme("[t][a]\n\n[a]: &#110;otallowed:x")
self.assertNoForbiddenScheme("[t][a]\n\n[a]: not&#10;allowed:x")
def test_allowed_schemes_and_relative_urls_survive(self):
urls = rendered_urls(
"[a](https://example.com/a) [b][ref] ![c](/static/c.png)\n\n"
"[ref]: mailto:someone@example.com"
)
self.assertEqual(
urls,
["https://example.com/a", "mailto:someone@example.com", "/static/c.png"],
)
def test_code_block_language_class_is_kept(self):
self.assertIn('class="language-python"', get_markdown("```python\nx = 1\n```"))

7
tests/test_ticket_respond_layout.py
View File

@@ -75,6 +75,13 @@ class TicketRespondLayoutTests(TestCase):
page = self.get_page()
self.assertIn('id="respond-tab-pane"', page)
def test_unknown_layout_falls_back_to_tabs(self):
"""if a typo in setting ticket_respond_layout fallback to tabs"""
self.set_layout("TYPO")
page = self.get_page()
self.assertIn('id="respond-tab-pane"', page)
self.assertIn('id="followup-tab-pane"', page)
def test_user_settings_form_saves_layout(self):
settings = self.user.usersettings_helpdesk
form = UserSettingsForm(

112
tests/test_user_tickets_api.py Normal file
View File

@@ -0,0 +1,112 @@
"""`/api/user_tickets/` must not treat a missing address as an identity.
The endpoint answers "the tickets submitted by the current user" by matching
`Ticket.submitter_email` against `User.email`. Neither field is verified and
both can be empty, so an account with no address used to match every ticket
with no address. The model default for `submitter_email` is NULL, which does
not collide, but anything created through a form stores "" instead, and that
is the ordinary path.
"""
from django.contrib.auth import get_user_model
from django.test import TestCase
from rest_framework.test import APITestCase
from helpdesk.models import Queue, Ticket
class UserTicketsIdentityTestCase(APITestCase):
def setUp(self):
User = get_user_model()
self.queue = Queue.objects.create(title="Queue", slug="queue")
self.no_address = User.objects.create(username="no-address")
self.owner = User.objects.create(username="owner", email="owner@example.com")
def titles_for(self, user):
self.client.force_authenticate(user)
response = self.client.get("/api/user_tickets/")
self.assertEqual(response.status_code, 200)
body = response.json()
return sorted(t["title"] for t in body.get("results", body))
def test_an_account_without_an_address_gets_nothing(self):
"""Even when tickets carry an empty submitter address, which is what a
form stores when the field is left blank."""
Ticket.objects.create(
title="submitted anonymously through the form",
queue=self.queue,
description="d",
submitter_email="",
)
Ticket.objects.create(
title="created without a submitter at all",
queue=self.queue,
description="d",
)
self.assertEqual(self.titles_for(self.no_address), [])
def test_an_account_with_an_address_still_gets_its_own_tickets(self):
"""The control: the endpoint must keep working for everyone else."""
Ticket.objects.create(
title="mine",
queue=self.queue,
description="d",
submitter_email="owner@example.com",
)
Ticket.objects.create(
title="someone else's",
queue=self.queue,
description="d",
submitter_email="other@example.com",
)
Ticket.objects.create(
title="no submitter", queue=self.queue, description="d", submitter_email=""
)
self.assertEqual(self.titles_for(self.owner), ["mine"])
def test_the_match_stays_case_sensitive(self):
"""Deliberately exact. An address differing only in case identifies a
different mailbox as far as this endpoint can tell, since nothing
verifies either side. Switching to __iexact would look like a usability
fix and would widen the same bug: more accounts matching tickets they
did not submit."""
Ticket.objects.create(
title="lower case submitter",
queue=self.queue,
description="d",
submitter_email="owner@example.com",
)
User = get_user_model()
mixed_case = User.objects.create(
username="mixed-case", email="Owner@Example.com"
)
self.assertEqual(self.titles_for(mixed_case), [])
class SubmitterEmailStorageTestCase(TestCase):
"""Pins the premise of the test above: a blank field is stored as "" rather
than NULL, so the collision is reachable through the ordinary flow rather
than only through code that sets the column explicitly."""
def test_the_submit_form_stores_an_empty_string(self):
User = get_user_model()
queue = Queue.objects.create(
title="Queue", slug="queue", allow_public_submission=True
)
staff = User.objects.create(username="staff", is_staff=True, is_superuser=True)
staff.set_password("pass")
staff.save()
self.client.login(username="staff", password="pass")
self.client.post(
"/tickets/submit/",
{
"queue": queue.id,
"title": "no address given",
"body": "body",
"priority": 3,
"submitter_email": "",
},
follow=True,
)
ticket = Ticket.objects.get(title="no address given")
self.assertEqual(ticket.submitter_email, "")

90
yarn.lock
View File

@@ -14,73 +14,73 @@ __metadata:
languageName: node
linkType: hard
"datatables.net-bs5@npm:^3":
version: 3.0.2
resolution: "datatables.net-bs5@npm:3.0.2"
"datatables.net-bs5@npm:^3.1":
version: 3.1.2
resolution: "datatables.net-bs5@npm:3.1.2"
dependencies:
datatables.net: "npm:3.0.2"
checksum: 10c0/30920f3d75154a7a68d63dfe339327b40bc0fe044b397134243f0117e2c5f83cc3248eeb418b54f531ba577f474f4c39b40da37eb4aa1c0745cd28fb750d11fa
datatables.net: "npm:3.1.2"
checksum: 10c0/2c1db632e04e7af266de832e153bcf635df724dd9d82cbf6b4a08f7a4c56125aee5d9de8b21908757e2a37c16753ce038782d5f9560e67ee009cf44cf568a265
languageName: node
linkType: hard
"datatables.net-bs5@npm:^3.0.4":
version: 3.0.4
resolution: "datatables.net-bs5@npm:3.0.4"
"datatables.net-bs5@npm:^3.1.3":
version: 3.1.3
resolution: "datatables.net-bs5@npm:3.1.3"
dependencies:
datatables.net: "npm:3.0.4"
checksum: 10c0/b532214ee52f367f7448de60e5fdfc20d1df9dd1c9f3a9d4381294f90cea29e8388168ef5fc607af2a62783643dfbd07a937398aaad1c1ff646eac47852b2aa1
datatables.net: "npm:3.1.3"
checksum: 10c0/df064c445558756e8aebe1de05b5765eac96e454d29c1aafbc862b1ad4568bea60f8816d534daba8270b4580bdfb5c2d04c61786d1a7c49a3af3fe01a58e57d3
languageName: node
linkType: hard
"datatables.net-buttons-bs5@npm:^4.0.3":
version: 4.0.3
resolution: "datatables.net-buttons-bs5@npm:4.0.3"
"datatables.net-buttons-bs5@npm:^4.1.2":
version: 4.1.2
resolution: "datatables.net-buttons-bs5@npm:4.1.2"
dependencies:
datatables.net-bs5: "npm:^3"
datatables.net-buttons: "npm:4.0.3"
checksum: 10c0/00e55dd5282966622688b515b797e6777186d8287c233b98166ed7c464ec167540f0fb36e2286d51719cf93464f669a6b52d1b4254b2c729cd0222ef4a1d4b01
datatables.net-bs5: "npm:^3.1"
datatables.net-buttons: "npm:4.1.2"
checksum: 10c0/b633ea1fcfdf053f91bf375da20baaf8612a09d88df397ae2c0f1622a0c7c8688f13c1dc9877affb02c81408327c36058508b3969133dda2b903bfb21ccc07cf
languageName: node
linkType: hard
"datatables.net-buttons-dt@npm:^4.0.3":
version: 4.0.3
resolution: "datatables.net-buttons-dt@npm:4.0.3"
"datatables.net-buttons-dt@npm:^4.1.2":
version: 4.1.2
resolution: "datatables.net-buttons-dt@npm:4.1.2"
dependencies:
datatables.net-buttons: "npm:4.0.3"
datatables.net-dt: "npm:^3"
checksum: 10c0/01aa0c4bbebee0e15a81a273dce658e089702df0e976bf546351d49c334727d5e7e0274ed57615c7d19eab51303f2b7dd1a3422b0d233cba11741535fc905b8c
datatables.net-buttons: "npm:4.1.2"
datatables.net-dt: "npm:^3.1"
checksum: 10c0/d2996a431a4e341618978fdde001745cd4f261b2f0c1b41995c257fbba235583e1bcb1d8f54d2cf55ee34b1a20ef9428d36e4497760fd0e63820e10a0503e0a9
languageName: node
linkType: hard
"datatables.net-buttons@npm:4.0.3, datatables.net-buttons@npm:^4.0.3":
version: 4.0.3
resolution: "datatables.net-buttons@npm:4.0.3"
"datatables.net-buttons@npm:4.1.2, datatables.net-buttons@npm:^4.1.2":
version: 4.1.2
resolution: "datatables.net-buttons@npm:4.1.2"
dependencies:
datatables.net: "npm:^3"
checksum: 10c0/966bc7d89a76308e547dc1de1a5f3a6934baaee95b69c6121c2558c6bae9e50b62d0d1c6e42ff688e9a8b9e644c186359c7d15ccd218fa64483740d1cd91a5ec
datatables.net: "npm:^3.1"
checksum: 10c0/ef4b3e4252a2360132e92f02819133add8e98808151f678c47d9a82c1c2b38ccf49bda57027ebe23aedf64c0a144f35ab48b9502ef8798aa7b2de14fb07de4d9
languageName: node
linkType: hard
"datatables.net-dt@npm:^3":
version: 3.0.2
resolution: "datatables.net-dt@npm:3.0.2"
"datatables.net-dt@npm:^3.1":
version: 3.1.2
resolution: "datatables.net-dt@npm:3.1.2"
dependencies:
datatables.net: "npm:3.0.2"
checksum: 10c0/e49d194e32b43c91a329852559d026a18b02792de8990231958b675a7bf93020b404cd1aca0757847936ebe323f65b3bc8c733a6e6b0d3338adea1358b82c717
datatables.net: "npm:3.1.2"
checksum: 10c0/3bc552a95dad7275b59ef065c33c5abeadcdbe21c45b3adc08ea1314f245b35dcd77c1a1453fe46af177cba297f7ab64f3c552017b7f7833e36444e89b1b9700
languageName: node
linkType: hard
"datatables.net@npm:3.0.2, datatables.net@npm:^3":
version: 3.0.2
resolution: "datatables.net@npm:3.0.2"
checksum: 10c0/7a58f9c07b8f652afbf200bd6bfea7f1653c94d6a938ad318e72e5b8300273aa4cd4a1adc8428d5f5a0f0252ce8dba7c3e31c9241b64bfe41f899240e0af6f4d
"datatables.net@npm:3.1.2, datatables.net@npm:^3.1":
version: 3.1.2
resolution: "datatables.net@npm:3.1.2"
checksum: 10c0/f63d420b5dd56e0f32b88f1704e56ebf5cf8039d6d7b3a19863d4c2bfd463f8bc2091cfa1b6a4dc7340521dde61b04e6fb1c40068c53c6d328b1681bd5bef095
languageName: node
linkType: hard
"datatables.net@npm:3.0.4, datatables.net@npm:^3.0.4":
version: 3.0.4
resolution: "datatables.net@npm:3.0.4"
checksum: 10c0/2e54778f25f4fa68f8d0fdfa387a0761788b458bee8d6e4f9291a58760531a375791f5795e99d587613ca7be61f11e2d266055547dc08f26e60e6445161bc3ec
"datatables.net@npm:3.1.3, datatables.net@npm:^3.1.3":
version: 3.1.3
resolution: "datatables.net@npm:3.1.3"
checksum: 10c0/0166cb0275211392a6b00367141726de30be7d6dadf0da1ff88c66b09129ee71936b515f3851ed865aa82ed8c16ace38915d5be5065c040425b35c2cf14890ce
languageName: node
linkType: hard
@@ -89,11 +89,11 @@ __metadata:
resolution: "django-helpdesk@workspace:."
dependencies:
bootstrap: "npm:5.3.8"
datatables.net: "npm:^3.0.4"
datatables.net-bs5: "npm:^3.0.4"
datatables.net-buttons: "npm:^4.0.3"
datatables.net-buttons-bs5: "npm:^4.0.3"
datatables.net-buttons-dt: "npm:^4.0.3"
datatables.net: "npm:^3.1.3"
datatables.net-bs5: "npm:^3.1.3"
datatables.net-buttons: "npm:^4.1.2"
datatables.net-buttons-bs5: "npm:^4.1.2"
datatables.net-buttons-dt: "npm:^4.1.2"
fontawesome-free: "npm:@fortawesome/fontawesome-free@^6.7.2"
jquery: "npm:4.0.0"
jquery-ui: "npm:1.14.2"