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

Add configuration option for user access scopes strategy #1599

Merged

Conversation

andyw8
Copy link
Contributor

@andyw8 andyw8 commented Nov 30, 2022

What this PR does

The gem currently provides two strategies for verifying user scopes:

  • ShopifyApp::AccessScopes::NoopStrategy
  • ShopifyApp::AccessScopes::UserStrategy

Some apps (likely only first party apps made by Shopify) may have special requirements and need to use a different strategy. This PR adds a configuration option to allow that.

Reviewer's guide to testing

Reach out to me on Slack to discuss.

Things to focus on

  1. Is there any way this could break an existing app?

Checklist

Before submitting the PR, please consider if any of the following are needed:

  • Update CHANGELOG.md if the changes would impact users
  • Update README.md, if appropriate.
  • Update any relevant pages in /docs, if necessary
  • For security fixes, the Disclosure Policy must be followed.

@andyw8 andyw8 marked this pull request as ready for review November 30, 2022 19:32
@andyw8 andyw8 changed the title Add configuration option for user access strategy Add configuration option for user access scopes strategy Nov 30, 2022
test "user access scopes strategy is configurable" do
my_strategy = Object
ShopifyApp.configure do |config|
config.user_access_scopes_strategy = my_strategy

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use a string.

@andyw8 andyw8 force-pushed the andyw8/add-configuration-option-for-user-access-scopes-strategy branch from d52ee48 to af15eca Compare November 30, 2022 19:57
@@ -83,7 +83,17 @@ def shop_access_scopes_strategy
ShopifyApp::AccessScopes::ShopStrategy
end

def user_access_scopes_strategy=(class_name)
unless class_name.is_a?(String)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using a string to be consistent with user_session_repository and shop_session_repository.

@andyw8 andyw8 merged commit 2f8d75d into main Nov 30, 2022
@andyw8 andyw8 deleted the andyw8/add-configuration-option-for-user-access-scopes-strategy branch November 30, 2022 20:28
fabriazza pushed a commit to fabriazza/shopify_app that referenced this pull request Feb 1, 2023
Co-authored-by: Andy Waite <andyw8@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants