This is an automated email from the ASF dual-hosted git repository.
bharos pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 77eae699f6 [#13096] fix(trino-connector): Reuse service metadata when
the user token is missing (#13097)
77eae699f6 is described below
commit 77eae699f6b436c3d733052d9fe1c683b4274c0c
Author: Yuhui <[email protected]>
AuthorDate: Sat Sep 12 02:08:23 2026 +0800
[#13096] fix(trino-connector): Reuse service metadata when the user token
is missing (#13097)
### What changes were proposed in this pull request?
For OAuth2 user forwarding, reuse the configured service metadata when
the session's token credential is absent or blank. Keep user forwarding
for supplied tokens and preserve SIMPLE username forwarding. Add
regression tests and document the fallback behavior.
### Why are the changes needed?
Password-authenticated sessions, including internal JDBC
catalog-management sessions, do not carry an OAuth2 token. They
currently fail when Trino invokes the connector's metadata entry point
despite having a configured service identity.
Fix: #13096
### Does this PR introduce _any_ user-facing change?
Yes. With OAuth2 and `gravitino.client.session.forwardUser=true`,
sessions without a token access Gravitino metadata under the configured
service identity and its permissions. Failures using supplied tokens
still propagate. No configuration keys are added. Downstream catalog/IRC
authentication is unchanged.
### How was this patch tested?
Added regression tests for missing-token fallback and credential
forwarding. Trino connector unit tests and formatting checks passed.
---
docs/trino-connector/authentication.md | 34 +++----
.../trino/connector/GravitinoConnector.java | 6 ++
.../TestGravitinoConnectorForwardUser.java | 102 ++++++++++++++++++++-
3 files changed, 125 insertions(+), 17 deletions(-)
diff --git a/docs/trino-connector/authentication.md
b/docs/trino-connector/authentication.md
index e7166b3542..d95c926c1b 100644
--- a/docs/trino-connector/authentication.md
+++ b/docs/trino-connector/authentication.md
@@ -60,8 +60,8 @@ gravitino.client.basic.password=YourSecureGravitinoPassword
| Property | Description
| Default value | Required |
|-----------------------------------|-----------------------------------------------------------------|---------------|----------------------------|
| `gravitino.client.authType` | Authentication type: `simple`, `basic`,
`oauth2`, or `kerberos` | (none) | Yes (to enable Basic) |
-| `gravitino.client.basic.username` | Local user store username
| (none) | Yes if authType is `basic` |
-| `gravitino.client.basic.password` | Local user store password
| (none) | Yes if authType is `basic` |
+| `gravitino.client.basic.username` | Local user store username
| (none) | Yes if authType is `basic` |
+| `gravitino.client.basic.password` | Local user store password
| (none) | Yes if authType is `basic` |
### OAuth2 Authentication
@@ -154,7 +154,7 @@
gravitino.client.kerberos.keytabFilePath=/path/to/user.keytab
## Session Credential Forwarding
-Setting `gravitino.client.session.forwardUser=true` creates a dedicated
Gravitino client per Trino session user, so each user is visible in the
Gravitino audit log instead of the shared `gravitino.user` or service identity.
It is supported with `authType=simple` and `authType=oauth2`.
+Setting `gravitino.client.session.forwardUser=true` creates a dedicated
Gravitino client per Trino session user, so each user is visible in the
Gravitino audit log instead of the shared `gravitino.user` or service identity.
It is supported with `authType=simple` and `authType=oauth2`. For OAuth2
sessions without a forwarded token, the connector reuses the shared service
metadata instead.
**Configuration (`authType=simple`):**
@@ -184,7 +184,7 @@ gravitino.client.oauth2.scope=gravitino
gravitino.client.session.forwardUser=true
```
-With `authType=oauth2`, the end user's IdP access token is presented to
Gravitino directly instead of the shared client-credentials identity. This
requires the Trino coordinator to populate the session's extra-credentials with
the caller's access token under the key `token`; the connector reads it from
there, and `buildForSession` fails with a clear error if it's missing.
+With `authType=oauth2`, the end user's IdP access token is presented to
Gravitino directly when the Trino coordinator populates the session's
extra-credentials with the caller's access token under the key `token` (or the
configured `gravitino.client.session.userTokenCredentialKey`). If that
credential is absent, empty, or whitespace-only, the connector reuses the
shared service metadata. This allows password-authenticated sessions, including
internal catalog-management JDBC sessions, to [...]
Whether the coordinator can populate this extra-credential depends on the
Trino distribution:
@@ -215,11 +215,11 @@ which the connector does not re-route), the connector
does not set `iceberg.rest
**Configuration properties:**
-| Property | Description
| Default value | Required | Since version |
-|--------------------------------------------------------------|--------------------------------------------------------------------------------------------------|-----------------|------------|-----------------|
-| `gravitino.client.session.forwardUser` | When `true`
with `authType=simple` or `authType=oauth2`, forwards the Trino session
user/token to Gravitino per-query | `false` | No | 1.3.0
|
-| `gravitino.client.session.cache.maxSize` | Maximum
number of per-user sessions to keep in the cache
| `500` | No | 1.3.0 |
-| `gravitino.client.session.cache.expireAfterAccessSeconds` | Seconds
before an idle per-user session is evicted from the cache
| `3600` | No | 1.3.0 |
+| Property | Description
| Default value | Required | Since version |
+|-----------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|----------|---------------|
+| `gravitino.client.session.forwardUser` | When `true` with
`authType=simple` or `authType=oauth2`, forwards the Trino session user/token
to Gravitino per-query; OAuth2 sessions without a token use the shared service
metadata | `false` | No | 1.3.0 |
+| `gravitino.client.session.cache.maxSize` | Maximum number
of per-user sessions to keep in the cache
| `500` | No | 1.3.0 |
+| `gravitino.client.session.cache.expireAfterAccessSeconds` | Seconds before
an idle per-user session is evicted from the cache
| `3600` | No | 1.3.0 |
### Example: OAuth2 Per-User Token Forwarding
@@ -244,8 +244,8 @@ populate the session's extra-credentials with the caller's
access token under th
- **Open-source Trino**: there is currently no equivalent coordinator setting.
Track
[trinodb/trino discussion
#24403](https://github.com/trinodb/trino/discussions/24403) and
[issue #27917](https://github.com/trinodb/trino/issues/27917) for this
feature request. Until
- it lands upstream, this connector's `authType=oauth2` forwardUser path
requires a Trino
- distribution that provides this extra-credential itself.
+ it lands upstream, forwarding OAuth2 user tokens requires a Trino
distribution that
+ provides this extra-credential itself. Sessions without it use the shared
service metadata.
**2. Gravitino server: enable OAuth2** (in `conf/gravitino.conf`):
@@ -273,8 +273,9 @@ gravitino.client.session.forwardUser=true
```
The `gravitino.client.oauth2.*` properties configure the shared service
identity used for catalog
-discovery; the per-user forwarded token (from step 1) is what each query
actually authenticates
-with once `forwardUser=true`.
+discovery and for metadata access by sessions without a forwarded token. With
+`forwardUser=true`, sessions carrying a token from step 1 authenticate
metadata requests with
+that token instead.
**4. Create the metalake and catalog.** Create the metalake `my_metalake`
first (via the
Gravitino REST API, SDK, or CLI — see
@@ -304,9 +305,10 @@ call gravitino.system.create_catalog(
);
```
-This call itself runs with the connector's own shared service identity, not
any forwarded user
-token — `forwardUser` only affects `SELECT`/`SHOW`-style queries against the
catalog afterward,
-not catalog registration itself. `create_catalog` both creates the catalog in
Gravitino and loads
+The procedure uses the connector's shared service client to create the catalog
in Gravitino.
+Catalog registration can also invoke the connector's metadata entry point; an
internal
+password-authenticated JDBC session without a forwarded token uses the shared
service metadata
+there. `create_catalog` both creates the catalog in Gravitino and loads
it into Trino as its own top-level catalog — not as a schema nested under a
single `gravitino`
catalog. If the two `trino.bypass.iceberg.rest-catalog.*` properties above are
omitted, the REST
catalog keeps its own default security setting, independent of
diff --git
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java
index 66f7b78036..57dc9913c6 100644
---
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java
+++
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java
@@ -242,6 +242,12 @@ public class GravitinoConnector implements Connector {
GravitinoAuthProvider.USER_TOKEN_CREDENTIAL_KEY,
GravitinoAuthProvider.DEFAULT_USER_TOKEN_CREDENTIAL_KEY);
String token =
session.getIdentity().getExtraCredentials().get(credentialKey);
+ // Password-authenticated sessions have no OAuth token. Reuse the
configured service
+ // identity in that case; failures with a supplied token must still
propagate.
+ if (GravitinoAuthProvider.parseAuthType(authType) ==
GravitinoAuthProvider.AuthType.OAUTH2
+ && StringUtils.isBlank(token)) {
+ return connectorMetadata;
+ }
String credKey = sessionCacheKey(authType, session.getUser(), token);
try {
return perUserSessionCache.get(
diff --git
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorForwardUser.java
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorForwardUser.java
index 29b91f8ca3..82e73441e2 100644
---
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorForwardUser.java
+++
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorForwardUser.java
@@ -28,12 +28,18 @@ import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableMap;
import io.trino.spi.TrinoException;
+import io.trino.spi.connector.Connector;
+import io.trino.spi.connector.ConnectorMetadata;
import io.trino.spi.connector.ConnectorSession;
+import io.trino.spi.connector.ConnectorTransactionHandle;
import io.trino.spi.security.ConnectorIdentity;
+import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
import org.apache.gravitino.Catalog;
@@ -44,6 +50,7 @@ import org.apache.gravitino.client.GravitinoMetalake;
import org.apache.gravitino.rel.TableCatalog;
import org.apache.gravitino.trino.connector.catalog.CatalogConnectorContext;
import org.apache.gravitino.trino.connector.catalog.CatalogConnectorMetadata;
+import
org.apache.gravitino.trino.connector.catalog.CatalogConnectorMetadataAdapter;
import org.apache.gravitino.trino.connector.metadata.GravitinoCatalog;
import org.apache.gravitino.trino.connector.security.GravitinoAuthProvider;
import org.junit.jupiter.api.Test;
@@ -51,6 +58,98 @@ import org.junit.jupiter.api.Test;
/** Tests for forwardUser startup validation in {@link GravitinoConnector}. */
class TestGravitinoConnectorForwardUser {
+ @Test
+ void testPasswordSessionUsesServiceMetadataForCatalogLifecycleAndQueries() {
+ CatalogConnectorContext ctx =
+ mockContextWithConfig(
+ ImmutableMap.of(
+ GravitinoAuthProvider.FORWARD_SESSION_USER_KEY, "true",
+ GravitinoAuthProvider.AUTH_TYPE_KEY, "oauth2"));
+ Connector internal = mock(Connector.class);
+ ConnectorMetadata internalMetadata = mock(ConnectorMetadata.class);
+ when(ctx.getInternalConnector()).thenReturn(internal);
+ when(internal.getMetadata(any(), any())).thenReturn(internalMetadata);
+ GravitinoConnector connector =
+ new GravitinoConnector(ctx) {
+ @Override
+ protected GravitinoMetadata createGravitinoMetadata(
+ CatalogConnectorMetadata metadata,
+ CatalogConnectorMetadataAdapter adapter,
+ ConnectorMetadata delegate) {
+ return new GravitinoMetadata(metadata, adapter, delegate) {};
+ }
+ };
+ ConnectorSession session = mockSession("gravitino_catalog_manager", "");
+ GravitinoTransactionHandle transaction =
+ new GravitinoTransactionHandle(mock(ConnectorTransactionHandle.class));
+ ConnectorMetadata metadata =
+ assertDoesNotThrow(() -> connector.getMetadata(session, transaction));
+ assertDoesNotThrow(() -> metadata.beginQuery(session));
+ assertDoesNotThrow(() -> metadata.cleanupQuery(session));
+ verify(internalMetadata).beginQuery(session);
+ verify(internalMetadata).cleanupQuery(session);
+ SupportsSchemas schemas =
ctx.getMetalake().loadCatalog("catalog").asSchemas();
+ when(schemas.listSchemas()).thenReturn(new String[] {"test_schema"});
+ assertEquals(List.of("test_schema"), metadata.listSchemaNames(session));
+ verify(schemas).listSchemas();
+ verify(internalMetadata, never()).listSchemaNames(any());
+ }
+
+ @Test
+ void testMissingOAuthTokenReusesServiceMetadataWithoutBuildingUserClient() {
+ CatalogConnectorContext ctx =
+ mockContextWithConfig(
+ ImmutableMap.of(
+ GravitinoAuthProvider.FORWARD_SESSION_USER_KEY, "true",
+ GravitinoAuthProvider.AUTH_TYPE_KEY, "oauth2"));
+ GravitinoConnector connector =
+ newConnectorWithAuthClient(
+ ctx,
+ session -> {
+ throw new AssertionError("A missing token must not create a user
client");
+ });
+ CatalogConnectorMetadata service =
+ connector.resolveSessionMetadata(mockSession("manager", null));
+ assertSame(service, connector.resolveSessionMetadata(mockSession("alice",
"")));
+ assertSame(service, connector.resolveSessionMetadata(mockSession("bob", "
")));
+ }
+
+ @Test
+ void testCustomTokenKeyControlsForwardingAndFallback() {
+ CatalogConnectorContext ctx =
+ mockContextWithConfig(
+ ImmutableMap.of(
+ GravitinoAuthProvider.FORWARD_SESSION_USER_KEY, "true",
+ GravitinoAuthProvider.AUTH_TYPE_KEY, "oauth2",
+ GravitinoAuthProvider.USER_TOKEN_CREDENTIAL_KEY,
"custom-token"));
+ AtomicInteger count = new AtomicInteger();
+ GravitinoConnector connector =
+ newConnectorWithAuthClient(ctx, session ->
mockAdminClient(ctx.getMetalake(), count));
+ ConnectorSession session = mockSession("alice", "ignored-default-token");
+ CatalogConnectorMetadata service =
connector.resolveSessionMetadata(session);
+ assertEquals(0, count.get());
+ when(session.getIdentity().getExtraCredentials())
+ .thenReturn(ImmutableMap.of("custom-token", "user-token"));
+ assertNotSame(service, connector.resolveSessionMetadata(session));
+ assertEquals(1, count.get());
+ }
+
+ @Test
+ void testSimpleAuthStillForwardsUsernameWithoutToken() {
+ CatalogConnectorContext ctx =
+ mockContextWithConfig(
+ ImmutableMap.of(
+ GravitinoAuthProvider.FORWARD_SESSION_USER_KEY, "true",
+ GravitinoAuthProvider.AUTH_TYPE_KEY, "simple"));
+ AtomicInteger count = new AtomicInteger();
+ GravitinoConnector connector =
+ newConnectorWithAuthClient(ctx, session ->
mockAdminClient(ctx.getMetalake(), count));
+ assertNotSame(
+ connector.resolveSessionMetadata(mockSession("alice", null)),
+ connector.resolveSessionMetadata(mockSession("bob", null)));
+ assertEquals(2, count.get());
+ }
+
@Test
void testForwardUserWithoutAuthTypeThrowsAtConstruction() {
CatalogConnectorContext ctx =
@@ -249,7 +348,8 @@ class TestGravitinoConnectorForwardUser {
private static ConnectorSession mockSession(String user, String token) {
ConnectorIdentity identity = mock(ConnectorIdentity.class);
- when(identity.getExtraCredentials()).thenReturn(ImmutableMap.of("token",
token));
+ when(identity.getExtraCredentials())
+ .thenReturn(token == null ? ImmutableMap.of() :
ImmutableMap.of("token", token));
ConnectorSession session = mock(ConnectorSession.class);
when(session.getUser()).thenReturn(user);
when(session.getIdentity()).thenReturn(identity);
