diff --git a/arches/app/templates/errors/400.htm b/arches/app/templates/errors/400.htm new file mode 100644 index 00000000000..3a84b61b2f4 --- /dev/null +++ b/arches/app/templates/errors/400.htm @@ -0,0 +1,29 @@ +{% extends "errors/error_base.htm" %} +{% load i18n %} +{% load static %} + +{% block body %} +
+ +
+ +
+ + + +
+

{% trans "Oops!" %}

+

{% trans "Bad Request" %}

+
+ {% trans "Something went wrong and we couldn't process your request." %}" +
+ +
+ +
+
+{% endblock body %} diff --git a/arches/app/templates/errors/403.htm b/arches/app/templates/errors/403.htm new file mode 100644 index 00000000000..dc6b2cf0d9b --- /dev/null +++ b/arches/app/templates/errors/403.htm @@ -0,0 +1,29 @@ +{% extends "errors/error_base.htm" %} +{% load i18n %} +{% load static %} + +{% block body %} +
+ +
+ +
+ + + +
+

{% trans "Oops!" %}

+

{% trans "Access Denied" %}

+
+ {% trans "You don't have permission to access this resource." %}" +
+ +
+ +
+
+{% endblock body %} diff --git a/arches/app/templates/errors/404.htm b/arches/app/templates/errors/404.htm index 587952f7e77..4ed86a50d79 100644 --- a/arches/app/templates/errors/404.htm +++ b/arches/app/templates/errors/404.htm @@ -19,11 +19,11 @@

{% trans "Oops!" %}

{% trans "Page Not Found!" %}

- {% trans "Sorry, but the page you are looking for has not been found on our server." %} + {% trans "Sorry, but the page you are looking for has not been found on our server." %}

{% trans "Back to Homepage" %}
-{% endblock body %} \ No newline at end of file +{% endblock body %} diff --git a/arches/app/templates/errors/500.htm b/arches/app/templates/errors/500.htm index db9b301bf84..dde91106091 100644 --- a/arches/app/templates/errors/500.htm +++ b/arches/app/templates/errors/500.htm @@ -15,7 +15,7 @@ -
+

{% trans "Oops!" %}

{% trans "Internal Server Error!" %}

