Fix management UI and API access permissions for #1425

rdmo/accounts/models.py

@@ -3,6 +3,7 @@
 from django.db import models
 from django.db.models.signals import post_save
 from django.dispatch import receiver
+from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
 
 from rdmo.core.models import Model as RDMOTimeStampedModel
@@ -205,6 +206,14 @@ class Meta:
     def __str__(self):
         return self.user.username
 
+    @cached_property
+    def is_editor(self):
+        return self.editor.filter(id=settings.SITE_ID).exists()
+
+    @cached_property
+    def is_reviewer(self):
+        return self.reviewer.filter(id=settings.SITE_ID).exists()
+
 
 @receiver(post_save, sender=settings.AUTH_USER_MODEL)
 def post_save_user(sender, **kwargs):

rdmo/accounts/tests/test_models.py

@@ -0,0 +1,45 @@
+import pytest
+
+from django.contrib.auth import get_user_model
+
+normal_users = (
+    ('user', 'user', 'user@example.com'),
+)
+
+site_editors = (
+    ('editor', 'editor', 'editor@example.com'),
+    ('example-editor', 'example-editor', 'example-editor@example.com'),
+)
+
+site_reviewers = (
+    ('reviewer', 'reviewer', 'reviewer@example.com'),
+    ('example-reviewer', 'example-reviewer', 'example-reviewer@example.com'),
+)
+
+
+
+@pytest.mark.parametrize('username,password,email', normal_users)
+def test_is_site_editor_returns_false_for_normal_users(db, client, username, password, email):
+    client.login(username=username, password=password)
+    user = get_user_model().objects.get(username=username, email=email)
+    assert user.role.is_editor is False
+
+
+@pytest.mark.parametrize('username,password,email', site_editors)
+def test_is_site_editor_returns_true_for_site_managers(db, client, username, password, email):
+    client.login(username=username, password=password)
+    user = get_user_model().objects.get(username=username, email=email)
+    assert user.role.is_editor is True
+
+@pytest.mark.parametrize('username,password,email', normal_users)
+def test_is_site_reviewer_returns_false_for_normal_users(db, client, username, password, email):
+    client.login(username=username, password=password)
+    user = get_user_model().objects.get(username=username, email=email)
+    assert user.role.is_reviewer is False
+
+
+@pytest.mark.parametrize('username,password,email', site_reviewers)
+def test_is_site_reviewer_returns_true_for_site_managers(db, client, username, password, email):
+    client.login(username=username, password=password)
+    user = get_user_model().objects.get(username=username, email=email)
+    assert user.role.is_reviewer is True

rdmo/conditions/tests/test_viewset_condition_multisite.py

@@ -54,7 +54,9 @@ def test_detail(db, client, username, password):
     for instance in instances:
         url = reverse(urlnames['detail'], args=[instance.pk])
         response = client.get(url)
-        assert response.status_code == status_map['detail'][username], response.json()
+        assert response.status_code == get_obj_perms_status_code(
+            instance, username, 'detail'
+        ), (response.json(), instance.editors.all())
 
 
 @pytest.mark.parametrize('username,password', users)
@@ -229,7 +231,9 @@ def test_update(db, client, username, password):
             'target_option': instance.target_option.pk if instance.target_option else None
         }
         response = client.put(url, data, content_type='application/json')
-        assert response.status_code == get_obj_perms_status_code(instance, username, 'update'), response.json()
+        assert response.status_code == get_obj_perms_status_code(
+            instance, username, 'update'
+        ), (response.json(), instance.editors.all())
 
 
 @pytest.mark.parametrize('username,password', users)
@@ -238,9 +242,10 @@ def test_delete(db, client, username, password):
     instances = Condition.objects.all()
 
     for instance in instances:
+        editors = list(instance.editors.values_list('domain', flat=True))
         url = reverse(urlnames['detail'], args=[instance.pk])
         response = client.delete(url)
-        assert response.status_code == get_obj_perms_status_code(instance, username, 'delete'), response.json()
+        assert response.status_code == get_obj_perms_status_code(instance, username, 'delete', editors=editors)
 
 
 @pytest.mark.parametrize('username,password', users)

