Last active
October 22, 2019 22:33
-
-
Save julik/4cc53525ea14345363c46063d75cc2af to your computer and use it in GitHub Desktop.
A very very very rough version. There probably will be a gem of this
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
module Kelder | |
module PrefixKeyWithToken | |
# Prefix all generated blob keys with the tenant. Do not | |
# use slash as a delimiter because it needs different escaping | |
# depending on the storage service adapter - in some cases it | |
# might be significant, in other cases it might get escaped as a path | |
# component etc. | |
def generate_unique_secure_token | |
tenant_slug = Apartment::Tenant.current.split('_').last | |
"#{tenant_slug}-#{super}" | |
end | |
end | |
module FindSignedWithTenant | |
# Finds in the correct database | |
def find_signed(signed_id_with_tenant_prefix) | |
tenant, id = ActiveStorage.verifier.verify(signed_id_with_tenant_prefix, purpose: :blob_id_with_tenant) | |
Apartment::Tenant.switch(tenant) { find(id) } | |
end | |
end | |
module PrefixedSignedId | |
def signed_id | |
ActiveStorage.verifier.generate([Apartment::Tenant.current, id], purpose: :blob_id_with_tenant) | |
end | |
end | |
module FolderForPrefix | |
# The disk storage service creates folders which avoid having too many files | |
# within the same directory. If we leave it unchanged, it will generate very | |
# few folders for our project slugs but will not reduce the number of files | |
# in a single directory - which will create severe performance problems once | |
# we have tens of thousands or hundreds of thousands of files per directory. | |
# To avoid it, we need to make sure the directories are "spread" underneath | |
# the project slug subdirectory. So if a standard ActiveStorage method | |
# would generate this: | |
# | |
# "abcdefg" => "ab/cd/abcdefg" | |
# | |
# without our patch, having a prefix and a dash: | |
# | |
# "prj1-abcdefg" => "pr/j1/prj1-abcdefg" | |
# | |
# This would kill performance - especially when globbing. | |
# Instead we want to do this: | |
# | |
# "prj1-abcdefg" => "prj1/ab/cd/prj1-abcdefg" | |
# | |
# which is a combination of both. It will also make our project deletes faster. | |
def folder_for(key_with_prefix) | |
return super unless key_with_prefix.include?('-') | |
# This is a tenant key, so we need to prefix the key with | |
# the tenant slug which should create a single directory. | |
# Underneath that - use the original method only for the pseudorandom | |
# component of the key. | |
tenant_slug, random_component = key_with_prefix.split('-') | |
path_for_random_component = super(random_component) | |
[tenant_slug, path_for_random_component].join('/') | |
end | |
end | |
# The ActiveStorage controllers get mounted under the Rails routes root, or - at best - | |
# under a static scope. Inside of those controllers we need to know which tenant is | |
# being used for this particular operation. To do that we are going to "sidechannel" | |
# the tenant name, signed, into one of the query string parameters. This query string | |
# parameter needs to be given to the Rails URL helpers when generating the blob | |
# URL or the direct upload URL, otherwise the main tenant is going to be used. | |
# | |
# These controllers need a "service elevator". | |
module ControllerElevator | |
# We cannot use around_action because it is going to be applied _after_ | |
# the before_action of the builtin ActiveStorage controllers. That before_action | |
# actually calls set_blob, and set_blob already does the first ActiveRecord query. | |
# | |
# We need to intercept prior to set_blob, so around_action won't really work all that well. | |
def process_action(*args) | |
if signed_tenant_name = request.query_parameters['signed_tenant_name'] | |
# The tenant name is signed because otherwise we would be allowing uploads | |
# from any project or even by not signed-in users, we want to control these URLs real tight. | |
tenant_database_name = ActiveStorage.verifier.verify(signed_tenant_name, purpose: :active_storage_controllers) | |
Apartment::Tenant.switch(tenant_database_name) do | |
super | |
end | |
else | |
super | |
end | |
end | |
end | |
# Install our patches via railtie (this is going to become a gem) | |
class Railtie < Rails::Railtie | |
# ActiveStorage can get reloaded as well, and once it does get reloaded | |
# our injected modules will be lost. We need to ensure the patch is applied | |
# on every reload. | |
ActiveSupport::Reloader.to_prepare do | |
# Install the prefixed key patch and the prefixed signed ID patches. | |
ActiveStorage::Blob.singleton_class.send(:prepend, PrefixKeyWithToken) | |
ActiveStorage::Blob.singleton_class.send(:prepend, FindSignedWithTenant) | |
ActiveStorage::Blob.prepend(PrefixedSignedId) | |
# Install DiskService patch to have prefixed folder trees | |
require 'active_storage/service/disk_service' unless defined?(ActiveStorage::Service::DiskService) | |
ActiveStorage::Service::DiskService.prepend(FolderForPrefix) | |
# Install the elevator into controllers | |
ActiveStorage::BaseController.prepend(ControllerElevator) | |
end | |
end | |
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
require 'rails_helper' | |
# Note that you need to disable transactional fixtures here, we do it with | |
# database_cleaner_strategy: :truncation but this has to be configured ahead of time | |
# in your spec helper | |
RSpec.describe Kelder do | |
before :all do | |
Apartment::Tenant.create("test_tenant_kelder_tenant123") | |
end | |
after :all do | |
Apartment::Tenant.drop("test_tenant_kelder_tenant123") | |
end | |
describe 'ActiveStorage::Blob overrides' do | |
it 'stay intact after app reload' do | |
if Rails.application.config.cache_classes | |
skip("Requires Rails.application.config.cache_classes to be false") | |
end | |
expect(ActiveStorage::Blob.ancestors).to include(Kelder::PrefixedSignedId) | |
Rails.application.reloader.reload! | |
expect(ActiveStorage::Blob.ancestors).to include(Kelder::PrefixedSignedId) | |
end | |
it 'prefixes the "key" attribute with the last component of the current tenant database name' do | |
Apartment::Tenant.switch("test_tenant_kelder_tenant123") do | |
blob = ActiveStorage::Blob.new | |
expect(blob.key).to start_with("tenant123") | |
end | |
end | |
it 'returns an altered payload from #signed_id and can use it to find itself later via .find_signed' do | |
Apartment::Tenant.switch("test_tenant_kelder_tenant123") do | |
blob = ActiveStorage::Blob.new(id: 10) | |
expect(blob.id).to eq(10) | |
signed_id_with_tenant = blob.signed_id | |
tenant_name, id = ActiveStorage.verifier.verify(signed_id_with_tenant, purpose: :blob_id_with_tenant) | |
expect(tenant_name).to eq("test_tenant_kelder_tenant123") | |
expect(ActiveStorage::Blob).to receive(:find).with(10) | |
ActiveStorage::Blob.find_signed(signed_id_with_tenant) | |
end | |
end | |
end | |
describe 'ActiveStorage::DirectUploadsController overrides', type: :request do | |
it 'performs the original action if there is no "signed_tenant_name"' do | |
blob_params = {blob: {filename: 'file.txt', byte_size: 123, checksum: "abefg"}} | |
expect(Apartment::Tenant).not_to receive(:switch) | |
post "/rails/active_storage/direct_uploads", params: blob_params | |
expect(response).to be_ok | |
parsed_response = JSON.load(response.body) | |
expect(parsed_response["key"]).not_to start_with("tenant123") | |
end | |
it 'switches into the tenant before returning the direct upload URL' do | |
st = ActiveStorage.verifier.generate("test_tenant_kelder_tenant123", purpose: :active_storage_controllers) | |
blob_params = {blob: {filename: 'file.txt', byte_size: 123, checksum: "abefg"}} | |
expect(Apartment::Tenant).to receive(:switch).with("test_tenant_kelder_tenant123").and_call_original | |
post "/rails/active_storage/direct_uploads?signed_tenant_name=#{st}", params: blob_params | |
expect(response).to be_ok | |
parsed_response = JSON.load(response.body) | |
expect(parsed_response["key"]).to start_with("tenant123") | |
end | |
end | |
describe 'ActiveStorage::BlobsController overrides', type: :request do | |
it 'with the main tenant, does not suffix the blob key' do | |
data = StringIO.new(Random.new.bytes(1024*5)) | |
signed_id = ActiveStorage::Blob.create_after_upload!(io: data, filename: 'test.bin').signed_id | |
get "/rails/active_storage/blobs/#{signed_id}/test.bin" | |
expect(response).to be_redirect | |
location_on_storage_service = response.location | |
get location_on_storage_service | |
expect(response).to be_ok | |
expect(response.body.bytesize).to eq(1024*5) | |
end | |
it 'returns the correct blob even though the blob URL does not contain the query string parameter' do | |
data = StringIO.new(Random.new.bytes(1024*5)) | |
signed_id = Apartment::Tenant.switch("test_tenant_kelder_tenant123") do | |
ActiveStorage::Blob.create_after_upload!(io: data, filename: 'test.bin').signed_id | |
end | |
expect(Apartment::Tenant).to receive(:switch).with("test_tenant_kelder_tenant123").and_call_original | |
get "/rails/active_storage/blobs/#{signed_id}/test.bin" | |
expect(response).to be_redirect | |
location_on_storage_service = response.location | |
get location_on_storage_service | |
expect(response).to be_ok | |
expect(response.body.bytesize).to eq(1024*5) | |
end | |
end | |
describe 'ActiveStorage::Service::DiskService override' do | |
it 'stores the files in a prefixed subdirectory' do | |
Dir.mktmpdir do |tempdir_path| | |
subject = ActiveStorage::Service::DiskService.new(root: tempdir_path) | |
data = StringIO.new(Random.new.bytes(98721)) | |
subject.upload("tenant-abcdefg123", data) | |
expect(File).to be_exist(tempdir_path + '/tenant/ab/cd/tenant-abcdefg123') | |
end | |
end | |
it 'does not tenant-prefix a key which does not contain a "-"' do | |
Dir.mktmpdir do |tempdir_path| | |
subject = ActiveStorage::Service::DiskService.new(root: tempdir_path) | |
data = StringIO.new(Random.new.bytes(98721)) | |
subject.upload("notenant", data) | |
expect(File).to be_exist(tempdir_path + '/no/te/notenant') | |
end | |
end | |
end | |
end |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment