Skip to content
This repository has been archived by the owner on Mar 10, 2022. It is now read-only.

Remove cookie for shopOrigin and accessToken #325

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hannachen
Copy link

@hannachen hannachen commented Dec 10, 2020

For https://github.com/Shopify/app-bridge/issues/1742

To be shipped with: https://github.com/Shopify/shopify-dev/pull/4722

Remove the use of cookies to keep track of shopOrigin and get it from URL query params instead.

During the oauth flow, don't read accessToken from a cookie. Instead, read it from ctx.state.shopify, which is passed from @shopify/koa-shopify-auth.

Also, update getSubscriptionUrl.js so that code samples for handling app billing can work without using 3rd-party cookies.

Changes here need to be reflected in the tutorial as well: https://shopify.dev/tutorials/build-a-shopify-app-with-node-and-react/charge-a-fee-using-the-billing-api

@hannachen hannachen marked this pull request as ready for review December 10, 2020 21:57
Copy link
Contributor

@paulomarg paulomarg left a comment

Choose a reason for hiding this comment

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

🎩 ed, was able to install and log in, everything seems to be working fine!

One point though: I noticed that when I went to perform OAuth for the first time, I got an error about the shop param being missing in the URL.

Adding that param fixed everything, so it doesn't really cause any problems, but I wonder if the tutorial needs to be updated to account for that. Alternatively, would it be possible to fill that from the .env file?

@Steven4294
Copy link

Steven4294 commented Dec 19, 2020

Hi could we get this merged please? Im running into problems related to this.

