OAuth2 auto-refresh (proactive and reactive-on-401) is silently disabled whenever CLIENT_ID is empty, even with a fully valid client_secret/oauth2_server_uri #1378

Environment

• DuckDB v1.5.5 (Variegata, d8cdaa33fd)
• iceberg extension d987bfc2 (installed via core_nightly)
• macOS arm64
• (Source references below point at duckdb-iceberg commit 4a07bc48 — the closest public commit I could confirm the logic against; the exact nightly build installed may differ slightly.)

Summary

An OAuth2 ICEBERG secret configured with CLIENT_ID '' (empty string) — a valid, spec-compliant configuration required by at least one real-world provider (Snowflake Horizon/Polaris Catalog, see below) — never auto-refreshes its access token, either proactively before expiry or reactively after a 401. Once the cached token in the secret expires, every subsequent ATTACH using that secret fails hard with Unauthorized_401, even though fully valid client_secret + oauth2_server_uri + oauth2_scope are present and a fresh client_credentials exchange would succeed.

Root cause

OAuth2Authorization::CanRefreshUnlocked() gates all refresh attempts (both the proactive pre-request check and the reactive post-401 retry) on client_id being non-empty:

bool OAuth2Authorization::CanRefreshUnlocked(std::lock_guard<std::mutex> &lock) const {
	(void)lock;
	return !refresh_token.empty() || (!client_id.empty() && !client_secret.empty() && !uri.empty());
}

This is called from both the proactive path and the reactive 401-retry path in Request():

...which is the exact method invoked by the initial catalog handshake:

When client_id is empty but client_secret/uri are present and valid, CanRefreshUnlocked() returns false, so Request() never attempts a refresh — the stale/invalid token is used (or, on 401, no retry happens) and the error surfaces straight to the caller.

A secondary, related issue: UpdateTokenState() computes the cached token's expiry as now() + expires_in whenever a secret is loaded (not just when freshly issued):

Because the persisted secret only stores the relative expires_in, never an absolute issued-at timestamp, a new process loading an already-expired cached token believes it's fresh — so the proactive check never catches it either. This makes the CLIENT_ID-empty bug worse in practice (it's not just "no refresh on 401," it's "the token gets reused across process restarts long past its real expiry, then fails with no recovery path").

Why an empty CLIENT_ID is a legitimate configuration, not misuse

Snowflake's Horizon/Polaris Iceberg REST Catalog requires this exact shape when authenticating with a Programmatic Access Token (PAT): grant_type, scope, and client_secret only — no client_id field at all. Snowflake's own documentation shows this explicitly:

curl -i --fail -X POST "https://<account_identifier>.snowflakecomputing.com/polaris/api/catalog/v1/oauth/tokens" \
 --header 'Content-Type: application/x-www-form-urlencoded' \
 --data-urlencode 'grant_type=client_credentials' \
 --data-urlencode 'scope=session:role:<role>' \
 --data-urlencode 'client_secret=<PAT_token>'

— Access Apache Iceberg tables with an external engine through Snowflake Horizon Catalog (Snowflake Documentation)

We independently confirmed (via direct curl testing) that supplying any non-empty client_id to this endpoint gets rejected (unauthorized_client/invalid_scope), while DuckDB's CREATE SECRET ... TYPE ICEBERG requires the CLIENT_ID field to be present syntactically — CLIENT_ID '' is the only way to satisfy both constraints simultaneously. CanRefreshUnlocked() then punishes that exact, necessary workaround by treating "empty" as "not configured."

Minimal repro

-- Using any provider with a client_credentials endpoint that accepts an
-- empty client_id (e.g. Snowflake Horizon Catalog with a PAT):
CREATE OR REPLACE PERSISTENT SECRET repro_secret (
    TYPE ICEBERG,
    CLIENT_ID '',
    CLIENT_SECRET '<valid PAT/client_secret>',
    OAUTH2_SERVER_URI 'https://<account>.snowflakecomputing.com/polaris/api/catalog/v1/oauth/tokens',
    OAUTH2_SCOPE 'session:role:<role>',
    -- Force a broken cached token to simulate "expired token reused after
    -- a process restart" without waiting out the real ~1hr expiry:
    TOKEN 'deliberately_garbage_token_to_force_401',
    EXPIRES_IN 3600
);

ATTACH '<warehouse>' AS repro (
    TYPE ICEBERG,
    SECRET repro_secret,
    ENDPOINT 'https://<account>.snowflakecomputing.com/polaris/api/catalog'
);
-- Expected: transparent refresh + retry, ATTACH succeeds.
-- Actual: hard failure --
-- Invalid Configuration Error: Request to '.../v1/config?warehouse=...'
-- returned a non-200 status code (Unauthorized_401), with reason: Unauthorized

Expected behavior

CanRefreshUnlocked() should treat client_id as "configured" whenever it's been explicitly set (including to an empty string) alongside a non-empty client_secret and uri — i.e., gate on whether the client_credentials fields were provided at all, not on client_id being non-empty. Separately, persisting an absolute token-issued-at (or expires-at) timestamp in the secret, rather than only a relative expires_in, would let the proactive check correctly detect staleness across process restarts.

Suggested fix

bool OAuth2Authorization::CanRefreshUnlocked(std::lock_guard<std::mutex> &lock) const {
	(void)lock;
	return !refresh_token.empty() || (!client_secret.empty() && !uri.empty());
}

(dropping the !client_id.empty() condition — an empty client_id is a valid value for the client_credentials grant per RFC 6749 §2.3.1, which only requires client authentication when the client "was issued client credentials," not that a non-empty id string be present)