Add ability to restrict users' access by queue if so configured, and migration and tests for same.

This commit is contained in:
Jonathan Barratt 2015-06-11 17:18:45 +07:00
parent 9807356e84
commit bfcd8fc024
9 changed files with 361 additions and 17 deletions

View File

@ -61,6 +61,7 @@ If a user is a staff member, they get general helpdesk access, including:
6. Assign tickets to themselves or other staff members
7. Resolve tickets
Optionally, their access to view tickets, both on the dashboard and through searches and reports, may be restricted by a list of queues to which they have been granted membership. Create and update permissions for individual tickets are not limited by this optional restriction.
Licensing
---------

View File

@ -120,6 +120,14 @@ Staff Ticket Creation Settings
**Default:** ``HELPDESK_CREATE_TICKET_HIDE_ASSIGNED_TO = False``
Staff Ticket View Settings
------------------------------
- **HELPDESK_ENABLE_PER_QUEUE_MEMBERSHIP** Only show queues to which staff users have specifically been granted access to in the dashboard, ticket query, and ticket report views? Note that this does not prevent them from creating new tickets in any queue, or from editing existing tickets in other queues if they know the ticket ID or otherwise access the ticket URL directly. It is meant to keep work loads segregated for staff convenience, not to prevent malicious behavior. It also does not limit superuser accounts regardless of whatever gueue memberships they have or have not been granted.
**Default:** ``HELPDESK_ENABLE_PER_QUEUE_MEMBERSHIP = False``
Default E-Mail Settings
-----------------------

View File

@ -1,8 +1,12 @@
from django.contrib import admin
from django.contrib.auth import get_user_model
from django.contrib.auth.admin import UserAdmin
from helpdesk.models import Queue, Ticket, FollowUp, PreSetReply, KBCategory
from helpdesk.models import EscalationExclusion, EmailTemplate, KBItem
from helpdesk.models import TicketChange, Attachment, IgnoreEmail
from helpdesk.models import CustomField
from helpdesk.models import QueueMembership
from helpdesk import settings as helpdesk_settings
class QueueAdmin(admin.ModelAdmin):
list_display = ('title', 'slug', 'email_address', 'locale')
@ -24,7 +28,7 @@ class FollowUpAdmin(admin.ModelAdmin):
class KBItemAdmin(admin.ModelAdmin):
list_display = ('category', 'title', 'last_updated',)
list_display_links = ('title',)
class CustomFieldAdmin(admin.ModelAdmin):
list_display = ('name', 'label', 'data_type')
@ -32,6 +36,18 @@ class EmailTemplateAdmin(admin.ModelAdmin):
list_display = ('template_name', 'heading', 'locale')
list_filter = ('locale', )
class QueueMembershipInline(admin.StackedInline):
model = QueueMembership
class UserAdminWithQueueMemberships(UserAdmin):
def change_view(self, request, object_id, form_url='', extra_context=None):
self.inlines = (QueueMembershipInline,)
return super(UserAdminWithQueueMemberships, self).change_view(
request, object_id, form_url=form_url, extra_context=extra_context)
admin.site.register(Ticket, TicketAdmin)
admin.site.register(Queue, QueueAdmin)
admin.site.register(FollowUp, FollowUpAdmin)
@ -42,3 +58,6 @@ admin.site.register(KBCategory)
admin.site.register(KBItem, KBItemAdmin)
admin.site.register(IgnoreEmail)
admin.site.register(CustomField, CustomFieldAdmin)
if helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP:
admin.site.unregister(get_user_model())
admin.site.register(get_user_model(), UserAdminWithQueueMemberships)

View File

