From 8018c9214aeb5970c9db14a1f1f85e26d1a4dc3a Mon Sep 17 00:00:00 2001 From: Krzysztof Sobolewski Date: Thu, 24 Mar 2022 14:00:20 +0100 Subject: [PATCH 1/3] Remove unnecessary collect to list before toArray Can just collect to array directly. --- .../main/java/io/trino/sql/gen/LambdaBytecodeGenerator.java | 2 +- .../plugin/base/security/TestFileBasedAccessControl.java | 4 +--- .../base/security/TestFileBasedSystemAccessControl.java | 4 +--- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/sql/gen/LambdaBytecodeGenerator.java b/core/trino-main/src/main/java/io/trino/sql/gen/LambdaBytecodeGenerator.java index 7dd17ebae195..2cb355d0523a 100644 --- a/core/trino-main/src/main/java/io/trino/sql/gen/LambdaBytecodeGenerator.java +++ b/core/trino-main/src/main/java/io/trino/sql/gen/LambdaBytecodeGenerator.java @@ -213,7 +213,7 @@ public static BytecodeNode generateLambda( compiledLambda.getParameterTypes().stream() .skip(captureExpressions.size() + 1) // skip capture variables and ConnectorSession .map(ParameterizedType::getAsmType) - .collect(toImmutableList()).toArray(new Type[0])); + .toArray(Type[]::new)); block.append( invokeDynamic( diff --git a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java index e937842a0a82..44bdc79f53cb 100644 --- a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java +++ b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java @@ -37,7 +37,6 @@ import java.util.Set; import java.util.stream.Stream; -import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.io.Resources.getResource; import static io.trino.spi.security.Privilege.UPDATE; import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden; @@ -258,8 +257,7 @@ public Object[][] privilegeGrantOption() return EnumSet.allOf(Privilege.class) .stream() .flatMap(privilege -> Stream.of(true, false).map(grantOption -> new Object[] {privilege, grantOption})) - .collect(toImmutableList()) - .toArray(new Object[0][0]); + .toArray(Object[][]::new); } @Test diff --git a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java index 8e36a49cbf68..546bf42dbb25 100644 --- a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java +++ b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java @@ -42,7 +42,6 @@ import java.util.Set; import java.util.stream.Stream; -import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.io.Files.copy; import static io.trino.plugin.base.security.FileBasedAccessControlConfig.SECURITY_CONFIG_FILE; import static io.trino.plugin.base.security.FileBasedAccessControlConfig.SECURITY_REFRESH_PERIOD; @@ -367,8 +366,7 @@ public Object[][] privilegeGrantOption() return EnumSet.allOf(Privilege.class) .stream() .flatMap(privilege -> Stream.of(true, false).map(grantOption -> new Object[] {privilege, grantOption})) - .collect(toImmutableList()) - .toArray(new Object[0][0]); + .toArray(Object[][]::new); } @Test From 212b55d40e51318d07309834ad2f3727fa001f7d Mon Sep 17 00:00:00 2001 From: Krzysztof Sobolewski Date: Wed, 23 Mar 2022 15:46:17 +0100 Subject: [PATCH 2/3] Allow returning multiple filters and masks in SystemAccessControl The engine is perfectly capable of processing multiple row filter and column mask expressions, given that it supports running multiple system access controls and each of them can provide an expression. See: `io.trino.security.AccessControlManager#getColumnMasks` and `io.trino.security.AccessControlManager#getRowFilters`. So inability to provide more than one expression per access control looks like an artificial restriction. Case in point: the file-based access control, which is also already capable of providing multiple expressions for both row filters and column masks (it just picked the first one and discarded the rest). --- .../trino/security/AccessControlManager.java | 8 ++--- .../security/TestAccessControlManager.java | 4 +-- .../spi/security/SystemAccessControl.java | 30 +++++++++++++++++++ .../security/AllowAllSystemAccessControl.java | 10 ++++--- .../FileBasedSystemAccessControl.java | 22 ++++++++------ .../ForwardingSystemAccessControl.java | 13 ++++++++ .../TestAllowAllSystemAccessControl.java | 8 ++++- .../TestFileBasedSystemAccessControl.java | 28 +++++++++-------- 8 files changed, 91 insertions(+), 32 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java b/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java index 8b05b5da61c4..08dd4c33658d 100644 --- a/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java +++ b/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java @@ -1170,8 +1170,8 @@ public List getRowFilters(SecurityContext context, QualifiedObje } for (SystemAccessControl systemAccessControl : getSystemAccessControls()) { - systemAccessControl.getRowFilter(context.toSystemSecurityContext(), tableName.asCatalogSchemaTableName()) - .ifPresent(filters::add); + systemAccessControl.getRowFilters(context.toSystemSecurityContext(), tableName.asCatalogSchemaTableName()) + .forEach(filters::add); } return filters.build(); @@ -1193,8 +1193,8 @@ public List getColumnMasks(SecurityContext context, QualifiedObj } for (SystemAccessControl systemAccessControl : getSystemAccessControls()) { - systemAccessControl.getColumnMask(context.toSystemSecurityContext(), tableName.asCatalogSchemaTableName(), columnName, type) - .ifPresent(masks::add); + systemAccessControl.getColumnMasks(context.toSystemSecurityContext(), tableName.asCatalogSchemaTableName(), columnName, type) + .forEach(masks::add); } return masks.build(); diff --git a/core/trino-main/src/test/java/io/trino/security/TestAccessControlManager.java b/core/trino-main/src/test/java/io/trino/security/TestAccessControlManager.java index 3dcaf06348ba..b8268298c82b 100644 --- a/core/trino-main/src/test/java/io/trino/security/TestAccessControlManager.java +++ b/core/trino-main/src/test/java/io/trino/security/TestAccessControlManager.java @@ -206,9 +206,9 @@ public SystemAccessControl create(Map config) return new SystemAccessControl() { @Override - public Optional getColumnMask(SystemSecurityContext context, CatalogSchemaTableName tableName, String column, Type type) + public List getColumnMasks(SystemSecurityContext context, CatalogSchemaTableName tableName, String column, Type type) { - return Optional.of(new ViewExpression("user", Optional.empty(), Optional.empty(), "system mask")); + return ImmutableList.of(new ViewExpression("user", Optional.empty(), Optional.empty(), "system mask")); } @Override 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 cae9262257a5..f31536a1bb7d 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 @@ -22,6 +22,7 @@ import java.security.Principal; import java.util.Collection; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; @@ -823,12 +824,26 @@ default void checkCanExecuteTableProcedure(SystemSecurityContext systemSecurityC * The filter must be a scalar SQL expression of boolean type over the columns in the table. * * @return the filter, or {@link Optional#empty()} if not applicable + * @deprecated use {@link #getRowFilters(SystemSecurityContext, CatalogSchemaTableName)} instead */ + @Deprecated default Optional getRowFilter(SystemSecurityContext context, CatalogSchemaTableName tableName) { return Optional.empty(); } + /** + * Get row filters associated with the given table and identity. + *

+ * Each filter must be a scalar SQL expression of boolean type over the columns in the table. + * + * @return the list of filters, or empty list if not applicable + */ + default List getRowFilters(SystemSecurityContext context, CatalogSchemaTableName tableName) + { + return getRowFilter(context, tableName).map(List::of).orElseGet(List::of); + } + /** * Get a column mask associated with the given table, column and identity. *

@@ -836,12 +851,27 @@ default Optional getRowFilter(SystemSecurityContext context, Cat * must be written in terms of columns in the table. * * @return the mask, or {@link Optional#empty()} if not applicable + * @deprecated use {@link #getColumnMasks(SystemSecurityContext, CatalogSchemaTableName, String, Type)} instead */ + @Deprecated default Optional getColumnMask(SystemSecurityContext context, CatalogSchemaTableName tableName, String columnName, Type type) { return Optional.empty(); } + /** + * Get column masks associated with the given table, column and identity. + *

+ * Each mask must be a scalar SQL expression of a type coercible to the type of the column being masked. The expression + * must be written in terms of columns in the table. + * + * @return the list of masks, or empty list if not applicable + */ + default List getColumnMasks(SystemSecurityContext context, CatalogSchemaTableName tableName, String columnName, Type type) + { + return getColumnMask(context, tableName, columnName, type).map(List::of).orElseGet(List::of); + } + /** * @return the event listeners provided by this system access control */ 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 1e25c076f964..9e2e3b024bb4 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 @@ -30,11 +30,13 @@ import java.security.Principal; import java.util.Collection; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Collections.emptyList; import static java.util.Objects.requireNonNull; public class AllowAllSystemAccessControl @@ -435,14 +437,14 @@ public Iterable getEventListeners() } @Override - public Optional getRowFilter(SystemSecurityContext context, CatalogSchemaTableName tableName) + public List getRowFilters(SystemSecurityContext context, CatalogSchemaTableName tableName) { - return Optional.empty(); + return emptyList(); } @Override - public Optional getColumnMask(SystemSecurityContext context, CatalogSchemaTableName tableName, String columnName, Type type) + public List getColumnMasks(SystemSecurityContext context, CatalogSchemaTableName tableName, String columnName, Type type) { - return Optional.empty(); + return emptyList(); } } 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 ac8c439165ad..a01ee62f85fc 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 @@ -42,11 +42,11 @@ import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.function.Function; import java.util.function.Predicate; import java.util.regex.Pattern; import static com.google.common.base.Suppliers.memoizeWithExpiration; +import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static io.airlift.configuration.ConfigBinder.configBinder; import static io.trino.plugin.base.security.CatalogAccessControlRule.AccessMode.ALL; @@ -950,35 +950,39 @@ public Iterable getEventListeners() } @Override - public Optional getRowFilter(SystemSecurityContext context, CatalogSchemaTableName table) + public List getRowFilters(SystemSecurityContext context, CatalogSchemaTableName table) { SchemaTableName tableName = table.getSchemaTableName(); if (INFORMATION_SCHEMA_NAME.equals(tableName.getSchemaName())) { - return Optional.empty(); + return ImmutableList.of(); } Identity identity = context.getIdentity(); return tableRules.stream() .filter(rule -> rule.matches(identity.getUser(), identity.getEnabledRoles(), identity.getGroups(), table)) .map(rule -> rule.getFilter(identity.getUser(), table.getCatalogName(), tableName.getSchemaName())) - .findFirst() - .flatMap(Function.identity()); + .flatMap(Optional::stream) + // we return the first one we find + .limit(1) + .collect(toImmutableList()); } @Override - public Optional getColumnMask(SystemSecurityContext context, CatalogSchemaTableName table, String columnName, Type type) + public List getColumnMasks(SystemSecurityContext context, CatalogSchemaTableName table, String columnName, Type type) { SchemaTableName tableName = table.getSchemaTableName(); if (INFORMATION_SCHEMA_NAME.equals(tableName.getSchemaName())) { - return Optional.empty(); + return ImmutableList.of(); } Identity identity = context.getIdentity(); return tableRules.stream() .filter(rule -> rule.matches(identity.getUser(), identity.getEnabledRoles(), identity.getGroups(), table)) .map(rule -> rule.getColumnMask(identity.getUser(), table.getCatalogName(), table.getSchemaTableName().getSchemaName(), columnName)) - .findFirst() - .flatMap(Function.identity()); + .flatMap(Optional::stream) + // we return the first one we find + .limit(1) + .collect(toImmutableList()); } private boolean checkAnyCatalogAccess(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 e8a71874f12c..9da0a8db83b4 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 @@ -28,6 +28,7 @@ import java.security.Principal; import java.util.Collection; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -485,9 +486,21 @@ public Optional getRowFilter(SystemSecurityContext context, Cata return delegate().getRowFilter(context, tableName); } + @Override + public List getRowFilters(SystemSecurityContext context, CatalogSchemaTableName tableName) + { + return delegate().getRowFilters(context, tableName); + } + @Override public Optional getColumnMask(SystemSecurityContext context, CatalogSchemaTableName tableName, String columnName, Type type) { return delegate().getColumnMask(context, tableName, columnName, type); } + + @Override + public List getColumnMasks(SystemSecurityContext context, CatalogSchemaTableName tableName, String columnName, Type type) + { + return delegate().getColumnMasks(context, tableName, columnName, type); + } } diff --git a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestAllowAllSystemAccessControl.java b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestAllowAllSystemAccessControl.java index cb30228b39b4..e7f0b2a078db 100644 --- a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestAllowAllSystemAccessControl.java +++ b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestAllowAllSystemAccessControl.java @@ -13,7 +13,11 @@ */ package io.trino.plugin.base.security; +import com.google.common.collect.ImmutableSet; +import io.trino.spi.connector.CatalogSchemaTableName; import io.trino.spi.security.SystemAccessControl; +import io.trino.spi.security.SystemSecurityContext; +import io.trino.spi.type.Type; import org.testng.annotations.Test; import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden; @@ -24,6 +28,8 @@ public class TestAllowAllSystemAccessControl public void testEverythingImplemented() throws ReflectiveOperationException { - assertAllMethodsOverridden(SystemAccessControl.class, AllowAllSystemAccessControl.class); + assertAllMethodsOverridden(SystemAccessControl.class, AllowAllSystemAccessControl.class, ImmutableSet.of( + AllowAllSystemAccessControl.class.getMethod("getRowFilter", SystemSecurityContext.class, CatalogSchemaTableName.class), + AllowAllSystemAccessControl.class.getMethod("getColumnMask", SystemSecurityContext.class, CatalogSchemaTableName.class, String.class, Type.class))); } } diff --git a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java index 546bf42dbb25..146cce6985c2 100644 --- a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java +++ b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.base.security; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; @@ -28,6 +29,7 @@ import io.trino.spi.security.SystemSecurityContext; import io.trino.spi.security.TrinoPrincipal; import io.trino.spi.security.ViewExpression; +import io.trino.spi.type.Type; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -37,6 +39,7 @@ import java.io.File; import java.util.Collection; import java.util.EnumSet; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -54,7 +57,6 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.util.Files.newTemporaryFile; import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertTrue; public class TestFileBasedSystemAccessControl { @@ -1267,15 +1269,15 @@ public void testGetColumnMask() SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-table.json"); assertEquals( - accessControl.getColumnMask( + accessControl.getColumnMasks( ALICE, new CatalogSchemaTableName("some-catalog", "bobschema", "bobcolumns"), "masked", VARCHAR), - Optional.empty()); + ImmutableList.of()); assertViewExpressionEquals( - accessControl.getColumnMask( + accessControl.getColumnMasks( CHARLIE, new CatalogSchemaTableName("some-catalog", "bobschema", "bobcolumns"), "masked", @@ -1283,7 +1285,7 @@ public void testGetColumnMask() new ViewExpression(CHARLIE.getIdentity().getUser(), Optional.of("some-catalog"), Optional.of("bobschema"), "'mask'")); assertViewExpressionEquals( - accessControl.getColumnMask( + accessControl.getColumnMasks( CHARLIE, new CatalogSchemaTableName("some-catalog", "bobschema", "bobcolumns"), "masked_with_user", @@ -1297,22 +1299,22 @@ public void testGetRowFilter() SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-table.json"); assertEquals( - accessControl.getRowFilter(ALICE, new CatalogSchemaTableName("some-catalog", "bobschema", "bobcolumns")), - Optional.empty()); + accessControl.getRowFilters(ALICE, new CatalogSchemaTableName("some-catalog", "bobschema", "bobcolumns")), + ImmutableList.of()); assertViewExpressionEquals( - accessControl.getRowFilter(CHARLIE, new CatalogSchemaTableName("some-catalog", "bobschema", "bobcolumns")), + accessControl.getRowFilters(CHARLIE, new CatalogSchemaTableName("some-catalog", "bobschema", "bobcolumns")), new ViewExpression(CHARLIE.getIdentity().getUser(), Optional.of("some-catalog"), Optional.of("bobschema"), "starts_with(value, 'filter')")); assertViewExpressionEquals( - accessControl.getRowFilter(CHARLIE, new CatalogSchemaTableName("some-catalog", "bobschema", "bobcolumns_with_grant")), + accessControl.getRowFilters(CHARLIE, new CatalogSchemaTableName("some-catalog", "bobschema", "bobcolumns_with_grant")), new ViewExpression("filter-user", Optional.of("some-catalog"), Optional.of("bobschema"), "starts_with(value, 'filter-with-user')")); } - private static void assertViewExpressionEquals(Optional result, ViewExpression expected) + private static void assertViewExpressionEquals(List result, ViewExpression expected) { - assertTrue(result.isPresent()); - ViewExpression actual = result.get(); + assertEquals(result.size(), 1); + ViewExpression actual = result.get(0); assertEquals(actual.getIdentity(), expected.getIdentity(), "Identity"); assertEquals(actual.getCatalog(), expected.getCatalog(), "Catalog"); assertEquals(actual.getSchema(), expected.getSchema(), "Schema"); @@ -1324,6 +1326,8 @@ public void testEverythingImplemented() throws NoSuchMethodException { assertAllMethodsOverridden(SystemAccessControl.class, FileBasedSystemAccessControl.class, ImmutableSet.of( + FileBasedSystemAccessControl.class.getMethod("getRowFilter", SystemSecurityContext.class, CatalogSchemaTableName.class), + FileBasedSystemAccessControl.class.getMethod("getColumnMask", SystemSecurityContext.class, CatalogSchemaTableName.class, String.class, Type.class), FileBasedSystemAccessControl.class.getMethod("checkCanViewQueryOwnedBy", SystemSecurityContext.class, Identity.class), FileBasedSystemAccessControl.class.getMethod("filterViewQueryOwnedBy", SystemSecurityContext.class, Collection.class), FileBasedSystemAccessControl.class.getMethod("checkCanKillQueryOwnedBy", SystemSecurityContext.class, Identity.class))); From a63e04b9200e05da971af7ec2f623b6adec668a7 Mon Sep 17 00:00:00 2001 From: Krzysztof Sobolewski Date: Wed, 23 Mar 2022 15:55:24 +0100 Subject: [PATCH 3/3] Allow returning multiple filters and masks in ConnectorAccessControl Now that the `SystemAccessControl` can provide multiple filtering and masking expressions, there's no reason for the `ConnectorAccessControl` not to follow suit. --- .../trino/security/AccessControlManager.java | 8 ++--- .../InjectedConnectorAccessControl.java | 10 +++--- .../connector/MockConnectorAccessControl.java | 14 ++++++--- .../security/TestAccessControlManager.java | 4 +-- .../TestInjectedConnectorAccessControl.java | 9 +++++- .../spi/connector/ConnectorAccessControl.java | 31 +++++++++++++++++++ ...ClassLoaderSafeConnectorAccessControl.java | 9 +++--- .../base/security/AllowAllAccessControl.java | 10 +++--- .../base/security/FileBasedAccessControl.java | 23 ++++++++------ .../ForwardingConnectorAccessControl.java | 13 ++++++++ .../TestClassLoaderSafeWrappers.java | 9 +++++- .../security/TestAllowAllAccessControl.java | 9 +++++- .../security/TestFileBasedAccessControl.java | 6 +++- .../hive/security/LegacyAccessControl.java | 10 +++--- .../security/SqlStandardAccessControl.java | 10 +++--- .../security/TestLegacyAccessControl.java | 9 +++++- .../TestSqlStandardAccessControl.java | 9 +++++- 17 files changed, 148 insertions(+), 45 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java b/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java index 08dd4c33658d..0b1df4f88a00 100644 --- a/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java +++ b/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java @@ -1165,8 +1165,8 @@ public List getRowFilters(SecurityContext context, QualifiedObje CatalogAccessControlEntry entry = getConnectorAccessControl(context.getTransactionId(), tableName.getCatalogName()); if (entry != null) { - entry.getAccessControl().getRowFilter(entry.toConnectorSecurityContext(context), tableName.asSchemaTableName()) - .ifPresent(filters::add); + entry.getAccessControl().getRowFilters(entry.toConnectorSecurityContext(context), tableName.asSchemaTableName()) + .forEach(filters::add); } for (SystemAccessControl systemAccessControl : getSystemAccessControls()) { @@ -1188,8 +1188,8 @@ public List getColumnMasks(SecurityContext context, QualifiedObj // connector-provided masks take precedence over global masks CatalogAccessControlEntry entry = getConnectorAccessControl(context.getTransactionId(), tableName.getCatalogName()); if (entry != null) { - entry.getAccessControl().getColumnMask(entry.toConnectorSecurityContext(context), tableName.asSchemaTableName(), columnName, type) - .ifPresent(masks::add); + entry.getAccessControl().getColumnMasks(entry.toConnectorSecurityContext(context), tableName.asSchemaTableName(), columnName, type) + .forEach(masks::add); } for (SystemAccessControl systemAccessControl : getSystemAccessControls()) { diff --git a/core/trino-main/src/main/java/io/trino/security/InjectedConnectorAccessControl.java b/core/trino-main/src/main/java/io/trino/security/InjectedConnectorAccessControl.java index 9ef20a4cd92f..a825110157cf 100644 --- a/core/trino-main/src/main/java/io/trino/security/InjectedConnectorAccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/InjectedConnectorAccessControl.java @@ -13,6 +13,7 @@ */ package io.trino.security; +import com.google.common.collect.ImmutableList; import io.trino.metadata.QualifiedObjectName; import io.trino.spi.TrinoException; import io.trino.spi.connector.CatalogSchemaName; @@ -26,6 +27,7 @@ import io.trino.spi.security.ViewExpression; import io.trino.spi.type.Type; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -445,21 +447,21 @@ public void checkCanExecuteTableProcedure(ConnectorSecurityContext context, Sche } @Override - public Optional getRowFilter(ConnectorSecurityContext context, SchemaTableName tableName) + public List getRowFilters(ConnectorSecurityContext context, SchemaTableName tableName) { checkArgument(context == null, "context must be null"); if (accessControl.getRowFilters(securityContext, new QualifiedObjectName(catalogName, tableName.getSchemaName(), tableName.getTableName())).isEmpty()) { - return Optional.empty(); + return ImmutableList.of(); } throw new TrinoException(NOT_SUPPORTED, "Row filtering not supported"); } @Override - public Optional getColumnMask(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) + public List getColumnMasks(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) { checkArgument(context == null, "context must be null"); if (accessControl.getColumnMasks(securityContext, new QualifiedObjectName(catalogName, tableName.getSchemaName(), tableName.getTableName()), columnName, type).isEmpty()) { - return Optional.empty(); + return ImmutableList.of(); } throw new TrinoException(NOT_SUPPORTED, "Column masking not supported"); } diff --git a/core/trino-main/src/test/java/io/trino/connector/MockConnectorAccessControl.java b/core/trino-main/src/test/java/io/trino/connector/MockConnectorAccessControl.java index 9f96b9d4e779..dd50b7d68311 100644 --- a/core/trino-main/src/test/java/io/trino/connector/MockConnectorAccessControl.java +++ b/core/trino-main/src/test/java/io/trino/connector/MockConnectorAccessControl.java @@ -13,6 +13,7 @@ */ package io.trino.connector; +import com.google.common.collect.ImmutableList; import io.trino.plugin.base.security.AllowAllAccessControl; import io.trino.spi.connector.ConnectorSecurityContext; import io.trino.spi.connector.SchemaTableName; @@ -23,6 +24,7 @@ import io.trino.spi.type.Type; import java.util.Arrays; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.function.BiFunction; @@ -120,15 +122,19 @@ public void checkCanRevokeTablePrivilege(ConnectorSecurityContext context, Privi } @Override - public Optional getRowFilter(ConnectorSecurityContext context, SchemaTableName tableName) + public List getRowFilters(ConnectorSecurityContext context, SchemaTableName tableName) { - return Optional.ofNullable(rowFilters.apply(tableName)); + return Optional.ofNullable(rowFilters.apply(tableName)) + .map(ImmutableList::of) + .orElseGet(ImmutableList::of); } @Override - public Optional getColumnMask(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) + public List getColumnMasks(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) { - return Optional.ofNullable(columnMasks.apply(tableName, columnName)); + return Optional.ofNullable(columnMasks.apply(tableName, columnName)) + .map(ImmutableList::of) + .orElseGet(ImmutableList::of); } public void grantSchemaPrivileges(String schemaName, Set privileges, TrinoPrincipal grantee, boolean grantOption) diff --git a/core/trino-main/src/test/java/io/trino/security/TestAccessControlManager.java b/core/trino-main/src/test/java/io/trino/security/TestAccessControlManager.java index b8268298c82b..f127aabc22f8 100644 --- a/core/trino-main/src/test/java/io/trino/security/TestAccessControlManager.java +++ b/core/trino-main/src/test/java/io/trino/security/TestAccessControlManager.java @@ -224,9 +224,9 @@ public void checkCanSetSystemSessionProperty(SystemSecurityContext context, Stri accessControlManager.addCatalogAccessControl(new CatalogName("catalog"), new ConnectorAccessControl() { @Override - public Optional getColumnMask(ConnectorSecurityContext context, SchemaTableName tableName, String column, Type type) + public List getColumnMasks(ConnectorSecurityContext context, SchemaTableName tableName, String column, Type type) { - return Optional.of(new ViewExpression("user", Optional.empty(), Optional.empty(), "connector mask")); + return ImmutableList.of(new ViewExpression("user", Optional.empty(), Optional.empty(), "connector mask")); } @Override diff --git a/core/trino-main/src/test/java/io/trino/security/TestInjectedConnectorAccessControl.java b/core/trino-main/src/test/java/io/trino/security/TestInjectedConnectorAccessControl.java index 151424cf0f73..3e2e0c0bc626 100644 --- a/core/trino-main/src/test/java/io/trino/security/TestInjectedConnectorAccessControl.java +++ b/core/trino-main/src/test/java/io/trino/security/TestInjectedConnectorAccessControl.java @@ -13,7 +13,11 @@ */ package io.trino.security; +import com.google.common.collect.ImmutableSet; import io.trino.spi.connector.ConnectorAccessControl; +import io.trino.spi.connector.ConnectorSecurityContext; +import io.trino.spi.connector.SchemaTableName; +import io.trino.spi.type.Type; import org.testng.annotations.Test; import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden; @@ -22,7 +26,10 @@ public class TestInjectedConnectorAccessControl { @Test public void testEverythingImplemented() + throws NoSuchMethodException { - assertAllMethodsOverridden(ConnectorAccessControl.class, InjectedConnectorAccessControl.class); + assertAllMethodsOverridden(ConnectorAccessControl.class, InjectedConnectorAccessControl.class, ImmutableSet.of( + InjectedConnectorAccessControl.class.getMethod("getRowFilter", ConnectorSecurityContext.class, SchemaTableName.class), + InjectedConnectorAccessControl.class.getMethod("getColumnMask", ConnectorSecurityContext.class, SchemaTableName.class, String.class, Type.class))); } } diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java index 441b6a247072..3bdb9f1a7340 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java @@ -18,6 +18,7 @@ import io.trino.spi.security.ViewExpression; import io.trino.spi.type.Type; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -74,6 +75,7 @@ import static io.trino.spi.security.AccessDeniedException.denyShowTables; import static io.trino.spi.security.AccessDeniedException.denyTruncateTable; import static io.trino.spi.security.AccessDeniedException.denyUpdateTableColumns; +import static java.util.Collections.emptyList; import static java.util.Collections.emptySet; public interface ConnectorAccessControl @@ -600,12 +602,26 @@ default void checkCanExecuteTableProcedure(ConnectorSecurityContext context, Sch * The filter must be a scalar SQL expression of boolean type over the columns in the table. * * @return the filter, or {@link Optional#empty()} if not applicable + * @deprecated use {@link #getRowFilters(ConnectorSecurityContext, SchemaTableName)} instead */ + @Deprecated default Optional getRowFilter(ConnectorSecurityContext context, SchemaTableName tableName) { return Optional.empty(); } + /** + * Get row filters associated with the given table and identity. + *

+ * Each filter must be a scalar SQL expression of boolean type over the columns in the table. + * + * @return the list of filters, or empty list if not applicable + */ + default List getRowFilters(ConnectorSecurityContext context, SchemaTableName tableName) + { + return emptyList(); + } + /** * Get a column mask associated with the given table, column and identity. *

@@ -613,9 +629,24 @@ default Optional getRowFilter(ConnectorSecurityContext context, * must be written in terms of columns in the table. * * @return the mask, or {@link Optional#empty()} if not applicable + * @deprecated use {@link #getColumnMasks(ConnectorSecurityContext, SchemaTableName, String, Type)} instead */ + @Deprecated default Optional getColumnMask(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) { return Optional.empty(); } + + /** + * Get column masks associated with the given table, column and identity. + *

+ * Each mask must be a scalar SQL expression of a type coercible to the type of the column being masked. The expression + * must be written in terms of columns in the table. + * + * @return the list of masks, or empty list if not applicable + */ + default List getColumnMasks(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) + { + return emptyList(); + } } diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorAccessControl.java index f10be3892d88..e20d201224bc 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorAccessControl.java @@ -25,6 +25,7 @@ import javax.inject.Inject; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -493,18 +494,18 @@ public void checkCanExecuteTableProcedure(ConnectorSecurityContext context, Sche } @Override - public Optional getRowFilter(ConnectorSecurityContext context, SchemaTableName tableName) + public List getRowFilters(ConnectorSecurityContext context, SchemaTableName tableName) { try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { - return delegate.getRowFilter(context, tableName); + return delegate.getRowFilters(context, tableName); } } @Override - public Optional getColumnMask(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) + public List getColumnMasks(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) { try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { - return delegate.getColumnMask(context, tableName, columnName, type); + return delegate.getColumnMasks(context, tableName, columnName, type); } } } diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllAccessControl.java index 8ab0f2a35642..3c32116427d6 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllAccessControl.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.base.security; +import com.google.common.collect.ImmutableList; import io.trino.spi.connector.ConnectorAccessControl; import io.trino.spi.connector.ConnectorSecurityContext; import io.trino.spi.connector.SchemaRoutineName; @@ -22,6 +23,7 @@ import io.trino.spi.security.ViewExpression; import io.trino.spi.type.Type; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -316,14 +318,14 @@ public void checkCanExecuteTableProcedure(ConnectorSecurityContext context, Sche } @Override - public Optional getRowFilter(ConnectorSecurityContext context, SchemaTableName tableName) + public List getRowFilters(ConnectorSecurityContext context, SchemaTableName tableName) { - return Optional.empty(); + return ImmutableList.of(); } @Override - public Optional getColumnMask(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) + public List getColumnMasks(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) { - return Optional.empty(); + return ImmutableList.of(); } } diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControl.java index aae17963100e..381c0a0b95c3 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControl.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.base.security; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import io.trino.plugin.base.CatalogName; import io.trino.plugin.base.security.TableAccessControlRule.TablePrivilege; @@ -31,10 +32,10 @@ import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.function.Function; import java.util.function.Predicate; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static io.trino.plugin.base.security.TableAccessControlRule.TablePrivilege.DELETE; import static io.trino.plugin.base.security.TableAccessControlRule.TablePrivilege.GRANT_SELECT; @@ -591,33 +592,37 @@ public void checkCanExecuteTableProcedure(ConnectorSecurityContext context, Sche } @Override - public Optional getRowFilter(ConnectorSecurityContext context, SchemaTableName tableName) + public List getRowFilters(ConnectorSecurityContext context, SchemaTableName tableName) { if (INFORMATION_SCHEMA_NAME.equals(tableName.getSchemaName())) { - return Optional.empty(); + return ImmutableList.of(); } ConnectorIdentity identity = context.getIdentity(); return tableRules.stream() .filter(rule -> rule.matches(identity.getUser(), identity.getEnabledSystemRoles(), identity.getGroups(), tableName)) .map(rule -> rule.getFilter(identity.getUser(), catalogName, tableName.getSchemaName())) - .findFirst() - .flatMap(Function.identity()); + .flatMap(Optional::stream) + // we return the first one we find + .limit(1) + .collect(toImmutableList()); } @Override - public Optional getColumnMask(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) + public List getColumnMasks(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) { if (INFORMATION_SCHEMA_NAME.equals(tableName.getSchemaName())) { - return Optional.empty(); + return ImmutableList.of(); } ConnectorIdentity identity = context.getIdentity(); return tableRules.stream() .filter(rule -> rule.matches(identity.getUser(), identity.getEnabledSystemRoles(), identity.getGroups(), tableName)) .map(rule -> rule.getColumnMask(identity.getUser(), catalogName, tableName.getSchemaName(), columnName)) - .findFirst() - .flatMap(Function.identity()); + .flatMap(Optional::stream) + // we return the first one we find + .limit(1) + .collect(toImmutableList()); } private boolean canSetSessionProperty(ConnectorSecurityContext context, String property) diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java index 1de3a6c9ef22..7ca4f8af67bd 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java @@ -22,6 +22,7 @@ import io.trino.spi.security.ViewExpression; import io.trino.spi.type.Type; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -391,9 +392,21 @@ public Optional getRowFilter(ConnectorSecurityContext context, S return delegate().getRowFilter(context, tableName); } + @Override + public List getRowFilters(ConnectorSecurityContext context, SchemaTableName tableName) + { + return delegate().getRowFilters(context, tableName); + } + @Override public Optional getColumnMask(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) { return delegate().getColumnMask(context, tableName, columnName, type); } + + @Override + public List getColumnMasks(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) + { + return delegate().getColumnMasks(context, tableName, columnName, type); + } } diff --git a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/classloader/TestClassLoaderSafeWrappers.java b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/classloader/TestClassLoaderSafeWrappers.java index 39c17fa663b9..afb9a64ced67 100644 --- a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/classloader/TestClassLoaderSafeWrappers.java +++ b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/classloader/TestClassLoaderSafeWrappers.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.base.classloader; +import com.google.common.collect.ImmutableSet; import io.trino.spi.connector.ConnectorAccessControl; import io.trino.spi.connector.ConnectorMetadata; import io.trino.spi.connector.ConnectorNodePartitioningProvider; @@ -20,11 +21,14 @@ import io.trino.spi.connector.ConnectorPageSinkProvider; import io.trino.spi.connector.ConnectorPageSourceProvider; import io.trino.spi.connector.ConnectorRecordSetProvider; +import io.trino.spi.connector.ConnectorSecurityContext; import io.trino.spi.connector.ConnectorSplitManager; import io.trino.spi.connector.ConnectorSplitSource; import io.trino.spi.connector.RecordSet; +import io.trino.spi.connector.SchemaTableName; import io.trino.spi.connector.SystemTable; import io.trino.spi.eventlistener.EventListener; +import io.trino.spi.type.Type; import org.testng.annotations.Test; import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden; @@ -33,8 +37,11 @@ public class TestClassLoaderSafeWrappers { @Test public void testAllMethodsOverridden() + throws NoSuchMethodException { - assertAllMethodsOverridden(ConnectorAccessControl.class, ClassLoaderSafeConnectorAccessControl.class); + assertAllMethodsOverridden(ConnectorAccessControl.class, ClassLoaderSafeConnectorAccessControl.class, ImmutableSet.of( + ClassLoaderSafeConnectorAccessControl.class.getMethod("getRowFilter", ConnectorSecurityContext.class, SchemaTableName.class), + ClassLoaderSafeConnectorAccessControl.class.getMethod("getColumnMask", ConnectorSecurityContext.class, SchemaTableName.class, String.class, Type.class))); assertAllMethodsOverridden(ConnectorMetadata.class, ClassLoaderSafeConnectorMetadata.class); assertAllMethodsOverridden(ConnectorPageSink.class, ClassLoaderSafeConnectorPageSink.class); assertAllMethodsOverridden(ConnectorPageSinkProvider.class, ClassLoaderSafeConnectorPageSinkProvider.class); diff --git a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestAllowAllAccessControl.java b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestAllowAllAccessControl.java index 5ba6e98a7570..f53c0c0c4739 100644 --- a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestAllowAllAccessControl.java +++ b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestAllowAllAccessControl.java @@ -13,7 +13,11 @@ */ package io.trino.plugin.base.security; +import com.google.common.collect.ImmutableSet; import io.trino.spi.connector.ConnectorAccessControl; +import io.trino.spi.connector.ConnectorSecurityContext; +import io.trino.spi.connector.SchemaTableName; +import io.trino.spi.type.Type; import org.testng.annotations.Test; import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden; @@ -22,7 +26,10 @@ public class TestAllowAllAccessControl { @Test public void testEverythingImplemented() + throws NoSuchMethodException { - assertAllMethodsOverridden(ConnectorAccessControl.class, AllowAllAccessControl.class); + assertAllMethodsOverridden(ConnectorAccessControl.class, AllowAllAccessControl.class, ImmutableSet.of( + AllowAllAccessControl.class.getMethod("getRowFilter", ConnectorSecurityContext.class, SchemaTableName.class), + AllowAllAccessControl.class.getMethod("getColumnMask", ConnectorSecurityContext.class, SchemaTableName.class, String.class, Type.class))); } } diff --git a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java index 44bdc79f53cb..51616dd9b92a 100644 --- a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java +++ b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java @@ -26,6 +26,7 @@ import io.trino.spi.security.PrincipalType; import io.trino.spi.security.Privilege; import io.trino.spi.security.TrinoPrincipal; +import io.trino.spi.type.Type; import org.testng.Assert.ThrowingRunnable; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -461,8 +462,11 @@ public void testSchemaRulesForCheckCanShowTables() @Test public void testEverythingImplemented() + throws NoSuchMethodException { - assertAllMethodsOverridden(ConnectorAccessControl.class, FileBasedAccessControl.class); + assertAllMethodsOverridden(ConnectorAccessControl.class, FileBasedAccessControl.class, ImmutableSet.of( + FileBasedAccessControl.class.getMethod("getRowFilter", ConnectorSecurityContext.class, SchemaTableName.class), + FileBasedAccessControl.class.getMethod("getColumnMask", ConnectorSecurityContext.class, SchemaTableName.class, String.class, Type.class))); } private static ConnectorSecurityContext user(String name, Set groups) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/LegacyAccessControl.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/LegacyAccessControl.java index 054430bffeca..56dd4f1946e1 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/LegacyAccessControl.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/LegacyAccessControl.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.hive.security; +import com.google.common.collect.ImmutableList; import io.trino.plugin.hive.metastore.Table; import io.trino.spi.connector.ConnectorAccessControl; import io.trino.spi.connector.ConnectorSecurityContext; @@ -25,6 +26,7 @@ import javax.inject.Inject; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -387,14 +389,14 @@ public void checkCanExecuteTableProcedure(ConnectorSecurityContext context, Sche } @Override - public Optional getRowFilter(ConnectorSecurityContext context, SchemaTableName tableName) + public List getRowFilters(ConnectorSecurityContext context, SchemaTableName tableName) { - return Optional.empty(); + return ImmutableList.of(); } @Override - public Optional getColumnMask(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) + public List getColumnMasks(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) { - return Optional.empty(); + return ImmutableList.of(); } } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SqlStandardAccessControl.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SqlStandardAccessControl.java index d6b49367e4c7..2cb23f9db4db 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SqlStandardAccessControl.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SqlStandardAccessControl.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.hive.security; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import io.trino.plugin.base.CatalogName; import io.trino.plugin.hive.metastore.Database; @@ -33,6 +34,7 @@ import javax.inject.Inject; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -568,15 +570,15 @@ public void checkCanExecuteTableProcedure(ConnectorSecurityContext context, Sche } @Override - public Optional getRowFilter(ConnectorSecurityContext context, SchemaTableName tableName) + public List getRowFilters(ConnectorSecurityContext context, SchemaTableName tableName) { - return Optional.empty(); + return ImmutableList.of(); } @Override - public Optional getColumnMask(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) + public List getColumnMasks(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type) { - return Optional.empty(); + return ImmutableList.of(); } private boolean isAdmin(ConnectorSecurityContext context) diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/security/TestLegacyAccessControl.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/security/TestLegacyAccessControl.java index ece785051db0..cf2081b62a11 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/security/TestLegacyAccessControl.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/security/TestLegacyAccessControl.java @@ -13,7 +13,11 @@ */ package io.trino.plugin.hive.security; +import com.google.common.collect.ImmutableSet; import io.trino.spi.connector.ConnectorAccessControl; +import io.trino.spi.connector.ConnectorSecurityContext; +import io.trino.spi.connector.SchemaTableName; +import io.trino.spi.type.Type; import org.testng.annotations.Test; import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden; @@ -22,7 +26,10 @@ public class TestLegacyAccessControl { @Test public void testEverythingImplemented() + throws NoSuchMethodException { - assertAllMethodsOverridden(ConnectorAccessControl.class, LegacyAccessControl.class); + assertAllMethodsOverridden(ConnectorAccessControl.class, LegacyAccessControl.class, ImmutableSet.of( + LegacyAccessControl.class.getMethod("getRowFilter", ConnectorSecurityContext.class, SchemaTableName.class), + LegacyAccessControl.class.getMethod("getColumnMask", ConnectorSecurityContext.class, SchemaTableName.class, String.class, Type.class))); } } diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/security/TestSqlStandardAccessControl.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/security/TestSqlStandardAccessControl.java index 89c36c22e38a..b27d4fa16497 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/security/TestSqlStandardAccessControl.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/security/TestSqlStandardAccessControl.java @@ -13,7 +13,11 @@ */ package io.trino.plugin.hive.security; +import com.google.common.collect.ImmutableSet; import io.trino.spi.connector.ConnectorAccessControl; +import io.trino.spi.connector.ConnectorSecurityContext; +import io.trino.spi.connector.SchemaTableName; +import io.trino.spi.type.Type; import org.testng.annotations.Test; import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden; @@ -22,7 +26,10 @@ public class TestSqlStandardAccessControl { @Test public void testEverythingImplemented() + throws NoSuchMethodException { - assertAllMethodsOverridden(ConnectorAccessControl.class, SqlStandardAccessControl.class); + assertAllMethodsOverridden(ConnectorAccessControl.class, SqlStandardAccessControl.class, ImmutableSet.of( + SqlStandardAccessControl.class.getMethod("getRowFilter", ConnectorSecurityContext.class, SchemaTableName.class), + SqlStandardAccessControl.class.getMethod("getColumnMask", ConnectorSecurityContext.class, SchemaTableName.class, String.class, Type.class))); } }