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

feat(policies): blacklist/whitelist add support for IPv6, CIDR, ranges. #2027

Merged
merged 5 commits into from
May 13, 2022

Conversation

msavy
Copy link
Member

@msavy msavy commented May 10, 2022

No description provided.

@msavy msavy self-assigned this May 10, 2022
@msavy
Copy link
Member Author

msavy commented May 11, 2022

apiman-new-blocklist.mov

@msavy msavy marked this pull request as ready for review May 11, 2022 21:25
@msavy msavy requested a review from volkflo May 11, 2022 21:25
@msavy
Copy link
Member Author

msavy commented May 11, 2022

BTW, this has been implemented in a way that should be fully backwards compatible (the parser on the backend infers whether it's IPv6 vs IPv4, there's some simple logic to pick out ranges, etc.).

So, please test out any existing rulesets you have.

@volkflo
Copy link
Member

volkflo commented May 12, 2022

@msavy There might be a problem with the cache as the cache entries are never invalidated.

Currently if I make a request that should pass the IP is added into the cache as (IP, false) which is okay.
Later I decide to add a rule to the policy that should block this IP but the IP is not blocked because the cache still contains the old entry.
After I restart the gateway the cache is clean again and the IP is added as (IP, true).

@msavy
Copy link
Member Author

msavy commented May 12, 2022

@msavy There might be a problem with the cache as the cache entries are never invalidated.

Currently if I make a request that should pass the IP is added into the cache as (IP, false) which is okay. Later I decide to add a rule to the policy that should block this IP but the IP is not blocked because the cache still contains the old entry. After I restart the gateway the cache is clean again and the IP is added as (IP, true).

Thanks. I will double check my logic for the cache keys, I was meant to include the configuration in the key (hence adding a hash function to the config!), but looks like I forgot about it at some point.

@msavy msavy force-pushed the whitelist-blacklist-ranges-and-ipv6 branch from abb36bb to 5b149c0 Compare May 12, 2022 10:21
@msavy
Copy link
Member Author

msavy commented May 12, 2022

@volkflo please try again 👍

@volkflo
Copy link
Member

volkflo commented May 13, 2022

Okay tested it again, as far as I can tell it works fine now 👍
These changes seem like a really nice addition to the policy!

<button id="add" ng-disabled="!ipAddress || isEntityDisabled()" ng-click="add(ipAddress)" data-field="add" apiman-i18n-key="add" class="btn btn-default" style="min-width: 75px">Add</button>
</div>
<!-- Use ng-model-options allowInvalid, otherwise value is not set until it passes validation, meaning we can't display any useful hints/prompts for user -->
<input id="ip-address" name="ipinput" ng-model="ipAddress" data-field="ipAddress" class="form-control" style="max-width: 300px; float: left; margin-right: 5px" type="text" apiman-i18n-key="iplist.enter-ip-address" placeholder="Enter an IP address..." ng-disabled="isEntityDisabled()" ng-model-options="{ updateOn: 'default', allowInvalid:'true' }" validate-ip-address/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@msavy Can we try to avoid these inline styles, as they are really annoying to overwrite if someone wants to do a little bit of customization

Copy link
Member Author

@msavy msavy May 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to completely re-format the TS and HTML of the project once we've done 3.0.0.Final (using prettier or similar).

I agree having very long lines like this makes it difficult to spot the difference (I only added name and changed width slightly.

@msavy
Copy link
Member Author

msavy commented May 13, 2022

Thanks for testing and feedback, will merge today.

@msavy msavy changed the title Blacklist/whitelist support for IPv6, CIDR, ranges, etc feat(policies): blacklist/whitelist add support for IPv6, CIDR, ranges. May 13, 2022
@msavy msavy merged commit 759def9 into apiman:master May 13, 2022
@msavy msavy deleted the whitelist-blacklist-ranges-and-ipv6 branch May 13, 2022 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add range, CIDR, etc, support for whitelist/blacklist policy IPv6 Support for allow/deny policies
2 participants