fix(push): reject extra push-payloads properties instead of removing …

…them
mozilla · Mar 21, 2017 · c90719a · c90719a
1 parent c77df31
commit c90719a
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 19 deletions.
diff --git a/docs/pushpayloads.schema.json b/docs/pushpayloads.schema.json
@@ -39,9 +39,11 @@
               "type":"string",
               "description":"The name of the device who joined this account"
             }
-          }
+          },
+          "additionalProperties": false
         }
-      }
+      },
+      "additionalProperties": false
     },
     "deviceDisconnected":{
       "required":[
@@ -69,9 +71,11 @@
               "type":"string",
               "description":"The id of the device who was disconnected remotely"
             }
-          }
+          },
+          "additionalProperties": false
         }
-      }
+      },
+      "additionalProperties": false
     },
     "collectionsChanged":{
       "required":[
@@ -105,9 +109,11 @@
                   "tabs", "passwords", "clients" ]
               }
             }
-          }
+          },
+          "additionalProperties": false
         }
-      }
+      },
+      "additionalProperties": false
     },
     "passwordChanged":{
       "required":[
@@ -124,7 +130,8 @@
             "fxaccounts:password_changed"
           ]
         }
-      }
+      },
+      "additionalProperties": false
     },
     "passwordReset":{
       "required":[
@@ -141,7 +148,8 @@
             "fxaccounts:password_reset"
           ]
         }
-      }
+      },
+      "additionalProperties": false
     }
   }
 }
diff --git a/lib/routes/account.js b/lib/routes/account.js
@@ -21,7 +21,7 @@ var MS_ONE_MONTH = MS_ONE_DAY * 30
 
 var path = require('path')
 var Ajv = require('ajv')
-var ajv = new Ajv({ removeAdditional: 'all' })
+var ajv = new Ajv()
 var fs = require('fs')
 var butil = require('../crypto/butil')
 var userAgent = require('../userAgent')

diff --git a/test/local/routes/account_devices.js b/test/local/routes/account_devices.js
@@ -254,7 +254,7 @@ describe('/account/devices/notify', function () {
     })
   })
 
-  it('extra push payload properties are stripped', function () {
+  it('extra push payload properties are rejected', function () {
     var extraPropsPayload = JSON.parse(JSON.stringify(pushPayload))
     extraPropsPayload.extra = true
     extraPropsPayload.data.extra = true
@@ -271,14 +271,12 @@ describe('/account/devices/notify', function () {
       pushToAllDevicesPromise.resolve()
       return Promise.resolve()
     })
-    return runTest(route, mockRequest, function (response) {
-      return pushToAllDevicesPromise.promise.then(function () {
-        assert.deepEqual(mockPush.pushToAllDevices.args[0][2], {
-          data: Buffer.from(JSON.stringify(pushPayload)),
-          excludedDeviceIds: ['bogusid'],
-          TTL: 60
-        }, 'third argument payload properties has no extra properties')
-      })
+    return runTest(route, mockRequest, function () {
+      assert(false, 'should have thrown')
+    })
+    .then(() => assert.ok(false), function (err) {
+      assert.equal(err.output.statusCode, 400, 'correct status code is returned')
+      assert.equal(err.errno, error.ERRNO.INVALID_PARAMETER, 'correct errno is returned')
     })
   })
 
@@ -334,7 +332,6 @@ describe('/account/devices/notify', function () {
       to: ['bogusid1', 'bogusid2'],
       TTL: 60,
       payload: {
-        isValid: true,
         version: 1,
         command: 'fxaccounts:password_reset'
       }