diff --git a/ydb/core/kqp/session_actor/kqp_query_state.cpp b/ydb/core/kqp/session_actor/kqp_query_state.cpp index 74571ae2f8cc..151f44b79708 100644 --- a/ydb/core/kqp/session_actor/kqp_query_state.cpp +++ b/ydb/core/kqp/session_actor/kqp_query_state.cpp @@ -260,11 +260,14 @@ std::unique_ptr TKqpQueryState::BuildSchemeC } bool TKqpQueryState::IsAccessDenied(const NSchemeCache::TSchemeCacheNavigate& response, TString& message) { - auto rights = NACLib::EAccessRights::ReadAttributes | NACLib::EAccessRights::WriteAttributes; + auto checkAccessDenied = [&] (const NSchemeCache::TSchemeCacheNavigate::TEntry& result) { + static const auto selectRowRights = NACLib::EAccessRights::SelectRow; + static const auto accessAttributesRights = NACLib::EAccessRights::ReadAttributes | NACLib::EAccessRights::WriteAttributes; + // in future check right UseConsumer + return result.SecurityObject && !(result.SecurityObject->CheckAccess(selectRowRights, *UserToken) || result.SecurityObject->CheckAccess(accessAttributesRights, *UserToken)); + }; // don't build message string on success path - bool denied = std::any_of(response.ResultSet.begin(), response.ResultSet.end(), [&] (auto& result) { - return result.SecurityObject && !result.SecurityObject->CheckAccess(rights, *UserToken); - }); + bool denied = std::any_of(response.ResultSet.begin(), response.ResultSet.end(), checkAccessDenied); if (!denied) { return false; @@ -277,7 +280,7 @@ bool TKqpQueryState::IsAccessDenied(const NSchemeCache::TSchemeCacheNavigate& re continue; } - if (result.SecurityObject && !result.SecurityObject->CheckAccess(rights, *UserToken)) { + if (checkAccessDenied(result)) { builder << " '" << JoinPath(result.Path) << "'"; } } diff --git a/ydb/core/testlib/test_pq_client.h b/ydb/core/testlib/test_pq_client.h index dbf150ec9d4c..d2fcfe117d47 100644 --- a/ydb/core/testlib/test_pq_client.h +++ b/ydb/core/testlib/test_pq_client.h @@ -879,8 +879,8 @@ class TFlatMsgBusPQClient : public NFlatTests::TFlatMsgBusClient { void GrantConsumerAccess(const TString& oldName, const TString& subj) { NACLib::TDiffACL acl; - acl.AddAccess(NACLib::EAccessType::Allow, NACLib::ReadAttributes, subj); - acl.AddAccess(NACLib::EAccessType::Allow, NACLib::WriteAttributes, subj); + // in future use right UseConsumer + acl.AddAccess(NACLib::EAccessType::Allow, NACLib::SelectRow, subj); auto name = NPersQueue::ConvertOldConsumerName(oldName); auto pos = name.rfind("/"); Y_ABORT_UNLESS(pos != TString::npos); diff --git a/ydb/services/persqueue_v1/actors/read_init_auth_actor.cpp b/ydb/services/persqueue_v1/actors/read_init_auth_actor.cpp index 460e4810b070..a2a7c0e6f069 100644 --- a/ydb/services/persqueue_v1/actors/read_init_auth_actor.cpp +++ b/ydb/services/persqueue_v1/actors/read_init_auth_actor.cpp @@ -238,10 +238,13 @@ void TReadInitAndAuthActor::HandleClientSchemeCacheResponse( return; } - NACLib::EAccessRights rights = (NACLib::EAccessRights)(NACLib::EAccessRights::ReadAttributes + NACLib::EAccessRights::WriteAttributes); - if ( - !CheckACLPermissionsForNavigate(entry.SecurityObject, path, rights, "No ReadAsConsumer permissions", ctx) - ) { + // in future use right UseConsumer + auto selectRowRights = NACLib::EAccessRights::SelectRow; + auto accessAttributesRights = NACLib::EAccessRights::ReadAttributes | NACLib::EAccessRights::WriteAttributes; + if (DoCheckACL && !(entry.SecurityObject->CheckAccess(selectRowRights, *Token) || entry.SecurityObject->CheckAccess(accessAttributesRights, *Token))) { + CloseSession(TStringBuilder() << "No ReadAsConsumer permissions" << " for '" << path + << "' for subject '" << Token->GetUserSID() << "'", + PersQueue::ErrorCode::ACCESS_DENIED, ctx); return; } FinishInitialization(ctx); diff --git a/ydb/services/persqueue_v1/ut/topic_service_ut.cpp b/ydb/services/persqueue_v1/ut/topic_service_ut.cpp index a0c9a0e0b70a..edd2d8166862 100644 --- a/ydb/services/persqueue_v1/ut/topic_service_ut.cpp +++ b/ydb/services/persqueue_v1/ut/topic_service_ut.cpp @@ -156,8 +156,8 @@ class TUpdateOffsetsInTransactionFixture : public NUnitTest::TBaseFixture { NACLib::TDiffACL acl; acl.AddAccess(NACLib::EAccessType::Allow, NACLib::DescribeSchema, AUTH_TOKEN); - acl.AddAccess(NACLib::EAccessType::Allow, NACLib::ReadAttributes, AUTH_TOKEN); - acl.AddAccess(NACLib::EAccessType::Allow, NACLib::WriteAttributes, AUTH_TOKEN); + // in future use right UseConsumer + acl.AddAccess(NACLib::EAccessType::Allow, NACLib::SelectRow, AUTH_TOKEN); server->AnnoyingClient->ModifyACL(TOPIC_PARENT, VALID_TOPIC_NAME, acl.SerializeAsString()); auto driverCfg = NYdb::TDriverConfig() @@ -328,7 +328,8 @@ Y_UNIT_TEST_F(AccessRights, TUpdateOffsetsInTransactionFixture) { UNIT_ASSERT_VALUES_EQUAL(response.operation().status(), Ydb::StatusIds::SUCCESS); NACLib::TDiffACL acl; - acl.RemoveAccess(NACLib::EAccessType::Allow, NACLib::ReadAttributes, AUTH_TOKEN); + // in future use right UseConsumer + acl.RemoveAccess(NACLib::EAccessType::Allow, NACLib::SelectRow, AUTH_TOKEN); server->AnnoyingClient->ModifyACL(TOPIC_PARENT, VALID_TOPIC_NAME, acl.SerializeAsString()); response = Call_UpdateOffsetsInTransaction({