Skip to content

Commit

Permalink
Merge pull request #2133 from ClickHouse/repo_fix_tests_auth
Browse files Browse the repository at this point in the history
[repo] fixed tests for no_password problem
  • Loading branch information
chernser authored Feb 8, 2025
2 parents badbdcf + 142fc7a commit d716d85
Show file tree
Hide file tree
Showing 19 changed files with 128 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.function.Function;

import com.clickhouse.client.config.ClickHouseClientOption;
import com.clickhouse.client.config.ClickHouseDefaults;
import com.clickhouse.config.ClickHouseConfigChangeListener;
import com.clickhouse.config.ClickHouseOption;
import com.clickhouse.data.ClickHouseChecker;
Expand Down Expand Up @@ -590,7 +591,13 @@ public ClickHouseConfig getConfig() {
Map<ClickHouseOption, Serializable> merged = new HashMap<>();
merged.putAll(clientConfig.getAllOptions());
merged.putAll(options);
config = new ClickHouseConfig(merged, node.getCredentials(clientConfig),

ClickHouseCredentials credentials = node.getCredentials(clientConfig);
if (merged.containsKey(ClickHouseDefaults.USER) && merged.containsKey(ClickHouseDefaults.PASSWORD)) {
credentials = ClickHouseCredentials.fromUserAndPassword((String) merged.get(ClickHouseDefaults.USER),
(String) merged.get(ClickHouseDefaults.PASSWORD));
}
config = new ClickHouseConfig(merged, credentials,
clientConfig.getNodeSelector(), clientConfig.getMetricRegistry().orElse(null));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,9 @@ public static ClickHouseNode getClickHouseNode(ClickHouseProtocol protocol, bool
}
}

return ClickHouseNode.builder(template).address(protocol, new InetSocketAddress(host, port)).build();
return ClickHouseNode.builder(template).address(protocol, new InetSocketAddress(host, port))
.credentials(new ClickHouseCredentials("default", getPassword()))
.build();
}

