Fixes 20210205 (#935)

* Add model check so you cannot double approve an admin org affiliation

* remove django namespace perms form INSTALLED_APPS

* script to fix users affected by #930

* black formatted

* add nsp back to installed apps for now
remove remaining nsp references

* fix initial_data fixtures (nsp->grainy)

* fixes #934: privacy permissions scope for pocs and ixf urls

* test that poc pirvacy scope works correctly on GUI (#934)

* black format

Co-authored-by: Elliot Frank <elliot@20c.com>
Co-authored-by: Stefan Pratter <stefan@20c.com>

fixtures/initial_data.json

@@ -14,30 +14,30 @@
     }
   },
   {
-    "model" : "django_namespace_perms.grouppermission",
+    "model" : "django_grainy.grouppermission",
     "pk" : 1,
     "fields" : {
       "group" : 1,
       "namespace" : "peeringdb.organization",
-      "permissions" : 1
+      "permission" : 1
     }
   },
   {
-    "model" : "django_namespace_perms.grouppermission",
+    "model" : "django_grainy.grouppermission",
     "pk" : 5,
     "fields" : {
       "group" : 2,
       "namespace" : "peeringdb.organization",
-      "permissions" : 1
+      "permission" : 1
     }
   },
   {
-    "model" : "django_namespace_perms.grouppermission",
+    "model" : "django_grainy.grouppermission",
     "pk" : 6,
     "fields" : {
       "group" : 2,
       "namespace" : "peeringdb.organization.*.network.*.poc_set.users",
-      "permissions" : 1
+      "permission" : 1
     }
   }
 ]

mainsite/settings/__init__.py

@@ -585,9 +585,8 @@ OAUTH2_PROVIDER = {
 }
 
 
-## NSP
+## grainy
 
-NSP_MODE = "crud"
 AUTHENTICATION_BACKENDS += ("django_grainy.backends.GrainyBackend",)
 
 

peeringdb_server/management/commands/pdb_fix_930_users.py

@@ -0,0 +1,34 @@
+from django.core.management.base import BaseCommand
+
+from peeringdb_server.models import User
+from peeringdb_server.management.commands.pdb_base_command import PeeringDBBaseCommand
+
+
+class Command(PeeringDBBaseCommand):
+
+    """
+    Fixes users affected by being both in the org admin
+    and org user group when it should be one or the other
+    """
+
+    def handle(self, *args, **options):
+        super().handle(*args, **options)
+        self.fix_users()
+
+    def fix_users(self):
+        issues = 0
+
+        for user in User.objects.all():
+            groups = {}
+            for group in user.groups.all():
+                groups[group.name] = group
+            for group_name, group in groups.items():
+                if f"{group_name}.admin" in groups:
+                    # user is in both groups
+                    if self.commit:
+                        # remove user from the normal users group
+                        user.groups.remove(group)
+                    self.log(f"removed {user} from {group_name}")
+                    issues += 1
+
+        self.log(f"{issues} fixed")

peeringdb_server/models.py

@@ -492,6 +492,11 @@ class UserOrgAffiliationRequest(models.Model):
         """
 
         if self.org_id:
+
+            if self.user.is_org_admin(self.org) or self.user.is_org_member(self.org):
+                self.delete()
+                return
+
             if (
                 self.org.admin_usergroup.user_set.count() > 0
                 or self.org.usergroup.user_set.count() > 0
@@ -519,6 +524,11 @@ class UserOrgAffiliationRequest(models.Model):
         it around until requesting user deletes it
         """
 
+        if self.user and self.org:
+            if self.user.is_org_admin(self.org) or self.user.is_org_member(self.org):
+                self.delete()
+                return
+
         self.status = "denied"
         self.save()
 

peeringdb_server/org_admin_views.py

@@ -10,7 +10,6 @@ from .forms import OrgAdminUserPermissionForm
 
 from grainy.const import *
 from django_grainy.models import UserPermission
-from django_namespace_perms.constants import *
 
 from django_handleref.models import HandleRefModel
 

peeringdb_server/views.py

@@ -4,6 +4,8 @@ import datetime
 import re
 import uuid
 
