From b49046c597c10390913c3d939916c63602d82ea5 Mon Sep 17 00:00:00 2001
From: Thorsten Eckel <te@zammad.com>
Date: Mon, 12 Apr 2021 16:03:51 +0200
Subject: [PATCH] Maintenance: Move User avatar logic into dedicated concern to
 slim down model file.

---
 app/models/user.rb        | 30 +--------------------
 app/models/user/avatar.rb | 56 +++++++++++++++++++++++++++++++++++++++
 spec/models/user_spec.rb  | 31 ++++++++++++++++++++++
 3 files changed, 88 insertions(+), 29 deletions(-)
 create mode 100644 app/models/user/avatar.rb

diff --git a/app/models/user.rb b/app/models/user.rb
index dbf0063f2..7cd485e53 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -18,6 +18,7 @@ class User < ApplicationModel
   include HasTaskbars
   include User::HasTicketCreateScreenImpact
   include User::Assets
+  include User::Avatar
   include User::Search
   include User::SearchIndex
   include User::TouchesOrganization
@@ -50,9 +51,7 @@ class User < ApplicationModel
   before_validation :check_name, :check_email, :check_login, :ensure_uniq_email, :ensure_password, :ensure_roles, :ensure_identifier
   before_validation :check_mail_delivery_failed, on: :update
   before_create     :check_preferences_default, :validate_preferences, :validate_ooo, :domain_based_assignment, :set_locale
-  after_create      :avatar_for_email_check, unless: -> { BulkImportInfo.enabled? }
   before_update     :check_preferences_default, :validate_preferences, :validate_ooo, :reset_login_failed, :validate_agent_limit_by_attributes, :last_admin_check_by_attribute
-  after_update      :avatar_for_email_check, unless: -> { BulkImportInfo.enabled? }
   before_destroy    :destroy_longer_required_objects, :destroy_move_dependency_ownership
   after_commit      :update_caller_id
 
@@ -1140,33 +1139,6 @@ raise 'Minimum one user need to have admin permissions'
     true
   end
 
-  def avatar_for_email_check
-    return true if Setting.get('import_mode')
-    return true if email.blank?
-
-    email_address_validation = EmailAddressValidation.new(email)
-    return true if !email_address_validation.valid_format?
-
-    return true if !saved_change_to_attribute?('email') && updated_at > Time.zone.now - 10.days
-
-    # save/update avatar
-    avatar = Avatar.auto_detection(
-      object:        'User',
-      o_id:          id,
-      url:           email,
-      source:        'app',
-      updated_by_id: updated_by_id,
-      created_by_id: updated_by_id,
-    )
-
-    # update user link
-    return true if !avatar
-
-    update_column(:image, avatar.store_hash) # rubocop:disable Rails/SkipsModelValidations
-    cache_delete
-    true
-  end
-
   def destroy_longer_required_objects
     ::Avatar.remove(self.class.to_s, id)
     ::UserDevice.remove(id)
diff --git a/app/models/user/avatar.rb b/app/models/user/avatar.rb
new file mode 100644
index 000000000..b5de46c56
--- /dev/null
+++ b/app/models/user/avatar.rb
@@ -0,0 +1,56 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+class User
+  module Avatar
+    extend ActiveSupport::Concern
+
+    included do
+      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
+    end
+
+    def avatar_for_email_check
+      return if Setting.get('import_mode')
+      return if email.blank?
+
+      email_address_validation = EmailAddressValidation.new(email)
+      return if !email_address_validation.valid_format?
+
+      return if !saved_change_to_attribute?('email') && updated_at > Time.zone.now - 10.days
+
+      avatar_auto_detection
+    end
+
+    def ensure_existing_image
+      return if Setting.get('import_mode')
+      return if changes['image'].blank?
+      return if ::Avatar.exists?(store_hash: image)
+
+      raise Exceptions::UnprocessableEntity, "Invalid Store reference '#{image}' in 'image' attribute."
+    end
+
+    private
+
+    def avatar_auto_detection
+      # save/update avatar
+      avatar = ::Avatar.auto_detection(
+        object:        'User',
+        o_id:          id,
+        url:           email,
+        source:        'app',
+        updated_by_id: updated_by_id,
+        created_by_id: updated_by_id,
+      )
+
+      # update user link
+      return if !avatar
+
+      update_column(:image, avatar.store_hash) # rubocop:disable Rails/SkipsModelValidations
+      cache_delete
+    end
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index e5622f833..0b7bac5b5 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -815,6 +815,37 @@ RSpec.describe User, type: :model do
         end
       end
     end
+
+    describe '#image' do
+
+      describe 'when value is invalid' do
+        let(:value) { 'Th1515n0t4v4l1dh45h' }
+
+        it 'prevents create' do
+          expect { create(:user, image: value) }.to raise_error(Exceptions::UnprocessableEntity, %r{#{value}})
+        end
+
+        it 'prevents update' do
+          expect { create(:user).update!(image: value) }.to raise_error(Exceptions::UnprocessableEntity, %r{#{value}})
+        end
+      end
+    end
+
+    describe '#image_source' do
+
+      describe 'when value is invalid' 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})
+        end
+
+        it 'prevents update' do
+          expect { create(:user).update!(image_source: value) }.to raise_error(ActiveRecord::RecordInvalid, %r{Image source})
+        end
+      end
+    end
   end
 
   describe 'Associations:' do