From 3f5df6fe6b5665892ce07875bb85afcd67e9a311 Mon Sep 17 00:00:00 2001 From: Krzysztof Sobolewski Date: Tue, 14 May 2024 10:04:05 +0200 Subject: [PATCH 1/8] Remove deprecated SystemAccessControl#checkCanExecuteQuery It was deprecated in 445 when a replacement method was added; I guess 10 versions is long enough for a transition period. --- .../io/trino/spi/security/SystemAccessControl.java | 14 +------------- .../base/security/AllowAllSystemAccessControl.java | 3 --- .../security/FileBasedSystemAccessControl.java | 8 +------- .../security/ForwardingSystemAccessControl.java | 6 ------ .../java/io/trino/plugin/opa/OpaAccessControl.java | 3 ++- .../java/io/trino/plugin/opa/TestConstants.java | 2 ++ .../io/trino/plugin/opa/TestOpaAccessControl.java | 7 ++++--- 7 files changed, 10 insertions(+), 33 deletions(-) diff --git a/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java b/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java index dabf8b685d950..a6b15823f7332 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java +++ b/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java @@ -127,18 +127,6 @@ default void checkCanSetUser(Optional principal, String userName) denySetUser(principal, userName); } - /** - * Checks if identity can execute a query. - * - * @throws AccessDeniedException if not allowed - * @deprecated use {@link #checkCanExecuteQuery(Identity, QueryId)} - */ - @Deprecated - default void checkCanExecuteQuery(Identity identity) - { - denyExecuteQuery(); - } - /** * Checks if identity can execute a query. * @@ -146,7 +134,7 @@ default void checkCanExecuteQuery(Identity identity) */ default void checkCanExecuteQuery(Identity identity, QueryId queryId) { - checkCanExecuteQuery(identity); + denyExecuteQuery(); } /** diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllSystemAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllSystemAccessControl.java index 9b02a6d598ab0..2dd71d12d01f4 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllSystemAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllSystemAccessControl.java @@ -80,9 +80,6 @@ public void checkCanReadSystemInformation(Identity identity) {} @Override public void checkCanWriteSystemInformation(Identity identity) {} - @Override - public void checkCanExecuteQuery(Identity identity) {} - @Override public void checkCanExecuteQuery(Identity identity, QueryId queryId) {} diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControl.java index 53a42f958d733..00fe5f58cb8d1 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControl.java @@ -304,19 +304,13 @@ public void checkCanSetUser(Optional principal, String userName) } @Override - public void checkCanExecuteQuery(Identity identity) + public void checkCanExecuteQuery(Identity identity, QueryId queryId) { if (!canAccessQuery(identity, Optional.empty(), QueryAccessRule.AccessMode.EXECUTE)) { denyExecuteQuery(); } } - @Override - public void checkCanExecuteQuery(Identity identity, QueryId queryId) - { - checkCanExecuteQuery(identity); - } - @Override public void checkCanViewQueryOwnedBy(Identity identity, Identity queryOwner) { diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java index 8824207b5d8c3..9e1a62743408d 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java @@ -83,12 +83,6 @@ public void checkCanWriteSystemInformation(Identity identity) delegate().checkCanWriteSystemInformation(identity); } - @Override - public void checkCanExecuteQuery(Identity identity) - { - delegate().checkCanExecuteQuery(identity); - } - @Override public void checkCanExecuteQuery(Identity identity, QueryId queryId) { diff --git a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java index 55dd422073887..5dfab1437655e 100644 --- a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java +++ b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java @@ -31,6 +31,7 @@ import io.trino.plugin.opa.schema.TrinoSchema; import io.trino.plugin.opa.schema.TrinoTable; import io.trino.plugin.opa.schema.TrinoUser; +import io.trino.spi.QueryId; import io.trino.spi.connector.CatalogSchemaName; import io.trino.spi.connector.CatalogSchemaRoutineName; import io.trino.spi.connector.CatalogSchemaTableName; @@ -115,7 +116,7 @@ public void checkCanSetUser(Optional principal, String userName) {} @Override - public void checkCanExecuteQuery(Identity identity) + public void checkCanExecuteQuery(Identity identity, QueryId queryId) { opaHighLevelClient.queryAndEnforce(buildQueryContext(identity), "ExecuteQuery", AccessDeniedException::denyExecuteQuery); } diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestConstants.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestConstants.java index a361e89fc4bc0..0e202233959e3 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestConstants.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestConstants.java @@ -17,6 +17,7 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.Tracer; import io.trino.execution.QueryIdGenerator; +import io.trino.spi.QueryId; import io.trino.spi.connector.CatalogSchemaTableName; import io.trino.spi.security.Identity; import io.trino.spi.security.SystemAccessControlFactory; @@ -59,6 +60,7 @@ private TestConstants() {} public static final URI OPA_ROW_FILTERING_URI = URI.create("http://my-row-filtering-uri/"); public static final URI OPA_COLUMN_MASKING_URI = URI.create("http://my-column-masking-uri/"); public static final Identity TEST_IDENTITY = Identity.forUser("source-user").withGroups(ImmutableSet.of("some-group")).build(); + public static final QueryId TEST_QUERY_ID = QueryId.valueOf("abcde"); public static final SystemSecurityContext TEST_SECURITY_CONTEXT = new SystemSecurityContext(TEST_IDENTITY, new QueryIdGenerator().createNextQueryId(), Instant.now()); public static final CatalogSchemaTableName TEST_COLUMN_MASKING_TABLE_NAME = new CatalogSchemaTableName("some_catalog", "some_schema", "some_table"); diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java index 8bfd09ec93e16..e9cb830259087 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java @@ -61,6 +61,7 @@ import static io.trino.plugin.opa.TestConstants.SERVER_ERROR_RESPONSE; import static io.trino.plugin.opa.TestConstants.TEST_COLUMN_MASKING_TABLE_NAME; import static io.trino.plugin.opa.TestConstants.TEST_IDENTITY; +import static io.trino.plugin.opa.TestConstants.TEST_QUERY_ID; import static io.trino.plugin.opa.TestConstants.TEST_SECURITY_CONTEXT; import static io.trino.plugin.opa.TestConstants.UNDEFINED_RESPONSE; import static io.trino.plugin.opa.TestConstants.columnMaskingOpaConfig; @@ -93,13 +94,13 @@ public void testResponseHasExtraFields() }\ """)); OpaAccessControl authorizer = createOpaAuthorizer(simpleOpaConfig(), mockClient); - authorizer.checkCanExecuteQuery(TEST_IDENTITY); + authorizer.checkCanExecuteQuery(TEST_IDENTITY, TEST_QUERY_ID); } @Test public void testNoResourceAction() { - testNoResourceAction("ExecuteQuery", OpaAccessControl::checkCanExecuteQuery); + testNoResourceAction("ExecuteQuery", (opaAccessControl, identity) -> opaAccessControl.checkCanExecuteQuery(identity, TEST_QUERY_ID)); testNoResourceAction("ReadSystemInformation", OpaAccessControl::checkCanReadSystemInformation); testNoResourceAction("WriteSystemInformation", OpaAccessControl::checkCanWriteSystemInformation); } @@ -665,7 +666,7 @@ private void testRequestContextContentsForGivenTrinoVersion(Optional Date: Tue, 14 May 2024 10:07:26 +0200 Subject: [PATCH 2/8] Remove deprecated SystemAccessControl#checkCanSetSystemSessionProperty It was deprecated in 445 when a replacement method was added; I guess 10 versions is long enough for a transition period. --- .../io/trino/spi/security/SystemAccessControl.java | 14 +------------- .../base/security/AllowAllSystemAccessControl.java | 3 --- .../security/FileBasedSystemAccessControl.java | 8 +------- .../security/ForwardingSystemAccessControl.java | 6 ------ .../java/io/trino/plugin/opa/OpaAccessControl.java | 2 +- .../io/trino/plugin/opa/TestOpaAccessControl.java | 2 +- 6 files changed, 4 insertions(+), 31 deletions(-) diff --git a/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java b/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java index a6b15823f7332..eb4a2f0784113 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java +++ b/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java @@ -191,18 +191,6 @@ default void checkCanWriteSystemInformation(Identity identity) denyWriteSystemInformationAccess(); } - /** - * Check if identity is allowed to set the specified system property. - * - * @throws AccessDeniedException if not allowed - * @deprecated use {@link #checkCanSetSystemSessionProperty(Identity, QueryId, String)} - */ - @Deprecated - default void checkCanSetSystemSessionProperty(Identity identity, String propertyName) - { - denySetSystemSessionProperty(propertyName); - } - /** * Check if identity is allowed to set the specified system property. * @@ -210,7 +198,7 @@ default void checkCanSetSystemSessionProperty(Identity identity, String property */ default void checkCanSetSystemSessionProperty(Identity identity, QueryId queryId, String propertyName) { - checkCanSetSystemSessionProperty(identity, propertyName); + denySetSystemSessionProperty(propertyName); } /** diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllSystemAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllSystemAccessControl.java index 2dd71d12d01f4..df6c76e4ab753 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllSystemAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllSystemAccessControl.java @@ -95,9 +95,6 @@ public Collection filterViewQueryOwnedBy(Identity identity, Collection return queryOwners; } - @Override - public void checkCanSetSystemSessionProperty(Identity identity, String propertyName) {} - @Override public void checkCanSetSystemSessionProperty(Identity identity, QueryId queryId, String propertyName) {} diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControl.java index 00fe5f58cb8d1..032242c027908 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControl.java @@ -380,7 +380,7 @@ private boolean checkCanSystemInformation(Identity identity, SystemInformationRu } @Override - public void checkCanSetSystemSessionProperty(Identity identity, String propertyName) + public void checkCanSetSystemSessionProperty(Identity identity, QueryId queryId, String propertyName) { boolean allowed = sessionPropertyRules.stream() .map(rule -> rule.match(identity.getUser(), identity.getEnabledRoles(), identity.getGroups(), propertyName)) @@ -392,12 +392,6 @@ public void checkCanSetSystemSessionProperty(Identity identity, String propertyN } } - @Override - public void checkCanSetSystemSessionProperty(Identity identity, QueryId queryId, String propertyName) - { - checkCanSetSystemSessionProperty(identity, propertyName); - } - @Override public boolean canAccessCatalog(SystemSecurityContext context, String catalogName) { diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java index 9e1a62743408d..8370b3b49ca99 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java @@ -107,12 +107,6 @@ public void checkCanKillQueryOwnedBy(Identity identity, Identity queryOwner) delegate().checkCanKillQueryOwnedBy(identity, queryOwner); } - @Override - public void checkCanSetSystemSessionProperty(Identity identity, String propertyName) - { - delegate().checkCanSetSystemSessionProperty(identity, propertyName); - } - @Override public void checkCanSetSystemSessionProperty(Identity identity, QueryId queryId, String propertyName) { diff --git a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java index 5dfab1437655e..a0b92afee6409 100644 --- a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java +++ b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java @@ -157,7 +157,7 @@ public void checkCanWriteSystemInformation(Identity identity) } @Override - public void checkCanSetSystemSessionProperty(Identity identity, String propertyName) + public void checkCanSetSystemSessionProperty(Identity identity, QueryId queryId, String propertyName) { opaHighLevelClient.queryAndEnforce( buildQueryContext(identity), diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java index e9cb830259087..ad046a8f3a25e 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java @@ -243,7 +243,7 @@ private void testIdentityResourceActions( @Test public void testStringResourceAction() { - testStringResourceAction("SetSystemSessionProperty", "systemSessionProperty", (accessControl, systemSecurityContext, argument) -> accessControl.checkCanSetSystemSessionProperty(systemSecurityContext.getIdentity(), argument)); + testStringResourceAction("SetSystemSessionProperty", "systemSessionProperty", (accessControl, systemSecurityContext, argument) -> accessControl.checkCanSetSystemSessionProperty(systemSecurityContext.getIdentity(), TEST_QUERY_ID, argument)); testStringResourceAction("CreateCatalog", "catalog", OpaAccessControl::checkCanCreateCatalog); testStringResourceAction("DropCatalog", "catalog", OpaAccessControl::checkCanDropCatalog); testStringResourceAction("ShowSchemas", "catalog", OpaAccessControl::checkCanShowSchemas); From 9cba5ede581d69552b782c6fada7ae8a0351d2b6 Mon Sep 17 00:00:00 2001 From: "Mateusz \"Serafin\" Gajewski" Date: Fri, 8 Nov 2024 11:13:55 +0100 Subject: [PATCH 3/8] Update wire-schema to 4.9.11 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 81303c0028f82..1cba863f19c3a 100644 --- a/pom.xml +++ b/pom.xml @@ -212,7 +212,7 @@ 2.2.0 2.0.69.Final 202 - 4.9.9 + 4.9.11 3.9.3 --add-modules=jdk.incubator.vector diff --git a/pom.xml b/pom.xml index 48ad1da3f1aa0..5e81326932bd0 100644 --- a/pom.xml +++ b/pom.xml @@ -1913,13 +1913,13 @@ org.apache.httpcomponents.client5 httpclient5 - 5.3.1 + 5.4.1 org.apache.httpcomponents.core5 httpcore5 - 5.3 + 5.3.1 From 0f11193345a8ab8a5cbaaa63d1eb3c486ce10acc Mon Sep 17 00:00:00 2001 From: "Mateusz \"Serafin\" Gajewski" Date: Fri, 8 Nov 2024 11:17:44 +0100 Subject: [PATCH 7/8] Update mongodb to 5.2.1 --- plugin/trino-mongodb/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/trino-mongodb/pom.xml b/plugin/trino-mongodb/pom.xml index 85a0c0e8d404c..6a33691b1b52f 100644 --- a/plugin/trino-mongodb/pom.xml +++ b/plugin/trino-mongodb/pom.xml @@ -14,7 +14,7 @@ true - 5.2.0 + 5.2.1 From 2ebc6f850cc0fc60f6d134fc45045dc91d5229bc Mon Sep 17 00:00:00 2001 From: "Mateusz \"Serafin\" Gajewski" Date: Fri, 8 Nov 2024 11:20:11 +0100 Subject: [PATCH 8/8] Update opensearch to 2.18.0 --- plugin/trino-opensearch/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/trino-opensearch/pom.xml b/plugin/trino-opensearch/pom.xml index ac17934fbda17..f03bc213a9f17 100644 --- a/plugin/trino-opensearch/pom.xml +++ b/plugin/trino-opensearch/pom.xml @@ -15,7 +15,7 @@ true - 2.17.1 + 2.18.0