Merge pull request 'Add user avatars to LDAP, upload on profile settings page' (#148) from feature/123-user_avatars into master

Reviewed-on: #148
Reviewed-by: galfert <garret.alfert@gmail.com>

.drone.yml

@@ -17,7 +17,7 @@ steps:
       branch:
         - master
   - name: rspec
-    image: guildeducation/rails:2.7.2-14.20.0
+    image: gitea.kosmos.org/kosmos/akkounts-ci:0.1.0
     environment:
       RAILS_ENV: test
       REDIS_URL: redis://redis:6379/0

.gitignore

@@ -23,6 +23,7 @@
 !/tmp/pids/
 !/tmp/pids/.keep
 
+/storage
 
 /public/assets
 .byebug_history

Dockerfile

@@ -4,7 +4,7 @@ FROM ruby:2.7.6
 SHELL ["/bin/bash", "-o", "pipefail", "-c"]
 
 RUN apt-get update -qq && apt-get install -y --no-install-recommends curl \
-      ldap-utils tini
+      ldap-utils tini libvips
 RUN curl -fsSL https://deb.nodesource.com/setup_lts.x | bash -
 RUN apt-get update && apt-get install -y nodejs
 

Gemfile

@@ -37,6 +37,7 @@ gem 'devise_ldap_authenticatable'
 gem 'net-ldap'
 
 # Utilities
+gem "image_processing", "~> 1.12.2"
 gem "rqrcode", "~> 2.0"
 gem 'rails-settings-cached', '~> 2.8.3'
 gem 'pagy', '~> 6.0', '>= 6.0.2'

Gemfile.lock

@@ -182,6 +182,9 @@ GEM
     hashdiff (1.0.1)
     i18n (1.14.1)
       concurrent-ruby (~> 1.0)
+    image_processing (1.12.2)
+      mini_magick (>= 4.9.5, < 5)
+      ruby-vips (>= 2.0.17, < 3)
     importmap-rails (1.1.6)
       actionpack (>= 6.0.0)
       railties (>= 6.0.0)
@@ -220,6 +223,7 @@ GEM
     marcel (1.0.2)
     matrix (0.4.2)
     method_source (1.0.0)
+    mini_magick (4.12.0)
     mini_mime (1.1.2)
     minitest (5.18.0)
     multipart-post (2.3.0)
@@ -337,6 +341,8 @@ GEM
     rubocop-ast (1.29.0)
       parser (>= 3.2.1.0)
     ruby-progressbar (1.13.0)
+    ruby-vips (2.1.4)
+      ffi (~> 1.12)
     ruby2_keywords (0.0.5)
     rufus-scheduler (3.9.1)
       fugit (~> 1.1, >= 1.1.6)
@@ -435,6 +441,7 @@ DEPENDENCIES
   flipper
   flipper-active_record
   flipper-ui
+  image_processing (~> 1.12.2)
   importmap-rails
   jbuilder (~> 2.7)
   letter_opener

app/controllers/admin/users_controller.rb

@@ -20,6 +20,8 @@ class Admin::UsersController < Admin::BaseController
     end
 
     @services_enabled = @user.services_enabled
+
+    @avatar = LdapManager::FetchAvatar.call(cn: @user.cn, ou: @user.ou)
   end
 
   private

app/controllers/settings_controller.rb

@@ -19,10 +19,15 @@ class SettingsController < ApplicationController
   def update
     @user.preferences.merge!(user_params[:preferences] || {})
     @user.display_name = user_params[:display_name]
+    @user.avatar_new = user_params[:avatar]
 
     if @user.save
       if @user.display_name && (@user.display_name != @user.ldap_entry[:display_name])
-        LdapManager::UpdateDisplayName.call(@user.dn, user_params[:display_name])
+        LdapManager::UpdateDisplayName.call(@user.dn, @user.display_name)
+      end
+
+      if @user.avatar_new.present?
+        LdapManager::UpdateAvatar.call(@user.dn, @user.avatar_new)
       end
 
       redirect_to setting_path(@settings_section), flash: {
@@ -117,7 +122,7 @@ class SettingsController < ApplicationController
     end
 
     def user_params
-      params.require(:user).permit(:display_name, preferences: [
+      params.require(:user).permit(:display_name, :avatar, preferences: [
         :lightning_notify_sats_received,
         :xmpp_exchange_contacts_with_invitees
       ])

app/models/user.rb

@@ -2,10 +2,14 @@ class User < ApplicationRecord
   include EmailValidatable
 
   attr_accessor :display_name
+  attr_accessor :avatar_new
 
   serialize :preferences, UserPreferences
 
+  #
   # Relations
+  #
+
   has_many :invitations, dependent: :destroy
   has_one  :invitation, inverse_of: :invitee, foreign_key: 'invited_user_id'
   has_one  :inviter, through: :invitation, source: :user
@@ -20,6 +24,10 @@ class User < ApplicationRecord
 
   has_many :remote_storage_authorizations
 
+  #
+  # Validations
+  #
+
   validates_uniqueness_of :cn, scope: :ou
   validates_length_of :cn, minimum: 3
   validates_format_of :cn, with: /\A([a-z0-9\-])*\z/,
@@ -40,10 +48,20 @@ class User < ApplicationRecord
 
   validates_uniqueness_of :nostr_pubkey, allow_blank: true
 
+  validate :acceptable_avatar
+
+  #
+  # Scopes
+  #
+
   scope :confirmed,  -> { where.not(confirmed_at: nil) }
   scope :pending,    -> { where(confirmed_at: nil) }
   scope :all_except, -> (user) { where.not(id: user) }
 
+  #
+  # Encrypted database columns
+  #
+
   has_encrypted :ln_login, :ln_password
 
   # Include default devise modules. Others available are:
@@ -140,6 +158,10 @@ class User < ApplicationRecord
     @display_name ||= ldap_entry[:display_name]
   end
 
+  def avatar
+    @avatar_base64 ||= LdapManager::FetchAvatar.call(cn: cn, ou: ou)
+  end
+
   def services_enabled
     ldap_entry[:service] || []
   end
@@ -168,4 +190,17 @@ class User < ApplicationRecord
       return @ldap_service if defined?(@ldap_service)
       @ldap_service = LdapService.new
     end
+
+    def acceptable_avatar
+      return unless avatar_new.present?
+
+      if avatar_new.size > 1.megabyte
+        errors.add(:avatar, "file size is too large")
+      end
+
+      acceptable_types = ["image/jpeg", "image/png"]
+      unless acceptable_types.include?(avatar_new.content_type)
+        errors.add(:avatar, "must be a JPEG or PNG file")
+      end
+    end
 end

app/services/ldap_manager/fetch_avatar.rb

@@ -0,0 +1,17 @@
+module LdapManager
+  class FetchAvatar < LdapManagerService
+    def initialize(cn:, ou: nil)
+      @cn = cn
+      @ou = ou
+    end
+
+    def call
+      treebase   = @ou ? "ou=#{@ou},cn=users,#{suffix}" : ldap_config["base"]
+      attributes = %w{ jpegPhoto }
+      filter     = Net::LDAP::Filter.eq("cn", @cn)
+
+      entry = ldap_client.search(base: treebase, filter: filter, attributes: attributes).first
+      entry.try(:jpegPhoto) ? entry.jpegPhoto.first : nil
+    end
+  end
+end

app/services/ldap_manager/update_avatar.rb

@@ -0,0 +1,27 @@
+require "image_processing/vips"
+
+module LdapManager
+  class UpdateAvatar < LdapManagerService
+    def initialize(dn, file)
+      @dn = dn
+      @img_data = process(file)
+    end
+
+    def call
+      replace_attribute @dn, :jpegPhoto, @img_data
+    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
+    end
+  end
+end

app/services/ldap_manager_service.rb

@@ -1,2 +1,5 @@
 class LdapManagerService < LdapService
+  def suffix
+    @suffix ||= ENV["LDAP_SUFFIX"] || "dc=kosmos,dc=org"
+  end
 end

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

@@ -63,6 +63,10 @@
     </section>
 
     <section class="sm:flex-1 sm:pt-0">
+      <h3>LDAP<h3>
+      <p>
+        <img src="data:image/jpeg;base64,<%= @avatar %>" class="h-48 w-48" />
+      </p>
       <!-- <h3>Actions</h3> -->
     </section>
   </div>

app/views/settings/_profile.html.erb

@@ -1,33 +1,62 @@
 <section>
   <h3>Profile</h3>
-  <p class="mb-2">
-    <%= label :user_address, 'User address', class: 'font-bold' %>
-  </p>
-  <p data-controller="clipboard" class="flex gap-1 mb-2 sm:w-3/5">
-    <input type="text" id="user_address" class="grow"
-           value=<%= @user.address %> disabled="disabled"
-           data-clipboard-target="source" />
-    <button id="copy-user-address" class="btn-md btn-icon btn-outline shrink-0"
-            data-clipboard-target="trigger" data-action="clipboard#copy"
-            title="Copy to clipboard">
-      <span class="content-initial">
-        <%= render partial: "icons/copy", locals: { custom_class: "text-blue-600  h-4 w-4 inline" } %>
-      </span>
-      <span class="content-active hidden">
-        <%= render partial: "icons/check", locals: { custom_class: "text-blue-600  h-4 w-4 inline" } %>
-      </span>
-    </button>
-  </p>
-  <p class="text-sm text-gray-500">
-    Your user address for Chat and Lightning Network.
-  </p>
+  <div class="mb-6">
+    <p class="mb-2">
+      <%= label :user_address, 'User address', class: 'font-bold' %>
+    </p>
+    <p data-controller="clipboard" class="flex gap-1 mb-2 sm:w-3/5">
+      <input type="text" id="user_address" class="grow"
+             value=<%= @user.address %> disabled="disabled"
+             data-clipboard-target="source" />
+      <button id="copy-user-address" class="btn-md btn-icon btn-outline shrink-0"
+              data-clipboard-target="trigger" data-action="clipboard#copy"
+              title="Copy to clipboard">
+        <span class="content-initial">
+          <%= render partial: "icons/copy", locals: { custom_class: "text-blue-600  h-4 w-4 inline" } %>
+        </span>
+        <span class="content-active hidden">
+          <%= render partial: "icons/check", locals: { custom_class: "text-blue-600  h-4 w-4 inline" } %>
+        </span>
+      </button>
+    </p>
+    <p class="text-sm text-gray-500">
+      Your user address for Chat and Lightning Network.
+    </p>
+  </div>
   <%= form_for(@user, url: setting_path(:profile), html: { :method => :put }) do |f| %>
     <%= render FormElements::FieldsetComponent.new(tag: "div", title: "Display name") do %>
-      <%= f.text_field :display_name, class: "w-full sm:w-3/5 mb-2" %>
+      <%= f.text_field :display_name, class: "w-full sm:w-3/5" %>
       <% if @validation_errors.present? && @validation_errors[:display_name].present? %>
-        <p class="error-msg"><%= @validation_errors[:display_name].first %></p>
+        <p class="error-msg mt-2"><%= @validation_errors[:display_name].first %></p>
       <% end %>
     <% end %>
+
+    <label class="block">
+      <p class="font-bold mb-1">
+        Avatar
+      </p>
+      <p class="text-gray-500">
+        Default profile picture
+      </p>
+      <div class="flex items-center gap-6">
+        <% if current_user.avatar.present? %>
+        <p class="flex-none">
+          <%= image_tag "data:image/jpeg;base64,#{current_user.avatar}", class: "h-24 w-24 rounded-lg" %>
+        </p>
+        <% end %>
+        <div class="grow">
+          <p class="mb-2">
+          <%= f.file_field :avatar, class: "" %>
+          <p class="text-sm text-gray-500">
+            JPEG or PNG image, not larger than 1 megabyte
+          </p>
+          <% if @validation_errors.present? && @validation_errors[:avatar].present? %>
+            <p class="error-msg mb-2"><%= @validation_errors[:avatar].first %></p>
+          <% end %>
+        </div>
+      </div>
+    </label>
+
     <p class="mt-8 pt-6 border-t border-gray-200 text-right">
       <%= f.submit 'Save', class: "btn-md btn-blue w-full md:w-auto" %>
     </p>

ci/Dockerfile

@@ -0,0 +1,4 @@
+# syntax=docker/dockerfile:1
+FROM guildeducation/rails:2.7.2-14.20.0
+
+RUN apt-get update -qq && apt-get install -y --no-install-recommends ldap-utils libvips

config/application.rb

@@ -5,7 +5,7 @@ require "rails"
 require "active_model/railtie"
 require "active_job/railtie"
 require "active_record/railtie"
-# require "active_storage/engine"
+require "active_storage/engine"
 require "action_controller/railtie"
 require "action_mailer/railtie"
 require "action_mailbox/engine"

config/environments/development.rb

@@ -70,4 +70,7 @@ Rails.application.configure do
 
   # Allow requests from any IP
   config.web_console.whiny_requests = false
+
+  # Store attachments on the local disk (in ./storage)
+  config.active_storage.service = :local
 end

config/environments/production.rb

@@ -110,6 +110,10 @@ Rails.application.configure do
   # Set this to true and configure the email server for immediate delivery to raise delivery errors.
   config.action_mailer.raise_delivery_errors = true
 
+  # TODO make configurable
+  # Store attachments in S3-compatible back-end
+  config.active_storage.service = :local
+
   # Enable locale fallbacks for I18n (makes lookups for any locale fall back to
   # the I18n.default_locale when a translation cannot be found).
   config.i18n.fallbacks = true

config/environments/test.rb

@@ -51,4 +51,7 @@ Rails.application.configure do
   }
 
   config.active_job.queue_adapter = :test
+
+  # Store attachments on the local disk (in ./tmp)
+  config.active_storage.service = :test
 end

config/storage.yml

@@ -1,7 +1,7 @@
-test:
-  service: Disk
-  root: <%= Rails.root.join("tmp/storage") %>
-
 local:
   service: Disk
   root: <%= Rails.root.join("storage") %>
+
+test:
+  service: Disk
+  root: <%= Rails.root.join("tmp/storage") %>

db/migrate/20230906073324_create_active_storage_tables.active_storage.rb

@@ -0,0 +1,57 @@
+# This migration comes from active_storage (originally 20170806125915)
+class CreateActiveStorageTables < ActiveRecord::Migration[5.2]
+  def change
+    # Use Active Record's configured type for primary and foreign keys
+    primary_key_type, foreign_key_type = primary_and_foreign_key_types
+
+    create_table :active_storage_blobs, id: primary_key_type do |t|
+      t.string   :key,          null: false
+      t.string   :filename,     null: false
+      t.string   :content_type
+      t.text     :metadata
+      t.string   :service_name, null: false
+      t.bigint   :byte_size,    null: false
+      t.string   :checksum
+
+      if connection.supports_datetime_with_precision?
+        t.datetime :created_at, precision: 6, null: false
+      else
+        t.datetime :created_at, null: false
+      end
+
+      t.index [ :key ], unique: true
+    end
+
+    create_table :active_storage_attachments, id: primary_key_type do |t|
+      t.string     :name,     null: false
+      t.references :record,   null: false, polymorphic: true, index: false, type: foreign_key_type
+      t.references :blob,     null: false, type: foreign_key_type
+
+      if connection.supports_datetime_with_precision?
+        t.datetime :created_at, precision: 6, null: false
+      else
+        t.datetime :created_at, null: false
+      end
+
+      t.index [ :record_type, :record_id, :name, :blob_id ], name: :index_active_storage_attachments_uniqueness, unique: true
+      t.foreign_key :active_storage_blobs, column: :blob_id
+    end
+
+    create_table :active_storage_variant_records, id: primary_key_type do |t|
+      t.belongs_to :blob, null: false, index: false, type: foreign_key_type
+      t.string :variation_digest, null: false
+
+      t.index [ :blob_id, :variation_digest ], name: :index_active_storage_variant_records_uniqueness, unique: true
+      t.foreign_key :active_storage_blobs, column: :blob_id
+    end
+  end
+
+  private
+    def primary_and_foreign_key_types
+      config = Rails.configuration.generators
+      setting = config.options[config.orm][:primary_key_type]
+      primary_key_type = setting || :primary_key
+      foreign_key_type = setting || :bigint
+      [primary_key_type, foreign_key_type]
+    end
+end

db/schema.rb

@@ -10,7 +10,35 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_05_23_120753) do
+ActiveRecord::Schema[7.0].define(version: 2023_09_06_073324) do
+  create_table "active_storage_attachments", force: :cascade do |t|
+    t.string "name", null: false
+    t.string "record_type", null: false
+    t.bigint "record_id", null: false
+    t.bigint "blob_id", null: false
+    t.datetime "created_at", null: false
+    t.index ["blob_id"], name: "index_active_storage_attachments_on_blob_id"
+    t.index ["record_type", "record_id", "name", "blob_id"], name: "index_active_storage_attachments_uniqueness", unique: true
+  end
+
+  create_table "active_storage_blobs", force: :cascade do |t|
+    t.string "key", null: false
+    t.string "filename", null: false
+    t.string "content_type"
+    t.text "metadata"
+    t.string "service_name", null: false
+    t.bigint "byte_size", null: false
+    t.string "checksum"
+    t.datetime "created_at", null: false
+    t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true
+  end
+
+  create_table "active_storage_variant_records", force: :cascade do |t|
+    t.bigint "blob_id", null: false
+    t.string "variation_digest", null: false
+    t.index ["blob_id", "variation_digest"], name: "index_active_storage_variant_records_uniqueness", unique: true
+  end
+
   create_table "donations", force: :cascade do |t|
     t.integer "user_id"
     t.integer "amount_sats"
@@ -94,5 +122,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_05_23_120753) do
     t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
   end
 
+  add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id"
+  add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id"
   add_foreign_key "remote_storage_authorizations", "users"
 end

spec/features/settings/profile_spec.rb

@@ -2,20 +2,19 @@ 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") }
 
   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"
+    })
+    allow_any_instance_of(User).to receive(:avatar).and_return(avatar_base64)
   end
 
   feature "Update display name" do