diff --git a/arches/app/utils/request.py b/arches/app/utils/request.py new file mode 100644 index 00000000000..7b6a975e0c5 --- /dev/null +++ b/arches/app/utils/request.py @@ -0,0 +1,17 @@ +def looks_like_api_call(request): + known_api_paths_without_api_signposting = [ + # Until these are renamed for consistency, just list them. + "arches.app.views.workflow_history", + ] + if "/api/" in request.path: + return True + if not request.resolver_match: + return False + if ".api." in request.resolver_match.func.__module__: + return True + if ( + request.resolver_match.func.__module__ + in known_api_paths_without_api_signposting + ): + return True + return False diff --git a/arches/app/views/main.py b/arches/app/views/main.py index 29800d2d185..61886ec208f 100644 --- a/arches/app/views/main.py +++ b/arches/app/views/main.py @@ -16,14 +16,18 @@ along with this program. If not, see . """ -import re import urllib.request, urllib.error, urllib.parse +from http import HTTPStatus from urllib.parse import urlparse + +from django.http import HttpResponseNotFound, HttpResponse +from django.shortcuts import render +from django.utils.translation import gettext as _ + from arches import __version__ from arches.app.models.system_settings import settings -from django.shortcuts import render, redirect -from django.http import HttpResponseNotFound, HttpResponse, HttpResponseRedirect -from django.utils import translation +from arches.app.utils.request import looks_like_api_call +from arches.app.utils.response import JSONErrorResponse def index(request): @@ -88,11 +92,36 @@ def feature_popup_content(request): return HttpResponseNotFound() -def custom_404(request): - request = None - return render(request, "errors/404.htm") +def custom_400(request, exception=None): + status = HTTPStatus.BAD_REQUEST + if looks_like_api_call(request): + return JSONErrorResponse(_("Request Failed"), _("Bad Request"), status=status) + return render(request, "errors/400.htm", status=status) + + +def custom_403(request, exception=None): + status = HTTPStatus.FORBIDDEN + if looks_like_api_call(request): + return JSONErrorResponse( + _("Request Failed"), _("Permission Denied"), status=status + ) + return render(request, "errors/403.htm", status=status) + + +def custom_404(request, exception=None): + status = HTTPStatus.NOT_FOUND + if looks_like_api_call(request): + return JSONErrorResponse(_("Request Failed"), _("Not Found"), status=status) + return render(request, "errors/404.htm", status=status) -def custom_500(request): - request = None - return render(request, "errors/500.htm") +def custom_500(request, exception=None): + status = HTTPStatus.INTERNAL_SERVER_ERROR + if looks_like_api_call(request): + return JSONErrorResponse( + # Exclamation point matches translated string in template. + _("Request Failed"), + _("Internal Server Error!"), + status=status, + ) + return render(request, "errors/500.htm", status=status) diff --git a/arches/install/arches-templates/project_name/urls.py-tpl b/arches/install/arches-templates/project_name/urls.py-tpl index b541b3f3059..2de99fdb88a 100644 --- a/arches/install/arches-templates/project_name/urls.py-tpl +++ b/arches/install/arches-templates/project_name/urls.py-tpl @@ -7,6 +7,11 @@ urlpatterns = [ # project-level urls ] +handler400 = "arches.app.views.main.custom_400" +handler403 = "arches.app.views.main.custom_403" +handler404 = "arches.app.views.main.custom_404" +handler500 = "arches.app.views.main.custom_500" + # Ensure Arches core urls are superseded by project-level urls urlpatterns.append(path('', include('arches.urls'))) diff --git a/arches/urls.py b/arches/urls.py index 476110cef57..b84c82c89c2 100644 --- a/arches/urls.py +++ b/arches/urls.py @@ -781,9 +781,13 @@ api.SpatialView.as_view(), name="spatialview_api", ), - re_path("^api", api.API404.as_view(), name="api_404"), ] +handler400 = "arches.app.views.main.custom_400" +handler403 = "arches.app.views.main.custom_403" +handler404 = "arches.app.views.main.custom_404" +handler500 = "arches.app.views.main.custom_500" + # This must be included in core to keep webpack happy, but cannot be appended when running a project. # See https://github.com/archesproject/arches/pull/10754 if settings.ROOT_URLCONF == __name__: diff --git a/releases/8.0.0.md b/releases/8.0.0.md index aafdc34c8b6..a6d6781aded 100644 --- a/releases/8.0.0.md +++ b/releases/8.0.0.md @@ -31,6 +31,7 @@ Arches 8.0.0 Release Notes - Add system dark mode detection for Vue apps [#11624](https://github.com/archesproject/arches/issues/11624) - Make number datatype node values searchable in the main search [#11619](https://github.com/archesproject/arches/issues/11619) - Prevent navigation to a new browser tab when clicking Manage link in index.htm [#11635](https://github.com/archesproject/arches/issues/11635) +- Mitigate the tendency of JSON APIs to respond with HTML under error conditions [#11722](https://github.com/archesproject/arches/pull/11722) - Add support for tile sort order to the bulk data manager [#11638](https://github.com/archesproject/arches/pull/11638) - Fix issue that produced an error when cycling between the dropdown options in Advanced search [#11442](https://github.com/archesproject/arches/issues/11442) @@ -131,6 +132,15 @@ JavaScript: python manage.py updateproject ``` +1. Update your urls.py to include generic error handlers: + + ``` + handler400 = "arches.app.views.main.custom_400" + handler403 = "arches.app.views.main.custom_403" + handler404 = "arches.app.views.main.custom_404" + handler500 = "arches.app.views.main.custom_500" + ``` + 3. Create editable_future_graphs for your Resource Models using the command `python manage.py graph create_editable_future_graphs`. This will publish new versions of each Graph. 4. Update your Graph publications and Resource instances to point to the newly published Graphs by running `python manage.py graph publish --update -ui` diff --git a/tests/views/api/test_resources.py b/tests/views/api/test_resources.py index 9ee85774d56..d0655dd0d66 100644 --- a/tests/views/api/test_resources.py +++ b/tests/views/api/test_resources.py @@ -26,7 +26,7 @@ from django.urls import reverse from django.contrib.auth.models import User from django.test.client import RequestFactory -from django.test.utils import captured_stdout +from django.test.utils import captured_stdout, override_settings from unittest.mock import patch, MagicMock from arches.app.views.api import APIBase @@ -120,11 +120,14 @@ def test_api_base_view(self): response = view(request) self.assertEqual(request.GET.get("ver"), "2.1") - def test_api_404(self): + @override_settings(DEBUG=False) + def test_api_404_returns_json(self): with self.assertLogs("django.request", level="WARNING"): - response = self.client.get(reverse("api_404")) - self.assertEqual( - set(json.loads(response.content)), {"message", "status", "success", "title"} + response = self.client.get("/api/doesnotexist") + self.assertContains( + response, + "Not Found", + status_code=HTTPStatus.NOT_FOUND, ) def test_api_resources_archesjson(self): diff --git a/tests/views/workflow_tests.py b/tests/views/workflow_tests.py index a2b93b29da6..f4e11f9c374 100644 --- a/tests/views/workflow_tests.py +++ b/tests/views/workflow_tests.py @@ -1,5 +1,6 @@ import uuid import datetime +from http import HTTPStatus from django.contrib.auth.models import Group, User from django.urls import reverse @@ -83,8 +84,9 @@ def test_get_workflow_history(self): ) ) - self.assertEqual(response.status_code, 403) - self.assertIn(b"Forbidden", response.content) + self.assertContains( + response, "Permission Denied", status_code=HTTPStatus.FORBIDDEN + ) self.client.force_login(self.admin) response = self.client.get( @@ -93,8 +95,7 @@ def test_get_workflow_history(self): ) ) - self.assertEqual(response.status_code, 200) - self.assertIn(b"sample name", response.content) + self.assertContains(response, "sample name", status_code=HTTPStatus.OK) def test_post_workflow_history(self): """Partial updates of componentdata and stepdata are allowed.""" @@ -148,8 +149,9 @@ def test_post_workflow_history(self): content_type="application/json", ) - self.assertEqual(response.status_code, 403) - self.assertIn(b"Forbidden", response.content) + self.assertContains( + response, "Permission Denied", status_code=HTTPStatus.FORBIDDEN + ) self.client.force_login(self.admin) response = self.client.post(