Ensure first table matched rule in access control

although it doesn't have masked columns or table filters

lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControl.java

@@ -607,9 +607,10 @@ public List<ViewExpression> getRowFilters(ConnectorSecurityContext context, Sche
         return tableRules.stream()
                 .filter(rule -> rule.matches(identity.getUser(), identity.getEnabledSystemRoles(), identity.getGroups(), tableName))
                 .map(rule -> rule.getFilter(identity.getUser(), catalogName, tableName.getSchemaName()))
-                .flatMap(Optional::stream)
                 // we return the first one we find
-                .limit(1)
+                .findFirst()
+                .stream()
+                .flatMap(Optional::stream)
                 .collect(toImmutableList());
     }
 
@@ -624,9 +625,10 @@ public List<ViewExpression> getColumnMasks(ConnectorSecurityContext context, Sch
         return tableRules.stream()
                 .filter(rule -> rule.matches(identity.getUser(), identity.getEnabledSystemRoles(), identity.getGroups(), tableName))
                 .map(rule -> rule.getColumnMask(identity.getUser(), catalogName, tableName.getSchemaName(), columnName))
-                .flatMap(Optional::stream)
                 // we return the first one we find
-                .limit(1)
+                .findFirst()
+                .stream()
+                .flatMap(Optional::stream)
                 .collect(toImmutableList());
     }
 

lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControl.java

@@ -966,9 +966,10 @@ public List<ViewExpression> getRowFilters(SystemSecurityContext context, Catalog
         return tableRules.stream()
                 .filter(rule -> rule.matches(identity.getUser(), identity.getEnabledRoles(), identity.getGroups(), table))
                 .map(rule -> rule.getFilter(identity.getUser(), table.getCatalogName(), tableName.getSchemaName()))
-                .flatMap(Optional::stream)
                 // we return the first one we find
-                .limit(1)
+                .findFirst()
+                .stream()
+                .flatMap(Optional::stream)
                 .collect(toImmutableList());
     }
 
@@ -984,9 +985,10 @@ public List<ViewExpression> getColumnMasks(SystemSecurityContext context, Catalo
         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))
-                .flatMap(Optional::stream)
                 // we return the first one we find
-                .limit(1)
+                .findFirst()
+                .stream()
+                .flatMap(Optional::stream)
                 .collect(toImmutableList());
     }
 

lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.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 io.trino.plugin.base.CatalogName;
@@ -26,6 +27,7 @@
 import io.trino.spi.security.PrincipalType;
 import io.trino.spi.security.Privilege;
 import io.trino.spi.security.TrinoPrincipal;
+import io.trino.spi.security.ViewExpression;
 import io.trino.spi.type.Type;
 import org.testng.Assert.ThrowingRunnable;
 import org.testng.annotations.DataProvider;
@@ -34,6 +36,7 @@
 import java.io.File;
 import java.net.URISyntaxException;
 import java.util.EnumSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
@@ -42,6 +45,7 @@
 import static com.google.common.io.Resources.getResource;
 import static io.trino.spi.security.Privilege.UPDATE;
 import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden;
+import static io.trino.spi.type.VarcharType.VARCHAR;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.testng.Assert.assertEquals;
 
@@ -349,6 +353,75 @@ public void testTableRules()
         assertDenied(() -> accessControl.checkCanSetViewAuthorization(ALICE, bobTable, new TrinoPrincipal(PrincipalType.USER, "some_user")));
     }
 
