Authorize table parameters in CTAS and remove deprecated check methods

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()
     {

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<Void> 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());

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<String> specifiedPropertyKeys = statement.getProperties().stream()
                 .map(property -> property.getName().getValue())

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<String> columnNames);
 
-    /**
-     * Check if identity is allowed to create the specified materialized view.
-     *
-     * @throws AccessDeniedException if not allowed
-     * @deprecated use {@link #checkCanCreateMaterializedView(SecurityContext, QualifiedObjectName, Map<String, Object>) instead}
-     */
-    @Deprecated
-    void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName);
-
     /**
      * Check if identity is allowed to create the specified materialized view.
      *

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<String, Object> 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<String, Object> properties)
     {

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<String, Object> 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<String, Object> properties)
     {

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<String, Object> 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<String, Object> properties)
     {
@@ -506,6 +494,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);
     }
 }

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<String, Object> 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<String, Object> properties)
     {

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<String, Object> 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<String, Object> properties)
     {

core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java

@@ -804,7 +804,16 @@ protected Scope visitCreateTableAsSelect(CreateTableAsSelect node, Optional<Scop
 
             validateProperties(node.getProperties(), scope);
 
-            accessControl.checkCanCreateTable(session.toSecurityContext(), targetTable);
+            CatalogName catalogName = getRequiredCatalogHandle(metadata, session, node, targetTable.getCatalogName());
+            Map<String, Object> properties = tablePropertyManager.getProperties(
+                    catalogName,
+                    node.getProperties(),
+                    session,
+                    plannerContext,
+                    accessControl,
+                    analysis.getParameters(),
+                    true);
+            accessControl.checkCanCreateTable(session.toSecurityContext(), targetTable, properties);
 
             // analyze the query that creates the table
             Scope queryScope = analyze(node.getQuery(), createScope(scope));
@@ -838,16 +847,6 @@ protected Scope visitCreateTableAsSelect(CreateTableAsSelect node, Optional<Scop
             }
 
             // create target table metadata
-            CatalogName catalogName = getRequiredCatalogHandle(metadata, session, node, targetTable.getCatalogName());
-
-            Map<String, Object> 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

core/trino-main/src/main/java/io/trino/testing/AllowAllAccessControlManager.java

@@ -92,9 +92,6 @@ public void checkCanShowCreateSchema(SecurityContext context, CatalogSchemaName
     @Override
     public void checkCanShowCreateTable(SecurityContext context, QualifiedObjectName tableName) {}
 
-    @Override
-    public void checkCanCreateTable(SecurityContext context, QualifiedObjectName tableName) {}
-
     @Override
     public void checkCanCreateTable(SecurityContext context, QualifiedObjectName tableName, Map<String, Object> properties) {}
 
@@ -167,9 +164,6 @@ public void checkCanDropView(SecurityContext context, QualifiedObjectName viewNa
     @Override
     public void checkCanCreateViewWithSelectFromColumns(SecurityContext context, QualifiedObjectName tableName, Set<String> columnNames) {}
 
-    @Override
-    public void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName) {}
-
     @Override
     public void checkCanCreateMaterializedView(SecurityContext context, QualifiedObjectName materializedViewName, Map<String, Object> properties) {}
 

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<String, Object> 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<String, Object> properties)
     {