Add config for default view security mode

Presto has a default view creation security mode as 'DEFINER'. However, some administrators may prefer to use 'INVOKER' as the default. To address this, a new configuration flag `default-view-security-mode` has been added that allows the default view security mode to be changed according to the administrator's preference.
prestodb · Feb 23, 2024 · fcfc114 · fcfc114
1 parent c33c3a3
commit fcfc114
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 5 deletions.
diff --git a/presto-docs/src/main/sphinx/sql/create-view.rst b/presto-docs/src/main/sphinx/sql/create-view.rst
@@ -38,6 +38,9 @@ In the ``INVOKER`` security mode, tables referenced in the view are
 accessed using the permissions of the query user (the *invoker* of the
 view). A view created in this mode is simply a stored query.
 
+The ``default-view-security-mode`` can be used to configure the default
+security mode for view creation.
+
 Examples
 --------
 
@@ -60,6 +63,10 @@ Create a view that replaces an existing view::
     SELECT orderkey, orderstatus, totalprice / 4 AS quarter
     FROM orders
 
+Set the default view security mode to ``INVOKER``::
+
+    SET SESSION default_view_security_mode='INVOKER'
+
 See Also
 --------
 

diff --git a/presto-main/src/main/java/com/facebook/presto/SystemSessionProperties.java b/presto-main/src/main/java/com/facebook/presto/SystemSessionProperties.java
@@ -43,6 +43,7 @@
 import com.facebook.presto.sql.analyzer.FeaturesConfig.ShardedJoinStrategy;
 import com.facebook.presto.sql.analyzer.FeaturesConfig.SingleStreamSpillerChoice;
 import com.facebook.presto.sql.planner.CompilerConfig;
+import com.facebook.presto.sql.tree.CreateView;
 import com.facebook.presto.tracing.TracingConfig;
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
@@ -341,6 +342,7 @@ public final class SystemSessionProperties
     public static final String NATIVE_EXECUTION_PROGRAM_ARGUMENTS = "native_execution_program_arguments";
     public static final String NATIVE_EXECUTION_PROCESS_REUSE_ENABLED = "native_execution_process_reuse_enabled";
     public static final String NATIVE_DEBUG_VALIDATE_OUTPUT_FROM_OPERATORS = "native_debug_validate_output_from_operators";
+    public static final String DEFAULT_VIEW_SECURITY_MODE = "default_view_security_mode";
 
     private final List<PropertyMetadata<?>> sessionProperties;
 
@@ -1929,7 +1931,19 @@ public SystemSessionProperties(
                         REWRITE_EXPRESSION_WITH_CONSTANT_EXPRESSION,
                         "Rewrite left join with is null check to semi join",
                         featuresConfig.isRewriteExpressionWithConstantVariable(),
-                        false));
+                        false),
+                new PropertyMetadata<>(
+                        DEFAULT_VIEW_SECURITY_MODE,
+                        format("Set default view security mode. Options are: %s",
+                                Stream.of(CreateView.Security.values())
+                                        .map(CreateView.Security::name)
+                                        .collect(joining(","))),
+                        VARCHAR,
+                        CreateView.Security.class,
+                        featuresConfig.getDefaultViewSecurityMode(),
+                        false,
+                        value -> CreateView.Security.valueOf(((String) value).toUpperCase()),
+                        CreateView.Security::name));
     }
 
     public static boolean isSpoolingOutputBufferEnabled(Session session)
@@ -3197,4 +3211,9 @@ public static boolean isRewriteExpressionWithConstantEnabled(Session session)
     {
         return session.getSystemProperty(REWRITE_EXPRESSION_WITH_CONSTANT_EXPRESSION, Boolean.class);
     }
+
+    public static CreateView.Security getDefaultViewSecurityMode(Session session)
+    {
+        return session.getSystemProperty(DEFAULT_VIEW_SECURITY_MODE, CreateView.Security.class);
+    }
 }
diff --git a/presto-main/src/main/java/com/facebook/presto/execution/CreateViewTask.java b/presto-main/src/main/java/com/facebook/presto/execution/CreateViewTask.java
@@ -37,6 +37,7 @@
 import java.util.List;
 import java.util.Optional;
 
