Adding schema access rules to FileBasedSystemAccessControl

presto-main/src/test/resources/security-config-file-with-unknown-rules.json

@@ -1,9 +1,13 @@
 {
-  "schemas": [
-    {
-      "user": "(alice|bob)",
-      "schema": "(default|pv)",
-      "owner": true
-    }
-  ]
+    "sessionProperties": [
+        {
+            "property": "force_local_scheduling",
+            "allow": true
+        },
+        {
+            "user": "admin",
+            "property": "max_split_size",
+            "allow": true
+        }
+    ]
 }

presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/FileBasedSystemAccessControl.java

@@ -91,17 +91,20 @@ public class FileBasedSystemAccessControl
     private final Optional<List<QueryAccessRule>> queryAccessRules;
     private final Optional<List<ImpersonationRule>> impersonationRules;
     private final Optional<List<PrincipalUserMatchRule>> principalUserMatchRules;
+    private final Optional<List<SchemaAccessControlRule>> schemaRules;
 
     private FileBasedSystemAccessControl(
             List<CatalogAccessControlRule> catalogRules,
             Optional<List<QueryAccessRule>> queryAccessRules,
             Optional<List<ImpersonationRule>> impersonationRules,
-            Optional<List<PrincipalUserMatchRule>> principalUserMatchRules)
+            Optional<List<PrincipalUserMatchRule>> principalUserMatchRules,
+            Optional<List<SchemaAccessControlRule>> schemaRules)
     {
         this.catalogRules = catalogRules;
         this.queryAccessRules = queryAccessRules;
         this.impersonationRules = impersonationRules;
         this.principalUserMatchRules = principalUserMatchRules;
+        this.schemaRules = schemaRules;
     }
 
     public static class Factory
@@ -169,7 +172,8 @@ private SystemAccessControl create(String configFileName)
                     catalogRulesBuilder.build(),
                     rules.getQueryAccessRules(),
                     rules.getImpersonationRules(),
-                    rules.getPrincipalUserMatchRules());
+                    rules.getPrincipalUserMatchRules(),
+                    rules.getSchemaRules());
         }
     }
 
@@ -335,6 +339,10 @@ public void checkCanDropSchema(SystemSecurityContext context, CatalogSchemaName
         if (!canAccessCatalog(context.getIdentity(), schema.getCatalogName(), ALL)) {
             denyDropSchema(schema.toString());
         }
+
+        if (!isSchemaOwner(context, schema.getSchemaName())) {
+            denyDropSchema(schema.getSchemaName());
+        }
     }
 
     @Override
@@ -343,6 +351,10 @@ public void checkCanRenameSchema(SystemSecurityContext context, CatalogSchemaNam
         if (!canAccessCatalog(context.getIdentity(), schema.getCatalogName(), ALL)) {
             denyRenameSchema(schema.toString(), newSchemaName);
         }
+
+        if (!isSchemaOwner(context, schema.getSchemaName()) || !isSchemaOwner(context, newSchemaName)) {
+            denyRenameSchema(schema.getSchemaName(), newSchemaName);
+        }
     }
 
     @Override
@@ -351,6 +363,10 @@ public void checkCanSetSchemaAuthorization(SystemSecurityContext context, Catalo
         if (!canAccessCatalog(context.getIdentity(), schema.getCatalogName(), ALL)) {
             denySetSchemaAuthorization(schema.toString(), principal);
         }
+
+        if (!isSchemaOwner(context, schema.getSchemaName())) {
+            denySetSchemaAuthorization(schema.getSchemaName(), principal);
+        }
     }
 
     @Override
@@ -382,6 +398,10 @@ public void checkCanShowCreateSchema(SystemSecurityContext context, CatalogSchem
         if (!canAccessCatalog(context.getIdentity(), schemaName.getCatalogName(), ALL)) {
             denyShowCreateSchema(schemaName.toString());
         }
+
+        if (!isSchemaOwner(context, schemaName.getSchemaName())) {
+            denyShowCreateSchema(schemaName.getSchemaName());
+        }
     }
 
     @Override
