From 9f45b245bfc1dd7c693dddd9bd7d97e819d84bd5 Mon Sep 17 00:00:00 2001
From: Tanner Prestegard <tanner.prestegard@ligo.org>
Date: Tue, 18 Dec 2018 09:55:50 -0600
Subject: [PATCH] Update to event view 403s

Make the event view 403 pages more specific (for event views only),
but more generalized (standard message in the template, not taken
from the context).  Also fix up some unit tests which expected the
old behavior.
---
 gracedb/events/tests/test_access.py | 30 ++++++++++++++---------------
 gracedb/events/views.py             |  7 ++-----
 gracedb/templates/403.html          |  2 +-
 gracedb/templates/gracedb/403.html  | 11 +++++++++++
 4 files changed, 29 insertions(+), 21 deletions(-)
 create mode 100644 gracedb/templates/gracedb/403.html

diff --git a/gracedb/events/tests/test_access.py b/gracedb/events/tests/test_access.py
index 2b6c7cf73..9f8d32271 100644
--- a/gracedb/events/tests/test_access.py
+++ b/gracedb/events/tests/test_access.py
@@ -533,14 +533,14 @@ class TestEventModifyT90(EventSetup, GraceDbTestBase):
         response = self.request_as_user(url, "POST", self.lvem_user,
                 data=self.t90_data)
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, "Forbidden")
+        self.assertEqual(response.templates[0].name, '403.html')
 
     def test_public_user_t90(self):
         """Public user can't t90 GRB events"""
         url = reverse('modify_t90', args=[self.grb_event.graceid])
         response = self.request_as_user(url, "POST", data=self.t90_data)
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, "Forbidden")
+        self.assertEqual(response.templates[0].name, '403.html')
 
 
 class TestEventModifyPermissions(EventSetup, GraceDbTestBase):
@@ -611,7 +611,7 @@ class TestEventModifyPermissions(EventSetup, GraceDbTestBase):
         response = self.request_as_user(url, "POST", self.lvem_user,
             data=self.perm_data)
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, "Forbidden")
+        self.assertEqual(response.templates[0].name, '403.html')
 
     def test_lvem_user_modify_permissions_for_exposed_event(self):
         """LV-EM user can't modify exposed event permissions"""
@@ -627,7 +627,7 @@ class TestEventModifyPermissions(EventSetup, GraceDbTestBase):
         url = reverse('modify_permissions', args=[self.internal_event.graceid])
         response = self.request_as_user(url, "POST", data=self.perm_data)
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, "Forbidden")
+        self.assertEqual(response.templates[0].name, '403.html')
 
 
 class TestEventModifySignoff(SignoffGroupsAndUsersSetup, EventSetup,
@@ -688,7 +688,7 @@ class TestEventModifySignoff(SignoffGroupsAndUsersSetup, EventSetup,
         response = self.request_as_user(url, "POST", self.lvem_user,
             self.op_signoff_data)
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, "Forbidden")
+        self.assertEqual(response.templates[0].name, '403.html')
 
         # Exposed event
         url = reverse('modify_signoff', args=[self.lvem_event.graceid])
@@ -703,7 +703,7 @@ class TestEventModifySignoff(SignoffGroupsAndUsersSetup, EventSetup,
         url = reverse('modify_signoff', args=[self.internal_event.graceid])
         response = self.request_as_user(url, "POST", data=self.op_signoff_data)
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, "Forbidden")
+        self.assertEqual(response.templates[0].name, '403.html')
 
 
 class TestEventCreateLog(EventSetup, GraceDbTestBase):
@@ -741,7 +741,7 @@ class TestEventCreateLog(EventSetup, GraceDbTestBase):
 
         # Check response and content
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, 'Forbidden')
+        self.assertEqual(response.templates[0].name, '403.html')
 
     def test_lvem_user_create_log_exposed_event(self):
         """LV-EM user can create log for exposed event"""
@@ -768,7 +768,7 @@ class TestEventCreateLog(EventSetup, GraceDbTestBase):
 
         # Check response and content
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, 'Forbidden')
+        self.assertEqual(response.templates[0].name, '403.html')
 
 
 class TestEventLogTag(EventSetup, GraceDbTestBase):
