Browse Source

system user access to system statistics table (#7679)

Aleksandr Dmitriev 7 months ago
parent
commit
53496ddfa7

+ 6 - 4
ydb/core/statistics/database/database.cpp

@@ -32,7 +32,9 @@ public:
                     Col("data", NScheme::NTypeIds::String),
                 },
                 { "owner_id", "local_path_id", "stat_type", "column_tag"},
-                NKikimrServices::STATISTICS
+                NKikimrServices::STATISTICS,
+                Nothing(),
+                true
             )
         );
     }
@@ -77,7 +79,7 @@ private:
 public:
     TSaveStatisticsQuery(const TPathId& pathId, ui64 statType,
         std::vector<ui32>&& columnTags, std::vector<TString>&& data)
-        : NKikimr::TQueryBase(NKikimrServices::STATISTICS, {})
+        : NKikimr::TQueryBase(NKikimrServices::STATISTICS, {}, {}, true)
         , PathId(pathId)
         , StatType(statType)
         , ColumnTags(std::move(columnTags))
@@ -164,7 +166,7 @@ private:
 
 public:
     TLoadStatisticsQuery(const TPathId& pathId, ui64 statType, ui32 columnTag, ui64 cookie)
-        : NKikimr::TQueryBase(NKikimrServices::STATISTICS, {})
+        : NKikimr::TQueryBase(NKikimrServices::STATISTICS, {}, {}, true)
         , PathId(pathId)
         , StatType(statType)
         , ColumnTag(columnTag)
@@ -246,7 +248,7 @@ private:
 
 public:
     TDeleteStatisticsQuery(const TPathId& pathId)
-        : NKikimr::TQueryBase(NKikimrServices::STATISTICS, {})
+        : NKikimr::TQueryBase(NKikimrServices::STATISTICS, {}, {}, true)
         , PathId(pathId)
     {
     }

+ 37 - 0
ydb/core/statistics/database/ut/ut_database.cpp

@@ -3,6 +3,8 @@
 #include <ydb/core/statistics/events.h>
 #include <ydb/core/statistics/database/database.h>
 
+#include <ydb/public/sdk/cpp/client/ydb_table/table.h>
+
 #include <thread>
 
 namespace NKikimr::NStat {
@@ -87,6 +89,41 @@ Y_UNIT_TEST_SUITE(StatisticsSaveLoad) {
         auto loadResponseA = runtime.GrabEdgeEvent<TEvStatistics::TEvLoadStatisticsQueryResponse>(sender);
         UNIT_ASSERT(!loadResponseA->Get()->Success);
     }
+
+    Y_UNIT_TEST(ForbidAccess) {
+        TTestEnv env(1, 1);
+        auto init = [&] () {
+            CreateDatabase(env, "Database");
+            CreateUniformTable(env, "Database", "Table");
+        };
+        std::thread initThread(init);
+
+        auto& runtime = *env.GetServer().GetRuntime();
+        runtime.SimulateSleep(TDuration::Seconds(10));
+        initThread.join();
+
+        NYdb::EStatus status;
+        auto test = [&] () {
+            auto driverConfig = NYdb::TDriverConfig()
+                .SetEndpoint(env.GetEndpoint())
+                .SetAuthToken("user@builtin");
+            auto driver = NYdb::TDriver(driverConfig);
+            auto db = NYdb::NTable::TTableClient(driver);
+            auto session = db.CreateSession().GetValueSync().GetSession();
+
+            auto result = session.ExecuteDataQuery(R"(
+                SELECT * FROM `/Root/Database/.metadata/_statistics`;
+            )", NYdb::NTable::TTxControl::BeginTx().CommitTx()).ExtractValueSync();
+            status = result.GetStatus();
+        };
+        std::thread testThread(test);
+
+        runtime.SimulateSleep(TDuration::Seconds(10));
+        testThread.join();
+
+        UNIT_ASSERT_VALUES_EQUAL(status, NYdb::EStatus::SCHEME_ERROR);
+    }
+
 }
 
 } // NKikimr::NStat

+ 9 - 2
ydb/library/query_actor/query_actor.cpp

@@ -99,10 +99,11 @@ TQueryBase::TEvQueryBasePrivate::TEvCommitTransactionResponse::TEvCommitTransact
 
 //// TQueryBase
 
-TQueryBase::TQueryBase(ui64 logComponent, TString sessionId, TString database)
+TQueryBase::TQueryBase(ui64 logComponent, TString sessionId, TString database, bool isSystemUser)
     : LogComponent(logComponent)
     , Database(std::move(database))
     , SessionId(std::move(sessionId))
