From 36902c66a0eb8cbc73352152431634667b6a5a57 Mon Sep 17 00:00:00 2001
From: Nikita Mikhaylov <nikitamikhaylov@clickhouse.com>
Date: Tue, 19 Nov 2024 00:20:51 +0000
Subject: [PATCH] Corrections

---
 src/Storages/StorageDictionary.cpp            | 14 ++++++-
 .../03199_dictionary_table_access.reference   |  2 -
 .../03199_dictionary_table_access.sh          | 41 -------------------
 .../03273_dictionary_rbac.reference           |  1 -
 .../0_stateless/03273_dictionary_rbac.sh      |  6 +--
 5 files changed, 16 insertions(+), 48 deletions(-)
 delete mode 100644 tests/queries/0_stateless/03199_dictionary_table_access.reference
 delete mode 100755 tests/queries/0_stateless/03199_dictionary_table_access.sh

diff --git a/src/Storages/StorageDictionary.cpp b/src/Storages/StorageDictionary.cpp
index 754f61f00eb..80cb1cc285d 100644
--- a/src/Storages/StorageDictionary.cpp
+++ b/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 | AccessType::SELECT, 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);
 }
 
diff --git a/tests/queries/0_stateless/03199_dictionary_table_access.reference b/tests/queries/0_stateless/03199_dictionary_table_access.reference
deleted file mode 100644
index 4a703b3be84..00000000000
--- a/tests/queries/0_stateless/03199_dictionary_table_access.reference
+++ /dev/null
@@ -1,2 +0,0 @@
-ACCESS_DENIED
-ACCESS_DENIED
diff --git a/tests/queries/0_stateless/03199_dictionary_table_access.sh b/tests/queries/0_stateless/03199_dictionary_table_access.sh
deleted file mode 100755
index 14f017c7fbc..00000000000
--- a/tests/queries/0_stateless/03199_dictionary_table_access.sh
+++ /dev/null
@@ -1,41 +0,0 @@
-#!/usr/bin/env bash
-
-CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
-# shellcheck source=../shell_config.sh
-. "$CUR_DIR"/../shell_config.sh
-
-username="user_${CLICKHOUSE_TEST_UNIQUE_NAME}"
-dictname="dict_${CLICKHOUSE_TEST_UNIQUE_NAME}"
-dicttablename="dict_table_${CLICKHOUSE_TEST_UNIQUE_NAME}"
-
-${CLICKHOUSE_CLIENT} -m --query "
-    CREATE DICTIONARY IF NOT EXISTS ${dictname}
-    (
-        id UInt64,
-        value UInt64
-    )
-    PRIMARY KEY id
-    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 "
-    SELECT * FROM ${dictname};
-" 2>&1 | grep -o ACCESS_DENIED | uniq
-
-$CLICKHOUSE_CLIENT -m --user="${username}" --query "
-    SELECT * FROM ${dicttablename};
-" 2>&1 | grep -o ACCESS_DENIED | uniq
-
-${CLICKHOUSE_CLIENT} -m --query "
-    DROP TABLE IF EXISTS ${dicttablename} SYNC;
-    DROP DICTIONARY IF EXISTS ${dictname};
-    DROP USER IF EXISTS ${username};
-"
diff --git a/tests/queries/0_stateless/03273_dictionary_rbac.reference b/tests/queries/0_stateless/03273_dictionary_rbac.reference
index 970842ef197..f7ff98213ba 100644
--- a/tests/queries/0_stateless/03273_dictionary_rbac.reference
+++ b/tests/queries/0_stateless/03273_dictionary_rbac.reference
@@ -2,4 +2,3 @@ Ok.
 Ok.
 Ok.
 ACCESS_DENIED
-
diff --git a/tests/queries/0_stateless/03273_dictionary_rbac.sh b/tests/queries/0_stateless/03273_dictionary_rbac.sh
index 2b66f55fa07..d74039b4d61 100755
--- a/tests/queries/0_stateless/03273_dictionary_rbac.sh
+++ b/tests/queries/0_stateless/03273_dictionary_rbac.sh
@@ -32,17 +32,17 @@ ${CLICKHOUSE_CLIENT} -nm --query "
 # Reading from dictionary via direct SELECT is Ok.
 $CLICKHOUSE_CLIENT -nm --user="${username}" --query "
     SELECT * FROM ${dictname};
-" 2>&1 > /dev/null && echo "Ok."
+" >/dev/null 2>&1 && echo "Ok."
 
 # Reading from dictionary via dictionary storage is Ok.
 $CLICKHOUSE_CLIENT -nm --user="${username}" --query "
     SELECT * FROM ${dicttablename};
-" 2>&1 > /dev/null && echo "Ok."
+" >/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});
-" 2>&1 > /dev/null && echo "Ok."
+" >/dev/null 2>&1 && echo "Ok."
 
 # Function dictGet requires a permission dictGet to use.
 $CLICKHOUSE_CLIENT -nm --user="${username}" --query "