diff --git a/app/components/app_catalog/web_app_icon_component.rb b/app/components/app_catalog/web_app_icon_component.rb index 8421d4f..0ce082e 100644 --- a/app/components/app_catalog/web_app_icon_component.rb +++ b/app/components/app_catalog/web_app_icon_component.rb @@ -9,13 +9,5 @@ module AppCatalog @image_url = image_url_for(web_app.apple_touch_icon) end end - - def image_url_for(attachment) - if Setting.s3_enabled? - s3_image_url(attachment) - else - Rails.application.routes.url_helpers.rails_blob_path(attachment, only_path: true) - end - end end end 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/avatars_controller.rb b/app/controllers/avatars_controller.rb new file mode 100644 index 0000000..8a7284e --- /dev/null +++ b/app/controllers/avatars_controller.rb @@ -0,0 +1,27 @@ +class AvatarsController < ApplicationController + def show + if user = User.find_by(cn: params[:username]) + http_status :not_found and return unless user.avatar.attached? + + sha256_hash = params[:hash] + format = params[:format]&.to_sym || :png + # size = params[:size]&.to_sym || :original + + unless user.avatar.filename.to_s == "#{sha256_hash}.#{format}" + http_status :not_found and return + 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 = user.avatar.blob.download + send_data data, type: "image/#{format}", disposition: "inline" + else + http_status :not_found + end + end +end diff --git a/app/controllers/discourse/sso_controller.rb b/app/controllers/discourse/sso_controller.rb index 658f434..ca47e68 100644 --- a/app/controllers/discourse/sso_controller.rb +++ b/app/controllers/discourse/sso_controller.rb @@ -8,6 +8,9 @@ class Discourse::SsoController < ApplicationController sso.email = current_user.email sso.username = current_user.cn sso.name = current_user.display_name + if current_user.avatar.attached? + sso.avatar_url = helpers.image_url_for(current_user.avatar) + end sso.admin = current_user.is_admin? sso.sso_secret = secret diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index e469d66..ef03455 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,12 @@ class SettingsController < ApplicationController end if @user.avatar_new.present? - LdapManager::UpdateAvatar.call(dn: @user.dn, file: @user.avatar_new) + if store_user_avatar + LdapManager::UpdateAvatar.call(user: @user) + else + @validation_errors = @user.errors + render :show, status: :unprocessable_entity and return + end end if @user.pgp_pubkey && (@user.pgp_pubkey != @user.ldap_entry[:pgp_key]) @@ -162,7 +167,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 @@ -184,4 +189,40 @@ class SettingsController < ApplicationController salt = BCrypt::Engine.generate_salt BCrypt::Engine.hash_secret(password, salt) end + + def store_user_avatar + 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}" + + if filename == @user.avatar.filename.to_s + @user.errors.add(:avatar, "must be a new file/picture") + false + else + key = "users/#{@user.cn}/avatars/#{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 diff --git a/app/controllers/webfinger_controller.rb b/app/controllers/webfinger_controller.rb index 7b28719..a8e00f7 100644 --- a/app/controllers/webfinger_controller.rb +++ b/app/controllers/webfinger_controller.rb @@ -33,6 +33,10 @@ class WebfingerController < WellKnownController links: [] } + if @user.avatar.attached? + jrd[:links] += avatar_link + end + if Setting.mastodon_enabled && @user.service_enabled?(:mastodon) # https://docs.joinmastodon.org/spec/webfinger/ jrd[:aliases] += mastodon_aliases @@ -47,6 +51,16 @@ class WebfingerController < WellKnownController jrd end + def avatar_link + [ + { + rel: "http://webfinger.net/rel/avatar", + type: @user.avatar.content_type, + href: helpers.image_url_for(@user.avatar) + } + ] + end + def mastodon_aliases [ "#{Setting.mastodon_public_url}/@#{@user.cn}", diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 50959f6..c166e06 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -14,4 +14,19 @@ module ApplicationHelper def badge(text, color) tag.span text, class: "inline-flex items-center rounded-full bg-#{color}-100 px-2.5 py-0.5 text-xs font-medium text-#{color}-800" end + + def image_url_for(attachment) + return s3_image_url(attachment) if Setting.s3_enabled? + + if attachment.record.is_a?(User) && attachment.name == "avatar" + hash, format = attachment.blob.filename.to_s.split(".", 2) + user_avatar_url( + username: attachment.record.cn, + hash: hash, + format: format + ) + else + Rails.application.routes.url_helpers.rails_blob_path(attachment, only_path: true) + end + end end 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/app/models/user.rb b/app/models/user.rb index 1886b1f..a0c49b0 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 # @@ -50,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? } @@ -77,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(',') @@ -159,6 +170,20 @@ class User < ApplicationRecord @display_name ||= ldap_entry[:display_name] 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] end @@ -186,10 +211,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 @@ -227,7 +248,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 11035ae..982bff2 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].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 f3c8975..793409d 100644 --- a/app/services/ldap_manager/update_avatar.rb +++ b/app/services/ldap_manager/update_avatar.rb @@ -2,26 +2,41 @@ 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 - replace_attribute @dn, :jpegPhoto, @img_data + 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_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(file) - processed = ImageProcessing::Vips - .resize_to_fill(512, 512) - .source(file) - .convert("jpeg") - .saver(strip: true) - .call - - Base64.strict_encode64 processed.read + def process_avatar + @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 4443e25..379e34a 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| %> @@ -31,23 +31,19 @@ <% end %> <% end %> - <% if Flipper.enabled?(:avatar_upload, current_user) %> - <% end %>

