Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trust Proxy settings not being respected #3118

Closed
sanchitgera opened this issue Nov 2, 2016 · 7 comments
Closed

Trust Proxy settings not being respected #3118

sanchitgera opened this issue Nov 2, 2016 · 7 comments
Assignees

Comments

@sanchitgera
Copy link

sanchitgera commented Nov 2, 2016

I have set up an Express app with Morgan for logging behind an NGINX proxy. I have configured the trust proxy setting in my root file like so:

app.set('trust proxy', true);

let controllers = require('./controllers');
app.use('/', controllers); 

if (process.env.NODE_ENV !== 'test') {
    app.use(morgan('combined', {
      'stream': logger.stream
    }));
}

But when I actually run the app and start making requests, I see the following error trace:

   "stack":[  
      "TypeError: trust argument is required",
      "    at proxyaddr (/home/cadror_deploy/app/gatekeeper/staging/releases/20161101222206/node_modules/proxy-addr/index.js:259:11)",
      "    at IncomingMessage.ip (/home/cadror_deploy/app/gatekeeper/staging/releases/20161101222206/node_modules/express/lib/request.js:328:10)",
      "    at Function.getip [as remote-addr] (/home/cadror_deploy/app/gatekeeper/staging/releases/20161101222206/node_modules/morgan/index.js:458:13)",
      "    at eval (eval at compile (/home/cadror_deploy/app/gatekeeper/staging/releases/20161101222206/node_modules/morgan/index.js:1:0), <anonymous>:4:29)",
      "    at Array.logRequest (/home/cadror_deploy/app/gatekeeper/staging/releases/20161101222206/node_modules/morgan/index.js:122:18)",
      "    at listener (/home/cadror_deploy/app/gatekeeper/staging/releases/20161101222206/node_modules/on-finished/index.js:169:15)",
      "    at onFinish (/home/cadror_deploy/app/gatekeeper/staging/releases/20161101222206/node_modules/on-finished/index.js:100:5)",
      "    at callback (/home/cadror_deploy/app/gatekeeper/staging/releases/20161101222206/node_modules/ee-first/index.js:55:10)",
      "    at ServerResponse.onevent (/home/cadror_deploy/app/gatekeeper/staging/releases/20161101222206/node_modules/ee-first/index.js:93:5)",
      "    at emitNone (events.js:85:20)",
      "    at ServerResponse.emit (events.js:179:7)",
      "    at finish (_http_outgoing.js:551:10)",
      "    at _combinedTickCallback (node.js:376:9)",
      "    at process._tickDomainCallback (node.js:431:11)"
   ],
   "level":"error",
   "message":"uncaughtException: trust argument is required",
   "timestamp":"2016-11-02T18:20:20.026Z"

It appears that Morgan attempts to get the ip address of the incoming request using the getter defined by Express, but fails because the trust argument is set to false. Any thoughts?

I have also tried wrapping the second argument in `app.set('trust proxy' in a function but that doesn't make a difference. Thanks!

@dougwilson
Copy link
Contributor

Hi @sanchitgera your stack trace shows node_modules/express/lib/request.js:328:10, but in the current version (4.14.0), line 328 is empty. What version of Express are you using?

@sanchitgera
Copy link
Author

I'm on a slightly older version. 4.13.4 @dougwilson

@dougwilson
Copy link
Contributor

Thanks, @sanchitgera I tried with 4.13.4 using your information above, but had no issue logging with Morgan through NGINX. I had to guess a lot about what is happening. Is there any additional information you can provide to help me reproduce the issue?

@sanchitgera
Copy link
Author

I'm afraid I can't share too much from the codebase. :( But the error isn't very consistent. I don't get the error on every request. But from what I can tell, there is nothing inherently different about the requests that do lead to the error.

I have a custom authentication scheme in place relying on specific Authorization headers. So if I were to make a request like so:
GET /app/route1 without passing in any headers, the request is processed normally with a 400 response and no logging errors.
But if I were to provide the correct headers, I see the error mentioned above.

@dougwilson
Copy link
Contributor

Ah, I see @sanchitgera . I'm afraid I'm not sure how I should proceed here, unless you have any suggestions?

@sanchitgera
Copy link
Author

sanchitgera commented Nov 2, 2016

That's alright! Thanks for all your help! I really appreciate it.
I wrote a custom logger to replace morgan and that seems to be working fine. I think it's just that call to req.ip that messes things up somehow but I can't pinpoint what exactly causes it. If I can better repro steps I'll let you know! :-)

@dougwilson
Copy link
Contributor

Yea, I would love to figure it out, if you can even post a simplified app that I can use to reproduce the issue. As for morgan, you can always overwrite the :remote-addr token to do whatever you want to get the IP.

@dougwilson dougwilson self-assigned this Nov 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants