OIDC device-flow review follow-ups

Follow-ups from the device-flow review:

- README: the OIDC quick-start still called the pre-rename API (getToken()
  as the interactive sign-in, auth::getTokenSilently). Use signIn() for the
  one-time sign-in and auth::getToken for the token provider, matching the
  renamed methods.

- OidcDeviceAuth: pre-encode the invariant form params (client_id, scope,
  audience) once in the constructor and the device_code once in pollForToken,
  instead of re-running URLEncoder on every poll (~once/5s through a sign-in)
  and every silent refresh. Mirrors the existing GRANT_TYPE_*_ENCODED
  constants; the wire output is unchanged. Add appendEncodedParam for the
  pre-encoded values and keep appendParam for the dynamic refresh_token.

- Reorder getToken() before signIn() and rename the lagging
  testGetTokenSilently* / testConcurrentGetToken* methods to match the
  renamed API.

- QwpQueryClientTokenProviderTest: cover the real connect path, not just the
  test hook - a throwing provider fails the connection attempt, the pulled
  token reaches the actual upgrade Authorization header, and a null, empty or
  blank provider return is rejected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: glasstiger

README.md

@@ -168,15 +168,15 @@ import io.questdb.client.cutlass.auth.OidcDeviceAuth;
 
 // Discover the client id, scope and endpoints from the QuestDB server's /settings:
 try (OidcDeviceAuth auth = OidcDeviceAuth.fromQuestDB("https://questdb.example.com:9000")) {
-    auth.getToken(); // sign in once: prompts on first use, then caches and refreshes
+    auth.signIn(); // sign in once: prompts on first use, then caches and refreshes
 
     // Pass a token provider, not a fixed string: the sender pulls a freshly refreshed token on each
-    // request, so a long-lived sender keeps working as the token rotates. getTokenSilently() refreshes
+    // request, so a long-lived sender keeps working as the token rotates. getToken() refreshes
     // silently and never prompts on the flush path.
     try (Sender sender = Sender.builder(Sender.Transport.HTTP)
             .address("questdb.example.com:9000")
             .enableTls()
-            .httpTokenProvider(auth::getTokenSilently)
+            .httpTokenProvider(auth::getToken)
             .build()) {
         sender.table("trades")
                 .symbol("symbol", "ETH-USD")
@@ -186,7 +186,7 @@ try (OidcDeviceAuth auth = OidcDeviceAuth.fromQuestDB("https://questdb.example.c
 }
 ```
 
-Prefer `httpTokenProvider(auth::getTokenSilently)` for a long-lived sender: it pulls a freshly refreshed token on every request, so the sender keeps working as the token rotates. A fixed `httpToken(token)` captures the token once, so a sender that outlives the token's lifetime starts failing with 401s. Either way, hand the token to the client through the builder (or the header/password fields below), not by embedding it in a `Sender.fromConfig(...)` string or the `QDB_CLIENT_CONF` environment variable, which are easily logged, persisted, or left in shell history.
+Prefer `httpTokenProvider(auth::getToken)` for a long-lived sender: it pulls a freshly refreshed token on every request, so the sender keeps working as the token rotates. A fixed `httpToken(token)` captures the token once, so a sender that outlives the token's lifetime starts failing with 401s. Either way, hand the token to the client through the builder (or the header/password fields below), not by embedding it in a `Sender.fromConfig(...)` string or the `QDB_CLIENT_CONF` environment variable, which are easily logged, persisted, or left in shell history.
 
 By default the prompt prints the verification URL and code to `System.out` **and** tries to open the URL in your default browser. The browser open is best-effort: it only opens an `http(s)` URL, is skipped on a headless host or a JVM without the `java.desktop` module, and never blocks sign-in — the URL and code are always printed too, so a remote or browserless process still works. To disable the browser launch for a whole process (a server, automation, CI), set the system property `-Dquestdb.client.oidc.open.browser=false`. To print only (no browser) for a single client, pass `DeviceCodePrompt.SYSTEM_OUT`; to render the challenge yourself (a clickable link or QR code in a notebook), pass any `DeviceCodePrompt`:
 
@@ -195,7 +195,7 @@ By default the prompt prints the verification URL and code to `System.out` **and
 try (OidcDeviceAuth auth = OidcDeviceAuth.fromQuestDB(
         "https://questdb.example.com:9000",
         new OidcDeviceAuth.DiscoveryOptions().prompt(DeviceCodePrompt.SYSTEM_OUT))) {
-    auth.getToken();
+    auth.signIn();
 }
 ```
 
@@ -222,7 +222,7 @@ Discovery via `fromQuestDB(...)` reads the OIDC client id, scope, audience and e
 try (OidcDeviceAuth auth = OidcDeviceAuth.fromQuestDB(
         "https://questdb.example.com:9000",
         new OidcDeviceAuth.DiscoveryOptions().issuer("https://idp.example.com"))) {
-    auth.getToken();
+    auth.signIn();
 }
 ```
 

core/src/main/java/io/questdb/client/cutlass/auth/OidcDeviceAuth.java

@@ -45,8 +45,8 @@
 import io.questdb.client.std.str.DirectUtf8Sequence;
 import io.questdb.client.std.str.StringSink;
 
-import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.util.concurrent.locks.ReentrantLock;
 
 /**
@@ -146,8 +146,8 @@ public class OidcDeviceAuth implements QuietCloseable {
     private static final int SLOW_DOWN_INCREMENT_SECONDS = 5;
     private static final String USER_AGENT = "questdb/java-client-oidc";
     private static final String WELL_KNOWN_OPENID_CONFIGURATION_PATH = "/.well-known/openid-configuration";
-    private final String audience;
-    private final String clientId;
+    private final String audienceEncoded;
+    private final String clientIdEncoded;
     private final DeviceAuthorizationResponseParser deviceAuthParser = new DeviceAuthorizationResponseParser();
     private final Endpoint deviceAuthorizationEndpoint;
     private final StringSink formSink = new StringSink();
@@ -158,7 +158,7 @@ public class OidcDeviceAuth implements QuietCloseable {
     private final ReentrantLock lock = new ReentrantLock();
     private final DeviceCodePrompt prompt;
     private final StringSink responseStatus = new StringSink();
-    private final String scope;
+    private final String scopeEncoded;
     private final ClientTlsConfiguration tlsConfig;
     private final Endpoint tokenEndpoint;
     private final TokenResponseParser tokenParser = new TokenResponseParser();
@@ -175,11 +175,16 @@ public class OidcDeviceAuth implements QuietCloseable {
     private long tokenTtlMillis;
 
     private OidcDeviceAuth(Builder builder, ClientTlsConfiguration tlsConfig) {
-        this.clientId = builder.clientId;
+        String clientId = builder.clientId;
+        // pre-encode the invariant form params once here, so the poll loop and silent refresh do not
+        // re-run URLEncoder on every request (mirrors the pre-encoded GRANT_TYPE_* constants)
+        this.clientIdEncoded = urlEncode(clientId);
         this.deviceAuthorizationEndpoint = Endpoint.parse(builder.deviceAuthorizationEndpoint);
         this.tokenEndpoint = Endpoint.parse(builder.tokenEndpoint);
-        this.scope = builder.scope;
-        this.audience = builder.audience;
+        String scope = builder.scope;
+        this.scopeEncoded = urlEncode(scope);
+        String audience = builder.audience;
+        this.audienceEncoded = audience != null ? urlEncode(audience) : null;
         this.groupsInToken = builder.groupsInToken;
         this.httpTimeoutMillis = builder.httpTimeoutMillis;
         this.prompt = builder.prompt;
@@ -406,39 +411,6 @@ public String getAuthorizationHeaderValue() {
         return "Bearer " + signIn();
     }
 
-    /**
-     * Returns a valid token to present to QuestDB: the cached token while still valid, otherwise a
-     * silent refresh when possible, otherwise the interactive device flow. The token is the id token
-     * when the server expects groups encoded in the token, the access token otherwise.
-     *
-     * @return a non-null, non-empty token
-     * @throws OidcAuthException if the interactive flow fails, times out, or the identity provider
-     *                           does not return the expected token
-     */
-    public String signIn() {
-        lock.lock();
-        try {
-            throwIfClosed();
-            // only the kind of token signIn() actually serves counts as a cache hit; a grant that
-            // returned the other kind (access token when the server wants the id token, or vice versa)
-            // leaves the served token null, so re-run the flow rather than report the unusable grant as
-            // valid and have selectToken() throw on this and every later call
-            final String cachedToken = groupsInToken ? idToken : accessToken;
-            if (cachedToken != null) {
-                if (System.currentTimeMillis() < expiresAtMillis - effectiveSkewMillis()) {
-                    return cachedToken;
-                }
-                if (refreshToken != null && tryRefresh()) {
-                    return selectToken();
-                }
-            }
-            runDeviceFlow();
-            return selectToken();
-        } finally {
-            lock.unlock();
-        }
-    }
-
     /**
      * Like {@link #signIn()} but never starts the interactive device flow, never prompts, and never waits
      * on interactive input: returns the cached token while valid, silently refreshes when a refresh token is
@@ -485,6 +457,39 @@ public String getToken() {
         }
     }
 
+    /**
+     * Returns a valid token to present to QuestDB: the cached token while still valid, otherwise a
+     * silent refresh when possible, otherwise the interactive device flow. The token is the id token
+     * when the server expects groups encoded in the token, the access token otherwise.
+     *
+     * @return a non-null, non-empty token
+     * @throws OidcAuthException if the interactive flow fails, times out, or the identity provider
+     *                           does not return the expected token
+     */
+    public String signIn() {
+        lock.lock();
+        try {
+            throwIfClosed();
+            // only the kind of token signIn() actually serves counts as a cache hit; a grant that
+            // returned the other kind (access token when the server wants the id token, or vice versa)
+            // leaves the served token null, so re-run the flow rather than report the unusable grant as
+            // valid and have selectToken() throw on this and every later call
+            final String cachedToken = groupsInToken ? idToken : accessToken;
+            if (cachedToken != null) {
+                if (System.currentTimeMillis() < expiresAtMillis - effectiveSkewMillis()) {
+                    return cachedToken;
+                }
+                if (refreshToken != null && tryRefresh()) {
+                    return selectToken();
+                }
+            }
+            runDeviceFlow();
+            return selectToken();
+        } finally {
+            lock.unlock();
+        }
+    }
+
     private static String appendSettingsPath(String basePath) {
         String trimmed = basePath;
         while (trimmed.length() > 1 && trimmed.charAt(trimmed.length() - 1) == '/') {
@@ -887,13 +892,8 @@ private static boolean settingsChannelIsPlaintext(Endpoint server) {
     }
 
     private static String urlEncode(String value) {
-        try {
-            // the Charset overload is Java 10; the client targets Java 8, so use the String-charset form
-            return URLEncoder.encode(value, "UTF-8");
-        } catch (UnsupportedEncodingException e) {
-            // UTF-8 is guaranteed present on every JVM, so this is unreachable; rethrow defensively
-            throw new OidcAuthException(e).put("UTF-8 encoding is not supported");
-        }
+        // the Charset overload is Java 10; the client targets Java 8, so use the String-charset form
+        return URLEncoder.encode(value, StandardCharsets.UTF_8);
     }
 
     private static void validateEndpointOrigins(Endpoint tokenEndpoint, Endpoint deviceAuthorizationEndpoint, Endpoint issuer) {
@@ -953,6 +953,10 @@ private static String wellKnownUrl(String issuer) {
         return trimmed + WELL_KNOWN_OPENID_CONFIGURATION_PATH;
     }
 
+    private void appendEncodedParam(StringSink sink, String name, String encodedValue) {
+        sink.putAscii('&').putAscii(name).putAscii('=').putAscii(encodedValue);
+    }
+
     private void appendParam(StringSink sink, String name, String value) {
         sink.putAscii('&').putAscii(name).putAscii('=').putAscii(urlEncode(value));
     }
@@ -1001,6 +1005,9 @@ private boolean isHttpStatusTransient() {
     }
 
     private void pollForToken(String deviceCode, int expiresInSeconds, int intervalSeconds) {
+        // url-encode the opaque device code once here, not on every poll: it is invariant for the whole
+        // poll loop (the grant_type and client_id are likewise pre-encoded)
+        final String deviceCodeEncoded = urlEncode(deviceCode);
         final long deadlineNanos = System.nanoTime() + expiresInSeconds * 1_000_000_000L;
         long intervalMillis = (long) intervalSeconds * 1000L;
         while (true) {
@@ -1011,7 +1018,7 @@ private void pollForToken(String deviceCode, int expiresInSeconds, int intervalS
                 throw new OidcAuthException("timed out waiting for authorization, the device code expired; please retry");
             }
             try {
-                int result = pollOnce(deviceCode);
+                int result = pollOnce(deviceCodeEncoded);
                 if (result == POLL_SUCCESS) {
                     return;
                 }
@@ -1040,11 +1047,11 @@ private void pollForToken(String deviceCode, int expiresInSeconds, int intervalS
         }
     }
 
-    private int pollOnce(String deviceCode) {
+    private int pollOnce(String deviceCodeEncoded) {
         formSink.clear();
         formSink.putAscii("grant_type=").putAscii(GRANT_TYPE_DEVICE_CODE_ENCODED);
-        appendParam(formSink, "device_code", deviceCode);
-        appendParam(formSink, "client_id", clientId);
+        appendEncodedParam(formSink, "device_code", deviceCodeEncoded);
+        appendEncodedParam(formSink, "client_id", clientIdEncoded);
 
         tokenParser.clear();
         // a transport failure here propagates to pollForToken, which keeps polling (a transient blip) until
@@ -1161,10 +1168,10 @@ private void readResponse(HttpClient client, HttpClient.ResponseHeaders response
 
     private void runDeviceFlow() {
         formSink.clear();
-        formSink.putAscii("client_id=").putAscii(urlEncode(clientId));
-        appendParam(formSink, "scope", scope);
-        if (audience != null) {
-            appendParam(formSink, "audience", audience);
+        formSink.putAscii("client_id=").putAscii(clientIdEncoded);
+        appendEncodedParam(formSink, "scope", scopeEncoded);
+        if (audienceEncoded != null) {
+            appendEncodedParam(formSink, "audience", audienceEncoded);
         }
 
         deviceAuthParser.clear();
@@ -1281,12 +1288,12 @@ private boolean tryRefresh() {
         formSink.clear();
         formSink.putAscii("grant_type=").putAscii(GRANT_TYPE_REFRESH_TOKEN_ENCODED);
         appendParam(formSink, "refresh_token", refreshToken);
-        appendParam(formSink, "client_id", clientId);
-        if (scope != null) {
-            appendParam(formSink, "scope", scope);
+        appendEncodedParam(formSink, "client_id", clientIdEncoded);
+        if (scopeEncoded != null) {
+            appendEncodedParam(formSink, "scope", scopeEncoded);
         }
-        if (audience != null) {
-            appendParam(formSink, "audience", audience);
+        if (audienceEncoded != null) {
+            appendEncodedParam(formSink, "audience", audienceEncoded);
         }
 
         tokenParser.clear();

core/src/test/java/io/questdb/client/test/cutlass/auth/OidcDeviceAuthTest.java

@@ -564,7 +564,7 @@ public void testCloseCancelsInFlightSignIn() throws Exception {
     }
 
     @Test(timeout = 30_000)
-    public void testConcurrentGetTokenStartsSingleSignIn() throws Exception {
+    public void testConcurrentSignInStartsSingleSignIn() throws Exception {
         assertMemoryLeak(() -> {
             // several callers race signIn() on a fresh instance; the synchronized method must serialize
             // them so exactly one interactive sign-in runs and the rest get the cached token
@@ -1388,7 +1388,7 @@ public void testGarbledRefreshResponseFallsBackToInteractiveFlow() throws Except
     }
 
     @Test(timeout = 30_000)
-    public void testGetTokenSilentlyDoesNotBlockBehindInteractiveSignIn() throws Exception {
+    public void testGetTokenDoesNotBlockBehindInteractiveSignIn() throws Exception {
         assertMemoryLeak(() -> {
             // an interactive signIn() is parked polling (authorization_pending), holding the instance
             // lock for the whole device-code lifetime. A flush-path getToken() on another thread
@@ -1436,7 +1436,7 @@ public void testGetTokenSilentlyDoesNotBlockBehindInteractiveSignIn() throws Exc
     }
 
     @Test(timeout = 30_000)
-    public void testGetTokenSilentlyDoesNotBlockBehindSilentRefresh() throws Exception {
+    public void testGetTokenDoesNotBlockBehindSilentRefresh() throws Exception {
         assertMemoryLeak(() -> {
             // the flush-path contract also holds when the lock is held by another thread's SILENT REFRESH, not
             // just an interactive sign-in: getToken() must fail fast rather than queue behind it. The
@@ -1503,7 +1503,7 @@ public void testGetTokenSilentlyDoesNotBlockBehindSilentRefresh() throws Excepti
     }
 
     @Test(timeout = 30_000)
-    public void testGetTokenSilentlyRefreshesWithoutPrompting() throws Exception {
+    public void testGetTokenRefreshesWithoutPrompting() throws Exception {
         assertMemoryLeak(() -> {
             // getToken() returns the cached token, silently refreshes it when it expires, and never
             // prompts; if it cannot produce a token without an interactive sign-in, it throws