Node request clean up

.docker-compose.env

@@ -19,4 +19,4 @@ BROKER_URL=amqp://guest:guest@192.168.168.167:5672/
 REDIS_URL=redis://192.168.168.167:6379/1
 EMBER_DOMAIN=192.168.168.167
 
-#PYTHONUNBUFFERED=0 # This when set to 0 will allow print statements to be visible in the Docker logs
+PYTHONUNBUFFERED=0 # This when set to 0 will allow print statements to be visible in the Docker logs

api/requests/permissions.py

@@ -9,51 +9,76 @@
 from osf.utils import permissions as osf_permissions
 
 
+from rest_framework import permissions as drf_permissions
+from api.base.utils import get_user_auth
+from osf.models.action import NodeRequestAction
+from osf.models.mixins import NodeRequestableMixin
+from osf.models.node import Node
+from osf.utils.workflows import DefaultTriggers
+from osf.utils import permissions as osf_permissions
+
+
 class NodeRequestPermission(drf_permissions.BasePermission):
     def has_object_permission(self, request, view, obj):
         auth = get_user_auth(request)
-        if auth.user is None:
+        if not auth.user:
             return False
 
-        target = None
+        # Determine target, node, and trigger based on object type
+        target, node, trigger = None, None, request.data.get('trigger', None)
+
         if isinstance(obj, NodeRequestAction):
-            target = obj.target
-            node = obj.target.target
-            trigger = request.data.get('trigger', None)
+            target, node, trigger = self.handle_node_request_action(obj, trigger)
         elif isinstance(obj, NodeRequestableMixin):
-            target = obj
-            node = obj.target
-            # Creating a Request is "submitting"
-            trigger = request.data.get('trigger', DefaultTriggers.SUBMIT.value if request.method not in drf_permissions.SAFE_METHODS else None)
+            target, node, trigger = self.handle_node_requestable_mixin(request, obj, trigger)
         elif isinstance(obj, Node):
-            node = obj
-            trigger = DefaultTriggers.SUBMIT.value if request.method not in drf_permissions.SAFE_METHODS else None
+            node, trigger = self.handle_node(obj, request, trigger)
         else:
             raise ValueError(f'Not a request-related model: {obj}')
 
         if not node.access_requests_enabled:
             return False
 
-        is_requester = target is not None and target.creator == auth.user or trigger == DefaultTriggers.SUBMIT.value
+        return self.check_permissions(request, auth, target, node, trigger)
+
+    def handle_node_request_action(self, obj, trigger):
+        """Handle permission logic for NodeRequestAction."""
+        target = obj.target
+        node = obj.target.target
+        return target, node, trigger
+
+    def handle_node_requestable_mixin(self, request, obj, trigger):
+        """Handle permission logic for NodeRequestableMixin."""
+        target = obj
+        node = obj.target
+        if request.method not in drf_permissions.SAFE_METHODS:
+            trigger = trigger or DefaultTriggers.SUBMIT.value
+        return target, node, trigger
+
+    def handle_node(self, obj, request, trigger):
+        """Handle permission logic for Node."""
+        if request.method not in drf_permissions.SAFE_METHODS:
+            trigger = DefaultTriggers.SUBMIT.value
+        return obj, trigger
+
+    def check_permissions(self, request, auth, target, node, trigger):
+        """Check if the user has the appropriate permissions."""
+        is_requester = target and target.creator == auth.user or trigger == DefaultTriggers.SUBMIT.value
         is_node_admin = node.has_permission(auth.user, osf_permissions.ADMIN)
         has_view_permission = is_requester or is_node_admin
 
         if request.method in drf_permissions.SAFE_METHODS:
-            # Requesters and node admins can view actions
             return has_view_permission
-        else:
-            if not has_view_permission:
-                return False
 
-            if trigger in [DefaultTriggers.ACCEPT.value, DefaultTriggers.REJECT.value]:
-                # Node admins can only approve or reject requests
-                return is_node_admin
-            if trigger in [DefaultTriggers.EDIT_COMMENT.value, DefaultTriggers.SUBMIT.value]:
-                # Requesters may not be contributors
-                # Requesters may edit their comment or submit their request
-                return is_requester and auth.user not in node.contributors
+        if not has_view_permission:
             return False
 
+        if trigger in {DefaultTriggers.ACCEPT.value, DefaultTriggers.REJECT.value}:
+            return is_node_admin
+        if trigger in {DefaultTriggers.EDIT_COMMENT.value, DefaultTriggers.SUBMIT.value}:
+            return is_requester and auth.user not in node.contributors
+        return False
+
 
 class PreprintRequestPermission(drf_permissions.BasePermission):
     def has_object_permission(self, request, view, obj):

docker-compose.yml

