From f4d1da0ab72c16afb931868a57bffab7eecefddc Mon Sep 17 00:00:00 2001 From: Scot Hacker Date: Mon, 26 Mar 2018 00:37:29 -0700 Subject: [PATCH] Smoke and permissions tests And various bug / permissions config tweaks to accompany --- todo/models.py | 2 +- todo/settings.py | 7 --- todo/tests/__init__.py | 0 todo/tests/conftest.py | 17 ++++++ todo/tests/test_views.py | 112 +++++++++++++++++++++++++++++++++++++++ todo/utils.py | 15 ------ todo/views.py | 57 +++++++++++--------- 7 files changed, 161 insertions(+), 49 deletions(-) delete mode 100644 todo/settings.py create mode 100644 todo/tests/__init__.py create mode 100644 todo/tests/conftest.py create mode 100644 todo/tests/test_views.py diff --git a/todo/models.py b/todo/models.py index 0c81e41..01e4163 100644 --- a/todo/models.py +++ b/todo/models.py @@ -56,7 +56,7 @@ class Item(models.Model): return reverse('todo:task_detail', kwargs={'task_id': self.id, }) # Auto-set the item creation / completed date - def save(self): + def save(self, **kwargs): # If Item is being marked complete, set the completed_date if self.completed: self.completed_date = datetime.datetime.now() diff --git a/todo/settings.py b/todo/settings.py deleted file mode 100644 index e9231b5..0000000 --- a/todo/settings.py +++ /dev/null @@ -1,7 +0,0 @@ -from django.conf import settings - - -STAFF_ONLY = getattr(settings, 'TODO_STAFF_ONLY', False) -DEFAULT_LIST_ID = getattr(settings, 'TODO_DEFAULT_LIST_ID', 1) -DEFAULT_ASSIGNEE = getattr(settings, 'TODO_DEFAULT_ASSIGNEE', None) -PUBLIC_SUBMIT_REDIRECT = getattr(settings, 'TODO_PUBLIC_SUBMIT_REDIRECT', '/') diff --git a/todo/tests/__init__.py b/todo/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/todo/tests/conftest.py b/todo/tests/conftest.py new file mode 100644 index 0000000..747b5db --- /dev/null +++ b/todo/tests/conftest.py @@ -0,0 +1,17 @@ +import pytest + +from django.contrib.auth.models import Group + +from todo.models import Item, TaskList + + +@pytest.fixture +def todo_setup(django_user_model): + g1 = Group.objects.create(name="Weavers") + u1 = django_user_model.objects.create(username="you", password="password") + u1.groups.add(g1) + tlist = TaskList.objects.create(group=g1, name="Zip", slug="zip") + + Item.objects.create(created_by=u1, title="Task 1", task_list=tlist, priority=1) + Item.objects.create(created_by=u1, title="Task 2", task_list=tlist, priority=2) + Item.objects.create(created_by=u1, title="Task 3", task_list=tlist, priority=3) diff --git a/todo/tests/test_views.py b/todo/tests/test_views.py new file mode 100644 index 0000000..129f656 --- /dev/null +++ b/todo/tests/test_views.py @@ -0,0 +1,112 @@ +import pytest + +from django.urls import reverse + +from todo.models import Item, TaskList + +""" +First the "smoketests" - do they respond at all for a logged in admin user? +Next permissions tests - some views should respond for staffers only. +After that, view contents and behaviors. +""" + +# ### SMOKETESTS ### + +@pytest.mark.django_db +def test_todo_setup(todo_setup): + assert Item.objects.all().count() == 3 + + +def test_view_list_lists(todo_setup, admin_client): + url = reverse('todo:lists') + response = admin_client.get(url) + assert response.status_code == 200 + + +def test_view_reorder(todo_setup, admin_client): + url = reverse('todo:reorder_tasks') + response = admin_client.get(url) + assert response.status_code == 201 # Special case return value expected + + +def test_view_external_add(todo_setup, admin_client, settings): + default_list = TaskList.objects.first() + settings.TODO_DEFAULT_LIST_ID = default_list.id + assert settings.TODO_DEFAULT_LIST_ID == default_list.id + url = reverse('todo:external_add') + response = admin_client.get(url) + assert response.status_code == 200 + + +def test_view_mine(todo_setup, admin_client): + url = reverse('todo:mine') + response = admin_client.get(url) + assert response.status_code == 200 + + +def test_view_list_completed(todo_setup, admin_client): + tlist = TaskList.objects.get(slug="zip") + url = reverse('todo:list_detail_completed', kwargs={'list_id': tlist.id, 'list_slug': tlist.slug}) + response = admin_client.get(url) + assert response.status_code == 200 + + +def test_view_list(todo_setup, admin_client): + tlist = TaskList.objects.get(slug="zip") + url = reverse('todo:list_detail', kwargs={'list_id': tlist.id, 'list_slug': tlist.slug}) + response = admin_client.get(url) + assert response.status_code == 200 + + +def test_del_list(todo_setup, admin_client): + tlist = TaskList.objects.get(slug="zip") + url = reverse('todo:del_list', kwargs={'list_id': tlist.id, 'list_slug': tlist.slug}) + response = admin_client.get(url) + assert response.status_code == 200 + + +def test_view_add_list(todo_setup, admin_client): + url = reverse('todo:add_list') + response = admin_client.get(url) + assert response.status_code == 200 + + +def test_view_task_detail(todo_setup, admin_client): + task = Item.objects.first() + url = reverse('todo:task_detail', kwargs={'task_id': task.id}) + response = admin_client.get(url) + assert response.status_code == 200 + + +def test_view_search(todo_setup, admin_client): + url = reverse('todo:search') + response = admin_client.get(url) + assert response.status_code == 200 + + +# ### PERMISSIONS ### + +""" +Some views are for staff users only. We've already smoke-tested with Admin user - +try these with normal user. +""" + + +def test_view_add_list_nonadmin(todo_setup, client): + url = reverse('todo:add_list') + client.login(username="you", password="password") + response = client.get(url) + assert response.status_code == 302 # Redirects to login. Would prefer 403... + + +def test_view_del_list_nonadmin(todo_setup, client): + tlist = TaskList.objects.get(slug="zip") + url = reverse('todo:del_list', kwargs={'list_id': tlist.id, 'list_slug': tlist.slug}) + client.login(username="you", password="password") + response = client.get(url) + assert response.status_code == 302 # Redirects to login. Would prefer 403... + + +# TODO +# View a task that's not in one of my groups? +# Mark complete \ No newline at end of file diff --git a/todo/utils.py b/todo/utils.py index c46dc01..8b291ad 100644 --- a/todo/utils.py +++ b/todo/utils.py @@ -1,28 +1,13 @@ import datetime -from django.conf import settings from django.contrib import messages -from django.contrib.auth.models import User from django.contrib.sites.models import Site from django.core.mail import send_mail -from django.http import HttpResponse from django.template.loader import render_to_string from todo.models import Item, Comment -def check_user_allowed(user: User) -> HttpResponse: - """ - Verifies user is logged in, and in staff if that setting is enabled. - Per-object permission checks (e.g. to view a particular list) are in the views that handle those objects. - """ - - if hasattr(settings, "STAFF_ONLY") and getattr(settings, "STAFF_ONLY"): - return user.is_authenticated and user.is_staff - else: - return user.is_authenticated - - def toggle_done(request, items): """Check for items in the mark_done POST array. If present, change status to complete. Takes a list of task IDs. No return value. diff --git a/todo/views.py b/todo/views.py index 0a50e00..7f34f50 100644 --- a/todo/views.py +++ b/todo/views.py @@ -1,7 +1,9 @@ import datetime +from django.conf import settings from django.contrib import messages -from django.contrib.auth.decorators import user_passes_test, login_required +from django.contrib.auth.decorators import login_required +from django.contrib.admin.views.decorators import staff_member_required from django.contrib.auth.models import User from django.contrib.sites.models import Site from django.core.exceptions import PermissionDenied @@ -13,7 +15,6 @@ from django.shortcuts import get_object_or_404, render, redirect from django.template.loader import render_to_string from django.views.decorators.csrf import csrf_exempt -from todo import settings from todo.forms import AddTaskListForm, AddEditItemForm, AddExternalItemForm, SearchForm from todo.models import Item, TaskList, Comment from todo.utils import ( @@ -21,10 +22,10 @@ from todo.utils import ( toggle_deleted, send_notify_mail, send_email_to_thread_participants, - check_user_allowed) + ) -@user_passes_test(check_user_allowed) +@login_required def list_lists(request) -> HttpResponse: """Homepage view - list of lists a user can view, and ability to add a list. """ @@ -61,7 +62,8 @@ def list_lists(request) -> HttpResponse: return render(request, 'todo/list_lists.html', context) -@user_passes_test(check_user_allowed) +@staff_member_required +@login_required def del_list(request, list_id: int, list_slug: str) -> HttpResponse: """Delete an entire list. Danger Will Robinson! Only staff members should be allowed to access this view. """ @@ -70,7 +72,7 @@ def del_list(request, list_id: int, list_slug: str) -> HttpResponse: # Ensure user has permission to delete list. Admins can delete all lists. # Get the group this list belongs to, and check whether current user is a member of that group. # FIXME: This means any group member can delete lists, which is probably too permissive. - if task_list.group not in request.user.groups.all() or not request.user.is_staff: + if task_list.group not in request.user.groups.all() and not request.user.is_staff: raise PermissionDenied if request.method == 'POST': @@ -92,6 +94,7 @@ def del_list(request, list_id: int, list_slug: str) -> HttpResponse: return render(request, 'todo/del_list.html', context) +@login_required def list_detail(request, list_id=None, list_slug=None, view_completed=False): """Display and manage items in a todo list. """ @@ -163,7 +166,7 @@ def list_detail(request, list_id=None, list_slug=None, view_completed=False): return render(request, 'todo/list_detail.html', context) -@user_passes_test(check_user_allowed) +@login_required def task_detail(request, task_id: int) -> HttpResponse: """View task details. Allow task details to be edited. Process new comments on task. """ @@ -219,28 +222,30 @@ def task_detail(request, task_id: int) -> HttpResponse: @csrf_exempt -@user_passes_test(check_user_allowed) +@login_required def reorder_tasks(request) -> HttpResponse: """Handle task re-ordering (priorities) from JQuery drag/drop in list_detail.html """ newtasklist = request.POST.getlist('tasktable[]') - # First item in received list is always empty - remove it - del newtasklist[0] + if newtasklist: + # First item in received list is always empty - remove it + del newtasklist[0] - # Re-prioritize each item in list - i = 1 - for t in newtasklist: - newitem = Item.objects.get(pk=t) - newitem.priority = i - newitem.save() - i += 1 + # Re-prioritize each item in list + i = 1 + for t in newtasklist: + newitem = Item.objects.get(pk=t) + newitem.priority = i + newitem.save() + i += 1 # All views must return an httpresponse of some kind ... without this we get # error 500s in the log even though things look peachy in the browser. return HttpResponse(status=201) -@user_passes_test(check_user_allowed) +@staff_member_required +@login_required def add_list(request) -> HttpResponse: """Allow users to add a new todo list to the group they're in. """ @@ -271,7 +276,7 @@ def add_list(request) -> HttpResponse: return render(request, 'todo/add_list.html', context) -@user_passes_test(check_user_allowed) +@login_required def search(request) -> HttpResponse: """Search for tasks user has permission to see. """ @@ -318,10 +323,10 @@ def external_add(request) -> HttpResponse: Publicly filed tickets are unassigned unless settings.DEFAULT_ASSIGNEE exists. """ - if not settings.DEFAULT_LIST_ID: - raise RuntimeError("This feature requires DEFAULT_LIST_ID in settings. See documentation.") + if not settings.TODO_DEFAULT_LIST_ID: + raise RuntimeError("This feature requires TODO_DEFAULT_LIST_ID: in settings. See documentation.") - if not TaskList.objects.filter(id=settings.DEFAULT_LIST_ID).exists(): + if not TaskList.objects.filter(id=settings.TODO_DEFAULT_LIST_ID).exists(): raise RuntimeError("There is no TaskList with ID specified for DEFAULT_LIST_ID in settings.") if request.POST: @@ -330,10 +335,10 @@ def external_add(request) -> HttpResponse: if form.is_valid(): current_site = Site.objects.get_current() item = form.save(commit=False) - item.task_list = TaskList.objects.get(id=settings.DEFAULT_LIST_ID) + item.task_list = TaskList.objects.get(id=settings.TODO_DEFAULT_LIST_ID) item.created_by = request.user - if settings.DEFAULT_ASSIGNEE: - item.assigned_to = User.objects.get(username=settings.DEFAULT_ASSIGNEE) + if settings.TODO_DEFAULT_ASSIGNEE: + item.assigned_to = User.objects.get(username=settings.TODO_DEFAULT_ASSIGNEE) item.save() # Send email to assignee if we have one @@ -349,7 +354,7 @@ def external_add(request) -> HttpResponse: messages.success(request, "Your trouble ticket has been submitted. We'll get back to you soon.") - return redirect(settings.PUBLIC_SUBMIT_REDIRECT) + return redirect(settings.TODO_PUBLIC_SUBMIT_REDIRECT) else: form = AddExternalItemForm(initial={'priority': 999})