From e5413999ec9e36eca693e918ed17bb4a2a85ac51 Mon Sep 17 00:00:00 2001 From: Sumit Halder Date: Sun, 17 May 2020 00:07:02 +0530 Subject: [PATCH] Add schema access rules to FileBasedSystemAccessControl Add schema access rules to FileBasedSystemAccessControl Add schema access rules to FileBasedSystemAccessControl Add schema access rules to FileBasedSystemAccessControl Add schema access rules to FileBasedSystemAccessControl Add schema access rules to FileBasedSystemAccessControl --- ...curity-config-file-with-unknown-rules.json | 18 ++-- .../FileBasedSystemAccessControl.java | 40 ++++++- .../FileBasedSystemAccessControlRules.java | 10 +- .../TestFileBasedSystemAccessControl.java | 102 +++++++++++++++++- .../file-based-system-access-schema.json | 33 ++++++ ...curity-config-file-with-unknown-rules.json | 18 ++-- 6 files changed, 201 insertions(+), 20 deletions(-) create mode 100644 presto-plugin-toolkit/src/test/resources/file-based-system-access-schema.json diff --git a/presto-main/src/test/resources/security-config-file-with-unknown-rules.json b/presto-main/src/test/resources/security-config-file-with-unknown-rules.json index 02b0a2b7eca32..e0c9043a9e6df 100644 --- a/presto-main/src/test/resources/security-config-file-with-unknown-rules.json +++ b/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 + } + ] } diff --git a/presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/FileBasedSystemAccessControl.java b/presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/FileBasedSystemAccessControl.java index bf7caa5223faa..331fa8275a194 100644 --- a/presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/FileBasedSystemAccessControl.java +++ b/presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/FileBasedSystemAccessControl.java @@ -91,17 +91,20 @@ public class FileBasedSystemAccessControl private final Optional> queryAccessRules; private final Optional> impersonationRules; private final Optional> principalUserMatchRules; + private final Optional> schemaRules; private FileBasedSystemAccessControl( List catalogRules, Optional> queryAccessRules, Optional> impersonationRules, - Optional> principalUserMatchRules) + Optional> principalUserMatchRules, + Optional> 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 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 owner = rule.match(identity.getUser(), identity.getGroups(), schemaName); + if (owner.isPresent()) { + return owner.get(); + } + } + return false; + } } diff --git a/presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/FileBasedSystemAccessControlRules.java b/presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/FileBasedSystemAccessControlRules.java index 2e2e9124a2827..580d07ef1a0fd 100644 --- a/presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/FileBasedSystemAccessControlRules.java +++ b/presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/FileBasedSystemAccessControlRules.java @@ -26,18 +26,21 @@ public class FileBasedSystemAccessControlRules private final Optional> queryAccessRules; private final Optional> impersonationRules; private final Optional> principalUserMatchRules; + private final Optional> schemaRules; @JsonCreator public FileBasedSystemAccessControlRules( @JsonProperty("catalogs") Optional> catalogRules, @JsonProperty("queries") Optional> queryAccessRules, @JsonProperty("impersonation") Optional> impersonationRules, - @JsonProperty("principals") Optional> principalUserMatchRules) + @JsonProperty("principals") Optional> principalUserMatchRules, + @JsonProperty("schemas") Optional> 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 getCatalogRules() @@ -59,4 +62,9 @@ public Optional> getPrincipalUserMatchRules() { return principalUserMatchRules; } + + public Optional> getSchemaRules() + { + return schemaRules; + } } diff --git a/presto-plugin-toolkit/src/test/java/io/prestosql/plugin/base/security/TestFileBasedSystemAccessControl.java b/presto-plugin-toolkit/src/test/java/io/prestosql/plugin/base/security/TestFileBasedSystemAccessControl.java index 7379f3a28a95f..150849fc55e0a 100644 --- a/presto-plugin-toolkit/src/test/java/io/prestosql/plugin/base/security/TestFileBasedSystemAccessControl.java +++ b/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/example.com@EXAMPLE.COM")).build(); private static final Identity kerberosValidNonAsciiUser = Identity.forUser("\u0194\u0194\u0194").withPrincipal(new KerberosPrincipal("\u0194\u0194\u0194/example.com@EXAMPLE.COM")).build(); private static final Identity kerberosInvalidAlice = Identity.forUser("alice").withPrincipal(new KerberosPrincipal("mallory/example.com@EXAMPLE.COM")).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/\\E@EXAMPLE.COM")).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 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 = 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); + } } diff --git a/presto-plugin-toolkit/src/test/resources/file-based-system-access-schema.json b/presto-plugin-toolkit/src/test/resources/file-based-system-access-schema.json new file mode 100644 index 0000000000000..c9f3318cc00ed --- /dev/null +++ b/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 + } + ] +} diff --git a/presto-plugin-toolkit/src/test/resources/security-config-file-with-unknown-rules.json b/presto-plugin-toolkit/src/test/resources/security-config-file-with-unknown-rules.json index 02b0a2b7eca32..e0c9043a9e6df 100644 --- a/presto-plugin-toolkit/src/test/resources/security-config-file-with-unknown-rules.json +++ b/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 + } + ] }