fix: Correct documents to strongly recommend well formatted UUIDs

The code normalizes UUIDs used for channelIDs and UIADs to lower case,
dashed format. The docs now state that and the code will reject UUIDs
that are not well formatted. This is important for clients that may
normalize differently.

BREAKING CHANGE: This patch will reject UUIDs that are not expressed as
lower case, dash formatted (e.g. "00001111-aaaa-bbbb-cccc-222233334444")

Closes #392

autopush/tests/test_websocket.py

@@ -50,6 +50,7 @@ def tearDown():
 
 
 class WebsocketTestCase(unittest.TestCase):
+
     def setUp(self):
         from twisted.logger import Logger
         twisted.internet.base.DelayedCall.debug = True
@@ -411,7 +412,7 @@ def wait_for_agent_call():  # pragma: nocover
         return d
 
     def test_hello_old(self):
-        orig_uaid = "deadbeef12345678decafbad12345678"
+        orig_uaid = "deadbeef-0000-0000-abad-1dea00000000"
         # router.register_user returns (registered, previous
         target_day = datetime.date(2016, 2, 29)
         msg_day = datetime.date(2015, 12, 15)
@@ -459,7 +460,7 @@ def check_result(msg):
         return self._check_response(check_result)
 
     def test_hello_tomorrow(self):
-        orig_uaid = "deadbeef12345678decafbad12345678"
+        orig_uaid = "deadbeef-0000-0000-abad-1dea00000000"
         # router.register_user returns (registered, previous
         target_day = datetime.date(2016, 2, 29)
         msg_day = datetime.date(2016, 3, 1)
@@ -1007,6 +1008,51 @@ def check_hello_result(msg):
         f.addErrback(lambda x: d.errback(x))
         return d
 
+    def test_register_bad_chid_upper(self):
+        self._connect()
+        self._send_message(dict(messageType="hello", channelIDs=[]))
+
+        d = Deferred()
+        d.addCallback(lambda x: True)
+
+        def check_register_result(msg):
+            eq_(msg["status"], 401)
+            eq_(msg["messageType"], "register")
+            d.callback(True)
+
+        def check_hello_result(msg):
+            assert "messageType" in msg
+            self._send_message(dict(messageType="register",
+                                    channelID=str(uuid.uuid4()).upper()))
+            self._check_response(check_register_result)
+
+        f = self._check_response(check_hello_result)
+        f.addErrback(lambda x: d.errback(x))
+        return d
+
+    def test_register_bad_chid_nodash(self):
+        self._connect()
+        self._send_message(dict(messageType="hello", channelIDs=[]))
+
+        d = Deferred()
+        d.addCallback(lambda x: True)
+
+        def check_register_result(msg):
+            eq_(msg["status"], 401)
+            eq_(msg["messageType"], "register")
+            d.callback(True)
+
+        def check_hello_result(msg):
+            assert "messageType" in msg
+            self._send_message(
+                dict(messageType="register",
+                     channelID=str(uuid.uuid4()).replace('-', '')))
+            self._check_response(check_register_result)
+
+        f = self._check_response(check_hello_result)
+        f.addErrback(lambda x: d.errback(x))
+        return d
+
     def test_register_bad_crypto(self):
         self._connect()
         self.proto.ps.uaid = str(uuid.uuid4())

autopush/websocket.py

@@ -1002,9 +1002,11 @@ def process_register(self, data):
             return self.bad_message("register")
         chid = data["channelID"]
         try:
-            uuid.UUID(chid)
+            if str(uuid.UUID(chid)) != chid:
+                return self.bad_message("register", "Bad UUID format, use"
+                                        "lower case, dashed format")
         except ValueError:
