OIDC proxy: Handle response with bad request from protected resource (#8105)

Author: molotkov-and

ydb/mvp/oidc_proxy/oidc_protected_page.h

@@ -73,14 +73,9 @@ class THandlerSessionServiceCheck : public NActors::TActorBootstrapped<THandlerS
         if (event->Get()->Response != nullptr) {
             NHttp::THttpIncomingResponsePtr response = event->Get()->Response;
             LOG_DEBUG_S(ctx, EService::MVP, "Incoming response for protected resource: " << response->Status);
-            if ((response->Status == "400" || response->Status.empty()) && RequestedPageScheme.empty()) {
-                NHttp::THttpOutgoingRequestPtr request = response->GetRequest();
-                if (!request->Secure) {
-                    LOG_DEBUG_S(ctx, EService::MVP, "Try to send request to HTTPS port");
-                    NHttp::THeadersBuilder headers {request->Headers};
-                    ForwardUserRequest(headers.Get(AUTH_HEADER_NAME), ctx, true);
-                    return;
-                }
+            if (NeedSendSecureHttpRequest(response)) {
+                SendSecureHttpRequest(response, ctx);
+                return;
             }
             NHttp::THeadersBuilder headers = GetResponseHeaders(response);
             TStringBuf contentType = headers.Get("Content-Type").NextTok(';');
@@ -117,7 +112,7 @@ class THandlerSessionServiceCheck : public NActors::TActorBootstrapped<THandlerS
         return it != Settings.AllowedProxyHosts.cend();
     }
 
-    bool IsAuthorizedRequest(TStringBuf authHeader) {
+    static bool IsAuthorizedRequest(TStringBuf authHeader) {
         if (authHeader.empty()) {
             return false;
         }
@@ -142,7 +137,9 @@ class THandlerSessionServiceCheck : public NActors::TActorBootstrapped<THandlerS
         ctx.Send(HttpProxyId, new NHttp::TEvHttpProxy::TEvHttpOutgoingRequest(httpRequest));
     }
 
-    TString FixReferenceInHtml(TStringBuf html, TStringBuf host, TStringBuf findStr) {
+    virtual bool NeedSendSecureHttpRequest(const NHttp::THttpIncomingResponsePtr& response) const = 0;
+
+    static TString FixReferenceInHtml(TStringBuf html, TStringBuf host, TStringBuf findStr) {
         TStringBuilder result;
         size_t n = html.find(findStr);
         if (n == TStringBuf::npos) {
@@ -166,14 +163,14 @@ class THandlerSessionServiceCheck : public NActors::TActorBootstrapped<THandlerS
         return result;
     }
 
-    TString FixReferenceInHtml(TStringBuf html, TStringBuf host) {
+    static TString FixReferenceInHtml(TStringBuf html, TStringBuf host) {
         TStringBuf findString = "href=";
         auto result = FixReferenceInHtml(html, host, findString);
         findString = "src=";
         return FixReferenceInHtml(result, host, findString);
     }
 
-    void ForwardRequestHeaders(NHttp::THttpOutgoingRequestPtr& request) {
+    void ForwardRequestHeaders(NHttp::THttpOutgoingRequestPtr& request) const {
         static const TVector<TStringBuf> HEADERS_WHITE_LIST = {
             "Connection",
             "Accept-Language",
@@ -195,7 +192,7 @@ class THandlerSessionServiceCheck : public NActors::TActorBootstrapped<THandlerS
         request->Set("Accept-Encoding", "deflate");
     }
 
-    NHttp::THeadersBuilder GetResponseHeaders(const NHttp::THttpIncomingResponsePtr& response) {
+    static NHttp::THeadersBuilder GetResponseHeaders(const NHttp::THttpIncomingResponsePtr& response) {
         static const TVector<TStringBuf> HEADERS_WHITE_LIST = {
             "Content-Type",
             "Connection",
@@ -216,6 +213,14 @@ class THandlerSessionServiceCheck : public NActors::TActorBootstrapped<THandlerS
         }
         return resultHeaders;
     }
+
+private:
+    void SendSecureHttpRequest(const NHttp::THttpIncomingResponsePtr& response, const NActors::TActorContext& ctx) {
+        NHttp::THttpOutgoingRequestPtr request = response->GetRequest();
+        LOG_DEBUG_S(ctx, EService::MVP, "Try to send request to HTTPS port");
+        NHttp::THeadersBuilder headers {request->Headers};
+        ForwardUserRequest(headers.Get(AUTH_HEADER_NAME), ctx, true);
+    }
 };
 
 }  // NMVP

ydb/mvp/oidc_proxy/oidc_protected_page_nebius.h

@@ -121,6 +121,13 @@ class THandlerSessionServiceCheckNebius : public THandlerSessionServiceCheck {
         THandlerSessionServiceCheck::ForwardUserRequest(authHeader, ctx, secure);
         Become(&THandlerSessionServiceCheckNebius::StateWork);
     }
+
+    bool NeedSendSecureHttpRequest(const NHttp::THttpIncomingResponsePtr& response) const override {
+        if ((response->Status == "400" || response->Status.empty()) && RequestedPageScheme.empty()) {
+            return !response->GetRequest()->Secure;
+        }
+        return false;
+    }
 };
 
 }  // NMVP

ydb/mvp/oidc_proxy/oidc_protected_page_yandex.h

@@ -80,6 +80,20 @@ class THandlerSessionServiceCheckYandex : public THandlerSessionServiceCheck {
         meta.Timeout = TDuration::Seconds(10);
         connection->DoRequest(request, std::move(responseCb), &yandex::cloud::priv::oauth::v1::SessionService::Stub::AsyncCheck, meta);
     }
+
+    bool NeedSendSecureHttpRequest(const NHttp::THttpIncomingResponsePtr& response) const override {
+        if ((response->Status == "400" || response->Status.empty()) && RequestedPageScheme.empty()) {
+            NHttp::THttpOutgoingRequestPtr request = response->GetRequest();
+            if (!request->Secure) {
+                static const TStringBuf bodyContent = "The plain HTTP request was sent to HTTPS port";
+                NHttp::THeadersBuilder headers(response->Headers);
+                TStringBuf contentType = headers.Get("Content-Type").NextTok(';');
+                TStringBuf body = response->Body;
+                return contentType == "text/html" && body.find(bodyContent) != TStringBuf::npos;
+            }
+        }
+        return false;
+    }
 };
 
 }  // NMVP

ydb/mvp/oidc_proxy/oidc_proxy_ut.cpp

@@ -231,7 +231,25 @@ Y_UNIT_TEST_SUITE(Mvp) {
 
         auto outgoingResponseEv = runtime.GrabEdgeEvent<NHttp::TEvHttpProxy::TEvHttpOutgoingResponse>(handle);
         UNIT_ASSERT_STRINGS_EQUAL(outgoingResponseEv->Response->Status, "200");
-        UNIT_ASSERT_STRINGS_EQUAL(outgoingResponseEv->Response->Body, "this is test");
+        UNIT_ASSERT_STRINGS_EQUAL(outgoingResponseEv->Response->Body, okResponseBody);
+
+        runtime.Send(new IEventHandle(target, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingRequest(incomingRequest)));
+        outgoingRequestEv = runtime.GrabEdgeEvent<NHttp::TEvHttpProxy::TEvHttpOutgoingRequest>(handle);
+        UNIT_ASSERT_STRINGS_EQUAL(outgoingRequestEv->Request->Host, allowedProxyHost);
+        UNIT_ASSERT_STRINGS_EQUAL(outgoingRequestEv->Request->URL, "/counters");
+        UNIT_ASSERT_STRING_CONTAINS(outgoingRequestEv->Request->Headers, "Authorization: Bearer protected_page_iam_token");
+        UNIT_ASSERT_EQUAL(outgoingRequestEv->Request->Secure, false);
+        incomingResponse = new NHttp::THttpIncomingResponse(outgoingRequestEv->Request);
+        const TString errorJsonResponseBody {"{\"status\":\"400\", \"message\":\"Table does not exist\"}"};
+        EatWholeString(incomingResponse, "HTTP/1.1 400 Bad Request\r\n"
+                                         "Connection: close\r\n"
+                                         "Content-Type: application/json; charset=utf-8\r\n"
+                                         "Content-Length: " + ToString(errorJsonResponseBody.size()) + "\r\n\r\n" + errorJsonResponseBody);
+        runtime.Send(new IEventHandle(handle->Sender, edge, new NHttp::TEvHttpProxy::TEvHttpIncomingResponse(outgoingRequestEv->Request, incomingResponse)));
+
+        outgoingResponseEv = runtime.GrabEdgeEvent<NHttp::TEvHttpProxy::TEvHttpOutgoingResponse>(handle);
+        UNIT_ASSERT_STRINGS_EQUAL(outgoingResponseEv->Response->Status, "400");
+        UNIT_ASSERT_STRINGS_EQUAL(outgoingResponseEv->Response->Body, errorJsonResponseBody);
     }