WIP Store avatars as ActiveStorage attachments

Also push to LDAP as jpegPhoto

app/controllers/admin/users_controller.rb

@@ -22,7 +22,7 @@ class Admin::UsersController < Admin::BaseController
 
     @services_enabled = @user.services_enabled
 
-    @avatar = LdapManager::FetchAvatar.call(cn: @user.cn)
+    @ldap_avatar = LdapManager::FetchAvatar.call(cn: @user.cn)
   end
 
   # POST /admin/users/:username/invitations

app/controllers/settings_controller.rb

@@ -25,7 +25,7 @@ class SettingsController < ApplicationController
   def update
     @user.preferences.merge!(user_params[:preferences] || {})
     @user.display_name = user_params[:display_name]
-    @user.avatar_new   = user_params[:avatar]
+    @user.avatar_new   = user_params[:avatar_new]
     @user.pgp_pubkey   = user_params[:pgp_pubkey]
 
     if @user.save
@@ -34,7 +34,10 @@ class SettingsController < ApplicationController
       end
 
       if @user.avatar_new.present?
-        LdapManager::UpdateAvatar.call(dn: @user.dn, file: @user.avatar_new)
+        @user.avatar.attach(@user.avatar_new)
+        @user.avatar.blob.update(filename: @user.avatar_filename)
+        @user.save!
+        LdapManager::UpdateAvatar.call(user: @user)
       end
 
       if @user.pgp_pubkey && (@user.pgp_pubkey != @user.ldap_entry[:pgp_key])
@@ -162,7 +165,7 @@ class SettingsController < ApplicationController
 
     def user_params
       params.require(:user).permit(
-        :display_name, :avatar, :pgp_pubkey,
+        :display_name, :avatar_new, :pgp_pubkey,
         preferences: UserPreferences.pref_keys
       )
     end

app/models/user.rb

@@ -4,8 +4,8 @@ class User < ApplicationRecord
   include EmailValidatable
 
   attr_accessor :current_password
-  attr_accessor :avatar_new
   attr_accessor :display_name
+  attr_accessor :avatar_new
   attr_accessor :pgp_pubkey
 
   serialize :preferences, coder: UserPreferences
@@ -27,6 +27,12 @@ class User < ApplicationRecord
 
   has_many :accounts, through: :lndhub_user
 
+  #
+  # Attachments
+  #
+
+  has_one_attached :avatar
+
   #
   # Validations
   #
@@ -159,13 +165,30 @@ class User < ApplicationRecord
     @display_name ||= ldap_entry[:display_name]
   end
 
-  def avatar
-    @avatar ||= LdapManager::FetchAvatar.call(cn: cn)
+  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_base64
-    return nil if avatar.nil?
-    @avatar_base64 ||= Base64.strict_encode64(avatar)
+  def avatar_filename
+    return nil unless avatar.attached?
+    data = ActiveStorage::Blob.service.download(avatar.key)
+    hash = Digest::SHA256.hexdigest(data)
+    ext = avatar.content_type == "image/png" ? "png" : "jpg"
+    "#{hash}.#{ext}"
+  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
 
   def nostr_pubkey
@@ -232,7 +255,7 @@ class User < ApplicationRecord
       return unless avatar_new.present?
 
       if avatar_new.size > 1.megabyte
-        errors.add(:avatar, "file size is too large")
+        errors.add(:avatar, "must be less than 1MB file size")
       end
 
       acceptable_types = ["image/jpeg", "image/png"]

app/services/ldap_manager/fetch_avatar.rb

@@ -10,7 +10,7 @@ module LdapManager
       filter = Net::LDAP::Filter.eq("cn", @cn)
 
       entry = client.search(base: treebase, filter: filter, attributes: attributes).first
-      entry&.jpegPhoto ? entry.jpegPhoto.first : nil
+      entry[:jpegPhoto].present? ? entry.jpegPhoto.first : nil
     end
   end
 end

app/services/ldap_manager/update_avatar.rb

