Merge pull request #640 from auto-mat/public_ticket_security

Public ticket security improvements
This commit is contained in:
Garret Wassermann 2018-09-08 22:13:42 -04:00 committed by GitHub
commit a3bc6ed46b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 288 additions and 139 deletions

1
.gitignore vendored
View File

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

View File

@ -301,21 +301,27 @@ class TicketForm(AbstractTicketForm):
help_text=_('This e-mail address will receive copies of all public '
'updates to this ticket.'),
)
assigned_to = forms.ChoiceField(
widget=forms.Select(attrs={'class': 'form-control'}),
choices=(),
widget=forms.Select(attrs={'class': 'form-control'}) if not helpdesk_settings.HELPDESK_CREATE_TICKET_HIDE_ASSIGNED_TO else forms.HiddenInput(),
required=False,
label=_('Case owner'),
help_text=_('If you select an owner other than yourself, they\'ll be '
'e-mailed details of this ticket immediately.'),
choices=()
)
def __init__(self, *args, **kwargs):
"""
Add any custom fields that are defined to the form.
"""
super(TicketForm, self).__init__(*args, **kwargs)
super().__init__(*args, **kwargs)
self.fields['queue'].choices = [('', '--------')] + [(q.id, q.title) for q in Queue.objects.all()]
if helpdesk_settings.HELPDESK_STAFF_ONLY_TICKET_OWNERS:
assignable_users = User.objects.filter(is_active=True, is_staff=True).order_by(User.USERNAME_FIELD)
else:
assignable_users = User.objects.filter(is_active=True).order_by(User.USERNAME_FIELD)
self.fields['assigned_to'].choices = [('', '--------')] + [(u.id, u.get_username()) for u in assignable_users]
self._add_form_custom_fields()
def save(self, user=None):
@ -375,8 +381,8 @@ class PublicTicketForm(AbstractTicketForm):
self.fields['priority'].widget = forms.HiddenInput()
if hasattr(settings, 'HELPDESK_PUBLIC_TICKET_DUE_DATE'):
self.fields['due_date'].widget = forms.HiddenInput()
self._add_form_custom_fields(False)
self.fields['queue'].choices = [('', '--------')] + [
(q.id, q.title) for q in Queue.objects.filter(allow_public_submission=True)]
def save(self):
"""

View File

@ -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),
]

View File

@ -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'),
),
]

View File

@ -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)

View File

@ -0,0 +1,47 @@
{% extends "helpdesk/public_base.html" %}
{% load i18n bootstrap %}
{% block helpdesk_body %}
{% if helpdesk_settings.HELPDESK_SUBMIT_A_TICKET_PUBLIC %}
<div class="col-xs-6">
<div class="panel panel-default">
<div class="panel-body">
<h2 name='submit'>{% trans "Submit a Ticket" %}</h2>
<p>{% trans "Please provide as descriptive a title and description as possible." %}</p>
<form role="form" method='post' action='./#submit' enctype='multipart/form-data'>
<fieldset>
{{ form|bootstrap }}
{% comment %}
{% for field in form %}
{% if field.is_hidden %}
{{ field }}
{% else %}
<div class="form-group {% if field.errors %}has-error{% endif %}">
<label class="control-label" for='id_{{ field.name }}'>{{ field.label }}</label>{% if not field.field.required %} <span class='form_optional'>{% trans "(Optional)" %}</span>{% endif %}</dt>
<div class="input-group">{{ field }}</div>
{% if field.errors %}<div class="help-block">{{ field.errors }}</div>{% endif %}
{% if field.help_text %}<span class='fhelp-block'>{{ field.help_text }}</span>{% endif %}
</div>
{% endif %}
{% endfor %}
{% endcomment %}
<div class='buttons form-group'>
<input type='submit' class="btn btn-primary" value='{% trans "Submit Ticket" %}' />
</div>
</fieldset>
{% csrf_token %}</form>
</div>
</div>
</div>
{% endif %}
{% endblock %}

View File

@ -26,7 +26,7 @@
<div class="panel-body">
<h2 name='submit'>{% trans "Submit a Ticket" %}</h2>
<p>{% trans "All fields are required." %} {% trans "Please provide as descriptive a title and description as possible." %}</p>
<p>{% trans "Please provide as descriptive a title and description as possible." %}</p>
<form role="form" method='post' action='./#submit' enctype='multipart/form-data'>
<fieldset>

View File

@ -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)

View File

@ -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()

View File

@ -12,7 +12,7 @@ from django.contrib.auth.decorators import login_required
from django.contrib.auth import views as auth_views
from django.views.generic import TemplateView
from helpdesk.decorators import helpdesk_staff_member_required
from helpdesk.decorators import helpdesk_staff_member_required, protect_view
from helpdesk import settings as helpdesk_settings
from helpdesk.views import feeds, staff, public, kb
@ -46,10 +46,6 @@ urlpatterns = [
staff.mass_update,
name='mass_update'),
url(r'^tickets/submit/$',
staff.create_ticket,
name='submit'),
url(r'^tickets/(?P<ticket_id>[0-9]+)/$',
staff.view_ticket,
name='view'),
@ -149,9 +145,13 @@ urlpatterns = [
urlpatterns += [
url(r'^$',
public.homepage,
protect_view(public.Homepage.as_view()),
name='home'),
url(r'^tickets/submit/$',
public.create_ticket,
name='submit'),
url(r'^view/$',
public.view_ticket,
name='public_view'),

View File

@ -0,0 +1,8 @@
from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin
from helpdesk.decorators import is_helpdesk_staff
class MustBeStaffMixin(LoginRequiredMixin, UserPassesTestMixin):
def test_func(self):
return is_helpdesk_staff(self.request.user)

View File

@ -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,55 +18,59 @@ 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
from helpdesk.decorators import protect_view, is_helpdesk_staff
import helpdesk.views.staff as staff
from helpdesk.forms import PublicTicketForm
from helpdesk.lib import text_is_spam
from helpdesk.models import Ticket, Queue, UserSettings, KBCategory
@protect_view
def homepage(request):
if not request.user.is_authenticated and helpdesk_settings.HELPDESK_REDIRECT_TO_LOGIN_BY_DEFAULT:
return HttpResponseRedirect(reverse('login'))
if is_helpdesk_staff(request.user) or \
(request.user.is_authenticated and
helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE):
try:
if request.user.usersettings_helpdesk.settings.get('login_view_ticketlist', False):
return HttpResponseRedirect(reverse('helpdesk:list'))
else:
return HttpResponseRedirect(reverse('helpdesk:dashboard'))
except UserSettings.DoesNotExist:
return HttpResponseRedirect(reverse('helpdesk:dashboard'))
if request.method == 'POST':
form = PublicTicketForm(request.POST, request.FILES)
form.fields['queue'].choices = [('', '--------')] + [
(q.id, q.title) for q in Queue.objects.filter(allow_public_submission=True)]
if form.is_valid():
if text_is_spam(form.cleaned_data['body'], request):
# This submission is spam. Let's not save it.
return render(request, template_name='helpdesk/public_spam.html')
else:
ticket = form.save()
try:
return HttpResponseRedirect('%s?ticket=%s&email=%s' % (
reverse('helpdesk:public_view'),
ticket.ticket_for_url,
urlquote(ticket.submitter_email))
)
except ValueError:
# if someone enters a non-int string for the ticket
return HttpResponseRedirect(reverse('helpdesk:home'))
def create_ticket(request, *args, **kwargs):
if is_helpdesk_staff(request.user):
return staff.CreateTicketView.as_view()(request, *args, **kwargs)
else:
return CreateTicketView.as_view()(request, *args, **kwargs)
class CreateTicketView(FormView):
template_name = 'helpdesk/public_create_ticket.html'
form_class = PublicTicketForm
def dispatch(self, *args, **kwargs):
request = self.request
if not request.user.is_authenticated and helpdesk_settings.HELPDESK_REDIRECT_TO_LOGIN_BY_DEFAULT:
return HttpResponseRedirect(reverse('login'))
if is_helpdesk_staff(request.user) or \
(request.user.is_authenticated and
helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE):
try:
if request.user.usersettings_helpdesk.settings.get('login_view_ticketlist', False):
return HttpResponseRedirect(reverse('helpdesk:list'))
else:
return HttpResponseRedirect(reverse('helpdesk:dashboard'))
except UserSettings.DoesNotExist:
return HttpResponseRedirect(reverse('helpdesk:dashboard'))
return super().dispatch(*args, **kwargs)
def get_context(self):
knowledgebase_categories = KBCategory.objects.all()
return {
'helpdesk_settings': helpdesk_settings,
'kb_categories': knowledgebase_categories
}
def get_initial(self):
request = self.request
initial_data = {}
try:
queue = Queue.objects.get(slug=request.GET.get('queue', None))
except Queue.DoesNotExist:
queue = None
initial_data = {}
# add pre-defined data for public ticket
if hasattr(settings, 'HELPDESK_PUBLIC_TICKET_QUEUE'):
@ -85,76 +89,99 @@ def homepage(request):
if request.user.is_authenticated and request.user.email:
initial_data['submitter_email'] = request.user.email
return initial_data
form = PublicTicketForm(initial=initial_data)
form.fields['queue'].choices = [('', '--------')] + [
(q.id, q.title) for q in Queue.objects.filter(allow_public_submission=True)]
def form_valid(self, form):
request = self.request
if text_is_spam(form.cleaned_data['body'], request):
# This submission is spam. Let's not save it.
return render(request, template_name='helpdesk/public_spam.html')
else:
ticket = form.save()
try:
return HttpResponseRedirect('%s?ticket=%s&email=%s&key=%s' % (
reverse('helpdesk:public_view'),
ticket.ticket_for_url,
urlquote(ticket.submitter_email),
ticket.secret_key)
)
except ValueError:
# if someone enters a non-int string for the ticket
return HttpResponseRedirect(reverse('helpdesk:home'))
knowledgebase_categories = KBCategory.objects.all()
def get_success_url(self):
request = self.request
return render(request, 'helpdesk/public_homepage.html', {
'form': form,
'helpdesk_settings': helpdesk_settings,
'kb_categories': knowledgebase_categories
})
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,
})

View File

@ -26,6 +26,7 @@ from django.utils.translation import ugettext as _
from django.utils.html import escape
from django import forms
from django.utils import timezone
from django.views.generic.edit import FormView
from django.utils import six
@ -46,6 +47,7 @@ from helpdesk.models import (
IgnoreEmail, TicketCC, TicketDependency,
)
from helpdesk import settings as helpdesk_settings
from helpdesk.views.permissions import MustBeStaffMixin
User = get_user_model()
@ -1019,44 +1021,29 @@ def edit_ticket(request, ticket_id):
edit_ticket = staff_member_required(edit_ticket)
@helpdesk_staff_member_required
def create_ticket(request):
if helpdesk_settings.HELPDESK_STAFF_ONLY_TICKET_OWNERS:
assignable_users = User.objects.filter(is_active=True, is_staff=True).order_by(User.USERNAME_FIELD)
else:
assignable_users = User.objects.filter(is_active=True).order_by(User.USERNAME_FIELD)
class CreateTicketView(MustBeStaffMixin, FormView):
template_name = 'helpdesk/create_ticket.html'
form_class = TicketForm
if request.method == 'POST':
form = TicketForm(request.POST, request.FILES)
form.fields['queue'].choices = [('', '--------')] + [
(q.id, q.title) for q in Queue.objects.all()]
form.fields['assigned_to'].choices = [('', '--------')] + [
(u.id, u.get_username()) for u in assignable_users]
if form.is_valid():
ticket = form.save(user=request.user)
if _has_access_to_queue(request.user, ticket.queue):
return HttpResponseRedirect(ticket.get_absolute_url())
else:
return HttpResponseRedirect(reverse('helpdesk:dashboard'))
else:
def get_initial(self):
initial_data = {}
request = self.request
if request.user.usersettings_helpdesk.settings.get('use_email_as_submitter', False) and request.user.email:
initial_data['submitter_email'] = request.user.email
if 'queue' in request.GET:
initial_data['queue'] = request.GET['queue']
return initial_data
form = TicketForm(initial=initial_data)
form.fields['queue'].choices = [('', '--------')] + [
(q.id, q.title) for q in Queue.objects.all()]
form.fields['assigned_to'].choices = [('', '--------')] + [
(u.id, u.get_username()) for u in assignable_users]
if helpdesk_settings.HELPDESK_CREATE_TICKET_HIDE_ASSIGNED_TO:
form.fields['assigned_to'].widget = forms.HiddenInput()
def form_valid(self, form):
self.ticket = form.save()
return super().form_valid(form)
return render(request, 'helpdesk/create_ticket.html', {'form': form})
create_ticket = staff_member_required(create_ticket)
def get_success_url(self):
request = self.request
if _has_access_to_queue(request.user, self.ticket.queue):
return self.ticket.get_absolute_url()
else:
return reverse('helpdesk:dashboard')
@helpdesk_staff_member_required