+import static com.facebook.presto.SystemSessionProperties.getDefaultViewSecurityMode;
 import static com.facebook.presto.metadata.MetadataUtil.createQualifiedObjectName;
 import static com.facebook.presto.metadata.MetadataUtil.toSchemaTableName;
 import static com.facebook.presto.spi.analyzer.ViewDefinition.ViewColumn;
@@ -97,9 +98,10 @@ public ListenableFuture<?> execute(CreateView statement, TransactionManager tran
                 .collect(toImmutableList());
 
         ConnectorTableMetadata viewMetadata = new ConnectorTableMetadata(toSchemaTableName(name), columnMetadata);
-        // use DEFINER security by default
+
+        CreateView.Security defaultViewSecurityMode = getDefaultViewSecurityMode(session);
         Optional<String> owner = Optional.of(session.getUser());
-        if (statement.getSecurity().orElse(null) == INVOKER) {
+        if (statement.getSecurity().orElse(defaultViewSecurityMode) == INVOKER) {
             owner = Optional.empty();
         }
 

diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/FeaturesConfig.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/FeaturesConfig.java
@@ -22,6 +22,7 @@
 import com.facebook.presto.operator.aggregation.histogram.HistogramGroupImplementation;
 import com.facebook.presto.operator.aggregation.multimapagg.MultimapAggGroupImplementation;
 import com.facebook.presto.spi.function.FunctionMetadata;
+import com.facebook.presto.sql.tree.CreateView;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
@@ -43,6 +44,7 @@
 import static com.facebook.presto.sql.analyzer.FeaturesConfig.JoinNotNullInferenceStrategy.NONE;
 import static com.facebook.presto.sql.analyzer.FeaturesConfig.TaskSpillingStrategy.ORDER_BY_CREATE_TIME;
 import static com.facebook.presto.sql.analyzer.RegexLibrary.JONI;
+import static com.facebook.presto.sql.tree.CreateView.Security.DEFINER;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static io.airlift.units.DataSize.Unit.KILOBYTE;
 import static io.airlift.units.DataSize.Unit.MEGABYTE;
@@ -302,6 +304,7 @@ public class FeaturesConfig
     private long kHyperLogLogAggregationGroupNumberLimit;
     private boolean limitNumberOfGroupsForKHyperLogLogAggregations = true;
     private boolean generateDomainFilters;
+    private CreateView.Security defaultViewSecurityMode = DEFINER;
 
     public enum PartitioningPrecisionStrategy
     {
@@ -3055,4 +3058,17 @@ public FeaturesConfig setRewriteExpressionWithConstantVariable(boolean rewriteEx
         this.rewriteExpressionWithConstantVariable = rewriteExpressionWithConstantVariable;
         return this;
     }
+
+    public CreateView.Security getDefaultViewSecurityMode()
+    {
+        return this.defaultViewSecurityMode;
+    }
+
+    @Config("default-view-security-mode")
+    @ConfigDescription("Sets the default security mode for view creation. The options are definer/invoker.")
+    public FeaturesConfig setDefaultViewSecurityMode(CreateView.Security securityMode)
+    {
+        this.defaultViewSecurityMode = securityMode;
+        return this;
+    }
 }
diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java
@@ -2803,6 +2803,7 @@ private RelationType analyzeView(Query query, QualifiedObjectName name, Optional
                 Identity identity;
                 AccessControl viewAccessControl;
                 if (owner.isPresent() && !owner.get().equals(session.getIdentity().getUser())) {
+                    // definer mode
                     identity = new Identity(owner.get(), Optional.empty(), session.getIdentity().getExtraCredentials());
                     viewAccessControl = new ViewAccessControl(accessControl);
                 }

diff --git a/presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestFeaturesConfig.java b/presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestFeaturesConfig.java
@@ -49,6 +49,8 @@
 import static com.facebook.presto.sql.analyzer.FeaturesConfig.TaskSpillingStrategy.PER_TASK_MEMORY_THRESHOLD;
 import static com.facebook.presto.sql.analyzer.RegexLibrary.JONI;
 import static com.facebook.presto.sql.analyzer.RegexLibrary.RE2J;
+import static com.facebook.presto.sql.tree.CreateView.Security.DEFINER;
+import static com.facebook.presto.sql.tree.CreateView.Security.INVOKER;
 import static io.airlift.units.DataSize.Unit.GIGABYTE;
 import static io.airlift.units.DataSize.Unit.KILOBYTE;
 import static io.airlift.units.DataSize.Unit.MEGABYTE;
@@ -265,7 +267,8 @@ public void testDefaults()
                 .setLimitNumberOfGroupsForKHyperLogLogAggregations(true)
                 .setGenerateDomainFilters(false)
                 .setRewriteExpressionWithConstantVariable(true)
-                .setDefaultWriterReplicationCoefficient(3.0));
+                .setDefaultWriterReplicationCoefficient(3.0)
+                .setDefaultViewSecurityMode(DEFINER));
     }
 
     @Test
