Skip to content

25-1: security: fix query service for alter-login and domain_login_only #15405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ijon merged 2 commits into from
Mar 6, 2025
Merged

25-1: security: fix query service for alter-login and domain_login_only #15405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
babef42
security: fix query service for alter-login and domain_login_only (#1…
ijon Mar 6, 2025
b8c33c4
kqp: unify database selection logic for AlterLogin scheme operations …
ijon Mar 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 9 additions & 18 deletions ydb/core/kqp/executer_actor/kqp_scheme_executer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,15 +178,6 @@ class TKqpSchemeExecuter : public TActorBootstrapped<TKqpSchemeExecuter> {
});
}

TString GetDatabaseForLoginOperation() const {
const auto domainLoginOnly = AppData()->AuthConfig.GetDomainLoginOnly();
const auto domain = AppData()->DomainsInfo ? AppData()->DomainsInfo->GetDomain() : nullptr;
const auto domainName = domain ? domain->Name : "";
TString database;
return NSchemeHelpers::SetDatabaseForLoginOperation(database, domainLoginOnly, domainName, Database)
? database : Database;
}

void MakeSchemeOperationRequest() {
using TRequest = TEvTxUserProxy::TEvProposeTransaction;

Expand Down Expand Up @@ -248,21 +239,21 @@ class TKqpSchemeExecuter : public TActorBootstrapped<TKqpSchemeExecuter> {
case NKqpProto::TKqpSchemeOperation::kCreateUser: {
const auto& modifyScheme = schemeOp.GetCreateUser();
ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme);
ev->Record.SetDatabaseName(GetDatabaseForLoginOperation());
ev->Record.SetDatabaseName(NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(), Database));
break;
}

case NKqpProto::TKqpSchemeOperation::kAlterUser: {
const auto& modifyScheme = schemeOp.GetAlterUser();
ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme);
ev->Record.SetDatabaseName(GetDatabaseForLoginOperation());
ev->Record.SetDatabaseName(NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(), Database));
break;
}

case NKqpProto::TKqpSchemeOperation::kDropUser: {
const auto& modifyScheme = schemeOp.GetDropUser();
ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme);
ev->Record.SetDatabaseName(GetDatabaseForLoginOperation());
ev->Record.SetDatabaseName(NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(), Database));
break;
}
case NKqpProto::TKqpSchemeOperation::kCreateExternalTable: {
Expand All @@ -284,35 +275,35 @@ class TKqpSchemeExecuter : public TActorBootstrapped<TKqpSchemeExecuter> {
case NKqpProto::TKqpSchemeOperation::kCreateGroup: {
const auto& modifyScheme = schemeOp.GetCreateGroup();
ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme);
ev->Record.SetDatabaseName(GetDatabaseForLoginOperation());
ev->Record.SetDatabaseName(NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(), Database));
break;
}

case NKqpProto::TKqpSchemeOperation::kAddGroupMembership: {
const auto& modifyScheme = schemeOp.GetAddGroupMembership();
ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme);
ev->Record.SetDatabaseName(GetDatabaseForLoginOperation());
ev->Record.SetDatabaseName(NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(), Database));
break;
}

case NKqpProto::TKqpSchemeOperation::kRemoveGroupMembership: {
const auto& modifyScheme = schemeOp.GetRemoveGroupMembership();
ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme);
ev->Record.SetDatabaseName(GetDatabaseForLoginOperation());
ev->Record.SetDatabaseName(NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(), Database));
break;
}

case NKqpProto::TKqpSchemeOperation::kRenameGroup: {
const auto& modifyScheme = schemeOp.GetRenameGroup();
ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme);
ev->Record.SetDatabaseName(GetDatabaseForLoginOperation());
ev->Record.SetDatabaseName(NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(), Database));
break;
}

case NKqpProto::TKqpSchemeOperation::kDropGroup: {
const auto& modifyScheme = schemeOp.GetDropGroup();
ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme);
ev->Record.SetDatabaseName(GetDatabaseForLoginOperation());
ev->Record.SetDatabaseName(NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(), Database));
break;
}

Expand Down Expand Up @@ -619,7 +610,7 @@ class TKqpSchemeExecuter : public TActorBootstrapped<TKqpSchemeExecuter> {
<< "Error creating temporary directory: "
<< result->Get()->Result.Issues().ToString(true));
}

