[False Positive] invalid_scope, Scope does not match, it may have been tampered with.

[minasmart]

An issue presented in v1.1.12 and v1.1.13. Scope validation was added to the shopify auth strategy. The issue exists because we pass the desired scopes as a comma-separated list, and in those gem versions, split that string on commas and did no further normalization. Example:

"write_products, read_products"

is not equivalent to

"write_products,read_products"

Note the space in the first version for readability. This is also the recommendation in the shopify_app gem documentation. This issue may present for any user who followed the docs exactly and had spaces in their scope string.

The Fix

If you can bump the gem version:

Upgrading to v1.1.14 fixes this issue.

If you can not bump the gem version:

Remove the spaces from your api client's request string "scope1, scope2" -> "scope1,scope2"

Affected code

Rails.application.config.middleware.use OmniAuth::Builder do
    provider :shopify, Rails.application.config.shopify.api_key, Rails.application.config.shopify.secret,
    # Bad code:
    # scope: 'write_products, read_channels, write_admin_notifications',
    # Good code:
    scope: 'write_products,read_channels,write_admin_notifications',
end

The way forward

I would like to recommend that we introduce a new major version where scopes passed to the omniauth builder are an array, removing the likelihood of a user introducing this trivial difference.

Note:

v1.1.12 and v1.1.13 should be pulled on Monday, Feb. 29. We don't want to break any gem users' deploys for the weekend. This issue exists so that users with this error have some documentation to find a solution.

CC: @EiNSTeiN- @richgilbank @harismahmood89 @alexaitken

[dylanahsmith]

If you can not bump the gem version:

Remove the spaces from your api client's request string "scope1, scope2" -> "scope1,scope2"

This issue doesn't actually mention the problem in v1.1.13, since that version actually fixed that problem with spaces in the scope. The problem was that there was further normalization of the scope that was needed to check the scope that came back from Shopify with the access scope, which is that we removed the read scope when the write scope was also requested, since we implicitly give read access to an app that has write access.

For example, "write_products,read_products" would need to be changed to "write_products" to workaround the issue without bumping the gem to v1.1.14

I would like to recommend that we introduce a new major version where scopes passed to the omniauth builder are an array, removing the likelihood of a user introducing this trivial difference.

I don't think we need a major version to allow an array of scopes to be passed. We can introduce a minor release that supports both ways of passing the scope option.

Also, after upgrading to v1.1.14 user's won't have to worry about introducing this difference. They also won't have to worry about further normalization.

[minasmart]

Thanks for the clarification @dylanahsmith 😄

[EiNSTeiN-]

Just closing, I don't think the issue needs to remain open to be searchable!