Skip to content

Commit

Permalink
Merge pull request #2467 from alphagov/fix-broken-asset-redirect
Browse files Browse the repository at this point in the history
Fix redirecting to incorrect assets domain
  • Loading branch information
kevindew authored Jun 21, 2022
2 parents 84f64c5 + aa3e27d commit 8be158d
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 24 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ end

group :test do
gem "capybara"
gem "climate_control"
gem "faker"
gem "i18n-coverage"
gem "minitest-reporters"
Expand Down
2 changes: 2 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ GEM
regexp_parser (~> 1.5)
xpath (~> 3.2)
childprocess (3.0.0)
climate_control (1.1.1)
coderay (1.1.3)
concurrent-ruby (1.1.10)
crack (0.4.3)
Expand Down Expand Up @@ -377,6 +378,7 @@ DEPENDENCIES
better_errors
binding_of_caller
capybara
climate_control
dalli
faker
gds-api-adapters
Expand Down
6 changes: 1 addition & 5 deletions app/controllers/asset_manager_redirect_controller.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
class AssetManagerRedirectController < ApplicationController
def show
asset_url = Plek.new.asset_root
if request.host.start_with?("draft-")
asset_url = Plek.new.external_url_for("draft-assets")
end

asset_url = Plek.new.external_url_for("assets")
expires_in 1.day, public: true
redirect_to host: URI.parse(asset_url).host, status: :moved_permanently
end
Expand Down
26 changes: 7 additions & 19 deletions test/controllers/asset_manager_redirect_controller_test.rb
Original file line number Diff line number Diff line change
@@ -1,36 +1,24 @@
require "test_helper"

class AssetManagerRedirectControllerTest < ActionController::TestCase
setup do
Plek.any_instance.stubs(:asset_root).returns("http://asset-host.com")
Plek.any_instance.stubs(:external_url_for).returns("http://draft-asset-host.com")
end

test "sets the cache-control max-age to 1 day" do
request.host = "some-host.com"
get :show, params: { path: "asset.txt" }

assert_equal @response.headers["Cache-Control"], "max-age=86400, public"
end

test "redirects asset requests made via public host to the public asset host" do
request.host = "some-host.com"
get :show, params: { path: "asset.txt" }

assert_redirected_to "http://asset-host.com/government/uploads/asset.txt"
end

test "redirects asset requests made via draft host to the draft asset host" do
request.host = "draft-some-host.com"
test "redirects asset requests to the assets hostname" do
get :show, params: { path: "asset.txt" }

assert_redirected_to "http://draft-asset-host.com/government/uploads/asset.txt"
assert_redirected_to "http://assets.test.gov.uk/government/uploads/asset.txt"
end

test "redirects asset preview requests made via public host to the public asset host" do
request.host = "some-host.com"
get :show, params: { path: "asset.csv/preview" }
test "redirects to assets hostname respect the draft stack" do
ClimateControl.modify PLEK_HOSTNAME_PREFIX: "draft-" do
get :show, params: { path: "asset.txt" }

assert_redirected_to "http://asset-host.com/government/uploads/asset.csv/preview"
assert_redirected_to "http://draft-assets.test.gov.uk/government/uploads/asset.txt"
end
end
end

0 comments on commit 8be158d

Please sign in to comment.