Skip to content

Commit 50d8bb0

Browse files
molotkov-andmolotkov-and
authored
[oidc proxy] Improve error message for forbidden host and null response (#8785)
1 parent dadb557 commit 50d8bb0

File tree

4 files changed

+85
-16
lines changed

4 files changed

+85
-16
lines changed

‎ydb/mvp/oidc_proxy/oidc_protected_page.h

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ class THandlerSessionServiceCheck : public NActors::TActorBootstrapped<THandlerS
3434
TString RequestedPageScheme;
3535
bool IsAjaxRequest = false;
3636

37-
const static inline TStringBuf NOT_FOUND_HTML_PAGE = "<html><head><title>404 Not Found</title></head><body bgcolor=\"white\"><center><h1>404 Not Found</h1></center></body></html>";
3837
const static inline TStringBuf IAM_TOKEN_SCHEME = "Bearer ";
3938
const static inline TStringBuf IAM_TOKEN_SCHEME_LOWER = "bearer ";
4039
const static inline TStringBuf AUTH_HEADER_NAME = "Authorization";
@@ -54,7 +53,7 @@ class THandlerSessionServiceCheck : public NActors::TActorBootstrapped<THandlerS
5453

5554
virtual void Bootstrap(const NActors::TActorContext& ctx) {
5655
if (!CheckRequestedHost()) {
57-
ctx.Send(Sender, new NHttp::TEvHttpProxy::TEvHttpOutgoingResponse(Request->CreateResponseNotFound(NOT_FOUND_HTML_PAGE, "text/html")));
56+
ctx.Send(Sender, new NHttp::TEvHttpProxy::TEvHttpOutgoingResponse(CreateResponseForbiddenHost()));
5857
Die(ctx);
5958
return;
6059
}
@@ -86,7 +85,9 @@ class THandlerSessionServiceCheck : public NActors::TActorBootstrapped<THandlerS
8685
httpResponse = Request->CreateResponse( response->Status, response->Message, headers, response->Body);
8786
}
8887
} else {
89-
httpResponse = Request->CreateResponseNotFound(NOT_FOUND_HTML_PAGE, "text/html");
88+
static constexpr size_t MAX_LOGGED_SIZE = 1024;
89+
LOG_DEBUG_S(ctx, EService::MVP, "Can not process request to protected resource:\n" << event->Get()->Request->GetRawData().substr(0, MAX_LOGGED_SIZE));
90+
httpResponse = CreateResponseForNotExistingResponseFromProtectedResource(event->Get()->GetError());
9091
}
9192
ctx.Send(Sender, new NHttp::TEvHttpProxy::TEvHttpOutgoingResponse(httpResponse));
9293
Die(ctx);
@@ -230,6 +231,33 @@ class THandlerSessionServiceCheck : public NActors::TActorBootstrapped<THandlerS
230231
NHttp::CrackURL(ProtectedPageUrl, scheme, host, uri);
231232
return TStringBuilder() << '/' << host << location;
232233
}
234+
235+
NHttp::THttpOutgoingResponsePtr CreateResponseForbiddenHost() {
236+
NHttp::THeadersBuilder headers;
237+
headers.Set("Content-Type", "text/html");
238+
SetCORS(Request, &headers);
239+
240+
TStringBuf scheme, host, uri;
241+
NHttp::CrackURL(ProtectedPageUrl, scheme, host, uri);
242+
TStringBuilder html;
243+
html << "<html><head><title>403 Forbidden</title></head><body bgcolor=\"white\"><center><h1>";
244+
html << "403 Forbidden host: " << host;
245+
html << "</h1></center></body></html>";
246+
247+
return Request->CreateResponse("403", "Forbidden", headers, html);
248+
}
249+
250+
NHttp::THttpOutgoingResponsePtr CreateResponseForNotExistingResponseFromProtectedResource(const TString& errorMessage) {
251+
NHttp::THeadersBuilder headers;
252+
headers.Set("Content-Type", "text/html");
253+
SetCORS(Request, &headers);
254+
255+
TStringBuilder html;
256+
html << "<html><head><title>400 Bad Request</title></head><body bgcolor=\"white\"><center><h1>";
257+
html << "400 Bad Request. Can not process request to protected resource: " << errorMessage;
258+
html << "</h1></center></body></html>";
259+
return Request->CreateResponse("400", "Bad Request", headers, html);
260+
}
233261
};
234262

