This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new d044fcb9d0 [Cherry-pick to branch-1.3] [#13068] fix(trino-connector):
Enable Iceberg REST per-user sessions only under OAuth2 (#13078) (#13081)
d044fcb9d0 is described below
commit d044fcb9d06de541391a16fd11ea24b7d8532eeb
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Sep 10 22:31:04 2026 +0800
[Cherry-pick to branch-1.3] [#13068] fix(trino-connector): Enable Iceberg
REST per-user sessions only under OAuth2 (#13078) (#13081)
**Cherry-pick Information:**
- Original commit: b76d94243f89b16dd9f4d06ef10a7f8fc52537e4
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: Yuhui <[email protected]>
---
docs/trino-connector/authentication.md | 3 +-
docs/trino-connector/catalog-iceberg.md | 16 +++++--
.../iceberg/IcebergCatalogPropertyConverter.java | 30 +++++++++++--
.../TestIcebergCatalogPropertyConverter.java | 52 ++++++++++++++++++++--
4 files changed, 90 insertions(+), 11 deletions(-)
diff --git a/docs/trino-connector/authentication.md
b/docs/trino-connector/authentication.md
index 42c7932f6b..e7166b3542 100644
--- a/docs/trino-connector/authentication.md
+++ b/docs/trino-connector/authentication.md
@@ -197,7 +197,8 @@ For an Iceberg catalog reached through the Gravitino
Iceberg REST server (IRC)
`lakehouse-iceberg` catalog for which the Gravitino server reports a running
IRC; see [Iceberg
catalog](./catalog-iceberg.md#how-trino-reaches-the-catalog) — the IRC's own
authentication is
configured once per Trino cluster with the `gravitino.iceberg.rest-catalog.`
prefix, and
-`iceberg.rest-catalog.session=USER` is set automatically when
`forwardUser=true`:
+`iceberg.rest-catalog.session=USER` is set automatically when
`forwardUser=true` and the IRC is
+configured with `gravitino.iceberg.rest-catalog.security=OAUTH2` (as below):
```properties
gravitino.iceberg.rest-catalog.security=OAUTH2
diff --git a/docs/trino-connector/catalog-iceberg.md
b/docs/trino-connector/catalog-iceberg.md
index 4fc7b35336..6244590e72 100644
--- a/docs/trino-connector/catalog-iceberg.md
+++ b/docs/trino-connector/catalog-iceberg.md
@@ -111,10 +111,18 @@ Four keys are reserved: `iceberg.rest-catalog.uri`,
`.warehouse`, `.prefix` and
`gravitino.iceberg.rest-catalog.` or a catalog's `trino.bypass.` has no effect
— the connector logs
when it ignores one.
-When `gravitino.client.session.forwardUser=true`, the connector also sets
-`iceberg.rest-catalog.session=USER` so that each query carries the end user's
identity to the IRC,
-keeping per-user credential vending and per-user authorization intact. Set
-`gravitino.iceberg.rest-catalog.session` explicitly to override it. See
+The connector sets `iceberg.rest-catalog.session=USER` automatically when both
hold:
+
+- `gravitino.client.session.forwardUser=true`
+- the IRC authenticates with OAuth2, through any one of:
+ - `gravitino.client.authType=oauth2`
+ - `gravitino.iceberg.rest-catalog.security=OAUTH2`
+ - `trino.bypass.iceberg.rest-catalog.security=OAUTH2`, on a catalog with its
own REST backend
+
+Each query then carries the end user's identity to the IRC, keeping per-user
credential vending and
+per-user authorization intact. Otherwise the session mode is deliberately left
off: the forwarded
+token cannot be exchanged, so it would carry no identity to the IRC. Set
+`gravitino.iceberg.rest-catalog.session` explicitly to override either way. See
[Authentication](./authentication.md) for the full setup.
### Limitations
diff --git
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/iceberg/IcebergCatalogPropertyConverter.java
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/iceberg/IcebergCatalogPropertyConverter.java
index 57762c0df1..2ca2f6b39d 100644
---
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/iceberg/IcebergCatalogPropertyConverter.java
+++
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/iceberg/IcebergCatalogPropertyConverter.java
@@ -65,6 +65,9 @@ public class IcebergCatalogPropertyConverter extends
CatalogPropertyConverter {
private static final String TRINO_ICEBERG_REST_VENDED_CREDENTIALS =
"iceberg.rest-catalog.vended-credentials-enabled";
private static final String TRINO_ICEBERG_REST_SESSION =
"iceberg.rest-catalog.session";
+ private static final String TRINO_ICEBERG_REST_SECURITY =
"iceberg.rest-catalog.security";
+ private static final String TRINO_ICEBERG_REST_SECURITY_OAUTH2 = "OAUTH2";
+ private static final String TRINO_ICEBERG_REST_SESSION_USER = "USER";
private static final String TRINO_FS_HADOOP_ENABLED = "fs.hadoop.enabled";
private static final String TRINO_FS_NATIVE_S3_ENABLED =
"fs.native-s3.enabled";
private static final String TRINO_FS_NATIVE_GCS_ENABLED =
"fs.native-gcs.enabled";
@@ -174,15 +177,15 @@ public class IcebergCatalogPropertyConverter extends
CatalogPropertyConverter {
// precedence, unrelated to HashMap's (unspecified) iteration order.
config.putAll(buildStorageProperties(catalog.getProperties()));
config.put(TRINO_ICEBERG_REST_VENDED_CREDENTIALS, "true");
- if (gravitinoConfig.isForwardUser()) {
- config.put(TRINO_ICEBERG_REST_SESSION, "USER");
- }
// The catalog's own trino.bypass properties override the defaults above,
so that a Trino
// release renaming one of them can be worked around without a connector
change.
config.putAll(super.gravitinoToEngineProperties(catalog.getProperties()));
// The IRC's own authentication is a cluster-level operational setting, so
it takes precedence
// over anything set on a single catalog.
config.putAll(gravitinoConfig.getIcebergRestCatalogConfig());
+ // Runs after the two putAll calls above, because it depends on both the
security mode and
+ // any explicit session value they settle.
+ applyForwardUserSession(gravitinoConfig, config);
warnOnReservedOverrides(catalog, config);
@@ -243,6 +246,27 @@ public class IcebergCatalogPropertyConverter extends
CatalogPropertyConverter {
return jdbcProperties;
}
+ /**
+ * Turns on Trino's per-user Iceberg REST sessions when user forwarding is
enabled and the REST
+ * catalog authenticates with OAuth2. In that session mode Trino signs a
subject JWT for the
+ * session user and attaches it to every request; the Iceberg client
consumes such a token through
+ * an OAuth2 token exchange. Under any other security mode there is no token
endpoint to exchange
+ * it at, so the token carries no user identity and the mode buys nothing.
+ *
+ * <p>An explicit {@code iceberg.rest-catalog.session} coming from the
catalog or the connector
+ * config is left untouched.
+ */
+ private void applyForwardUserSession(
+ GravitinoConfig gravitinoConfig, Map<String, String> config) {
+ if (!gravitinoConfig.isForwardUser() ||
config.containsKey(TRINO_ICEBERG_REST_SESSION)) {
+ return;
+ }
+ if (TRINO_ICEBERG_REST_SECURITY_OAUTH2.equalsIgnoreCase(
+ config.get(TRINO_ICEBERG_REST_SECURITY))) {
+ config.put(TRINO_ICEBERG_REST_SESSION, TRINO_ICEBERG_REST_SESSION_USER);
+ }
+ }
+
// Called before the reserved keys (type/uri/warehouse/prefix) are put into
`config` below, so
// any value already present at this point came from the catalog's own
properties or bypass
// config and is about to be silently overwritten; this logs that so it
isn't a silent no-op.
diff --git
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/iceberg/TestIcebergCatalogPropertyConverter.java
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/iceberg/TestIcebergCatalogPropertyConverter.java
index 74bea3b161..3775d8444b 100644
---
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/iceberg/TestIcebergCatalogPropertyConverter.java
+++
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/iceberg/TestIcebergCatalogPropertyConverter.java
@@ -497,13 +497,27 @@ public class TestIcebergCatalogPropertyConverter {
.put("jdbc-driver", "org.postgresql.Driver")
.build();
- Map<String, String> config =
+ Map<String, String> oauth2Config =
+ buildConnectorConfig(
+ "catalog1",
+ properties,
+ icebergRestConfiguredConfig(
+ ImmutableMap.of(
+ "gravitino.client.session.forwardUser", "true",
+ "gravitino.client.authType", "oauth2")));
+ Assertions.assertEquals("USER",
oauth2Config.get("iceberg.rest-catalog.session"));
+
+ // The session mode can also be reached without the Gravitino client
itself using OAuth2, by
+ // pointing the REST catalog at its own OAuth2 provider.
+ Map<String, String> restOnlyOauth2Config =
buildConnectorConfig(
"catalog1",
properties,
icebergRestConfiguredConfig(
- ImmutableMap.of("gravitino.client.session.forwardUser",
"true")));
- Assertions.assertEquals("USER",
config.get("iceberg.rest-catalog.session"));
+ ImmutableMap.of(
+ "gravitino.client.session.forwardUser", "true",
+ "gravitino.iceberg.rest-catalog.security", "OAUTH2")));
+ Assertions.assertEquals("USER",
restOnlyOauth2Config.get("iceberg.rest-catalog.session"));
Map<String, String> explicitConfig =
buildConnectorConfig(
@@ -512,15 +526,47 @@ public class TestIcebergCatalogPropertyConverter {
icebergRestConfiguredConfig(
ImmutableMap.of(
"gravitino.client.session.forwardUser", "true",
+ "gravitino.client.authType", "oauth2",
"gravitino.iceberg.rest-catalog.session", "NONE")));
Assertions.assertEquals("NONE",
explicitConfig.get("iceberg.rest-catalog.session"));
+ // OAuth2 alone does not enable the mode; forwarding has to be asked for.
+ Map<String, String> noForwardingConfig =
+ buildConnectorConfig(
+ "catalog1",
+ properties,
+
icebergRestConfiguredConfig(ImmutableMap.of("gravitino.client.authType",
"oauth2")));
+
Assertions.assertNull(noForwardingConfig.get("iceberg.rest-catalog.session"));
+
Map<String, String> defaultConfig =
buildConnectorConfig(
"catalog1", properties,
icebergRestConfiguredConfig(ImmutableMap.of()));
Assertions.assertNull(defaultConfig.get("iceberg.rest-catalog.session"));
}
+ @Test
+ public void testBuildConnectorPropertiesSkipsSessionUserWithoutOauth2()
throws Exception {
+ Map<String, String> properties =
+ ImmutableMap.<String, String>builder()
+ .put("catalog-backend", "jdbc")
+ .put("uri", "jdbc:postgresql://localhost:5432/iceberg")
+ .put("jdbc-driver", "org.postgresql.Driver")
+ .build();
+
+ // With authType=simple no iceberg.rest-catalog.security is emitted, so
the REST catalog has no
+ // token endpoint to exchange Trino's subject JWT at and the per-user
session mode must stay
+ // off.
+ Map<String, String> config =
+ buildConnectorConfig(
+ "catalog1",
+ properties,
+ icebergRestConfiguredConfig(
+ ImmutableMap.of(
+ "gravitino.client.session.forwardUser", "true",
+ "gravitino.client.authType", "simple")));
+ Assertions.assertNull(config.get("iceberg.rest-catalog.session"));
+ }
+
@Test
public void testBuildConnectorPropertiesStorageDetection() throws Exception {
Map<String, String> s3Config =