-            return self.bad_message("register")
+            return self.bad_message("register", "Invalid UUID specified")
         self.transport.pauseProducing()
 
         d = self.deferToThread(self.ap_settings.make_endpoint, self.ps.uaid,
@@ -1047,12 +1049,12 @@ def send_register_finish(self, result, endpoint, chid):
     def process_unregister(self, data):
         """Process an unregister message"""
         if "channelID" not in data:
-            return self.bad_message("unregister")
+            return self.bad_message("unregister", "Missing ChannelID")
         chid = data["channelID"]
         try:
             uuid.UUID(chid)
         except ValueError:
-            return self.bad_message("unregister")
+            return self.bad_message("unregister", "Invalid ChannelID")
 
         self.ps.metrics.increment("updates.client.unregister",
                                   tags=self.base_tags)
@@ -1231,9 +1233,11 @@ def check_missed_notifications(self, results, resume=False):
         if self.ps._check_notifications or self.ps._more_notifications:
             self.process_notifications()
 
-    def bad_message(self, typ):
+    def bad_message(self, typ, message=None):
         """Error helper for sending a 401 status back"""
         msg = {"messageType": typ, "status": 401}
+        if message:
+            msg["message"] = message
         self.sendJSON(msg)
 
     def _newer_notification_sent(self, channel_id, version):

docs/architecture.rst

@@ -142,7 +142,9 @@ The Endpoint URL may seem excessively long. This may seem needless and
 confusing since the URL consists of the unique User Agent Identifier (UAID)
 and the Subscription Channel Identifier (CHID). Both of these are class 4
 Universially Unique Identifiers (UUID) meaning that an endpoint contains
-256 bits of entropy (2 * 128 bits).
+256 bits of entropy (2 * 128 bits). When used in string format, these UUIDs
+are always in lower case, dashed format (e.g.
+"01234567-0123-abcd-0123-0123456789ab").
 
 Unfortunately, since the endpoint contains an identifier that can be
 easily traced back to a specific device, and therefore a specific user,

docs/http.rst

@@ -7,12 +7,13 @@ Autopush exposes three HTTP endpoints:
 
 `/push/...`
 
-This is tied to :class:`EndpointHandler` (:ref:`endpoint_module`). This endpoint is returned by the Push registration process and is used by the :term:`AppServer` to send Push alerts
-to the Application. See :ref:`send`.
+This is tied to :class:`EndpointHandler` (:ref:`endpoint_module`). This endpoint is returned by the Push registration process and is used by the
+:term:`AppServer` to send Push alerts to the Application. See :ref:`send`.
 
 `/m/...`
 
-This is tied to :class:`MessageHandler` (:ref:`endpoint_module`). This endpoint handles individual
+This is tied to :class:`MessageHandler` (:ref:`endpoint_module`). This
+endpoint handles individual
 message operations on messages pending delivery, such as deleting the
 message or updating the contents or Time To Live (TTL). See :ref:`cancel`
 and :ref:`update`.
@@ -42,12 +43,13 @@ Lexicon
 
    :{UAID}: The Push User Agent Registration ID
 
-Push assigns each remote recipient a unique identifier. This value is
-assigned during **Registration**
+Push assigns each remote recipient a unique identifier. {UAID}s are UUIDs in
+lower case, dashed format. (e.g. '01234567-abcd-abcd-abcd-012345678abc') This value is assigned during **Registration**
 
    :{CHID}: The :term:`Channel` Subscription ID
 
 Push assigns a unique identifier for each subscription for a given {UAID}.
+Like {UAID}s, {CHID}s are UUIDs in lower case, dashed format.
 This value is assigned during **Channel Subscription**
 
    :{message-id}: The unique Message ID
@@ -323,10 +325,10 @@ example:
 
 .. code-block:: json
 
-    < {"uaid": "abcdef012345",
+    < {"uaid": "01234567-0000-1111-2222-0123456789ab",
     < "secret": "0123abcdef",
     < "endpoint": "https://updates-push.services.mozaws.net/push/...",
-    < "channelID": "01234abcd"}
+    < "channelID": "00000000-0000-1111-2222-0123456789ab"}
 
 **Return Codes:**
 
@@ -420,7 +422,7 @@ example:
 
 .. code-block:: json
 
-    < {"channelID": "43210efgh"
+    < {"channelID": "01234567-0000-1111-2222-0123456789ab",
     < "endpoint": "https://updates-push.services.mozaws.net/push/..."}
 
 **Return Codes:**