Fixes #4644 - Client secret change in MS365 Channel not working.

Co-authored-by: Dominik Klein <dk@zammad.com>

app/models/external_credential.rb

@@ -6,6 +6,8 @@ class ExternalCredential < ApplicationModel
   validates :name, presence: true
   store     :credentials
 
+  after_update_commit :update_client_secret
+
   def self.app_verify(params)
     backend = load_backend(params[:provider])
     backend.app_verify(params)
@@ -38,4 +40,20 @@ class ExternalCredential < ApplicationModel
     "ExternalCredential::#{provider.camelcase}".constantize
   end
 
+  private
+
+  def update_client_secret
+    return if !saved_change_to_credentials?
+
+    previous_client_secret = saved_changes['credentials'].first['client_secret']
+    current_client_secret = saved_changes['credentials'].last['client_secret']
+    return if previous_client_secret.blank? || current_client_secret.blank?
+    return if previous_client_secret == current_client_secret
+
+    backend = ExternalCredential.load_backend(name)
+    return if !backend.respond_to?(:update_client_secret)
+
+    backend.update_client_secret(previous_client_secret, current_client_secret)
+  end
+
 end

lib/external_credential/base/channel_xoauth2.rb

@@ -0,0 +1,20 @@
+# Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
+
+class ExternalCredential::Base::ChannelXoauth2
+  def self.channel_area
+    raise NotImplementedError
+  end
+
+  def self.update_client_secret(previous_client_secret, current_client_secret)
+    Channel.in_area(channel_area).find_each do |channel|
+      channel_client_secret = channel.options.dig(:auth, :client_secret)
+      next if channel_client_secret.blank?
+      next if channel_client_secret != previous_client_secret
+
+      channel.options[:auth][:client_secret] = current_client_secret
+      channel.save!
+    end
+
+    true
+  end
+end

lib/external_credential/exchange.rb

@@ -215,4 +215,13 @@ class ExternalCredential::Exchange
     JSON.parse(Base64.decode64(split)).symbolize_keys
   end
 
+  def self.update_client_secret(previous_client_secret, current_client_secret)
+    config = Setting.get('exchange_oauth')
+    return true if config.blank?
+    return true if config['client_secret'].blank? || config['client_secret'] != previous_client_secret
+
+    Setting.set('exchange_oauth', config.merge(client_secret: current_client_secret))
+
+    true
+  end
 end

lib/external_credential/google.rb

@@ -1,6 +1,9 @@
 # Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
 
-class ExternalCredential::Google
+class ExternalCredential::Google < ExternalCredential::Base::ChannelXoauth2
+  def self.channel_area
+    'Google::Account'.freeze
+  end
 
   def self.app_verify(params)
     request_account_to_link(params, false)
@@ -281,5 +284,4 @@ class ExternalCredential::Google
 
     JSON.parse(Base64.decode64(split)).symbolize_keys
   end
-
 end

lib/external_credential/microsoft_base.rb

@@ -1,10 +1,6 @@
 # Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
 
-class ExternalCredential::MicrosoftBase
-  def self.channel_area
-    raise NotImplementedError
-  end
-
+class ExternalCredential::MicrosoftBase < ExternalCredential::Base::ChannelXoauth2
   def self.provider_name
     name.demodulize.underscore
   end

spec/lib/external_credential/exchange_spec.rb

@@ -27,4 +27,45 @@ RSpec.describe ExternalCredential::Exchange do
       end
     end
   end
+
+  describe '.update_client_secret' do
+    let(:external_credential) { create(:external_credential, name: 'exchange', credentials:) }
+    let(:credentials)         { { client_id: 'id1337', client_secret: 'dummy' } }
+
+    context 'when Exchange integration is not completely configured' do
+      before do
+        Setting.set('exchange_oauth', { client_id: 'id1337' })
+      end
+
+      it 'does not update the setting' do
+        external_credential.update!(credentials: credentials)
+
+        expect(Setting.get('exchange_oauth')).to eq({ 'client_id' => 'id1337' })
+      end
+    end
+
+    context 'when Exchange integration is completely configured' do
+      before do
+        Setting.set('exchange_oauth', { client_id: 'id1337', client_secret: 'dummy' })
+      end
+
+      context 'when client_secret is different' do
+        let(:credentials)         { { client_id: 'id1337', client_secret: 'dummy-other' } }
+
+        it 'does not update the setting' do
+          external_credential.update!(credentials: credentials.merge(client_secret: 'new-dummy'))
+
+          expect(Setting.get('exchange_oauth')).to eq({ 'client_id' => 'id1337', 'client_secret' => 'dummy' })
+        end
+      end
+
+      context 'when client_secret is the same' do
+        it 'does update the setting' do
+          external_credential.update!(credentials: credentials.merge(client_secret: 'new-dummy'))
+
+          expect(Setting.get('exchange_oauth')).to eq({ 'client_id' => 'id1337', 'client_secret' => 'new-dummy' })
+        end
+      end
+    end
+  end
 end

spec/lib/external_credential/microsoft365_spec.rb

@@ -372,4 +372,41 @@ RSpec.describe ExternalCredential::Microsoft365 do
       expect(info[:email]).to eq(email_address)
     end
   end