@@ -581,4 +601,20 @@ public Optional<ViewExpression> getColumnMask(SystemSecurityContext context, Cat
     {
         return Optional.empty();
     }
+
+    private boolean isSchemaOwner(SystemSecurityContext context, String schemaName)
+    {
+        if (schemaRules.isEmpty()) {
+            return true;
+        }
+
+        Identity identity = context.getIdentity();
+        for (SchemaAccessControlRule rule : schemaRules.get()) {
+            Optional<Boolean> owner = rule.match(identity.getUser(), identity.getGroups(), schemaName);
+            if (owner.isPresent()) {
+                return owner.get();
+            }
+        }
+        return false;
+    }
 }

presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/FileBasedSystemAccessControlRules.java

@@ -26,18 +26,21 @@ public class FileBasedSystemAccessControlRules
     private final Optional<List<QueryAccessRule>> queryAccessRules;
     private final Optional<List<ImpersonationRule>> impersonationRules;
     private final Optional<List<PrincipalUserMatchRule>> principalUserMatchRules;
+    private final Optional<List<SchemaAccessControlRule>> schemaRules;
 
     @JsonCreator
     public FileBasedSystemAccessControlRules(
             @JsonProperty("catalogs") Optional<List<CatalogAccessControlRule>> catalogRules,
             @JsonProperty("queries") Optional<List<QueryAccessRule>> queryAccessRules,
             @JsonProperty("impersonation") Optional<List<ImpersonationRule>> impersonationRules,
-            @JsonProperty("principals") Optional<List<PrincipalUserMatchRule>> principalUserMatchRules)
+            @JsonProperty("principals") Optional<List<PrincipalUserMatchRule>> principalUserMatchRules,
+            @JsonProperty("schemas") Optional<List<SchemaAccessControlRule>> schemaAccessControlRules)
     {
         this.catalogRules = catalogRules.map(ImmutableList::copyOf).orElse(ImmutableList.of());
         this.queryAccessRules = queryAccessRules.map(ImmutableList::copyOf);
         this.principalUserMatchRules = principalUserMatchRules.map(ImmutableList::copyOf);
         this.impersonationRules = impersonationRules.map(ImmutableList::copyOf);
+        this.schemaRules = schemaAccessControlRules.map(ImmutableList::copyOf);
     }
 
     public List<CatalogAccessControlRule> getCatalogRules()
@@ -59,4 +62,9 @@ public Optional<List<PrincipalUserMatchRule>> getPrincipalUserMatchRules()
     {
         return principalUserMatchRules;
     }
+
+    public Optional<List<SchemaAccessControlRule>> getSchemaRules()
+    {
+        return schemaRules;
+    }
 }

presto-plugin-toolkit/src/test/java/io/prestosql/plugin/base/security/TestFileBasedSystemAccessControl.java

@@ -24,6 +24,7 @@
 import io.prestosql.spi.security.PrincipalType;
 import io.prestosql.spi.security.SystemAccessControl;
 import io.prestosql.spi.security.SystemSecurityContext;
+import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
 import org.testng.annotations.Test;
 
 import javax.security.auth.kerberos.KerberosPrincipal;
