From f5ee87cb87b330ff9a9a5b90a88ccc400a65fe33 Mon Sep 17 00:00:00 2001 From: zhangjingfang Date: Thu, 10 Sep 2020 10:45:40 +0800 Subject: [PATCH] Dynamic catalog support file access control. --- .../io/prestosql/security/AccessControl.java | 2 +- .../security/AccessControlManager.java | 8 +++++ .../security/AllowAllSystemAccessControl.java | 5 +++ .../security/CatalogAccessControlRule.java | 4 +-- .../FileBasedSystemAccessControl.java | 34 +++++++++++++++---- .../TestFileBasedSystemAccessControl.java | 21 ++++++++++++ .../ForwardingSystemAccessControl.java | 6 ++++ .../spi/security/AccessDeniedException.java | 7 +++- .../spi/security/SystemAccessControl.java | 7 ++++ 9 files changed, 84 insertions(+), 10 deletions(-) diff --git a/presto-main/src/main/java/io/prestosql/security/AccessControl.java b/presto-main/src/main/java/io/prestosql/security/AccessControl.java index c413bdeb5..0f13282b7 100644 --- a/presto-main/src/main/java/io/prestosql/security/AccessControl.java +++ b/presto-main/src/main/java/io/prestosql/security/AccessControl.java @@ -42,7 +42,7 @@ public interface AccessControl */ Set filterCatalogs(Identity identity, Set catalogs); - /* + /** * Check whether identity is allowed to access catalogs */ default void checkCanAccessCatalogs(Identity identity) {} diff --git a/presto-main/src/main/java/io/prestosql/security/AccessControlManager.java b/presto-main/src/main/java/io/prestosql/security/AccessControlManager.java index 7e591e460..742791c77 100644 --- a/presto-main/src/main/java/io/prestosql/security/AccessControlManager.java +++ b/presto-main/src/main/java/io/prestosql/security/AccessControlManager.java @@ -163,6 +163,14 @@ public class AccessControlManager return systemAccessControl.get().filterCatalogs(identity, catalogs); } + @Override + public void checkCanAccessCatalogs(Identity identity) + { + requireNonNull(identity, "identity is null"); + + authenticationCheck(() -> systemAccessControl.get().checkCanShowCatalogs(identity)); + } + @Override public void checkCanAccessCatalog(Identity identity, String catalogName) { diff --git a/presto-main/src/main/java/io/prestosql/security/AllowAllSystemAccessControl.java b/presto-main/src/main/java/io/prestosql/security/AllowAllSystemAccessControl.java index ff735d8e2..426e82755 100644 --- a/presto-main/src/main/java/io/prestosql/security/AllowAllSystemAccessControl.java +++ b/presto-main/src/main/java/io/prestosql/security/AllowAllSystemAccessControl.java @@ -193,6 +193,11 @@ public class AllowAllSystemAccessControl { } + @Override + public void checkCanShowCatalogs(Identity identity) + { + } + @Override public void checkCanCreateCatalog(Identity identity, String catalogName) { diff --git a/presto-main/src/main/java/io/prestosql/security/CatalogAccessControlRule.java b/presto-main/src/main/java/io/prestosql/security/CatalogAccessControlRule.java index b8ff4c1b2..56390e0f5 100644 --- a/presto-main/src/main/java/io/prestosql/security/CatalogAccessControlRule.java +++ b/presto-main/src/main/java/io/prestosql/security/CatalogAccessControlRule.java @@ -38,10 +38,10 @@ public class CatalogAccessControlRule this.catalogRegex = requireNonNull(catalogRegex, "catalogRegex is null"); } - public Optional match(String user, String catalog) + public Optional match(String user, Optional catalog) { if (userRegex.map(regex -> regex.matcher(user).matches()).orElse(true) && - catalogRegex.map(regex -> regex.matcher(catalog).matches()).orElse(true)) { + catalogRegex.map(regex -> regex.matcher(catalog.orElse("")).matches()).orElse(true)) { return Optional.of(allow); } return Optional.empty(); diff --git a/presto-main/src/main/java/io/prestosql/security/FileBasedSystemAccessControl.java b/presto-main/src/main/java/io/prestosql/security/FileBasedSystemAccessControl.java index f1454f39a..ff266d206 100644 --- a/presto-main/src/main/java/io/prestosql/security/FileBasedSystemAccessControl.java +++ b/presto-main/src/main/java/io/prestosql/security/FileBasedSystemAccessControl.java @@ -63,19 +63,36 @@ public class FileBasedSystemAccessControl this.principalUserMatchRules = principalUserMatchRules; } + @Override + public void checkCanShowCatalogs(Identity identity) + { + if (!canAccessCatalog(identity)) { + denyCatalogAccess(); + } + } + @Override public void checkCanCreateCatalog(Identity identity, String catalogName) { + if (!canAccessCatalog(identity, Optional.of(catalogName))) { + denyCatalogAccess(catalogName); + } } @Override public void checkCanDropCatalog(Identity identity, String catalogName) { + if (!canAccessCatalog(identity, Optional.of(catalogName))) { + denyCatalogAccess(catalogName); + } } @Override public void checkCanUpdateCatalog(Identity identity, String catalogName) { + if (!canAccessCatalog(identity, Optional.of(catalogName))) { + denyCatalogAccess(catalogName); + } } @Override @@ -120,7 +137,7 @@ public class FileBasedSystemAccessControl @Override public void checkCanAccessCatalog(Identity identity, String catalogName) { - if (!canAccessCatalog(identity, catalogName)) { + if (!canAccessCatalog(identity, Optional.of(catalogName))) { denyCatalogAccess(catalogName); } } @@ -130,14 +147,19 @@ public class FileBasedSystemAccessControl { ImmutableSet.Builder filteredCatalogs = ImmutableSet.builder(); for (String catalog : catalogs) { - if (canAccessCatalog(identity, catalog)) { + if (canAccessCatalog(identity, Optional.of(catalog))) { filteredCatalogs.add(catalog); } } return filteredCatalogs.build(); } - private boolean canAccessCatalog(Identity identity, String catalogName) + private boolean canAccessCatalog(Identity identity) + { + return canAccessCatalog(identity, Optional.empty()); + } + + private boolean canAccessCatalog(Identity identity, Optional catalogName) { for (CatalogAccessControlRule rule : catalogRules) { Optional allowed = rule.match(identity.getUser(), catalogName); @@ -171,7 +193,7 @@ public class FileBasedSystemAccessControl @Override public Set filterSchemas(Identity identity, String catalogName, Set schemaNames) { - if (!canAccessCatalog(identity, catalogName)) { + if (!canAccessCatalog(identity, Optional.of(catalogName))) { return ImmutableSet.of(); } @@ -206,7 +228,7 @@ public class FileBasedSystemAccessControl @Override public Set filterTables(Identity identity, String catalogName, Set tableNames) { - if (!canAccessCatalog(identity, catalogName)) { + if (!canAccessCatalog(identity, Optional.of(catalogName))) { return ImmutableSet.of(); } @@ -221,7 +243,7 @@ public class FileBasedSystemAccessControl @Override public List filterColumns(Identity identity, CatalogSchemaTableName tableName, List columns) { - if (!canAccessCatalog(identity, tableName.getCatalogName())) { + if (!canAccessCatalog(identity, Optional.of(tableName.getCatalogName()))) { return ImmutableList.of(); } diff --git a/presto-main/src/test/java/io/prestosql/security/TestFileBasedSystemAccessControl.java b/presto-main/src/test/java/io/prestosql/security/TestFileBasedSystemAccessControl.java index 0b9263b0c..e82ffddab 100644 --- a/presto-main/src/test/java/io/prestosql/security/TestFileBasedSystemAccessControl.java +++ b/presto-main/src/test/java/io/prestosql/security/TestFileBasedSystemAccessControl.java @@ -123,7 +123,28 @@ public class TestFileBasedSystemAccessControl assertEquals(accessControlManager.filterCatalogs(bob, allCatalogs), bobCatalogs); Set nonAsciiUserCatalogs = ImmutableSet.of("open-to-all", "all-allowed", "\u0200\u0200\u0200"); assertEquals(accessControlManager.filterCatalogs(nonAsciiUser, allCatalogs), nonAsciiUserCatalogs); + + accessControlManager.checkCanCreateCatalog(alice, "alice-catalog"); + accessControlManager.checkCanDropCatalog(alice, "alice-catalog"); + accessControlManager.checkCanUpdateCatalog(alice, "alice-catalog"); + accessControlManager.checkCanAccessCatalog(alice, "alice-catalog"); + accessControlManager.checkCanAccessCatalogs(admin); }); + assertThrows(AccessDeniedException.class, () -> transaction(transactionManager, accessControlManager).execute(transactionId -> { + accessControlManager.checkCanCreateCatalog(bob, "alice-catalog"); + })); + assertThrows(AccessDeniedException.class, () -> transaction(transactionManager, accessControlManager).execute(transactionId -> { + accessControlManager.checkCanDropCatalog(bob, "alice-catalog"); + })); + assertThrows(AccessDeniedException.class, () -> transaction(transactionManager, accessControlManager).execute(transactionId -> { + accessControlManager.checkCanUpdateCatalog(bob, "alice-catalog"); + })); + assertThrows(AccessDeniedException.class, () -> transaction(transactionManager, accessControlManager).execute(transactionId -> { + accessControlManager.checkCanAccessCatalog(bob, "alice-catalog"); + })); + assertThrows(AccessDeniedException.class, () -> transaction(transactionManager, accessControlManager).execute(transactionId -> { + accessControlManager.checkCanAccessCatalogs(bob); + })); } @Test diff --git a/presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/ForwardingSystemAccessControl.java b/presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/ForwardingSystemAccessControl.java index 352263f6e..cfc219ddd 100644 --- a/presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/ForwardingSystemAccessControl.java +++ b/presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/security/ForwardingSystemAccessControl.java @@ -72,6 +72,12 @@ public abstract class ForwardingSystemAccessControl return delegate().filterCatalogs(identity, catalogs); } + @Override + public void checkCanShowCatalogs(Identity identity) + { + delegate().checkCanShowCatalogs(identity); + } + @Override public void checkCanCreateCatalog(Identity identity, String catalogName) { diff --git a/presto-spi/src/main/java/io/prestosql/spi/security/AccessDeniedException.java b/presto-spi/src/main/java/io/prestosql/spi/security/AccessDeniedException.java index 41824883c..3bfd045f2 100644 --- a/presto-spi/src/main/java/io/prestosql/spi/security/AccessDeniedException.java +++ b/presto-spi/src/main/java/io/prestosql/spi/security/AccessDeniedException.java @@ -41,6 +41,11 @@ public class AccessDeniedException throw new AccessDeniedException(format("Principal %s cannot become user %s%s", principal.orElse(null), userName, formatExtraInfo(extraInfo))); } + public static void denyCatalogAccess() + { + denyCatalogAccess(null); + } + public static void denyCatalogAccess(String catalogName) { denyCatalogAccess(catalogName, null); @@ -48,7 +53,7 @@ public class AccessDeniedException public static void denyCatalogAccess(String catalogName, String extraInfo) { - throw new AccessDeniedException(format("Cannot access catalog %s%s", catalogName, formatExtraInfo(extraInfo))); + throw new AccessDeniedException(format("Cannot access catalog %s%s", catalogName == null ? "" : catalogName, formatExtraInfo(extraInfo))); } public static void denyCreateSchema(String schemaName) diff --git a/presto-spi/src/main/java/io/prestosql/spi/security/SystemAccessControl.java b/presto-spi/src/main/java/io/prestosql/spi/security/SystemAccessControl.java index 684ede6bf..4ea5e6090 100644 --- a/presto-spi/src/main/java/io/prestosql/spi/security/SystemAccessControl.java +++ b/presto-spi/src/main/java/io/prestosql/spi/security/SystemAccessControl.java @@ -83,6 +83,13 @@ public interface SystemAccessControl return Collections.emptySet(); } + /** + * Check whether identity is can show catalogs + * + * @throws AccessDeniedException if not allowed + */ + default void checkCanShowCatalogs(Identity identity) {} + /** * Check whether identity is can create a catalog *