oidc impersonation #11449
oidc impersonation #11449
Conversation
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
2 times, most recently
from
6964f88
to
67b9241
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
from
67b9241
to
2274f9e
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
from
2274f9e
to
09ddc18
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
from
09ddc18
to
700e978
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
from
700e978
to
4c2f031
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
from
4c2f031
to
f12608f
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
from
f12608f
to
94ddfa3
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
from
94ddfa3
to
c348947
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
| try { | ||
| Base64StrictDecode(cookie, token); | ||
| } catch (std::exception& e) { | ||
| LOG_DEBUG_S(ctx, EService::MVP, "Base64Decode " << cookie << " cookie: " << e.what()); |
There was a problem hiding this comment.
You wrote user token into logs just now ;)
There was a problem hiding this comment.
it was masked. but when exception happens, I think that's a good idea to log whole cookie value
There was a problem hiding this comment.
It can not be masked. If it is masked, Base64StrictDecode will generate exception. At least in this function it is not masked.
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
from
6be1878
to
5d4b133
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢 |
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
from
5d4b133
to
37fb58b
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
from
37fb58b
to
767932d
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
| namespace NOIDC { | ||
| namespace NMVP::NOIDC { | ||
|
|
||
| using namespace NActors; |
There was a problem hiding this comment.
using namespace in headers is not recommended
| namespace NOIDC { | ||
| namespace NMVP::NOIDC { | ||
|
|
||
| using namespace NActors; |
There was a problem hiding this comment.
using namespace in headers is not recommended
| @@ -3,15 +3,15 @@ | |||
| #include <util/string/hex.h> | |||
| #include <library/cpp/json/json_reader.h> | |||
| #include <library/cpp/string_utils/base64/base64.h> | |||
| #include <ydb/library/security/util.h> | |||
There was a problem hiding this comment.
According to arcadia style guide, headers should be listed in the following order:
- Corresponding header of our cpp
- Local library headers
- Headers of libraries of our project
- Library
- Util
- standard headers
From less common to more common
| try { | ||
| Base64StrictDecode(cookie, token); | ||
| } catch (std::exception& e) { | ||
| LOG_DEBUG_S(ctx, EService::MVP, "Base64Decode " << cookie << " cookie: " << e.what()); |
There was a problem hiding this comment.
It can not be masked. If it is masked, Base64StrictDecode will generate exception. At least in this function it is not masked.
| namespace NOIDC { | ||
| namespace NMVP::NOIDC { | ||
|
|
||
| using namespace NActors; |
There was a problem hiding this comment.
using namespace in headers is not recommended
| } // NOIDC | ||
| } // NMVP | ||
| void THandlerSessionServiceCheck::ReplyAndDie(NHttp::THttpOutgoingResponsePtr httpResponse, const NActors::TActorContext& ctx) { | ||
| ctx.Send(Sender, new NHttp::TEvHttpProxy::TEvHttpOutgoingResponse(httpResponse)); |
There was a problem hiding this comment.
std::move(httpResponse)
| @@ -233,5 +226,9 @@ NHttp::THttpOutgoingResponsePtr THandlerSessionServiceCheck::CreateResponseForNo | |||
| return Request->CreateResponse("400", "Bad Request", headers, html); | |||
| } | |||
|
|
|||
| } // NOIDC | |||
| } // NMVP | |||
| void THandlerSessionServiceCheck::ReplyAndDie(NHttp::THttpOutgoingResponsePtr httpResponse, const NActors::TActorContext& ctx) { | |||
There was a problem hiding this comment.
We can here not use ctx - there are all these methods without ctx
| STFUNC(StateWork) { | ||
| switch (ev->GetTypeRewrite()) { | ||
| HFunc(NHttp::TEvHttpProxy::TEvHttpIncomingResponse, Handle); | ||
| cFunc(TEvents::TEvPoisonPill::EventType, PassAway); |
There was a problem hiding this comment.
Do we receive Poison here? From who?
| ReplyAndDie(httpResponse, ctx); | ||
| } | ||
|
|
||
| void THandlerImpersonateStop::ReplyAndDie(NHttp::THttpOutgoingResponsePtr httpResponse, const NActors::TActorContext& ctx) { |
There was a problem hiding this comment.
It is not neccessary to use ctx here. There are all these methods without ctx
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
from
31a8bd5
to
25c51d0
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
StekPerepolnen
force-pushed
the
oidc-impersonation
branch
from
25c51d0
to
18ba292
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢 |
Changelog entry
https://nebius.atlassian.net/browse/NBYDB-594
oidc impersonation adds impersonated cookie that could be used instead session cookie
to get impersonated cookie session cookie and service-account-id could be used
2 oidc impersonate handler were added
/impersonate/startto add impersonated cookie/impersonate/stopto remove impersonated cookieIf request to oidc happens, it checks new impersonated cookie. if exists, oidc uses it for next authentification
If the authentication domain returns a status code of 400 or 401, OIDC performs a redirect with a 307 status code to the same address and clears the impersonated cookie.
Changelog category
Additional information
Examples:
curl -i 'https://oidc.net/impersonate/start?service_account_id=serviceaccount-e0tydb-dev'
-H 'accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,/;q=0.8,application/signed-exchange;v=b3;q=0.7'
-H 'cookie: __Host_session_cookie_6D76702D6F6964632D74657374696E67=****'
--insecure
expected result:
set-cookie: __Host_impersonated_cookie_6D76702D6F6964632D74657374696E67=****; Path=/; Secure; HttpOnly; SameSite=None; Partitioned
curl -i 'https://oidc.net/impersonate/start?service_account_id=service-account-name'
-H 'accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,/;q=0.8,application/signed-exchange;v=b3;q=0.7'
-H 'cookie: __Host_session_cookie_6D76702D6F6964632D74657374696E67=****'
--insecure
expected result:
set-cookie: __Host_impersonated_cookie_6D76702D6F6964632D74657374696E67=; Path=/; Secure; HttpOnly; SameSite=None; Partitioned; Max-Age=0
curl 'https://oidc.net/storage-status.ydb-global.ik8s-beta.ik8s.man.nbhost.net:8765/viewer/json/describe?database=%2Ftesting%2Fcharlotte&path=%2Ftesting%2Fcharlotte&enums=true&backup=false&private=true&partition_config=false&partition_stats=false&partitioning_info=false&subs=1'
-H 'accept: application/json, text/plain, /'
-H 'cookie: __Host_session_cookie_6D76702D6F6964632D74657374696E67=; __Host_impersonated_cookie_6D76702D6F6964632D74657374696E67=; _ga_N2V9EEXZGE=GS1.1.1731507944.10.1.1731507976.0.0.0'
--insecure
If the authentication domain returns a status code of 400 or 401, OIDC performs a redirect with a 307 status code to the same address and clears the impersonated cookie.
...