Let the choice to write credential for HTTP source on external dictionaries #7092
Conversation
|
|
||
| if (config.has(credentials_prefix)) | ||
| { | ||
| this->credentials.setUsername(config.getString(credentials_prefix + ".user", "")); |
There was a problem hiding this comment.
this-> qualification looks unneeded.
| @@ -86,6 +86,10 @@ namespace detail | |||
| template <typename UpdatableSessionPtr> | |||
| class ReadWriteBufferFromHTTPBase : public ReadBuffer | |||
| { | |||
| public: | |||
| using HttpHeaderEntry = std::tuple<std::string, std::string>; | |||
There was a problem hiding this comment.
We prefer HTTP, not Http.
| @@ -84,6 +84,16 @@ Example of settings: | |||
| <http> | |||
| <url>http://[::1]/os.tsv</url> | |||
| <format>TabSeparated</format> | |||
| <credentials> | |||
There was a problem hiding this comment.
Need to mention that it's optional.
| <user>user</user> | ||
| <password>password</password> | ||
| </credentials> | ||
| <http-headers> |
There was a problem hiding this comment.
Maybe we can omit http- here because it's http source.
| @@ -84,6 +84,16 @@ Example of settings: | |||
| <http> | |||
| <url>http://[::1]/os.tsv</url> | |||
There was a problem hiding this comment.
Why we cannot include credentials in URL:
http://user:password@[::1]/os.tsv
There was a problem hiding this comment.
I suspect a bit the code who handle authentication to not handle it if we set the auth on the URI, I tried to do some test locally with an HTTP server, the server returned a 401 error if we put the auth on the URL.
https://github.com/ClickHouse/ClickHouse/blob/master/dbms/src/IO/ReadWriteBufferFromHTTP.h#L112-L113
YiuRULE
force-pushed
the
acl_file_storage
branch
from
663d707
to
53b10d5
Compare
|
Ok done :) |
|
Ok. Now it's almost ready! Look at or
|
YiuRULE
force-pushed
the
acl_file_storage
branch
from
7d66ab9
to
7c93ef1
Compare
|
Done ! :) I modified one integration test where the http server do a simili-authentication system |
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
For changelog. Remove if this is non-significant change.
Category (leave one):
Short description (up to few sentences):
Set two configuration options for a dictionary based on an HTTP source.
credentialsandhttp-headers.Detailed description (optional):
Two options has been made, mostly for improve security when we use a private dataset using an HTTP dictionary.
The first one is
credentials, used when a server use a basic http auth. With an user and a password.The second is
http-headersfor set some datas on the HTTP request header. The motivation of it is for service who can possibly get an API key from an header, to be able let ClickHouse get the datas.