diff --git a/.env.example b/.env.example index 8977c107f9..588d8941e4 100644 --- a/.env.example +++ b/.env.example @@ -1,3 +1,4 @@ +DOMAIN=bops.localhost:3000 PAAPI_HOST=paapi.services OS_VECTOR_TILES_API_KEY=xxxxx OTP_SECRET_ENCRYPTION_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx @@ -6,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 diff --git a/.env.test b/.env.test index ba4332a0de..f895d54bc2 100644 --- a/.env.test +++ b/.env.test @@ -1,6 +1,8 @@ +DOMAIN="example.com" STAGING_API_URL="bops-staging.services" STAGING_API_BEARER="fjisdfjsdiofjdsoi" 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 diff --git a/Gemfile b/Gemfile index ed5d1ddf91..57e910c221 100644 --- a/Gemfile +++ b/Gemfile @@ -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" diff --git a/Gemfile.lock b/Gemfile.lock index 6df0fe0ea7..ffe527518d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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) @@ -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! diff --git a/app/controllers/documents_controller.rb b/app/controllers/documents_controller.rb index 42bcf74081..8dea7584e0 100644 --- a/app/controllers/documents_controller.rb +++ b/app/controllers/documents_controller.rb @@ -11,7 +11,7 @@ class DocumentsController < AuthenticationController before_action :replacement_document_validation_request, only: %i[edit update] def index - @documents = @planning_application.documents.with_file_attachment + @documents = @planning_application.documents.default.with_file_attachment @additional_document_validation_requests = @planning_application .additional_document_validation_requests .post_validation diff --git a/app/helpers/document_helper.rb b/app/helpers/document_helper.rb index 4593dc8b75..f1b5c7dc18 100644 --- a/app/helpers/document_helper.rb +++ b/app/helpers/document_helper.rb @@ -40,7 +40,6 @@ def link_to_document(link_text, document, **args) link_text, url_for_document(document), new_tab:, - download: reference_or_file_name(document), **args ) end diff --git a/app/models/document.rb b/app/models/document.rb index f179ea307e..ec52f0cd75 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -4,6 +4,22 @@ class Document < ApplicationRecord class Routing include Rails.application.routes.url_helpers include Rails.application.routes.mounted_helpers + + def initialize(subdomain) + @subdomain = subdomain + end + + def default_url_options + {host: "#{subdomain}.#{domain}"} + end + + private + + attr_reader :subdomain + + def domain + Rails.configuration.domain + end end class NotArchiveableError < StandardError; end @@ -29,6 +45,7 @@ class NotArchiveableError < StandardError; end inverse_of: false delegate :audits, to: :planning_application + delegate :local_authority, to: :planning_application delegate :blob, :representable?, to: :file include Auditable @@ -209,12 +226,11 @@ class NotArchiveableError < StandardError; end validate :numbered validate :created_date_is_in_the_past - default_scope -> { no_owner.or(not_excluded_owners) } - scope :no_owner, -> { where(owner_type: nil) } scope :not_excluded_owners, -> { where.not(owner_type: EXCLUDED_OWNERS) } + scope :default, -> { no_owner.or(not_excluded_owners) } scope :by_created_at, -> { order(created_at: :asc) } - scope :active, -> { where(archived_at: nil) } + scope :active, -> { default.where(archived_at: nil) } scope :invalidated, -> { where(validated: false) } scope :validated, -> { where(validated: true) } scope :redacted, -> { where(redacted: true) } @@ -225,8 +241,8 @@ class NotArchiveableError < StandardError; end ) scope :publishable, -> { where(publishable: true) } - scope :for_publication, -> { active.publishable } - scope :for_display, -> { active.referenced_in_decision_notice } + scope :for_publication, -> { publishable } + scope :for_display, -> { referenced_in_decision_notice } scope :with_tag, ->(tag) { where(arel_table[:tags].contains(Array.wrap(tag))) } scope :with_siteplan_tags, -> { where(arel_table[:tags].overlaps(%w[sitePlan.existing sitePlan.proposed])) } @@ -247,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" @@ -359,7 +401,7 @@ def representation_url(transformations = {resize_to_limit: [1000, 1000]}) private def routes - @_routes ||= Routing.new + @_routes ||= Routing.new(local_authority.subdomain) end def no_open_replacement_request diff --git a/app/models/local_authority.rb b/app/models/local_authority.rb index 4c9a1e5766..c0fbee0195 100644 --- a/app/models/local_authority.rb +++ b/app/models/local_authority.rb @@ -23,6 +23,7 @@ class LocalAuthority < ApplicationRecord with_options through: :planning_applications do has_many :audits + has_many :documents has_many :validation_requests end diff --git a/app/models/planning_application.rb b/app/models/planning_application.rb index 247ac25215..a9bb0d2667 100644 --- a/app/models/planning_application.rb +++ b/app/models/planning_application.rb @@ -471,7 +471,7 @@ def agent_or_applicant_name end def documents_for_decision_notice - documents.for_display + documents.active.for_display end def valid_from @@ -972,7 +972,7 @@ def make_public=(value) end def documents_for_publication - documents.for_publication.or(site_notice_documents_for_publication) + documents.active.for_publication.or(site_notice_documents_for_publication) end def reporting_type_detail diff --git a/app/views/api/v1/planning_applications/_show.json.jbuilder b/app/views/api/v1/planning_applications/_show.json.jbuilder index 82241f1a2e..9b4316ae1c 100644 --- a/app/views/api/v1/planning_applications/_show.json.jbuilder +++ b/app/views/api/v1/planning_applications/_show.json.jbuilder @@ -56,7 +56,7 @@ end json.received_date planning_application.received_at json.decision planning_application.decision if planning_application.determined? json.constraints planning_application.planning_application_constraints.map(&:constraint).map(&:type_code) if planning_application.planning_application_constraints.any? -json.documents planning_application.documents.for_publication do |document| +json.documents planning_application.documents.active.for_publication do |document| json.url api_v1_planning_application_document_url(planning_application, document) json.blob_url document.representation_url json.extract! document, diff --git a/config/application.rb b/config/application.rb index b623d96fc3..fc721bab4e 100644 --- a/config/application.rb +++ b/config/application.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require_relative "boot" -require_relative "../lib/constraints/subdomain" require "rails" # Pick the frameworks you want: diff --git a/config/application.yml b/config/application.yml index 1bb0d4867a..d0b960d13c 100644 --- a/config/application.yml +++ b/config/application.yml @@ -12,3 +12,5 @@ shared: staging_api_url: <%= ENV["STAGING_API_URL"] %> 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" %> diff --git a/config/routes.rb b/config/routes.rb index 9cb8263d0a..4157d0511a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,9 +1,11 @@ # frozen_string_literal: true Rails.application.routes.draw do + extend BopsCore::Routing + get :healthcheck, to: proc { [200, {}, %w[OK]] } - constraints Constraints::DeviseSubdomain do + devise_subdomain do devise_for :users, controllers: { sessions: "users/sessions", confirmations: "confirmations" @@ -16,7 +18,7 @@ end end - constraints Constraints::LocalAuthoritySubdomain do + local_authority_subdomain do root to: "planning_applications#index" concern :positionable do |options| @@ -380,11 +382,9 @@ end end - constraints Constraints::ConfigSubdomain do + config_subdomain do mount BopsConfig::Engine, at: "/", as: :bops_config end - constraints Constraints::UploadsSubdomain do - mount BopsUploads::Engine, at: "/", as: :bops_uploads - end + mount BopsUploads::Engine, at: "/", as: :bops_uploads end diff --git a/engines/bops_api/app/views/bops_api/v2/planning_applications/_show.json.jbuilder b/engines/bops_api/app/views/bops_api/v2/planning_applications/_show.json.jbuilder index 28321458a5..335e2e777f 100644 --- a/engines/bops_api/app/views/bops_api/v2/planning_applications/_show.json.jbuilder +++ b/engines/bops_api/app/views/bops_api/v2/planning_applications/_show.json.jbuilder @@ -55,7 +55,7 @@ json.validAt planning_application.validated_at json.publishedAt planning_application.published_at json.decision planning_application.decision if planning_application.determined? json.constraints planning_application.planning_application_constraints.map(&:constraint).map(&:type_code) if planning_application.planning_application_constraints.any? -json.documents planning_application.documents.for_publication do |document| +json.documents planning_application.documents.active.for_publication do |document| json.url main_app.api_v1_planning_application_document_url(planning_application, document) json.extract! document, :created_at, diff --git a/engines/bops_core/lib/bops_core.rb b/engines/bops_core/lib/bops_core.rb index 3b3821b34b..5c199c597b 100644 --- a/engines/bops_core/lib/bops_core.rb +++ b/engines/bops_core/lib/bops_core.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "bops_core/engine" +require "bops_core/routing" module BopsCore class << self diff --git a/engines/bops_core/lib/bops_core/routing.rb b/engines/bops_core/lib/bops_core/routing.rb new file mode 100644 index 0000000000..0702ed2bf6 --- /dev/null +++ b/engines/bops_core/lib/bops_core/routing.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module BopsCore + module Routing + extend ActiveSupport::Concern + + class LocalAuthoritySubdomain + class << self + def matches?(request) + LocalAuthority.pluck(:subdomain).include?(request.subdomain) + end + end + end + + class ConfigSubdomain + class << self + def matches?(request) + request.subdomain == "config" + end + end + end + + class DeviseSubdomain + class << self + def matches?(request) + ConfigSubdomain.matches?(request) || LocalAuthoritySubdomain.matches?(request) + end + end + end + + class UploadsSubdomain + class << self + def matches?(request) + request.subdomain == "uploads" + end + end + end + + def local_authority_subdomain(&) + constraints(LocalAuthoritySubdomain, &) + end + + def config_subdomain(&) + constraints(ConfigSubdomain, &) + end + + def devise_subdomain(&) + constraints(DeviseSubdomain, &) + end + + def uploads_subdomain(&) + constraints(UploadsSubdomain, &) + end + end +end diff --git a/engines/bops_uploads/app/controllers/bops_uploads/application_controller.rb b/engines/bops_uploads/app/controllers/bops_uploads/application_controller.rb index 7472e75fc9..c9f4407cd0 100644 --- a/engines/bops_uploads/app/controllers/bops_uploads/application_controller.rb +++ b/engines/bops_uploads/app/controllers/bops_uploads/application_controller.rb @@ -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 diff --git a/engines/bops_uploads/app/controllers/bops_uploads/blobs_controller.rb b/engines/bops_uploads/app/controllers/bops_uploads/blobs_controller.rb new file mode 100644 index 0000000000..8b41a956a6 --- /dev/null +++ b/engines/bops_uploads/app/controllers/bops_uploads/blobs_controller.rb @@ -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 diff --git a/engines/bops_uploads/app/controllers/bops_uploads/files_controller.rb b/engines/bops_uploads/app/controllers/bops_uploads/files_controller.rb index 6f166dd015..c216832b3d 100644 --- a/engines/bops_uploads/app/controllers/bops_uploads/files_controller.rb +++ b/engines/bops_uploads/app/controllers/bops_uploads/files_controller.rb @@ -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 diff --git a/engines/bops_uploads/config/routes.rb b/engines/bops_uploads/config/routes.rb index 3cf93c80a3..926a54363d 100644 --- a/engines/bops_uploads/config/routes.rb +++ b/engines/bops_uploads/config/routes.rb @@ -1,14 +1,27 @@ # frozen_string_literal: true BopsUploads::Engine.routes.draw do - get "/:key", to: "files#show", as: "file" + extend BopsCore::Routing + + local_authority_subdomain do + get "/blobs/:key", to: "blobs#show", as: "blob" + get "/files/:key", to: "files#show", as: "file" + end + + uploads_subdomain do + 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.file_url(blob.key, host: Rails.configuration.uploads_base_url) + 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) } diff --git a/engines/bops_uploads/lib/bops_uploads.rb b/engines/bops_uploads/lib/bops_uploads.rb index 658e16cdb4..79b5fdbc05 100644 --- a/engines/bops_uploads/lib/bops_uploads.rb +++ b/engines/bops_uploads/lib/bops_uploads.rb @@ -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")) diff --git a/engines/bops_uploads/lib/bops_uploads/engine.rb b/engines/bops_uploads/lib/bops_uploads/engine.rb index b97e1c7e6e..130ff2e2f6 100644 --- a/engines/bops_uploads/lib/bops_uploads/engine.rb +++ b/engines/bops_uploads/lib/bops_uploads/engine.rb @@ -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 diff --git a/engines/bops_uploads/spec/bops_uploads_helper.rb b/engines/bops_uploads/spec/bops_uploads_helper.rb index 32b68168e0..2329421d64 100644 --- a/engines/bops_uploads/spec/bops_uploads_helper.rb +++ b/engines/bops_uploads/spec/bops_uploads_helper.rb @@ -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 diff --git a/engines/bops_uploads/spec/helpers/url_helpers_spec.rb b/engines/bops_uploads/spec/helpers/url_helpers_spec.rb new file mode 100644 index 0000000000..086cb05ffd --- /dev/null +++ b/engines/bops_uploads/spec/helpers/url_helpers_spec.rb @@ -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 diff --git a/engines/bops_uploads/spec/requests/files_spec.rb b/engines/bops_uploads/spec/requests/files_spec.rb index a78d4e2fd2..26e8b5319d 100644 --- a/engines/bops_uploads/spec/requests/files_spec.rb +++ b/engines/bops_uploads/spec/requests/files_spec.rb @@ -3,36 +3,110 @@ require "bops_uploads_helper" RSpec.describe "Downloading files", show_exceptions: true do - context "when a blob exists" do - let(:document) { create(:document) } + context "on the uploads subdomain" do + before do + host!("uploads.bops.services") + end + + context "when a blob exists" do + let(:document) { create(:document) } + let(:blob) { document.file } + let(:key) { blob.key } + let(:path) { blob.service.path_for(blob.key) } + + it "returns 200 OK" do + get "/#{key}" + expect(response).to have_http_status(:ok) + expect(response.content_type).to eq("image/png") + end + + context "but the file is missing" do + before do + File.unlink(path) + end + + it "returns 404 Not Found" do + get "/#{key}" + expect(response).to have_http_status(:not_found) + end + end + end + + context "when a blob doesn't exist" do + let(:key) { SecureRandom.base36(28) } + + it "returns 404 Not Found" do + get "/#{key}" + expect(response).to have_http_status(:not_found) + end + end + end + + context "on the local authority subdomain" do + let!(:local_authority) { create(:local_authority, :southwark) } + let!(:subdomain) { local_authority.subdomain } + + let(:document) { create(:document, planning_application: planning_application) } let(:blob) { document.file } let(:key) { blob.key } - let(:path) { blob.service.path_for(blob.key) } - it "returns 200 OK" do - get "/#{key}" - expect(response).to have_http_status(:ok) - expect(response.content_type).to eq("image/png") + before do + host!("#{subdomain}.bops.services") + end + + context "when a blob doesn't exist" do + let(:key) { SecureRandom.base36(28) } + + before do + get "/files/#{key}" + end + + it "returns 404 Not Found" do + expect(response).to have_http_status(:not_found) + end end - context "but the file is missing" do + context "when a blob exists but for another local authority" do + let(:other_authority) { create(:local_authority, :lambeth) } + let(:planning_application) { create(:planning_application, local_authority: other_authority) } + before do - File.unlink(path) + get "/files/#{key}" end it "returns 404 Not Found" do - get "/#{key}" expect(response).to have_http_status(:not_found) end end - end - context "when a blob doesn't exist" do - let(:key) { SecureRandom.base36(28) } + context "when a blob exists for the local authority" do + let(:planning_application) { create(:planning_application, local_authority: local_authority) } + + before do + get "/files/#{key}" + end + + it "returns 302 Found" do + expect(response).to have_http_status(:found) + expect(response).to redirect_to("http://southwark.bops.services/blobs/#{key}") + end + + it "sets signed cookies" do + expect(cookies["CloudFront-Expires"]).to be_present + expect(cookies["CloudFront-Key-Pair-Id"]).to be_present + expect(cookies["CloudFront-Signature"]).to be_present + end - it "returns 404 Not Found" do - get "/#{key}" - expect(response).to have_http_status(:not_found) + context "and the redirect is followed" do + before do + follow_redirect! + end + + it "returns 200 OK" do + expect(response).to have_http_status(:ok) + expect(response.content_type).to eq("image/png") + end + end end end end diff --git a/lib/constraints/subdomain.rb b/lib/constraints/subdomain.rb deleted file mode 100644 index 5fa81308f0..0000000000 --- a/lib/constraints/subdomain.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -module Constraints - class LocalAuthoritySubdomain - class << self - def matches?(request) - LocalAuthority.pluck(:subdomain).include?(request.subdomain) - end - end - end - - class ConfigSubdomain - class << self - def matches?(request) - request.subdomain == "config" - end - end - end - - class DeviseSubdomain - class << self - def matches?(request) - Constraints::ConfigSubdomain.matches?(request) || Constraints::LocalAuthoritySubdomain.matches?(request) - end - end - end - - class UploadsSubdomain - class << self - def matches?(request) - request.subdomain == "uploads" - end - end - end -end diff --git a/spec/requests/api/replacement_document_validation_request_spec.rb b/spec/requests/api/replacement_document_validation_request_spec.rb index bab5051609..4a1ee1c657 100644 --- a/spec/requests/api/replacement_document_validation_request_spec.rb +++ b/spec/requests/api/replacement_document_validation_request_spec.rb @@ -6,7 +6,7 @@ let!(:default_local_authority) { create(:local_authority, :default) } let!(:api_user) { create(:api_user, local_authority: default_local_authority) } let!(:planning_application) { create(:planning_application, :invalidated, local_authority: default_local_authority) } - let(:old_document) { create(:document) } + let(:old_document) { create(:document, planning_application:) } let!(:replacement_document_validation_request) do create( @@ -20,8 +20,8 @@ let(:token) { "Bearer #{api_user.token}" } describe "#index" do - let(:old_document2) { create(:document) } - let(:old_document3) { create(:document) } + let(:old_document2) { create(:document, planning_application:) } + let(:old_document3) { create(:document, planning_application:) } let(:path) do api_v1_planning_application_replacement_document_validation_requests_path( @@ -50,7 +50,7 @@ ) end - let!(:new_document) { create(:document, owner: replacement_document_validation_request2) } + let!(:new_document) { create(:document, planning_application:, owner: replacement_document_validation_request2) } context "when the request is valid" do it "is successful" do diff --git a/spec/system/documents/display_and_publish_documents_spec.rb b/spec/system/documents/display_and_publish_documents_spec.rb index a41c3b0afe..36d4c60fe6 100644 --- a/spec/system/documents/display_and_publish_documents_spec.rb +++ b/spec/system/documents/display_and_publish_documents_spec.rb @@ -117,8 +117,8 @@ click_button "Save" - expect(planning_application.documents.for_display.count).to eq(1) - expect(planning_application.documents.for_publication.count).to eq(0) + expect(planning_application.documents.active.for_display.count).to eq(1) + expect(planning_application.documents.active.for_publication.count).to eq(0) end it "Assessor is able to publish documents without adding to the decision notice" do @@ -137,8 +137,8 @@ click_button "Save" - expect(planning_application.documents.for_display.count).to eq(0) - expect(planning_application.documents.for_publication.count).to eq(1) + expect(planning_application.documents.active.for_display.count).to eq(0) + expect(planning_application.documents.active.for_publication.count).to eq(1) end it "Error message is shown if document referenced is true without document number" do