Skip to content

Conversation

harpm
Copy link

@harpm harpm commented Apr 24, 2025

Added Flag AllowAnanymousIPs to the SimpleTcpServerSettings in order to prevent anonymous connections even when PermittedIPs list is null or empty when needed.

scenario:
- Sometimes we need to prevent all connections but PermittedIPs for security reasons and PermittedIPs are still empty, So there must be a flag that prevent connections in this scenario
- Also there is a time we have some PermittedIPs but we don't need to prevent new connections, again we need a flag for this exceptional scenario

@jchristn
Copy link
Owner

Hi @harpm I appreciate the contribution to the project. I'm a little unsure of how to proceed with it. The intent of the permitted IP list is to specify who can connect. If it's set, it's checked. If it's not, it isn't checked. This seems like a bypass for a very specific use case and I don't know that it would be widely applicable to others.

@harpm
Copy link
Author

harpm commented Apr 28, 2025

Hi @harpm I appreciate the contribution to the project. I'm a little unsure of how to proceed with it. The intent of the permitted IP list is to specify who can connect. If it's set, it's checked. If it's not, it isn't checked. This seems like a bypass for a very specific use case and I don't know that it would be widely applicable to others.

First of all thank you for putting time on reviewing my pull request and my issue; the problem with the old approach is that for the time that we need to prevent any anonymous connection for security reasons the only thing I can do is to add a fake IpPort to the permitted list that reduces my codes readability and also in the near future someone else may work on that code and know nothing about the reason I did this, in addition somewhere else in the code someone may clear the list for other logic or business and this may create side-effects and conflict between these parts of code.
But this is understandable for me that other repositories which uses this package might be affected by this change, So I'll take a deeper look and make a table for different possibilities in order to see in which circumstance we can have both of conditions working like before and also have the feature I'm talking about.

…se with AllowAnonymous flag you need to change the setting PermitType property
@harpm
Copy link
Author

harpm commented Apr 29, 2025

Hello again @jchristn
I have pushed new changes due to the concerns you have mentioned previously
please give me feedbacks in case of need
thanks for your attention

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

Successfully merging this pull request may close these issues.

2 participants