@@ -477,6 +480,7 @@ public void testExplicitPropertyMappings()
                 .put("optimizer.generate-domain-filters", "true")
                 .put("optimizer.rewrite-expression-with-constant-variable", "false")
                 .put("optimizer.default-writer-replication-coefficient", "5.0")
+                .put("default-view-security-mode", INVOKER.name())
                 .build();
 
         FeaturesConfig expected = new FeaturesConfig()
@@ -685,7 +689,8 @@ public void testExplicitPropertyMappings()
                 .setLimitNumberOfGroupsForKHyperLogLogAggregations(false)
                 .setGenerateDomainFilters(true)
                 .setRewriteExpressionWithConstantVariable(false)
-                .setDefaultWriterReplicationCoefficient(5.0);
+                .setDefaultWriterReplicationCoefficient(5.0)
+                .setDefaultViewSecurityMode(INVOKER);
         assertFullMapping(properties, expected);
     }
 

diff --git a/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestDistributedQueries.java b/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestDistributedQueries.java
@@ -50,6 +50,7 @@
 import static com.facebook.presto.SystemSessionProperties.VERBOSE_OPTIMIZER_INFO_ENABLED;
 import static com.facebook.presto.common.type.VarcharType.VARCHAR;
 import static com.facebook.presto.connector.informationSchema.InformationSchemaMetadata.INFORMATION_SCHEMA;
+import static com.facebook.presto.sql.tree.CreateView.Security.INVOKER;
 import static com.facebook.presto.testing.MaterializedResult.resultBuilder;
 import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.ADD_COLUMN;
 import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.CREATE_TABLE;
@@ -1093,6 +1094,46 @@ public void testNonQueryAccessControl()
         }
     }
 
+    @Test
+    public void testViewAccessControlInvokerDefault()
+    {
+        skipTestUnless(supportsViews());
+
+        Session viewOwnerSession = TestingSession.testSessionBuilder()
+                .setIdentity(new Identity("test_view_access_owner", Optional.empty()))
+                .setCatalog(getSession().getCatalog().get())
+                .setSchema(getSession().getSchema().get())
+                .setSystemProperty("default_view_security_mode", INVOKER.name())
+                .build();
+
+        assertAccessAllowed(
+                viewOwnerSession,
+                "CREATE VIEW test_view_access AS SELECT * FROM orders",
+                privilege("orders", CREATE_VIEW_WITH_SELECT_COLUMNS));
+
+        assertAccessAllowed(
+                "SELECT * FROM test_view_access",
+                privilege(viewOwnerSession.getUser(), "orders", SELECT_COLUMN));
+
+        assertAccessDenied(
+                "SELECT * FROM test_view_access",
+                "Cannot select from columns.*",
+                privilege(getSession().getUser(), "orders", SELECT_COLUMN));
+
+        assertAccessAllowed(
+                viewOwnerSession,
+                "CREATE VIEW test_view_access1 SECURITY DEFINER AS SELECT * FROM orders",
+                privilege("orders", CREATE_VIEW_WITH_SELECT_COLUMNS));
+
+        assertAccessAllowed(
+                "SELECT * FROM test_view_access1",
+                privilege(viewOwnerSession.getUser(), "orders", SELECT_COLUMN));
+
+        assertAccessAllowed(
+                "SELECT * FROM test_view_access1",
+                privilege(getSession().getUser(), "orders", SELECT_COLUMN));
+    }
+
     @Test
     public void testViewAccessControl()
     {