From c1750a74612b0e2eb2a1eff0ae6dd3cf131ac0a1 Mon Sep 17 00:00:00 2001 From: Timothy Hobbs Date: Sat, 8 Sep 2018 20:36:35 +0200 Subject: [PATCH] Require a secret key for viewing tickets unless HELPDESK_VIEW_A_TICKET_PUBLIC is set Fixes #629, #639 --- .gitignore | 1 + helpdesk/migrations/0018_ticket_secret_key.py | 28 +++++ helpdesk/migrations/0019_ticket_secret_key.py | 19 ++++ helpdesk/models.py | 16 ++- helpdesk/tests/test_public_actions.py | 16 ++- helpdesk/tests/test_ticket_lookup.py | 6 +- helpdesk/views/public.py | 105 ++++++++++-------- 7 files changed, 138 insertions(+), 53 deletions(-) create mode 100644 helpdesk/migrations/0018_ticket_secret_key.py create mode 100644 helpdesk/migrations/0019_ticket_secret_key.py diff --git a/.gitignore b/.gitignore index d5885822..9e6dab0f 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ docs/doctrees/* .project .pydevproject .directory +*.swp # ignore demo attachments that user might have added helpdesk/attachments/ diff --git a/helpdesk/migrations/0018_ticket_secret_key.py b/helpdesk/migrations/0018_ticket_secret_key.py new file mode 100644 index 00000000..f3b2f4ee --- /dev/null +++ b/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), + ] diff --git a/helpdesk/migrations/0019_ticket_secret_key.py b/helpdesk/migrations/0019_ticket_secret_key.py new file mode 100644 index 00000000..be1dd385 --- /dev/null +++ b/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'), + ), + ] diff --git a/helpdesk/models.py b/helpdesk/models.py index 9b0b822a..cb80219d 100644 --- a/helpdesk/models.py +++ b/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) diff --git a/helpdesk/tests/test_public_actions.py b/helpdesk/tests/test_public_actions.py index a88a1c8e..ffae6d61 100644 --- a/helpdesk/tests/test_public_actions.py +++ b/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) diff --git a/helpdesk/tests/test_ticket_lookup.py b/helpdesk/tests/test_ticket_lookup.py index fa2b516a..54faf1d1 100644 --- a/helpdesk/tests/test_ticket_lookup.py +++ b/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() diff --git a/helpdesk/views/public.py b/helpdesk/views/public.py index 0bcc17a9..e198eec0 100644 --- a/helpdesk/views/public.py +++ b/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: - queue, ticket_id = Ticket.queue_and_id_from_query(ticket_req) - try: - 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.') + if not (ticket_req and email): + if ticket_req is None and email is None: + return search_for_ticket(request) else: - 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) + return search_for_ticket(request, _('Missing ticket ID or e-mail address. Please try again.')) - if 'close' in request.GET and ticket.status == Ticket.RESOLVED_STATUS: - from helpdesk.views.staff import update_ticket - # Trick the update_ticket() view into thinking it's being called with - # 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 = {} + queue, ticket_id = Ticket.queue_and_id_from_query(ticket_req) + try: + if hasattr(settings, 'HELPDESK_VIEW_A_TICKET_PUBLIC') and settings.HELPDESK_VIEW_A_TICKET_PUBLIC: + ticket = Ticket.objects.get(id=ticket_id, submitter_email__iexact=email) + else: + ticket = Ticket.objects.get(id=ticket_id, submitter_email__iexact=email, secret_key__iexact=key) + except (ObjectDoesNotExist, ValueError): + return search_for_ticket(request, _('Invalid ticket ID or e-mail address. Please try again.')) - 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. - redirect_url = '' - if helpdesk_settings.HELPDESK_NAVIGATION_ENABLED: - redirect_url = reverse('helpdesk:view', args=[ticket_id]) + if 'close' in request.GET and ticket.status == Ticket.RESOLVED_STATUS: + from helpdesk.views.staff import update_ticket + # Trick the update_ticket() view into thinking it's being called with + # 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', { - '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 update_ticket(request, ticket_id, public=True) - return render(request, 'helpdesk/public_view_form.html', { - 'ticket': False, - 'email': email, - 'error_message': error_message, + # redirect user back to this ticket if possible. + redirect_url = '' + if helpdesk_settings.HELPDESK_NAVIGATION_ENABLED: + 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, })