From b59d34e209762eea870fbb77a84c42365387f929 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Thu, 3 Feb 2022 17:46:16 +0100 Subject: [PATCH 1/3] Remove redundant String.toString call --- .../src/main/java/io/trino/security/DenyAllAccessControl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java b/core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java index b824608601a3..e695c4ad1ca7 100644 --- a/core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java @@ -506,6 +506,6 @@ public void checkCanExecuteFunction(SecurityContext context, String functionName @Override public void checkCanExecuteTableProcedure(SecurityContext context, QualifiedObjectName tableName, String procedureName) { - denyExecuteTableProcedure(tableName.toString(), procedureName.toString()); + denyExecuteTableProcedure(tableName.toString(), procedureName); } } From db9bcffeb949d86cf01a938d5c898a3a55665655 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Thu, 3 Feb 2022 17:33:06 +0100 Subject: [PATCH 2/3] Authorize table parameters in CTAS Access controls interfaces allow implementor to inspect new table's properties. This is done for CREATE TABLE`, but was not done for `CREATE TABLE AS`. Instead, a deprecated access control method was called. --- .../trino/sql/analyzer/StatementAnalyzer.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java index ee29cc8dbe3c..513bb539cd8d 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java @@ -804,7 +804,17 @@ protected Scope visitCreateTableAsSelect(CreateTableAsSelect node, Optional properties = tablePropertyManager.getProperties( + catalogName, + node.getProperties(), + session, + plannerContext, + accessControl, + analysis.getParameters(), + true); + // TODO respect FeaturesConfig.isDisableSetPropertiesSecurityCheckForCreateDdl + accessControl.checkCanCreateTable(session.toSecurityContext(), targetTable, properties); // analyze the query that creates the table Scope queryScope = analyze(node.getQuery(), createScope(scope)); @@ -838,16 +848,6 @@ protected Scope visitCreateTableAsSelect(CreateTableAsSelect node, Optional properties = tablePropertyManager.getProperties( - catalogName, - node.getProperties(), - session, - plannerContext, - accessControl, - analysis.getParameters(), - true); ConnectorTableMetadata tableMetadata = new ConnectorTableMetadata(targetTable.asSchemaTableName(), columns.build(), properties, node.getComment()); // analyze target table layout From d0399ada81deee82493e51d7b6c1eba2e7cce47c Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Thu, 3 Feb 2022 17:34:04 +0100 Subject: [PATCH 3/3] Remove create access checks without properties Remove `ConnectorAccessControl` and `SystemAccessControl`'s `checkCanCreateTable` and `checkCanCreateMaterializedView` checks that do not take properties that were deprecated some time ago. Remove associated fallback configuration toggle. Among other things, this forces plugin implementors to implement the correct method. This is important, because the old method did not delegate to the new, nor vice versa. --- .../main/java/io/trino/FeaturesConfig.java | 14 +-------- .../execution/CreateMaterializedViewTask.java | 9 +----- .../io/trino/execution/CreateTableTask.java | 13 ++------ .../java/io/trino/security/AccessControl.java | 18 ----------- .../trino/security/AccessControlManager.java | 26 ---------------- .../trino/security/AllowAllAccessControl.java | 10 ------ .../trino/security/DenyAllAccessControl.java | 12 ------- .../security/ForwardingAccessControl.java | 12 ------- .../InjectedConnectorAccessControl.java | 14 --------- .../trino/sql/analyzer/StatementAnalyzer.java | 1 - .../testing/AllowAllAccessControlManager.java | 6 ---- .../testing/TestingAccessControlManager.java | 22 ------------- .../trino/execution/TestCreateTableTask.java | 19 ++++++------ .../TestFileBasedSystemAccessControl.java | 31 ++++++++++--------- .../sql/analyzer/TestFeaturesConfig.java | 3 -- .../spi/connector/ConnectorAccessControl.java | 24 -------------- .../spi/security/SystemAccessControl.java | 24 -------------- ...ClassLoaderSafeConnectorAccessControl.java | 16 ---------- .../base/security/AllowAllAccessControl.java | 10 ------ .../security/AllowAllSystemAccessControl.java | 10 ------ .../base/security/FileBasedAccessControl.java | 18 ----------- .../FileBasedSystemAccessControl.java | 18 ----------- .../ForwardingConnectorAccessControl.java | 12 ------- .../ForwardingSystemAccessControl.java | 12 ------- .../base/security/ReadOnlyAccessControl.java | 12 ------- .../security/TestFileBasedAccessControl.java | 19 ++++++------ .../TestFileBasedSystemAccessControl.java | 9 +++--- .../hive/security/LegacyAccessControl.java | 10 ------ .../security/SqlStandardAccessControl.java | 16 ---------- 29 files changed, 44 insertions(+), 376 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 465b049fc3d5..2e89a7c55959 100644 --- a/core/trino-main/src/main/java/io/trino/FeaturesConfig.java +++ b/core/trino-main/src/main/java/io/trino/FeaturesConfig.java @@ -44,6 +44,7 @@ @DefunctConfig({ "analyzer.experimental-syntax-enabled", "arrayagg.implementation", + "deprecated.disable-set-properties-security-check-for-create-ddl", "deprecated.group-by-uses-equal", "deprecated.legacy-char-to-varchar-coercion", "deprecated.legacy-join-using", @@ -144,7 +145,6 @@ public class FeaturesConfig private int maxGroupingSets = 2048; private boolean legacyCatalogRoles; - private boolean disableSetPropertiesSecurityCheckForCreateDdl; private boolean incrementalHashArrayLoadFactorEnabled = true; private boolean allowSetViewAuthorization; @@ -1061,18 +1061,6 @@ public FeaturesConfig setLegacyCatalogRoles(boolean legacyCatalogRoles) return this; } - public boolean isDisableSetPropertiesSecurityCheckForCreateDdl() - { - return disableSetPropertiesSecurityCheckForCreateDdl; - } - - @Config("deprecated.disable-set-properties-security-check-for-create-ddl") - public FeaturesConfig setDisableSetPropertiesSecurityCheckForCreateDdl(boolean disableSetPropertiesSecurityCheckForCreateDdl) - { - this.disableSetPropertiesSecurityCheckForCreateDdl = disableSetPropertiesSecurityCheckForCreateDdl; - return this; - } - @Deprecated public boolean isIncrementalHashArrayLoadFactorEnabled() { diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java index 47b59dd50775..265eb86ed4e4 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java @@ -54,7 +54,6 @@ public class CreateMaterializedViewTask private final SqlParser sqlParser; private final AnalyzerFactory analyzerFactory; private final MaterializedViewPropertyManager materializedViewPropertyManager; - private final boolean disableSetPropertiesSecurityCheckForCreateDdl; @Inject public CreateMaterializedViewTask( @@ -70,7 +69,6 @@ public CreateMaterializedViewTask( this.sqlParser = requireNonNull(sqlParser, "sqlParser is null"); this.analyzerFactory = requireNonNull(analyzerFactory, "analyzerFactory is null"); this.materializedViewPropertyManager = requireNonNull(materializedViewPropertyManager, "materializedViewPropertyManager is null"); - this.disableSetPropertiesSecurityCheckForCreateDdl = featuresConfig.isDisableSetPropertiesSecurityCheckForCreateDdl(); } @Override @@ -121,12 +119,7 @@ public ListenableFuture execute( Optional.empty(), properties); - if (!disableSetPropertiesSecurityCheckForCreateDdl) { - accessControl.checkCanCreateMaterializedView(session.toSecurityContext(), name, properties); - } - else { - accessControl.checkCanCreateMaterializedView(session.toSecurityContext(), name); - } + accessControl.checkCanCreateMaterializedView(session.toSecurityContext(), name, properties); plannerContext.getMetadata().createMaterializedView(session, name, definition, statement.isReplace(), statement.isNotExists()); stateMachine.setOutput(analysis.getTarget()); diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java index 73fb10c14af2..955b1cff695c 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java @@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.ListenableFuture; -import io.trino.FeaturesConfig; import io.trino.Session; import io.trino.connector.CatalogName; import io.trino.execution.warnings.WarningCollector; @@ -89,21 +88,18 @@ public class CreateTableTask private final AccessControl accessControl; private final ColumnPropertyManager columnPropertyManager; private final TablePropertyManager tablePropertyManager; - private final boolean disableSetPropertiesSecurityCheckForCreateDdl; @Inject public CreateTableTask( PlannerContext plannerContext, AccessControl accessControl, ColumnPropertyManager columnPropertyManager, - TablePropertyManager tablePropertyManager, - FeaturesConfig featuresConfig) + TablePropertyManager tablePropertyManager) { this.plannerContext = requireNonNull(plannerContext, "plannerContext is null"); this.accessControl = requireNonNull(accessControl, "accessControl is null"); this.columnPropertyManager = requireNonNull(columnPropertyManager, "columnPropertyManager is null"); this.tablePropertyManager = requireNonNull(tablePropertyManager, "tablePropertyManager is null"); - this.disableSetPropertiesSecurityCheckForCreateDdl = featuresConfig.isDisableSetPropertiesSecurityCheckForCreateDdl(); } @Override @@ -252,12 +248,7 @@ else if (element instanceof LikeClause) { parameterLookup, true); - if (!disableSetPropertiesSecurityCheckForCreateDdl) { - accessControl.checkCanCreateTable(session.toSecurityContext(), tableName, properties); - } - else { - accessControl.checkCanCreateTable(session.toSecurityContext(), tableName); - } + accessControl.checkCanCreateTable(session.toSecurityContext(), tableName, properties); Set specifiedPropertyKeys = statement.getProperties().stream() .map(property -> property.getName().getValue()) diff --git a/core/trino-main/src/main/java/io/trino/security/AccessControl.java b/core/trino-main/src/main/java/io/trino/security/AccessControl.java index c7ebe8738a89..9e74179a58ce 100644 --- a/core/trino-main/src/main/java/io/trino/security/AccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/AccessControl.java @@ -161,15 +161,6 @@ public interface AccessControl */ void checkCanShowCreateTable(SecurityContext context, QualifiedObjectName tableName); - /** - * Check if identity is allowed to create the specified table. - * - * @throws AccessDeniedException if not allowed - * @deprecated use {@link #checkCanCreateTable(SecurityContext context, QualifiedObjectName tableName, Map properties)} - */ - @Deprecated - void checkCanCreateTable(SecurityContext context, QualifiedObjectName tableName); - /** * Check if identity is allowed to create the specified table with properties. * @@ -338,15 +329,6 @@ default void checkCanSetViewAuthorization(SecurityContext context, QualifiedObje */ void checkCanCreateViewWithSelectFromColumns(SecurityContext context, QualifiedObjectName tableName, Set columnNames); - /** - * Check if identity is allowed to create the specified materialized view. - * - * @throws AccessDeniedException if not allowed - * @deprecated use {@link #checkCanCreateMaterializedView(SecurityContext, QualifiedObjectName, Map) instead} - */ - @Deprecated - void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName); - /** * Check if identity is allowed to create the specified materialized view. * diff --git a/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java b/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java index 45728968bd32..9e14f0f8d3b6 100644 --- a/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java +++ b/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java @@ -414,19 +414,6 @@ public void checkCanShowCreateTable(SecurityContext securityContext, QualifiedOb catalogAuthorizationCheck(tableName.getCatalogName(), securityContext, (control, context) -> control.checkCanShowCreateTable(context, tableName.asSchemaTableName())); } - @Override - public void checkCanCreateTable(SecurityContext securityContext, QualifiedObjectName tableName) - { - requireNonNull(securityContext, "securityContext is null"); - requireNonNull(tableName, "tableName is null"); - - checkCanAccessCatalog(securityContext, tableName.getCatalogName()); - - systemAuthorizationCheck(control -> control.checkCanCreateTable(securityContext.toSystemSecurityContext(), tableName.asCatalogSchemaTableName())); - - catalogAuthorizationCheck(tableName.getCatalogName(), securityContext, (control, context) -> control.checkCanCreateTable(context, tableName.asSchemaTableName())); - } - @Override public void checkCanCreateTable(SecurityContext securityContext, QualifiedObjectName tableName, Map properties) { @@ -754,19 +741,6 @@ public void checkCanCreateViewWithSelectFromColumns(SecurityContext securityCont catalogAuthorizationCheck(tableName.getCatalogName(), securityContext, (control, context) -> control.checkCanCreateViewWithSelectFromColumns(context, tableName.asSchemaTableName(), columnNames)); } - @Override - public void checkCanCreateMaterializedView(SecurityContext securityContext, QualifiedObjectName materializedViewName) - { - requireNonNull(securityContext, "securityContext is null"); - requireNonNull(materializedViewName, "materializedViewName is null"); - - checkCanAccessCatalog(securityContext, materializedViewName.getCatalogName()); - - systemAuthorizationCheck(control -> control.checkCanCreateMaterializedView(securityContext.toSystemSecurityContext(), materializedViewName.asCatalogSchemaTableName())); - - catalogAuthorizationCheck(materializedViewName.getCatalogName(), securityContext, (control, context) -> control.checkCanCreateMaterializedView(context, materializedViewName.asSchemaTableName())); - } - @Override public void checkCanCreateMaterializedView(SecurityContext securityContext, QualifiedObjectName materializedViewName, Map properties) { diff --git a/core/trino-main/src/main/java/io/trino/security/AllowAllAccessControl.java b/core/trino-main/src/main/java/io/trino/security/AllowAllAccessControl.java index 06c659b4b30f..e4f213782b6e 100644 --- a/core/trino-main/src/main/java/io/trino/security/AllowAllAccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/AllowAllAccessControl.java @@ -118,11 +118,6 @@ public void checkCanShowCreateTable(SecurityContext context, QualifiedObjectName { } - @Override - public void checkCanCreateTable(SecurityContext context, QualifiedObjectName tableName) - { - } - @Override public void checkCanCreateTable(SecurityContext context, QualifiedObjectName tableName, Map properties) { @@ -240,11 +235,6 @@ public void checkCanCreateViewWithSelectFromColumns(SecurityContext context, Qua { } - @Override - public void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName) - { - } - @Override public void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName, Map properties) { diff --git a/core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java b/core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java index e695c4ad1ca7..857f7d99fa70 100644 --- a/core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java @@ -185,12 +185,6 @@ public void checkCanSetSchemaAuthorization(SecurityContext context, CatalogSchem denySetSchemaAuthorization(schemaName.toString(), principal); } - @Override - public void checkCanCreateTable(SecurityContext context, QualifiedObjectName tableName) - { - denyCreateTable(tableName.toString()); - } - @Override public void checkCanCreateTable(SecurityContext context, QualifiedObjectName tableName, Map properties) { @@ -341,12 +335,6 @@ public void checkCanCreateViewWithSelectFromColumns(SecurityContext context, Qua denyCreateViewWithSelect(tableName.toString(), context.getIdentity()); } - @Override - public void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName) - { - denyCreateMaterializedView(materializedViewName.toString()); - } - @Override public void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName, Map properties) { diff --git a/core/trino-main/src/main/java/io/trino/security/ForwardingAccessControl.java b/core/trino-main/src/main/java/io/trino/security/ForwardingAccessControl.java index 5a6451e9dca7..4720ad6d305e 100644 --- a/core/trino-main/src/main/java/io/trino/security/ForwardingAccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/ForwardingAccessControl.java @@ -154,12 +154,6 @@ public void checkCanShowCreateTable(SecurityContext context, QualifiedObjectName delegate().checkCanShowCreateTable(context, tableName); } - @Override - public void checkCanCreateTable(SecurityContext context, QualifiedObjectName tableName) - { - delegate().checkCanCreateTable(context, tableName); - } - @Override public void checkCanCreateTable(SecurityContext context, QualifiedObjectName tableName, Map properties) { @@ -298,12 +292,6 @@ public void checkCanCreateViewWithSelectFromColumns(SecurityContext context, Qua delegate().checkCanCreateViewWithSelectFromColumns(context, tableName, columnNames); } - @Override - public void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName) - { - delegate().checkCanCreateMaterializedView(context, materializedViewName); - } - @Override public void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName, Map properties) { diff --git a/core/trino-main/src/main/java/io/trino/security/InjectedConnectorAccessControl.java b/core/trino-main/src/main/java/io/trino/security/InjectedConnectorAccessControl.java index 0335b2524777..9ef20a4cd92f 100644 --- a/core/trino-main/src/main/java/io/trino/security/InjectedConnectorAccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/InjectedConnectorAccessControl.java @@ -104,13 +104,6 @@ public void checkCanShowCreateTable(ConnectorSecurityContext context, SchemaTabl accessControl.checkCanShowCreateTable(securityContext, getQualifiedObjectName(tableName)); } - @Override - public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName) - { - checkArgument(context == null, "context must be null"); - accessControl.checkCanCreateTable(securityContext, getQualifiedObjectName(tableName)); - } - @Override public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName, Map properties) { @@ -279,13 +272,6 @@ public void checkCanCreateViewWithSelectFromColumns(ConnectorSecurityContext con accessControl.checkCanCreateViewWithSelectFromColumns(securityContext, getQualifiedObjectName(tableName), columnNames); } - @Override - public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName) - { - checkArgument(context == null, "context must be null"); - accessControl.checkCanCreateMaterializedView(securityContext, getQualifiedObjectName(materializedViewName)); - } - @Override public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName, Map properties) { diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java index 513bb539cd8d..b6acee946d62 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java @@ -813,7 +813,6 @@ protected Scope visitCreateTableAsSelect(CreateTableAsSelect node, Optional properties) {} @@ -167,9 +164,6 @@ public void checkCanDropView(SecurityContext context, QualifiedObjectName viewNa @Override public void checkCanCreateViewWithSelectFromColumns(SecurityContext context, QualifiedObjectName tableName, Set columnNames) {} - @Override - public void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName) {} - @Override public void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName, Map properties) {} diff --git a/core/trino-main/src/main/java/io/trino/testing/TestingAccessControlManager.java b/core/trino-main/src/main/java/io/trino/testing/TestingAccessControlManager.java index 0e3b8fa96049..73ddb30b7801 100644 --- a/core/trino-main/src/main/java/io/trino/testing/TestingAccessControlManager.java +++ b/core/trino-main/src/main/java/io/trino/testing/TestingAccessControlManager.java @@ -334,17 +334,6 @@ public void checkCanShowCreateTable(SecurityContext context, QualifiedObjectName } } - @Override - public void checkCanCreateTable(SecurityContext context, QualifiedObjectName tableName) - { - if (shouldDenyPrivilege(context.getIdentity().getUser(), tableName.getObjectName(), CREATE_TABLE)) { - denyCreateTable(tableName.toString()); - } - if (denyPrivileges.isEmpty()) { - super.checkCanCreateTable(context, tableName); - } - } - @Override public void checkCanCreateTable(SecurityContext context, QualifiedObjectName tableName, Map properties) { @@ -543,17 +532,6 @@ public void checkCanCreateViewWithSelectFromColumns(SecurityContext context, Qua } } - @Override - public void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName) - { - if (shouldDenyPrivilege(context.getIdentity().getUser(), materializedViewName.getObjectName(), CREATE_MATERIALIZED_VIEW)) { - denyCreateMaterializedView(materializedViewName.toString()); - } - if (denyPrivileges.isEmpty()) { - super.checkCanCreateMaterializedView(context, materializedViewName); - } - } - @Override public void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName, Map properties) { diff --git a/core/trino-main/src/test/java/io/trino/execution/TestCreateTableTask.java b/core/trino-main/src/test/java/io/trino/execution/TestCreateTableTask.java index 0d42724d72ef..8886980f8032 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestCreateTableTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestCreateTableTask.java @@ -16,7 +16,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import io.trino.FeaturesConfig; import io.trino.Session; import io.trino.connector.CatalogName; import io.trino.connector.MockConnectorFactory; @@ -147,7 +146,7 @@ public void testCreateTableNotExistsTrue() ImmutableList.of(), Optional.empty()); - CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager, new FeaturesConfig()); + CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager); getFutureValue(createTableTask.internalExecute(statement, testSession, emptyList(), output -> {})); assertEquals(metadata.getCreateTableCallCount(), 1); } @@ -161,7 +160,7 @@ public void testCreateTableNotExistsFalse() ImmutableList.of(), Optional.empty()); - CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager, new FeaturesConfig()); + CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager); assertTrinoExceptionThrownBy(() -> getFutureValue(createTableTask.internalExecute(statement, testSession, emptyList(), output -> {}))) .hasErrorCode(ALREADY_EXISTS) .hasMessage("Table already exists"); @@ -178,7 +177,7 @@ public void testCreateTableWithMaterializedViewPropertyFails() ImmutableList.of(new Property(new Identifier("foo"), new StringLiteral("bar"))), Optional.empty()); - CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager, new FeaturesConfig()); + CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager); assertTrinoExceptionThrownBy(() -> getFutureValue(createTableTask.internalExecute(statement, testSession, emptyList(), output -> {}))) .hasErrorCode(INVALID_TABLE_PROPERTY) .hasMessage("Catalog 'catalog' table property 'foo' does not exist"); @@ -196,7 +195,7 @@ public void testCreateWithNotNullColumns() new ColumnDefinition(identifier("c"), toSqlType(VARBINARY), false, emptyList(), Optional.empty())); CreateTable statement = new CreateTable(QualifiedName.of("test_table"), inputColumns, true, ImmutableList.of(), Optional.empty()); - CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager, new FeaturesConfig()); + CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager); getFutureValue(createTableTask.internalExecute(statement, testSession, emptyList(), output -> {})); assertEquals(metadata.getCreateTableCallCount(), 1); List columns = metadata.getReceivedTableMetadata().get(0).getColumns(); @@ -229,7 +228,7 @@ public void testCreateWithUnsupportedConnectorThrowsWhenNotNull() ImmutableList.of(), Optional.empty()); - CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager, new FeaturesConfig()); + CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager); assertTrinoExceptionThrownBy(() -> getFutureValue(createTableTask.internalExecute(statement, testSession, emptyList(), output -> {}))) .hasErrorCode(NOT_SUPPORTED) @@ -241,7 +240,7 @@ public void testCreateLike() { CreateTable statement = getCreatleLikeStatement(false); - CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager, new FeaturesConfig()); + CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager); getFutureValue(createTableTask.internalExecute(statement, testSession, List.of(), output -> {})); assertEquals(metadata.getCreateTableCallCount(), 1); @@ -255,7 +254,7 @@ public void testCreateLikeWithProperties() { CreateTable statement = getCreatleLikeStatement(true); - CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager, new FeaturesConfig()); + CreateTableTask createTableTask = new CreateTableTask(plannerContext, new AllowAllAccessControl(), columnPropertyManager, tablePropertyManager); getFutureValue(createTableTask.internalExecute(statement, testSession, List.of(), output -> {})); assertEquals(metadata.getCreateTableCallCount(), 1); @@ -273,7 +272,7 @@ public void testCreateLikeDenyPermission() TestingAccessControlManager accessControl = new TestingAccessControlManager(transactionManager, new EventListenerManager(new EventListenerConfig())); accessControl.deny(privilege("parent_table", SELECT_COLUMN)); - CreateTableTask createTableTask = new CreateTableTask(plannerContext, accessControl, columnPropertyManager, tablePropertyManager, new FeaturesConfig()); + CreateTableTask createTableTask = new CreateTableTask(plannerContext, accessControl, columnPropertyManager, tablePropertyManager); assertThatThrownBy(() -> getFutureValue(createTableTask.internalExecute(statement, testSession, List.of(), output -> {}))) .isInstanceOf(AccessDeniedException.class) .hasMessageContaining("Cannot reference columns of table"); @@ -287,7 +286,7 @@ public void testCreateLikeWithPropertiesDenyPermission() TestingAccessControlManager accessControl = new TestingAccessControlManager(transactionManager, new EventListenerManager(new EventListenerConfig())); accessControl.deny(privilege("parent_table", SHOW_CREATE_TABLE)); - CreateTableTask createTableTask = new CreateTableTask(plannerContext, accessControl, columnPropertyManager, tablePropertyManager, new FeaturesConfig()); + CreateTableTask createTableTask = new CreateTableTask(plannerContext, accessControl, columnPropertyManager, tablePropertyManager); assertThatThrownBy(() -> getFutureValue(createTableTask.internalExecute(statement, testSession, List.of(), output -> {}))) .isInstanceOf(AccessDeniedException.class) .hasMessageContaining("Cannot reference properties of table"); diff --git a/core/trino-main/src/test/java/io/trino/security/TestFileBasedSystemAccessControl.java b/core/trino-main/src/test/java/io/trino/security/TestFileBasedSystemAccessControl.java index 0504df58e110..7915ebe9da62 100644 --- a/core/trino-main/src/test/java/io/trino/security/TestFileBasedSystemAccessControl.java +++ b/core/trino-main/src/test/java/io/trino/security/TestFileBasedSystemAccessControl.java @@ -31,6 +31,7 @@ import java.io.File; import java.net.URISyntaxException; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -323,7 +324,7 @@ public void testTableOperations() assertEquals(accessControlManager.filterTables(nonAsciiContext, "alice-catalog", aliceTables), ImmutableSet.of()); assertEquals(accessControlManager.filterTables(nonAsciiContext, "staff-catalog", aliceTables), ImmutableSet.of()); - accessControlManager.checkCanCreateTable(aliceContext, aliceTable); + accessControlManager.checkCanCreateTable(aliceContext, aliceTable, Map.of()); accessControlManager.checkCanDropTable(aliceContext, aliceTable); accessControlManager.checkCanTruncateTable(aliceContext, aliceTable); accessControlManager.checkCanSelectFromColumns(aliceContext, aliceTable, ImmutableSet.of()); @@ -334,7 +335,7 @@ public void testTableOperations() accessControlManager.checkCanAddColumns(aliceContext, aliceTable); accessControlManager.checkCanRenameColumn(aliceContext, aliceTable); - accessControlManager.checkCanCreateTable(aliceContext, staffTable); + accessControlManager.checkCanCreateTable(aliceContext, staffTable, Map.of()); accessControlManager.checkCanDropTable(aliceContext, staffTable); accessControlManager.checkCanTruncateTable(aliceContext, staffTable); accessControlManager.checkCanSelectFromColumns(aliceContext, staffTable, ImmutableSet.of()); @@ -345,7 +346,7 @@ public void testTableOperations() accessControlManager.checkCanAddColumns(aliceContext, staffTable); accessControlManager.checkCanRenameColumn(aliceContext, staffTable); - assertThatThrownBy(() -> accessControlManager.checkCanCreateTable(bobContext, aliceTable)) + assertThatThrownBy(() -> accessControlManager.checkCanCreateTable(bobContext, aliceTable, Map.of())) .isInstanceOf(AccessDeniedException.class) .hasMessage("Access Denied: Cannot access catalog alice-catalog"); assertThatThrownBy(() -> accessControlManager.checkCanDropTable(bobContext, aliceTable)) @@ -376,7 +377,7 @@ public void testTableOperations() .isInstanceOf(AccessDeniedException.class) .hasMessage("Access Denied: Cannot access catalog alice-catalog"); - accessControlManager.checkCanCreateTable(bobContext, staffTable); + accessControlManager.checkCanCreateTable(bobContext, staffTable, Map.of()); accessControlManager.checkCanDropTable(bobContext, staffTable); accessControlManager.checkCanTruncateTable(bobContext, staffTable); accessControlManager.checkCanSelectFromColumns(bobContext, staffTable, ImmutableSet.of()); @@ -387,7 +388,7 @@ public void testTableOperations() accessControlManager.checkCanAddColumns(bobContext, staffTable); accessControlManager.checkCanRenameColumn(bobContext, staffTable); - assertThatThrownBy(() -> accessControlManager.checkCanCreateTable(nonAsciiContext, aliceTable)) + assertThatThrownBy(() -> accessControlManager.checkCanCreateTable(nonAsciiContext, aliceTable, Map.of())) .isInstanceOf(AccessDeniedException.class) .hasMessage("Access Denied: Cannot access catalog alice-catalog"); assertThatThrownBy(() -> accessControlManager.checkCanDropTable(nonAsciiContext, aliceTable)) @@ -418,7 +419,7 @@ public void testTableOperations() .isInstanceOf(AccessDeniedException.class) .hasMessage("Access Denied: Cannot access catalog alice-catalog"); - assertThatThrownBy(() -> accessControlManager.checkCanCreateTable(nonAsciiContext, staffTable)) + assertThatThrownBy(() -> accessControlManager.checkCanCreateTable(nonAsciiContext, staffTable, Map.of())) .isInstanceOf(AccessDeniedException.class) .hasMessage("Access Denied: Cannot access catalog staff-catalog"); assertThatThrownBy(() -> accessControlManager.checkCanDropTable(nonAsciiContext, staffTable)) @@ -467,7 +468,7 @@ public void testTableOperationsReadOnly() }); assertThatThrownBy(() -> transaction(transactionManager, accessControlManager).execute(transactionId -> { - accessControlManager.checkCanCreateTable(new SecurityContext(transactionId, alice, queryId), aliceTable); + accessControlManager.checkCanCreateTable(new SecurityContext(transactionId, alice, queryId), aliceTable, Map.of()); })).isInstanceOf(AccessDeniedException.class) .hasMessage("Access Denied: Cannot create table alice-catalog.schema.table"); @@ -507,7 +508,7 @@ public void testTableOperationsReadOnly() .hasMessage("Access Denied: Cannot rename a column in table alice-catalog.schema.table"); assertThatThrownBy(() -> transaction(transactionManager, accessControlManager).execute(transactionId -> { - accessControlManager.checkCanCreateTable(new SecurityContext(transactionId, bob, queryId), aliceTable); + accessControlManager.checkCanCreateTable(new SecurityContext(transactionId, bob, queryId), aliceTable, Map.of()); })).isInstanceOf(AccessDeniedException.class) .hasMessage("Access Denied: Cannot access catalog alice-catalog"); } @@ -680,18 +681,18 @@ public void testMaterializedViewAccess() SecurityContext nonAsciiContext = new SecurityContext(transactionId, nonAsciiUser, queryId); // User alice is allowed access to alice-catalog - accessControlManager.checkCanCreateMaterializedView(aliceContext, aliceMaterializedView); + accessControlManager.checkCanCreateMaterializedView(aliceContext, aliceMaterializedView, Map.of()); accessControlManager.checkCanDropMaterializedView(aliceContext, aliceMaterializedView); accessControlManager.checkCanRefreshMaterializedView(aliceContext, aliceMaterializedView); accessControlManager.checkCanSetMaterializedViewProperties(aliceContext, aliceMaterializedView, ImmutableMap.of()); // User alice is part of staff group which is allowed access to staff-catalog - accessControlManager.checkCanCreateMaterializedView(aliceContext, staffMaterializedView); + accessControlManager.checkCanCreateMaterializedView(aliceContext, staffMaterializedView, Map.of()); accessControlManager.checkCanDropMaterializedView(aliceContext, staffMaterializedView); accessControlManager.checkCanRefreshMaterializedView(aliceContext, staffMaterializedView); accessControlManager.checkCanSetMaterializedViewProperties(aliceContext, staffMaterializedView, ImmutableMap.of()); - assertThatThrownBy(() -> accessControlManager.checkCanCreateMaterializedView(bobContext, aliceMaterializedView)) + assertThatThrownBy(() -> accessControlManager.checkCanCreateMaterializedView(bobContext, aliceMaterializedView, Map.of())) .isInstanceOf(AccessDeniedException.class) .hasMessage("Access Denied: Cannot access catalog alice-catalog"); assertThatThrownBy(() -> accessControlManager.checkCanDropMaterializedView(bobContext, aliceMaterializedView)) @@ -705,12 +706,12 @@ public void testMaterializedViewAccess() .hasMessage("Access Denied: Cannot access catalog alice-catalog"); // User bob is part of staff group which is allowed access to staff-catalog - accessControlManager.checkCanCreateMaterializedView(bobContext, staffMaterializedView); + accessControlManager.checkCanCreateMaterializedView(bobContext, staffMaterializedView, Map.of()); accessControlManager.checkCanDropMaterializedView(bobContext, staffMaterializedView); accessControlManager.checkCanRefreshMaterializedView(bobContext, staffMaterializedView); accessControlManager.checkCanSetMaterializedViewProperties(bobContext, staffMaterializedView, ImmutableMap.of()); - assertThatThrownBy(() -> accessControlManager.checkCanCreateMaterializedView(nonAsciiContext, aliceMaterializedView)) + assertThatThrownBy(() -> accessControlManager.checkCanCreateMaterializedView(nonAsciiContext, aliceMaterializedView, Map.of())) .isInstanceOf(AccessDeniedException.class) .hasMessage("Access Denied: Cannot access catalog alice-catalog"); assertThatThrownBy(() -> accessControlManager.checkCanDropMaterializedView(nonAsciiContext, aliceMaterializedView)) @@ -739,7 +740,7 @@ public void testReadOnlyMaterializedViewAccess() }); assertThatThrownBy(() -> transaction(transactionManager, accessControlManager).execute(transactionId -> { - accessControlManager.checkCanCreateMaterializedView(new SecurityContext(transactionId, alice, queryId), aliceMaterializedView); + accessControlManager.checkCanCreateMaterializedView(new SecurityContext(transactionId, alice, queryId), aliceMaterializedView, Map.of()); })).isInstanceOf(AccessDeniedException.class) .hasMessage("Access Denied: Cannot create materialized view alice-catalog.schema.materialized-view"); @@ -759,7 +760,7 @@ public void testReadOnlyMaterializedViewAccess() .hasMessage("Access Denied: Cannot set properties of materialized view alice-catalog.schema.materialized-view"); assertThatThrownBy(() -> transaction(transactionManager, accessControlManager).execute(transactionId -> { - accessControlManager.checkCanCreateMaterializedView(new SecurityContext(transactionId, bob, queryId), aliceMaterializedView); + accessControlManager.checkCanCreateMaterializedView(new SecurityContext(transactionId, bob, queryId), aliceMaterializedView, Map.of()); })).isInstanceOf(AccessDeniedException.class) .hasMessage("Access Denied: Cannot access catalog alice-catalog"); 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 4f01f1f6583a..aac3dde2a068 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 @@ -112,7 +112,6 @@ public void testDefaults() .setTableScanNodePartitioningMinBucketToTaskRatio(0.5) .setMergeProjectWithValues(true) .setLegacyCatalogRoles(false) - .setDisableSetPropertiesSecurityCheckForCreateDdl(false) .setIncrementalHashArrayLoadFactorEnabled(true) .setHideInaccesibleColumns(false) .setAllowSetViewAuthorization(false)); @@ -191,7 +190,6 @@ public void testExplicitPropertyMappings() .put("optimizer.table-scan-node-partitioning-min-bucket-to-task-ratio", "0.0") .put("optimizer.merge-project-with-values", "false") .put("deprecated.legacy-catalog-roles", "true") - .put("deprecated.disable-set-properties-security-check-for-create-ddl", "true") .put("incremental-hash-array-load-factor.enabled", "false") .put("hide-inaccessible-columns", "true") .put("legacy.allow-set-view-authorization", "true") @@ -267,7 +265,6 @@ public void testExplicitPropertyMappings() .setTableScanNodePartitioningMinBucketToTaskRatio(0.0) .setMergeProjectWithValues(false) .setLegacyCatalogRoles(true) - .setDisableSetPropertiesSecurityCheckForCreateDdl(true) .setIncrementalHashArrayLoadFactorEnabled(false) .setHideInaccesibleColumns(true) .setAllowSetViewAuthorization(true); diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java index a8fb43fd6958..441b6a247072 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java @@ -160,18 +160,6 @@ default void checkCanShowCreateTable(ConnectorSecurityContext context, SchemaTab denyShowCreateTable(tableName.toString(), null); } - /** - * Check if identity is allowed to create the specified table. - * - * @throws io.trino.spi.security.AccessDeniedException if not allowed - * @deprecated use {@link #checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName, Map properties)} instead - */ - @Deprecated - default void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName) - { - denyCreateTable(tableName.toString()); - } - /** * Check if identity is allowed to create the specified table with properties. * @@ -416,18 +404,6 @@ default void checkCanCreateViewWithSelectFromColumns(ConnectorSecurityContext co denyCreateViewWithSelect(tableName.toString(), context.getIdentity()); } - /** - * Check if identity is allowed to create the specified materialized view. - * - * @throws io.trino.spi.security.AccessDeniedException if not allowed - * @deprecated use {@link #checkCanCreateMaterializedView(ConnectorSecurityContext, SchemaTableName, Map)} instead - */ - @Deprecated - default void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName) - { - denyCreateMaterializedView(materializedViewName.toString()); - } - /** * Check if identity is allowed to create the specified materialized view. * diff --git a/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java b/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java index 64b12af34339..cae9262257a5 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java +++ b/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java @@ -332,18 +332,6 @@ default void checkCanShowCreateTable(SystemSecurityContext context, CatalogSchem denyShowCreateTable(table.toString()); } - /** - * Check if identity is allowed to create the specified table in a catalog. - * - * @throws AccessDeniedException if not allowed - * @deprecated use {@link #checkCanCreateTable(SystemSecurityContext context, CatalogSchemaTableName table, Map properties)} instead - */ - @Deprecated - default void checkCanCreateTable(SystemSecurityContext context, CatalogSchemaTableName table) - { - denyCreateTable(table.toString()); - } - /** * Check if identity is allowed to create the specified table with properties in a catalog. * @@ -588,18 +576,6 @@ default void checkCanCreateViewWithSelectFromColumns(SystemSecurityContext conte denyCreateViewWithSelect(table.toString(), context.getIdentity()); } - /** - * Check if identity is allowed to create the specified materialized view in a catalog. - * - * @throws AccessDeniedException if not allowed - * @deprecated use {@link #checkCanCreateMaterializedView(SystemSecurityContext, CatalogSchemaTableName, Map)} instead - */ - @Deprecated - default void checkCanCreateMaterializedView(SystemSecurityContext context, CatalogSchemaTableName materializedView) - { - denyCreateMaterializedView(materializedView.toString()); - } - /** * Check if identity is allowed to create the specified materialized view in a catalog. * diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorAccessControl.java index 853c69e97e97..f10be3892d88 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorAccessControl.java @@ -108,14 +108,6 @@ public void checkCanShowCreateTable(ConnectorSecurityContext context, SchemaTabl } } - @Override - public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName) - { - try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { - delegate.checkCanCreateTable(context, tableName); - } - } - @Override public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName, Map properties) { @@ -308,14 +300,6 @@ public void checkCanCreateViewWithSelectFromColumns(ConnectorSecurityContext con } } - @Override - public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName) - { - try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { - delegate.checkCanCreateMaterializedView(context, materializedViewName); - } - } - @Override public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName, Map properties) { diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllAccessControl.java index 07dbbe2eb027..8ab0f2a35642 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllAccessControl.java @@ -70,11 +70,6 @@ public void checkCanShowCreateTable(ConnectorSecurityContext context, SchemaTabl { } - @Override - public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName) - { - } - @Override public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName, Map properties) { @@ -197,11 +192,6 @@ public void checkCanCreateViewWithSelectFromColumns(ConnectorSecurityContext con { } - @Override - public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName) - { - } - @Override public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName, Map properties) { diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllSystemAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllSystemAccessControl.java index af8465dc80dd..1e25c076f964 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllSystemAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllSystemAccessControl.java @@ -176,11 +176,6 @@ public void checkCanShowCreateTable(SystemSecurityContext context, CatalogSchema { } - @Override - public void checkCanCreateTable(SystemSecurityContext context, CatalogSchemaTableName table) - { - } - @Override public void checkCanCreateTable(SystemSecurityContext context, CatalogSchemaTableName table, Map properties) { @@ -303,11 +298,6 @@ public void checkCanCreateViewWithSelectFromColumns(SystemSecurityContext contex { } - @Override - public void checkCanCreateMaterializedView(SystemSecurityContext context, CatalogSchemaTableName materializedView) - { - } - @Override public void checkCanCreateMaterializedView(SystemSecurityContext context, CatalogSchemaTableName materializedView, Map properties) { 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 2ec422b15b91..aae17963100e 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 @@ -186,15 +186,6 @@ public void checkCanShowCreateTable(ConnectorSecurityContext context, SchemaTabl } } - @Override - public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName) - { - // check if user will be an owner of the table after creation - if (!checkTablePermission(context, tableName, OWNERSHIP)) { - denyCreateTable(tableName.toString()); - } - } - @Override public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName, Map properties) { @@ -432,15 +423,6 @@ public void checkCanCreateViewWithSelectFromColumns(ConnectorSecurityContext con } } - @Override - public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName) - { - // check if user will be an owner of the view after creation - if (!checkTablePermission(context, materializedViewName, OWNERSHIP)) { - denyCreateMaterializedView(materializedViewName.toString()); - } - } - @Override public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName, Map properties) { 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 d7aff81e8f75..ac8c439165ad 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 @@ -503,15 +503,6 @@ public void checkCanShowCreateSchema(SystemSecurityContext context, CatalogSchem } } - @Override - public void checkCanCreateTable(SystemSecurityContext context, CatalogSchemaTableName table) - { - // check if user will be an owner of the table after creation - if (!checkTablePermission(context, table, OWNERSHIP)) { - denyCreateTable(table.toString()); - } - } - @Override public void checkCanCreateTable(SystemSecurityContext context, CatalogSchemaTableName table, Map properties) { @@ -762,15 +753,6 @@ public void checkCanCreateViewWithSelectFromColumns(SystemSecurityContext contex } } - @Override - public void checkCanCreateMaterializedView(SystemSecurityContext context, CatalogSchemaTableName materializedView) - { - // check if user will be an owner of the materialize view after creation - if (!checkTablePermission(context, materializedView, OWNERSHIP)) { - denyCreateMaterializedView(materializedView.toString()); - } - } - @Override public void checkCanCreateMaterializedView(SystemSecurityContext context, CatalogSchemaTableName materializedView, Map properties) { diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java index ede7623a37d3..1de3a6c9ef22 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java @@ -95,12 +95,6 @@ public void checkCanShowCreateTable(ConnectorSecurityContext context, SchemaTabl delegate().checkCanShowCreateTable(context, tableName); } - @Override - public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName) - { - delegate().checkCanCreateTable(context, tableName); - } - @Override public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName, Map properties) { @@ -245,12 +239,6 @@ public void checkCanCreateViewWithSelectFromColumns(ConnectorSecurityContext con delegate().checkCanCreateViewWithSelectFromColumns(context, tableName, columnNames); } - @Override - public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName) - { - delegate().checkCanCreateMaterializedView(context, materializedViewName); - } - @Override public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName, Map properties) { diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java index 8cd5e2e99770..e8a71874f12c 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java @@ -185,12 +185,6 @@ public void checkCanShowCreateTable(SystemSecurityContext context, CatalogSchema delegate().checkCanShowCreateTable(context, table); } - @Override - public void checkCanCreateTable(SystemSecurityContext context, CatalogSchemaTableName table) - { - delegate().checkCanCreateTable(context, table); - } - @Override public void checkCanCreateTable(SystemSecurityContext context, CatalogSchemaTableName table, Map properties) { @@ -335,12 +329,6 @@ public void checkCanCreateViewWithSelectFromColumns(SystemSecurityContext contex delegate().checkCanCreateViewWithSelectFromColumns(context, table, columns); } - @Override - public void checkCanCreateMaterializedView(SystemSecurityContext context, CatalogSchemaTableName materializedView) - { - delegate().checkCanCreateMaterializedView(context, materializedView); - } - @Override public void checkCanCreateMaterializedView(SystemSecurityContext context, CatalogSchemaTableName materializedView, Map properties) { diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ReadOnlyAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ReadOnlyAccessControl.java index a645edff754d..c57d43c2986e 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ReadOnlyAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ReadOnlyAccessControl.java @@ -82,12 +82,6 @@ public void checkCanShowCreateTable(ConnectorSecurityContext context, SchemaTabl { } - @Override - public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName) - { - denyCreateTable(tableName.toString()); - } - @Override public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName, Map properties) { @@ -201,12 +195,6 @@ public void checkCanCreateViewWithSelectFromColumns(ConnectorSecurityContext con // allow } - @Override - public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName) - { - denyCreateMaterializedView(materializedViewName.toString()); - } - @Override public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName, Map properties) { diff --git a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java index 3aac31330a04..e937842a0a82 100644 --- a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java +++ b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java @@ -32,6 +32,7 @@ import java.io.File; import java.util.EnumSet; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Stream; @@ -68,7 +69,7 @@ public void testEmptyFile() accessControl.checkCanInsertIntoTable(UNKNOWN, new SchemaTableName("unknown", "unknown")); accessControl.checkCanDeleteFromTable(UNKNOWN, new SchemaTableName("unknown", "unknown")); - accessControl.checkCanCreateTable(UNKNOWN, new SchemaTableName("unknown", "unknown")); + accessControl.checkCanCreateTable(UNKNOWN, new SchemaTableName("unknown", "unknown"), Map.of()); accessControl.checkCanDropTable(UNKNOWN, new SchemaTableName("unknown", "unknown")); accessControl.checkCanTruncateTable(UNKNOWN, new SchemaTableName("unknown", "unknown")); accessControl.checkCanRenameTable(UNKNOWN, @@ -291,15 +292,15 @@ public void testTableRules() accessControl.checkCanInsertIntoTable(CHARLIE, bobTable); accessControl.checkCanSelectFromColumns(JOE, bobTable, ImmutableSet.of()); - accessControl.checkCanCreateTable(ADMIN, new SchemaTableName("bob", "test")); - accessControl.checkCanCreateTable(ADMIN, testTable); - accessControl.checkCanCreateTable(ADMIN, new SchemaTableName("authenticated", "test")); - assertDenied(() -> accessControl.checkCanCreateTable(ADMIN, new SchemaTableName("secret", "test"))); + accessControl.checkCanCreateTable(ADMIN, new SchemaTableName("bob", "test"), Map.of()); + accessControl.checkCanCreateTable(ADMIN, testTable, Map.of()); + accessControl.checkCanCreateTable(ADMIN, new SchemaTableName("authenticated", "test"), Map.of()); + assertDenied(() -> accessControl.checkCanCreateTable(ADMIN, new SchemaTableName("secret", "test"), Map.of())); - accessControl.checkCanCreateTable(ALICE, new SchemaTableName("aliceschema", "test")); - assertDenied(() -> accessControl.checkCanCreateTable(ALICE, testTable)); - assertDenied(() -> accessControl.checkCanCreateTable(CHARLIE, new SchemaTableName("aliceschema", "test"))); - assertDenied(() -> accessControl.checkCanCreateTable(CHARLIE, testTable)); + accessControl.checkCanCreateTable(ALICE, new SchemaTableName("aliceschema", "test"), Map.of()); + assertDenied(() -> accessControl.checkCanCreateTable(ALICE, testTable, Map.of())); + assertDenied(() -> accessControl.checkCanCreateTable(CHARLIE, new SchemaTableName("aliceschema", "test"), Map.of())); + assertDenied(() -> accessControl.checkCanCreateTable(CHARLIE, testTable, Map.of())); accessControl.checkCanCreateViewWithSelectFromColumns(BOB, bobTable, ImmutableSet.of()); accessControl.checkCanDropTable(ADMIN, bobTable); diff --git a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java index 109c9581bd9c..8e36a49cbf68 100644 --- a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java +++ b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java @@ -37,6 +37,7 @@ import java.io.File; import java.util.Collection; import java.util.EnumSet; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Stream; @@ -142,14 +143,14 @@ public void testEmptyFile() accessControl.checkCanDeleteFromTable(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown")); accessControl.checkCanTruncateTable(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown")); - accessControl.checkCanCreateTable(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown")); + accessControl.checkCanCreateTable(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown"), Map.of()); accessControl.checkCanDropTable(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown")); accessControl.checkCanTruncateTable(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown")); accessControl.checkCanRenameTable(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown"), new CatalogSchemaTableName("some-catalog", "unknown", "new_unknown")); - accessControl.checkCanCreateMaterializedView(UNKNOWN, new CatalogSchemaTableName("some-catalog", "unknown", "unknown")); + 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")); @@ -565,8 +566,8 @@ public void testTableRulesForCheckCanCreateMaterializedView() { SystemAccessControl accessControl = newFileBasedSystemAccessControl("file-based-system-access-table.json"); - accessControl.checkCanCreateMaterializedView(ADMIN, new CatalogSchemaTableName("some-catalog", "bobschema", "bob-materialized-view")); - assertAccessDenied(() -> accessControl.checkCanCreateMaterializedView(BOB, new CatalogSchemaTableName("some-catalog", "bobschema", "bob-materialized-view")), CREATE_MATERIALIZED_VIEW_ACCESS_DENIED_MESSAGE); + accessControl.checkCanCreateMaterializedView(ADMIN, new CatalogSchemaTableName("some-catalog", "bobschema", "bob-materialized-view"), Map.of()); + assertAccessDenied(() -> accessControl.checkCanCreateMaterializedView(BOB, new CatalogSchemaTableName("some-catalog", "bobschema", "bob-materialized-view"), Map.of()), CREATE_MATERIALIZED_VIEW_ACCESS_DENIED_MESSAGE); } @Test diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/LegacyAccessControl.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/LegacyAccessControl.java index e53ea9d61cad..d1ac8e85385b 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/LegacyAccessControl.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/LegacyAccessControl.java @@ -110,11 +110,6 @@ public void checkCanShowCreateTable(ConnectorSecurityContext context, SchemaTabl { } - @Override - public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName) - { - } - @Override public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName, Map properties) { @@ -269,11 +264,6 @@ public void checkCanCreateViewWithSelectFromColumns(ConnectorSecurityContext con { } - @Override - public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName) - { - } - @Override public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName, Map properties) { 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 4576a6ddb017..43e977d0ac12 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 @@ -181,14 +181,6 @@ public void checkCanShowCreateSchema(ConnectorSecurityContext context, String sc } } - @Override - public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName) - { - if (!isDatabaseOwner(context, tableName.getSchemaName())) { - denyCreateTable(tableName.toString()); - } - } - @Override public void checkCanCreateTable(ConnectorSecurityContext context, SchemaTableName tableName, Map properties) { @@ -381,14 +373,6 @@ public void checkCanCreateViewWithSelectFromColumns(ConnectorSecurityContext con } } - @Override - public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName) - { - if (!isDatabaseOwner(context, materializedViewName.getSchemaName())) { - denyCreateMaterializedView(materializedViewName.toString()); - } - } - @Override public void checkCanCreateMaterializedView(ConnectorSecurityContext context, SchemaTableName materializedViewName, Map properties) {