Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renaming some Controller Concerns #1565

Closed
wants to merge 62 commits into from
Closed
Show file tree
Hide file tree
Changes from 61 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
57fbed5
first set of logs
klenotiw Oct 21, 2022
d1d0a05
webhooks manager logs added
klenotiw Oct 21, 2022
d4d3946
adding ShopifyApp before all errors
klenotiw Oct 21, 2022
df4947b
missed these
klenotiw Oct 21, 2022
f71f823
Merge branch 'main' into klenotiw/add-log-levels
klenotiw Oct 21, 2022
45ee7e9
rubocop
klenotiw Oct 21, 2022
3ae6a87
getting this weird error from adding the logs
klenotiw Oct 21, 2022
774dbb5
this was the line that was causing products not to load
klenotiw Oct 21, 2022
1de873b
add more login protection logs
klenotiw Oct 21, 2022
69479ac
change error msg
klenotiw Oct 24, 2022
a305805
add two logger functions to the utils module and add log levels to th…
klenotiw Oct 28, 2022
f84172c
didn't mean to add this
klenotiw Oct 28, 2022
5e38b94
Merge branch 'main' into klenotiw/add-log-levels
klenotiw Oct 28, 2022
f4b8842
add log for andy
klenotiw Oct 28, 2022
69e0f04
Use standard logger
andyw8 Oct 28, 2022
d5092db
changing rails logger to utils logger
klenotiw Oct 28, 2022
11049a0
More logging
andyw8 Oct 28, 2022
5acf291
Merge branch 'klenotiw/add-log-levels' of https://github.com/Shopify/…
klenotiw Oct 31, 2022
29bd368
levels should layer
klenotiw Oct 31, 2022
283e143
add warn and error
klenotiw Nov 1, 2022
bbd0efd
fix comparison
klenotiw Nov 1, 2022
3bd2e3d
default should be info
klenotiw Nov 1, 2022
8b7b67b
change and pull prefix out
klenotiw Nov 1, 2022
36b3ed4
move everything over to new module
klenotiw Nov 1, 2022
5a52b53
messed up syntax
klenotiw Nov 1, 2022
5886ca1
actually fix comparisions
klenotiw Nov 1, 2022
349adf6
rubocop
klenotiw Nov 1, 2022
ad3b784
more logs and update old ones
klenotiw Nov 2, 2022
6f704a3
refactor logger
klenotiw Nov 2, 2022
ae2bad1
add log and remove pry
klenotiw Nov 2, 2022
c4fee42
session repository
klenotiw Nov 2, 2022
84c92fb
change any rails logs to our logger
klenotiw Nov 2, 2022
1a072a9
rubocop
klenotiw Nov 2, 2022
cc47406
remove time and format log_level
klenotiw Nov 3, 2022
6222053
rename two controller concern
klenotiw Nov 3, 2022
0d1c795
Merge remote-tracking branch 'origin/klenotiw/add-log-levels' into kl…
klenotiw Nov 3, 2022
7235f2d
add log levels to readme
klenotiw Nov 4, 2022
2ead6bd
add deprecation to logs
klenotiw Nov 4, 2022
09f4e92
Update README.md
klenotiw Nov 7, 2022
02333fe
Update docs/shopify_app/logging.md
klenotiw Nov 7, 2022
5355a16
move logger out of utils and into its own file
klenotiw Nov 8, 2022
8670e87
Merge branch 'klenotiw/add-log-levels' of https://github.com/Shopify/…
klenotiw Nov 8, 2022
c5fec08
fix log to be under if
klenotiw Nov 8, 2022
776a11f
Update docs/shopify_app/logging.md
klenotiw Nov 8, 2022
3eb2eb4
Update lib/generators/shopify_app/install/templates/shopify_app.rb.tt
klenotiw Nov 8, 2022
3a2fee7
change namespaces
klenotiw Nov 8, 2022
07a703a
handled errors will be under info
klenotiw Nov 8, 2022
8661bbf
pull out logic from logs and into a variable
klenotiw Nov 8, 2022
55ad138
require logger file
klenotiw Nov 8, 2022
25bbb15
change some logs
klenotiw Nov 8, 2022
25d81cc
Merge branch 'klenotiw/add-log-levels' of https://github.com/Shopify/…
klenotiw Nov 9, 2022
482751e
Merge branch 'main' into klenotiw/add-log-levels
klenotiw Nov 9, 2022
dccc327
fix dup message
klenotiw Nov 9, 2022
38d8a74
fix enabled_for_log_level
klenotiw Nov 9, 2022
5fe3abd
add configuration heading
klenotiw Nov 9, 2022
003b38a
add logs to has payment conditionals
klenotiw Nov 9, 2022
68ee4f9
change log
klenotiw Nov 9, 2022
ab72180
fix tests and lint
klenotiw Nov 9, 2022
5fcaa00
Merge branch 'klenotiw/add-log-levels' into klenotiw/renaming-control…
klenotiw Nov 9, 2022
268687a
use activesupport for deprecation
klenotiw Nov 9, 2022
d273e76
require_know_shop has methods so including/extending doesn't hold all…
klenotiw Nov 9, 2022
728fbb7
fixing include issue
klenotiw Nov 14, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,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

