diff --git a/src/ctypes/ctypes.module.spec.ts b/src/ctypes/ctypes.module.spec.ts index 0e22955..656ecb3 100644 --- a/src/ctypes/ctypes.module.spec.ts +++ b/src/ctypes/ctypes.module.spec.ts @@ -224,7 +224,7 @@ describe('CType Module', () => { .mockReturnValue({ exec: async () => [] as CTypeDB[] }) public static findOne = jest .fn() - .mockReturnValue({ exec: async () => Optional.ofNullable(null) }) + .mockReturnValue({ exec: async (): Promise => null }) public static deleteMany = jest.fn().mockReturnValue({ exec: async (): Promise => { return diff --git a/src/messaging/exceptions/message-forbidden.exception.ts b/src/messaging/exceptions/message-forbidden.exception.ts new file mode 100644 index 0000000..c91e40b --- /dev/null +++ b/src/messaging/exceptions/message-forbidden.exception.ts @@ -0,0 +1,11 @@ +import { HttpException } from '@nestjs/common/exceptions' +import { HttpStatus } from '@nestjs/common' + +export class ForbiddenMessageAccessException extends HttpException { + constructor() { + super( + 'Message owner signature could not be verified!', + HttpStatus.FORBIDDEN + ) + } +} diff --git a/src/messaging/exceptions/message-not-found.exception.ts b/src/messaging/exceptions/message-not-found.exception.ts new file mode 100644 index 0000000..a090331 --- /dev/null +++ b/src/messaging/exceptions/message-not-found.exception.ts @@ -0,0 +1,8 @@ +import { HttpException } from '@nestjs/common/exceptions' +import { HttpStatus } from '@nestjs/common' + +export class MessageNotFoundException extends HttpException { + constructor() { + super('Message requested not found', HttpStatus.NOT_FOUND) + } +} diff --git a/src/messaging/interfaces/messaging.interfaces.ts b/src/messaging/interfaces/messaging.interfaces.ts index 7e89063..c9332ce 100644 --- a/src/messaging/interfaces/messaging.interfaces.ts +++ b/src/messaging/interfaces/messaging.interfaces.ts @@ -1,12 +1,17 @@ import { Document } from 'mongoose' import { Contact } from '../../contacts/interfaces/contacts.interfaces' import { IEncryptedMessage } from '@kiltprotocol/sdk-js' +import Optional from 'typescript-optional' export interface MessageDB extends Document, IEncryptedMessage {} export declare interface MessagingService { add(message: IEncryptedMessage): Promise + findById( + messageId: IEncryptedMessage['messageId'] + ): Promise> + findBySenderAddress( senderAddress: Contact['publicIdentity']['address'] ): Promise diff --git a/src/messaging/messaging.controller.ts b/src/messaging/messaging.controller.ts index e5998c0..4b7d1f3 100644 --- a/src/messaging/messaging.controller.ts +++ b/src/messaging/messaging.controller.ts @@ -4,6 +4,7 @@ import { Get, Inject, Param, + Headers, Post, Delete, BadRequestException, @@ -12,6 +13,9 @@ import { import { MessagingService } from './interfaces/messaging.interfaces' import { IEncryptedMessage } from '@kiltprotocol/sdk-js' import { AuthGuard } from '../auth/auth.guard' +import { verify } from '@kiltprotocol/sdk-js/build/crypto' +import { ForbiddenMessageAccessException } from './exceptions/message-forbidden.exception' +import { MessageNotFoundException } from './exceptions/message-not-found.exception' export const uuidv4 = () => { return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, c => { @@ -29,8 +33,22 @@ export class MessagingController { ) {} @Delete(':id') - public async removeMessage(@Param('id') id): Promise { - console.log(`Remove message for id ${id}`) + public async removeMessage( + @Param('id') id, + @Headers('signature') signature + ): Promise { + const { receiverAddress } = (await this.messagingService.findById( + id + )).orElseThrow(() => { + throw new MessageNotFoundException() + }) + + if (!signature) { + throw new BadRequestException('No signature provided') + } else if (!verify(id, signature, receiverAddress)) { + throw new ForbiddenMessageAccessException() + } + console.log(`Remove message for id ${id} with signature ${signature}`) await this.messagingService.remove(id) } diff --git a/src/messaging/messaging.module.spec.ts b/src/messaging/messaging.module.spec.ts index 40e9a8a..1f32347 100644 --- a/src/messaging/messaging.module.spec.ts +++ b/src/messaging/messaging.module.spec.ts @@ -4,9 +4,12 @@ import { BadRequestException } from '@nestjs/common/exceptions' import { getModelToken } from '@nestjs/mongoose' import { MessagingController } from './messaging.controller' import { MessagingService, MessageDB } from './interfaces/messaging.interfaces' -import { IEncryptedMessage } from '@kiltprotocol/sdk-js' +import { IEncryptedMessage, Identity } from '@kiltprotocol/sdk-js' import * as Controller from './messaging.controller' import { MongoDbMessagingService } from './mongodb-messaging.service' +import Optional from 'typescript-optional' +import { ForbiddenMessageAccessException } from './exceptions/message-forbidden.exception' +import { MessageNotFoundException } from './exceptions/message-not-found.exception' describe('Messaging Module', () => { const encryptedMessage: IEncryptedMessage = { @@ -16,17 +19,18 @@ describe('Messaging Module', () => { hash: '0xa46441cfbeb4ebd517c810fe78718eb43e891e1603e1db5665f751a1ef632991', signature: '0x00f0a10b39879d6bc9ee7fe54260e6becc4becd4d5e2ca2cfdaf5c8b079fdda852368e56dc776020086f481694374cfcef3b5b9469d1916714db81541b77f7f10d', - receiverAddress: '5D5D5fSDUFVvn6RroC85zgaKL93oFv7R332RGwdCdBvAQzUn', + receiverAddress: '5HYCKhYheTbkB5tPwKWXvs9qimDV4g6TrRuYyXBdFqED2w9J', senderAddress: '5CKq9ovoHUFb5Qg2q7YmQ2waNhgQm4C22qwb1Wgehnn2eBcb', senderBoxPublicKey: '0x5640c86ce5a99b1caf37882197b17572fa8ac33db8387861ef24dd2b497edd43', messageId: 'e545724a-00ad-495c-b314-c66750ae14e4', receivedAt: 1598438707577, } - describe('Controller', () => { let messagesController: MessagingController let messagesService: MessagingService + let receiverIdentity: Identity + let receiverSignature: string const fakeMessagingService: MessagingService = { add: jest.fn( @@ -34,6 +38,9 @@ describe('Messaging Module', () => { return } ), + findById: jest.fn(async () => + Optional.ofNullable(null) + ), findBySenderAddress: jest.fn( async (): Promise => [] ), @@ -69,14 +76,68 @@ describe('Messaging Module', () => { messagesService = moduleRef.get('MessagingService') }) afterEach(() => jest.clearAllMocks()) - + beforeAll(async () => { + receiverIdentity = await Identity.buildFromMnemonic( + 'layer donor village public cruel caution learn bronze fish come embrace hurt' + ) + receiverSignature = receiverIdentity.signStr(encryptedMessage.messageId) + }) describe('removeMessage', () => { it('removes a message for an id from the service', async () => { const removeSpy = jest.spyOn(messagesService, 'remove') - messagesController.removeMessage(encryptedMessage.messageId) + const findByIdSpy = jest + .spyOn(messagesService, 'findById') + .mockResolvedValue(Optional.ofNullable(encryptedMessage)) + await messagesController.removeMessage( + encryptedMessage.messageId, + receiverSignature + ) + expect(findByIdSpy).toHaveBeenCalledTimes(1) + expect(findByIdSpy).toHaveBeenCalledWith(encryptedMessage.messageId) expect(removeSpy).toHaveBeenCalledTimes(1) expect(removeSpy).toHaveBeenCalledWith(encryptedMessage.messageId) - removeSpy.mockRestore() + }) + it('rejects removal request when requirements are not met', async () => { + const removeSpy = jest.spyOn(messagesService, 'remove') + const findByIdSpy = jest + .spyOn(messagesService, 'findById') + .mockResolvedValue(Optional.ofNullable(encryptedMessage)) + await expect( + messagesController.removeMessage( + encryptedMessage.messageId, + receiverSignature.replace('d', 'a') + ) + ).rejects.toThrow(ForbiddenMessageAccessException) + expect(findByIdSpy).toHaveBeenCalledTimes(1) + expect(findByIdSpy).toHaveBeenCalledWith(encryptedMessage.messageId) + + expect(removeSpy).not.toHaveBeenCalled() + removeSpy.mockClear() + findByIdSpy.mockClear() + await expect( + messagesController.removeMessage(encryptedMessage.messageId, '') + ).rejects.toThrow(BadRequestException) + expect(findByIdSpy).toHaveBeenCalledTimes(1) + expect(findByIdSpy).toHaveBeenCalledWith(encryptedMessage.messageId) + + expect(removeSpy).not.toHaveBeenCalled() + removeSpy.mockClear() + findByIdSpy.mockClear() + findByIdSpy.mockResolvedValue( + Optional.ofNullable(null) + ) + await expect( + messagesController.removeMessage( + encryptedMessage.messageId, + receiverSignature + ) + ).rejects.toThrow(MessageNotFoundException) + expect(findByIdSpy).toHaveBeenCalledTimes(1) + expect(findByIdSpy).toHaveBeenCalledWith(encryptedMessage.messageId) + + expect(removeSpy).not.toHaveBeenCalled() + removeSpy.mockClear() + findByIdSpy.mockClear() }) }) describe('removeAll', () => { @@ -184,6 +245,11 @@ describe('Messaging Module', () => { }) class MessageModel { + public static findOne = jest.fn().mockReturnValue({ + exec: async (): Promise => { + return null + }, + }) public static find = jest .fn() .mockReturnValue({ exec: async (): Promise => [] }) @@ -236,6 +302,40 @@ describe('Messaging Module', () => { saveSpy.mockRestore() }) }) + describe('findById', () => { + it('returns Optional for Id and converts to EncryptedMessage', async () => { + const findOneSpy = jest + .spyOn(messagingService['messageModel'], 'findOne') + .mockReturnValue({ + exec: async (): Promise => { + return encryptedMessage as MessageDB + }, + }) + expect( + await messagingService.findById(encryptedMessage.messageId) + ).toEqual(Optional.ofNullable(encryptedMessage)) + expect(findOneSpy).toHaveBeenCalledTimes(1) + expect(findOneSpy).toHaveBeenLastCalledWith({ + messageId: encryptedMessage.messageId, + }) + }) + it('returns nulled Optional for Id and converts to EncryptedMessage', async () => { + const findOneSpy = jest + .spyOn(messagingService['messageModel'], 'findOne') + .mockReturnValue({ + exec: async (): Promise => { + return null + }, + }) + expect( + await messagingService.findById(encryptedMessage.messageId) + ).toEqual(Optional.ofNullable(null)) + expect(findOneSpy).toHaveBeenCalledTimes(1) + expect(findOneSpy).toHaveBeenLastCalledWith({ + messageId: encryptedMessage.messageId, + }) + }) + }) describe('findBySenderAddress', () => { it('queries database and converts matches', async () => { const reverseMessage = { diff --git a/src/messaging/mongodb-messaging.service.ts b/src/messaging/mongodb-messaging.service.ts index aa25064..1d8ec0f 100644 --- a/src/messaging/mongodb-messaging.service.ts +++ b/src/messaging/mongodb-messaging.service.ts @@ -2,7 +2,8 @@ import { Injectable } from '@nestjs/common' import { InjectModel } from '@nestjs/mongoose' import { Model } from 'mongoose' import { MessageDB, MessagingService } from './interfaces/messaging.interfaces' -import { IEncryptedMessage } from '@kiltprotocol/sdk-js' +import { IEncryptedMessage, Crypto } from '@kiltprotocol/sdk-js' +import Optional from 'typescript-optional' @Injectable() export class MongoDbMessagingService implements MessagingService { @@ -17,6 +18,14 @@ export class MongoDbMessagingService implements MessagingService { await createdMessage.save() } + public async findById( + messageId: IEncryptedMessage['messageId'] + ): Promise> { + return Optional.ofNullable( + await this.messageModel.findOne({ messageId }).exec() + ).map((messageDB: MessageDB) => this.convertToEncryptedMessage(messageDB)) + } + public async findBySenderAddress( senderAddress: IEncryptedMessage['senderAddress'] ): Promise { diff --git a/test/messaging.e2e-spec.ts b/test/messaging.e2e-spec.ts index 99a7579..37a1d93 100644 --- a/test/messaging.e2e-spec.ts +++ b/test/messaging.e2e-spec.ts @@ -15,10 +15,11 @@ function assertErrorMessageIs( message: string, response: supertest.Response ): void { - if (response.body['message'] !== message) + if (response.body['message'] !== message) { throw new Error( `Expected error message '${message}', got '${response.body['message']}'` ) + } } let app: INestApplication @@ -228,18 +229,36 @@ describe('messaging (e2e)', () => { await expect( messagingService.findByReceiverAddress(recipient.address) ).resolves.toHaveLength(2) - await request.delete('/messaging/id1').expect(200) + await request + .delete('/messaging/id1') + .set({ signature: recipient.signStr('id1') }) + .expect(200) inbox = await messagingService.findByReceiverAddress(recipient.address) expect(inbox).toHaveLength(1) expect(inbox[0]).toMatchObject(message2) - await request.delete('/messaging/id2').expect(200) + await request + .delete('/messaging/id2') + .set({ signature: recipient.signStr('id2') }) + .expect(200) inbox = await messagingService.findByReceiverAddress(recipient.address) expect(inbox).toHaveLength(0) }) - - // TODO, see KILTprotocol/ticket#685 - xit('rejects delete requests for unknown id', async () => { - await request.delete('/messaging/idx').expect(400) + it('rejects delete unauthorized requests', async () => { + await request + .delete('/messaging/id1') + .set({ signature: recipient.signStr('id2') }) + .expect(403) + await request + .delete('/messaging/id1') + .set({ signature: '' }) + .expect(400) + }) + // should this give 400 or 403, or rather do we want to provide information if the id is unregistered? + it('rejects delete requests for unknown id', async () => { + await request + .delete('/messaging/idx') + .set({ signature: recipient.signStr('idx') }) + .expect(404) }) it('rejects unauthorized delete-all requests', async () => { @@ -371,7 +390,10 @@ describe('messaging (e2e)', () => { }) const decrypted = Message.decrypt(encrypted, recipient) expect(decrypted).toMatchObject(message) - await request.delete(`/messaging/${messageId}`).expect(200) + await request + .delete(`/messaging/${messageId}`) + .set({ signature: recipient.signStr(messageId) }) + .expect(200) await request.get(`/messaging/inbox/${recipient.address}`).expect(200, []) }) })