+    @Test
+    public void testTableRulesForMixedGroupUsers()
+    {
+        SchemaTableName myTable = new SchemaTableName("my_schema", "my_table");
+
+        ConnectorAccessControl accessControl = createAccessControl("table-mixed-groups.json");
+
+        ConnectorSecurityContext userGroup1Group2 = user("user_1_2", ImmutableSet.of("group1", "group2"));
+        ConnectorSecurityContext userGroup2 = user("user_2", ImmutableSet.of("group2"));
+
+        accessControl.checkCanCreateTable(userGroup1Group2, myTable, Map.of());
+        accessControl.checkCanInsertIntoTable(userGroup1Group2, myTable);
+        accessControl.checkCanDeleteFromTable(userGroup1Group2, myTable);
+        accessControl.checkCanDropTable(userGroup1Group2, myTable);
+        accessControl.checkCanSelectFromColumns(userGroup1Group2, myTable, ImmutableSet.of());
+        assertEquals(
+                accessControl.getColumnMasks(userGroup1Group2, myTable, "col_a", VARCHAR),
+                ImmutableList.of());
+        assertEquals(
+                accessControl.getRowFilters(userGroup1Group2, myTable),
+                ImmutableList.of());
+
+        assertDenied(() -> accessControl.checkCanCreateTable(userGroup2, myTable, Map.of()));
+        assertDenied(() -> accessControl.checkCanInsertIntoTable(userGroup2, myTable));
+        assertDenied(() -> accessControl.checkCanDeleteFromTable(userGroup2, myTable));
+        assertDenied(() -> accessControl.checkCanDropTable(userGroup2, myTable));
+        accessControl.checkCanSelectFromColumns(userGroup2, myTable, ImmutableSet.of());
+        assertViewExpressionEquals(
+                accessControl.getColumnMasks(userGroup2, myTable, "col_a", VARCHAR),
+                new ViewExpression(userGroup2.getIdentity().getUser(), Optional.of("test_catalog"), Optional.of("my_schema"), "'mask_a'"));
+        assertEquals(
+                accessControl.getRowFilters(userGroup2, myTable),
+                ImmutableList.of());
+
+        ConnectorSecurityContext userGroup1Group3 = user("user_1_3", ImmutableSet.of("group1", "group3"));
+        ConnectorSecurityContext userGroup3 = user("user_3", ImmutableSet.of("group3"));
+
+        accessControl.checkCanCreateTable(userGroup1Group3, myTable, Map.of());
+        accessControl.checkCanInsertIntoTable(userGroup1Group3, myTable);
+        accessControl.checkCanDeleteFromTable(userGroup1Group3, myTable);
+        accessControl.checkCanDropTable(userGroup1Group3, myTable);
+        accessControl.checkCanSelectFromColumns(userGroup1Group3, myTable, ImmutableSet.of());
+        assertEquals(
+                accessControl.getColumnMasks(userGroup1Group3, myTable, "col_a", VARCHAR),
+                ImmutableList.of());
+
+        assertDenied(() -> accessControl.checkCanCreateTable(userGroup3, myTable, Map.of()));
+        assertDenied(() -> accessControl.checkCanInsertIntoTable(userGroup3, myTable));
+        assertDenied(() -> accessControl.checkCanDeleteFromTable(userGroup3, myTable));
+        assertDenied(() -> accessControl.checkCanDropTable(userGroup3, myTable));
+        accessControl.checkCanSelectFromColumns(userGroup3, myTable, ImmutableSet.of());
+        assertViewExpressionEquals(
+                accessControl.getColumnMasks(userGroup3, myTable, "col_a", VARCHAR),
+                new ViewExpression(userGroup3.getIdentity().getUser(), Optional.of("test_catalog"), Optional.of("my_schema"), "'mask_a'"));
+        assertViewExpressionEquals(
+                accessControl.getRowFilters(userGroup3, myTable),
+                new ViewExpression(userGroup3.getIdentity().getUser(), Optional.of("test_catalog"), Optional.of("my_schema"), "country='US'"));
+    }
+
+    private static void assertViewExpressionEquals(List<ViewExpression> result, ViewExpression expected)
+    {
+        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");
+        assertEquals(actual.getExpression(), expected.getExpression(), "Expression");
+    }
+
     @Test
     public void testTableFilter()
     {

lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java

@@ -687,6 +687,50 @@ public void testTableRulesForCheckCanRenameColumn()
         assertAccessDenied(() -> accessControl.checkCanRenameColumn(BOB, new CatalogSchemaTableName("some-catalog", "bobschema", "bobtable")), RENAME_COLUMNS_ACCESS_DENIED_MESSAGE);
     }
 
