Skip to content

Commit

Permalink
Optionally use redirects and signed cookies for blobs
Browse files Browse the repository at this point in the history
  • Loading branch information
pixeltrix committed Jan 14, 2025
1 parent e5e5b99 commit 159f73d
Show file tree
Hide file tree
Showing 21 changed files with 293 additions and 92 deletions.
1 change: 1 addition & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ GROUPED_PROPOSAL_DETAILS_FEATURE=enabled
GOOGLE_TAG_MANAGER_ID=GTM-XXXXXXXX
UPLOADS_HOSTNAME=uploads.bops.localhost
UPLOADS_BASE_URL=http://uploads.bops.localhost:3000
USE_SIGNED_URLS=false
1 change: 1 addition & 0 deletions .env.test
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ OS_VECTOR_TILES_API_KEY="testtest"
NOTIFY_LETTER_API_KEY='testtest'
UPLOADS_HOSTNAME=uploads.example.com
UPLOADS_BASE_URL=http://uploads.example.com
USE_SIGNED_URLS=false
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ gem "aasm"
gem "activerecord-postgis-adapter", "~> 10.0"
gem "acts_as_list"
gem "appsignal"
gem "aws-sdk-cloudfront", require: false
gem "aws-sdk-s3", require: false
gem "bootsnap", ">= 1.4.2", require: false
gem "business_time"
Expand Down
4 changes: 4 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ GEM
ast (2.4.2)
aws-eventstream (1.1.1)
aws-partitions (1.489.0)
aws-sdk-cloudfront (1.55.0)
aws-sdk-core (~> 3, >= 3.119.0)
aws-sigv4 (~> 1.1)
aws-sdk-core (3.119.1)
aws-eventstream (~> 1, >= 1.0.2)
aws-partitions (~> 1, >= 1.239.0)
Expand Down Expand Up @@ -655,6 +658,7 @@ DEPENDENCIES
activerecord-postgis-adapter (~> 10.0)
acts_as_list
appsignal
aws-sdk-cloudfront
aws-sdk-s3
bootsnap (>= 1.4.2)
bops_admin!
Expand Down
26 changes: 26 additions & 0 deletions app/models/document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,33 @@ class NotArchiveableError < StandardError; end
self.user ||= Current.user
end

has_many :file_variant_records, through: :file_blob, source: :variant_records
has_many :file_variant_attachments, through: :file_variant_records, source: :image_attachment
has_many :file_variant_blobs, through: :file_variant_attachments, source: :blob

has_one :file_preview_image_attachment, through: :file_blob, source: :preview_image_attachment
has_one :file_preview_blob, through: :file_preview_image_attachment, source: :blob
has_many :file_preview_variant_records, through: :file_preview_blob, source: :variant_records
has_many :file_preview_variant_attachments, through: :file_preview_variant_records, source: :image_attachment
has_many :file_preview_variant_blobs, through: :file_preview_variant_attachments, source: :blob

class << self
def find_by_blob!(key:)
blob_associations = %i[
file_blob
file_preview_blob
file_variant_blobs
file_preview_variant_blobs
]

left_joins(*blob_associations)
.where(file_blob: {key:})
.or(where(file_variant_blobs: {key:}))
.or(where(file_preview_blob: {key:}))
.or(where(file_preview_variant_blobs: {key:}))
.distinct.sole
end

def tags(key)
case key.to_s
when "plans"
Expand Down
1 change: 1 addition & 0 deletions app/models/local_authority.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class LocalAuthority < ApplicationRecord

with_options through: :planning_applications do
has_many :audits
has_many :documents
has_many :validation_requests
end

Expand Down
1 change: 1 addition & 0 deletions config/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ shared:
uploads_hostname: <%= ENV.fetch("UPLOADS_HOSTNAME", "uploads.bops.localhost:3000") %>
uploads_base_url: <%= ENV.fetch("UPLOADS_BASE_URL", "http://uploads.bops.localhost:3000") %>
domain: <%= ENV.fetch("DOMAIN", "bops.localhost:3000") %>
use_signed_urls: <%= ENV.fetch("USE_SIGNED_URLS", "false") == "true" %>
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,23 @@

module BopsUploads
class ApplicationController < ActionController::Base
include BopsCore::ApplicationController

with_options to: :BopsUploads do
delegate :key_pair_id, :private_key, :cookie_signer
end

before_action :set_service
before_action :set_blob

private

def set_service
@service = ActiveStorage::Blob.service
end

def set_blob
@blob = ActiveStorage::Blob.find_by!(key: params[:key])
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# frozen_string_literal: true

module BopsUploads
class BlobsController < ApplicationController
def show
serve_file(blob_path, content_type:, disposition:)
rescue Errno::ENOENT
head :not_found
end

private

def blob_path
@service.path_for(@blob.key)
end

def forcibly_serve_as_binary?
ActiveStorage.content_types_to_serve_as_binary.include?(@blob.content_type)
end

def allowed_inline?
ActiveStorage.content_types_allowed_inline.include?(@blob.content_type)
end

def content_type
forcibly_serve_as_binary? ? ActiveStorage.binary_content_type : @blob.content_type
end

def disposition
if forcibly_serve_as_binary? || !allowed_inline?
:attachment
else
:inline
end
end

def serve_file(path, content_type:, disposition:)
::Rack::Files.new(nil).serving(request, path).tap do |(status, headers, body)|
self.status = status
self.response_body = body

headers.each do |name, value|
response.headers[name] = value
end

response.headers.except!("X-Cascade", "x-cascade") if status == 416
response.headers["Content-Type"] = content_type || DEFAULT_SEND_FILE_TYPE
response.headers["Content-Disposition"] = disposition || DEFAULT_SEND_FILE_DISPOSITION
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -2,62 +2,45 @@

