From f07fad0df838b8798a4b084c96e131264025e95a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gre=CC=81gory=20Le=20Garec?= <gregory.legarec@gmail.com>
Date: Wed, 17 May 2017 16:30:26 +0200
Subject: [PATCH] feat: add types for success and cancel messages for intents,
 increase security :police_officer:

---
 src/intents.js       | 32 +++++++++++++++++++++++++-------
 test/unit/intents.js | 27 ++++++++++++++++++++++-----
 2 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/src/intents.js b/src/intents.js
index 9a263512..ecbddd67 100644
--- a/src/intents.js
+++ b/src/intents.js
@@ -35,9 +35,24 @@ function injectService (url, element, intent, data) {
         return reject(new Error('Intent error'))
       }
 
-      return handshaken
-        ? resolve(event.data)
-        : reject(new Error('Unexpected handshake message from intent service'))
+      if (handshaken && event.data.type === `intent-${intent._id}:cancel`) {
+        return resolve(null)
+      }
+
+      if (handshaken && event.data.type === `intent-${intent._id}:done`) {
+        return resolve(event.data.document)
+      }
+
+      if (!handshaken) {
+        return reject(new Error('Unexpected handshake message from intent service'))
+      }
+
+      // We may be in a state where the messageHandler is still attached to then
+      // window, but will not be needed anymore. For example, the service failed
+      // before adding the `unload` listener, so no `intent:cancel` message has
+      // never been sent.
+      // So we simply ignore other messages, and this listener will stay here,
+      // waiting for a message which will never come, forever (almost).
     }
 
     window.addEventListener('message', messageHandler)
@@ -101,14 +116,14 @@ export function createService (cozy, intentId, serviceWindow) {
     .then(intent => {
       let terminated = false
 
-      const terminate = (doc) => {
+      const terminate = (message) => {
         if (terminated) throw new Error('Intent service has already been terminated')
         terminated = true
-        serviceWindow.parent.postMessage(doc, intent.attributes.client)
+        serviceWindow.parent.postMessage(message, intent.attributes.client)
       }
 
       const cancel = () => {
-        terminate(null)
+        terminate({type: `intent-${intent._id}:cancel`})
       }
 
       // Prevent unfulfilled client promises when this window unloads for a
@@ -120,7 +135,10 @@ export function createService (cozy, intentId, serviceWindow) {
           return {
             getData: () => data,
             getIntent: () => intent,
-            terminate: terminate,
+            terminate: (doc) => terminate({
+              type: `intent-${intent._id}:done`,
+              document: doc
+            }),
             cancel: cancel
           }
         })
diff --git a/test/unit/intents.js b/test/unit/intents.js
index ead75c34..d2de5172 100644
--- a/test/unit/intents.js
+++ b/test/unit/intents.js
@@ -238,7 +238,10 @@ describe('Intents', function () {
 
       const resolveEventMessageMock = {
         origin: serviceUrl,
-        data: result,
+        data: {
+          type: 'intent-77bcc42c-0fd8-11e7-ac95-8f605f6e8338:done',
+          document: result
+        },
         source: iframeWindowMock
       }
 
@@ -348,8 +351,13 @@ describe('Intents', function () {
 
           service.terminate(result)
 
+          const messageMatch = sinon.match({
+            type: 'intent-77bcc42c-0fd8-11e7-ac95-8f605f6e8338:done',
+            document: result
+          })
+
           windowMock.parent.postMessage
-            .withArgs(result, expectedIntent.attributes.client).calledOnce.should.be.true()
+            .withArgs(messageMatch, expectedIntent.attributes.client).calledOnce.should.be.true()
         })
 
         it('should send result document to Client also with no params', async function () {
@@ -373,8 +381,13 @@ describe('Intents', function () {
 
           service.terminate(result)
 
+          const messageMatch = sinon.match({
+            type: 'intent-77bcc42c-0fd8-11e7-ac95-8f605f6e8338:done',
+            document: result
+          })
+
           window.parent.postMessage
-            .withArgs(result, expectedIntent.attributes.client).calledOnce.should.be.true()
+            .withArgs(messageMatch, expectedIntent.attributes.client).calledOnce.should.be.true()
 
           delete global.window
         })
@@ -424,8 +437,10 @@ describe('Intents', function () {
 
           service.cancel()
 
+          const messageMatch = sinon.match({type: 'intent-77bcc42c-0fd8-11e7-ac95-8f605f6e8338:cancel'})
+
           windowMock.parent.postMessage
-            .withArgs(null, expectedIntent.attributes.client).calledOnce.should.be.true()
+            .withArgs(messageMatch, expectedIntent.attributes.client).calledOnce.should.be.true()
         })
 
         it('should send null to Client also with no parameters', async function () {
@@ -445,8 +460,10 @@ describe('Intents', function () {
 
           service.cancel()
 
+          const messageMatch = sinon.match({type: 'intent-77bcc42c-0fd8-11e7-ac95-8f605f6e8338:cancel'})
+
           window.parent.postMessage
-            .withArgs(null, expectedIntent.attributes.client).calledOnce.should.be.true()
+            .withArgs(messageMatch, expectedIntent.attributes.client).calledOnce.should.be.true()
 
           delete global.window
         })