@@ -2,26 +2,40 @@ require "image_processing/vips"
 
 module LdapManager
   class UpdateAvatar < LdapManagerService
-    def initialize(dn:, file:)
-      @dn = dn
-      @img_data = process(file)
+    def initialize(user:)
+      @user = user
+      @dn = user.dn
     end
 
     def call
-      result = replace_attribute @dn, :jpegPhoto, @img_data
-      result
+      unless @user.avatar.attached?
+        Rails.logger.error { "Cannot store empty jpegPhoto for user #{@user.cn}" }
+        return false
+      end
+
+      img_data = @user.avatar.blob.download
+      jpg_data = process(img_data)
+
+      result = replace_attribute(@dn, :jpegPhoto, jpg_data)
+      result == 0
     end
 
     private
 
-    def process(file)
+    def process(data)
+      @user.avatar.blob.open do |file|
         processed = ImageProcessing::Vips
-        .resize_to_fill(256, 256)
           .source(file)
+          .resize_to_fill(256, 256)
           .convert("jpeg")
           .saver(strip: true)
           .call
         processed.read
       end
+    rescue Vips::Error => e
+      Sentry.capture_exception(e) if Setting.sentry_enabled?
+      Rails.logger.error { "Image processing failed for LDAP avatar: #{e.message}" }
+      nil
+    end
   end
 end

app/views/admin/users/show.html.erb

@@ -95,8 +95,8 @@
           <tr>
             <th>Avatar</th>
             <td>
-              <% if @avatar.present? %>
-                <img src="data:image/jpeg;base64,<%= @avatar %>" class="h-48 w-48" />
+              <% if @ldap_avatar.present? %>
+                JPEG size: <%= @ldap_avatar.size %>
               <% else %>
                 &mdash;
               <% end %>

app/views/settings/_profile.html.erb

@@ -33,22 +33,19 @@
 
     <% if Flipper.enabled?(:avatar_upload, current_user) %>
     <label class="block">
-      <p class="font-bold mb-1">
-        Avatar
-      </p>
-      <p class="text-gray-500">
-        Default profile picture
-      </p>
+      <p class="font-bold mb-1">Avatar</p>
+      <p class="text-gray-500">Default profile picture</p>
       <div class="flex items-center gap-6">
-        <% unless current_user.avatar.nil? %>
+        <% if @user.avatar.attached? %>
           <p class="flex-none">
-          <%= image_tag "data:image/jpeg;base64,#{current_user.avatar_base64}",
+            <%= image_tag @user.avatar_variant(size: :medium),
               class: "h-24 w-24 rounded-lg" %>
           </p>
         <% end %>
         <div class="grow">
           <p class="mb-2">
-          <%= f.file_field :avatar, class: "" %>
+            <%= f.file_field :avatar_new, accept: "image/jpeg,image/png" %>
+          </p>
           <p class="text-sm text-gray-500">
             JPEG or PNG image, not larger than 1 megabyte
           </p>

spec/features/settings/profile_spec.rb

@@ -16,8 +16,6 @@ RSpec.describe 'Profile settings', type: :feature do
       .and_return({
         uid: user.cn, ou: user.ou, display_name: "Mark", pgp_key: nil
       })
-    allow_any_instance_of(User).to receive(:avatar)
-      .and_return(avatar_jpeg)
   end
 
   feature "Update display name" do
@@ -61,13 +59,16 @@ RSpec.describe 'Profile settings', type: :feature do
     end
 
     scenario "fails with validation error for file size too large" do
+      expect_any_instance_of(LdapManager::UpdateAvatar)
+        .not_to receive(:replace_attribute).and_return(true)
+
       visit setting_path(:profile)
       attach_file "Avatar", "#{Rails.root}/spec/fixtures/files/fsociety-irc.png"
       click_button "Save"
 
       expect(current_url).to eq(setting_url(:profile))
       within ".error-msg" do
-        expect(page).to have_content("file size is too large")
+        expect(page).to have_content("must be less than 1MB")
       end
     end