From 6d7d722c5d34f69a40c0c64662ab72751c767968 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 11 Apr 2025 16:12:32 +0400 Subject: [PATCH 01/10] Add inetOrgPerson objectclass to user entries refs #174 --- app/jobs/create_ldap_user_job.rb | 2 +- spec/jobs/create_ldap_user_job_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/jobs/create_ldap_user_job.rb b/app/jobs/create_ldap_user_job.rb index 4eff181..8fbab0c 100644 --- a/app/jobs/create_ldap_user_job.rb +++ b/app/jobs/create_ldap_user_job.rb @@ -4,7 +4,7 @@ class CreateLdapUserJob < ApplicationJob def perform(username:, domain:, email:, hashed_pw:, confirmed: false) dn = "cn=#{username},ou=#{domain},cn=users,dc=kosmos,dc=org" attr = { - objectclass: ["top", "account", "person", "extensibleObject"], + objectclass: ["top", "account", "person", "inetOrgPerson", "extensibleObject"], cn: username, sn: username, uid: username, diff --git a/spec/jobs/create_ldap_user_job_spec.rb b/spec/jobs/create_ldap_user_job_spec.rb index 87a9a44..b86214f 100644 --- a/spec/jobs/create_ldap_user_job_spec.rb +++ b/spec/jobs/create_ldap_user_job_spec.rb @@ -32,7 +32,7 @@ RSpec.describe CreateLdapUserJob, type: :job do expect(ldap_client_mock).to have_received(:add).with( dn: "cn=halfinney,ou=kosmos.org,cn=users,dc=kosmos,dc=org", attributes: { - objectclass: ["top", "account", "person", "extensibleObject"], + objectclass: ["top", "account", "person", "inetOrgPerson", "extensibleObject"], cn: "halfinney", sn: "halfinney", uid: "halfinney", @@ -51,7 +51,7 @@ RSpec.describe CreateLdapUserJob, type: :job do expect(ldap_client_mock).to have_received(:add).with( dn: "cn=halfinney,ou=kosmos.org,cn=users,dc=kosmos,dc=org", attributes: { - objectclass: ["top", "account", "person", "extensibleObject"], + objectclass: ["top", "account", "person", "inetOrgPerson", "extensibleObject"], cn: "halfinney", sn: "halfinney", uid: "halfinney", From 9e2210c45b3b6077d29daee6fe3e2cfec3621667 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Wed, 24 Jan 2024 15:56:34 +0300 Subject: [PATCH 02/10] Store avatars as binary instead of base64 --- app/models/user.rb | 13 +++++++---- app/services/ldap_manager/fetch_avatar.rb | 6 +++--- app/services/ldap_manager/update_avatar.rb | 8 +++---- app/views/settings/_profile.html.erb | 7 +++--- spec/features/settings/profile_spec.rb | 25 +++++++++++++--------- 5 files changed, 35 insertions(+), 24 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 1886b1f..d73d780 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -159,6 +159,15 @@ class User < ApplicationRecord @display_name ||= ldap_entry[:display_name] end + def avatar + @avatar ||= LdapManager::FetchAvatar.call(cn: cn) + end + + def avatar_base64 + return nil if avatar.nil? + @avatar_base64 ||= Base64.strict_encode64(avatar) + end + def nostr_pubkey @nostr_pubkey ||= ldap_entry[:nostr_key] end @@ -186,10 +195,6 @@ class User < ApplicationRecord ZBase32.encode(Digest::SHA1.digest(cn)) end - def avatar - @avatar_base64 ||= LdapManager::FetchAvatar.call(cn: cn) - end - def services_enabled ldap_entry[:services_enabled] || [] end diff --git a/app/services/ldap_manager/fetch_avatar.rb b/app/services/ldap_manager/fetch_avatar.rb index 11035ae..a838bb8 100644 --- a/app/services/ldap_manager/fetch_avatar.rb +++ b/app/services/ldap_manager/fetch_avatar.rb @@ -5,12 +5,12 @@ module LdapManager end def call - treebase = ldap_config["base"] + treebase = ldap_config["base"] attributes = %w{ jpegPhoto } - filter = Net::LDAP::Filter.eq("cn", @cn) + filter = Net::LDAP::Filter.eq("cn", @cn) entry = client.search(base: treebase, filter: filter, attributes: attributes).first - entry.try(:jpegPhoto) ? entry.jpegPhoto.first : nil + entry&.jpegPhoto ? entry.jpegPhoto.first : nil end end end diff --git a/app/services/ldap_manager/update_avatar.rb b/app/services/ldap_manager/update_avatar.rb index f3c8975..d238303 100644 --- a/app/services/ldap_manager/update_avatar.rb +++ b/app/services/ldap_manager/update_avatar.rb @@ -8,20 +8,20 @@ module LdapManager end def call - replace_attribute @dn, :jpegPhoto, @img_data + result = replace_attribute @dn, :jpegPhoto, @img_data + result end private def process(file) processed = ImageProcessing::Vips - .resize_to_fill(512, 512) + .resize_to_fill(256, 256) .source(file) .convert("jpeg") .saver(strip: true) .call - - Base64.strict_encode64 processed.read + processed.read end end end diff --git a/app/views/settings/_profile.html.erb b/app/views/settings/_profile.html.erb index 4443e25..45ffc37 100644 --- a/app/views/settings/_profile.html.erb +++ b/app/views/settings/_profile.html.erb @@ -20,7 +20,7 @@