@ -0,0 +1,29 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import models, migrations
from django.conf import settings
class Migration(migrations.Migration):
dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('helpdesk', '0004_initial_data_import'),
]
operations = [
migrations.CreateModel(
name='QueueMembership',
fields=[
('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
('queues', models.ManyToManyField(to='helpdesk.Queue', verbose_name='Authorized Queues')),
('user', models.OneToOneField(verbose_name='User', to=settings.AUTH_USER_MODEL)),
],
options={
'verbose_name': 'Queue Membership',
'verbose_name_plural': 'Queue Memberships',
},
bases=(models.Model,),
),
]

View File

@ -1359,3 +1359,25 @@ class TicketDependency(models.Model):
unique_together = ('ticket', 'depends_on')
verbose_name = _('Ticket dependency')
verbose_name_plural = _('Ticket dependencies')
class QueueMembership(models.Model):
"""
Used to restrict staff members to certain queues only
"""
user = models.OneToOneField(
settings.AUTH_USER_MODEL,
verbose_name=_('User'),
)
queues = models.ManyToManyField(
Queue,
verbose_name=_('Authorized Queues'),
)
def __unicode__(self):
return '%s authorized for queues %s' % (self.user, ", ".join(self.queues.values_list('title', flat=True)))
class Meta:
verbose_name = _('Queue Membership')
verbose_name_plural = _('Queue Memberships')

View File

@ -60,7 +60,7 @@ 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)
@ -73,10 +73,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)
@ -97,3 +97,7 @@ QUEUE_EMAIL_BOX_SSL = getattr(settings, 'QUEUE_EMAIL_BOX_SSL', None)
QUEUE_EMAIL_BOX_HOST = getattr(settings, 'QUEUE_EMAIL_BOX_HOST', None)
QUEUE_EMAIL_BOX_USER = getattr(settings, 'QUEUE_EMAIL_BOX_USER', None)
QUEUE_EMAIL_BOX_PASSWORD = getattr(settings, 'QUEUE_EMAIL_BOX_PASSWORD', None)
# only allow users to access queues that they are members of?
HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP = getattr(settings, 'HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP', False)

View File

@ -1,3 +1,4 @@
from helpdesk.tests.ticket_submission import *
from helpdesk.tests.public_actions import *
from helpdesk.tests.navigation import *
from helpdesk.tests.navigation import *
from helpdesk.tests.per_queue_staff_membership import *

View File

