Skip to content

Commit

Permalink
Merge pull request #1610 from Shopify/klenotiw/remove-jwtmiddlewarn-w…
Browse files Browse the repository at this point in the history
…arnings

Remove Logged output for rescued JWT exceptions
  • Loading branch information
klenotiw committed Dec 15, 2022
2 parents 82616dd + 968b2cb commit 613fef7
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 24 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
Unreleased
----------

* Removed Logged output for rescued JWT exceptions [#1610](https://github.com/Shopify/shopify_app/pull/1610)
* Fixes a bug with `ShopifyApp::WebhooksManager.destroy_webhooks` causing not passing session arguments to [unregister](https://github.com/Shopify/shopify-api-ruby/blob/main/lib/shopify_api/webhooks/registry.rb#L99) method [#1569](https://github.com/Shopify/shopify_app/pull/1569)
* Validates shop's offline session token is still valid when using `EnsureInstalled`[#1612](https://github.com/Shopify/shopify_app/pull/1612)

Expand Down
3 changes: 1 addition & 2 deletions lib/shopify_app/session/jwt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ def expire_at
def set_payload
payload, _ = parse_token_data(ShopifyApp.configuration&.secret, ShopifyApp.configuration&.old_secret)
@payload = validate_payload(payload)
rescue *WARN_EXCEPTIONS => error
ShopifyApp::Logger.warn("Failed to validate JWT: [#{error.class}] #{error}")
rescue *WARN_EXCEPTIONS
nil
end

Expand Down
22 changes: 0 additions & 22 deletions test/shopify_app/session/jwt_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,13 @@ class JWTTest < ActiveSupport::TestCase
end

test "shopify_domain and shopify_user_id are nil if the jwt is invalid" do
expect_jwt_error(::JWT::DecodeError, "Not enough or too many segments")

jwt = JWT.new("token")

assert_nil jwt.shopify_domain
assert_nil jwt.shopify_user_id
end

test "#shopify_domain and #shopify_user_id are nil if the jwt is unsigned" do
# The library expects there to be a 3rd segment if we want to verify signature
expect_jwt_error(::JWT::IncorrectAlgorithm, "Expected a different algorithm")

t = ::JWT.encode(payload, nil, "none")
jwt = JWT.new(t)

Expand All @@ -63,8 +58,6 @@ class JWTTest < ActiveSupport::TestCase
end

test "#shopify_domain and #shopify_user_id are nil if the signature is bad" do
expect_jwt_error(::JWT::VerificationError, "Signature verification failed")

t = ::JWT.encode(payload, "bad", "HS256")
jwt = JWT.new(t)

Expand All @@ -73,8 +66,6 @@ class JWTTest < ActiveSupport::TestCase
end

test "#shopify_domain and #shopify_user_id are nil if 'aud' claim doesn't match api_key" do
expect_jwt_error(::ShopifyApp::InvalidAudienceError, "'aud' claim does not match api_key")

ShopifyApp.configuration.api_key = "other_key"

p = payload
Expand All @@ -85,8 +76,6 @@ class JWTTest < ActiveSupport::TestCase
end

test "#shopify_domain and #shopify_user_id are nil if 'exp' claim is in the past" do
expect_jwt_error(::JWT::ExpiredSignature, "Signature has expired")

p = payload(exp: 1.day.ago)
jwt = JWT.new(token(p))

Expand All @@ -95,8 +84,6 @@ class JWTTest < ActiveSupport::TestCase
end

test "#shopify_domain and #shopify_user_id are nil if 'nbf' claim is in the future" do
expect_jwt_error(::JWT::ImmatureSignature, "Signature nbf has not been reached")

p = payload(nbf: 1.day.from_now)
jwt = JWT.new(token(p))

Expand All @@ -105,8 +92,6 @@ class JWTTest < ActiveSupport::TestCase
end

test "#shopify_domain and #shopify_user_id are nil if `dest` is not a valid shopify domain" do
expect_jwt_error(::ShopifyApp::InvalidDestinationError, "'dest' claim host not a valid shopify host")

p = payload(dest: "https://example.com")
jwt = JWT.new(token(p))

Expand All @@ -115,8 +100,6 @@ class JWTTest < ActiveSupport::TestCase
end

test "#shopify_domain and #shopify_user_id are nil if `iss` host doesn't match `dest` host" do
expect_jwt_error(::ShopifyApp::MismatchedHostsError, "'dest' claim host does not match 'iss' claim host")

p = payload(dest: "https://other.myshopify.io")
jwt = JWT.new(token(p))

Expand All @@ -134,11 +117,6 @@ class JWTTest < ActiveSupport::TestCase

private

def expect_jwt_error(klass, message)
message = "Failed to validate JWT: [#{klass}] #{message}"
ShopifyApp::Logger.expects(:warn).with(message)
end

def token(payload)
::JWT.encode(payload, ShopifyApp.configuration.secret, "HS256")
end
Expand Down

0 comments on commit 613fef7

Please sign in to comment.