From b7a4cc6d6acd42a2a9c3425f9832ac9b4c17599d Mon Sep 17 00:00:00 2001 From: Thorsten Eckel Date: Fri, 20 Nov 2020 11:54:09 +0100 Subject: [PATCH] - Fixes #3292: Unable to manage Microsoft365/Google Channel accounts if XOAUTH2 token can't get refreshed. - Fixes #3294: Failing XOAUTH2 Channel (Gmail / Office365) token refresh causes following Channels not to be fetched. --- app/models/channel.rb | 87 +++++++++---------- spec/factories/channel.rb | 22 +++-- spec/models/channel_spec.rb | 84 ++++++++++++++++++ spec/requests/integration/gmail_spec.rb | 4 +- .../requests/integration/microsoft365_spec.rb | 4 +- 5 files changed, 145 insertions(+), 56 deletions(-) diff --git a/app/models/channel.rb b/app/models/channel.rb index 372adec54..fa00f54ac 100644 --- a/app/models/channel.rb +++ b/app/models/channel.rb @@ -8,7 +8,6 @@ class Channel < ApplicationModel store :options store :preferences - after_initialize :refresh_xoauth2! after_create :email_address_check after_update :email_address_check after_destroy :email_address_check @@ -48,27 +47,27 @@ fetch one account adapter_options = options[:inbound][:options] end - begin - driver_class = self.class.driver_class(adapter) - driver_instance = driver_class.new - return if !force && !driver_instance.fetchable?(self) + refresh_xoauth2! - result = driver_instance.fetch(adapter_options, self) - self.status_in = result[:result] - self.last_log_in = result[:notice] - preferences[:last_fetch] = Time.zone.now - save! - true - rescue => e - error = "Can't use Channel::Driver::#{adapter.to_classname}: #{e.inspect}" - logger.error error - logger.error e - self.status_in = 'error' - self.last_log_in = error - preferences[:last_fetch] = Time.zone.now - save! - false - end + driver_class = self.class.driver_class(adapter) + driver_instance = driver_class.new + return if !force && !driver_instance.fetchable?(self) + + result = driver_instance.fetch(adapter_options, self) + self.status_in = result[:result] + self.last_log_in = result[:notice] + preferences[:last_fetch] = Time.zone.now + save! + true + rescue => e + error = "Can't use Channel::Driver::#{adapter.to_classname}: #{e.inspect}" + logger.error error + logger.error e + self.status_in = 'error' + self.last_log_in = error + preferences[:last_fetch] = Time.zone.now + save! + false end =begin @@ -250,24 +249,25 @@ send via account adapter = options[:outbound][:adapter] adapter_options = options[:outbound][:options] end - result = nil - begin - driver_class = self.class.driver_class(adapter) - driver_instance = driver_class.new - result = driver_instance.send(adapter_options, params, notification) - self.status_out = 'ok' - self.last_log_out = '' - save! - rescue => e - error = "Can't use Channel::Driver::#{adapter.to_classname}: #{e.inspect}" - logger.error error - logger.error e - self.status_out = 'error' - self.last_log_out = error - save! - raise error - end + + refresh_xoauth2! + + driver_class = self.class.driver_class(adapter) + driver_instance = driver_class.new + result = driver_instance.send(adapter_options, params, notification) + self.status_out = 'ok' + self.last_log_out = '' + save! + result + rescue => e + error = "Can't use Channel::Driver::#{adapter.to_classname}: #{e.inspect}" + logger.error error + logger.error e + self.status_out = 'error' + self.last_log_out = error + save! + raise error end =begin @@ -338,6 +338,7 @@ get instance of channel driver def refresh_xoauth2! return if options.dig(:auth, :type) != 'XOAUTH2' + return if ApplicationHandleInfo.current == 'application_server' result = ExternalCredential.refresh_token(options[:auth][:provider], options[:auth]) @@ -347,16 +348,10 @@ get instance of channel driver return if new_record? - # ATTENTION: We don't want to execute any other callbacks here - # because `after_initialize` leaks the current scope of the Channel class - # as described here: https://api.rubyonrails.org/classes/ActiveRecord/Relation.html#method-i-new - # which leads to unexpected effects like: - # Channel.where(area: 'Google::Account').limit(1).find_each { |c| puts Channel.all.to_sql } - # => "SELECT "channels".* FROM "channels" WHERE "channels"."area" = 'Google::Account'" - update_column(:options, options) # rubocop:disable Rails/SkipsModelValidations + save! rescue => e logger.error e - raise "Failed to refresh XOAUTH2 access_token of provider '#{options[:auth][:provider]}'! #{e.inspect}" + raise "Failed to refresh XOAUTH2 access_token of provider '#{options[:auth][:provider]}': #{e.message}" end private diff --git a/spec/factories/channel.rb b/spec/factories/channel.rb index 498fae20a..ccae68a57 100644 --- a/spec/factories/channel.rb +++ b/spec/factories/channel.rb @@ -1,9 +1,5 @@ FactoryBot.define do factory :channel do - # ensure the `refresh_xoauth2!` `after_initialize` callback gets executed - # https://stackoverflow.com/questions/5916162/problem-with-factory-girl-association-and-after-initialize#comment51639005_28057070 - initialize_with { new(attributes) } - area { 'Email::Dummy' } group { ::Group.find(1) } active { true } @@ -16,13 +12,23 @@ FactoryBot.define do area { 'Email::Account' } options do { - inbound: { + inbound: inbound, + outbound: outbound, + } + end + + transient do + inbound do + { adapter: 'null', options: {} - }, - outbound: { + } + end + + outbound do + { adapter: 'sendmail' } - } + end end end diff --git a/spec/models/channel_spec.rb b/spec/models/channel_spec.rb index 2f7ebc566..bafe1f696 100644 --- a/spec/models/channel_spec.rb +++ b/spec/models/channel_spec.rb @@ -2,10 +2,94 @@ require 'rails_helper' RSpec.describe Channel, type: :model do + describe '.fetch' do + + describe '#refresh_xoauth2! fails' do + + let(:channel) { create(:channel, area: 'SomeXOAUTH2::Account', options: { adapter: 'DummyXOAUTH2', auth: { type: 'XOAUTH2' } }) } + + before do + allow(ExternalCredential).to receive(:refresh_token).and_raise(RuntimeError) + end + + it 'changes Channel status to error' do + expect { described_class.fetch }.to change { channel.reload.status_in }.to('error') + end + end + + context 'when one adapter fetch fails' do + + let(:failing_adapter_class) do + Class.new(Channel::Driver::Null) do + def fetchable?(*) + true + end + + def fetch(*) + raise 'some error' + end + end + end + + let(:dummy_adapter_class) do + Class.new(Channel::Driver::Null) do + def fetchable?(*) + true + end + end + end + + let(:failing_channel) do + create(:email_channel, inbound: { + adapter: 'failing', + options: {} + }) + end + + let(:other_channel) do + create(:email_channel, inbound: { + adapter: 'dummy', + options: {} + }) + end + + before do + allow(described_class).to receive(:driver_class).with('dummy').and_return(dummy_adapter_class) + allow(described_class).to receive(:driver_class).with('failing').and_return(failing_adapter_class) + + failing_channel + other_channel + end + + it 'adds error flag to the failing Channel' do + expect { described_class.fetch }.to change { failing_channel.reload.preferences[:last_fetch] }.and change { failing_channel.reload.status_in }.to('error') + end + + it 'fetches others anyway' do + expect { described_class.fetch }.to change { other_channel.reload.preferences[:last_fetch] }.and change { other_channel.reload.status_in }.to('ok') + end + end + end + context 'when authentication type is XOAUTH2' do shared_examples 'common XOAUTH2' do + context 'when token refresh fails' do + + let(:exception) { DummyExternalCredentialsBackendError.new('something unexpected happened here') } + + before do + stub_const('DummyExternalCredentialsBackendError', Class.new(StandardError)) + + allow(ExternalCredential).to receive(:refresh_token).and_raise(exception) + end + + it 'raises RuntimeError' do + expect { channel.refresh_xoauth2! }.to raise_exception(RuntimeError, /#{exception.message}/) + end + end + context 'when non-XOAUTH2 channels are present' do let!(:email_address) { create(:email_address, channel: create(:channel, area: 'Some::Other')) } diff --git a/spec/requests/integration/gmail_spec.rb b/spec/requests/integration/gmail_spec.rb index a98fc5076..e2c732dda 100644 --- a/spec/requests/integration/gmail_spec.rb +++ b/spec/requests/integration/gmail_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' RSpec.describe 'Gmail XOAUTH2' do # rubocop:disable RSpec/DescribeClass - let(:channel) { create(:google_channel) } + let(:channel) do + create(:google_channel).tap(&:refresh_xoauth2!) + end before do required_envs = %w[GMAIL_REFRESH_TOKEN GMAIL_CLIENT_ID GMAIL_CLIENT_SECRET GMAIL_USER] diff --git a/spec/requests/integration/microsoft365_spec.rb b/spec/requests/integration/microsoft365_spec.rb index 0ab596aa0..64f58efce 100644 --- a/spec/requests/integration/microsoft365_spec.rb +++ b/spec/requests/integration/microsoft365_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' RSpec.describe 'Microsoft365 XOAUTH2' do # rubocop:disable RSpec/DescribeClass - let(:channel) { create(:microsoft365_channel) } + let(:channel) do + create(:microsoft365_channel).tap(&:refresh_xoauth2!) + end before do required_envs = %w[MICROSOFT365_REFRESH_TOKEN MICROSOFT365_CLIENT_ID MICROSOFT365_CLIENT_SECRET MICROSOFT365_USER]