-
-
Notifications
You must be signed in to change notification settings - Fork 210
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
fix: enable "real" client IP visibility even behind proxies #2963
Conversation
To enable monitoring via fail2ban as discussed in zwave-js#2933. Mirrors similar code addressing same feature for the zwave logs as per zwave-js/node-zwave-js#3796.
Pull Request Test Coverage Report for Build 4174501850
💛 - Coveralls |
I think it would be better to enable this based on an env var. @chrisns agree? |
I don't think we actually care in zwjs-ui what the ip is, apart from maybe logging? agree, if we're going to define it must be an env var / config setting trust to I suspect most users don't have this behind a proxy (which is why auth exists within the app etc) |
moved to draft, if its changed to be configurable, somewhere then it can be ready to review again. |
Got your points, thank you. I'll resubmit based on an environment variable configuration (with default to 'false' so that it is really a user conscious decision) |
On thinking further about this « origin ip » topic, I think it is better to withdraw that pull request indeed since it does not bring much but creates other risks. Apologies. |
Pull Request Test Coverage Report for Build 4174501850Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Complements b1dc1a4