+    , IsSystemUser(isSystemUser)
 {}
 
 void TQueryBase::Registered(NActors::TActorSystem* sys, const NActors::TActorId& owner) {
@@ -221,7 +222,13 @@ void TQueryBase::RunDataQuery(const TString& sql, NYdb::TParamsBuilder* params,
         txControlProto->set_commit_tx(true);
     }
 
-    Subscribe<Table::ExecuteDataQueryResponse, TEvQueryBasePrivate::TEvDataQueryResult>(DoLocalRpc<TExecuteDataQueryRequest>(std::move(request), Database, Nothing(), TActivationContext::ActorSystem(), true));
+    TMaybe<TString> token = Nothing();
+    if (IsSystemUser) {
+        token = NACLib::TSystemUsers::Metadata().SerializeAsString();
+    }
+
+    Subscribe<Table::ExecuteDataQueryResponse, TEvQueryBasePrivate::TEvDataQueryResult>(
+        DoLocalRpc<TExecuteDataQueryRequest>(std::move(request), Database, token, TActivationContext::ActorSystem(), true));
 }
 
 void TQueryBase::Handle(TEvQueryBasePrivate::TEvDataQueryResult::TPtr& ev) {

+ 2 - 1
ydb/library/query_actor/query_actor.h

@@ -115,7 +115,7 @@ private:
 public:
     static constexpr char ActorName[] = "SQL_QUERY";
 
-    explicit TQueryBase(ui64 logComponent, TString sessionId = {}, TString database = {});
+    explicit TQueryBase(ui64 logComponent, TString sessionId = {}, TString database = {}, bool isSystemUser = false);
 
     void Bootstrap();
 
@@ -199,6 +199,7 @@ protected:
     const ui64 LogComponent;
     TString Database;
     TString SessionId;
+    bool IsSystemUser = false;
     TString TxId;
     bool DeleteSession = false;
     bool RunningQuery = false;

+ 12 - 5
ydb/library/table_creator/table_creator.cpp

@@ -34,12 +34,14 @@ public:
         TVector<NKikimrSchemeOp::TColumnDescription> columns,
         TVector<TString> keyColumns,
         NKikimrServices::EServiceKikimr logService,
-        TMaybe<NKikimrSchemeOp::TTTLSettings> ttlSettings = Nothing())
+        TMaybe<NKikimrSchemeOp::TTTLSettings> ttlSettings = Nothing(),
+        bool isSystemUser = false)
         : PathComponents(std::move(pathComponents))
         , Columns(std::move(columns))
         , KeyColumns(std::move(keyColumns))
         , LogService(logService)
         , TtlSettings(std::move(ttlSettings))
+        , IsSystemUser(isSystemUser)
         , LogPrefix("Table " + TableName() + " updater. ")
     {
         Y_ABORT_UNLESS(!PathComponents.empty());
@@ -86,8 +88,8 @@ public:
             pathComponents.emplace_back(PathComponents[i]);
         }
         modifyScheme.SetWorkingDir(CanonizePath(pathComponents));
-        LOG_DEBUG_S(*TlsActivationContext, LogService,
-            LogPrefix << "Full table path:" << modifyScheme.GetWorkingDir() << "/" << TableName());
+        TString fullPath = modifyScheme.GetWorkingDir() + "/" + TableName();
+        LOG_DEBUG_S(*TlsActivationContext, LogService, LogPrefix << "Full table path:" << fullPath);
         modifyScheme.SetOperationType(OperationType);
         modifyScheme.SetInternal(true);
         modifyScheme.SetAllowAccessToPrivatePaths(true);
@@ -108,6 +110,9 @@ public:
         if (TtlSettings) {
             tableDesc->MutableTTLSettings()->CopyFrom(*TtlSettings);
         }
+        if (IsSystemUser) {
+            request->Record.SetUserToken(NACLib::TSystemUsers::Metadata().SerializeAsString());
+        }
         Send(MakeTxProxyID(), std::move(request));
     }
 
@@ -376,6 +381,7 @@ private:
     const TVector<TString> KeyColumns;
     NKikimrServices::EServiceKikimr LogService;
     const TMaybe<NKikimrSchemeOp::TTTLSettings> TtlSettings;
+    bool IsSystemUser = false;
     NKikimrSchemeOp::EOperationType OperationType = NKikimrSchemeOp::EOperationType::ESchemeOpCreateTable;
     NActors::TActorId Owner;
     NActors::TActorId SchemePipeActorId;
@@ -473,10 +479,11 @@ NActors::IActor* CreateTableCreator(
     TVector<NKikimrSchemeOp::TColumnDescription> columns,
     TVector<TString> keyColumns,
     NKikimrServices::EServiceKikimr logService,
-    TMaybe<NKikimrSchemeOp::TTTLSettings> ttlSettings)
+    TMaybe<NKikimrSchemeOp::TTTLSettings> ttlSettings,
+    bool isSystemUser)
 {
     return new TTableCreator(std::move(pathComponents), std::move(columns),
-        std::move(keyColumns), logService, std::move(ttlSettings));
+        std::move(keyColumns), logService, std::move(ttlSettings), isSystemUser);
 }
 
 } // namespace NKikimr

+ 2 - 1
ydb/library/table_creator/table_creator.h

@@ -76,6 +76,7 @@ NActors::IActor* CreateTableCreator(
     TVector<NKikimrSchemeOp::TColumnDescription> columns,
     TVector<TString> keyColumns,
     NKikimrServices::EServiceKikimr logService,
-    TMaybe<NKikimrSchemeOp::TTTLSettings> ttlSettings = Nothing());
+    TMaybe<NKikimrSchemeOp::TTTLSettings> ttlSettings = Nothing(),
+    bool isSystemUser = false);
 
 } // namespace NKikimr