diff --git a/sci-log-db/src/__tests__/acceptance/basesnippet.controller.acceptance.ts b/sci-log-db/src/__tests__/acceptance/basesnippet.controller.acceptance.ts index af6b7dc7..6b4534b4 100644 --- a/sci-log-db/src/__tests__/acceptance/basesnippet.controller.acceptance.ts +++ b/sci-log-db/src/__tests__/acceptance/basesnippet.controller.acceptance.ts @@ -423,6 +423,15 @@ describe('Basesnippet', function (this: Suite) { .expect(204); }); + it('patch snippet by id with ownergroup and token should return 404', async () => { + await client + .patch(`/basesnippets/${baseSnippetId}`) + .set('Authorization', 'Bearer ' + token) + .set('Content-Type', 'application/json') + .send({createACL: ['aNonAllowedACL']}) + .expect(404); + }); + it('post a basesnippet with authentication and parentId from existing snippet should return 200 and have ownergroup with priority on parent ACLS', async () => { await client .post('/basesnippets') @@ -673,8 +682,23 @@ describe('Basesnippet', function (this: Suite) { ownerGroup: 'basesnippetAcceptance', accessGroups: ['someNew'], }, + expected: 204, + }, + { + input: {deleteACL: ['basesnippetAcceptance', 'someNew']}, expected: 404, }, + { + input: { + deleteACL: ['basesnippetAcceptance', 'someNew'], + token: 'adminToken', + }, + expected: 204, + }, + { + input: {readACL: ['basesnippetAcceptance', 'someNew']}, + expected: 204, + }, { input: { accessGroups: ['someNew'], @@ -682,10 +706,11 @@ describe('Basesnippet', function (this: Suite) { expected: 403, }, ].forEach((t, i) => { - it(`patch snippet by id changing ownerGroup should return error ${i}`, async () => { + it(`patch snippet by id changing ownerGroup should return ${i}`, async () => { + const blockToken = t.input.token === 'adminToken' ? adminToken : token; await client .patch(`/basesnippets/${baseSnippetId}`) - .set('Authorization', 'Bearer ' + token) + .set('Authorization', 'Bearer ' + blockToken) .set('Content-Type', 'application/json') .send({ tags: ['aSearchExcludedTag'], @@ -707,4 +732,24 @@ describe('Basesnippet', function (this: Suite) { }); }); }); + + it(`patch snippet by id with non-authorised user should return 404`, async () => { + const bs = await client + .post('/basesnippets') + .set('Authorization', 'Bearer ' + token) + .set('Content-Type', 'application/json') + .send({ + ..._.omit(baseSnippet, 'updateACL'), + updateACL: ['nonAuthorised'], + }); + await client + .patch(`/basesnippets/${bs.body.id}`) + .set('Authorization', 'Bearer ' + token) + .set('Content-Type', 'application/json') + .send({name: 'something'}) + .expect(404) + .catch(err => { + throw err; + }); + }); }); diff --git a/sci-log-db/src/__tests__/acceptance/file.controller.acceptance.ts b/sci-log-db/src/__tests__/acceptance/file.controller.acceptance.ts index c832f6d3..20f2fcc8 100644 --- a/sci-log-db/src/__tests__/acceptance/file.controller.acceptance.ts +++ b/sci-log-db/src/__tests__/acceptance/file.controller.acceptance.ts @@ -367,7 +367,7 @@ describe('File controller services', function (this: Suite) { .post('/filesnippet/files') .set('Authorization', 'Bearer ' + token) .type('form') - .field('fields', '{"readACL": ["any-authenticated-user"]}') + .field('fields', '{"updateACL": ["any-authenticated-user"]}') .attach('file', __filename) .then() .catch(err => { @@ -379,8 +379,8 @@ describe('File controller services', function (this: Suite) { .set('Authorization', 'Bearer ' + token) .field('fields', '{"description": "something"}') .attach('file', __filename) - .expect(204) - .then() + .expect(404) + .then(result => result) .catch(err => { throw err; }); diff --git a/sci-log-db/src/__tests__/acceptance/logbook.controller.acceptance.ts b/sci-log-db/src/__tests__/acceptance/logbook.controller.acceptance.ts index 9a06f3b0..33b6d3b1 100644 --- a/sci-log-db/src/__tests__/acceptance/logbook.controller.acceptance.ts +++ b/sci-log-db/src/__tests__/acceptance/logbook.controller.acceptance.ts @@ -15,6 +15,7 @@ describe('Logbook', function (this: Suite) { let app: SciLogDbApplication; let client: Client; let token: string; + let adminToken: string; let logbookSnippetId: string; const logbookSnippet = { ownerGroup: 'logbookAcceptance', @@ -31,7 +32,7 @@ describe('Logbook', function (this: Suite) { before('setupApplication', async () => { ({app, client} = await setupApplication()); await clearDatabase(app); - await createAdminToken(app, client); + adminToken = await createAdminToken(app, client); token = await createUserToken(app, client, ['logbookAcceptance']); }); @@ -241,6 +242,146 @@ describe('Logbook', function (this: Suite) { }); }); + [ + { + input: { + readACL: ['logbookAcceptance', 'toPropagate'], + }, + output: [ + ['logbookAcceptance', 'toPropagate'], + ['logbookAcceptance', 'toPropagate'], + ], + }, + { + input: { + ownerGroup: 'logbookAcceptance', + accessGroups: ['accessGroupPropagated'], + }, + output: [ + ['logbookAcceptance', 'accessGroupPropagated'], + ['logbookAcceptance', 'accessGroupPropagated', 'toPropagate'], + ], + }, + ].forEach(t => + it(`patch logbook with children and grand children by id should modify all with ${t.output[1]}`, async () => { + const child = await client + .post(`/paragraphs`) + .set('Authorization', 'Bearer ' + token) + .set('Content-Type', 'application/json') + .send({ + ..._.omit(logbookSnippet, 'location'), + parentId: logbookSnippetId, + }); + + const grandChild = await client + .post(`/paragraphs`) + .set('Authorization', 'Bearer ' + token) + .set('Content-Type', 'application/json') + .send({ + ..._.omit(logbookSnippet, 'location'), + parentId: child.body.id, + }); + + await client + .patch(`/logbooks/${logbookSnippetId}`) + .set('Authorization', 'Bearer ' + token) + .set('Content-Type', 'application/json') + .send(t.input) + .expect(204); + + const filter = { + where: { + id: {inq: [logbookSnippetId, child.body.id, grandChild.body.id]}, + }, + }; + await client + .get(`/basesnippets?filter=${JSON.stringify(filter)}`) + .set('Authorization', 'Bearer ' + token) + .set('Content-Type', 'application/json') + .then(result => + result.body.map((r: {readACL: string[]}, i: number) => { + if (i === 0) expect(r.readACL).to.be.eql(t.output[0]); + else expect(r.readACL).to.be.eql(t.output[1]); + }), + ); + }), + ); + + [ + { + input: { + ownerGroup: 'logbookAcceptance', + accessGroups: ['someNew'], + }, + expected: 204, + }, + { + input: { + ownerGroup: 'notAllowedForNonAdmin', + accessGroups: ['someNew'], + }, + expected: 404, + }, + { + input: {readACL: ['logbookAcceptance', 'someNew1']}, + expected: 204, + }, + { + input: { + ownerGroup: 'notAllowedForNonAdmin', + accessGroups: ['someNew', 'logbookAcceptance'], + token: 'adminToken', + }, + expected: 204, + }, + { + input: { + ownerGroup: 'logbookAcceptance', + accessGroups: ['someNew', 'logbookAcceptance'], + token: 'adminToken', + }, + expected: 204, + }, + { + input: { + deleteACL: ['notAllowedForNonAdmin', 'someNew'], + token: 'adminToken', + }, + expected: 204, + }, + { + input: {deleteACL: ['someOtherNotAllowedForNonAdmin', 'someNew']}, + expected: 404, + }, + { + input: { + accessGroups: ['someNew'], + }, + expected: 403, + }, + ].forEach((t, i) => { + it(`patch logbook by id changing ownerGroup should return ${i}`, async () => { + const blockToken = t.input.token === 'adminToken' ? adminToken : token; + await client + .patch(`/logbooks/${logbookSnippetId}`) + .set('Authorization', 'Bearer ' + blockToken) + .set('Content-Type', 'application/json') + .send({ + ...t.input, + }) + .expect(t.expected) + .then(result => { + if (t.expected === 403) + expect(result.body.error.message).to.be.eql( + 'Cannot modify data snippet. Please provide both ownerGroup and accessGroup', + ); + }) + .catch(err => { + throw err; + }); + }); + }); + it('delete snippet by id without token should return 401', async () => { await client .delete(`/logbooks/${logbookSnippetId}`) diff --git a/sci-log-db/src/__tests__/unit/utils.misc.unit.ts b/sci-log-db/src/__tests__/unit/utils.misc.unit.ts index 324fa6f8..e61a292a 100644 --- a/sci-log-db/src/__tests__/unit/utils.misc.unit.ts +++ b/sci-log-db/src/__tests__/unit/utils.misc.unit.ts @@ -7,7 +7,7 @@ import { getModelSchemaRef, validateFieldsVSModel, defaultSequentially, - addReadACLFromOwnerAccessGroups, + concatOwnerAccessGroups, } from '../../utils/misc'; describe('Utils unit tests', function (this: Suite) { @@ -98,7 +98,7 @@ describe('Utils unit tests', function (this: Suite) { ownerGroup: 'a', accessGroups: ['b'], }, - expected: {readACL: ['a', 'b']}, + expected: {ownerGroup: 'a', accessGroups: ['a', 'b']}, }, { input: {}, @@ -110,7 +110,7 @@ describe('Utils unit tests', function (this: Suite) { }, { input: {readACL: ['a'], ownerGroup: 'b'}, - expected: {readACL: ['a']}, + expected: {readACL: ['a'], ownerGroup: 'b'}, }, { input: {ownerGroup: 'b'}, @@ -121,15 +121,15 @@ describe('Utils unit tests', function (this: Suite) { expected: 'error', }, ].forEach((t, i) => { - it(`Should test addReadACLFromOwnerAccessGroups ${i}`, () => { + it(`Should test concatOwnerAccessGroups ${i}`, () => { if (t.expected === 'error') try { - addReadACLFromOwnerAccessGroups(t.input); + concatOwnerAccessGroups(t.input); } catch (err) { expect(err.name).to.be.eql('ForbiddenError'); } else { - addReadACLFromOwnerAccessGroups(t.input); + concatOwnerAccessGroups(t.input); expect(t.input).to.be.eql(t.expected); } }); diff --git a/sci-log-db/src/mixins/basesnippet.repository-mixin.ts b/sci-log-db/src/mixins/basesnippet.repository-mixin.ts index b4170271..17c0c1fc 100644 --- a/sci-log-db/src/mixins/basesnippet.repository-mixin.ts +++ b/sci-log-db/src/mixins/basesnippet.repository-mixin.ts @@ -18,9 +18,15 @@ import {HttpErrors, Response} from '@loopback/rest'; import _ from 'lodash'; import {EXPORT_SERVICE} from '../keys'; import {ExportService} from '../services/export-snippets.service'; -import {addReadACLFromOwnerAccessGroups} from '../utils/misc'; +import {concatOwnerAccessGroups} from '../utils/misc'; +import {AutoAddRepository} from '../repositories/autoadd.repository.base'; const fs = require('fs'); +type ExpandedBasesnippet = Basesnippet & { + ownerGroup: string; + accessGroups: string[]; +}; + function UpdateAndDeleteRepositoryMixin< M extends Entity & { expiresAt: Date; @@ -37,46 +43,117 @@ function UpdateAndDeleteRepositoryMixin< class Mixed extends superClass { readonly baseSnippetRepository: Function; + get baseACLS() { + const self = this as unknown as AutoAddRepository; + return self.acls; + } + + get expandedACLS() { + return [...this.baseACLS, 'ownerGroup', 'accessGroups'] as const; + } + async updateByIdWithHistory( id: ID, - basesnippet: Basesnippet & {ownerGroup: string; accessGroups: string[]}, + basesnippet: ExpandedBasesnippet, options?: Options, ): Promise { - const snippet = await this.findById(id, {}, options); - const patches = this.getChanged(basesnippet, snippet); - if ( - typeof basesnippet.deleted == 'undefined' || - basesnippet.deleted === false - ) { + const baseSnippetRepository = await this.baseSnippetRepository(); + const snippet = await baseSnippetRepository.findById(id, {}, options); + const patches = await this.applyFromOwnerAccessAndGetChanged( + basesnippet, + snippet, + ); + if (!basesnippet.deleted) { if ( !this.isSharing(patches) && - ((typeof snippet?.expiresAt != 'undefined' && - snippet.expiresAt.getTime() < Date.now()) || - typeof snippet?.expiresAt == 'undefined') + (snippet.expiresAt?.getTime() < Date.now() || !snippet?.expiresAt) ) { throw new HttpErrors.Forbidden('Cannot modify expired data snippet.'); } - await this.updateById(id, patches, options); + await baseSnippetRepository.updateById(id, patches, options); if (snippet?.versionable) { await this.addToHistory(snippet, options?.currentUser); } - } else await this.updateById(id, patches, options); + if (this.isSharing(patches)) + await this.applyACLSToChildren( + { + ...patches, + id: snippet.id, + snippetType: snippet.snippetType, + } as Basesnippet, + options?.currentUser, + ); + } else await baseSnippetRepository.updateById(id, patches, options); } - private getChanged( - basesnippet: Basesnippet & {ownerGroup?: string; accessGroups?: string[]}, + private async applyACLSToChildren( + basesnippet: Basesnippet, + user: UserProfile, + ) { + if ( + !basesnippet || + basesnippet?.snippetType === 'location' || + !basesnippet.id + ) + return; + const currentUser = {currentUser: user}; + const baseSnippetRepository = await this.baseSnippetRepository(); + const children = await baseSnippetRepository.find( + {where: {parentId: basesnippet.id, snippetType: 'paragraph'}}, + currentUser, + ); + for (const child of children) { + await this.applyACLSToChild(basesnippet, child, currentUser); + } + } + + private async applyACLSToChild( + basesnippet: Basesnippet, + child: Basesnippet, + currentUser: {currentUser: UserProfile}, + ) { + const updateAcls = this.baseACLS.reduce((currentValue, previousValue) => { + if (basesnippet[previousValue]) + currentValue[previousValue] = [ + ...new Set([ + ...basesnippet[previousValue].concat(child[previousValue] ?? []), + ]), + ]; + return currentValue; + }, {} as ExpandedBasesnippet); + await this.updateByIdWithHistory(child.id as ID, updateAcls, currentUser); + } + + private async applyFromOwnerAccessAndGetChanged( + basesnippet: ExpandedBasesnippet, snippet: M & Relations, ) { - addReadACLFromOwnerAccessGroups(basesnippet); + const self = this as unknown as AutoAddRepository; + const merge: typeof basesnippet = { + ..._.omit(snippet, this.expandedACLS), + ...basesnippet, + }; + concatOwnerAccessGroups(merge); + if ( + ['ownerGroup', 'accessGroups'].some( + acl => basesnippet[acl as keyof typeof basesnippet], + ) + ) + await self['aclDefaultOnCreation'](merge); + return this.getChanged(merge, snippet); + } + + private getChanged(merge: ExpandedBasesnippet, snippet: M & Relations) { return _.pickBy( - basesnippet, + merge, (v, k: keyof M) => JSON.stringify(snippet[k]) !== JSON.stringify(v), ); } private isSharing(patches: Partial) { - const allowed = ['ownerGroup', 'accessGroups', 'readACL']; - return Object.keys(patches).every(d => allowed.includes(d)); + return Object.keys(patches).every(d => + this.expandedACLS.includes(d as typeof this.expandedACLS[number]), + ); } private async addToHistory(snippet: M, user: UserProfile): Promise { diff --git a/sci-log-db/src/repositories/autoadd.repository.base.ts b/sci-log-db/src/repositories/autoadd.repository.base.ts index 3f123dcf..e2b88f0b 100644 --- a/sci-log-db/src/repositories/autoadd.repository.base.ts +++ b/sci-log-db/src/repositories/autoadd.repository.base.ts @@ -12,12 +12,9 @@ import { repository, Where, } from '@loopback/repository'; -import {Location, Logbook} from '../models'; +import {Location, Logbook, User} from '../models'; import {Basesnippet} from '../models/basesnippet.model'; -import { - addReadACLFromOwnerAccessGroups, - defaultSequentially, -} from '../utils/misc'; +import {defaultSequentially} from '../utils/misc'; import {BasesnippetRepository} from './basesnippet.repository'; import _ from 'lodash'; import {HttpErrors} from '@loopback/rest'; @@ -32,15 +29,14 @@ export class AutoAddRepository< public readonly subsnippets: HasManyRepositoryFactory; - private readonly acls = [ + readonly acls = [ 'createACL', 'readACL', 'shareACL', 'updateACL', 'deleteACL', - 'shareACL', 'adminACL', - ]; + ] as const; @repository('UserRepository') readonly userRepository: UserRepository; @@ -83,7 +79,7 @@ export class AutoAddRepository< private async aclDefaultOnCreation( data: (Basesnippet | Logbook) & { ownerGroup?: string; - unsetAttribute: Function; + accessGroups?: string[]; }, ) { const emptyAcls = this.acls.filter(acl => !data[acl as keyof Basesnippet]); @@ -111,8 +107,8 @@ export class AutoAddRepository< _.partial(this.defaultAllButLocationLogbookACL.bind(this), parent), ); } - data.unsetAttribute('ownerGroup'); - data.unsetAttribute('accessGroups'); + delete data.ownerGroup; + delete data.accessGroups; } private async addToACLIfNotEmpty( @@ -135,7 +131,7 @@ export class AutoAddRepository< private async getParent( data: (Basesnippet | Logbook) & { ownerGroup?: string | undefined; - unsetAttribute: Function; + accessGroups?: string[]; }, ) { const parentId = @@ -315,20 +311,21 @@ export class AutoAddRepository< // console.log("PATCH case") ctx.data.updatedAt = new Date(); ctx.data.updatedBy = currentUser?.email ?? 'unknown@domain.org'; - addReadACLFromOwnerAccessGroups(ctx.data); // remove all auto generated fields delete ctx.data.createdAt; delete ctx.data.createdBy; delete ctx.data.expiresAt; - if (this.acls.some(acl => ctx.data[acl])) { - const adminCondition = { - adminACL: { - inq: [ - ...(ctx?.options?.currentUser?.roles ?? []), - ctx?.options?.currentUser?.email, - ], - }, - }; + if (currentUser?.roles?.includes('admin')) return; + const updateCondition = this.updateACLCondition( + currentUser, + 'updateACL', + ); + ctx.where = this.addACLToFilter(ctx.where, updateCondition); + if (this.acls.some(acl => acl !== 'readACL' && ctx.data[acl])) { + const adminCondition = this.updateACLCondition( + currentUser, + 'adminACL', + ); ctx.where = this.addACLToFilter(ctx.where, adminCondition); } } else { @@ -454,6 +451,14 @@ export class AutoAddRepository< return modelClass; } + private updateACLCondition(currentUser: User, acl: string) { + return { + [acl]: { + inq: [...(currentUser?.roles ?? []), currentUser?.email], + }, + }; + } + // eslint-disable-next-line @typescript-eslint/no-explicit-any private groupsToAclFilter(object: any) { if (!object) return; diff --git a/sci-log-db/src/utils/misc.ts b/sci-log-db/src/utils/misc.ts index 6253c399..7228ea62 100644 --- a/sci-log-db/src/utils/misc.ts +++ b/sci-log-db/src/utils/misc.ts @@ -143,7 +143,7 @@ export function defaultSequentially(...args: any[]) { }, undefined); } -export function addReadACLFromOwnerAccessGroups(data: { +export function concatOwnerAccessGroups(data: { ownerGroup?: string; accessGroups?: string[]; readACL?: string[]; @@ -154,12 +154,11 @@ export function addReadACLFromOwnerAccessGroups(data: { throw new HttpErrors.Forbidden( 'Cannot modify data snippet. Please provide both ownerGroup and accessGroup', ); - data.readACL = [ - ...new Set( - [data.ownerGroup as string].concat(data.accessGroups as string[]), - ), + data.accessGroups = [ + ...new Set([ + ...(data.ownerGroup ? [data.ownerGroup] : []), + ...(data.accessGroups ?? []), + ]), ]; } - delete data.ownerGroup; - delete data.accessGroups; }