From 6cfd7d38368a961f5bf38cf8ddc2371199504a39 Mon Sep 17 00:00:00 2001 From: Santhosh C Date: Tue, 9 Nov 2021 21:36:32 +0530 Subject: [PATCH] feat: autogenerate vapid keys for push notifications (#3128) * feat: Autogenerate push notification keys * add vapid service class and remove pushkey model * add spec for vapid service * unset vapid env keys * Unset VAPID_PRIVATE_KEY env variable Co-authored-by: Sojan Jose Co-authored-by: Vishnu Narayanan --- app/controllers/dashboard_controller.rb | 1 + .../shared/mixins/specs/campaignMixin.spec.js | 1 - app/javascript/widget/store/modules/events.js | 4 +- .../notification/push_notification_service.rb | 6 +-- app/views/layouts/vueapp.html.erb | 4 +- lib/vapid_service.rb | 25 +++++++++ spec/lib/vapid_service_spec.rb | 53 +++++++++++++++++++ 7 files changed, 86 insertions(+), 8 deletions(-) create mode 100644 lib/vapid_service.rb create mode 100644 spec/lib/vapid_service_spec.rb diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index eb7a17caa..ad5b0ba24 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -28,6 +28,7 @@ class DashboardController < ActionController::Base 'ANALYTICS_HOST' ).merge( APP_VERSION: Chatwoot.config[:version], + VAPID_PUBLIC_KEY: VapidService.public_key, ENABLE_ACCOUNT_SIGNUP: GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false') ) end diff --git a/app/javascript/shared/mixins/specs/campaignMixin.spec.js b/app/javascript/shared/mixins/specs/campaignMixin.spec.js index 5919a6c32..71c90673a 100644 --- a/app/javascript/shared/mixins/specs/campaignMixin.spec.js +++ b/app/javascript/shared/mixins/specs/campaignMixin.spec.js @@ -34,5 +34,4 @@ describe('campaignMixin', () => { const wrapper = shallowMount(Component); expect(wrapper.vm.isOnOffType).toBe(true); }); - }); diff --git a/app/javascript/widget/store/modules/events.js b/app/javascript/widget/store/modules/events.js index 16ffba869..cadf0e18e 100644 --- a/app/javascript/widget/store/modules/events.js +++ b/app/javascript/widget/store/modules/events.js @@ -2,7 +2,7 @@ import events from 'widget/api/events'; const state = { isOpen: false, -} +}; const actions = { create: async (_, { name }) => { @@ -17,7 +17,7 @@ const actions = { const mutations = { toggleOpen($state) { $state.isOpen = !$state.isOpen; - } + }, }; export default { diff --git a/app/services/notification/push_notification_service.rb b/app/services/notification/push_notification_service.rb index f425f36b2..d0413a1d3 100644 --- a/app/services/notification/push_notification_service.rb +++ b/app/services/notification/push_notification_service.rb @@ -43,7 +43,7 @@ class Notification::PushNotificationService end def send_browser_push?(subscription) - ENV['VAPID_PUBLIC_KEY'] && subscription.browser_push? + VapidService.public_key && subscription.browser_push? end def send_browser_push(subscription) @@ -56,8 +56,8 @@ class Notification::PushNotificationService auth: subscription.subscription_attributes['auth'], vapid: { subject: push_url, - public_key: ENV['VAPID_PUBLIC_KEY'], - private_key: ENV['VAPID_PRIVATE_KEY'] + public_key: VapidService.public_key, + private_key: VapidService.private_key }, ssl_timeout: 5, open_timeout: 5, diff --git a/app/views/layouts/vueapp.html.erb b/app/views/layouts/vueapp.html.erb index 4e4a3d971..d20de94a7 100644 --- a/app/views/layouts/vueapp.html.erb +++ b/app/views/layouts/vueapp.html.erb @@ -35,8 +35,8 @@ hostURL: '<%= ENV.fetch('FRONTEND_URL', '') %>', fbAppId: '<%= ENV.fetch('FB_APP_ID', nil) %>', signupEnabled: '<%= @global_config['ENABLE_ACCOUNT_SIGNUP'] %>', - <% if ENV['VAPID_PUBLIC_KEY'] %> - vapidPublicKey: new Uint8Array(<%= Base64.urlsafe_decode64(ENV['VAPID_PUBLIC_KEY']).bytes %>), + <% if @global_config['VAPID_PUBLIC_KEY'] %> + vapidPublicKey: new Uint8Array(<%= Base64.urlsafe_decode64(@global_config['VAPID_PUBLIC_KEY']).bytes %>), <% end %> enabledLanguages: <%= available_locales_with_name.to_json.html_safe %>, selectedLocale: '<%= I18n.locale %>' diff --git a/lib/vapid_service.rb b/lib/vapid_service.rb new file mode 100644 index 000000000..52c8cea7e --- /dev/null +++ b/lib/vapid_service.rb @@ -0,0 +1,25 @@ +class VapidService + def self.public_key + vapid_keys['public_key'] + end + + def self.private_key + vapid_keys['private_key'] + end + + def self.vapid_keys + config = GlobalConfig.get('VAPID_KEYS') + return config['VAPID_KEYS'] if config['VAPID_KEYS'].present? + + # keys don't exist in the database. so let's generate and save them + keys = Webpush.generate_key + # TODO: remove the logic on environment variables when we completely deprecate + public_key = ENV['VAPID_PUBLIC_KEY'] || keys.public_key + private_key = ENV['VAPID_PRIVATE_KEY'] || keys.private_key + + i = InstallationConfig.where(name: 'VAPID_KEYS').first_or_create(value: { public_key: public_key, private_key: private_key }) + i.value + end + + private_class_method :vapid_keys +end diff --git a/spec/lib/vapid_service_spec.rb b/spec/lib/vapid_service_spec.rb new file mode 100644 index 000000000..bdadeabf5 --- /dev/null +++ b/spec/lib/vapid_service_spec.rb @@ -0,0 +1,53 @@ +require 'rails_helper' + +describe VapidService do + subject(:trigger) { described_class } + + describe 'execute' do + context 'when called with default options' do + before do + GlobalConfig.clear_cache + end + + it 'hit DB for the first call' do + expect(InstallationConfig).to receive(:find_by) + GlobalConfig.get('VAPID_KEYS') + end + + it 'get public key from env' do + # this gets public key + ENV['VAPID_PUBLIC_KEY'] = 'test' + described_class.public_key + + # subsequent calls should not hit DB + expect(InstallationConfig).not_to receive(:find_by) + described_class.public_key + ENV['VAPID_PUBLIC_KEY'] = nil + end + + it 'get private key from env' do + # this gets private key + ENV['VAPID_PRIVATE_KEY'] = 'test' + described_class.private_key + + # subsequent calls should not hit DB + expect(InstallationConfig).not_to receive(:find_by) + described_class.private_key + ENV['VAPID_PRIVATE_KEY'] = nil + ENV['VAPID_PRIVATE_KEY'] = nil + end + + it 'clears cache and fetch from DB next time, when clear_cache is called' do + # this loads from DB and is cached + GlobalConfig.get('VAPID_KEYS') + + # clears the cache + GlobalConfig.clear_cache + + # should be loaded from DB + expect(InstallationConfig).to receive(:find_by).with({ name: 'VAPID_KEYS' }).and_return(nil) + GlobalConfig.get('VAPID_KEYS') + end + end + end +end