module BopsUploads
class FilesController < ApplicationController
before_action :set_service
before_action :set_blob
before_action :set_document
before_action :set_planning_application

def show
serve_file(blob_path, content_type:, disposition:)
rescue Errno::ENOENT
head :not_found
end

private
signed_cookies.each do |key, value|
cookies[key] = {
value: value,
path: blob_path(@blob.key),
expires: expiry_time
}
end

def set_service
@service = ActiveStorage::Blob.service
redirect_to blob_url(@blob.key)
end

def set_blob
@blob = ActiveStorage::Blob.find_by!(key: params[:key])
end
private

def blob_path
@service.path_for(@blob.key)
def set_document
@document = current_local_authority.documents.find_by_blob!(key: @blob.key)
end

def forcibly_serve_as_binary?
ActiveStorage.content_types_to_serve_as_binary.include?(@blob.content_type)
def set_planning_application
@planning_application = @document.planning_application
end

def allowed_inline?
ActiveStorage.content_types_allowed_inline.include?(@blob.content_type)
def signed_cookies
cookie_signer.signed_cookie(url_to_be_signed, signing_options)
end

def content_type
forcibly_serve_as_binary? ? ActiveStorage.binary_content_type : @blob.content_type
def url_to_be_signed
blob_url(@blob.key)
end

def disposition
if forcibly_serve_as_binary? || !allowed_inline?
:attachment
else
:inline
end
def signing_options
{expires: expiry_time}
end

def serve_file(path, content_type:, disposition:)
::Rack::Files.new(nil).serving(request, path).tap do |(status, headers, body)|
self.status = status
self.response_body = body

headers.each do |name, value|
response.headers[name] = value
end

response.headers.except!("X-Cascade", "x-cascade") if status == 416
response.headers["Content-Type"] = content_type || DEFAULT_SEND_FILE_TYPE
response.headers["Content-Disposition"] = disposition || DEFAULT_SEND_FILE_DISPOSITION
end
def expiry_time
10.minutes.from_now
end
end
end

This file was deleted.

11 changes: 8 additions & 3 deletions engines/bops_uploads/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,24 @@
extend BopsCore::Routing

local_authority_subdomain do
get "/files/:key", to: "redirects#show", as: "redirect"
get "/blobs/:key", to: "blobs#show", as: "blob"
get "/files/:key", to: "files#show", as: "file"
end

uploads_subdomain do
get "/:key", to: "files#show", as: "file"
get "/:key", to: "blobs#show", as: "upload"
end
end

Rails.application.routes.draw do
direct :uploaded_file do |blob, options|
next "" if blob.blank?

bops_uploads.redirect_url(blob.key)
if Rails.configuration.use_signed_urls
bops_uploads.file_url(blob.key)
else
bops_uploads.upload_url(blob.key, host: Rails.configuration.uploads_base_url)
end
end

resolve("ActiveStorage::Attachment") { |attachment, options| route_for(:uploaded_file, attachment.blob, options) }
Expand Down
4 changes: 4 additions & 0 deletions engines/bops_uploads/lib/bops_uploads.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
require "bops_uploads/engine"

module BopsUploads
with_options instance_accessor: false do
mattr_accessor :key_pair_id, :private_key, :cookie_signer
end

class << self
def env
ActiveSupport::StringInquirer.new(ENV.fetch("BOPS_ENVIRONMENT", "development"))
Expand Down
20 changes: 20 additions & 0 deletions engines/bops_uploads/lib/bops_uploads/engine.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,27 @@
# frozen_string_literal: true

require "aws-sdk-cloudfront"

module BopsUploads
class Engine < ::Rails::Engine
isolate_namespace BopsUploads

initializer "bops_uploads.cookie_signer" do |app|
config.after_initialize do
key_pair_id = ENV.fetch("UPLOADS_PUBLIC_KEY_ID") do
SecureRandom.alphanumeric(14).upcase
end

private_key = ENV.fetch("UPLOADS_PRIVATE_KEY") do
OpenSSL::PKey::RSA.generate(2048).to_pem
end

cookie_signer = Aws::CloudFront::CookieSigner.new(key_pair_id:, private_key:)

BopsUploads.key_pair_id = key_pair_id
BopsUploads.private_key = OpenSSL::PKey::RSA.new(private_key)
BopsUploads.cookie_signer = cookie_signer
end
end
end
end
6 changes: 0 additions & 6 deletions engines/bops_uploads/spec/bops_uploads_helper.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
# frozen_string_literal: true

require "rails_helper"

RSpec.configure do |config|
config.before type: :request do
host!("uploads.bops.services")
end
end
35 changes: 35 additions & 0 deletions engines/bops_uploads/spec/helpers/url_helpers_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# frozen_string_literal: true

require "bops_uploads_helper"

RSpec.describe "Generating urls" do
let(:config) { Rails.configuration }
let(:document) { create(:document) }
let(:blob) { document.file }
let(:key) { blob.key }

before do
request.headers["HTTP_HOST"] = "southwark.bops.services"
allow(config).to receive(:uploads_base_url).and_return("http://uploads.bops.services")
end

context "when use_signed_urls is false" do
before do
allow(config).to receive(:use_signed_urls).and_return(false)
end

it "generates urls with the uploads subdomain" do
expect(helper.uploaded_file_url(blob)).to eq("http://uploads.bops.services/#{key}")
end
end

context "when use_signed_urls is true" do
before do
allow(config).to receive(:use_signed_urls).and_return(true)
end

it "generates urls with the local authority subdomain" do
expect(helper.uploaded_file_url(blob)).to eq("http://southwark.bops.services/files/#{key}")
end
end
end
Loading

0 comments on commit 159f73d

Please sign in to comment.