From 05f23df3b7ceff2e9debac5f6232271620b78447 Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Wed, 23 Oct 2024 10:02:31 +0200 Subject: [PATCH] Add endpoint to remove web push subscription (#32626) --- .../api/web/push_subscriptions_controller.rb | 9 ++- app/models/web/push_subscription.rb | 2 + app/workers/web/push_notification_worker.rb | 12 +++- config/routes/api.rb | 2 +- .../api/web/push_subscriptions_spec.rb | 55 +++++++++++++++++++ .../web/push_notification_worker_spec.rb | 1 + 6 files changed, 76 insertions(+), 5 deletions(-) create mode 100644 spec/requests/api/web/push_subscriptions_spec.rb diff --git a/app/controllers/api/web/push_subscriptions_controller.rb b/app/controllers/api/web/push_subscriptions_controller.rb index 167d16fc4d..f515961427 100644 --- a/app/controllers/api/web/push_subscriptions_controller.rb +++ b/app/controllers/api/web/push_subscriptions_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Api::Web::PushSubscriptionsController < Api::Web::BaseController - before_action :require_user! + before_action :require_user!, except: :destroy before_action :set_push_subscription, only: :update before_action :destroy_previous_subscriptions, only: :create, if: :prior_subscriptions? after_action :update_session_with_subscription, only: :create @@ -17,6 +17,13 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer end + def destroy + push_subscription = ::Web::PushSubscription.find_by_token_for(:unsubscribe, params[:id]) + push_subscription&.destroy + + head 200 + end + private def active_session diff --git a/app/models/web/push_subscription.rb b/app/models/web/push_subscription.rb index 9d30881bf3..656040d2ce 100644 --- a/app/models/web/push_subscription.rb +++ b/app/models/web/push_subscription.rb @@ -29,6 +29,8 @@ class Web::PushSubscription < ApplicationRecord delegate :locale, to: :associated_user + generates_token_for :unsubscribe, expires_in: Web::PushNotificationWorker::TTL + def pushable?(notification) policy_allows_notification?(notification) && alert_enabled_for_notification_type?(notification) end diff --git a/app/workers/web/push_notification_worker.rb b/app/workers/web/push_notification_worker.rb index e771928ef3..824e7b5940 100644 --- a/app/workers/web/push_notification_worker.rb +++ b/app/workers/web/push_notification_worker.rb @@ -2,10 +2,11 @@ class Web::PushNotificationWorker include Sidekiq::Worker + include RoutingHelper sidekiq_options queue: 'push', retry: 5 - TTL = 48.hours.to_s + TTL = 48.hours URGENCY = 'normal' def perform(subscription_id, notification_id) @@ -23,12 +24,13 @@ class Web::PushNotificationWorker request.add_headers( 'Content-Type' => 'application/octet-stream', - 'Ttl' => TTL, + 'Ttl' => TTL.to_s, 'Urgency' => URGENCY, 'Content-Encoding' => 'aesgcm', 'Encryption' => "salt=#{Webpush.encode64(payload.fetch(:salt)).delete('=')}", 'Crypto-Key' => "dh=#{Webpush.encode64(payload.fetch(:server_public_key)).delete('=')};#{web_push_request.crypto_key_header}", - 'Authorization' => web_push_request.authorization_header + 'Authorization' => web_push_request.authorization_header, + 'Unsubscribe-URL' => subscription_url ) request.perform do |response| @@ -72,4 +74,8 @@ class Web::PushNotificationWorker def request_pool RequestPool.current end + + def subscription_url + api_web_push_subscription_url(id: @subscription.generate_token_for(:unsubscribe)) + end end diff --git a/config/routes/api.rb b/config/routes/api.rb index 46907a1ce2..57ce3ba9fd 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -346,7 +346,7 @@ namespace :api, format: false do namespace :web do resource :settings, only: [:update] resources :embeds, only: [:show] - resources :push_subscriptions, only: [:create] do + resources :push_subscriptions, only: [:create, :destroy] do member do put :update end diff --git a/spec/requests/api/web/push_subscriptions_spec.rb b/spec/requests/api/web/push_subscriptions_spec.rb new file mode 100644 index 0000000000..a903dc6f89 --- /dev/null +++ b/spec/requests/api/web/push_subscriptions_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'API Web Push Subscriptions' do + describe 'DELETE /api/web/push_subscriptions/:id' do + subject { delete api_web_push_subscription_path(token) } + + context 'when the subscription exists' do + let!(:web_push_subscription) do + Fabricate(:web_push_subscription) + end + let(:token) do + web_push_subscription.generate_token_for(:unsubscribe) + end + + it 'deletes the subscription' do + expect { subject } + .to change(Web::PushSubscription, :count).by(-1) + + expect(response).to have_http_status(200) + end + end + + context 'when the subscription does not exist' do + let(:web_push_subscription) do + Fabricate(:web_push_subscription) + end + let(:token) do + web_push_subscription.generate_token_for(:unsubscribe) + end + + before do + token # memoize before destroying the record + web_push_subscription.destroy! + end + + it 'does nothing' do + subject + + expect(response).to have_http_status(200) + end + end + + context 'when the token is invalid' do + let(:token) { 'invalid--invalid' } + + it 'does nothing' do + subject + + expect(response).to have_http_status(200) + end + end + end +end diff --git a/spec/workers/web/push_notification_worker_spec.rb b/spec/workers/web/push_notification_worker_spec.rb index 7f836d99e4..4993d467b3 100644 --- a/spec/workers/web/push_notification_worker_spec.rb +++ b/spec/workers/web/push_notification_worker_spec.rb @@ -61,6 +61,7 @@ RSpec.describe Web::PushNotificationWorker do 'Ttl' => '172800', 'Urgency' => 'normal', 'Authorization' => 'WebPush jwt.encoded.payload', + 'Unsubscribe-URL' => %r{/api/web/push_subscriptions/}, }, body: "+\xB8\xDBT}\u0013\xB6\xDD.\xF9\xB0\xA7\xC8Ҁ\xFD\x99#\xF7\xAC\x83\xA4\xDB,\u001F\xB5\xB9w\x85>\xF7\xADr" )