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

Authorization doesn't work with later versions of Clickhouse #42

Open
drapadubok opened this issue Jul 27, 2021 · 3 comments
Open

Authorization doesn't work with later versions of Clickhouse #42

drapadubok opened this issue Jul 27, 2021 · 3 comments

Comments

@drapadubok
Copy link

When trying to connect to 21.7.4.18 version of CH, I get the following:

:erlang.apply("Code: 516, e.displayText() = DB::Exception: Invalid authentication: 'Basic:' HTTP Authorization scheme is not supported (version 21.7.4.18 (official build))\n", :reason, [])
(clickhousex 0.5.0) lib/clickhousex/http_client.ex:103: Clickhousex.HTTPClient.decode_response/3
(clickhousex 0.5.0) lib/clickhousex/protocol.ex:37: Clickhousex.Protocol.connect/1
(db_connection 2.4.0) lib/db_connection/connection.ex:82: DBConnection.Connection.connect/2
(connection 1.1.0) lib/connection.ex:622: Connection.enter_connect/5
(stdlib 3.15.1) proc_lib.erl:226: :proc_lib.init_p_do_apply/3

Judging from the implementation, it seems that the expected header is different from the one supplied:
https://clickhouse.tech/codebrowser/html_report/ClickHouse/src/Server/HTTPHandler.cpp.html

E.g. changing from auth_header = {"Authorization", "Basic: #{auth_hash}"} -> auth_header = {"Authorization", "Basic #{auth_hash}"} should fix this. I can submit a PR, if this change makes sense, but I suspect this would affect backwards compatibility.

@sirikid
Copy link
Member

sirikid commented Aug 4, 2021

E.g. changing from auth_header = {"Authorization", "Basic: #{auth_hash}"} -> auth_header = {"Authorization", "Basic #{auth_hash}"} should fix this. I can submit a PR, if this change makes sense, but I suspect this would affect backwards compatibility.

Authorization is not currently covered by tests, it would be nice if you can write one.

@florius0
Copy link

@drapadubok Please, submit PR. It's an issue worth fixing IMO

@florius0
Copy link

Perhaps, making this string configurable may solve backwards compatibility problem

superhawk610 added a commit to superhawk610/clickhousex that referenced this issue Feb 7, 2023
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

No branches or pull requests

3 participants