Skip to content
This repository has been archived by the owner on Jan 30, 2023. It is now read-only.

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

Conversation

ilyavysotski
Copy link

@ilyavysotski ilyavysotski commented Oct 1, 2020

WHY are these changes introduced?

I am facing a problem were the redirect URL is having the backend host as its host because of the reverse proxy on my server and thus having to whitelist this URI and face cookies problem or having to edit the proxy behavior to add the X-Forwarded-Host.

Fixes #11

WHAT is this pull request doing?

It adds the ability to specify the host of the redirect url using SHOPIFY_APP_HOST environment variable instead of ctx.host

Type of change

  • Patch: Bug (non-breaking change which fixes an issue)
  • Minor: New feature (non-breaking change which adds functionality)
  • Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above
  • I have added/updated tests for this change

@ilyavysotski ilyavysotski changed the title Add the ability to specify the host Add the ability to specify the host of the redirect url instead of ctx.host Oct 1, 2020
@tanema
Copy link
Contributor

tanema commented Oct 1, 2020

I appreciate the effort but I am not sure this is a good idea because this won't stop here. You will add one config but people host their app behind many hosts so then the config needs to change again. This library stays flexible by using the host that the application is running on.

I would like to understand why you are against adding X-Forwarded-Host to your application.

@ilyavysotski
Copy link
Author

My server is configured in such a way that X-Forwarded-Host just returns the wrong redirect_uri localhost:3000/auth/callback instead of example.com/auth/callback. If I use SHOPIFY_APP_HOST=example.com then app works fine.

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

Successfully merging this pull request may close these issues.

[koa-shopify-auth] add the ability to specify the host of the redirect url instead of ctx.host
2 participants