Shopify / shopify_app

A Rails Engine for building Shopify Apps
MIT License
1.77k stars 692 forks source link

Associated User Info Different in Shopify App Gem vs Online Access Token Exchange #1894

Closed TheAbyss closed 2 months ago

TheAbyss commented 3 months ago

Issue summary

Before opening this issue, I have:

Background

Our biggest technical roadblock to migrate to BFS (Built for Shopify) is identifying associated user information on the Shopify App gem. Currently, we store and use associated user info for multi-store permission controls/navigation, fraud prevention, and onboarding via Online Access Token Exchange. This approach forces many redirects degrading admin portal performance.

Therefore, we are switching to utilize Shopify App Gem/SDK to meet and exceed Built for Shopify spirit and requirements. But we ran into a couple issues that don't allow us to migrate our Shopify authentication flow:

Expected behavior

Actual behavior

Steps to reproduce the problem

Online Access Token Exchange approach to getting Associated User

  1. Login to Store A (ensure you are a collaborator & NOT owner of Shop)
  2. Install/Login to App A with Store A
  3. Save JSON Response to compare later, looks something like this:
{"access_token":"shpat_****",
"scope":"read_shipping,write_order_edits,read_inventory,write_checkouts,write_draft_orders,write_products,write_customers,write_script_tags,write_price_rules,write_discounts,write_orders", "expires_in":86399,
"associated_user_scope":"read_shipping,write_order_edits,read_inventory,write_checkouts,write_draft_orders,write_products,write_customers,write_script_tags,write_price_rules,write_discounts,write_orders",
"session":"****", "account_number":nil, "associated_user":{"id":******, "first_name":"******", "last_name":"******", "email":"******", "account_owner":true,
"locale":"en-US", "collaborator":false, "email_verified":true}}
  1. Login to Store B (ensure you are owner of Shop)
  2. Install/Login to App A with Store B
  3. Save JSON Response to compare later

Shopify App Gem approach to getting Associated User

  1. Create Following Class
    • without this class associated_user attributes will return nil
    • SIDE QUEST: improve gem to eliminate workaround below
      
      module ShopifyApp
        module UserSessionStorageWithScopesAndAssociatedUser
          extend ActiveSupport::Concern
          include ::ShopifyApp::SessionStorage

    included do       validates :shopify_domain, presence: true     end

    class_methods do       def store(auth_session, logged_in_user)         Rails.logger.debug "Storing user session - user: #{logged_in_user.inspect} - auth_session: #{auth_session.inspect}"

        user                = find_or_initialize_by(shopify_user_id: logged_in_user.id)         user.shopify_token  = auth_session.access_token         user.shopify_domain = auth_session.shop         user.access_scopes  = auth_session.scope.to_s         user.expires_at     = auth_session.expires

        user.first_name     = logged_in_user.first_name         user.last_name      = logged_in_user.last_name         user.email          = logged_in_user.email         user.email_verified = logged_in_user.email_verified         user.account_owner  = logged_in_user.account_owner         user.locale         = logged_in_user.locale         user.collaborator   = logged_in_user.collaborator

        user.save!         user.id       end

      def retrieve(id)         user = find_by(id: id)         construct_session(user)       end

      def retrieve_by_shopify_user_id(user_id)         user = find_by(shopify_user_id: user_id)         construct_session(user)       end

      def destroy_by_shopify_user_id(user_id)         destroy_by(shopify_user_id: user_id)       end

      private

      def construct_session(user)         return unless user

        associated_user = ShopifyAPI::Auth::AssociatedUser.new(           id:             user.shopify_user_id,           first_name:     user.first_name,           last_name:      user.last_name,           email:          user.email,           email_verified: user.email_verified,           account_owner:  user.account_owner,           locale:         user.locale,           collaborator:   user.collaborator,         )

        ShopifyAPI::Auth::Session.new(           shop:                  user.shopify_domain,           access_token:          user.shopify_token,           scope:                 user.access_scopes,           associated_user_scope: user.access_scopes,           associated_user:       associated_user,           expires:               user.expires_at,         )       end     end

    def access_scopes=(scopes)       super(scopes)     rescue NotImplementedError, NoMethodError       raise NotImplementedError, "#access_scopes= must be defined to handle storing access scopes: #{scopes}"     end

    def access_scopes       super     rescue NotImplementedError, NoMethodError       raise NotImplementedError, "#access_scopes= must be defined to hook into stored access scopes"     end

    def expires_at=(expires_at)       super     rescue NotImplementedError, NoMethodError       if ShopifyApp.configuration.check_session_expiry_date         raise NotImplementedError,               "#expires_at= must be defined to handle storing the session expiry date"       end     end

    def expires_at       super     rescue NotImplementedError, NoMethodError       if ShopifyApp.configuration.check_session_expiry_date         raise NotImplementedError, "#expires_at must be defined to check the session expiry date"       end

      nil     end   end end



2. Login to Store A (ensure you are a collaborator & NOT owner of Shop)
3. Install/Login to App A with Store A
4. Login to Store B (ensure you are owner of Shop)
5. Install/Login to App A with Store B
6. Check user model in DB and compare to json responses from [Online Access Token Exchange](https://shopify.dev/docs/apps/build/authentication-authorization/access-tokens/online-access-tokens) approach
lizkenyon commented 3 months ago

Hi there 👋

I just wanted to clarify a few things, to make sure we are all on the same page about your current functionality and what you are hoping to do! 😃

Currently, we store and use associated user info for multi-store permission controls/navigation, fraud prevention, and onboarding via Online Access Token Exchange. This approach forces many redirects degrading admin portal performance.

With token exchange we wouldn't expect to see the redirects, that sounds like instead you are doing Authorization code grant with online access tokens.

With the Shopify app gem you can do both formats of OAuth, Auth Code Grant and Token Exchange. With either online or offline access tokens. But it is not necessary to use the Shopify App gem. These can both be implemented without the app gem. (Token exchange, Auth code grant) So in summary there are 4 different scenarios

And these 4 scenario can be implemented manually or you can use the Shopify App gem.

Could you confirm what OAuth flow you are doing are doing in each scenario? Could you also confirm in your current OAuth flow with the redirects, do you currently see the same user id across shops.

eni9889 commented 3 months ago

@lizkenyon the main issues are:

  1. The ID for the user that is coming through is different per store whereas when we were using the online callback that required redirect the ids were for a user were the same. So say if user A is a collaborator on two different stores that user would have the same ID.
  2. The collaborator flag is always set to false from the token exchange regardless if the user is a collaborator or not
  3. Lastly on the ruby app UserSessionStorageWithScopes module there is no saving of the user info including name etc so we had to write our own module named UserSessionStorageWithScopesAndAssociatedUser which you can see above that actually saves the name, email etc.
lizkenyon commented 3 months ago

Thank you for clarifying! @eni9889

For issue 1 and possibly 2 this sounds like an issue with the API itself. I am going to flag this with the ownership team, to look into.

For 3 I will triage this ticket for my team to look into.

ragalie commented 3 months ago

Hi @eni9889,

I just checked the code that generates the response for the access_token endpoint, and the id that is returned for the user should be specific to the shop (i.e. it will be different for each shop). This behaviour is true for both the authorization code flow (the one that uses the callback) and the token exchange flow.

I also verified that the collaborator value should be correct (and the same) for both the authorization code flow and token exchange.

If you have a specific user/shop example you can DM it to me on the Partners Slack and I can take a closer look.

Thanks!