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 dabf8b685d95..a6b15823f733 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 9b02a6d598ab..2dd71d12d01f 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 53a42f958d73..00fe5f58cb8d 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 8824207b5d8c..9e1a62743408 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 55dd42207388..5dfab1437655 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 a361e89fc4bc..0e202233959e 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 8bfd09ec93e1..e9cb83025908 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