Merge pull request #3464 from mozilla-services/register-validate-devi…

…ceid validate deviceId in /api/register
mozilla-services · Sep 11, 2017 · 452776f · 452776f
2 parents 041fde4 + 24eef08
commit 452776f
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 5 deletions.
diff --git a/bin/load_test_exercise.py b/bin/load_test_exercise.py
@@ -49,11 +49,11 @@ def make_device_info():
 
 
 def make_uuid():
-    return str(uuid.uuid1()).replace("-", "")
+    return str(uuid.uuid1())
 
 
 def make_random_id():
-    return make_uuid()[:16]
+    return make_uuid().replace("-", "")[:16]
 
 
 def reset_device_info():

diff --git a/server/src/server.js b/server/src/server.js
@@ -24,7 +24,8 @@ const {
   fetchProfileData,
   saveProfileData,
   disconnectDevice,
-  retrieveAccount
+  retrieveAccount,
+  isValidDeviceId
 } = require("./users");
 const dbschema = require("./dbschema");
 const express = require("express");
@@ -458,6 +459,12 @@ app.post("/api/register", csrfProtection, function(req, res) {
     simpleResponse(res, "Bad request, no deviceId", 400);
     return;
   }
+  if (!isValidDeviceId(vars.deviceId)) {
+    mozlog.error("bad-api-register", {msg: "Bad register request", vars: JSON.stringify(vars, null, "  ")});
+    sendRavenMessage(req, "Attempted to register with invalid deviceId");
+    simpleResponse(res, "Bad request, invalid deviceId", 400);
+    return;
+  }
   if (!vars.secret) {
     mozlog.error("bad-api-register", {msg: "Bad register request", vars: JSON.stringify(vars, null, "  ")});
     sendRavenMessage(req, "Attempted to register without secret");

diff --git a/server/src/users.js b/server/src/users.js
@@ -50,6 +50,11 @@ function getForceAbTests() {
   return result;
 }
 
+function isValidDeviceId(deviceId) {
+  return /^(anon)?[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}$/i.test(deviceId);
+}
+exports.isValidDeviceId = isValidDeviceId;
+
 exports.checkLogin = function(deviceId, secret, addonVersion) {
   if (!secret) {
     throw new Error("No secret given");
@@ -89,6 +94,9 @@ exports.registerLogin = function(deviceId, data, canUpdate) {
   if (!deviceId) {
     throw new Error("No deviceId given");
   }
+  if (!isValidDeviceId(deviceId)) {
+    throw new Error("Invalid deviceId given");
+  }
   if (!(data && data.secret)) {
     throw new Error("No data or data.secret given");
   }

diff --git a/test/server/clientlib.py b/test/server/clientlib.py
@@ -196,11 +196,11 @@ def make_device_info():
 
 
 def make_uuid():
-    return str(uuid.uuid1()).replace("-", "")
+    return str(uuid.uuid1())
 
 
 def make_random_id():
-    return make_uuid()[:16]
+    return make_uuid().replace("-", "")[:16]
 
 
 @contextlib.contextmanager

diff --git a/test/server/test_auth.py b/test/server/test_auth.py
@@ -20,6 +20,17 @@ def test_register_without_deviceid_fails():
     assert resp.status_code == 400, "register without device id worked"
 
 
+def test_register_with_invalid_deviceid_fails():
+    unauthed_user = ScreenshotsClient()
+    resp = unauthed_user.session.post(
+        urljoin(unauthed_user.backend, "/api/register"),
+        data=dict(deviceId='exists_but_is_invalid',
+                  secret=unauthed_user.secret,
+                  deviceInfo=json.dumps(unauthed_user.deviceInfo)))
+
+    assert resp.status_code == 400, "register with invalid device id worked"
+
+
 def test_register_without_secret_fails():
     unauthed_user = ScreenshotsClient()
     resp = unauthed_user.session.post(