Add flag permissive policies always required

programs/server/Server.cpp

@@ -940,7 +940,10 @@ if (ThreadFuzzer::instance().isEffective())
             updateLevels(*config, logger());
             global_context->setClustersConfig(config, has_zookeeper);
             global_context->setMacros(std::make_unique<Macros>(*config, "macros", log));
-            global_context->setExternalAuthenticatorsConfig(*config);
+
+            auto & access_control = global_context->getAccessControl();
+            access_control.setExternalAuthenticatorsConfig(*config);
+            access_control.setRowPoliciesConfig(*config);
 
             global_context->loadOrReloadDictionaries(*config);
             global_context->loadOrReloadModels(*config);
@@ -1069,6 +1072,7 @@ if (ThreadFuzzer::instance().isEffective())
     auto & access_control = global_context->getAccessControl();
     if (config().has("custom_settings_prefixes"))
         access_control.setCustomSettingsPrefixes(config().getString("custom_settings_prefixes"));
+    access_control.setRowPoliciesConfig(config());
 
     /// Initialize access storages.
     try

programs/server/config.xml

@@ -559,6 +559,34 @@
     <!-- Default database. -->
     <default_database>default</default_database>
 
+    <!-- Version of row policies' implementation. By default it's 1.
+         Version 1:
+            If for some table only restrictive policies exist without permissive ones any user will see no rows.
+            By default row policies are permissive.
+            Permissive row policies affect all users by default, but that can be changed with the OF clause.
+            Row policies defined by users.xml are permissive.
+            Row policies defined by users.xml affect all users.
+
+         Version 2:
+            If for some table only restrictive policies exist without permissive ones each user will see rows chosen by applied restrictive policies.
+            By default row policies are restrictive.
+            Permissive row policies affect only users specified in the TO clause, but that can be changed with the OF clause.
+            Row policies defined by users.xml are restrictive.
+            Row policies defined by users.xml affect only users from users.xml.
+    -->
+    <row_policies_version>1</row_policies_version>
+
+
+        Whether the permissive row policies are always required to see any rows.
+            If the flag is 1 then if for some table only restrictive policies exist without permissive ones any user won't see any rows.
+            If the flag is 0 then in the above case each user will see rows chosen by applied restrictive policies. -->
+        <permissive_policies_always_required>0</permissive_policies_always_required>
+
+        <permissive_policies_always_required>0</permissive_policies_always_required>
+        <permissive_by_default>
+        <permissive_policies_affect_all_users_by_default>
+    </row_policies>
+
     <!-- Server time zone could be set here.
 
          Time zone is used when converting between String and DateTime types,

src/Access/AccessControl.cpp

@@ -425,6 +425,12 @@ void AccessControl::setExternalAuthenticatorsConfig(const Poco::Util::AbstractCo
 }
 
 
+void AccessControl::setRowPoliciesConfig(const Poco::Util::AbstractConfiguration & config)
+{
+    row_policy_cache->setConfiguration(config);
+}
+
+
 void AccessControl::setDefaultProfileName(const String & default_profile_name)
 {
     settings_profiles_cache->setDefaultProfileName(default_profile_name);

src/Access/AccessControl.h

@@ -116,6 +116,8 @@ class AccessControl : public MultipleAccessStorage
     UUID authenticate(const Credentials & credentials, const Poco::Net::IPAddress & address) const;
     void setExternalAuthenticatorsConfig(const Poco::Util::AbstractConfiguration & config);
 
+    void setRowPoliciesConfig(const Poco::Util::AbstractConfiguration & config);
+
     std::shared_ptr<const ContextAccess> getContextAccess(
         const UUID & user_id,
         const std::vector<UUID> & current_roles,

src/Access/RowPolicyCache.cpp

@@ -24,23 +24,23 @@ namespace
         {
             if (kind == RowPolicyKind::PERMISSIVE)
             {
-                setPermissiveFiltersExist();
                 permissive_filters.push_back(filter);
+                setNeedApplyPermissiveFilters();
             }
             else
             {
                 restrictive_filters.push_back(filter);
             }
         }
 
-        void setPermissiveFiltersExist()
+        void setNeedApplyPermissiveFilters()
         {
-            permissive_filters_exist = true;
+            need_apply_permissive_filters = true;
         }
 
         ASTPtr getResult() &&
         {
-            if (permissive_filters_exist)
+            if (need_apply_permissive_filters)
             {
                 /// Process permissive filters.
                 restrictive_filters.push_back(makeASTForLogicalOr(std::move(permissive_filters)));
@@ -58,7 +58,7 @@ namespace
 
     private:
         ASTs permissive_filters;
-        bool permissive_filters_exist = false;
+        bool need_apply_permissive_filters = false;
         ASTs restrictive_filters;
     };
 }
@@ -237,10 +237,10 @@ void RowPolicyCache::mixFiltersFor(EnabledRowPolicies & enabled)
                 key.filter_type = filter_type;
                 auto & mixer = mixers[key];
                 mixer.database_and_table_name = info.database_and_table_name;
-                if (policy.getKind() == RowPolicyKind::PERMISSIVE)
+                if ((policy.getKind() == RowPolicyKind::PERMISSIVE) || permissive_policies_always_required)
                 {
-                    /// We call setPermissiveFiltersExist() even if the current user doesn't match to the current policy's TO clause.
-                    mixer.mixer.setPermissiveFiltersExist();
+                    /// We call setNeedApplyPermissiveFilters() even if the current user doesn't match to the current policy's TO clause.
+                    mixer.mixer.setNeedApplyPermissiveFilters();
                 }
                 if (match)
                     mixer.mixer.add(info.parsed_filters[filter_type_i], policy.getKind());
@@ -259,4 +259,16 @@ void RowPolicyCache::mixFiltersFor(EnabledRowPolicies & enabled)
     enabled.mixed_filters.store(mixed_filters);
 }
 
+
+void RowPolicyCache::setConfiguration(const Poco::Util::AbstractConfiguration & config)
+{
+    std::lock_guard lock{mutex};
+    bool old_permissive_policies_always_required = permissive_policies_always_required;
+
+    permissive_policies_always_required = config.getBool("row_policies.permissive_policies_always_required", true);
+
+    if (permissive_policies_always_required != old_permissive_policies_always_required)
+        mixFilters();
+}
+
 }

src/Access/RowPolicyCache.h

@@ -23,6 +23,8 @@ class RowPolicyCache
 
     std::shared_ptr<const EnabledRowPolicies> getEnabledRowPolicies(const UUID & user_id, const boost::container::flat_set<UUID> & enabled_roles);
 
+    void setConfiguration(const Poco::Util::AbstractConfiguration & config);
+
 private:
     struct PolicyInfo
     {
@@ -42,6 +44,7 @@ class RowPolicyCache
     void mixFiltersFor(EnabledRowPolicies & enabled);
 
     const AccessControl & access_control;
+    bool permissive_policies_always_required = false;
     std::unordered_map<UUID, PolicyInfo> all_policies;
     bool all_policies_read = false;
     scope_guard subscription;

tests/integration/test_row_policy/permissive_policies_always_required_0.xml

@@ -0,0 +1,5 @@
+<clickhouse>
+    <row_policies>
+        <permissive_policies_always_required>0</permissive_policies_always_required>
+    </row_policies>
+</clickhouse>

tests/integration/test_row_policy/permissive_policies_always_required_1.xml

@@ -0,0 +1,5 @@
+<clickhouse>
+    <row_policies>
+        <permissive_policies_always_required>1</permissive_policies_always_required>
+    </row_policies>
+</clickhouse>

tests/integration/test_row_policy/test.py

@@ -7,6 +7,7 @@
 from helpers.test_tools import assert_eq_with_retry, TSV
 
 cluster = ClickHouseCluster(__file__)
+script_dir = os.path.dirname(os.path.realpath(__file__))
 node = cluster.add_instance('node', main_configs=["configs/config.d/remote_servers.xml"],
                             user_configs=["configs/users.d/row_policy.xml", "configs/users.d/another_user.xml",
                                           "configs/users.d/any_join_distinct_right_table_keys.xml"],
@@ -19,7 +20,7 @@
 
 
 def copy_policy_xml(local_file_name, reload_immediately=True):
-    script_dir = os.path.dirname(os.path.realpath(__file__))
+
     for current_node in nodes:
         current_node.copy_file_to_container(os.path.join(script_dir, local_file_name),
                                             '/etc/clickhouse-server/users.d/row_policy.xml')
@@ -435,8 +436,12 @@ def test_grant_create_row_policy():
     node.query("DROP USER X")
 
 
-def test_some_users_without_policies():
+@pytest.mark.parametrize("permissive_policies_always_required", [0, 1])
+def test_some_users_without_policies(permissive_policies_always_required):
     copy_policy_xml('no_filters.xml')
+    node.copy_file_to_container(os.path.join(script_dir, f'permissive_policies_always_required_{permissive_policies_always_required}.xml'), '/etc/clickhouse-server/config.d/permissive_policies_always_required.xml')
+    node.query("SYSTEM RELOAD CONFIG")
+
     assert node.query("SHOW POLICIES") == ""
     node.query("CREATE USER X, Y")
     node.query("GRANT SELECT ON mydb.filtered_table1 TO X, Y")
@@ -446,12 +451,67 @@ def test_some_users_without_policies():
     assert node.query("SELECT * FROM mydb.filtered_table1", user='Y') == ""
 
     node.query("ALTER POLICY pA ON mydb.filtered_table1 AS restrictive")
-    assert node.query("SELECT * FROM mydb.filtered_table1", user='X') == TSV([[0, 1]])
-    assert node.query("SELECT * FROM mydb.filtered_table1", user='Y') == TSV([[0, 0], [0, 1], [1, 0], [1, 1]])
+
+    if permissive_policies_always_required:
+        assert node.query("SELECT * FROM mydb.filtered_table1", user='X') == ""
+        assert node.query("SELECT * FROM mydb.filtered_table1", user='Y') == ""
+    else:
+        assert node.query("SELECT * FROM mydb.filtered_table1", user='X') == TSV([[0, 1]])
+        assert node.query("SELECT * FROM mydb.filtered_table1", user='Y') == TSV([[0, 0], [0, 1], [1, 0], [1, 1]])
 
     node.query("DROP USER X, Y")
 
 
+@pytest.mark.parametrize("permissive_policies_always_required", [0, 1])
+def test_multiple_row_policies_on_same_column(permissive_policies_always_required):
+    copy_policy_xml('no_filters.xml')
+    node.copy_file_to_container(os.path.join(script_dir, f'permissive_policies_always_required_{permissive_policies_always_required}.xml'), '/etc/clickhouse-server/config.d/permissive_policies_always_required.xml')
+    node.query("SYSTEM RELOAD CONFIG")
+
+    node.query("CREATE TABLE multiple_row_policies_on_same_column (x UInt8) ENGINE = MergeTree ORDER BY x")
+    node.query("INSERT INTO 02131_multiple_row_policies_on_same_column VALUES (1), (2), (3), (4)")
+
+    assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "1\n2\n3\n4\n" # No policies applied
+
+    node.query("CREATE ROW POLICY R1 ON multiple_row_policies_on_same_column USING x=1 AS permissive TO ALL")
+    assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "1\n" # Applied R1: (x == 1)
+
+    node.query("CREATE ROW POLICY R2 ON multiple_row_policies_on_same_column USING x=2 AS permissive TO ALL")
+    assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "1\n2\n" # Applied R1, R2: (x == 1) OR (x == 2)
+
+    node.query("CREATE ROW POLICY R3 ON multiple_row_policies_on_same_column USING x=3 AS permissive TO ALL")
+    assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "1\n2\n3\n" # Applied R1, R2, R3: (x == 1) OR (x == 2) OR (x == 3)
+
+    node.query("CREATE ROW POLICY R4 ON multiple_row_policies_on_same_column USING x<=2 AS restrictive TO ALL")
+    assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "1\n2\n" # Applied R1, R2, R3, R4: ((x == 1) OR (x == 2) OR (x == 3)) AND (x <= 2)
+
+    node.query("CREATE ROW POLICY R5 ON multiple_row_policies_on_same_column USING x>=2 AS restrictive TO ALL")
+    assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "2\n" # Applied R1, R2, R3, R4, R5: ((x == 1) OR (x == 2) OR (x == 3)) AND (x <= 2) AND (x >= 2)
+
+    node.query("DROP ROW POLICY R1 ON multiple_row_policies_on_same_column")
+    assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "2\n" # Applied R2, R3, R4, R5: ((x == 2) OR (x == 3)) AND (x <= 2) AND (x >= 2)
+
+    node.query("DROP ROW POLICY R2 ON multiple_row_policies_on_same_column")
+    assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "" # Applied R3, R4, R5: ((x == 3)) AND (x <= 2) AND (x >= 2)
+
+    node.query("DROP ROW POLICY R3 ON multiple_row_policies_on_same_column")
+    if permissive_policies_always_required:
+        assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "" # Applied R4, R5: FALSE AND (x <= 2) AND (x >= 2)
+    else:
+        assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "2\n" # Applied R4, R5: (x <= 2) AND (x >= 2)
+
+    node.query("DROP ROW POLICY R4 ON multiple_row_policies_on_same_column")
+    if permissive_policies_always_required:
+        assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "" # Applied R5: FALSE AND (x >= 2)
+    else:
+        assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "2\n3\n4\n" # Applied R5: (x >= 2)
+
+    node.query("DROP ROW POLICY R5 ON multiple_row_policies_on_same_column")
+    assert node.query("SELECT * FROM multiple_row_policies_on_same_column") == "1\n2\n3\n4\n" # No policies applied
+
+    node.query("DROP TABLE 02131_multiple_row_policies_on_same_column")
+
+
 def test_users_xml_is_readonly():
     assert re.search("storage is readonly", node.query_and_get_error("DROP POLICY default ON mydb.filtered_table1"))