Add more tests, change endpoint

server/server.go

@@ -85,6 +85,7 @@ var (
 	metricsPath                                          = "/metrics"
 	apiHealthPath                                        = "/v1/health"
 	apiStatsPath                                         = "/v1/stats"
+	apiWebPushPath                                       = "/v1/webpush"
 	apiTiersPath                                         = "/v1/tiers"
 	apiUsersPath                                         = "/v1/users"
 	apiUsersAccessPath                                   = "/v1/users/access"
@@ -94,7 +95,6 @@ var (
 	apiAccountSettingsPath                               = "/v1/account/settings"
 	apiAccountSubscriptionPath                           = "/v1/account/subscription"
 	apiAccountReservationPath                            = "/v1/account/reservation"
-	apiAccountWebPushPath                                = "/v1/account/webpush"
 	apiAccountPhonePath                                  = "/v1/account/phone"
 	apiAccountPhoneVerifyPath                            = "/v1/account/phone/verify"
 	apiAccountBillingPortalPath                          = "/v1/account/billing/portal"
@@ -490,9 +490,9 @@ func (s *Server) handleInternal(w http.ResponseWriter, r *http.Request, v *visit
 		return s.ensureUser(s.ensureCallsEnabled(s.withAccountSync(s.handleAccountPhoneNumberAdd)))(w, r, v)
 	} else if r.Method == http.MethodDelete && r.URL.Path == apiAccountPhonePath {
 		return s.ensureUser(s.ensureCallsEnabled(s.withAccountSync(s.handleAccountPhoneNumberDelete)))(w, r, v)
-	} else if r.Method == http.MethodPost && apiAccountWebPushPath == r.URL.Path {
+	} else if r.Method == http.MethodPost && apiWebPushPath == r.URL.Path {
 		return s.ensureWebPushEnabled(s.limitRequests(s.handleWebPushUpdate))(w, r, v)
-	} else if r.Method == http.MethodDelete && apiAccountWebPushPath == r.URL.Path {
+	} else if r.Method == http.MethodDelete && apiWebPushPath == r.URL.Path {
 		return s.ensureWebPushEnabled(s.limitRequests(s.handleWebPushDelete))(w, r, v)
 	} else if r.Method == http.MethodGet && r.URL.Path == apiStatsPath {
 		return s.handleStats(w, r, v)

server/server_account.go

@@ -170,7 +170,7 @@ func (s *Server) handleAccountDelete(w http.ResponseWriter, r *http.Request, v *
 	if _, err := s.userManager.Authenticate(u.Name, req.Password); err != nil {
 		return errHTTPBadRequestIncorrectPasswordConfirmation
 	}
-	if s.webPush != nil {
+	if s.webPush != nil && u.ID != "" {
 		if err := s.webPush.RemoveSubscriptionsByUserID(u.ID); err != nil {
 			logvr(v, r).Err(err).Warn("Error removing web push subscriptions for %s", u.Name)
 		}

server/server_webpush_test.go

@@ -23,7 +23,7 @@ const (
 func TestServer_WebPush_TopicAdd(t *testing.T) {
 	s := newTestServer(t, newTestConfigWithWebPush(t))
 
-	response := request(t, s, "POST", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), nil)
+	response := request(t, s, "POST", "/v1/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), nil)
 	require.Equal(t, 200, response.Code)
 	require.Equal(t, `{"success":true}`+"\n", response.Body.String())
 
@@ -40,7 +40,7 @@ func TestServer_WebPush_TopicAdd(t *testing.T) {
 func TestServer_WebPush_TopicAdd_InvalidEndpoint(t *testing.T) {
 	s := newTestServer(t, newTestConfigWithWebPush(t))
 
-	response := request(t, s, "POST", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, "https://ddos-target.example.com/webpush"), nil)
+	response := request(t, s, "POST", "/v1/webpush", payloadForTopics(t, []string{"test-topic"}, "https://ddos-target.example.com/webpush"), nil)
 	require.Equal(t, 400, response.Code)
 	require.Equal(t, `{"code":40039,"http":400,"error":"invalid request: web push endpoint unknown"}`+"\n", response.Body.String())
 }
@@ -53,7 +53,7 @@ func TestServer_WebPush_TopicAdd_TooManyTopics(t *testing.T) {
 		topicList[i] = util.RandomString(5)
 	}
 
-	response := request(t, s, "POST", "/v1/account/webpush", payloadForTopics(t, topicList, testWebPushEndpoint), nil)
+	response := request(t, s, "POST", "/v1/webpush", payloadForTopics(t, topicList, testWebPushEndpoint), nil)
 	require.Equal(t, 400, response.Code)
 	require.Equal(t, `{"code":40040,"http":400,"error":"invalid request: too many web push topic subscriptions"}`+"\n", response.Body.String())
 }
@@ -64,7 +64,7 @@ func TestServer_WebPush_TopicUnsubscribe(t *testing.T) {
 	addSubscription(t, s, testWebPushEndpoint, "test-topic")
 	requireSubscriptionCount(t, s, "test-topic", 1)
 
-	response := request(t, s, "POST", "/v1/account/webpush", payloadForTopics(t, []string{}, testWebPushEndpoint), nil)
+	response := request(t, s, "POST", "/v1/webpush", payloadForTopics(t, []string{}, testWebPushEndpoint), nil)
 	require.Equal(t, 200, response.Code)
 	require.Equal(t, `{"success":true}`+"\n", response.Body.String())
 
@@ -79,7 +79,7 @@ func TestServer_WebPush_TopicSubscribeProtected_Allowed(t *testing.T) {
 	require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser))
 	require.Nil(t, s.userManager.AllowAccess("ben", "test-topic", user.PermissionReadWrite))
 
-	response := request(t, s, "POST", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), map[string]string{
+	response := request(t, s, "POST", "/v1/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), map[string]string{
 		"Authorization": util.BasicAuth("ben", "ben"),
 	})
 	require.Equal(t, 200, response.Code)
@@ -96,7 +96,7 @@ func TestServer_WebPush_TopicSubscribeProtected_Denied(t *testing.T) {
 	config.AuthDefault = user.PermissionDenyAll
 	s := newTestServer(t, config)
 
-	response := request(t, s, "POST", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), nil)
+	response := request(t, s, "POST", "/v1/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), nil)
 	require.Equal(t, 403, response.Code)
 
 	requireSubscriptionCount(t, s, "test-topic", 0)
@@ -109,7 +109,7 @@ func TestServer_WebPush_DeleteAccountUnsubscribe(t *testing.T) {
 	require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser))
 	require.Nil(t, s.userManager.AllowAccess("ben", "test-topic", user.PermissionReadWrite))
 
-	response := request(t, s, "POST", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), map[string]string{
+	response := request(t, s, "POST", "/v1/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), map[string]string{
 		"Authorization": util.BasicAuth("ben", "ben"),
 	})
 

server/webpush_store.go

@@ -11,14 +11,15 @@ import (
 )
 
 const (
-	subscriptionIDPrefix             = "wps_"
+	subscriptionIDPrefix                     = "wps_"
-	subscriptionIDLength             = 10
+	subscriptionIDLength                     = 10
-	subscriptionLimitPerSubscriberIP = 10
+	subscriptionEndpointLimitPerSubscriberIP = 10
 )
 
 var (
 	errWebPushNoRows               = errors.New("no rows found")
 	errWebPushTooManySubscriptions = errors.New("too many subscriptions")
+	errWebPushUserIDCannotBeEmpty  = errors.New("user ID cannot be empty")
 )
 
 const (
@@ -60,6 +61,7 @@ const (
 		FROM subscription_topic st
 		JOIN subscription s ON s.id = st.subscription_id
 		WHERE st.topic = ?
+		ORDER BY endpoint
 	`
 	selectWebPushSubscriptionsExpiringSoonQuery = `SELECT id, endpoint, key_auth, key_p256dh, user_id FROM subscription WHERE warned_at = 0 AND updated_at <= ?`
 	insertWebPushSubscriptionQuery              = `
@@ -164,7 +166,7 @@ func (c *webPushStore) UpsertSubscription(endpoint string, auth, p256dh, userID
 			return err
 		}
 	} else {
-		if subscriptionCount >= subscriptionLimitPerSubscriberIP {
+		if subscriptionCount >= subscriptionEndpointLimitPerSubscriberIP {
 			return errWebPushTooManySubscriptions
 		}
 		subscriptionID = util.RandomStringPrefix(subscriptionIDPrefix, subscriptionIDLength)
@@ -250,6 +252,9 @@ func (c *webPushStore) RemoveSubscriptionsByEndpoint(endpoint string) error {
 
 // RemoveSubscriptionsByUserID removes all subscriptions for the given user ID
 func (c *webPushStore) RemoveSubscriptionsByUserID(userID string) error {
+	if userID == "" {
+		return errWebPushUserIDCannotBeEmpty
+	}
 	_, err := c.db.Exec(deleteWebPushSubscriptionByUserIDQuery, userID)
 	return err
 }

server/webpush_store_test.go

@@ -6,16 +6,11 @@ import (
 	"net/netip"
 	"path/filepath"
 	"testing"
+	"time"
 )
 
-func newTestWebPushStore(t *testing.T, filename string) *webPushStore {
-	webPush, err := newWebPushStore(filename)
-	require.Nil(t, err)
-	return webPush
-}
-
 func TestWebPushStore_UpsertSubscription_SubscriptionsForTopic(t *testing.T) {
-	webPush := newTestWebPushStore(t, filepath.Join(t.TempDir(), "webpush.db"))
+	webPush := newTestWebPushStore(t)
 	defer webPush.Close()
 
 	require.Nil(t, webPush.UpsertSubscription(testWebPushEndpoint, "auth-key", "p256dh-key", "u_1234", netip.MustParseAddr("1.2.3.4"), []string{"test-topic", "mytopic"}))
@@ -35,7 +30,7 @@ func TestWebPushStore_UpsertSubscription_SubscriptionsForTopic(t *testing.T) {
 }
 
 func TestWebPushStore_UpsertSubscription_SubscriberIPLimitReached(t *testing.T) {
-	webPush := newTestWebPushStore(t, filepath.Join(t.TempDir(), "webpush.db"))
+	webPush := newTestWebPushStore(t)
 	defer webPush.Close()
 
 	// Insert 10 subscriptions with the same IP address
@@ -53,3 +48,152 @@ func TestWebPushStore_UpsertSubscription_SubscriberIPLimitReached(t *testing.T)
 	// But with a different IP address it should be fine again
 	require.Nil(t, webPush.UpsertSubscription(testWebPushEndpoint+"99", "auth-key", "p256dh-key", "u_1234", netip.MustParseAddr("9.9.9.9"), []string{"test-topic", "mytopic"}))
 }
+
+func TestWebPushStore_UpsertSubscription_UpdateTopics(t *testing.T) {
+	webPush := newTestWebPushStore(t)
+	defer webPush.Close()
+
+	// Insert subscription with two topics, and another with one topic
+	require.Nil(t, webPush.UpsertSubscription(testWebPushEndpoint+"0", "auth-key", "p256dh-key", "u_1234", netip.MustParseAddr("1.2.3.4"), []string{"topic1", "topic2"}))
+	require.Nil(t, webPush.UpsertSubscription(testWebPushEndpoint+"1", "auth-key", "p256dh-key", "", netip.MustParseAddr("9.9.9.9"), []string{"topic1"}))
+
+	subs, err := webPush.SubscriptionsForTopic("topic1")
+	require.Nil(t, err)
+	require.Len(t, subs, 2)
+	require.Equal(t, testWebPushEndpoint+"0", subs[0].Endpoint)
+	require.Equal(t, testWebPushEndpoint+"1", subs[1].Endpoint)
+
+	subs, err = webPush.SubscriptionsForTopic("topic2")
+	require.Nil(t, err)
+	require.Len(t, subs, 1)
+	require.Equal(t, testWebPushEndpoint+"0", subs[0].Endpoint)
+
+	// Update the first subscription to have only one topic
+	require.Nil(t, webPush.UpsertSubscription(testWebPushEndpoint+"0", "auth-key", "p256dh-key", "u_1234", netip.MustParseAddr("1.2.3.4"), []string{"topic1"}))
+
+	subs, err = webPush.SubscriptionsForTopic("topic1")
+	require.Nil(t, err)
+	require.Len(t, subs, 2)
+	require.Equal(t, testWebPushEndpoint+"0", subs[0].Endpoint)
+
+	subs, err = webPush.SubscriptionsForTopic("topic2")
+	require.Nil(t, err)
+	require.Len(t, subs, 0)
+}
+
+func TestWebPushStore_RemoveSubscriptionsByEndpoint(t *testing.T) {
+	webPush := newTestWebPushStore(t)
+	defer webPush.Close()
+
+	// Insert subscription with two topics
+	require.Nil(t, webPush.UpsertSubscription(testWebPushEndpoint, "auth-key", "p256dh-key", "u_1234", netip.MustParseAddr("1.2.3.4"), []string{"topic1", "topic2"}))
+	subs, err := webPush.SubscriptionsForTopic("topic1")
+	require.Nil(t, err)
+	require.Len(t, subs, 1)
+
+	// And remove it again
+	require.Nil(t, webPush.RemoveSubscriptionsByEndpoint(testWebPushEndpoint))
+	subs, err = webPush.SubscriptionsForTopic("topic1")
+	require.Nil(t, err)
+	require.Len(t, subs, 0)
+}
+
+func TestWebPushStore_RemoveSubscriptionsByUserID(t *testing.T) {
+	webPush := newTestWebPushStore(t)
+	defer webPush.Close()
+
+	// Insert subscription with two topics
+	require.Nil(t, webPush.UpsertSubscription(testWebPushEndpoint, "auth-key", "p256dh-key", "u_1234", netip.MustParseAddr("1.2.3.4"), []string{"topic1", "topic2"}))
+	subs, err := webPush.SubscriptionsForTopic("topic1")
+	require.Nil(t, err)
+	require.Len(t, subs, 1)
+
+	// And remove it again
+	require.Nil(t, webPush.RemoveSubscriptionsByUserID("u_1234"))
+	subs, err = webPush.SubscriptionsForTopic("topic1")
+	require.Nil(t, err)
+	require.Len(t, subs, 0)
+}
+
+func TestWebPushStore_RemoveSubscriptionsByUserID_Empty(t *testing.T) {
+	webPush := newTestWebPushStore(t)
+	defer webPush.Close()
+	require.Equal(t, errWebPushUserIDCannotBeEmpty, webPush.RemoveSubscriptionsByUserID(""))
+}
+
+func TestWebPushStore_MarkExpiryWarningSent(t *testing.T) {
+	webPush := newTestWebPushStore(t)
+	defer webPush.Close()
+
+	// Insert subscription with two topics
+	require.Nil(t, webPush.UpsertSubscription(testWebPushEndpoint, "auth-key", "p256dh-key", "u_1234", netip.MustParseAddr("1.2.3.4"), []string{"topic1", "topic2"}))
+	subs, err := webPush.SubscriptionsForTopic("topic1")
+	require.Nil(t, err)
+	require.Len(t, subs, 1)
+
+	// Mark them as warning sent
+	require.Nil(t, webPush.MarkExpiryWarningSent(subs))
+
+	rows, err := webPush.db.Query("SELECT endpoint FROM subscription WHERE warned_at > 0")
+	require.Nil(t, err)
+	defer rows.Close()
+	var endpoint string
+	require.True(t, rows.Next())
+	require.Nil(t, rows.Scan(&endpoint))
+	require.Nil(t, err)
+	require.Equal(t, testWebPushEndpoint, endpoint)
+	require.False(t, rows.Next())
+}
+
+func TestWebPushStore_SubscriptionsExpiring(t *testing.T) {
+	webPush := newTestWebPushStore(t)
+	defer webPush.Close()
+
+	// Insert subscription with two topics
+	require.Nil(t, webPush.UpsertSubscription(testWebPushEndpoint, "auth-key", "p256dh-key", "u_1234", netip.MustParseAddr("1.2.3.4"), []string{"topic1", "topic2"}))
+	subs, err := webPush.SubscriptionsForTopic("topic1")
+	require.Nil(t, err)
+	require.Len(t, subs, 1)
+
+	// Fake-mark them as soon-to-expire
+	_, err = webPush.db.Exec("UPDATE subscription SET updated_at = ? WHERE endpoint = ?", time.Now().Add(-8*24*time.Hour).Unix(), testWebPushEndpoint)
+	require.Nil(t, err)
+
+	// Should not be cleaned up yet
+	require.Nil(t, webPush.RemoveExpiredSubscriptions(9*24*time.Hour))
+
+	// Run expiration
+	subs, err = webPush.SubscriptionsExpiring(7 * 24 * time.Hour)
+	require.Nil(t, err)
+	require.Len(t, subs, 1)
+	require.Equal(t, testWebPushEndpoint, subs[0].Endpoint)
+}
+
+func TestWebPushStore_RemoveExpiredSubscriptions(t *testing.T) {
+	webPush := newTestWebPushStore(t)
+	defer webPush.Close()
+
+	// Insert subscription with two topics
+	require.Nil(t, webPush.UpsertSubscription(testWebPushEndpoint, "auth-key", "p256dh-key", "u_1234", netip.MustParseAddr("1.2.3.4"), []string{"topic1", "topic2"}))
+	subs, err := webPush.SubscriptionsForTopic("topic1")
+	require.Nil(t, err)
+	require.Len(t, subs, 1)
+
+	// Fake-mark them as expired
+	_, err = webPush.db.Exec("UPDATE subscription SET updated_at = ? WHERE endpoint = ?", time.Now().Add(-10*24*time.Hour).Unix(), testWebPushEndpoint)
+	require.Nil(t, err)
+
+	// Run expiration
+	require.Nil(t, webPush.RemoveExpiredSubscriptions(9*24*time.Hour))
+
+	// List again, should be 0
+	subs, err = webPush.SubscriptionsForTopic("topic1")
+	require.Nil(t, err)
+	require.Len(t, subs, 0)
+}
+
+func newTestWebPushStore(t *testing.T) *webPushStore {
+	webPush, err := newWebPushStore(filepath.Join(t.TempDir(), "webpush.db"))
+	require.Nil(t, err)
+	return webPush
+}

web/src/app/Api.js

@@ -6,7 +6,7 @@ import {
   topicUrlAuth,
   topicUrlJsonPoll,
   topicUrlJsonPollWithSince,
-  accountWebPushUrl,
+  webPushUrl,
 } from "./utils";
 import userManager from "./UserManager";
 import { fetchOrThrow } from "./errors";
@@ -117,7 +117,7 @@ class Api {
 
   async updateWebPush(pushSubscription, topics) {
     const user = await userManager.get(config.base_url);
-    const url = accountWebPushUrl(config.base_url);
+    const url = webPushUrl(config.base_url);
     console.log(`[Api] Updating Web Push subscription`, { url, topics, endpoint: pushSubscription.endpoint });
     const serializedSubscription = JSON.parse(JSON.stringify(pushSubscription)); // Ugh ... https://stackoverflow.com/a/40525434/1440785
     await fetchOrThrow(url, {
@@ -134,7 +134,7 @@ class Api {
 
   async deleteWebPush(pushSubscription) {
     const user = await userManager.get(config.base_url);
-    const url = accountWebPushUrl(config.base_url);
+    const url = webPushUrl(config.base_url);
     console.log(`[Api] Deleting Web Push subscription`, { url, endpoint: pushSubscription.endpoint });
     await fetchOrThrow(url, {
       method: "DELETE",

web/src/app/utils.js

@@ -21,6 +21,7 @@ export const topicUrlJsonPoll = (baseUrl, topic) => `${topicUrlJson(baseUrl, top
 export const topicUrlJsonPollWithSince = (baseUrl, topic, since) => `${topicUrlJson(baseUrl, topic)}?poll=1&since=${since}`;
 export const topicUrlAuth = (baseUrl, topic) => `${topicUrl(baseUrl, topic)}/auth`;
 export const topicShortUrl = (baseUrl, topic) => shortUrl(topicUrl(baseUrl, topic));
+export const webPushUrl = (baseUrl) => `${baseUrl}/v1/webpush`;
 export const accountUrl = (baseUrl) => `${baseUrl}/v1/account`;
 export const accountPasswordUrl = (baseUrl) => `${baseUrl}/v1/account/password`;
 export const accountTokenUrl = (baseUrl) => `${baseUrl}/v1/account/token`;
@@ -32,7 +33,6 @@ export const accountBillingSubscriptionUrl = (baseUrl) => `${baseUrl}/v1/account
 export const accountBillingPortalUrl = (baseUrl) => `${baseUrl}/v1/account/billing/portal`;
 export const accountPhoneUrl = (baseUrl) => `${baseUrl}/v1/account/phone`;
 export const accountPhoneVerifyUrl = (baseUrl) => `${baseUrl}/v1/account/phone/verify`;
-export const accountWebPushUrl = (baseUrl) => `${baseUrl}/v1/account/webpush`;
 
 export const validUrl = (url) => url.match(/^https?:\/\/.+/);