-
Notifications
You must be signed in to change notification settings - Fork 253
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
[Feature] Configuration option to allow passing all query parameters #338
Comments
Hi, we've discussed a bit with the maintainers.
It will take time to see if there are other side effects with the params and if we can avoid the issue with the cache by reworking the way the key is generated. In the meantime, if the topic is urging, you can create a PR by adding an |
Thanks for looking into it! I understand the cache key concern and I'm not sure what the right balance is without adding a lot of complexity. I'll see about doing a fairly simple PR as you suggested for |
one advice, you should wait before the PR #330 is merged (should be merge in max a few days) because it will be in conflict with some parts of the code you need to modify |
FYI: I have merged #330 so this should no longer block you. |
Is your feature request related to a problem? Please describe.
Currently CHProxy severely restricts the query parameters that can be passed to ClickHouse server for "security reasons". This was the source of a major incompatibility with the official Python client (ClickHouse/clickhouse-connect#191), and also cripples a lot of other behavior for HTTP based ClickHouse clients. The workaround by changing settings at the user/params_group level is somewhat clunky and incomplete.
This sort of security issue seems better handled at the client and not the proxy level in any case.
Describe the solution you'd like
Checking the allowed parameters list here should be optional and controlled by a configuration setting.
Describe alternatives you've considered
Setting user level parameters in parameter groups is insufficient, since it reduces flexibility and doesn't work with, for example, query parameters used for ClickHouse server side binding. Another possibility might be adding another configuration option to dynamically add to the "allowed parameter names", but this again adds additional complexity and overhead with little apparently "security" benefit.
The text was updated successfully, but these errors were encountered: