From 75a4b5bd888e7211af8d8018544404c7b5b07847 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Thu, 8 Jun 2023 12:20:12 -0400 Subject: [PATCH] Small refactor --- server/errors.go | 1 + server/log.go | 2 +- server/server_web_push.go | 95 +++++++++++------------- server/types.go | 18 +++-- server/web_push.go | 26 ++++--- web/public/static/langs/en.json | 10 +-- web/src/components/Preferences.jsx | 7 +- web/src/components/SubscriptionPopup.jsx | 52 +++++-------- 8 files changed, 98 insertions(+), 113 deletions(-) diff --git a/server/errors.go b/server/errors.go index c8d96edb..27ba3df0 100644 --- a/server/errors.go +++ b/server/errors.go @@ -141,5 +141,6 @@ var ( errHTTPInternalError = &errHTTP{50001, http.StatusInternalServerError, "internal server error", "", nil} errHTTPInternalErrorInvalidPath = &errHTTP{50002, http.StatusInternalServerError, "internal server error: invalid path", "", nil} errHTTPInternalErrorMissingBaseURL = &errHTTP{50003, http.StatusInternalServerError, "internal server error: base-url must be be configured for this feature", "https://ntfy.sh/docs/config/", nil} + errHTTPInternalErrorWebPushUnableToPublish = &errHTTP{50004, http.StatusInternalServerError, "internal server error: unable to publish web push message", "", nil} errHTTPInsufficientStorageUnifiedPush = &errHTTP{50701, http.StatusInsufficientStorage, "cannot publish to UnifiedPush topic without previously active subscriber", "", nil} ) diff --git a/server/log.go b/server/log.go index 9f922b6a..978d0593 100644 --- a/server/log.go +++ b/server/log.go @@ -29,7 +29,7 @@ const ( tagResetter = "resetter" tagWebsocket = "websocket" tagMatrix = "matrix" - tagWebPush = "web_push" + tagWebPush = "webpush" ) var ( diff --git a/server/server_web_push.go b/server/server_web_push.go index caccce92..6b3e4adc 100644 --- a/server/server_web_push.go +++ b/server/server_web_push.go @@ -30,27 +30,19 @@ var webPushEndpointAllowRegex = regexp.MustCompile(webPushEndpointAllowRegexStr) func (s *Server) handleWebPushUpdate(w http.ResponseWriter, r *http.Request, v *visitor) error { payload, err := readJSONWithLimit[webPushSubscriptionPayload](r.Body, jsonBodyBytesLimit, false) - if err != nil || payload.BrowserSubscription.Endpoint == "" || payload.BrowserSubscription.Keys.P256dh == "" || payload.BrowserSubscription.Keys.Auth == "" { return errHTTPBadRequestWebPushSubscriptionInvalid - } - - if !webPushEndpointAllowRegex.MatchString(payload.BrowserSubscription.Endpoint) { + } else if !webPushEndpointAllowRegex.MatchString(payload.BrowserSubscription.Endpoint) { return errHTTPBadRequestWebPushEndpointUnknown - } - - if len(payload.Topics) > webPushTopicSubscribeLimit { + } else if len(payload.Topics) > webPushTopicSubscribeLimit { return errHTTPBadRequestWebPushTopicCountTooHigh } - - u := v.User() - topics, err := s.topicsFromIDs(payload.Topics...) if err != nil { return err } - if s.userManager != nil { + u := v.User() for _, t := range topics { if err := s.userManager.Authorize(u, t.ID, user.PermissionRead); err != nil { logvr(v, r).With(t).Err(err).Debug("Access to topic %s not authorized", t.ID) @@ -58,11 +50,9 @@ func (s *Server) handleWebPushUpdate(w http.ResponseWriter, r *http.Request, v * } } } - if err := s.webPush.UpdateSubscriptions(payload.Topics, v.MaybeUserID(), payload.BrowserSubscription); err != nil { return err } - return s.writeJSON(w, newSuccessResponse()) } @@ -70,69 +60,68 @@ func (s *Server) publishToWebPushEndpoints(v *visitor, m *message) { subscriptions, err := s.webPush.SubscriptionsForTopic(m.Topic) if err != nil { logvm(v, m).Err(err).Warn("Unable to publish web push messages") - return } - - for i, xi := range subscriptions { - go func(i int, sub webPushSubscription) { - ctx := log.Context{"endpoint": sub.BrowserSubscription.Endpoint, "username": sub.UserID, "topic": m.Topic, "message_id": m.ID} - - s.sendWebPushNotification(newWebPushPayload(fmt.Sprintf("%s/%s", s.config.BaseURL, m.Topic), *m), &sub, &ctx) - }(i, xi) + payload, err := json.Marshal(newWebPushPayload(fmt.Sprintf("%s/%s", s.config.BaseURL, m.Topic), m)) + if err != nil { + log.Tag(tagWebPush).Err(err).Warn("Unable to marshal expiring payload") + return + } + for _, subscription := range subscriptions { + ctx := log.Context{"endpoint": subscription.BrowserSubscription.Endpoint, "username": subscription.UserID, "topic": m.Topic, "message_id": m.ID} + if err := s.sendWebPushNotification(payload, subscription, &ctx); err != nil { + log.Tag(tagWebPush).Err(err).Fields(ctx).Warn("Unable to publish web push message") + } } } +// TODO this should return error +// TODO the updated_at field is not actually updated ever + func (s *Server) expireOrNotifyOldSubscriptions() { subscriptions, err := s.webPush.ExpireAndGetExpiringSubscriptions(s.config.WebPushExpiryWarningDuration, s.config.WebPushExpiryDuration) if err != nil { log.Tag(tagWebPush).Err(err).Warn("Unable to publish expiry imminent warning") - + return + } else if len(subscriptions) == 0 { return } - - for i, xi := range subscriptions { - go func(i int, sub webPushSubscription) { - ctx := log.Context{"endpoint": sub.BrowserSubscription.Endpoint} - - s.sendWebPushNotification(newWebPushSubscriptionExpiringPayload(), &sub, &ctx) - }(i, xi) + payload, err := json.Marshal(newWebPushSubscriptionExpiringPayload()) + if err != nil { + log.Tag(tagWebPush).Err(err).Warn("Unable to marshal expiring payload") + return } - - log.Tag(tagWebPush).Debug("Expired old subscriptions and published %d expiry imminent warnings", len(subscriptions)) + go func() { + for _, subscription := range subscriptions { + ctx := log.Context{"endpoint": subscription.BrowserSubscription.Endpoint} + if err := s.sendWebPushNotification(payload, &subscription, &ctx); err != nil { + log.Tag(tagWebPush).Err(err).Fields(ctx).Warn("Unable to publish expiry imminent warning") + } + } + }() + log.Tag(tagWebPush).Debug("Expiring old subscriptions and published %d expiry imminent warnings", len(subscriptions)) } -func (s *Server) sendWebPushNotification(payload any, sub *webPushSubscription, ctx *log.Context) { - jsonPayload, err := json.Marshal(payload) - - if err != nil { - log.Tag(tagWebPush).Err(err).Fields(*ctx).Debug("Unable to publish web push message") - return - } - - resp, err := webpush.SendNotification(jsonPayload, &sub.BrowserSubscription, &webpush.Options{ +func (s *Server) sendWebPushNotification(message []byte, sub *webPushSubscription, ctx *log.Context) error { + resp, err := webpush.SendNotification(message, &sub.BrowserSubscription, &webpush.Options{ Subscriber: s.config.WebPushEmailAddress, VAPIDPublicKey: s.config.WebPushPublicKey, VAPIDPrivateKey: s.config.WebPushPrivateKey, - // Deliverability on iOS isn't great with lower urgency values, - // and thus we can't really map lower ntfy priorities to lower urgency values - Urgency: webpush.UrgencyHigh, + Urgency: webpush.UrgencyHigh, // iOS requires this to ensure delivery }) - if err != nil { - log.Tag(tagWebPush).Err(err).Fields(*ctx).Debug("Unable to publish web push message") + log.Tag(tagWebPush).Err(err).Fields(*ctx).Debug("Unable to publish web push message, removing endpoint") if err := s.webPush.RemoveByEndpoint(sub.BrowserSubscription.Endpoint); err != nil { - log.Tag(tagWebPush).Err(err).Fields(*ctx).Warn("Unable to expire subscription") + return err } - return + return err } - - // May want to handle at least 429 differently, but for now treat all errors the same - if !(200 <= resp.StatusCode && resp.StatusCode <= 299) { - log.Tag(tagWebPush).Fields(*ctx).Field("response", resp).Debug("Unable to publish web push message") + if (resp.StatusCode < 200 || resp.StatusCode > 299) && resp.StatusCode != 429 { + log.Tag(tagWebPush).Fields(*ctx).Field("response_code", resp.StatusCode).Debug("Unable to publish web push message, unexpected response") if err := s.webPush.RemoveByEndpoint(sub.BrowserSubscription.Endpoint); err != nil { - log.Tag(tagWebPush).Err(err).Fields(*ctx).Warn("Unable to expire subscription") + return err } - return + return errHTTPInternalErrorWebPushUnableToPublish.Fields(*ctx) } + return nil } diff --git a/server/types.go b/server/types.go index 3ddfcbba..a8a01301 100644 --- a/server/types.go +++ b/server/types.go @@ -467,15 +467,21 @@ type apiStripeSubscriptionDeletedEvent struct { Customer string `json:"customer"` } +// List of possible Web Push events +const ( + webPushMessageEvent = "message" + webPushExpiringEvent = "subscription_expiring" +) + type webPushPayload struct { - Event string `json:"event"` - SubscriptionID string `json:"subscription_id"` - Message message `json:"message"` + Event string `json:"event"` + SubscriptionID string `json:"subscription_id"` + Message *message `json:"message"` } -func newWebPushPayload(subscriptionID string, message message) webPushPayload { +func newWebPushPayload(subscriptionID string, message *message) webPushPayload { return webPushPayload{ - Event: "message", + Event: webPushMessageEvent, SubscriptionID: subscriptionID, Message: message, } @@ -487,7 +493,7 @@ type webPushControlMessagePayload struct { func newWebPushSubscriptionExpiringPayload() webPushControlMessagePayload { return webPushControlMessagePayload{ - Event: "subscription_expiring", + Event: webPushExpiringEvent, } } diff --git a/server/web_push.go b/server/web_push.go index a98d6ad8..0bea0857 100644 --- a/server/web_push.go +++ b/server/web_push.go @@ -63,11 +63,11 @@ func newWebPushStore(filename string) (*webPushStore, error) { func setupSubscriptionsDB(db *sql.DB) error { // If 'subscriptions' table does not exist, this must be a new database - rowsMC, err := db.Query(selectWebPushSubscriptionsCountQuery) + rows, err := db.Query(selectWebPushSubscriptionsCountQuery) if err != nil { return setupNewSubscriptionsDB(db) } - return rowsMC.Close() + return rows.Close() } func setupNewSubscriptionsDB(db *sql.DB) error { @@ -83,7 +83,6 @@ func (c *webPushStore) UpdateSubscriptions(topics []string, userID string, subsc return err } defer tx.Rollback() - if err = c.RemoveByEndpoint(subscription.Endpoint); err != nil { return err } @@ -107,26 +106,35 @@ func (c *webPushStore) AddSubscription(topic string, userID string, subscription return err } -func (c *webPushStore) SubscriptionsForTopic(topic string) (subscriptions []webPushSubscription, err error) { +func (c *webPushStore) SubscriptionsForTopic(topic string) (subscriptions []*webPushSubscription, err error) { rows, err := c.db.Query(selectWebPushSubscriptionsForTopicQuery, topic) if err != nil { return nil, err } defer rows.Close() - var data []webPushSubscription + var data []*webPushSubscription for rows.Next() { - i := webPushSubscription{} - err = rows.Scan(&i.BrowserSubscription.Endpoint, &i.BrowserSubscription.Keys.Auth, &i.BrowserSubscription.Keys.P256dh, &i.UserID) - if err != nil { + var userID, endpoint, auth, p256dh string + if err = rows.Scan(&endpoint, &auth, &p256dh, &userID); err != nil { return nil, err } - data = append(data, i) + data = append(data, &webPushSubscription{ + UserID: userID, + BrowserSubscription: webpush.Subscription{ + Endpoint: endpoint, + Keys: webpush.Keys{ + Auth: auth, + P256dh: p256dh, + }, + }, + }) } return data, nil } func (c *webPushStore) ExpireAndGetExpiringSubscriptions(warningDuration time.Duration, expiryDuration time.Duration) (subscriptions []webPushSubscription, err error) { + // TODO this should be two functions tx, err := c.db.Begin() if err != nil { return nil, err diff --git a/web/public/static/langs/en.json b/web/public/static/langs/en.json index d776ac05..9571272f 100644 --- a/web/public/static/langs/en.json +++ b/web/public/static/langs/en.json @@ -29,6 +29,8 @@ "action_bar_reservation_limit_reached": "Limit reached", "action_bar_send_test_notification": "Send test notification", "action_bar_clear_notifications": "Clear all notifications", + "action_bar_mute_notifications": "Mute notifications", + "action_bar_unmute_notifications": "Unmute notifications", "action_bar_unsubscribe": "Unsubscribe", "action_bar_toggle_mute": "Mute/unmute notifications", "action_bar_toggle_action_menu": "Open/close action menu", @@ -95,9 +97,6 @@ "notifications_no_subscriptions_description": "Click the \"{{linktext}}\" link to create or subscribe to a topic. After that, you can send messages via PUT or POST and you'll receive notifications here.", "notifications_example": "Example", "notifications_more_details": "For more information, check out the website or documentation.", - "notification_toggle_mute": "Mute", - "notification_toggle_unmute": "Unmute", - "notification_toggle_background": "Background notifications", "display_name_dialog_title": "Change display name", "display_name_dialog_description": "Set an alternative name for a topic that is displayed in the subscription list. This helps identify topics with complicated names more easily.", "display_name_dialog_placeholder": "Display name", @@ -170,7 +169,6 @@ "subscribe_dialog_subscribe_description": "Topics may not be password-protected, so choose a name that's not easy to guess. Once subscribed, you can PUT/POST notifications.", "subscribe_dialog_subscribe_topic_placeholder": "Topic name, e.g. phil_alerts", "subscribe_dialog_subscribe_use_another_label": "Use another server", - "subscribe_dialog_subscribe_enable_background_notifications_label": "Enable background notifications (web push)", "subscribe_dialog_subscribe_base_url_label": "Service URL", "subscribe_dialog_subscribe_button_generate_topic_name": "Generate name", "subscribe_dialog_subscribe_button_cancel": "Cancel", @@ -370,8 +368,8 @@ "prefs_reservations_dialog_description": "Reserving a topic gives you ownership over the topic, and allows you to define access permissions for other users over the topic.", "prefs_reservations_dialog_topic_label": "Topic", "prefs_reservations_dialog_access_label": "Access", - "prefs_notifications_web_push_title": "Enable web push notifications", - "prefs_notifications_web_push_description": "Enable this to receive notifications in the background even when ntfy isn't running", + "prefs_notifications_web_push_title": "Background notifications", + "prefs_notifications_web_push_description": "Receive notifications in the background via Web Push, even when app is not running", "prefs_notifications_web_push_enabled": "Enabled", "prefs_notifications_web_push_disabled": "Disabled", "reservation_delete_dialog_description": "Removing a reservation gives up ownership over the topic, and allows others to reserve it. You can keep, or delete existing messages and attachments.", diff --git a/web/src/components/Preferences.jsx b/web/src/components/Preferences.jsx index 7ef5a01e..7944f9be 100644 --- a/web/src/components/Preferences.jsx +++ b/web/src/components/Preferences.jsx @@ -242,11 +242,6 @@ const WebPushEnabled = () => { await prefs.setWebPushEnabled(ev.target.value); }; - // while loading - if (defaultEnabled == null) { - return null; - } - if (!notifier.pushPossible()) { return null; } @@ -254,7 +249,7 @@ const WebPushEnabled = () => { return ( - {t("prefs_notifications_web_push_enabled")} {t("prefs_notifications_web_push_disabled")} diff --git a/web/src/components/SubscriptionPopup.jsx b/web/src/components/SubscriptionPopup.jsx index ee162972..74438f9a 100644 --- a/web/src/components/SubscriptionPopup.jsx +++ b/web/src/components/SubscriptionPopup.jsx @@ -142,6 +142,10 @@ export const SubscriptionPopup = (props) => { await subscriptionManager.deleteNotifications(props.subscription.id); }; + const handleSetMutedUntil = async (mutedUntil) => { + await subscriptionManager.setMutedUntil(subscription.id, mutedUntil); + }; + const handleUnsubscribe = async () => { console.log(`[SubscriptionPopup] Unsubscribing from ${props.subscription.id}`, props.subscription); await subscriptionManager.remove(props.subscription); @@ -166,8 +170,6 @@ export const SubscriptionPopup = (props) => { return ( <> - - @@ -198,7 +200,6 @@ export const SubscriptionPopup = (props) => { - {t("action_bar_reservation_edit")} )} @@ -207,7 +208,6 @@ export const SubscriptionPopup = (props) => { - {t("action_bar_reservation_delete")} )} @@ -215,21 +215,34 @@ export const SubscriptionPopup = (props) => { - {t("action_bar_send_test_notification")} - {t("action_bar_clear_notifications")} + {!!subscription.mutedUntil && ( + handleSetMutedUntil(0)}> + + + + {t("action_bar_unmute_notifications")} + + )} + {!subscription.mutedUntil && ( + handleSetMutedUntil(1)}> + + + + {t("action_bar_mute_notifications")} + + )} - {t("action_bar_unsubscribe")} @@ -331,31 +344,6 @@ const DisplayNameDialog = (props) => { ); }; -const NotificationToggle = ({ subscription }) => { - const { t } = useTranslation(); - - const handleToggleMute = async () => { - const mutedUntil = subscription.mutedUntil ? 0 : 1; // Make this a timestamp in the future - await subscriptionManager.setMutedUntil(subscription.id, mutedUntil); - }; - - return subscription.mutedUntil ? ( - - - - - {t("notification_toggle_unmute")} - - ) : ( - - - - - {t("notification_toggle_mute")} - - ); -}; - export const ReserveLimitChip = () => { const { account } = useContext(AccountContext); if (account?.role === Role.ADMIN || account?.stats.reservations_remaining > 0) {