From 61f34876ed0f8362f7185bd5eecfc830a9de5cb0 Mon Sep 17 00:00:00 2001 From: Jon Date: Thu, 28 Nov 2024 01:40:24 +0930 Subject: [PATCH] fix(core): Ensure user cant view tickets in orgs they are not part of ref: #399 --- app/access/models.py | 5 ++-- app/api/views/core/tickets.py | 43 +++++++++++++++++++++++--------- app/api/views/mixin.py | 27 +++++++++++++++++--- app/core/views/ticket_comment.py | 30 ++++++++++++++++------ 4 files changed, 79 insertions(+), 26 deletions(-) diff --git a/app/access/models.py b/app/access/models.py index a46332da4..1b4bb5dfb 100644 --- a/app/access/models.py +++ b/app/access/models.py @@ -126,9 +126,10 @@ def get_queryset(self): user_organizations += [ team_user.team.organization.id ] - if len(user_organizations) > 0 and not user.is_superuser and self.model.is_global is not None: + # if len(user_organizations) > 0 and not user.is_superuser and self.model.is_global is not None: + if len(user_organizations) > 0 and not user.is_superuser: - if self.model.is_global: + if getattr(self.model, 'is_global', False) is True: return super().get_queryset().filter( models.Q(organization__in=user_organizations) diff --git a/app/api/views/core/tickets.py b/app/api/views/core/tickets.py index 764cef6c3..18b6aa4de 100644 --- a/app/api/views/core/tickets.py +++ b/app/api/views/core/tickets.py @@ -72,7 +72,10 @@ def get_dynamic_permissions(self): return super().get_permission_required() - queryset = Ticket.objects.all() + # queryset = Ticket.objects.all() + queryset = None + + model = Ticket def get_serializer(self, *args, **kwargs): @@ -114,32 +117,48 @@ def get_queryset(self): if self._ticket_type == 'change': - ticket_type = self.queryset.model.TicketType.CHANGE.value + ticket_type = self.model.TicketType.CHANGE.value elif self._ticket_type == 'incident': - ticket_type = self.queryset.model.TicketType.INCIDENT.value + ticket_type = self.model.TicketType.INCIDENT.value elif self._ticket_type == 'problem': - ticket_type = self.queryset.model.TicketType.PROBLEM.value + ticket_type = self.model.TicketType.PROBLEM.value elif self._ticket_type == 'request': - ticket_type = self.queryset.model.TicketType.REQUEST.value + ticket_type = self.model.TicketType.REQUEST.value elif self._ticket_type == 'project_task': - ticket_type = self.queryset.model.TicketType.REQUEST.value + ticket_type = self.model.TicketType.REQUEST.value - return self.queryset.filter( - project = self.kwargs['project_id'] - ) + # return self.queryset.filter( + # project = self.kwargs['project_id'] + # ) else: raise ValueError('Unknown ticket type. kwarg `ticket_type` must be set') - return self.queryset.filter( - ticket_type = ticket_type - ) + + if not self.queryset: + + queryset = Ticket.objects.all() + + queryset = queryset.filter( + ticket_type = ticket_type + ) + + if self._ticket_type == 'project_task': + + queryset = queryset.filter( + project = self.kwargs['project_id'] + ) + + self.queryset = queryset + + + return self.queryset diff --git a/app/api/views/mixin.py b/app/api/views/mixin.py index d965f9b6d..f93686423 100644 --- a/app/api/views/mixin.py +++ b/app/api/views/mixin.py @@ -1,4 +1,4 @@ -from django.core.exceptions import PermissionDenied +from django.core.exceptions import ObjectDoesNotExist, PermissionDenied from django.forms import ValidationError from rest_framework import exceptions @@ -35,7 +35,14 @@ def permission_check(self, request, view, obj=None) -> bool: view.http_method_not_allowed(request._request) - if hasattr(view, 'queryset'): + if hasattr(view, 'get_queryset'): + + queryset = view.get_queryset() + + self.obj = queryset.model + + elif hasattr(view, 'queryset'): + if view.queryset.model._meta: self.obj = view.queryset.model @@ -91,7 +98,13 @@ def permission_check(self, request, view, obj=None) -> bool: if object_organization is None and 'pk' in view.kwargs: - self.obj = view.queryset.get(pk=view.kwargs['pk']) + try: + + self.obj = view.queryset.get(pk=view.kwargs['pk']) # Here + + except ObjectDoesNotExist: + + return False if obj: @@ -115,7 +128,13 @@ def permission_check(self, request, view, obj=None) -> bool: if object_organization is None: - self.obj = view.queryset.get() + try: + + self.obj = view.queryset.get() + + except ObjectDoesNotExist: + + return False if hasattr(self, 'obj') and object_organization is None and 'pk' in view.kwargs: diff --git a/app/core/views/ticket_comment.py b/app/core/views/ticket_comment.py index 0e55d1ba1..71333c801 100644 --- a/app/core/views/ticket_comment.py +++ b/app/core/views/ticket_comment.py @@ -1,3 +1,5 @@ +from django.core.exceptions import ObjectDoesNotExist +from django.http import Http404 from django.urls import reverse from django.views import generic @@ -30,11 +32,17 @@ def get_dynamic_permissions(self): if self.request.user.is_authenticated: - ticket = Ticket.objects.get(pk=int(self.kwargs['ticket_id'])) + try: - if ticket.opened_by.id == self.request.user.id: + ticket = Ticket.objects.get(pk=int(self.kwargs['ticket_id'])) - return [] + if ticket.opened_by.id == self.request.user.id: + + return [] + + except ObjectDoesNotExist: + + pass return [ str('core.add_ticketcomment'), @@ -104,12 +112,18 @@ class Change(ChangeView): def get_dynamic_permissions(self): - if ( - self.request.user.is_authenticated and - self.get_object().user.id == self.request.user.id - ): + try: + + if ( + self.request.user.is_authenticated and + self.get_object().user.id == self.request.user.id + ): + + return [] + + except Http404: # Although the model not found, permissions must return HTTP/403 for authenticated user - return [] + pass return [ str('core.change_ticketcomment'),