- 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.
This commit is contained in:
parent
21708969fd
commit
b7a4cc6d6a
5 changed files with 145 additions and 56 deletions
|
@ -8,7 +8,6 @@ class Channel < ApplicationModel
|
||||||
store :options
|
store :options
|
||||||
store :preferences
|
store :preferences
|
||||||
|
|
||||||
after_initialize :refresh_xoauth2!
|
|
||||||
after_create :email_address_check
|
after_create :email_address_check
|
||||||
after_update :email_address_check
|
after_update :email_address_check
|
||||||
after_destroy :email_address_check
|
after_destroy :email_address_check
|
||||||
|
@ -48,7 +47,8 @@ fetch one account
|
||||||
adapter_options = options[:inbound][:options]
|
adapter_options = options[:inbound][:options]
|
||||||
end
|
end
|
||||||
|
|
||||||
begin
|
refresh_xoauth2!
|
||||||
|
|
||||||
driver_class = self.class.driver_class(adapter)
|
driver_class = self.class.driver_class(adapter)
|
||||||
driver_instance = driver_class.new
|
driver_instance = driver_class.new
|
||||||
return if !force && !driver_instance.fetchable?(self)
|
return if !force && !driver_instance.fetchable?(self)
|
||||||
|
@ -69,7 +69,6 @@ fetch one account
|
||||||
save!
|
save!
|
||||||
false
|
false
|
||||||
end
|
end
|
||||||
end
|
|
||||||
|
|
||||||
=begin
|
=begin
|
||||||
|
|
||||||
|
@ -250,14 +249,17 @@ send via account
|
||||||
adapter = options[:outbound][:adapter]
|
adapter = options[:outbound][:adapter]
|
||||||
adapter_options = options[:outbound][:options]
|
adapter_options = options[:outbound][:options]
|
||||||
end
|
end
|
||||||
result = nil
|
|
||||||
begin
|
refresh_xoauth2!
|
||||||
|
|
||||||
driver_class = self.class.driver_class(adapter)
|
driver_class = self.class.driver_class(adapter)
|
||||||
driver_instance = driver_class.new
|
driver_instance = driver_class.new
|
||||||
result = driver_instance.send(adapter_options, params, notification)
|
result = driver_instance.send(adapter_options, params, notification)
|
||||||
self.status_out = 'ok'
|
self.status_out = 'ok'
|
||||||
self.last_log_out = ''
|
self.last_log_out = ''
|
||||||
save!
|
save!
|
||||||
|
|
||||||
|
result
|
||||||
rescue => e
|
rescue => e
|
||||||
error = "Can't use Channel::Driver::#{adapter.to_classname}: #{e.inspect}"
|
error = "Can't use Channel::Driver::#{adapter.to_classname}: #{e.inspect}"
|
||||||
logger.error error
|
logger.error error
|
||||||
|
@ -267,8 +269,6 @@ send via account
|
||||||
save!
|
save!
|
||||||
raise error
|
raise error
|
||||||
end
|
end
|
||||||
result
|
|
||||||
end
|
|
||||||
|
|
||||||
=begin
|
=begin
|
||||||
|
|
||||||
|
@ -338,6 +338,7 @@ get instance of channel driver
|
||||||
|
|
||||||
def refresh_xoauth2!
|
def refresh_xoauth2!
|
||||||
return if options.dig(:auth, :type) != 'XOAUTH2'
|
return if options.dig(:auth, :type) != 'XOAUTH2'
|
||||||
|
return if ApplicationHandleInfo.current == 'application_server'
|
||||||
|
|
||||||
result = ExternalCredential.refresh_token(options[:auth][:provider], options[:auth])
|
result = ExternalCredential.refresh_token(options[:auth][:provider], options[:auth])
|
||||||
|
|
||||||
|
@ -347,16 +348,10 @@ get instance of channel driver
|
||||||
|
|
||||||
return if new_record?
|
return if new_record?
|
||||||
|
|
||||||
# ATTENTION: We don't want to execute any other callbacks here
|
save!
|
||||||
# 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
|
|
||||||
rescue => e
|
rescue => e
|
||||||
logger.error 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
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
|
@ -1,9 +1,5 @@
|
||||||
FactoryBot.define do
|
FactoryBot.define do
|
||||||
factory :channel 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' }
|
area { 'Email::Dummy' }
|
||||||
group { ::Group.find(1) }
|
group { ::Group.find(1) }
|
||||||
active { true }
|
active { true }
|
||||||
|
@ -16,13 +12,23 @@ FactoryBot.define do
|
||||||
area { 'Email::Account' }
|
area { 'Email::Account' }
|
||||||
options do
|
options do
|
||||||
{
|
{
|
||||||
inbound: {
|
inbound: inbound,
|
||||||
|
outbound: outbound,
|
||||||
|
}
|
||||||
|
end
|
||||||
|
|
||||||
|
transient do
|
||||||
|
inbound do
|
||||||
|
{
|
||||||
adapter: 'null', options: {}
|
adapter: 'null', options: {}
|
||||||
},
|
}
|
||||||
outbound: {
|
end
|
||||||
|
|
||||||
|
outbound do
|
||||||
|
{
|
||||||
adapter: 'sendmail'
|
adapter: 'sendmail'
|
||||||
}
|
}
|
||||||
}
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -2,10 +2,94 @@ require 'rails_helper'
|
||||||
|
|
||||||
RSpec.describe Channel, type: :model do
|
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
|
context 'when authentication type is XOAUTH2' do
|
||||||
|
|
||||||
shared_examples 'common 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
|
context 'when non-XOAUTH2 channels are present' do
|
||||||
|
|
||||||
let!(:email_address) { create(:email_address, channel: create(:channel, area: 'Some::Other')) }
|
let!(:email_address) { create(:email_address, channel: create(:channel, area: 'Some::Other')) }
|
||||||
|
|
|
@ -1,6 +1,8 @@
|
||||||
require 'rails_helper'
|
require 'rails_helper'
|
||||||
RSpec.describe 'Gmail XOAUTH2' do # rubocop:disable RSpec/DescribeClass
|
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
|
before do
|
||||||
required_envs = %w[GMAIL_REFRESH_TOKEN GMAIL_CLIENT_ID GMAIL_CLIENT_SECRET GMAIL_USER]
|
required_envs = %w[GMAIL_REFRESH_TOKEN GMAIL_CLIENT_ID GMAIL_CLIENT_SECRET GMAIL_USER]
|
||||||
|
|
|
@ -1,6 +1,8 @@
|
||||||
require 'rails_helper'
|
require 'rails_helper'
|
||||||
RSpec.describe 'Microsoft365 XOAUTH2' do # rubocop:disable RSpec/DescribeClass
|
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
|
before do
|
||||||
required_envs = %w[MICROSOFT365_REFRESH_TOKEN MICROSOFT365_CLIENT_ID MICROSOFT365_CLIENT_SECRET MICROSOFT365_USER]
|
required_envs = %w[MICROSOFT365_REFRESH_TOKEN MICROSOFT365_CLIENT_ID MICROSOFT365_CLIENT_SECRET MICROSOFT365_USER]
|
||||||
|
|
Loading…
Reference in a new issue