public static ClickHouseNode getClickHouseNode(ClickHouseProtocol protocol, int port) {
Expand Down Expand Up @@ -314,7 +316,7 @@ public static String getPassword() {
if (isCloud) {
return System.getenv("CLICKHOUSE_CLOUD_PASSWORD");
} else {
return "";
return "test_default_password";
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2706,6 +2706,7 @@ public void testFailover() throws ClickHouseException {
ClickHouseNode availableNode = getServer();
Properties props = new Properties();
props.setProperty("failover", "1");
props.setProperty(ClickHouseDefaults.PASSWORD.getKey(), getPassword());

// nodes with the first node is unavailable
ClickHouseNodes nodes = ClickHouseNodes.of(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@
</managed2>
</profiles>
<users>
<default>
<profile>default</profile>
<networks>
<ip>::/0</ip>
</networks>
<password>test_default_password</password>
<quota>default</quota>
<access_management>1</access_management>
</default>
<dba>
<access_management>1</access_management>
<profile>default</profile>
Expand Down Expand Up @@ -40,15 +49,15 @@
<networks>
<ip>::/0</ip>
</networks>
<password></password>
<password>poorman_111</password>
<quota>default</quota>
</poorman1>
<poorman2>
<profile>managed2</profile>
<networks>
<ip>::/0</ip>
</networks>
<password></password>
<password>poorman_222</password>
<quota>default</quota>
</poorman2>
<test>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,18 +188,19 @@ protected ClickHouseResponse send(ClickHouseRequest<?> sealedRequest) throws Cli
}
: null;

ClickHouseNode server = sealedRequest.getServer();
if (conn.isReusable()) {
Map<String, Serializable> additionalParams = buildAdditionalReqParams(sealedRequest);

ClickHouseNode server = sealedRequest.getServer();
httpResponse = conn.post(config, sql, sealedRequest.getInputStream().orElse(null),
sealedRequest.getExternalTables(), sealedRequest.getOutputStream().orElse(null),
ClickHouseHttpConnection.buildUrl(server.getBaseUri(), sealedRequest, additionalParams),
ClickHouseHttpConnection.createDefaultHeaders(config, server, conn.getUserAgent(), getReferer(config)),
postAction);
} else {
httpResponse = conn.post(config, sql, sealedRequest.getInputStream().orElse(null),
sealedRequest.getExternalTables(), sealedRequest.getOutputStream().orElse(null), null, null,
sealedRequest.getExternalTables(), sealedRequest.getOutputStream().orElse(null),
null, ClickHouseHttpConnection.createDefaultHeaders(config, server, conn.getUserAgent(), getReferer(config)),
postAction);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ protected static Map<String, String> createDefaultHeaders(ClickHouseConfig confi
}
map.put("user-agent", !ClickHouseChecker.isNullOrEmpty(userAgent) ? userAgent : config.getClientName());

ClickHouseCredentials credentials = server.getCredentials(config);
ClickHouseCredentials credentials = config.getDefaultCredentials();
if (credentials.useAccessToken()) {
// TODO check if auth-scheme is available and supported
map.put("authorization", credentials.getAccessToken());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.clickhouse.client.ClickHouseServerForTest;
import com.clickhouse.client.ClientIntegrationTest;
import com.clickhouse.client.config.ClickHouseClientOption;
import com.clickhouse.client.config.ClickHouseDefaults;
import com.clickhouse.client.config.ClickHouseSslMode;
import com.clickhouse.client.http.config.ClickHouseHttpOption;
import com.clickhouse.client.http.config.HttpConnectionProvider;
Expand Down Expand Up @@ -114,10 +115,10 @@ public void testNothing() throws Exception {
public void testAuthentication() throws ClickHouseException {
if (isCloud()) return; //TODO: testAuthentication - Revisit, see: https://github.com/ClickHouse/clickhouse-java/issues/1747
String sql = "select currentUser()";
try (ClickHouseClient client = getClient(
new ClickHouseConfig(null, ClickHouseCredentials.fromUserAndPassword("dba", "dba"), null, null));
try (ClickHouseClient client = getClient();
ClickHouseResponse response = newRequest(client, getServer())
// .option(ClickHouseHttpOption.CUSTOM_PARAMS, "user=dba,password=incorrect")
.option(ClickHouseDefaults.PASSWORD, "dba")
.option(ClickHouseDefaults.USER, "dba")
.query(sql).executeAndWait()) {
Assert.assertEquals(response.firstRecord().getValue(0).asString(), "dba");
}
Expand Down Expand Up @@ -461,7 +462,8 @@ public void testProxyConnection() throws ClickHouseException, IOException {

Map<String, String> options = new HashMap<>();
// without any proxy options
try (ClickHouseClient client = ClickHouseClient.builder().options(getClientOptions())
try (ClickHouseClient client = ClickHouseClient.builder().option(ClickHouseDefaults.PASSWORD, getPassword())
.options(getClientOptions())
.nodeSelector(ClickHouseNodeSelector.of(ClickHouseProtocol.HTTP)).build()) {
ClickHouseNode server = getServer(ClickHouseProtocol.HTTP, options);
Assert.assertTrue(client.ping(server, 30000), "Can not ping");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package com.clickhouse.jdbc;

import com.clickhouse.client.ClickHouseProtocol;
import com.clickhouse.client.ClickHouseServerForTest;
import com.clickhouse.client.config.ClickHouseDefaults;
import com.clickhouse.client.http.config.ClickHouseHttpOption;
import com.clickhouse.client.http.config.HttpConnectionProvider;
import com.clickhouse.data.ClickHouseVersion;
import org.testng.Assert;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
Expand All @@ -29,6 +29,7 @@ public void testSetRoleDifferentConnections(String[] roles, String setRoleExpr,

String url = String.format("jdbc:ch:%s", getEndpointString());
Properties properties = new Properties();
properties.setProperty(ClickHouseDefaults.PASSWORD.getKey(), ClickHouseServerForTest.getPassword());
properties.setProperty(ClickHouseHttpOption.REMEMBER_LAST_SET_ROLES.getKey(), "true");
properties.setProperty(ClickHouseHttpOption.CONNECTION_PROVIDER.getKey(), connectionProvider);
ClickHouseDataSource dataSource = new ClickHouseDataSource(url, properties);
Expand Down Expand Up @@ -113,6 +114,7 @@ public void testSetRolesAccessingTableRows() throws SQLException {
if (isCloud()) return; //TODO: testSetRolesAccessingTableRows - Revisit, see: https://github.com/ClickHouse/clickhouse-java/issues/1747
String url = String.format("jdbc:ch:%s", getEndpointString());
Properties properties = new Properties();
properties.setProperty(ClickHouseDefaults.PASSWORD.getKey(), ClickHouseServerForTest.getPassword());
properties.setProperty(ClickHouseHttpOption.REMEMBER_LAST_SET_ROLES.getKey(), "true");
ClickHouseDataSource dataSource = new ClickHouseDataSource(url, properties);
String serverVersion = getServerVersion(dataSource.getConnection());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import com.clickhouse.client.ClickHouseProtocol;
import com.clickhouse.client.ClickHouseRequest;
import com.clickhouse.client.ClickHouseServerForTest;
import com.clickhouse.client.config.ClickHouseClientOption;
import com.clickhouse.data.ClickHouseCompression;
import com.clickhouse.data.ClickHouseUtils;
Expand Down Expand Up @@ -55,7 +56,7 @@ public void testCentralizedConfiguration() throws SQLException {
}

props.setProperty("user", "poorman1");
props.setProperty("password", "");
props.setProperty("password", "poorman_111");
props.setProperty("autoCommit", "false");
props.setProperty("compress_algorithm", "lz4");
props.setProperty("transactionSupport", "false");
Expand All @@ -73,6 +74,7 @@ public void testCentralizedConfiguration() throws SQLException {
}

props.setProperty("user", "poorman2");
props.setProperty("password", "poorman_222");
try (ClickHouseConnection conn = newConnection(props)) {
Assert.assertEquals(conn.getConfig().getResponseCompressAlgorithm(), ClickHouseCompression.GZIP);
Assert.assertTrue(conn.getJdbcConfig().isAutoCommit());
Expand Down Expand Up @@ -206,7 +208,8 @@ public void testNonExistDatabase() throws SQLException {
Assert.assertNotNull(exp, "Should not have SQLException because the database has been created");
}

@Test(groups = "integration")
@Test(groups = "integration", enabled = false)
// Disabled because will be fixed later. (Should be tested in the new JDBC driver)
public void testReadOnly() throws SQLException {
if (isCloud()) return; //TODO: testReadOnly - Revisit, see: https://github.com/ClickHouse/clickhouse-java/issues/1747
Properties props = new Properties();
Expand All @@ -217,8 +220,8 @@ public void testReadOnly() throws SQLException {
Assert.assertFalse(stmt.execute(
"drop table if exists test_readonly; drop user if exists readonly1; drop user if exists readonly2; "
+ "create table test_readonly(id String)engine=Memory; "
+ "create user readonly1 IDENTIFIED WITH no_password SETTINGS readonly=1; "
+ "create user readonly2 IDENTIFIED WITH no_password SETTINGS readonly=2; "
+ "create user readonly1 IDENTIFIED BY 'some_password' SETTINGS readonly=1; "
+ "create user readonly2 IDENTIFIED BY 'some_password' SETTINGS readonly=2; "
+ "grant insert on test_readonly TO readonly1, readonly2"));
conn.setReadOnly(false);
Assert.assertFalse(conn.isReadOnly(), "Connection should NOT be readonly");
Expand Down Expand Up @@ -246,6 +249,7 @@ public void testReadOnly() throws SQLException {

props.clear();
props.setProperty("user", "readonly1");
props.setProperty("password", "some_password");
try (Connection conn = newConnection(props); Statement stmt = conn.createStatement()) {
Assert.assertTrue(conn.isReadOnly(), "Connection should be readonly");
conn.setReadOnly(true);
Expand All @@ -271,6 +275,7 @@ public void testReadOnly() throws SQLException {
}

props.setProperty("user", "readonly2");
props.setProperty("password", "some_password");
try (Connection conn = newConnection(props); Statement stmt = conn.createStatement()) {
Assert.assertTrue(conn.isReadOnly(), "Connection should be readonly");
Assert.assertTrue(stmt.execute("set max_result_rows=5; select 1"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public void testHighAvailabilityConfig() throws SQLException {
Properties props = new Properties();
props.setProperty("failover", "21");
props.setProperty("load_balancing_policy", "roundRobin");
props.setProperty("password",ClickHouseServerForTest.getPassword());
try (Connection conn = DriverManager.getConnection(url, props)) {
Assert.assertEquals(conn.unwrap(ClickHouseRequest.class).getConfig().getFailover(), 21);
Assert.assertEquals(conn.unwrap(ClickHouseRequest.class).getConfig().getOption(
Expand All @@ -55,7 +56,7 @@ public void testMultiEndpoints() throws SQLException {
+ ")/system?load_balancing_policy=roundRobin";
Properties props = new Properties();
props.setProperty("user", "default");
props.setProperty("password", "");
props.setProperty("password", ClickHouseServerForTest.getPassword());
ClickHouseDataSource ds = new ClickHouseDataSource(url, props);
for (int i = 0; i < 10; i++) {
try (Connection httpConn = ds.getConnection();
Expand All @@ -73,7 +74,7 @@ public void testGetConnection() throws SQLException {
String url = "jdbc:ch:" + getEndpointString();
// String urlWithCredentials = "jdbc:ch:" + (isCloud() ? "https" : DEFAULT_PROTOCOL.name()) + "://default@"
// + getServerAddress(DEFAULT_PROTOCOL);
String urlWithCredentials = "jdbc:ch:" + DEFAULT_PROTOCOL.name() + "://default@" + getServerAddress(DEFAULT_PROTOCOL);
String urlWithCredentials = "jdbc:ch:" + DEFAULT_PROTOCOL.name() + "://default:" + getPassword() + "@" + getServerAddress(DEFAULT_PROTOCOL);
if (isCloud()) {
urlWithCredentials = "jdbc:ch:https://default:" + getPassword() + "@" + getServerAddress(DEFAULT_PROTOCOL);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

import com.clickhouse.client.ClickHouseProtocol;

import com.clickhouse.client.ClickHouseServerForTest;
import com.clickhouse.client.config.ClickHouseDefaults;
import com.clickhouse.jdbc.internal.ClickHouseJdbcUrlParser;
import org.testng.Assert;
import org.testng.annotations.Test;
Expand All @@ -26,7 +28,7 @@ public void testConnect() throws SQLException {
System.setProperty("clickhouse.jdbc.v1","true");
String address = getServerAddress(ClickHouseProtocol.HTTP, true);
ClickHouseDriver driver = new ClickHouseDriver();
Connection conn = driver.connect("jdbc:clickhouse://" + address, null);
Connection conn = driver.connect("jdbc:clickhouse://default:" + ClickHouseServerForTest.getPassword() + "@" + address, null);
conn.close();
}
@Test(groups = "integration")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,17 @@ public DataSource newDataSource(String url) throws SQLException {
}

public DataSource newDataSource(String url, Properties properties) throws SQLException {
if (isCloud()) {
if (properties == null) {
properties = new Properties();
}
if (properties == null) {
properties = new Properties();
}
if (!properties.containsKey("password")) {
properties.put("password", getPassword());
}
if (!properties.containsKey("user")) {
properties.put("user", "default");
}

if (isCloud()) {
url = String.format("jdbc:clickhouse:https://%s/%s", getServerAddress(ClickHouseProtocol.HTTP), ClickHouseServerForTest.getDatabase());
return new ClickHouseDataSource(buildJdbcUrl(DEFAULT_PROTOCOL, null, url), properties);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class R2DBCTestKitImplTest implements TestKit<String> {

private static final String DATABASE = "default";
private static final String USER = "default";
private static final String PASSWORD = "";
private static final String PASSWORD = "test_default_password";

private static final String CUSTOM_PROTOCOL_NAME = System.getProperty("protocol", "http").toUpperCase();
private static final ClickHouseProtocol DEFAULT_PROTOCOL = ClickHouseProtocol
Expand Down
Loading

0 comments on commit d716d85

Please sign in to comment.