- Your user address for Chat and Lightning Network. + Your account's address on the Internet

<%= form_for(@user, url: setting_path(:profile), html: { :method => :put }) do |f| %> @@ -40,9 +40,10 @@ Default profile picture

- <% if current_user.avatar.present? %> + <% unless current_user.avatar.nil? %>

- <%= image_tag "data:image/jpeg;base64,#{current_user.avatar}", class: "h-24 w-24 rounded-lg" %> + <%= image_tag "data:image/jpeg;base64,#{current_user.avatar_base64}", + class: "h-24 w-24 rounded-lg" %>

<% end %>
diff --git a/spec/features/settings/profile_spec.rb b/spec/features/settings/profile_spec.rb index 55b861e..8797890 100644 --- a/spec/features/settings/profile_spec.rb +++ b/spec/features/settings/profile_spec.rb @@ -2,18 +2,22 @@ require 'rails_helper' RSpec.describe 'Profile settings', type: :feature do let(:user) { create :user, cn: "mwahlberg" } - let(:avatar_base64) { File.read("#{Rails.root}/spec/fixtures/files/avatar-base64.txt") } + let(:avatar_jpeg) { File.binread("#{Rails.root}/spec/fixtures/files/taipei.jpg") } before do + Flipper.enable "avatar_upload" + login_as user, :scope => :user allow(user).to receive(:display_name).and_return("Mark") - allow_any_instance_of(User).to receive(:dn).and_return("cn=mwahlberg,ou=kosmos.org,cn=users,dc=kosmos,dc=org") - allow_any_instance_of(User).to receive(:ldap_entry).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_base64) - Flipper.enable "avatar_upload" + allow_any_instance_of(User).to receive(:dn) + .and_return("cn=mwahlberg,ou=kosmos.org,cn=users,dc=kosmos,dc=org") + allow_any_instance_of(User).to receive(:ldap_entry) + .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 @@ -70,8 +74,8 @@ RSpec.describe 'Profile settings', type: :feature do scenario 'works with valid JPG file' do file_path = "#{Rails.root}/spec/fixtures/files/taipei.jpg" - expect_any_instance_of(LdapManager::UpdateAvatar).to receive(:replace_attribute) - .with(user.dn, :jpegPhoto, avatar_base64).and_return(true) + expect_any_instance_of(LdapManager::UpdateAvatar) + .to receive(:replace_attribute).and_return(true) visit setting_path(:profile) attach_file "Avatar", file_path @@ -86,7 +90,8 @@ RSpec.describe 'Profile settings', type: :feature do scenario 'works with valid PNG file' do file_path = "#{Rails.root}/spec/fixtures/files/bender.png" - expect(LdapManager::UpdateAvatar).to receive(:call).and_return(true) + expect_any_instance_of(LdapManager::UpdateAvatar) + .to receive(:replace_attribute).and_return(true) visit setting_path(:profile) attach_file "Avatar", file_path From 17ffbde03a83d43bf7356ed565fa686d0314a656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Sun, 11 May 2025 18:43:21 +0400 Subject: [PATCH 03/10] WIP Store avatars as ActiveStorage attachments Also push to LDAP as jpegPhoto --- app/controllers/admin/users_controller.rb | 2 +- app/controllers/settings_controller.rb | 9 +++-- app/models/user.rb | 37 ++++++++++++++++---- app/services/ldap_manager/fetch_avatar.rb | 2 +- app/services/ldap_manager/update_avatar.rb | 40 +++++++++++++++------- app/views/admin/users/show.html.erb | 4 +-- app/views/settings/_profile.html.erb | 21 +++++------- spec/features/settings/profile_spec.rb | 7 ++-- 8 files changed, 80 insertions(+), 42 deletions(-) diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 82b91da..e73d208 100644 --- a/app/controllers/admin/users_controller.rb +++ b/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 diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index e469d66..fc3477b 100644 --- a/app/controllers/settings_controller.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index d73d780..2905748 100644 --- a/app/models/user.rb +++ b/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"] diff --git a/app/services/ldap_manager/fetch_avatar.rb b/app/services/ldap_manager/fetch_avatar.rb index a838bb8..982bff2 100644 --- a/app/services/ldap_manager/fetch_avatar.rb +++ b/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 diff --git a/app/services/ldap_manager/update_avatar.rb b/app/services/ldap_manager/update_avatar.rb index d238303..163af3f 100644 --- a/app/services/ldap_manager/update_avatar.rb +++ b/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) - processed = ImageProcessing::Vips - .resize_to_fill(256, 256) - .source(file) - .convert("jpeg") - .saver(strip: true) - .call - processed.read + def process(data) + @user.avatar.blob.open do |file| + processed = ImageProcessing::Vips + .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 diff --git a/app/views/admin/users/show.html.erb b/app/views/admin/users/show.html.erb index 832e930..1007d4a 100644 --- a/app/views/admin/users/show.html.erb +++ b/app/views/admin/users/show.html.erb @@ -95,8 +95,8 @@ Avatar - <% if @avatar.present? %> - + <% if @ldap_avatar.present? %> + JPEG size: <%= @ldap_avatar.size %> <% else %> — <% end %> diff --git a/app/views/settings/_profile.html.erb b/app/views/settings/_profile.html.erb index 45ffc37..7dc0f22 100644 --- a/app/views/settings/_profile.html.erb +++ b/app/views/settings/_profile.html.erb @@ -33,22 +33,19 @@ <% if Flipper.enabled?(:avatar_upload, current_user) %>