From caf27a4822e5ab53cbd72b51ddaee73fd45afa15 Mon Sep 17 00:00:00 2001
From: Aditya Mattos <aditya.mattos@shopify.com>
Date: Tue, 2 Jun 2020 11:52:36 +0530
Subject: [PATCH] Skip CSRF protection if a valid session token is present

---
 .../concerns/shopify_app/authenticated.rb     |  1 +
 lib/shopify_app.rb                            |  1 +
 .../controller_concerns/csrf_protection.rb    | 22 +++++++
 .../concerns/authenticated_test.rb            |  1 +
 .../csrf_protection_test.rb                   | 62 +++++++++++++++++++
 5 files changed, 87 insertions(+)
 create mode 100644 lib/shopify_app/controller_concerns/csrf_protection.rb
 create mode 100644 test/shopify_app/controller_concerns/csrf_protection_test.rb

diff --git a/app/controllers/concerns/shopify_app/authenticated.rb b/app/controllers/concerns/shopify_app/authenticated.rb
index 45542a9dc..7f4e4a62b 100644
--- a/app/controllers/concerns/shopify_app/authenticated.rb
+++ b/app/controllers/concerns/shopify_app/authenticated.rb
@@ -7,6 +7,7 @@ module Authenticated
     included do
       include ShopifyApp::Localization
       include ShopifyApp::LoginProtection
+      include ShopifyApp::CsrfProtection
       include ShopifyApp::EmbeddedApp
       before_action :login_again_if_different_user_or_shop
       around_action :activate_shopify_session
diff --git a/lib/shopify_app.rb b/lib/shopify_app.rb
index b4c7108b0..b9c66dca3 100644
--- a/lib/shopify_app.rb
+++ b/lib/shopify_app.rb
@@ -27,6 +27,7 @@ def self.use_webpacker?
   require 'shopify_app/utils'
 
   # controller concerns
+  require 'shopify_app/controller_concerns/csrf_protection'
   require 'shopify_app/controller_concerns/localization'
   require 'shopify_app/controller_concerns/itp'
   require 'shopify_app/controller_concerns/login_protection'
diff --git a/lib/shopify_app/controller_concerns/csrf_protection.rb b/lib/shopify_app/controller_concerns/csrf_protection.rb
new file mode 100644
index 000000000..c2ec806d0
--- /dev/null
+++ b/lib/shopify_app/controller_concerns/csrf_protection.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+module ShopifyApp
+  module CsrfProtection
+    extend ActiveSupport::Concern
+
+    MissingIncludeError = Class.new(StandardError)
+
+    included do
+      unless ancestors.include?(ShopifyApp::LoginProtection)
+        raise(MissingIncludeError, 'You must include ShopifyApp::LoginProtection before including this module.')
+      end
+
+      protect_from_forgery with: :exception, unless: :valid_session_token?
+    end
+
+    private
+
+    def valid_session_token?
+      jwt_shopify_domain.present?
+    end
+  end
+end
diff --git a/test/controllers/concerns/authenticated_test.rb b/test/controllers/concerns/authenticated_test.rb
index f0f2b7b58..503b7ea42 100644
--- a/test/controllers/concerns/authenticated_test.rb
+++ b/test/controllers/concerns/authenticated_test.rb
@@ -13,6 +13,7 @@ def index
   test "includes all the needed concerns" do
     AuthenticatedTestController.include?(ShopifyApp::Localization)
     AuthenticatedTestController.include?(ShopifyApp::LoginProtection)
+    AuthenticatedTestController.include?(ShopifyApp::CsrfProtection)
     AuthenticatedTestController.include?(ShopifyApp::EmbeddedApp)
   end
 end
diff --git a/test/shopify_app/controller_concerns/csrf_protection_test.rb b/test/shopify_app/controller_concerns/csrf_protection_test.rb
new file mode 100644
index 000000000..c8b292762
--- /dev/null
+++ b/test/shopify_app/controller_concerns/csrf_protection_test.rb
@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+class CsrfProtectionController < ActionController::Base
+  include ShopifyApp::LoginProtection
+  include ShopifyApp::CsrfProtection
+
+  def authenticity_token
+  render(json: { authenticity_token: form_authenticity_token })
+  end
+
+  def csrf_test
+    head(:ok)
+  end
+end
+
+class CsrfProtectionTest < ActionDispatch::IntegrationTest
+  setup do
+    @authenticity_protection = ActionController::Base.allow_forgery_protection
+    ActionController::Base.allow_forgery_protection = true
+    Rails.application.routes.draw do
+      get '/authenticity_token', to: 'csrf_protection#authenticity_token'
+      post '/csrf_protection_test', to: 'csrf_protection#csrf_test'
+    end
+  end
+
+  teardown do
+    ActionController::Base.allow_forgery_protection = @authenticity_protection
+    Rails.application.reload_routes!
+  end
+
+  test 'it raises an error if module is included without including ShopifyApp::LoginProtection first' do
+    error = assert_raises ShopifyApp::CsrfProtection::MissingIncludeError do
+      class Test
+        include ShopifyApp::CsrfProtection
+      end
+    end
+
+    assert_equal 'You must include ShopifyApp::LoginProtection before including this module.', error.message
+  end
+
+  test 'it raises an invalid authenticity token error if a valid session token or csrf token is not provided' do
+    assert_raises ActionController::InvalidAuthenticityToken do
+      post '/csrf_protection_test'
+    end
+  end
+
+  test 'it does not raise if a valid CSRF token was provided' do
+    get '/authenticity_token'
+
+    csrf_token = JSON.parse(response.body)['authenticity_token']
+
+    post '/csrf_protection_test', headers: { 'X-CSRF-Token': csrf_token }
+
+    assert_response :ok
+  end
+
+  test 'it does not raise if a valid session token was provided' do
+    post '/csrf_protection_test', env: { 'jwt.shopify_domain': "exampleshop.myshopify.com" }
+
+    assert_response :ok
+  end
+end