+
+  describe '.update_client_secret' do
+    let(:channel) do
+      ENV['MICROSOFT365_USER'] = 'zammad@outlook.com'
+      ENV['MICROSOFT365_CLIENT_ID']     = 'id1337'
+      ENV['MICROSOFT365_CLIENT_SECRET'] = 'dummy'
+      ENV['MICROSOFT365_CLIENT_TENANT'] = 'xxx'
+
+      create(:microsoft365_channel)
+    end
+
+    let(:external_credential) { create(:external_credential, name: provider, credentials: { client_id: 'id1337', client_secret: 'dummy' }) }
+
+    context 'when client_secret was updated' do
+      context 'when secret is different' do
+        before do
+          channel.options[:auth][:client_secret] = 'dummy-other'
+          channel.save!
+        end
+
+        it 'does not update the channel' do
+          external_credential.update!(credentials: { client_id: 'id1337', client_secret: 'dummy-new' })
+
+          expect(channel.reload.options[:auth][:client_secret]).to eq('dummy-other')
+        end
+      end
+
+      context 'when secret is the same' do
+        it 'updates the setting' do
+          channel
+          external_credential.update!(credentials: { client_id: 'id1337', client_secret: 'dummy-new' })
+
+          expect(channel.reload.options[:auth][:client_secret]).to eq('dummy-new')
+        end
+      end
+    end
+  end
 end

spec/lib/external_credential/microsoft_graph_spec.rb

@@ -365,4 +365,42 @@ RSpec.describe ExternalCredential::MicrosoftGraph do
       expect(info[:email]).to eq(email_address)
     end
   end
+
+  describe '.update_client_secret' do
+    let(:channel) do
+      # TODO: change ENV
+      ENV['MICROSOFT365_USER'] = 'zammad@outlook.com'
+      ENV['MICROSOFT365_CLIENT_ID']     = 'id1337'
+      ENV['MICROSOFT365_CLIENT_SECRET'] = 'dummy'
+      ENV['MICROSOFT365_CLIENT_TENANT'] = 'xxx'
+
+      create(:microsoft_graph_channel)
+    end
+
+    let(:external_credential) { create(:external_credential, name: provider, credentials: { client_id: 'id1337', client_secret: 'dummy' }) }
+
+    context 'when client_secret was updated' do
+      context 'when secret is different' do
+        before do
+          channel.options[:auth][:client_secret] = 'dummy-other'
+          channel.save!
+        end
+
+        it 'does not update the channel' do
+          external_credential.update!(credentials: { client_id: 'id1337', client_secret: 'dummy-new' })
+
+          expect(channel.reload.options[:auth][:client_secret]).to eq('dummy-other')
+        end
+      end
+
+      context 'when secret is the same' do
+        it 'updates the setting' do
+          channel
+          external_credential.update!(credentials: { client_id: 'id1337', client_secret: 'dummy-new' })
+
+          expect(channel.reload.options[:auth][:client_secret]).to eq('dummy-new')
+        end
+      end
+    end
+  end
 end

spec/models/external_credential_spec.rb

@@ -0,0 +1,65 @@
+# Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe ExternalCredential, :aggregate_failures, current_user_id: 1, type: :model do
+  describe '#update_client_secret' do
+    let(:external_credential) { create(:external_credential, name: 'google', credentials:) }
+
+    let(:credentials) do
+      {
+        'client_secret' => 'dummy-1337',
+        'code'          => 'code123',
+        'grant_type'    => 'authorization_code',
+        'client_id'     => 'dummy123',
+        'redirect_uri'  => described_class.callback_url('google'),
+      }
+    end
+
+    before do
+      allow(ExternalCredential::Google).to receive(:update_client_secret).and_call_original
+    end
+
+    context 'when credentials are not changed' do
+      it 'does not call update_client_secret on the backend module' do
+        external_credential.update!(created_at: Time.zone.now)
+
+        expect(ExternalCredential::Google).not_to have_received(:update_client_secret)
+      end
+    end
+
+    context 'when credentials are changed' do
+      context 'when client_secret is blank' do
+        it 'does not call update_client_secret on the backend module' do
+          external_credential.update!(credentials: credentials.merge('client_secret' => ''))
+
+          expect(ExternalCredential::Google).not_to have_received(:update_client_secret)
+        end
+      end
+
+      context 'when client_secret is not changed' do
+        it 'does not call update_client_secret on the backend module' do
+          external_credential.update!(credentials: credentials)
+
+          expect(ExternalCredential::Google).not_to have_received(:update_client_secret)
+        end
+      end
+
+      context 'when client_id is changed' do
+        it 'does not call update_client_secret on the backend module' do
+          external_credential.update!(credentials: credentials.merge('client_id' => 'dummy456'))
+
+          expect(ExternalCredential::Google).not_to have_received(:update_client_secret)
+        end
+      end
+
+      context 'when client_secret is changed' do
+        it 'calls update_client_secret on the backend module' do
+          external_credential.update!(credentials: credentials.merge('client_secret' => 'new-dummy-1337'))
+
+          expect(ExternalCredential::Google).to have_received(:update_client_secret).with('dummy-1337', 'new-dummy-1337')
+        end
+      end
+    end
+  end
+end