From 976f57ac20205c48e9af20b1cf4db3a6faf72e9c Mon Sep 17 00:00:00 2001 From: Ross Date: Mon, 4 Dec 2023 12:19:52 +0000 Subject: [PATCH] fix a few major permission holes --- anatomy/views.py | 4 +++ atlas/decorators.py | 6 +++- atlas/filters.py | 3 +- atlas/templates/atlas/case_view.html | 21 ------------ atlas/templates/atlas/condition_form.html | 2 +- atlas/templates/atlas/index.html | 1 + atlas/templates/atlas/view.html | 1 + atlas/tests/test_cases.py | 15 +++++++++ atlas/views.py | 40 +++++++++++++++++------ generic/views.py | 4 +++ longs/views.py | 3 ++ rad/views.py | 3 +- rapids/views.py | 4 +++ templates/403.html | 6 +++- 14 files changed, 77 insertions(+), 36 deletions(-) delete mode 100644 atlas/templates/atlas/case_view.html create mode 100644 atlas/tests/test_cases.py diff --git a/anatomy/views.py b/anatomy/views.py index 86e1b8e4..5d48ebeb 100644 --- a/anatomy/views.py +++ b/anatomy/views.py @@ -608,6 +608,10 @@ class QuestionClone(AnatomyQuestionCreateBase): def get_initial(self): # print(self.request) old_object = get_object_or_404(AnatomyQuestion, pk=self.kwargs["pk"]) + + if self.request.user not in old_object.get_author_objects() and not self.request.user.is_superuser: + raise PermissionDenied() # or Http404 + initial_data = model_to_dict(old_object, exclude=["id"]) # We don't want to clone the exam details diff --git a/atlas/decorators.py b/atlas/decorators.py index 28966320..5c063341 100755 --- a/atlas/decorators.py +++ b/atlas/decorators.py @@ -37,9 +37,13 @@ def user_is_author_or_atlas_series_checker(function): return wrap -def user_is_author_or_atlas_editor_or_atlas_marker(function): +def user_has_case_view_access(function): def wrap(request, *args, **kwargs): atlas = Case.objects.get(pk=kwargs["pk"]) + + # If open access everyone can view + if atlas.open_access: + return function(request, *args, **kwargs) if ( request.user in atlas.author.all() or request.user.groups.filter(name="atlas_editor").exists() diff --git a/atlas/filters.py b/atlas/filters.py index 4b809553..81f14d2e 100755 --- a/atlas/filters.py +++ b/atlas/filters.py @@ -12,6 +12,7 @@ from .models import ( Subspecialty, ) from django.contrib.auth.models import User +from django.db.models import Q def get_authors(request): @@ -118,7 +119,7 @@ class CaseFilter(django_filters.FilterSet): ): if not request.user.groups.filter(name="atlas_editor").exists(): queryset = queryset.prefetch_related("author").filter( - author__id=request.user.id + (Q(author__id=request.user.id) | Q(open_access=True)) ) super(CaseFilter, self).__init__( data=data, queryset=queryset, prefix=prefix, request=request diff --git a/atlas/templates/atlas/case_view.html b/atlas/templates/atlas/case_view.html deleted file mode 100644 index eca27813..00000000 --- a/atlas/templates/atlas/case_view.html +++ /dev/null @@ -1,21 +0,0 @@ -{% extends 'atlas/base.html' %} - -{% load render_table from django_tables2 %} -{% block css %} -{% endblock %} - -{% block content %} - -
-

Filter Long

-
- {{ filter.form }} - -
-
- {% render_table table %} - -{% endblock %} - -{% block js %} -{% endblock %} diff --git a/atlas/templates/atlas/condition_form.html b/atlas/templates/atlas/condition_form.html index 9033a97f..25a20517 100755 --- a/atlas/templates/atlas/condition_form.html +++ b/atlas/templates/atlas/condition_form.html @@ -15,7 +15,7 @@ {% block content %}