+    @Test
+    public void testTableRulesForMixedGroupUsers()
+    {
+        SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-table-mixed-groups.json");
+
+        SystemSecurityContext userGroup1Group2 = new SystemSecurityContext(Identity.forUser("user_1_2")
+                .withGroups(ImmutableSet.of("group1", "group2")).build(), Optional.empty());
+        SystemSecurityContext userGroup2 = new SystemSecurityContext(Identity.forUser("user_2")
+                .withGroups(ImmutableSet.of("group2")).build(), Optional.empty());
+
+        assertEquals(
+                accessControl.getColumnMasks(
+                        userGroup1Group2,
+                        new CatalogSchemaTableName("some-catalog", "my_schema", "my_table"),
+                        "col_a",
+                        VARCHAR),
+                ImmutableList.of());
+
+        assertViewExpressionEquals(
+                accessControl.getColumnMasks(
+                        userGroup2,
+                        new CatalogSchemaTableName("some-catalog", "my_schema", "my_table"),
+                        "col_a",
+                        VARCHAR),
+                new ViewExpression(userGroup2.getIdentity().getUser(), Optional.of("some-catalog"), Optional.of("my_schema"), "'mask_a'"));
+
+        SystemSecurityContext userGroup1Group3 = new SystemSecurityContext(Identity.forUser("user_1_3")
+                .withGroups(ImmutableSet.of("group1", "group3")).build(), Optional.empty());
+        SystemSecurityContext userGroup3 = new SystemSecurityContext(Identity.forUser("user_3")
+                .withGroups(ImmutableSet.of("group3")).build(), Optional.empty());
+
+        assertEquals(
+                accessControl.getRowFilters(
+                        userGroup1Group3,
+                        new CatalogSchemaTableName("some-catalog", "my_schema", "my_table")),
+                ImmutableList.of());
+
+        assertViewExpressionEquals(
+                accessControl.getRowFilters(
+                        userGroup3,
+                        new CatalogSchemaTableName("some-catalog", "my_schema", "my_table")),
+                new ViewExpression(userGroup3.getIdentity().getUser(), Optional.of("some-catalog"), Optional.of("my_schema"), "country='US'"));
+    }
+
     @Test
     public void testCheckCanSetTableAuthorizationForAdmin()
     {

lib/trino-plugin-toolkit/src/test/resources/file-based-system-access-table-mixed-groups.json

@@ -0,0 +1,46 @@
+{
+  "tables": [
+    {
+      "catalog": "some-catalog",
+      "schema": "my_schema",
+      "table": "my_table",
+      "privileges": ["SELECT", "GRANT_SELECT", "OWNERSHIP", "INSERT", "DELETE", "UPDATE"],
+      "group": "group1"
+    },
+    {
+      "catalog": "some-catalog",
+      "schema": "my_schema",
+      "table": "my_table",
+      "columns" : [
+         {
+           "name": "col_a",
+           "mask": "'mask_a'"
+         },
+         {
+           "name": "col_b",
+           "mask": "'mask_b'"
+         }
+      ],
+      "privileges": ["SELECT", "GRANT_SELECT"],
+      "group": "group2"
+    },
+    {
+      "catalog": "some-catalog",
+      "schema": "my_schema",
+      "table": "my_table",
+      "columns" : [
+        {
+          "name": "col_a",
+          "mask": "'mask_a'"
+        },
+        {
+          "name": "col_b",
+          "mask": "'mask_b'"
+        }
+      ],
+      "privileges": ["SELECT", "GRANT_SELECT"],
+      "filter": "country='US'",
+      "group": "group3"
+    }
+  ]
+}

lib/trino-plugin-toolkit/src/test/resources/table-mixed-groups.json

@@ -0,0 +1,43 @@
+{
+  "tables": [
+    {
+      "schema": "my_schema",
+      "table": "my_table",
+      "privileges": ["SELECT", "GRANT_SELECT", "OWNERSHIP", "INSERT", "DELETE", "UPDATE"],
+      "group": "group1"
+    },
+    {
+      "schema": "my_schema",
+      "table": "my_table",
+      "columns" : [
+        {
+          "name": "col_a",
+          "mask": "'mask_a'"
+        },
+        {
+          "name": "col_b",
+          "mask": "'mask_b'"
+        }
+      ],
+      "privileges": ["SELECT", "GRANT_SELECT"],
+      "group": "group2"
+    },
+    {
+      "schema": "my_schema",
+      "table": "my_table",
+      "columns" : [
+        {
+          "name": "col_a",
+          "mask": "'mask_a'"
+        },
+        {
+          "name": "col_b",
+          "mask": "'mask_b'"
+        }
+      ],
+      "privileges": ["SELECT", "GRANT_SELECT"],
+      "filter": "country='US'",
+      "group": "group3"
+    }
+  ]
+}