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,32 @@
+import {put, get, post} from "./utils"
+import {VerboseContainer} from "./types";
+
+const common = {
+    container: {
+        list: type =>
+                async (ownerUrl: string, token: string=null, verbose: boolean=true, otherQueryParams: string=''): Promise<VerboseContainer[]> =>
+                    (await get(`${ownerUrl}${type}/?verbose=${verbose}${otherQueryParams}`, token)).json(),
+        new: type =>
+                async (ownerUrl: string, body:object, token:string): Promise<VerboseContainer[]> =>
+                    (await post(`${ownerUrl}${type}/`, body, token)).json(),
+    },
+};
+
+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: common.container.list('collections'),
+        new: common.container.new('collections'),
+    },
+    sources: {
+        list: common.container.list('sources'),
+        new: common.container.new('sources'),
+    },
+};
+
+export default api;

integration-tests/src/container.spec.ts

@@ -0,0 +1,163 @@
+import * as faker from 'faker';
+import {authenticate, authenticateAdmin, newUser, del} from "./utils";
+import containerFixture from "./fixtures/container";
+import api from "./api";
+
+const sortCriteria = (container1, container2) => {
+    if (container1.id < container2.id) return -1;
+    else if(container1.id > container2.id) return 1;
+    else return 0;
+};
+
+const cleanupAnyPublicContainers = async (adminToken, containerType) => {
+    const containers = await api[containerType].list('/');
+    for(let i in containers) await del(containers[i].url, adminToken);
+};
+
+describe.each([
+    'sources',
+    'collections',
+])('View Authorization Tests: %s', container => {
+    let adminToken;
+    let user1InOrg1;
+    let user2InOrg1;
+    let user3NotInOrg1;
+    let org1;
+    let privateContainerOwnedByUser1;
+    let privateContainerOwnedByOrg1;
+    let publicContainerOwnedByUser1;
+    let publicContainerOwnedByOrg1;
+
+    beforeAll(async () => {
+        adminToken = await authenticateAdmin();
+        // in case an afterAll is not executed, public containers would not be deleted
+        // and would interfere with the public access tests since they are not namespaced
+        await cleanupAnyPublicContainers(adminToken, container);
+
+        // 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);
+
+        privateContainerOwnedByUser1 = await api[container].new(user1InOrg1.url, containerFixture(), user1InOrg1.token);
+        privateContainerOwnedByOrg1 = await api[container].new(org1.url, containerFixture(), user1InOrg1.token);
+        publicContainerOwnedByUser1 = await api[container].new(user1InOrg1.url, containerFixture('View'), user1InOrg1.token);
+        publicContainerOwnedByOrg1 = await api[container].new(org1.url, containerFixture('View'), user1InOrg1.token);
+    });
+
+    afterAll(async () => {
+        const items = [
+            publicContainerOwnedByOrg1,
+            publicContainerOwnedByUser1,
+            privateContainerOwnedByOrg1,
+            privateContainerOwnedByUser1,
+            org1,
+            user3NotInOrg1,
+            user2InOrg1,
+            user1InOrg1,
+        ];
+        for(let i in items) await del(items[i].url, adminToken);
+    });
+
+    describe('logged in user', () => {
+        test(`can view own ${container}`, async () => {
+            const results = (await api[container].list(`${user1InOrg1.url}`, user1InOrg1.token)).sort(sortCriteria);
+            const expected = [publicContainerOwnedByUser1, privateContainerOwnedByUser1].sort(sortCriteria);
+            expect(results).toEqual(expected);
+        });
+
+        test(`can view their orgs ${container}`, async () => {
+            const results = (await api[container].list(`${org1.url}`, user2InOrg1.token)).sort(sortCriteria);
+            const expected = [publicContainerOwnedByOrg1, privateContainerOwnedByOrg1].sort(sortCriteria);
+            expect(results).toEqual(expected);
+        });
+
+        test(`can view another users public ${container}`, async () => {
+            const results = (await api[container].list(`${user1InOrg1.url}`, user2InOrg1.token)).sort(sortCriteria);
+            const expected = [publicContainerOwnedByUser1].sort(sortCriteria);
+            expect(results).toEqual(expected);
+        });
+
+        test(`can view another orgs public ${container}`, async () => {
+            const results = (await api[container].list(`${org1.url}`, user3NotInOrg1.token)).sort(sortCriteria);
+            const expected = [publicContainerOwnedByOrg1].sort(sortCriteria);
+            expect(results).toEqual(expected);
+        });
+
+        test(`cannot view another users private ${container}`, async () => {
+            const results = await api[container].list(`${user1InOrg1.url}`, user2InOrg1.token);
+            expect(results).not.toContain(privateContainerOwnedByUser1);
+        });
+
+        test(`cannot view another orgs private ${container}`, async () => {
+            const results = await api[container].list(`${org1.url}`, user3NotInOrg1.token);
+            expect(results).not.toContain(privateContainerOwnedByOrg1);
+        });
+    });
+
+    describe('not logged in user', () => {
+        test(`can view another users public ${container}`, async () => {
+            const results = (await api[container].list(`${user1InOrg1.url}`)).sort(sortCriteria);
+            const expected = [publicContainerOwnedByUser1].sort(sortCriteria);
+            expect(results).toEqual(expected);
+        });
+
+        test(`can view another orgs public ${container}`, async () => {
+            const results = (await api[container].list(`${org1.url}`)).sort(sortCriteria);
+            const expected = [publicContainerOwnedByOrg1].sort(sortCriteria);
+            expect(results).toEqual(expected);
+        });
+
+        test(`cannot view another users private ${container}`, async () => {
+            const results = await api[container].list(`${user1InOrg1.url}`, user2InOrg1.token);
+            expect(results).not.toContain(privateContainerOwnedByUser1);
+        });
+
+        test(`cannot view another orgs private ${container}`, async () => {
+            const results = await api[container].list(`${org1.url}`);
+            expect(results).not.toContain(privateContainerOwnedByOrg1);
+        });
+    });
+
+    describe(`view all public ${container}`, () => {
+        test('logged in user', async () => {
+            const expected = [publicContainerOwnedByUser1, publicContainerOwnedByOrg1].sort(sortCriteria);
+
+            let results = (await api[container].list('/', user1InOrg1.token)).sort(sortCriteria);
+            expect(results).toEqual(expected);
+
+            results = (await api[container].list('/', user2InOrg1.token)).sort(sortCriteria);
+            expect(results).toEqual(expected);
+
+            results = (await api[container].list('/', user3NotInOrg1.token)).sort(sortCriteria);
+            expect(results).toEqual(expected);
+        });
+
+        test('not logged in user', async () => {
+            const expected = [publicContainerOwnedByUser1, publicContainerOwnedByOrg1].sort(sortCriteria);
+            const results = (await api[container].list('/')).sort(sortCriteria);
+            expect(results).toEqual(expected);
+        });
+    });
+
+    describe('query param tests', () => {
+        test('using query params does not reset the queryset', async () => {
+            const expected = [publicContainerOwnedByUser1, publicContainerOwnedByOrg1].sort(sortCriteria);
+            const results = (await api[container].list('/', undefined, undefined, '&customValidationSchema=OpenMRS')).sort(sortCriteria);
+            expect(results).toEqual(expected);
+        });
+    });
+});

integration-tests/src/fixtures/container.ts

@@ -0,0 +1,22 @@
+import * as faker from 'faker';
+
+const container = (publicAccess = 'None', customValidationSchema='OpenMRS') => {
+    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: {},
+        custom_validation_schema: customValidationSchema,
+    }
+};
+
+export default container;

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/types.ts

@@ -0,0 +1,20 @@
+export interface VerboseContainer {
+    type: string,
+    uuid: string,
+    id: string,
+    external_id: string,
+    short_code: string,
+    name: string,
+    full_name: string,
+    collection_type: string,
+    public_access: string,
+    supported_locales: string[],
+    website: string,
+    description: string,
+    extras: object,
+    url: string,
+    active_concepts: number,
+    active_mappings: number,
+    concepts_url: string,
+    created_by: string,
+}

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