rdmo/core/permissions.py

@@ -1,6 +1,6 @@
 import logging
 
-from rest_framework.permissions import DjangoModelPermissions, DjangoObjectPermissions
+from rest_framework.permissions import BasePermission, DjangoModelPermissions, DjangoObjectPermissions
 
 logger = logging.getLogger(__name__)
 
@@ -85,3 +85,14 @@ class CanToggleElementCurrentSite(DjangoModelPermissions):
     @log_result
     def has_permission(self, request, view):
         return super().has_permission(request, view)
+
+
+class HasPermission(BasePermission):
+
+    @log_result
+    def has_permission(self, request, view) -> bool:
+        if not (request.user and request.user.is_authenticated):
+            return False
+
+        # the viewset needs to set permission_required
+        return request.user.has_perm(view.permission_required)

rdmo/core/templates/core/base_navigation.html

@@ -19,7 +19,7 @@
             <ul class="nav navbar-nav">
                 {% back_to_project_link %}
 
-                {% test_rule 'management.can_view_management' request.user request.site as can_view_management %}
+                {% has_perm 'management.view_management' request.user as can_view_management %}
 
                 {% if can_view_management %}
                 <li>

rdmo/core/tests/constants.py

@@ -1,26 +1,26 @@
 
 multisite_status_map = {
     'list': {
-        'foo-user': 403, 'foo-reviewer': 200, 'foo-editor': 200,
-        'bar-user': 403, 'bar-reviewer': 200, 'bar-editor': 200,
+        'foo-user': 403, 'foo-reviewer': 403, 'foo-editor': 403,
+        'bar-user': 403, 'bar-reviewer': 403, 'bar-editor': 403,
         'user': 403, 'example-reviewer': 200, 'example-editor': 200,
         'anonymous': 401, 'reviewer': 200, 'editor': 200,
     },
     'detail': {
-        'foo-user': 404, 'foo-reviewer': 200, 'foo-editor': 200,
-        'bar-user': 404, 'bar-reviewer': 200, 'bar-editor': 200,
+        'foo-user': 404, 'foo-reviewer': 404, 'foo-editor': 404,
+        'bar-user': 404, 'bar-reviewer': 404, 'bar-editor': 404,
         'user': 404, 'example-reviewer': 200, 'example-editor': 200,
         'anonymous': 401, 'reviewer': 200, 'editor': 200,
     },
     'nested': {
-        'foo-user': 404, 'foo-reviewer': 200, 'foo-editor': 200,
-        'bar-user': 404, 'bar-reviewer': 200, 'bar-editor': 200,
+        'foo-user': 404, 'foo-reviewer': 404, 'foo-editor': 404,
+        'bar-user': 404, 'bar-reviewer': 404, 'bar-editor': 404,
         'user': 404, 'example-reviewer': 200, 'example-editor': 200,
         'anonymous': 401, 'reviewer': 200, 'editor': 200,
     },
     'create': {
-        'foo-user': 403, 'foo-reviewer': 403, 'foo-editor': 201,
-        'bar-user': 403, 'bar-reviewer': 403, 'bar-editor': 201,
+        'foo-user': 403, 'foo-reviewer': 403, 'foo-editor': 403,
+        'bar-user': 403, 'bar-reviewer': 403, 'bar-editor': 403,
         'user': 403, 'example-reviewer': 403, 'example-editor': 201,
         'anonymous': 401, 'reviewer': 403, 'editor': 201,
     },
@@ -31,14 +31,14 @@
         'anonymous': 401, 'reviewer': 403, 'editor': 201,
     },
     'update': {
-        'foo-user': 404, 'foo-reviewer': 403, 'foo-editor': 200,
-        'bar-user': 404, 'bar-reviewer': 403, 'bar-editor': 200,
+        'foo-user': 404, 'foo-reviewer': 404, 'foo-editor': 404,
+        'bar-user': 404, 'bar-reviewer': 404, 'bar-editor': 404,
         'user': 404, 'example-reviewer': 403, 'example-editor': 200,
         'anonymous': 401, 'reviewer': 403, 'editor': 200,
     },
     'delete': {
-        'foo-user': 404, 'foo-reviewer': 403, 'foo-editor': 204,
-        'bar-user': 404, 'bar-reviewer': 403, 'bar-editor': 204,
+        'foo-user': 404, 'foo-reviewer': 404, 'foo-editor': 404,
+        'bar-user': 404, 'bar-reviewer': 404, 'bar-editor': 404,
         'user': 404, 'example-reviewer': 403, 'example-editor': 204,
         'anonymous': 401, 'reviewer': 403, 'editor': 204,
     },