Expand Down
11 changes: 2 additions & 9 deletions app/controllers/concerns/shopify_app/authenticated.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,11 @@

module ShopifyApp
module Authenticated
extend ShopifyApp::EnsureHasSession
extend ActiveSupport::Concern

included do
include ShopifyApp::Localization
include ShopifyApp::LoginProtection
include ShopifyApp::CsrfProtection
include ShopifyApp::EmbeddedApp
include ShopifyApp::EnsureBilling

before_action :login_again_if_different_user_or_shop
around_action :activate_shopify_session
after_action :add_top_level_redirection_headers
ShopifyApp::Logger.deprecated("Authenticated has been renamed to EnsureHasSession")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ShopifyApp::Logger.deprecated("Authenticated has been renamed to EnsureHasSession")
ShopifyApp::Logger.deprecated("Authenticated has been renamed to EnsureHasSession")

I would say 'replaced by' rather than renamed to here, since if someone is upgrading from an older version then there may also be behavior changes.

end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -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("[ShopifyApp::EnsureAuthenticatedLinks] Redirecting to login: [#{error.class}]"\
" Could not determine current shop domain")
redirect_to(ShopifyApp.configuration.login_url)
end

Expand Down
20 changes: 20 additions & 0 deletions app/controllers/concerns/shopify_app/ensure_has_session.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# frozen_string_literal: true

module ShopifyApp
module EnsureHasSession
extend ActiveSupport::Concern

included do
include ShopifyApp::Localization
include ShopifyApp::LoginProtection
include ShopifyApp::CsrfProtection
include ShopifyApp::EmbeddedApp
include ShopifyApp::EnsureBilling

before_action :login_again_if_different_user_or_shop
around_action :activate_shopify_session
after_action :add_top_level_redirection_headers
end
end
end

48 changes: 48 additions & 0 deletions app/controllers/concerns/shopify_app/ensure_installed.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# frozen_string_literal: true

module ShopifyApp
module EnsureInstalled
extend ActiveSupport::Concern
include ShopifyApp::RedirectForEmbedded

included do
before_action :check_shop_domain
before_action :check_shop_known
end

def current_shopify_domain
return if params[:shop].blank?

@shopify_domain ||= ShopifyApp::Utils.sanitize_shop_domain(params[:shop])
end

private

def check_shop_domain
redirect_to(ShopifyApp.configuration.login_url) unless current_shopify_domain
end

def check_shop_known
@shop = SessionRepository.retrieve_shop_session_by_shopify_domain(current_shopify_domain)
unless @shop
if embedded_param?
redirect_for_embedded
else
redirect_to(shop_login)
end
end
end

def shop_login
url = URI(ShopifyApp.configuration.login_url)

url.query = URI.encode_www_form(
shop: params[:shop],
host: params[:host],
return_to: request.fullpath,
)

url.to_s
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module RequireKnownShop
include ShopifyApp::RedirectForEmbedded

