From d620bd7350d0aa428e4ccebc27ac885720673d1c Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sat, 18 Nov 2023 12:36:03 +1100 Subject: [PATCH 1/8] add handler for 403s fixes #3104 --- bookwyrm/templates/403.html | 16 ++++++++++++++++ bookwyrm/templatetags/utilities.py | 7 +++++++ bookwyrm/urls.py | 3 +++ bookwyrm/views/__init__.py | 1 + bookwyrm/views/permission_required.py | 8 ++++++++ 5 files changed, 35 insertions(+) create mode 100644 bookwyrm/templates/403.html create mode 100644 bookwyrm/views/permission_required.py diff --git a/bookwyrm/templates/403.html b/bookwyrm/templates/403.html new file mode 100644 index 000000000..b7927da7e --- /dev/null +++ b/bookwyrm/templates/403.html @@ -0,0 +1,16 @@ +{% extends 'layout.html' %} +{% load i18n %} +{% load utilities %} + +{% block title %}{% trans "Oh no!" %}{% endblock %} + +{% block content %} +
+

{% trans "Permission Denied" %}

+ {% blocktrans trimmed with level=request.user|get_user_permission %} +

You do not have permission to view this page. Your user permission level is {{ level }}.

+

If you think you should have access to this page, please speak to your BookWyrm server administrator.

+ {% endblocktrans %} +
+{% endblock %} + diff --git a/bookwyrm/templatetags/utilities.py b/bookwyrm/templatetags/utilities.py index 42e67990f..99575d85f 100644 --- a/bookwyrm/templatetags/utilities.py +++ b/bookwyrm/templatetags/utilities.py @@ -125,3 +125,10 @@ def id_to_username(user_id): value = f"{name}@{domain}" return value + + +@register.filter(name="get_user_permission") +def get_user_permission(user): + """given a user, return their permission level""" + + return user.groups.first() if user.groups.first() else "User" diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py index 8541f4fb6..2cab2aaa9 100644 --- a/bookwyrm/urls.py +++ b/bookwyrm/urls.py @@ -792,3 +792,6 @@ urlpatterns.extend(staticfiles_urlpatterns()) # pylint: disable=invalid-name handler500 = "bookwyrm.views.server_error" + +# pylint: disable=invalid-name +handler403 = "bookwyrm.views.permission_required" diff --git a/bookwyrm/views/__init__.py b/bookwyrm/views/__init__.py index 2d2e97f52..5af42f02b 100644 --- a/bookwyrm/views/__init__.py +++ b/bookwyrm/views/__init__.py @@ -167,3 +167,4 @@ from .annual_summary import ( summary_revoke_key, ) from .server_error import server_error +from .permission_required import permission_required diff --git a/bookwyrm/views/permission_required.py b/bookwyrm/views/permission_required.py new file mode 100644 index 000000000..8e12c38ef --- /dev/null +++ b/bookwyrm/views/permission_required.py @@ -0,0 +1,8 @@ +"""custom 403 handler to enable context processors""" +from django.template.response import TemplateResponse + + +def permission_required(request, exception): # pylint: disable=unused-argument + """permission denied page""" + + return TemplateResponse(request, "403.html") From 8ddafafa84b4dd70e49583cbb7373f6afb3691ab Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sat, 18 Nov 2023 12:40:36 +1100 Subject: [PATCH 2/8] make naming consistent --- bookwyrm/urls.py | 2 +- bookwyrm/views/__init__.py | 2 +- bookwyrm/views/{permission_required.py => permission_denied.py} | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename bookwyrm/views/{permission_required.py => permission_denied.py} (70%) diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py index 2cab2aaa9..0b65018cf 100644 --- a/bookwyrm/urls.py +++ b/bookwyrm/urls.py @@ -794,4 +794,4 @@ urlpatterns.extend(staticfiles_urlpatterns()) handler500 = "bookwyrm.views.server_error" # pylint: disable=invalid-name -handler403 = "bookwyrm.views.permission_required" +handler403 = "bookwyrm.views.permission_denied" diff --git a/bookwyrm/views/__init__.py b/bookwyrm/views/__init__.py index 5af42f02b..63f9d54b5 100644 --- a/bookwyrm/views/__init__.py +++ b/bookwyrm/views/__init__.py @@ -167,4 +167,4 @@ from .annual_summary import ( summary_revoke_key, ) from .server_error import server_error -from .permission_required import permission_required +from .permission_denied import permission_denied diff --git a/bookwyrm/views/permission_required.py b/bookwyrm/views/permission_denied.py similarity index 70% rename from bookwyrm/views/permission_required.py rename to bookwyrm/views/permission_denied.py index 8e12c38ef..b42ada4ab 100644 --- a/bookwyrm/views/permission_required.py +++ b/bookwyrm/views/permission_denied.py @@ -2,7 +2,7 @@ from django.template.response import TemplateResponse -def permission_required(request, exception): # pylint: disable=unused-argument +def permission_denied(request, exception): # pylint: disable=unused-argument """permission denied page""" return TemplateResponse(request, "403.html") From a56ba0ce1c594270113cefcd51203d955b1258aa Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sat, 18 Nov 2023 13:41:52 +1100 Subject: [PATCH 3/8] always return 403 to POST requests - POST requests need to receive a 403 error code - minor wording updates --- bookwyrm/templates/403.html | 4 ++-- bookwyrm/templatetags/utilities.py | 2 +- bookwyrm/views/permission_denied.py | 7 +++++++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/bookwyrm/templates/403.html b/bookwyrm/templates/403.html index b7927da7e..64fd43536 100644 --- a/bookwyrm/templates/403.html +++ b/bookwyrm/templates/403.html @@ -8,8 +8,8 @@

