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 AppBridgeMiddleware #1345

Closed

Conversation

kirillplatonov
Copy link
Contributor

What this PR does

When the app is installed via "Add app" button from the app store listing Shopify redirects the user to the login page without host param:
CleanShot 2022-01-07 at 18 47 36@2x

As a result, the app gets AppBridge2 error on the frontend and the user is stuck on the Splash screen. I had issues with this on production after AppBridge2.0 upgrade. I was talking with Shopify Partner support about this behavior and they said it's not a bug but the intended behavior 🤷‍♂️ Until this will change we need a middleware that will generate host from the shop param on the fly.

@kirillplatonov
Copy link
Contributor Author

cc @NabeelAhsen

@NabeelAhsen
Copy link
Contributor

NabeelAhsen commented Jan 7, 2022

When the "Add App" button is clicked, the app is prompted to initiate OAuth by visiting the app's login URL: https://<app>/login?shop=<shop>... To my understanding (and please correct me if this is not the case), the /login endpoint does not need to instantiate app bridge.

The OAuth flow completes and correctly returns the host parameter to the app. Why does your app redirect to the splash page when sent to /login?

Comment on lines +11 to +14
if request.params.has_key?("shop") && !request.params.has_key?("host")
shop = request.params["shop"]
host = Base64.urlsafe_encode64("#{shop}/admin", padding: false)
request.update_param("host", host)
Copy link
Contributor

Choose a reason for hiding this comment

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

From my understanding, the host parameter is subject to change; it's not always going to be the shop Base64 encoded.

@NabeelAhsen
Copy link
Contributor

NabeelAhsen commented Jan 7, 2022

@jesalerno84 @pepicrft Could I get your input here? I'm not convinced that this is a bug

For context, @kirillplatonov suggests that this is critical to fix before releasing v18.0.3: #1344

@kirillplatonov
Copy link
Contributor Author

@NabeelAhsen I just double-checked and everything seems to work correctly without middleware. I definitely was able to reproduce this issue in November and had "APP::ERROR::INVALID_CONFIG: host must be provided" errors in Sentry after app install. Not sure if it was before or after login, to be honest. Let's close this for now then. I will keep an eye on Sentry and will let you know if the issue will happen again.

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.

2 participants