235263
} // NMVP

‎ydb/mvp/oidc_proxy/oidc_proxy_ut.cpp

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -952,7 +952,7 @@ Y_UNIT_TEST_SUITE(Mvp) {
952952
const TString hostProxy = "oidcproxy.net";
953953
const TString protectedPage = "/counters";
954954

955-
auto checkAllowedHostList = [&] (const TString& requestedHost, const TString& expectedStatus) {
955+
auto checkAllowedHostList = [&] (const TString& requestedHost, const TString& expectedStatus, const TString& expectedBodyContent = "") {
956956
const TString url = "/" + requestedHost + protectedPage;
957957
TStringBuilder httpRequest;
958958
httpRequest << "GET " + url + " HTTP/1.1\r\n"
@@ -968,14 +968,54 @@ Y_UNIT_TEST_SUITE(Mvp) {
968968
TAutoPtr<IEventHandle> handle;
969969
NHttp::TEvHttpProxy::TEvHttpOutgoingResponse* outgoingResponseEv = runtime.GrabEdgeEvent<NHttp::TEvHttpProxy::TEvHttpOutgoingResponse>(handle);
970970
UNIT_ASSERT_STRINGS_EQUAL(outgoingResponseEv->Response->Status, expectedStatus);
971+
if (!expectedBodyContent.empty()) {
972+
UNIT_ASSERT_STRING_CONTAINS(outgoingResponseEv->Response->Body, expectedBodyContent);
973+
}
971974
};
972975

973976
for (const TString& allowedHost : allowedProxyHosts) {
974977
checkAllowedHostList(allowedHost, "302");
975978
}
976979

977980
for (const TString& forbiddenHost : forbiddenProxyHosts) {
978-
checkAllowedHostList(forbiddenHost, "404");
981+
checkAllowedHostList(forbiddenHost, "403", "403 Forbidden host: " + forbiddenHost);
979982
}
980983
}
984+
985+
Y_UNIT_TEST(OpenIdConnectHandleNullResponseFromProtectedResource) {
986+
TPortManager tp;
987+
ui16 sessionServicePort = tp.GetPort(8655);
988+
TMvpTestRuntime runtime;
989+
runtime.Initialize();
990+
991+
const TString allowedProxyHost {"ydb.viewer.page"};
992+
993+
TOpenIdConnectSettings settings {
994+
.SessionServiceEndpoint = "localhost:" + ToString(sessionServicePort),
995+
.AllowedProxyHosts = {allowedProxyHost},
996+
};
997+
998+
const NActors::TActorId edge = runtime.AllocateEdgeActor();
999+
const NActors::TActorId target = runtime.Register(new NMVP::TProtectedPageHandler(edge, settings));
1000+
1001+
const TString iamToken {"protected_page_iam_token"};
1002+
NHttp::THttpIncomingRequestPtr incomingRequest = new NHttp::THttpIncomingRequest();
1003+
EatWholeString(incomingRequest, "GET /" + allowedProxyHost + "/counters HTTP/1.1\r\n"
1004+
"Host: oidcproxy.net\r\n"
1005+
"Authorization: Bearer " + iamToken + "\r\n");
1006+
runtime.Send(new IEventHandle(target, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingRequest(incomingRequest)));
1007+
TAutoPtr<IEventHandle> handle;
1008+
1009+
auto outgoingRequestEv = runtime.GrabEdgeEvent<NHttp::TEvHttpProxy::TEvHttpOutgoingRequest>(handle);
1010+
UNIT_ASSERT_STRINGS_EQUAL(outgoingRequestEv->Request->Host, allowedProxyHost);
1011+
UNIT_ASSERT_STRINGS_EQUAL(outgoingRequestEv->Request->URL, "/counters");
1012+
UNIT_ASSERT_STRING_CONTAINS(outgoingRequestEv->Request->Headers, "Authorization: Bearer " + iamToken);
1013+
1014+
const TString expectedError = "Response is NULL for some reason";
1015+
runtime.Send(new IEventHandle(handle->Sender, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingResponse(outgoingRequestEv->Request, nullptr, expectedError)));
1016+
1017+
auto outgoingResponseEv = runtime.GrabEdgeEvent<NHttp::TEvHttpProxy::TEvHttpOutgoingResponse>(handle);
1018+
UNIT_ASSERT_STRINGS_EQUAL(outgoingResponseEv->Response->Status, "400");
1019+
UNIT_ASSERT_STRING_CONTAINS(outgoingResponseEv->Response->Body, expectedError);
1020+
}
9811021
}

‎ydb/mvp/oidc_proxy/openid_connect.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,6 @@ TString CreateRedirectUrl(const TRedirectUrlParameters& parameters) {
5555
return locationHeaderValue;
5656
}
5757

58-
void SetCORS(const NHttp::THttpIncomingRequestPtr& request, NHttp::THeadersBuilder* const headers) {
59-
TString origin = TString(NHttp::THeaders(request->Headers)["Origin"]);
60-
if (origin.empty()) {
61-
origin = "*";
62-
}
63-
headers->Set("Access-Control-Allow-Origin", origin);
64-
headers->Set("Access-Control-Allow-Credentials", "true");
65-
headers->Set("Access-Control-Allow-Headers", "Content-Type,Authorization,Origin,Accept");
66-
headers->Set("Access-Control-Allow-Methods", "OPTIONS, GET, POST");
67-
}
68-
6958
NHttp::THttpOutgoingResponsePtr CreateResponseForAjaxRequest(const NHttp::THttpIncomingRequestPtr& request, NHttp::THeadersBuilder& headers, const TString& redirectUrl) {
7059
headers.Set("Content-Type", "application/json; charset=utf-8");
7160
SetCORS(request, &headers);
@@ -84,6 +73,17 @@ TStringBuf GetRequestedUrl(const NHttp::THttpIncomingRequestPtr& request, bool i
8473

8574
} // namespace
8675

76+
void SetCORS(const NHttp::THttpIncomingRequestPtr& request, NHttp::THeadersBuilder* const headers) {
77+
TString origin = TString(NHttp::THeaders(request->Headers)["Origin"]);
78+
if (origin.empty()) {
79+
origin = "*";
80+
}
81+
headers->Set("Access-Control-Allow-Origin", origin);
82+
headers->Set("Access-Control-Allow-Credentials", "true");
83+
headers->Set("Access-Control-Allow-Headers", "Content-Type,Authorization,Origin,Accept");
84+
headers->Set("Access-Control-Allow-Methods", "OPTIONS, GET, POST");
85+
}
86+
8787
TString HmacSHA256(TStringBuf key, TStringBuf data) {
8888
unsigned char hash[SHA256_DIGEST_LENGTH];
8989
ui32 hl = SHA256_DIGEST_LENGTH;

‎ydb/mvp/oidc_proxy/openid_connect.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ TString CreateNameYdbOidcCookie(TStringBuf key, TStringBuf state);
5858
TString CreateNameSessionCookie(TStringBuf key);
5959
const TString& GetAuthCallbackUrl();
6060
TString CreateSecureCookie(const TString& name, const TString& value);
61+
void SetCORS(const NHttp::THttpIncomingRequestPtr& request, NHttp::THeadersBuilder* const headers);
6162

6263
template <typename TSessionService>
6364
std::unique_ptr<NYdbGrpc::TServiceConnection<TSessionService>> CreateGRpcServiceConnection(const TString& endpoint) {

0 commit comments

Comments
 (0)