{% trans "Permission Denied" %}

{% blocktrans trimmed with level=request.user|get_user_permission %} -

You do not have permission to view this page. Your user permission level is {{ level }}.

-

If you think you should have access to this page, please speak to your BookWyrm server administrator.

+

You do not have permission to view this page or perform this action. Your user permission level is {{ level }}.

+

If you think you should have access, please speak to your BookWyrm server administrator.

{% endblocktrans %}
{% endblock %} diff --git a/bookwyrm/templatetags/utilities.py b/bookwyrm/templatetags/utilities.py index 99575d85f..6df6d2183 100644 --- a/bookwyrm/templatetags/utilities.py +++ b/bookwyrm/templatetags/utilities.py @@ -131,4 +131,4 @@ def id_to_username(user_id): def get_user_permission(user): """given a user, return their permission level""" - return user.groups.first() if user.groups.first() else "User" + return user.groups.first() or "User" diff --git a/bookwyrm/views/permission_denied.py b/bookwyrm/views/permission_denied.py index b42ada4ab..9e62b0933 100644 --- a/bookwyrm/views/permission_denied.py +++ b/bookwyrm/views/permission_denied.py @@ -1,8 +1,15 @@ """custom 403 handler to enable context processors""" + +from django.http import HttpResponse from django.template.response import TemplateResponse +from .helpers import is_api_request + def permission_denied(request, exception): # pylint: disable=unused-argument """permission denied page""" + if request.method == "POST" or is_api_request(request): + return HttpResponse(status=403) + return TemplateResponse(request, "403.html") From 97757fa1eeb6047269324b8abac52c16dc77e3f3 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sat, 18 Nov 2023 15:58:01 +1100 Subject: [PATCH 4/8] fix blocktrans --- bookwyrm/templates/403.html | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/bookwyrm/templates/403.html b/bookwyrm/templates/403.html index 64fd43536..0b78bc6b8 100644 --- a/bookwyrm/templates/403.html +++ b/bookwyrm/templates/403.html @@ -7,10 +7,14 @@ {% block content %}

{% trans "Permission Denied" %}

- {% blocktrans trimmed with level=request.user|get_user_permission %} -

You do not have permission to view this page or perform this action. Your user permission level is {{ level }}.

-

If you think you should have access, please speak to your BookWyrm server administrator.

- {% endblocktrans %} +

+ {% blocktrans trimmed with level=request.user|get_user_permission %} + You do not have permission to view this page or perform this action. Your user permission level is {{ level }}. + {% endblocktrans %} +

+

{% trans "If you think you should have access, please speak to your BookWyrm server administrator." %} +