Add/Edit Condition

Use this form to create or edit a condition.

-

Please check if it already exists before doing so!

+

Please check if it already exists before doing so!

{% csrf_token %} diff --git a/atlas/templates/atlas/index.html b/atlas/templates/atlas/index.html index 1b8bbe1f..957d1292 100644 --- a/atlas/templates/atlas/index.html +++ b/atlas/templates/atlas/index.html @@ -4,6 +4,7 @@

Atlas

Available collections
+View my cases.
My uploads {% endblock %} \ No newline at end of file diff --git a/atlas/templates/atlas/view.html b/atlas/templates/atlas/view.html index afe59580..1db110bb 100755 --- a/atlas/templates/atlas/view.html +++ b/atlas/templates/atlas/view.html @@ -18,6 +18,7 @@
+ View my cases. {% render_table table %} diff --git a/atlas/tests/test_cases.py b/atlas/tests/test_cases.py new file mode 100644 index 00000000..cb0d7918 --- /dev/null +++ b/atlas/tests/test_cases.py @@ -0,0 +1,15 @@ + +import json +import pytest + +# from django.contrib.auth.models import User +from django.urls import reverse + +from rich.pretty import pprint + +from bs4 import BeautifulSoup + +from django.contrib.auth.models import Group + + + diff --git a/atlas/views.py b/atlas/views.py index 8113ce23..8d43f287 100755 --- a/atlas/views.py +++ b/atlas/views.py @@ -99,12 +99,11 @@ from autocomplete import HTMXAutoComplete from .decorators import ( user_is_author_or_atlas_editor, - user_is_author_or_atlas_editor_or_atlas_marker, + user_has_case_view_access, user_is_author_or_atlas_series_checker_or_atlas_marker, user_is_atlas_editor, user_is_author_or_atlas_series_checker, user_is_atlas_marker, - user_is_author_or_atlas_editor_or_atlas_marker, user_is_collection_author_or_atlas_editor, ) @@ -148,9 +147,18 @@ class AuthorOrCheckerRequiredMixin(object): raise PermissionDenied() # or Http404 return obj +class AtlasEditorRequiredMixin(object): + def get_object(self, *args, **kwargs): + obj = super().get_object(*args, **kwargs) + if ( + self.request.user.groups.filter(name="atlas_editor").exists() + or self.request.user.is_superuser + ): + return obj + raise PermissionDenied("You must be an atlas editor to do this.") # or Http404 @login_required -@user_is_author_or_atlas_editor_or_atlas_marker +@user_has_case_view_access def case_detail(request, pk): case = get_object_or_404(Case, pk=pk) @@ -417,31 +425,31 @@ class SeriesFindingDelete(RevisionMixin, PermissionRequiredMixin, DeleteView): return reverse("atlas:series_detail", kwargs={"pk": series_id}) -class SeriesDelete(RevisionMixin, AuthorOrCheckerRequiredMixin, DeleteView): +class SeriesDelete(RevisionMixin, AtlasEditorRequiredMixin, DeleteView): model = Series template_name = "confirm_delete.html" success_url = reverse_lazy("atlas:series_view") -class CaseCollectionDelete(RevisionMixin, AuthorOrCheckerRequiredMixin, DeleteView): +class CaseCollectionDelete(RevisionMixin, AtlasEditorRequiredMixin, DeleteView): model = CaseCollection template_name = "confirm_delete.html" success_url = reverse_lazy("atlas:collection_view") -class ConditionDelete(RevisionMixin, AuthorOrCheckerRequiredMixin, DeleteView): +class ConditionDelete(RevisionMixin, AtlasEditorRequiredMixin, DeleteView): model = Condition template_name = "confirm_delete.html" success_url = reverse_lazy("atlas:condition_view") -class FindingDelete(RevisionMixin, AuthorOrCheckerRequiredMixin, DeleteView): +class FindingDelete(RevisionMixin, AtlasEditorRequiredMixin, DeleteView): model = Finding template_name = "confirm_delete.html" success_url = reverse_lazy("atlas:finding_view") -class StructureDelete(RevisionMixin, AuthorOrCheckerRequiredMixin, DeleteView): +class StructureDelete(RevisionMixin, AtlasEditorRequiredMixin, DeleteView): model = Structure template_name = "confirm_delete.html" success_url = reverse_lazy("atlas:structure_view") @@ -867,7 +875,7 @@ def add_collection_to_case_form(request, case_id): # return reverse("atlas:case_detail", kwargs={"pk": pk}) -class AtlasClone(AtlasCreateBase, CreateView): +class AtlasClone(AtlasCreateBase, AuthorOrCheckerRequiredMixin, CreateView): """Clones a existing atlas""" # fields = '__all__' @@ -877,11 +885,23 @@ class AtlasClone(AtlasCreateBase, CreateView): def get_initial(self): # print(self.request) old_object = get_object_or_404(Case, pk=self.kwargs["pk"]) - initial_data = model_to_dict(old_object, exclude=["id"]) + if ( + self.request.user.groups.filter(name="atlas_editor").exists() + or self.request.user.is_superuser + ): + pass + else: + if self.request.user not in old_object.get_author_objects(): + raise PermissionDenied() # or Http404 + else: + pass + + initial_data = model_to_dict(old_object, exclude=["id"]) return initial_data + @login_required @user_is_author_or_atlas_editor def atlas_scrap(request, pk): diff --git a/generic/views.py b/generic/views.py index 9c1feb32..f270e958 100644 --- a/generic/views.py +++ b/generic/views.py @@ -2327,6 +2327,10 @@ class GenericViewBase: class ExamCloneMixin: def get_initial(self): old_object = get_object_or_404(self.model, pk=self.kwargs["exam_id"]) + + if self.request.user not in old_object.get_author_objects() and not self.request.user.is_superuser: + raise PermissionDenied() # or Http404 + initial_data = model_to_dict(old_object, exclude=["id"]) # We manually transfer the forign keys / m2m relationships diff --git a/longs/views.py b/longs/views.py index 55aca0a2..e4c5664e 100755 --- a/longs/views.py +++ b/longs/views.py @@ -271,6 +271,9 @@ def long_clone(request, pk): new_item.pk = None # autogen a new pk (item_id) # new_item.name = "Copy of " + new_item.name #need to change uniques + if request.user not in new_item.get_author_objects() and not request.user.is_superuser: + raise PermissionDenied() # or Http404 + form = LongForm(request.POST or None, instance=new_item) series_formset = SeriesFormSet() diff --git a/rad/views.py b/rad/views.py index 957e13d3..83b6131d 100644 --- a/rad/views.py +++ b/rad/views.py @@ -519,7 +519,8 @@ def page_not_found(request, exception): def page_forbidden(request, exception): - response = render(request, "403.html") + + response = render(request, "403.html", context={"exception": exception}) response.status_code = 403 diff --git a/rapids/views.py b/rapids/views.py index ee98f852..c2984e0c 100755 --- a/rapids/views.py +++ b/rapids/views.py @@ -212,8 +212,12 @@ def rapid_clone(request, pk): new_item.pk = None # autogen a new pk (item_id) # new_item.name = "Copy of " + new_item.name #need to change uniques + if request.user not in new_item.get_author_objects() and not request.user.is_superuser: + raise PermissionDenied() # or Http404 + form = RapidForm(request.POST or None, instance=new_item) + image_formset = ImageFormSet() answer_formset = AnswerFormSet() diff --git a/templates/403.html b/templates/403.html index 3e02be8d..85a2eb24 100644 --- a/templates/403.html +++ b/templates/403.html @@ -4,7 +4,11 @@ {% load static %}

403 error

+{% if exception and not exception.strip %} +

{{ exception }}

+{% else %} +

Forbidden

+{% endif %} -Forbidden
{% endblock %} \ No newline at end of file