From 18c7a2e698e549ad0ff70acb0d33bb60d4a33a7a Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Wed, 16 Dec 2015 16:08:46 +0100 Subject: [PATCH 01/13] adding required model extra info for permissions --- .../migrations/0008_extra_for_permissions.py | 19 +++++++++++++++++++ helpdesk/models.py | 10 ++++++++++ 2 files changed, 29 insertions(+) create mode 100644 helpdesk/migrations/0008_extra_for_permissions.py diff --git a/helpdesk/migrations/0008_extra_for_permissions.py b/helpdesk/migrations/0008_extra_for_permissions.py new file mode 100644 index 00000000..ac6d1433 --- /dev/null +++ b/helpdesk/migrations/0008_extra_for_permissions.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('helpdesk', '0007_max_length_by_integer'), + ] + + operations = [ + migrations.AddField( + model_name='queue', + name='permission_name', + field=models.CharField(help_text='Name used in the django.contrib.auth permission system', max_length=50, null=True, verbose_name='Django auth permission name', blank=True), + ), + ] diff --git a/helpdesk/models.py b/helpdesk/models.py index 06e76d00..024be2b6 100644 --- a/helpdesk/models.py +++ b/helpdesk/models.py @@ -39,6 +39,7 @@ class Queue(models.Model): slug = models.SlugField( _('Slug'), + max_length=50, help_text=_('This slug is used when building ticket ID\'s. Once set, ' 'try not to change it or e-mailing may get messy.'), ) @@ -168,6 +169,15 @@ class Queue(models.Model): 'folders. Default: INBOX.'), ) + permission_name = models.CharField( + _('Django auth permission name'), + max_length=50, + blank=True, + null=True, + help_text=_('Name used in the django.contrib.auth permission system'), + ) + + email_box_interval = models.IntegerField( _('E-Mail Check Interval'), help_text=_('How often do you wish to check this mailbox? (in Minutes)'), From c1fd47043ac36247b9ce71ee1bef2302ce6e4328 Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Wed, 16 Dec 2015 15:06:06 +0100 Subject: [PATCH 02/13] migration for removing semantics from QueueMembership objects --- .../0009_migrate_queuemembership.py | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 helpdesk/migrations/0009_migrate_queuemembership.py diff --git a/helpdesk/migrations/0009_migrate_queuemembership.py b/helpdesk/migrations/0009_migrate_queuemembership.py new file mode 100644 index 00000000..62144de7 --- /dev/null +++ b/helpdesk/migrations/0009_migrate_queuemembership.py @@ -0,0 +1,56 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals +from django.db import migrations +from django.conf import settings +from django.utils.translation import ugettext_lazy as _ + + +def create_and_assign_permissions(apps, schema_editor): + + # If neither Permission nor Membership mechanism are enabled, ignore the migration + if not ((hasattr(settings, 'HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION') and + settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION) or + (hasattr(settings, 'HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP') and + settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP)): + return + + Permission = apps.get_model('auth', 'Permission') + ContentType = apps.get_model('contenttypes', 'ContentType') + # Otherwise, two steps: + # 1. Create the permission for existing Queues + # 2. Assign the permission to user according to QueueMembership objects + + # First step: prepare the permission for each queue + Queue = apps.get_model('helpdesk', 'Queue') + + for q in Queue.objects.all(): + # Prepare the permission associated to this Queue + basename = "queue_access_%s" % q.slug + q.permission_name = "helpdesk.%s" % basename + Permission.objects.create( + name=_("Permission for queue: ") + q.title, + content_type=ContentType.objects.get(model="queue"), + codename=basename, + ) + q.save() + + # Second step: map the permissions according to QueueMembership + QueueMembership = apps.get_model('helpdesk', 'QueueMembership') + for qm in QueueMembership.objects.all(): + user = qm.user + for q in qm.queues.all(): + # Strip the `helpdesk.` prefix + p = Permission.objects.get(codename=q.permission_name[9:]) + user.user_permissions.add(p) + qm.delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('helpdesk', '0008_extra_for_permissions'), + ] + + operations = [ + migrations.RunPython(create_and_assign_permissions) + ] From 230d14b3ca1b76fd8a28aed0c66097c88793c0a0 Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Wed, 16 Dec 2015 15:01:39 +0100 Subject: [PATCH 03/13] removing QueueMembership mechanisms from admin.py --- helpdesk/admin.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/helpdesk/admin.py b/helpdesk/admin.py index 901614c5..eeeb4346 100644 --- a/helpdesk/admin.py +++ b/helpdesk/admin.py @@ -1,12 +1,8 @@ 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') @@ -36,17 +32,6 @@ 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) @@ -58,6 +43,3 @@ 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) From fb66fea86ed17150406bf327afeafce0128c4e46 Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Wed, 16 Dec 2015 16:09:15 +0100 Subject: [PATCH 04/13] change basic features of membership into permissions --- helpdesk/models.py | 15 +++ helpdesk/settings.py | 2 +- ....py => test_per_queue_staff_permission.py} | 17 ++- helpdesk/views/staff.py | 110 +++++++++--------- 4 files changed, 81 insertions(+), 63 deletions(-) rename helpdesk/tests/{test_per_queue_staff_membership.py => test_per_queue_staff_permission.py} (93%) diff --git a/helpdesk/models.py b/helpdesk/models.py index 024be2b6..70cce880 100644 --- a/helpdesk/models.py +++ b/helpdesk/models.py @@ -8,6 +8,8 @@ models.py - Model (and hence database) definitions. This is the core of the """ from __future__ import unicode_literals +from django.contrib.auth.models import Permission +from django.contrib.contenttypes.models import ContentType from django.db import models from django.contrib.auth import get_user_model from django.conf import settings @@ -15,6 +17,8 @@ from django.utils.translation import ugettext_lazy as _, ugettext from django import VERSION from django.utils.encoding import python_2_unicode_compatible +from helpdesk import settings as helpdesk_settings + try: from django.utils import timezone except ImportError: @@ -258,6 +262,17 @@ class Queue(models.Model): self.email_box_port = 995 elif self.email_box_type == 'pop3' and not self.email_box_ssl: self.email_box_port = 110 + + if not self.id and helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION: + # Prepare the permission associated to this Queue + basename = "queue_access_%s" % self.slug + self.permission_name = "helpdesk.%s" % basename + Permission.objects.create( + name=_("Permission for queue: ") + self.title, + content_type=ContentType.objects.get(model="queue"), + codename=basename, + ) + super(Queue, self).save(*args, **kwargs) diff --git a/helpdesk/settings.py b/helpdesk/settings.py index 9771d202..c7993e65 100644 --- a/helpdesk/settings.py +++ b/helpdesk/settings.py @@ -100,4 +100,4 @@ 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) +HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION = getattr(settings, 'HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION', False) diff --git a/helpdesk/tests/test_per_queue_staff_membership.py b/helpdesk/tests/test_per_queue_staff_permission.py similarity index 93% rename from helpdesk/tests/test_per_queue_staff_membership.py rename to helpdesk/tests/test_per_queue_staff_permission.py index 65ff7605..8362307a 100644 --- a/helpdesk/tests/test_per_queue_staff_membership.py +++ b/helpdesk/tests/test_per_queue_staff_permission.py @@ -1,9 +1,10 @@ from django.contrib.auth import get_user_model +from django.contrib.auth.models import Permission 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.models import Queue, Ticket from helpdesk import settings @@ -17,8 +18,8 @@ class PerQueueStaffMembershipTestCase(TestCase): and user_2 with access to queue_2 containing 2 tickets and superuser who should be able to access both queues """ - self.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP = settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP - settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP = True + self.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION = settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION + settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION = True self.client = Client() User = get_user_model() @@ -43,11 +44,9 @@ class PerQueueStaffMembershipTestCase(TestCase): 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() + # The prefix 'helpdesk.' must be trimmed + p = Permission.objects.get(codename=queue.permission_name[9:]) + user.user_permissions.add(p) for ticket_number in range(1, identifier + 1): Ticket.objects.create( @@ -64,7 +63,7 @@ class PerQueueStaffMembershipTestCase(TestCase): """ Reset HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP to original value """ - settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP = self.HELPDESK_ENABLE_PER_QUEUE_STAFF_MEMBERSHIP + settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION = self.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION def test_dashboard_ticket_counts(self): """ diff --git a/helpdesk/views/staff.py b/helpdesk/views/staff.py index 8b52ca72..99ecc5bc 100644 --- a/helpdesk/views/staff.py +++ b/helpdesk/views/staff.py @@ -40,7 +40,7 @@ except ImportError: 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, QueueMembership +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: @@ -53,6 +53,34 @@ else: superuser_required = user_passes_test(lambda u: u.is_authenticated() and u.is_active and u.is_superuser) +def _get_user_queues(user): + """Return the list of Queues the user can access. + + :param user: The User (the class should have the has_perm method) + :return: A Python list of Queues + """ + all_queues = Queue.objects.all() + limit_queues_by_user = helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION and not user.is_superuser + if limit_queues_by_user: + id_list = [q.pk for q in all_queues if user.has_perm(q.permission_name)] + return all_queues.filter(pk__in=id_list) + else: + return all_queues + + +def _has_access_to_queue(user, queue): + """Check if a certain user can access a certain queue. + + :param user: The User (the class should have the has_perm method) + :param queue: The django-helpdesk Queue instance + :return: True if the user has permission (either by default or explicitly), false otherwise + """ + if user.is_superuser or not helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION: + return True + else: + return user.has_perm(queue.permission_name) + + def dashboard(request): """ A quick summary overview for users: A list of their own tickets, a table @@ -72,19 +100,14 @@ def dashboard(request): assigned_to=request.user, status__in = [Ticket.CLOSED_STATUS, Ticket.RESOLVED_STATUS]) + user_queues = _get_user_queues(request.user) + unassigned_tickets = Ticket.objects.select_related('queue').filter( assigned_to__isnull=True, + queue__in=user_queues ).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: - try: - unassigned_tickets = unassigned_tickets.filter( - queue__in=request.user.queuemembership.queues.all(), - ) - except QueueMembership.DoesNotExist: - unassigned_tickets = unassigned_tickets.none() # all tickets, reported by current user all_tickets_reported_by_current_user = '' @@ -94,14 +117,9 @@ def dashboard(request): submitter_email=email_current_user, ).order_by('status') - Tickets = Ticket.objects - if limit_queues_by_user: - try: - Tickets = Tickets.filter( - queue__in=request.user.queuemembership.queues.all(), + Tickets = Ticket.objects.filter( + queue__in=user_queues, ) - except QueueMembership.DoesNotExist: - Tickets = Tickets.none() basic_ticket_stats = calc_basic_ticket_stats(Tickets) # The following query builds a grid of queues & ticket statuses, @@ -110,17 +128,13 @@ def dashboard(request): # Queue 1 10 4 # Queue 2 4 12 + queues = _get_user_queues(request.user).values_list('id', flat=True) + 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) + where_clause = """WHERE q.id = t.queue_id AND + q.id IN (%s)""" % (",".join(("%d" % pk for pk in queues))) + cursor = connection.cursor() cursor.execute(""" SELECT q.id as queue, @@ -687,6 +701,10 @@ mass_update = staff_member_required(mass_update) def ticket_list(request): context = {} + user_queues = _get_user_queues(request.user) + # Prefilter the allowed tickets + base_tickets = Ticket.objects.filter(queue__in=user_queues) + # Query_params will hold a dictionary of parameters relating to # a query, to be saved if needed: query_params = { @@ -727,7 +745,7 @@ def ticket_list(request): if filter: try: - ticket = Ticket.objects.get(**filter) + ticket = base_tickets.get(**filter) return HttpResponseRedirect(ticket.staff_url) except Ticket.DoesNotExist: # Go on to standard keyword searching @@ -821,14 +839,7 @@ 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 + tickets = base_tickets.select_related() try: ticket_qs = apply_query(tickets, query_params) @@ -875,7 +886,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_choices, + queue_choices=user_queues, status_choices=Ticket.STATUS_CHOICES, urlsafe_query=urlsafe_query, user_saved_queries=user_saved_queries, @@ -889,6 +900,9 @@ ticket_list = staff_member_required(ticket_list) def edit_ticket(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) + if not _has_access_to_queue(request.user, ticket.queue): + return HttpResponseRedirect(reverse('helpdesk_dashboard')) + if request.method == 'POST': form = EditTicketForm(request.POST, instance=ticket) if form.is_valid(): @@ -915,7 +929,10 @@ def create_ticket(request): 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) - return HttpResponseRedirect(ticket.get_absolute_url()) + if _has_access_to_queue(request.user, ticket.queue): + return HttpResponseRedirect(ticket.get_absolute_url()) + else: + return HttpResponseRedirect(reverse('helpdesk_dashboard')) else: initial_data = {} if request.user.usersettings.settings.get('use_email_as_submitter', False) and request.user.email: @@ -1008,15 +1025,9 @@ 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")) - 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: - try: - report_queryset = report_queryset.filter( - queue__in=request.user.queuemembership.queues.all(), - ) - except QueueMembership.DoesNotExist: - report_queryset = report_queryset.none() + report_queryset = Ticket.objects.all().select_related().filter( + queue__in=_get_user_queues(request.user) + ) from_saved_query = False saved_query = None @@ -1076,14 +1087,7 @@ def run_report(request, report): elif report == 'userqueue': title = _('User by Queue') col1heading = _('User') - queue_options = Queue.objects.all() - if limit_queues_by_user: - try: - queue_options = queue_options.filter( - pk__in=request.user.queuemembership.queues.all(), - ) - except QueueMembership.DoesNotExist: - queue_options = queue_options.none() + queue_options = _get_user_queues(request.user) possible_options = [q.title for q in queue_options] charttype = 'bar' From d760d9ee95906b77239c0cba02fce877c7f49e1f Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Wed, 16 Dec 2015 15:02:13 +0100 Subject: [PATCH 05/13] adding basic information to tickets, and masking email, on the admin views --- helpdesk/admin.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/helpdesk/admin.py b/helpdesk/admin.py index eeeb4346..e0c09d9e 100644 --- a/helpdesk/admin.py +++ b/helpdesk/admin.py @@ -1,4 +1,5 @@ from django.contrib import admin +from django.utils.translation import ugettext_lazy as _ from helpdesk.models import Queue, Ticket, FollowUp, PreSetReply, KBCategory from helpdesk.models import EscalationExclusion, EmailTemplate, KBItem from helpdesk.models import TicketChange, Attachment, IgnoreEmail @@ -8,9 +9,19 @@ class QueueAdmin(admin.ModelAdmin): list_display = ('title', 'slug', 'email_address', 'locale') class TicketAdmin(admin.ModelAdmin): - list_display = ('title', 'status', 'assigned_to', 'submitter_email',) + list_display = ('title', 'status', 'assigned_to', 'queue', 'hidden_submitter_email',) date_hierarchy = 'created' - list_filter = ('assigned_to', 'status', ) + list_filter = ('queue', 'assigned_to', 'status') + + def hidden_submitter_email(self, ticket): + if ticket.submitter_email: + username, domain = ticket.submitter_email.split("@") + username = username[:2] + "*" * (len(username) - 2) + domain = domain[:1] + "*" * (len(domain) - 2) + domain[-1:] + return "%s@%s" % (username, domain) + else: + return ticket.submitter_email + hidden_submitter_email.short_description = _('Submitter E-Mail') class TicketChangeInline(admin.StackedInline): model = TicketChange From 64bc1791d45ff96491a0909a6d9728d1bdf3eb6d Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Wed, 16 Dec 2015 15:06:19 +0100 Subject: [PATCH 06/13] removing completely QueueMembership model --- .../migrations/0010_remove_queuemembership.py | 30 +++++++++++++++++++ helpdesk/models.py | 23 -------------- 2 files changed, 30 insertions(+), 23 deletions(-) create mode 100644 helpdesk/migrations/0010_remove_queuemembership.py diff --git a/helpdesk/migrations/0010_remove_queuemembership.py b/helpdesk/migrations/0010_remove_queuemembership.py new file mode 100644 index 00000000..a43d467b --- /dev/null +++ b/helpdesk/migrations/0010_remove_queuemembership.py @@ -0,0 +1,30 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('helpdesk', '0009_migrate_queuemembership'), + ] + + operations = [ + migrations.RemoveField( + model_name='queuemembership', + name='queues', + ), + migrations.RemoveField( + model_name='queuemembership', + name='user', + ), + migrations.AddField( + model_name='queue', + name='permission_name', + field=models.CharField(help_text='Name used in the django.contrib.auth permission system', max_length=50, null=True, verbose_name='Django auth permission name', blank=True), + ), + migrations.DeleteModel( + name='QueueMembership', + ), + ] diff --git a/helpdesk/models.py b/helpdesk/models.py index 70cce880..664fec8d 100644 --- a/helpdesk/models.py +++ b/helpdesk/models.py @@ -1403,26 +1403,3 @@ class TicketDependency(models.Model): unique_together = ('ticket', 'depends_on') verbose_name = _('Ticket dependency') verbose_name_plural = _('Ticket dependencies') - - -@python_2_unicode_compatible -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 __str__(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') From 830fcbe79604cd76961702c81598ed998f96b647 Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Wed, 16 Dec 2015 15:03:13 +0100 Subject: [PATCH 07/13] updating documentation regarding membership/permission --- docs/settings.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/settings.rst b/docs/settings.rst index a4e955a0..59c97aee 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -123,9 +123,9 @@ Staff Ticket Creation Settings Staff Ticket View Settings ------------------------------ -- **HELPDESK_ENABLE_PER_QUEUE_MEMBERSHIP** If ``True``, logged in staff users only see queues and tickets to which they have specifically been granted access - this holds for the dashboard, ticket query, and ticket report views. User assignment can be modified in the ``User`` section of the standard ``django.contrib.admin`` app. *Note*: This setting does not prevent staff users from creating tickets for all queues or editing tickets in any queue, should they know the ticket ID or editing URL. It is meant to keep work loads segregated for staff convenience, not to prevent malicious behavior. Also, superuser accounts have full access to all queues, regardless of whatever queue memberships they have been granted. +- **HELPDESK_ENABLE_PER_QUEUE_PERMISSION** If ``True``, logged in staff users only see queues and tickets to which they have specifically been granted access - this holds for the dashboard, ticket query, and ticket report views. User assignment is done through the standard ``django.admin.admin`` permissions. *Note*: Staff with access to admin interface will be able to see the full list of tickets, but won't have access to details and could not modify them. This setting does not prevent staff users from creating tickets for all queues. Also, superuser accounts have full access to all queues, regardless of whatever queue memberships they have been granted. - **Default:** ``HELPDESK_ENABLE_PER_QUEUE_MEMBERSHIP = False`` + **Default:** ``HELPDESK_ENABLE_PER_QUEUE_PERMISSION = False`` From 92d8ca3effc9221417acfdcaafc912a849a8df01 Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Wed, 16 Dec 2015 17:05:25 +0100 Subject: [PATCH 08/13] more modular management of permission codenames --- .../0009_migrate_queuemembership.py | 23 ++++++++++----- helpdesk/models.py | 29 +++++++++++++------ 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/helpdesk/migrations/0009_migrate_queuemembership.py b/helpdesk/migrations/0009_migrate_queuemembership.py index 62144de7..5bf1f176 100644 --- a/helpdesk/migrations/0009_migrate_queuemembership.py +++ b/helpdesk/migrations/0009_migrate_queuemembership.py @@ -2,6 +2,7 @@ from __future__ import unicode_literals from django.db import migrations from django.conf import settings +from django.db.utils import IntegrityError from django.utils.translation import ugettext_lazy as _ @@ -24,14 +25,20 @@ def create_and_assign_permissions(apps, schema_editor): Queue = apps.get_model('helpdesk', 'Queue') for q in Queue.objects.all(): - # Prepare the permission associated to this Queue - basename = "queue_access_%s" % q.slug - q.permission_name = "helpdesk.%s" % basename - Permission.objects.create( - name=_("Permission for queue: ") + q.title, - content_type=ContentType.objects.get(model="queue"), - codename=basename, - ) + if not q.permission_name: + basename = q.prepare_permission_name + else: + basename = q.permission_name[9:] + + try: + Permission.objects.create( + name=_("Permission for queue: ") + q.title, + content_type=ContentType.objects.get(model="queue"), + codename=basename, + ) + except IntegrityError: + # Seems that it already existed, safely ignore it + pass q.save() # Second step: map the permissions according to QueueMembership diff --git a/helpdesk/models.py b/helpdesk/models.py index 664fec8d..8f496bde 100644 --- a/helpdesk/models.py +++ b/helpdesk/models.py @@ -240,6 +240,15 @@ class Queue(models.Model): return u'%s <%s>' % (self.title, self.email_address) from_address = property(_from_address) + def prepare_permission_name(self): + """Prepare internally the codename for the permission and store it in permission_name. + :return: The codename that can be used to create a new Permission object. + """ + # Prepare the permission associated to this Queue + basename = "queue_access_%s" % self.slug + self.permission_name = "helpdesk.%s" % basename + return basename + def save(self, *args, **kwargs): if self.email_box_type == 'imap' and not self.email_box_imap_folder: self.email_box_imap_folder = 'INBOX' @@ -263,15 +272,17 @@ class Queue(models.Model): elif self.email_box_type == 'pop3' and not self.email_box_ssl: self.email_box_port = 110 - if not self.id and helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION: - # Prepare the permission associated to this Queue - basename = "queue_access_%s" % self.slug - self.permission_name = "helpdesk.%s" % basename - Permission.objects.create( - name=_("Permission for queue: ") + self.title, - content_type=ContentType.objects.get(model="queue"), - codename=basename, - ) + if not self.id: + # Always prepare the permission codename + basename = self.prepare_permission_name() + + # Create the permission only if the flag is active + if helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION: + Permission.objects.create( + name=_("Permission for queue: ") + self.title, + content_type=ContentType.objects.get(model="queue"), + codename=basename, + ) super(Queue, self).save(*args, **kwargs) From 43e5ff7c0e2cf8554467575fa1232fa0c5ef3287 Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Wed, 16 Dec 2015 17:15:54 +0100 Subject: [PATCH 09/13] added delete mechanism on Queue for auto-clean of permissions --- helpdesk/models.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/helpdesk/models.py b/helpdesk/models.py index 8f496bde..1d533e52 100644 --- a/helpdesk/models.py +++ b/helpdesk/models.py @@ -10,6 +10,7 @@ models.py - Model (and hence database) definitions. This is the core of the from __future__ import unicode_literals from django.contrib.auth.models import Permission from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ObjectDoesNotExist from django.db import models from django.contrib.auth import get_user_model from django.conf import settings @@ -286,6 +287,18 @@ class Queue(models.Model): super(Queue, self).save(*args, **kwargs) + def delete(self, *args, **kwargs): + permission_name = self.permission_name + super(Queue, self).delete(*args, **kwargs) + + # once the Queue is safely deleted, remove the permission (if exists) + if permission_name: + try: + p = Permission.objects.get(codename=permission_name[9:]) + p.delete() + except ObjectDoesNotExist: + pass + class Ticket(models.Model): """ From 1ea70ad1be13dfe53059a2fbeb4200cc3504a421 Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Wed, 16 Dec 2015 17:31:51 +0100 Subject: [PATCH 10/13] adding management command for batch-creation of Queue permission --- .../commands/create_queue_permissions.py | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 helpdesk/management/commands/create_queue_permissions.py diff --git a/helpdesk/management/commands/create_queue_permissions.py b/helpdesk/management/commands/create_queue_permissions.py new file mode 100644 index 00000000..ca42375d --- /dev/null +++ b/helpdesk/management/commands/create_queue_permissions.py @@ -0,0 +1,71 @@ +#!/usr/bin/python +""" +scripts/create_queue_permissions.py - + Create automatically permissions for all Queues. + + This is typically used when some queues are created before using the flag + HELPDESK_ENABLE_PER_QUEUE_PERMISSION, and then Permissions must be added. + + It should be safe to call this script multiple times or with partial + permissions. +""" + +from optparse import make_option + +from django.contrib.auth.models import Permission +from django.contrib.contenttypes.models import ContentType +from django.core.management.base import BaseCommand, CommandError +from django.db.utils import IntegrityError +from django.utils.translation import ugettext_lazy as _ + +from helpdesk.models import Queue + + +class Command(BaseCommand): + def __init__(self): + BaseCommand.__init__(self) + + self.option_list += ( + make_option( + '--queues', '-q', + help='Queues to include (default: all). Use queue slugs'), + ) + + def handle(self, *args, **options): + queue_slugs = options['queues'] + queues = [] + + if queue_slugs is not None: + queue_set = queue_slugs.split(',') + for queue in queue_set: + try: + q = Queue.objects.get(slug__exact=queue) + except Queue.DoesNotExist: + raise CommandError("Queue %s does not exist." % queue) + queues.append(q) + else: + queues = list(Queue.objects.all()) + + # Create permissions for the queues, which may be all or not + for q in queues: + self.stdout.write("Preparing Queue %s [%s]" % (q.title, q.slug)) + + if q.permission_name: + self.stdout.write(" .. already has `permission_name=%s`" % q.permission_name) + basename = q.permission_name[9:] + else: + basename = q.generate_permission_name() + self.stdout.write(" .. generated `permission_name=%s`" % q.permission_name) + q.save() + + self.stdout.write(" .. checking permission codename `%s`" % basename) + + try: + Permission.objects.create( + name=_("Permission for queue: ") + q.title, + content_type=ContentType.objects.get(model="queue"), + codename=basename, + ) + except IntegrityError: + self.stdout.write(" .. permission already existed, skipping") + From 01598826bf28b50b549372a60073be1b48af9c3d Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Wed, 16 Dec 2015 17:51:15 +0100 Subject: [PATCH 11/13] protecting through 403 several staff views --- helpdesk/views/staff.py | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/helpdesk/views/staff.py b/helpdesk/views/staff.py index 99ecc5bc..2a00f090 100644 --- a/helpdesk/views/staff.py +++ b/helpdesk/views/staff.py @@ -21,11 +21,11 @@ except ImportError: 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.exceptions import ValidationError, PermissionDenied 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.utils.dates import MONTHS_3 @@ -164,6 +164,8 @@ dashboard = staff_member_required(dashboard) def delete_ticket(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) + if not _has_access_to_queue(request.user, ticket.queue): + raise PermissionDenied() if request.method == 'GET': return render_to_response('helpdesk/delete_ticket.html', @@ -179,6 +181,8 @@ 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) ticket = get_object_or_404(Ticket, id=ticket_id) + if not _has_access_to_queue(request.user, ticket.queue): + raise PermissionDenied() if request.method == 'GET': form = EditFollowUpForm(initial= {'title': escape(followup.title), @@ -237,6 +241,8 @@ followup_delete = staff_member_required(followup_delete) def view_ticket(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) + if not _has_access_to_queue(request.user, ticket.queue): + raise PermissionDenied() if 'take' in request.GET: # Allow the user to assign the ticket to themselves whilst viewing it. @@ -623,6 +629,9 @@ def mass_update(request): action = 'assign' for t in Ticket.objects.filter(id__in=tickets): + if not _has_access_to_queue(request.user, t.queue): + continue + if action == 'assign' and t.assigned_to != user: t.assigned_to = user t.save() @@ -901,7 +910,7 @@ ticket_list = staff_member_required(ticket_list) def edit_ticket(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) if not _has_access_to_queue(request.user, ticket.queue): - return HttpResponseRedirect(reverse('helpdesk_dashboard')) + raise PermissionDenied() if request.method == 'POST': form = EditTicketForm(request.POST, instance=ticket) @@ -974,6 +983,8 @@ raw_details = staff_member_required(raw_details) def hold_ticket(request, ticket_id, unhold=False): ticket = get_object_or_404(Ticket, id=ticket_id) + if not _has_access_to_queue(request.user, ticket.queue): + raise PermissionDenied() if unhold: ticket.on_hold = False @@ -1282,6 +1293,9 @@ email_ignore_del = superuser_required(email_ignore_del) def ticket_cc(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) + if not _has_access_to_queue(request.user, ticket.queue): + raise PermissionDenied() + copies_to = ticket.ticketcc_set.all() return render_to_response('helpdesk/ticket_cc_list.html', RequestContext(request, { @@ -1292,6 +1306,9 @@ ticket_cc = staff_member_required(ticket_cc) def ticket_cc_add(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) + if not _has_access_to_queue(request.user, ticket.queue): + raise PermissionDenied() + if request.method == 'POST': form = TicketCCForm(request.POST) if form.is_valid(): @@ -1310,6 +1327,7 @@ ticket_cc_add = staff_member_required(ticket_cc_add) 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': cc.delete() return HttpResponseRedirect(reverse('helpdesk_ticket_cc', kwargs={'ticket_id': cc.ticket.id})) @@ -1321,6 +1339,8 @@ ticket_cc_del = staff_member_required(ticket_cc_del) def ticket_dependency_add(request, ticket_id): ticket = get_object_or_404(Ticket, id=ticket_id) + if not _has_access_to_queue(request.user, ticket.queue): + raise PermissionDenied() if request.method == 'POST': form = TicketDependencyForm(request.POST) if form.is_valid(): @@ -1351,6 +1371,8 @@ ticket_dependency_del = staff_member_required(ticket_dependency_del) def attachment_del(request, ticket_id, attachment_id): ticket = get_object_or_404(Ticket, id=ticket_id) + if not _has_access_to_queue(request.user, ticket.queue): + raise PermissionDenied() attachment = get_object_or_404(Attachment, id=attachment_id) attachment.delete() return HttpResponseRedirect(reverse('helpdesk_view', args=[ticket_id])) From 0b5af429a5b1d4747b57b9b0dcb5896d40edaefb Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Thu, 17 Dec 2015 18:11:02 +0100 Subject: [PATCH 12/13] reverting error due to methods and migrations --- helpdesk/migrations/0009_migrate_queuemembership.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/helpdesk/migrations/0009_migrate_queuemembership.py b/helpdesk/migrations/0009_migrate_queuemembership.py index 5bf1f176..4660a3ec 100644 --- a/helpdesk/migrations/0009_migrate_queuemembership.py +++ b/helpdesk/migrations/0009_migrate_queuemembership.py @@ -7,7 +7,6 @@ from django.utils.translation import ugettext_lazy as _ def create_and_assign_permissions(apps, schema_editor): - # If neither Permission nor Membership mechanism are enabled, ignore the migration if not ((hasattr(settings, 'HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION') and settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION) or @@ -26,8 +25,10 @@ def create_and_assign_permissions(apps, schema_editor): for q in Queue.objects.all(): if not q.permission_name: - basename = q.prepare_permission_name + basename = "queue_access_%s" % q.slug + q.permission_name = "helpdesk.%s" % basename else: + # Strip the `helpdesk.` prefix basename = q.permission_name[9:] try: From 4545fc925fbd600ea8081ba54ff2f9cdf36ee9d6 Mon Sep 17 00:00:00 2001 From: Alex Barcelo Date: Thu, 17 Dec 2015 18:44:10 +0100 Subject: [PATCH 13/13] fixing migration and allowing valid rollback on typical migrations --- .../0009_migrate_queuemembership.py | 24 ++++++++++++++++++- .../migrations/0010_remove_queuemembership.py | 5 ---- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/helpdesk/migrations/0009_migrate_queuemembership.py b/helpdesk/migrations/0009_migrate_queuemembership.py index 4660a3ec..3ba6ae62 100644 --- a/helpdesk/migrations/0009_migrate_queuemembership.py +++ b/helpdesk/migrations/0009_migrate_queuemembership.py @@ -1,5 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals + +from django.core.exceptions import ObjectDoesNotExist from django.db import migrations from django.conf import settings from django.db.utils import IntegrityError @@ -53,6 +55,25 @@ def create_and_assign_permissions(apps, schema_editor): qm.delete() +def revert_queue_membership(apps, schema_editor): + Permission = apps.get_model('auth', 'Permission') + Queue = apps.get_model('helpdesk', 'Queue') + QueueMembership = apps.get_model('helpdesk', 'QueueMembership') + for p in Permission.objects.all(): + if p.codename.startswith("queue_access_"): + slug = p.codename[13:] + try: + q = Queue.objects.get(slug=slug) + except ObjectDoesNotExist: + continue + + for user in p.user_set.all(): + qm, _ = QueueMembership.objects.get_or_create(user=user) + qm.queues.add(q) + + p.delete() + + class Migration(migrations.Migration): dependencies = [ @@ -60,5 +81,6 @@ class Migration(migrations.Migration): ] operations = [ - migrations.RunPython(create_and_assign_permissions) + migrations.RunPython(create_and_assign_permissions, + revert_queue_membership) ] diff --git a/helpdesk/migrations/0010_remove_queuemembership.py b/helpdesk/migrations/0010_remove_queuemembership.py index a43d467b..3b097365 100644 --- a/helpdesk/migrations/0010_remove_queuemembership.py +++ b/helpdesk/migrations/0010_remove_queuemembership.py @@ -19,11 +19,6 @@ class Migration(migrations.Migration): model_name='queuemembership', name='user', ), - migrations.AddField( - model_name='queue', - name='permission_name', - field=models.CharField(help_text='Name used in the django.contrib.auth permission system', max_length=50, null=True, verbose_name='Django auth permission name', blank=True), - ), migrations.DeleteModel( name='QueueMembership', ),