Have Authorization for deletion of messages

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<CTypeDB> => null })
     public static deleteMany = jest.fn().mockReturnValue({
       exec: async (): Promise<void> => {
         return

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
+    )
+  }
+}

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)
+  }
+}

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<void>
 
+  findById(
+    messageId: IEncryptedMessage['messageId']
+  ): Promise<Optional<IEncryptedMessage>>
+
   findBySenderAddress(
     senderAddress: Contact['publicIdentity']['address']
   ): Promise<IEncryptedMessage[]>

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<void> {
-    console.log(`Remove message for id ${id}`)
+  public async removeMessage(
+    @Param('id') id,
+    @Headers('signature') signature
+  ): Promise<void> {
+    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)
   }
 

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,24 +19,28 @@ 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(
         async (): Promise<void> => {
           return
         }
       ),
+      findById: jest.fn(async () =>
+        Optional.ofNullable<IEncryptedMessage>(null)
+      ),
       findBySenderAddress: jest.fn(
         async (): Promise<IEncryptedMessage[]> => []
       ),
@@ -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<IEncryptedMessage>(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<MessageDB> => {
+        return null
+      },
+    })
     public static find = jest
       .fn()
       .mockReturnValue({ exec: async (): Promise<MessageDB[]> => [] })
@@ -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<MessageDB> => {
+              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<MessageDB> => {
+              return null
+            },
+          })
+        expect(
+          await messagingService.findById(encryptedMessage.messageId)
+        ).toEqual(Optional.ofNullable<IEncryptedMessage>(null))
+        expect(findOneSpy).toHaveBeenCalledTimes(1)
+        expect(findOneSpy).toHaveBeenLastCalledWith({
+          messageId: encryptedMessage.messageId,
+        })
+      })
+    })
     describe('findBySenderAddress', () => {
       it('queries database and converts matches', async () => {
         const reverseMessage = {

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<Optional<IEncryptedMessage>> {
+    return Optional.ofNullable<MessageDB>(
+      await this.messageModel.findOne({ messageId }).exec()
+    ).map((messageDB: MessageDB) => this.convertToEncryptedMessage(messageDB))
+  }
+
   public async findBySenderAddress(
     senderAddress: IEncryptedMessage['senderAddress']
   ): Promise<IEncryptedMessage[]> {

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, [])
   })
 })