Skip to content

Commit 30160e5

Browse files
UgnineSirdisUgnineSirdis
committed
Fixes + test
1 parent d44809f commit 30160e5

File tree

10 files changed

+189
-25
lines changed

10 files changed

+189
-25
lines changed

‎ydb/core/grpc_services/grpc_request_check_actor.h‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ class TGrpcRequestCheckActor
8787
}
8888

8989
void ProcessCommonAttributes(const TSchemeBoardEvents::TDescribeSchemeResult& schemeData) {
90+
TVector<TEvTicketParser::TEvAuthorizeTicket::TEntry> entries;
9091
static std::vector<TString> allowedAttributes = {"folder_id", "service_account_id", "database_id"};
9192
TVector<std::pair<TString, TString>> attributes;
9293
attributes.reserve(schemeData.GetPathDescription().UserAttributesSize());
@@ -96,16 +97,16 @@ class TGrpcRequestCheckActor
9697
}
9798
}
9899
if (!attributes.empty()) {
99-
SetEntries({{GetPermissions(), attributes}});
100-
} else {
101-
if constexpr (std::is_same_v<TEvent, TEvRequestAuthAndCheck>) {
102-
if (!Request_->Get()->GetDatabaseName()) {
103-
const auto& entries = GetEntriesForAuthAndCheckRequest(Request_);
104-
if (!entries.empty()) {
105-
SetEntries(entries);
106-
}
107-
}
108-
}
100+
entries.emplace_back(GetPermissions(), attributes);
101+
}
102+
103+
if constexpr (std::is_same_v<TEvent, TEvRequestAuthAndCheck>) {
104+
const auto& e = GetEntriesForAuthAndCheckRequest(Request_);
105+
entries.insert(entries.end(), e.begin(), e.end());
106+
}
107+
108+
if (!entries.empty()) {
109+
SetEntries(entries);
109110
}
110111
}
111112

‎ydb/core/mon/mon.cpp‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,29 @@
99
#include <library/cpp/json/json_value.h>
1010
#include <library/cpp/json/json_reader.h>
1111

