From 649e7317b495ace7b4cda436f3427b9d09a9455b Mon Sep 17 00:00:00 2001 From: Benoit Devos Date: Mon, 10 Jan 2022 17:36:16 +0100 Subject: [PATCH] fix: enforce that loc-related resources are called by node owner. logion-network/logion-internal#316 --- .../controllers/locrequest.controller.ts | 28 +++++++++---------- test/helpers/testapp.ts | 8 ++++-- .../controllers/locrequest.controller.spec.ts | 24 ++++++++++++++++ 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/logion/controllers/locrequest.controller.ts b/src/logion/controllers/locrequest.controller.ts index 92ee6527..7680e267 100644 --- a/src/logion/controllers/locrequest.controller.ts +++ b/src/logion/controllers/locrequest.controller.ts @@ -351,7 +351,7 @@ export class LocRequestController extends ApiController { async rejectLocRequest(rejectLocRequestView: RejectLocRequestView, requestId: string) { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); request.reject(rejectLocRequestView.rejectReason!, moment()); await this.locRequestRepository.save(request) } @@ -369,7 +369,7 @@ export class LocRequestController extends ApiController { async acceptLocRequest(_ignoredBody: any, requestId: string) { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); request.accept(moment()); await this.locRequestRepository.save(request) } @@ -387,7 +387,7 @@ export class LocRequestController extends ApiController { async addFile(addFileView: AddFileView, requestId: string): Promise { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); const files: fileUpload.FileArray = this.request.files; if(files === undefined || files === null) { @@ -434,7 +434,7 @@ export class LocRequestController extends ApiController { async downloadFile(_body: any, requestId: string, hash: string): Promise { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); const file = request.getFile(hash); const tempFilePath = "/tmp/download-" + requestId + "-" + hash; @@ -461,7 +461,7 @@ export class LocRequestController extends ApiController { async deleteFile(_body: any, requestId: string, hash: string): Promise { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); const file = request.removeFile(hash); await this.locRequestRepository.save(request); @@ -484,7 +484,7 @@ export class LocRequestController extends ApiController { async confirmFile(_body: any, requestId: string, hash: string) { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); request.confirmFile(hash); await this.locRequestRepository.save(request); @@ -506,7 +506,7 @@ export class LocRequestController extends ApiController { async closeLoc(_body: any, requestId: string) { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); request.preClose(); await this.locRequestRepository.save(request); @@ -532,7 +532,7 @@ export class LocRequestController extends ApiController { async voidLoc(body: VoidLocView, requestId: string) { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); request.preVoid(body.reason!); await this.locRequestRepository.save(request); @@ -554,7 +554,7 @@ export class LocRequestController extends ApiController { async addLink(addLinkView: AddLinkView, requestId: string): Promise { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); const targetRequest = requireDefined(await this.locRequestRepository.findById(addLinkView.target!)); request.addLink({ target: targetRequest.id!, @@ -578,7 +578,7 @@ export class LocRequestController extends ApiController { async deleteLink(_body: any, requestId: string, target: string): Promise { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); request.removeLink(target); await this.locRequestRepository.save(request); @@ -599,7 +599,7 @@ export class LocRequestController extends ApiController { async confirmLink(_body: any, requestId: string, target: string) { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); request.confirmLink(target); await this.locRequestRepository.save(request); @@ -621,7 +621,7 @@ export class LocRequestController extends ApiController { async addMetadata(addMetadataView: AddMetadataView, requestId: string): Promise { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); const name = requireLength(addMetadataView, "name", 3, 40) const value = requireLength(addMetadataView, "value", 1, 4096) request.addMetadataItem({ name, value }) @@ -643,7 +643,7 @@ export class LocRequestController extends ApiController { async deleteMetadata(_body: any, requestId: string, name: string): Promise { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); const decodedName = decodeURIComponent(name); request.removeMetadataItem(decodedName); @@ -665,7 +665,7 @@ export class LocRequestController extends ApiController { async confirmMetadata(_body: any, requestId: string, name: string) { const request = requireDefined(await this.locRequestRepository.findById(requestId)); this.authenticationService.authenticatedUser(this.request) - .is(request.ownerAddress); + .requireNodeOwner(); const decodedName = decodeURIComponent(name); request.confirmMetadataItem(decodedName); diff --git a/test/helpers/testapp.ts b/test/helpers/testapp.ts index 73a8598f..5446f854 100644 --- a/test/helpers/testapp.ts +++ b/test/helpers/testapp.ts @@ -15,7 +15,7 @@ export function setupApp( controller: Function & { prototype: T; }, mockBinder: (container: Container) => void, authSucceed: boolean = true, - isNodeOwner: boolean = false, + isNodeOwner: boolean = true, ): Express { const app = express(); @@ -58,7 +58,11 @@ function mockAuthenticationSuccess(isNodeOwner: boolean): AuthenticationService authenticatedUser.setup(instance => instance.require).returns(() => {}); authenticatedUser.setup(instance => instance.requireIs).returns(() => {}); authenticatedUser.setup(instance => instance.requireLegalOfficer).returns(() => {}); - authenticatedUser.setup(instance => instance.requireNodeOwner).returns(() => {}); + authenticatedUser.setup(instance => instance.requireNodeOwner).returns(() => { + if (!isNodeOwner) { + throw new UnauthorizedException("") + } + }); authenticatedUser.setup(instance => instance.isNodeOwner).returns(() => isNodeOwner); const authenticationService = new Mock(); diff --git a/test/unit/controllers/locrequest.controller.spec.ts b/test/unit/controllers/locrequest.controller.spec.ts index bf828c9f..31362fd6 100644 --- a/test/unit/controllers/locrequest.controller.spec.ts +++ b/test/unit/controllers/locrequest.controller.spec.ts @@ -93,6 +93,7 @@ describe('LocRequestController', () => { LocRequestController, mockModelForCreation, true, + false ) await request(app) .post('/api/loc-request') @@ -190,6 +191,7 @@ describe('LocRequestController', () => { LocRequestController, container => mockModelForCreation(container, true), true, + false ) await request(app) .post('/api/loc-request') @@ -318,6 +320,19 @@ describe('LocRequestController', () => { }); }) + it('fails to add file to loc when not owner', async () => { + const app = setupApp(LocRequestController, mockModelForAddFile, true, false); + const buffer = Buffer.from(SOME_DATA); + await request(app) + .post(`/api/loc-request/${ REQUEST_ID }/files`) + .field({ nature: "some nature" }) + .attach('file', buffer, { + filename: FILE_NAME, + contentType: 'text/plain', + }) + .expect(401); + }) + it('downloads existing file given its hash', async () => { const app = setupApp(LocRequestController, mockModelForDownloadFile); const filePath = "/tmp/download-" + REQUEST_ID + "-" + SOME_DATA_HASH; @@ -387,6 +402,15 @@ describe('LocRequestController', () => { It.Is(item => item.name == SOME_DATA_NAME && item.value == SOME_DATA_VALUE))) }) + it('fails to adds a metadata item when not owner', async () => { + const locRequest = mockRequestForMetadata(); + const app = setupApp(LocRequestController, (container) => mockModelForAllItems(container, locRequest), true, false) + await request(app) + .post(`/api/loc-request/${ REQUEST_ID }/metadata`) + .send({ name: SOME_DATA_NAME, value: SOME_DATA_VALUE }) + .expect(401) + }) + it('deletes a metadata item', async () => { const locRequest = mockRequestForMetadata(); const app = setupApp(LocRequestController, (container) => mockModelForAllItems(container, locRequest))