Return permission denied instead of redirecting in stats views
authorMitja Nikolaus <mitja@fairphone.com>
Wed, 21 Nov 2018 11:14:45 +0000 (12:14 +0100)
committerMitja Nikolaus <mitja@fairphone.com>
Wed, 5 Dec 2018 12:34:49 +0000 (13:34 +0100)
Return a 403 permission denied error if a user is not part of the
Fairphone staff group. This breaks the infinite redirect loop.

Issue: HIC-260
Change-Id: I682127c2165f826257a84b0bf246fc9fb86813ea

crashreport_stats/permissions.py [new file with mode: 0644]
crashreport_stats/tests/test_views.py
crashreport_stats/views.py

diff --git a/crashreport_stats/permissions.py b/crashreport_stats/permissions.py
new file mode 100644 (file)
index 0000000..c457f42
--- /dev/null
@@ -0,0 +1,21 @@
+"""Permissions for accessing the stats API."""
+from django.core.exceptions import PermissionDenied
+
+from crashreports.permissions import user_is_hiccup_staff
+from hiccup.allauth_adapters import FP_STAFF_GROUP_NAME
+
+
+def check_user_is_hiccup_staff(user):
+    """Check if the user is part of the Hiccup staff.
+
+    Returns: True if the user is part of the Hiccup staff group.
+
+    Raises:
+        PermissionDenied: If the user is not part of the Hiccup staff group.
+
+    """
+    if not user_is_hiccup_staff(user):
+        raise PermissionDenied(
+            "User %s not part of the %s group" % (user, FP_STAFF_GROUP_NAME)
+        )
+    return True
index adcec7d..ab46fce 100644 (file)
@@ -3,7 +3,6 @@
 import unittest
 from urllib.parse import urlencode
 
-from django.conf import settings
 from django.urls import reverse
 
 from rest_framework import status
@@ -37,16 +36,16 @@ class ViewsTestCase(HiccupStatsAPITestCase):
 
     def test_home_view_as_device_owner(self):
         """Test that device owner users can not access the home view."""
-        # Assert that the response is a redirect (to the login page)
+        # Assert that the permission is denied.
         self._assert_get_as_device_owner_fails(
-            self.home_url, expected_status=status.HTTP_302_FOUND
+            self.home_url, expected_status=status.HTTP_403_FORBIDDEN
         )
 
     def test_home_view_no_auth(self):
         """Test that one can not access the home view without auth."""
-        # Assert that the response is a redirect (to the login page)
+        # Assert that the permission is denied
         self._assert_get_without_authentication_fails(
-            self.home_url, expected_status=status.HTTP_302_FOUND
+            self.home_url, expected_status=status.HTTP_403_FORBIDDEN
         )
 
     def test_device_view_as_fp_staff(self):
@@ -59,22 +58,21 @@ class ViewsTestCase(HiccupStatsAPITestCase):
 
     def test_device_view_as_device_owner(self):
         """Test that device owner users can not access the device view."""
-        # Assert that the response is a redirect (to the login page)
+        # Assert that the the permission is denied.
         self._assert_get_as_device_owner_fails(
             self._url_with_params(
                 self.device_url, {"uuid": self.device_owner_device.uuid}
-            ),
-            expected_status=status.HTTP_302_FOUND,
+            )
         )
 
     def test_device_view_no_auth(self):
         """Test that non-authenticated users can not access the device view."""
-        # Assert that the response is a redirect (to the login page)
+        # Assert that the permission is denied.
         self._assert_get_without_authentication_fails(
             self._url_with_params(
                 self.device_url, {"uuid": self.device_owner_device.uuid}
             ),
-            expected_status=status.HTTP_302_FOUND,
+            expected_status=status.HTTP_403_FORBIDDEN,
         )
 
     def test_versions_view_as_fp_staff(self):
@@ -83,16 +81,16 @@ class ViewsTestCase(HiccupStatsAPITestCase):
 
     def test_versions_view_as_device_owner(self):
         """Test that device owner users can not access the versions view."""
-        # Assert that the response is a redirect (to the login page)
+        # Assert that the permission is denied
         self._assert_get_as_device_owner_fails(
-            self.versions_url, expected_status=status.HTTP_302_FOUND
+            self.versions_url, expected_status=status.HTTP_403_FORBIDDEN
         )
 
     def test_versions_view_no_auth(self):
         """Test one can not access the versions view without auth."""
