From a91d66415cee77687b365d73c3a5cb0e53dd68a7 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Sun, 17 Mar 2024 10:46:14 +0800 Subject: [PATCH 01/11] support forwarding username to backends Signed-off-by: huabing zhao --- .../filters/http/basic_auth/v3/basic_auth.proto | 8 ++++++++ .../filters/http/basic_auth/basic_auth_filter.cc | 10 ++++++++-- .../filters/http/basic_auth/basic_auth_filter.h | 5 ++++- source/extensions/filters/http/basic_auth/config.cc | 4 ++-- .../http/basic_auth/basic_auth_integration_test.cc | 6 ++++++ test/extensions/filters/http/basic_auth/filter_test.cc | 5 ++++- 6 files changed, 32 insertions(+), 6 deletions(-) diff --git a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto index df23868a42602..c99d281620632 100644 --- a/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto +++ b/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto @@ -6,6 +6,7 @@ import "envoy/config/core/v3/base.proto"; import "udpa/annotations/sensitive.proto"; import "udpa/annotations/status.proto"; +import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.filters.http.basic_auth.v3"; option java_outer_classname = "BasicAuthProto"; @@ -33,4 +34,11 @@ message BasicAuth { // The value needs to be the htpasswd format. // Reference to https://httpd.apache.org/docs/2.4/programs/htpasswd.html config.core.v3.DataSource users = 1 [(udpa.annotations.sensitive) = true]; + + // This field specifies the header name to forward a successfully authenticated user to + // the backend. The header will be added to the request with the username as the value. + // + // If it is not specified, the username will not be forwarded. + string forward_username_header = 2 + [(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME strict: false}]; } diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc index d9946ae458512..058d1c4747950 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc @@ -26,8 +26,10 @@ std::string computeSHA1(absl::string_view password) { } // namespace -FilterConfig::FilterConfig(UserMap&& users, const std::string& stats_prefix, Stats::Scope& scope) - : users_(std::move(users)), stats_(generateStats(stats_prefix + "basic_auth.", scope)) {} +FilterConfig::FilterConfig(UserMap&& users, const std::string& forward_username_header, + const std::string& stats_prefix, Stats::Scope& scope) + : users_(std::move(users)), forward_username_header_(forward_username_header), + stats_(generateStats(stats_prefix + "basic_auth.", scope)) {} bool FilterConfig::validateUser(absl::string_view username, absl::string_view password) const { auto user = users_.find(username); @@ -75,6 +77,10 @@ Http::FilterHeadersStatus BasicAuthFilter::decodeHeaders(Http::RequestHeaderMap& "invalid_credential_for_basic_auth"); } + if (!config_->forwardUsernameHeader().empty()) { + headers.addCopy(Http::LowerCaseString(config_->forwardUsernameHeader()), username); + } + config_->stats().allowed_.inc(); return Http::FilterHeadersStatus::Continue; } diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.h b/source/extensions/filters/http/basic_auth/basic_auth_filter.h index 38553f655eb1b..cfbc178d300db 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.h +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.h @@ -43,9 +43,11 @@ using UserMap = absl::flat_hash_map; */ class FilterConfig { public: - FilterConfig(UserMap&& users, const std::string& stats_prefix, Stats::Scope& scope); + FilterConfig(UserMap&& users, const std::string& forward_username_header, + const std::string& stats_prefix, Stats::Scope& scope); const BasicAuthStats& stats() const { return stats_; } bool validateUser(absl::string_view username, absl::string_view password) const; + const std::string& forwardUsernameHeader() const { return forward_username_header_; } private: static BasicAuthStats generateStats(const std::string& prefix, Stats::Scope& scope) { @@ -53,6 +55,7 @@ class FilterConfig { } const UserMap users_; + const std::string forward_username_header_; BasicAuthStats stats_; }; using FilterConfigConstSharedPtr = std::shared_ptr; diff --git a/source/extensions/filters/http/basic_auth/config.cc b/source/extensions/filters/http/basic_auth/config.cc index 0b0332d8ec99f..118d6351c246e 100644 --- a/source/extensions/filters/http/basic_auth/config.cc +++ b/source/extensions/filters/http/basic_auth/config.cc @@ -66,8 +66,8 @@ Http::FilterFactoryCb BasicAuthFilterFactory::createFilterFactoryFromProtoTyped( UserMap users = readHtpasswd(THROW_OR_RETURN_VALUE( Config::DataSource::read(proto_config.users(), false, context.serverFactoryContext().api()), std::string)); - FilterConfigConstSharedPtr config = - std::make_unique(std::move(users), stats_prefix, context.scope()); + FilterConfigConstSharedPtr config = std::make_unique( + std::move(users), proto_config.forward_username_header(), stats_prefix, context.scope()); return [config](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamDecoderFilter(std::make_shared(config)); }; diff --git a/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc b/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc index c66d98084b1f3..cf57075779fa6 100644 --- a/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc +++ b/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc @@ -23,6 +23,7 @@ name: envoy.filters.http.basic_auth inline_string: |- user1:{SHA}tESsBmE/yNY3lb6a0L6vVQEZNqw= user2:{SHA}EJ9LPFDXsN9ynSmbxvjp75Bmlx8= + forward_username_header: x-username )EOF"; config_helper_.prependFilter(filter_config); initialize(); @@ -51,6 +52,11 @@ TEST_P(BasicAuthIntegrationTestAllProtocols, ValidCredential) { }); waitForNextUpstreamRequest(); + + const auto username_entry = upstream_request_->headers().get(Http::LowerCaseString("x-username")); + EXPECT_FALSE(username_entry.empty()); + EXPECT_EQ(username_entry[0]->value().getStringView(), "user1"); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); ASSERT_TRUE(response->waitForEndStream()); ASSERT_TRUE(response->complete()); diff --git a/test/extensions/filters/http/basic_auth/filter_test.cc b/test/extensions/filters/http/basic_auth/filter_test.cc index 375c005705bc9..91551d730a64c 100644 --- a/test/extensions/filters/http/basic_auth/filter_test.cc +++ b/test/extensions/filters/http/basic_auth/filter_test.cc @@ -18,7 +18,8 @@ class FilterTest : public testing::Test { UserMap users; users.insert({"user1", {"user1", "tESsBmE/yNY3lb6a0L6vVQEZNqw="}}); // user1:test1 users.insert({"user2", {"user2", "EJ9LPFDXsN9ynSmbxvjp75Bmlx8="}}); // user2:test2 - config_ = std::make_unique(std::move(users), "stats", *stats_.rootScope()); + config_ = std::make_unique(std::move(users), "x-username", "stats", + *stats_.rootScope()); filter_ = std::make_shared(config_); filter_->setDecoderFilterCallbacks(decoder_filter_callbacks_); } @@ -35,12 +36,14 @@ TEST_F(FilterTest, BasicAuth) { EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_user1, true)); + EXPECT_EQ("user1", request_headers_user1.get_("x-username")); // user2:test2 Http::TestRequestHeaderMapImpl request_headers_user2{{"Authorization", "Basic dXNlcjI6dGVzdDI="}}; EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_user2, true)); + EXPECT_EQ("user2", request_headers_user2.get_("x-username")); } TEST_F(FilterTest, UserNotExist) { From b39bc7efb46669f7774e902d37b7aeab16818406 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Mon, 18 Mar 2024 01:27:42 +0000 Subject: [PATCH 02/11] fix format Signed-off-by: Huabing Zhao --- .../filters/http/basic_auth/basic_auth_integration_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc b/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc index cf57075779fa6..8fdf4a9c3bf77 100644 --- a/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc +++ b/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc @@ -23,7 +23,7 @@ name: envoy.filters.http.basic_auth inline_string: |- user1:{SHA}tESsBmE/yNY3lb6a0L6vVQEZNqw= user2:{SHA}EJ9LPFDXsN9ynSmbxvjp75Bmlx8= - forward_username_header: x-username + forward_username_header: x-username )EOF"; config_helper_.prependFilter(filter_config); initialize(); From 5a91ceb6b9815009ee27a1d89745d70ac3eeeaf7 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Mon, 18 Mar 2024 12:16:20 +0800 Subject: [PATCH 03/11] add change log Signed-off-by: huabing zhao --- changelogs/current.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 4bb71b22796cf..cfba60022eaa1 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -203,6 +203,10 @@ removed_config_or_runtime: removed ``envoy_reloadable_features_initialize_upstream_filters`` and legacy code paths. new_features: + area: basic_auth + change: | + Added :ref:`forward_username_header ` config to + forward the username to the backend. - area: ext_proc change: | Added From 61e45d115c778c511ce1b89b6e5df23f166a6135 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Mon, 18 Mar 2024 06:17:43 +0000 Subject: [PATCH 04/11] fix format Signed-off-by: Huabing Zhao --- changelogs/current.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index cfba60022eaa1..9b5cc6fbf39c7 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -205,8 +205,8 @@ removed_config_or_runtime: new_features: area: basic_auth change: | - Added :ref:`forward_username_header ` config to - forward the username to the backend. + Added :ref:`forward_username_header ` + config to forward the username to the backend. - area: ext_proc change: | Added From 00bfe8c3e9193979e5e5fe7628b4c64bf826706b Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Mon, 18 Mar 2024 15:42:34 +0800 Subject: [PATCH 05/11] fix lint Signed-off-by: huabing zhao --- changelogs/current.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 9b5cc6fbf39c7..354780471c1b9 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -203,7 +203,7 @@ removed_config_or_runtime: removed ``envoy_reloadable_features_initialize_upstream_filters`` and legacy code paths. new_features: - area: basic_auth +- area: basic_auth change: | Added :ref:`forward_username_header ` config to forward the username to the backend. From 2e4d4aab87bfeaea450dc095793216d782ea85ed Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Tue, 19 Mar 2024 11:30:03 +0800 Subject: [PATCH 06/11] address comments Signed-off-by: huabing zhao --- source/extensions/filters/http/basic_auth/basic_auth_filter.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc index 058d1c4747950..862470d96f582 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc @@ -78,7 +78,7 @@ Http::FilterHeadersStatus BasicAuthFilter::decodeHeaders(Http::RequestHeaderMap& } if (!config_->forwardUsernameHeader().empty()) { - headers.addCopy(Http::LowerCaseString(config_->forwardUsernameHeader()), username); + headers.setCopy(Http::LowerCaseString(config_->forwardUsernameHeader()), username); } config_->stats().allowed_.inc(); From 7c63ef6758ad31a677ed0fa90000f81d40084d84 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Thu, 21 Mar 2024 16:01:37 +0800 Subject: [PATCH 07/11] add test for existing user in headers Signed-off-by: huabing zhao --- .../basic_auth/basic_auth_integration_test.cc | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc b/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc index 8fdf4a9c3bf77..482c428eabd0d 100644 --- a/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc +++ b/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc @@ -118,6 +118,33 @@ TEST_P(BasicAuthIntegrationTestAllProtocols, NoneExistedUser) { EXPECT_EQ("401", response->headers().getStatusValue()); EXPECT_EQ("User authentication failed. Invalid username/password combination.", response->body()); } + +// Request with existing username +TEST_P(BasicAuthIntegrationTestAllProtocols, ValidCredential) { + initializeFilter(); + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ + {":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"Authorization", "Basic dXNlcjE6dGVzdDE="}, // user1, test1 + {"x-username", "existingUser"}, + }); + + waitForNextUpstreamRequest(); + + const auto username_entry = upstream_request_->headers().get(Http::LowerCaseString("x-username")); + EXPECT_FALSE(username_entry.empty()); + EXPECT_EQ(username_entry[0]->value().getStringView(), "user1"); + + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); +} + } // namespace } // namespace BasicAuth } // namespace HttpFilters From ab55348dc6792c00d6a0ff0de4f8a6263d55fd5e Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Thu, 21 Mar 2024 16:18:43 +0800 Subject: [PATCH 08/11] add test for existing user in headers Signed-off-by: huabing zhao --- .../basic_auth/basic_auth_integration_test.cc | 6 +++--- .../filters/http/basic_auth/filter_test.cc | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc b/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc index 482c428eabd0d..fd997b75c00d9 100644 --- a/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc +++ b/test/extensions/filters/http/basic_auth/basic_auth_integration_test.cc @@ -119,8 +119,8 @@ TEST_P(BasicAuthIntegrationTestAllProtocols, NoneExistedUser) { EXPECT_EQ("User authentication failed. Invalid username/password combination.", response->body()); } -// Request with existing username -TEST_P(BasicAuthIntegrationTestAllProtocols, ValidCredential) { +// Request with existing username header +TEST_P(BasicAuthIntegrationTestAllProtocols, ExistingUsernameHeader) { initializeFilter(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -130,7 +130,7 @@ TEST_P(BasicAuthIntegrationTestAllProtocols, ValidCredential) { {":scheme", "http"}, {":authority", "host"}, {"Authorization", "Basic dXNlcjE6dGVzdDE="}, // user1, test1 - {"x-username", "existingUser"}, + {"x-username", "existingUsername"}, }); waitForNextUpstreamRequest(); diff --git a/test/extensions/filters/http/basic_auth/filter_test.cc b/test/extensions/filters/http/basic_auth/filter_test.cc index 91551d730a64c..7f3931cb86c01 100644 --- a/test/extensions/filters/http/basic_auth/filter_test.cc +++ b/test/extensions/filters/http/basic_auth/filter_test.cc @@ -133,6 +133,23 @@ TEST_F(FilterTest, HasAuthHeaderButNoColon) { filter_->decodeHeaders(request_headers_user1, true)); } +TEST_F(FilterTest, ExistingUsernameHeader) { + // user1:test1 + Http::TestRequestHeaderMapImpl request_headers_user1{{"Authorization", "Basic dXNlcjE6dGVzdDE="}}, + {"x-username", "existingUsername"}; + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_->decodeHeaders(request_headers_user1, true)); + EXPECT_EQ("user1", request_headers_user1.get_("x-username")); + + // user2:test2 + Http::TestRequestHeaderMapImpl request_headers_user2{{"Authorization", "Basic dXNlcjI6dGVzdDI="}}; + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_->decodeHeaders(request_headers_user2, true)); + EXPECT_EQ("user2", request_headers_user2.get_("x-username")); +} + } // namespace BasicAuth } // namespace HttpFilters } // namespace Extensions From d142e74cd78fb79a7ef0aaef9a5ded8cdf73a1cd Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Fri, 22 Mar 2024 08:56:10 +0800 Subject: [PATCH 09/11] fix test Signed-off-by: huabing zhao --- .../filters/http/basic_auth/filter_test.cc | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/test/extensions/filters/http/basic_auth/filter_test.cc b/test/extensions/filters/http/basic_auth/filter_test.cc index 7f3931cb86c01..c395310494c9c 100644 --- a/test/extensions/filters/http/basic_auth/filter_test.cc +++ b/test/extensions/filters/http/basic_auth/filter_test.cc @@ -135,19 +135,13 @@ TEST_F(FilterTest, HasAuthHeaderButNoColon) { TEST_F(FilterTest, ExistingUsernameHeader) { // user1:test1 - Http::TestRequestHeaderMapImpl request_headers_user1{{"Authorization", "Basic dXNlcjE6dGVzdDE="}}, - {"x-username", "existingUsername"}; + Http::TestRequestHeaderMapImpl request_headers_user1 { + {"Authorization", "Basic dXNlcjE6dGVzdDE="}, {"x-username", "existingUsername"}; - EXPECT_EQ(Http::FilterHeadersStatus::Continue, - filter_->decodeHeaders(request_headers_user1, true)); - EXPECT_EQ("user1", request_headers_user1.get_("x-username")); - - // user2:test2 - Http::TestRequestHeaderMapImpl request_headers_user2{{"Authorization", "Basic dXNlcjI6dGVzdDI="}}; - - EXPECT_EQ(Http::FilterHeadersStatus::Continue, - filter_->decodeHeaders(request_headers_user2, true)); - EXPECT_EQ("user2", request_headers_user2.get_("x-username")); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_->decodeHeaders(request_headers_user1, true)); + EXPECT_EQ("user1", request_headers_user1.get_("x-username")); + } } } // namespace BasicAuth From 671f671c05c90b8c8873dc538aa71391053021cf Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Fri, 22 Mar 2024 08:58:16 +0800 Subject: [PATCH 10/11] fix test Signed-off-by: huabing zhao --- .../filters/http/basic_auth/filter_test.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/extensions/filters/http/basic_auth/filter_test.cc b/test/extensions/filters/http/basic_auth/filter_test.cc index c395310494c9c..f5d1ce950a8ca 100644 --- a/test/extensions/filters/http/basic_auth/filter_test.cc +++ b/test/extensions/filters/http/basic_auth/filter_test.cc @@ -135,13 +135,13 @@ TEST_F(FilterTest, HasAuthHeaderButNoColon) { TEST_F(FilterTest, ExistingUsernameHeader) { // user1:test1 - Http::TestRequestHeaderMapImpl request_headers_user1 { - {"Authorization", "Basic dXNlcjE6dGVzdDE="}, {"x-username", "existingUsername"}; + Http::TestRequestHeaderMapImpl request_headers_user1{{"Authorization", "Basic dXNlcjE6dGVzdDE="}, + {"x-username", "existingUsername"}}; - EXPECT_EQ(Http::FilterHeadersStatus::Continue, - filter_->decodeHeaders(request_headers_user1, true)); - EXPECT_EQ("user1", request_headers_user1.get_("x-username")); - } + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_->decodeHeaders(request_headers_user1, true)); + EXPECT_EQ("user1", request_headers_user1.get_("x-username")); +} } } // namespace BasicAuth From d63daf078985cd65e731470f49955656154b2b85 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Fri, 22 Mar 2024 08:59:54 +0800 Subject: [PATCH 11/11] fix test Signed-off-by: huabing zhao --- test/extensions/filters/http/basic_auth/filter_test.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/extensions/filters/http/basic_auth/filter_test.cc b/test/extensions/filters/http/basic_auth/filter_test.cc index f5d1ce950a8ca..2eb950292f005 100644 --- a/test/extensions/filters/http/basic_auth/filter_test.cc +++ b/test/extensions/filters/http/basic_auth/filter_test.cc @@ -142,7 +142,6 @@ TEST_F(FilterTest, ExistingUsernameHeader) { filter_->decodeHeaders(request_headers_user1, true)); EXPECT_EQ("user1", request_headers_user1.get_("x-username")); } -} } // namespace BasicAuth } // namespace HttpFilters