Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

fix: drop users with no router_type recorded #1065

Merged
merged 1 commit into from
Oct 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion autopush/ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
from OpenSSL import SSL
from twisted.internet.ssl import DefaultOpenSSLContextFactory

try:
SSL_PROTO = ssl.PROTOCOL_TLS
except AttributeError:
SSL_PROTO = ssl.PROTOCOL_SSLv23


MOZILLA_INTERMEDIATE_CIPHERS = (
'ECDHE-RSA-AES128-GCM-SHA256:'
'ECDHE-ECDSA-AES128-GCM-SHA256:'
Expand Down Expand Up @@ -111,7 +117,7 @@ def ssl_wrap_socket_cached(
certfile=None, # type: Optional[str]
server_side=False, # type: bool
cert_reqs=ssl.CERT_NONE, # type: int
ssl_version=ssl.PROTOCOL_TLS, # type: int
ssl_version=SSL_PROTO, # type: int
ca_certs=None, # type: Optional[str]
do_handshake_on_connect=True, # type: bool
suppress_ragged_eofs=True, # type: bool
Expand Down
13 changes: 13 additions & 0 deletions autopush/tests/test_web_webpush.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,19 @@ def test_request_v2_id_no_crypt_auth(self):
)
assert resp.get_status() == 401

@inlineCallbacks
def test_request_no_router_type(self):
self.fernet_mock.decrypt.return_value = 'a' * 32
self.db.router.get_uaid.return_value = dict(
uaid=dummy_uaid,
chid=dummy_chid,
)
resp = yield self.client.post(
self.url(api_ver='v1', token='ignored'),
headers={'authorization': 'webpush dummy.key'}
)
assert resp.get_status() == 410

@inlineCallbacks
def test_request_bad_v2_id_bad_pubkey(self):
self.fernet_mock.decrypt.return_value = 'a' * 64
Expand Down
24 changes: 19 additions & 5 deletions autopush/web/webpush.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
# Base64 URL validation
VALID_BASE64_URL = re.compile(r'^[0-9A-Za-z\-_]+=*$')

VALID_ROUTER_TYPES = ["simplepush", "webpush", "gcm", "fcm", "apns"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really thought I had something like this before. There's a similar list in autopush.registration.conditional_token_check Wonder if it might make sense to have this list somewhere common and use it in both locations?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, or heck, maybe a constants.py for the shared universal constants we end up using all over.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

absolutely 👍 to a constants.py. I've got one for boto I put in __init__.py, but happy to move it in favor of just keeping version there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will open a separate PR to refactor some of the constants together.



class WebPushSubscriptionSchema(Schema):
uaid = fields.UUID(required=True)
Expand Down Expand Up @@ -86,13 +88,25 @@ def validate_uaid_month_and_chid(self, d):
except ItemNotFound:
raise InvalidRequest("UAID not found", status_code=410, errno=103)

if (result.get("router_type") in ["gcm", "fcm"]
# We must have a router_type to validate the user
router_type = result.get("router_type")
if router_type not in VALID_ROUTER_TYPES:
self.context["log"].debug(format="Dropping User", code=102,
uaid_hash=hasher(result["uaid"]),
uaid_record=dump_uaid(result))
self.context["metrics"].increment("updates.drop_user",
tags=make_tags(errno=102))
self.context["db"].router.drop_user(result["uaid"])
raise InvalidRequest("No such subscription", status_code=410,
errno=106)

if (router_type in ["gcm", "fcm"]
and 'senderID' not in result.get('router_data',
{}).get("creds", {})):
# Make sure we note that this record is bad.
result['critical_failure'] = \
result.get('critical_failure', "Missing SenderID")
db.router.register_user(result)
# Make sure we note that this record is bad.
result['critical_failure'] = \
result.get('critical_failure', "Missing SenderID")
db.router.register_user(result)

if result.get("critical_failure"):
raise InvalidRequest("Critical Failure: %s" %
Expand Down