-
Notifications
You must be signed in to change notification settings - Fork 476
MSC4174: add support for WebPush pusher kind #17987
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
base: develop
Are you sure you want to change the base?
Changes from 15 commits
a3f58ad
920a3f1
5515c37
c029e9a
20f4a31
e2fe3b1
8f22240
4c20c10
c8a7847
9727f55
e2d67d6
ad0f8fe
94e946c
bf0cbd1
5f67af1
f54ad43
18dcea0
9eb04a5
12fb074
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| MSC4174: add support for WebPush pusher kind. | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,97 @@ | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # WebPush | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ## Setup & configuration | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| In the synapse virtualenv, generate the server key pair by running | ||||||||||||||||||||||||
| `vapid --gen --applicationServerKey`. This will generate a `private_key.pem` | ||||||||||||||||||||||||
| (which you'll refer to in the config file with `vapid_private_key`) | ||||||||||||||||||||||||
| and `public_key.pem` file, and also a string labeled `Application Server Key`. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| You'll copy the Application Server Key to `vapid_app_server_key` so that | ||||||||||||||||||||||||
| web applications can fetch it through `/capabilities` and use it to subscribe | ||||||||||||||||||||||||
| to the push manager: | ||||||||||||||||||||||||
|
Comment on lines
6
to
13
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a hassle. Any way to improve this? Is it possible to make it generate if the file does not exist like we do for synapse/docs/usage/configuration/config_documentation.md Lines 3128 to 3138 in 90a6bd0
Does it matter if this key changes from time to time? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It matters if the client isn't aware of this change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We would have to edit the config file since we need to also specify |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ```js | ||||||||||||||||||||||||
| serviceWorkerRegistration.pushManager.subscribe({ | ||||||||||||||||||||||||
| userVisibleOnly: true, | ||||||||||||||||||||||||
| applicationServerKey: "...", | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| You also need to set an e-mail address in `vapid_contact_email` in the config file, | ||||||||||||||||||||||||
| where the push server operator can reach you in case they need to notify you | ||||||||||||||||||||||||
| about your usage of their API. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Since for webpush, the push server endpoint is variable and comes from the browser | ||||||||||||||||||||||||
| through the push data, you may not want to have your synapse instance connect to any | ||||||||||||||||||||||||
| random addressable server. | ||||||||||||||||||||||||
| You can use the global options `ip_range_blacklist` and `ip_range_allowlist` to manage that. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| A default time-to-live of a day is set for webpush, but you can adjust this by setting | ||||||||||||||||||||||||
| the `ttl: <number of seconds>` configuration option for the pusher. | ||||||||||||||||||||||||
| If notifications can't be delivered by the push server aftet this time, they are dropped. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ## Push key and expected push data | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| In your web application, [the push manager subscribe method](https://developer.mozilla.org/en-US/docs/Web/API/PushManager/subscribe) | ||||||||||||||||||||||||
| will return | ||||||||||||||||||||||||
| [a subscription](https://developer.mozilla.org/en-US/docs/Web/API/PushSubscription) | ||||||||||||||||||||||||
| with an `endpoint` and `keys` property, the latter containing a `p256dh` and `auth` | ||||||||||||||||||||||||
| property. The `p256dh` key is used as the push key, and the push data must contain | ||||||||||||||||||||||||
| `endpoint` and `auth`. You can also set `default_payload` in the push data; | ||||||||||||||||||||||||
| any properties set in it will be present in the push messages you receive, | ||||||||||||||||||||||||
| so it can be used to pass identifiers specific to your client | ||||||||||||||||||||||||
| (like which account the notification is for). | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ### `events_only` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| As of the time of writing, all webpush-supporting browsers require you to set | ||||||||||||||||||||||||
| `userVisibleOnly: true` when calling (`pushManager.subscribe`) | ||||||||||||||||||||||||
| [https://developer.mozilla.org/en-US/docs/Web/API/PushManager/subscribe], to | ||||||||||||||||||||||||
| (prevent abusing webpush to track users)[https://goo.gl/yqv4Q4] without their | ||||||||||||||||||||||||
| knowledge. With this (mandatory) flag, the browser will show a "site has been | ||||||||||||||||||||||||
| updated in the background" notification if no notifications are visible after | ||||||||||||||||||||||||
| your service worker processes a `push` event. This can easily happen when synapse | ||||||||||||||||||||||||
| sends a push message to clear the unread count, which is not specific | ||||||||||||||||||||||||
| to an event. With `events_only: true` in the pusher data, synapse won't forward | ||||||||||||||||||||||||
| any push message without a event id. This prevents your service worker being | ||||||||||||||||||||||||
| forced to show a notification to push messages that clear the unread count. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ### `only_last_per_room` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| You can opt in to only receive the last notification per room by setting | ||||||||||||||||||||||||
| `only_last_per_room: true` in the push data. Note that if the first notification | ||||||||||||||||||||||||
| can be delivered before the second one is sent, you will still get both; | ||||||||||||||||||||||||
| it only has an effect when notifications are queued up on the gateway. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ### Multiple pushers on one origin | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Also note that because you can only have one push subscription per service worker, | ||||||||||||||||||||||||
| and hence per origin, you might create pushers for different accounts with the same | ||||||||||||||||||||||||
| p256dh push key. To prevent the server from removing other pushers with the same | ||||||||||||||||||||||||
| push key for your other users, you should set `append` to `true` when uploading | ||||||||||||||||||||||||
| your pusher. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ## Notification format | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| The notification as received by your web application will contain the following keys | ||||||||||||||||||||||||
| (assuming non-null values were sent by the homeserver). These are the | ||||||||||||||||||||||||
| same as specified in [the push gateway spec](https://matrix.org/docs/spec/push_gateway/r0.1.0#post-matrix-push-v1-notify), | ||||||||||||||||||||||||
| but the sub-keys of `counts` (`unread` and `missed_calls`) are flattened into | ||||||||||||||||||||||||
| the notification object. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
| room_id | ||||||||||||||||||||||||
| room_name | ||||||||||||||||||||||||
| room_alias | ||||||||||||||||||||||||
| membership | ||||||||||||||||||||||||
| event_id | ||||||||||||||||||||||||
| sender | ||||||||||||||||||||||||
| sender_display_name | ||||||||||||||||||||||||
| user_is_target | ||||||||||||||||||||||||
| type | ||||||||||||||||||||||||
| content | ||||||||||||||||||||||||
| unread | ||||||||||||||||||||||||
| missed_calls | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,14 @@ | |
| except ImportError: | ||
| HAS_AUTHLIB = False | ||
|
|
||
| # Determine whether pywebpush is installed. | ||
| try: | ||
| import pywebpush # noqa: F401 | ||
MatMaul marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| HAS_PYWEBPUSH = True | ||
| except ImportError: | ||
| HAS_PYWEBPUSH = False | ||
|
|
||
| if TYPE_CHECKING: | ||
| # Only import this if we're type checking, as it might not be installed at runtime. | ||
| from authlib.jose.rfc7517 import JsonWebKey | ||
|
|
@@ -365,6 +373,28 @@ class MSC3866Config: | |
| require_approval_for_new_accounts: bool = False | ||
|
|
||
|
|
||
| @attr.s(auto_attribs=True, frozen=True, slots=True) | ||
| class MSC4174Config: | ||
| """Configuration for MSC4174: webpush push kind""" | ||
|
|
||
| enabled: bool = attr.ib(default=False, validator=attr.validators.instance_of(bool)) | ||
|
|
||
| @enabled.validator | ||
| def _check_enabled(self, attribute: attr.Attribute, value: bool) -> None: | ||
| # Only allow enabling MSC4174 if pywebpush is installed | ||
| if value and not HAS_PYWEBPUSH: | ||
| raise ConfigError( | ||
| "MSC4174 is enabled but pywebpush is not installed. " | ||
| "Please install pywebpush to use MSC4174.", | ||
| ("experimental", "msc4174", "enabled"), | ||
| ) | ||
|
|
||
| vapid_contact_email: str = "" | ||
| vapid_private_key: str = "" | ||
| vapid_app_server_key: str = "" | ||
| ttl: int = 12 * 60 * 60 | ||
MatMaul marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| class ExperimentalConfig(Config): | ||
| """Config section for enabling experimental features | ||
|
|
||
|
|
@@ -606,3 +636,26 @@ def read_config( | |
| # Note that sticky events persisted before this feature is enabled will not be | ||
| # considered sticky by the local homeserver. | ||
| self.msc4354_enabled: bool = experimental.get("msc4354_enabled", False) | ||
|
|
||
| # MSC4380: Invite blocking | ||
| self.msc4380_enabled: bool = experimental.get("msc4380_enabled", False) | ||
|
|
||
| # MSC4174: webpush push kind | ||
| raw_msc4174_config = experimental.get("msc4174", {}) | ||
| self.msc4174 = MSC4174Config(**raw_msc4174_config) | ||
| if self.msc4174.enabled: | ||
| if not self.msc4174.vapid_contact_email: | ||
| raise ConfigError( | ||
| "'vapid_contact_email' must be provided when enabling WebPush support", | ||
| ("experimental", "msc4174", "vapid_contact_email"), | ||
| ) | ||
| if not self.msc4174.vapid_private_key: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is a file path, we should use the standard idiom:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be either a path or directly the key value so I am not sure here, cf https://github.com/element-hq/synapse/pull/17987/changes/BASE..e2fe3b17b3afa47205199799d20396d766013576#diff-88195b8cc507a14d155b4e50509ad80c9ddf829894ad48a530ff5d380663ce60R142. |
||
| raise ConfigError( | ||
| "'vapid_private_key' must be provided when enabling WebPush support", | ||
| ("experimental", "msc4174", "vapid_private_key"), | ||
| ) | ||
| if not self.msc4174.vapid_app_server_key: | ||
| raise ConfigError( | ||
| "'vapid_app_server_key' must be provided when enabling WebPush support", | ||
| ("experimental", "msc4174", "vapid_app_server_key"), | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,7 +117,6 @@ def __init__(self, hs: "HomeServer", pusher_config: PusherConfig): | |
| self.device_display_name = pusher_config.device_display_name | ||
| self.device_id = pusher_config.device_id | ||
| self.pushkey_ts = pusher_config.ts | ||
| self.data = pusher_config.data | ||
| self.backoff_delay = HttpPusher.INITIAL_BACKOFF_SEC | ||
| self.failing_since = pusher_config.failing_since | ||
| self.timed_call: Optional[IDelayedCall] = None | ||
|
|
@@ -129,9 +128,9 @@ def __init__(self, hs: "HomeServer", pusher_config: PusherConfig): | |
|
|
||
| self.push_jitter_delay_ms = hs.config.push.push_jitter_delay_ms | ||
|
|
||
| self.data = pusher_config.data | ||
| if self.data is None: | ||
| if pusher_config.data is None: | ||
| raise PusherConfigException("'data' key can not be null for HTTP pusher") | ||
| self.data = pusher_config.data | ||
|
|
||
| # Check if badge counts should be disabled for this push gateway | ||
| self.disable_badge_count = self.hs.config.experimental.msc4076_enabled and bool( | ||
|
|
@@ -144,22 +143,28 @@ def __init__(self, hs: "HomeServer", pusher_config: PusherConfig): | |
| pusher_config.pushkey, | ||
| ) | ||
|
|
||
| # Validate that there's a URL and it is of the proper form. | ||
| if "url" not in self.data: | ||
| raise PusherConfigException("'url' required in data for HTTP pusher") | ||
|
|
||
| url = self.data["url"] | ||
| if not isinstance(url, str): | ||
| raise PusherConfigException("'url' must be a string") | ||
| url_parts = urllib.parse.urlparse(url) | ||
| # Note that the specification also says the scheme must be HTTPS, but | ||
| # it isn't up to the homeserver to verify that. | ||
| if url_parts.path != "/_matrix/push/v1/notify": | ||
| raise PusherConfigException( | ||
| "'url' must have a path of '/_matrix/push/v1/notify'" | ||
| ) | ||
| self.url = "" | ||
| if pusher_config.kind == "http": | ||
MatMaul marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| # Validate that there's a URL and it is of the proper form. | ||
| if "url" not in self.data: | ||
| raise PusherConfigException("'url' required in data for HTTP pusher") | ||
|
|
||
| url = self.data["url"] | ||
| if not isinstance(url, str): | ||
| raise PusherConfigException("'url' must be a string") | ||
| url_parts = urllib.parse.urlparse(url) | ||
| # Note that the specification also says the scheme must be HTTPS, but | ||
| # it isn't up to the homeserver to verify that. | ||
| if url_parts.path != "/_matrix/push/v1/notify": | ||
| raise PusherConfigException( | ||
| "'url' must have a path of '/_matrix/push/v1/notify'" | ||
| ) | ||
| self.url = url | ||
|
|
||
| self.data_minus_url = {} | ||
| self.data_minus_url.update(self.data) | ||
| del self.data_minus_url["url"] | ||
|
|
||
| self.url = url | ||
| self.http_client = hs.get_proxied_blocklisted_http_client() | ||
| self.data_minus_url = {} | ||
| self.data_minus_url.update(self.data) | ||
|
|
@@ -196,7 +201,14 @@ async def _update_badge(self) -> None: | |
| ) | ||
| if self.badge_count_last_call is None or self.badge_count_last_call != badge: | ||
| self.badge_count_last_call = badge | ||
| await self._send_badge(badge) | ||
| if await self.send_badge(badge): | ||
| http_badges_processed_counter.labels( | ||
| **{SERVER_NAME_LABEL: self.server_name} | ||
| ).inc() | ||
| else: | ||
| http_badges_failed_counter.labels( | ||
| **{SERVER_NAME_LABEL: self.server_name} | ||
| ).inc() | ||
|
|
||
| def on_timer(self) -> None: | ||
| self._start_processing() | ||
|
|
@@ -529,7 +541,7 @@ async def dispatch_push_event( | |
|
|
||
| return res | ||
|
|
||
| async def _send_badge(self, badge: int) -> None: | ||
| async def send_badge(self, badge: int) -> bool: | ||
| """ | ||
| Args: | ||
| badge: number of unread messages | ||
|
Comment on lines
545
to
547
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should update comment doc with what the returned bool represents (same for the other |
||
|
|
@@ -553,13 +565,9 @@ async def _send_badge(self, badge: int) -> None: | |
| } | ||
| try: | ||
| await self.http_client.post_json_get_json(self.url, d) | ||
| http_badges_processed_counter.labels( | ||
| **{SERVER_NAME_LABEL: self.server_name} | ||
| ).inc() | ||
| return True | ||
| except Exception as e: | ||
| logger.warning( | ||
| "Failed to send badge count to %s: %s %s", self.name, type(e), e | ||
| ) | ||
| http_badges_failed_counter.labels( | ||
| **{SERVER_NAME_LABEL: self.server_name} | ||
| ).inc() | ||
| return False | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MatMaul Any interest in continuing this implementation?
WebPush came up recently as an alternative to MSC3013: 'Encrypted Push' in terms of leaking
room_id/event_idmetadata.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am at it again 💪