-    before do
-      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"
-      })
-    end
-
     scenario 'fails with validation error' do
       visit setting_path(:profile)
       fill_in 'Display name', with: "M"
@@ -42,4 +41,59 @@ RSpec.describe 'Profile settings', type: :feature do
       end
     end
   end
+
+  feature "Update avatar" do
+    scenario "fails with validation error for wrong content type" do
+      visit setting_path(:profile)
+      attach_file "Avatar", "#{Rails.root}/spec/fixtures/files/bitcoin.pdf"
+      click_button "Save"
+
+      expect(current_url).to eq(setting_url(:profile))
+      within ".error-msg" do
+        expect(page).to have_content("must be a JPEG or PNG file")
+      end
+    end
+
+    scenario "fails with validation error for file size too large" do
+      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")
+      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)
+
+      visit setting_path(:profile)
+      attach_file "Avatar", file_path
+      click_button "Save"
+
+      expect(current_url).to eq(setting_url(:profile))
+      within ".flash-msg" do
+        expect(page).to have_content("Settings saved")
+      end
+    end
+
+    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)
+
+      visit setting_path(:profile)
+      attach_file "Avatar", file_path
+      click_button "Save"
+
+      expect(current_url).to eq(setting_url(:profile))
+      within ".flash-msg" do
+        expect(page).to have_content("Settings saved")
+      end
+    end
+  end
 end