+from grainy.const import *
+
 from allauth.account.models import EmailAddress
 from django.http import (
     JsonResponse,
@@ -28,12 +30,6 @@ from django.utils.translation import ugettext_lazy as _
 from django.utils.crypto import constant_time_compare
 from django.utils.decorators import method_decorator
 from django_grainy.util import Permissions
-from django_namespace_perms.constants import (
-    PERM_CRUD,
-    PERM_CREATE,
-    PERM_DELETE,
-    PERM_WRITE,
-)
 import requests
 
 from oauth2_provider.decorators import protected_resource
@@ -43,7 +39,7 @@ from django_otp.plugins.otp_email.models import EmailDevice
 import two_factor.views
 
 from peeringdb_server import settings
-from peeringdb_server.util import check_permissions, PERM_CRUD
+from peeringdb_server.util import check_permissions, PERM_CRUD, APIPermissionsApplicator
 from peeringdb_server.search import search
 from peeringdb_server.stats import stats as global_stats
 from peeringdb_server.org_admin_views import load_all_user_permissions
@@ -132,7 +128,7 @@ def export_permissions(user, entity):
         return {}
 
     perms = {
-        "can_write": check_permissions(user, entity, PERM_WRITE),
+        "can_write": check_permissions(user, entity, PERM_UPDATE),
         "can_create": check_permissions(user, entity, PERM_CREATE),
         "can_delete": check_permissions(user, entity, PERM_DELETE),
     }
@@ -1095,6 +1091,11 @@ def view_facility(request, id):
 
     data = FacilitySerializer(facility, context={"user": request.user}).data
 
+    applicator = APIPermissionsApplicator(request.user)
+
+    if not applicator.is_generating_api_cache:
+        data = applicator.apply(data)
+
     if not data:
         return view_http_error_403(request)
 
@@ -1245,6 +1246,11 @@ def view_exchange(request, id):
 
     data = InternetExchangeSerializer(exchange, context={"user": request.user}).data
 
+    applicator = APIPermissionsApplicator(request.user)
+
+    if not applicator.is_generating_api_cache:
+        data = applicator.apply(data)
+
     # find out if user can write to object
     perms = export_permissions(request.user, exchange)
 
@@ -1503,6 +1509,10 @@ def view_network(request, id):
         return view_http_error_404(request)
 
     network_d = NetworkSerializer(network, context={"user": request.user}).data
+    applicator = APIPermissionsApplicator(request.user)
+
+    if not applicator.is_generating_api_cache:
+        network_d = applicator.apply(network_d)
 
     if not network_d:
         return view_http_error_403(request)

tests/test_entity_view.py

@@ -144,24 +144,37 @@ class TestNetworkView(ViewTestCase):
         # test #1 - not logged in
         c = Client()
         resp = c.get("/net/%d" % self.net.id, follow=True)
+        content = resp.content.decode("utf-8")
         self.assertEqual(resp.status_code, 200)
         assert resp.status_code == 200
-        assert TEXT_NOT_LOGGED_IN in resp.content.decode("utf-8")
+        assert TEXT_NOT_LOGGED_IN in content
+        assert "Contact Public" in content
+        assert "Contact Private" not in content
+        assert "Contact Users" not in content
 
         # test #2 - guest logged in (not affiliated to any org)
         c = Client()
         c.login(username="guest", password="guest")
         resp = c.get("/net/%d" % self.net.id)
+        content = resp.content.decode("utf-8")
         assert resp.status_code == 200
-        assert TEXT_NOT_VERIFIED in resp.content.decode("utf-8")
+        assert TEXT_NOT_VERIFIED in content
+        assert "Contact Public" in content
+        assert "Contact Private" not in content
+        assert "Contact Users" not in content
 
         # test #3 - user logged in
         c = Client()
         c.login(username="user_a", password="user_a")
         resp = c.get("/net/%d" % self.net.id)
+        content = resp.content.decode("utf-8")
         assert resp.status_code == 200
-        assert TEXT_NOT_LOGGED_IN not in resp.content.decode("utf-8")
-        assert TEXT_NOT_VERIFIED not in resp.content.decode("utf-8")
+        assert TEXT_NOT_LOGGED_IN not in content
+        assert TEXT_NOT_VERIFIED not in content
+
+        assert "Contact Public" in content
+        assert "Contact Private" in content
+        assert "Contact Users" in content
 
     def test_search_asn_redirect(self):
         """

tests/test_orgadmin.py

@@ -644,6 +644,65 @@ class OrgAdminTests(TestCase):
 
         uoar_b.delete()
 
+    def test_uoar_double_affiliation_request(self):
+        """
+        Test that making a second affiliation request
+        does nothing for normal members bc
+        a signal intercepts the request (issue 930)
+        """
+        self.assertTrue(self.user_a.is_org_member(self.org))
+        self.assertTrue(self.user_a not in self.org.admin_usergroup.user_set.all())
+        self.assertTrue(self.user_a in self.org.usergroup.user_set.all())
+
+        models.UserOrgAffiliationRequest.objects.create(
+            user=self.user_a, org=self.org, status="pending"
+        )
+        self.assertTrue(self.user_a not in self.org.admin_usergroup.user_set.all())
+        self.assertTrue(self.user_a in self.org.usergroup.user_set.all())
+        self.assertEqual(models.UserOrgAffiliationRequest.objects.count(), 0)
+
+    def test_uoar_double_deny_admin(self):
+        """
+        Test that making and denying a second affiliation request
+        does nothing for admin members (issue 930)
+        """
+        self.assertTrue(self.org_admin.is_org_admin(self.org))
+        self.assertTrue(self.org_admin in self.org.admin_usergroup.user_set.all())
+        self.assertTrue(self.org_admin not in self.org.usergroup.user_set.all())
+
+        uoar_admin = models.UserOrgAffiliationRequest.objects.create(
+            user=self.org_admin, asn=1, status="pending"
+        )
+        """
+        Denying an additional request should not change the affiliation
+        and should delete the request
+        """
+        uoar_admin.deny()
+        self.assertTrue(self.org_admin in self.org.admin_usergroup.user_set.all())
+        self.assertTrue(self.org_admin not in self.org.usergroup.user_set.all())
+        self.assertEqual(models.UserOrgAffiliationRequest.objects.count(), 0)
+
+    def test_uoar_double_approve_admin(self):
+        """
+        Test that making and approving a second affiliation request
+        does nothing for admin members (issue 930)
+        """
+        self.assertTrue(self.org_admin.is_org_admin(self.org))
+        self.assertTrue(self.org_admin in self.org.admin_usergroup.user_set.all())
+        self.assertTrue(self.org_admin not in self.org.usergroup.user_set.all())
+
+        uoar_admin = models.UserOrgAffiliationRequest.objects.create(
+            user=self.org_admin, asn=1, status="pending"
+        )
+        """
+        Approving an additional request should not change the affiliation
+        and should delete the request
+        """
+        uoar_admin.approve()
+        self.assertTrue(self.org_admin in self.org.admin_usergroup.user_set.all())
+        self.assertTrue(self.org_admin not in self.org.usergroup.user_set.all())
+        self.assertEqual(models.UserOrgAffiliationRequest.objects.count(), 0)
+
     def test_uoar_cancel_on_delete(self):
         """
         Test that user affiliation requests get canceled if the