Skip to content

Commit f479763

Browse files
chernserchernser
committed
moved new values validation to CredentialsManager
1 parent 67f51fd commit f479763

3 files changed

Lines changed: 103 additions & 24 deletions

File tree

‎client-v2/src/main/java/com/clickhouse/client/api/Client.java‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@
106106
*
107107
*
108108
* <p>Client is thread-safe. It uses exclusive set of object to perform an operation.
109-
* Exception is client global authentication configuration. Application should handle it in the way it is designed./p>
109+
* Exception is client global authentication configuration. Application should handle it in the way it is designed.</p>
110110
*
111111
*/
112112
public class Client implements AutoCloseable {
@@ -2182,6 +2182,7 @@ public Collection<String> getDBRoles() {
21822182
return unmodifiableDbRolesView;
21832183
}
21842184

2185+
21852186
/**
21862187
* Updates Bearer token for other requests.
21872188
* This method is not thread-safe with respect to other credential updates
@@ -2191,9 +2192,7 @@ public Collection<String> getDBRoles() {
21912192
* @param bearer - token to use without {@code "Bearer"} prefix.
21922193
*/
21932194
public void updateBearerToken(String bearer) {
2194-
if (!credentialsManager.isHasAccessToken()) {
2195-
throw new ClientMisconfigurationException("Authentication type cannot be switch at runtime");
2196-
}
2195+
ValidationUtils.checkNonBlank(bearer, "Bearer token");
21972196
updateAccessToken(CredentialsManager.AUTH_HEADER_BEARER_PREFIX + bearer);
21982197
}
21992198

@@ -2208,9 +2207,6 @@ public void updateBearerToken(String bearer) {
22082207
* @throws ClientMisconfigurationException if another authentication type in use.
22092208
*/
22102209
public void updateUserAndPassword(String username, String password) {
2211-
if (!credentialsManager.isHasUserPassword()) {
2212-
throw new ClientMisconfigurationException("Authentication type cannot be switch at runtime");
2213-
}
22142210
this.credentialsManager.setCredentials(username, password);
22152211
}
22162212

@@ -2223,9 +2219,6 @@ public void updateUserAndPassword(String username, String password) {
22232219
* @param accessToken - plain text access token
22242220
*/
22252221
public void updateAccessToken(String accessToken) {
2226-
if (!credentialsManager.isHasAccessToken()) {
2227-
throw new ClientMisconfigurationException("Authentication type cannot be switch at runtime");
2228-
}
22292222
this.credentialsManager.setAccessToken(accessToken);
22302223
}
22312224

‎client-v2/src/main/java/com/clickhouse/client/api/internal/CredentialsManager.java‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import java.util.Arrays;
88
import java.util.HashMap;
99
import java.util.Map;
10-
import java.util.Objects;
1110
import java.util.concurrent.atomic.AtomicReference;
1211

1312
/**
@@ -35,8 +34,8 @@ public CredentialsManager(Map<String, String> config) {
3534
boolean hasAuthHeader = isCustomAuthHeader(config);
3635

3736
final long authMethodsCount = Arrays
38-
.stream(new Boolean[] {hasUserPassword, hasAccessToken, sslAuthEnabled, hasAuthHeader})
39-
.filter(b-> b).count();
37+
.stream(new Boolean[]{hasUserPassword, hasAccessToken, sslAuthEnabled, hasAuthHeader})
38+
.filter(b -> b).count();
4039

4140
String username = config.get(ClientConfigProperties.USER.getKey());
4241
if (authMethodsCount == 1 && !hasAuthHeader) {
@@ -64,26 +63,25 @@ public CredentialsManager(Map<String, String> config) {
6463
}
6564
}
6665

67-
public boolean isHasAccessToken() {
68-
return hasAccessToken;
69-
}
70-
71-
public boolean isHasUserPassword() {
72-
return hasUserPassword;
73-
}
74-
7566
public void applyCredentials(Map<String, Object> target) {
7667
Map<String, Object> properties = authConfig.get();
7768
target.putAll(properties);
7869
}
7970

71+
private static final String AUTH_CANNOT_BE_SWITCHED_ERR_MSG = "Authentication type cannot be switched at runtime";
72+
8073
/**
8174
* Replaces the current username/password credentials.
8275
*
8376
* <p>This class does not synchronize credential updates. Callers must
8477
* serialize updates and request execution if they require thread safety.
8578
*/
8679
public void setCredentials(String username, String password) {
80+
ValidationUtils.checkNonBlank(username, "username");
81+
ValidationUtils.checkNonBlank(password, "password");
82+
if (!hasUserPassword) {
83+
throw new ClientMisconfigurationException(AUTH_CANNOT_BE_SWITCHED_ERR_MSG);
84+
}
8785
updateBackedConfig(username, password, false, null);
8886
}
8987

@@ -94,6 +92,10 @@ public void setCredentials(String username, String password) {
9492
* serialize updates and request execution if they require thread safety.
9593
*/
9694
public void setAccessToken(String accessToken) {
95+
if (!hasAccessToken) {
96+
throw new ClientMisconfigurationException(AUTH_CANNOT_BE_SWITCHED_ERR_MSG);
97+
}
98+
ValidationUtils.checkNonBlank(accessToken, "accessToken");
9799
updateBackedConfig(null, null, false, accessToken);
98100
}
99101

‎client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java‎

Lines changed: 87 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,22 @@
33
import com.clickhouse.client.api.Client;
44
import com.clickhouse.client.api.ClientConfigProperties;
55
import com.clickhouse.client.api.ClientException;
6-
import com.clickhouse.client.api.Session;
76
import com.clickhouse.client.api.ClientFaultCause;
87
import com.clickhouse.client.api.ClientMisconfigurationException;
98
import com.clickhouse.client.api.ConnectionInitiationException;
109
import com.clickhouse.client.api.ConnectionReuseStrategy;
1110
import com.clickhouse.client.api.ServerException;
11+
import com.clickhouse.client.api.Session;
1212
import com.clickhouse.client.api.command.CommandResponse;
1313
import com.clickhouse.client.api.command.CommandSettings;
1414
import com.clickhouse.client.api.data_formats.ClickHouseBinaryFormatReader;
1515
import com.clickhouse.client.api.enums.Protocol;
1616
import com.clickhouse.client.api.enums.ProxyType;
17-
import com.clickhouse.client.api.insert.InsertSettings;
1817
import com.clickhouse.client.api.insert.InsertResponse;
18+
import com.clickhouse.client.api.insert.InsertSettings;
1919
import com.clickhouse.client.api.internal.DataTypeConverter;
2020
import com.clickhouse.client.api.internal.ServerSettings;
21+
import com.clickhouse.client.api.internal.ValidationUtils;
2122
import com.clickhouse.client.api.query.GenericRecord;
2223
import com.clickhouse.client.api.query.QueryResponse;
2324
import com.clickhouse.client.api.query.QuerySettings;
@@ -57,6 +58,7 @@
5758
import java.util.UUID;
5859
import java.util.concurrent.CompletableFuture;
5960
import java.util.concurrent.ExecutionException;
61+
import java.util.concurrent.ThreadLocalRandom;
6062
import java.util.concurrent.TimeUnit;
6163
import java.util.concurrent.atomic.AtomicInteger;
6264
import java.util.regex.Pattern;
@@ -1047,8 +1049,9 @@ public void testBearerTokenAuth() throws Exception {
10471049
return; // mocked server
10481050
}
10491051

1052+
int randomPort = ThreadLocalRandom.current().nextInt(3000,65535);
10501053
WireMockServer mockServer = new WireMockServer( WireMockConfiguration
1051-
.options().port(9090).notifier(new ConsoleNotifier(false)));
1054+
.options().port(randomPort).notifier(new ConsoleNotifier(false)));
10521055
mockServer.start();
10531056

10541057
try {
@@ -1108,14 +1111,95 @@ public void testBearerTokenAuth() throws Exception {
11081111
.build());
11091112

11101113
client.updateBearerToken(jwtToken2);
1114+
client.execute("SELECT 1").get();
1115+
1116+
Assert.expectThrows(ValidationUtils.SettingsValidationException.class, () -> client.updateBearerToken(null));
1117+
Assert.expectThrows(ValidationUtils.SettingsValidationException.class, () -> client.updateBearerToken(""));
1118+
}
1119+
} finally {
1120+
mockServer.stop();
1121+
}
1122+
}
1123+
1124+
@Test(groups = { "integration" })
1125+
public void testAccessTokenAuth() throws Exception {
1126+
if (isCloud()) {
1127+
return; // mocked server
1128+
}
1129+
1130+
int randomPort = ThreadLocalRandom.current().nextInt(3000,65535);
1131+
WireMockServer mockServer = new WireMockServer( WireMockConfiguration
1132+
.options().port(randomPort).notifier(new ConsoleNotifier(false)));
1133+
mockServer.start();
1134+
1135+
try {
1136+
String accessToken1 = Arrays.stream(
1137+
new String[]{"header", "payload", "signature"})
1138+
.map(s -> Base64.getEncoder().encodeToString(s.getBytes(StandardCharsets.UTF_8)))
1139+
.reduce((s1, s2) -> s1 + "." + s2).get();
1140+
try (Client client = new Client.Builder().addEndpoint(Protocol.HTTP, "localhost", mockServer.port(), false)
1141+
.setAccessToken(accessToken1)
1142+
.compressServerResponse(false)
1143+
.build()) {
11111144

1145+
mockServer.addStubMapping(WireMock.post(WireMock.anyUrl())
1146+
.withHeader("Authorization", WireMock.equalTo(accessToken1))
1147+
.willReturn(WireMock.aResponse()
1148+
.withHeader("X-ClickHouse-Summary",
1149+
"{ \"read_bytes\": \"10\", \"read_rows\": \"1\"}")).build());
1150+
1151+
try (QueryResponse response = client.query("SELECT 1").get(1, TimeUnit.SECONDS)) {
1152+
Assert.assertEquals(response.getReadBytes(), 10);
1153+
} catch (Exception e) {
1154+
Assert.fail("Unexpected exception", e);
1155+
}
1156+
}
1157+
1158+
String accessToken2 = Arrays.stream(
1159+
new String[]{"header2", "payload2", "signature2"})
1160+
.map(s -> Base64.getEncoder().encodeToString(s.getBytes(StandardCharsets.UTF_8)))
1161+
.reduce((s1, s2) -> s1 + "." + s2).get();
1162+
1163+
mockServer.resetAll();
1164+
mockServer.addStubMapping(WireMock.post(WireMock.anyUrl())
1165+
.withHeader("Authorization", WireMock.equalTo(accessToken1))
1166+
.willReturn(WireMock.aResponse()
1167+
.withStatus(HttpStatus.SC_UNAUTHORIZED))
1168+
.build());
1169+
1170+
try (Client client = new Client.Builder().addEndpoint(Protocol.HTTP, "localhost", mockServer.port(), false)
1171+
.setAccessToken(accessToken1)
1172+
.compressServerResponse(false)
1173+
.build()) {
1174+
1175+
try {
1176+
client.execute("SELECT 1").get();
1177+
fail("Exception expected");
1178+
} catch (ServerException e) {
1179+
Assert.assertEquals(e.getTransportProtocolCode(), HttpStatus.SC_UNAUTHORIZED);
1180+
}
1181+
1182+
mockServer.resetAll();
1183+
mockServer.addStubMapping(WireMock.post(WireMock.anyUrl())
1184+
.withHeader("Authorization", WireMock.equalTo(accessToken2))
1185+
.willReturn(WireMock.aResponse()
1186+
.withHeader("X-ClickHouse-Summary",
1187+
"{ \"read_bytes\": \"10\", \"read_rows\": \"1\"}"))
1188+
1189+
.build());
1190+
1191+
client.updateAccessToken(accessToken2);
11121192
client.execute("SELECT 1").get();
1193+
1194+
Assert.expectThrows(ValidationUtils.SettingsValidationException.class, () -> client.updateAccessToken(null));
1195+
Assert.expectThrows(ValidationUtils.SettingsValidationException.class, () -> client.updateAccessToken(""));
11131196
}
11141197
} finally {
11151198
mockServer.stop();
11161199
}
11171200
}
11181201

1202+
11191203
@Test(groups = { "integration" })
11201204
public void testUpdateSessionIdAtRuntime() throws Exception {
11211205
if (isCloud()) {

0 commit comments

Comments
 (0)