Fix a 403 when transferring a project to a read-only team member

CHANGELOG.md

@@ -7,9 +7,11 @@ All notable changes to this project will be documented in this file.
 - Use multicolor channel icons for better appearance in the dark mode
 - Add SITE_LOGO_URL setting (#323)
 - Add admin action to log in as any user
+- Add a "Manager" role (#484)
 
 ### Bug Fixes
 - Fix dark mode styling issues in Cron Syntax Cheatsheet
+- Fix a 403 when transferring a project to a read-only team member
 
 ## v1.21.0 - 2020-07-02
 

hc/accounts/tests/test_project.py

@@ -49,20 +49,34 @@ class ProjectTestCase(BaseTestCase):
         self.assertTrue(len(api_key) > 10)
         self.assertFalse("b'" in api_key)
 
+    def test_it_requires_rw_access_to_create_api_key(self):
+        self.bobs_membership.role = "r"
+        self.bobs_membership.save()
+
+        self.client.login(username="bob@example.org", password="password")
+        r = self.client.post(self.url, {"create_api_keys": "1"})
+        self.assertEqual(r.status_code, 403)
+
     def test_it_revokes_api_key(self):
         self.project.api_key_readonly = "R" * 32
         self.project.save()
 
         self.client.login(username="alice@example.org", password="password")
-
-        form = {"revoke_api_keys": "1"}
-        r = self.client.post(self.url, form)
+        r = self.client.post(self.url, {"revoke_api_keys": "1"})
         self.assertEqual(r.status_code, 200)
 
         self.project.refresh_from_db()
         self.assertEqual(self.project.api_key, "")
         self.assertEqual(self.project.api_key_readonly, "")
 
+    def test_it_requires_rw_access_to_revoke_api_key(self):
+        self.bobs_membership.role = "r"
+        self.bobs_membership.save()
+
+        self.client.login(username="bob@example.org", password="password")
+        r = self.client.post(self.url, {"revoke_api_keys": "1"})
+        self.assertEqual(r.status_code, 403)
+
     def test_it_adds_team_member(self):
         self.client.login(username="alice@example.org", password="password")
 
@@ -160,7 +174,11 @@ class ProjectTestCase(BaseTestCase):
         self.client.login(username="alice@example.org", password="password")
 
         aaa = "a" * 300
-        form = {"invite_team_member": "1", "email": f"frank+{aaa}@example.org", "role": "r"}
+        form = {
+            "invite_team_member": "1",
+            "email": f"frank+{aaa}@example.org",
+            "role": "r",
+        }
         r = self.client.post(self.url, form)
         self.assertEqual(r.status_code, 200)
 
@@ -245,6 +263,15 @@ class ProjectTestCase(BaseTestCase):
         self.project.refresh_from_db()
         self.assertEqual(self.project.name, "Alpha Team")
 
+    def test_it_requires_rw_access_to_set_project_name(self):
+        self.bobs_membership.role = "r"
+        self.bobs_membership.save()
+
+        self.client.login(username="bob@example.org", password="password")
+        form = {"set_project_name": "1", "name": "Alpha Team"}
+        r = self.client.post(self.url, form)
+        self.assertEqual(r.status_code, 403)
+
     def test_it_shows_invite_suggestions(self):
         p2 = Project.objects.create(owner=self.alice)
 
@@ -254,7 +281,7 @@ class ProjectTestCase(BaseTestCase):
         self.assertContains(r, "Add Users from Other Teams")
         self.assertContains(r, "bob@example.org")
 
-    def test_it_checks_rw_access_when_updating_project_name(self):
+    def test_it_requires_rw_access_to_update_project_name(self):
         self.bobs_membership.role = "r"
         self.bobs_membership.save()
 
@@ -280,9 +307,15 @@ class ProjectTestCase(BaseTestCase):
         self.project.save()
 
         self.client.login(username="alice@example.org", password="password")
-
-        form = {"show_api_keys": "1"}
-        r = self.client.post(self.url, form)
+        r = self.client.post(self.url, {"show_api_keys": "1"})
         self.assertEqual(r.status_code, 200)
 
         self.assertNotContains(r, "Prometheus metrics endpoint")
+
+    def test_it_requires_rw_access_to_show_api_key(self):
+        self.bobs_membership.role = "r"
+        self.bobs_membership.save()
+
+        self.client.login(username="bob@example.org", password="password")
+        r = self.client.post(self.url, {"show_api_keys": "1"})
+        self.assertEqual(r.status_code, 403)

hc/accounts/tests/test_transfer_project.py

@@ -1,5 +1,6 @@
 from django.core import mail
 from django.utils.timezone import now
+from hc.accounts.models import Member
 from hc.api.models import Check
 from hc.test import BaseTestCase
 
@@ -149,3 +150,19 @@ class TransferProjectTestCase(BaseTestCase):
         self.client.login(username="alice@example.org", password="password")
         r = self.client.post(self.url, {"reject_transfer": "1"})
         self.assertEqual(r.status_code, 403)
+
+    def test_readonly_user_can_accept(self):
+        self.bobs_membership.transfer_request_date = now()
+        self.bobs_membership.role = "r"
+        self.bobs_membership.save()
+
+        self.client.login(username="bob@example.org", password="password")
+        self.client.post(self.url, {"accept_transfer": "1"})
+
+        self.project.refresh_from_db()
+        # Bob should now be the owner
+        self.assertEqual(self.project.owner, self.bob)
+
+        # Alice, the previous owner, should now be a *regular* member
+        m = Member.objects.get(user=self.alice, project=self.project)
+        self.assertEqual(m.role, "w")

hc/accounts/views.py

@@ -302,10 +302,10 @@ def project(request, code):
     }
 
     if request.method == "POST":
-        if not rw:
-            return HttpResponseForbidden()
-
         if "create_api_keys" in request.POST:
+            if not rw:
+                return HttpResponseForbidden()
+
             project.set_api_keys()
             project.save()
 
@@ -313,6 +313,9 @@ def project(request, code):
             ctx["api_keys_created"] = True
             ctx["api_status"] = "success"
         elif "revoke_api_keys" in request.POST:
+            if not rw:
+                return HttpResponseForbidden()
+
             project.api_key = ""
             project.api_key_readonly = ""
             project.save()
@@ -320,6 +323,9 @@ def project(request, code):
             ctx["api_keys_revoked"] = True
             ctx["api_status"] = "info"
         elif "show_api_keys" in request.POST:
+            if not rw:
+                return HttpResponseForbidden()
+
             ctx["show_api_keys"] = True
         elif "invite_team_member" in request.POST:
             if not is_manager:
@@ -372,6 +378,9 @@ def project(request, code):
                 ctx["team_member_removed"] = form.cleaned_data["email"]
                 ctx["team_status"] = "info"
         elif "set_project_name" in request.POST:
+            if not rw:
+                return HttpResponseForbidden()
+
             form = forms.ProjectNameForm(request.POST)
             if form.is_valid():
                 project.name = form.cleaned_data["name"]
@@ -427,6 +436,9 @@ def project(request, code):
                 # 1. Reuse the existing membership, and change its user
                 tr.user = project.owner
                 tr.transfer_request_date = None
+                # The previous owner becomes a regular member
+                # (not readonly, not manager):
+                tr.role = Member.Role.REGULAR
                 tr.save()
 
                 # 2. Change project's owner

templates/accounts/project.html

@@ -167,7 +167,7 @@
                         {% for m in project.member_set.all %}
                         <tr>
                             <td class="email">{{ m.user.email }}</td>
-                            <td>{{ m.get_role_display}}</td>
+                            <td>{{ m.get_role_display }}</td>
                             <td>
                                 {% if is_manager and m.user != request.user %}
                                 <a