Merge branch 'main' into billing-embedded

.github/workflows/stale.yml

@@ -29,3 +29,4 @@ jobs:
           days-before-pr-close: -1
           repo-token: ${{ secrets.GITHUB_TOKEN }}
           exempt-issue-labels: "feature request"
+          close-issue-reason: "not_planned"

.spin/rails/prepare-application

@@ -0,0 +1,8 @@
+#!/bin/bash
+
+# This file exists to prevent the Spin infrastucture from infering that this is a Rails application:
+# https://github.com/Shopify/wave/issues/244
+
+set -ex
+
+bundle install

CHANGELOG.md

@@ -2,6 +2,10 @@ Unreleased
 ----------
 * Fixes a bug with `EnsureAuthenticatedLinks` causing deep links to not work [#1549](https://github.com/Shopify/shopify_app/pull/1549)
 * Ensure online token is properly used when using `current_shopify_session` [#1566](https://github.com/Shopify/shopify_app/pull/1566)
+* Added debug logs, you can read more about logging (here)[./docs/logging.md]. [#1545](https://github.com/Shopify/shopify_app/pull/1545)
+* Emit a deprecation notice for wrongly-rescued exceptions [#1530](https://github.com/Shopify/shopify_app/pull/1530)
+* Log a deprecation warning for the use of incompatible controller concerns [#1560](https://github.com/Shopify/shopify_app/pull/1560)
+* Fixes bug with expired sessions for embedded apps returning a 500 instead of 401 [#1580](https://github.com/Shopify/shopify_app/pull/1580)
 * Fixes a bug with `EnsureBilling` causing infinite redirect in embedded apps [#1578](https://github.com/Shopify/shopify_app/pull/1578)
 
 21.2.0 (Oct 25, 2022)

README.md

@@ -37,22 +37,22 @@ This gem requires that you have the following credentials:
 rails new my_shopify_app
 ```
 
-2. Add the Shopify App gem to `my_shopify_app`'s Gemfile.
+2. Add the Shopify App gem to the app's Gemfile:
 
 ```sh
 bundle add shopify_app
 ```
 
-3. Create a `.env` file in the root of `my_shopify_app` to specify your full host and Shopify API credentials:
+3. You will need to provide several environment variables to the app.
+There are a variety of way of doing this, but for a development environment we recommended the [`dotenv-rails`](https://github.com/bkeepers/dotenv) gem.
+Create a `.env` file in the root of your Rails app to specify the full host and Shopify API credentials:
 
 ```sh
 HOST=http://localhost:3000
 SHOPIFY_API_KEY=<Your Shopify API key>
 SHOPIFY_API_SECRET=<Your Shopify API secret>
 ```
 
-> In a development environment, you can use a gem like `dotenv-rails` to manage environment variables.
-
 4. Run the default Shopify App generator to create an app that can be embedded in the Shopify Admin:
 
 ```sh
@@ -71,9 +71,14 @@ rails db:migrate
 rails server
 ```
 
-7. Install the app by visiting the server's URL (e.g. http://127.0.0.1:3000) and specifying the subdomain of the shop where you want it to be installed to.
+7. Within [Shopify Partners](https://www.shopify.com/partners), navigate to your App, then App Setup, and configure the URLs, e.g.:
+
+  * App URL: http://locahost:3000/
+  * Allowed redirection URL(s): http://localhost:3000/auth/shopify/callback
+
+8. Install the app by visiting the server's URL (e.g. http://localhost:3000) and specifying the subdomain of the shop where you want it to be installed to.
 
-8. After the app is installed, you're redirected to the embedded app.
+9. After the app is installed, you're redirected to the embedded app.
 
 This app implements [OAuth 2.0](https://shopify.dev/tutorials/authenticate-with-oauth) with Shopify to authenticate requests made to Shopify APIs. By default, this app is configured to use [session tokens](https://shopify.dev/concepts/apps/building-embedded-apps-using-session-tokens) to authenticate merchants when embedded in the Shopify Admin.
 
@@ -107,6 +112,7 @@ You can find documentation on gem usage, concepts, mixins, installation, and mor
   * [Testing](/docs/shopify_app/testing.md)
   * [Webhooks](/docs/shopify_app/webhooks.md)
   * [Content Security Policy](/docs/shopify_app/content-security-policy.md)
+  * [Logging](/docs/shopify_app/logging.md)
 
 ### Engine
 

app/controllers/concerns/shopify_app/ensure_authenticated_links.rb

@@ -28,8 +28,8 @@ def splash_page_with_params(params)
     def redirect_to_splash_page
       redirect_to(splash_page)
     rescue ::ShopifyApp::ShopifyDomainNotFound => error
-      Rails.logger.warn("[ShopifyApp::EnsureAuthenticatedLinks] Redirecting to login: [#{error.class}] "\
-        "Could not determine current shop domain")
+      ShopifyApp::Logger.warn("Redirecting to login: [#{error.class}]"\
+        " Could not determine current shop domain")
       redirect_to(ShopifyApp.configuration.login_url)
     end
 

app/controllers/concerns/shopify_app/require_known_shop.rb

@@ -6,6 +6,14 @@ module RequireKnownShop
     include ShopifyApp::RedirectForEmbedded
 
     included do
+      if ancestors.include?(ShopifyApp::LoginProtection)
+        ActiveSupport::Deprecation.warn(<<~EOS)
+          We detected the use of incompatible concerns (RequireKnownShop and LoginProtection) in #{name},
+          which may lead to unpredictable behavior. In a future release of this library this will raise an error.
+        EOS
+
+      end
+
       before_action :check_shop_domain
       before_action :check_shop_known
     end

app/controllers/shopify_app/callback_controller.rb

@@ -17,7 +17,14 @@ def callback
           },
           auth_query: ShopifyAPI::Auth::Oauth::AuthQuery.new(**filtered_params),
         )
-      rescue
+      rescue => e
+        unless e.class.module_parent == ShopifyAPI::Errors
+          ActiveSupport::Deprecation.warn(<<~EOS)
+            An error of type #{e.class} was rescued. This is not part of `ShopifyAPI::Errors`, which could indicate a
+            bug in your app, or a bug in the shopify_app gem. Future versions of the gem may re-raise this error rather
+            than rescuing it.
+          EOS
+        end
         return respond_with_error
       end
 
@@ -28,7 +35,10 @@ def callback
         value: auth_result[:cookie].value,
       }
 
-      session[:shopify_user_id] = auth_result[:session].associated_user.id if auth_result[:session].online?
+      if auth_result[:session].online?
+        session[:shopify_user_id] = auth_result[:session].associated_user.id
+        ShopifyApp::Logger.debug("Saving Shopify user ID to cookie")
+      end
 
       if start_user_token_flow?(auth_result[:session])
         return respond_with_user_token_flow

app/controllers/shopify_app/extension_verification_controller.rb

@@ -9,7 +9,10 @@ class ExtensionVerificationController < ActionController::Base
     private
 
     def verify_request
-      head(:unauthorized) unless hmac_valid?(request.body.read)
+      unless hmac_valid?(request.body.read)
+        head(:unauthorized)
+        ShopifyApp::Logger.debug("Extension verification failed due to invalid HMAC")
+      end
     end
   end
 end

app/controllers/shopify_app/sessions_controller.rb

@@ -27,6 +27,8 @@ def top_level_interaction
     def destroy
       reset_session
       flash[:notice] = I18n.t(".logged_out")
+      ShopifyApp::Logger.debug("Session destroyed")
+      ShopifyApp::Logger.debug("Redirecting to #{login_url_with_optional_shop}")
       redirect_to(login_url_with_optional_shop)
     end
 
@@ -38,6 +40,7 @@ def authenticate
       copy_return_to_param_to_session
 
       if embedded_redirect_url?
+        ShopifyApp::Logger.debug("Embedded URL within / authenticate")
         if embedded_param?
           redirect_for_embedded
         else
@@ -52,6 +55,7 @@ def authenticate
 
     def start_oauth
       callback_url = ShopifyApp.configuration.login_callback_url.gsub(%r{^/}, "")
+      ShopifyApp::Logger.debug("Starting OAuth with the following callback URL: #{callback_url}")
 
       auth_attributes = ShopifyAPI::Auth::Oauth.begin_auth(
         shop: sanitized_shop_name,
@@ -65,7 +69,10 @@ def start_oauth
         value: auth_attributes[:cookie].value,
       }
 
-      redirect_to(auth_attributes[:auth_route], allow_other_host: true)
+      auth_route = auth_attributes[:auth_route]
+
+      ShopifyApp::Logger.debug("Redirecting to auth_route - #{auth_route}")
+      redirect_to(auth_route, allow_other_host: true)
     end
 
     def validate_shop_presence
@@ -94,7 +101,9 @@ def top_level?
     end
 
     def redirect_auth_to_top_level
-      fullpage_redirect_to(login_url_with_optional_shop(top_level: true))
+      url = login_url_with_optional_shop(top_level: true)
+      ShopifyApp::Logger.debug("Redirecting to top level - #{url}")
+      fullpage_redirect_to(url)
     end
   end
 end

docs/Upgrading.md

@@ -6,6 +6,8 @@ This file documents important changes needed to upgrade your app's Shopify App v
 
 [General Advice](#general-advice)
 
+[Upgrading to `v20.3.0`](#upgrading-to-v2030)
+
 [Upgrading to `v20.2.0`](#upgrading-to-v2020)
 
 [Upgrading to `v20.1.0`](#upgrading-to-v2010)
@@ -34,6 +36,9 @@ We also recommend the use of a staging site which matches your production enviro
 
 If you do run into issues, we recommend looking at our [debugging tips.](https://github.com/Shopify/shopify_app/blob/main/docs/Troubleshooting.md#debugging-tips)
 
+## Upgrading to `v20.3.0`
+Calling `LoginProtection#current_shopify_domain` will no longer raise an error if there is no active session. It will now return a nil value. The internal behavior of raising an error on OAuth redirect is still in place, however. If you were calling `current_shopify_domain` in authenticated actions and expecting an error if nil, you'll need to do a presence check and raise that error within your app.
+
 ## Upgrading to `v20.2.0`
 
 All custom errors defined inline within the `ShopifyApp` gem have been moved to `lib/shopify_app/errors.rb`.

docs/shopify_app/logging.md

@@ -0,0 +1,21 @@
+# Logging
+
+## Log Levels
+
+There are four log levels with `error` being the most severe.
+
+1. Debug
+2. Info
+3. Warn
+4. Error
+
+## Configuration
+
+The logging is controlled by the `log_level` configuration setting.
+The default log level is `:info`.
+You can disable all logs by setting this to `:off`.
+
+## Upgrading
+
+For a newly-generated app, the `shopify_app` initializer will contain the `log_level` setting.
+If you are upgrading from a previous version of the `shopify_app` gem then you will need to add this manually, otherwise it will default to `:info`.

lib/generators/shopify_app/add_marketing_activity_extension/templates/marketing_activities_controller.rb

@@ -55,7 +55,8 @@ def errors
     request_id = params[:request_id]
     message = params[:message]
 
-    Rails.logger.info("[Marketing Activity App Error Feedback] Request id: #{request_id}, message: #{message}")
+    ShopifyApp::Logger.info("[Marketing Activity App Error Feedback]"\
+      "Request id: #{request_id}, message: #{message}")
 
     render(json: {}, status: :ok)
   end

lib/generators/shopify_app/home_controller/templates/index.html.erb

@@ -24,7 +24,7 @@
           });
 
           var fetchProducts = function() {
-            var headers = new Headers({ "Authorization": "Bearer " + window.sessionToken });
+            var headers = new Headers({ "Content-Type": "text/javascript", "Authorization": "Bearer " + window.sessionToken });
             return fetch("/products", { headers })
               .then(response => response.json())
               .then(data => {

lib/generators/shopify_app/install/templates/shopify_app.rb.tt

@@ -7,7 +7,7 @@ ShopifyApp.configure do |config|
   config.after_authenticate_job = false
   config.api_version = "<%= @api_version %>"
   config.shop_session_repository = 'Shop'
-
+  config.log_level = :info
   config.reauth_on_access_scope_changes = true
 
   config.api_key = ENV.fetch('SHOPIFY_API_KEY', '').presence

lib/generators/shopify_app/rotate_shopify_token_job/templates/rotate_shopify_token_job.rb

@@ -27,7 +27,7 @@ def perform(params)
     private
 
     def log_error(message)
-      Rails.logger.error(message)
+      ShopifyApp::Logger.error(message)
     end
 
     def no_access_token_error_message

lib/shopify_app.rb

@@ -37,6 +37,8 @@ def self.use_webpacker?
   # errors
   require "shopify_app/errors"
 
+  require "shopify_app/logger"
+
   # controller concerns
   require "shopify_app/controller_concerns/csrf_protection"
   require "shopify_app/controller_concerns/localization"

lib/shopify_app/configuration.rb

@@ -20,6 +20,7 @@ class Configuration
     attr_accessor :api_version
 
     attr_accessor :reauth_on_access_scope_changes
+    attr_accessor :log_level
 
     # customise urls
     attr_accessor :root_url

lib/shopify_app/controller_concerns/ensure_billing.rb

@@ -28,6 +28,7 @@ def check_billing(session = current_shopify_session)
       unless has_payment
         if request.xhr?
           add_top_level_redirection_headers(url: confirmation_url, ignore_response_code: true)
+          ShopifyApp::Logger.debug("Responding with 401 unauthorized")
           head(:unauthorized)
         elsif ShopifyApp.configuration.embedded_app?
           fullpage_redirect_to(confirmation_url)
@@ -57,6 +58,7 @@ def has_active_payment?(session)
     end
 
     def has_subscription?(session)
+      ShopifyApp::Logger.debug("Checking if shop has subscription")
       response = run_query(session: session, query: RECURRING_PURCHASES_QUERY)
       subscriptions = response.body["data"]["currentAppInstallation"]["activeSubscriptions"]
 
@@ -72,6 +74,7 @@ def has_subscription?(session)
     end
 
     def has_one_time_payment?(session)
+      ShopifyApp::Logger.debug("Checking if has one time payment")
       purchases = nil
       end_cursor = nil