Add WebPush support (fix #337)#4233
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.
|
I get an error with the test of the new config: I don't understand why the first option now is |
|
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. |
|
You'r right, I will look at the spec and try to re implement something |
There was a problem hiding this comment.
Pull request overview
Adds WebPush notifications to Miniflux so browsers (notably Chrome) can receive push notifications for newly discovered entries, backed by persisted VAPID keys and stored push subscriptions.
Changes:
- Add DB tables + migration to store VAPID keys and WebPush subscriptions.
- Add server endpoints to expose the VAPID public key and to register client subscriptions.
- Add client/service-worker logic to subscribe and display push notifications; send a push for each new entry.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 26 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/ui/webpush.go | Adds /register-webpush UI handler to store subscription data. |
| internal/ui/ui.go | Registers the new UI route for subscription registration. |
| internal/ui/static/js/service_worker.js | Handles push events and displays notifications. |
| internal/ui/static/js/app.js | Subscribes to WebPush and POSTs subscriptions to the server. |
| internal/storage/webpush.go | Storage layer for VAPID keys and subscription CRUD. |
| internal/reader/webpush/webpush.go | Sends WebPush notifications via webpush-go. |
| internal/reader/processor/processor.go | Triggers WebPush notifications on new entries during feed processing. |
| internal/model/webpush.go | Adds models for webpush subscriptions and notification payloads. |
| internal/http/server/vapid.go | Adds /vapid handler to return the VAPID public key. |
| internal/http/server/routes.go | Wires /vapid into the main router. |
| internal/database/migrations.go | Creates webpush_subscriptions + vapid_key tables and generates initial VAPID keys. |
| internal/config/options.go | Adds ADMIN_EMAIL config option used as the WebPush subscriber contact. |
| go.mod | Adds github.com/SherClockHolmes/webpush-go dependency. |
| go.sum | Records module checksums for the new dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| // Make the public VAPID key accessible | ||
| appMux.Handle("/vapid", newVAPIDProbe(store)) |
There was a problem hiding this comment.
appMux.Handle("/vapid", ...) registers the handler for all HTTP methods. Since this endpoint is meant to serve the public key, restrict it to GET /vapid (to match how other routes are declared) to reduce attack surface and make intent explicit.
| appMux.Handle("/vapid", newVAPIDProbe(store)) | |
| appMux.Handle("GET /vapid", newVAPIDProbe(store)) |
| if err != nil { | ||
| http.Error(w, fmt.Sprintf("VAPID public key not found. Error: %q", err), http.StatusServiceUnavailable) | ||
| return | ||
| } | ||
| w.WriteHeader(http.StatusOK) | ||
| w.Write([]byte(publicKey)) |
There was a problem hiding this comment.
The /vapid handler returns plain text but doesn't set a Content-Type, and on errors it echoes the raw internal error back to the client. Set an explicit content type (e.g., text/plain; charset=utf-8) and avoid including internal error details in the response body (log server-side instead).
| sendPOSTRequest('/register-webpush', { | ||
| endpoint: subscription.endpoint, | ||
| key: _arrayBufferToBase64(subscription.getKey("p256dh")), | ||
| auth: _arrayBufferToBase64(subscription.getKey("auth")), | ||
| authscheme: authScheme, | ||
| }) |
There was a problem hiding this comment.
sendPOSTRequest('/register-webpush', ...) is also used in subscribeWebpush() with an absolute path, which breaks subpath (BASE_PATH) deployments. Use a base-path-aware URL (e.g., provided via a data-* attribute from the template) here as well.
| // If we get a 401, 404 or 410 return codes, that means that the | ||
| // subscription is invalid and needs to be removed. | ||
| if response.StatusCode == 401 || response.StatusCode == 404 || response.StatusCode == 410 || err != nil { | ||
| store.RemoveUserSubscription(userID, subs.Endpoint) | ||
| slog.Debug( | ||
| "Removed webpush subscription", | ||
| slog.String("endpoint", subs.Endpoint), | ||
| ) | ||
| } |
There was a problem hiding this comment.
This removes the user subscription on any send error (err != nil), which can drop valid subscriptions on transient failures (network issues, push service hiccups). Only delete subscriptions when the push service indicates the subscription is invalid (e.g., 401/404/410), and log/return other errors without deleting.
| sendPOSTRequest('/register-webpush', { | ||
| endpoint: subscription.endpoint, | ||
| key: _arrayBufferToBase64(subscription.getKey("p256dh")), | ||
| auth: _arrayBufferToBase64(subscription.getKey("auth")), | ||
| authscheme: authScheme, | ||
| }) |
There was a problem hiding this comment.
sendPOSTRequest('/register-webpush', ...) hard-codes an absolute path, which bypasses BASE_PATH and will fail for subpath deployments. Add a base-path-aware URL in the layout (similar to data-entries-status-url, etc.) and use that value here.
| parsedStringValue: "", | ||
| rawValue: "", | ||
| valueType: stringType, | ||
| targetKey: "ADMIN_EMAIL", |
There was a problem hiding this comment.
ADMIN_EMAIL is declared as a stringType option but sets targetKey, which is only used for *_FILE options in the config parser. Drop targetKey here to avoid confusion and keep option definitions consistent.
| targetKey: "ADMIN_EMAIL", |
| Endpoint string `json:"endpoint"` | ||
| Key string `json:"key"` | ||
| Auth string `json:"auth"` | ||
| AuthScheme string `json:"authscheme"` |
There was a problem hiding this comment.
AuthScheme is persisted (model + DB column) and sent from the client, but it is not used when sending notifications (the only AuthScheme usage is a commented line). Since DB schema changes are permanent, consider either wiring this through end-to-end or removing the column/field until the library supports it to avoid storing unused data.
| Endpoint string `json:"endpoint"` | |
| Key string `json:"key"` | |
| Auth string `json:"auth"` | |
| AuthScheme string `json:"authscheme"` | |
| Endpoint string `json:"endpoint"` | |
| Key string `json:"key"` | |
| Auth string `json:"auth"` |
| func (s *Storage) GetVAPIDKeys() (string, string, error) { | ||
| var privateKey string | ||
| var publicKey string | ||
| query := `SELECT private_key, public_key FROM vapid_key` | ||
|
|
||
| err := s.db.QueryRow(query).Scan(&privateKey, &publicKey) | ||
|
|
||
| return privateKey, publicKey, err |
There was a problem hiding this comment.
GetVAPIDKeys() uses SELECT ... FROM vapid_key without LIMIT 1/ordering. If the table ever ends up with multiple rows (even accidentally), the returned row is nondeterministic. Consider enforcing single-row semantics (schema constraint) and/or adding LIMIT 1 to the query.
| primary key (endpoint) | ||
| ); | ||
|
|
||
| CREATE TABLE vapid_key ( |
There was a problem hiding this comment.
vapid_key is intended to store a single keypair but the schema has no primary key/constraint to enforce single-row semantics. Consider adding an id primary key with a single row, or a CHECK/UNIQUE constraint (e.g., a constant key) so accidental multiple inserts don't create ambiguous reads.
| CREATE TABLE vapid_key ( | |
| CREATE TABLE vapid_key ( | |
| id smallint not null default 1 primary key check (id = 1), |
| .then(function(registration) { | ||
| return fetch('/vapid').then(response => response.text()).then(vapidPublicKey => { | ||
| return registration[0].pushManager.subscribe({ | ||
| userVisibleOnly: true, | ||
| applicationServerKey: urlBase64ToUint8Array(vapidPublicKey), | ||
| }); | ||
| }) | ||
| }) | ||
| .then(function(subscription) { | ||
| // Send the subscription to the server | ||
| let authScheme = 'vapid'; | ||
| const userAgent = navigator.userAgent; | ||
| if (userAgent.indexOf("Chrome") > -1 && userAgent.indexOf("Edg") === -1) { | ||
| authScheme = 'webpush'; | ||
| } | ||
| sendPOSTRequest('/register-webpush', { | ||
| endpoint: subscription.endpoint, | ||
| key: _arrayBufferToBase64(subscription.getKey("p256dh")), | ||
| auth: _arrayBufferToBase64(subscription.getKey("auth")), | ||
| authscheme: authScheme, | ||
| }) |
There was a problem hiding this comment.
fetch('/vapid') uses an absolute path, which will break when Miniflux is served under a non-empty BASE_PATH (templates use routePath for this reason). Use a base-path-aware URL (e.g., inject it via a data-* attribute like other endpoints) instead of hard-coding /vapid.
| .then(function(registration) { | |
| return fetch('/vapid').then(response => response.text()).then(vapidPublicKey => { | |
| return registration[0].pushManager.subscribe({ | |
| userVisibleOnly: true, | |
| applicationServerKey: urlBase64ToUint8Array(vapidPublicKey), | |
| }); | |
| }) | |
| }) | |
| .then(function(subscription) { | |
| // Send the subscription to the server | |
| let authScheme = 'vapid'; | |
| const userAgent = navigator.userAgent; | |
| if (userAgent.indexOf("Chrome") > -1 && userAgent.indexOf("Edg") === -1) { | |
| authScheme = 'webpush'; | |
| } | |
| sendPOSTRequest('/register-webpush', { | |
| endpoint: subscription.endpoint, | |
| key: _arrayBufferToBase64(subscription.getKey("p256dh")), | |
| auth: _arrayBufferToBase64(subscription.getKey("auth")), | |
| authscheme: authScheme, | |
| }) | |
| .then(function(registrations) { | |
| const registration = registrations[0]; | |
| const vapidURL = new URL('vapid', registration.scope).toString(); | |
| const registerWebpushURL = new URL('register-webpush', registration.scope).toString(); | |
| return fetch(vapidURL) | |
| .then(response => response.text()) | |
| .then(vapidPublicKey => { | |
| return registration.pushManager.subscribe({ | |
| userVisibleOnly: true, | |
| applicationServerKey: urlBase64ToUint8Array(vapidPublicKey), | |
| }); | |
| }) | |
| .then(subscription => ({ subscription, registerWebpushURL })); | |
| }) | |
| .then(function(result) { | |
| // Send the subscription to the server | |
| let authScheme = 'vapid'; | |
| const userAgent = navigator.userAgent; | |
| if (userAgent.indexOf("Chrome") > -1 && userAgent.indexOf("Edg") === -1) { | |
| authScheme = 'webpush'; | |
| } | |
| sendPOSTRequest(result.registerWebpushURL, { | |
| endpoint: result.subscription.endpoint, | |
| key: _arrayBufferToBase64(result.subscription.getKey("p256dh")), | |
| auth: _arrayBufferToBase64(result.subscription.getKey("auth")), | |
| authscheme: authScheme, | |
| }); |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within 14 days. |
|
This pull request has been automatically closed due to inactivity. Please feel free to reopen it if you would like to continue working on it. |
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.