From 4655be0da6c0f9a58f4d09a32189cbe5619c42d1 Mon Sep 17 00:00:00 2001
From: Emelia Smith <ThisIsMissEm@users.noreply.github.com>
Date: Wed, 5 Jun 2024 21:16:47 +0200
Subject: [PATCH] Fix add validation to webpush subscription keys (#30542)

---
 app/models/web/push_subscription.rb           |  2 +
 app/validators/web_push_key_validator.rb      | 11 +++++
 .../web_push_subscription_fabricator.rb       |  8 +++-
 .../api/v1/push/subscriptions_spec.rb         | 47 +++++++++++++++----
 4 files changed, 57 insertions(+), 11 deletions(-)
 create mode 100644 app/validators/web_push_key_validator.rb

diff --git a/app/models/web/push_subscription.rb b/app/models/web/push_subscription.rb
index 1860f0aa3..b482ad3af 100644
--- a/app/models/web/push_subscription.rb
+++ b/app/models/web/push_subscription.rb
@@ -25,6 +25,8 @@ class Web::PushSubscription < ApplicationRecord
   validates :key_p256dh, presence: true
   validates :key_auth, presence: true
 
+  validates_with WebPushKeyValidator
+
   delegate :locale, to: :associated_user
 
   def encrypt(payload)
diff --git a/app/validators/web_push_key_validator.rb b/app/validators/web_push_key_validator.rb
new file mode 100644
index 000000000..a8ad5c9c6
--- /dev/null
+++ b/app/validators/web_push_key_validator.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class WebPushKeyValidator < ActiveModel::Validator
+  def validate(subscription)
+    begin
+      Webpush::Encryption.encrypt('validation_test', subscription.key_p256dh, subscription.key_auth)
+    rescue ArgumentError, OpenSSL::PKey::EC::Point::Error
+      subscription.errors.add(:base, I18n.t('crypto.errors.invalid_key'))
+    end
+  end
+end
diff --git a/spec/fabricators/web_push_subscription_fabricator.rb b/spec/fabricators/web_push_subscription_fabricator.rb
index baffdbf83..6b4028342 100644
--- a/spec/fabricators/web_push_subscription_fabricator.rb
+++ b/spec/fabricators/web_push_subscription_fabricator.rb
@@ -2,6 +2,10 @@
 
 Fabricator(:web_push_subscription, from: Web::PushSubscription) do
   endpoint   Faker::Internet.url
-  key_p256dh Faker::Internet.password
-  key_auth   Faker::Internet.password
+  key_p256dh do
+    curve = OpenSSL::PKey::EC.generate('prime256v1')
+    ecdh_key = curve.public_key.to_bn.to_s(2)
+    Base64.urlsafe_encode64(ecdh_key)
+  end
+  key_auth { Base64.urlsafe_encode64(Random.new.bytes(16)) }
 end
diff --git a/spec/requests/api/v1/push/subscriptions_spec.rb b/spec/requests/api/v1/push/subscriptions_spec.rb
index 82ea308cd..54ef5a13a 100644
--- a/spec/requests/api/v1/push/subscriptions_spec.rb
+++ b/spec/requests/api/v1/push/subscriptions_spec.rb
@@ -5,14 +5,17 @@ require 'rails_helper'
 describe 'API V1 Push Subscriptions' do
   let(:user) { Fabricate(:user) }
   let(:endpoint) { 'https://fcm.googleapis.com/fcm/send/fiuH06a27qE:APA91bHnSiGcLwdaxdyqVXNDR9w1NlztsHb6lyt5WDKOC_Z_Q8BlFxQoR8tWFSXUIDdkyw0EdvxTu63iqamSaqVSevW5LfoFwojws8XYDXv_NRRLH6vo2CdgiN4jgHv5VLt2A8ah6lUX' }
+  let(:keys) do
+    {
+      p256dh: 'BEm_a0bdPDhf0SOsrnB2-ategf1hHoCnpXgQsFj5JCkcoMrMt2WHoPfEYOYPzOIs9mZE8ZUaD7VA5vouy0kEkr8=',
+      auth: 'eH_C8rq2raXqlcBVDa1gLg==',
+    }
+  end
   let(:create_payload) do
     {
       subscription: {
         endpoint: endpoint,
-        keys: {
-          p256dh: 'BEm_a0bdPDhf0SOsrnB2-ategf1hHoCnpXgQsFj5JCkcoMrMt2WHoPfEYOYPzOIs9mZE8ZUaD7VA5vouy0kEkr8=',
-          auth: 'eH_C8rq2raXqlcBVDa1gLg==',
-        },
+        keys: keys,
       },
     }.with_indifferent_access
   end
@@ -37,6 +40,16 @@ describe 'API V1 Push Subscriptions' do
   let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) }
   let(:headers) { { 'Authorization' => "Bearer #{token.token}" } }
 
+  shared_examples 'validation error' do
+    it 'returns a validation error' do
+      subject
+
+      expect(response).to have_http_status(422)
+      expect(endpoint_push_subscriptions.count).to eq(0)
+      expect(endpoint_push_subscription).to be_nil
+    end
+  end
+
   describe 'POST /api/v1/push/subscription' do
     subject { post '/api/v1/push/subscription', params: create_payload, headers: headers }
 
@@ -68,13 +81,29 @@ describe 'API V1 Push Subscriptions' do
     context 'with invalid endpoint URL' do
       let(:endpoint) { 'app://example.foo' }
 
-      it 'returns a validation error' do
-        subject
+      it_behaves_like 'validation error'
+    end
 
-        expect(response).to have_http_status(422)
-        expect(endpoint_push_subscriptions.count).to eq(0)
-        expect(endpoint_push_subscription).to be_nil
+    context 'with invalid p256dh key' do
+      let(:keys) do
+        {
+          p256dh: 'BEm_invalidf0SOsrnB2-ategf1hHoCnpXgQsFj5JCkcoMrMt2WHoPfEYOYPzOIs9mZE8ZUaD7VA5vouy0kEkr8=',
+          auth: 'eH_C8rq2raXqlcBVDa1gLg==',
+        }
       end
+
+      it_behaves_like 'validation error'
+    end
+
+    context 'with invalid base64 p256dh key' do
+      let(:keys) do
+        {
+          p256dh: 'not base64',
+          auth: 'eH_C8rq2raXqlcBVDa1gLg==',
+        }
+      end
+
+      it_behaves_like 'validation error'
     end
   end