From f07707a9bb990de3e9be85adc28cdd620451b3c5 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 2 Oct 2024 07:11:52 -0400 Subject: [PATCH] Extract `WebPushRequest` from push notification worker and subscription (#32208) --- app/lib/web_push_request.rb | 72 +++++++++++++++++++ app/models/web/push_subscription.rb | 28 -------- app/workers/web/push_notification_worker.rb | 40 +++++++---- .../web/push_notification_worker_spec.rb | 47 ++++++++---- 4 files changed, 131 insertions(+), 56 deletions(-) create mode 100644 app/lib/web_push_request.rb diff --git a/app/lib/web_push_request.rb b/app/lib/web_push_request.rb new file mode 100644 index 0000000000..a43e22480e --- /dev/null +++ b/app/lib/web_push_request.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +class WebPushRequest + SIGNATURE_ALGORITHM = 'p256ecdsa' + AUTH_HEADER = 'WebPush' + PAYLOAD_EXPIRATION = 24.hours + JWT_ALGORITHM = 'ES256' + JWT_TYPE = 'JWT' + + attr_reader :web_push_subscription + + delegate( + :endpoint, + :key_auth, + :key_p256dh, + to: :web_push_subscription + ) + + def initialize(web_push_subscription) + @web_push_subscription = web_push_subscription + end + + def audience + @audience ||= Addressable::URI.parse(endpoint).normalized_site + end + + def authorization_header + [AUTH_HEADER, encoded_json_web_token].join(' ') + end + + def crypto_key_header + [SIGNATURE_ALGORITHM, vapid_key.public_key_for_push_header].join('=') + end + + def encrypt(payload) + Webpush::Encryption.encrypt(payload, key_p256dh, key_auth) + end + + private + + def encoded_json_web_token + JWT.encode( + web_token_payload, + vapid_key.curve, + JWT_ALGORITHM, + typ: JWT_TYPE + ) + end + + def web_token_payload + { + aud: audience, + exp: PAYLOAD_EXPIRATION.from_now.to_i, + sub: payload_subject, + } + end + + def payload_subject + [:mailto, contact_email].join(':') + end + + def vapid_key + @vapid_key ||= Webpush::VapidKey.from_keys( + Rails.configuration.x.vapid_public_key, + Rails.configuration.x.vapid_private_key + ) + end + + def contact_email + @contact_email ||= ::Setting.site_contact_email + end +end diff --git a/app/models/web/push_subscription.rb b/app/models/web/push_subscription.rb index ddfd08146e..9d30881bf3 100644 --- a/app/models/web/push_subscription.rb +++ b/app/models/web/push_subscription.rb @@ -29,26 +29,6 @@ class Web::PushSubscription < ApplicationRecord delegate :locale, to: :associated_user - def encrypt(payload) - Webpush::Encryption.encrypt(payload, key_p256dh, key_auth) - end - - def audience - @audience ||= Addressable::URI.parse(endpoint).normalized_site - end - - def crypto_key_header - p256ecdsa = vapid_key.public_key_for_push_header - - "p256ecdsa=#{p256ecdsa}" - end - - def authorization_header - jwt = JWT.encode({ aud: audience, exp: 24.hours.from_now.to_i, sub: "mailto:#{contact_email}" }, vapid_key.curve, 'ES256', typ: 'JWT') - - "WebPush #{jwt}" - end - def pushable?(notification) policy_allows_notification?(notification) && alert_enabled_for_notification_type?(notification) end @@ -92,14 +72,6 @@ class Web::PushSubscription < ApplicationRecord ) end - def vapid_key - @vapid_key ||= Webpush::VapidKey.from_keys(Rails.configuration.x.vapid_public_key, Rails.configuration.x.vapid_private_key) - end - - def contact_email - @contact_email ||= ::Setting.site_contact_email - end - def alert_enabled_for_notification_type?(notification) truthy?(data&.dig('alerts', notification.type.to_s)) end diff --git a/app/workers/web/push_notification_worker.rb b/app/workers/web/push_notification_worker.rb index 7e9691aaba..104503f130 100644 --- a/app/workers/web/push_notification_worker.rb +++ b/app/workers/web/push_notification_worker.rb @@ -16,10 +16,10 @@ class Web::PushNotificationWorker # in the meantime, so we have to double-check before proceeding return unless @notification.activity.present? && @subscription.pushable?(@notification) - payload = @subscription.encrypt(push_notification_json) + payload = web_push_request.encrypt(push_notification_json) - request_pool.with(@subscription.audience) do |http_client| - request = Request.new(:post, @subscription.endpoint, body: payload.fetch(:ciphertext), http_client: http_client) + request_pool.with(web_push_request.audience) do |http_client| + request = Request.new(:post, web_push_request.endpoint, body: payload.fetch(:ciphertext), http_client: http_client) request.add_headers( 'Content-Type' => 'application/octet-stream', @@ -27,8 +27,8 @@ class Web::PushNotificationWorker 'Urgency' => URGENCY, 'Content-Encoding' => 'aesgcm', 'Encryption' => "salt=#{Webpush.encode64(payload.fetch(:salt)).delete('=')}", - 'Crypto-Key' => "dh=#{Webpush.encode64(payload.fetch(:server_public_key)).delete('=')};#{@subscription.crypto_key_header}", - 'Authorization' => @subscription.authorization_header + 'Crypto-Key' => "dh=#{Webpush.encode64(payload.fetch(:server_public_key)).delete('=')};#{web_push_request.crypto_key_header}", + 'Authorization' => web_push_request.authorization_header ) request.perform do |response| @@ -50,17 +50,27 @@ class Web::PushNotificationWorker private - def push_notification_json - json = I18n.with_locale(@subscription.locale.presence || I18n.default_locale) do - ActiveModelSerializers::SerializableResource.new( - @notification, - serializer: Web::NotificationSerializer, - scope: @subscription, - scope_name: :current_push_subscription - ).as_json - end + def web_push_request + @web_push_request || WebPushRequest.new(@subscription) + end - Oj.dump(json) + def push_notification_json + Oj.dump(serialized_notification_in_subscription_locale.as_json) + end + + def serialized_notification_in_subscription_locale + I18n.with_locale(@subscription.locale.presence || I18n.default_locale) do + serialized_notification + end + end + + def serialized_notification + ActiveModelSerializers::SerializableResource.new( + @notification, + serializer: Web::NotificationSerializer, + scope: @subscription, + scope_name: :current_push_subscription + ) end def request_pool diff --git a/spec/workers/web/push_notification_worker_spec.rb b/spec/workers/web/push_notification_worker_spec.rb index ced21d5bf7..7f836d99e4 100644 --- a/spec/workers/web/push_notification_worker_spec.rb +++ b/spec/workers/web/push_notification_worker_spec.rb @@ -22,27 +22,48 @@ RSpec.describe Web::PushNotificationWorker do let(:payload) { { ciphertext: ciphertext, salt: salt, server_public_key: server_public_key, shared_secret: shared_secret } } describe 'perform' do + around do |example| + original_private = Rails.configuration.x.vapid_private_key + original_public = Rails.configuration.x.vapid_public_key + Rails.configuration.x.vapid_private_key = vapid_private_key + Rails.configuration.x.vapid_public_key = vapid_public_key + example.run + Rails.configuration.x.vapid_private_key = original_private + Rails.configuration.x.vapid_public_key = original_public + end + before do - allow(subscription).to receive_messages(contact_email: contact_email, vapid_key: vapid_key) - allow(Web::PushSubscription).to receive(:find).with(subscription.id).and_return(subscription) + Setting.site_contact_email = contact_email + allow(Webpush::Encryption).to receive(:encrypt).and_return(payload) allow(JWT).to receive(:encode).and_return('jwt.encoded.payload') stub_request(:post, endpoint).to_return(status: 201, body: '') - - subject.perform(subscription.id, notification.id) end it 'calls the relevant service with the correct headers' do - expect(a_request(:post, endpoint).with(headers: { - 'Content-Encoding' => 'aesgcm', - 'Content-Type' => 'application/octet-stream', - 'Crypto-Key' => "dh=BAgtUks5d90kFmxGevk9tH7GEmvz9DB0qcEMUsOBgKwMf-TMjsKIIG6LQvGcFAf6jcmAod15VVwmYwGIIxE4VWE;p256ecdsa=#{vapid_public_key.delete('=')}", - 'Encryption' => 'salt=WJeVM-RY-F9351SVxTFx_g', - 'Ttl' => '172800', - 'Urgency' => 'normal', - 'Authorization' => 'WebPush jwt.encoded.payload', - }, body: "+\xB8\xDBT}\u0013\xB6\xDD.\xF9\xB0\xA7\xC8Ҁ\xFD\x99#\xF7\xAC\x83\xA4\xDB,\u001F\xB5\xB9w\x85>\xF7\xADr")).to have_been_made + subject.perform(subscription.id, notification.id) + + expect(web_push_endpoint_request) + .to have_been_made + end + + def web_push_endpoint_request + a_request( + :post, + endpoint + ).with( + headers: { + 'Content-Encoding' => 'aesgcm', + 'Content-Type' => 'application/octet-stream', + 'Crypto-Key' => "dh=BAgtUks5d90kFmxGevk9tH7GEmvz9DB0qcEMUsOBgKwMf-TMjsKIIG6LQvGcFAf6jcmAod15VVwmYwGIIxE4VWE;p256ecdsa=#{vapid_public_key.delete('=')}", + 'Encryption' => 'salt=WJeVM-RY-F9351SVxTFx_g', + 'Ttl' => '172800', + 'Urgency' => 'normal', + 'Authorization' => 'WebPush jwt.encoded.payload', + }, + body: "+\xB8\xDBT}\u0013\xB6\xDD.\xF9\xB0\xA7\xC8Ҁ\xFD\x99#\xF7\xAC\x83\xA4\xDB,\u001F\xB5\xB9w\x85>\xF7\xADr" + ) end end end