From 196d685f8d5c8d2604709867c09a7d3a5ad20ed9 Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 19 Dec 2025 11:13:06 +0100 Subject: [PATCH] Implement authorization for Active Storage endpoints Consider blobs attached to any public records accessible to anyone with the URL. --- app/models/account/export.rb | 4 + app/models/board/publishable.rb | 1 + app/models/card.rb | 4 + app/models/comment.rb | 2 +- config/initializers/action_text.rb | 8 + lib/rails_ext/active_storage_authorization.rb | 49 +++++ .../active_storage_authorization_test.rb | 197 ++++++++++++++++++ 7 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 lib/rails_ext/active_storage_authorization.rb create mode 100644 test/integration/active_storage_authorization_test.rb diff --git a/app/models/account/export.rb b/app/models/account/export.rb index c5e3d64c3..1fd2689d2 100644 --- a/app/models/account/export.rb +++ b/app/models/account/export.rb @@ -37,6 +37,10 @@ class Account::Export < ApplicationRecord update!(status: :completed, completed_at: Time.current) end + def accessible_to?(accessor) + accessor == user + end + private def generate_zip Tempfile.new([ "export", ".zip" ]).tap do |tempfile| diff --git a/app/models/board/publishable.rb b/app/models/board/publishable.rb index 6c1ce47d4..8b113cc9e 100644 --- a/app/models/board/publishable.rb +++ b/app/models/board/publishable.rb @@ -15,6 +15,7 @@ module Board::Publishable def published? publication.present? end + alias_method :publicly_accessible?, :published? def publish create_publication! unless published? diff --git a/app/models/card.rb b/app/models/card.rb index dbb45f184..18957cb9b 100644 --- a/app/models/card.rb +++ b/app/models/card.rb @@ -48,6 +48,10 @@ class Card < ApplicationRecord delegate :accessible_to?, to: :board + def publicly_accessible? + published? && board.publicly_accessible? + end + def card self end diff --git a/app/models/comment.rb b/app/models/comment.rb index d0315eea8..30706e8b5 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -15,7 +15,7 @@ class Comment < ApplicationRecord after_create_commit :watch_card_by_creator - delegate :board, :watch_by, to: :card + delegate :publicly_accessible?, :accessible_to?, :board, :watch_by, to: :card def to_partial_path "cards/#{super}" diff --git a/config/initializers/action_text.rb b/config/initializers/action_text.rb index 0f804e57b..6aed0616d 100644 --- a/config/initializers/action_text.rb +++ b/config/initializers/action_text.rb @@ -16,6 +16,14 @@ module ActionText def storage_tracked_record record.try(:storage_tracked_record) end + + def accessible_to?(user) + record.try(:accessible_to?, user) || record.try(:publicly_accessible?) + end + + def publicly_accessible? + record.try(:publicly_accessible?) + end end end end diff --git a/lib/rails_ext/active_storage_authorization.rb b/lib/rails_ext/active_storage_authorization.rb new file mode 100644 index 000000000..55d0de3bc --- /dev/null +++ b/lib/rails_ext/active_storage_authorization.rb @@ -0,0 +1,49 @@ +ActiveSupport.on_load :active_storage_blob do + def accessible_to?(user) + attachments.includes(:record).any? { |attachment| attachment.accessible_to?(user) } || attachments.none? + end + + def publicly_accessible? + attachments.includes(:record).any? { |attachment| attachment.publicly_accessible? } + end +end + +ActiveSupport.on_load :active_storage_attachment do + def accessible_to?(user) + record.try(:accessible_to?, user) + end + + def publicly_accessible? + record.try(:publicly_accessible?) + end +end + +Rails.application.config.to_prepare do + module ActiveStorage::Authorize + extend ActiveSupport::Concern + + include Authentication + + included do + # Ensure require_authentication runs after set_blob. + skip_before_action :require_authentication + before_action :require_authentication, :ensure_accessible, unless: :publicly_accessible_blob? + end + + private + def publicly_accessible_blob? + @blob.publicly_accessible? + end + + def ensure_accessible + unless @blob.accessible_to?(Current.user) + head :forbidden + end + end + end + + ActiveStorage::Blobs::RedirectController.include ActiveStorage::Authorize + ActiveStorage::Blobs::ProxyController.include ActiveStorage::Authorize + ActiveStorage::Representations::RedirectController.include ActiveStorage::Authorize + ActiveStorage::Representations::ProxyController.include ActiveStorage::Authorize +end diff --git a/test/integration/active_storage_authorization_test.rb b/test/integration/active_storage_authorization_test.rb new file mode 100644 index 000000000..85919ea13 --- /dev/null +++ b/test/integration/active_storage_authorization_test.rb @@ -0,0 +1,197 @@ +require "test_helper" + +class ActiveStorageAuthorizationTest < ActionDispatch::IntegrationTest + setup do + Current.session = sessions(:david) + @account = accounts("37s") + @board = boards(:writebook) + @card = cards(:logo) + @blob = attach_blob_to_card(@card) + end + + test "authenticated user with board access can view blob" do + sign_in_as :david + + get rails_blob_path(@blob, disposition: :inline) + assert_response :redirect + assert_match %r{rails/active_storage}, response.location + end + + test "authenticated user without board access cannot view blob" do + sign_in_as :mike + + get rails_blob_path(@blob, disposition: :inline) + assert_response :forbidden + end + + test "unauthenticated user cannot view blob" do + get rails_blob_path(@blob, disposition: :inline) + assert_response :redirect + assert_match %r{/session/new}, response.location + end + + test "authenticated user with board access can view representation" do + sign_in_as :david + + get rails_representation_path(@blob.representation(resize_to_limit: [ 100, 100 ])) + assert_response :redirect + assert_match %r{rails/active_storage/}, response.location + end + + test "authenticated user without board access cannot view representation" do + sign_in_as :mike + + get rails_representation_path(@blob.representation(resize_to_limit: [ 100, 100 ])) + assert_response :forbidden + end + + test "unauthenticated user can view blob on published board with published card" do + @board.publish + + get rails_blob_path(@blob, disposition: :inline) + assert_response :redirect + assert_match %r{rails/active_storage}, response.location + end + + test "unauthenticated user cannot view blob on published board with draft card" do + @board.publish + + # Create the draft card and attachment with proper Current context + draft_blob = nil + Current.with(account: @account, session: sessions(:david)) do + draft_card = @board.cards.create!(title: "Draft", status: :drafted, creator: users(:david)) + draft_card.image.attach io: file_fixture("moon.jpg").open, filename: "draft.jpg", content_type: "image/jpeg" + draft_blob = draft_card.image.blob + end + + get rails_blob_path(draft_blob, disposition: :inline) + assert_response :redirect + assert_match %r{/session/new}, response.location + end + + # Rich text embeds in cards + + test "authenticated user with board access can view rich text embed in card" do + sign_in_as :david + + blob = attach_blob_as_rich_text_embed(@card) + + get rails_blob_path(blob, disposition: :inline) + assert_response :redirect + assert_match %r{rails/active_storage}, response.location + end + + test "authenticated user without board access cannot view rich text embed in card" do + sign_in_as :mike + + blob = attach_blob_as_rich_text_embed(@card) + + get rails_blob_path(blob, disposition: :inline) + assert_response :forbidden + end + + test "unauthenticated user can view rich text embed in card on published board" do + @board.publish + + blob = attach_blob_as_rich_text_embed(@card) + + get rails_blob_path(blob, disposition: :inline) + assert_response :redirect + assert_match %r{rails/active_storage}, response.location + end + + # Rich text embeds in comments + + test "authenticated user with board access can view rich text embed in comment" do + sign_in_as :david + + comment = comments(:logo_1) + blob = attach_blob_as_rich_text_embed(comment) + + get rails_blob_path(blob, disposition: :inline) + assert_response :redirect + assert_match %r{rails/active_storage}, response.location + end + + test "authenticated user without board access cannot view rich text embed in comment" do + sign_in_as :mike + + comment = comments(:logo_1) + blob = attach_blob_as_rich_text_embed(comment) + + get rails_blob_path(blob, disposition: :inline) + assert_response :forbidden + end + + test "unauthenticated user can view rich text embed in comment on published board" do + @board.publish + + comment = comments(:logo_1) + blob = attach_blob_as_rich_text_embed(comment) + + get rails_blob_path(blob, disposition: :inline) + assert_response :redirect + assert_match %r{rails/active_storage}, response.location + end + + # Account exports + + test "export owner can download their export" do + sign_in_as :david + + blob = create_export_blob_for(users(:david)) + + get rails_blob_path(blob, disposition: :attachment) + assert_response :redirect + assert_match %r{rails/active_storage}, response.location + end + + test "non-owner cannot download another user's export" do + sign_in_as :jz + + blob = create_export_blob_for(users(:david)) + + get rails_blob_path(blob, disposition: :attachment) + assert_response :forbidden + end + + test "unauthenticated user cannot download export" do + blob = create_export_blob_for(users(:david)) + + get rails_blob_path(blob, disposition: :attachment) + assert_response :redirect + assert_match %r{/session/new}, response.location + end + + private + def attach_blob_to_card(card) + Current.with(session: sessions(:david)) do + card.image.attach io: file_fixture("moon.jpg").open, filename: "test.jpg", content_type: "image/jpeg" + card.image.blob + end + end + + def attach_blob_as_rich_text_embed(container) + Current.with(account: @account, session: sessions(:david)) do + blob = ActiveStorage::Blob.create_and_upload! \ + io: file_fixture("moon.jpg").open, + filename: "embed.jpg", + content_type: "image/jpeg" + + attachment_html = ActionText::Attachment.from_attachable(blob).to_html + if container.respond_to?(:description) + container.update!(description: "

Description with image: #{attachment_html}

") + else + container.update!(body: "

Body with image: #{attachment_html}

") + end + + blob.reload + end + end + + def create_export_blob_for(user) + export = Account::Export.create!(account: @account, user: user) + export.file.attach io: StringIO.new("test export content"), filename: "export.zip", content_type: "application/zip" + export.file.blob + end +end