Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix bookmark access restrictions
  • Loading branch information
sissbruecker committed Mar 22, 2022
1 parent 66995cf commit 1ffc3e0
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 7 deletions.
11 changes: 11 additions & 0 deletions bookmarks/tests/test_bookmark_archive_view.py
@@ -1,3 +1,4 @@
from django.contrib.auth.models import User
from django.test import TestCase
from django.urls import reverse

Expand Down Expand Up @@ -33,3 +34,13 @@ def test_should_redirect_to_return_url_when_specified(self):
)

self.assertRedirects(response, reverse('bookmarks:close'))

def test_can_only_archive_own_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
bookmark = self.setup_bookmark(user=other_user)

response = self.client.get(reverse('bookmarks:archive', args=[bookmark.id]))
bookmark.refresh_from_db()

self.assertEqual(response.status_code, 404)
self.assertFalse(bookmark.is_archived)
94 changes: 92 additions & 2 deletions bookmarks/tests/test_bookmark_bulk_edit_view.py
@@ -1,3 +1,4 @@
from django.contrib.auth.models import User
from django.forms import model_to_dict
from django.test import TestCase
from django.urls import reverse
Expand Down Expand Up @@ -32,6 +33,21 @@ def test_bulk_archive(self):
self.assertTrue(Bookmark.objects.get(id=bookmark2.id).is_archived)
self.assertTrue(Bookmark.objects.get(id=bookmark3.id).is_archived)

def test_can_only_bulk_archive_own_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
bookmark1 = self.setup_bookmark(user=other_user)
bookmark2 = self.setup_bookmark(user=other_user)
bookmark3 = self.setup_bookmark(user=other_user)

self.client.post(reverse('bookmarks:bulk_edit'), {
'bulk_archive': [''],
'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)],
})

self.assertFalse(Bookmark.objects.get(id=bookmark1.id).is_archived)
self.assertFalse(Bookmark.objects.get(id=bookmark2.id).is_archived)
self.assertFalse(Bookmark.objects.get(id=bookmark3.id).is_archived)

def test_bulk_unarchive(self):
bookmark1 = self.setup_bookmark(is_archived=True)
bookmark2 = self.setup_bookmark(is_archived=True)
Expand All @@ -46,6 +62,21 @@ def test_bulk_unarchive(self):
self.assertFalse(Bookmark.objects.get(id=bookmark2.id).is_archived)
self.assertFalse(Bookmark.objects.get(id=bookmark3.id).is_archived)

def test_can_only_bulk_unarchive_own_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
bookmark1 = self.setup_bookmark(is_archived=True, user=other_user)
bookmark2 = self.setup_bookmark(is_archived=True, user=other_user)
bookmark3 = self.setup_bookmark(is_archived=True, user=other_user)

self.client.post(reverse('bookmarks:bulk_edit'), {
'bulk_unarchive': [''],
'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)],
})

self.assertTrue(Bookmark.objects.get(id=bookmark1.id).is_archived)
self.assertTrue(Bookmark.objects.get(id=bookmark2.id).is_archived)
self.assertTrue(Bookmark.objects.get(id=bookmark3.id).is_archived)

def test_bulk_delete(self):
bookmark1 = self.setup_bookmark()
bookmark2 = self.setup_bookmark()
Expand All @@ -57,8 +88,23 @@ def test_bulk_delete(self):
})

self.assertIsNone(Bookmark.objects.filter(id=bookmark1.id).first())
self.assertFalse(Bookmark.objects.filter(id=bookmark2.id).first())
self.assertFalse(Bookmark.objects.filter(id=bookmark3.id).first())
self.assertIsNone(Bookmark.objects.filter(id=bookmark2.id).first())
self.assertIsNone(Bookmark.objects.filter(id=bookmark3.id).first())

def test_can_only_bulk_delete_own_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
bookmark1 = self.setup_bookmark(user=other_user)
bookmark2 = self.setup_bookmark(user=other_user)
bookmark3 = self.setup_bookmark(user=other_user)

self.client.post(reverse('bookmarks:bulk_edit'), {
'bulk_delete': [''],
'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)],
})

self.assertIsNotNone(Bookmark.objects.filter(id=bookmark1.id).first())
self.assertIsNotNone(Bookmark.objects.filter(id=bookmark2.id).first())
self.assertIsNotNone(Bookmark.objects.filter(id=bookmark3.id).first())

def test_bulk_tag(self):
bookmark1 = self.setup_bookmark()
Expand All @@ -81,6 +127,28 @@ def test_bulk_tag(self):
self.assertCountEqual(bookmark2.tags.all(), [tag1, tag2])
self.assertCountEqual(bookmark3.tags.all(), [tag1, tag2])

def test_can_only_bulk_tag_own_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
bookmark1 = self.setup_bookmark(user=other_user)
bookmark2 = self.setup_bookmark(user=other_user)
bookmark3 = self.setup_bookmark(user=other_user)
tag1 = self.setup_tag()
tag2 = self.setup_tag()

self.client.post(reverse('bookmarks:bulk_edit'), {
'bulk_tag': [''],
'bulk_tag_string': [f'{tag1.name} {tag2.name}'],
'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)],
})

bookmark1.refresh_from_db()
bookmark2.refresh_from_db()
bookmark3.refresh_from_db()

self.assertCountEqual(bookmark1.tags.all(), [])
self.assertCountEqual(bookmark2.tags.all(), [])
self.assertCountEqual(bookmark3.tags.all(), [])

def test_bulk_untag(self):
tag1 = self.setup_tag()
tag2 = self.setup_tag()
Expand All @@ -102,6 +170,28 @@ def test_bulk_untag(self):
self.assertCountEqual(bookmark2.tags.all(), [])
self.assertCountEqual(bookmark3.tags.all(), [])

