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

EnsureAuthenticatedLinks doesn't understand deep linked pages #1539

Closed
netwire88 opened this issue Oct 19, 2022 · 0 comments · Fixed by #1549
Closed

EnsureAuthenticatedLinks doesn't understand deep linked pages #1539

netwire88 opened this issue Oct 19, 2022 · 0 comments · Fixed by #1549
Labels

Comments

@netwire88
Copy link

netwire88 commented Oct 19, 2022

Description

Summary: What's the proper way to setup the app so you can deep link into the app directly from URL?

Update:

After much debugging, we think we found the culprit - For a deep link, EnsureAuthenticatedLinks will redirect back to splash page even if embedded=1 and host, session, hmac all exist (missing_expected_jwt? returns true). Which causes loops.

Issue:

In earlier versions of shopify_app, it was recommended to use load_path in order to support deep linking with apps with Turbolinks (see ERB code below).

However, in the latest versions (v20+, testing with v21.1.1), Shopify is recommending to use embedded_redirect_url. However, this doesn't work with deep linking (even after 2 weeks of testing using every single permutation of different settings).

Can someone update the shopify_app template app to use embedded_redirect_url and explain how it's supposed to work?

Here are various things we discovered:

  • Isinclude ShopifyApp::EnsureAuthenticatedLinks still needed? It doesn't seem to be working with error uninitialized constant ShopifyAPI::Auth::Oauth::SessionCookie (called by redirect_to_splash_page)
  • Per Controller code below, should we be using redirect_to(redirect_url, allow_other_host: true) or full_page_redirect? Currently, we get this error: in a frame because it set 'X-Frame-Options' to 'deny'.
  • Should still use load_path in 'shopify-app-init' block (see ERB code below) or is that not needed?
  • Adding include ShopifyApp::ShopAccessScopesVerification causes loops
  • When deep linking to the app, AppBridge (v3.4.0) will try to load the same page 2-3 times
  • Removing load_path in ERB and adding redirect_to to HomeController doesn't work (see below)
  • Even in the shopify_app template, if we try to go to a link directly (i.e. https://store-name.myshopify.com/admin/apps/app-name/sub_path), we get "This app can’t load due to an issue with browser cookies. Try enabling cookies in your browser, switching to another browser, or contacting the developer to get support."

ERB

    <%= content_tag(:div, nil, id: 'shopify-app-init', data: {
      api_key: ShopifyApp.configuration.api_key,
      shop_origin: @shop_origin || (@current_shopify_session.shop if @current_shopify_session),
      host: @host,
      load_path: params[:return_to] || home_path,
      debug: Rails.logger.level == Logger::DEBUG
    } ) %>

Controller:

if ShopifyAPI::Context.embedded? && (!params[:embedded].present? || params[:embedded] != "1")
      if params[:return_to].present?
        redirect_url = ShopifyAPI::Auth.embedded_app_url(params[:host]) + params[:return_to]
      else
        redirect_url = ShopifyAPI::Auth.embedded_app_url(params[:host]) + request.path
      end
      redirect_to(redirect_url, allow_other_host: true)
    end
@netwire88 netwire88 added the bug label Oct 19, 2022
@netwire88 netwire88 changed the title embedded_redirect_url vs. content_tag load_path embedded_redirect_url vs. shopify-app-init's load_path Oct 19, 2022
@netwire88 netwire88 changed the title embedded_redirect_url vs. shopify-app-init's load_path embedded_redirect_url doesn't work with deep linking Oct 21, 2022
@netwire88 netwire88 changed the title embedded_redirect_url doesn't work with deep linking How to deep link in v20+? Oct 21, 2022
@netwire88 netwire88 changed the title How to deep link in v20+? redirect_to_splash_page doesn't understand deep linked pages Oct 22, 2022
@netwire88 netwire88 changed the title redirect_to_splash_page doesn't understand deep linked pages EnsureAuthenticatedLinks.redirect_to_splash_page doesn't understand deep linked pages Oct 22, 2022
@netwire88 netwire88 changed the title EnsureAuthenticatedLinks.redirect_to_splash_page doesn't understand deep linked pages EnsureAuthenticatedLinks doesn't understand deep linked pages Oct 22, 2022
rdillensnyder added a commit that referenced this issue Oct 25, 2022
Related to #1539 
The 'embedded' param from the updated OAuth flow is lost by the EnsureAuthenticatedLinks concern during redirects for deep linked pages or Admin extensions. Adding this param to ensure the flow completes successfully.
@rdillensnyder rdillensnyder linked a pull request Nov 4, 2022 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants