Open thib3113 opened 7 months ago
What is your proposal?
@icebob in fact ... just let the user the responsibility of getting the ip .
Else, we need to manage trusted proxies, and so parse the x-forwarded-for header, to extract the first "not trusted proxy" . Also, sometimes "trusted proxies" can be a cidr, so, we need to check if the ips come from the cidr .... not an esay task .
Also, I think managing trusted proxies (like express do), can be interesting for moleculer-web … but It's not an easy task
Ok, but as you see, it's covered, because this logic just a default value, you can overwrite it in rateLimit
options.
yes I know for the default value . But if it's only a default value, I think it's better to don't use the header req.headers["x-forwarded-for"]
.
Users without reverse proxy will be secure, and user with reverse proxy will need to adapt correctly depending on the reverse proxy configuration
Just found this reading the code :
https://github.com/moleculerjs/moleculer-web/blob/master/src/index.js#L1351C47-L1351C47
getting ip from
x-forwarded-for
without checking ifconnection.remoteAddress
is a trusted proxy is a security problem .because I can send the header
x-forwarded-for
manually .( not a big problem, but keep in mind that someone can bypass the rate limiter ) .
Also, while headers are lowercase in node.js, this is not the case for all implementations. For example, node.js in AWS Lambda does not convert headers to lowercase.