Skip to content

Commit 1d92a1f

Browse files
authored
SystemView Auth Fix sid timestamps (#15001)
1 parent 29e70be commit 1d92a1f

File tree

8 files changed

+27
-20
lines changed

8 files changed

+27
-20
lines changed

ydb/core/security/login_page.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ class TLoginRequest : public NActors::TActorBootstrapped<TLoginRequest> {
199199
ALOG_DEBUG(NActorsServices::HTTP, "Login success for " << User);
200200
NHttp::THeadersBuilder headers;
201201
SetCORS(headers);
202-
TDuration maxAge = (ToInstant(NLogin::TLoginProvider::GetTokenExpiresAt(cookie)) - TInstant::Now());
202+
TDuration maxAge = ToInstant(NLogin::TLoginProvider::GetTokenExpiresAt(cookie)) - TInstant::Now();
203203
headers.Set("Set-Cookie", TStringBuilder() << "ydb_session_id=" << cookie << "; Max-Age=" << maxAge.Seconds());
204204
Send(Sender, new NHttp::TEvHttpProxy::TEvHttpOutgoingResponse(Request->CreateResponse("200", "OK", headers)));
205205
PassAway();

ydb/core/sys_view/ut_kqp.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2192,7 +2192,7 @@ Y_UNIT_TEST_SUITE(SystemView) {
21922192
)").GetValueSync();
21932193

21942194
auto expected = R"([
2195-
[["user1"];[%true];[%false];[0u];[0u];[0u]];
2195+
[["user1"];[%true];[%false];#;#;[0u]];
21962196
])";
21972197

21982198
NKqp::CompareYson(expected, NKqp::StreamResultToYson(it));
@@ -2218,7 +2218,7 @@ Y_UNIT_TEST_SUITE(SystemView) {
22182218
)").GetValueSync();
22192219

22202220
auto expected = R"([
2221-
[["user2"];[%true];[%false];[0u];[0u];[0u]];
2221+
[["user2"];[%true];[%false];#;#;[0u]];
22222222
])";
22232223

22242224
NKqp::CompareYson(expected, NKqp::StreamResultToYson(it));
@@ -2231,8 +2231,8 @@ Y_UNIT_TEST_SUITE(SystemView) {
22312231
)").GetValueSync();
22322232

22332233
auto expected = R"([
2234-
[["user3"];[%true];[%false];[0u];[0u];[0u]];
2235-
[["user4"];[%true];[%false];[0u];[0u];[0u]];
2234+
[["user3"];[%true];[%false];#;#;[0u]];
2235+
[["user4"];[%true];[%false];#;#;[0u]];
22362236
])";
22372237

22382238
NKqp::CompareYson(expected, NKqp::StreamResultToYson(it));

