Merge pull request #283 from iaebots/222-fix-uploaders

Image uploaders

.gitignore

@@ -37,3 +37,4 @@ yarn-debug.log*
 
 /public/uploads
 .vscode/
+.idea

app/models/bot.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 class Bot < ApplicationRecord
   has_many :posts, dependent: :destroy
   has_many :comments, dependent: :destroy
@@ -19,16 +21,19 @@ class Bot < ApplicationRecord
   validate :validate_minimum_cover_image_size
   validate :validate_maximum_cover_image_size
 
+  # validates avatar image size
+  validate :validate_maximum_avatar_image_size
+  validate :validate_minimum_avatar_image_size
+
   validates_length_of :bio, minimum: 1, maximum: 512 # validates length of bot's bio
 
   # ensure bot's username doesn't contain symbols nor special characters
   validates_format_of :username, with: /\A[a-zA-Z0-9_-]*\z/
   validates_length_of :username, minimum: 3, maximum: 32
 
-  #URL max value
+  # URL max value
   validates_length_of :repository, maximum: 64
 
-
   extend FriendlyId
   friendly_id :username, use: :slugged # username as friendly_id
 
@@ -79,16 +84,18 @@ def tag_list_count
     errors.add(:tag_list, '1 tags minimum') if tag_list.count < 1
     errors.add(:tag_list, '16 tags maximum') if tag_list.count > 16
 
-    self.tag_list.each do |tag|
+    tag_list.each do |tag|
       errors.add(:tag_list, "#{tag} must be shorter than 32 characters maximum") if tag.length > 32
