From 6ab0c3b2413ad60c862b02f73b4943d45bfa248e Mon Sep 17 00:00:00 2001 From: aspedrosa Date: Tue, 25 Jan 2022 20:35:31 +0000 Subject: [PATCH 1/5] fix block uploader tests --- dashboard_viewer/dashboard_viewer/settings.py | 2 -- dashboard_viewer/uploader/decorators.py | 3 +++ dashboard_viewer/uploader/tests.py | 10 ---------- dashboard_viewer/uploader/views.py | 4 ---- 4 files changed, 3 insertions(+), 16 deletions(-) diff --git a/dashboard_viewer/dashboard_viewer/settings.py b/dashboard_viewer/dashboard_viewer/settings.py index d1a60a76..34f7bd70 100644 --- a/dashboard_viewer/dashboard_viewer/settings.py +++ b/dashboard_viewer/dashboard_viewer/settings.py @@ -337,7 +337,5 @@ def constance_updated(key, old_value, **_): "Only include the hostname part of the URL." ) - X_FRAME_OPTIONS = f"ALLOW-FROM https://{MAIN_APPLICATION_HOST}/" - # required since django 3.2 DEFAULT_AUTO_FIELD = "django.db.models.AutoField" diff --git a/dashboard_viewer/uploader/decorators.py b/dashboard_viewer/uploader/decorators.py index bbdf95f8..7da5c199 100644 --- a/dashboard_viewer/uploader/decorators.py +++ b/dashboard_viewer/uploader/decorators.py @@ -2,6 +2,7 @@ from django.conf import settings from django.http import HttpResponseForbidden +from django.views.decorators.clickjacking import xframe_options_exempt from django.views.decorators.csrf import csrf_exempt @@ -13,6 +14,8 @@ def uploader_decorator(view_func): Else don't do any verification """ wrapped_view = csrf_exempt(view_func) + wrapped_view = xframe_options_exempt(wrapped_view) + if not settings.SINGLE_APPLICATION_MODE: def check_host(request, *args, **kwargs): diff --git a/dashboard_viewer/uploader/tests.py b/dashboard_viewer/uploader/tests.py index d4bf83ba..ebf3a9c6 100644 --- a/dashboard_viewer/uploader/tests.py +++ b/dashboard_viewer/uploader/tests.py @@ -46,19 +46,11 @@ def test_not_block_if_correct_host(self): response = self.client.get("/uploader/test/", HTTP_HOST="mainapp.host.com") self.assertEqual(200, response.status_code) - self.assertTrue(response.has_header("X-Frame-Options")) - self.assertEqual( - "ALLOW-FROM HTTPS://MAINAPP.HOST.COM/", response["X-Frame-Options"] - ) def test_not_block_other_urls(self): response = self.client.get("/admin/login/", HTTP_HOST="thisapp.host.com") self.assertEqual(200, response.status_code) - self.assertTrue(response.has_header("X-Frame-Options")) - self.assertEqual( - "ALLOW-FROM HTTPS://MAINAPP.HOST.COM/", response["X-Frame-Options"] - ) class UploaderNonRestrictedAccess(TestCase): @@ -74,8 +66,6 @@ def test_not_block_if_single_application(self): response = self.client.get("/uploader/test/", HTTP_HOST="some.domain.com") self.assertEqual(200, response.status_code) - if response.has_header("X-Frame-Options"): - self.assertNotIn("ALLOW-FROM ", response.get("X-Frame-Options")) class DataSourceCreator: diff --git a/dashboard_viewer/uploader/views.py b/dashboard_viewer/uploader/views.py index 78ea20f3..003b6d3f 100644 --- a/dashboard_viewer/uploader/views.py +++ b/dashboard_viewer/uploader/views.py @@ -6,7 +6,6 @@ from django.http import JsonResponse from django.shortcuts import get_object_or_404, redirect, render from django.utils.html import format_html, mark_safe -from django.views.decorators.clickjacking import xframe_options_exempt from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet @@ -22,7 +21,6 @@ @uploader_decorator -@xframe_options_exempt def upload_achilles_results(request, *args, **kwargs): data_source = kwargs.get("data_source") try: @@ -157,7 +155,6 @@ def _leave_valid_fields_values_only(request, initial, aux_form): @uploader_decorator -@xframe_options_exempt def create_data_source(request, *_, **kwargs): data_source = kwargs.get("data_source") if request.method == "GET": @@ -239,7 +236,6 @@ def create_data_source(request, *_, **kwargs): @uploader_decorator -@xframe_options_exempt def edit_data_source(request, *_, **kwargs): data_source = kwargs.get("data_source") try: From fb26bf16aa9fa08f16d4939dde0df805c112a35e Mon Sep 17 00:00:00 2001 From: aspedrosa Date: Thu, 14 Apr 2022 13:04:56 +0100 Subject: [PATCH 2/5] add local settings --- .gitignore | 3 +++ dashboard_viewer/dashboard_viewer/settings.py | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index c3bd3f72..473d416f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,6 @@ +# django local settings +local_settings.py + # documentation temporary files docs/src/_book diff --git a/dashboard_viewer/dashboard_viewer/settings.py b/dashboard_viewer/dashboard_viewer/settings.py index 34f7bd70..97d72361 100644 --- a/dashboard_viewer/dashboard_viewer/settings.py +++ b/dashboard_viewer/dashboard_viewer/settings.py @@ -320,6 +320,16 @@ def constance_updated(key, old_value, **_): TEST_RUNNER = "dashboard_viewer.runners.CeleryTestSuiteRunner" +# required since django 3.2 +DEFAULT_AUTO_FIELD = "django.db.models.AutoField" + + +try: + import local_settings + from local_settings import * # noqa +except ImportError: + pass + # Variables that allow restricting the access to the uploader app if this Django # app is being used as a third-party tool and is being iframed. SINGLE_APPLICATION_MODE = strtobool(os.environ.get("SINGLE_APPLICATION_MODE", "y")) == 1 @@ -337,5 +347,5 @@ def constance_updated(key, old_value, **_): "Only include the hostname part of the URL." ) -# required since django 3.2 -DEFAULT_AUTO_FIELD = "django.db.models.AutoField" + if (len(ALLOWED_HOSTS) > 1 or ALLOWED_HOSTS[0] != "*") and MAIN_APPLICATION_HOST not in ALLOWED_HOSTS: + ALLOWED_HOSTS.append(MAIN_APPLICATION_HOST) From 6be8f91785621d35ed12e112ea36421ee7e2fd94 Mon Sep 17 00:00:00 2001 From: aspedrosa Date: Thu, 14 Apr 2022 13:05:08 +0100 Subject: [PATCH 3/5] enable third party tests --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 289d0527..decab2c3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -39,4 +39,4 @@ jobs: export $(grep -v '^#' tests/.env | xargs -d '\n') cd dashboard_viewer python manage.py test --exclude-tag third-party-app - #SINGLE_APPLICATION_MODE=n MAIN_APPLICATION_HOST=mainapp.host.com python manage.py test --tag third-party-app + SINGLE_APPLICATION_MODE=n MAIN_APPLICATION_HOST=mainapp.host.com python manage.py test --tag third-party-app From 1c37637b4582d8d04b54b6364c65a1769c2d6d5c Mon Sep 17 00:00:00 2001 From: aspedrosa Date: Thu, 14 Apr 2022 13:13:55 +0100 Subject: [PATCH 4/5] remove csrf_exempt from uploader views the variable CSRF_TRUSTED_ORIGINS must be set instead to the appropriate hosts --- dashboard_viewer/uploader/decorators.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/dashboard_viewer/uploader/decorators.py b/dashboard_viewer/uploader/decorators.py index 7da5c199..76daa74c 100644 --- a/dashboard_viewer/uploader/decorators.py +++ b/dashboard_viewer/uploader/decorators.py @@ -3,7 +3,6 @@ from django.conf import settings from django.http import HttpResponseForbidden from django.views.decorators.clickjacking import xframe_options_exempt -from django.views.decorators.csrf import csrf_exempt def uploader_decorator(view_func): @@ -13,16 +12,14 @@ def uploader_decorator(view_func): If not response with 403 Else don't do any verification """ - wrapped_view = csrf_exempt(view_func) - wrapped_view = xframe_options_exempt(wrapped_view) - if not settings.SINGLE_APPLICATION_MODE: + wrapped_view = xframe_options_exempt(view_func) def check_host(request, *args, **kwargs): if request.get_host() != settings.MAIN_APPLICATION_HOST: return HttpResponseForbidden() return view_func(request, *args, **kwargs) - wrapped_view = wraps(wrapped_view)(check_host) + return wraps(wrapped_view)(check_host) - return wrapped_view + return view_func From 97ac4d6704d26edf9b5377f52f8fa67605b4087f Mon Sep 17 00:00:00 2001 From: aspedrosa Date: Thu, 14 Apr 2022 16:06:04 +0100 Subject: [PATCH 5/5] fix code analysis checks --- dashboard_viewer/dashboard_viewer/settings.py | 6 ++++-- .../management/commands/refresh_mat_views.py | 1 + dashboard_viewer/materialized_queries_manager/tasks.py | 1 + dashboard_viewer/materialized_queries_manager/utils.py | 3 ++- dashboard_viewer/uploader/file_handler/updates.py | 1 + dashboard_viewer/uploader/tasks.py | 2 +- dashboard_viewer/uploader/views.py | 6 ++++-- setup.cfg | 3 +++ 8 files changed, 17 insertions(+), 6 deletions(-) diff --git a/dashboard_viewer/dashboard_viewer/settings.py b/dashboard_viewer/dashboard_viewer/settings.py index 97d72361..c83343c8 100644 --- a/dashboard_viewer/dashboard_viewer/settings.py +++ b/dashboard_viewer/dashboard_viewer/settings.py @@ -325,7 +325,7 @@ def constance_updated(key, old_value, **_): try: - import local_settings + import local_settings # noqa from local_settings import * # noqa except ImportError: pass @@ -347,5 +347,7 @@ def constance_updated(key, old_value, **_): "Only include the hostname part of the URL." ) - if (len(ALLOWED_HOSTS) > 1 or ALLOWED_HOSTS[0] != "*") and MAIN_APPLICATION_HOST not in ALLOWED_HOSTS: + if ( + len(ALLOWED_HOSTS) > 1 or ALLOWED_HOSTS[0] != "*" + ) and MAIN_APPLICATION_HOST not in ALLOWED_HOSTS: ALLOWED_HOSTS.append(MAIN_APPLICATION_HOST) diff --git a/dashboard_viewer/materialized_queries_manager/management/commands/refresh_mat_views.py b/dashboard_viewer/materialized_queries_manager/management/commands/refresh_mat_views.py index 53ca5148..c3b497a6 100644 --- a/dashboard_viewer/materialized_queries_manager/management/commands/refresh_mat_views.py +++ b/dashboard_viewer/materialized_queries_manager/management/commands/refresh_mat_views.py @@ -2,6 +2,7 @@ from django.core.cache import cache from django.core.management.base import BaseCommand + from materialized_queries_manager.utils import refresh logger = logging.getLogger(__name__) diff --git a/dashboard_viewer/materialized_queries_manager/tasks.py b/dashboard_viewer/materialized_queries_manager/tasks.py index a8f5e937..aed85222 100644 --- a/dashboard_viewer/materialized_queries_manager/tasks.py +++ b/dashboard_viewer/materialized_queries_manager/tasks.py @@ -9,6 +9,7 @@ from django.core import serializers from django.core.cache import cache from django.db import connections, ProgrammingError, router, transaction + from materialized_queries_manager.models import MaterializedQuery from materialized_queries_manager.utils import refresh diff --git a/dashboard_viewer/materialized_queries_manager/utils.py b/dashboard_viewer/materialized_queries_manager/utils.py index c8509d12..857ea4e7 100644 --- a/dashboard_viewer/materialized_queries_manager/utils.py +++ b/dashboard_viewer/materialized_queries_manager/utils.py @@ -1,8 +1,9 @@ from django.core.cache import cache from django.db import connections -from materialized_queries_manager.models import MaterializedQuery from redis_rw_lock import RWLock +from materialized_queries_manager.models import MaterializedQuery + def refresh(logger, db_id=None, query_set=None): # Only one worker can update the materialized views at the same time -> same as -> only one thread diff --git a/dashboard_viewer/uploader/file_handler/updates.py b/dashboard_viewer/uploader/file_handler/updates.py index df0ed447..d14ba639 100644 --- a/dashboard_viewer/uploader/file_handler/updates.py +++ b/dashboard_viewer/uploader/file_handler/updates.py @@ -2,6 +2,7 @@ from django.conf import settings from django.db import connections from sqlalchemy import create_engine + from uploader.models import ( AchillesResults, AchillesResultsArchive, diff --git a/dashboard_viewer/uploader/tasks.py b/dashboard_viewer/uploader/tasks.py index 87e5dd6a..8e8e898f 100644 --- a/dashboard_viewer/uploader/tasks.py +++ b/dashboard_viewer/uploader/tasks.py @@ -3,9 +3,9 @@ from django.core import serializers from django.core.cache import cache from django.db import router, transaction -from materialized_queries_manager.utils import refresh from redis_rw_lock import RWLock +from materialized_queries_manager.utils import refresh from .file_handler.checks import extract_data_from_uploaded_file from .file_handler.updates import update_achilles_results_data from .models import AchillesResults, PendingUpload, UploadHistory diff --git a/dashboard_viewer/uploader/views.py b/dashboard_viewer/uploader/views.py index 003b6d3f..2e478c6c 100644 --- a/dashboard_viewer/uploader/views.py +++ b/dashboard_viewer/uploader/views.py @@ -9,8 +9,8 @@ from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet -from materialized_queries_manager.tasks import refresh_materialized_views_task from materialized_queries_manager.models import MaterializedQuery +from materialized_queries_manager.tasks import refresh_materialized_views_task from .decorators import uploader_decorator from .forms import AchillesResultsForm, EditSourceForm, SourceForm from .models import Country, DataSource, PendingUpload, UploadHistory @@ -303,7 +303,9 @@ def partial_update(self, request, *_, **__): serializer.is_valid(raise_exception=True) serializer.save() - refresh_materialized_views_task.delay([obj.matviewname for obj in MaterializedQuery.objects.all()]) + refresh_materialized_views_task.delay( + [obj.matviewname for obj in MaterializedQuery.objects.all()] + ) if getattr(instance, "_prefetched_objects_cache", None): # If 'prefetch_related' has been applied to a queryset, we need to diff --git a/setup.cfg b/setup.cfg index 76553c53..968a67c6 100644 --- a/setup.cfg +++ b/setup.cfg @@ -2,5 +2,8 @@ combine_as_imports = true include_trailing_comma = true known_third_party = bootstrap_datepicker_plus,celery,django,model_utils,rest_framework,uploader +known_first_party = dashboard_viewer,materialized_queries_manager,tabsManager,uploader +no_lines_before = LOCALFOLDER multi_line_output = 3 order_by_type = false +extend_skip = migrations