From b7611cfe3bd42bce90dbb62b5a7c1f2c5461e321 Mon Sep 17 00:00:00 2001 From: StekPerepolnen Date: Fri, 17 May 2024 10:43:17 +0000 Subject: [PATCH] oidc_proxy_return_401 --- ydb/mvp/oidc_proxy/oidc_proxy_ut.cpp | 90 ++++++++++++++++++++++++ ydb/mvp/oidc_proxy/oidc_session_create.h | 5 +- ydb/mvp/oidc_proxy/openid_connect.cpp | 12 ++-- 3 files changed, 100 insertions(+), 7 deletions(-) diff --git a/ydb/mvp/oidc_proxy/oidc_proxy_ut.cpp b/ydb/mvp/oidc_proxy/oidc_proxy_ut.cpp index ba063b62e2ef..1d81c69337ae 100644 --- a/ydb/mvp/oidc_proxy/oidc_proxy_ut.cpp +++ b/ydb/mvp/oidc_proxy/oidc_proxy_ut.cpp @@ -467,6 +467,96 @@ Y_UNIT_TEST_SUITE(Mvp) { OidcFullAuthorizationFlow(redirectStrategy); } + void OidcWrongStateAuthorizationFlow(TRedirectStrategyBase& redirectStrategy) { + TPortManager tp; + ui16 sessionServicePort = tp.GetPort(8655); + TMvpTestRuntime runtime; + runtime.Initialize(); + + const TString allowedProxyHost {"ydb.viewer.page"}; + + TOpenIdConnectSettings settings { + .SessionServiceEndpoint = "localhost:" + ToString(sessionServicePort), + .AuthorizationServerAddress = "https://auth.cloud.yandex.ru", + .ClientSecret = "0123456789abcdef", + .AllowedProxyHosts = {allowedProxyHost} + }; + + const NActors::TActorId edge = runtime.AllocateEdgeActor(); + const NActors::TActorId target = runtime.Register(new NMVP::TProtectedPageHandler(edge, settings)); + + TSessionServiceMock sessionServiceMock; + sessionServiceMock.AllowedCookies.second = "allowed_session_cookie"; + sessionServiceMock.AllowedAccessTokens.insert("access_token_value"); + grpc::ServerBuilder builder; + builder.AddListeningPort(settings.SessionServiceEndpoint, grpc::InsecureServerCredentials()).RegisterService(&sessionServiceMock); + std::unique_ptr sessionServer(builder.BuildAndStart()); + + NHttp::THttpIncomingRequestPtr incomingRequest = new NHttp::THttpIncomingRequest(); + incomingRequest->Endpoint->Secure = true; + + const TString hostProxy = "oidcproxy.yandex.net"; + const TString protectedPage = "/" + allowedProxyHost + "/counters"; + + EatWholeString(incomingRequest, redirectStrategy.CreateRequest("GET " + protectedPage + " HTTP/1.1\r\n" + "Host: " + hostProxy + "\r\n" + "Cookie: yc_session=invalid_cookie\r\n" + "Referer: https://" + hostProxy + protectedPage + "\r\n")); + runtime.Send(new IEventHandle(target, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingRequest(incomingRequest))); + + TAutoPtr handle; + NHttp::TEvHttpProxy::TEvHttpOutgoingResponse* outgoingResponseEv = runtime.GrabEdgeEvent(handle); + redirectStrategy.CheckRedirectStatus(outgoingResponseEv); + TString location = redirectStrategy.GetRedirectUrl(outgoingResponseEv); + UNIT_ASSERT_STRING_CONTAINS(location, "https://auth.cloud.yandex.ru/oauth/authorize"); + UNIT_ASSERT_STRING_CONTAINS(location, "response_type=code"); + UNIT_ASSERT_STRING_CONTAINS(location, "scope=openid"); + UNIT_ASSERT_STRING_CONTAINS(location, "client_id=" + TOpenIdConnectSettings::CLIENT_ID); + UNIT_ASSERT_STRING_CONTAINS(location, "redirect_uri=https://" + hostProxy + "/auth/callback"); + + NHttp::TUrlParameters urlParameters(location); + const TString state = urlParameters["state"]; + + const NHttp::THeaders headers(outgoingResponseEv->Response->Headers); + UNIT_ASSERT(headers.Has("Set-Cookie")); + TStringBuf setCookie = headers.Get("Set-Cookie"); + UNIT_ASSERT_STRING_CONTAINS(setCookie, CreateNameYdbOidcCookie(settings.ClientSecret, state)); + redirectStrategy.CheckSpecificHeaders(headers); + + const NActors::TActorId sessionCreator = runtime.Register(new NMVP::TSessionCreator(edge, settings)); + incomingRequest = new NHttp::THttpIncomingRequest(); + TStringBuilder request; + request << "GET /auth/callback?code=code_template&state=" << "wrong_state!" << " HTTP/1.1\r\n"; + request << "Host: " + hostProxy + "\r\n"; + request << "Cookie: " << setCookie.NextTok(";") << "\r\n"; + EatWholeString(incomingRequest, redirectStrategy.CreateRequest(request)); + incomingRequest->Endpoint->Secure = true; + runtime.Send(new IEventHandle(sessionCreator, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingRequest(incomingRequest))); + + outgoingResponseEv = runtime.GrabEdgeEvent(handle); + UNIT_ASSERT_STRINGS_EQUAL(outgoingResponseEv->Response->Status, "302"); + { + const NHttp::THeaders headers(outgoingResponseEv->Response->Headers); + UNIT_ASSERT(headers.Has("Location")); + TString location = TString(headers.Get("Location")); + UNIT_ASSERT_STRING_CONTAINS(location, "https://auth.cloud.yandex.ru/oauth/authorize"); + UNIT_ASSERT_STRING_CONTAINS(location, "response_type=code"); + UNIT_ASSERT_STRING_CONTAINS(location, "scope=openid"); + UNIT_ASSERT_STRING_CONTAINS(location, "client_id=" + TOpenIdConnectSettings::CLIENT_ID); + UNIT_ASSERT_STRING_CONTAINS(location, "redirect_uri=https://" + hostProxy + "/auth/callback"); + } + } + + Y_UNIT_TEST(OpenIdConnectotWrongStateAuthorizationFlow) { + TRedirectStrategy redirectStrategy; + OidcWrongStateAuthorizationFlow(redirectStrategy); + } + + Y_UNIT_TEST(OpenIdConnectotWrongStateAuthorizationFlowAjax) { + TAjaxRedirectStrategy redirectStrategy; + OidcWrongStateAuthorizationFlow(redirectStrategy); + } + Y_UNIT_TEST(OpenIdConnectSessionServiceCreateAuthorizationFail) { TPortManager tp; ui16 sessionServicePort = tp.GetPort(8655); diff --git a/ydb/mvp/oidc_proxy/oidc_session_create.h b/ydb/mvp/oidc_proxy/oidc_session_create.h index 2e3ac5e9e232..a2e4cf39e7b0 100644 --- a/ydb/mvp/oidc_proxy/oidc_session_create.h +++ b/ydb/mvp/oidc_proxy/oidc_session_create.h @@ -142,7 +142,7 @@ class THandlerSessionCreate : public NActors::TActorBootstrappedHeaders); NHttp::TCookies cookies(headers.Get("cookie")); - if (!code.Empty() && IsStateValid(state, cookies, ctx)) { + if (IsStateValid(state, cookies, ctx) && !code.Empty()) { NHttp::THttpOutgoingRequestPtr httpRequest = NHttp::THttpOutgoingRequest::CreateRequestPost(Settings.AuthorizationServerAddress + "/oauth/token"); httpRequest->Set<&NHttp::THttpRequest::ContentType>("application/x-www-form-urlencoded"); httpRequest->Set("Authorization", "Basic " + Settings.GetAuthorizationString()); @@ -151,8 +151,7 @@ class THandlerSessionCreate : public NActors::TActorBootstrappedSet<&NHttp::THttpRequest::Body>(body); ctx.Send(HttpProxyId, new NHttp::TEvHttpProxy::TEvHttpOutgoingRequest(httpRequest)); } else { - ResponseHeaders.Set("Content-Type", "text/plain"); - NHttp::THttpOutgoingResponsePtr response = Request->CreateResponse("400", "Bad Request", ResponseHeaders, "Parameters state and code are invalid"); + NHttp::THttpOutgoingResponsePtr response = GetHttpOutgoingResponsePtr(TStringBuf(), Request, Settings, ResponseHeaders, IsAjaxRequest); ctx.Send(Sender, new NHttp::TEvHttpProxy::TEvHttpOutgoingResponse(response)); TBase::Die(ctx); return; diff --git a/ydb/mvp/oidc_proxy/openid_connect.cpp b/ydb/mvp/oidc_proxy/openid_connect.cpp index eb865ca52f4c..4e140f26ea95 100644 --- a/ydb/mvp/oidc_proxy/openid_connect.cpp +++ b/ydb/mvp/oidc_proxy/openid_connect.cpp @@ -30,10 +30,14 @@ TString CreateRedirectUrl(const TRedirectUrlParameters& parameters) { TStringBuilder locationHeaderValue; TStringBuf authUrl = "/oauth/authorize"; const auto& eventDetails = parameters.SessionServerCheckDetails; - size_t posAuthUrl = eventDetails.find(authUrl); - if (posAuthUrl != TStringBuf::npos) { - size_t pos = eventDetails.rfind("https://", posAuthUrl); - locationHeaderValue << eventDetails.substr(pos, posAuthUrl - pos); + if (eventDetails) { + size_t posAuthUrl = eventDetails.find(authUrl); + if (posAuthUrl != TStringBuf::npos) { + size_t pos = eventDetails.rfind("https://", posAuthUrl); + locationHeaderValue << eventDetails.substr(pos, posAuthUrl - pos); + } else { + locationHeaderValue << parameters.OidcSettings.AuthorizationServerAddress; + } } else { locationHeaderValue << parameters.OidcSettings.AuthorizationServerAddress; }