From e716b9f072670a273437ed1a94d2a025a040861b Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Tue, 24 Mar 2020 13:54:05 -0700 Subject: [PATCH] Add X-Forwarded-Authorization header (#90) * Add X-Forwarded-Authorization header Signed-off-by: Wayne Zhang * move test cases to backend_auth_disable_auth_tests.go * rebase and fix --- src/envoy/http/backend_auth/filter.cc | 12 ++++-- src/envoy/http/backend_auth/filter_test.cc | 42 +++++++++++++++++++ tests/endpoints/echo/server/app.go | 7 +++- tests/env/platform/files.go | 2 +- .../backend_auth_disable_auth_test.go | 17 +++++++- 5 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/envoy/http/backend_auth/filter.cc b/src/envoy/http/backend_auth/filter.cc index e3da71eda..3f894bd16 100644 --- a/src/envoy/http/backend_auth/filter.cc +++ b/src/envoy/http/backend_auth/filter.cc @@ -41,6 +41,10 @@ struct RcDetailsValues { }; typedef Envoy::ConstSingleton RcDetails; +// The Http header to copy the original Authorization before it is overwritten. +const Envoy::Http::LowerCaseString kXForwardedAuthorization{ + "x-forwarded-authorization"}; + } // namespace FilterHeadersStatus Filter::decodeHeaders(RequestHeaderMap& headers, bool) { @@ -72,9 +76,11 @@ FilterHeadersStatus Filter::decodeHeaders(RequestHeaderMap& headers, bool) { return FilterHeadersStatus::StopIteration; } - const auto& authorization = Envoy::Http::Headers::get().Authorization; - headers.remove(authorization); - headers.addCopy(authorization, kBearer + *jwt_token); + if (headers.Authorization()) { + headers.addCopy(kXForwardedAuthorization, + headers.Authorization()->value().getStringView()); + } + headers.setAuthorization(kBearer + *jwt_token); config_->stats().token_added_.inc(); return FilterHeadersStatus::Continue; } diff --git a/src/envoy/http/backend_auth/filter_test.cc b/src/envoy/http/backend_auth/filter_test.cc index 911f757ab..17e444af2 100644 --- a/src/envoy/http/backend_auth/filter_test.cc +++ b/src/envoy/http/backend_auth/filter_test.cc @@ -31,6 +31,9 @@ namespace envoy { namespace http_filters { namespace backend_auth { +const Envoy::Http::LowerCaseString kXForwardedAuthorization{ + "x-forwarded-authorization"}; + /** * Base class for testing the Backend Auth filter. Makes a simple request * with no query parameters in the request URL. @@ -142,6 +145,45 @@ TEST_F(BackendAuthFilterTest, SucceedAppendToken) { ->value() .getStringView(), "Bearer this-is-token"); + EXPECT_EQ(headers.get(kXForwardedAuthorization), nullptr); + EXPECT_EQ(status, Envoy::Http::FilterHeadersStatus::Continue); +} + +TEST_F(BackendAuthFilterTest, SucceedTokenCopied) { + Envoy::Http::TestRequestHeaderMapImpl headers{ + {":method", "GET"}, + {":path", "/books/1"}, + {"authorization", "Bearer origin-token"}}; + utils::setStringFilterState( + *mock_decoder_callbacks_.stream_info_.filter_state_, utils::kOperation, + "operation-with-audience"); + testing::NiceMock scope; + const std::string prefix = Envoy::EMPTY_STRING; + FilterStats filter_stats{ + ALL_BACKEND_AUTH_FILTER_STATS(POOL_COUNTER_PREFIX(scope, prefix))}; + + EXPECT_CALL(*mock_filter_config_, cfg_parser) + .WillRepeatedly(testing::ReturnRef(*mock_filter_config_parser_)); + EXPECT_CALL(*mock_filter_config_, stats) + .WillRepeatedly(testing::ReturnRef(filter_stats)); + + EXPECT_CALL(*mock_filter_config_parser_, getAudience) + .Times(1) + .WillRepeatedly(testing::Return("this-is-audience")); + EXPECT_CALL(*mock_filter_config_parser_, getJwtToken) + .Times(1) + .WillRepeatedly( + testing::Return(std::make_shared("new-id-token"))); + + Envoy::Http::FilterHeadersStatus status = + filter_->decodeHeaders(headers, false); + + EXPECT_EQ(headers.get(Envoy::Http::Headers::get().Authorization) + ->value() + .getStringView(), + "Bearer new-id-token"); + EXPECT_EQ(headers.get(kXForwardedAuthorization)->value().getStringView(), + "Bearer origin-token"); EXPECT_EQ(status, Envoy::Http::FilterHeadersStatus::Continue); } diff --git a/tests/endpoints/echo/server/app.go b/tests/endpoints/echo/server/app.go index 30725a346..96c56d7dd 100644 --- a/tests/endpoints/echo/server/app.go +++ b/tests/endpoints/echo/server/app.go @@ -255,7 +255,12 @@ func bearerTokenHandler(w http.ResponseWriter, r *http.Request) { w.Header().Set("Access-Control-Expose-Headers", fmt.Sprintf("X-Token: %s", bearerToken)) } reqURI := r.URL.RequestURI() - resp := fmt.Sprintf(`{"Authorization": "%s", "RequestURI": "%s"}`, bearerToken, reqURI) + xForwarded := r.Header.Get("X-Forwarded-Authorization") + resp := fmt.Sprintf(`{"Authorization": "%s", "RequestURI": "%s"`, bearerToken, reqURI) + if xForwarded != "" { + resp += fmt.Sprintf(`, "X-Forwarded-Authorization": "%s"`, xForwarded) + } + resp += "}" w.Write([]byte(resp)) } diff --git a/tests/env/platform/files.go b/tests/env/platform/files.go index 6b1f7860e..de5a48733 100644 --- a/tests/env/platform/files.go +++ b/tests/env/platform/files.go @@ -89,7 +89,7 @@ var fileMap = map[RuntimeFile]string{ PmEnvoyConfig: "../../../../src/go/bootstrap/static/testdata/path_matcher/envoy_config.json", GrpcEchoServiceConfig: "../../../../examples/grpc_dynamic_routing/service_config_generated.json", GrpcEchoEnvoyConfig: "../../../../examples/grpc_dynamic_routing/envoy_config.json", - FakeServiceAccount: "./testdata/fake_service_account.json", + FakeServiceAccount: "./testdata/fake_service_account.json", } // Get the runtime file path for the specified file. diff --git a/tests/integration_test/backend_auth_disable_auth_test.go b/tests/integration_test/backend_auth_disable_auth_test.go index 25c60d2fc..036d5e25a 100644 --- a/tests/integration_test/backend_auth_disable_auth_test.go +++ b/tests/integration_test/backend_auth_disable_auth_test.go @@ -46,6 +46,7 @@ func TestBackendAuthDisableAuth(t *testing.T) { desc string method string path string + headers map[string]string message string wantResp string }{ @@ -67,6 +68,20 @@ func TestBackendAuthDisableAuth(t *testing.T) { path: "/disableauthsettofalse/constant/disableauthsettofalse", wantResp: `{"Authorization": "Bearer ya29.DefaultAuth", "RequestURI": "/bearertoken/constant?foo=disableauthsettofalse"}`, }, + { + desc: "With disable_auth=true, original Authorization header is preversed, X-Forwarded-Authorization is not set", + method: "GET", + path: "/disableauthsettotrue/constant/disableauthsettotrue", + headers: map[string]string{"Authorization": "Bearer origin-token"}, + wantResp: `{"Authorization": "Bearer origin-token", "RequestURI": "/bearertoken/constant?foo=disableauthsettotrue"}`, + }, + { + desc: "With disable_auth=false, original Authorization header is copied to X-Forwarded-Authorization", + method: "GET", + path: "/disableauthsettofalse/constant/disableauthsettofalse", + headers: map[string]string{"Authorization": "Bearer origin-token"}, + wantResp: `{"Authorization": "Bearer ya29.DefaultAuth", "RequestURI": "/bearertoken/constant?foo=disableauthsettofalse", "X-Forwarded-Authorization":"Bearer origin-token"}`, + }, { desc: "Authentication is not set so JwtAudience is set with the backend address", method: "GET", @@ -77,7 +92,7 @@ func TestBackendAuthDisableAuth(t *testing.T) { for _, tc := range testData { url := fmt.Sprintf("http://localhost:%v%v", s.Ports().ListenerPort, tc.path) - resp, err := client.DoWithHeaders(url, tc.method, tc.message, nil) + resp, err := client.DoWithHeaders(url, tc.method, tc.message, tc.headers) if err != nil { t.Fatalf("Test Desc(%s): %v", tc.desc, err)