From cb3dced00252f3cfc930739050841008b9c1796b Mon Sep 17 00:00:00 2001
From: Rolf Schmidt <rolf.schmidt@zammad.com>
Date: Thu, 1 Jul 2021 08:58:09 +0000
Subject: [PATCH] Fixes #3617 - Creating and editing users via office 365 fails
 with image source is invalid.

---
 app/models/user/avatar.rb                     | 11 +++++--
 ...144000_issue_3617_user_image_source_fix.rb | 12 +++++++
 .../issue_3617_user_image_source_fix_spec.rb  | 31 +++++++++++++++++++
 spec/models/user_spec.rb                      | 14 ++++++---
 4 files changed, 62 insertions(+), 6 deletions(-)
 create mode 100644 db/migrate/20210622144000_issue_3617_user_image_source_fix.rb
 create mode 100644 spec/db/migrate/issue_3617_user_image_source_fix_spec.rb

diff --git a/app/models/user/avatar.rb b/app/models/user/avatar.rb
index b5de46c56..c034bfb23 100644
--- a/app/models/user/avatar.rb
+++ b/app/models/user/avatar.rb
@@ -8,9 +8,16 @@ class User
       after_create :avatar_for_email_check, unless: -> { BulkImportInfo.enabled? }
       after_update :avatar_for_email_check, unless: -> { BulkImportInfo.enabled? }
 
-      validates :image_source, format: URI::DEFAULT_PARSER.make_regexp(%w[http https]), allow_blank: true
+      before_validation :ensure_existing_image, :remove_invalid_image_source
+    end
 
-      before_validation :ensure_existing_image
+    def remove_invalid_image_source
+      return if image_source.blank?
+      return if image_source.match?(URI::DEFAULT_PARSER.make_regexp(%w[http https]))
+
+      Rails.logger.debug "Removed invalid image source '#{image_source}' for user '#{email}'"
+
+      self.image_source = nil
     end
 
     def avatar_for_email_check
diff --git a/db/migrate/20210622144000_issue_3617_user_image_source_fix.rb b/db/migrate/20210622144000_issue_3617_user_image_source_fix.rb
new file mode 100644
index 000000000..c89910757
--- /dev/null
+++ b/db/migrate/20210622144000_issue_3617_user_image_source_fix.rb
@@ -0,0 +1,12 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+class Issue3617UserImageSourceFix < ActiveRecord::Migration[5.2]
+  def change
+    return if !Setting.exists?(name: 'system_init_done')
+
+    User.where.not(image_source: nil).find_each do |user|
+      user.remove_invalid_image_source
+      user.save!
+    end
+  end
+end
diff --git a/spec/db/migrate/issue_3617_user_image_source_fix_spec.rb b/spec/db/migrate/issue_3617_user_image_source_fix_spec.rb
new file mode 100644
index 000000000..ff76ef538
--- /dev/null
+++ b/spec/db/migrate/issue_3617_user_image_source_fix_spec.rb
@@ -0,0 +1,31 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe Issue3617UserImageSourceFix, type: :db_migration, db_strategy: :reset do
+  describe 'when invalid user' do
+    let!(:user) do
+      user = create(:user)
+      user.update_column(:image_source, 'invalid stuff!!!')
+      user
+    end
+
+    it 'removes invalid image sources' do
+      migrate
+      expect(user.reload.image_source).to eq(nil)
+    end
+  end
+
+  describe 'when valid user' do
+    let!(:user) do
+      user = create(:user)
+      user.update_column(:image_source, 'https://zammad.org/avatar.png')
+      user
+    end
+
+    it 'does not change anything' do
+      migrate
+      expect(user.reload.image_source).to eq('https://zammad.org/avatar.png')
+    end
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index ad2b531b1..cc59b2c07 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -837,12 +837,18 @@ RSpec.describe User, type: :model do
         let(:value) { 'Th1515n0t4v4l1dh45h' }
         let(:escaped) { Regexp.escape(value) }
 
-        it 'prevents create' do
-          expect { create(:user, image_source: value) }.to raise_error(ActiveRecord::RecordInvalid, %r{Image source})
+        it 'valid create' do
+          expect(create(:user, image_source: 'https://zammad.org/avatar.png').image_source).not_to eq(nil)
         end
 
-        it 'prevents update' do
-          expect { create(:user).update!(image_source: value) }.to raise_error(ActiveRecord::RecordInvalid, %r{Image source})
+        it 'removes invalid image source of create' do
+          expect(create(:user, image_source: value).image_source).to eq(nil)
+        end
+
+        it 'removes invalid image source of update' do
+          user = create(:user)
+          user.update!(image_source: value)
+          expect(user.image_source).to eq(nil)
         end
       end
     end