-
Notifications
You must be signed in to change notification settings - Fork 72
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
adding IP range support to remoteAddress #58
Conversation
return true; | ||
} | ||
} | ||
if (range === context.remoteAddress) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: Maybe this should the the first if
statement. Feels like this could be faster than the range check?
Awesome 🔥 . Range-support has been on my wishlist for a while. We should port it to the other client implementations as well. I can release this as |
1 similar comment
released as 2.3.1 (https://www.npmjs.com/package/unleash-client) |
…#79) The `IpAddressMatcher` class has been taken from Spring Security 5.1.5.RELEASE (also Apache 2 licensed). Source: https://github.com/spring-projects/spring-security/blob/5.1.5.RELEASE/web/src/main/java/org/springframework/security/web/util/matcher/IpAddressMatcher.java Refs Unleash/unleash-client-node#58 Closes #47
…#79) The `IpAddressMatcher` class has been taken from Spring Security 5.1.5.RELEASE (also Apache 2 licensed). Source: https://github.com/spring-projects/spring-security/blob/5.1.5.RELEASE/web/src/main/java/org/springframework/security/web/util/matcher/IpAddressMatcher.java Refs Unleash/unleash-client-node#58 Closes #47
Adding a gazilion IPs manually is somewhat of a pain, therefor adding support for IPv4 ranges would really make things easier.
This PR uses the ip module to validate the ranges. I have not updated the docs on this as I wasn't sure if that should go into the PR.