Fixes #5111 - Email Channel cannot update XOauth2 token if identical …

…channel exists

app/models/channel.rb

@@ -12,7 +12,7 @@ class Channel < ApplicationModel
   scope :active, -> { where(active: true) }
   scope :in_area, ->(area) { where(area: area) }
 
-  validates_with Validations::EmailAccountUniquenessValidator
+  validates_with Validations::ChannelEmailAccountUniquenessValidator
 
   after_create   :email_address_check
   after_update   :email_address_check

i18n/zammad.pot

@@ -12524,7 +12524,7 @@ msgstr ""
 msgid "The provided credentials are invalid."
 msgstr ""
 
-#: lib/validations/email_account_uniqueness_validator.rb
+#: lib/validations/channel_email_account_uniqueness_validator.rb
 msgid "The provided email account is already in use."
 msgstr ""
 

lib/validations/email_account_uniqueness_validator.rb → lib/validations/channel_email_account_uniqueness_validator.rb

@@ -1,14 +1,15 @@
 # Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
 
-class Validations::EmailAccountUniquenessValidator < ActiveModel::Validator
+class Validations::ChannelEmailAccountUniquenessValidator < ActiveModel::Validator
   MATCHED_KEYS = %i[adapter].freeze
   MATCHED_OPTIONS_KEYS = %i[host port user folder].freeze
   MATCHED_AREAS = %w[Email::Account Google::Account Microsoft365::Account].freeze
 
   def validate(record)
     return if MATCHED_AREAS.exclude?(record.area)
+    return if !matching_changes?(record)
 
-    record_data = extract_matched_data(record)
+    record_data = extract_matched_data(record.options)
 
     return if scope(record).none? { matches?(record_data, _1) }
 
@@ -17,6 +18,13 @@ def validate(record)
 
   private
 
+  # https://github.com/zammad/zammad/issues/5111
+  # Some systems may have duplicate channels created before this validation was added
+  # Checking uniqueness on any update blocks XOAuth2 token update on such channels
+  def matching_changes?(record)
+    extract_matched_data(record.options) != extract_matched_data(record.options_was)
+  end
+
   def scope(record)
     record
       .class
@@ -25,15 +33,15 @@ def scope(record)
   end
 
   def matches?(record_data, other_record)
-    other_record_data = extract_matched_data(other_record)
+    other_record_data = extract_matched_data(other_record.options)
 
     return false if other_record_data.blank?
 
     record_data == other_record_data
   end
 
-  def extract_matched_data(record)
-    server_data = record.options&.dig(:inbound)
+  def extract_matched_data(options)
+    server_data = options&.dig(:inbound)
 
     return if !server_data
 

spec/factories/channel.rb

@@ -244,6 +244,10 @@
     end
 
     factory :google_channel do
+      transient do
+        gmail_user { ENV['GMAIL_USER'] }
+      end
+
       area { 'Google::Account' }
       options do
         {
@@ -253,7 +257,7 @@
               'auth_type'      => 'XOAUTH2',
               'host'           => 'imap.gmail.com',
               'ssl'            => 'ssl',
-              'user'           => ENV['GMAIL_USER'],
+              'user'           => gmail_user,
               'folder'         => '',
               'keep_on_server' => false,
             }
@@ -264,7 +268,7 @@
               'host'           => 'smtp.gmail.com',
               'port'           => 465,
               'ssl'            => true,
-              'user'           => ENV['GMAIL_USER'],
+              'user'           => gmail_user,
               'authentication' => 'xoauth2',
             }
           },

spec/lib/validations/email_account_uniqueness_validator_spec.rb → spec/lib/validations/channel_email_account_uniqueness_validator_spec.rb

@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe Validations::EmailAccountUniquenessValidator do
+RSpec.describe Validations::ChannelEmailAccountUniquenessValidator do
   subject(:validator) { described_class.new }
 
   before { Channel.destroy_all }
@@ -55,5 +55,32 @@
         expect(another_channel.errors).to be_blank
       end
     end
+
+    # https://github.com/zammad/zammad/issues/5111
+    context 'with multiple identical channels' do
+      let(:duplicate_channel) { create(:google_channel, gmail_user: 'email@example.com') }
+      let(:editable_channel) do
+        build(:google_channel, gmail_user: 'email@example.com')
+          .tap { _1.save!(validate: false) }
+      end
+
+      let(:new_token) { 'new_xoauth2_token' }
+
+      before do
+        duplicate_channel
+
+        allow(ExternalCredential)
+          .to receive(:refresh_token).and_return(access_token: new_token)
+      end
+
+      it 'allows to edit XOauth2 token if identical channel exists' do
+        editable_channel.refresh_xoauth2!(force: true)
+
+        expect(editable_channel.options).to include(
+          inbound:  include(options: include(password: new_token)),
+          outbound: include(options: include(password: new_token)),
+        )
+      end
+    end
   end
 end

spec/models/channel_spec.rb

@@ -126,7 +126,7 @@ def fetchable?(...)
 
   describe 'validations' do
     it 'validates email account uniqueness' do
-      expect_any_instance_of(Validations::EmailAccountUniquenessValidator)
+      expect_any_instance_of(Validations::ChannelEmailAccountUniquenessValidator)
         .to receive(:validate).once
 
       create(:email_channel)