From ea240eefd9061dc1a7a712be356b7f9633a02e63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Tue, 15 Aug 2023 00:20:52 +0200 Subject: [PATCH] Add option to share bookmarks publicly (#503) * Make shared view public, add user profile fallback * Allow unauthenticated access to shared bookmarks API * Link shared bookmarks in unauthenticated layout * Add public sharing setting * Only show shared bookmarks link if there are publicly shared bookmarks * Disable public sharing if sharing is disabled * Show specific helper text when public sharing is enabled * Fix tests * Add more tests * Improve setting description --- bookmarks/api/routes.py | 15 ++- bookmarks/context_processors.py | 17 ++- bookmarks/e2e/e2e_test_settings_general.py | 39 ++++++ bookmarks/middlewares.py | 18 +++ .../0024_userprofile_enable_public_sharing.py | 18 +++ bookmarks/models.py | 3 +- bookmarks/queries.py | 20 +-- .../templates/bookmarks/bookmark_list.html | 12 +- bookmarks/templates/bookmarks/form.html | 8 +- bookmarks/templates/bookmarks/layout.html | 11 +- bookmarks/templates/bookmarks/nav_menu.html | 4 +- bookmarks/templates/settings/general.html | 34 +++++- bookmarks/templatetags/shared.py | 2 +- bookmarks/tests/helpers.py | 3 +- bookmarks/tests/test_anonymous_view.py | 26 ++++ bookmarks/tests/test_bookmark_new_view.py | 36 +++++- bookmarks/tests/test_bookmark_search_tag.py | 2 + bookmarks/tests/test_bookmark_shared_view.py | 115 +++++++++++++++--- bookmarks/tests/test_bookmarks_api.py | 84 ++++++++++++- .../tests/test_bookmarks_api_permissions.py | 113 +++++++++++++++++ bookmarks/tests/test_bookmarks_list_tag.py | 45 +++++-- bookmarks/tests/test_pagination_tag.py | 8 +- bookmarks/tests/test_queries.py | 46 ++++++- bookmarks/tests/test_settings_general_view.py | 3 + bookmarks/tests/test_tag_cloud_tag.py | 44 ++++++- bookmarks/tests/test_user_select_tag.py | 2 + bookmarks/views/bookmarks.py | 20 +-- bookmarks/views/settings.py | 4 +- siteroot/settings/base.py | 2 + 29 files changed, 667 insertions(+), 87 deletions(-) create mode 100644 bookmarks/e2e/e2e_test_settings_general.py create mode 100644 bookmarks/migrations/0024_userprofile_enable_public_sharing.py create mode 100644 bookmarks/tests/test_anonymous_view.py create mode 100644 bookmarks/tests/test_bookmarks_api_permissions.py diff --git a/bookmarks/api/routes.py b/bookmarks/api/routes.py index e99612d..1f1005b 100644 --- a/bookmarks/api/routes.py +++ b/bookmarks/api/routes.py @@ -1,5 +1,6 @@ from rest_framework import viewsets, mixins, status from rest_framework.decorators import action +from rest_framework.permissions import AllowAny from rest_framework.response import Response from rest_framework.routers import DefaultRouter @@ -18,6 +19,17 @@ class BookmarkViewSet(viewsets.GenericViewSet, mixins.DestroyModelMixin): serializer_class = BookmarkSerializer + def get_permissions(self): + # Allow unauthenticated access to shared bookmarks. + # The shared action should still filter bookmarks so that + # unauthenticated users only see bookmarks from users that have public + # sharing explicitly enabled + if self.action == 'shared': + return [AllowAny()] + + # Otherwise use default permissions which should require authentication + return super().get_permissions() + def get_queryset(self): user = self.request.user # For list action, use query set that applies search and tag projections @@ -45,7 +57,8 @@ class BookmarkViewSet(viewsets.GenericViewSet, def shared(self, request): filters = BookmarkFilters(request) user = User.objects.filter(username=filters.user).first() - query_set = queries.query_shared_bookmarks(user, request.user.profile, filters.query) + public_only = not request.user.is_authenticated + query_set = queries.query_shared_bookmarks(user, request.user_profile, filters.query, public_only) page = self.paginate_queryset(query_set) serializer = self.get_serializer_class() data = serializer(page, many=True).data diff --git a/bookmarks/context_processors.py b/bookmarks/context_processors.py index 1773b6e..e1c171d 100644 --- a/bookmarks/context_processors.py +++ b/bookmarks/context_processors.py @@ -1,12 +1,25 @@ +from bookmarks import queries from bookmarks.models import Toast def toasts(request): - user = request.user if hasattr(request, 'user') else None - toast_messages = Toast.objects.filter(owner=user, acknowledged=False) if user and user.is_authenticated else [] + user = request.user + toast_messages = Toast.objects.filter(owner=user, acknowledged=False) if user.is_authenticated else [] has_toasts = len(toast_messages) > 0 return { 'has_toasts': has_toasts, 'toast_messages': toast_messages, } + + +def public_shares(request): + # Only check for public shares for anonymous users + if not request.user.is_authenticated: + query_set = queries.query_shared_bookmarks(None, request.user_profile, '', True) + has_public_shares = query_set.count() > 0 + return { + 'has_public_shares': has_public_shares, + } + + return {} diff --git a/bookmarks/e2e/e2e_test_settings_general.py b/bookmarks/e2e/e2e_test_settings_general.py new file mode 100644 index 0000000..9e761f1 --- /dev/null +++ b/bookmarks/e2e/e2e_test_settings_general.py @@ -0,0 +1,39 @@ +from django.urls import reverse +from playwright.sync_api import sync_playwright, expect + +from bookmarks.e2e.helpers import LinkdingE2ETestCase + + +class SettingsGeneralE2ETestCase(LinkdingE2ETestCase): + def test_should_only_enable_public_sharing_if_sharing_is_enabled(self): + with sync_playwright() as p: + browser = self.setup_browser(p) + page = browser.new_page() + page.goto(self.live_server_url + reverse('bookmarks:settings.general')) + + enable_sharing = page.get_by_label('Enable bookmark sharing') + enable_sharing_label = page.get_by_text('Enable bookmark sharing') + enable_public_sharing = page.get_by_label('Enable public bookmark sharing') + enable_public_sharing_label = page.get_by_text('Enable public bookmark sharing') + + # Public sharing is disabled by default + expect(enable_sharing).not_to_be_checked() + expect(enable_public_sharing).not_to_be_checked() + expect(enable_public_sharing).to_be_disabled() + + # Enable sharing + enable_sharing_label.click() + expect(enable_sharing).to_be_checked() + expect(enable_public_sharing).not_to_be_checked() + expect(enable_public_sharing).to_be_enabled() + + # Enable public sharing + enable_public_sharing_label.click() + expect(enable_public_sharing).to_be_checked() + expect(enable_public_sharing).to_be_enabled() + + # Disable sharing + enable_sharing_label.click() + expect(enable_sharing).not_to_be_checked() + expect(enable_public_sharing).not_to_be_checked() + expect(enable_public_sharing).to_be_disabled() diff --git a/bookmarks/middlewares.py b/bookmarks/middlewares.py index 3caded1..981137f 100644 --- a/bookmarks/middlewares.py +++ b/bookmarks/middlewares.py @@ -1,6 +1,24 @@ from django.conf import settings from django.contrib.auth.middleware import RemoteUserMiddleware +from bookmarks.models import UserProfile + class CustomRemoteUserMiddleware(RemoteUserMiddleware): header = settings.LD_AUTH_PROXY_USERNAME_HEADER + + +class UserProfileMiddleware: + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + if request.user.is_authenticated: + request.user_profile = request.user.profile + else: + request.user_profile = UserProfile() + request.user_profile.enable_favicons = True + + response = self.get_response(request) + + return response diff --git a/bookmarks/migrations/0024_userprofile_enable_public_sharing.py b/bookmarks/migrations/0024_userprofile_enable_public_sharing.py new file mode 100644 index 0000000..db5f40b --- /dev/null +++ b/bookmarks/migrations/0024_userprofile_enable_public_sharing.py @@ -0,0 +1,18 @@ +# Generated by Django 4.1.9 on 2023-08-14 07:08 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('bookmarks', '0023_userprofile_permanent_notes'), + ] + + operations = [ + migrations.AddField( + model_name='userprofile', + name='enable_public_sharing', + field=models.BooleanField(default=False), + ), + ] diff --git a/bookmarks/models.py b/bookmarks/models.py index 253ec59..5023b29 100644 --- a/bookmarks/models.py +++ b/bookmarks/models.py @@ -176,6 +176,7 @@ class UserProfile(models.Model): tag_search = models.CharField(max_length=10, choices=TAG_SEARCH_CHOICES, blank=False, default=TAG_SEARCH_STRICT) enable_sharing = models.BooleanField(default=False, null=False) + enable_public_sharing = models.BooleanField(default=False, null=False) enable_favicons = models.BooleanField(default=False, null=False) display_url = models.BooleanField(default=False, null=False) permanent_notes = models.BooleanField(default=False, null=False) @@ -185,7 +186,7 @@ class UserProfileForm(forms.ModelForm): class Meta: model = UserProfile fields = ['theme', 'bookmark_date_display', 'bookmark_link_target', 'web_archive_integration', 'tag_search', - 'enable_sharing', 'enable_favicons', 'display_url', 'permanent_notes'] + 'enable_sharing', 'enable_public_sharing', 'enable_favicons', 'display_url', 'permanent_notes'] @receiver(post_save, sender=get_user_model()) diff --git a/bookmarks/queries.py b/bookmarks/queries.py index fc07239..6af1b5c 100644 --- a/bookmarks/queries.py +++ b/bookmarks/queries.py @@ -17,10 +17,13 @@ def query_archived_bookmarks(user: User, profile: UserProfile, query_string: str .filter(is_archived=True) -def query_shared_bookmarks(user: Optional[User], profile: UserProfile, query_string: str) -> QuerySet: - return _base_bookmarks_query(user, profile, query_string) \ - .filter(shared=True) \ - .filter(owner__profile__enable_sharing=True) +def query_shared_bookmarks(user: Optional[User], profile: UserProfile, query_string: str, + public_only: bool) -> QuerySet: + conditions = Q(shared=True) & Q(owner__profile__enable_sharing=True) + if public_only: + conditions = conditions & Q(owner__profile__enable_public_sharing=True) + + return _base_bookmarks_query(user, profile, query_string).filter(conditions) def _base_bookmarks_query(user: Optional[User], profile: UserProfile, query_string: str) -> QuerySet: @@ -85,16 +88,17 @@ def query_archived_bookmark_tags(user: User, profile: UserProfile, query_string: return query_set.distinct() -def query_shared_bookmark_tags(user: Optional[User], profile: UserProfile, query_string: str) -> QuerySet: - bookmarks_query = query_shared_bookmarks(user, profile, query_string) +def query_shared_bookmark_tags(user: Optional[User], profile: UserProfile, query_string: str, + public_only: bool) -> QuerySet: + bookmarks_query = query_shared_bookmarks(user, profile, query_string, public_only) query_set = Tag.objects.filter(bookmark__in=bookmarks_query) return query_set.distinct() -def query_shared_bookmark_users(profile: UserProfile, query_string: str) -> QuerySet: - bookmarks_query = query_shared_bookmarks(None, profile, query_string) +def query_shared_bookmark_users(profile: UserProfile, query_string: str, public_only: bool) -> QuerySet: + bookmarks_query = query_shared_bookmarks(None, profile, query_string, public_only) query_set = User.objects.filter(bookmark__in=bookmarks_query) diff --git a/bookmarks/templates/bookmarks/bookmark_list.html b/bookmarks/templates/bookmarks/bookmark_list.html index 8f1b54f..51733ae 100644 --- a/bookmarks/templates/bookmarks/bookmark_list.html +++ b/bookmarks/templates/bookmarks/bookmark_list.html @@ -1,7 +1,7 @@ {% load static %} {% load shared %} {% load pagination %} -