<%= f.submit 'Save', class: "btn-md btn-blue w-full md:w-auto" %> diff --git a/config/routes.rb b/config/routes.rb index 8344410..759abcc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -15,6 +15,9 @@ Rails.application.routes.draw do match 'signup/:step', to: 'signup#steps', as: :signup_steps, via: [:get, :post] post 'signup_validate', to: 'signup#validate' + + get "users/:username/avatars/:hash", to: "avatars#show", as: :user_avatar + namespace :contributions do root to: 'donations#index' resources :donations, only: ['index', 'create'] do diff --git a/spec/features/settings/profile_spec.rb b/spec/features/settings/profile_spec.rb index 55b861e..95a4798 100644 --- a/spec/features/settings/profile_spec.rb +++ b/spec/features/settings/profile_spec.rb @@ -2,18 +2,18 @@ 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 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 + }) end feature "Update display name" do @@ -57,21 +57,24 @@ 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 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 +89,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 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", diff --git a/spec/requests/webfinger_spec.rb b/spec/requests/webfinger_spec.rb index 285c146..a37d3cb 100644 --- a/spec/requests/webfinger_spec.rb +++ b/spec/requests/webfinger_spec.rb @@ -18,6 +18,46 @@ RSpec.describe "WebFinger", type: :request do }) end + describe "Avatar" do + context "not available" do + it "does not include an avatar link" do + get "/.well-known/webfinger?resource=acct%3Atony%40kosmos.org" + expect(response).to have_http_status(:ok) + res = JSON.parse(response.body) + + link = res["links"].find{|l| l["rel"] == "http://webfinger.net/rel/avatar"} + expect(link).to be_nil + end + end + + context "available" do + let(:fixture_path) { Rails.root.join("spec/fixtures/files/taipei.jpg") } + let(:img_data) { File.read(fixture_path) } + let(:img_hash) { Digest::SHA256.hexdigest(img_data) } + + before do + ActiveStorage::Blob.create_and_upload!( + io: File.open(fixture_path), + filename: "#{img_hash}.jpg", + content_type: "image/jpeg" + ).tap do |blob| + user.avatar.attach(blob) + end + end + + it "includes a public avatar link" do + get "/.well-known/webfinger?resource=acct%3Atony%40kosmos.org" + expect(response).to have_http_status(:ok) + res = JSON.parse(response.body) + + link = res["links"].find { |l| l["rel"] == "http://webfinger.net/rel/avatar" } + expect(link).to be_present + expect(link["type"]).to eq("image/jpeg") + expect(link["href"]).to match(%r{users/tony/avatars/#{img_hash}.jpg}) + end + end + end + describe "Mastodon entries" do context "Mastodon available" do it "includes the Mastodon aliases and links for the user" do