Skip to content

Commit

Permalink
Add CKAN_DOMAIN to the CSP on previews
Browse files Browse the repository at this point in the history
Some organograms are hosted on ckan.publishing.service.gov.uk, instead
of s3-eu-west-1.amazonaws.com.

The previews for these organograms are currently broken because the
content security policy prevents the JavaScript from dowloading the
files from the CKAN domain.

I confirmed that the CSP was the only issue by disabling CSP in my
brower (using a plugin) and confirming that the broken previews worked
correctly.

Since the CSP already permits all of S3 eu-west-1 in the connect_src,
adding CKAN to the CSP feels like a very small piece of extra security
attack surface. And it should be a quick way to fix the bug where some
organogram previews don't show up.

I tried to add a test for this, but rails controller tests don't execute
enough of the stack for the SecureHeaders gem to do its thing and set
the CSP header. It might be possible to test with a feature test, but
that feels like overkill.
  • Loading branch information
richardTowers committed Aug 6, 2021
1 parent 9e5e48b commit 3c5475e
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion app/controllers/previews_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class PreviewsController < ApplicationController
def show
append_content_security_policy_directives(
connect_src: %w[s3-eu-west-1.amazonaws.com],
connect_src: ["s3-eu-west-1.amazonaws.com", ENV["CKAN_DOMAIN"]].compact,
)

@dataset = Dataset.get_by_uuid(uuid: params[:dataset_uuid])
Expand Down

0 comments on commit 3c5475e

Please sign in to comment.