@@ -824,7 +824,7 @@ class TestEventLogTag(EventSetup, GraceDbTestBase):
             log.N, self.tag_name])
         response = self.request_as_user(url, "POST", self.lvem_user)
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, 'Forbidden')
+        self.assertEqual(response.templates[0].name, '403.html')
 
         # Exposed log on hidden event
         log = self.internal_event.eventlog_set.get(
@@ -833,7 +833,7 @@ class TestEventLogTag(EventSetup, GraceDbTestBase):
             log.N, self.tag_name])
         response = self.request_as_user(url, "POST", self.lvem_user)
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, 'Forbidden')
+        self.assertEqual(response.templates[0].name, '403.html')
 
     def test_lvem_user_tag_log_exposed_event(self):
         """LV-EM user can only tag exposed logs for exposed events"""
@@ -870,7 +870,7 @@ class TestEventLogTag(EventSetup, GraceDbTestBase):
                     self.tag_name])
                 response = self.request_as_user(url, "POST")
                 self.assertEqual(response.status_code, 403)
-                self.assertEqual(response.content, 'Forbidden')
+                self.assertEqual(response.templates[0].name, '403.html')
 
 
 class TestEventLogUntag(EventSetup, GraceDbTestBase):
@@ -927,7 +927,7 @@ class TestEventLogUntag(EventSetup, GraceDbTestBase):
             log.N, self.test_tag.name])
         response = self.request_as_user(url, "DELETE", self.lvem_user)
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, 'Forbidden')
+        self.assertEqual(response.templates[0].name, '403.html')
 
         # Exposed log on hidden event
         log = self.internal_event.eventlog_set.get(
@@ -936,7 +936,7 @@ class TestEventLogUntag(EventSetup, GraceDbTestBase):
             log.N, self.test_tag.name])
         response = self.request_as_user(url, "DELETE", self.lvem_user)
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, 'Forbidden')
+        self.assertEqual(response.templates[0].name, '403.html')
 
     def test_lvem_user_untag_log_exposed_event(self):
         """
@@ -974,7 +974,7 @@ class TestEventLogUntag(EventSetup, GraceDbTestBase):
                     self.test_tag.name])
                 response = self.request_as_user(url, "DELETE")
                 self.assertEqual(response.status_code, 403)
-                self.assertEqual(response.content, 'Forbidden')
+                self.assertEqual(response.templates[0].name, '403.html')
 
 
 class TestEventCreateEMObservation(EventSetup, GraceDbTestBase):
@@ -1034,7 +1034,7 @@ class TestEventCreateEMObservation(EventSetup, GraceDbTestBase):
 
         # Check response and content
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(response.content, 'Forbidden')
+        self.assertEqual(response.templates[0].name, '403.html')
 
     def test_lvem_user_create_emobservation_exposed_event(self):
         """LV-EM user can create emobservation for exposed event"""
diff --git a/gracedb/events/views.py b/gracedb/events/views.py
index db8d52468..691890d50 100644
--- a/gracedb/events/views.py
+++ b/gracedb/events/views.py
@@ -75,11 +75,8 @@ def event_and_auth_required(view):
         # maps to 'view', and unsafe methods map to 'CHANGE'
         if request.method=='GET':
             if not user_has_perm(request.user, 'view', event):
-                msg = ('You do not have permission to view this event. '
-                    'If you think you should be able to view it, make sure '
-                    'you are logged in.')
-                return render(request, '403.html', status=403,
-                    context={'graceid': graceid, 'message': msg})
+                return render(request, 'gracedb/403.html', status=403,
+                    context={'graceid': graceid})
         elif request.method in ['POST', 'DELETE']:                
             if not user_has_perm(request.user, 'change', event):
                 raise PermissionDenied
diff --git a/gracedb/templates/403.html b/gracedb/templates/403.html
index 57e9606da..520e13a80 100644
--- a/gracedb/templates/403.html
+++ b/gracedb/templates/403.html
@@ -1,7 +1,7 @@
 {% extends "base.html" %}
 
 {% block title %}403 &ndash; Forbidden{% endblock %}
-{% block heading %}Forbidden {{ graceid }}{% endblock %}
+{% block heading %}Forbidden{% endblock %}
 
 {% block content %}
     {% if message %}
diff --git a/gracedb/templates/gracedb/403.html b/gracedb/templates/gracedb/403.html
new file mode 100644
index 000000000..b81ad82e9
--- /dev/null
+++ b/gracedb/templates/gracedb/403.html
@@ -0,0 +1,11 @@
+{% extends "base.html" %}
+
+{% block title %}403 &ndash; Forbidden{% endblock %}
+{% block heading %}Forbidden {{ graceid }}{% endblock %}
+
+{% block content %}
+    <p>
+    You do not have permission to view event {{ graceid }} or the related data.
+    If you think that you should be able to view it, make sure you are logged in.
+    </p>
+{% endblock %}
-- 
GitLab