OpenConceptLab/ocl_issues#147: Users can view public collections that…

… are marked private if the user query is specified

integration-tests/package.json

@@ -7,6 +7,7 @@
   "devDependencies": {
     "@types/jest": "^24.0.15",
     "@types/node": "^12.6.8",
+    "faker": "^4.1.0",
     "jest": "^24.8.0",
     "node-fetch": "^2.6.0",
     "ts-jest": "^24.0.2",

integration-tests/src/api.ts

@@ -0,0 +1,16 @@
+import {put, get, post} from "./utils"
+
+const api = {
+    organizations: {
+        new: async (orgId: string, token: string, publicAccess: boolean= true): Promise<Response> => publicAccess ?
+            post('orgs/', {id: orgId, name: orgId}, token) :
+            post('orgs/', {id: orgId, name: orgId, public_access: 'None'}, token),
+        addNewMember: async (membersUrl, user, token) => put(`${membersUrl}${user}/`, undefined, token),
+    },
+    collections: {
+        list: async (ownerUrl, token, verbose=true) => (await get(`${ownerUrl}collections/?verbose=${verbose}`, token)).json(),
+        new: async (ownerUrl, body, token) => (await post(`${ownerUrl}collections/`, body, token)).json(),
+    },
+};
+
+export default api;

integration-tests/src/collection.spec.ts

@@ -0,0 +1,63 @@
+import * as faker from 'faker';
+import {get, post, authenticate, authenticateAdmin, newUser, del} from "./utils";
+import collectionFixture from "./fixtures/collection";
+import api from "./api";
+
+describe('List collections for a specific user or organization', () => {
+    let adminToken;
+    let user1InOrg1;
+    let user2InOrg1;
+    let user3NotInOrg1;
+    let org1;
+    let privateCollectionOwnedByUser1;
+    let privateCollectionOwnedByOrg1;
+    let publicCollectionOwnedByUser1;
+    let publicCollectionOwnedByOrg1;
+
+    beforeAll(async () => {
+        adminToken = await authenticateAdmin();
+        // making this as random as possible since we can't delete users
+        const generateRandomName = () => faker.name.firstName() + faker.name.lastName();
+        const username1 = generateRandomName();
+        const username2 = generateRandomName();
+        const username3 = generateRandomName();
+        const orgId1 = faker.company.bsNoun();
+
+        user1InOrg1 = await newUser(username1, username1, adminToken);
+        user2InOrg1 = await newUser(username2, username2, adminToken);
+        user3NotInOrg1 = await newUser(username3, username3, adminToken);
+        user1InOrg1.token = await authenticate(username1, username1);
+        user2InOrg1.token = await authenticate(username2, username2);
+        user3NotInOrg1.token = await authenticate(username3, username3);
+
+        org1 = await (await api.organizations.new(orgId1, adminToken)).json();
+        await api.organizations.addNewMember(org1.members_url, user1InOrg1.username, adminToken);
+        await api.organizations.addNewMember(org1.members_url, user2InOrg1.username, adminToken);
+
+        privateCollectionOwnedByUser1 = await api.collections.new(user1InOrg1.url, collectionFixture(), user1InOrg1.token);
+        privateCollectionOwnedByOrg1 = await api.collections.new(org1.url, collectionFixture(), user1InOrg1.token);
+        publicCollectionOwnedByUser1 = await api.collections.new(user1InOrg1.url, collectionFixture('View'), user1InOrg1.token);
+        publicCollectionOwnedByOrg1 = await api.collections.new(org1.url, collectionFixture('View'), user1InOrg1.token);
+    });
+
+    afterAll(async () => {
+        const items = [
+            publicCollectionOwnedByOrg1,
+            publicCollectionOwnedByUser1,
+            privateCollectionOwnedByOrg1,
+            privateCollectionOwnedByUser1,
+            org1,
+            user3NotInOrg1,
+            user2InOrg1,
+            user1InOrg1,
+        ];
+        for(let i in items) await del(items[i].url, adminToken);
+    });
+
+    test('user can retrieve own collections', async () => {
+        expect(await api.collections.list(`${user1InOrg1.url}`, user1InOrg1.token)).toEqual([
+            publicCollectionOwnedByUser1,
+            privateCollectionOwnedByUser1,
+        ])
+    });
+});

integration-tests/src/fixtures/collection.ts

@@ -0,0 +1,21 @@
+import * as faker from 'faker';
+
+const collection = (publicAccess = 'None') => {
+    return {
+        "type": "Collection",
+        "uuid": faker.random.uuid(),
+        "id": faker.random.number(),
+        "external_id": "",
+        "short_code": faker.company.bsNoun(),
+        "name": faker.company.companyName(),
+        "full_name": faker.company.companyName(),
+        "collection_type": "Core Dataset",
+        "public_access": publicAccess,
+        "supported_locales": "en,es",
+        "website": "",
+        "description": "",
+        "extras": {},
+    }
+};
+
+export default collection;

integration-tests/src/org.spec.ts

@@ -1,4 +1,5 @@
 import {get, post, del, newUser, authenticateAdmin, authenticate} from './utils';
+import api from "./api";
 
 describe('Org', () => {
     const uniqueId = '6a5bb179';
@@ -20,13 +21,7 @@ describe('Org', () => {
         return 'Test' + uniqueId + new Date().getTime() + 'Org';
     };
 
-    const newOrg = async (orgId: string, token: string, publicAccess: boolean= true): Promise<Response> => {
-        if (publicAccess) {
-            return post('orgs/', {id: orgId, name: orgId}, token);
-        } else {
-            return post('orgs/', {id: orgId, name: orgId, public_access: 'None'}, token);
-        }
-    };
+    const newOrg = api.organizations.new;
 
     const newCleanup = (url: string): (() => Promise<void>) => {
         return async () => {

integration-tests/src/utils.ts

@@ -21,8 +21,9 @@ export const joinUrl = function (url, part) {
 };
 
 export const newUser = async function(username, password, adminToken) {
-    await post('users/', {username: username, password: password, name: username, email: username + '@openconceptlab.org'}, adminToken);
+    const user = await post('users/', {username: username, password: password, name: username, email: username + '@openconceptlab.org'}, adminToken);
     await put('users/' + username + '/reactivate/', {username: username, password: password, name: username, email: username + '@openconceptlab.org'}, adminToken);
+    return await user.json();
 };
 
 export const post = async function(url, body, token=null) {

ocl/collection/filters.py

@@ -1,15 +1,7 @@
 __author__ = 'snyaggarwal'
 
-from oclapi.filters import HaystackSearchFilter
+from oclapi.filters import ConceptContainerPermissionedSearchFilter
 
 
-class CollectionSearchFilter(HaystackSearchFilter):
-    def get_filters(self, request, view):
-        filters = super(CollectionSearchFilter, self).get_filters(request, view)
-        if view.parent_resource:
-            filters.update({'owner': view.parent_resource.mnemonic})
-            filters.update({'ownerType': view.parent_resource.resource_type()})
-        else:
-            filters.update({'public_can_view': True})
-
-        return filters
+class CollectionSearchFilter(ConceptContainerPermissionedSearchFilter):
+    pass

ocl/collection/views.py

@@ -1,6 +1,8 @@
 import logging
 from django.conf import settings
+from django.contrib.contenttypes.models import ContentType
 from django.db import IntegrityError
+from django.db.models import Q
 
 from collection.validation_messages import HEAD_OF_CONCEPT_ADDED_TO_COLLECTION, CONCEPT_ADDED_TO_COLLECTION_FMT, \
     HEAD_OF_MAPPING_ADDED_TO_COLLECTION, MAPPING_ADDED_TO_COLLECTION_FMT
@@ -9,32 +11,25 @@
     CollectionVersionListSerializer, CollectionVersionCreateSerializer, CollectionVersionDetailSerializer, \
     CollectionVersionUpdateSerializer, \
     CollectionReferenceSerializer
-from concepts.models import Concept, ConceptVersion
-from mappings.models import MappingVersion
-from sources.models import SourceVersion
-from concepts.serializers import ConceptListSerializer
+from concepts.models import Concept
 from django.http import HttpResponse, HttpResponseForbidden
-from mappings.models import Mapping
-from mappings.serializers import MappingDetailSerializer
 from oclapi.mixins import ListWithHeadersMixin
 from oclapi.permissions import CanViewConceptDictionary, CanEditConceptDictionary, CanViewConceptDictionaryVersion, \
     CanEditConceptDictionaryVersion, HasOwnership
 from oclapi.permissions import HasAccessToVersionedObject
 from oclapi.views import ResourceVersionMixin, ResourceAttributeChildMixin, ConceptDictionaryUpdateMixin, \
-    ConceptDictionaryCreateMixin, ConceptDictionaryExtrasView, ConceptDictionaryExtraRetrieveUpdateDestroyView, \
-    BaseAPIView
-from oclapi.models import ACCESS_TYPE_EDIT, ACCESS_TYPE_VIEW
+    ConceptDictionaryCreateMixin, ConceptDictionaryExtrasView, ConceptDictionaryExtraRetrieveUpdateDestroyView
+from oclapi.models import ACCESS_TYPE_NONE
 from rest_framework import mixins, status
-from rest_framework.generics import RetrieveAPIView, UpdateAPIView, get_object_or_404, DestroyAPIView
+from rest_framework.generics import RetrieveAPIView, UpdateAPIView, DestroyAPIView
 from rest_framework.response import Response
 from users.models import UserProfile
 from orgs.models import Organization
 from tasks import export_collection, add_references
 from celery_once import AlreadyQueued
 from django.shortcuts import get_list_or_404
 from collection.filters import CollectionSearchFilter
-from tasks import update_collection_in_solr, delete_resources_from_collection_in_solr
-from django.core.exceptions import ValidationError
+from tasks import delete_resources_from_collection_in_solr
 
 logger = logging.getLogger('oclapi')
 
@@ -53,22 +48,19 @@ def get_version_detail_serializer(self, obj, data=None, files=None, partial=Fals
 
     def get_queryset(self):
         owner = self.get_owner()
-        if not self.kwargs:
+        if not owner:
             return self.queryset
         elif 'collection' in self.kwargs:
-            return Collection.objects.filter(parent_id=owner.id, mnemonic=self.kwargs['collection'])
+            return self.model.objects.filter(
+                parent_id=owner.id,
+                mnemonic=self.kwargs['collection'],
+                parent_type=ContentType.objects.get_for_model(UserProfile),
+            )
         else:
-            return self.queryset.filter(parent_id=owner.id)
+            return self.queryset.filter(parent_id=owner.id, parent_type=ContentType.objects.get_for_model(UserProfile))
 
     def get_owner(self):
-        owner = None
-        if 'user' in self.kwargs:
-            owner_id = self.kwargs['user']
-            owner = UserProfile.objects.get(mnemonic=owner_id)
-        elif 'org' in self.kwargs:
-            owner_id = self.kwargs['org']
-            owner = Organization.objects.get(mnemonic=owner_id)
-        return owner
+        return self.parent_resource
 
 
 class CollectionVersionBaseView(ResourceVersionMixin):
@@ -291,31 +283,14 @@ def get(self, request, *args, **kwargs):
         self.serializer_class = CollectionDetailSerializer if self.is_verbose(request) else CollectionListSerializer
         self.contains_uri = request.QUERY_PARAMS.get('contains', None)
         self.user = request.QUERY_PARAMS.get('user', None)
-        # Running the filter_backends seems to reset changes made to the queryset.
-        # Therefore, remove the filter_backends when the 'contains' parameter is passed, and
-        # apply the appropriate public_access filter in get_queryset
-        # TODO correct the behavior of filter_backends, and remove this hack to get around it
-        if self.contains_uri != None:
-            self.filter_backends=[]
-        if self.user != None:
-            self.filter_backends=[]
         collection_list = self.list(request, *args, **kwargs)
         return collection_list
 
     def get_queryset(self):
         queryset = super(CollectionListView, self).get_queryset()
-        # If the 'contains' parameter is used, the filter_backends have been cleared to prevent
-        # reset of the queryset.  Therefore, add a public_access filter to the queryset.
-        # TODO correct the behavior of filter_backends, and remove this hack to get around it
-        if self.contains_uri != None:
+        if self.contains_uri is not None:
             from django_mongodb_engine.query import A
-            queryset = queryset.filter(references=A('expression', self.contains_uri), public_access__in=[ACCESS_TYPE_EDIT, ACCESS_TYPE_VIEW])
-        if self.user:
-            if self.user != 'root':
-                from users.models import UserProfile
-                user_profile = UserProfile.objects.filter(mnemonic=self.user)
-                if user_profile:
-                    queryset = queryset.filter(parent_id__in=[user_profile[0].id] + user_profile[0].organizations, public_access__in=[ACCESS_TYPE_EDIT, ACCESS_TYPE_VIEW])
+            queryset = queryset.filter(references=A('expression', self.contains_uri))
         return queryset
 
     def get_csv_rows(self, queryset=None):

ocl/concepts/views.py

@@ -1,11 +1,10 @@
-from django.core.exceptions import ValidationError
 from django.db.models import Q
 from django.db.models.query import EmptyQuerySet
 from django.http import Http404
 from django.views.decorators.csrf import csrf_exempt
 from rest_framework import mixins, status
 from rest_framework.generics import (RetrieveAPIView, get_object_or_404, UpdateAPIView,
-                                     DestroyAPIView, RetrieveUpdateDestroyAPIView, CreateAPIView,
+                                     DestroyAPIView, RetrieveUpdateDestroyAPIView,
                                      ListCreateAPIView, ListAPIView)
 from rest_framework.response import Response
 from concepts.filters import LimitSourceVersionFilter, PublicConceptsSearchFilter, LimitCollectionVersionFilter

ocl/oclapi/filters.py

@@ -1,8 +1,14 @@
 from django.conf import settings
+from django.contrib.contenttypes.models import ContentType
+from django.db.models import Q
 from haystack.inputs import Raw
-from haystack.query import RelatedSearchQuerySet, SearchQuerySet
+from haystack.query import RelatedSearchQuerySet
 from rest_framework.filters import BaseFilterBackend
 
+from oclapi.models import ACCESS_TYPE_NONE
+from orgs.models import Organization
+from users.models import UserProfile
+
 
 class SearchQuerySetWrapper(object):
 
@@ -128,7 +134,7 @@ def _filter_queryset(self, request, queryset, view, sqs):
                     sqs = sqs.order_by(default_sort)
             sqs = sqs.models(view.model)
             if hasattr(sqs, 'load_all_queryset'):
-                sqs = sqs.load_all_queryset(view.model, queryset)
+                sqs = sqs.load_all().load_all_queryset(view.model, queryset)
             return SearchQuerySetWrapper(sqs)
 
         if hasattr(view, 'default_order_by'):
@@ -138,4 +144,27 @@ def _filter_queryset(self, request, queryset, view, sqs):
 
 class HaystackSearchFilter(BaseHaystackSearchFilter):
     def filter_queryset(self, request, queryset, view):
-        return self._filter_queryset(request, queryset, view, SearchQuerySet())
+        return self._filter_queryset(request, queryset, view, RelatedSearchQuerySet())
+
+
+class ConceptContainerPermissionedSearchFilter(HaystackSearchFilter):
+    def filter_queryset(self, request, queryset, view):
+        current_user = request.user
+        print queryset.query, queryset, 'original qs \n\n'
+
+        permissioned_qs = queryset.exclude(public_access=ACCESS_TYPE_NONE)
+        print permissioned_qs.query, permissioned_qs, 'qs from public access \n\n'
+
+        if not current_user.is_anonymous():
+            user_profile = UserProfile.objects.get(user=current_user)
+            private_collections = queryset.filter(
+                Q(parent_id=user_profile.id, parent_type=ContentType.objects.get_for_model(UserProfile)) |
+                Q(parent_id__in=user_profile.organizations, parent_type=ContentType.objects.get_for_model(Organization))
+            )
+            print private_collections.query, private_collections, 'qs from permissions \n\n'
+
+            permissioned_qs |= private_collections
+
+            print permissioned_qs.query, permissioned_qs, 'qs from both', '\n'
+
+        return super(ConceptContainerPermissionedSearchFilter, self).filter_queryset(request, permissioned_qs, view)

ocl/sources/filters.py

@@ -1,16 +1,7 @@
-__author__ = 'misternando'
-
-from oclapi.filters import HaystackSearchFilter
+from oclapi.filters import ConceptContainerPermissionedSearchFilter
 
 __author__ = 'misternando'
 
 
-class SourceSearchFilter(HaystackSearchFilter):
-    def get_filters(self, request, view):
-        filters = super(SourceSearchFilter, self).get_filters(request, view)
-        if view.parent_resource:
-            filters.update({'owner': view.parent_resource.mnemonic})
-            filters.update({'ownerType': view.parent_resource.resource_type()})
-        else:
-            filters.update({'public_can_view': True})
-        return filters
+class SourceSearchFilter(ConceptContainerPermissionedSearchFilter):
+    pass