included do
# ShopifyApp::Logger.deprecated("RequireKnownShop has been renamed to EnsureInstalled")
before_action :check_shop_domain
before_action :check_shop_known
end
Expand Down
5 changes: 4 additions & 1 deletion app/controllers/shopify_app/callback_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,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("Setting :shopify_user_id to Rails cookie")
end

if start_user_token_flow?(auth_result[:session])
return respond_with_user_token_flow
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 - invalid HMAC")
end
end
end
end
8 changes: 8 additions & 0 deletions app/controllers/shopify_app/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -38,12 +40,15 @@ def authenticate
copy_return_to_param_to_session

if embedded_redirect_url?
ShopifyApp::Logger.debug("Embedded URL within / authenticate")
if embedded_param?
ShopifyApp::Logger.debug("Embedded param")
redirect_for_embedded
else
start_oauth
end
elsif top_level?
ShopifyApp::Logger.debug("Top level redirect")
start_oauth
else
redirect_auth_to_top_level
Expand All @@ -52,6 +57,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,
Expand All @@ -65,6 +71,7 @@ def start_oauth
value: auth_attributes[:cookie].value,
}

ShopifyApp::Logger.debug("Redirecting to auth_route - #{auth_attributes[:auth_route]}")
redirect_to(auth_attributes[:auth_route], allow_other_host: true)
end

Expand Down Expand Up @@ -94,6 +101,7 @@ def top_level?
end

def redirect_auth_to_top_level
ShopifyApp::Logger.debug("Redirecting to top level - #{login_url_with_optional_shop(top_level: true)}")
fullpage_redirect_to(login_url_with_optional_shop(top_level: true))
end
end
Expand Down
14 changes: 14 additions & 0 deletions docs/shopify_app/logging.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Logging

## Log Levels

1. Debug
2. Info
3. Warn
4. Error

We have four log levels with `error` being the most severe.

## Configuration

You can configure your log level by changing your `SHOPIFY_LOG_LEVEL` environment variable. If `SHOPIFY_LOG_LEVEL` is not set the configuration file with default to `info`. To turn off all shopify_app logs you can change this environment variable to `off`.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ ShopifyApp.configure do |config|
config.api_key = ENV.fetch('SHOPIFY_API_KEY', '').presence
config.secret = ENV.fetch('SHOPIFY_API_SECRET', '').presence

config.log_level = ENV["SHOPIFY_LOG_LEVEL"]&.to_sym || :info

# You may want to charge merchants for using your app. Setting the billing configuration will cause the Authenticated
# controller concern to check that the session is for a merchant that has an active one-time payment or subscription.
# If no payment is found, it starts off the process and sends the merchant to a confirmation URL so that they can
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions lib/shopify_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ def self.use_webpacker?
# errors
require "shopify_app/errors"

# logger
require "shopify_app/logger"

# controller concerns
require "shopify_app/controller_concerns/csrf_protection"
require "shopify_app/controller_concerns/localization"
Expand Down
1 change: 1 addition & 0 deletions lib/shopify_app/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions lib/shopify_app/controller_concerns/ensure_billing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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("Setting 401 from EnsureBilling")
head(:unauthorized)
else
redirect_to(confirmation_url, allow_other_host: true)
Expand Down Expand Up @@ -55,6 +56,7 @@ def has_active_payment?(session)
end

def has_subscription?(session)
ShopifyApp::Logger.debug("Has Subscription")
response = run_query(session: session, query: RECURRING_PURCHASES_QUERY)
subscriptions = response.body["data"]["currentAppInstallation"]["activeSubscriptions"]

Expand All @@ -70,6 +72,7 @@ def has_subscription?(session)
end

def has_one_time_payment?(session)
ShopifyApp::Logger.debug("Has One Time Payment")
purchases = nil
end_cursor = nil

Expand Down Expand Up @@ -163,6 +166,7 @@ def recurring?
def run_query(session:, query:, variables: nil)
client = ShopifyAPI::Clients::Graphql::Admin.new(session: session)

ShopifyApp::Logger.debug("Client query - Query: #{query}, Variables: #{variables} ")
response = client.query(query: query, variables: variables)