CreateSessionDirectory();
}

Expand Down
2 changes: 0 additions & 2 deletions ydb/core/kqp/gateway/kqp_gateway.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,6 @@ class IKqpGateway : public NYql::IKikimrGateway {
public:
virtual TString GetDatabase() = 0;
virtual TString GetDatabaseId() = 0;
virtual bool GetDomainLoginOnly() = 0;
virtual TMaybe<TString> GetDomainName() = 0;

/* Scheme */
virtual NThreading::TFuture<TKqpTableProfilesResult> GetTableProfiles() = 0;
Expand Down
51 changes: 16 additions & 35 deletions ydb/core/kqp/gateway/kqp_ic_gateway.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,21 +773,6 @@ class TKikimrIcGateway : public IKqpGateway {
ClientAddress = clientAddress;
}

bool GetDomainLoginOnly() override {
TAppData* appData = AppData(ActorSystem);
return appData && appData->AuthConfig.GetDomainLoginOnly();
}

TMaybe<TString> GetDomainName() override {
TAppData* appData = AppData(ActorSystem);
if (GetDomainLoginOnly()) {
if (appData->DomainsInfo && appData->DomainsInfo->Domain) {
return appData->DomainsInfo->GetDomain()->Name;
}
}
return {};
}

TVector<NKikimrKqp::TKqpTableMetadataProto> GetCollectedSchemeData() override {
return MetadataLoader->GetCollectedSchemeData();
}
Expand Down Expand Up @@ -1361,8 +1346,8 @@ class TKikimrIcGateway : public IKqpGateway {
return InvalidCluster<TGenericResult>(cluster);
}

TString database;
if (!GetDatabaseForLoginOperation(database)) {
TString database = NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(ActorSystem), Database);
if (database.empty()) {
return MakeFuture(ResultFromError<TGenericResult>("Couldn't get domain name"));
}

Expand Down Expand Up @@ -1407,8 +1392,8 @@ class TKikimrIcGateway : public IKqpGateway {
return InvalidCluster<TGenericResult>(cluster);
}

TString database;
if (!GetDatabaseForLoginOperation(database)) {
TString database = NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(ActorSystem), Database);
if (database.empty()) {
return MakeFuture(ResultFromError<TGenericResult>("Couldn't get domain name"));
}

Expand Down Expand Up @@ -1456,8 +1441,8 @@ class TKikimrIcGateway : public IKqpGateway {
return InvalidCluster<TGenericResult>(cluster);
}

TString database;
if (!GetDatabaseForLoginOperation(database)) {
TString database = NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(ActorSystem), Database);
if (database.empty()) {
return MakeFuture(ResultFromError<TGenericResult>("Couldn't get domain name"));
}

Expand Down Expand Up @@ -1532,8 +1517,8 @@ class TKikimrIcGateway : public IKqpGateway {
if (!Owner.CheckCluster(cluster)) {
return InvalidCluster<TGenericResult>(cluster);
}
TString database;
if (!Owner.GetDatabaseForLoginOperation(database)) {
const auto appData = AppData(Owner.ActorSystem);
if (!(appData && appData->DomainsInfo && appData->DomainsInfo->Domain)) {
return MakeFuture(ResultFromError<TGenericResult>("Couldn't get domain name"));
}
NMetadata::IClassBehaviour::TPtr cBehaviour(NMetadata::IClassBehaviour::TFactory::Construct(settings.GetTypeId()));
Expand Down Expand Up @@ -1651,8 +1636,8 @@ class TKikimrIcGateway : public IKqpGateway {
return InvalidCluster<TGenericResult>(cluster);
}

TString database;
if (!GetDatabaseForLoginOperation(database)) {
TString database = NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(ActorSystem), Database);
if (database.empty()) {
return MakeFuture(ResultFromError<TGenericResult>("Couldn't get domain name"));
}

Expand Down Expand Up @@ -1691,8 +1676,8 @@ class TKikimrIcGateway : public IKqpGateway {
return InvalidCluster<TGenericResult>(cluster);
}

TString database;
if (!GetDatabaseForLoginOperation(database)) {
TString database = NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(ActorSystem), Database);
if (database.empty()) {
return MakeFuture(ResultFromError<TGenericResult>("Couldn't get domain name"));
}

Expand Down Expand Up @@ -1783,8 +1768,8 @@ class TKikimrIcGateway : public IKqpGateway {
return InvalidCluster<TGenericResult>(cluster);
}

TString database;
if (!GetDatabaseForLoginOperation(database)) {
TString database = NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(ActorSystem), Database);
if (database.empty()) {
return MakeFuture(ResultFromError<TGenericResult>("Couldn't get domain name"));
}

Expand Down Expand Up @@ -1824,8 +1809,8 @@ class TKikimrIcGateway : public IKqpGateway {
return InvalidCluster<TGenericResult>(cluster);
}

TString database;
if (!GetDatabaseForLoginOperation(database)) {
TString database = NSchemeHelpers::SelectDatabaseForAlterLoginOperations(AppData(ActorSystem), Database);
if (database.empty()) {
return MakeFuture(ResultFromError<TGenericResult>("Couldn't get domain name"));
}

Expand Down Expand Up @@ -2331,10 +2316,6 @@ class TKikimrIcGateway : public IKqpGateway {
return cluster == Cluster;
}

bool GetDatabaseForLoginOperation(TString& database) {
return NSchemeHelpers::SetDatabaseForLoginOperation(database, GetDomainLoginOnly(), GetDomainName(), GetDatabase());
}

bool GetPathPair(const TString& tableName, std::pair<TString, TString>& pathPair,
TString& error, bool createDir)
{
Expand Down
26 changes: 19 additions & 7 deletions ydb/core/kqp/gateway/utils/scheme_helpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
#include "scheme_helpers.h"

#include <ydb/core/base/appdata.h>
#include <ydb/core/base/path.h>
#include <ydb/core/base/table_index.h>
#include <ydb/core/protos/external_sources.pb.h>
#include <ydb/core/protos/auth.pb.h>

namespace NKikimr::NKqp::NSchemeHelpers {

Expand Down Expand Up @@ -56,14 +58,24 @@ TVector<TString> CreateIndexTablePath(const TString& tableName, const NYql::TInd
return paths;
}

bool SetDatabaseForLoginOperation(TString& result, bool getDomainLoginOnly, TMaybe<TString> domainName,
const TString& database)
{
if (getDomainLoginOnly && !domainName) {
return false;
TString GetDomainDatabase(const TAppData* appData) {
if (appData->DomainsInfo && appData->DomainsInfo->Domain) {
if (const auto& name = appData->DomainsInfo->GetDomain()->Name) {
return "/" + name;
}
}
return {};
}

// DomainLoginOnly setting determine what database should handle user|group administration operations (AlterLogin).
// DomainLoginOnly = false -- database where request is directed to
// DomainLoginOnly = true -- domain (root) database
TString SelectDatabaseForAlterLoginOperations(const TAppData* appData, const TString& requestDatabase) {
if (appData->AuthConfig.GetDomainLoginOnly()) {
return GetDomainDatabase(appData);
} else {
return requestDatabase;
}
result = domainName ? "/" + *domainName : database;
return true;
}

void FillCreateExternalTableColumnDesc(NKikimrSchemeOp::TExternalTableDescription& externalTableDesc,
Expand Down
6 changes: 4 additions & 2 deletions ydb/core/kqp/gateway/utils/scheme_helpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once
#include <ydb/core/kqp/provider/yql_kikimr_gateway.h>
#include <ydb/core/protos/flat_scheme_op.pb.h>
#include <ydb/core/base/appdata_fwd.h>

#include <util/generic/string.h>
#include <util/string/join.h>
Expand All @@ -24,8 +25,9 @@ bool SplitTablePath(const TString& tableName, const TString& database, std::pair

TVector<TString> CreateIndexTablePath(const TString& tableName, const NYql::TIndexDescription& index);

bool SetDatabaseForLoginOperation(TString& result, bool getDomainLoginOnly, TMaybe<TString> domainName,
const TString& database);
TString GetDomainDatabase(const TAppData* appData);

TString SelectDatabaseForAlterLoginOperations(const TAppData* appData, const TString& requestDatabase);

void FillCreateExternalTableColumnDesc(NKikimrSchemeOp::TExternalTableDescription& externalTableDesc,
const TString& name,
Expand Down
Loading
Loading