Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix webpush vapid_claims from one request bleeding into others #180

Merged
merged 10 commits into from
Mar 22, 2021
1 change: 1 addition & 0 deletions changelog.d/180.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix vapid_claims from one webpush request bleeding into others
27 changes: 19 additions & 8 deletions sygnal/webpushpushkin.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import logging
import os.path
from io import BytesIO
from urllib.parse import urlparse

from prometheus_client import Gauge, Histogram
from py_vapid import Vapid, VapidException
Expand Down Expand Up @@ -104,16 +105,15 @@ def __init__(self, name, sygnal, config):
self.vapid_private_key = Vapid.from_file(private_key_file=privkey_filename)
except VapidException as e:
raise PushkinSetupException("invalid 'vapid_private_key' file") from e
vapid_contact_email = self.get_config("vapid_contact_email")
if not vapid_contact_email:
self.vapid_contact_email = self.get_config("vapid_contact_email")
if not self.vapid_contact_email:
raise PushkinSetupException("'vapid_contact_email' not set in config")
self.vapid_claims = {"sub": "mailto:{}".format(vapid_contact_email)}

async def _dispatch_notification_unlimited(self, n, device, context):
p256dh = device.pushkey
if not isinstance(device.data, dict):
logger.warn(
"device.data is not a dict for pushkey %s, rejecting pushkey", p256dh
"Rejecting pushkey %s; device.data is not a dict", device.pushkey
)
return [device.pushkey]

Expand All @@ -122,8 +122,8 @@ async def _dispatch_notification_unlimited(self, n, device, context):

if not p256dh or not endpoint or not auth:
logger.warn(
"subscription info incomplete "
+ "(p256dh: %s, endpoint: %s, auth: %s), rejecting pushkey",
"Rejecting pushkey; subscription info incomplete "
+ "(p256dh: %s, endpoint: %s, auth: %s)",
p256dh,
endpoint,
auth,
Expand All @@ -137,6 +137,10 @@ async def _dispatch_notification_unlimited(self, n, device, context):
payload = WebpushPushkin._build_payload(n, device)
data = json.dumps(payload)

# note that webpush modifies vapid_claims, so make sure it's only used once
vapid_claims = {
"sub": "mailto:{}".format(self.vapid_contact_email),
}
# we use the semaphore to actually limit the number of concurrent
# requests, since the HTTPConnectionPool will actually just lead to more
# requests being created but not pooled – it does not perform limiting.
Expand All @@ -151,16 +155,23 @@ async def _dispatch_notification_unlimited(self, n, device, context):
subscription_info=subscription_info,
data=data,
vapid_private_key=self.vapid_private_key,
vapid_claims=self.vapid_claims,
vapid_claims=vapid_claims,
requests_session=self.http_agent_wrapper,
)
response = await response_wrapper.deferred
await readBody(response)
response_text = (await readBody(response)).decode()
finally:
self.connection_semaphore.release()

# assume 4xx is permanent and 5xx is temporary
if 400 <= response.code < 500:
logger.warn(
"Rejecting pushkey %s; gateway %s failed with %d: %s",
device.pushkey,
urlparse(endpoint).netloc,
response.code,
response_text,
)
return [device.pushkey]
return []

Expand Down