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

[Blacklist] Opposite of FilterByClientIpPlugin #1123

Closed
LetMeR00t opened this issue Apr 19, 2022 · 12 comments
Closed

[Blacklist] Opposite of FilterByClientIpPlugin #1123

LetMeR00t opened this issue Apr 19, 2022 · 12 comments
Assignees
Labels
Proposal Proposals for futuristic feature requests

Comments

@LetMeR00t
Copy link
Contributor

LetMeR00t commented Apr 19, 2022

Hello everyone,

First, thank you for this very complete library which brings a lot of features regarding proxy and also quite fast to understand how it works.
I plan to use this proxy in order to restrict the access to several ressources only to specific IPs.
I know you already have the plugin "FilterByClientIpPlugin" but this is used to blacklist specific IPs to access when I would like to do the opposite meaning allow the access to only a specific list of source IPs.
Is it something already implemented ?
If not, and if you are interested, I could copy/paste and adapt the "FilterByClientIpPlugin" to do so (by having a new dedicated plugin or by reviewing the logic behind this plugin (best option from my point of view).

Thank you

@abhinavsingh
Copy link
Owner

@LetMeR00t Thanks a lot. Indeed, your best bet is copy paste the existing plugin and modify for your needs. Also, IIUC, you probably are thinking that plugins must be within proxy.py repo. If so, that is not the case.

You can keep your plugins private in your own repo. See skeleton app example for a starter template.

@abhinavsingh abhinavsingh added the Question Questions related to proxy server label Apr 19, 2022
@LetMeR00t
Copy link
Contributor Author

Thank you @abhinavsingh for your quick answer.
Is it something you would be interested to have within a PR regarding this ? I don't like to use a custom version a plugin, I prefer to use builtins plugins :)
If you are interested, I can work on this and propose you a PR.

@abhinavsingh
Copy link
Owner

@LetMeR00t Sounds good. What we can do is:

  1. Add a new flag , let's say --allowed-client-ips in existing plugin https://github.com/abhinavsingh/proxy.py/blob/develop/proxy/plugin/filter_by_client_ip.py

  2. Update the logic to also accomodate --allowed-client-ips flag values. Idea will be to respect both --filtered-client-ips (blacklist) and --allowed-client-ips (whitelisted) flags.

For obvious reasons, an IP in blacklist will supersede values in whitelisted IP list.

Wdyt?

@LetMeR00t
Copy link
Contributor Author

I think it's quite clear.
May I propose you maybe another solution and see which one you prefer?

  1. Add a new flag , let's say --filtered-client-ips-mode in existing plugin https://github.com/abhinavsingh/proxy.py/blob/develop/proxy/plugin/filter_by_client_ip.py

  2. Update the logic to also accomodate --filtered-client-ips-mode flag values. The mode could be either "Whitelist" or either "Blacklist" and will apply the logic based on the --filtered-client-ips data.
    If the mode is set to "Blacklist" (default mode), it will block all IPs indicated within the flag --filtered-client-ips
    If the mode is set to "Whitelist", it will allow only IPs indicated within the flag --filtered-client-ips

What is your opinion on that ?

@abhinavsingh
Copy link
Owner

Sounds perfect to me. Defaults value will also preserve the current behavior. That will be awesome. Go for it please 🙏

@LetMeR00t
Copy link
Contributor Author

Is it possible to enable login for a plugin ? I've tried to import the logging library but it seems not working or maybe it's because it's handled by one of the acceptor instead of the main script ?
I want to debug the current situation regarding the plugin but I can't get so much information as I can't log what I want :) So I prefer to ask
Thank you

@abhinavsingh
Copy link
Owner

@LetMeR00t Yes there is an issue with logging in general. It works in some environment and Python versions and it silently fails on others. May be you could just print it out for now. I am sorry, fixing logging in a multiprocess environment will be another task in itself. We will likely use proxy.py inbuilt event publish/subscribe mechanism to build it.

@LetMeR00t
Copy link
Contributor Author

May you help me to distinguish "inputs_args" from "opts" for the "initialize" function of the main class Proxy ?
I understood how worked the opts field of course but for the "input_args", it seems to be a list of string.
Is every string must follow the rule "--field value" ? I was expecting a dictionary in fact, not a list of strings :)

@abhinavsingh
Copy link
Owner

abhinavsingh commented Apr 20, 2022

To add --my-flag doesn't need any special treatment. But, if you want to use my_flag as a kwarg to Proxy constructor you will run into opt requirement.

Unfortunately, this is still tied into a core file and you will end up fiddling with that file.

A better approach would be to keep using --my-flag and pass then as *args to constructor instead of **kwargs. All *args will go through Python argparser giving to access to flags.my_flag in ur code.

Let me know if it works or ain't clear still.

TL;DR -- Try to use flags and args, not kwargs because it requires opt treatment

@abhinavsingh
Copy link
Owner

@LetMeR00t For core plugins it is ok to inject opt treatment into the code. This is done within initialize method here https://github.com/abhinavsingh/proxy.py/blob/develop/proxy/common/flag.py#L94

I started a PR to decouple opt and initialize from the code. #919. Unfortunately I never got back to it. May be in future soon :)

Let me know.

@LetMeR00t
Copy link
Contributor Author

Thank you for your feedback. As you will see, I've made the first version of the PR: #1127

@abhinavsingh abhinavsingh added Proposal Proposals for futuristic feature requests and removed Question Questions related to proxy server labels Apr 20, 2022
@abhinavsingh
Copy link
Owner

v2.4.2 is out now with these enhancements. Closing this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposals for futuristic feature requests
Projects
None yet
Development

No branches or pull requests

2 participants