Skip to content

Commit

Permalink
Add CSP headers to unauthenticated controller
Browse files Browse the repository at this point in the history
  • Loading branch information
paulomarg committed Jul 18, 2022
1 parent 692fcee commit 90840f9
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Unreleased
----------

* Set the appropriate CSP `frame-ancestor` directive in controllers using the `EmbeddedApp` concern. [#1474](https://github.com/Shopify/shopify_app/pull/1474)

20.0.2 (July 7, 2022)
----------

Expand Down
1 change: 1 addition & 0 deletions lib/shopify_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def self.use_webpacker?
# controller concerns
require "shopify_app/controller_concerns/csrf_protection"
require "shopify_app/controller_concerns/localization"
require "shopify_app/controller_concerns/frame_ancestors"
require "shopify_app/controller_concerns/itp"
require "shopify_app/controller_concerns/login_protection"
require "shopify_app/controller_concerns/ensure_billing"
Expand Down
2 changes: 2 additions & 0 deletions lib/shopify_app/controller_concerns/embedded_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ module ShopifyApp
module EmbeddedApp
extend ActiveSupport::Concern

include ShopifyApp::FrameAncestors

included do
if ShopifyApp.configuration.embedded_app?
after_action(:set_esdk_headers)
Expand Down
16 changes: 16 additions & 0 deletions lib/shopify_app/controller_concerns/frame_ancestors.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

module ShopifyApp
module FrameAncestors
extend ActiveSupport::Concern

included do
content_security_policy do |policy|
policy.frame_ancestors(-> do
domain_host = current_shopify_domain || "*.myshopify.com"

This comment has been minimized.

Copy link
@hrdwdmrbl

hrdwdmrbl Nov 30, 2022

For an embedded app, because current_shopify_domain will throw an exception if a JWT token isn't found, this means that this will never be nil and that this line of code will cause an exception.

It's is fine when an unauthenticated page of the app is loaded, because then this code won't ever run.

But if a user tries to visit a different page of the app directly (example.myshopify.com/admin/apps/my_app/foo), then this will blow them up. The app will not redirect to load the skeleton and then continue. The user will just encounter a seemingly broken app.

"https://#{domain_host} https://admin.shopify.com;"
end)
end
end
end
end

0 comments on commit 90840f9

Please sign in to comment.