-        # Assert that the response is a redirect (to the login page)
+        # Assert that the permission is denied
         self._assert_get_without_authentication_fails(
-            self.versions_url, expected_status=status.HTTP_302_FOUND
+            self.versions_url, expected_status=status.HTTP_403_FORBIDDEN
         )
 
     def test_versions_all_view_as_fp_staff(self):
@@ -101,16 +99,16 @@ class ViewsTestCase(HiccupStatsAPITestCase):
 
     def test_versions_all_view_as_device_owner(self):
         """Test that device owner users can not access the versions all view."""
-        # Assert that the response is a redirect (to the login page)
+        # Assert that the permission is denied
         self._assert_get_as_device_owner_fails(
-            self.versions_all_url, expected_status=status.HTTP_302_FOUND
+            self.versions_all_url, expected_status=status.HTTP_403_FORBIDDEN
         )
 
     def test_versions_all_view_no_auth(self):
         """Test that one can not access the versions all view without auth."""
-        # Assert that the response is a redirect (to the login page)
+        # Assert that the permission is denied
         self._assert_get_without_authentication_fails(
-            self.versions_all_url, expected_status=status.HTTP_302_FOUND
+            self.versions_all_url, expected_status=status.HTTP_403_FORBIDDEN
         )
 
     def test_home_view_post_as_fp_staff(self):
@@ -133,14 +131,8 @@ class ViewsTestCase(HiccupStatsAPITestCase):
             self.home_url, data={"uuid": str(self.device_owner_device.uuid)}
         )
 
-        # Assert that the response is a redirect to the login page
-        self.assertRedirects(
-            response,
-            self._url_with_params(
-                settings.ACCOUNT_LOGOUT_REDIRECT_URL,
-                {"next": settings.LOGIN_REDIRECT_URL},
-            ),
-        )
+        # Assert that the permission is denied
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
 
     def test_home_view_post_as_device_owner(self):
         """Test HTTP POST method to home view as device owner."""
@@ -148,15 +140,8 @@ class ViewsTestCase(HiccupStatsAPITestCase):
             self.home_url, data={"uuid": str(self.device_owner_device.uuid)}
         )
 
-        # Assert that the response is a redirect to the login page
-
-        self.assertRedirects(
-            response,
-            self._url_with_params(
-                settings.ACCOUNT_LOGOUT_REDIRECT_URL,
-                {"next": settings.LOGIN_REDIRECT_URL},
-            ),
-        )
+        # Assert that the permission is denied
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
 
     def test_get_home_view(self):
         """Test getting the home view with device search form."""
index 07691df..9e097ec 100644 (file)
@@ -1,5 +1,4 @@
 """Views for the Hiccup statistics."""
-
 from django.http import HttpResponse, Http404, HttpResponseRedirect
 from django.template import loader
 from django.contrib.auth.decorators import user_passes_test
@@ -7,8 +6,8 @@ from django import forms
 from django.contrib import messages
 from django.urls import reverse
 
+from crashreport_stats.permissions import check_user_is_hiccup_staff
 from crashreports.models import Device
-from crashreports.permissions import user_is_hiccup_staff
 
 
 class DeviceUUIDForm(forms.Form):
@@ -17,7 +16,7 @@ class DeviceUUIDForm(forms.Form):
     uuid = forms.CharField(label="Device UUID:", max_length=100)
 
 
-@user_passes_test(user_is_hiccup_staff)
+@user_passes_test(check_user_is_hiccup_staff)
 def device_stats(request):
     """Respond with statistics for a specific device."""
     template = loader.get_template("crashreport_stats/device.html")
@@ -27,21 +26,21 @@ def device_stats(request):
     return HttpResponse(template.render({"uuid": uuid}, request))
 
 
-@user_passes_test(user_is_hiccup_staff)
+@user_passes_test(check_user_is_hiccup_staff)
 def versions_all_overview(request):
     """Respond with the distribution of official release versions."""
     template = loader.get_template("crashreport_stats/versions.html")
     return HttpResponse(template.render({"is_official_release": "1"}, request))
 
 
-@user_passes_test(user_is_hiccup_staff)
+@user_passes_test(check_user_is_hiccup_staff)
 def versions_overview(request):
     """Respond with the distribution of non-official release versions."""
     template = loader.get_template("crashreport_stats/versions.html")
     return HttpResponse(template.render({"is_official_release": "2"}, request))
 
 
-@user_passes_test(user_is_hiccup_staff)
+@user_passes_test(check_user_is_hiccup_staff)
 def home(request):
     """Respond with a form for searching devices by UUID.