@ -0,0 +1,221 @@
from django.contrib.auth import get_user_model
from django.core.urlresolvers import reverse
from django.test import TestCase
from django.test.client import Client
from helpdesk.models import Queue, Ticket, QueueMembership
from helpdesk import settings
class PerQueueStaffMembershipTestCase(TestCase):
IDENTIFIERS = (1, 2)
def setUp(self):
"""
Create user_1 with access to queue_1 containing 1 ticket
and user_2 with access to queue_2 containing 2 tickets
and superuser who should be able to access both queues
"""
settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP = True
self.client = Client()
User = get_user_model()
self.superuser = User.objects.create(
username='superuser',
is_staff=True,
is_superuser=True,
)
self.superuser.set_password('superuser')
self.superuser.save()
for identifier in self.IDENTIFIERS:
queue = self.__dict__['queue_%d' % identifier] = Queue.objects.create(
title='Queue %d' % identifier,
slug='q%d' % identifier,
)
user = self.__dict__['user_%d' % identifier] = User.objects.create(
username='User_%d' % identifier,
is_staff=True,
)
user.set_password(identifier)
user.save()
queue_membership = self.__dict__['queue_membership_%d' % identifier] = QueueMembership.objects.create(
user=user,
)
queue_membership.queues = (queue,)
queue_membership.save()
for ticket_number in range(1, identifier + 1):
Ticket.objects.create(
title='Unassigned Ticket %d in Queue %d' % (ticket_number, identifier),
queue=queue,
)
Ticket.objects.create(
title='Ticket %d in Queue %d Assigned to User_%d' % (ticket_number, identifier, identifier),
queue=queue,
assigned_to=user,
)
def test_dashboard_ticket_counts(self):
"""
Check that the regular users' dashboard only shows 1 of the 2 queues,
that user_1 only sees a total of 1 ticket, that user_2 sees a total of 2
tickets, but that the superuser's dashboard shows all queues and tickets.
"""
# Regular users
for identifier in self.IDENTIFIERS:
self.client.login(username='User_%d' % identifier, password=identifier)
response = self.client.get(reverse('helpdesk_dashboard'))
self.assertEqual(
len(response.context['unassigned_tickets']),
identifier,
'Unassigned tickets were not properly limited by queue membership'
)
self.assertEqual(
len(response.context['dash_tickets']),
1,
'The queues in dash_tickets were not properly limited by queue membership'
)
self.assertEqual(
response.context['dash_tickets'][0]['open'],
identifier * 2,
'The tickets in dash_tickets were not properly limited by queue membership'
)
self.assertEqual(
response.context['basic_ticket_stats']['open_ticket_stats'][0][1],
identifier * 2,
'Basic ticket stats were not properly limited by queue membership'
)
# Superuser
self.client.login(username='superuser', password='superuser')
response = self.client.get(reverse('helpdesk_dashboard'))
self.assertEqual(
len(response.context['unassigned_tickets']),
3,
'Unassigned tickets were limited by queue membership for a superuser'
)
self.assertEqual(
len(response.context['dash_tickets']),
2,
'The queues in dash_tickets were limited by queue membership for a superuser'
)
self.assertEqual(
response.context['dash_tickets'][0]['open'] +
response.context['dash_tickets'][1]['open'],
6,
'The tickets in dash_tickets were limited by queue membership for a superuser'
)
self.assertEqual(
response.context['basic_ticket_stats']['open_ticket_stats'][0][1] +
response.context['basic_ticket_stats']['open_ticket_stats'][1][1],
6,
'Basic ticket stats were limited by queue membership for a superuser'
)
def test_ticket_list_per_queue_user_restrictions(self):
"""
Ensure that while the superuser can list all tickets, user_1 can only
list the 1 ticket in his queue and user_2 can list only the 2 tickets
in his queue.
"""
# Regular users
for identifier in self.IDENTIFIERS:
self.client.login(username='User_%d' % identifier, password=identifier)
response = self.client.get(reverse('helpdesk_list'))
self.assertEqual(
len(response.context['tickets']),
identifier * 2,
'Ticket list was not properly limited by queue membership'
)
self.assertEqual(
len(response.context['queue_choices']),
1,
'Queue choices were not properly limited by queue membership'
)
self.assertEqual(
response.context['queue_choices'][0],
Queue.objects.get(title="Queue %d" % identifier),
'Queue choices were not properly limited by queue membership'
)
# Superuser
self.client.login(username='superuser', password='superuser')
response = self.client.get(reverse('helpdesk_list'))
self.assertEqual(
len(response.context['tickets']),
6,
'Ticket list was limited by queue membership for a superuser'
)
def test_ticket_reports_per_queue_user_restrictions(self):
"""
Ensure that while the superuser can generate reports on all queues and
tickets, user_1 can only generate reports for queue 1 and user_2 can
only do so for queue 2
"""
# Regular users
for identifier in self.IDENTIFIERS:
self.client.login(username='User_%d' % identifier, password=identifier)
response = self.client.get(
reverse('helpdesk_run_report', kwargs={'report': 'userqueue'})
)
# Only two columns of data should be present: ticket counts for
# unassigned and this user only
self.assertEqual(
len(response.context['data']),
2,
'Queues in report were not properly limited by queue membership'
)
# Each user should see a total number of tickets equal to twice their ID
self.assertEqual(
sum([sum(user_tickets[1:]) for user_tickets in response.context['data']]),
identifier * 2,
'Tickets in report were not properly limited by queue membership'
)
# Each user should only be able to pick 1 queue
self.assertEqual(
len(response.context['headings']),
2,
'Queue choices were not properly limited by queue membership'
)
# The queue each user can pick should be the queue named after their ID
self.assertEqual(
response.context['headings'][1],
"Queue %d" % identifier,
'Queue choices were not properly limited by queue membership'
)
# Superuser
self.client.login(username='superuser', password='superuser')
response = self.client.get(
reverse('helpdesk_run_report', kwargs={'report': 'userqueue'})
)
# Superuser should see ticket counts for all two queues, which includes
# three columns: unassigned and both user 1 and user 2
self.assertEqual(
len(response.context['data'][0]),
3,
'Queues in report were improperly limited by queue membership for a superuser'
)
# Superuser should see the total ticket count of three tickets
self.assertEqual(
sum([sum(user_tickets[1:]) for user_tickets in response.context['data']]),
6,
'Tickets in report were improperly limited by queue membership for a superuser'
)
self.assertEqual(
len(response.context['headings']),
3,
'Queue choices were improperly limited by queue membership for a superuser'
)
def tearDown(self):
"""
Don't interfere with subsequent tests that do not expect this setting
"""
settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP = False

View File

