Heroku-friendly, Production deployable, Dashboard. #196
Conversation
|
|
|
By analyzing the blame information on this pull request, we identified @drew-gross, @flovilmart and @hallucinogen to be potential reviewers. |
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! |
On some platforms, like Heroku in this case, the way routing works is that all requests appear to be from localhost *without* SSL. This is just because TLS is terminated way before the app server. Headers are set with the original request information, though. In this case we care about X-FORWARDED-FOR, which is the original IP address of the request sender, and X-FORWARDED-PROTO, which is the protocol used (http/https). If the headers don't exist (like they wouldn't locally) it falls back to the normal Express `req.connection` data.
f3aa48a to
cce4430
Compare
|
@mnoble updated the pull request. |
|
Awesome! Looks like this overlaps with #179 a lot, and has one of the same issue, which is that this would make the dashboard insecure when you aren't deploying behind any load balancers or anything like that. The |
|
@drew-gross Heh, yea, I saw that PR last week right before I was going to push this one and held off. I figured there's more to this one that fixes some of the build issues of #179, so why not push it up. re: re: Windows: No, I don't even have a Windows VM setup, tbh. I'll try to get around to that later today. |
|
Good! Love It! 👍 |
|
@mnoble But, It didn't works on Heroku. |
|
@gimdongwoo You'd need to Once you do that, it should build and work (it does for me, at least). If you still run into issues, can you paste the error message/output you're getting? |
|
@mnoble It works perfectly! 👍 Thank you!! |
|
I'm also using it. Thanks! |
|
I'm closing this in favor of splitting this work up into a few PRs. Mainly #216 which sets up the dependencies and scripts to build on deploy. The |
This PR is two-fold.
First, this makes the project buildable via a production deploy. For that, build dependencies need to be moved to the
dependenciesinstead ofdevDependenciesso that they're available on the server, post-deploy. I also added a couple scripts to build assets and put them into the correct place. This happenspost-install, but only when you explicitly set thePOST_BUILD_INSTALLenv var. This should prevent it from running when installing deps locally on your machine (or when running locally).Second, this makes Dashboard Heroku-friendly. Because of the way routing works on Heroku, all requests appear to be from localhost and non-HTTPS. This is just because TLS termination happens way before the app. We set the usual
X-FORWARDED-FORandX-FORWARDED-PROTOheaders, so this uses those when available.