diff --git a/autopush/endpoint.py b/autopush/endpoint.py index f3e6a412..6557469d 100644 --- a/autopush/endpoint.py +++ b/autopush/endpoint.py @@ -584,11 +584,23 @@ def _route_notification(self, version, result, data, ttl=None): def _router_completed(self, response, uaid_data, warning=""): """Called after router has completed successfully""" # TODO: Add some custom wake logic here - # Were we told to update the router data? - if response.router_data: + if response.router_data is not None: + if not response.router_data: + # An empty router_data object indicates that the record should + # be deleted. There is no longer valid route information for + # this record. + d = deferToThread(self.ap_settings.router.drop_user, + self.uaid) + d.addCallback(lambda x: self._router_response(response)) + return d + # The router data needs to be updated to include any changes + # requested by the bridge system. uaid_data["router_data"] = response.router_data + # set the AWS mandatory data. uaid_data["connected_at"] = ms_time() + uaid_data["router_type"] = uaid_data.get("router_type", + self.router_key) d = deferToThread(self.ap_settings.router.register_user, uaid_data) response.router_data = None @@ -597,6 +609,7 @@ def _router_completed(self, response, uaid_data, warning=""): warning)) return d else: + # No changes are requested by the bridge system, proceed as normal if response.status_code == 200 or response.logged_status == 200: self.log.info(format="Successful delivery", **self._client_info) diff --git a/autopush/router/fcm.py b/autopush/router/fcm.py index f88227ef..8cc3150f 100644 --- a/autopush/router/fcm.py +++ b/autopush/router/fcm.py @@ -45,18 +45,19 @@ def amend_msg(self, msg, data=None): msg["senderid"] = data.get('creds', {}).get('senderID') return msg - def register(self, uaid, router_data, router_token=None, *kwargs): + def register(self, uaid, router_data, senderid=None, *kwargs): """Validate that the FCM Instance Token is in the ``router_data``""" + # "token" is the GCM registration id token generated by the client. if "token" not in router_data: raise self._error("connect info missing FCM Instance 'token'", status=401) - # router_token and router_data['token'] are semi-legacy from when - # we were considering having multiple senderids for outbound - # GCM support. That was abandoned, but it is still useful to - # ensure that the client's senderid value matches what we need - # it to be. (If the client has an unexpected or invalid SenderID, + # senderid is the remote client's senderID value. This value is + # very difficult for the client to change, and there was a problem + # where some clients had an older, invalid senderID. We need to + # be able to match senderID to it's corresponding auth key. + # If the client has an unexpected or invalid SenderID, # it is impossible for us to reach them. - if not (router_token == router_data['token'] == self.senderID): + if not (senderid == self.senderID): raise self._error("Invalid SenderID", status=410, errno=105) # Assign a senderid router_data["creds"] = {"senderID": self.senderID, "auth": self.auth} diff --git a/autopush/router/gcm.py b/autopush/router/gcm.py index da903fce..274135ba 100644 --- a/autopush/router/gcm.py +++ b/autopush/router/gcm.py @@ -12,7 +12,6 @@ class GCMRouter(object): """GCM Router Implementation""" log = Logger() - gcm = None dryRun = 0 collapseKey = "simplepush" @@ -22,17 +21,21 @@ def __init__(self, ap_settings, router_conf): self.min_ttl = router_conf.get("ttl", 60) self.dryRun = router_conf.get("dryrun", False) self.collapseKey = router_conf.get("collapseKey", "simplepush") - self.senderIDs = router_conf.get("senderIDs") + self.gcm = {} + self.senderIDs = {} + # Flatten the SenderID list from human readable and init gcmclient + if not router_conf.get("senderIDs"): + raise IOError("SenderIDs not configured.") + for sid in router_conf.get("senderIDs"): + auth = router_conf.get("senderIDs").get(sid).get("auth") + self.senderIDs[sid] = auth + try: + self.gcm[sid] = gcmclient.GCM(auth) + except: + raise IOError("GCM Bridge not initiated in main") self.metrics = ap_settings.metrics self._base_tags = [] self.router_table = ap_settings.router - try: - sid = self.senderIDs.keys() - self.senderID = sid[0] - self.auth = self.senderIDs.get(sid[0], {}).get('auth') - self.gcm = gcmclient.GCM(self.auth) - except: - raise IOError("GCM Bridge not initiated in main") self.log.debug("Starting GCM router...") def amend_msg(self, msg, data=None): @@ -40,21 +43,23 @@ def amend_msg(self, msg, data=None): msg["senderid"] = data.get('creds', {}).get('senderID') return msg - def register(self, uaid, router_data, router_token=None, *kwargs): + def register(self, uaid, router_data, senderid=None, *kwargs): """Validate that the GCM Instance Token is in the ``router_data``""" + # "token" is the GCM registration id token generated by the client. if "token" not in router_data: raise self._error("connect info missing GCM Instance 'token'", status=401) - # router_token and router_data['token'] are semi-legacy from when - # we were considering having multiple senderids for outbound - # GCM support. That was abandoned, but it is still useful to - # ensure that the client's senderid value matches what we need - # it to be. (If the client has an unexpected or invalid SenderID, + # senderid is the remote client's senderID value. This value is + # very difficult for the client to change, and there was a problem + # where some clients had an older, invalid senderID. We need to + # be able to match senderID to it's corresponding auth key. + # If the client has an unexpected or invalid SenderID, # it is impossible for us to reach them. - if not (router_token == router_data['token'] == self.senderID): + if senderid not in self.senderIDs: raise self._error("Invalid SenderID", status=410, errno=105) # Assign a senderid - router_data["creds"] = {"senderID": self.senderID, "auth": self.auth} + router_data["creds"] = {"senderID": senderid, + "auth": self.senderIDs[senderid]} return router_data def route_notification(self, notification, uaid_data): @@ -99,8 +104,8 @@ def _route(self, notification, uaid_data): ) creds = router_data.get("creds", {"senderID": "missing id"}) try: - self.gcm.api_key = creds["auth"] - result = self.gcm.send(payload) + gcm = self.gcm[creds['senderID']] + result = gcm.send(payload) except KeyError: raise self._error("Server error, missing bridge credentials " + "for %s" % creds.get("senderID"), 500) diff --git a/autopush/tests/test_endpoint.py b/autopush/tests/test_endpoint.py index 6317f5d4..6df1a0a1 100644 --- a/autopush/tests/test_endpoint.py +++ b/autopush/tests/test_endpoint.py @@ -618,13 +618,16 @@ def test_put_router_needs_change(self): ) self.sp_router_mock.route_notification.return_value = RouterResponse( status_code=500, - router_data={}, + router_data=dict(token="new_connect"), ) def handle_finish(result): self.assertTrue(result) self.endpoint.set_status.assert_called_with(500, None) - ok_(not self.router_mock.register_user.called) + ru = self.router_mock.register_user + ok_(ru.called) + eq_('simplepush', ru.call_args[0][0].get('router_type')) + self.finish_deferred.addCallback(handle_finish) self.endpoint.put(None, dummy_uaid) @@ -638,13 +641,14 @@ def test_put_router_needs_update(self): ) self.sp_router_mock.route_notification.return_value = RouterResponse( status_code=503, - router_data=dict(token="new_connect"), + router_data=dict(), ) def handle_finish(result): self.assertTrue(result) self.endpoint.set_status.assert_called_with(503, None) - assert(self.router_mock.register_user.called) + self.router_mock.drop_user.assert_called() + self.finish_deferred.addCallback(handle_finish) self.endpoint.put(None, dummy_uaid) diff --git a/autopush/tests/test_router.py b/autopush/tests/test_router.py index 03ed8cb3..ae094ff9 100644 --- a/autopush/tests/test_router.py +++ b/autopush/tests/test_router.py @@ -236,7 +236,7 @@ def setUp(self, fgcm): def _check_error_call(self, exc, code): ok_(isinstance(exc, RouterException)) eq_(exc.status_code, code) - assert(self.router.gcm.send.called) + assert(self.router.gcm['test123'].send.called) self.flushLoggedErrors() def test_init(self): @@ -250,7 +250,7 @@ def test_init(self): def test_register(self): result = self.router.register(uaid="uaid", router_data={"token": "test123"}, - router_token="test123") + senderid="test123") # Check the information that will be recorded for this user eq_(result, {"token": "test123", "creds": {"senderID": "test123", @@ -258,16 +258,31 @@ def test_register(self): def test_register_bad(self): self.assertRaises(RouterException, self.router.register, "uaid", {}) + self.assertRaises(RouterException, + self.router.register, + "uaid", + {"token": "abcd1234"}, + "test123") + + @patch("gcmclient.GCM") + def test_gcmclient_fail(self, fgcm): + fgcm.side_effect = Exception + settings = AutopushSettings( + hostname="localhost", + statsd_host=None, + ) + self.assertRaises(IOError, GCMRouter, settings, + {"senderIDs": {"test123": {"auth": "abcd"}}}) def test_route_notification(self): - self.router.gcm = self.gcm + self.router.gcm['test123'] = self.gcm d = self.router.route_notification(self.notif, self.router_data) def check_results(result): ok_(isinstance(result, RouterResponse)) - assert(self.router.gcm.send.called) + assert(self.router.gcm['test123'].send.called) # Make sure the data was encoded as base64 - data = self.router.gcm.send.call_args[0][0].data + data = self.router.gcm['test123'].send.call_args[0][0].data eq_(data['body'], 'q60d6g') eq_(data['enc'], 'test') eq_(data['enckey'], 'test') @@ -276,7 +291,7 @@ def check_results(result): return d def test_ttl_none(self): - self.router.gcm = self.gcm + self.router.gcm['test123'] = self.gcm self.notif = Notification(version=10, data="q60d6g", channel_id=dummy_chid, @@ -286,10 +301,10 @@ def test_ttl_none(self): def check_results(result): ok_(isinstance(result, RouterResponse)) - assert(self.router.gcm.send.called) + assert(self.router.gcm['test123'].send.called) # Make sure the data was encoded as base64 - data = self.router.gcm.send.call_args[0][0].data - options = self.router.gcm.send.call_args[0][0].options + data = self.router.gcm['test123'].send.call_args[0][0].data + options = self.router.gcm['test123'].send.call_args[0][0].options eq_(data['body'], 'q60d6g') eq_(data['enc'], 'test') eq_(data['enckey'], 'test') @@ -300,7 +315,7 @@ def check_results(result): return d def test_long_data(self): - self.router.gcm = self.gcm + self.router.gcm['test123'] = self.gcm badNotif = Notification( 10, "\x01abcdefghijklmnopqrstuvwxyz0123456789", dummy_chid, self.headers, 200) @@ -315,14 +330,14 @@ def check_results(result): return d def test_route_crypto_notification(self): - self.router.gcm = self.gcm + self.router.gcm['test123'] = self.gcm del(self.notif.headers['encryption-key']) self.notif.headers['crypto-key'] = 'crypto' d = self.router.route_notification(self.notif, self.router_data) def check_results(result): ok_(isinstance(result, RouterResponse)) - assert(self.router.gcm.send.called) + assert(self.router.gcm['test123'].send.called) d.addCallback(check_results) return d @@ -330,7 +345,7 @@ def test_router_notification_gcm_auth_error(self): def throw_auth(arg): raise gcmclient.GCMAuthenticationError() self.gcm.send.side_effect = throw_auth - self.router.gcm = self.gcm + self.router.gcm['test123'] = self.gcm d = self.router.route_notification(self.notif, self.router_data) def check_results(fail): @@ -342,7 +357,7 @@ def test_router_notification_gcm_other_error(self): def throw_other(arg): raise Exception("oh my!") self.gcm.send.side_effect = throw_other - self.router.gcm = self.gcm + self.router.gcm['test123'] = self.gcm d = self.router.route_notification(self.notif, self.router_data) def check_results(fail): @@ -352,31 +367,31 @@ def check_results(fail): def test_router_notification_gcm_id_change(self): self.mock_result.canonical["old"] = "new" - self.router.gcm = self.gcm + self.router.gcm['test123'] = self.gcm d = self.router.route_notification(self.notif, self.router_data) def check_results(result): ok_(isinstance(result, RouterResponse)) eq_(result.router_data, dict(token="new")) - assert(self.router.gcm.send.called) + assert(self.router.gcm['test123'].send.called) d.addCallback(check_results) return d def test_router_notification_gcm_not_regged(self): self.mock_result.not_registered = {"connect_data": True} - self.router.gcm = self.gcm + self.router.gcm['test123'] = self.gcm d = self.router.route_notification(self.notif, self.router_data) def check_results(result): ok_(isinstance(result, RouterResponse)) eq_(result.router_data, dict()) - assert(self.router.gcm.send.called) + assert(self.router.gcm['test123'].send.called) d.addCallback(check_results) return d def test_router_notification_gcm_failed_items(self): self.mock_result.failed = dict(connect_data=True) - self.router.gcm = self.gcm + self.router.gcm['test123'] = self.gcm d = self.router.route_notification(self.notif, self.router_data) def check_results(fail): @@ -386,7 +401,7 @@ def check_results(fail): def test_router_notification_gcm_needs_retry(self): self.mock_result.needs_retry.return_value = True - self.router.gcm = self.gcm + self.router.gcm['test123'] = self.gcm d = self.router.route_notification(self.notif, self.router_data) def check_results(fail): @@ -406,7 +421,7 @@ def check_results(fail): def test_amend(self): self.router.register(uaid="uaid", router_data={"token": "test123"}, - router_token="test123") + senderid="test123") resp = {"key": "value"} result = self.router.amend_msg(resp, self.router_data.get('router_data')) @@ -416,7 +431,7 @@ def test_amend(self): def test_register_invalid_token(self): self.assertRaises(RouterException, self.router.register, uaid="uaid", router_data={"token": "invalid"}, - router_token="invalid") + senderid="invalid") class FCMRouterTestCase(unittest.TestCase): @@ -473,7 +488,7 @@ def throw_auth(arg): def test_register(self): result = self.router.register(uaid="uaid", router_data={"token": "test123"}, - router_token="test123") + senderid="test123") # Check the information that will be recorded for this user eq_(result, {"token": "test123", "creds": {"senderID": "test123", @@ -629,7 +644,7 @@ def check_results(fail): def test_amend(self): self.router.register(uaid="uaid", router_data={"token": "test123"}, - router_token="test123") + senderid="test123") resp = {"key": "value"} result = self.router.amend_msg(resp, self.router_data.get('router_data')) @@ -639,7 +654,7 @@ def test_amend(self): def test_register_invalid_token(self): self.assertRaises(RouterException, self.router.register, uaid="uaid", router_data={"token": "invalid"}, - router_token="invalid") + senderid="invalid") class SimplePushRouterTestCase(unittest.TestCase): diff --git a/autopush/tests/test_web_webpush.py b/autopush/tests/test_web_webpush.py index b71288d8..ff253942 100644 --- a/autopush/tests/test_web_webpush.py +++ b/autopush/tests/test_web_webpush.py @@ -108,7 +108,8 @@ def test_router_returns_data_without_detail(self): def handle_finish(result): eq_(result, True) self.wp.set_status.assert_called_with(503) - assert(self.router_mock.register_user.called) + assert(self.router_mock.drop_user.called) + self.finish_deferred.addCallback(handle_finish) self.wp.post("v1", dummy_token) diff --git a/autopush/web/webpush.py b/autopush/web/webpush.py index f95a8cbf..542dbf6f 100644 --- a/autopush/web/webpush.py +++ b/autopush/web/webpush.py @@ -56,12 +56,19 @@ def post(self, api_ver="v1", token=None): def _router_completed(self, response, uaid_data, warning=""): """Called after router has completed successfully""" # Were we told to update the router data? - # GCM/APNS bridges can result in data updates if response.router_data is not None: if not response.router_data: - del uaid_data["router_data"] - else: - uaid_data["router_data"] = response.router_data + # An empty router_data object indicates that the record should + # be deleted. There is no longer valid route information for + # this record. + d = deferToThread(self.ap_settings.router.drop_user, + self.uaid) + d.addCallback(lambda x: self._router_response(response)) + return d + # The router data needs to be updated to include any changes + # requested by the bridge system + uaid_data["router_data"] = response.router_data + # set the AWS mandatory data uaid_data["connected_at"] = ms_time() d = deferToThread(self.ap_settings.router.register_user, uaid_data) @@ -71,6 +78,7 @@ def _router_completed(self, response, uaid_data, warning=""): warning)) return d else: + # No changes are requested by the bridge system, proceed as normal if response.status_code == 200 or response.logged_status == 200: self.log.info(format="Successful delivery", **self._client_info)