+
{% endblock %} From d828ba0bc60cc62f0b8274986610c78457f7d39d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 20 Nov 2023 09:56:51 -0800 Subject: [PATCH 5/8] Give admins option to test if a theme loads correctly If a theme is uploaded incorrectly or has errors in it, users can still select the theme but it will cause a 500 error on every page, making the app unusable and also making it impossible for them to switch to a functional theme. A better fix would be to fail gracefully, but in lieu of that, this will at least let admins confirm if a theme is broken safely. --- bookwyrm/migrations/0187_theme_loads.py | 18 +++++++++++ bookwyrm/models/site.py | 1 + bookwyrm/templates/settings/themes.html | 43 +++++++++++++++++++++++++ bookwyrm/urls.py | 5 +++ bookwyrm/views/__init__.py | 2 +- bookwyrm/views/admin/themes.py | 20 ++++++++++++ 6 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 bookwyrm/migrations/0187_theme_loads.py diff --git a/bookwyrm/migrations/0187_theme_loads.py b/bookwyrm/migrations/0187_theme_loads.py new file mode 100644 index 000000000..3649c991c --- /dev/null +++ b/bookwyrm/migrations/0187_theme_loads.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.23 on 2023-11-20 17:44 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0186_invite_request_notification"), + ] + + operations = [ + migrations.AddField( + model_name="theme", + name="loads", + field=models.BooleanField(blank=True, null=True), + ), + ] diff --git a/bookwyrm/models/site.py b/bookwyrm/models/site.py index a27c4b70d..b962d597b 100644 --- a/bookwyrm/models/site.py +++ b/bookwyrm/models/site.py @@ -149,6 +149,7 @@ class Theme(SiteModel): created_date = models.DateTimeField(auto_now_add=True) name = models.CharField(max_length=50, unique=True) path = models.CharField(max_length=50, unique=True) + loads = models.BooleanField(null=True, blank=True) def __str__(self): # pylint: disable=invalid-str-returned diff --git a/bookwyrm/templates/settings/themes.html b/bookwyrm/templates/settings/themes.html index d27aeb0ce..c077fa5e3 100644 --- a/bookwyrm/templates/settings/themes.html +++ b/bookwyrm/templates/settings/themes.html @@ -12,6 +12,15 @@ {% endblock %} {% block panel %} +{% if broken_theme %} +
+ + + {% trans "One of your themes appears to be broken. Selecting this theme will make the application unusable." %} + +
+{% endif %} + {% if success %}
@@ -98,6 +107,9 @@ {% trans "Actions" %} + + {% trans "Status" %} + {% for theme in themes %} @@ -112,6 +124,37 @@ + + {% if theme.loads is None %} + +
+ {% csrf_token %} + +
+ + {% elif not theme.loads %} + + + + + {% trans "Broken theme" %} + + + + {% else %} + + + + + {% trans "Loaded successfully" %} + + + + {% endif %} + {% endfor %} diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py index 8541f4fb6..233e7786e 100644 --- a/bookwyrm/urls.py +++ b/bookwyrm/urls.py @@ -109,6 +109,11 @@ urlpatterns = [ views.delete_theme, name="settings-themes-delete", ), + re_path( + r"^settings/themes/(?P\d+)/test/?$", + views.test_theme, + name="settings-themes-test", + ), re_path( r"^settings/announcements/?$", views.Announcements.as_view(), diff --git a/bookwyrm/views/__init__.py b/bookwyrm/views/__init__.py index 2d2e97f52..4d298617c 100644 --- a/bookwyrm/views/__init__.py +++ b/bookwyrm/views/__init__.py @@ -30,7 +30,7 @@ from .admin.reports import ( moderator_delete_user, ) from .admin.site import Site, Registration, RegistrationLimited -from .admin.themes import Themes, delete_theme +from .admin.themes import Themes, delete_theme, test_theme from .admin.user_admin import UserAdmin, UserAdminList, ActivateUserAdmin # user preferences diff --git a/bookwyrm/views/admin/themes.py b/bookwyrm/views/admin/themes.py index 5658d243a..284a90833 100644 --- a/bookwyrm/views/admin/themes.py +++ b/bookwyrm/views/admin/themes.py @@ -6,6 +6,8 @@ from django.utils.decorators import method_decorator from django.views import View from django.views.decorators.http import require_POST +from sass_processor.processor import sass_processor + from bookwyrm import forms, models @@ -40,6 +42,7 @@ class Themes(View): def get_view_data(): """data for view""" return { + "broken_theme": models.Theme.objects.filter(loads=False).exists(), "themes": models.Theme.objects.all(), "theme_form": forms.ThemeForm(), } @@ -52,3 +55,20 @@ def delete_theme(request, theme_id): """Remove a theme""" get_object_or_404(models.Theme, id=theme_id).delete() return redirect("settings-themes") + + +@require_POST +@permission_required("bookwyrm.system_administration", raise_exception=True) +# pylint: disable=unused-argument +def test_theme(request, theme_id): + """Remove a theme""" + theme = get_object_or_404(models.Theme, id=theme_id) + + try: + sass_processor(theme.path) + theme.loads = True + except Exception: # pylint: disable=broad-except + theme.loads = False + + theme.save() + return redirect("settings-themes") From c2742b4d8078ab35c409e0ea2bf7cdb1eb544b0b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 20 Nov 2023 10:02:49 -0800 Subject: [PATCH 6/8] Updates migrations --- .../migrations/{0187_theme_loads.py => 0188_theme_loads.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename bookwyrm/migrations/{0187_theme_loads.py => 0188_theme_loads.py} (73%) diff --git a/bookwyrm/migrations/0187_theme_loads.py b/bookwyrm/migrations/0188_theme_loads.py similarity index 73% rename from bookwyrm/migrations/0187_theme_loads.py rename to bookwyrm/migrations/0188_theme_loads.py index 3649c991c..846aaf549 100644 --- a/bookwyrm/migrations/0187_theme_loads.py +++ b/bookwyrm/migrations/0188_theme_loads.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.23 on 2023-11-20 17:44 +# Generated by Django 3.2.23 on 2023-11-20 18:02 from django.db import migrations, models @@ -6,7 +6,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ("bookwyrm", "0186_invite_request_notification"), + ("bookwyrm", "0187_partial_publication_dates"), ] operations = [ From 179dbd75aa5aca7a08c0adf732de5c650cbe8175 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 20 Nov 2023 10:23:59 -0800 Subject: [PATCH 7/8] Adds tests --- bookwyrm/tests/views/admin/test_themes.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/bookwyrm/tests/views/admin/test_themes.py b/bookwyrm/tests/views/admin/test_themes.py index bc6377681..3f9cc5e0e 100644 --- a/bookwyrm/tests/views/admin/test_themes.py +++ b/bookwyrm/tests/views/admin/test_themes.py @@ -86,3 +86,25 @@ class AdminThemesViews(TestCase): with self.assertRaises(PermissionDenied): view(request) + + def test_test_theme(self): + """Testing testing testing test""" + theme = models.Theme.objects.first() + self.assertIsNone(theme.loads) + request = self.factory.post("") + request.user = self.local_user + + views.test_theme(request, theme.id) + theme.refresh_from_db() + self.assertTrue(theme.loads) + + def test_test_theme_broken(self): + """Testing test for testing when it's a bad theme""" + theme = models.Theme.objects.create(name="bad theme", path="dsf/sdf/sdf.sdf") + self.assertIsNone(theme.loads) + request = self.factory.post("") + request.user = self.local_user + + views.test_theme(request, theme.id) + theme.refresh_from_db() + self.assertFalse(theme.loads) From b6325da9ab0e0813b906aecd6e156659af00ef31 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 20 Nov 2023 10:37:12 -0800 Subject: [PATCH 8/8] Update bookwyrm/tests/views/admin/test_themes.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Adeodato Simó <73768+dato@users.noreply.github.com> --- bookwyrm/tests/views/admin/test_themes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/tests/views/admin/test_themes.py b/bookwyrm/tests/views/admin/test_themes.py index 3f9cc5e0e..296cd4d8d 100644 --- a/bookwyrm/tests/views/admin/test_themes.py +++ b/bookwyrm/tests/views/admin/test_themes.py @@ -107,4 +107,4 @@ class AdminThemesViews(TestCase): views.test_theme(request, theme.id) theme.refresh_from_db() - self.assertFalse(theme.loads) + self.assertIs(False, theme.loads)