Require a secret key for viewing tickets unless HELPDESK_VIEW_A_TICKET_PUBLIC is set

Fixes #629, #639

.gitignore

@@ -10,6 +10,7 @@ docs/doctrees/*
 .project
 .pydevproject
 .directory
+*.swp
 
 # ignore demo attachments that user might have added
 helpdesk/attachments/

helpdesk/migrations/0018_ticket_secret_key.py

@@ -0,0 +1,28 @@
+# Generated by Django 2.0.1 on 2018-09-07 21:22
+
+from django.db import migrations, models
+import helpdesk.models
+
+
+def clear_secret_keys(apps, schema_editor):
+    Ticket = apps.get_model("helpdesk", "Ticket")
+    db_alias = schema_editor.connection.alias
+
+    for ticket in Ticket.objects.using(db_alias).all():
+         ticket.secret_key=''
+         ticket.save()
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('helpdesk', '0017_default_owner_on_delete_null'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='ticket',
+            name='secret_key',
+            field=models.CharField(default=helpdesk.models.mk_secret, max_length=36, null=True, verbose_name='Secret key needed for viewing/editing ticket by non-logged in users'),
+        ),
+        migrations.RunPython(clear_secret_keys),
+    ]

helpdesk/migrations/0019_ticket_secret_key.py

@@ -0,0 +1,19 @@
+# Generated by Django 2.0.1 on 2018-09-07 21:22
+
+from django.db import migrations, models
+import helpdesk.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('helpdesk', '0018_ticket_secret_key'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='ticket',
+            name='secret_key',
+            field=models.CharField(default=helpdesk.models.mk_secret, max_length=36, verbose_name='Secret key needed for viewing/editing ticket by non-logged in users'),
+        ),
+    ]

helpdesk/models.py

@@ -21,6 +21,7 @@ from django.utils.encoding import python_2_unicode_compatible
 import re
 
 import six
+import uuid
 
 
 @python_2_unicode_compatible
@@ -351,6 +352,10 @@ class Queue(models.Model):
                 pass
 
 
+def mk_secret():
+    return str(uuid.uuid4())
+
+
 @python_2_unicode_compatible
 class Ticket(models.Model):
     """
@@ -480,6 +485,12 @@ class Ticket(models.Model):
                     'automatically by management/commands/escalate_tickets.py.'),
     )
 
+    secret_key = models.CharField(
+        _("Secret key needed for viewing/editing ticket by non-logged in users"),
+        max_length=36,
+        default=mk_secret,
+    )
+
     def _get_assigned_to(self):
         """ Custom property to allow us to easily print 'Unassigned' if a
         ticket has no owner, or the users name if it's assigned. If the user
@@ -544,11 +555,12 @@ class Ticket(models.Model):
             site = Site.objects.get_current()
         except ImproperlyConfigured:
             site = Site(domain='configure-django-sites.com')
-        return u"http://%s%s?ticket=%s&email=%s" % (
+        return u"http://%s%s?ticket=%s&email=%s&key=%s" % (
             site.domain,
             reverse('helpdesk:public_view'),
             self.ticket_for_url,
-            self.submitter_email
+            self.submitter_email,
+            self.secret_key
         )
     ticket_url = property(_get_ticket_url)
 

helpdesk/tests/test_public_actions.py

@@ -29,12 +29,21 @@ class PublicActionsTestCase(TestCase):
         self.client = Client()
 
     def test_public_view_ticket(self):
+        # Without key, we get 403
         response = self.client.get('%s?ticket=%s&email=%s' % (
             reverse('helpdesk:public_view'),
             self.ticket.ticket_for_url,
             'test.submitter@example.com'))
-        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.status_code, 403)
         self.assertTemplateNotUsed(response, 'helpdesk/public_view_form.html')
+        # With a key it works
+        response = self.client.get('%s?ticket=%s&email=%s&key=%s' % (
+            reverse('helpdesk:public_view'),
+            self.ticket.ticket_for_url,
+            'test.submitter@example.com',
+            self.ticket.secret_key))
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'helpdesk/public_view_ticket.html')
 
     def test_public_close(self):
         old_status = self.ticket.status
@@ -49,10 +58,11 @@ class PublicActionsTestCase(TestCase):
 
         current_followups = ticket.followup_set.all().count()
 
-        response = self.client.get('%s?ticket=%s&email=%s&close' % (
+        response = self.client.get('%s?ticket=%s&email=%s&close&key=%s' % (
             reverse('helpdesk:public_view'),
             ticket.ticket_for_url,
-            'test.submitter@example.com'))
+            'test.submitter@example.com',
+            ticket.secret_key))
 
         ticket = Ticket.objects.get(id=self.ticket.id)
 

helpdesk/tests/test_ticket_lookup.py

@@ -2,9 +2,13 @@
 from django.urls import reverse
 from django.test import TestCase
 from helpdesk.models import Ticket, Queue
+from django.test.utils import override_settings
 
 
-class TestKBDisabled(TestCase):
+@override_settings(
+    HELPDESK_VIEW_A_TICKET_PUBLIC=True
+)
+class TestTicketLookupPublicEnabled(TestCase):
     def setUp(self):
         q = Queue(title='Q1', slug='q1')
         q.save()

helpdesk/views/public.py

@@ -6,7 +6,7 @@ django-helpdesk - A Django powered ticket tracker for small enterprise.
 views/public.py - All public facing views, eg non-staff (no authentication
                   required) views.
 """
-from django.core.exceptions import ObjectDoesNotExist
+from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
 try:
     # Django 2.0+
     from django.urls import reverse
@@ -18,6 +18,7 @@ from django.shortcuts import render
 from django.utils.http import urlquote
 from django.utils.translation import ugettext as _
 from django.conf import settings
+from django.views.generic.base import TemplateView
 from django.views.generic.edit import FormView
 
 from helpdesk import settings as helpdesk_settings
@@ -98,10 +99,11 @@ class CreateTicketView(FormView):
         else:
             ticket = form.save()
             try:
-                return HttpResponseRedirect('%s?ticket=%s&email=%s' % (
+                return HttpResponseRedirect('%s?ticket=%s&email=%s&key=%s' % (
                     reverse('helpdesk:public_view'),
                     ticket.ticket_for_url,
-                    urlquote(ticket.submitter_email))
+                    urlquote(ticket.submitter_email),
+                    ticket.secret_key)
                 )
             except ValueError:
                 # if someone enters a non-int string for the ticket
@@ -115,62 +117,71 @@ class Homepage(CreateTicketView):
     template_name = 'helpdesk/public_homepage.html'
 
 
+def search_for_ticket(request, error_message=None):
+    if hasattr(settings, 'HELPDESK_VIEW_A_TICKET_PUBLIC') and settings.HELPDESK_VIEW_A_TICKET_PUBLIC:
+        email = request.GET.get('email', None)
+        return render(request, 'helpdesk/public_view_form.html', {
+            'ticket': False,
+            'email': email,
+            'error_message': error_message,
+            'helpdesk_settings': helpdesk_settings,
+        })
+    else:
+        raise PermissionDenied("Public viewing of tickets without a secret key is forbidden.")
+
+
 @protect_view
 def view_ticket(request):
     ticket_req = request.GET.get('ticket', None)
     email = request.GET.get('email', None)
+    key = request.GET.get('key', '')
 
-    if ticket_req and email:
+    if not (ticket_req and email):
-        queue, ticket_id = Ticket.queue_and_id_from_query(ticket_req)
+        if ticket_req is None and email is None:
-        try:
+            return search_for_ticket(request)
-            ticket = Ticket.objects.get(id=ticket_id, submitter_email__iexact=email)
-        except ObjectDoesNotExist:
-            error_message = _('Invalid ticket ID or e-mail address. Please try again.')
-        except ValueError:
-            error_message = _('Invalid ticket ID or e-mail address. Please try again.')
         else:
-            if is_helpdesk_staff(request.user):
+            return search_for_ticket(request, _('Missing ticket ID or e-mail address. Please try again.'))
-                redirect_url = reverse('helpdesk:view', args=[ticket_id])
-                if 'close' in request.GET:
-                    redirect_url += '?close'
-                return HttpResponseRedirect(redirect_url)
 
-            if 'close' in request.GET and ticket.status == Ticket.RESOLVED_STATUS:
+    queue, ticket_id = Ticket.queue_and_id_from_query(ticket_req)
-                from helpdesk.views.staff import update_ticket
+    try:
-                # Trick the update_ticket() view into thinking it's being called with
+        if hasattr(settings, 'HELPDESK_VIEW_A_TICKET_PUBLIC') and settings.HELPDESK_VIEW_A_TICKET_PUBLIC:
-                # a valid POST.
+            ticket = Ticket.objects.get(id=ticket_id, submitter_email__iexact=email)
-                request.POST = {
+        else:
-                    'new_status': Ticket.CLOSED_STATUS,
+            ticket = Ticket.objects.get(id=ticket_id, submitter_email__iexact=email, secret_key__iexact=key)
-                    'public': 1,
+    except (ObjectDoesNotExist, ValueError):
-                    'title': ticket.title,
+        return search_for_ticket(request, _('Invalid ticket ID or e-mail address. Please try again.'))
-                    'comment': _('Submitter accepted resolution and closed ticket'),
-                }
-                if ticket.assigned_to:
-                    request.POST['owner'] = ticket.assigned_to.id
-                request.GET = {}
 
-                return update_ticket(request, ticket_id, public=True)
+    if is_helpdesk_staff(request.user):
+        redirect_url = reverse('helpdesk:view', args=[ticket_id])
+        if 'close' in request.GET:
+            redirect_url += '?close'
+        return HttpResponseRedirect(redirect_url)
 
-            # redirect user back to this ticket if possible.
+    if 'close' in request.GET and ticket.status == Ticket.RESOLVED_STATUS:
-            redirect_url = ''
+        from helpdesk.views.staff import update_ticket
-            if helpdesk_settings.HELPDESK_NAVIGATION_ENABLED:
+        # Trick the update_ticket() view into thinking it's being called with
-                redirect_url = reverse('helpdesk:view', args=[ticket_id])
+        # a valid POST.
+        request.POST = {
+            'new_status': Ticket.CLOSED_STATUS,
+            'public': 1,
+            'title': ticket.title,
+            'comment': _('Submitter accepted resolution and closed ticket'),
+        }
+        if ticket.assigned_to:
+            request.POST['owner'] = ticket.assigned_to.id
+        request.GET = {}
 
-            return render(request, 'helpdesk/public_view_ticket.html', {
+        return update_ticket(request, ticket_id, public=True)
-                'ticket': ticket,
-                'helpdesk_settings': helpdesk_settings,
-                'next': redirect_url,
-            })
-    elif ticket_req is None and email is None:
-        error_message = None
-    else:
-        error_message = _('Missing ticket ID or e-mail address. Please try again.')
 
-    return render(request, 'helpdesk/public_view_form.html', {
+    # redirect user back to this ticket if possible.
-        'ticket': False,
+    redirect_url = ''
-        'email': email,
+    if helpdesk_settings.HELPDESK_NAVIGATION_ENABLED:
-        'error_message': error_message,
+        redirect_url = reverse('helpdesk:view', args=[ticket_id])
+
+    return render(request, 'helpdesk/public_view_ticket.html', {
+        'ticket': ticket,
         'helpdesk_settings': helpdesk_settings,
+        'next': redirect_url,
     })