-      errors.add(:tag_list, 'must only contain letters, numbers or _-.
-            Tags must be separated by commas.') unless tag =~ /^[a-zA-z][a-zA-Z0-9_-]*$/
+      unless tag =~ /^[a-zA-z][a-zA-Z0-9_-]*$/
+        errors.add(:tag_list, 'must only contain letters, numbers or _-.
+              Tags must be separated by commas.')
+      end
       errors.add(:tag_list, "#{tag} must be longer than 4 characters minimum") if tag.length < 4
     end
   end
 
-   # Generate a unique API key
-   def self.generate_api_key
+  # Generate a unique API key
+  def self.generate_api_key
     loop do
       token = SecureRandom.hex(16)
       break token unless Bot.exists?(api_key: token)
@@ -106,17 +113,31 @@ def self.generate_api_secret
   def validate_minimum_cover_image_size
     if cover.path
       image = MiniMagick::Image.open(cover.path)
-      unless image[:width] > 640 && image[:height] > 180
-        errors.add :cover, "should be 640x180px minimum!"
-      end
+      errors.add :cover, 'should be 640x180px minimum!' unless image[:width] >= 640 && image[:height] >= 180
     end
   end
 
   def validate_maximum_cover_image_size
     if cover.path
       image = MiniMagick::Image.open(cover.path)
-      unless image[:width] <= 1280 && image[:height] <= 360
-        errors.add :cover, "should be 1280x360px maximum!"
+      errors.add :cover, 'should be 1280x360px maximum!' unless image[:width] <= 1280 && image[:height] <= 360
+    end
+  end
+
+  def validate_minimum_avatar_image_size
+    if avatar.path
+      image = MiniMagick::Image.open(avatar.path)
+      unless image[:width].to_i >= image[:heigth].to_i / 2 && image[:height].to_i >= image[:width].to_i / 2
+        errors.add :avatar, 'image size not accepted. Try another image size'
+      end
+    end
+  end
+
+  def validate_maximum_avatar_image_size
+    if avatar.path
+      image = MiniMagick::Image.open(avatar.path)
+      unless image[:width].to_i <= image[:height].to_i * 2 && image[:height].to_i <= image[:width].to_i * 2
+        errors.add :avatar, 'image size not accepted. Try another image size'
       end
     end
   end

app/models/developer.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 class Developer < ApplicationRecord
   devise :database_authenticatable, :registerable, :confirmable, :lockable,
          :recoverable, :rememberable, :validatable, authentication_keys: [:login]
@@ -17,12 +19,16 @@ class Developer < ApplicationRecord
   validate :validate_minimum_cover_image_size
   validate :validate_maximum_cover_image_size
 
+  # validates avatar image size
+  validate :validate_maximum_avatar_image_size
+  validate :validate_minimum_avatar_image_size
+
   attr_writer :login
 
   validate :validate_username, if: :username_changed?
 
   # validate checkbox guidelines
-  validates :accept_terms, :acceptance => true
+  validates :accept_terms, acceptance: true
 
   # regex to assure username doesn't have a @
   validates_format_of :username, with: /\A[a-zA-Z0-9_-]*\z/
@@ -36,7 +42,7 @@ class Developer < ApplicationRecord
   # validates if password has at least 1 capital, at least 1 number and at least
   # one lower case. Min length 6, max length 64
   validates_format_of :password, with: /\A(?=.*[A-Z].*)(?=.*[0-9].*)(?=.*[a-z].*).{6,64}\z/,
-    message: 'must contain at least one capital, one lowercase and one number', if: :encrypted_password_changed?
+                                 message: 'must contain at least one capital, one lowercase and one number', if: :encrypted_password_changed?
 
   has_many :bots, dependent: :destroy
   has_many :likes, dependent: :destroy
@@ -58,10 +64,10 @@ def login
   # override auth method to allow login with different params
   def self.find_for_database_authentication(warden_conditions)
     conditions = warden_conditions.dup
-    if login = conditions.delete(:login)
+    if (login = conditions.delete(:login))
       where(conditions.to_h).where(['lower(username) = :value OR lower(email) = :value ',
                                     { value: login.downcase }]).first
-    elsif conditions.has_key?(:username) || conditions.has_key?(:email)
+    elsif conditions.key?(:username) || conditions.key?(:email)
       where(conditions.to_h).first
     end
   end
@@ -80,17 +86,31 @@ def validate_username
   def validate_minimum_cover_image_size
     if cover.path
       image = MiniMagick::Image.open(cover.path)
-      unless image[:width] > 640 && image[:height] > 180
-        errors.add :cover, "should be 640x180px minimum!"
-      end
+      errors.add :cover, 'should be 640x180px minimum!' unless image[:width] >= 640 && image[:height] >= 180
     end
   end
 
   def validate_maximum_cover_image_size
     if cover.path
       image = MiniMagick::Image.open(cover.path)
-      unless image[:width] <= 1280 && image[:height] <= 360
-        errors.add :cover, "should be 1280x360px maximum!"
+      errors.add :cover, 'should be 1280x360px maximum!' unless image[:width] <= 1280 && image[:height] <= 360
+    end
+  end
+
+  def validate_minimum_avatar_image_size
+    if avatar.path
+      image = MiniMagick::Image.open(avatar.path)
+      unless image[:width].to_i >= image[:height].to_i / 2 && image[:height].to_i >= image[:width].to_i / 2
+        errors.add :avatar, 'image size not accepted. Try another image size'
+      end
+    end
+  end
+
+  def validate_maximum_avatar_image_size
+    if avatar.path
+      image = MiniMagick::Image.open(avatar.path)
+      unless image[:width].to_i <= image[:height].to_i * 2 && image[:height].to_i <= image[:width].to_i * 2
+        errors.add :avatar, 'image size not accepted. Try another image size'
       end
     end
   end

app/uploaders/avatar_uploader.rb

@@ -4,14 +4,16 @@ class AvatarUploader < CarrierWave::Uploader::Base
   # Choose what kind of storage to use for this uploader:
   storage :file
 
+  before :process, :validate
+
   # Directory where uploaded files will be stored.
   def store_dir
     "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
   end
 
   # Provide a default URL as a default if there hasn't been a file uploaded
-  def default_url(*args)
-    ActionController::Base.helpers.asset_path("fallback/" + ["default.png"].compact.join('_'))
+  def default_url(*_args)
+    ActionController::Base.helpers.asset_path("fallback/#{['default.png'].compact.join('_')}")
   end
 
   # All uploads will be resized to 400x400
@@ -24,11 +26,43 @@ def default_url(*args)
 
   # Allowlist of extensions which are allowed to be uploaded.
   def extension_allowlist
-    %w(jpg jpeg png gif)
+    %w[jpg jpeg png gif]
   end
 
   # Override the filename of the uploaded files
   def filename
     "#{model.username.to_s.underscore}.#{file.extension}" if original_filename.present?
   end
+
+  # validate content type of image before processing it
+  def validate(file)
+    unless file_is_image(file.path)
+      StandardError
+      raise CarrierWave::IntegrityError, I18n.translate(:"errors.messages.content_type_whitelist_error", content_type: content_type,
+                                                                                                         allowed_types: Array(content_type_allowlist).join(', '), default: :"errors.messages.content_type_allowlist_error")
+    end
+  end
+
+  # list of valid signatures for this uploader
+  VALID_IMAGE_SIGNATURES = [
+    "\x89PNG\r\n\x1A\n".force_encoding(Encoding::ASCII_8BIT), # PNG
+    'GIF87a'.force_encoding(Encoding::ASCII_8BIT), # GIF87
+    'GIF89a'.force_encoding(Encoding::ASCII_8BIT), # GIF89
+    "\xFF\xD8".force_encoding(Encoding::ASCII_8BIT) # JPEG/JPG
+  ].freeze
+
+  # read first 8 bytes of file and check if it's a valid signature
+  def file_is_image(temporary_file_path)
+    return false unless temporary_file_path
+
+    file_stream = File.new(temporary_file_path, 'r')
+    first_eight_bytes = file_stream.readpartial(8)
+    file_stream.close
+
+    VALID_IMAGE_SIGNATURES.each do |signature|
+      return true if first_eight_bytes.start_with?(signature)
+    end
+
+    false
+  end
 end

app/uploaders/cover_uploader.rb

@@ -4,26 +4,58 @@ class CoverUploader < CarrierWave::Uploader::Base
   # Choose what kind of storage to use for this uploader:
   storage :file
 
+  before :process, :validate
+
   # Override the directory where uploaded files will be stored.
   def store_dir
     "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
   end
 
   # Provide a default URL as a default if there hasn't been a file uploaded:
-  def default_url(*args)
-    ActionController::Base.helpers.asset_path("fallback/" + ["default_cover.png"].compact.join('_'))
+  def default_url(*_args)
+    ActionController::Base.helpers.asset_path("fallback/#{['default_cover.png'].compact.join('_')}")
   end
 
   # Process files as they are uploaded:
   # process resize_to_fit: [780, 180]
 
   # Add an allowlist of extensions which are allowed to be uploaded.
   def extension_allowlist
-    %w(jpg jpeg gif png)
+    %w[jpg jpeg gif png]
   end
 
   # Override the filename of the uploaded files:
   def filename
     "#{model.username.to_s.underscore}-cover.#{file.extension}" if original_filename
   end
+
+  # validate content type of image before processing it
+  def validate(file)
+    unless file_is_image(file.path)
+      StandardError
+      raise CarrierWave::IntegrityError, I18n.translate(:"errors.messages.content_type_whitelist_error", content_type: content_type,
+                                                                                                       allowed_types: Array(content_type_allowlist).join(', '), default: :"errors.messages.content_type_allowlist_error")
+    end
+  end
+
+  # list of valid signatures for this uploader
+  VALID_IMAGE_SIGNATURES = [
+    "\x89PNG\r\n\x1A\n".force_encoding(Encoding::ASCII_8BIT), # PNG
+    "\xFF\xD8".force_encoding(Encoding::ASCII_8BIT) # JPEG/JPG
+  ].freeze
+
+  # read first 8 bytes of file and check if it's a valid signature
+  def file_is_image(temporary_file_path)
+    return false unless temporary_file_path
+
+    file_stream = File.new(temporary_file_path, 'r')
+    first_eight_bytes = file_stream.readpartial(8)
+    file_stream.close
+
+    VALID_IMAGE_SIGNATURES.each do |signature|
+      return true if first_eight_bytes.start_with?(signature)
+    end
+
+    false
+  end
 end

config/locales/en.yml

@@ -6,13 +6,17 @@ en:
           attributes:
             username:
               already_taken: "has already been taken"
-    
+
         guest:
           attributes:
             username:
               already_taken: "has already been taken"
 
-        bot:       
+        bot:
           attributes:
             username:
-              already_taken: "has already been taken"
+              already_taken: "has already been taken"
+
+  errors:
+    messages:
+      content_type_whitelist_error: "content type or extension not allowed"