basic access authentication for http/https #31648

michael1589 · 2021-11-23T03:00:24Z

Changelog category:

New Feature

Changelog entry:

Basic access authentication for http/url functions

Detailed description / Documentation draft:

issue here: Basic Auth for http/url functions not supported

Basic auth for http/https is not supported before 21.11. You can see this error such like:

root@b0ebdb3a98f3:/# clickhouse-client --query="select * FROM url('https://guest:guest@jigsaw.w3.org/HTTP/Basic/','RawBLOB', 'a String');"
Received exception from server (version 21.11.3):
Code: 86. DB::Exception: Received from localhost:9000. DB::Exception: Received error from remote server /HTTP/Basic/. HTTP status code: 401 , body: <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
               "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
 <title>Unauthorized</title>
</head>
<body>
<h1>Unauthorized access</h1><p>You are denied access to this resource.</body>
</html>: While executing URL. (RECEIVED_ERROR_FROM_REMOTE_IO_SERVER)
(query: select * FROM url('https://guest:guest@jigsaw.w3.org/HTTP/Basic/','RawBLOB', 'a String');)

Now we can access a format avro schema registry url like this:

peng-liudemacbook-pro.local :) select * from url('https://guest:guest@127.0.0.1:32345', CSV, 'word String, value UInt64');

SELECT *
FROM url('http://guest:guest@127.0.0.1:32345', CSV, 'word String, value UInt64')

Query id: 63625678-f39b-45c6-9379-09589168f940

┌─word──┬─value─┐
│ Hello │     1 │
│ World │     2 │
└───────┴───────┘

2 rows in set. Elapsed: 0.007 sec.

Basic auth for http/https is supported now.
HTTP Authentication: Basic and Digest Access Authentication

CLAassistant · 2021-11-23T03:00:30Z

All committers have signed the CLA.

src/Storages/StorageURL.cpp

kssenii · 2021-11-23T07:26:15Z

src/Storages/StorageURL.cpp

@@ -136,6 +148,7 @@ namespace
                                timeouts,
                                credentials,
                                context->getSettingsRef().max_http_get_redirects,
+                                userInfo.length() == 0 ? Poco::Net::HTTPBasicCredentials{} : Poco::Net::HTTPBasicCredentials(ostr.str()),


A little strange, this is now not consistent with ReadWriteBufferFromHTTP constructor, I think build check will fail now.
Probably merged with master incorrectly?

OK, I will chang to if...else...patten.

No this is what I meant, credentials arguments is passed 2 lines above. See

ClickHouse/src/IO/ReadWriteBufferFromHTTP.h

Lines 466 to 473 in 341f705

explicit ReadWriteBufferFromHTTP(

Poco::URI uri_,

const std::string & method_,

OutStreamCallback out_stream_callback_,

const ConnectionTimeouts & timeouts,

const Poco::Net::HTTPBasicCredentials & credentials_,

const UInt64 max_redirects = 0,

size_t buffer_size_ = DBMS_DEFAULT_BUFFER_SIZE,

.

Got it! It was a mistake of merge codes. I will push again.

kssenii · 2021-11-23T07:27:08Z

tests/queries/0_stateless/02114_basic_acess_auth.sql

@@ -0,0 +1 @@
+select * FROM url('https://guest:guest@jigsaw.w3.org/HTTP/Basic/','RawBLOB', 'a String');


Also I believe this test will not work in ci from docker.

Why? Can the docker evironment access outside internet such as www.jigsaw.w3.org?

Even if it will be ok, this is not good.
Can you please try to use http server in python in stateless tests like here https://github.com/ClickHouse/ClickHouse/blob/master/tests/queries/0_stateless/00646_url_engine.python?

OK, I will write a python script to run this test later.

michael1589 · 2021-11-28T14:19:17Z

/ClickHouse/src/Storages/StorageURL.cpp:135: if(n != std::string::npos)
This check fails, can you tell me why, pls?

kssenii

/ClickHouse/src/Storages/StorageURL.cpp:135: if(n != std::string::npos)
This check fails, can you tell me why, pls?

When you run this locally, did you try to print user_info string and see how it looks?

kssenii · 2021-11-29T06:34:58Z

src/Storages/StorageURL.cpp

@@ -128,6 +128,17 @@ namespace

                    try
                    {
+                        std::string userInfo = request_uri.getUserInfo();


Suggested change

std::string userInfo = request_uri.getUserInfo();

std::string user_info = request_uri.getUserInfo();

Yes, I do. userInfo string is "guest:guest" when input "https://guest:guest@localhost:someport/".
By the way, FunctionalStatelessTestAsan fails(01622_defaults_for_url_engine.sh). I think that's none business with my commit.

2021-11-29 01:11:23 /usr/share/clickhouse-test/queries/0_stateless/01622_defaults_for_url_engine.sh: line 38: 67794 Killed timeout $TIMEOUT bash -c "thread1 $PORT" > /dev/null 2>&1
2021-11-29 01:11:23
2021-11-29 01:11:23 stdout:
2021-11-29 01:11:23 Ok
2021-11-29 01:11:23
2021-11-29 01:11:23
2021-11-29 01:11:23 Database: test_momh37

Actually looks like it must work, are you sure that the problem is in if(n != std::string::npos)?

I think that's none business with my commit.

yes, it is not related.

userInfo string stores "username:password" string. Now the condition judge( if(n != std::string::npos)) passed the test in the file src/Storages/StorageURL.cpp. I think the other fails are none business with my commit.

try { std::string userInfo = request_uri.getUserInfo(); if (!userInfo.empty()) { std::size_t n = userInfo.find(":"); if(n != std::string::npos) { credentials.setUsername(userInfo.substr(0, n)); credentials.setPassword(userInfo.substr(n+1)); } }

/ClickHouse/src/Storages/StorageURL.cpp:135: if(n != std::string::npos)

This is style check, I did not get about what check were you talking about, and thought you solution fails on this if statement check...

You are just missing a space between if and (

Also build check is related:

src/CMakeFiles/dbms.dir/Storages/StorageURL.cpp.o.d -o src/CMakeFiles/dbms.dir/Storages/StorageURL.cpp.o -c ../src/Storages/StorageURL.cpp 2021-11-29 07:13:08 /build/obj-x86_64-linux-gnu/../src/Storages/StorageURL.cpp:131:37: error: invalid case style for local variable 'userInfo' [readability-identifier-naming,-warnings-as-errors] 2021-11-29 07:13:08 std::string userInfo = request_uri.getUserInfo(); 2021-11-29 07:13:08 ^~~~~~~~ 2021-11-29 07:13:08 user_info 2021-11-29 07:13:08 /build/obj-x86_64-linux-gnu/../src/Storages/StorageURL.cpp:134:59: error: 'find' called with a string literal consisting of a single character; consider using the more effective overload accepting a character [performance-faster-string-find,-warnings-as-errors] 2021-11-29 07:13:08 std::size_t n = userInfo.find(":"); 2021-11-29 07:13:08 ^~~ 2021-11-29 07:13:08 ':' 2021-11-29 07:13:08 91053 warnings generated.

Other checks are not related.

kssenii · 2021-11-30T12:39:32Z

Can you please fix build check, it is still broken

src/CMakeFiles/dbms.dir/Storages/StorageURL.cpp.o.d -o src/CMakeFiles/dbms.dir/Storages/StorageURL.cpp.o -c ../src/Storages/StorageURL.cpp
2021-11-30 08:16:47 /build/obj-x86_64-linux-gnu/../src/Storages/StorageURL.cpp:131:37: error: invalid case style for local variable 'userInfo' [readability-identifier-naming,-warnings-as-errors]
2021-11-30 08:16:47                         std::string userInfo = request_uri.getUserInfo();
2021-11-30 08:16:47                                     ^~~~~~~~
2021-11-30 08:16:47                                     user_info
2021-11-30 08:16:47 91052 warnings generated.

michael1589 · 2021-11-30T12:56:06Z

Can you please fix build check, it is still broken

src/CMakeFiles/dbms.dir/Storages/StorageURL.cpp.o.d -o src/CMakeFiles/dbms.dir/Storages/StorageURL.cpp.o -c ../src/Storages/StorageURL.cpp
2021-11-30 08:16:47 /build/obj-x86_64-linux-gnu/../src/Storages/StorageURL.cpp:131:37: error: invalid case style for local variable 'userInfo' [readability-identifier-naming,-warnings-as-errors]
2021-11-30 08:16:47                         std::string userInfo = request_uri.getUserInfo();
2021-11-30 08:16:47                                     ^~~~~~~~
2021-11-30 08:16:47                                     user_info
2021-11-30 08:16:47 91052 warnings generated.

Sorry, finished a few minutes ago!

michael1589 · 2021-12-01T02:58:42Z

ClickHouse Functional Stateless Tests fail (02098_date32_comparison | FAIL). I think this fail is none business with my commit. And ClickHouse Stress Test(Hung check failed/FLAKY). Can you give me more info pls?

kssenii · 2021-12-01T06:47:42Z

Functional stateless tests (release, DatabaseReplicated) — fail: 1 -- not related to changes (02098_date32_comparison)
Stress test -- not related to changes.

michael1589 added 2 commits November 22, 2021 17:59

support HTTP/HTTPS basic access authentication

8222896

HTTP/HTTPS basic access authentication test

1ca6f7b

robot-clickhouse added doc-alert pr-feature Pull request with new product feature labels Nov 23, 2021

Merge branch 'master' into master

75b0c0a

kssenii self-assigned this Nov 23, 2021

kssenii added the can be tested label Nov 23, 2021

kssenii reviewed Nov 23, 2021

View reviewed changes

michael1589 added 8 commits November 23, 2021 20:21

Merge branch 'ClickHouse:master' into master

2bbb90e

Merge branch 'ClickHouse:master' into master

8b99138

fix merge error

fe501d4

Merge branch 'ClickHouse:master' into master

dbfe637

add test file for url basic auth

26a117c

style check fail

70264b3

Merge branch 'ClickHouse:master' into master

efeb9b5

for test url auth

082105f

michael1589 requested a review from kssenii November 28, 2021 13:24

style check fail

1bf49ab

delete old test files

b54caf9

kssenii reviewed Nov 29, 2021

View reviewed changes

michael1589 requested a review from kssenii November 30, 2021 06:00

michael1589 added 3 commits November 30, 2021 15:05

Merge branch 'ClickHouse:master' into master

93efc75

style check fail

3f56dde

style check fail

80dd01e

michael1589 added 3 commits November 30, 2021 20:46

build check fail

f2f8826

Merge branch 'ClickHouse:master' into master

92feada

Merge branch 'master' of https://github.com/michael1589/ClickHouse

a74cf9c

kssenii approved these changes Nov 30, 2021

View reviewed changes

michael1589 requested a review from kssenii December 1, 2021 01:23

kssenii merged commit 3ab014d into ClickHouse:master Dec 1, 2021

zhenzhongxu mentioned this pull request Dec 17, 2021

Kafka Engine returns 401 Authentication issue when connecting to Confluent schema registry with a URL-encoded basic authentication URL #32887

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

basic access authentication for http/https #31648

basic access authentication for http/https #31648

michael1589 commented Nov 23, 2021 •

edited

Loading

CLAassistant commented Nov 23, 2021 •

edited

Loading

kssenii Nov 23, 2021

michael1589 Nov 23, 2021

kssenii Nov 23, 2021

michael1589 Nov 24, 2021

kssenii Nov 23, 2021

michael1589 Nov 23, 2021

kssenii Nov 23, 2021

michael1589 Nov 24, 2021

michael1589 commented Nov 28, 2021

kssenii left a comment

kssenii Nov 29, 2021

michael1589 Nov 29, 2021 •

edited

Loading

kssenii Nov 29, 2021 •

edited

Loading

michael1589 Nov 30, 2021 •

edited

Loading

kssenii Nov 30, 2021

kssenii Nov 30, 2021

kssenii commented Nov 30, 2021

michael1589 commented Nov 30, 2021

michael1589 commented Dec 1, 2021

kssenii commented Dec 1, 2021

	explicit ReadWriteBufferFromHTTP(
	Poco::URI uri_,
	const std::string & method_,
	OutStreamCallback out_stream_callback_,
	const ConnectionTimeouts & timeouts,
	const Poco::Net::HTTPBasicCredentials & credentials_,
	const UInt64 max_redirects = 0,
	size_t buffer_size_ = DBMS_DEFAULT_BUFFER_SIZE,

		@@ -0,0 +1 @@
		select * FROM url('https://guest:guest@jigsaw.w3.org/HTTP/Basic/','RawBLOB', 'a String');

	std::string userInfo = request_uri.getUserInfo();
	std::string user_info = request_uri.getUserInfo();

basic access authentication for http/https #31648

basic access authentication for http/https #31648

Conversation

michael1589 commented Nov 23, 2021 • edited Loading

issue here: Basic Auth for http/url functions not supported

CLAassistant commented Nov 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michael1589 commented Nov 28, 2021

kssenii left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michael1589 Nov 29, 2021 • edited Loading

Choose a reason for hiding this comment

kssenii Nov 29, 2021 • edited Loading

Choose a reason for hiding this comment

michael1589 Nov 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kssenii commented Nov 30, 2021

michael1589 commented Nov 30, 2021

michael1589 commented Dec 1, 2021

kssenii commented Dec 1, 2021

michael1589 commented Nov 23, 2021 •

edited

Loading

CLAassistant commented Nov 23, 2021 •

edited

Loading

michael1589 Nov 29, 2021 •

edited

Loading

kssenii Nov 29, 2021 •

edited

Loading

michael1589 Nov 30, 2021 •

edited

Loading