-
Notifications
You must be signed in to change notification settings - Fork 80
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
Add Web Push Extension #471
base: master
Are you sure you want to change the base?
Conversation
Sorry if this is obvious, but if the server needs to wait for the client to send the |
It's used to signal the client that the commands are supported. Could use an |
capability doesn't prevent server from changing the behavior after cap was enabled. There's even |
Nah, I mean that a cap would inform the server that the client supports the ext. For instance the chat history spec uses this to make servers not send playback on connect when the |
Okay, and why this information is needed upon connect? I understand why it's needed for chathistory |
As said above, I'm not necessarily opposed to making this an |
extensions/webpush.md
Outdated
## Implementation | ||
|
||
The `webpush` capability allows clients to subscribe to Web Push and receive | ||
notifications for messages of interest. |
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.
How does a user/client indicate which messages would be of interest? There doesn't appear to be mechanisms in-place in the currently offered commands to indicate which messages would be of interest.
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.
Yup, as discussed in #ircv3, this part isn't addressed yet. For now, it's spec'ed as a "server-defined subset of IRC messages".
I think there are multiple ways to go about this:
- Keep as-is, let servers decide completely. Leave it to a future extension to let clients indicate messages of interest. This can be useful for chathistory as well, to avoid fetching all unread history when connecting.
- Indicate a minimal set of messages to forward (e.g. PRIVMSG/NOTICE/INVITE with user as target, plus PRIVMSG/NOTICE highlights). Let servers send more if they want to. Clients can always ignore some push notifications.
- Build a webpush-specific command to indicate the messages of interest. This is a slippery slope, because the rules can be pretty complicated. If we go down this road I'd rather keep it simple and have just simple word matching.
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 think (1) is the superior option here.
Two issues that came up in discussion with @kylef:
|
Yeah, the size limit is going to be an issue. FCM alone also has a 4096 bytes limit for the payload.
I don't think JSON would help, the limit still applies regardless of the format of the payload. Also, servers can extend messages already with tags. Am I missing something? |
Sorry, that was unclear. I meant something like: abandon RFC1459-and-derivatives entirely in the format of the push message, parse prefix/tags/command/params directly into JSON. |
How is this going to help with the size limit of the push notification payload? |
extensions/webpush.md
Outdated
|
||
The `<endpoint>` is an URL pointing to a push server, which can be used to send push messages for this particular subscription. | ||
|
||
`<keys>` is a string encoded in the message-tag format. The values are [URL-safe base64-encoded][RFC 4648 section 5]. It MUST contain at least: |
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.
This uses the URL-safe base64 encoding because that's what the W3C API uses. So this allows Web clients to just use the result of subscription.toJSON().keys
as-is.
But maybe that isn't such a good idea. The W3C API also doesn't provide any function to encode an arbitrary byte buffer to URL-safe base64, so users need to roll out their own if we use it elsewhere.
Instead we could just use regular base64, just like SASL does. Users can call subscription.getKey()
.
Do you intent to add a command to view the list of subscriptions? I could see how as a user this could be useful to debug problems and be able to manually clean up or unsubscribe old clients for which you do not know the subscription URL for. This could very well be something that a NickServ or similar service could facilitate for a user and not be included in the spec though. |
Yeah, if it's just a debugging tool, I think I'd rather let servers include it as a special service or custom commands. Also need to investigate if/how subscription expiration can be integrated. |
UnifiedPush is basically Web Push without any kind of encryption. I've been discussing with them to add encryption, they seem open to the idea (they considered Web Push in the past but didn't understand fully how encryption works). |
My plan is to go ahead and ship this as a soju vendored extension, then gather field testing feedback and report back what I've learnt here. |
References: ircv3/ircv3-specifications#471 Co-authored-by: delthas <delthas@dille.cc>
Allows clients to update their keys.
Fixes some race conditions.
This is necessary to let clients know when the registration has completed.
References: ircv3/ircv3-specifications#471 Co-authored-by: delthas <delthas@dille.cc>
References: ircv3/ircv3-specifications#471 Co-authored-by: delthas <delthas@dille.cc>
References: ircv3/ircv3-specifications#471 Co-authored-by: delthas <delthas@dille.cc>
References: ircv3/ircv3-specifications#471 Co-authored-by: delthas <delthas@dille.cc>
References: ircv3/ircv3-specifications#471 Co-authored-by: delthas <delthas@dille.cc>
References: ircv3/ircv3-specifications#471 Co-authored-by: delthas <delthas@dille.cc>
References: ircv3/ircv3-specifications#471 Co-authored-by: delthas <delthas@dille.cc>
References: ircv3/ircv3-specifications#471 Co-authored-by: delthas <delthas@dille.cc>
The vendored extension has been merged in soju and goguma. |
Goguma and pushgarden now support Apple Push Notification service, as more evidence that this approach works on all platforms. |
The vendored extension is now supported by Igloo and Ergo. |
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.
Let me say, thanks very much for your work on this spec; it's well-designed and solves an important problem.
More discussion of implementation considerations and security considerations would be helpful. For example, the implications of sending a POST to a user-chosen address should be discussed in more detail. The Soju restrictions (which I copied in Ergo) are to only allow https endpoints, and to disallow contacting loopback or internal IP addresses.
|
||
If the server supports [Voluntary Application Server Identification (VAPID)][RFC 8292] and the client has enabled the `draft/webpush` capability, the server MUST advertise its public key in the `VAPID` ISUPPORT token. This key can be used to verify notifications upon reception by the Web Push server. | ||
|
||
The value MUST be the [URL-safe base64-encoded][RFC 4648 section 5] public key usable with the Elliptic Curve Digital Signature Algorithm (ECDSA) over the P-256 curve. The value MUST NOT change over the lifetime of the connection to avoid race conditions. |
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.
What is the connection referenced here? Is it the transient IRC connection established by the client in order to send WEBPUSH REGISTER
?
It's not clear on the basis of this spec how to do key rotation, or if it's even possible (I'm not sure what the right answer is, but the MUST NOT here seems excessive).
|
||
### Errors | ||
|
||
Errors are returned using the standard replies syntax. |
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 added a FAIL WEBPUSH FORBIDDEN
code to address situations where the command is administratively disallowed for any reason (e.g. too many existing subscriptions, or the user is not logged into an account).
|
||
The `draft/webpush` capability allows clients to subscribe to Web Push and receive notifications for messages of interest. | ||
|
||
Once a client has subscribed, the server will send push notifications for a server-defined subset of IRC messages. Each push notification MUST contain exactly one IRC message as the payload, without the final CRLF. |
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.
Is it intentional that this disallows sending a draft/multiline?
I'm not sure what the right approach is (the practical upper limit on push message sizes appears to be 4096 bytes, and multilines could exceed that), but it's worth thinking about. In my implementation, you'll get the first line of the multiline in the push message (even if it doesn't actually contain the highlight), and it will be tagged with the overall msgid of the multiline (which is the normal fallback behavior).
A primary goal of this spec is to not tie the extension to any proprietary push service. This is achieved by using the standard Web Push protocol. Web Push also provides encryption of the payload.
IRC clients are responsible for providing a push endpoint to the IRC server. This is usually provided by the platform (e.g. by the browser for the Web). IRC servers don't need to register or anything like that, all they need is send an HTTP request when they wish to deliver a push notification.
See the spec for more details.
Not mentioned in the spec is the integration with proprietary push services. For instance, Android apps can't (unfortunately) use Web Push: they must use Firebase Cloud Messaging (FCM). Since FCM is also used on Chrome for their Web Push implementation, I've tried to see if there was a way to figure out some Web Push parameters from the FCM ones, but it sounds like the Web Push functionality is limited to the Chrome API keys.
My approach is to introduce a relay which listens for Web Push messages and forwards them to FCM. This does require an additional moving part that IRC clients need to rely on, however this sounds like a reasonable trade-off to me. Since the relay doesn't see any clear-text privacy-sensitive information (payload is encrypted by the IRC server, no client IP address is visible), I could provide a free relay service for client developers who don't want to setup their own.
So far, I've implemented proof-of-concepts for soju and two client platforms:
References:
This is an incomplete specification. I opened this PR to gather feedback on the approach. Let me know what you think!
WEBPUSH UNREGISTER
Figure out how to specify "messages of interest"Consider turning the cap into anISUPPORT
ISUPPORT