12+
#include <util/string/ascii.h>
13+
1214
namespace NActors {
1315

1416
using namespace NMonitoring;
1517
using namespace NKikimr;
1618

1719
namespace {
1820

21+
bool HasJsonContent(NMonitoring::IMonHttpRequest& request) {
22+
const TStringBuf header = request.GetHeader("Content-Type");
23+
return header.empty() || AsciiEqualsIgnoreCase(header, "application/json"); // by default we will try to parse json, no error will be generated if parsing fails
24+
}
25+
1926
TString GetDatabase(NMonitoring::IMonHttpRequest& request) {
2027
if (const auto dbIt = request.GetParams().Find("database"); dbIt != request.GetParams().end()) {
2128
return dbIt->second;
2229
}
23-
if (request.GetMethod() == HTTP_METHOD_POST) {
30+
if (request.GetMethod() == HTTP_METHOD_POST && HasJsonContent(request)) {
2431
static NJson::TJsonReaderConfig JsonConfig;
2532
NJson::TJsonValue requestData;
2633
if (NJson::ReadJsonTree(request.GetPostContent(), &JsonConfig, &requestData)) {
27-
return requestData["database"].GetString();
34+
return requestData["database"].GetString(); // empty if not string or no such key
2835
}
2936
}
3037
return {};

‎ydb/core/testlib/actors/test_runtime.cpp‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <ydb/core/base/blobstorage.h>
55
#include <ydb/core/base/counters.h>
66
#include <ydb/core/mon/sync_http_mon.h>
7+
#include <ydb/core/mon/async_http_mon.h>
78
#include <ydb/core/mon_alloc/profiler.h>
89
#include <ydb/core/tablet/tablet_impl.h>
910

@@ -175,11 +176,19 @@ namespace NActors {
175176

176177
if (NeedMonitoring && !SingleSysEnv) {
177178
ui16 port = MonitoringPortOffset ? MonitoringPortOffset + nodeIndex : GetPortManager().GetPort();
178-
node->Mon.Reset(new NActors::TSyncHttpMon({
179-
.Port = port,
180-
.Threads = 10,
181-
.Title = "KIKIMR monitoring"
182-
}));
179+
if (MonitoringTypeAsync) {
180+
node->Mon.Reset(new NActors::TAsyncHttpMon({
181+
.Port = port,
182+
.Threads = 10,
183+
.Title = "KIKIMR monitoring"
184+
}));
185+
} else {
186+
node->Mon.Reset(new NActors::TSyncHttpMon({
187+
.Port = port,
188+
.Threads = 10,
189+
.Title = "KIKIMR monitoring"
190+
}));
191+
}
183192
nodeAppData->Mon = node->Mon.Get();
184193
node->Mon->RegisterCountersPage("counters", "Counters", node->DynamicCounters);
185194
auto actorsMonPage = node->Mon->RegisterIndexPage("actors", "Actors");
@@ -191,7 +200,7 @@ namespace NActors {
191200

192201
node->ActorSystem->Start();
193202
if (nodeAppData->Mon) {
194-
nodeAppData->Mon->Start();
203+
nodeAppData->Mon->Start(node->ActorSystem.Get());
195204
}
196205
}
197206

‎ydb/core/testlib/test_client.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ namespace Tests {
225225

226226
NKikimr::SetupChannelProfiles(app);
227227

228-
Runtime->SetupMonitoring(Settings->MonitoringPortOffset);
228+
Runtime->SetupMonitoring(Settings->MonitoringPortOffset, Settings->MonitoringTypeAsync);
229229
Runtime->SetLogBackend(Settings->LogBackend);
230230

231231
Runtime->AddAppDataInit([this](ui32 nodeIdx, NKikimr::TAppData& appData) {

‎ydb/core/testlib/test_client.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ namespace Tests {
108108
ui16 GrpcPort = 0;
109109
int GrpcMaxMessageSize = 0; // 0 - default (4_MB), -1 - no limit
110110
ui16 MonitoringPortOffset = 0;
111+
bool MonitoringTypeAsync = false;
111112
NKikimrProto::TAuthConfig AuthConfig;
112113
NKikimrPQ::TPQConfig PQConfig;
113114
NKikimrPQ::TPQClusterDiscoveryConfig PQClusterDiscoveryConfig;
@@ -162,7 +163,7 @@ namespace Tests {
162163

163164
TServerSettings& SetGrpcPort(ui16 value) { GrpcPort = value; return *this; }
164165
TServerSettings& SetGrpcMaxMessageSize(int value) { GrpcMaxMessageSize = value; return *this; }
165-
TServerSettings& SetMonitoringPortOffset(ui16 value) { MonitoringPortOffset = value; return *this; }
166+
TServerSettings& SetMonitoringPortOffset(ui16 value, bool monitoringTypeAsync = false) { MonitoringPortOffset = value; MonitoringTypeAsync = monitoringTypeAsync; return *this; }
166167
TServerSettings& SetSupportsRedirect(bool value) { SupportsRedirect = value; return *this; }
167168
TServerSettings& SetTracePath(const TString& value) { TracePath = value; return *this; }
168169
TServerSettings& SetDomain(ui32 value) { Domain = value; return *this; }

‎ydb/core/viewer/json_query.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ class TJsonQuery : public TViewerPipeClient<TJsonQuery> {
122122
if (IsPostContent()) {
123123
TStringBuf content = Event->Get()->Request.GetPostContent();
124124
if (!ParsePostContent(content)) {
125-
return ReplyAndPassAway(Viewer->GetHTTPBADREQUEST(Event->Get(), "text/plain", "Bad content recieved"));
125+
return ReplyAndPassAway(Viewer->GetHTTPBADREQUEST(Event->Get(), "text/plain", "Bad content received"));
126126
}
127127
}
128128
if (Query.empty() && Action != "cancel-query") {

‎ydb/core/viewer/ut/ya.make‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ UNITTEST_FOR(ydb/core/viewer)
22

33
FORK_SUBTESTS()
44

5-
TIMEOUT(600)
5+
TIMEOUT(30)
66

77
SIZE(MEDIUM)
88

@@ -13,6 +13,8 @@ SRCS(
1313
)
1414

1515
PEERDIR(
16+
library/cpp/http/misc
17+
library/cpp/http/simple
1618
ydb/core/testlib/default
1719
)
1820

‎ydb/core/viewer/viewer_ut.cpp‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
#include <library/cpp/testing/unittest/tests_data.h>
33
#include <ydb/library/actors/interconnect/interconnect.h>
44
#include <ydb/library/actors/helpers/selfping_actor.h>
5+
#include <library/cpp/http/misc/httpcodes.h>
6+
#include <library/cpp/http/simple/http_client.h>
57
#include <library/cpp/json/json_value.h>
68
#include <library/cpp/json/json_reader.h>
79
#include <util/stream/null.h>
@@ -23,6 +25,8 @@
2325
#include <ydb/core/node_whiteboard/node_whiteboard.h>
2426
#include <ydb/library/actors/core/interconnect.h>
2527

28+
#include <util/string/builder.h>
29+
2630
using namespace NKikimr;
2731
using namespace NViewer;
2832
using namespace NKikimrWhiteboard;
@@ -1503,4 +1507,142 @@ Y_UNIT_TEST_SUITE(Viewer) {
15031507
Y_UNIT_TEST(JsonStorageListingV2PDiskIdFilter) {
15041508
JsonStorage9Nodes9GroupsListingTest("v2", false, true, true, 4, 8);
15051509
}
1510+
1511+
struct TFakeTicketParserActor : public TActor<TFakeTicketParserActor> {
1512+
TFakeTicketParserActor()
1513+
: TActor<TFakeTicketParserActor>(&TFakeTicketParserActor::StFunc)
1514+
{}
1515+
1516+
STFUNC(StFunc) {
1517+
switch (ev->GetTypeRewrite()) {
1518+
hFunc(TEvTicketParser::TEvAuthorizeTicket, Handle);
1519+
default:
1520+
break;
1521+
}
1522+
}
1523+
1524+
void Handle(TEvTicketParser::TEvAuthorizeTicket::TPtr& ev) {
1525+
LOG_INFO_S(*TlsActivationContext, NKikimrServices::TICKET_PARSER, "Ticket parser: got TEvAuthorizeTicket event: " << ev->Get()->Ticket << " " << ev->Get()->Database << " " << ev->Get()->Entries.size());
1526+
++AuthorizeTicketRequests;
1527+
1528+
if (ev->Get()->Database != "/Root") {
1529+
Fail(ev, TStringBuilder() << "Incorrect database " << ev->Get()->Database);
1530+
return;
1531+
}
1532+
1533+
if (ev->Get()->Ticket != "test_ydb_token") {
1534+
Fail(ev, TStringBuilder() << "Incorrect token " << ev->Get()->Ticket);
1535+
return;
1536+
}
1537+
1538+
bool databaseIdFound = false;
1539+
bool folderIdFound = false;
1540+
for (const TEvTicketParser::TEvAuthorizeTicket::TEntry& entry : ev->Get()->Entries) {
1541+
for (const std::pair<TString, TString>& attr : entry.Attributes) {
1542+
if (attr.first == "database_id") {
1543+
databaseIdFound = true;
1544+
if (attr.second != "test_database_id") {
1545+
Fail(ev, TStringBuilder() << "Incorrect database_id " << attr.second);
1546+
return;
1547+
}
1548+
} else if (attr.first == "folder_id") {
1549+
folderIdFound = true;
1550+
if (attr.second != "test_folder_id") {
1551+
Fail(ev, TStringBuilder() << "Incorrect folder_id " << attr.second);
1552+
return;
1553+
}
1554+
}
1555+
}
1556+
}
1557+
if (!databaseIdFound) {
1558+
Fail(ev, "database_id not found");
1559+
return;
1560+
}
1561+
if (!folderIdFound) {
1562+
Fail(ev, "folder_id not found");
1563+
return;
1564+
}
1565+
1566+
Success(ev);
1567+
}
1568+
1569+
void Fail(TEvTicketParser::TEvAuthorizeTicket::TPtr& ev, const TString& message) {
1570+
++AuthorizeTicketFails;
1571+
TEvTicketParser::TError err;
1572+
err.Retryable = false;
1573+
err.Message = message ? message : "Test error";
1574+
LOG_INFO_S(*TlsActivationContext, NKikimrServices::TICKET_PARSER, "Send TEvAuthorizeTicketResult: " << err.Message);
1575+
Send(ev->Sender, new TEvTicketParser::TEvAuthorizeTicketResult(ev->Get()->Ticket, err));
1576+
}
1577+
1578+
void Success(TEvTicketParser::TEvAuthorizeTicket::TPtr& ev) {
1579+
++AuthorizeTicketSuccesses;
1580+
NACLib::TUserToken::TUserTokenInitFields args;
1581+
args.UserSID = "user_name";
1582+
args.GroupSIDs.push_back("group_name");
1583+
TIntrusivePtr<NACLib::TUserToken> userToken = MakeIntrusive<NACLib::TUserToken>(args);
1584+
LOG_INFO_S(*TlsActivationContext, NKikimrServices::TICKET_PARSER, "Send TEvAuthorizeTicketResult success");
1585+
Send(ev->Sender, new TEvTicketParser::TEvAuthorizeTicketResult(ev->Get()->Ticket, userToken));
1586+
}
1587+
1588+
size_t AuthorizeTicketRequests = 0;
1589+
size_t AuthorizeTicketSuccesses = 0;
1590+
size_t AuthorizeTicketFails = 0;
1591+
};
1592+
1593+
Y_UNIT_TEST(AuthorizeYdbTokenWithDatabaseAttributes) {
1594+
TPortManager tp;
1595+
ui16 port = tp.GetPort(2134);
1596+
ui16 grpcPort = tp.GetPort(2135);
1597+
ui16 monPort = tp.GetPort(8765);
1598+
auto settings = TServerSettings(port);
1599+
settings.InitKikimrRunConfig()
1600+
.SetNodeCount(1)
1601+
.SetUseRealThreads(true)
1602+
.SetDomainName("Root")
1603+
.SetMonitoringPortOffset(monPort, true); // authorization is implemented only in async mon
1604+
1605+
auto& securityConfig = *settings.AppConfig->MutableDomainsConfig()->MutableSecurityConfig();
1606+
securityConfig.SetEnforceUserTokenCheckRequirement(true);
1607+
1608+
TFakeTicketParserActor* ticketParser = nullptr;
1609+
settings.CreateTicketParser = [&](const TTicketParserSettings&) -> IActor* {
1610+
ticketParser = new TFakeTicketParserActor();
1611+
return ticketParser;
1612+
};
1613+
1614+
TServer server(settings);
1615+
server.EnableGRpc(grpcPort);
1616+
TClient client(settings);
1617+
1618+
const auto alterAttrsStatus = client.AlterUserAttributes("/", "Root", {
1619+
{ "folder_id", "test_folder_id" },
1620+
{ "database_id", "test_database_id" },
1621+
});
1622+
UNIT_ASSERT_EQUAL(alterAttrsStatus, NMsgBusProxy::MSTATUS_OK);
1623+
1624+
TTestActorRuntime& runtime = *server.GetRuntime();
1625+
runtime.SetLogPriority(NKikimrServices::GRPC_SERVER, NLog::PRI_TRACE);
1626+
runtime.SetLogPriority(NKikimrServices::TICKET_PARSER, NLog::PRI_TRACE);
1627+
1628+
TKeepAliveHttpClient httpClient("localhost", monPort);
1629+
TStringStream responseStream;
1630+
TKeepAliveHttpClient::THeaders headers;
1631+
headers["Content-Type"] = "application/json";
1632+
headers["Authorization"] = "test_ydb_token";
1633+
TString requestBody = R"json({
1634+
"query": "SELECT 42;",
1635+
"database": "/Root",
1636+
"action": "execute-script",
1637+
"syntax": "yql_v1",
1638+
"stats": "profile"
1639+
})json";
1640+
const TKeepAliveHttpClient::THttpCode statusCode = httpClient.DoPost("/viewer/query?timeout=600000&base64=false&schema=modern", requestBody, &responseStream, headers);
1641+
const TString response = responseStream.ReadAll();
1642+
UNIT_ASSERT_EQUAL_C(statusCode, HTTP_OK, statusCode << ": " << response);
1643+
1644+
UNIT_ASSERT(ticketParser);
1645+
UNIT_ASSERT_VALUES_EQUAL_C(ticketParser->AuthorizeTicketRequests, 1, response);
1646+
UNIT_ASSERT_VALUES_EQUAL_C(ticketParser->AuthorizeTicketSuccesses, 1, response);
1647+
}
15061648
}

‎ydb/library/actors/testlib/test_runtime.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1594,9 +1594,10 @@ namespace NActors {
15941594
return node->DynamicCounters;
15951595
}
15961596

1597-
void TTestActorRuntimeBase::SetupMonitoring(ui16 monitoringPortOffset) {
1597+
void TTestActorRuntimeBase::SetupMonitoring(ui16 monitoringPortOffset, bool monitoringTypeAsync) {
15981598
NeedMonitoring = true;
15991599
MonitoringPortOffset = monitoringPortOffset;
1600+
MonitoringTypeAsync = monitoringTypeAsync;
16001601
}
16011602

16021603
void TTestActorRuntimeBase::SendInternal(TAutoPtr<IEventHandle> ev, ui32 nodeIndex, bool viaActorSystem) {

‎ydb/library/actors/testlib/test_runtime.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ namespace NActors {
295295
void EnableScheduleForActor(const TActorId& actorId, bool allow = true);
296296
bool IsScheduleForActorEnabled(const TActorId& actorId) const;
297297
TIntrusivePtr<NMonitoring::TDynamicCounters> GetDynamicCounters(ui32 nodeIndex = 0);
298-
void SetupMonitoring(ui16 monitoringPortOffset = 0);
298+
void SetupMonitoring(ui16 monitoringPortOffset = 0, bool monitoringTypeAsync = false);
299299

300300
using TEventObserverCollection = std::list<std::function<void(TAutoPtr<IEventHandle>& event)>>;
301301
class TEventObserverHolder {
@@ -321,7 +321,7 @@ namespace NActors {
321321
if (this != &other)
322322
{
323323
Remove();
324-
324+
325325
List = std::move(other.List);
326326
Iter = std::move(other.Iter);
327327

@@ -656,6 +656,7 @@ namespace NActors {
656656
TAutoPtr<TLogBackend> LogBackend;
657657
bool NeedMonitoring;
658658
ui16 MonitoringPortOffset = 0;
659+
bool MonitoringTypeAsync = false;
659660

660661
TIntrusivePtr<IRandomProvider> RandomProvider;
661662
TIntrusivePtr<ITimeProvider> TimeProvider;

0 commit comments

Comments
 (0)