TheSecurityDev / simple-koa-shopify-auth

An unofficial, simplified version of the @Shopify/koa-shopify-auth middleware library.
MIT License
25 stars 6 forks source link

Add the ability to specify the host of the redirect url instead of ctx.host #5

Closed regexj closed 2 years ago

regexj commented 2 years ago

I'm having the same issue as this person here on the original koa-shopify-auth and so am suggesting the exact same thing: https://github.com/Shopify/koa-shopify-auth/issues/11

Overview The developer should be able to define the host of the redirect URL to be https://{Host} instead of taking the host from context (ctx).

https://github.com/TheSecurityDev/simple-koa-shopify-auth/blob/e113d012a114b9ec3230f687b5ed51cd33a0490f/src/top-level-oauth-redirect.ts#L36

When running createShopifyAuth() there could be an extra optional parameter, host, which if set the above function will use the provided host instead of ctx.host, which can be wrong when using proxies.

regexj commented 2 years ago

image For some context, this is the issue I'm facing. Somewhat confusingly it does this about 50% of the time, the other 50% the host is correct and the app works as expected.

regexj commented 2 years ago

This was taking me too long looking for a workaround trying to use koa-proxies and stuff like that. Have added a PR which solves this issue, if you could review https://github.com/TheSecurityDev/simple-koa-shopify-auth/pull/6

TheSecurityDev commented 2 years ago

So I was making a few minor changes to your PR, when I had the idea that instead of using ctx.host and instead of passing in the host name, we should use the host specified on Shopify.Context.HOST_NAME. Is that the same as the host you are passing in and would that work in your case?

TheSecurityDev commented 2 years ago

Also, have you tried initializing your Koa server like this:

new Koa({ proxy: true });

I had to do this for ngrok and heroku so I could get the client's IP address correctly, so maybe it's a similar situation with getting the host header.

regexj commented 2 years ago

That's a good idea. Yeah, I am passing that same string as pulled from the .env so that should work in my case.

createShopifyAuth({
      accessMode: "online",
      authPath: "/auth",
      host: config.shopify.HOST_NAME,
regexj commented 2 years ago

Also, have you tried initializing your Koa server like this:

new Koa({ proxy: true });

I had to do this for ngrok and heroku so I could get the client's IP address correctly, so maybe it's a similar situation with getting the host header.

I have not, but we've just resubmitted the app for listing after using the forked version of this package, so just going to wait on a reply from Shopify now before making more changes.

TheSecurityDev commented 2 years ago

I just published the new package v1.0.5, so I'll go ahead and close this. The new version uses Shopify.Context.HOST_NAME instead, so you don't have to pass in the host name or anything. I suspect the issue would also have been solved by enabling Koa proxy as I mentioned, but this would fix any future issues other people may have like this.