@ -76,6 +76,11 @@ def dashboard(request):
).exclude(
status=Ticket.CLOSED_STATUS,
)
limit_queues_by_user = helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP and not request.user.is_superuser
if limit_queues_by_user:
unassigned_tickets = unassigned_tickets.filter(
queue__in=request.user.queuemembership.queues.all(),
)
# all tickets, reported by current user
all_tickets_reported_by_current_user = ''
@ -85,7 +90,12 @@ def dashboard(request):
submitter_email=email_current_user,
).order_by('status')
basic_ticket_stats = calc_basic_ticket_stats(Ticket)
Tickets = Ticket.objects
if limit_queues_by_user:
Tickets = Tickets.filter(
queue__in=request.user.queuemembership.queues.all(),
)
basic_ticket_stats = calc_basic_ticket_stats(Tickets)
# The following query builds a grid of queues & ticket statuses,
# to be displayed to the user. EG:
@ -93,6 +103,17 @@ def dashboard(request):
# Queue 1 10 4
# Queue 2 4 12
from_clause = """FROM helpdesk_ticket t,
helpdesk_queue q"""
where_clause = """WHERE q.id = t.queue_id"""
if limit_queues_by_user:
from_clause = """%s,
helpdesk_queuemembership qm,
helpdesk_queuemembership_queues qm_queues""" % from_clause
where_clause = """%s AND
qm.user_id = %d AND
qm.id = qm_queues.queuemembership_id AND
q.id = qm_queues.queue_id""" % (where_clause, request.user.id)
cursor = connection.cursor()
cursor.execute("""
SELECT q.id as queue,
@ -100,12 +121,11 @@ def dashboard(request):
COUNT(CASE t.status WHEN '1' THEN t.id WHEN '2' THEN t.id END) AS open,
COUNT(CASE t.status WHEN '3' THEN t.id END) AS resolved,
COUNT(CASE t.status WHEN '4' THEN t.id END) AS closed
FROM helpdesk_ticket t,
helpdesk_queue q
WHERE q.id = t.queue_id
%s
%s
GROUP BY queue, name
ORDER BY q.id;
""")
""" % (from_clause, where_clause))
dash_tickets = query_to_dict(cursor.fetchall(), cursor.description)
@ -781,15 +801,24 @@ def ticket_list(request):
sortreverse = request.GET.get('sortreverse', None)
query_params['sortreverse'] = sortreverse
tickets = Ticket.objects.select_related()
queue_choices = Queue.objects.all()
if helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP and not request.user.is_superuser:
user_queues = request.user.queuemembership.queues.all()
tickets = tickets.filter(
queue__in=user_queues,
)
queue_choices = user_queues
try:
ticket_qs = apply_query(Ticket.objects.select_related(), query_params)
ticket_qs = apply_query(tickets, query_params)
except ValidationError:
# invalid parameters in query, return default query
query_params = {
'filtering': {'status__in': [1, 2, 3]},
'sorting': 'created',
}
ticket_qs = apply_query(Ticket.objects.select_related(), query_params)
ticket_qs = apply_query(tickets, query_params)
ticket_paginator = paginator.Paginator(ticket_qs, request.user.usersettings.settings.get('tickets_per_page') or 20)
try:
@ -826,7 +855,7 @@ def ticket_list(request):
query_string=querydict.urlencode(),
tickets=tickets,
user_choices=User.objects.filter(is_active=True,is_staff=True),
queue_choices=Queue.objects.all(),
queue_choices=queue_choices,
status_choices=Ticket.STATUS_CHOICES,
urlsafe_query=urlsafe_query,
user_saved_queries=user_saved_queries,
@ -960,6 +989,11 @@ def run_report(request, report):
return HttpResponseRedirect(reverse("helpdesk_report_index"))
report_queryset = Ticket.objects.all().select_related()
limit_queues_by_user = helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP and not request.user.is_superuser
if limit_queues_by_user:
report_queryset = report_queryset.filter(
queue__in=request.user.queuemembership.queues.all(),
)
from_saved_query = False
saved_query = None
@ -1019,7 +1053,12 @@ def run_report(request, report):
elif report == 'userqueue':
title = _('User by Queue')
col1heading = _('User')
possible_options = [q.title.encode('utf-8') for q in Queue.objects.all()]
queue_options = Queue.objects.all()
if limit_queues_by_user:
queue_options = queue_options.filter(
pk__in=request.user.queuemembership.queues.all(),
)
possible_options = [q.title.encode('utf-8') for q in queue_options]
charttype = 'bar'
elif report == 'userstatus':
@ -1307,9 +1346,9 @@ def calc_average_nbr_days_until_ticket_resolved(Tickets):
return mean_per_ticket
def calc_basic_ticket_stats(Ticket):
def calc_basic_ticket_stats(Tickets):
# all not closed tickets (open, reopened, resolved,) - independent of user
all_open_tickets = Ticket.objects.exclude(status = Ticket.CLOSED_STATUS)
all_open_tickets = Tickets.exclude(status = Ticket.CLOSED_STATUS)
today = datetime.today()
date_30 = date_rel_to_today(today, 30)
@ -1337,7 +1376,7 @@ def calc_basic_ticket_stats(Ticket):
ots.append(['> 60 days', N_ota_ge_60, get_color_for_nbr_days(N_ota_ge_60), sort_string('', date_60_str), ])
# all closed tickets - independent of user.
all_closed_tickets = Ticket.objects.filter(status = Ticket.CLOSED_STATUS)
all_closed_tickets = Tickets.filter(status = Ticket.CLOSED_STATUS)
average_nbr_days_until_ticket_closed = calc_average_nbr_days_until_ticket_resolved(all_closed_tickets)
# all closed tickets that were opened in the last 60 days.
all_closed_last_60_days = all_closed_tickets.filter(created__gte = date_60_str)