fix: read X-Forwarded-For for client IP in security audit events#422
fix: read X-Forwarded-For for client IP in security audit events#422cschuerings wants to merge 2 commits intocap-js:mainfrom
Conversation
On reverse-proxy deployments (e.g. BTP Cloud Foundry), socket.remoteAddress returns the internal proxy IP, not the real client IP. Read X-Forwarded-For first and fall back to socket.remoteAddress when absent, consistent with how @sap/approuter resolves the client IP (lib/utils/logger.js:76). Fixes cap-js#421
KoblerS
left a comment
There was a problem hiding this comment.
According to https://expressjs.com/en/5x/api.html#req.ip req.ip should be set. Also verified locally using this example that this parameter is available:
const express = require("express");
const app = express();
const PORT = 3000;
app.get("/", (req, res) => {
console.log("Client IP:", req.ip);
res.send(`Your IP is: ${req.ip}`);
});
app.listen(PORT, () => {
console.log(`Server running on http://localhost:${PORT}`);
});| const ipAddress = | ||
| req.req?.headers?.["x-forwarded-for"] || req.req?.socket?.remoteAddress |
There was a problem hiding this comment.
| const ipAddress = | |
| req.req?.headers?.["x-forwarded-for"] || req.req?.socket?.remoteAddress | |
| const {ipAddress = req.req.ip |
| const ipAddress = | ||
| req.req?.headers?.["x-forwarded-for"] || req.req?.socket?.remoteAddress |
There was a problem hiding this comment.
| const ipAddress = | |
| req.req?.headers?.["x-forwarded-for"] || req.req?.socket?.remoteAddress | |
| const {ipAddress = req.req.ip |
|
Thanks for checking! However,
I verified this with a deployed test app (approuter → Express server) that logs all three values. When switching between networks, Network 1: "req.ip": "10.x.x.x",
"socket.remoteAddress": "10.x.x.x",
"x-forwarded-for": "203.x.x.x, 10.x.x.x, 52.x.x.x, 10.x.x.x"Network 2 (different hotspot): "req.ip": "10.x.x.x",
"socket.remoteAddress": "10.x.x.x",
"x-forwarded-for": "80.x.x.x, 10.x.x.x, 52.x.x.x, 10.x.x.x"The internal IPs stay constant regardless of which network the client is on, while the first entry in |
schiwekM
left a comment
There was a problem hiding this comment.
Thanks for noticing and please excuse that I overlooked the approuter scenario in the initial impl.
Description
Fixes #421.
On reverse-proxy deployments (e.g. BTP Cloud Foundry),
socket.remoteAddressreturns the internal proxy/Go Router IP, not the real client IP. This makes theipAddressfield in security audit events (AttachmentSizeExceeded,AttachmentUploadRejected,AttachmentDownloadRejected) useless for security forensics.Changes
Read
X-Forwarded-Forfirst and fall back tosocket.remoteAddresswhen the header is absent — consistent with how@sap/approuterresolves the client IP (lib/utils/logger.js):The full
X-Forwarded-Forvalue is logged as-is (may be comma-separated when multiple proxies are in the chain). Falls back tosocket.remoteAddressfor direct connections without a proxy (e.g. local development).