diff --git a/bookmarks/api/routes.py b/bookmarks/api/routes.py index 2ecade3..78d9c58 100644 --- a/bookmarks/api/routes.py +++ b/bookmarks/api/routes.py @@ -20,6 +20,7 @@ from bookmarks.api.serializers import ( from bookmarks.models import Bookmark, BookmarkAsset, BookmarkSearch, Tag, User from bookmarks.services import assets, bookmarks, auto_tagging, website_loader from bookmarks.type_defs import HttpRequest +from bookmarks.views import access logger = logging.getLogger(__name__) @@ -169,11 +170,10 @@ class BookmarkAssetViewSet( def get_queryset(self): user = self.request.user - bookmark_id = self.kwargs["bookmark_id"] - if not Bookmark.objects.filter(id=bookmark_id, owner=user).exists(): - raise Http404("Bookmark does not exist") + # limit access to assets to the owner of the bookmark for now + bookmark = access.bookmark_write(self.request, self.kwargs["bookmark_id"]) return BookmarkAsset.objects.filter( - bookmark_id=bookmark_id, bookmark__owner=user + bookmark_id=bookmark.id, bookmark__owner=user ) def get_serializer_context(self): @@ -214,9 +214,7 @@ class BookmarkAssetViewSet( {"error": "Asset upload is disabled."}, status=status.HTTP_403_FORBIDDEN, ) - bookmark = Bookmark.objects.filter(id=bookmark_id, owner=request.user).first() - if not bookmark: - raise Http404("Bookmark does not exist") + bookmark = access.bookmark_write(request, bookmark_id) upload_file = request.FILES.get("file") if not upload_file: diff --git a/bookmarks/tests/test_bookmark_details_modal.py b/bookmarks/tests/test_bookmark_details_modal.py index f5bb41c..c99c58d 100644 --- a/bookmarks/tests/test_bookmark_details_modal.py +++ b/bookmarks/tests/test_bookmark_details_modal.py @@ -23,16 +23,18 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def get_index_details_modal(self, bookmark): url = reverse("linkding:bookmarks.index") + f"?details={bookmark.id}" response = self.client.get(url) - soup = self.make_soup(response.content) - modal = soup.find("turbo-frame", {"id": "details-modal"}) - return modal + soup = self.make_soup(response.content.decode()) + return soup.select_one("div.modal.bookmark-details") def get_shared_details_modal(self, bookmark): url = reverse("linkding:bookmarks.shared") + f"?details={bookmark.id}" response = self.client.get(url) - soup = self.make_soup(response.content) - modal = soup.find("turbo-frame", {"id": "details-modal"}) - return modal + soup = self.make_soup(response.content.decode()) + return soup.select_one("div.modal.bookmark-details") + + def has_details_modal(self, response): + soup = self.make_soup(response.content.decode()) + return soup.select_one("div.modal.bookmark-details") is not None def find_section_content(self, soup, section_name): h3 = soup.find("h3", string=section_name) @@ -53,35 +55,6 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def find_asset(self, soup, asset): return soup.find("div", {"data-asset-id": asset.id}) - def details_route_access_test(self): - # own bookmark - bookmark = self.setup_bookmark() - response = self.client.get( - reverse("linkding:bookmarks.index") + f"?details={bookmark.id}" - ) - self.assertEqual(response.status_code, 200) - - # other user's bookmark - other_user = self.setup_user() - bookmark = self.setup_bookmark(user=other_user) - response = self.client.get( - reverse("linkding:bookmarks.index") + f"?details={bookmark.id}" - ) - self.assertEqual(response.status_code, 404) - - # non-existent bookmark - just returns without modal in response - response = self.client.get( - reverse("linkding:bookmarks.index") + "?details=9999" - ) - self.assertEqual(response.status_code, 200) - - # guest user - self.client.logout() - response = self.client.get( - reverse("linkding:bookmarks.shared") + f"?details={bookmark.id}" - ) - self.assertEqual(response.status_code, 404) - def test_access(self): # own bookmark bookmark = self.setup_bookmark() @@ -89,6 +62,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin reverse("linkding:bookmarks.index") + f"?details={bookmark.id}" ) self.assertEqual(response.status_code, 200) + self.assertTrue(self.has_details_modal(response)) # other user's bookmark other_user = self.setup_user() @@ -96,20 +70,23 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin response = self.client.get( reverse("linkding:bookmarks.index") + f"?details={bookmark.id}" ) - self.assertEqual(response.status_code, 404) + self.assertEqual(response.status_code, 200) + self.assertFalse(self.has_details_modal(response)) # non-existent bookmark - just returns without modal in response response = self.client.get( reverse("linkding:bookmarks.index") + "?details=9999" ) self.assertEqual(response.status_code, 200) + self.assertFalse(self.has_details_modal(response)) # guest user self.client.logout() response = self.client.get( reverse("linkding:bookmarks.shared") + f"?details={bookmark.id}" ) - self.assertEqual(response.status_code, 404) + self.assertEqual(response.status_code, 200) + self.assertFalse(self.has_details_modal(response)) def test_access_with_sharing(self): # shared bookmark, sharing disabled @@ -119,7 +96,8 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin response = self.client.get( reverse("linkding:bookmarks.shared") + f"?details={bookmark.id}" ) - self.assertEqual(response.status_code, 404) + self.assertEqual(response.status_code, 200) + self.assertFalse(self.has_details_modal(response)) # shared bookmark, sharing enabled profile = other_user.profile @@ -130,13 +108,15 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin reverse("linkding:bookmarks.shared") + f"?details={bookmark.id}" ) self.assertEqual(response.status_code, 200) + self.assertTrue(self.has_details_modal(response)) # shared bookmark, guest user, no public sharing self.client.logout() response = self.client.get( reverse("linkding:bookmarks.shared") + f"?details={bookmark.id}" ) - self.assertEqual(response.status_code, 404) + self.assertEqual(response.status_code, 200) + self.assertFalse(self.has_details_modal(response)) # shared bookmark, guest user, public sharing profile.enable_public_sharing = True @@ -146,6 +126,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin reverse("linkding:bookmarks.shared") + f"?details={bookmark.id}" ) self.assertEqual(response.status_code, 200) + self.assertTrue(self.has_details_modal(response)) def test_displays_title(self): # with title diff --git a/bookmarks/views/access.py b/bookmarks/views/access.py new file mode 100644 index 0000000..a22070e --- /dev/null +++ b/bookmarks/views/access.py @@ -0,0 +1,56 @@ +from django.http import Http404 + +from bookmarks.models import Bookmark, BookmarkAsset, Toast +from bookmarks.type_defs import HttpRequest + + +def bookmark_read(request: HttpRequest, bookmark_id: int | str): + try: + bookmark = Bookmark.objects.get(pk=int(bookmark_id)) + except Bookmark.DoesNotExist: + raise Http404("Bookmark does not exist") + + is_owner = bookmark.owner == request.user + is_shared = ( + request.user.is_authenticated + and bookmark.shared + and bookmark.owner.profile.enable_sharing + ) + is_public_shared = bookmark.shared and bookmark.owner.profile.enable_public_sharing + if not is_owner and not is_shared and not is_public_shared: + raise Http404("Bookmark does not exist") + if request.method == "POST" and not is_owner: + raise Http404("Bookmark does not exist") + + return bookmark + + +def bookmark_write(request: HttpRequest, bookmark_id: int | str): + try: + return Bookmark.objects.get(pk=bookmark_id, owner=request.user) + except Bookmark.DoesNotExist: + raise Http404("Bookmark does not exist") + + +def asset_read(request: HttpRequest, asset_id: int | str): + try: + asset = BookmarkAsset.objects.get(pk=asset_id) + except BookmarkAsset.DoesNotExist: + raise Http404("Asset does not exist") + + bookmark_read(request, asset.bookmark_id) + return asset + + +def asset_write(request: HttpRequest, asset_id: int | str): + try: + return BookmarkAsset.objects.get(pk=asset_id, bookmark__owner=request.user) + except BookmarkAsset.DoesNotExist: + raise Http404("Asset does not exist") + + +def toast_write(request: HttpRequest, toast_id: int | str): + try: + return Toast.objects.get(pk=toast_id, owner=request.user) + except Toast.DoesNotExist: + raise Http404("Toast does not exist") diff --git a/bookmarks/views/assets.py b/bookmarks/views/assets.py index d5173a3..c41d957 100644 --- a/bookmarks/views/assets.py +++ b/bookmarks/views/assets.py @@ -8,28 +8,7 @@ from django.http import ( ) from django.shortcuts import render -from bookmarks.models import BookmarkAsset - - -def _access_asset(request, asset_id: int): - try: - asset = BookmarkAsset.objects.get(pk=asset_id) - except BookmarkAsset.DoesNotExist: - raise Http404("Asset does not exist") - - bookmark = asset.bookmark - is_owner = bookmark.owner == request.user - is_shared = ( - request.user.is_authenticated - and bookmark.shared - and bookmark.owner.profile.enable_sharing - ) - is_public_shared = bookmark.shared and bookmark.owner.profile.enable_public_sharing - - if not is_owner and not is_shared and not is_public_shared: - raise Http404("Bookmark does not exist") - - return asset +from bookmarks.views import access def _get_asset_content(asset): @@ -49,14 +28,14 @@ def _get_asset_content(asset): def view(request, asset_id: int): - asset = _access_asset(request, asset_id) + asset = access.asset_read(request, asset_id) content = _get_asset_content(asset) return HttpResponse(content, content_type=asset.content_type) def read(request, asset_id: int): - asset = _access_asset(request, asset_id) + asset = access.asset_read(request, asset_id) content = _get_asset_content(asset) content = content.decode("utf-8") diff --git a/bookmarks/views/bookmarks.py b/bookmarks/views/bookmarks.py index 9314f69..9ae2989 100644 --- a/bookmarks/views/bookmarks.py +++ b/bookmarks/views/bookmarks.py @@ -5,7 +5,6 @@ from django.contrib.auth.decorators import login_required from django.db.models import QuerySet from django.http import ( HttpResponseRedirect, - Http404, HttpResponseBadRequest, HttpResponseForbidden, ) @@ -15,7 +14,6 @@ from django.urls import reverse from bookmarks import queries, utils from bookmarks.models import ( Bookmark, - BookmarkAsset, BookmarkForm, BookmarkSearch, build_tag_string, @@ -38,7 +36,7 @@ from bookmarks.services.bookmarks import ( ) from bookmarks.type_defs import HttpRequest from bookmarks.utils import get_safe_return_url -from bookmarks.views import contexts, partials, turbo +from bookmarks.views import access, contexts, partials, turbo @login_required @@ -190,10 +188,7 @@ def new(request: HttpRequest): @login_required def edit(request: HttpRequest, bookmark_id: int): - try: - bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) - except Bookmark.DoesNotExist: - raise Http404("Bookmark does not exist") + bookmark = access.bookmark_write(request, bookmark_id) return_url = get_safe_return_url( request.GET.get("return_url"), reverse("linkding:bookmarks.index") ) @@ -216,58 +211,34 @@ def edit(request: HttpRequest, bookmark_id: int): def remove(request: HttpRequest, bookmark_id: int | str): - try: - bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) - except Bookmark.DoesNotExist: - raise Http404("Bookmark does not exist") - + bookmark = access.bookmark_write(request, bookmark_id) bookmark.delete() def archive(request: HttpRequest, bookmark_id: int | str): - try: - bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) - except Bookmark.DoesNotExist: - raise Http404("Bookmark does not exist") - + bookmark = access.bookmark_write(request, bookmark_id) archive_bookmark(bookmark) def unarchive(request: HttpRequest, bookmark_id: int | str): - try: - bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) - except Bookmark.DoesNotExist: - raise Http404("Bookmark does not exist") - + bookmark = access.bookmark_write(request, bookmark_id) unarchive_bookmark(bookmark) def unshare(request: HttpRequest, bookmark_id: int | str): - try: - bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) - except Bookmark.DoesNotExist: - raise Http404("Bookmark does not exist") - + bookmark = access.bookmark_write(request, bookmark_id) bookmark.shared = False bookmark.save() def mark_as_read(request: HttpRequest, bookmark_id: int | str): - try: - bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) - except Bookmark.DoesNotExist: - raise Http404("Bookmark does not exist") - + bookmark = access.bookmark_write(request, bookmark_id) bookmark.unread = False bookmark.save() def create_html_snapshot(request: HttpRequest, bookmark_id: int | str): - try: - bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) - except Bookmark.DoesNotExist: - raise Http404("Bookmark does not exist") - + bookmark = access.bookmark_write(request, bookmark_id) tasks.create_html_snapshot(bookmark) @@ -275,11 +246,7 @@ def upload_asset(request: HttpRequest, bookmark_id: int | str): if settings.LD_DISABLE_ASSET_UPLOAD: return HttpResponseForbidden("Asset upload is disabled") - try: - bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) - except Bookmark.DoesNotExist: - raise Http404("Bookmark does not exist") - + bookmark = access.bookmark_write(request, bookmark_id) file = request.FILES.get("upload_asset_file") if not file: return HttpResponseBadRequest("No file provided") @@ -288,20 +255,12 @@ def upload_asset(request: HttpRequest, bookmark_id: int | str): def remove_asset(request: HttpRequest, asset_id: int | str): - try: - asset = BookmarkAsset.objects.get(pk=asset_id, bookmark__owner=request.user) - except BookmarkAsset.DoesNotExist: - raise Http404("Asset does not exist") - + asset = access.asset_write(request, asset_id) asset.delete() def update_state(request: HttpRequest, bookmark_id: int | str): - try: - bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) - except Bookmark.DoesNotExist: - raise Http404("Bookmark does not exist") - + bookmark = access.bookmark_write(request, bookmark_id) bookmark.is_archived = request.POST.get("is_archived") == "on" bookmark.unread = request.POST.get("unread") == "on" bookmark.shared = request.POST.get("shared") == "on" diff --git a/bookmarks/views/contexts.py b/bookmarks/views/contexts.py index 52ea1e1..d046b81 100644 --- a/bookmarks/views/contexts.py +++ b/bookmarks/views/contexts.py @@ -20,6 +20,7 @@ from bookmarks.models import ( ) from bookmarks.services.wayback import generate_fallback_webarchive_url from bookmarks.type_defs import HttpRequest +from bookmarks.views import access CJK_RE = re.compile(r"[\u4e00-\u9fff]+") @@ -444,22 +445,10 @@ def get_details_context( return None try: - bookmark = Bookmark.objects.get(pk=int(bookmark_id)) - except Bookmark.DoesNotExist: + bookmark = access.bookmark_read(request, bookmark_id) + except Http404: # just ignore, might end up in a situation where the bookmark was deleted # in between navigating back and forth return None - is_owner = bookmark.owner == request.user - is_shared = ( - request.user.is_authenticated - and bookmark.shared - and bookmark.owner.profile.enable_sharing - ) - is_public_shared = bookmark.shared and bookmark.owner.profile.enable_public_sharing - if not is_owner and not is_shared and not is_public_shared: - raise Http404("Bookmark does not exist") - if request.method == "POST" and not is_owner: - raise Http404("Bookmark does not exist") - return context_type(request, bookmark) diff --git a/bookmarks/views/toasts.py b/bookmarks/views/toasts.py index d6d9de3..907fc3b 100644 --- a/bookmarks/views/toasts.py +++ b/bookmarks/views/toasts.py @@ -1,18 +1,14 @@ from django.contrib.auth.decorators import login_required -from django.http import HttpResponseRedirect, Http404 +from django.http import HttpResponseRedirect from django.urls import reverse -from bookmarks.models import Toast from bookmarks.utils import get_safe_return_url +from bookmarks.views import access @login_required def acknowledge(request): - toast_id = request.POST["toast"] - try: - toast = Toast.objects.get(pk=toast_id, owner=request.user) - except Toast.DoesNotExist: - raise Http404("Toast does not exist") + toast = access.toast_write(request, request.POST["toast"]) toast.acknowledged = True toast.save()