Extract WebPushRequest from push notification worker and subscription (#32208)

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

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

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_pool.with(web_push_request.audience) do |http_client|
-      request = Request.new(:post, @subscription.endpoint, body: payload.fetch(:ciphertext), http_client: 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}",
+        'Crypto-Key' => "dh=#{Webpush.encode64(payload.fetch(:server_public_key)).delete('=')};#{web_push_request.crypto_key_header}",
-        'Authorization' => @subscription.authorization_header
+        'Authorization' => web_push_request.authorization_header
       )
 
       request.perform do |response|
@@ -50,17 +50,27 @@ class Web::PushNotificationWorker
 
   private
 
+  def web_push_request
+    @web_push_request || WebPushRequest.new(@subscription)
+  end
+
   def push_notification_json
-    json = I18n.with_locale(@subscription.locale.presence || I18n.default_locale) do
+    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
-      ).as_json
+    )
-    end
-
-    Oj.dump(json)
   end
 
   def request_pool

spec/workers/web/push_notification_worker_spec.rb

@@ -22,19 +22,38 @@ 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)
+      Setting.site_contact_email = contact_email
-      allow(Web::PushSubscription).to receive(:find).with(subscription.id).and_return(subscription)
+
       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: {
+      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('=')}",
@@ -42,7 +61,9 @@ RSpec.describe Web::PushNotificationWorker do
           '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
+        },
+        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