def test_can_only_bulk_untag_own_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
tag1 = self.setup_tag()
tag2 = self.setup_tag()
bookmark1 = self.setup_bookmark(tags=[tag1, tag2], user=other_user)
bookmark2 = self.setup_bookmark(tags=[tag1, tag2], user=other_user)
bookmark3 = self.setup_bookmark(tags=[tag1, tag2], user=other_user)

self.client.post(reverse('bookmarks:bulk_edit'), {
'bulk_untag': [''],
'bulk_tag_string': [f'{tag1.name} {tag2.name}'],
'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)],
})

bookmark1.refresh_from_db()
bookmark2.refresh_from_db()
bookmark3.refresh_from_db()

self.assertCountEqual(bookmark1.tags.all(), [tag1, tag2])
self.assertCountEqual(bookmark2.tags.all(), [tag1, tag2])
self.assertCountEqual(bookmark3.tags.all(), [tag1, tag2])

def test_bulk_edit_handles_empty_bookmark_id(self):
bookmark1 = self.setup_bookmark()
bookmark2 = self.setup_bookmark()
Expand Down
12 changes: 12 additions & 0 deletions bookmarks/tests/test_bookmark_edit_view.py
@@ -1,3 +1,4 @@
from django.contrib.auth.models import User
from django.test import TestCase
from django.urls import reverse

Expand Down Expand Up @@ -95,3 +96,14 @@ def test_should_redirect_to_return_url(self):
response = self.client.post(reverse('bookmarks:edit', args=[bookmark.id]), form_data)

self.assertRedirects(response, form_data['return_url'])

def test_can_only_edit_own_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
bookmark = self.setup_bookmark(user=other_user)
form_data = self.create_form_data({'id': bookmark.id})

response = self.client.post(reverse('bookmarks:edit', args=[bookmark.id]), form_data)
bookmark.refresh_from_db()
self.assertNotEqual(bookmark.url, form_data['url'])
self.assertEqual(response.status_code, 404)

10 changes: 10 additions & 0 deletions bookmarks/tests/test_bookmark_remove_view.py
@@ -1,3 +1,4 @@
from django.contrib.auth.models import User
from django.test import TestCase
from django.urls import reverse

Expand Down Expand Up @@ -33,3 +34,12 @@ def test_should_redirect_to_return_url_when_specified(self):
)

self.assertRedirects(response, reverse('bookmarks:close'))

def test_can_only_edit_own_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
bookmark = self.setup_bookmark(user=other_user)

response = self.client.get(reverse('bookmarks:remove', args=[bookmark.id]))

self.assertEqual(response.status_code, 404)
self.assertTrue(Bookmark.objects.filter(id=bookmark.id).exists())
11 changes: 11 additions & 0 deletions bookmarks/tests/test_bookmark_unarchive_view.py
@@ -1,3 +1,4 @@
from django.contrib.auth.models import User
from django.test import TestCase
from django.urls import reverse

Expand Down Expand Up @@ -33,3 +34,13 @@ def test_should_redirect_to_return_url_when_specified(self):
)

self.assertRedirects(response, reverse('bookmarks:close'))

def test_can_only_archive_own_bookmarks(self):
other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123')
bookmark = self.setup_bookmark(is_archived=True, user=other_user)

response = self.client.get(reverse('bookmarks:unarchive', args=[bookmark.id]))
bookmark.refresh_from_db()

self.assertEqual(response.status_code, 404)
self.assertTrue(bookmark.is_archived)
25 changes: 20 additions & 5 deletions bookmarks/views/bookmarks.py
Expand Up @@ -2,7 +2,7 @@

from django.contrib.auth.decorators import login_required
from django.core.paginator import Paginator
from django.http import HttpResponseRedirect
from django.http import HttpResponseRedirect, Http404
from django.shortcuts import render
from django.urls import reverse

Expand Down Expand Up @@ -108,7 +108,10 @@ def new(request):

@login_required
def edit(request, bookmark_id: int):
bookmark = Bookmark.objects.get(pk=bookmark_id)
try:
bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user)
except Bookmark.DoesNotExist:
raise Http404('Bookmark does not exist')

if request.method == 'POST':
form = BookmarkForm(request.POST, instance=bookmark)
Expand Down Expand Up @@ -137,7 +140,11 @@ def edit(request, bookmark_id: int):

@login_required
def remove(request, bookmark_id: int):
bookmark = Bookmark.objects.get(pk=bookmark_id)
try:
bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user)
except Bookmark.DoesNotExist:
raise Http404('Bookmark does not exist')

bookmark.delete()
return_url = request.GET.get('return_url')
return_url = return_url if return_url else reverse('bookmarks:index')
Expand All @@ -146,7 +153,11 @@ def remove(request, bookmark_id: int):

@login_required
def archive(request, bookmark_id: int):
bookmark = Bookmark.objects.get(pk=bookmark_id)
try:
bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user)
except Bookmark.DoesNotExist:
raise Http404('Bookmark does not exist')

archive_bookmark(bookmark)
return_url = request.GET.get('return_url')
return_url = return_url if return_url else reverse('bookmarks:index')
Expand All @@ -155,7 +166,11 @@ def archive(request, bookmark_id: int):

@login_required
def unarchive(request, bookmark_id: int):
bookmark = Bookmark.objects.get(pk=bookmark_id)
try:
bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user)
except Bookmark.DoesNotExist:
raise Http404('Bookmark does not exist')

unarchive_bookmark(bookmark)
return_url = request.GET.get('return_url')
return_url = return_url if return_url else reverse('bookmarks:archived')
Expand Down

0 comments on commit 1ffc3e0

Please sign in to comment.