raise BillingError.new("Error while billing the store", []) unless response.ok?
Expand Down
14 changes: 14 additions & 0 deletions lib/shopify_app/controller_concerns/login_protection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@ module LoginProtection
ACCESS_TOKEN_REQUIRED_HEADER = "X-Shopify-API-Request-Failure-Unauthorized"

def activate_shopify_session
ShopifyApp::Logger.debug("Activating Shopify Session")

if current_shopify_session.blank?
signal_access_token_required
ShopifyApp::Logger.debug("No session found for request in JWT or cookie.")
return redirect_to_login
end

Expand All @@ -32,6 +35,7 @@ def activate_shopify_session
ShopifyAPI::Context.activate_session(current_shopify_session)
yield
ensure
ShopifyApp::Logger.info("Deactivating Session")
ShopifyAPI::Context.deactivate_session
end
end
Expand All @@ -45,8 +49,10 @@ def current_shopify_session
is_online: online_token_configured?,
)
rescue ShopifyAPI::Errors::CookieNotFoundError
ShopifyApp::Logger.info("CookiesNotFound for current shopify session")
nil
rescue ShopifyAPI::Errors::InvalidJwtTokenError
ShopifyApp::Logger.info("Invalid Jwt token for current shopify session")
nil
end
end
Expand All @@ -55,6 +61,7 @@ def login_again_if_different_user_or_shop
return unless session_id_conflicts_with_params || session_shop_conflicts_with_params

clear_shopify_session
ShopifyApp::Logger.debug("session id or session shop conflicts with params")
redirect_to_login
end

Expand All @@ -71,8 +78,10 @@ def jwt_expire_at

def add_top_level_redirection_headers(url: nil, ignore_response_code: false)
if request.xhr? && (ignore_response_code || response.code.to_i == 401)
ShopifyApp::Logger.debug("Adding top level redirection headers")
# Make sure the shop is set in the redirection URL
unless params[:shop]
ShopifyApp::Logger.debug("Setting current shop session")
params[:shop] = if current_shopify_session
current_shopify_session.shop
elsif (matches = request.headers["HTTP_AUTHORIZATION"]&.match(/^Bearer (.+)$/))
Expand All @@ -83,6 +92,7 @@ def add_top_level_redirection_headers(url: nil, ignore_response_code: false)

url ||= login_url_with_optional_shop

ShopifyApp::Logger.debug("Setting Reauthorize-Url to #{url}")
response.set_header("X-Shopify-API-Request-Failure-Reauthorize", "1")
response.set_header("X-Shopify-API-Request-Failure-Reauthorize-Url", url)
end
Expand All @@ -105,6 +115,7 @@ def host
def redirect_to_login
if request.xhr?
add_top_level_redirection_headers(ignore_response_code: true)
ShopifyApp::Logger.debug("Login Redirect request is a xhr")
head(:unauthorized)
else
if request.get?
Expand All @@ -117,12 +128,15 @@ def redirect_to_login
query = query.merge(sanitized_params).to_query
end
session[:return_to] = query.blank? ? path.to_s : "#{path}?#{query}"
ShopifyApp::Logger.debug("Redirecting to #{login_url_with_optional_shop}")
redirect_to(login_url_with_optional_shop)
end
end

def close_session
clear_shopify_session
ShopifyApp::Logger.debug("Closing Session")
ShopifyApp::Logger.debug("Redirecting to #{login_url_with_optional_shop}")
redirect_to(login_url_with_optional_shop)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ def embedded_param?

def redirect_for_embedded
# Don't actually redirect if we're already in the redirect route - we want the request to reach the FE
redirect_to(redirect_uri_for_embedded) unless request.path == ShopifyApp.configuration.embedded_redirect_url
unless request.path == ShopifyApp.configuration.embedded_redirect_url
ShopifyApp::Logger.debug("Redirecting to #{redirect_uri_for_embedded}")
redirect_to(redirect_uri_for_embedded)
end
end

def redirect_uri_for_embedded
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module WebhookVerification

def verify_request
data = request.raw_post
ShopifyApp::Logger.debug("Webhook verification failed - HMAC invalid")
return head(:unauthorized) unless hmac_valid?(data)
end

Expand Down
Loading