From 8ef6cfe53b803c3f4c8821d650dec5e1ff609046 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Kokosi=C5=84ski?= Date: Mon, 29 May 2023 17:53:31 +0200 Subject: [PATCH 1/3] Require admin privileges for SET AUTHORIZATION in SqlStandard --- .../security/SqlStandardAccessControl.java | 6 +- .../plugin/hive/BaseHiveConnectorTest.java | 92 ++++++++++++++----- .../TestSqlStandardAccessControlChecks.java | 20 +++- 3 files changed, 87 insertions(+), 31 deletions(-) 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 fc23237392b3..c75464fa7e8d 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 @@ -155,7 +155,7 @@ public void checkCanRenameSchema(ConnectorSecurityContext context, String schema @Override public void checkCanSetSchemaAuthorization(ConnectorSecurityContext context, String schemaName, TrinoPrincipal principal) { - if (!isDatabaseOwner(context, schemaName)) { + if (!isAdmin(context)) { denySetSchemaAuthorization(schemaName, principal); } } @@ -307,7 +307,7 @@ public void checkCanAlterColumn(ConnectorSecurityContext context, SchemaTableNam @Override public void checkCanSetTableAuthorization(ConnectorSecurityContext context, SchemaTableName tableName, TrinoPrincipal principal) { - if (!isTableOwner(context, tableName)) { + if (!isAdmin(context)) { denySetTableAuthorization(tableName.toString(), principal); } } @@ -372,7 +372,7 @@ public void checkCanRenameView(ConnectorSecurityContext context, SchemaTableName @Override public void checkCanSetViewAuthorization(ConnectorSecurityContext context, SchemaTableName viewName, TrinoPrincipal principal) { - if (!isTableOwner(context, viewName)) { + if (!isAdmin(context)) { denySetViewAuthorization(viewName.toString(), principal); } } diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java index 9f18399f5ebe..c4816f7c44d7 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java @@ -813,20 +813,33 @@ public void testSchemaAuthorization() .build(); assertUpdate(admin, "CREATE SCHEMA test_schema_authorization"); - + assertAccessDenied(user, "ALTER SCHEMA test_schema_authorization SET AUTHORIZATION user2", "Cannot set authorization for schema test_schema_authorization to USER user2"); + assertAccessDenied(user, "DROP SCHEMA test_schema_authorization", "Cannot drop schema test_schema_authorization"); assertUpdate(admin, "ALTER SCHEMA test_schema_authorization SET AUTHORIZATION user"); - assertUpdate(user, "ALTER SCHEMA test_schema_authorization SET AUTHORIZATION ROLE admin"); - assertQueryFails(user, "ALTER SCHEMA test_schema_authorization SET AUTHORIZATION ROLE admin", "Access Denied: Cannot set authorization for schema test_schema_authorization to ROLE admin"); + // only admin can change the owner + assertAccessDenied(user, "ALTER SCHEMA test_schema_authorization SET AUTHORIZATION user2", "Cannot set authorization for schema test_schema_authorization to USER user2"); + assertUpdate(user, "DROP SCHEMA test_schema_authorization"); // new onwer can drop schema // switch owner back to user, and then change the owner to ROLE admin from a different catalog to verify roles are relative to the catalog of the schema - assertUpdate(admin, "ALTER SCHEMA test_schema_authorization SET AUTHORIZATION user"); Session userSessionInDifferentCatalog = testSessionBuilder() .setIdentity(Identity.forUser("user").withPrincipal(getSession().getIdentity().getPrincipal()).build()) .build(); - assertUpdate(userSessionInDifferentCatalog, "ALTER SCHEMA hive.test_schema_authorization SET AUTHORIZATION ROLE admin"); - assertUpdate(admin, "ALTER SCHEMA test_schema_authorization SET AUTHORIZATION user"); - - assertUpdate(admin, "DROP SCHEMA test_schema_authorization"); + assertUpdate(admin, "CREATE SCHEMA test_schema_authorization"); + assertAccessDenied( + userSessionInDifferentCatalog, + "ALTER SCHEMA hive.test_schema_authorization SET AUTHORIZATION user", + "Cannot set authorization for schema test_schema_authorization to USER user"); + assertAccessDenied( + userSessionInDifferentCatalog, + "DROP SCHEMA hive.test_schema_authorization", + "Cannot drop schema test_schema_authorization"); + assertUpdate(admin, "ALTER SCHEMA hive.test_schema_authorization SET AUTHORIZATION user"); + assertAccessDenied( + userSessionInDifferentCatalog, + "ALTER SCHEMA hive.test_schema_authorization SET AUTHORIZATION user", + "Cannot set authorization for schema test_schema_authorization to USER user"); + // new owner can drop schema + assertUpdate(userSessionInDifferentCatalog, "DROP SCHEMA hive.test_schema_authorization"); } @Test @@ -850,9 +863,14 @@ public void testTableAuthorization() "ALTER TABLE test_table_authorization.foo SET AUTHORIZATION alice", "Cannot set authorization for table test_table_authorization.foo to USER alice"); assertUpdate(admin, "ALTER TABLE test_table_authorization.foo SET AUTHORIZATION alice"); - assertUpdate(alice, "ALTER TABLE test_table_authorization.foo SET AUTHORIZATION admin"); + // only admin can change the owner + assertAccessDenied( + alice, + "ALTER TABLE test_table_authorization.foo SET AUTHORIZATION alice", + "Cannot set authorization for table test_table_authorization.foo to USER alice"); + // alice as new owner can now drop table + assertUpdate(alice, "DROP TABLE test_table_authorization.foo"); - assertUpdate(admin, "DROP TABLE test_table_authorization.foo"); assertUpdate(admin, "DROP SCHEMA test_table_authorization"); } @@ -877,13 +895,18 @@ public void testTableAuthorizationForRole() alice, "ALTER TABLE test_table_authorization_role.foo SET AUTHORIZATION ROLE admin", "Cannot set authorization for table test_table_authorization_role.foo to ROLE admin"); + assertAccessDenied( + alice, + "DROP TABLE test_table_authorization_role.foo", + "Cannot drop table test_table_authorization_role.foo"); assertUpdate(admin, "ALTER TABLE test_table_authorization_role.foo SET AUTHORIZATION alice"); - assertQueryFails( + // Only admin can change the owner + assertAccessDenied( alice, "ALTER TABLE test_table_authorization_role.foo SET AUTHORIZATION ROLE admin", - "Setting table owner type as a role is not supported"); - - assertUpdate(admin, "DROP TABLE test_table_authorization_role.foo"); + "Cannot set authorization for table test_table_authorization_role.foo to ROLE admin"); + // new owner can drop table + assertUpdate(alice, "DROP TABLE test_table_authorization_role.foo"); assertUpdate(admin, "DROP SCHEMA test_table_authorization_role"); } @@ -907,10 +930,14 @@ public void testViewAuthorization() assertAccessDenied( alice, - "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION alice", - "Cannot set authorization for view " + schema + ".test_view to USER alice"); + "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION admin", + "Cannot set authorization for view " + schema + ".test_view to USER admin"); assertUpdate(admin, "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION alice"); - assertUpdate(alice, "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION admin"); + // only admin can change the owner + assertAccessDenied( + alice, + "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION admin", + "Cannot set authorization for view " + schema + ".test_view to USER admin"); assertUpdate(admin, "DROP VIEW " + schema + ".test_view"); assertUpdate(admin, "DROP SCHEMA " + schema); @@ -936,13 +963,22 @@ public void testViewAuthorizationSecurityDefiner() assertUpdate(admin, "INSERT INTO " + schema + ".test_table VALUES (1)", 1); assertUpdate(admin, "CREATE VIEW " + schema + ".test_view SECURITY DEFINER AS SELECT * from " + schema + ".test_table"); assertUpdate(admin, "GRANT SELECT ON " + schema + ".test_view TO alice"); + assertAccessDenied( + alice, + "DROP VIEW " + schema + ".test_view", + "Cannot drop view " + schema + ".test_view"); assertQuery(alice, "SELECT * FROM " + schema + ".test_view", "VALUES (1)"); assertUpdate(admin, "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION alice"); assertQueryFails(alice, "SELECT * FROM " + schema + ".test_view", "Access Denied: Cannot select from table " + schema + ".test_table"); - assertUpdate(alice, "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION admin"); - assertUpdate(admin, "DROP VIEW " + schema + ".test_view"); + // only admin can change the owner + assertAccessDenied( + alice, + "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION admin", + "Cannot set authorization for view " + schema + ".test_view to USER admin"); + // new owner can drop the view + assertUpdate(alice, "DROP VIEW " + schema + ".test_view"); assertUpdate(admin, "DROP TABLE " + schema + ".test_table"); assertUpdate(admin, "DROP SCHEMA " + schema); } @@ -967,13 +1003,22 @@ public void testViewAuthorizationSecurityInvoker() assertUpdate(admin, "INSERT INTO " + schema + ".test_table VALUES (1)", 1); assertUpdate(admin, "CREATE VIEW " + schema + ".test_view SECURITY INVOKER AS SELECT * from " + schema + ".test_table"); assertUpdate(admin, "GRANT SELECT ON " + schema + ".test_view TO alice"); + assertAccessDenied( + alice, + "DROP VIEW " + schema + ".test_view", + "Cannot drop view " + schema + ".test_view"); assertQueryFails(alice, "SELECT * FROM " + schema + ".test_view", "Access Denied: Cannot select from table " + schema + ".test_table"); assertUpdate(admin, "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION alice"); assertQueryFails(alice, "SELECT * FROM " + schema + ".test_view", "Access Denied: Cannot select from table " + schema + ".test_table"); - assertUpdate(alice, "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION admin"); - assertUpdate(admin, "DROP VIEW " + schema + ".test_view"); + // only admin can change the owner + assertAccessDenied( + alice, + "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION admin", + "Cannot set authorization for view " + schema + ".test_view to USER admin"); + // new owner can drop the view + assertUpdate(alice, "DROP VIEW " + schema + ".test_view"); assertUpdate(admin, "DROP TABLE " + schema + ".test_table"); assertUpdate(admin, "DROP SCHEMA " + schema); } @@ -1003,10 +1048,11 @@ public void testViewAuthorizationForRole() "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION ROLE admin", "Cannot set authorization for view " + schema + ".test_view to ROLE admin"); assertUpdate(admin, "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION alice"); - assertQueryFails( + // only admin can change the owner + assertAccessDenied( alice, "ALTER VIEW " + schema + ".test_view SET AUTHORIZATION ROLE admin", - "Setting table owner type as a role is not supported"); + "Cannot set authorization for view " + schema + ".test_view to ROLE admin"); assertUpdate(admin, "DROP VIEW " + schema + ".test_view"); assertUpdate(admin, "DROP TABLE " + schema + ".test_table"); diff --git a/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestSqlStandardAccessControlChecks.java b/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestSqlStandardAccessControlChecks.java index b155c9416506..6e3fcb184537 100644 --- a/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestSqlStandardAccessControlChecks.java +++ b/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestSqlStandardAccessControlChecks.java @@ -37,6 +37,7 @@ public class TestSqlStandardAccessControlChecks private QueryExecutor bobExecutor; private QueryExecutor charlieExecutor; private QueryExecutor caseSensitiveUserNameExecutor; + private QueryExecutor hdfsExecutor; @BeforeMethodWithContext public void setup() @@ -45,6 +46,7 @@ public void setup() bobExecutor = connectToTrino("bob@presto"); charlieExecutor = connectToTrino("charlie@presto"); caseSensitiveUserNameExecutor = connectToTrino("CaseSensitiveUserName@presto"); + hdfsExecutor = connectToTrino("hdfs@presto"); aliceExecutor.executeQuery(format("DROP TABLE IF EXISTS %s", tableName)); aliceExecutor.executeQuery(format("CREATE TABLE %s(month bigint, day bigint) WITH (partitioned_by = ARRAY['day'])", tableName)); @@ -61,6 +63,7 @@ public void cleanup() bobExecutor = null; charlieExecutor = null; caseSensitiveUserNameExecutor = null; + hdfsExecutor = null; } @Test(groups = {AUTHORIZATION, PROFILE_SPECIFIC_TESTS}) @@ -243,10 +246,15 @@ public void testAccessControlSelectWithCaseSensitiveUserName() @Test(groups = {AUTHORIZATION, PROFILE_SPECIFIC_TESTS}) public void testAccessControlSetTableAuthorization() { + // Using custom table name as drop table as different user may leave some data in HDFS + String tableName = "set_table_authorization_test"; + aliceExecutor.executeQuery(format("CREATE TABLE %s(col1 bigint, col2 bigint)", tableName)); assertQueryFailure(() -> bobExecutor.executeQuery(format("ALTER TABLE %s SET AUTHORIZATION bob", tableName))) .hasMessageContaining("Access Denied: Cannot set authorization for table default.%s to USER bob", tableName); - aliceExecutor.executeQuery(format("ALTER TABLE %s SET AUTHORIZATION bob", tableName)); - bobExecutor.executeQuery(format("ALTER TABLE %s SET AUTHORIZATION alice", tableName)); + hdfsExecutor.executeQuery("SET SESSION legacy_catalog_roles=true"); + hdfsExecutor.executeQuery("SET ROLE ADMIN"); + hdfsExecutor.executeQuery(format("ALTER TABLE %s SET AUTHORIZATION bob", tableName)); + bobExecutor.executeQuery("DROP TABLE " + tableName); } @Test(groups = {AUTHORIZATION, PROFILE_SPECIFIC_TESTS}) @@ -257,7 +265,9 @@ public void testAccessControlSetViewAuthorization() assertQueryFailure(() -> bobExecutor.executeQuery(format("DROP VIEW %s", viewName))) .hasMessageContaining("Access Denied: Cannot drop view default.%s", viewName); - aliceExecutor.executeQuery(format("ALTER VIEW %s SET AUTHORIZATION bob", viewName)); + hdfsExecutor.executeQuery("SET SESSION legacy_catalog_roles=true"); + hdfsExecutor.executeQuery("SET ROLE ADMIN"); + hdfsExecutor.executeQuery(format("ALTER VIEW %s SET AUTHORIZATION bob", viewName)); bobExecutor.executeQuery(format("DROP VIEW %s", viewName)); } @@ -267,13 +277,13 @@ public void testAccessControlSetHiveViewAuthorization() onHive().executeQuery("CREATE TABLE test_hive_table (col1 int)"); onHive().executeQuery("CREATE VIEW test_hive_view AS SELECT * FROM test_hive_table"); - QueryExecutor hdfsExecutor = connectToTrino("hdfs@presto"); - assertQueryFailure(() -> bobExecutor.executeQuery("ALTER VIEW test_hive_view SET AUTHORIZATION bob")) .hasMessageContaining("Access Denied: Cannot set authorization for view default.test_hive_view to USER bob"); assertQueryFailure(() -> bobExecutor.executeQuery("DROP VIEW test_hive_view")) .hasMessageContaining("Access Denied: Cannot drop view default.test_hive_view"); + hdfsExecutor.executeQuery("SET SESSION legacy_catalog_roles=true"); + hdfsExecutor.executeQuery("SET ROLE ADMIN"); hdfsExecutor.executeQuery("ALTER VIEW test_hive_view SET AUTHORIZATION bob"); bobExecutor.executeQuery("DROP VIEW test_hive_view"); From 402898df3a66f9024c1f6f263907063f7801e5d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Kokosi=C5=84ski?= Date: Wed, 31 May 2023 15:48:18 +0200 Subject: [PATCH 2/3] Introduce SET AUTHORIZATION rules to file based access controls Using new rules will allow to control who can grant AUTHORIZATION (move ownership) to whom. In order to perform the SET AUTHORIZATION statement one has also ownership access to referred entity. So to be authorized to perform: ALTER TABLE T SET AUTHORIZATION USER U; one has to have access to modify the table T and proper authorization rule to move ownership to user U: --- .../main/sphinx/security/authorization.json | 26 ++ .../security/file-system-access-control.rst | 44 +++ .../base/security/AccessControlRules.java | 14 +- .../base/security/AuthorizationRule.java | 85 +++++ .../base/security/FileBasedAccessControl.java | 23 ++ .../FileBasedSystemAccessControl.java | 30 ++ .../FileBasedSystemAccessControlModule.java | 1 + .../FileBasedSystemAccessControlRules.java | 8 + ...seFileBasedConnectorAccessControlTest.java | 103 +++++- .../BaseFileBasedSystemAccessControlTest.java | 343 ++++++++++++------ .../resources/authorization-no-roles.json | 37 ++ .../src/test/resources/authorization.json | 69 ++++ 12 files changed, 664 insertions(+), 119 deletions(-) create mode 100644 docs/src/main/sphinx/security/authorization.json create mode 100644 lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AuthorizationRule.java create mode 100644 lib/trino-plugin-toolkit/src/test/resources/authorization-no-roles.json create mode 100644 lib/trino-plugin-toolkit/src/test/resources/authorization.json diff --git a/docs/src/main/sphinx/security/authorization.json b/docs/src/main/sphinx/security/authorization.json new file mode 100644 index 000000000000..07c147557de1 --- /dev/null +++ b/docs/src/main/sphinx/security/authorization.json @@ -0,0 +1,26 @@ +{ + "authorization": [ + { + "original_role": "admin", + "new_user": "bob", + "allow": false + }, + { + "original_role": "admin", + "new_user": ".*", + "new_role": ".*" + } + ], + "schemas": [ + { + "role": "admin", + "owner": true + } + ], + "tables": [ + { + "role": "admin", + "privileges": ["OWNERSHIP"] + } + ] +} diff --git a/docs/src/main/sphinx/security/file-system-access-control.rst b/docs/src/main/sphinx/security/file-system-access-control.rst index 67dd69754e7b..f8e66f101db3 100644 --- a/docs/src/main/sphinx/security/file-system-access-control.rst +++ b/docs/src/main/sphinx/security/file-system-access-control.rst @@ -722,6 +722,50 @@ The fixed management user only applies to HTTP by default. To enable the fixed user over HTTPS, set the ``management.user.https-enabled`` configuration property. +.. _system-file-auth-authorization: + +Authorization rules +------------------- + +These rules control the ability of how owner of schema, table or view can +be altered. These rules are applicable to commands like: + + ALTER SCHEMA name SET AUTHORIZATION ( user | USER user | ROLE role ) + ALTER TABLE name SET AUTHORIZATION ( user | USER user | ROLE role ) + ALTER VIEW name SET AUTHORIZATION ( user | USER user | ROLE role ) + +When these rules are present, the authorization is based on the first matching +rule, processed from top to bottom. If no rules match, the authorization is +denied. + +Notice that in order to execute ``ALTER`` command on schema, table or view user requires ``OWNERSHIP`` +privilege. + +Each authorization rule is composed of the following fields: + +* ``original_user`` (optional): regex to match against the user requesting the + authorization. Defaults to ``.*``. +* ``original_group`` (optional): regex to match against group names of the + requesting authorization. Defaults to ``.*``. +* ``original_role`` (optional): regex to match against role names of the + requesting authorization. Defaults to ``.*``. +* ``new_user`` (optional): regex to match against the new owner user of the schema, table or view. + By default it does not match. +* ``new_role`` (optional): regex to match against the new owner role of the schema, table or view. + By default it does not match. +* ``allow`` (optional): boolean indicating if the authentication should be + allowed. Defaults to ``true``. + +Notice that ``new_user`` and ``new_role`` are optional, however it is required to provide at least one of them. + +The following example allows the ``admin`` role, to change owner of any schema, table or view +to any user, except to``bob``. + +.. literalinclude:: authorization.json + :language: json + +.. _system-file-auth-system_information: + .. _catalog-file-based-access-control: Catalog-level access control files diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AccessControlRules.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AccessControlRules.java index 3a5962872660..f5866422ac60 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AccessControlRules.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AccessControlRules.java @@ -27,18 +27,21 @@ public class AccessControlRules private final List tableRules; private final List sessionPropertyRules; private final List functionRules; + private final List authorizationRules; @JsonCreator public AccessControlRules( @JsonProperty("schemas") Optional> schemaRules, @JsonProperty("tables") Optional> tableRules, @JsonProperty("session_properties") @JsonAlias("sessionProperties") Optional> sessionPropertyRules, - @JsonProperty("functions") Optional> functionRules) + @JsonProperty("functions") Optional> functionRules, + @JsonProperty("authorization") Optional> authorizationRules) { this.schemaRules = schemaRules.orElse(ImmutableList.of(SchemaAccessControlRule.ALLOW_ALL)); this.tableRules = tableRules.orElse(ImmutableList.of(TableAccessControlRule.ALLOW_ALL)); this.sessionPropertyRules = sessionPropertyRules.orElse(ImmutableList.of(SessionPropertyAccessControlRule.ALLOW_ALL)); this.functionRules = functionRules.orElse(ImmutableList.of(FunctionAccessControlRule.ALLOW_ALL)); + this.authorizationRules = authorizationRules.orElse(ImmutableList.of()); } public List getSchemaRules() @@ -61,11 +64,18 @@ public List getFunctionRules() return functionRules; } + public List getAuthorizationRules() + { + return authorizationRules; + } + public boolean hasRoleRules() { return schemaRules.stream().anyMatch(rule -> rule.getRoleRegex().isPresent()) || tableRules.stream().anyMatch(rule -> rule.getRoleRegex().isPresent()) || sessionPropertyRules.stream().anyMatch(rule -> rule.getRoleRegex().isPresent()) || - functionRules.stream().anyMatch(rule -> rule.getRoleRegex().isPresent()); + functionRules.stream().anyMatch(rule -> rule.getRoleRegex().isPresent()) || + authorizationRules.stream().anyMatch(rule -> rule.getOriginalRolePattern().isPresent()) || + authorizationRules.stream().anyMatch(rule -> rule.getNewRolePattern().isPresent()); } } diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AuthorizationRule.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AuthorizationRule.java new file mode 100644 index 000000000000..d716649260a4 --- /dev/null +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AuthorizationRule.java @@ -0,0 +1,85 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.plugin.base.security; + +import com.fasterxml.jackson.annotation.JsonAlias; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import io.trino.spi.security.TrinoPrincipal; + +import java.util.Optional; +import java.util.Set; +import java.util.regex.Pattern; + +import static com.google.common.base.MoreObjects.firstNonNull; +import static com.google.common.base.Preconditions.checkArgument; +import static java.lang.Boolean.TRUE; +import static java.util.Objects.requireNonNull; + +public class AuthorizationRule +{ + private final Optional originalUserPattern; + private final Optional originalGroupPattern; + private final Optional originalRolePattern; + private final Optional newUserPattern; + private final Optional newRolePattern; + private final boolean allow; + + @JsonCreator + public AuthorizationRule( + @JsonProperty("original_user") @JsonAlias("originalUser") Optional originalUserPattern, + @JsonProperty("original_group") @JsonAlias("originalGroup") Optional originalGroupPattern, + @JsonProperty("original_role") @JsonAlias("originalRole") Optional originalRolePattern, + @JsonProperty("new_user") @JsonAlias("newUser") Optional newUserPattern, + @JsonProperty("new_role") @JsonAlias("newRole") Optional newRolePattern, + @JsonProperty("allow") Boolean allow) + { + checkArgument(newUserPattern.isPresent() || newRolePattern.isPresent(), "At least one of new_use or new_role is required, none were provided"); + this.originalUserPattern = requireNonNull(originalUserPattern, "originalUserPattern is null"); + this.originalGroupPattern = requireNonNull(originalGroupPattern, "originalGroupPattern is null"); + this.originalRolePattern = requireNonNull(originalRolePattern, "originalRolePattern is null"); + this.newUserPattern = requireNonNull(newUserPattern, "newUserPattern is null"); + this.newRolePattern = requireNonNull(newRolePattern, "newRolePattern is null"); + this.allow = firstNonNull(allow, TRUE); + } + + public Optional match(String user, Set groups, Set roles, TrinoPrincipal newPrincipal) + { + if (originalUserPattern.map(regex -> regex.matcher(user).matches()).orElse(true) && + (originalGroupPattern.isEmpty() || groups.stream().anyMatch(group -> originalGroupPattern.get().matcher(group).matches())) && + (originalRolePattern.isEmpty() || roles.stream().anyMatch(role -> originalRolePattern.get().matcher(role).matches())) && + matches(newPrincipal)) { + return Optional.of(allow); + } + return Optional.empty(); + } + + private boolean matches(TrinoPrincipal newPrincipal) + { + return switch (newPrincipal.getType()) { + case USER -> newUserPattern.map(regex -> regex.matcher(newPrincipal.getName()).matches()).orElse(false); + case ROLE -> newRolePattern.map(regex -> regex.matcher(newPrincipal.getName()).matches()).orElse(false); + }; + } + + public Optional getOriginalRolePattern() + { + return originalRolePattern; + } + + public Optional getNewRolePattern() + { + return newRolePattern; + } +} 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 590744dd89cc..360332b02dc2 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 @@ -108,6 +108,7 @@ public class FileBasedAccessControl private final List tableRules; private final List sessionPropertyRules; private final List functionRules; + private final List authorizationRules; private final Set anySchemaPermissionsRules; public FileBasedAccessControl(CatalogName catalogName, AccessControlRules rules) @@ -120,6 +121,7 @@ public FileBasedAccessControl(CatalogName catalogName, AccessControlRules rules) this.tableRules = rules.getTableRules(); this.sessionPropertyRules = rules.getSessionPropertyRules(); this.functionRules = rules.getFunctionRules(); + this.authorizationRules = rules.getAuthorizationRules(); ImmutableSet.Builder anySchemaPermissionsRules = ImmutableSet.builder(); schemaRules.stream() .map(SchemaAccessControlRule::toAnySchemaPermissionsRule) @@ -169,6 +171,9 @@ public void checkCanSetSchemaAuthorization(ConnectorSecurityContext context, Str if (!isSchemaOwner(context, schemaName)) { denySetSchemaAuthorization(schemaName, principal); } + if (!checkCanSetAuthorization(context, principal)) { + denySetSchemaAuthorization(schemaName, principal); + } } @Override @@ -347,6 +352,9 @@ public void checkCanSetTableAuthorization(ConnectorSecurityContext context, Sche if (!checkTablePermission(context, tableName, OWNERSHIP)) { denySetTableAuthorization(tableName.toString(), principal); } + if (!checkCanSetAuthorization(context, principal)) { + denySetTableAuthorization(tableName.toString(), principal); + } } @Override @@ -423,6 +431,9 @@ public void checkCanSetViewAuthorization(ConnectorSecurityContext context, Schem if (!checkTablePermission(context, viewName, OWNERSHIP)) { denySetViewAuthorization(viewName.toString(), principal); } + if (!checkCanSetAuthorization(context, principal)) { + denySetViewAuthorization(viewName.toString(), principal); + } } @Override @@ -749,4 +760,16 @@ private boolean checkFunctionPermission(ConnectorSecurityContext context, Functi .filter(executePredicate) .isPresent(); } + + private boolean checkCanSetAuthorization(ConnectorSecurityContext context, TrinoPrincipal principal) + { + ConnectorIdentity identity = context.getIdentity(); + Set roles = identity.getConnectorRole().stream() + .flatMap(role -> role.getRole().stream()) + .collect(toImmutableSet()); + return authorizationRules.stream() + .flatMap(rule -> rule.match(identity.getUser(), identity.getGroups(), roles, principal).stream()) + .findFirst() + .orElse(false); + } } 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 20c5d5d4a9bf..f8a4f09d2c3e 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 @@ -129,6 +129,7 @@ public class FileBasedSystemAccessControl private final Optional> impersonationRules; private final Optional> principalUserMatchRules; private final Optional> systemInformationRules; + private final List authorizationRules; private final List schemaRules; private final List tableRules; private final List sessionPropertyRules; @@ -143,6 +144,7 @@ private FileBasedSystemAccessControl( Optional> impersonationRules, Optional> principalUserMatchRules, Optional> systemInformationRules, + List authorizationRules, List schemaRules, List tableRules, List sessionPropertyRules, @@ -154,6 +156,7 @@ private FileBasedSystemAccessControl( this.impersonationRules = impersonationRules; this.principalUserMatchRules = principalUserMatchRules; this.systemInformationRules = systemInformationRules; + this.authorizationRules = authorizationRules; this.schemaRules = schemaRules; this.tableRules = tableRules; this.sessionPropertyRules = sessionPropertyRules; @@ -434,6 +437,9 @@ public void checkCanSetSchemaAuthorization(SystemSecurityContext context, Catalo if (!isSchemaOwner(context, schema)) { denySetSchemaAuthorization(schema.toString(), principal); } + if (!checkCanSetAuthorization(context, principal)) { + denySetSchemaAuthorization(schema.toString(), principal); + } } @Override @@ -628,6 +634,9 @@ public void checkCanSetTableAuthorization(SystemSecurityContext context, Catalog if (!checkTablePermission(context, table, OWNERSHIP)) { denySetTableAuthorization(table.toString(), principal); } + if (!checkCanSetAuthorization(context, principal)) { + denySetTableAuthorization(table.toString(), principal); + } } @Override @@ -700,6 +709,9 @@ public void checkCanSetViewAuthorization(SystemSecurityContext context, CatalogS if (!checkTablePermission(context, view, OWNERSHIP)) { denySetViewAuthorization(view.toString(), principal); } + if (!checkCanSetAuthorization(context, principal)) { + denySetViewAuthorization(view.toString(), principal); + } } @Override @@ -1109,6 +1121,15 @@ private boolean checkFunctionPermission(SystemSecurityContext context, FunctionK .isPresent(); } + private boolean checkCanSetAuthorization(SystemSecurityContext context, TrinoPrincipal principal) + { + Identity identity = context.getIdentity(); + return authorizationRules.stream() + .flatMap(rule -> rule.match(identity.getUser(), identity.getGroups(), identity.getEnabledRoles(), principal).stream()) + .findFirst() + .orElse(false); + } + public static Builder builder() { return new Builder(); @@ -1121,6 +1142,7 @@ public static final class Builder private Optional> impersonationRules = Optional.empty(); private Optional> principalUserMatchRules = Optional.empty(); private Optional> systemInformationRules = Optional.empty(); + private List authorizationRules = ImmutableList.of(); private List schemaRules = ImmutableList.of(CatalogSchemaAccessControlRule.ALLOW_ALL); private List tableRules = ImmutableList.of(CatalogTableAccessControlRule.ALLOW_ALL); private List sessionPropertyRules = ImmutableList.of(SessionPropertyAccessControlRule.ALLOW_ALL); @@ -1135,6 +1157,7 @@ public Builder denyAllAccess() impersonationRules = Optional.of(ImmutableList.of()); principalUserMatchRules = Optional.of(ImmutableList.of()); systemInformationRules = Optional.of(ImmutableList.of()); + authorizationRules = ImmutableList.of(); schemaRules = ImmutableList.of(); tableRules = ImmutableList.of(); sessionPropertyRules = ImmutableList.of(); @@ -1173,6 +1196,12 @@ public Builder setSystemInformationRules(Optional> s return this; } + public Builder setAuthorizationRules(List authorizationRules) + { + this.authorizationRules = authorizationRules; + return this; + } + public Builder setSchemaRules(List schemaRules) { this.schemaRules = schemaRules; @@ -1211,6 +1240,7 @@ public FileBasedSystemAccessControl build() impersonationRules, principalUserMatchRules, systemInformationRules, + authorizationRules, schemaRules, tableRules, sessionPropertyRules, diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControlModule.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControlModule.java index 891d5ca2dff2..2a01587a2ead 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControlModule.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControlModule.java @@ -103,6 +103,7 @@ private SystemAccessControl create(FileBasedSystemAccessControlRules rules) .setImpersonationRules(rules.getImpersonationRules()) .setPrincipalUserMatchRules(rules.getPrincipalUserMatchRules()) .setSystemInformationRules(rules.getSystemInformationRules()) + .setAuthorizationRules(rules.getAuthorizationRules()) .setSchemaRules(rules.getSchemaRules().orElse(ImmutableList.of(CatalogSchemaAccessControlRule.ALLOW_ALL))) .setTableRules(rules.getTableRules().orElse(ImmutableList.of(CatalogTableAccessControlRule.ALLOW_ALL))) .setSessionPropertyRules(rules.getSessionPropertyRules().orElse(ImmutableList.of(SessionPropertyAccessControlRule.ALLOW_ALL))) diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControlRules.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControlRules.java index 8665b91fdcd5..ebb3436ef327 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControlRules.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedSystemAccessControlRules.java @@ -27,6 +27,7 @@ public class FileBasedSystemAccessControlRules private final Optional> impersonationRules; private final Optional> principalUserMatchRules; private final Optional> systemInformationRules; + private final Optional> authorizationRules; private final Optional> schemaRules; private final Optional> tableRules; private final Optional> sessionPropertyRules; @@ -40,6 +41,7 @@ public FileBasedSystemAccessControlRules( @JsonProperty("impersonation") Optional> impersonationRules, @JsonProperty("principals") Optional> principalUserMatchRules, @JsonProperty("system_information") Optional> systemInformationRules, + @JsonProperty("authorization") Optional> authorizationRules, @JsonProperty("schemas") Optional> schemaAccessControlRules, @JsonProperty("tables") Optional> tableAccessControlRules, @JsonProperty("system_session_properties") Optional> sessionPropertyRules, @@ -51,6 +53,7 @@ public FileBasedSystemAccessControlRules( this.principalUserMatchRules = principalUserMatchRules.map(ImmutableList::copyOf); this.impersonationRules = impersonationRules.map(ImmutableList::copyOf); this.systemInformationRules = systemInformationRules.map(ImmutableList::copyOf); + this.authorizationRules = authorizationRules.map(ImmutableList::copyOf); this.schemaRules = schemaAccessControlRules.map(ImmutableList::copyOf); this.tableRules = tableAccessControlRules.map(ImmutableList::copyOf); this.sessionPropertyRules = sessionPropertyRules.map(ImmutableList::copyOf); @@ -83,6 +86,11 @@ public Optional> getSystemInformationRules() return systemInformationRules; } + public List getAuthorizationRules() + { + return authorizationRules.orElseGet(ImmutableList::of); + } + public Optional> getSchemaRules() { return schemaRules; diff --git a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/BaseFileBasedConnectorAccessControlTest.java b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/BaseFileBasedConnectorAccessControlTest.java index d1133a1c0d1a..3c7d63eb3137 100644 --- a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/BaseFileBasedConnectorAccessControlTest.java +++ b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/BaseFileBasedConnectorAccessControlTest.java @@ -77,7 +77,7 @@ public void testEmptyFile() accessControl.checkCanCreateSchema(UNKNOWN, "unknown", ImmutableMap.of()); accessControl.checkCanDropSchema(UNKNOWN, "unknown"); accessControl.checkCanRenameSchema(UNKNOWN, "unknown", "new_unknown"); - accessControl.checkCanSetSchemaAuthorization(UNKNOWN, "unknown", new TrinoPrincipal(ROLE, "some_role")); + assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(UNKNOWN, "unknown", new TrinoPrincipal(ROLE, "some_role"))); accessControl.checkCanShowCreateSchema(UNKNOWN, "unknown"); accessControl.checkCanSelectFromColumns(UNKNOWN, new SchemaTableName("unknown", "unknown"), ImmutableSet.of()); @@ -200,10 +200,10 @@ public void testSchemaRules() accessControl.checkCanRenameSchema(CHARLIE, "authenticated", "authenticated"); assertDenied(() -> accessControl.checkCanRenameSchema(CHARLIE, "test", "new_schema")); - accessControl.checkCanSetSchemaAuthorization(ADMIN, "test", new TrinoPrincipal(ROLE, "some_role")); - accessControl.checkCanSetSchemaAuthorization(ADMIN, "test", new TrinoPrincipal(USER, "some_user")); - accessControl.checkCanSetSchemaAuthorization(BOB, "bob", new TrinoPrincipal(ROLE, "some_role")); - accessControl.checkCanSetSchemaAuthorization(BOB, "bob", new TrinoPrincipal(USER, "some_user")); + assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(ADMIN, "test", new TrinoPrincipal(ROLE, "some_role"))); + assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(ADMIN, "test", new TrinoPrincipal(USER, "some_user"))); + assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(BOB, "bob", new TrinoPrincipal(ROLE, "some_role"))); + assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(BOB, "bob", new TrinoPrincipal(USER, "some_user"))); assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(BOB, "test", new TrinoPrincipal(ROLE, "some_role"))); assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(BOB, "test", new TrinoPrincipal(USER, "some_user"))); @@ -376,17 +376,17 @@ public void testTableRules() assertDenied(() -> accessControl.checkCanSetMaterializedViewProperties(ALICE, new SchemaTableName("bobschema", "bobmaterializedview"), ImmutableMap.of())); assertDenied(() -> accessControl.checkCanSetMaterializedViewProperties(BOB, new SchemaTableName("bobschema", "bobmaterializedview"), ImmutableMap.of())); - accessControl.checkCanSetTableAuthorization(ADMIN, testTable, new TrinoPrincipal(ROLE, "some_role")); - accessControl.checkCanSetTableAuthorization(ADMIN, testTable, new TrinoPrincipal(USER, "some_user")); - accessControl.checkCanSetTableAuthorization(ALICE, aliceTable, new TrinoPrincipal(ROLE, "some_role")); - accessControl.checkCanSetTableAuthorization(ALICE, aliceTable, new TrinoPrincipal(USER, "some_user")); + assertDenied(() -> accessControl.checkCanSetTableAuthorization(ADMIN, testTable, new TrinoPrincipal(ROLE, "some_role"))); + assertDenied(() -> accessControl.checkCanSetTableAuthorization(ADMIN, testTable, new TrinoPrincipal(USER, "some_user"))); + assertDenied(() -> accessControl.checkCanSetTableAuthorization(ALICE, aliceTable, new TrinoPrincipal(ROLE, "some_role"))); + assertDenied(() -> accessControl.checkCanSetTableAuthorization(ALICE, aliceTable, new TrinoPrincipal(USER, "some_user"))); assertDenied(() -> accessControl.checkCanSetTableAuthorization(ALICE, bobTable, new TrinoPrincipal(ROLE, "some_role"))); assertDenied(() -> accessControl.checkCanSetTableAuthorization(ALICE, bobTable, new TrinoPrincipal(USER, "some_user"))); - accessControl.checkCanSetViewAuthorization(ADMIN, testTable, new TrinoPrincipal(ROLE, "some_role")); - accessControl.checkCanSetViewAuthorization(ADMIN, testTable, new TrinoPrincipal(USER, "some_user")); - accessControl.checkCanSetViewAuthorization(ALICE, aliceTable, new TrinoPrincipal(ROLE, "some_role")); - accessControl.checkCanSetViewAuthorization(ALICE, aliceTable, new TrinoPrincipal(USER, "some_user")); + assertDenied(() -> accessControl.checkCanSetViewAuthorization(ADMIN, testTable, new TrinoPrincipal(ROLE, "some_role"))); + assertDenied(() -> accessControl.checkCanSetViewAuthorization(ADMIN, testTable, new TrinoPrincipal(USER, "some_user"))); + assertDenied(() -> accessControl.checkCanSetViewAuthorization(ALICE, aliceTable, new TrinoPrincipal(ROLE, "some_role"))); + assertDenied(() -> accessControl.checkCanSetViewAuthorization(ALICE, aliceTable, new TrinoPrincipal(USER, "some_user"))); assertDenied(() -> accessControl.checkCanSetViewAuthorization(ALICE, bobTable, new TrinoPrincipal(ROLE, "some_role"))); assertDenied(() -> accessControl.checkCanSetViewAuthorization(ALICE, bobTable, new TrinoPrincipal(USER, "some_user"))); } @@ -656,6 +656,78 @@ public void testFilterSchemasWithJsonPointer() assertFilterSchemas(accessControl); } + @Test + public void testSchemaAuthorization() + { + ConnectorAccessControl accessControl = createAccessControl("authorization-no-roles.json"); + + String schema = "test"; + String ownedByUser = "owned_by_user"; + String ownedByGroup = "owned_by_group"; + + assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(user("user", "group"), schema, new TrinoPrincipal(ROLE, "new_role"))); + + // access to schema granted to user + accessControl.checkCanSetSchemaAuthorization(user("owner_authorized", "group"), ownedByUser, new TrinoPrincipal(USER, "new_user")); + assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(user("owner_DENY_authorized", "group"), ownedByUser, new TrinoPrincipal(USER, "new_user"))); + assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(user("owner", "group"), ownedByUser, new TrinoPrincipal(USER, "new_user"))); + assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(user("owner_authorized", "group"), ownedByUser, new TrinoPrincipal(ROLE, "new_role"))); + + // access to schema granted to group + accessControl.checkCanSetSchemaAuthorization(user("authorized", "owner"), ownedByGroup, new TrinoPrincipal(USER, "new_user")); + assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(user("DENY_authorized", "owner"), ownedByGroup, new TrinoPrincipal(USER, "new_user"))); + assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(user("user", "owner"), ownedByGroup, new TrinoPrincipal(USER, "new_user"))); + assertDenied(() -> accessControl.checkCanSetSchemaAuthorization(user("authorized", "owner"), ownedByGroup, new TrinoPrincipal(ROLE, "new_role"))); + } + + @Test + public void testTableAuthorization() + { + ConnectorAccessControl accessControl = createAccessControl("authorization-no-roles.json"); + + SchemaTableName table = new SchemaTableName("test", "table"); + SchemaTableName ownedByUser = new SchemaTableName("test", "owned_by_user"); + SchemaTableName ownedByGroup = new SchemaTableName("test", "owned_by_group"); + + assertDenied(() -> accessControl.checkCanSetTableAuthorization(user("user", "group"), table, new TrinoPrincipal(ROLE, "new_role"))); + + // access to table granted to user + accessControl.checkCanSetTableAuthorization(user("owner_authorized", "group"), ownedByUser, new TrinoPrincipal(USER, "new_user")); + assertDenied(() -> accessControl.checkCanSetTableAuthorization(user("owner", "group"), ownedByUser, new TrinoPrincipal(ROLE, "new_role"))); + assertDenied(() -> accessControl.checkCanSetTableAuthorization(user("owner_authorized", "group"), ownedByUser, new TrinoPrincipal(ROLE, "new_role"))); + assertDenied(() -> accessControl.checkCanSetTableAuthorization(user("owner_DENY_authorized", "group"), ownedByUser, new TrinoPrincipal(USER, "new_user"))); + + // access to table granted to group + assertDenied(() -> accessControl.checkCanSetTableAuthorization(user("user", "owner"), ownedByGroup, new TrinoPrincipal(USER, "new_user"))); + accessControl.checkCanSetTableAuthorization(user("authorized", "owner"), ownedByGroup, new TrinoPrincipal(USER, "new_user")); + assertDenied(() -> accessControl.checkCanSetTableAuthorization(user("authorized", "owner"), ownedByGroup, new TrinoPrincipal(ROLE, "new_role"))); + assertDenied(() -> accessControl.checkCanSetTableAuthorization(user("DENY_authorized", "owner"), ownedByGroup, new TrinoPrincipal(USER, "new_user"))); + } + + @Test + public void testViewAuthorization() + { + ConnectorAccessControl accessControl = createAccessControl("authorization-no-roles.json"); + + SchemaTableName table = new SchemaTableName("test", "table"); + SchemaTableName ownedByUser = new SchemaTableName("test", "owned_by_user"); + SchemaTableName ownedByGroup = new SchemaTableName("test", "owned_by_group"); + + assertDenied(() -> accessControl.checkCanSetViewAuthorization(user("user", "group"), table, new TrinoPrincipal(ROLE, "new_role"))); + + // access to schema granted to user + accessControl.checkCanSetViewAuthorization(user("owner_authorized", "group"), ownedByUser, new TrinoPrincipal(USER, "new_user")); + assertDenied(() -> accessControl.checkCanSetViewAuthorization(user("owner_DENY_authorized", "group"), ownedByUser, new TrinoPrincipal(USER, "new_user"))); + assertDenied(() -> accessControl.checkCanSetViewAuthorization(user("owner", "group"), ownedByUser, new TrinoPrincipal(USER, "new_user"))); + assertDenied(() -> accessControl.checkCanSetViewAuthorization(user("owner_authorized", "group"), ownedByUser, new TrinoPrincipal(ROLE, "new_role"))); + + // access to schema granted to group + accessControl.checkCanSetViewAuthorization(user("authorized", "owner"), ownedByGroup, new TrinoPrincipal(USER, "new_user")); + assertDenied(() -> accessControl.checkCanSetViewAuthorization(user("DENY_authorized", "owner"), ownedByGroup, new TrinoPrincipal(USER, "new_user"))); + assertDenied(() -> accessControl.checkCanSetViewAuthorization(user("user", "owner"), ownedByGroup, new TrinoPrincipal(USER, "new_user"))); + assertDenied(() -> accessControl.checkCanSetViewAuthorization(user("authorized", "owner"), ownedByGroup, new TrinoPrincipal(ROLE, "new_role"))); + } + @Test public void testEverythingImplemented() throws NoSuchMethodException @@ -712,6 +784,11 @@ private String getResourcePath(String resourceName) return requireNonNull(this.getClass().getClassLoader().getResource(resourceName), "Resource does not exist: " + resourceName).getPath(); } + private static ConnectorSecurityContext user(String user, String group) + { + return user(user, ImmutableSet.of(group)); + } + private static ConnectorSecurityContext user(String name, Set groups) { return new ConnectorSecurityContext( diff --git a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/BaseFileBasedSystemAccessControlTest.java b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/BaseFileBasedSystemAccessControlTest.java index 43114da01592..90396adbaac2 100644 --- a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/BaseFileBasedSystemAccessControlTest.java +++ b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/BaseFileBasedSystemAccessControlTest.java @@ -94,7 +94,6 @@ public abstract class BaseFileBasedSystemAccessControlTest private static final String CREATE_SCHEMA_ACCESS_DENIED_MESSAGE = "Cannot create schema .*"; private static final String DROP_SCHEMA_ACCESS_DENIED_MESSAGE = "Cannot drop schema .*"; private static final String RENAME_SCHEMA_ACCESS_DENIED_MESSAGE = "Cannot rename schema from .* to .*"; - private static final String AUTH_SCHEMA_ACCESS_DENIED_MESSAGE = "Cannot set authorization for schema .* to .*"; private static final String SHOW_CREATE_SCHEMA_ACCESS_DENIED_MESSAGE = "Cannot show create schema for .*"; private static final String GRANT_SCHEMA_ACCESS_DENIED_MESSAGE = "Cannot grant privilege %s on schema %s%s"; private static final String DENY_SCHEMA_ACCESS_DENIED_MESSAGE = "Cannot deny privilege %s on schema %s%s"; @@ -106,8 +105,6 @@ public abstract class BaseFileBasedSystemAccessControlTest private static final String ADD_COLUMNS_ACCESS_DENIED_MESSAGE = "Cannot add a column to table .*"; private static final String DROP_COLUMNS_ACCESS_DENIED_MESSAGE = "Cannot drop a column from table .*"; private static final String RENAME_COLUMNS_ACCESS_DENIED_MESSAGE = "Cannot rename a column in table .*"; - private static final String AUTH_TABLE_ACCESS_DENIED_MESSAGE = "Cannot set authorization for table .* to .*"; - private static final String AUTH_VIEW_ACCESS_DENIED_MESSAGE = "Cannot set authorization for view .* to .*"; private static final String TABLE_COMMENT_ACCESS_DENIED_MESSAGE = "Cannot comment table to .*"; private static final String INSERT_TABLE_ACCESS_DENIED_MESSAGE = "Cannot insert into table .*"; private static final String DELETE_TABLE_ACCESS_DENIED_MESSAGE = "Cannot delete from table .*"; @@ -182,9 +179,9 @@ public void testEmptyFile() accessControl.checkCanCreateSchema(UNKNOWN, new CatalogSchemaName("some-catalog", "unknown"), ImmutableMap.of()); accessControl.checkCanDropSchema(UNKNOWN, new CatalogSchemaName("some-catalog", "unknown")); accessControl.checkCanRenameSchema(UNKNOWN, new CatalogSchemaName("some-catalog", "unknown"), "new_unknown"); - accessControl.checkCanSetSchemaAuthorization(UNKNOWN, - new CatalogSchemaName("some-catalog", "unknown"), - new TrinoPrincipal(ROLE, "some_role")); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(UNKNOWN, new CatalogSchemaName("some-catalog", "unknown"), new TrinoPrincipal(ROLE, "some_role")), + "Cannot set authorization for schema some-catalog.unknown to ROLE some_role"); accessControl.checkCanShowCreateSchema(UNKNOWN, new CatalogSchemaName("some-catalog", "unknown")); accessControl.checkCanSelectFromColumns(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown"), ImmutableSet.of()); @@ -199,10 +196,20 @@ public void testEmptyFile() accessControl.checkCanRenameTable(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown"), new CatalogSchemaTableName("some-catalog", "unknown", "new_unknown")); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown"), new TrinoPrincipal(ROLE, "some_role")), + "Cannot set authorization for table some-catalog.unknown.unknown to ROLE some_role"); + + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown"), new TrinoPrincipal(ROLE, "some_role")), + "Cannot set authorization for view some-catalog.unknown.unknown to ROLE some_role"); accessControl.checkCanCreateMaterializedView(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown"), Map.of()); accessControl.checkCanDropMaterializedView(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown")); accessControl.checkCanRefreshMaterializedView(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown")); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown"), new TrinoPrincipal(ROLE, "some_role")), + "Cannot set authorization for view some-catalog.unknown.unknown to ROLE some_role"); accessControl.checkCanSetUser(Optional.empty(), "unknown"); accessControl.checkCanSetUser(Optional.of(new KerberosPrincipal("stuff@example.com")), "unknown"); @@ -307,19 +314,6 @@ public void testSchemaRulesForCheckCanRenameSchema() 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 TrinoPrincipal(ROLE, "some_role")); - accessControl.checkCanSetSchemaAuthorization(ADMIN, new CatalogSchemaName("some-catalog", "test"), new TrinoPrincipal(USER, "some_user")); - accessControl.checkCanSetSchemaAuthorization(BOB, new CatalogSchemaName("some-catalog", "bob"), new TrinoPrincipal(ROLE, "some_role")); - accessControl.checkCanSetSchemaAuthorization(BOB, new CatalogSchemaName("some-catalog", "bob"), new TrinoPrincipal(USER, "some_user")); - assertAccessDenied(() -> accessControl.checkCanSetSchemaAuthorization(BOB, new CatalogSchemaName("some-catalog", "test"), new TrinoPrincipal(ROLE, "some_role")), AUTH_SCHEMA_ACCESS_DENIED_MESSAGE); - assertAccessDenied(() -> accessControl.checkCanSetSchemaAuthorization(BOB, new CatalogSchemaName("some-catalog", "test"), new TrinoPrincipal(USER, "some_user")), AUTH_SCHEMA_ACCESS_DENIED_MESSAGE); - } - @Test public void testSchemaRulesForCheckCanShowCreateSchema() { @@ -824,60 +818,6 @@ public void testTableRulesForMixedGroupUsers() new ViewExpression(Optional.empty(), Optional.of("some-catalog"), Optional.of("my_schema"), "country='US'")); } - @Test - public void testCheckCanSetTableAuthorizationForAdmin() - { - SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-table.json"); - - accessControl.checkCanSetTableAuthorization(ADMIN, new CatalogSchemaTableName("some-catalog", "test", "test"), new TrinoPrincipal(ROLE, "some_role")); - accessControl.checkCanSetTableAuthorization(ADMIN, new CatalogSchemaTableName("some-catalog", "test", "test"), new TrinoPrincipal(USER, "some_user")); - } - - @Test - public void testCheckCanSetViewAuthorizationForAdmin() - { - SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-table.json"); - - accessControl.checkCanSetViewAuthorization(ADMIN, new CatalogSchemaTableName("some-catalog", "test", "test"), new TrinoPrincipal(ROLE, "some_role")); - accessControl.checkCanSetViewAuthorization(ADMIN, new CatalogSchemaTableName("some-catalog", "test", "test"), new TrinoPrincipal(USER, "some_user")); - } - - @Test - public void testCheckCanSetTableAuthorizationForOwner() - { - SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-table.json"); - - accessControl.checkCanSetTableAuthorization(ALICE, new CatalogSchemaTableName("some-catalog", "aliceschema", "test"), new TrinoPrincipal(ROLE, "some_role")); - accessControl.checkCanSetTableAuthorization(ALICE, new CatalogSchemaTableName("some-catalog", "aliceschema", "test"), new TrinoPrincipal(USER, "some_user")); - } - - @Test - public void testCheckCanSetViewAuthorizationForOwner() - { - SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-table.json"); - - accessControl.checkCanSetViewAuthorization(ALICE, new CatalogSchemaTableName("some-catalog", "aliceschema", "test"), new TrinoPrincipal(ROLE, "some_role")); - accessControl.checkCanSetViewAuthorization(ALICE, new CatalogSchemaTableName("some-catalog", "aliceschema", "test"), new TrinoPrincipal(USER, "some_user")); - } - - @Test - public void testCheckCanSetTableAuthorizationForNonOwner() - { - SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-table.json"); - - assertAccessDenied(() -> accessControl.checkCanSetTableAuthorization(ALICE, new CatalogSchemaTableName("some-catalog", "test", "test"), new TrinoPrincipal(ROLE, "some_role")), AUTH_TABLE_ACCESS_DENIED_MESSAGE); - assertAccessDenied(() -> accessControl.checkCanSetTableAuthorization(ALICE, new CatalogSchemaTableName("some-catalog", "test", "test"), new TrinoPrincipal(USER, "some_user")), AUTH_TABLE_ACCESS_DENIED_MESSAGE); - } - - @Test - public void testCheckCanSetViewAuthorizationForNonOwner() - { - SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-table.json"); - - assertAccessDenied(() -> accessControl.checkCanSetViewAuthorization(ALICE, new CatalogSchemaTableName("some-catalog", "test", "test"), new TrinoPrincipal(ROLE, "some_role")), AUTH_VIEW_ACCESS_DENIED_MESSAGE); - assertAccessDenied(() -> accessControl.checkCanSetViewAuthorization(ALICE, new CatalogSchemaTableName("some-catalog", "test", "test"), new TrinoPrincipal(USER, "some_user")), AUTH_VIEW_ACCESS_DENIED_MESSAGE); - } - @Test public void testTableRulesForCheckCanSetTableComment() { @@ -1124,37 +1064,6 @@ public void testSystemInformationDocsExample() "Cannot write system information"); } - @Test - public void testSchemaOperations() - { - SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-catalog.json"); - - TrinoPrincipal user = new TrinoPrincipal(USER, "some_user"); - TrinoPrincipal role = new TrinoPrincipal(ROLE, "some_user"); - - accessControl.checkCanSetSchemaAuthorization(ADMIN, new CatalogSchemaName("alice-catalog", "some_schema"), user); - accessControl.checkCanSetSchemaAuthorization(ADMIN, new CatalogSchemaName("alice-catalog", "some_schema"), role); - - accessControl.checkCanSetSchemaAuthorization(ALICE, new CatalogSchemaName("alice-catalog", "some_schema"), user); - accessControl.checkCanSetSchemaAuthorization(ALICE, new CatalogSchemaName("alice-catalog", "some_schema"), role); - - assertAccessDenied( - () -> accessControl.checkCanSetSchemaAuthorization(BOB, new CatalogSchemaName("alice-catalog", "some_schema"), user), - "Cannot set authorization for schema alice-catalog.some_schema.*"); - - assertAccessDenied( - () -> accessControl.checkCanSetSchemaAuthorization(BOB, new CatalogSchemaName("alice-catalog", "some_schema"), role), - "Cannot set authorization for schema alice-catalog.some_schema.*"); - - assertAccessDenied( - () -> accessControl.checkCanSetSchemaAuthorization(ALICE, new CatalogSchemaName("secret", "some_schema"), user), - "Cannot set authorization for schema secret.some_schema.*"); - - assertAccessDenied( - () -> accessControl.checkCanSetSchemaAuthorization(ALICE, new CatalogSchemaName("secret", "some_schema"), role), - "Cannot set authorization for schema secret.some_schema.*"); - } - @Test public void testSessionPropertyRules() { @@ -1490,6 +1399,232 @@ public void testFunctionRulesForCheckCanGrantExecute() accessControl.checkCanGrantExecuteFunctionPrivilege(BOB, "some_function", new TrinoPrincipal(USER, CHARLIE.getIdentity().getUser()), true); } + @Test + public void testSchemaAuthorization() + { + SystemAccessControl accessControl = newFileBasedSystemAccessControl("authorization.json"); + + CatalogSchemaName schema = new CatalogSchemaName("some-catalog", "test"); + CatalogSchemaName ownedByUser = new CatalogSchemaName("some-catalog", "owned_by_user"); + CatalogSchemaName ownedByGroup = new CatalogSchemaName("some-catalog", "owned_by_group"); + CatalogSchemaName ownedByRole = new CatalogSchemaName("some-catalog", "owned_by_role"); + + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("user", "group", "role"), schema, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for schema some-catalog.test to ROLE new_role"); + + // access to schema granted to user + accessControl.checkCanSetSchemaAuthorization(user("owner_authorized", "group", "role"), ownedByUser, new TrinoPrincipal(USER, "new_user")); + accessControl.checkCanSetSchemaAuthorization(user("owner", "authorized", "role"), ownedByUser, new TrinoPrincipal(ROLE, "new_role")); + accessControl.checkCanSetSchemaAuthorization(user("owner", "group", "authorized"), ownedByUser, new TrinoPrincipal(ROLE, "new_role")); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("owner_without_authorization_access", "group", "role"), ownedByUser, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for schema some-catalog.owned_by_user to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("owner_DENY_authorized", "group", "role"), ownedByUser, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for schema some-catalog.owned_by_user to USER new_user"); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("owner", "DENY_authorized", "role"), ownedByUser, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for schema some-catalog.owned_by_user to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("owner", "group", "DENY_authorized"), ownedByUser, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for schema some-catalog.owned_by_user to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("owner", "group", "authorized"), ownedByUser, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for schema some-catalog.owned_by_user to USER new_user"); + + // access to schema granted to group + accessControl.checkCanSetSchemaAuthorization(user("authorized", "owner", "role"), ownedByGroup, new TrinoPrincipal(USER, "new_user")); + accessControl.checkCanSetSchemaAuthorization(user("user", "owner", "authorized"), ownedByGroup, new TrinoPrincipal(ROLE, "new_role")); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("user", "owner", "role"), ownedByGroup, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for schema some-catalog.owned_by_group to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("DENY_authorized", "owner", "role"), ownedByGroup, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for schema some-catalog.owned_by_group to USER new_user"); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("user", "owner", "DENY_authorized"), ownedByGroup, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for schema some-catalog.owned_by_group to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("user", "owner", "authorized"), ownedByGroup, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for schema some-catalog.owned_by_group to USER new_user"); + + // access to schema granted to role + accessControl.checkCanSetSchemaAuthorization(user("authorized", "group", "owner"), ownedByRole, new TrinoPrincipal(USER, "new_user")); + accessControl.checkCanSetSchemaAuthorization(user("user", "group", "owner_authorized"), ownedByRole, new TrinoPrincipal(ROLE, "new_role")); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("user", "group", "owner"), ownedByRole, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for schema some-catalog.owned_by_role to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("DENY_authorized", "group", "owner"), ownedByRole, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for schema some-catalog.owned_by_role to USER new_user"); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("user", "group", "owner_DENY_authorized"), ownedByRole, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for schema some-catalog.owned_by_role to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetSchemaAuthorization(user("user", "group", "owner_authorized"), ownedByRole, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for schema some-catalog.owned_by_role to USER new_user"); + } + + @Test + public void testTableAuthorization() + { + SystemAccessControl accessControl = newFileBasedSystemAccessControl("authorization.json"); + + CatalogSchemaTableName table = new CatalogSchemaTableName("some-catalog", "test", "table"); + CatalogSchemaTableName ownedByUser = new CatalogSchemaTableName("some-catalog", "test", "owned_by_user"); + CatalogSchemaTableName ownedByGroup = new CatalogSchemaTableName("some-catalog", "test", "owned_by_group"); + CatalogSchemaTableName ownedByRole = new CatalogSchemaTableName("some-catalog", "test", "owned_by_role"); + + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("user", "group", "role"), table, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for table some-catalog.test.table to ROLE new_role"); + + // access to table granted to user + accessControl.checkCanSetTableAuthorization(user("owner_authorized", "group", "role"), ownedByUser, new TrinoPrincipal(USER, "new_user")); + accessControl.checkCanSetTableAuthorization(user("owner", "group", "authorized"), ownedByUser, new TrinoPrincipal(ROLE, "new_role")); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("owner_without_authorization_access", "group", "role"), ownedByUser, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for table some-catalog.test.owned_by_user to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("owner_DENY_authorized", "group", "role"), ownedByUser, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for table some-catalog.test.owned_by_user to USER new_user"); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("owner", "group", "DENY_authorized"), ownedByUser, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for table some-catalog.test.owned_by_user to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("owner", "group", "authorized"), ownedByUser, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for table some-catalog.test.owned_by_user to USER new_user"); + + // access to table granted to group + accessControl.checkCanSetTableAuthorization(user("authorized", "owner", "role"), ownedByGroup, new TrinoPrincipal(USER, "new_user")); + accessControl.checkCanSetTableAuthorization(user("user", "owner", "authorized"), ownedByGroup, new TrinoPrincipal(ROLE, "new_role")); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("user", "owner", "role"), ownedByGroup, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for table some-catalog.test.owned_by_group to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("DENY_authorized", "owner", "role"), ownedByGroup, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for table some-catalog.test.owned_by_group to USER new_user"); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("user", "owner", "DENY_authorized"), ownedByGroup, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for table some-catalog.test.owned_by_group to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("user", "owner", "authorized"), ownedByGroup, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for table some-catalog.test.owned_by_group to USER new_user"); + + // access to table granted to role + accessControl.checkCanSetTableAuthorization(user("authorized", "group", "owner"), ownedByRole, new TrinoPrincipal(USER, "new_user")); + accessControl.checkCanSetTableAuthorization(user("user", "group", "owner_authorized"), ownedByRole, new TrinoPrincipal(ROLE, "new_role")); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("user", "group", "owner"), ownedByRole, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for table some-catalog.test.owned_by_role to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("DENY_authorized", "group", "owner"), ownedByRole, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for table some-catalog.test.owned_by_role to USER new_user"); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("user", "group", "owner_DENY_authorized"), ownedByRole, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for table some-catalog.test.owned_by_role to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetTableAuthorization(user("user", "group", "owner_authorized"), ownedByRole, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for table some-catalog.test.owned_by_role to USER new_user"); + } + + @Test + public void testViewAuthorization() + { + SystemAccessControl accessControl = newFileBasedSystemAccessControl("authorization.json"); + + CatalogSchemaTableName table = new CatalogSchemaTableName("some-catalog", "test", "table"); + CatalogSchemaTableName ownedByUser = new CatalogSchemaTableName("some-catalog", "test", "owned_by_user"); + CatalogSchemaTableName ownedByGroup = new CatalogSchemaTableName("some-catalog", "test", "owned_by_group"); + CatalogSchemaTableName ownedByRole = new CatalogSchemaTableName("some-catalog", "test", "owned_by_role"); + + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("user", "group", "role"), table, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for view some-catalog.test.table to ROLE new_role"); + + // access to table granted to user + accessControl.checkCanSetViewAuthorization(user("owner_authorized", "group", "role"), ownedByUser, new TrinoPrincipal(USER, "new_user")); + accessControl.checkCanSetViewAuthorization(user("owner", "group", "authorized"), ownedByUser, new TrinoPrincipal(ROLE, "new_role")); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("owner_without_authorization_access", "group", "role"), ownedByUser, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for view some-catalog.test.owned_by_user to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("owner_DENY_authorized", "group", "role"), ownedByUser, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for view some-catalog.test.owned_by_user to USER new_user"); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("owner", "group", "DENY_authorized"), ownedByUser, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for view some-catalog.test.owned_by_user to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("owner", "group", "authorized"), ownedByUser, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for view some-catalog.test.owned_by_user to USER new_user"); + + // access to table granted to group + accessControl.checkCanSetViewAuthorization(user("authorized", "owner", "role"), ownedByGroup, new TrinoPrincipal(USER, "new_user")); + accessControl.checkCanSetViewAuthorization(user("user", "owner", "authorized"), ownedByGroup, new TrinoPrincipal(ROLE, "new_role")); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("user", "owner", "role"), ownedByGroup, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for view some-catalog.test.owned_by_group to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("DENY_authorized", "owner", "role"), ownedByGroup, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for view some-catalog.test.owned_by_group to USER new_user"); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("user", "owner", "DENY_authorized"), ownedByGroup, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for view some-catalog.test.owned_by_group to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("user", "owner", "authorized"), ownedByGroup, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for view some-catalog.test.owned_by_group to USER new_user"); + + // access to table granted to role + accessControl.checkCanSetViewAuthorization(user("authorized", "group", "owner"), ownedByRole, new TrinoPrincipal(USER, "new_user")); + accessControl.checkCanSetViewAuthorization(user("user", "group", "owner_authorized"), ownedByRole, new TrinoPrincipal(ROLE, "new_role")); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("user", "group", "owner"), ownedByRole, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for view some-catalog.test.owned_by_role to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("DENY_authorized", "group", "owner"), ownedByRole, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for view some-catalog.test.owned_by_role to USER new_user"); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("user", "group", "owner_DENY_authorized"), ownedByRole, new TrinoPrincipal(ROLE, "new_role")), + "Cannot set authorization for view some-catalog.test.owned_by_role to ROLE new_role"); + assertAccessDenied( + () -> accessControl.checkCanSetViewAuthorization(user("user", "group", "owner_authorized"), ownedByRole, new TrinoPrincipal(USER, "new_user")), + "Cannot set authorization for view some-catalog.test.owned_by_role to USER new_user"); + } + + @Test + public void testAuthorizationDocsExample() + { + File rulesFile = new File("../../docs/src/main/sphinx/security/authorization.json"); + SystemAccessControl accessControlManager = newFileBasedSystemAccessControl(rulesFile, ImmutableMap.of()); + CatalogSchemaName schema = new CatalogSchemaName("catalog", "schema"); + CatalogSchemaTableName tableOrView = new CatalogSchemaTableName("catalog", "schema", "table_or_view"); + accessControlManager.checkCanSetSchemaAuthorization(ADMIN, schema, new TrinoPrincipal(USER, "alice")); + accessControlManager.checkCanSetSchemaAuthorization(ADMIN, schema, new TrinoPrincipal(ROLE, "role")); + accessControlManager.checkCanSetTableAuthorization(ADMIN, tableOrView, new TrinoPrincipal(USER, "alice")); + accessControlManager.checkCanSetTableAuthorization(ADMIN, tableOrView, new TrinoPrincipal(ROLE, "role")); + accessControlManager.checkCanSetViewAuthorization(ADMIN, tableOrView, new TrinoPrincipal(USER, "alice")); + accessControlManager.checkCanSetViewAuthorization(ADMIN, tableOrView, new TrinoPrincipal(ROLE, "role")); + assertAccessDenied( + () -> accessControlManager.checkCanSetSchemaAuthorization(ADMIN, schema, new TrinoPrincipal(USER, "bob")), + "Cannot set authorization for schema catalog.schema to USER bob"); + assertAccessDenied( + () -> accessControlManager.checkCanSetTableAuthorization(ADMIN, tableOrView, new TrinoPrincipal(USER, "bob")), + "Cannot set authorization for table catalog.schema.table_or_view to USER bob"); + assertAccessDenied( + () -> accessControlManager.checkCanSetViewAuthorization(ADMIN, tableOrView, new TrinoPrincipal(USER, "bob")), + "Cannot set authorization for view catalog.schema.table_or_view to USER bob"); + } + + private static SystemSecurityContext user(String user, String group, String role) + { + Identity identity = Identity.forUser(user) + .withGroups(ImmutableSet.of(group)) + .withEnabledRoles(ImmutableSet.of(role)) + .build(); + return new SystemSecurityContext(identity, queryId); + } + @Test public void parseUnknownRules() { diff --git a/lib/trino-plugin-toolkit/src/test/resources/authorization-no-roles.json b/lib/trino-plugin-toolkit/src/test/resources/authorization-no-roles.json new file mode 100644 index 000000000000..03b46ce87488 --- /dev/null +++ b/lib/trino-plugin-toolkit/src/test/resources/authorization-no-roles.json @@ -0,0 +1,37 @@ +{ + "authorization": [ + { + "original_user": ".*DENY.*", + "new_user": ".*", + "allow": false + }, + { + "original_user": ".*authorized", + "new_user": ".*" + } + ], + "schemas": [ + { + "user": "owner.*", + "schema": "owned_by_user", + "owner": true + }, + { + "group": "owner.*", + "schema": "owned_by_group", + "owner": true + } + ], + "tables": [ + { + "user": "owner.*", + "table": "owned_by_user", + "privileges": ["OWNERSHIP"] + }, + { + "group": "owner*", + "table": "owned_by_group", + "privileges": ["OWNERSHIP"] + } + ] +} diff --git a/lib/trino-plugin-toolkit/src/test/resources/authorization.json b/lib/trino-plugin-toolkit/src/test/resources/authorization.json new file mode 100644 index 000000000000..4d7bc337201a --- /dev/null +++ b/lib/trino-plugin-toolkit/src/test/resources/authorization.json @@ -0,0 +1,69 @@ +{ + "authorization": [ + { + "original_user": ".*DENY.*", + "new_user": ".*", + "new_role": ".*", + "allow": false + }, + { + "original_group": ".*DENY.*", + "new_user": ".*", + "new_role": ".*", + "allow": false + }, + { + "original_role": ".*DENY.*", + "new_user": ".*", + "new_role": ".*", + "allow": false + }, + { + "original_user": ".*authorized", + "new_user": ".*" + }, + { + "original_group": ".*authorized", + "new_user": ".*", + "new_role": ".*" + }, + { + "original_role": ".*authorized", + "new_role": ".*" + } + ], + "schemas": [ + { + "user": "owner.*", + "schema": "owned_by_user", + "owner": true + }, + { + "group": "owner.*", + "schema": "owned_by_group", + "owner": true + }, + { + "role": "owner.*", + "schema": "owned_by_role", + "owner": true + } + ], + "tables": [ + { + "user": "owner.*", + "table": "owned_by_user", + "privileges": ["OWNERSHIP"] + }, + { + "group": "owner*", + "table": "owned_by_group", + "privileges": ["OWNERSHIP"] + }, + { + "role": "owner.*", + "table": "owned_by_role", + "privileges": ["OWNERSHIP"] + } + ] +} From c04f7c26bb8a0be8f89c61614cf307ac23479cfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Kokosi=C5=84ski?= Date: Mon, 5 Jun 2023 15:50:09 +0200 Subject: [PATCH 3/3] Remove legacy.allow-set-view-authorization This property is no longer needed. It is required that plugin that implements access control make sure that control for SET AUTHORIZATION is implemented securely. --- .../main/java/io/trino/FeaturesConfig.java | 16 +------------- .../execution/SetViewAuthorizationTask.java | 21 ++++--------------- .../sql/analyzer/TestFeaturesConfig.java | 3 --- .../plugin/hive/BaseHiveConnectorTest.java | 1 - .../common/hadoop-kerberos/config.properties | 1 - .../io/trino/security/TestAccessControl.java | 4 +--- 6 files changed, 6 insertions(+), 40 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/FeaturesConfig.java b/core/trino-main/src/main/java/io/trino/FeaturesConfig.java index 616095caa3b5..f7941957bcd9 100644 --- a/core/trino-main/src/main/java/io/trino/FeaturesConfig.java +++ b/core/trino-main/src/main/java/io/trino/FeaturesConfig.java @@ -63,6 +63,7 @@ "experimental.spill-order-by", "spill-window-operator", "experimental.spill-window-operator", + "legacy.allow-set-view-authorization", }) public class FeaturesConfig { @@ -100,7 +101,6 @@ public class FeaturesConfig private boolean legacyCatalogRoles; private boolean incrementalHashArrayLoadFactorEnabled = true; - private boolean allowSetViewAuthorization; private boolean legacyMaterializedViewGracePeriod; private boolean hideInaccessibleColumns; @@ -474,20 +474,6 @@ public FeaturesConfig setHideInaccessibleColumns(boolean hideInaccessibleColumns return this; } - public boolean isAllowSetViewAuthorization() - { - return allowSetViewAuthorization; - } - - @Config("legacy.allow-set-view-authorization") - @ConfigDescription("For security reasons ALTER VIEW SET AUTHORIZATION is disabled for SECURITY DEFINER; " + - "setting this option to true will re-enable this functionality") - public FeaturesConfig setAllowSetViewAuthorization(boolean allowSetViewAuthorization) - { - this.allowSetViewAuthorization = allowSetViewAuthorization; - return this; - } - public boolean isForceSpillingJoin() { return forceSpillingJoin; diff --git a/core/trino-main/src/main/java/io/trino/execution/SetViewAuthorizationTask.java b/core/trino-main/src/main/java/io/trino/execution/SetViewAuthorizationTask.java index 366873732e4b..e98cdb563a8d 100644 --- a/core/trino-main/src/main/java/io/trino/execution/SetViewAuthorizationTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/SetViewAuthorizationTask.java @@ -15,14 +15,11 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.inject.Inject; -import io.trino.FeaturesConfig; import io.trino.Session; import io.trino.execution.warnings.WarningCollector; import io.trino.metadata.Metadata; import io.trino.metadata.QualifiedObjectName; -import io.trino.metadata.ViewDefinition; import io.trino.security.AccessControl; -import io.trino.spi.TrinoException; import io.trino.spi.security.TrinoPrincipal; import io.trino.sql.tree.Expression; import io.trino.sql.tree.SetViewAuthorization; @@ -35,10 +32,8 @@ import static io.trino.metadata.MetadataUtil.createPrincipal; import static io.trino.metadata.MetadataUtil.createQualifiedObjectName; import static io.trino.metadata.MetadataUtil.getRequiredCatalogHandle; -import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; -import static java.lang.String.format; import static java.util.Objects.requireNonNull; public class SetViewAuthorizationTask @@ -46,14 +41,12 @@ public class SetViewAuthorizationTask { private final Metadata metadata; private final AccessControl accessControl; - private final boolean isAllowSetViewAuthorization; @Inject - public SetViewAuthorizationTask(Metadata metadata, AccessControl accessControl, FeaturesConfig featuresConfig) + public SetViewAuthorizationTask(Metadata metadata, AccessControl accessControl) { this.metadata = requireNonNull(metadata, "metadata is null"); this.accessControl = requireNonNull(accessControl, "accessControl is null"); - this.isAllowSetViewAuthorization = featuresConfig.isAllowSetViewAuthorization(); } @Override @@ -72,19 +65,13 @@ public ListenableFuture execute( Session session = stateMachine.getSession(); QualifiedObjectName viewName = createQualifiedObjectName(session, statement, statement.getSource()); getRequiredCatalogHandle(metadata, session, statement, viewName.getCatalogName()); - ViewDefinition view = metadata.getView(session, viewName) - .orElseThrow(() -> semanticException(TABLE_NOT_FOUND, statement, "View '%s' does not exist", viewName)); + if (metadata.getView(session, viewName).isEmpty()) { + throw semanticException(TABLE_NOT_FOUND, statement, "View '%s' does not exist", viewName); + } TrinoPrincipal principal = createPrincipal(statement.getPrincipal()); checkRoleExists(session, statement, metadata, principal, Optional.of(viewName.getCatalogName()).filter(catalog -> metadata.isCatalogManagedSecurity(session, catalog))); - if (!view.isRunAsInvoker() && !isAllowSetViewAuthorization) { - throw new TrinoException( - NOT_SUPPORTED, - format( - "Cannot set authorization for view %s to %s: this feature is disabled", - viewName.getCatalogName() + '.' + viewName.getSchemaName() + '.' + viewName.getObjectName(), principal)); - } accessControl.checkCanSetViewAuthorization(session.toSecurityContext(), viewName, principal); metadata.setViewAuthorization(session, viewName.asCatalogSchemaTableName(), principal); diff --git a/core/trino-main/src/test/java/io/trino/sql/analyzer/TestFeaturesConfig.java b/core/trino-main/src/test/java/io/trino/sql/analyzer/TestFeaturesConfig.java index 4e3321577add..e9edac5aaf0d 100644 --- a/core/trino-main/src/test/java/io/trino/sql/analyzer/TestFeaturesConfig.java +++ b/core/trino-main/src/test/java/io/trino/sql/analyzer/TestFeaturesConfig.java @@ -64,7 +64,6 @@ public void testDefaults() .setIncrementalHashArrayLoadFactorEnabled(true) .setLegacyMaterializedViewGracePeriod(false) .setHideInaccessibleColumns(false) - .setAllowSetViewAuthorization(false) .setForceSpillingJoin(false) .setFaultTolerantExecutionExchangeEncryptionEnabled(true)); } @@ -100,7 +99,6 @@ public void testExplicitPropertyMappings() .put("incremental-hash-array-load-factor.enabled", "false") .put("legacy.materialized-view-grace-period", "true") .put("hide-inaccessible-columns", "true") - .put("legacy.allow-set-view-authorization", "true") .put("force-spilling-join-operator", "true") .put("fault-tolerant-execution.exchange-encryption-enabled", "false") .buildOrThrow(); @@ -133,7 +131,6 @@ public void testExplicitPropertyMappings() .setIncrementalHashArrayLoadFactorEnabled(false) .setLegacyMaterializedViewGracePeriod(true) .setHideInaccessibleColumns(true) - .setAllowSetViewAuthorization(true) .setForceSpillingJoin(true) .setFaultTolerantExecutionExchangeEncryptionEnabled(false); assertFullMapping(properties, expected); diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java index c4816f7c44d7..f8729f5d16ac 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java @@ -220,7 +220,6 @@ protected static QueryRunner createHiveQueryRunner(Map extraProp "hive.writer-sort-buffer-size", "1MB", // Make weighted split scheduling more conservative to avoid OOMs in test "hive.minimum-assigned-split-weight", "0.5")) - .addExtraProperty("legacy.allow-set-view-authorization", "true") .setInitialTables(REQUIRED_TPCH_TABLES) .setTpchBucketedCatalogEnabled(true) .build(); diff --git a/testing/trino-product-tests-launcher/src/main/resources/docker/presto-product-tests/common/hadoop-kerberos/config.properties b/testing/trino-product-tests-launcher/src/main/resources/docker/presto-product-tests/common/hadoop-kerberos/config.properties index 05b292005d11..facbc5966c27 100644 --- a/testing/trino-product-tests-launcher/src/main/resources/docker/presto-product-tests/common/hadoop-kerberos/config.properties +++ b/testing/trino-product-tests-launcher/src/main/resources/docker/presto-product-tests/common/hadoop-kerberos/config.properties @@ -22,4 +22,3 @@ internal-communication.https.required=true internal-communication.shared-secret=internal-shared-secret internal-communication.https.keystore.path=/docker/presto-product-tests/conf/presto/etc/docker.cluster.jks internal-communication.https.keystore.key=123456 -legacy.allow-set-view-authorization=true diff --git a/testing/trino-tests/src/test/java/io/trino/security/TestAccessControl.java b/testing/trino-tests/src/test/java/io/trino/security/TestAccessControl.java index cccdef3f29cb..bfaeacac2991 100644 --- a/testing/trino-tests/src/test/java/io/trino/security/TestAccessControl.java +++ b/testing/trino-tests/src/test/java/io/trino/security/TestAccessControl.java @@ -797,9 +797,7 @@ public void testEmptyRoles() @Test public void testSetViewAuthorizationWithSecurityDefiner() { - assertQueryFails( - "ALTER VIEW mock.default.test_view_definer SET AUTHORIZATION some_other_user", - "Cannot set authorization for view mock.default.test_view_definer to USER some_other_user: this feature is disabled"); + assertQuerySucceeds("ALTER VIEW mock.default.test_view_definer SET AUTHORIZATION some_other_user"); } @Test