Rob--W / cors-anywhere

CORS Anywhere is a NodeJS reverse proxy which adds CORS headers to the proxied request.
MIT License
8.69k stars 6.13k forks source link

Running on Heroku with one-click deploy #320

Open Lewiscowles1986 opened 3 years ago

Lewiscowles1986 commented 3 years ago

In order to run on Heroku with one-click deploy, I setup a fork https://github.com/Lewiscowles1986/cors-anywhere

If Rob wants it, I'm happy to upstream and I kept each branch alive so I can rebase on their repo. I made some other choices too, such as renaming some bits and enabling CI.

This issue is here as "documentation" of how to setup your own cors anywhere using Heroku.

Summary of discussion of changes

Rob--W commented 3 years ago

To minimize backwards-incompatible breakage and confusion, I'm not going to accept the "black"/"white"->"allow"/"block" change.

Documentation for the Heroku one-click deploy feature is at https://devcenter.heroku.com/articles/heroku-button

A completely open proxy is against Heroku's Acceptable Use policy. The current version of your PR makes the proxy open by default. Is it possible to require explicit action from developers, so that whoever that wants to use the button knows that they have to change the configuration?

Lewiscowles1986 commented 3 years ago

It's written on the screen and in your docs. I've added to only have localhost in the allowlist and 1 1 in the rate limit. This seemed to block unwanted traffic.

bobpaul commented 3 years ago

Regarding backwards compatibility for black/block type change, might I suggest supporting "blackList" and "whiteList" as aliases? Eg: If both CORSANYWHERE_BLACKLIST and CORSANYWHERE_BLOCKLIST throw an error, but if only one is defined use that one. Update documentation to point to BLOCKLIST and either do or don't document the alias.

Lewiscowles1986 commented 3 years ago

Right there is a branch at https://github.com/Lewiscowles1986/cors-anywhere/tree/chore/backwards-compat-uninclusive-terms which does this, and console warns on use of the deprecated properties.

Logic is:

Perhaps this removes the rejection of the rename.

I'm just not happy using those terms, but I understand the need to roadmap changes. I Have opted not to document the alias. New users should not really be choosing to deliberately use discriminatory language. It is also not part of the Heroku one-click as that is similarly a greenfield, rather than for existing users.

bobpaul commented 3 years ago

@Lewiscowles1986, after I 1-click deployed from your repo's, I got an empty repo when I heroku git:clone the created app. I found a work around here, but I think if you included the repository property in app.json, I would not have had to do that.

Lewiscowles1986 commented 3 years ago

Hey Bob,

Sorry to hear you've had that difficulty. Could you share some more information about your setup? I Just did the same thing both from my main branch and the branch I linked here, and both are working for me in Chrome, Firefox, Edge and Safari (all latest) across Windows, Linux and OSx. Did you need to login to Heroku first?

I Do know that deploying from app.json does not automatically setup deploy from git commits. I think if I point it at my repository, then there would be more of a problem merging as it would be hosted here, but point to a potentially out-of-date branch, rather than official cors-anywhere.

bobpaul commented 3 years ago

The deploy worked fine. I clicked the button and got a screen asking for the environment variables. I was able to deploy the app to heroku as "myApp"

After it deployed and was running, I used the heroku cli and did heroku git:clone -a {myApp} and it gave me an empty repo. It's been a while since I deployed a click-to-deploy, but I thought they usually were attached to the original repo in some way for re-deploy and updates.

Lewiscowles1986 commented 3 years ago

As mentioned @bobpaul once merged upstream this could be a thing. Right now it would make me an unofficial maintainer of something I only encountered when helping a friend. I saw some problems (for us), and came up with a solution to fix them, and then offered what we had. It sounds like a great suggestion for a next step. I Think if we could get this merged first, then it would be easier to make a tiny patch referencing this comment on this issue. I'll add it to the top as an action item.

Rob--W commented 3 years ago

I'm planning to prepare a new release soonish (but not urgent), and am considering to accept the requested functionality.

The branch is doing too many unrelated things. Could you split it in separate pull requests so that they can be reviewed and merged independently? Please keep the number of unnecessary changes to a minimum (and have meaningful commits; otherwise I would need to squash them all into one commit). I suggest the following order (the Heroku one-click deploy last because it depends on the renamed variable).

  1. PR to rename "blacklist" to "blocklist" and "whitelist" to "allowlist" - with proper capitalizations (not just a blind search-and-replace like you've done right now). Do not print a deprecation warning, I don't want to force unnecessary warnings on existing users.
  2. PR to add the Heroku one-click deploy button.

I'm not sure what you mean by "suggested improvement. Specify repository, so source code updates can ship to heroku and heroku local-machine git can be used.". Why is that part of your branch? FYI: My heroku demo runs a fork of the master branch (with patches on top to enforce restrictions to comply with the policy). That use case should not be endangered by the proposed changes.

pierre1590 commented 3 years ago

Hi, I have the following code: const res = await fetch(https://cors-anywhere.herokuapp.com/${url}); const data = await res.json();

how can I fix the 403 (Forbidden) error?

Rob--W commented 3 years ago

@Lewiscowles1986 I just noticed that you added a commit there, but there is no open pull request to review. If your patches are ready for review, please open a PR.

@pierre1590 Your question is unrelated to this issue, please don't post unrelated comments on random issues. For your specific issue, see the pinned issue for the explanation.