Return permission denied instead of redirecting in stats views
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
diff --git a/crashreport_stats/permissions.py b/crashreport_stats/permissions.py
new file mode 100644
index 0000000..c457f42
--- /dev/null
+++ b/crashreport_stats/permissions.py
@@ -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
diff --git a/crashreport_stats/tests/test_views.py b/crashreport_stats/tests/test_views.py
index adcec7d..ab46fce 100644
--- a/crashreport_stats/tests/test_views.py
+++ b/crashreport_stats/tests/test_views.py
@@ -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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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."""
diff --git a/crashreport_stats/views.py b/crashreport_stats/views.py
index 07691df..9e097ec 100644
--- a/crashreport_stats/views.py
+++ b/crashreport_stats/views.py
@@ -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.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 @@
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 @@
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.