From 58992fc795dc0367c4081071190a1441457a1fae Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sat, 10 Jun 2023 21:09:01 -0400 Subject: [PATCH] Make DELETE endpoint, add different UI description --- server/server.go | 6 ++++-- server/server_web_push.go | 11 +++++++++++ server/server_web_push_test.go | 18 +++++++++--------- web/src/app/Api.js | 21 +++++++++++++++++---- web/src/app/SubscriptionManager.js | 7 ++++++- 5 files changed, 47 insertions(+), 16 deletions(-) diff --git a/server/server.go b/server/server.go index c9b95ba6..d053b3d3 100644 --- a/server/server.go +++ b/server/server.go @@ -490,6 +490,10 @@ 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 { + return s.ensureWebPushEnabled(s.limitRequests(s.handleWebPushUpdate))(w, r, v) + } else if r.Method == http.MethodDelete && apiAccountWebPushPath == 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) } else if r.Method == http.MethodGet && r.URL.Path == apiTiersPath { @@ -524,8 +528,6 @@ func (s *Server) handleInternal(w http.ResponseWriter, r *http.Request, v *visit return s.limitRequests(s.authorizeTopicRead(s.handleSubscribeWS))(w, r, v) } else if r.Method == http.MethodGet && authPathRegex.MatchString(r.URL.Path) { return s.limitRequests(s.authorizeTopicRead(s.handleTopicAuth))(w, r, v) - } else if r.Method == http.MethodPut && apiAccountWebPushPath == r.URL.Path { - return s.ensureWebPushEnabled(s.limitRequests(s.handleWebPushUpdate))(w, r, v) } else if r.Method == http.MethodGet && (topicPathRegex.MatchString(r.URL.Path) || externalTopicPathRegex.MatchString(r.URL.Path)) { return s.ensureWebEnabled(s.handleTopic)(w, r, v) } diff --git a/server/server_web_push.go b/server/server_web_push.go index 30a2cd02..d0da147f 100644 --- a/server/server_web_push.go +++ b/server/server_web_push.go @@ -56,6 +56,17 @@ func (s *Server) handleWebPushUpdate(w http.ResponseWriter, r *http.Request, v * return s.writeJSON(w, newSuccessResponse()) } +func (s *Server) handleWebPushDelete(w http.ResponseWriter, r *http.Request, _ *visitor) error { + req, err := readJSONWithLimit[apiWebPushUpdateSubscriptionRequest](r.Body, jsonBodyBytesLimit, false) + if err != nil || req.Endpoint == "" { + return errHTTPBadRequestWebPushSubscriptionInvalid + } + if err := s.webPush.RemoveSubscriptionsByEndpoint(req.Endpoint); err != nil { + return err + } + return s.writeJSON(w, newSuccessResponse()) +} + func (s *Server) publishToWebPushEndpoints(v *visitor, m *message) { subscriptions, err := s.webPush.SubscriptionsForTopic(m.Topic) if err != nil { diff --git a/server/server_web_push_test.go b/server/server_web_push_test.go index 82ad7215..7db82b8e 100644 --- a/server/server_web_push_test.go +++ b/server/server_web_push_test.go @@ -22,7 +22,7 @@ const ( func TestServer_WebPush_TopicAdd(t *testing.T) { s := newTestServer(t, newTestConfigWithWebPush(t)) - response := request(t, s, "PUT", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), nil) + response := request(t, s, "POST", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), nil) require.Equal(t, 200, response.Code) require.Equal(t, `{"success":true}`+"\n", response.Body.String()) @@ -39,7 +39,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, "PUT", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, "https://ddos-target.example.com/webpush"), nil) + response := request(t, s, "POST", "/v1/account/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()) } @@ -52,7 +52,7 @@ func TestServer_WebPush_TopicAdd_TooManyTopics(t *testing.T) { topicList[i] = util.RandomString(5) } - response := request(t, s, "PUT", "/v1/account/webpush", payloadForTopics(t, topicList, testWebPushEndpoint), nil) + response := request(t, s, "POST", "/v1/account/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()) } @@ -63,7 +63,7 @@ func TestServer_WebPush_TopicUnsubscribe(t *testing.T) { addSubscription(t, s, testWebPushEndpoint, "test-topic") requireSubscriptionCount(t, s, "test-topic", 1) - response := request(t, s, "PUT", "/v1/account/webpush", payloadForTopics(t, []string{}, testWebPushEndpoint), nil) + response := request(t, s, "POST", "/v1/account/webpush", payloadForTopics(t, []string{}, testWebPushEndpoint), nil) require.Equal(t, 200, response.Code) require.Equal(t, `{"success":true}`+"\n", response.Body.String()) @@ -78,7 +78,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, "PUT", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), map[string]string{ + response := request(t, s, "POST", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), map[string]string{ "Authorization": util.BasicAuth("ben", "ben"), }) require.Equal(t, 200, response.Code) @@ -95,7 +95,7 @@ func TestServer_WebPush_TopicSubscribeProtected_Denied(t *testing.T) { config.AuthDefault = user.PermissionDenyAll s := newTestServer(t, config) - response := request(t, s, "PUT", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), nil) + response := request(t, s, "POST", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), nil) require.Equal(t, 403, response.Code) requireSubscriptionCount(t, s, "test-topic", 0) @@ -108,7 +108,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, "PUT", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), map[string]string{ + response := request(t, s, "POST", "/v1/account/webpush", payloadForTopics(t, []string{"test-topic"}, testWebPushEndpoint), map[string]string{ "Authorization": util.BasicAuth("ben", "ben"), }) @@ -139,7 +139,7 @@ func TestServer_WebPush_Publish(t *testing.T) { defer pushService.Close() addSubscription(t, s, pushService.URL+"/push-receive", "test-topic") - request(t, s, "PUT", "/test-topic", "web push test", nil) + request(t, s, "POST", "/test-topic", "web push test", nil) waitFor(t, func() bool { return received.Load() @@ -162,7 +162,7 @@ func TestServer_WebPush_Publish_RemoveOnError(t *testing.T) { requireSubscriptionCount(t, s, "test-topic", 1) requireSubscriptionCount(t, s, "test-topic-abc", 1) - request(t, s, "PUT", "/test-topic", "web push test", nil) + request(t, s, "POST", "/test-topic", "web push test", nil) waitFor(t, func() bool { return received.Load() diff --git a/web/src/app/Api.js b/web/src/app/Api.js index 43bfe283..8b7fc79b 100644 --- a/web/src/app/Api.js +++ b/web/src/app/Api.js @@ -115,14 +115,13 @@ class Api { throw new Error(`Unexpected server response ${response.status}`); } - async updateWebPushSubscriptions(topics, pushSubscription) { + async updateWebPush(pushSubscription, topics) { const user = await userManager.get(config.base_url); const url = accountWebPushUrl(config.base_url); - console.log(`[Api] Sending Web Push Subscriptions`, { url, topics, endpoint: pushSubscription.endpoint }); - console.log(`[Api] Sending Web Push Subscriptions`, { pushSubscription }); + 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, { - method: "PUT", + method: "POST", headers: maybeWithAuth({}, user), body: JSON.stringify({ endpoint: serializedSubscription.endpoint, @@ -132,6 +131,20 @@ class Api { }), }); } + + + async deleteWebPush(pushSubscription) { + const user = await userManager.get(config.base_url); + const url = accountWebPushUrl(config.base_url); + console.log(`[Api] Deleting Web Push subscription`, { url, endpoint: pushSubscription.endpoint }); + await fetchOrThrow(url, { + method: "DELETE", + headers: maybeWithAuth({}, user), + body: JSON.stringify({ + endpoint: pushSubscription.endpoint + }), + }); + } } const api = new Api(); diff --git a/web/src/app/SubscriptionManager.js b/web/src/app/SubscriptionManager.js index 67b9faa3..b9e5d083 100644 --- a/web/src/app/SubscriptionManager.js +++ b/web/src/app/SubscriptionManager.js @@ -119,7 +119,12 @@ class SubscriptionManager { return; } - await api.updateWebPushSubscriptions(topics, browserSubscription); + if (topics.length > 0) { + await api.updateWebPush(browserSubscription, topics); + } else { + await api.deleteWebPush(browserSubscription); + } + } async updateState(subscriptionId, state) {