diff --git a/app/models/user.rb b/app/models/user.rb index 20ee1c40c..6503881a3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -14,8 +14,6 @@ class User < ApplicationRecord has_many :pinned_cards, through: :pins, source: :card has_many :exports, class_name: "Account::Export", dependent: :destroy - scope :with_avatars, -> { preload(:account, :avatar_attachment) } - def deactivate transaction do accesses.destroy_all diff --git a/app/models/user/avatar.rb b/app/models/user/avatar.rb index c635edddb..400738cb1 100644 --- a/app/models/user/avatar.rb +++ b/app/models/user/avatar.rb @@ -2,13 +2,20 @@ module User::Avatar extend ActiveSupport::Concern ALLOWED_AVATAR_CONTENT_TYPES = %w[ image/jpeg image/png image/gif image/webp ].freeze + MAX_AVATAR_DIMENSIONS = { width: 4096, height: 4096 }.freeze included do has_one_attached :avatar do |attachable| attachable.variant :thumb, resize_to_fill: [ 256, 256 ], process: :immediately end - validate :avatar_content_type_allowed + scope :with_avatars, -> { preload(:account, :avatar_attachment) } + + validate :avatar_content_type_allowed, :avatar_dimensions_allowed, if: :avatar_attached? + end + + def avatar_attached? + avatar.attached? end def avatar_thumbnail @@ -17,8 +24,23 @@ module User::Avatar private def avatar_content_type_allowed - if avatar.attached? && !ALLOWED_AVATAR_CONTENT_TYPES.include?(avatar.content_type) + if !ALLOWED_AVATAR_CONTENT_TYPES.include?(avatar.content_type) errors.add(:avatar, "must be a JPEG, PNG, GIF, or WebP image") end end + + def avatar_dimensions_allowed + return unless avatar.blob.analyzed? || avatar.blob.analyze + + width = avatar.blob.metadata[:width] + height = avatar.blob.metadata[:height] + + if width && width > MAX_AVATAR_DIMENSIONS[:width] + errors.add(:avatar, "width must be less than #{MAX_AVATAR_DIMENSIONS[:width]}px") + end + + if height && height > MAX_AVATAR_DIMENSIONS[:height] + errors.add(:avatar, "height must be less than #{MAX_AVATAR_DIMENSIONS[:height]}px") + end + end end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 20008dcee..8e99e4cef 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -65,7 +65,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_response :forbidden end - test "update with invalid avatar shows validation error" do + test "update with invalid avatar content type shows validation error" do sign_in_as :kevin svg_file = fixture_file_upload("avatar.svg", "image/svg+xml") @@ -76,6 +76,18 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_select ".txt-negative", text: /must be a JPEG, PNG, GIF, or WebP image/ end + test "update with oversized avatar shows validation error" do + sign_in_as :kevin + + png_file = fixture_file_upload("avatar.png", "image/png") + + ActiveStorage::Analyzer::ImageAnalyzer::Vips.any_instance.stubs(:metadata).returns({ width: 5000, height: 100 }) + + put user_path(users(:kevin)), params: { user: { avatar: png_file } } + assert_response :unprocessable_entity + assert_select ".txt-negative", text: /width must be less than 4096px/ + end + test "update with valid avatar" do sign_in_as :kevin diff --git a/test/models/user/avatar_test.rb b/test/models/user/avatar_test.rb index ed502d7ab..fcaf05d0b 100644 --- a/test/models/user/avatar_test.rb +++ b/test/models/user/avatar_test.rb @@ -33,4 +33,27 @@ class User::AvatarTest < ActiveSupport::TestCase assert users(:david).avatar.variant(:thumb).processed? end + + test "rejects images that are too wide" do + users(:david).avatar.attach(io: File.open(file_fixture("avatar.png")), filename: "avatar.png", content_type: "image/png") + users(:david).avatar.blob.update!(metadata: { analyzed: true, width: 5000, height: 100 }) + + assert_not users(:david).valid? + assert_includes users(:david).errors[:avatar], "width must be less than #{User::Avatar::MAX_AVATAR_DIMENSIONS[:width]}px" + end + + test "rejects images that are too tall" do + users(:david).avatar.attach(io: File.open(file_fixture("avatar.png")), filename: "avatar.png", content_type: "image/png") + users(:david).avatar.blob.update!(metadata: { analyzed: true, width: 100, height: 5000 }) + + assert_not users(:david).valid? + assert_includes users(:david).errors[:avatar], "height must be less than #{User::Avatar::MAX_AVATAR_DIMENSIONS[:height]}px" + end + + test "accepts images within dimension limits" do + users(:david).avatar.attach(io: File.open(file_fixture("avatar.png")), filename: "avatar.png", content_type: "image/png") + users(:david).avatar.blob.update!(metadata: { analyzed: true, width: 4096, height: 4096 }) + + assert users(:david).valid? + end end