diff --git a/ydb/core/security/login_page.cpp b/ydb/core/security/login_page.cpp index 6f5f53029773..88d0c1765190 100644 --- a/ydb/core/security/login_page.cpp +++ b/ydb/core/security/login_page.cpp @@ -6,6 +6,8 @@ #include #include +#include +#include #include #include #include @@ -21,6 +23,24 @@ using namespace NKikimr; using namespace NSchemeShard; using namespace NMonitoring; +void AuditLogWebUILogout(const NHttp::THttpIncomingRequest& request, const TString& userSID) { + static const TString WebLoginComponentName = "web-login"; + static const TString LogoutOperationName = "LOGOUT"; + static const TString EmptyValue = "{none}"; + + auto remoteAddress = NKikimr::NAddressClassifier::ExtractAddress(request.Address->ToString()); + + // NOTE: audit field set here must be in sync with ydb/core/tx/schemeshard/schemeshard_audit_log.h, AuditLogLogin() + AUDIT_LOG( + AUDIT_PART("component", WebLoginComponentName) + AUDIT_PART("remote_address", (!remoteAddress.empty() ? remoteAddress : EmptyValue)) + AUDIT_PART("subject", (!userSID.empty() ? userSID : EmptyValue)) + //NOTE: no database specified as web logout considered cluster-wide + AUDIT_PART("operation", LogoutOperationName) + AUDIT_PART("status", TString("SUCCESS")) + ); +} + using THttpResponsePtr = THolder; class TLoginRequest : public NActors::TActorBootstrapped { @@ -251,6 +271,8 @@ class TLogoutRequest : public NActors::TActorBootstrapped { STATEFN(StateWork) { switch (ev->GetTypeRewrite()) { hFunc(TEvents::TEvPoisonPill, HandlePoisonPill); + hFunc(TEvTicketParser::TEvAuthorizeTicketResult, Handle); + cFunc(TEvents::TSystem::Wakeup, HandleTimeout); } } @@ -265,13 +287,42 @@ class TLogoutRequest : public NActors::TActorBootstrapped { return ReplyErrorAndPassAway("400", "Bad Request", "Invalid method"); } - ReplyDeleteCookieAndPassAway(); + NHttp::TCookies cookies(NHttp::THeaders(Request->Headers)["Cookie"]); + TStringBuf ydbSessionId = cookies["ydb_session_id"]; + if (ydbSessionId.empty()) { + return ReplyErrorAndPassAway("401", "Unauthorized", "No ydb_session_id cookie"); + } + + Send(NKikimr::MakeTicketParserID(), new NKikimr::TEvTicketParser::TEvAuthorizeTicket({ + .Database = TString(), + .Ticket = TString("Login ") + ydbSessionId, + .PeerName = Request->Address->ToString(), + })); + + Become(&TThis::StateWork, Timeout, new TEvents::TEvWakeup()); + } + + void Handle(TEvTicketParser::TEvAuthorizeTicketResult::TPtr& ev) { + const TEvTicketParser::TEvAuthorizeTicketResult& result = *ev->Get(); + if (result.Error) { + return ReplyErrorAndPassAway("403", "Forbidden", result.Error.Message); + } + if (result.Token == nullptr) { + return ReplyErrorAndPassAway("403", "Forbidden", "Empty token"); + } + + ReplyDeleteCookieAndPassAway(result.Token->GetUserSID()); } void HandlePoisonPill(TEvents::TEvPoisonPill::TPtr&) { PassAway(); } + void HandleTimeout() { + ALOG_ERROR(NActorsServices::HTTP, Request->Address << " " << Request->Method << " " << Request->URL << " timeout"); + ReplyErrorAndPassAway("504", "Gateway Timeout", "Timeout"); + } + void ReplyOptionsAndPassAway() { NHttp::THeadersBuilder headers; headers.Set("Allow", "OPTIONS, POST"); @@ -291,12 +342,15 @@ class TLogoutRequest : public NActors::TActorBootstrapped { headers.Set("Access-Control-Allow-Methods", "OPTIONS, GET, POST"); } - void ReplyDeleteCookieAndPassAway() { + void ReplyDeleteCookieAndPassAway(const TString& userSID) { ALOG_DEBUG(NActorsServices::HTTP, "Logout success"); NHttp::THeadersBuilder headers; SetCORS(headers); headers.Set("Set-Cookie", "ydb_session_id=; Max-Age=0"); Send(Sender, new NHttp::TEvHttpProxy::TEvHttpOutgoingResponse(Request->CreateResponse("200", "OK", headers))); + + AuditLogWebUILogout(*Request, userSID); + PassAway(); } @@ -314,6 +368,7 @@ class TLogoutRequest : public NActors::TActorBootstrapped { protected: TActorId Sender; NHttp::THttpIncomingRequestPtr Request; + TDuration Timeout = TDuration::Seconds(5); }; class TLoginService : public TActor { diff --git a/ydb/core/security/ya.make b/ydb/core/security/ya.make index b4dd549e0dac..d9361c2de4d7 100644 --- a/ydb/core/security/ya.make +++ b/ydb/core/security/ya.make @@ -16,6 +16,7 @@ PEERDIR( ydb/library/grpc/actor_client library/cpp/monlib/service/pages library/cpp/openssl/io + ydb/core/audit ydb/core/base ydb/core/protos ydb/library/aclib diff --git a/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp b/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp index 7e0cd3f54d87..8cd097d34a02 100644 --- a/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp @@ -3,6 +3,8 @@ #include #include +#include + #include #include #include @@ -383,6 +385,7 @@ void AuditLogLogin(const NKikimrScheme::TEvLogin& request, const NKikimrScheme:: TPath databasePath = TPath::Root(SS); auto peerName = NKikimr::NAddressClassifier::ExtractAddress(request.GetPeerName()); + // NOTE: audit field set here must be in sync with ydb/core/security/login_page.cpp, AuditLogWebUILogout() AUDIT_LOG( AUDIT_PART("component", SchemeshardComponentName) AUDIT_PART("remote_address", (!peerName.empty() ? peerName : EmptyValue)) diff --git a/ydb/core/tx/schemeshard/schemeshard_audit_log.h b/ydb/core/tx/schemeshard/schemeshard_audit_log.h index 1053823b4d00..7d095db2122d 100644 --- a/ydb/core/tx/schemeshard/schemeshard_audit_log.h +++ b/ydb/core/tx/schemeshard/schemeshard_audit_log.h @@ -20,6 +20,10 @@ class TEvCreateImportRequest; class TEvCreateImportResponse; } +namespace NHttp { +class THttpIncomingRequest; +} + namespace NKikimr::NSchemeShard { class TSchemeShard; @@ -36,4 +40,6 @@ void AuditLogImportStart(const NKikimrImport::TEvCreateImportRequest& request, c void AuditLogImportEnd(const TImportInfo& importInfo, TSchemeShard* SS); void AuditLogLogin(const NKikimrScheme::TEvLogin& request, const NKikimrScheme::TEvLoginResult& response, TSchemeShard* SS); +void AuditLogWebUILogout(const NHttp::THttpIncomingRequest& request, const TString& userSID); + } diff --git a/ydb/core/tx/schemeshard/ut_login/ut_login.cpp b/ydb/core/tx/schemeshard/ut_login/ut_login.cpp index 40545632493f..2d1e4ef50e84 100644 --- a/ydb/core/tx/schemeshard/ut_login/ut_login.cpp +++ b/ydb/core/tx/schemeshard/ut_login/ut_login.cpp @@ -1,9 +1,12 @@ #include +#include +#include #include #include -#include #include +#include +#include using namespace NKikimr; using namespace NSchemeShard; @@ -123,3 +126,153 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { UNIT_ASSERT_STRING_CONTAINS(last, "login_auth_domain={none}"); } } + +namespace NSchemeShardUT_Private { + +void EatWholeString(NHttp::THttpIncomingRequestPtr request, const TString& data) { + request->EnsureEnoughSpaceAvailable(data.size()); + auto size = std::min(request->Avail(), data.size()); + memcpy(request->Pos(), data.data(), size); + request->Advance(size); +} + +NHttp::THttpIncomingRequestPtr MakeLoginRequest(const TString& user, const TString& password) { + TString payload = [](const auto& user, const auto& password) { + NJson::TJsonValue value; + value["user"] = user; + value["password"] = password; + return NJson::WriteJson(value, false); + }(user, password); + TStringBuilder text; + text << "POST /login HTTP/1.1\r\n" + << "Host: test.ydb\r\n" + << "Content-Type: application/json\r\n" + << "Content-Length: " << payload.size() << "\r\n" + << "\r\n" + << payload; + NHttp::THttpIncomingRequestPtr request = new NHttp::THttpIncomingRequest(); + EatWholeString(request, text); + // WebLoginService will crash without address + request->Address = std::make_shared("127.0.0.1", 0); + // Cerr << "TEST: http login request: " << text << Endl; + return request; +} + +NHttp::THttpIncomingRequestPtr MakeLogoutRequest(const TString& cookieName, const TString& cookieValue) { + TStringBuilder text; + text << "POST /logout HTTP/1.1\r\n" + << "Host: test.ydb\r\n" + << "Content-Type: text/plain\r\n" + << "Cookie: " << cookieName << "=" << cookieValue << "\r\n" + << "\r\n"; + NHttp::THttpIncomingRequestPtr request = new NHttp::THttpIncomingRequest(); + EatWholeString(request, text); + // WebLoginService will crash without address + request->Address = std::make_shared("127.0.0.1", 0); + // Cerr << "TEST: http logout request: " << text << Endl; + return request; +} + +} + +Y_UNIT_TEST_SUITE(TWebLoginService) { + + Y_UNIT_TEST(Logout) { + TTestBasicRuntime runtime; + std::vector lines; + runtime.AuditLogBackends = std::move(CreateTestAuditLogBackends(lines)); + TTestEnv env(runtime); + + // Add ticket parser to the mix + { + NKikimrProto::TAuthConfig authConfig; + authConfig.SetUseBlackBox(false); + authConfig.SetUseLoginProvider(true); + + IActor* ticketParser = NKikimr::CreateTicketParser({.AuthConfig = authConfig}); + TActorId ticketParserId = runtime.Register(ticketParser); + runtime.RegisterService(NKikimr::MakeTicketParserID(), ticketParserId); + } + + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 1); // alter root subdomain + + ui64 txId = 100; + + TestCreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", "user1", "password1", {{NKikimrScheme::StatusSuccess}}); + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 2); // +user creation + + // test body + const auto target = runtime.Register(CreateWebLoginService()); + const auto edge = runtime.AllocateEdgeActor(); + + TString ydbSessionId; + { + runtime.Send(new IEventHandle(target, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingRequest( + MakeLoginRequest("user1", "password1") + ))); + + TAutoPtr handle; + auto responseEv = runtime.GrabEdgeEvent(handle); + UNIT_ASSERT_STRINGS_EQUAL(responseEv->Response->Status, "200"); + NHttp::THeaders headers(responseEv->Response->Headers); + NHttp::TCookies cookies(headers["Set-Cookie"]); + ydbSessionId = cookies["ydb_session_id"]; + } + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 3); // +user login + + // New security keys are created in the subdomain as a consequence of a login. + // In real system they are transferred to the ticket parser by the grpc-proxy + // on receiving subdomain update notification. + // Here there are no grpc-proxy, so we should transfer keys to the ticket parser manually. + { + const auto describe = DescribePath(runtime, "/MyRoot"); + const auto& securityState = describe.GetPathDescription().GetDomainDescription().GetSecurityState(); + TActorId edge = runtime.AllocateEdgeActor(); + runtime.Send(new IEventHandle(MakeTicketParserID(), edge, new TEvTicketParser::TEvUpdateLoginSecurityState(securityState)), 0); + } + + // Then we are ready to test some authentication on /logout + { // no cookie + runtime.Send(new IEventHandle(target, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingRequest( + MakeLogoutRequest("not-an-ydb_session_id", ydbSessionId) + ))); + + TAutoPtr handle; + auto responseEv = runtime.GrabEdgeEvent(handle); + UNIT_ASSERT_STRINGS_EQUAL(responseEv->Response->Status, "401"); + + // no audit record for actions without auth + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 3); + } + { // bad cookie + runtime.Send(new IEventHandle(target, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingRequest( + MakeLogoutRequest("ydb_session_id", "jklhagsfjhg") + ))); + + TAutoPtr handle; + auto responseEv = runtime.GrabEdgeEvent(handle); + UNIT_ASSERT_STRINGS_EQUAL(responseEv->Response->Status, "403"); + + // no audit record for actions without auth + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 3); + } + { // good cookie + runtime.Send(new IEventHandle(target, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingRequest( + MakeLogoutRequest("ydb_session_id", ydbSessionId) + ))); + + TAutoPtr handle; + auto responseEv = runtime.GrabEdgeEvent(handle); + UNIT_ASSERT_STRINGS_EQUAL(responseEv->Response->Status, "200"); + + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 4); // +user web logout + + auto last = FindAuditLine(lines, "operation=LOGOUT"); + UNIT_ASSERT_STRING_CONTAINS(last, "component=web-login"); + UNIT_ASSERT_STRING_CONTAINS(last, "remote_address=127.0.0.1"); + UNIT_ASSERT_STRING_CONTAINS(last, "subject=user1"); + UNIT_ASSERT_STRING_CONTAINS(last, "operation=LOGOUT"); + UNIT_ASSERT_STRING_CONTAINS(last, "status=SUCCESS"); + } + } +} diff --git a/ydb/library/login/login.cpp b/ydb/library/login/login.cpp index 9e4bcd646156..403e2502f0ed 100644 --- a/ydb/library/login/login.cpp +++ b/ydb/library/login/login.cpp @@ -8,10 +8,7 @@ #include #include -#include #include -#include -#include #include @@ -339,7 +336,8 @@ TLoginProvider::TLoginUserResponse TLoginProvider::LoginUser(const TLoginUserReq .set_issued_at(now) .set_expires_at(expires_at); if (!Audience.empty()) { - token.set_audience(Audience); + // jwt.h require audience claim to be a set + token.set_audience(std::set{Audience}); } if (request.ExternalAuth) { @@ -381,7 +379,7 @@ TLoginProvider::TValidateTokenResponse TLoginProvider::ValidateToken(const TVali try { jwt::decoded_jwt decoded_token = jwt::decode(request.Token); if (Audience) { - // we check audience manually because we wan't this error instead of wrong key id in case of databases mismatch + // we check audience manually because we want an explicit error instead of wrong key id in case of databases mismatch auto audience = decoded_token.get_audience(); if (audience.empty() || TString(*audience.begin()) != Audience) { response.Error = "Wrong audience"; @@ -394,7 +392,8 @@ TLoginProvider::TValidateTokenResponse TLoginProvider::ValidateToken(const TVali auto verifier = jwt::verify() .allow_algorithm(jwt::algorithm::ps256(key->PublicKey)); if (Audience) { - verifier.with_audience(std::set({Audience})); + // jwt.h require audience claim to be a set + verifier.with_audience(std::set{Audience}); } verifier.verify(decoded_token); response.User = decoded_token.get_subject(); @@ -435,8 +434,10 @@ TLoginProvider::TValidateTokenResponse TLoginProvider::ValidateToken(const TVali response.Error = "Key not found"; } } - } catch (const jwt::token_verification_exception& e) { + } catch (const jwt::signature_verification_exception& e) { response.Error = e.what(); // invalid token signature + } catch (const jwt::token_verification_exception& e) { + response.Error = e.what(); // invalid token } catch (const std::invalid_argument& e) { response.Error = "Token is not in correct format"; response.TokenUnrecognized = true;