I'm trying to use the next.js tutorial project and actually use next.js APIs - namely making web request to an endpoint within api/pages - but I cannot do that in the current state. I've made an app prefix like so:

	server.use(
		createShopifyAuth({
		apiKey: SHOPIFY_API_KEY,
		prefix: "/shopifyapp",

However that doesn't seem to work (i've also added the /shopifyapp to my app URL manually), as well as updating my verify request handler. Is there an example of a shopify app running next.js that actually uses next.js endpoints?

@paulomarg

@Steven4294
Copy link

Namely an example of an app the prefixes the shopify app and then can send calls to an /api route. Cheers

@hannachen
Copy link
Author

One point though: I noticed that when I went to perform OAuth for the first time, I got an error about the shop param being missing in the URL.

Adding that param fixed everything, so it doesn't really cause any problems, but I wonder if the tutorial needs to be updated to account for that. Alternatively, would it be possible to fill that from the .env file?

Hmm, interesting! From my understanding, OAuth callback includes the shop param. I'll take a closer look. I think it would be possible to fill that from .env, but that would make the app hardcoded to a particular shop?

@hannachen
Copy link
Author

Namely an example of an app the prefixes the shopify app and then can send calls to an /api route. Cheers

Hi @Steven4294, unfortunately shopify-koa-auth used by this project doesn't support the scenario you described. There are a few examples in the works. In the meantime, a very helpful partner has created this excellent example as a reference: https://github.com/ctrlaltdylan/shopify-session-tokens-nextjs It shows how to make authenticated fetch calls to your app's back-end and verify the call.

@Smart387doo
Copy link

I'm facing a problem. As far as I understand, there is no check in code if shop is already subscribed to app and have webhooks and scripttag already registered?! If I install ma app from dev store, everything works as expected, I get past bill process and redirected to start page. Problem is if I open same dev shop on another browser, go to apps and try to get into installed app I get to subscribe page again where I have to approve subscription again. Bigger problem is that it try to register webhooks again, which is rejected, but successfully register scripttag every time which result in doubling my widget as many times as I change browser or clear cache. How to check and bypass webhook and scripttag as well as need to approve subscription again?

@paulomarg
Copy link
Contributor

Hmm, interesting! From my understanding, OAuth callback includes the shop param. I'll take a closer look. I think it would be possible to fill that from .env, but that would make the app hardcoded to a particular shop?

That's very true, we definitely don't want to tie the app to a single shop. The URL that lacked the shop param wasn't the OAuth callback, but rather the app expected the initial URL to contain one - so when I tried to open https://<my_app_url> it failed, but https://<my_app_url>?shop=<shop_url> worked just fine.

@hannachen
Copy link
Author

Hmm, interesting! From my understanding, OAuth callback includes the shop param. I'll take a closer look. I think it would be possible to fill that from .env, but that would make the app hardcoded to a particular shop?

That's very true, we definitely don't want to tie the app to a single shop. The URL that lacked the shop param wasn't the OAuth callback, but rather the app expected the initial URL to contain one - so when I tried to open https://<my_app_url> it failed, but https://<my_app_url>?shop=<shop_url> worked just fine.

Could we handle this in a similar way as the rails app gem? If no shop domain is supplied, display an "install" page to accept a shop domain:
Screen Shot 2021-01-05 at 12 55 53 PM

@hannachen
Copy link
Author

I'm facing a problem. As far as I understand, there is no check in code if shop is already subscribed to app and have webhooks and scripttag already registered?! If I install ma app from dev store, everything works as expected, I get past bill process and redirected to start page. Problem is if I open same dev shop on another browser, go to apps and try to get into installed app I get to subscribe page again where I have to approve subscription again. Bigger problem is that it try to register webhooks again, which is rejected, but successfully register scripttag every time which result in doubling my widget as many times as I change browser or clear cache. How to check and bypass webhook and scripttag as well as need to approve subscription again?

Hi @Smart387doo, I think I encountered a similar issue before, unfortunately, I don't think there's a way to list all subscribed webhooks to verify exactly what went wrong. Is this something that you're encountering during development? If so, all webhooks for an app are removed when you delete the app. From my understanding, the webhook subscription create returns an ID when successfully created for the first time. Would you be able to track that on your end, and use the ID to confirm its existence? https://shopify.dev/docs/admin-api/graphql/reference/events/webhooksubscription

@Smart387doo
Copy link

Hi @Smart387doo, I think I encountered a similar issue before, unfortunately, I don't think there's a way to list all subscribed webhooks to verify exactly what went wrong. Is this something that you're encountering during development? If so, all webhooks for an app are removed when you delete the app. From my understanding, the webhook subscription create returns an ID when successfully created for the first time. Would you be able to track that on your end, and use the ID to confirm its existence? https://shopify.dev/docs/admin-api/graphql/reference/events/webhooksubscription

Well, webhooks are not big issue, as it wont register again, I get following in console:
Failed to register webhook {
data: {
webhookSubscriptionCreate: { userErrors: [Array], webhookSubscription: null }
},
extensions: {
cost: {
requestedQueryCost: 10,
actualQueryCost: 10,
throttleStatus: [Object]
}
}
}
[
{
field: [ 'webhookSubscription', 'callbackUrl' ],
message: 'Address for this topic has already been taken'
}
]

Biggest issue is that it register ScriptTag every time, which we use to inject a widget to product page, and every time it register new ScriptTag it doubles the widget on product page. Not to mention that it require to subscribe to payment again even though shop it's already subscribed to app.

Yes it is in development phase, we have finished app but are afraid to launch it as it seems to be big issue.

@hannachen
Copy link
Author

Hi @Smart387doo, I think I encountered a similar issue before, unfortunately, I don't think there's a way to list all subscribed webhooks to verify exactly what went wrong. Is this something that you're encountering during development? If so, all webhooks for an app are removed when you delete the app. From my understanding, the webhook subscription create returns an ID when successfully created for the first time. Would you be able to track that on your end, and use the ID to confirm its existence? https://shopify.dev/docs/admin-api/graphql/reference/events/webhooksubscription

Well, webhooks are not big issue, as it wont register again, I get following in console:
Failed to register webhook {
data: {
webhookSubscriptionCreate: { userErrors: [Array], webhookSubscription: null }
},
extensions: {
cost: {
requestedQueryCost: 10,
actualQueryCost: 10,
throttleStatus: [Object]
}
}
}
[
{
field: [ 'webhookSubscription', 'callbackUrl' ],
message: 'Address for this topic has already been taken'
}
]

Biggest issue is that it register ScriptTag every time, which we use to inject a widget to product page, and every time it register new ScriptTag it doubles the widget on product page. Not to mention that it require to subscribe to payment again even though shop it's already subscribed to app.

Yes it is in development phase, we have finished app but are afraid to launch it as it seems to be big issue.

Thanks for walking me through the issue you're having. That scenario does sound problematic, but I don't have a good recommendation to offer at this time. @paulomarg do you know if there's already a recommendation for this? If not, where might be a good place to track it? Along with @Steven4294's use case with Next.js. I'd be happy to look into it sometime if we don't currently have a recommended approach.

@paulomarg
Copy link
Contributor

The way we usually handle this is be keeping track of what has already been registered for a shop in your app - that way you can fully avoid the requests to Shopify, which avoids the issue and speeds things up by not making unnecessary requests. Is that something you could do in your app?

@Smart387doo
Copy link

The way we usually handle this is be keeping track of what has already been registered for a shop in your app - that way you can fully avoid the requests to Shopify, which avoids the issue and speeds things up by not making unnecessary requests. Is that something you could do in your app?

How exactly you handle this? Save for every shop state of webhooks and scripttag to db? Do you have some example code available? Thank you in advance.

@paulomarg
Copy link
Contributor

How exactly you handle this? Save for every shop state of webhooks and scripttag to db? Do you have some example code available? Thank you in advance.

Unfortunately we don't have any examples in Typescript, but it might help to check out how the shopify_app Ruby gem handles Webhooks and Script Tags.

What that does is essentially to load the configuration for which webhooks / script tags are used by the app, and registers the ones that are missing when an OAuth is completed (so if you add more of them later, they will also be added). Once your app has successfully registered the webhook / script tag, you can store that Shopify knows about it, so you don't need to do it again.

You shouldn't need to store this information for each shop, as webhooks / script tags apply to your app as a whole. Sorry if my previous message was a bit unclear on that regard. The callbacks will contain which shop is currently running it.

If you don't want to store this information in your db, you might be able to fetch webhooks and script tags using the GraphQL API too, to check if you need to register anything:

{
  webhookSubscriptions(first: 10) {
    edges {
      node {
        topic
      }
    }
  },
  scriptTags(first: 10) {
    edges {
      node {
        src
      }
    }
  }
}

Hope this helps!

@Smart387doo
Copy link

How exactly you handle this? Save for every shop state of webhooks and scripttag to db? Do you have some example code available? Thank you in advance.

Unfortunately we don't have any examples in Typescript, but it might help to check out how the shopify_app Ruby gem handles Webhooks and Script Tags.

What that does is essentially to load the configuration for which webhooks / script tags are used by the app, and registers the ones that are missing when an OAuth is completed (so if you add more of them later, they will also be added). Once your app has successfully registered the webhook / script tag, you can store that Shopify knows about it, so you don't need to do it again.

You shouldn't need to store this information for each shop, as webhooks / script tags apply to your app as a whole. Sorry if my previous message was a bit unclear on that regard. The callbacks will contain which shop is currently running it.

If you don't want to store this information in your db, you might be able to fetch webhooks and script tags using the GraphQL API too, to check if you need to register anything:

{
  webhookSubscriptions(first: 10) {
    edges {
      node {
        topic
      }
    }
  },
  scriptTags(first: 10) {
    edges {
      node {
        src
      }
    }
  }
}

Hope this helps!

It helps for sure, we can for sure use this and check, already implemented and it works as it should, but problem I'm facing now is how to check if shop already have an active recurring subscription and avoid to call getSubscriptionUrl which tries to create new subscription every time. I tried to look at ruby code, but I can't see anything that can help?

I did look at https://shopify.dev/docs/admin-api/graphql/reference/billing/appsubscription, but can't seem to get luck, nor examples from https://shopify.dev/tutorials/bill-for-your-app-using-graphql-admin-api#example-queries works.

@paulomarg
Copy link
Contributor

Yes, getSubscriptionUrl will always trigger a mutation that adds a subscription. It seems that the example queries page is outdated, I will raise that internally to get it fixed.

You could maybe try looking for active subscriptions like so:

{
  currentAppInstallation {
    activeSubscriptions {
      createdAt
    }
  }
}

I believe that query should tell you whether or not the current shop currently has a subscription to your app.

@hannachen sorry if we took over your PR with this discussion! @Smart387doo if this doesn't solve your issue, would you mind opening a new issue in https://github.com/Shopify/shopify-app-node so we can discuss this without getting in the way of the PR?

@Smart387doo
Copy link

Yes, getSubscriptionUrl will always trigger a mutation that adds a subscription. It seems that the example queries page is outdated, I will raise that internally to get it fixed.

You could maybe try looking for active subscriptions like so:

{
  currentAppInstallation {
    activeSubscriptions {
      createdAt
    }
  }
}

I believe that query should tell you whether or not the current shop currently has a subscription to your app.

@hannachen sorry if we took over your PR with this discussion! @Smart387doo if this doesn't solve your issue, would you mind opening a new issue in https://github.com/Shopify/shopify-app-node so we can discuss this without getting in the way of the PR?

Sorry if I overflood this thread, will stop it, just want to give feedback.

Not sure if I do something wrong, but I tested many graphql queries, but not able to get if shop have active subscription. This one like other ones gives undefined, there is response but no payload as expected.

Thank you for help, will try to ask in another thread, I just can't believe that it should be so complicated or unclear how to gather such important information.

@paulomarg
Copy link
Contributor

Just a quick update: we've fixed the example queries docs page, so that query might be what you're looking for now.

Thank you for pointing out that the query was not working!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants