improve acl control for initializations and secret tables

correct test
format secret requests

8 files changed, 70 insertions, 22 deletions

diff --git a/ydb/core/tx/tiering/rule/initializer.cpp b/ydb/core/tx/tiering/rule/initializer.cpp
index a9514bb006..37f7ed4c36 100644
--- a/ydb/core/tx/tiering/rule/initializer.cpp
+++ b/ydb/core/tx/tiering/rule/initializer.cpp
@@ -25,10 +25,12 @@ TVector<NKikimr::NMetadataInitializer::ITableModifier::TPtr> TTierRulesInitializ
             column.set_name("description");
             column.mutable_type()->mutable_optional_type()->mutable_item()->set_type_id(Ydb::Type::STRING);
         }
-        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(request, "create_rules"));
+        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(request, "create"));
         auto hRequest = TTieringRule::AddHistoryTableScheme(request);
-        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(hRequest, "create_rules_history"));
+        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(hRequest, "create_history"));
     }
+    result.emplace_back(NMetadataInitializer::TACLModifierConstructor::GetReadOnlyModifier(TTieringRule::GetStorageTablePath(), "acl"));
+    result.emplace_back(NMetadataInitializer::TACLModifierConstructor::GetReadOnlyModifier(TTieringRule::GetStorageHistoryTablePath(), "acl_history"));
     return result;
 }
 
diff --git a/ydb/core/tx/tiering/tier/initializer.cpp b/ydb/core/tx/tiering/tier/initializer.cpp
index d414499460..1e91c624ec 100644
--- a/ydb/core/tx/tiering/tier/initializer.cpp
+++ b/ydb/core/tx/tiering/tier/initializer.cpp
@@ -20,10 +20,12 @@ TVector<NKikimr::NMetadataInitializer::ITableModifier::TPtr> TTiersInitializer::
             column.set_name("tierConfig");
             column.mutable_type()->mutable_optional_type()->mutable_item()->set_type_id(Ydb::Type::STRING);
         }
-        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(request, "create_tiers"));
+        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(request, "create"));
         auto hRequest = TTierConfig::AddHistoryTableScheme(request);
-        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(hRequest, "create_tiers_history"));
+        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(hRequest, "create_history"));
     }
+    result.emplace_back(NMetadataInitializer::TACLModifierConstructor::GetReadOnlyModifier(TTierConfig::GetStorageTablePath(), "acl"));
+    result.emplace_back(NMetadataInitializer::TACLModifierConstructor::GetReadOnlyModifier(TTierConfig::GetStorageHistoryTablePath(), "acl_history"));
     return result;
 }
 
diff --git a/ydb/services/metadata/initializer/common.cpp b/ydb/services/metadata/initializer/common.cpp
index e9b8208654..9629e0dd59 100644
--- a/ydb/services/metadata/initializer/common.cpp
+++ b/ydb/services/metadata/initializer/common.cpp
@@ -2,4 +2,26 @@
 
 namespace NKikimr::NMetadataInitializer {
 
+ITableModifier::TPtr TACLModifierConstructor::BuildModifier() const {
+    return std::make_shared<NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogModifyPermissions>>(Request, Id);
+}
+
+NKikimr::NMetadataInitializer::TACLModifierConstructor TACLModifierConstructor::GetNoAccessModifier(const TString& path, const TString& id) {
+    TACLModifierConstructor result(path, id);
+    result->set_clear_permissions(true);
+    result->set_interrupt_inheritance(true);
+    return result;
+}
+
+NKikimr::NMetadataInitializer::TACLModifierConstructor TACLModifierConstructor::GetReadOnlyModifier(const TString& path, const TString& id) {
+    TACLModifierConstructor result(path, id);
+    result->set_clear_permissions(true);
+    result->set_interrupt_inheritance(true);
+    auto* permission = result->add_actions();
+    permission->mutable_grant()->set_subject(AppData()->AllAuthenticatedUsers ? AppData()->AllAuthenticatedUsers : "USERS");
+    permission->mutable_grant()->add_permission_names("ydb.tables.read");
+    permission->mutable_grant()->add_permission_names("ydb.deprecated.describe_schema");
+    return result;
+}
+
 }
diff --git a/ydb/services/metadata/initializer/common.h b/ydb/services/metadata/initializer/common.h
index fa63bdb775..63f8207a00 100644
--- a/ydb/services/metadata/initializer/common.h
+++ b/ydb/services/metadata/initializer/common.h
@@ -4,6 +4,8 @@
 
 namespace NKikimr::NMetadataInitializer {
 
+class TACLModifierConstructor;
+
 class ITableModifier {
 private:
     YDB_READONLY_DEF(TString, ModificationId);
@@ -36,7 +38,7 @@ protected:
         return true;
     }
 public:
-    TGenericTableModifier(typename TDialogPolicy::TRequest& request, const TString& modificationId)
+    TGenericTableModifier(const typename TDialogPolicy::TRequest& request, const TString& modificationId)
         : TBase(modificationId)
         , Request(request)
     {
@@ -44,6 +46,26 @@ public:
     }
 };
 
+class TACLModifierConstructor {
+private:
+    const TString Id;
+    Ydb::Scheme::ModifyPermissionsRequest Request;
+    ITableModifier::TPtr BuildModifier() const;
+public:
+    TACLModifierConstructor(const TString& path, const TString& id)
+        : Id(id) {
+        Request.set_path(path);
+    }
+    Ydb::Scheme::ModifyPermissionsRequest* operator->() {
+        return &Request;
+    }
+    operator ITableModifier::TPtr() const {
+        return BuildModifier();
+    }
+    static TACLModifierConstructor GetNoAccessModifier(const TString& path, const TString& id);
+    static TACLModifierConstructor GetReadOnlyModifier(const TString& path, const TString& id);
+};
+
 class IInitializerInput {
 public:
     using TPtr = std::shared_ptr<IInitializerInput>;
diff --git a/ydb/services/metadata/initializer/initializer.cpp b/ydb/services/metadata/initializer/initializer.cpp
index 70b4ffd58f..066e85cdd4 100644
--- a/ydb/services/metadata/initializer/initializer.cpp
+++ b/ydb/services/metadata/initializer/initializer.cpp
@@ -27,15 +27,7 @@ void TInitializer::DoPrepare(NMetadataInitializer::IInitializerInput::TPtr contr
         }
         result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(request, "create"));
     }
-    {
-        Ydb::Scheme::ModifyPermissionsRequest request;
-        request.set_path(TDBInitialization::GetStorageTablePath());
-        request.set_clear_permissions(true);
-        auto* permission = request.add_actions();
-        permission->mutable_grant()->set_subject("*");
-        permission->mutable_grant()->add_permission_names("ydb.tables.read");
-        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogModifyPermissions>(request, "acl"));
-    }
+    result.emplace_back(TACLModifierConstructor::GetReadOnlyModifier(TDBInitialization::GetStorageTablePath(), "acl"));
     controller->PreparationFinished(result);
 }
 
diff --git a/ydb/services/metadata/secret/access.cpp b/ydb/services/metadata/secret/access.cpp
index b6967bf187..3d25812f03 100644
--- a/ydb/services/metadata/secret/access.cpp
+++ b/ydb/services/metadata/secret/access.cpp
@@ -50,8 +50,8 @@ NMetadata::TOperationParsingResult TAccess::BuildPatchFromSettings(const NYql::T
     TStringBuf sb(settings.GetObjectId().data(), settings.GetObjectId().size());
     TStringBuf l;
     TStringBuf r;
-    if (!sb.TrySplit('/', l, r)) {
-        return "incorrect objectId format (secretId/accessUserId)";
+    if (!sb.TrySplit(':', l, r)) {
+        return "incorrect objectId format (secretId:accessUserId)";
     }
     result.SetColumn(TDecoder::SecretId, NMetadataManager::TYDBValue::Bytes(l));
     result.SetColumn(TDecoder::AccessUserId, NMetadataManager::TYDBValue::Bytes(r));
diff --git a/ydb/services/metadata/secret/initializer.cpp b/ydb/services/metadata/secret/initializer.cpp
index d92c03727e..cc73a96b5c 100644
--- a/ydb/services/metadata/secret/initializer.cpp
+++ b/ydb/services/metadata/secret/initializer.cpp
@@ -27,10 +27,12 @@ void TSecretInitializer::DoPrepare(NMetadataInitializer::IInitializerInput::TPtr
             column.set_name(TSecret::TDecoder::Value);
             column.mutable_type()->mutable_optional_type()->mutable_item()->set_type_id(Ydb::Type::STRING);
         }
-        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(request, "create_secret"));
+        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(request, "create"));
         auto hRequest = TSecret::AddHistoryTableScheme(request);
-        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(hRequest, "create_secret_history"));
+        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(hRequest, "create_history"));
     }
+    result.emplace_back(NMetadataInitializer::TACLModifierConstructor::GetNoAccessModifier(TSecret::GetStorageTablePath(), "acl"));
+    result.emplace_back(NMetadataInitializer::TACLModifierConstructor::GetNoAccessModifier(TSecret::GetStorageHistoryTablePath(), "acl_history"));
     controller->PreparationFinished(result);
 }
 
@@ -58,10 +60,12 @@ void TAccessInitializer::DoPrepare(NMetadataInitializer::IInitializerInput::TPtr
             column.set_name(TAccess::TDecoder::AccessUserId);
             column.mutable_type()->mutable_optional_type()->mutable_item()->set_type_id(Ydb::Type::STRING);
         }
-        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(request, "create_access"));
+        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(request, "create"));
         auto hRequest = TAccess::AddHistoryTableScheme(request);
-        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(hRequest, "create_access_history"));
+        result.emplace_back(new NMetadataInitializer::TGenericTableModifier<NInternal::NRequest::TDialogCreateTable>(hRequest, "create_history"));
     }
+    result.emplace_back(NMetadataInitializer::TACLModifierConstructor::GetNoAccessModifier(TAccess::GetStorageTablePath(), "acl"));
+    result.emplace_back(NMetadataInitializer::TACLModifierConstructor::GetNoAccessModifier(TAccess::GetStorageHistoryTablePath(), "acl_history"));
     controller->PreparationFinished(result);
 }
 
diff --git a/ydb/services/metadata/secret/ut/ut_secret.cpp b/ydb/services/metadata/secret/ut/ut_secret.cpp
index 6e3d804afe..2a17b5e87c 100644
--- a/ydb/services/metadata/secret/ut/ut_secret.cpp
+++ b/ydb/services/metadata/secret/ut/ut_secret.cpp
@@ -185,7 +185,7 @@ Y_UNIT_TEST_SUITE(Secret) {
             }
 
             lHelper.StartSchemaRequest("ALTER OBJECT secret1 (TYPE SECRET) SET value = `abcde`");
-            lHelper.StartSchemaRequest("CREATE OBJECT `secret1/test@test1` (TYPE SECRET_ACCESS)");
+            lHelper.StartSchemaRequest("CREATE OBJECT `secret1:test@test1` (TYPE SECRET_ACCESS)");
 
             emulator->SetExpectedSecretsCount(1).SetExpectedAccessCount(1);
             {
@@ -196,8 +196,12 @@ Y_UNIT_TEST_SUITE(Secret) {
                 Y_VERIFY(emulator->IsFound());
             }
 
-            lHelper.StartSchemaRequest("DROP OBJECT `secret1/test@test1` (TYPE SECRET_ACCESS)");
+            lHelper.StartSchemaRequest("DROP OBJECT `secret1:test@test1` (TYPE SECRET_ACCESS)");
             lHelper.StartSchemaRequest("DROP OBJECT `secret1` (TYPE SECRET)");
+            lHelper.StartDataRequest("SELECT * FROM `/Root/.metadata/initializations`");
+            lHelper.StartSchemaRequest("DELETE FROM `/Root/.metadata/initializations`", false);
+            lHelper.StartSchemaRequest("DROP TABLE `/Root/.metadata/initializations`", false);
+            lHelper.StartDataRequest("SELECT * FROM `/Root/.metadata/secrets/values`", false);
 
             emulator->SetExpectedSecretsCount(0).SetExpectedAccessCount(0);
             {