[client-v2,jdbc-v2] Change credentials in realtime#2812
Open
chernser wants to merge 28 commits into
Open
Conversation
chernser
changed the title
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
There was a problem hiding this comment.
Pull request overview
Adds runtime credential update support across client-v2 and aligns jdbc-v2 with the newer access-token configuration key, along with new examples and tests to demonstrate/authenticate the behavior.
Changes:
- Introduces a
CredentialsManagerand newClientruntime APIs (setCredentials(),setAccessToken()) to update auth for subsequent requests. - Renames/aliases bearer-token config to preferred
access_tokenand updates JDBC parsing/tests accordingly. - Adds new client-v2 and JDBC examples plus additional integration/unit tests for credential updates.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
client-v2/src/main/java/com/clickhouse/client/api/Client.java |
Adds runtime auth update APIs and routes request settings through CredentialsManager. |
client-v2/src/main/java/com/clickhouse/client/api/internal/CredentialsManager.java |
New component to manage mutable credentials and validate auth config. |
client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java |
Adds ACCESS_TOKEN and documents legacy BEARERTOKEN_AUTH alias. |
client-v2/src/test/java/com/clickhouse/client/api/internal/CredentialsManagerTest.java |
Unit tests for auth validation and credential snapshot/apply behavior. |
client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java |
Updates token-auth tests to setAccessToken() and adds integration test for setCredentials(). |
client-v2/src/test/java/com/clickhouse/client/ClientTests.java |
Adds integration test switching users via setCredentials(). |
jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java |
Accepts access_token with fallback to legacy bearer token property. |
jdbc-v2/src/main/java/com/clickhouse/jdbc/DataSourceImpl.java |
Uses ClientConfigProperties keys and clears token/SSL-auth when connecting with username/password. |
jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java |
Updates integration test to use ACCESS_TOKEN property and renames test. |
examples/client-v2/src/main/java/com/clickhouse/examples/client_v2/Authentication.java |
New example demonstrating runtime auth updates on an existing client. |
examples/client-v2/src/main/java/com/clickhouse/examples/client_v2/RuntimeCredentialsTwoUsers.java |
New demo creating users and switching credentials at runtime. |
examples/client-v2/README.md |
Documents the runtime credentials switch demo usage. |
examples/jdbc/src/main/java/com/clickhouse/examples/jdbc/Authentication.java |
New JDBC example showing auth changes by reconnecting with new properties. |
docs/features.md |
Documents runtime credential updates as stable behavior/compat trait. |
Member
|
@claude review |
|
Claude finished @mshustov's task in 2m 28s —— View job Code Review: [client-v2,jdbc-v2] Change credentials in realtime
The PR is well-structured and the Critical Bugs1. Stale auth keys are never removed from request settings [ This is the most serious issue. Failing scenario: Client built with // Current (broken):
private Map<String, Object> buildRequestSettings(Map<String, Object> opSettings) {
Map<String, Object> requestSettings = new HashMap<>(configuration); // includes stale auth keys
credentialsManager.applyCredentials(requestSettings); // only adds, never removes
requestSettings.putAll(opSettings);
return requestSettings;
}The fix is to explicitly remove all auth-related keys from public void applyCredentials(Map<String, Object> target) {
// Clear all auth keys first, then apply only current state
target.remove(ClientConfigProperties.ACCESS_TOKEN.getKey());
target.remove(AUTHORIZATION_HEADER_KEY);
target.remove(ClientConfigProperties.SSL_AUTH.getKey());
target.remove(ClientConfigProperties.USER.getKey());
target.remove(ClientConfigProperties.PASSWORD.getKey());
// now apply current credentials
putIfNotNull(target, ClientConfigProperties.USER.getKey(), username);
...
}Note that this change would break the test 2. this.httpHeader(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken); // "Bearer null" if nullThe Similarly, Minor Issues3. Thread-safety documentation inconsistency [ The class-level Javadoc states "Client is thread-safe" but the new 4.
5. The Javadoc says "Prefer using 6. After 7. Missing test for the stale-key scenario
8. The method removes Nits
|
Contributor
Author
|
@claude review |
Contributor
Author
|
@claude review |
|
Claude finished @chernser's task in 4m 45s —— View job Code Review: [client-v2,jdbc-v2] Change credentials in realtime
ReviewThe critical bugs from the two prior reviews have been resolved. A few medium and low-severity issues remain. Previously Reported — Now Fixed
Remaining IssuesMedium:
Medium: Raised in both prior reviews and still not addressed. @Deprecated
BEARERTOKEN_AUTH ("bearer_token", String.class),Low:
Low: Missing unit tests for No Nit:
Nit: Thread-safety class Javadoc The class says "Client is thread-safe" with a trailing carve-out. The individual method Javadocs are correctly specific. Consider rewording the class-level description to "Client request operations are thread-safe; credential update operations are not synchronized with concurrent request execution." Verdict
|
|
Contributor
Author
|
/benchmark |
|
JMH benchmark run started: https://github.com/ClickHouse/clickhouse-java/actions/runs/25752441550 |
🟢 JMH benchmark comparison — no regressions, 1 improvement(s) over 10%Baseline:
Detailed metrics (threshold ±10%, Time =
|
| Benchmark | Stats |
|---|---|
InsertClient.insertV2RowBinarydatasetSourceName=file://default.csv, limit=100000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 115 ± 1.3 ms/op → 107 ± 1.4 ms/op (-6.90%) 🟢 Alloc/op 8.23e+07 ± 2.1e+04 B/op → 3.43e+07 ± 2.1e+04 B/op (-58.34%) |
QueryClient.queryV2WithTypesdatasetSourceName=file://default.csv, limit=10000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 25.5 ± 0.15 ms/op → 27.8 ± 0.15 ms/op (+8.65%) Alloc/op 3.01e+07 ± 5.6e+03 B/op → 3.01e+07 ± 5e+03 B/op (+0.00%) |
InsertClient.insertV1CompresseddatasetSourceName=file://default.csv, limit=100000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 641 ± 12 ms/op → 677 ± 11 ms/op (+5.67%) Alloc/op 1.83e+08 ± 2.1e+05 B/op → 1.83e+08 ± 2.1e+05 B/op (+0.00%) |
QueryClient.queryV1WithTypesdatasetSourceName=file://default.csv, limit=10000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 15.6 ± 0.092 ms/op → 16.4 ± 0.1 ms/op (+5.19%) Alloc/op 9.12e+06 ± 4.2e+03 B/op → 9.12e+06 ± 4.1e+03 B/op (-0.01%) |
InsertClient.insertV1RowBinarydatasetSourceName=file://default.csv, limit=10000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 74.7 ± 0.35 ms/op → 77.9 ± 0.68 ms/op (+4.36%) Alloc/op 1.77e+07 ± 1.7e+04 B/op → 1.77e+07 ± 1.9e+04 B/op (+0.04%) |
QueryClient.queryV2datasetSourceName=file://default.csv, limit=10000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 20.7 ± 0.1 ms/op → 21.6 ± 0.12 ms/op (+4.31%) Alloc/op 1.65e+07 ± 5.1e+03 B/op → 1.65e+07 ± 5e+03 B/op (+0.00%) |
QueryClient.queryV1datasetSourceName=file://default.csv, limit=10000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 17.6 ± 0.093 ms/op → 18.2 ± 0.098 ms/op (+3.56%) Alloc/op 1.39e+07 ± 4.5e+03 B/op → 1.39e+07 ± 4.7e+03 B/op (-0.00%) |
InsertClient.insertV1RowBinarydatasetSourceName=file://default.csv, limit=100000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 141 ± 2.6 ms/op → 144 ± 2.5 ms/op (+2.30%) Alloc/op 1.57e+08 ± 2.6e+04 B/op → 1.57e+08 ± 2.6e+04 B/op (+0.00%) |
InsertClient.insertV2CompresseddatasetSourceName=file://default.csv, limit=100000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 530 ± 4.3 ms/op → 539 ± 5.8 ms/op (+1.86%) Alloc/op 3.68e+07 ± 1e+05 B/op → 3.68e+07 ± 1e+05 B/op (+0.02%) |
QueryClient.queryV2WithTypesdatasetSourceName=file://default.csv, limit=100000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 207 ± 1.3 ms/op → 211 ± 1.2 ms/op (+1.72%) Alloc/op 2.91e+08 ± 3.2e+04 B/op → 2.91e+08 ± 3.1e+04 B/op (+0.00%) |
InsertClient.insertV1datasetSourceName=file://default.csv, limit=100000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 342 ± 21 ms/op → 348 ± 21 ms/op (+1.70%) Alloc/op 5.73e+07 ± 1.2e+05 B/op → 5.73e+07 ± 1e+05 B/op (+0.01%) |
InsertClient.insertV1datasetSourceName=file://default.csv, limit=10000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 91.2 ± 0.69 ms/op → 92.4 ± 0.52 ms/op (+1.35%) Alloc/op 7.74e+06 ± 2e+04 B/op → 7.75e+06 ± 1.6e+04 B/op (+0.09%) |
QueryClient.queryV1WithTypesdatasetSourceName=file://default.csv, limit=100000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 98 ± 0.67 ms/op → 99.3 ± 0.58 ms/op (+1.30%) Alloc/op 7.17e+07 ± 3.7e+04 B/op → 7.17e+07 ± 3.8e+04 B/op (+0.00%) |
InsertClient.insertV1CompresseddatasetSourceName=file://default.csv, limit=10000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 133 ± 0.72 ms/op → 134 ± 0.61 ms/op (+1.06%) Alloc/op 2.03e+07 ± 2.5e+04 B/op → 2.03e+07 ± 2e+04 B/op (+0.04%) |
InsertClient.insertV2datasetSourceName=file://default.csv, limit=100000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 165 ± 2 ms/op → 166 ± 2.2 ms/op (+0.71%) Alloc/op 2.33e+06 ± 2.8e+04 B/op → 2.34e+06 ± 2.5e+04 B/op (+0.31%) |
QueryClient.queryV1datasetSourceName=file://default.csv, limit=100000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 117 ± 0.71 ms/op → 118 ± 0.89 ms/op (+0.68%) Alloc/op 1.2e+08 ± 4.4e+04 B/op → 1.2e+08 ± 5.1e+04 B/op (+0.00%) |
QueryClient.queryV2datasetSourceName=file://default.csv, limit=100000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 148 ± 0.99 ms/op → 148 ± 1.1 ms/op (-0.59%) Alloc/op 1.55e+08 ± 1.4e+04 B/op → 1.55e+08 ± 1.3e+04 B/op (+0.00%) |
InsertClient.insertV2RowBinarydatasetSourceName=file://default.csv, limit=10000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 74.1 ± 0.23 ms/op → 73.7 ± 0.36 ms/op (-0.58%) Alloc/op 1.02e+07 ± 1.3e+04 B/op → 1.02e+07 ± 1.3e+04 B/op (+0.07%) |
InsertClient.insertV2datasetSourceName=file://default.csv, limit=10000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 83.9 ± 0.27 ms/op → 83.7 ± 0.29 ms/op (-0.23%) Alloc/op 2.21e+06 ± 1e+04 B/op → 2.21e+06 ± 1.1e+04 B/op (+0.32%) |
InsertClient.insertV2CompresseddatasetSourceName=file://default.csv, limit=10000, tableNameEmpty=data_empty, tableNameFilled=data_filled |
Time 125 ± 0.56 ms/op → 124 ± 0.66 ms/op (-0.17%) Alloc/op 5.77e+06 ± 2.4e+04 B/op → 5.77e+06 ± 2.2e+04 B/op (+0.12%) |
mzitnik
reviewed
May 26, 2026
Contributor
mzitnik
left a comment
mzitnik
left a comment
There was a problem hiding this comment.
- Should we have a concurrency test
Contributor
Author
|
@mzitnik |
Contributor
Author
|
@mzitnik done |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
mzitnik
approved these changes
May 27, 2026
Contributor
|
@chernser, I have approved, but please make sure the tests are passing |
Contributor
Author
|
@mzitnik thank you! |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4520f45. Configure here.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
There is a task to support changing credentials at runtime. First we need this to change bearer and access tokens. This is OAuth usecase. These authentication is based on
AuthorizationHTTP header. User can set custom http Authorization header withhttpHeader()method of client builder and operation settings.com.clickhouse.client.api.internal.HttpAPIClientHelper#addHeadershandles adding authorization part to a request. Note, all authentication information is sent via HTTP headers. By default, user and password are sent viaClickHouse headers com.clickhouse.client.api.http.ClickHouseHttpProto#HEADER_DB_USERandcom.clickhouse.client.api.http.ClickHouseHttpProto#HEADER_DB_PASSWORDrespectively.HEADER_DB_USERalso used in sslAuthentication. Ifcom.clickhouse.client.api.ClientConfigProperties#HTTP_USE_BASIC_AUTHis set then user/password are encoded inbase64and sent viaAuthorizationheader. If this header is set by user then user configuration will have higher priority and all client set headers will be removed (today it is done via post processing to be sure we applied all headers and working with final set of them). User may set header viahttpHeaderbuilder method orcom.clickhouse.client.api.Client.Builder#useBearerTokenAuth.AuthorizationHTTP header is a special case because it is allowed to have password and the header at the same time or header can be set alone as authentication option.Before this change
ACCESS_TOKENwas not hooked up to anything (yes, it is bad. But we had no complains). Client V1 also had incomplete implementation (see code below). So this change treatssetAccessTokenas alias for setting value forAUTHORIZATIONHTTP header.client v1 use access token
There is another method
com.clickhouse.client.api.Client.Builder#useBearerTokenAuththat does similar to access token setter but addsBearer. After this change it will callsetAccessTokenbut keep addingBearerprefix.After this change it should be possible to:
com.clickhouse.client.api.Client#setAccessTokento set access token.com.clickhouse.client.api.Client#setBearerTokento set bearer token.com.clickhouse.client.api.Client#setCredentialsto set user/password credentials.When client was configured with SSL authentication then nothing can be changed because it might require a lot extra logic to re-init ssl context.
com.clickhouse.client.api.internal.CredentialsManageris introduced by this PR to collect authentication logic in one place and make it future point of extension. Old validation logic is moved inside its constructor.There is a design issue that hides logic of null password aliased to empty string. This is moved to
CredentialsManagerfromHttpAPIClientHelper.Compatibility Risk
Client.updateBearerTokennow throws if the client was not built with an access token (silent regression for callers that switched auth type at runtime).Client.Builder.build()throwsClientMisconfigurationExceptioninstead ofIllegalArgumentExceptionand rejects more combinations.Client.Builder.setAccessToken(...)previously had no effect on requests; it now sets the Authorization header (withoutBearer).Closes #2801
Checklist
Delete items not relevant to your PR:
Note
High Risk
Changes authentication validation, exception types, and wire behavior for access tokens and Authorization headers; callers catching IllegalArgumentException or relying on silent access_token or mixed auth headers may break at runtime.
Overview
client-v2 centralizes authentication in a new
CredentialsManagerand applies it on every request viabuildRequestSettings, instead of validating only atClient.Builder#build().Runtime updates on an existing
Client:updateUserAndPassword,updateAccessToken, andupdateBearerToken(auth method fixed at build; mismatched updaters throwClientMisconfigurationException).getConfiguration()no longer reflects live auth values.Behavior fixes and breaking changes:
setAccessTokennow sends the fullAuthorizationvalue on the wire (was previously stored but unused);useBearerTokenAuthwritesaccess_tokenwith aBearerprefix instead ofhttp_header_authorization.build()throwsClientMisconfigurationException(notIllegalArgumentException) for auth/SSL misconfiguration; username/password plus a customAuthorizationheader fails unlessuseHTTPBasicAuth(false).Docs, changelog, examples, and integration tests cover OAuth-style token rotation and credential switching.
Reviewed by Cursor Bugbot for commit 4520f45. Bugbot is set up for automated code reviews on this repo. Configure here.