From 5eadb3ede382d49a415b6bc0b88fecdf783fbd38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Sat, 31 Aug 2024 15:39:22 +0200 Subject: [PATCH] Allow configuring landing page for unauthenticated users (#808) * allow configuring landing page * add tests --- bookmarks/migrations/0037_globalsettings.py | 38 ++++++++++++ bookmarks/models.py | 37 +++++++++++ bookmarks/templates/bookmarks/layout.html | 8 +-- bookmarks/templates/settings/general.html | 34 ++++++++-- bookmarks/tests/helpers.py | 5 ++ bookmarks/tests/test_anonymous_view.py | 29 --------- bookmarks/tests/test_root_view.py | 40 ++++++++++++ bookmarks/tests/test_settings_general_view.py | 62 ++++++++++++++++++- bookmarks/urls.py | 7 +-- bookmarks/views/__init__.py | 1 + bookmarks/views/root.py | 18 ++++++ bookmarks/views/settings.py | 28 ++++++++- 12 files changed, 261 insertions(+), 46 deletions(-) create mode 100644 bookmarks/migrations/0037_globalsettings.py delete mode 100644 bookmarks/tests/test_anonymous_view.py create mode 100644 bookmarks/tests/test_root_view.py create mode 100644 bookmarks/views/root.py diff --git a/bookmarks/migrations/0037_globalsettings.py b/bookmarks/migrations/0037_globalsettings.py new file mode 100644 index 0000000..48aa8e5 --- /dev/null +++ b/bookmarks/migrations/0037_globalsettings.py @@ -0,0 +1,38 @@ +# Generated by Django 5.0.8 on 2024-08-31 12:39 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookmarks", "0036_userprofile_auto_tagging_rules"), + ] + + operations = [ + migrations.CreateModel( + name="GlobalSettings", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "landing_page", + models.CharField( + choices=[ + ("login", "Login"), + ("shared_bookmarks", "Shared Bookmarks"), + ], + default="login", + max_length=50, + ), + ), + ], + ), + ] diff --git a/bookmarks/models.py b/bookmarks/models.py index a8120d5..71dda82 100644 --- a/bookmarks/models.py +++ b/bookmarks/models.py @@ -492,3 +492,40 @@ class FeedToken(models.Model): def __str__(self): return self.key + + +class GlobalSettings(models.Model): + LANDING_PAGE_LOGIN = "login" + LANDING_PAGE_SHARED_BOOKMARKS = "shared_bookmarks" + LANDING_PAGE_CHOICES = [ + (LANDING_PAGE_LOGIN, "Login"), + (LANDING_PAGE_SHARED_BOOKMARKS, "Shared Bookmarks"), + ] + + landing_page = models.CharField( + max_length=50, + choices=LANDING_PAGE_CHOICES, + blank=False, + default=LANDING_PAGE_LOGIN, + ) + + @classmethod + def get(cls): + instance = GlobalSettings.objects.first() + if not instance: + instance = GlobalSettings() + instance.save() + return instance + + def save(self, *args, **kwargs): + if not self.pk and GlobalSettings.objects.exists(): + raise Exception("There is already one instance of GlobalSettings") + return super(GlobalSettings, self).save(*args, **kwargs) + + +class GlobalSettingsForm(forms.ModelForm): + class Meta: + model = GlobalSettings + fields = [ + "landing_page", + ] diff --git a/bookmarks/templates/bookmarks/layout.html b/bookmarks/templates/bookmarks/layout.html index b75959d..3ec9d9c 100644 --- a/bookmarks/templates/bookmarks/layout.html +++ b/bookmarks/templates/bookmarks/layout.html @@ -114,16 +114,16 @@ {% endif %}
- +

LINKDING

{% if request.user.is_authenticated %} {# Only show nav items menu when logged in #} {% include 'bookmarks/nav_menu.html' %} - {% elif has_public_shares %} - {# Otherwise show link to shared bookmarks if there are publicly shared bookmarks #} - Shared bookmarks + {% else %} + {# Otherwise show login link #} + Login {% endif %}
diff --git a/bookmarks/templates/settings/general.html b/bookmarks/templates/settings/general.html index d12c64e..eae24bf 100644 --- a/bookmarks/templates/settings/general.html +++ b/bookmarks/templates/settings/general.html @@ -7,13 +7,14 @@ {% include 'settings/nav.html' %} {# Profile section #} + {% if success_message %} +
{{ success_message }}
+ {% endif %} + {% if error_message %} +
{{ error_message }}
+ {% endif %} +
- {% if success_message %} -
{{ success_message }}
- {% endif %} - {% if error_message %} -
{{ error_message }}
- {% endif %}

Profile

Change password @@ -238,6 +239,27 @@ reddit.com/r/Music music reddit

+ {# Global settings section #} + {% if global_settings_form %} +
+

Global settings

+
+ {% csrf_token %} +
+ + {{ global_settings_form.landing_page|add_class:"form-select width-25 width-sm-100" }} +
+ The page that unauthorized users are redirected to when accessing the root URL. +
+
+ +
+ +
+
+
+ {% endif %} + {# Import section #}

Import

diff --git a/bookmarks/tests/helpers.py b/bookmarks/tests/helpers.py index 1e8b092..dd97f2d 100644 --- a/bookmarks/tests/helpers.py +++ b/bookmarks/tests/helpers.py @@ -24,6 +24,11 @@ class BookmarkFactoryMixin: return self.user + def setup_superuser(self): + return User.objects.create_superuser( + "superuser", "superuser@example.com", "password123" + ) + def setup_bookmark( self, is_archived: bool = False, diff --git a/bookmarks/tests/test_anonymous_view.py b/bookmarks/tests/test_anonymous_view.py deleted file mode 100644 index 8b8515f..0000000 --- a/bookmarks/tests/test_anonymous_view.py +++ /dev/null @@ -1,29 +0,0 @@ -from django.test import TestCase -from django.urls import reverse - -from bookmarks.tests.helpers import BookmarkFactoryMixin - - -class AnonymousViewTestCase(TestCase, BookmarkFactoryMixin): - def assertSharedBookmarksLinkCount(self, response, count): - url = reverse("bookmarks:shared") - self.assertContains( - response, - f'Shared bookmarks', - count=count, - ) - - def test_publicly_shared_bookmarks_link(self): - # should not render link if no public shares exist - user = self.setup_user(enable_sharing=True) - self.setup_bookmark(user=user, shared=True) - - response = self.client.get(reverse("login")) - self.assertSharedBookmarksLinkCount(response, 0) - - # should render link if public shares exist - user.profile.enable_public_sharing = True - user.profile.save() - - response = self.client.get(reverse("login")) - self.assertSharedBookmarksLinkCount(response, 1) diff --git a/bookmarks/tests/test_root_view.py b/bookmarks/tests/test_root_view.py new file mode 100644 index 0000000..a02b598 --- /dev/null +++ b/bookmarks/tests/test_root_view.py @@ -0,0 +1,40 @@ +from django.test import TestCase +from django.urls import reverse + +from bookmarks.models import GlobalSettings +from bookmarks.tests.helpers import BookmarkFactoryMixin + + +class AnonymousViewTestCase(TestCase, BookmarkFactoryMixin): + def test_unauthenticated_user_redirect_to_login_by_default(self): + response = self.client.get(reverse("bookmarks:root")) + self.assertRedirects(response, reverse("login")) + + def test_unauthenticated_redirect_to_shared_bookmarks_if_configured_in_global_settings( + self, + ): + settings = GlobalSettings.get() + settings.landing_page = GlobalSettings.LANDING_PAGE_SHARED_BOOKMARKS + settings.save() + + response = self.client.get(reverse("bookmarks:root")) + self.assertRedirects(response, reverse("bookmarks:shared")) + + def test_authenticated_user_always_redirected_to_bookmarks(self): + self.client.force_login(self.get_or_create_test_user()) + + response = self.client.get(reverse("bookmarks:root")) + self.assertRedirects(response, reverse("bookmarks:index")) + + settings = GlobalSettings.get() + settings.landing_page = GlobalSettings.LANDING_PAGE_SHARED_BOOKMARKS + settings.save() + + response = self.client.get(reverse("bookmarks:root")) + self.assertRedirects(response, reverse("bookmarks:index")) + + settings.landing_page = GlobalSettings.LANDING_PAGE_LOGIN + settings.save() + + response = self.client.get(reverse("bookmarks:root")) + self.assertRedirects(response, reverse("bookmarks:index")) diff --git a/bookmarks/tests/test_settings_general_view.py b/bookmarks/tests/test_settings_general_view.py index 80a6651..568a7c7 100644 --- a/bookmarks/tests/test_settings_general_view.py +++ b/bookmarks/tests/test_settings_general_view.py @@ -6,7 +6,7 @@ from django.test import TestCase, override_settings from django.urls import reverse from requests import RequestException -from bookmarks.models import UserProfile +from bookmarks.models import UserProfile, GlobalSettings from bookmarks.services import tasks from bookmarks.tests.helpers import BookmarkFactoryMixin from bookmarks.views.settings import app_version, get_version_info @@ -465,3 +465,63 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin): self.assertSuccessMessage( html, "Queued 5 missing snapshots. This may take a while...", count=0 ) + + def test_update_global_settings(self): + superuser = self.setup_superuser() + self.client.force_login(superuser) + + form_data = { + "update_global_settings": "", + "landing_page": GlobalSettings.LANDING_PAGE_SHARED_BOOKMARKS, + } + response = self.client.post(reverse("bookmarks:settings.general"), form_data) + self.assertEqual(response.status_code, 200) + self.assertSuccessMessage(response.content.decode(), "Global settings updated") + + global_settings = GlobalSettings.get() + self.assertEqual(global_settings.landing_page, form_data["landing_page"]) + + def test_update_global_settings_should_not_be_called_without_respective_form_action( + self, + ): + superuser = self.setup_superuser() + self.client.force_login(superuser) + + form_data = { + "landing_page": GlobalSettings.LANDING_PAGE_SHARED_BOOKMARKS, + } + response = self.client.post(reverse("bookmarks:settings.general"), form_data) + self.assertEqual(response.status_code, 200) + self.assertSuccessMessage( + response.content.decode(), "Global settings updated", count=0 + ) + + def test_update_global_settings_checks_for_superuser(self): + form_data = { + "update_global_settings": "", + "landing_page": GlobalSettings.LANDING_PAGE_SHARED_BOOKMARKS, + } + response = self.client.post(reverse("bookmarks:settings.general"), form_data) + self.assertEqual(response.status_code, 403) + + def test_global_settings_only_visible_for_superuser(self): + response = self.client.get(reverse("bookmarks:settings.general")) + html = response.content.decode() + + self.assertInHTML( + "

Global settings

", + html, + count=0, + ) + + superuser = self.setup_superuser() + self.client.force_login(superuser) + + response = self.client.get(reverse("bookmarks:settings.general")) + html = response.content.decode() + + self.assertInHTML( + "

Global settings

", + html, + count=1, + ) diff --git a/bookmarks/urls.py b/bookmarks/urls.py index f92dd39..fd855e3 100644 --- a/bookmarks/urls.py +++ b/bookmarks/urls.py @@ -1,6 +1,5 @@ from django.urls import path, include from django.urls import re_path -from django.views.generic import RedirectView from bookmarks import views from bookmarks.api.routes import router @@ -14,10 +13,8 @@ from bookmarks.views import partials app_name = "bookmarks" urlpatterns = [ - # Redirect root to bookmarks index - re_path( - r"^$", RedirectView.as_view(pattern_name="bookmarks:index", permanent=False) - ), + # Root view handling redirection based on user authentication + re_path(r"^$", views.root, name="root"), # Bookmarks path("bookmarks", views.bookmarks.index, name="index"), path("bookmarks/action", views.bookmarks.index_action, name="index.action"), diff --git a/bookmarks/views/__init__.py b/bookmarks/views/__init__.py index 3f26b62..b9a9f0e 100644 --- a/bookmarks/views/__init__.py +++ b/bookmarks/views/__init__.py @@ -4,3 +4,4 @@ from .settings import * from .toasts import * from .health import health from .manifest import manifest +from .root import root diff --git a/bookmarks/views/root.py b/bookmarks/views/root.py new file mode 100644 index 0000000..8b72846 --- /dev/null +++ b/bookmarks/views/root.py @@ -0,0 +1,18 @@ +from django.http import HttpResponseRedirect +from django.urls import reverse + +from bookmarks.models import GlobalSettings + + +def root(request): + # Redirect unauthenticated users to the configured landing page + if not request.user.is_authenticated: + settings = GlobalSettings.get() + + if settings.landing_page == GlobalSettings.LANDING_PAGE_SHARED_BOOKMARKS: + return HttpResponseRedirect(reverse("bookmarks:shared")) + else: + return HttpResponseRedirect(reverse("login")) + + # Redirect authenticated users to the bookmarks page + return HttpResponseRedirect(reverse("bookmarks:index")) diff --git a/bookmarks/views/settings.py b/bookmarks/views/settings.py index 582c2a2..3c8a1f5 100644 --- a/bookmarks/views/settings.py +++ b/bookmarks/views/settings.py @@ -6,13 +6,20 @@ import requests from django.conf import settings as django_settings from django.contrib import messages from django.contrib.auth.decorators import login_required +from django.core.exceptions import PermissionDenied from django.db.models import prefetch_related_objects from django.http import HttpResponseRedirect, HttpResponse from django.shortcuts import render from django.urls import reverse from rest_framework.authtoken.models import Token -from bookmarks.models import Bookmark, UserProfileForm, FeedToken +from bookmarks.models import ( + Bookmark, + UserProfileForm, + FeedToken, + GlobalSettings, + GlobalSettingsForm, +) from bookmarks.services import exporter, tasks from bookmarks.services import importer from bookmarks.utils import app_version @@ -23,6 +30,7 @@ logger = logging.getLogger(__name__) @login_required def general(request): profile_form = None + global_settings_form = None enable_refresh_favicons = django_settings.LD_ENABLE_REFRESH_FAVICONS has_snapshot_support = django_settings.LD_ENABLE_SNAPSHOTS success_message = _find_message_with_tag( @@ -37,6 +45,9 @@ def general(request): if "update_profile" in request.POST: profile_form = update_profile(request) success_message = "Profile updated" + if "update_global_settings" in request.POST: + global_settings_form = update_global_settings(request) + success_message = "Global settings updated" if "refresh_favicons" in request.POST: tasks.schedule_refresh_favicons(request.user) success_message = "Scheduled favicon update. This may take a while..." @@ -52,11 +63,15 @@ def general(request): if not profile_form: profile_form = UserProfileForm(instance=request.user_profile) + if request.user.is_superuser and not global_settings_form: + global_settings_form = GlobalSettingsForm(instance=GlobalSettings.get()) + return render( request, "settings/general.html", { "form": profile_form, + "global_settings_form": global_settings_form, "enable_refresh_favicons": enable_refresh_favicons, "has_snapshot_support": has_snapshot_support, "success_message": success_message, @@ -83,6 +98,17 @@ def update_profile(request): return form +def update_global_settings(request): + user = request.user + if not user.is_superuser: + raise PermissionDenied() + + form = GlobalSettingsForm(request.POST, instance=GlobalSettings.get()) + if form.is_valid(): + form.save() + return form + + # Cache API call response, for one hour when using get_ttl_hash with default params @lru_cache(maxsize=1) def get_version_info(ttl_hash=None):