@@ -50,14 +50,22 @@
         'user': 403, 'example-reviewer': 403, 'example-editor': 200,
         'anonymous': 401, 'reviewer': 403, 'editor': 200,
     },
+    'management': { # access to the ui for example.com
+        'foo-user': 403, 'foo-reviewer': 403, 'foo-editor': 403,
+        'bar-user': 403, 'bar-reviewer': 403, 'bar-editor': 403,
+        'user': 403, 'example-reviewer': 200, 'example-editor': 200,
+        'anonymous': 302, 'reviewer': 200, 'editor': 200,
+    },
+    'upload-import': {  # access to the ui for example.com
+        'foo-user': 403, 'foo-reviewer': 403, 'foo-editor': 403,
+        'bar-user': 403, 'bar-reviewer': 403, 'bar-editor': 403,
+        'user': 403, 'example-reviewer': 403, 'example-editor': 200,
+        'anonymous': 401, 'reviewer': 403, 'editor': 200,
+    },
+
 }
 status_map_object_permissions = {
     'copy': {
-        'all-element': {
-            'foo-reviewer': 403, 'foo-editor': 201,
-            'bar-reviewer': 403, 'bar-editor': 201,
-            'example-reviewer': 403, 'example-editor': 201,
-        },
         'foo-element': {
             'foo-reviewer': 403, 'foo-editor': 201,
             'bar-reviewer': 404, 'bar-editor': 404,
@@ -69,10 +77,61 @@
             'example-reviewer': 404, 'example-editor': 404,
         }
     },
+    'detail': {
+        'foo-element': {
+            'foo-reviewer': 200, 'foo-editor': 200,
+            'bar-reviewer': 404, 'bar-editor': 404,
+            'example-reviewer': 200, 'example-editor': 200,  # because current site is example.com
+        },
+        'bar-element': {
+            'foo-reviewer': 404, 'foo-editor': 404,
+            'bar-reviewer': 200, 'bar-editor': 200,
+            'example-reviewer': 200, 'example-editor': 200, # because current site is example.com
+        },
+        'example-element': {
+            'foo-reviewer': 404, 'foo-editor': 404,
+            'bar-reviewer': 404, 'bar-editor': 404,
+            'example-reviewer': 200, 'example-editor': 200,
+        }
+    },
+    'nested': {
+        'foo-element': {
+            'foo-reviewer': 200, 'foo-editor': 200,
+            'bar-reviewer': 404, 'bar-editor': 404,
+            'example-reviewer': 200, 'example-editor': 200,  # because current site is example.com
+        },
+        'bar-element': {
+            'foo-reviewer': 404, 'foo-editor': 404,
+            'bar-reviewer': 200, 'bar-editor': 200,
+            'example-reviewer': 200, 'example-editor': 200,  # because current site is example.com
+        },
+        'example-element': {
+            'foo-reviewer': 404, 'foo-editor': 404,
+            'bar-reviewer': 404, 'bar-editor': 404,
+            'example-reviewer': 200, 'example-editor': 200,
+        }
+    },
     'update': {
         'all-element': {
+            'foo-reviewer': 404, 'foo-editor': 404,
+            'bar-reviewer': 404, 'bar-editor': 404,
+            'example-reviewer': 403, 'example-editor': 200,
+        },
+        'foo-element': {
             'foo-reviewer': 403, 'foo-editor': 200,
+            'bar-reviewer': 404, 'bar-editor': 404,
+            'example-reviewer': 404, 'example-editor': 404,
+        },
+        'bar-element': {
+            'foo-reviewer': 404, 'foo-editor': 404,
             'bar-reviewer': 403, 'bar-editor': 200,
+            'example-reviewer': 404, 'example-editor': 404,
+        }
+    },
+    'upload-import': {
+        'all-element': {
+            'foo-reviewer': 404, 'foo-editor': 404,
+            'bar-reviewer': 404, 'bar-editor': 404,
             'example-reviewer': 403, 'example-editor': 200,
         },
         'foo-element': {

rdmo/core/tests/test_views.py

@@ -68,7 +68,7 @@ def test_i18n_switcher(db, client):
 
 
 @pytest.mark.parametrize('username,password', users)
-def test_can_view_management(db, client, username, password):
+def test_view_management(db, client, username, password):
     client.login(username=username, password=password)
     response = client.get(reverse('management'))
     if username in ('editor', 'reviewer', 'api', 'example-editor', 'example-reviewer'):

rdmo/core/tests/utils.py

@@ -1,32 +1,62 @@
 import hashlib
 
-from rdmo.core.models import Model
+from django.db.models import Model
+
 from rdmo.core.tests.constants import multisite_status_map, status_map_object_permissions
 
 
-def get_obj_perms_status_code(instance, username, method):
+def get_obj_perms_status_code(instance, username, method, editors=None):
     ''' looks for the object permissions of the instance and returns the status code '''
 
-    if isinstance(instance, Model):
-        try:
-            if not instance.editors.exists():
-                return multisite_status_map[method][username]
-        except AttributeError as e:
-            raise AttributeError(f'instance {instance} should have an editors attribute') from e
+    if (isinstance(instance, Model) or hasattr(instance, 'editors')) and hasattr(instance, 'uri'):
+        instance_uri = instance.uri
+        instance_editors = instance.editors.values_list('domain', flat=True)
     elif isinstance(instance, str):
-        pass
+        instance_uri = instance
+        instance_editors = []
+    else:
+        raise TypeError(f'instance {instance} should be a str or a Model (and have an uri)')
 
-    if 'foo-' in str(instance):
+    if editors is not None and method == 'delete':
+        # override in case of deleted instance
+        instance_editors = editors
+
+    if 'foo-' in instance_uri:
         instance_obj_perms_key = 'foo-element'
-    elif 'bar-' in str(instance):
+        assert_editors = ['foo.com']
+    elif 'bar-' in instance_uri:
         instance_obj_perms_key = 'bar-element'
+        assert_editors = ['bar.com']
+    elif 'example-' in instance_uri:
+        instance_obj_perms_key = 'example-element'
+        assert_editors = ['example.com']
     else:
+        if instance_editors:
+            raise ValueError(f"uri {instance_uri} should contain the domain for {instance_editors}")
         instance_obj_perms_key = 'all-element'
+        assert_editors = ['foo.com', 'bar.com', 'example.com']
+
+    if not instance_editors and instance_obj_perms_key != 'all-element':
+        if 'import' in method:  # override for import when only uri is passed
+            instance_editors = assert_editors
+        else:
+            raise ValueError(f"instance_editors should be specified on {instance_uri}")
+    elif instance_editors:
+        assert all(
+            i in instance_editors for i in assert_editors
+        ), f"{assert_editors} should be specified on {instance_uri}"
+
+
+    if not instance_editors:
+        return multisite_status_map[method][username]
 
     try:
         method_instance_obj_perms_map = status_map_object_permissions[method][instance_obj_perms_key]
     except KeyError as e:
-        raise KeyError(f'instance ({instance_obj_perms_key}) should be defined in status_map_object_permissions') from e
+        raise KeyError(
+            f'instance (uri={instance_uri}, editors={instance_editors}) for {method} ({instance_obj_perms_key})'
+            f' should be defined in status_map_object_permissions'
+        ) from e
     try:
         return method_instance_obj_perms_map[username]
     except KeyError: