From a7cbd8ce36937db357145df04cd1e8d5c99ae408 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 8 Feb 2024 12:48:30 +0100 Subject: [PATCH 1/8] Allow disabling S3 explicitly, disable in Docker Compose For example when there is a .env.development for running the app on a host machine directly, but as a developer you also want to run it with Docker Compose from time to time. --- app/models/setting.rb | 3 +++ config/environments/development.rb | 2 +- config/environments/production.rb | 2 +- config/storage.yml | 2 +- docker-compose.yml | 2 ++ 5 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/models/setting.rb b/app/models/setting.rb index 27acdcc..daa4b29 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -15,6 +15,9 @@ class Setting < RailsSettings::Base field :redis_url, type: :string, default: ENV["REDIS_URL"] || "redis://localhost:6379/0" + field :s3_enabled, type: :boolean, + default: ENV["S3_ENABLED"] && ENV["S3_ENABLED"].to_s != "false" + # # Registrations # diff --git a/config/environments/development.rb b/config/environments/development.rb index e13b809..9f73578 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -71,7 +71,7 @@ Rails.application.configure do # Allow requests from any IP config.web_console.permissions = '0.0.0.0/0' - if ENV["S3_ENABLED"] + if ENV["S3_ENABLED"] && ENV["S3_ENABLED"].to_s != "false" config.active_storage.service = :s3 else config.active_storage.service = :local diff --git a/config/environments/production.rb b/config/environments/production.rb index 5adf41e..16e7fa5 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -110,7 +110,7 @@ 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 - if ENV["S3_ENABLED"] + if ENV["S3_ENABLED"] && ENV["S3_ENABLED"].to_s != "false" config.active_storage.service = :s3 else config.active_storage.service = :local diff --git a/config/storage.yml b/config/storage.yml index 3ddfbb8..9a15623 100644 --- a/config/storage.yml +++ b/config/storage.yml @@ -6,7 +6,7 @@ test: service: Disk root: <%= Rails.root.join("tmp/storage") %> -<% if ENV["S3_ENABLED"] %> +<% if ENV["S3_ENABLED"] && ENV["S3_ENABLED"].to_s != "false" %> s3: service: S3 endpoint: <%= ENV["S3_ENDPOINT"] %> diff --git a/docker-compose.yml b/docker-compose.yml index 5106dfd..6e0ff50 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -44,6 +44,7 @@ services: REDIS_URL: redis://redis:6379/0 RS_REDIS_URL: redis://redis:6379/1 RS_STORAGE_URL: "http://localhost:4567" + S3_ENABLED: false depends_on: - ldap - redis @@ -67,6 +68,7 @@ services: REDIS_URL: redis://redis:6379/0 RS_REDIS_URL: redis://redis:6379/1 RS_STORAGE_URL: "http://localhost:4567" + S3_ENABLED: false depends_on: - ldap - redis From 70ac3b0a7087a1ce9c13b6f1857ac68262237738 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 8 Feb 2024 12:51:53 +0100 Subject: [PATCH 2/8] Fix RS dashboard for auths without Web App RS auths without a valid domain name will not fetch any metadata and therefore not create a WebApp record. This fixes icons being looked up anyway, resulting in exceptions --- .../web_app_icon_component.html.erb | 5 +++++ .../app_catalog/web_app_icon_component.rb | 21 +++++++++++++++++++ app/components/rs_auth_component.html.erb | 10 ++------- app/views/icons/_remotestorage.html.erb | 2 +- .../web_app_icon_component_spec.rb | 11 ++++++++++ 5 files changed, 40 insertions(+), 9 deletions(-) create mode 100644 app/components/app_catalog/web_app_icon_component.html.erb create mode 100644 app/components/app_catalog/web_app_icon_component.rb create mode 100644 spec/components/app_catalog/web_app_icon_component_spec.rb diff --git a/app/components/app_catalog/web_app_icon_component.html.erb b/app/components/app_catalog/web_app_icon_component.html.erb new file mode 100644 index 0000000..3f9c5f3 --- /dev/null +++ b/app/components/app_catalog/web_app_icon_component.html.erb @@ -0,0 +1,5 @@ +<% if @image_url %> + <%= image_tag @image_url, class: "h-full w-full" %> +<% else %> + <%= render partial: "icons/remotestorage", locals: { custom_class: "h-full w-full p-0.5 text-gray-200" } %> +<% end %> diff --git a/app/components/app_catalog/web_app_icon_component.rb b/app/components/app_catalog/web_app_icon_component.rb new file mode 100644 index 0000000..8421d4f --- /dev/null +++ b/app/components/app_catalog/web_app_icon_component.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module AppCatalog + class WebAppIconComponent < ViewComponent::Base + def initialize(web_app:) + if web_app&.icon&.attached? + @image_url = image_url_for(web_app.icon) + elsif web_app&.apple_touch_icon&.attached? + @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/components/rs_auth_component.html.erb b/app/components/rs_auth_component.html.erb index 18ffcc8..535dc1b 100644 --- a/app/components/rs_auth_component.html.erb +++ b/app/components/rs_auth_component.html.erb @@ -1,16 +1,10 @@
- <% if @web_app.icon.attached? %> - <%= image_tag s3_image_url(@web_app.icon), class: "h-full w-full" %> - <% elsif @web_app.apple_touch_icon.attached? %> - <%= image_tag s3_image_url(@web_app.apple_touch_icon), class: "h-full w-full" %> - <% else %> - <%= render partial: "icons/remotestorage", locals: { custom_class: "h-full w-full p-0.5 text-gray-200" } %> - <% end %> + <%= render AppCatalog::WebAppIconComponent.new(web_app: @web_app) %>

- <%= @web_app.name %> + <%= @web_app&.name || @auth.app_name %>

<%= @auth.client_id %> diff --git a/app/views/icons/_remotestorage.html.erb b/app/views/icons/_remotestorage.html.erb index 2daafc4..4de0c04 100644 --- a/app/views/icons/_remotestorage.html.erb +++ b/app/views/icons/_remotestorage.html.erb @@ -1,5 +1,5 @@ - + diff --git a/spec/components/app_catalog/web_app_icon_component_spec.rb b/spec/components/app_catalog/web_app_icon_component_spec.rb new file mode 100644 index 0000000..f9ed3e5 --- /dev/null +++ b/spec/components/app_catalog/web_app_icon_component_spec.rb @@ -0,0 +1,11 @@ +require "rails_helper" + +RSpec.describe AppCatalog::WebAppIconComponent, type: :component do + describe "No web app given" do + it "renders the default icon" do + expect( + render_inline(described_class.new(web_app: nil)) {}.to_html + ).to include("icon-remotestorage") + end + end +end From 84e915ece95250a0eef1884bc4ad94bf7a3fb14e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 8 Feb 2024 12:59:37 +0100 Subject: [PATCH 3/8] Allow custom path for ActiveStorage local/disk backend --- config/storage.yml | 2 +- docker-compose.yml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/config/storage.yml b/config/storage.yml index 9a15623..010aeb8 100644 --- a/config/storage.yml +++ b/config/storage.yml @@ -1,6 +1,6 @@ local: service: Disk - root: <%= Rails.root.join("storage") %> + root: <%= ENV["ACTIVE_STORAGE_PATH"] || Rails.root.join("storage") %> test: service: Disk diff --git a/docker-compose.yml b/docker-compose.yml index 6e0ff50..340925f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -42,6 +42,7 @@ services: LDAP_ADMIN_PASSWORD: passthebutter LDAP_USE_TLS: "false" REDIS_URL: redis://redis:6379/0 + ACTIVE_STORAGE_PATH: "/akkounts/tmp/attachments" RS_REDIS_URL: redis://redis:6379/1 RS_STORAGE_URL: "http://localhost:4567" S3_ENABLED: false From 411587456b4d3c40354fc62fdf040ab31580e907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 8 Feb 2024 13:01:19 +0100 Subject: [PATCH 4/8] Destroy dependent RS auths when destroying a WebApp --- app/models/app_catalog/web_app.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/app_catalog/web_app.rb b/app/models/app_catalog/web_app.rb index 0b88333..388e610 100644 --- a/app/models/app_catalog/web_app.rb +++ b/app/models/app_catalog/web_app.rb @@ -1,7 +1,7 @@ class AppCatalog::WebApp < ApplicationRecord store :metadata, coder: JSON - has_many :remote_storage_authorizations + has_many :remote_storage_authorizations, dependent: :destroy has_one_attached :icon has_one_attached :apple_touch_icon From a7410058fadd27eef1147977499a244fb46c2494 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 8 Feb 2024 13:02:08 +0100 Subject: [PATCH 5/8] Save WebApp before fetching icons --- app/services/app_catalog_manager/update_metadata.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/app_catalog_manager/update_metadata.rb b/app/services/app_catalog_manager/update_metadata.rb index c11b864..a23b3d6 100644 --- a/app/services/app_catalog_manager/update_metadata.rb +++ b/app/services/app_catalog_manager/update_metadata.rb @@ -18,6 +18,8 @@ module AppCatalogManager @app.metadata[prop] = metadata.send(prop) if prop end + @app.save! + if icon = metadata.select_icon(sizes: "256x256") || icon = metadata.select_icon(sizes: "192x192") attach_remote_image(:icon, icon) @@ -27,8 +29,6 @@ module AppCatalogManager if apple_touch_icon = metadata.select_icon(purpose: "apple-touch-icon") attach_remote_image(:apple_touch_icon, apple_touch_icon) end - - @app.save! rescue Manifique::Error => e msg = "Fetching web app manifest failed for #{e.url}: #{e.type}" Rails.logger.warn(msg) From 3f110995a4ca8b440e5b0695a2d9dd9ef0aec65d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 8 Feb 2024 13:02:47 +0100 Subject: [PATCH 6/8] Add timestamp to icon filenames There can be race condition when a background job is supposed to delete an icon while there is a new one being attached. Also, this encodes the date/time when the icon has been added, for inspection and convenience. --- app/services/app_catalog_manager/update_metadata.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/app_catalog_manager/update_metadata.rb b/app/services/app_catalog_manager/update_metadata.rb index a23b3d6..d212a45 100644 --- a/app/services/app_catalog_manager/update_metadata.rb +++ b/app/services/app_catalog_manager/update_metadata.rb @@ -42,8 +42,8 @@ module AppCatalogManager else download_url = "#{@app.url}/#{icon["src"].gsub(/^\//,'')}" end - filename = "#{attachment_name}.png" - key = "web_apps/#{@app.id}/icons/#{attachment_name}.png" + filename = "#{attachment_name}-#{Time.now.to_i}.png" + key = "web_apps/#{@app.id}/icons/#{filename}" begin tempfile = Down.download(download_url) From bd1b177993f4189fd86d82a609442de164c94afa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 8 Feb 2024 13:36:17 +0100 Subject: [PATCH 7/8] Rescue all icon download/upload errors, send to Sentry --- app/services/app_catalog_manager/update_metadata.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/services/app_catalog_manager/update_metadata.rb b/app/services/app_catalog_manager/update_metadata.rb index d212a45..a506013 100644 --- a/app/services/app_catalog_manager/update_metadata.rb +++ b/app/services/app_catalog_manager/update_metadata.rb @@ -20,6 +20,8 @@ module AppCatalogManager @app.save! + # TODO move icon downloads to separate, async job + if icon = metadata.select_icon(sizes: "256x256") || icon = metadata.select_icon(sizes: "192x192") attach_remote_image(:icon, icon) @@ -49,7 +51,12 @@ module AppCatalogManager tempfile = Down.download(download_url) @app.send(attachment_name).attach(key: key, io: tempfile, filename: filename) rescue Down::NotFound - Rails.logger.warn "Icon download failed: NotFound error for #{download_url}" + msg = "Download of \"#{attachment_name}\" failed: NotFound error for #{download_url}" + Rails.logger.warn(msg) + Sentry.capture_message(msg) + rescue => e + Rails.logger.warn "Saving attachment \"#{attachment_name}\" failed: \"#{e.message}\"" + Sentry.capture_exception(e) if Setting.sentry_enabled? end end end From b33b8104a8cd5c96e5570d360e8f8c190c9b4965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 27 Feb 2024 14:33:37 +0100 Subject: [PATCH 8/8] Fix typo --- app/views/icons/{_kebab-menu.html.erb => _kebap-menu.html.erb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename app/views/icons/{_kebab-menu.html.erb => _kebap-menu.html.erb} (100%) diff --git a/app/views/icons/_kebab-menu.html.erb b/app/views/icons/_kebap-menu.html.erb similarity index 100% rename from app/views/icons/_kebab-menu.html.erb rename to app/views/icons/_kebap-menu.html.erb