[Fixes #6991] User info endpoint returns 403 (forbidden) to authetica… (

#6992)

* [Fixes #6991] User info endpoint returns 403 (forbidden) to autheticated users which are not "superuser"

* - Revert wrong commit

geonode/base/api/permissions.py

@@ -19,12 +19,87 @@
 #########################################################################
 import logging
 from django.conf import settings
+from django.contrib.auth import get_user_model
+
 from rest_framework import permissions
 from rest_framework.filters import BaseFilterBackend
 
 logger = logging.getLogger(__name__)
 
 
+class IsSelf(permissions.BasePermission):
+
+    """ Grant permission only if the current instance is the request user.
+    Used to allow users to edit their own account, nothing to others (even
+    superusers).
+    """
+
+    def has_permission(self, request, view):
+        """ Always return True here.
+        The fine-grained permissions are handled in has_object_permission().
+        """
+
+        return True
+
+    def has_object_permission(self, request, view, obj):
+        return obj.id == request.user.id
+
+
+class IsSelfOrReadOnly(IsSelf):
+
+    """ Grant permissions if instance *IS* the request user, or read-only.
+    Used to allow users to edit their own account, and others to read.
+    """
+
+    def has_object_permission(self, request, view, obj):
+
+        if request.method in permissions.SAFE_METHODS:
+            return True
+
+        return IsSelf.has_object_permission(self, request, view, obj)
+
+
+class IsSelfOrAdmin(IsSelf):
+
+    """ Grant R/W to self and superusers/staff members. Deny others. """
+
+    def has_object_permission(self, request, view, obj):
+
+        user = request.user
+
+        if user.is_superuser or user.is_staff:
+            return True
+
+        return IsSelf.has_object_permission(self, request, view, obj)
+
+
+class IsSelfOrAdminOrReadOnly(IsSelfOrAdmin):
+
+    """ Grant R/W to self and superusers/staff members, R/O to others. """
+
+    def has_object_permission(self, request, view, obj):
+
+        if request.method in permissions.SAFE_METHODS:
+            return True
+
+        return IsSelfOrAdmin.has_object_permission(self, request, view, obj)
+
+
+class IsSelfOrAdminOrAuthenticatedReadOnly(IsSelfOrAdmin):
+
+    """ Grant R/W to self and superusers/staff members, R/O to auth. """
+
+    def has_object_permission(self, request, view, obj):
+
+        user = request.user
+
+        if request.method in permissions.SAFE_METHODS:
+            if user.is_authenticated():
+                return True
+
+        return IsSelfOrAdmin.has_object_permission(self, request, view, obj)
+
+
 class IsOwnerOrReadOnly(permissions.BasePermission):
     """
     Object-level permission to only allow owners of an object to edit it.
@@ -39,13 +114,15 @@ def has_object_permission(self, request, view, obj):
 
         # Read permissions are allowed to any request,
         # so we'll always allow GET, HEAD or OPTIONS requests.
-        if request.method in permissions.SAFE_METHODS:
+        if request.method in permissions.SAFE_METHODS and not isinstance(obj, get_user_model()):
             return True
 
         # Instance must have an attribute named `owner`.
-        if hasattr(obj, 'owner'):
+        if isinstance(obj, get_user_model()) and obj == request.user:
+            return True
+        elif hasattr(obj, 'owner'):
             return obj.owner == request.user
-        if hasattr(obj, 'user'):
+        elif hasattr(obj, 'user'):
             return obj.user == request.user
         else:
             return False

geonode/base/api/tests.py

@@ -28,6 +28,7 @@
 from django.core.files import File
 from django.conf.urls import url, include
 from django.views.generic import TemplateView
+from django.contrib.auth import get_user_model
 from django.views.i18n import JavaScriptCatalog
 from rest_framework.test import APITestCase, URLPatternsTestCase
 
@@ -144,9 +145,13 @@ def test_users_list(self):
         Ensure we can access the users list.
         """
         url = reverse('users-list')
-        # Unauhtorized
+        # Anonymous
         response = self.client.get(url, format='json')
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(response.data), 5)
+        logger.debug(response.data)
+        self.assertEqual(response.data['total'], 0)
+        self.assertEqual(len(response.data['users']), 0)
 
         # Auhtorized
         self.assertTrue(self.client.login(username='admin', password='admin'))
@@ -164,6 +169,30 @@ def test_users_list(self):
         self.assertEqual(response.data['user']['username'], 'admin')
         self.assertIsNotNone(response.data['user']['avatar'])
 
+        # Bobby
+        self.assertTrue(self.client.login(username='bobby', password='bob'))
+        # Bobby cannot access other users' details
+        response = self.client.get(url, format='json')
+        self.assertEqual(response.status_code, 404)
+
+        # Bobby can see himself in the list
+        url = reverse('users-list')
+        self.assertEqual(len(response.data), 1)
+        response = self.client.get(url, format='json')
+        self.assertEqual(response.status_code, 200)
+        logger.debug(response.data)
+        self.assertEqual(response.data['total'], 1)
+        self.assertEqual(len(response.data['users']), 1)
+
+        # Bobby can access its own details
+        bobby = get_user_model().objects.filter(username='bobby').get()
+        url = reverse('users-detail', kwargs={'pk': bobby.id})
+        response = self.client.get(url, format='json')
+        self.assertEqual(response.status_code, 200)
+        logger.debug(response.data)
+        self.assertEqual(response.data['user']['username'], 'bobby')
+        self.assertIsNotNone(response.data['user']['avatar'])
+
     def test_base_resources(self):
         """
         Ensure we can access the Resource Base list.

geonode/base/api/views.py

@@ -39,6 +39,7 @@
 from guardian.shortcuts import get_objects_for_user
 
 from .permissions import (
+    IsSelfOrAdmin,
     IsOwnerOrReadOnly,
     ResourceBasePermissionsFilter
 )
@@ -60,14 +61,21 @@ class UserViewSet(DynamicModelViewSet):
     """
     API endpoint that allows users to be viewed or edited.
     """
-    authentication_classes = (SessionAuthentication, BasicAuthentication, OAuth2Authentication)
-    permission_classes = (IsAdminUser,)
+    authentication_classes = [SessionAuthentication, BasicAuthentication, OAuth2Authentication]
+    permission_classes = [IsSelfOrAdmin, ]
     queryset = get_user_model().objects.all()
     serializer_class = UserSerializer
     pagination_class = GeoNodeApiPagination
 
     def get_queryset(self):
-        queryset = get_user_model().objects.all()
+        """
+        Filter objects so a user only sees his own stuff.
+        If user is admin, let him see all.
+        """
+        if self.request.user.is_superuser or self.request.user.is_staff:
+            queryset = get_user_model().objects.all()
+        else:
+            queryset = get_user_model().objects.filter(id=self.request.user.id)
         # Set up eager loading to avoid N+1 selects
         queryset = self.get_serializer_class().setup_eager_loading(queryset)
         return queryset
@@ -102,8 +110,8 @@ class GroupViewSet(DynamicModelViewSet):
     """
     API endpoint that allows gropus to be viewed or edited.
     """
-    authentication_classes = (SessionAuthentication, BasicAuthentication, OAuth2Authentication)
-    permission_classes = (IsAdminUser,)
+    authentication_classes = [SessionAuthentication, BasicAuthentication, OAuth2Authentication]
+    permission_classes = [IsAuthenticated, ]
     queryset = GroupProfile.objects.all()
     serializer_class = GroupProfileSerializer
     pagination_class = GeoNodeApiPagination
@@ -202,8 +210,9 @@ def resource_types(self, request):
             if _model.__name__ == "ResourceBase":
                 resource_types = [_m.__name__.lower() for _m in _model.__subclasses__()]
         if "geoapp" in resource_types:
-            from geonode.geoapps.models import GeoApp
             resource_types.remove("geoapp")
+        if settings.GEONODE_APPS_ENABLE:
+            from geonode.geoapps.models import GeoApp
             for label, app in apps.app_configs.items():
                 if hasattr(app, 'type') and app.type == 'GEONODE_APP':
                     if hasattr(app, 'default_model'):