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 the ability to specify the host of the redirect url instead of ctx.host #5

Closed
regexj opened this issue Feb 8, 2022 · 7 comments

Comments

@regexj
Copy link
Contributor

regexj commented Feb 8, 2022

I'm having the same issue as this person here on the original koa-shopify-auth and so am suggesting the exact same thing: Shopify/koa-shopify-auth#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).

export function createTopLevelRedirect(apiKey: string, path: string) {

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
Copy link
Contributor Author

regexj commented Feb 8, 2022

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
Copy link
Contributor Author

regexj commented Feb 8, 2022

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 #6

@TheSecurityDev
Copy link
Owner

TheSecurityDev commented Feb 8, 2022

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
Copy link
Owner

TheSecurityDev commented Feb 8, 2022

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
Copy link
Contributor Author

regexj commented Feb 8, 2022

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
Copy link
Contributor Author

regexj commented Feb 8, 2022

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
Copy link
Owner

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.

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

No branches or pull requests

2 participants