ydb/core/tx/schemeshard/schemeshard__init_root.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ struct TSchemeShard::TTxInitRoot : public TSchemeShard::TRwTxBase {
5858
auto& sid = Self->LoginProvider.Sids[defaultUser.GetName()];
5959
db.Table<Schema::LoginSids>().Key(sid.Name).Update<Schema::LoginSids::SidType,
6060
Schema::LoginSids::SidHash,
61-
Schema::LoginSids::CreatedAt>(sid.Type, sid.PasswordHash, ToInstant(sid.CreatedAt).MilliSeconds());
61+
Schema::LoginSids::CreatedAt>(sid.Type, sid.PasswordHash, ToMicroSeconds(sid.CreatedAt));
6262
if (owner.empty()) {
6363
owner = defaultUser.GetName();
6464
}
@@ -81,7 +81,7 @@ struct TSchemeShard::TTxInitRoot : public TSchemeShard::TRwTxBase {
8181
} else {
8282
auto& sid = Self->LoginProvider.Sids[defaultGroup.GetName()];
8383
db.Table<Schema::LoginSids>().Key(sid.Name).Update<Schema::LoginSids::SidType,
84-
Schema::LoginSids::CreatedAt>(sid.Type, ToInstant(sid.CreatedAt).MilliSeconds());
84+
Schema::LoginSids::CreatedAt>(sid.Type, ToMicroSeconds(sid.CreatedAt));
8585
for (const auto& member : defaultGroup.GetMembers()) {
8686
auto response = Self->LoginProvider.AddGroupMembership({
8787
.Group = defaultGroup.GetName(),

ydb/core/tx/schemeshard/schemeshard__list_users.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,13 @@ struct TSchemeShard::TTxListUsers : TTransactionBase<TSchemeShard> {
3232
user->SetName(sid.Name);
3333
user->SetIsEnabled(sid.IsEnabled);
3434
user->SetIsLockedOut(Self->LoginProvider.IsLockedOut(sid));
35-
user->SetCreatedAt(ToInstant(sid.CreatedAt).MilliSeconds());
36-
user->SetLastSuccessfulAttemptAt(ToInstant(sid.LastSuccessfulLogin).MilliSeconds());
37-
user->SetLastFailedAttemptAt(ToInstant(sid.LastFailedLogin).MilliSeconds());
35+
user->SetCreatedAt(ToMicroSeconds(sid.CreatedAt));
36+
if (sid.LastSuccessfulLogin != std::chrono::system_clock::time_point()) {
37+
user->SetLastSuccessfulAttemptAt(ToMicroSeconds(sid.LastSuccessfulLogin));
38+
}
39+
if (sid.LastFailedLogin != std::chrono::system_clock::time_point()) {
40+
user->SetLastFailedAttemptAt(ToMicroSeconds(sid.LastFailedLogin));
41+
}
3842
user->SetFailedAttemptCount(sid.FailedLoginAttemptCount);
3943
user->SetPasswordHash(sid.PasswordHash);
4044
}

ydb/core/tx/schemeshard/schemeshard__login.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ struct TSchemeShard::TTxLogin : TSchemeShard::TRwTxBase {
153153
case TLoginProvider::TLoginUserResponse::EStatus::SUCCESS: {
154154
const auto& sid = Self->LoginProvider.Sids[loginRequest.User];
155155
db.Table<Schema::LoginSids>().Key(loginRequest.User).Update<Schema::LoginSids::LastSuccessfulAttempt,
156-
Schema::LoginSids::FailedAttemptCount>(ToInstant(sid.LastSuccessfulLogin).MilliSeconds(), sid.FailedLoginAttemptCount);
156+
Schema::LoginSids::FailedAttemptCount>(ToMicroSeconds(sid.LastSuccessfulLogin), sid.FailedLoginAttemptCount);
157157
Result->Record.SetToken(loginResponse.Token);
158158
Result->Record.SetSanitizedToken(loginResponse.SanitizedToken);
159159
Result->Record.SetIsAdmin(IsAdmin());
@@ -162,7 +162,7 @@ struct TSchemeShard::TTxLogin : TSchemeShard::TRwTxBase {
162162
case TLoginProvider::TLoginUserResponse::EStatus::INVALID_PASSWORD: {
163163
const auto& sid = Self->LoginProvider.Sids[loginRequest.User];
164164
db.Table<Schema::LoginSids>().Key(loginRequest.User).Update<Schema::LoginSids::LastFailedAttempt,
165-
Schema::LoginSids::FailedAttemptCount>(ToInstant(sid.LastFailedLogin).MilliSeconds(), sid.FailedLoginAttemptCount);
165+
Schema::LoginSids::FailedAttemptCount>(ToMicroSeconds(sid.LastFailedLogin), sid.FailedLoginAttemptCount);
166166
Result->Record.SetError(loginResponse.Error);
167167
break;
168168
}

ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class TAlterLogin: public TSubOperationBase {
5151
db.Table<Schema::LoginSids>().Key(sid.Name).Update<Schema::LoginSids::SidType,
5252
Schema::LoginSids::SidHash,
5353
Schema::LoginSids::CreatedAt,
54-
Schema::LoginSids::IsEnabled>(sid.Type, sid.PasswordHash, ToInstant(sid.CreatedAt).MilliSeconds(), sid.IsEnabled);
54+
Schema::LoginSids::IsEnabled>(sid.Type, sid.PasswordHash, ToMicroSeconds(sid.CreatedAt), sid.IsEnabled);
5555

5656
if (securityConfig.HasAllUsersGroup()) {
5757
auto response = context.SS->LoginProvider.AddGroupMembership({
@@ -127,7 +127,7 @@ class TAlterLogin: public TSubOperationBase {
127127
} else {
128128
auto& sid = context.SS->LoginProvider.Sids[group];
129129
db.Table<Schema::LoginSids>().Key(sid.Name).Update<Schema::LoginSids::SidType,
130-
Schema::LoginSids::CreatedAt>(sid.Type, ToInstant(sid.CreatedAt).MilliSeconds());
130+
Schema::LoginSids::CreatedAt>(sid.Type, ToMicroSeconds(sid.CreatedAt));
131131
result->SetStatus(NKikimrScheme::StatusSuccess);
132132
}
133133
break;

ydb/library/login/login.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ TLoginProvider::TBasicResponse TLoginProvider::CreateUser(const TCreateUserReque
102102
user.Name = request.User;
103103
user.PasswordHash = request.IsHashedPassword ? request.Password : Impl->GenerateHash(request.Password);
104104
user.CreatedAt = std::chrono::system_clock::now();
105-
user.LastFailedLogin = std::chrono::system_clock::time_point();
106105
user.IsEnabled = request.CanLogin;
107106
return response;
108107
}
@@ -799,10 +798,10 @@ void TLoginProvider::UpdateSecurityState(const NLoginProto::TSecurityState& stat
799798
sid.Members.emplace(pbSubSid);
800799
ChildToParentIndex[pbSubSid].emplace(sid.Name);
801800
}
802-
sid.CreatedAt = std::chrono::system_clock::time_point(std::chrono::milliseconds(pbSid.GetCreatedAt()));
801+
sid.CreatedAt = std::chrono::system_clock::time_point(std::chrono::microseconds(pbSid.GetCreatedAt()));
803802
sid.FailedLoginAttemptCount = pbSid.GetFailedLoginAttemptCount();
804-
sid.LastFailedLogin = std::chrono::system_clock::time_point(std::chrono::milliseconds(pbSid.GetLastFailedLogin()));
805-
sid.LastSuccessfulLogin = std::chrono::system_clock::time_point(std::chrono::milliseconds(pbSid.GetLastSuccessfulLogin()));
803+
sid.LastFailedLogin = std::chrono::system_clock::time_point(std::chrono::microseconds(pbSid.GetLastFailedLogin()));
804+
sid.LastSuccessfulLogin = std::chrono::system_clock::time_point(std::chrono::microseconds(pbSid.GetLastSuccessfulLogin()));
806805
}
807806
}
808807
}

ydb/library/security/util.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,16 @@ namespace NKikimr {
1212

1313
// copy-pasted from <robot/library/utils/time_convert.h>
1414
template<typename Rep, typename Period>
15-
constexpr ui64 ToMicroseconds(std::chrono::duration<Rep, Period> value) {
15+
constexpr ui64 ToMicroSeconds(std::chrono::duration<Rep, Period> value) {
1616
return std::chrono::duration_cast<std::chrono::microseconds>(value).count();
1717
}
1818

1919
template<typename Clock, typename Duration>
2020
constexpr TInstant ToInstant(std::chrono::time_point<Clock, Duration> value) {
21-
return TInstant::MicroSeconds(ToMicroseconds(value.time_since_epoch()));
21+
return TInstant::MicroSeconds(ToMicroSeconds(value.time_since_epoch()));
22+
}
23+
24+
constexpr ui64 ToMicroSeconds(std::chrono::system_clock::time_point value) {
25+
return ToMicroSeconds(value.time_since_epoch());
2226
}
2327
}

0 commit comments

Comments
 (0)