Merge pull request #72051 from ClickHouse/corrent_permissions_for_dictionaries

Correct permissions for dictionaries

src/Storages/StorageDictionary.cpp

@@ -1,4 +1,5 @@
 #include <Access/Common/AccessFlags.h>
+#include <Access/ContextAccess.h>
 #include <Storages/StorageDictionary.h>
 #include <Storages/StorageFactory.h>
 #include <DataTypes/DataTypesNumber.h>
@@ -179,7 +180,18 @@ Pipe StorageDictionary::read(
 {
     auto registered_dictionary_name = location == Location::SameDatabaseAndNameAsDictionary ? getStorageID().getInternalDictionaryName() : dictionary_name;
     auto dictionary = getContext()->getExternalDictionariesLoader().getDictionary(registered_dictionary_name, local_context);
-    local_context->checkAccess(AccessType::dictGet, dictionary->getDatabaseOrNoDatabaseTag(), dictionary->getDictionaryID().getTableName());
+
+    /**
+     * For backward compatibility reasons we require either SELECT or dictGet permission to read directly from the dictionary.
+     * If none of these conditions are met - we ask to grant a dictGet.
+     */
+    bool has_dict_get = local_context->getAccess()->isGranted(
+        AccessType::dictGet, dictionary->getDatabaseOrNoDatabaseTag(), dictionary->getDictionaryID().getTableName());
+    bool has_select = local_context->getAccess()->isGranted(
+        AccessType::SELECT, dictionary->getDatabaseOrNoDatabaseTag(), dictionary->getDictionaryID().getTableName());
+    if (!has_dict_get && !has_select)
+        local_context->checkAccess(AccessType::dictGet, dictionary->getDatabaseOrNoDatabaseTag(), dictionary->getDictionaryID().getTableName());
+
     return dictionary->read(column_names, max_block_size, threads);
 }
 

tests/queries/0_stateless/03273_dictionary_rbac.reference

@@ -1,2 +1,4 @@
-ACCESS_DENIED
+Ok.
+Ok.
+Ok.
 ACCESS_DENIED

tests/queries/0_stateless/03273_dictionary_rbac.sh

@@ -8,7 +8,8 @@ username="user_${CLICKHOUSE_TEST_UNIQUE_NAME}"
 dictname="dict_${CLICKHOUSE_TEST_UNIQUE_NAME}"
 dicttablename="dict_table_${CLICKHOUSE_TEST_UNIQUE_NAME}"
 
-${CLICKHOUSE_CLIENT} -m --query "
+# Create a dictionary and a table that points to the dictionary.
+${CLICKHOUSE_CLIENT} -nm --query "
     CREATE DICTIONARY IF NOT EXISTS ${dictname}
     (
         id UInt64,
@@ -18,23 +19,37 @@ ${CLICKHOUSE_CLIENT} -m --query "
     SOURCE(NULL())
     LAYOUT(FLAT())
     LIFETIME(MIN 0 MAX 1000);
-    CREATE USER IF NOT EXISTS ${username} NOT IDENTIFIED;
-    GRANT SELECT, CREATE TEMPORARY TABLE ON *.* to ${username};
-    SELECT * FROM ${dictname};
     CREATE TABLE ${dicttablename} (id UInt64, value UInt64)
     ENGINE = Dictionary(${CLICKHOUSE_DATABASE}.${dictname});
-    SELECT * FROM ${dicttablename};
 "
 
-$CLICKHOUSE_CLIENT -m --user="${username}" --query "
+# Create a user, assign only a few permissions.
+${CLICKHOUSE_CLIENT} -nm --query "
+    CREATE USER IF NOT EXISTS ${username} NOT IDENTIFIED;
+    GRANT SELECT, CREATE TEMPORARY TABLE ON *.* to ${username};
+"
+
+# Reading from dictionary via direct SELECT is Ok.
+$CLICKHOUSE_CLIENT -nm --user="${username}" --query "
     SELECT * FROM ${dictname};
-" 2>&1 | grep -o ACCESS_DENIED | uniq
+" >/dev/null 2>&1 && echo "Ok."
 
-$CLICKHOUSE_CLIENT -m --user="${username}" --query "
+# Reading from dictionary via dictionary storage is Ok.
+$CLICKHOUSE_CLIENT -nm --user="${username}" --query "
     SELECT * FROM ${dicttablename};
+" >/dev/null 2>&1 && echo "Ok."
+
+# Reading from dictionary via dictionary table function is Ok.
+$CLICKHOUSE_CLIENT -nm --user="${username}" --query "
+    SELECT * FROM dictionary(${dictname});
+" >/dev/null 2>&1 && echo "Ok."
+
+# Function dictGet requires a permission dictGet to use.
+$CLICKHOUSE_CLIENT -nm --user="${username}" --query "
+    SELECT dictGet(${dictname}, 'value', 1);
 " 2>&1 | grep -o ACCESS_DENIED | uniq
 
-${CLICKHOUSE_CLIENT} -m --query "
+${CLICKHOUSE_CLIENT} -nm --query "
     DROP TABLE IF EXISTS ${dicttablename} SYNC;
     DROP DICTIONARY IF EXISTS ${dictname};
     DROP USER IF EXISTS ${username};