Fix partners ability to login to external shops #1873
Conversation
molly-yu
requested review from
a team,
hannachen,
jesalerno84 and
MeredithCastile
and removed request for
a team
| @@ -415,7 +415,7 @@ module Messages | |||
| Usage: {{command:%s login [--store=STORE]}} | |||
| HELP | |||
| invalid_shop: <<~MESSAGE, | |||
| Invalid store provided (%s). Please provide the store in the following format: my-store.myshopify.com | |||
| Invalid store provided (%s). Please make sure that the store belongs to your partner organization, and provide the store in the following format: my-store.myshopify.com | |||
There was a problem hiding this comment.
@MeredithCastile This is the error message shown when either: the store is invalid, or the store doesn't belong to the partners account. I'm not entirely sure if it would be better to separate the cases into two error messages... Open to any suggestions!
There was a problem hiding this comment.
@molly-yu It would probably be better to separate it into two specific error messages. I am getting this error since upgrading to 2.9.0 even though I have been granted partner access to a store, and I don't know how to fix it.
…rtners-login-security
…shopify-cli into partners-login-security
lib/shopify_cli/commands/login.rb
Outdated
| end | ||
|
|
||
| def self.help | ||
| ShopifyCLI::Context.message("core.login.help", ShopifyCLI::TOOL_NAME) | ||
| end | ||
|
|
||
| def self.validate_shop(shop, context:) | ||
| def self.validate_shop(shop, org, context:) |
There was a problem hiding this comment.
Nitpick: I'd use keyed arguments to make it clear from the caller what every argument means.
def self.validate_shop(shop:, org:, context:)Fix test for #1873: Fix partners ability to login to external shops
|
Molly, does this comment feel like a related error ( |
Hmm interesting, this is definitely related to the other auth/login issues. It seems like there are a lot of commands that are affected by the wonky login. I'll check this out too. Thanks for catching this Steph! |
|
Maybe we can create a separate issue for this, and put it under authentication-related issues |
|
Tracking it in this new issue. |
WHY are these changes introduced?
Fixes #1833, #1652 (related).
Partners were able to log into stores that were not their own, through
shopify login --store=STORE, where STORE is not supposed to be accessible by the Partners account.WHAT is this pull request doing?
Previously, the login command was checking the store validity first, then dealing with the user authentication. I changed the code so that it checks that the store belongs to the Partner, after authentication.
How to test your changes?
shopify-dev login --store=STORE** when STORE belongs to the Partners account, login should succeed.
Post-release steps
Update checklist