Do not use ActiveStorage variants, process original avatar

Variants are currently broken. So we process the original file with the
most common avatar dimensions and stripping metadata, then hash and
upload only that version.

app/controllers/avatars_controller.rb

@@ -5,22 +5,20 @@ class AvatarsController < ApplicationController
 
       sha256_hash = params[:hash]
       format = params[:format]&.to_sym || :png
-      size = params[:size]&.to_sym || :original
+      # size = params[:size]&.to_sym || :original
 
       unless user.avatar.filename.to_s == "#{sha256_hash}.#{format}"
         http_status :not_found and return
       end
 
-      blob = if size == :original
-               user.avatar.blob
-             else
-               # TODO Variants use the same custom storage key/path, which
-               # makes blob downloads always fetch the original version instead
-               # of the variant. Needs to be fixed/added in Rails.
-               user.avatar_variant(size: size)&.blob
-             end
+      # TODO See note for avatar_variant in user model
+      # blob = if size == :original
+      #          user.avatar.blob
+      #        else
+      #          user.avatar_variant(size: size)&.blob
+      #        end
 
-      data = blob.download
+      data = user.avatar.blob.download
       send_data data, type: "image/#{format}", disposition: "inline"
     else
       http_status :not_found

app/controllers/settings_controller.rb

@@ -191,9 +191,14 @@ class SettingsController < ApplicationController
     end
 
     def store_user_avatar
-      data = @user.avatar_new.tempfile.read
-      @user.avatar_new.tempfile.rewind
-      hash = Digest::SHA256.hexdigest(data)
+      io = @user.avatar_new.tempfile
+      img_data = process_avatar(io)
+      tempfile = Tempfile.create
+      tempfile.binmode
+      tempfile.write(img_data)
+      tempfile.rewind
+
+      hash = Digest::SHA256.hexdigest(img_data)
       ext = @user.avatar_new.content_type == "image/png" ? "png" : "jpg"
       filename = "#{hash}.#{ext}"
 
@@ -202,8 +207,22 @@ class SettingsController < ApplicationController
         false
       else
         key = "users/#{@user.cn}/avatars/#{filename}"
-        @user.avatar.attach io: @user.avatar_new.tempfile, key: key, filename: filename
+        @user.avatar.attach io: tempfile, key: key, filename: filename
         @user.save
       end
     end
+
+    def process_avatar(io)
+      processed = ImageProcessing::Vips
+        .source(io)
+        .resize_to_fill(400, 400)
+        .saver(strip: true)
+        .call
+      io.rewind
+      processed.read
+    rescue Vips::Error => e
+      Sentry.capture_exception(e) if Setting.sentry_enabled?
+      Rails.logger.error { "Image processing failed for avatar: #{e.message}" }
+      nil
+    end
 end

app/models/user.rb

@@ -56,6 +56,7 @@ class User < ApplicationRecord
   validates_length_of :display_name, minimum: 3, maximum: 35, allow_blank: true,
                                      if: -> { defined?(@display_name) }
 
+
   validate :acceptable_avatar
 
   validate :acceptable_pgp_key_format, if: -> { defined?(@pgp_pubkey) && @pgp_pubkey.present? }
@@ -83,6 +84,10 @@ class User < ApplicationRecord
          :timeoutable,
          :rememberable
 
+  #
+  # Methods
+  #
+
   def ldap_before_save
     self.email = Devise::LDAP::Adapter.get_ldap_param(self.cn, "mail").first
     self.ou    = dn.split(',')
@@ -165,23 +170,19 @@ class User < ApplicationRecord
     @display_name ||= ldap_entry[:display_name]
   end
 
-  def avatar_base64(size: :medium)
-    return nil unless avatar.attached?
-    variant = avatar_variant(size: size)
-    data = ActiveStorage::Blob.service.download(variant.key)
-    Base64.strict_encode64(data)
-  end
-
-  def avatar_variant(size: :medium)
-    dimensions = case size
-                 when :large  then [400, 400]
-                 when :medium then [256, 256]
-                 when :small  then [64, 64]
-                 else [256, 256]
-                 end
-    format = avatar.content_type == "image/png" ? :png : :jpeg
-    avatar.variant(resize_to_fill: dimensions, format: format)
-  end
+  # TODO Variant keys are currently broken for some reason
+  # (They use the same key as the main blob, when it should be
+  # "/variants/#{key)"
+  # def avatar_variant(size: :medium)
+  #   dimensions = case size
+  #                when :large  then [400, 400]
+  #                when :medium then [256, 256]
+  #                when :small  then [64, 64]
+  #                else [256, 256]
+  #                end
+  #   format = avatar.content_type == "image/png" ? :png : :jpeg
+  #   avatar.variant(resize_to_fill: dimensions, format: format)
+  # end
 
   def nostr_pubkey
     @nostr_pubkey ||= ldap_entry[:nostr_key]

app/services/ldap_manager/update_avatar.rb

@@ -14,15 +14,16 @@ module LdapManager
       end
 
       img_data = @user.avatar.blob.download
-      jpg_data = process(img_data)
+      jpg_data = process_avatar
 
+      Rails.logger.debug { "Storing new jpegPhoto for user #{@user.cn} in LDAP" }
       result = replace_attribute(@dn, :jpegPhoto, jpg_data)
       result == 0
     end
 
     private
 
-    def process(data)
+    def process_avatar
       @user.avatar.blob.open do |file|
         processed = ImageProcessing::Vips
           .source(file)

app/views/settings/_profile.html.erb

@@ -38,8 +38,7 @@
       <div class="flex items-center gap-6">
         <% if @user.avatar.attached? %>
           <p class="flex-none">
-            <%= image_tag @user.avatar_variant(size: :medium),
-              class: "h-24 w-24 rounded-lg" %>
+            <%= image_tag image_url_for(@user.avatar), class: "h-24 w-24 rounded-lg" %>
           </p>
         <% end %>
         <div class="grow">