Skip to content

Commit

Permalink
Add account_type parameter for audit logs (ydb-platform#13269)
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewstalin authored Jan 15, 2025
1 parent b7192f6 commit 179f412
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 9 deletions.
6 changes: 5 additions & 1 deletion ydb/core/grpc_services/audit_logins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@
namespace NKikimr {
namespace NGRpcService {

void AuditLogLogin(IAuditCtx* ctx, const TString& database, const Ydb::Auth::LoginRequest& request, const Ydb::Auth::LoginResponse& response, const TString& errorDetails, const TString& sanitizedToken)
void AuditLogLogin(IAuditCtx* ctx, const TString& database, const Ydb::Auth::LoginRequest& request,
const Ydb::Auth::LoginResponse& response, const TString& errorDetails,
const TString& sanitizedToken, bool isAdmin)
{
static const TString GrpcLoginComponentName = "grpc-login";
static const TString LoginOperationName = "LOGIN";
static const TString AdminAccountType = "admin";

//NOTE: EmptyValue couldn't be an empty string as AUDIT_PART() skips parts with an empty values
static const TString EmptyValue = "{none}";
Expand Down Expand Up @@ -50,6 +53,7 @@ void AuditLogLogin(IAuditCtx* ctx, const TString& database, const Ydb::Auth::Log
// Login
AUDIT_PART("login_user", (!request.user().empty() ? request.user() : EmptyValue))
AUDIT_PART("sanitized_token", (!sanitizedToken.empty() ? sanitizedToken : EmptyValue))
AUDIT_PART("account_type", AdminAccountType, (isAdmin && status == Ydb::StatusIds::SUCCESS))

//TODO: (?) it is possible to show masked version of the resulting token here
);
Expand Down
4 changes: 3 additions & 1 deletion ydb/core/grpc_services/audit_logins.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ namespace NKikimr::NGRpcService {
class IAuditCtx;

// logins log
void AuditLogLogin(IAuditCtx* ctx, const TString& database, const Ydb::Auth::LoginRequest& request, const Ydb::Auth::LoginResponse& response, const TString& errorDetails, const TString& sanitizedToken);
void AuditLogLogin(IAuditCtx* ctx, const TString& database, const Ydb::Auth::LoginRequest& request,
const Ydb::Auth::LoginResponse& response, const TString& errorDetails,
const TString& sanitizedToken, bool isAdmin = false);

} // namespace NKikimr::NGRpcService
14 changes: 10 additions & 4 deletions ydb/core/grpc_services/rpc_login.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
#include <ydb/core/security/ldap_auth_provider/ldap_auth_provider.h>
#include <ydb/core/security/login_shared_func.h>

#include <ydb/library/aclib/aclib.h>
#include <ydb/library/login/login.h>

#include <ydb/public/api/protos/ydb_auth.pb.h>

namespace NKikimr {
Expand Down Expand Up @@ -88,7 +91,7 @@ class TLoginRPC : public TRpcRequestActor<TLoginRPC, TEvLoginRequest, true> {
ReplyErrorAndPassAway(Ydb::StatusIds::INTERNAL_ERROR, "Failed to produce a token");
} else {
// success = token + no errors
ReplyAndPassAway(loginResult.GetToken(), loginResult.GetSanitizedToken());
ReplyAndPassAway(loginResult);
}
}

Expand All @@ -113,9 +116,12 @@ class TLoginRPC : public TRpcRequestActor<TLoginRPC, TEvLoginRequest, true> {
}
}

void ReplyAndPassAway(const TString& resultToken, const TString& sanitizedToken) {
TResponse response;
void ReplyAndPassAway(const NKikimrScheme::TEvLoginResult& loginResult) {
const auto& resultToken = loginResult.GetToken();
const auto& sanitizedToken = loginResult.GetSanitizedToken();
const auto& isAdmin = loginResult.GetIsAdmin();

TResponse response;
Ydb::Operations::Operation& operation = *response.mutable_operation();
operation.set_ready(true);
operation.set_status(Ydb::StatusIds::SUCCESS);
Expand All @@ -126,7 +132,7 @@ class TLoginRPC : public TRpcRequestActor<TLoginRPC, TEvLoginRequest, true> {
operation.mutable_result()->PackFrom(result);
}

AuditLogLogin(Request.Get(), PathToDatabase, *GetProtoRequest(), response, /* errorDetails */ TString(), sanitizedToken);
AuditLogLogin(Request.Get(), PathToDatabase, *GetProtoRequest(), response, /* errorDetails */ TString(), sanitizedToken, isAdmin);

return CleanupAndReply(response);
}
Expand Down
1 change: 1 addition & 0 deletions ydb/core/protos/flat_tx_scheme.proto
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ message TEvLoginResult {
optional string Error = 1;
optional string Token = 2; // signed jwt token
optional string SanitizedToken = 3; // sanitized token without signature
optional bool IsAdmin = 4;
}

// Sending actor registers itself to be notified when tx completes
Expand Down
18 changes: 18 additions & 0 deletions ydb/core/tx/schemeshard/schemeshard__login.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,22 @@ struct TSchemeShard::TTxLogin : TTransactionBase<TSchemeShard> {
}

private:
bool IsAdmin() const {
const auto& adminSids = AppData()->AdministrationAllowedSIDs;
if (adminSids.empty()) {
return true;
}

const auto& user = Request->Get()->Record.GetUser();
const auto providerGroups = Self->LoginProvider.GetGroupsMembership(user);
const TVector<NACLib::TSID> groups(providerGroups.begin(), providerGroups.end());
const auto userToken = NACLib::TUserToken(user, groups);
auto hasSid = [&userToken](const TString& sid) -> bool {
return userToken.IsExist(sid);
};
return std::find_if(adminSids.begin(), adminSids.end(), hasSid) != adminSids.end();
}

bool LoginAttempt(NIceDb::TNiceDb& db, const TActorContext& ctx) {
const auto& loginRequest = GetLoginRequest();
if (!loginRequest.ExternalAuth && !AppData(ctx)->AuthConfig.GetEnableLoginAuthentication()) {
Expand All @@ -105,6 +121,7 @@ struct TSchemeShard::TTxLogin : TTransactionBase<TSchemeShard> {
case NLogin::TLoginProvider::TLoginUserResponse::EStatus::SUCCESS: {
Result->Record.SetToken(loginResponse.Token);
Result->Record.SetSanitizedToken(loginResponse.SanitizedToken);
Result->Record.SetIsAdmin(IsAdmin());
break;
}
case NLogin::TLoginProvider::TLoginUserResponse::EStatus::INVALID_PASSWORD:
Expand Down Expand Up @@ -145,6 +162,7 @@ struct TSchemeShard::TTxLogin : TTransactionBase<TSchemeShard> {
HandleLoginAuthSuccess(loginRequest, loginResponse, db);
Result->Record.SetToken(loginResponse.Token);
Result->Record.SetSanitizedToken(loginResponse.SanitizedToken);
Result->Record.SetIsAdmin(IsAdmin());
break;
}
case NLogin::TLoginProvider::TLoginUserResponse::EStatus::INVALID_PASSWORD: {
Expand Down
31 changes: 28 additions & 3 deletions ydb/core/tx/schemeshard/ut_login/ut_login.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -923,9 +923,7 @@ NHttp::THttpIncomingRequestPtr MakeLogoutRequest(const TString& cookieName, cons
}

Y_UNIT_TEST_SUITE(TWebLoginService) {

Y_UNIT_TEST(AuditLogLoginSuccess) {
TTestBasicRuntime runtime;
void AuditLogLoginTest(TTestBasicRuntime& runtime, bool isUserAdmin) {
std::vector<std::string> lines;
runtime.AuditLogBackends = std::move(CreateTestAuditLogBackends(lines));
TTestEnv env(runtime);
Expand Down Expand Up @@ -965,6 +963,33 @@ Y_UNIT_TEST_SUITE(TWebLoginService) {
UNIT_ASSERT_STRING_CONTAINS(last, "login_user=user1");
UNIT_ASSERT_STRING_CONTAINS(last, "sanitized_token=");
UNIT_ASSERT(last.find("sanitized_token={none}") == std::string::npos);

if (isUserAdmin) {
UNIT_ASSERT_STRING_CONTAINS(last, "account_type=admin");
} else {
UNIT_ASSERT(!last.contains("account_type=admin"));
}
}

Y_UNIT_TEST(AuditLogEmptySIDsLoginSuccess) {
TTestBasicRuntime runtime;
AuditLogLoginTest(runtime, true);
}

Y_UNIT_TEST(AuditLogAdminLoginSuccess) {
TTestBasicRuntime runtime;
runtime.AddAppDataInit([](ui32, NKikimr::TAppData& appData){
appData.AdministrationAllowedSIDs.emplace_back("user1");
});
AuditLogLoginTest(runtime, true);
}

Y_UNIT_TEST(AuditLogLoginSuccess) {
TTestBasicRuntime runtime;
runtime.AddAppDataInit([](ui32, NKikimr::TAppData& appData){
appData.AdministrationAllowedSIDs.emplace_back("user2");
});
AuditLogLoginTest(runtime, false);
}

Y_UNIT_TEST(AuditLogLoginBadPassword) {
Expand Down

0 comments on commit 179f412

Please sign in to comment.