Target restriction #111
Target restriction #111
Conversation
| * function `checkRateLimit` - If set, it is called with the origin (string) of the request. If this | ||
| function returns a non-empty string, the request is rejected and the string is send to the client. | ||
| * boolean `redirectSameOrigin` - If true, requests to URLs from the same origin will not be proxied but redirected. | ||
| The primary purpose for this option is to save server resources by delegating the request to the client | ||
| (since same-origin requests should always succeed, even without proxying). | ||
| * array of strings `requireHeader` - If set, the request must include this header or the API will refuse to proxy. | ||
| Recommended if you want to prevent users from using the proxy for normal browsing. |
There was a problem hiding this comment.
Please undo the removal of every two spaces at the end ( ). These are necessary to force a line break. After your line the Example: ... is not on a separate line any more.
| res.end('The target "' + target + '" was not whitelisted by the operator of this proxy.'); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
This check can be bypassed if the whitelisted target site has an open redirect. A better place to enforce this is in the proxyRequest function.
Locally I started with something like this (many months ago), but I never got to finish the implementation.
function proxyRequest(req, res, proxy) {
var location = req.corsAnywhereRequestState.location;
+ // TODO: Add something like this?
+ // For https://github.com/Rob--W/cors-anywhere/issues/67
+ if (req.corsAnywhereRequestState.checkRequestAllowed &&
+ !req.corsAnywhereRequestState.checkRequestAllowed(location)) {
+ res.writeHead(403, 'Forbidden', withCORS({}, req));
+ res.end('The requested resource was blocked by the operator of this proxy.');
+ return;
+ }The idea behind this is that the validator can be as flexible as anyone wants to, with some default implementation in server.js that simply looks at the environment variables and the prefix (like you're doing right now).
Matching by prefix is the simplest, but is it sufficient for most use cases? I recall another bug that wanted to match by "file extension".
| .expect(403, done); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Also add tests where the request to a whitelisted target is redirected. One test for redirection to a whitelisted target, and another test for redirection to a blacklisted target.
The server can now take a list of target servers (in the form of both whitelist and blacklist) it can serve.