@@ -69,7 +69,7 @@ services:
 
   # Temporary: Remove when we've upgraded to ES6
   elasticsearch6:
-    image: docker.elastic.co/elasticsearch/elasticsearch:6.3.1
+    image: quay.io/centerforopenscience/elasticsearch:es6-arm-6.3.1
     ports:
       - 9201:9200
     volumes:

website/project/decorators.py

@@ -202,54 +202,63 @@ def check_can_download_preprint_file(user, node):
     return user.has_perm('view_submissions', node.provider)
 
 
-def check_can_access(node, user, key=None, api_node=None, include_groups=True):
-    """View helper that returns whether a given user can access a node.
-    If ``user`` is None, returns False.
-
-    :rtype: boolean
-    :raises: HTTPError (403) if user cannot access the node
+def check_can_access(node, user, key=None):
+    """
+    View helper that determines whether a given user can access a node.
+    Returns False if `user` is None.
+
+    :param node: The node to check access for.
+    :param user: The user attempting to access the node.
+    :param key: Optional key for private links.
+    :param api_node: Optional API node for comparison.
+    :param include_groups: Whether to include group permissions.
+    :return: True if access is allowed, otherwise raises an HTTPError.
     """
-    if user is None:
+    if not user:
         return False
-    if request.args.get('action', '') == 'download':
-        if check_can_download_preprint_file(user, node):
-            return True
 
-    if (not node.can_view(Auth(user=user)) and api_node != node) or (not include_groups and not node.is_contributor(user)):
+    # Check for preprint file download permission
+    if request.args.get('action') == 'download' and check_can_download_preprint_file(user, node):
+        return True
+
+    user_auth = Auth(user=user)
+    user_can_view = node.can_view(user_auth)
+
+    if user_can_view:
         if node.is_deleted:
-            raise HTTPError(http_status.HTTP_410_GONE, data={'message_long': 'The node for this file has been deleted.'})
+            raise HTTPError(
+                http_status.HTTP_410_GONE,
+                data={'message_long': 'The node for this file has been deleted.'}
+            )
 
         if getattr(node, 'private_link_keys_deleted', False) and key in node.private_link_keys_deleted:
             status.push_status_message('The view-only links you used are expired.', trust=False)
 
         if getattr(node, 'access_requests_enabled', False):
-            access_request = node.requests.filter(creator=user).exclude(machine_state='accepted')
+            access_request = node.requests.filter(creator=user).exclude(machine_state='accepted').first()
             data = {
-                'node': {
-                    'id': node._id,
-                    'url': node.url
-                },
-                'user': {
-                    'access_request_state': access_request.get().machine_state if access_request else None
-                }
+                'node': {'id': node._id, 'url': node.url},
+                'user': {'access_request_state': access_request.machine_state if access_request else None}
             }
-            raise TemplateHTTPError(
-                http_status.HTTP_403_FORBIDDEN,
-                template='request_access.mako',
-                data=data
-            )
+            raise TemplateHTTPError(http_status.HTTP_403_FORBIDDEN, data=data)
 
         if isinstance(node, Registration):
-            return node.provider.get_group('moderator').user_set.filter(id=user.id).exists()
+            is_moderator = node.provider.get_group('moderator').user_set.filter(id=user.id).exists()
+            if is_moderator:
+                return True
 
         raise HTTPError(
             http_status.HTTP_403_FORBIDDEN,
-            data={'message_long': ('User has restricted access to this page. If this should not '
-                                   'have occurred and the issue persists, ' + language.SUPPORT_LINK)}
+            data={'message_long': (
+                'User has restricted access to this page. If this should not '
+                'have occurred and the issue persists, ' + language.SUPPORT_LINK
+            )}
         )
+
     return True
 
 
+
 def check_key_expired(key, node, url):
     """check if key expired if is return url with args so it will push status message
         else return url
@@ -457,10 +466,10 @@ def check_contributor_auth(node, auth, include_public, include_view_only_anon, i
 
     if not node.is_public or not include_public:
         if not include_view_only_anon and link_anon:
-            if not check_can_access(node=node, user=user, include_groups=include_groups):
+            if not check_can_access(node=node, user=user):
                 raise HTTPError(http_status.HTTP_401_UNAUTHORIZED)
         elif not getattr(node, 'private_link_keys_active', False) or auth.private_key not in node.private_link_keys_active:
-            if not check_can_access(node=node, user=user, key=auth.private_key, include_groups=include_groups):
+            if not check_can_access(node=node, user=user, key=auth.private_key):
                 redirect_url = check_key_expired(key=auth.private_key, node=node, url=request.url)
                 if request.headers.get('Content-Type') == 'application/json':
                     raise HTTPError(http_status.HTTP_401_UNAUTHORIZED)