Add WebPush support (fix #337)#4233
Open
benjamin-voisin wants to merge 24 commits intominiflux:mainfrom
Open
Conversation
The table webpush_subscriptions will store subscriptions in order to send webpush. A user can have multiple subscriptions. In order to send a webpush notification 3 informations are needed : the endpoint, the encryption key (p256dh) and the authentication method.
…ic key The vapid keys are necessary to authenticate miniplux to the push service. The keys should be generated once for the server.
I misunderstood how the migration were done and put the create table inside the first migration instead of at the end of the migrations
The vapid public key of the server is served under the basepath/vapid endpoint.
The endpoint is on /register-webpush and wants a form containing 3 informations : the endpoint, the authentication method (vapid or webpush) and the encryption key.
duplicates The webpush_subscriptions database now uses the endpoint as a primary key, as the endpoint should be unique per subscription. This allows to easilly avoid duplicates within the table.
Multiple authentication methods exists for webpush, namely "vapid" (used by firefox) and "webpush" used by chrome. The webpush-go lib currently only supports the vapid methods, but this is being resolved by SherClockHolmes/webpush-go#84
Two authentications methods currently co-exists. Currently, only the vapid methods is supported by the webpush-go lib, but once this is added, it should be very easy to add this data to be able to send notification to Chrome browsers : simply add "AuthScheme" to the webpush.SendNotification call.
Webpush allows to add the email of the service sending the webpush notifications. This corresponds to the mail of the miniflux server admin, so a new config vaule has been added. From testing, an empty values is not rejected, so this is not mandatory.
Author
|
I get an error with the test of the new config: I don't understand why the first option now is |
Collaborator
|
The new dependency looks short enough that it could be properly re-implemented in miniflux, instead of adding a new third-party dependency for a little-used feature. |
Author
|
You'r right, I will look at the spec and try to re implement something |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds support for sending webpush notifications using the webpush-go library, fixing #337
Currently, the push notifications don't work on chrome, because their push-server require a new authentication scheme, but this will soon be added to the webpush-go library (see webpush-go#PR 84). All the information needed to make this work are already present in this PR, the only change will be to uncomment the "AuthScheme" line in the SendPush function.
The important changes are:
/vapidthat serves the public vapid key. This key is created once at the migration step.register-webpushendpoint for the user to send the webpush subscription parameters (endpoint and encryption keys)app.jsfile to locally register for webpush, and send this information to the correct endpointWhat I'm not sure about is the approach to have regarding the subscriptions. Currently, the subscription is re-sent every time a page is loaded, because if it were done only once the subscription is made client-side, then a not logged user will send the subscription before being logged, and so it will not be registered. Maybe a settings “enable notifications” that triggers the subscription and send the request ?
Also, maybe an API endpoint might be usefull ? This would allow third-party client to register to notification, for example using UnifiedPush
Have you followed these guidelines?
Finally, I just want to say that it really has been a pleasure to work on this. It's my first time using Go, and the quality and simplicity of the codebase made it a real breeze to find what I needed to do and where to add it. Thanks for the great work, I liked miniflux before, I like it even more after seeing the codebase.