From 13830a84e5e5cf1ef608e87d60c22c6e6ab55384 Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Sun, 20 Jul 2014 10:36:24 +0200 Subject: [PATCH 01/15] Custom logic for determining if the user is considered helpdesk staff. --- docs/settings.rst | 25 +++++++++++++++--------- helpdesk/settings.py | 15 +++++++++++---- helpdesk/views/staff.py | 42 +++++++++++++++++++++-------------------- 3 files changed, 49 insertions(+), 33 deletions(-) diff --git a/docs/settings.rst b/docs/settings.rst index b6a5f03b..a8132c82 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -75,15 +75,15 @@ These changes are visible throughout django-helpdesk **Default:** ``HELPDESK_SHOW_CHANGE_PASSWORD = False`` - **HELPDESK_FOLLOWUP_MOD** Allow user to override default layout for 'followups' (work in progress) - + **Default:** ``HELPDESK_FOLLOWUP_MOD = False`` - **HELPDESK_CUSTOM_WELCOME** Show custom welcome message in dashboard? - + **Default:** ``HELPDESK_CUSTOM_WELCOME = False`` - **HELPDESK_AUTO_SUBSCRIBE_ON_TICKET_RESPONSE ** Auto-subscribe user to ticket as a 'CC' if (s)he responds to a ticket? - + **Default:** ``HELPDESK_AUTO_SUBSCRIBE_ON_TICKET_RESPONSE = False`` - **HELPDESK_EMAIL_SUBJECT_TEMPLATE ** Subject template for templated emails. ``%(subject)s`` represents the subject wording from the email template (e.g. "(Closed)"). @@ -97,28 +97,35 @@ Options shown on public pages These options only change display of items on public-facing pages, not staff pages. - **HELPDESK_VIEW_A_TICKET_PUBLIC** Show 'View a Ticket' section on public page? - + **Default:** ``HELPDESK_VIEW_A_TICKET_PUBLIC = True`` - **HELPDESK_SUBMIT_A_TICKET_PUBLIC** Show 'submit a ticket' section & form on public page? - + **Default:** ``HELPDESK_SUBMIT_A_TICKET_PUBLIC = True`` - **HELPDESK_SHOW_KB_ON_HOMEPAGE** Should we should the KB categories on the homepage? - + **Default:** ``HELPDESK_SHOW_KB_ON_HOMEPAGE = False`` Options that change ticket updates ---------------------------------- -- **HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE** Allow non-staff users to interact with tickets? This will also change how 'staff_member_required' +- **HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE** Allow non-staff users to interact with tickets? This will also change how 'staff_member_required' in staff.py will be defined. - + **Default:** ``HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False`` +- **HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK** Apply a custom authorisation logic when defining 'staff_member_required' in staff.py. + If set, `HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE` will be ignored when determining staff access. + The value should be a function accepting the active user as a parameter and returning True if the user is considered helpdesk + staff. + + **Default:** ``HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = None`` + - **HELPDESK_SHOW_EDIT_BUTTON_FOLLOW_UP** Show edit buttons in ticket follow ups? - + **Default:** ``HELPDESK_SHOW_EDIT_BUTTON_FOLLOW_UP = True`` - **HELPDESK_SHOW_DELETE_BUTTON_SUPERUSER_FOLLOW_UP** Show delete buttons in ticket follow ups if user is 'superuser'? diff --git a/helpdesk/settings.py b/helpdesk/settings.py index 52fb4900..7ba2acb5 100644 --- a/helpdesk/settings.py +++ b/helpdesk/settings.py @@ -2,7 +2,7 @@ Default settings for django-helpdesk. """ - +import warnings from django.conf import settings @@ -60,10 +60,17 @@ HELPDESK_SUBMIT_A_TICKET_PUBLIC = getattr(settings, 'HELPDESK_SUBMIT_A_TICKET_PU ''' options for update_ticket views ''' -# allow non-staff users to interact with tickets? this will also change how 'staff_member_required' +# allow non-staff users to interact with tickets? this will also change how 'staff_member_required' # in staff.py will be defined. HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = getattr(settings, 'HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE', False) +# apply a custom authorisation logic when defining 'staff_member_required' in staff.py. +HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = getattr(settings, 'HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK', None) +if HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK and HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: + warnings.warn( + "The HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE and HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK settings cannot be both defined. " + "Only HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK will be considered in determining staff access.", RuntimeWarning) + # show edit buttons in ticket follow ups. HELPDESK_SHOW_EDIT_BUTTON_FOLLOW_UP = getattr(settings, 'HELPDESK_SHOW_EDIT_BUTTON_FOLLOW_UP', True) @@ -73,10 +80,10 @@ HELPDESK_SHOW_DELETE_BUTTON_SUPERUSER_FOLLOW_UP = getattr(settings, 'HELPDESK_SH # make all updates public by default? this will hide the 'is this update public' checkbox HELPDESK_UPDATE_PUBLIC_DEFAULT = getattr(settings, 'HELPDESK_UPDATE_PUBLIC_DEFAULT', False) -# only show staff users in ticket owner drop-downs +# only show staff users in ticket owner drop-downs HELPDESK_STAFF_ONLY_TICKET_OWNERS = getattr(settings, 'HELPDESK_STAFF_ONLY_TICKET_OWNERS', False) -# only show staff users in ticket cc drop-down +# only show staff users in ticket cc drop-down HELPDESK_STAFF_ONLY_TICKET_CC = getattr(settings, 'HELPDESK_STAFF_ONLY_TICKET_CC', False) diff --git a/helpdesk/views/staff.py b/helpdesk/views/staff.py index e491c2d8..0aaa2cfe 100644 --- a/helpdesk/views/staff.py +++ b/helpdesk/views/staff.py @@ -39,8 +39,10 @@ from helpdesk.forms import TicketForm, UserSettingsForm, EmailIgnoreForm, EditTi from helpdesk.lib import send_templated_mail, query_to_dict, apply_query, safe_template_context from helpdesk.models import Ticket, Queue, FollowUp, TicketChange, PreSetReply, Attachment, SavedSearch, IgnoreEmail, TicketCC, TicketDependency from helpdesk import settings as helpdesk_settings - -if helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: + +if helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK: + staff_member_required = user_passes_test(helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK) +elif helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: # treat 'normal' users like 'staff' staff_member_required = user_passes_test(lambda u: u.is_authenticated() and u.is_active) else: @@ -69,7 +71,7 @@ def dashboard(request): # closed & resolved tickets, assigned to current user tickets_closed_resolved = Ticket.objects.filter( - assigned_to=request.user, + assigned_to=request.user, status__in = [Ticket.CLOSED_STATUS, Ticket.RESOLVED_STATUS]) unassigned_tickets = Ticket.objects.filter( @@ -107,7 +109,7 @@ def dashboard(request): GROUP BY queue, name ORDER BY q.id; """) - + dash_tickets = query_to_dict(cursor.fetchall(), cursor.description) return render_to_response('helpdesk/dashboard.html', @@ -200,7 +202,7 @@ def view_ticket(request, ticket_id): if request.GET.has_key('take'): # Allow the user to assign the ticket to themselves whilst viewing it. - + # Trick the update_ticket() view into thinking it's being called with # a valid POST. request.POST = { @@ -261,7 +263,7 @@ view_ticket = staff_member_required(view_ticket) def return_ticketccstring_and_show_subscribe(user, ticket): ''' used in view_ticket() and followup_edit()''' - # create the ticketcc_string and check whether current user is already + # create the ticketcc_string and check whether current user is already # subscribed username = user.username.upper() useremail = user.email.upper() @@ -448,7 +450,7 @@ def update_ticket(request, ticket_id, public=False): messages_sent_to = [] - # ticket might have changed above, so we re-instantiate context with the + # ticket might have changed above, so we re-instantiate context with the # (possibly) updated ticket. context = safe_template_context(ticket) context.update( @@ -457,7 +459,7 @@ def update_ticket(request, ticket_id, public=False): ) if public and (f.comment or (f.new_status in (Ticket.RESOLVED_STATUS, Ticket.CLOSED_STATUS))): - + if f.new_status == Ticket.RESOLVED_STATUS: template = 'resolved_' @@ -712,7 +714,7 @@ def ticket_list(request): or request.GET.has_key('status') or request.GET.has_key('q') or request.GET.has_key('sort') - or request.GET.has_key('sortreverse') + or request.GET.has_key('sortreverse') ): # Fall-back if no querying is being done, force the list to only @@ -751,7 +753,7 @@ def ticket_list(request): date_from = request.GET.get('date_from') if date_from: query_params['filtering']['created__gte'] = date_from - + date_to = request.GET.get('date_to') if date_to: query_params['filtering']['created__lte'] = date_to @@ -842,7 +844,7 @@ def edit_ticket(request, ticket_id): return HttpResponseRedirect(ticket.get_absolute_url()) else: form = EditTicketForm(instance=ticket) - + return render_to_response('helpdesk/edit_ticket.html', RequestContext(request, { 'form': form, @@ -854,7 +856,7 @@ def create_ticket(request): assignable_users = User.objects.filter(is_active=True, is_staff=True).order_by('username') else: assignable_users = User.objects.filter(is_active=True).order_by('username') - + if request.method == 'POST': form = TicketForm(request.POST, request.FILES) form.fields['queue'].choices = [('', '--------')] + [[q.id, q.title] for q in Queue.objects.all()] @@ -955,7 +957,7 @@ def run_report(request, report): return HttpResponseRedirect(reverse("helpdesk_report_index")) report_queryset = Ticket.objects.all().select_related() - + from_saved_query = False saved_query = None @@ -993,7 +995,7 @@ def run_report(request, report): _('Nov'), _('Dec'), ) - + first_ticket = Ticket.objects.all().order_by('created')[0] first_month = first_ticket.created.month first_year = first_ticket.created.year @@ -1106,15 +1108,15 @@ def run_report(request, report): if report == 'daysuntilticketclosedbymonth': summarytable2[metric1, metric2] += metric3 - + table = [] - + if report == 'daysuntilticketclosedbymonth': for key in summarytable2.keys(): summarytable[key] = summarytable2[key] / summarytable[key] header1 = sorted(set(list( i.encode('utf-8') for i,_ in summarytable.keys() ))) - + column_headings = [col1heading] + possible_options # Pivot the data so that 'header1' fields are always first column @@ -1330,11 +1332,11 @@ def calc_basic_ticket_stats(Ticket): date_30_str = date_30.strftime('%Y-%m-%d') date_60_str = date_60.strftime('%Y-%m-%d') - # > 0 & <= 30 + # > 0 & <= 30 ota_le_30 = all_open_tickets.filter(created__gte = date_30_str) N_ota_le_30 = len(ota_le_30) - # >= 30 & <= 60 + # >= 30 & <= 60 ota_le_60_ge_30 = all_open_tickets.filter(created__gte = date_60_str, created__lte = date_30_str) N_ota_le_60_ge_30 = len(ota_le_60_ge_30) @@ -1357,7 +1359,7 @@ def calc_basic_ticket_stats(Ticket): average_nbr_days_until_ticket_closed_last_60_days = calc_average_nbr_days_until_ticket_resolved(all_closed_last_60_days) # put together basic stats - basic_ticket_stats = { 'average_nbr_days_until_ticket_closed': average_nbr_days_until_ticket_closed, + basic_ticket_stats = { 'average_nbr_days_until_ticket_closed': average_nbr_days_until_ticket_closed, 'average_nbr_days_until_ticket_closed_last_60_days': average_nbr_days_until_ticket_closed_last_60_days, 'open_ticket_stats': ots, } From 97c317f83d2f1172adb7b1b05841e3834bf2667a Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Mon, 28 Jul 2014 06:47:19 +0200 Subject: [PATCH 02/15] add tests, example --- docs/settings.rst | 8 +-- helpdesk/tests/navigation.py | 98 +++++++++++++++++++++++++++++++++--- helpdesk/views/staff.py | 1 + quicktest.py | 10 ++-- 4 files changed, 104 insertions(+), 13 deletions(-) diff --git a/docs/settings.rst b/docs/settings.rst index 710abbeb..ea57a3ba 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -61,7 +61,7 @@ These changes are visible throughout django-helpdesk **Default:** ``HELPDESK_FOLLOWUP_MOD = False`` - **HELPDESK_AUTO_SUBSCRIBE_ON_TICKET_RESPONSE** Auto-subscribe user to ticket as a 'CC' if (s)he responds to a ticket? - + **Default:** ``HELPDESK_AUTO_SUBSCRIBE_ON_TICKET_RESPONSE = False`` - **HELPDESK_EMAIL_SUBJECT_TEMPLATE** Subject template for templated emails. ``%(subject)s`` represents the subject wording from the email template (e.g. "(Closed)"). @@ -94,7 +94,9 @@ Options that change ticket updates - **HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK** Apply a custom authorisation logic when defining 'staff_member_required' in staff.py. If set, `HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE` will be ignored when determining staff access. The value should be a function accepting the active user as a parameter and returning True if the user is considered helpdesk - staff. + staff, e.g. + + lambda u: u.is_authenticated() and u.is_active and u.groups.filter(name='helpdesk_staff').exists())) **Default:** ``HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = None`` @@ -172,7 +174,7 @@ Discontinued Settings The following settings were defined in previous versions and are no longer supported. -- **HELPDESK_CUSTOM_WELCOME** +- **HELPDESK_CUSTOM_WELCOME** - **HELDPESK_KB_ENABLED_STAFF** Now always True diff --git a/helpdesk/tests/navigation.py b/helpdesk/tests/navigation.py index 08b049b3..835ff23e 100644 --- a/helpdesk/tests/navigation.py +++ b/helpdesk/tests/navigation.py @@ -1,22 +1,20 @@ # -*- coding: utf-8 -*- +import sys from django.core.urlresolvers import reverse from django.test import TestCase -from helpdesk.tests.helpers import get_staff_user, reload_urlconf +from helpdesk import settings +from helpdesk.tests.helpers import get_staff_user, reload_urlconf, User -class TestKBDisabled(TestCase): +class KBDisabledTestCase(TestCase): def setUp(self): - from helpdesk import settings - self.HELPDESK_KB_ENABLED = settings.HELPDESK_KB_ENABLED if self.HELPDESK_KB_ENABLED: settings.HELPDESK_KB_ENABLED = False reload_urlconf() def tearDown(self): - from helpdesk import settings - if self.HELPDESK_KB_ENABLED: settings.HELPDESK_KB_ENABLED = True reload_urlconf() @@ -36,3 +34,91 @@ class TestKBDisabled(TestCase): raise else: self.assertEqual(response.status_code, 200) + + +class StaffUserTestCaseMixin(object): + HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False + HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = None + expected_login_template = 'admin/login.html' + + def setUp(self): + self.old_settings = settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE, settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK + settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = self.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE + settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = self.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK + self.reload_views() + + def tearDown(self): + settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE, settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = self.old_settings + self.reload_views() + + def reload_views(self): + try: + reload(sys.modules['helpdesk.views.staff']) + reload_urlconf() + except KeyError: + pass + + def test_anonymous_user(self): + """Access to the dashboard always requires a login""" + response = self.client.get(reverse('helpdesk_dashboard'), follow=True) + self.assertTemplateUsed(response, self.expected_login_template) + + +class NonStaffUsersAllowedTestCase(StaffUserTestCaseMixin, TestCase): + HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = True + HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = None + expected_login_template = 'helpdesk/registration/login.html' + + def test_non_staff_allowed(self): + """If HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE is True, + authenticated, non-staff users should be able to access + the dashboard. + """ + user = User.objects.create_user(username='henry.wensleydale', password='gouda', email='wensleydale@example.com') + + self.client.login(username=user.username, password='gouda') + response = self.client.get(reverse('helpdesk_dashboard'), follow=True) + self.assertTemplateUsed(response, 'helpdesk/dashboard.html') + + +class StaffUsersOnlyTestCase(StaffUserTestCaseMixin, TestCase): + # Use default values + HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False + HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = None + + def test_staff_only(self): + """If HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE is False, + only staff users should be able to access the dashboard. + """ + user = get_staff_user() + + self.client.login(username=user.username, password='password') + response = self.client.get(reverse('helpdesk_dashboard'), follow=True) + self.assertTemplateUsed(response, 'helpdesk/dashboard.html') + + +class CustomStaffUserTestCase(StaffUserTestCaseMixin, TestCase): + HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False + expected_login_template = 'helpdesk/registration/login.html' + + @staticmethod + def HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK(user): + """Arbitrary user validation function""" + return user.is_authenticated() and user.is_active and user.username.lower().endswith('wensleydale') + + def test_custom_staff_pass(self): + """If HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK is not None, + a custom access rule is applied. + """ + user = User.objects.create_user(username='henry.wensleydale', password='gouda', email='wensleydale@example.com') + + self.client.login(username=user.username, password='gouda') + response = self.client.get(reverse('helpdesk_dashboard'), follow=True) + self.assertTemplateUsed(response, 'helpdesk/dashboard.html') + + def test_custom_staff_fail(self): + user = User.objects.create_user(username='terry.milton', password='frog', email='milton@example.com') + + self.client.login(username=user.username, password='frog') + response = self.client.get(reverse('helpdesk_dashboard'), follow=True) + self.assertTemplateUsed(response, self.expected_login_template) \ No newline at end of file diff --git a/helpdesk/views/staff.py b/helpdesk/views/staff.py index aad7ae7a..527c08ee 100644 --- a/helpdesk/views/staff.py +++ b/helpdesk/views/staff.py @@ -42,6 +42,7 @@ from helpdesk.models import Ticket, Queue, FollowUp, TicketChange, PreSetReply, from helpdesk import settings as helpdesk_settings if helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK: + # apply a custom user validation condition staff_member_required = user_passes_test(helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK) elif helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: # treat 'normal' users like 'staff' diff --git a/quicktest.py b/quicktest.py index f6aa9af6..3d04ace0 100644 --- a/quicktest.py +++ b/quicktest.py @@ -65,8 +65,8 @@ class QuickDjangoTest(object): Fire up the Django test suite developed for version 1.2 """ settings.configure( - DEBUG = True, - DATABASES = { + DEBUG=True, + DATABASES={ 'default': { 'ENGINE': 'django.db.backends.sqlite3', 'NAME': os.path.join(self.DIRNAME, 'database.db'), @@ -76,8 +76,10 @@ class QuickDjangoTest(object): 'PORT': '', } }, - INSTALLED_APPS = self.INSTALLED_APPS + self.apps, - ROOT_URLCONF = self.apps[0] + '.urls', + INSTALLED_APPS=self.INSTALLED_APPS + self.apps, + ROOT_URLCONF=self.apps[0] + '.urls', + STATIC_URL='/static/', + LOGIN_URL='login', ) from django.test.simple import DjangoTestSuiteRunner failures = DjangoTestSuiteRunner().run_tests(self.apps, verbosity=1) From 3e517477b770d4b7f0417dc16862cddd4025629f Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Mon, 28 Jul 2014 07:32:12 +0200 Subject: [PATCH 03/15] fix lazy url in settings with Django 1.4 --- quicktest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/quicktest.py b/quicktest.py index 3d04ace0..834a77ac 100644 --- a/quicktest.py +++ b/quicktest.py @@ -64,6 +64,8 @@ class QuickDjangoTest(object): """ Fire up the Django test suite developed for version 1.2 """ + from django.core.urlresolvers import reverse_lazy + settings.configure( DEBUG=True, DATABASES={ @@ -79,7 +81,7 @@ class QuickDjangoTest(object): INSTALLED_APPS=self.INSTALLED_APPS + self.apps, ROOT_URLCONF=self.apps[0] + '.urls', STATIC_URL='/static/', - LOGIN_URL='login', + LOGIN_URL=reverse_lazy('login'), ) from django.test.simple import DjangoTestSuiteRunner failures = DjangoTestSuiteRunner().run_tests(self.apps, verbosity=1) From aea940cb3facea3b7654216a29a3a4571cad513e Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Mon, 28 Jul 2014 11:46:02 +0200 Subject: [PATCH 04/15] move decorators to their own module, rename to helpdesk_* --- helpdesk/decorators.py | 17 ++++++ helpdesk/settings.py | 4 +- helpdesk/tests/navigation.py | 1 + helpdesk/views/staff.py | 115 +++++++++++++++++------------------ 4 files changed, 75 insertions(+), 62 deletions(-) create mode 100644 helpdesk/decorators.py diff --git a/helpdesk/decorators.py b/helpdesk/decorators.py new file mode 100644 index 00000000..00624fc6 --- /dev/null +++ b/helpdesk/decorators.py @@ -0,0 +1,17 @@ +# -*- coding: utf-8 -*- +from django.contrib.auth.decorators import user_passes_test +from helpdesk import settings as helpdesk_settings + +if helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK: + # apply a custom user validation condition + helpdesk_staff_member_required = user_passes_test(helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK) +elif helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: + # treat 'normal' users like 'staff' + helpdesk_staff_member_required = user_passes_test(lambda u: u.is_authenticated() and u.is_active) +else: + try: + from django.contrib.admin.views.decorators import staff_member_required as helpdesk_staff_member_required + except ImportError: + helpdesk_staff_member_required = user_passes_test(lambda u: u.is_authenticated() and u.is_active and u.is_staff) + +helpdesk_superuser_required = user_passes_test(lambda u: u.is_authenticated() and u.is_active and u.is_superuser) diff --git a/helpdesk/settings.py b/helpdesk/settings.py index 7ba2acb5..f0492a97 100644 --- a/helpdesk/settings.py +++ b/helpdesk/settings.py @@ -60,11 +60,11 @@ HELPDESK_SUBMIT_A_TICKET_PUBLIC = getattr(settings, 'HELPDESK_SUBMIT_A_TICKET_PU ''' options for update_ticket views ''' -# allow non-staff users to interact with tickets? this will also change how 'staff_member_required' +# allow non-staff users to interact with tickets? this will also change how 'helpdesk_staff_member_required' # in staff.py will be defined. HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = getattr(settings, 'HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE', False) -# apply a custom authorisation logic when defining 'staff_member_required' in staff.py. +# apply a custom authorisation logic when defining 'helpdesk_staff_member_required' in staff.py. HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = getattr(settings, 'HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK', None) if HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK and HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: warnings.warn( diff --git a/helpdesk/tests/navigation.py b/helpdesk/tests/navigation.py index 835ff23e..ab3454db 100644 --- a/helpdesk/tests/navigation.py +++ b/helpdesk/tests/navigation.py @@ -53,6 +53,7 @@ class StaffUserTestCaseMixin(object): def reload_views(self): try: + reload(sys.modules['helpdesk.decorators']) reload(sys.modules['helpdesk.views.staff']) reload_urlconf() except KeyError: diff --git a/helpdesk/views/staff.py b/helpdesk/views/staff.py index 527c08ee..99a9a1dd 100644 --- a/helpdesk/views/staff.py +++ b/helpdesk/views/staff.py @@ -8,24 +8,20 @@ views/staff.py - The bulk of the application - provides most business logic and """ from datetime import datetime, timedelta -import sys - from django.conf import settings try: from django.contrib.auth import get_user_model User = get_user_model() except ImportError: from django.contrib.auth.models import User -from django.contrib.auth.decorators import login_required, user_passes_test -from django.core.files.base import ContentFile from django.core.urlresolvers import reverse from django.core.exceptions import ValidationError from django.core import paginator from django.db import connection from django.db.models import Q -from django.http import HttpResponseRedirect, Http404, HttpResponse, HttpResponseForbidden +from django.http import HttpResponseRedirect, Http404, HttpResponse from django.shortcuts import render_to_response, get_object_or_404 -from django.template import loader, Context, RequestContext +from django.template import RequestContext from django.utils.dates import MONTHS_3 from django.utils.translation import ugettext as _ from django.utils.html import escape @@ -36,27 +32,14 @@ try: except ImportError: from datetime import datetime as timezone +from helpdesk.decorators import helpdesk_staff_member_required, helpdesk_superuser_required from helpdesk.forms import TicketForm, UserSettingsForm, EmailIgnoreForm, EditTicketForm, TicketCCForm, EditFollowUpForm, TicketDependencyForm from helpdesk.lib import send_templated_mail, query_to_dict, apply_query, safe_template_context from helpdesk.models import Ticket, Queue, FollowUp, TicketChange, PreSetReply, Attachment, SavedSearch, IgnoreEmail, TicketCC, TicketDependency from helpdesk import settings as helpdesk_settings -if helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK: - # apply a custom user validation condition - staff_member_required = user_passes_test(helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK) -elif helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: - # treat 'normal' users like 'staff' - staff_member_required = user_passes_test(lambda u: u.is_authenticated() and u.is_active) -else: - try: - from django.contrib.admin.views.decorators import staff_member_required - except: - staff_member_required = user_passes_test(lambda u: u.is_authenticated() and u.is_active and u.is_staff) - - -superuser_required = user_passes_test(lambda u: u.is_authenticated() and u.is_active and u.is_superuser) - +@helpdesk_staff_member_required def dashboard(request): """ A quick summary overview for users: A list of their own tickets, a table @@ -123,9 +106,8 @@ def dashboard(request): 'dash_tickets': dash_tickets, 'basic_ticket_stats': basic_ticket_stats, })) -dashboard = staff_member_required(dashboard) - +@helpdesk_staff_member_required def delete_ticket(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) @@ -137,8 +119,9 @@ def delete_ticket(request, ticket_id): else: ticket.delete() return HttpResponseRedirect(reverse('helpdesk_home')) -delete_ticket = staff_member_required(delete_ticket) + +@helpdesk_staff_member_required def followup_edit(request, ticket_id, followup_id): "Edit followup options with an ability to change the ticket." followup = get_object_or_404(FollowUp, id=followup_id) @@ -184,8 +167,9 @@ def followup_edit(request, ticket_id, followup_id): # delete old followup followup.delete() return HttpResponseRedirect(reverse('helpdesk_view', args=[ticket.id])) -followup_edit = staff_member_required(followup_edit) + +@helpdesk_staff_member_required def followup_delete(request, ticket_id, followup_id): ''' followup delete for superuser''' @@ -196,9 +180,9 @@ def followup_delete(request, ticket_id, followup_id): followup = get_object_or_404(FollowUp, id=followup_id) followup.delete() return HttpResponseRedirect(reverse('helpdesk_view', args=[ticket.id])) -followup_delete = staff_member_required(followup_delete) +@helpdesk_staff_member_required def view_ticket(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) @@ -261,7 +245,7 @@ def view_ticket(request, ticket_id): 'ticketcc_string': ticketcc_string, 'SHOW_SUBSCRIBE': SHOW_SUBSCRIBE, })) -view_ticket = staff_member_required(view_ticket) + def return_ticketccstring_and_show_subscribe(user, ticket): ''' used in view_ticket() and followup_edit()''' @@ -308,7 +292,8 @@ def subscribe_staff_member_to_ticket(ticket, user): def update_ticket(request, ticket_id, public=False): - if not (public or (request.user.is_authenticated() and request.user.is_active and (request.user.is_staff or helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE))): + if not (public or (request.user.is_authenticated() and request.user.is_active and ( + request.user.is_staff or helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE))): return HttpResponseRedirect('%s?next=%s' % (reverse('login'), request.path)) ticket = get_object_or_404(Ticket, id=ticket_id) @@ -415,7 +400,6 @@ def update_ticket(request, ticket_id, public=False): # settings.MAX_EMAIL_ATTACHMENT_SIZE) are sent via email. files.append(a.file.path) - if title != ticket.title: c = TicketChange( followup=f, @@ -509,7 +493,7 @@ def update_ticket(request, ticket_id, public=False): else: template_staff = 'updated_owner' - if (not reassigned or ( reassigned and ticket.assigned_to.usersettings.settings.get('email_on_ticket_assign', False))) or (not reassigned and ticket.assigned_to.usersettings.settings.get('email_on_ticket_change', False)): + if (not reassigned or (reassigned and ticket.assigned_to.usersettings.settings.get('email_on_ticket_assign', False))) or (not reassigned and ticket.assigned_to.usersettings.settings.get('email_on_ticket_change', False)): send_templated_mail( template_staff, context, @@ -559,6 +543,7 @@ def return_to_ticket(user, helpdesk_settings, ticket): return HttpResponseRedirect(ticket.ticket_url) +@helpdesk_staff_member_required def mass_update(request): tickets = request.POST.getlist('ticket_id') action = request.POST.get('action', None) @@ -647,8 +632,9 @@ def mass_update(request): t.delete() return HttpResponseRedirect(reverse('helpdesk_list')) -mass_update = staff_member_required(mass_update) + +@helpdesk_staff_member_required def ticket_list(request): context = {} @@ -834,9 +820,9 @@ def ticket_list(request): saved_query=saved_query, search_message=search_message, ))) -ticket_list = staff_member_required(ticket_list) +@helpdesk_staff_member_required def edit_ticket(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) if request.method == 'POST': @@ -851,8 +837,9 @@ def edit_ticket(request, ticket_id): RequestContext(request, { 'form': form, })) -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('username') @@ -883,9 +870,9 @@ def create_ticket(request): RequestContext(request, { 'form': form, })) -create_ticket = staff_member_required(create_ticket) +@helpdesk_staff_member_required def raw_details(request, type): # TODO: This currently only supports spewing out 'PreSetReply' objects, # in the future it needs to be expanded to include other items. All it @@ -902,9 +889,9 @@ def raw_details(request, type): raise Http404 raise Http404 -raw_details = staff_member_required(raw_details) +@helpdesk_staff_member_required def hold_ticket(request, ticket_id, unhold=False): ticket = get_object_or_404(Ticket, id=ticket_id) @@ -927,22 +914,22 @@ def hold_ticket(request, ticket_id, unhold=False): ticket.save() return HttpResponseRedirect(ticket.get_absolute_url()) -hold_ticket = staff_member_required(hold_ticket) +@helpdesk_staff_member_required def unhold_ticket(request, ticket_id): return hold_ticket(request, ticket_id, unhold=True) -unhold_ticket = staff_member_required(unhold_ticket) +@helpdesk_staff_member_required def rss_list(request): return render_to_response('helpdesk/rss_list.html', RequestContext(request, { 'queues': Queue.objects.all(), })) -rss_list = staff_member_required(rss_list) +@helpdesk_staff_member_required def report_index(request): number_tickets = Ticket.objects.all().count() saved_query = request.GET.get('saved_query', None) @@ -951,9 +938,9 @@ def report_index(request): 'number_tickets': number_tickets, 'saved_query': saved_query, })) -report_index = staff_member_required(report_index) +@helpdesk_staff_member_required def run_report(request, report): if Ticket.objects.all().count() == 0 or report not in ('queuemonth', 'usermonth', 'queuestatus', 'queuepriority', 'userstatus', 'userpriority', 'userqueue', 'daysuntilticketclosedbymonth'): return HttpResponseRedirect(reverse("helpdesk_report_index")) @@ -1124,9 +1111,9 @@ def run_report(request, report): 'from_saved_query': from_saved_query, 'saved_query': saved_query, })) -run_report = staff_member_required(run_report) +@helpdesk_staff_member_required def save_query(request): title = request.POST.get('title', None) shared = request.POST.get('shared', False) @@ -1139,9 +1126,9 @@ def save_query(request): query.save() return HttpResponseRedirect('%s?saved_query=%s' % (reverse('helpdesk_list'), query.id)) -save_query = staff_member_required(save_query) +@helpdesk_staff_member_required def delete_saved_query(request, id): query = get_object_or_404(SavedSearch, id=id, user=request.user) @@ -1153,9 +1140,9 @@ def delete_saved_query(request, id): RequestContext(request, { 'query': query, })) -delete_saved_query = staff_member_required(delete_saved_query) +@helpdesk_staff_member_required def user_settings(request): s = request.user.usersettings if request.POST: @@ -1170,17 +1157,17 @@ def user_settings(request): RequestContext(request, { 'form': form, })) -user_settings = staff_member_required(user_settings) +@helpdesk_superuser_required def email_ignore(request): return render_to_response('helpdesk/email_ignore_list.html', RequestContext(request, { 'ignore_list': IgnoreEmail.objects.all(), })) -email_ignore = superuser_required(email_ignore) +@helpdesk_superuser_required def email_ignore_add(request): if request.method == 'POST': form = EmailIgnoreForm(request.POST) @@ -1194,9 +1181,9 @@ def email_ignore_add(request): RequestContext(request, { 'form': form, })) -email_ignore_add = superuser_required(email_ignore_add) +@helpdesk_superuser_required def email_ignore_del(request, id): ignore = get_object_or_404(IgnoreEmail, id=id) if request.method == 'POST': @@ -1207,8 +1194,8 @@ def email_ignore_del(request, id): RequestContext(request, { 'ignore': ignore, })) -email_ignore_del = superuser_required(email_ignore_del) +@helpdesk_staff_member_required def ticket_cc(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) copies_to = ticket.ticketcc_set.all() @@ -1217,8 +1204,9 @@ def ticket_cc(request, ticket_id): 'copies_to': copies_to, 'ticket': ticket, })) -ticket_cc = staff_member_required(ticket_cc) + +@helpdesk_staff_member_required def ticket_cc_add(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) if request.method == 'POST': @@ -1235,8 +1223,9 @@ def ticket_cc_add(request, ticket_id): 'ticket': ticket, 'form': form, })) -ticket_cc_add = staff_member_required(ticket_cc_add) + +@helpdesk_staff_member_required def ticket_cc_del(request, ticket_id, cc_id): cc = get_object_or_404(TicketCC, ticket__id=ticket_id, id=cc_id) if request.method == 'POST': @@ -1246,8 +1235,9 @@ def ticket_cc_del(request, ticket_id, cc_id): RequestContext(request, { 'cc': cc, })) -ticket_cc_del = staff_member_required(ticket_cc_del) + +@helpdesk_staff_member_required def ticket_dependency_add(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) if request.method == 'POST': @@ -1265,8 +1255,9 @@ def ticket_dependency_add(request, ticket_id): 'ticket': ticket, 'form': form, })) -ticket_dependency_add = staff_member_required(ticket_dependency_add) + +@helpdesk_staff_member_required def ticket_dependency_del(request, ticket_id, dependency_id): dependency = get_object_or_404(TicketDependency, ticket__id=ticket_id, id=dependency_id) if request.method == 'POST': @@ -1276,14 +1267,15 @@ def ticket_dependency_del(request, ticket_id, dependency_id): RequestContext(request, { 'dependency': dependency, })) -ticket_dependency_del = staff_member_required(ticket_dependency_del) + +@helpdesk_staff_member_required def attachment_del(request, ticket_id, attachment_id): ticket = get_object_or_404(Ticket, id=ticket_id) attachment = get_object_or_404(Attachment, id=attachment_id) attachment.delete() return HttpResponseRedirect(reverse('helpdesk_view', args=[ticket_id])) -attachment_del = staff_member_required(attachment_del) + def calc_average_nbr_days_until_ticket_resolved(Tickets): nbr_closed_tickets = len(Tickets) @@ -1303,6 +1295,7 @@ def calc_average_nbr_days_until_ticket_resolved(Tickets): return mean_per_ticket + def calc_basic_ticket_stats(Ticket): # all not closed tickets (open, reopened, resolved,) - independent of user all_open_tickets = Ticket.objects.exclude(status = Ticket.CLOSED_STATUS) @@ -1340,14 +1333,15 @@ def calc_basic_ticket_stats(Ticket): average_nbr_days_until_ticket_closed_last_60_days = calc_average_nbr_days_until_ticket_resolved(all_closed_last_60_days) # put together basic stats - basic_ticket_stats = { 'average_nbr_days_until_ticket_closed': average_nbr_days_until_ticket_closed, - 'average_nbr_days_until_ticket_closed_last_60_days': average_nbr_days_until_ticket_closed_last_60_days, - 'open_ticket_stats': ots, } + basic_ticket_stats = { + 'average_nbr_days_until_ticket_closed': average_nbr_days_until_ticket_closed, + 'average_nbr_days_until_ticket_closed_last_60_days': average_nbr_days_until_ticket_closed_last_60_days, + 'open_ticket_stats': ots, } return basic_ticket_stats + def get_color_for_nbr_days(nbr_days): - ''' ''' if nbr_days < 5: color_string = 'green' elif nbr_days >= 5 and nbr_days < 10: @@ -1357,14 +1351,15 @@ def get_color_for_nbr_days(nbr_days): return color_string + def days_since_created(today, ticket): return (today - ticket.created).days + def date_rel_to_today(today, offset): return today - timedelta(days = offset) + def sort_string(begin, end): - return 'sort=created&date_from=%s&date_to=%s&status=%s&status=%s&status=%s' %(begin, end, Ticket.OPEN_STATUS, Ticket.REOPENED_STATUS, Ticket.RESOLVED_STATUS) - - - + return 'sort=created&date_from=%s&date_to=%s&status=%s&status=%s&status=%s' % ( + begin, end, Ticket.OPEN_STATUS, Ticket.REOPENED_STATUS, Ticket.RESOLVED_STATUS) From 581e43a3b037279c46a7ad292cb71352abe95fb0 Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Tue, 29 Jul 2014 20:55:25 +0200 Subject: [PATCH 05/15] separate authorisation function from decorator --- helpdesk/decorators.py | 10 ++++------ helpdesk/tests/navigation.py | 7 ++----- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/helpdesk/decorators.py b/helpdesk/decorators.py index 00624fc6..1226bf0d 100644 --- a/helpdesk/decorators.py +++ b/helpdesk/decorators.py @@ -4,14 +4,12 @@ from helpdesk import settings as helpdesk_settings if helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK: # apply a custom user validation condition - helpdesk_staff_member_required = user_passes_test(helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK) + is_helpdesk_staff = helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK elif helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: # treat 'normal' users like 'staff' - helpdesk_staff_member_required = user_passes_test(lambda u: u.is_authenticated() and u.is_active) + is_helpdesk_staff = lambda u: u.is_authenticated() and u.is_active else: - try: - from django.contrib.admin.views.decorators import staff_member_required as helpdesk_staff_member_required - except ImportError: - helpdesk_staff_member_required = user_passes_test(lambda u: u.is_authenticated() and u.is_active and u.is_staff) + is_helpdesk_staff = lambda u: u.is_authenticated() and u.is_active and u.is_staff +helpdesk_staff_member_required = user_passes_test(is_helpdesk_staff) helpdesk_superuser_required = user_passes_test(lambda u: u.is_authenticated() and u.is_active and u.is_superuser) diff --git a/helpdesk/tests/navigation.py b/helpdesk/tests/navigation.py index ab3454db..32bbdb5c 100644 --- a/helpdesk/tests/navigation.py +++ b/helpdesk/tests/navigation.py @@ -39,7 +39,6 @@ class KBDisabledTestCase(TestCase): class StaffUserTestCaseMixin(object): HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = None - expected_login_template = 'admin/login.html' def setUp(self): self.old_settings = settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE, settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK @@ -62,13 +61,12 @@ class StaffUserTestCaseMixin(object): def test_anonymous_user(self): """Access to the dashboard always requires a login""" response = self.client.get(reverse('helpdesk_dashboard'), follow=True) - self.assertTemplateUsed(response, self.expected_login_template) + self.assertTemplateUsed(response, 'helpdesk/registration/login.html') class NonStaffUsersAllowedTestCase(StaffUserTestCaseMixin, TestCase): HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = True HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = None - expected_login_template = 'helpdesk/registration/login.html' def test_non_staff_allowed(self): """If HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE is True, @@ -100,7 +98,6 @@ class StaffUsersOnlyTestCase(StaffUserTestCaseMixin, TestCase): class CustomStaffUserTestCase(StaffUserTestCaseMixin, TestCase): HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False - expected_login_template = 'helpdesk/registration/login.html' @staticmethod def HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK(user): @@ -122,4 +119,4 @@ class CustomStaffUserTestCase(StaffUserTestCaseMixin, TestCase): self.client.login(username=user.username, password='frog') response = self.client.get(reverse('helpdesk_dashboard'), follow=True) - self.assertTemplateUsed(response, self.expected_login_template) \ No newline at end of file + self.assertTemplateUsed(response, 'helpdesk/registration/login.html') From 82df965d5f3d0810a0b8744046f5ec44f0132b5a Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Tue, 29 Jul 2014 22:03:47 +0200 Subject: [PATCH 06/15] replace HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE in homepage view and test behaviour --- helpdesk/tests/helpers.py | 18 ++++++++++ helpdesk/tests/navigation.py | 69 +++++++++++++++++++++++++++++++++++- helpdesk/views/public.py | 10 +++--- 3 files changed, 91 insertions(+), 6 deletions(-) diff --git a/helpdesk/tests/helpers.py b/helpdesk/tests/helpers.py index 6dccad08..ba73e990 100644 --- a/helpdesk/tests/helpers.py +++ b/helpdesk/tests/helpers.py @@ -31,3 +31,21 @@ def reload_urlconf(urlconf=None): reload(sys.modules[urlconf]) clear_url_caches() + + +def update_user_settings(user, **kwargs): + usersettings = user.usersettings + settings = usersettings.settings + settings.update(kwargs) + usersettings.settings = settings + usersettings.save() + + +def delete_user_settings(user, *args): + usersettings = user.usersettings + settings = usersettings.settings + for setting in args: + if setting in settings: + del settings[setting] + usersettings.settings = settings + usersettings.save() diff --git a/helpdesk/tests/navigation.py b/helpdesk/tests/navigation.py index 32bbdb5c..878d38b3 100644 --- a/helpdesk/tests/navigation.py +++ b/helpdesk/tests/navigation.py @@ -4,7 +4,7 @@ from django.core.urlresolvers import reverse from django.test import TestCase from helpdesk import settings -from helpdesk.tests.helpers import get_staff_user, reload_urlconf, User +from helpdesk.tests.helpers import get_staff_user, reload_urlconf, User, update_user_settings, delete_user_settings class KBDisabledTestCase(TestCase): @@ -120,3 +120,70 @@ class CustomStaffUserTestCase(StaffUserTestCaseMixin, TestCase): self.client.login(username=user.username, password='frog') response = self.client.get(reverse('helpdesk_dashboard'), follow=True) self.assertTemplateUsed(response, 'helpdesk/registration/login.html') + + +class HomePageAnonymousUserTest(TestCase): + def setUp(self): + self.redirect_to_login = settings.HELPDESK_REDIRECT_TO_LOGIN_BY_DEFAULT + + def tearDown(self): + settings.HELPDESK_REDIRECT_TO_LOGIN_BY_DEFAULT = self.redirect_to_login + + def test_homepage(self): + settings.HELPDESK_REDIRECT_TO_LOGIN_BY_DEFAULT = True + response = self.client.get(reverse('helpdesk_home')) + self.assertTemplateUsed('helpdesk/public_homepage.html') + + def test_redirect_to_login(self): + """Unauthenticated users are redirected to the login page if HELPDESK_REDIRECT_TO_LOGIN_BY_DEFAULT is True""" + settings.HELPDESK_REDIRECT_TO_LOGIN_BY_DEFAULT = True + response = self.client.get(reverse('helpdesk_home')) + self.assertRedirects(response, reverse('login')) + + +class HomePageTest(TestCase): + def setUp(self): + self.previous = settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE, settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK + settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False + settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = None + try: + reload(sys.modules['helpdesk.views.public']) + except KeyError: + pass + + def tearDown(self): + settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE, settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = self.previous + reload(sys.modules['helpdesk.views.public']) + + def assertUserRedirectedToView(self, user, view_name): + self.client.login(username=user.username, password='password') + response = self.client.get(reverse('helpdesk_home')) + self.assertRedirects(response, reverse(view_name)) + self.client.logout() + + def test_redirect_to_dashboard(self): + """Authenticated users are redirected to the dashboard""" + user = get_staff_user() + + # login_view_ticketlist is False... + update_user_settings(user, login_view_ticketlist=False) + self.assertUserRedirectedToView(user, 'helpdesk_dashboard') + + # ... or missing + delete_user_settings(user, 'login_view_ticketlist') + self.assertUserRedirectedToView(user, 'helpdesk_dashboard') + + def test_no_user_settings_redirect_to_dashboard(self): + """Authenticated users are redirected to the dashboard if user settings are missing""" + from helpdesk.models import UserSettings + user = get_staff_user() + + UserSettings.objects.filter(user=user).delete() + self.assertUserRedirectedToView(user, 'helpdesk_dashboard') + + def test_redirect_to_ticket_list(self): + """Authenticated users are redirected to the ticket list based on their user settings""" + user = get_staff_user() + update_user_settings(user, login_view_ticketlist=True) + + self.assertUserRedirectedToView(user, 'helpdesk_list') diff --git a/helpdesk/views/public.py b/helpdesk/views/public.py index e164f059..e65f9f96 100644 --- a/helpdesk/views/public.py +++ b/helpdesk/views/public.py @@ -14,6 +14,7 @@ from django.template import loader, Context, RequestContext from django.utils.translation import ugettext as _ from helpdesk import settings as helpdesk_settings +from helpdesk.decorators import is_helpdesk_staff from helpdesk.forms import PublicTicketForm from helpdesk.lib import send_templated_mail, text_is_spam from helpdesk.models import Ticket, Queue, UserSettings, KBCategory @@ -23,14 +24,13 @@ def homepage(request): if not request.user.is_authenticated() and helpdesk_settings.HELPDESK_REDIRECT_TO_LOGIN_BY_DEFAULT: return HttpResponseRedirect(reverse('login')) - if (request.user.is_staff or (request.user.is_authenticated() and helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE)): + if is_helpdesk_staff(request.user): try: - if getattr(request.user.usersettings.settings, 'login_view_ticketlist', False): + if request.user.usersettings.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')) + pass + return HttpResponseRedirect(reverse('helpdesk_dashboard')) if request.method == 'POST': form = PublicTicketForm(request.POST, request.FILES) From ccec2634526f09e3ba6daaef4d25f763ae20a8c7 Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Tue, 29 Jul 2014 22:41:09 +0200 Subject: [PATCH 07/15] Also test is_helpdesk_staff --- helpdesk/tests/navigation.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/helpdesk/tests/navigation.py b/helpdesk/tests/navigation.py index 878d38b3..2af8cd29 100644 --- a/helpdesk/tests/navigation.py +++ b/helpdesk/tests/navigation.py @@ -73,8 +73,12 @@ class NonStaffUsersAllowedTestCase(StaffUserTestCaseMixin, TestCase): authenticated, non-staff users should be able to access the dashboard. """ + from helpdesk.decorators import is_helpdesk_staff + user = User.objects.create_user(username='henry.wensleydale', password='gouda', email='wensleydale@example.com') + self.assertTrue(is_helpdesk_staff(user)) + self.client.login(username=user.username, password='gouda') response = self.client.get(reverse('helpdesk_dashboard'), follow=True) self.assertTemplateUsed(response, 'helpdesk/dashboard.html') @@ -85,12 +89,24 @@ class StaffUsersOnlyTestCase(StaffUserTestCaseMixin, TestCase): HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = None + def test_non_staff(self): + """Non-staff users are correctly identified""" + from helpdesk.decorators import is_helpdesk_staff + + user = User.objects.create_user(username='henry.wensleydale', password='gouda', email='wensleydale@example.com') + + self.assertFalse(is_helpdesk_staff(user)) + def test_staff_only(self): """If HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE is False, only staff users should be able to access the dashboard. """ + from helpdesk.decorators import is_helpdesk_staff + user = get_staff_user() + self.assertTrue(is_helpdesk_staff(user)) + self.client.login(username=user.username, password='password') response = self.client.get(reverse('helpdesk_dashboard'), follow=True) self.assertTemplateUsed(response, 'helpdesk/dashboard.html') @@ -108,15 +124,23 @@ class CustomStaffUserTestCase(StaffUserTestCaseMixin, TestCase): """If HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK is not None, a custom access rule is applied. """ + from helpdesk.decorators import is_helpdesk_staff + user = User.objects.create_user(username='henry.wensleydale', password='gouda', email='wensleydale@example.com') + self.assertTrue(is_helpdesk_staff(user)) + self.client.login(username=user.username, password='gouda') response = self.client.get(reverse('helpdesk_dashboard'), follow=True) self.assertTemplateUsed(response, 'helpdesk/dashboard.html') def test_custom_staff_fail(self): + from helpdesk.decorators import is_helpdesk_staff + user = User.objects.create_user(username='terry.milton', password='frog', email='milton@example.com') + self.assertFalse(is_helpdesk_staff(user)) + self.client.login(username=user.username, password='frog') response = self.client.get(reverse('helpdesk_dashboard'), follow=True) self.assertTemplateUsed(response, 'helpdesk/registration/login.html') From c0dd6ea075aba9e01b6b3ffc490fc3e91f464894 Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Wed, 30 Jul 2014 06:07:08 +0200 Subject: [PATCH 08/15] test helpdesk.views.staff.return_to_ticket --- helpdesk/tests/helpers.py | 17 +++++++++++++++++ helpdesk/tests/navigation.py | 25 ++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/helpdesk/tests/helpers.py b/helpdesk/tests/helpers.py index ba73e990..9d0a9a00 100644 --- a/helpdesk/tests/helpers.py +++ b/helpdesk/tests/helpers.py @@ -7,6 +7,8 @@ except ImportError: else: User = get_user_model() +from helpdesk.models import Ticket, Queue + def get_staff_user(username='helpdesk.staff', password='password'): try: @@ -49,3 +51,18 @@ def delete_user_settings(user, *args): del settings[setting] usersettings.settings = settings usersettings.save() + + +def create_ticket(**kwargs): + q = kwargs.get('queue', None) + if q is None: + try: + q = Queue.objects.all()[0] + except IndexError: + q = Queue.objects.create(title='Test Q', slug='test', ) + data = { + 'title': "I wish to register a complaint", + 'queue': q, + } + data.update(kwargs) + return Ticket.objects.create(**data) \ No newline at end of file diff --git a/helpdesk/tests/navigation.py b/helpdesk/tests/navigation.py index 2af8cd29..88986d69 100644 --- a/helpdesk/tests/navigation.py +++ b/helpdesk/tests/navigation.py @@ -4,7 +4,8 @@ from django.core.urlresolvers import reverse from django.test import TestCase from helpdesk import settings -from helpdesk.tests.helpers import get_staff_user, reload_urlconf, User, update_user_settings, delete_user_settings +from helpdesk.tests.helpers import (get_staff_user, reload_urlconf, User, update_user_settings, delete_user_settings, + create_ticket) class KBDisabledTestCase(TestCase): @@ -146,7 +147,7 @@ class CustomStaffUserTestCase(StaffUserTestCaseMixin, TestCase): self.assertTemplateUsed(response, 'helpdesk/registration/login.html') -class HomePageAnonymousUserTest(TestCase): +class HomePageAnonymousUserTestCase(TestCase): def setUp(self): self.redirect_to_login = settings.HELPDESK_REDIRECT_TO_LOGIN_BY_DEFAULT @@ -165,7 +166,7 @@ class HomePageAnonymousUserTest(TestCase): self.assertRedirects(response, reverse('login')) -class HomePageTest(TestCase): +class HomePageTestCase(TestCase): def setUp(self): self.previous = settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE, settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False @@ -211,3 +212,21 @@ class HomePageTest(TestCase): update_user_settings(user, login_view_ticketlist=True) self.assertUserRedirectedToView(user, 'helpdesk_list') + + +class ReturnToTicketTestCase(TestCase): + def test_staff_user(self): + from helpdesk.views.staff import return_to_ticket + + user = get_staff_user() + ticket = create_ticket() + response = return_to_ticket(user, settings, ticket) + self.assertEqual(response['location'], ticket.get_absolute_url()) + + def test_non_staff_user(self): + from helpdesk.views.staff import return_to_ticket + + user = User.objects.create_user(username='henry.wensleydale', password='gouda', email='wensleydale@example.com') + ticket = create_ticket() + response = return_to_ticket(user, settings, ticket) + self.assertEqual(response['location'], ticket.ticket_url) From b7e8ae5ee5198354942a70e3c81d9cb7f8b95ad2 Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Wed, 30 Jul 2014 06:35:52 +0200 Subject: [PATCH 09/15] validate that HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK is callable, if set --- helpdesk/settings.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/helpdesk/settings.py b/helpdesk/settings.py index f0492a97..dfa1b413 100644 --- a/helpdesk/settings.py +++ b/helpdesk/settings.py @@ -4,6 +4,7 @@ Default settings for django-helpdesk. """ import warnings from django.conf import settings +from django.core.exceptions import ImproperlyConfigured try: @@ -66,6 +67,8 @@ HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = getattr(settings, 'HELPDESK_ALLOW_NON_S # apply a custom authorisation logic when defining 'helpdesk_staff_member_required' in staff.py. HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = getattr(settings, 'HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK', None) +if not (HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK is None or callable(HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK)): + raise ImproperlyConfigured("HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK must be a callable or None") if HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK and HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: warnings.warn( "The HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE and HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK settings cannot be both defined. " From 14d72798441a8b30cdf3d62099becdfe11c4bb18 Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Wed, 30 Jul 2014 06:55:43 +0200 Subject: [PATCH 10/15] test non-public update by non-staff user is blocked --- helpdesk/tests/ticket_submission.py | 30 ++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/helpdesk/tests/ticket_submission.py b/helpdesk/tests/ticket_submission.py index 785f7ec1..078a3939 100644 --- a/helpdesk/tests/ticket_submission.py +++ b/helpdesk/tests/ticket_submission.py @@ -1,9 +1,12 @@ -from helpdesk.models import Queue, CustomField, Ticket from django.test import TestCase from django.core import mail from django.test.client import Client from django.core.urlresolvers import reverse +from helpdesk.models import Queue, CustomField, Ticket +from helpdesk.tests.helpers import create_ticket, get_staff_user, User + + class TicketBasicsTestCase(TestCase): def setUp(self): self.queue_public = Queue.objects.create(title='Queue 1', slug='q1', allow_public_submission=True, new_ticket_cc='new.public@example.com', updated_ticket_cc='update.public@example.com') @@ -22,7 +25,7 @@ class TicketBasicsTestCase(TestCase): ticket = Ticket.objects.create(**ticket_data) self.assertEqual(ticket.ticket_for_url, "q1-%s" % ticket.id) self.assertEqual(email_count, len(mail.outbox)) - + def test_create_ticket_public(self): email_count = len(mail.outbox) @@ -46,7 +49,7 @@ class TicketBasicsTestCase(TestCase): self.assertEqual(last_redirect_url.split('?')[0], 'http://testserver%s' % reverse('helpdesk_public_view')) # Ensure submitter, new-queue + update-queue were all emailed. self.assertEqual(email_count+3, len(mail.outbox)) - + def test_create_ticket_private(self): email_count = len(mail.outbox) post_data = { @@ -85,3 +88,24 @@ class TicketBasicsTestCase(TestCase): self.assertEqual(last_redirect_url.split('?')[0], 'http://testserver%s' % reverse('helpdesk_public_view')) # Ensure only two e-mails were sent - submitter & updated. self.assertEqual(email_count+2, len(mail.outbox)) + + +class TicketUpdateTestCase(TestCase): + def setUp(self): + from helpdesk.tests.helpers import create_ticket + + self.ticket = create_ticket() + self.update_url = reverse('helpdesk_update', kwargs={'ticket_id': self.ticket.pk}) + + def test_not_allowed(self): + data = { + 'title': self.ticket.title, + 'queue': self.ticket.queue.pk, + } + response = self.client.post(self.update_url, data=data) + self.assertRedirects(response, '%s?next=%s' % (reverse('login'), self.update_url)) + + user = User.objects.create_user(username='henry.wensleydale', password='gouda', email='wensleydale@example.com') + self.client.login(username=user.username, password='gouda') + response = self.client.post(self.update_url, data=data) + self.assertRedirects(response, '%s?next=%s' % (reverse('login'), self.update_url)) From b1b89d1d6f7da958db374d88d703f368dd5b90e7 Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Wed, 30 Jul 2014 06:58:57 +0200 Subject: [PATCH 11/15] replace all explicit uses of HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE in helpdesk.views.staff --- docs/settings.rst | 5 ++--- helpdesk/views/staff.py | 11 +++++------ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/docs/settings.rst b/docs/settings.rst index ea57a3ba..b5dfdaab 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -86,12 +86,11 @@ These options only change display of items on public-facing pages, not staff pag Options that change ticket updates ---------------------------------- -- **HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE** Allow non-staff users to interact with tickets? This will also change how 'staff_member_required' - in staff.py will be defined. +- **HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE** Allow non-staff users to interact with tickets? **Default:** ``HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False`` -- **HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK** Apply a custom authorisation logic when defining 'staff_member_required' in staff.py. +- **HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK** Apply a custom authorisation logic for identifying helpdesk staff members. If set, `HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE` will be ignored when determining staff access. The value should be a function accepting the active user as a parameter and returning True if the user is considered helpdesk staff, e.g. diff --git a/helpdesk/views/staff.py b/helpdesk/views/staff.py index 99a9a1dd..eeca737d 100644 --- a/helpdesk/views/staff.py +++ b/helpdesk/views/staff.py @@ -32,7 +32,7 @@ try: except ImportError: from datetime import datetime as timezone -from helpdesk.decorators import helpdesk_staff_member_required, helpdesk_superuser_required +from helpdesk.decorators import helpdesk_staff_member_required, helpdesk_superuser_required, is_helpdesk_staff from helpdesk.forms import TicketForm, UserSettingsForm, EmailIgnoreForm, EditTicketForm, TicketCCForm, EditFollowUpForm, TicketDependencyForm from helpdesk.lib import send_templated_mail, query_to_dict, apply_query, safe_template_context from helpdesk.models import Ticket, Queue, FollowUp, TicketChange, PreSetReply, Attachment, SavedSearch, IgnoreEmail, TicketCC, TicketDependency @@ -292,8 +292,7 @@ def subscribe_staff_member_to_ticket(ticket, user): def update_ticket(request, ticket_id, public=False): - if not (public or (request.user.is_authenticated() and request.user.is_active and ( - request.user.is_staff or helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE))): + if not (public or is_helpdesk_staff(request.user)): return HttpResponseRedirect('%s?next=%s' % (reverse('login'), request.path)) ticket = get_object_or_404(Ticket, id=ticket_id) @@ -344,7 +343,7 @@ def update_ticket(request, ticket_id, public=False): f = FollowUp(ticket=ticket, date=timezone.now(), comment=comment) - if request.user.is_staff or helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: + if is_helpdesk_staff(request.user): f.user = request.user f.public = public @@ -535,9 +534,9 @@ def update_ticket(request, ticket_id, public=False): def return_to_ticket(user, helpdesk_settings, ticket): - ''' Helpder function for update_ticket ''' + """ Helper function for update_ticket """ - if user.is_staff or helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: + if is_helpdesk_staff(user): return HttpResponseRedirect(ticket.get_absolute_url()) else: return HttpResponseRedirect(ticket.ticket_url) From 05647d75adc4a61844d99cb067a8601b9b628315 Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Wed, 30 Jul 2014 13:25:57 +0200 Subject: [PATCH 12/15] remove unused helpdesk_settings alias --- helpdesk/decorators.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/helpdesk/decorators.py b/helpdesk/decorators.py index 1226bf0d..0bc96b62 100644 --- a/helpdesk/decorators.py +++ b/helpdesk/decorators.py @@ -1,11 +1,11 @@ # -*- coding: utf-8 -*- from django.contrib.auth.decorators import user_passes_test -from helpdesk import settings as helpdesk_settings +from helpdesk import settings -if helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK: +if settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK: # apply a custom user validation condition - is_helpdesk_staff = helpdesk_settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK -elif helpdesk_settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: + is_helpdesk_staff = settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK +elif settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: # treat 'normal' users like 'staff' is_helpdesk_staff = lambda u: u.is_authenticated() and u.is_active else: From 4ed8f1175486f086551e628e729e3aed67c2ebed Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Fri, 1 Aug 2014 07:20:43 +0200 Subject: [PATCH 13/15] merge HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK into HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE --- docs/settings.rst | 12 ++++-------- helpdesk/decorators.py | 4 ++-- helpdesk/settings.py | 15 ++++----------- helpdesk/tests/navigation.py | 21 ++++++++------------- 4 files changed, 18 insertions(+), 34 deletions(-) diff --git a/docs/settings.rst b/docs/settings.rst index b5dfdaab..4209dd05 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -87,17 +87,13 @@ Options that change ticket updates ---------------------------------- - **HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE** Allow non-staff users to interact with tickets? - - **Default:** ``HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False`` - -- **HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK** Apply a custom authorisation logic for identifying helpdesk staff members. - If set, `HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE` will be ignored when determining staff access. - The value should be a function accepting the active user as a parameter and returning True if the user is considered helpdesk - staff, e.g. + Set to True to allow any authenticated user to manage tickets. + You can also apply a custom authorisation logic for identifying helpdesk staff members, by setting this to a callable. + In that case, the value should be a function accepting the active user as a parameter and returning True if the user is considered helpdesk staff, e.g. lambda u: u.is_authenticated() and u.is_active and u.groups.filter(name='helpdesk_staff').exists())) - **Default:** ``HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = None`` + **Default:** ``HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False`` - **HELPDESK_SHOW_EDIT_BUTTON_FOLLOW_UP** Show edit buttons in ticket follow ups? diff --git a/helpdesk/decorators.py b/helpdesk/decorators.py index 0bc96b62..5baed791 100644 --- a/helpdesk/decorators.py +++ b/helpdesk/decorators.py @@ -2,9 +2,9 @@ from django.contrib.auth.decorators import user_passes_test from helpdesk import settings -if settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK: +if callable(settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE): # apply a custom user validation condition - is_helpdesk_staff = settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK + is_helpdesk_staff = settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE elif settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: # treat 'normal' users like 'staff' is_helpdesk_staff = lambda u: u.is_authenticated() and u.is_active diff --git a/helpdesk/settings.py b/helpdesk/settings.py index dfa1b413..a44ee876 100644 --- a/helpdesk/settings.py +++ b/helpdesk/settings.py @@ -61,18 +61,11 @@ HELPDESK_SUBMIT_A_TICKET_PUBLIC = getattr(settings, 'HELPDESK_SUBMIT_A_TICKET_PU ''' options for update_ticket views ''' -# allow non-staff users to interact with tickets? this will also change how 'helpdesk_staff_member_required' -# in staff.py will be defined. +# allow non-staff users to interact with tickets? +# can be True/False or a callable accepting the active user and returning True if they must be considered helpdesk staff HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = getattr(settings, 'HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE', False) - -# apply a custom authorisation logic when defining 'helpdesk_staff_member_required' in staff.py. -HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = getattr(settings, 'HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK', None) -if not (HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK is None or callable(HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK)): - raise ImproperlyConfigured("HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK must be a callable or None") -if HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK and HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE: - warnings.warn( - "The HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE and HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK settings cannot be both defined. " - "Only HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK will be considered in determining staff access.", RuntimeWarning) +if not (HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE in (True, False) or callable(HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE)): + warnings.warn("HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE should be set to either True/False or a callable.", RuntimeWarning) # show edit buttons in ticket follow ups. HELPDESK_SHOW_EDIT_BUTTON_FOLLOW_UP = getattr(settings, 'HELPDESK_SHOW_EDIT_BUTTON_FOLLOW_UP', True) diff --git a/helpdesk/tests/navigation.py b/helpdesk/tests/navigation.py index 88986d69..96460088 100644 --- a/helpdesk/tests/navigation.py +++ b/helpdesk/tests/navigation.py @@ -39,16 +39,14 @@ class KBDisabledTestCase(TestCase): class StaffUserTestCaseMixin(object): HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False - HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = None def setUp(self): - self.old_settings = settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE, settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK + self.original_setting = settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = self.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE - settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = self.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK self.reload_views() def tearDown(self): - settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE, settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = self.old_settings + settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = self.original_setting self.reload_views() def reload_views(self): @@ -67,7 +65,6 @@ class StaffUserTestCaseMixin(object): class NonStaffUsersAllowedTestCase(StaffUserTestCaseMixin, TestCase): HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = True - HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = None def test_non_staff_allowed(self): """If HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE is True, @@ -88,7 +85,6 @@ class NonStaffUsersAllowedTestCase(StaffUserTestCaseMixin, TestCase): class StaffUsersOnlyTestCase(StaffUserTestCaseMixin, TestCase): # Use default values HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False - HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = None def test_non_staff(self): """Non-staff users are correctly identified""" @@ -114,15 +110,15 @@ class StaffUsersOnlyTestCase(StaffUserTestCaseMixin, TestCase): class CustomStaffUserTestCase(StaffUserTestCaseMixin, TestCase): - HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False - @staticmethod - def HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK(user): + def custom_staff_filter(user): """Arbitrary user validation function""" return user.is_authenticated() and user.is_active and user.username.lower().endswith('wensleydale') + HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = custom_staff_filter + def test_custom_staff_pass(self): - """If HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK is not None, + """If HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE is callable, a custom access rule is applied. """ from helpdesk.decorators import is_helpdesk_staff @@ -168,16 +164,15 @@ class HomePageAnonymousUserTestCase(TestCase): class HomePageTestCase(TestCase): def setUp(self): - self.previous = settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE, settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK + self.original_setting = settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = False - settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = None try: reload(sys.modules['helpdesk.views.public']) except KeyError: pass def tearDown(self): - settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE, settings.HELPDESK_CUSTOM_STAFF_FILTER_CALLBACK = self.previous + settings.HELPDESK_ALLOW_NON_STAFF_TICKET_UPDATE = self.original_setting reload(sys.modules['helpdesk.views.public']) def assertUserRedirectedToView(self, user, view_name): From 8be4480a3fbec04bc6710f68338c4552290212de Mon Sep 17 00:00:00 2001 From: Stefano Brentegani Date: Fri, 1 Aug 2014 09:25:43 +0200 Subject: [PATCH 14/15] is_helpdesk_staff template filter (applied in navigation.html) --- helpdesk/templates/helpdesk/navigation.html | 4 ++-- helpdesk/templatetags/helpdesk_staff.py | 22 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 helpdesk/templatetags/helpdesk_staff.py diff --git a/helpdesk/templates/helpdesk/navigation.html b/helpdesk/templates/helpdesk/navigation.html index 05fa27dd..14c8cf16 100644 --- a/helpdesk/templates/helpdesk/navigation.html +++ b/helpdesk/templates/helpdesk/navigation.html @@ -1,4 +1,4 @@ -{% load i18n %}{% load url from future %} +{% load i18n helpdesk_staff %}{% load url from future %}