@@ -44,7 +45,7 @@
 
 public class TestFileBasedSystemAccessControl
 {
-    private static final Identity alice = Identity.ofUser("alice");
+    private static final Identity alice = Identity.forUser("alice").withGroups(ImmutableSet.of("staff")).build();
     private static final Identity kerberosValidAlice = Identity.forUser("alice").withPrincipal(new KerberosPrincipal("alice/[email protected]")).build();
     private static final Identity kerberosValidNonAsciiUser = Identity.forUser("\u0194\u0194\u0194").withPrincipal(new KerberosPrincipal("\u0194\u0194\u0194/[email protected]")).build();
     private static final Identity kerberosInvalidAlice = Identity.forUser("alice").withPrincipal(new KerberosPrincipal("mallory/[email protected]")).build();
@@ -53,13 +54,101 @@ public class TestFileBasedSystemAccessControl
     private static final Identity validSpecialRegexWildDot = Identity.forUser(".*").withPrincipal(new KerberosPrincipal("special/.*@EXAMPLE.COM")).build();
     private static final Identity validSpecialRegexEndQuote = Identity.forUser("\\E").withPrincipal(new KerberosPrincipal("special/\\[email protected]")).build();
     private static final Identity invalidSpecialRegex = Identity.forUser("alice").withPrincipal(new KerberosPrincipal("special/.*@EXAMPLE.COM")).build();
-    private static final Identity bob = Identity.ofUser("bob");
-    private static final Identity admin = Identity.ofUser("admin");
+    private static final Identity bob = Identity.forUser("bob").withGroups(ImmutableSet.of("staff")).build();
+    private static final Identity admin = Identity.forUser("admin").withGroups(ImmutableSet.of("admin", "staff")).build();
     private static final Identity nonAsciiUser = Identity.ofUser("\u0194\u0194\u0194");
     private static final Set<String> allCatalogs = ImmutableSet.of("secret", "open-to-all", "all-allowed", "alice-catalog", "allowed-absent", "\u0200\u0200\u0200");
     private static final CatalogSchemaTableName aliceView = new CatalogSchemaTableName("alice-catalog", "schema", "view");
     private static final Optional<QueryId> queryId = Optional.empty();
 
+    private static final Identity charlie = Identity.forUser("charlie").withGroups(ImmutableSet.of("guests")).build();
+    private static final Identity joe = Identity.ofUser("joe");
+    private static final SystemSecurityContext ADMIN = new SystemSecurityContext(admin, queryId);
+    private static final SystemSecurityContext BOB = new SystemSecurityContext(bob, queryId);
+    private static final SystemSecurityContext CHARLIE = new SystemSecurityContext(charlie, queryId);
+
+    private static final String DROP_SCHEMA_ACCESS_DENIED_MESSAGE = "Access Denied: Cannot drop schema .*";
+    private static final String RENAME_SCHEMA_ACCESS_DENIED_MESSAGE = "Access Denied: Cannot rename schema from .* to .*";
+    private static final String AUTH_SCHEMA_ACCESS_DENIED_MESSAGE = "Access Denied: Cannot set authorization for schema .* to .*";
+    private static final String SHOW_CREATE_SCHEMA_ACCESS_DENIED_MESSAGE = "Access Denied: Cannot show create schema for .*";
+
+    @Test
+    public void testSchemaRulesForCheckCanDropSchema()
+    {
+        SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-schema.json");
+
+        accessControl.checkCanDropSchema(ADMIN, new CatalogSchemaName("some-catalog", "bob"));
+        accessControl.checkCanDropSchema(ADMIN, new CatalogSchemaName("some-catalog", "staff"));
+        accessControl.checkCanDropSchema(ADMIN, new CatalogSchemaName("some-catalog", "authenticated"));
+        accessControl.checkCanDropSchema(ADMIN, new CatalogSchemaName("some-catalog", "test"));
+
+        accessControl.checkCanDropSchema(BOB, new CatalogSchemaName("some-catalog", "bob"));
+        accessControl.checkCanDropSchema(BOB, new CatalogSchemaName("some-catalog", "staff"));
+        accessControl.checkCanDropSchema(BOB, new CatalogSchemaName("some-catalog", "authenticated"));
+        assertAccessDenied(() -> accessControl.checkCanDropSchema(BOB, new CatalogSchemaName("some-catalog", "test")), DROP_SCHEMA_ACCESS_DENIED_MESSAGE);
+
+        accessControl.checkCanDropSchema(CHARLIE, new CatalogSchemaName("some-catalog", "authenticated"));
+        assertAccessDenied(() -> accessControl.checkCanDropSchema(CHARLIE, new CatalogSchemaName("some-catalog", "bob")), DROP_SCHEMA_ACCESS_DENIED_MESSAGE);
+        assertAccessDenied(() -> accessControl.checkCanDropSchema(CHARLIE, new CatalogSchemaName("some-catalog", "staff")), DROP_SCHEMA_ACCESS_DENIED_MESSAGE);
+        assertAccessDenied(() -> accessControl.checkCanDropSchema(CHARLIE, new CatalogSchemaName("some-catalog", "test")), DROP_SCHEMA_ACCESS_DENIED_MESSAGE);
+    }
+
+    @Test
+    public void testSchemaRulesForCheckCanRenameSchema()
+    {
+        SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-schema.json");
+
+        accessControl.checkCanRenameSchema(ADMIN, new CatalogSchemaName("some-catalog", "bob"), "new_schema");
+        accessControl.checkCanRenameSchema(ADMIN, new CatalogSchemaName("some-catalog", "staff"), "new_schema");
+        accessControl.checkCanRenameSchema(ADMIN, new CatalogSchemaName("some-catalog", "authenticated"), "new_schema");
+        accessControl.checkCanRenameSchema(ADMIN, new CatalogSchemaName("some-catalog", "test"), "new_schema");
+
+        accessControl.checkCanRenameSchema(BOB, new CatalogSchemaName("some-catalog", "bob"), "staff");
+        accessControl.checkCanRenameSchema(BOB, new CatalogSchemaName("some-catalog", "staff"), "authenticated");
+        accessControl.checkCanRenameSchema(BOB, new CatalogSchemaName("some-catalog", "authenticated"), "bob");
+        assertAccessDenied(() -> accessControl.checkCanRenameSchema(BOB, new CatalogSchemaName("some-catalog", "test"), "bob"), RENAME_SCHEMA_ACCESS_DENIED_MESSAGE);
+        assertAccessDenied(() -> accessControl.checkCanRenameSchema(BOB, new CatalogSchemaName("some-catalog", "bob"), "test"), RENAME_SCHEMA_ACCESS_DENIED_MESSAGE);
+
+        assertAccessDenied(() -> accessControl.checkCanRenameSchema(CHARLIE, new CatalogSchemaName("some-catalog", "bob"), "new_schema"), RENAME_SCHEMA_ACCESS_DENIED_MESSAGE);
+        assertAccessDenied(() -> accessControl.checkCanRenameSchema(CHARLIE, new CatalogSchemaName("some-catalog", "staff"), "new_schema"), RENAME_SCHEMA_ACCESS_DENIED_MESSAGE);
+        accessControl.checkCanRenameSchema(CHARLIE, new CatalogSchemaName("some-catalog", "authenticated"), "authenticated");
+        assertAccessDenied(() -> accessControl.checkCanRenameSchema(CHARLIE, new CatalogSchemaName("some-catalog", "test"), "new_schema"), RENAME_SCHEMA_ACCESS_DENIED_MESSAGE);
+    }
+
+    @Test
+    public void testSchemaRulesForCheckCanSetSchemaAuthorization()
+    {
+        SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-schema.json");
+
+        accessControl.checkCanSetSchemaAuthorization(ADMIN, new CatalogSchemaName("some-catalog", "test"), new PrestoPrincipal(PrincipalType.ROLE, "some_role"));
+        accessControl.checkCanSetSchemaAuthorization(ADMIN, new CatalogSchemaName("some-catalog", "test"), new PrestoPrincipal(PrincipalType.USER, "some_user"));
+        accessControl.checkCanSetSchemaAuthorization(BOB, new CatalogSchemaName("some-catalog", "bob"), new PrestoPrincipal(PrincipalType.ROLE, "some_role"));
+        accessControl.checkCanSetSchemaAuthorization(BOB, new CatalogSchemaName("some-catalog", "bob"), new PrestoPrincipal(PrincipalType.USER, "some_user"));
+        assertAccessDenied(() -> accessControl.checkCanSetSchemaAuthorization(BOB, new CatalogSchemaName("some-catalog", "test"), new PrestoPrincipal(PrincipalType.ROLE, "some_role")), AUTH_SCHEMA_ACCESS_DENIED_MESSAGE);
+        assertAccessDenied(() -> accessControl.checkCanSetSchemaAuthorization(BOB, new CatalogSchemaName("some-catalog", "test"), new PrestoPrincipal(PrincipalType.USER, "some_user")), AUTH_SCHEMA_ACCESS_DENIED_MESSAGE);
+    }
+
+    @Test
+    public void testSchemaRulesForCheckCanShowCreateSchema()
+    {
+        SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-schema.json");
+
+        accessControl.checkCanShowCreateSchema(ADMIN, new CatalogSchemaName("some-catalog", "bob"));
+        accessControl.checkCanShowCreateSchema(ADMIN, new CatalogSchemaName("some-catalog", "staff"));
+        accessControl.checkCanShowCreateSchema(ADMIN, new CatalogSchemaName("some-catalog", "authenticated"));
+        accessControl.checkCanShowCreateSchema(ADMIN, new CatalogSchemaName("some-catalog", "test"));
+
+        accessControl.checkCanShowCreateSchema(BOB, new CatalogSchemaName("some-catalog", "bob"));
+        accessControl.checkCanShowCreateSchema(BOB, new CatalogSchemaName("some-catalog", "staff"));
+        accessControl.checkCanShowCreateSchema(BOB, new CatalogSchemaName("some-catalog", "authenticated"));
+        assertAccessDenied(() -> accessControl.checkCanShowCreateSchema(BOB, new CatalogSchemaName("some-catalog", "test")), SHOW_CREATE_SCHEMA_ACCESS_DENIED_MESSAGE);
+
+        accessControl.checkCanShowCreateSchema(CHARLIE, new CatalogSchemaName("some-catalog", "authenticated"));
+        assertAccessDenied(() -> accessControl.checkCanShowCreateSchema(CHARLIE, new CatalogSchemaName("some-catalog", "bob")), SHOW_CREATE_SCHEMA_ACCESS_DENIED_MESSAGE);
+        assertAccessDenied(() -> accessControl.checkCanShowCreateSchema(CHARLIE, new CatalogSchemaName("some-catalog", "staff")), SHOW_CREATE_SCHEMA_ACCESS_DENIED_MESSAGE);
+        assertAccessDenied(() -> accessControl.checkCanShowCreateSchema(CHARLIE, new CatalogSchemaName("some-catalog", "test")), SHOW_CREATE_SCHEMA_ACCESS_DENIED_MESSAGE);
+    }
+
     @Test
     public void testCanSetUserOperations()
     {
@@ -268,4 +357,11 @@ private String getResourcePath(String resourceName)
     {
         return this.getClass().getClassLoader().getResource(resourceName).getPath();
     }
+
+    private static void assertAccessDenied(ThrowingCallable callable, String expectedMessage)
+    {
+        assertThatThrownBy(callable)
+                .isInstanceOf(AccessDeniedException.class)
+                .hasMessageMatching(expectedMessage);
+    }
 }

presto-plugin-toolkit/src/test/resources/file-based-system-access-schema.json

@@ -0,0 +1,33 @@
+{
+    "catalogs": [
+        {
+            "allow": true
+        }
+    ],
+    "schemas": [
+        {
+            "schema": "secret",
+            "owner": false
+        },
+        {
+            "user": "admin",
+            "schema": ".*",
+            "owner": true
+        },
+        {
+            "group": "staff",
+            "schema": "staff",
+            "owner": true
+        },
+        {
+            "group": "staff|guests",
+            "schema": "authenticated",
+            "owner": true
+        },
+        {
+            "user": "bob",
+            "schema": "bob",
+            "owner": true
+        }
+    ]
+}

presto-plugin-toolkit/src/test/resources/security-config-file-with-unknown-rules.json

@@ -1,9 +1,13 @@
 {
-  "schemas": [
-    {
-      "user": "(alice|bob)",
-      "schema": "(default|pv)",
-      "owner": true
-    }
-  ]
+    "sessionProperties": [
+        {
+            "property": "force_local_scheduling",
+            "allow": true
+        },
+        {
+            "user": "admin",
+            "property": "max_split_size",
+            "allow": true
+        }
+    ]
 }