From 6972cbc7495f4e4c8a2f39dc320e0e4f085bcaed Mon Sep 17 00:00:00 2001 From: QuentinBisson Date: Wed, 11 Mar 2026 16:21:35 +0100 Subject: [PATCH] basic_auth: add allow_missing field and emit dynamic metadata on success When allow_missing is true the filter passes through requests that carry no Basic credentials (absent Authorization header, or a non-Basic scheme such as Bearer). Requests that do present Basic credentials are still fully validated and invalid credentials are still rejected. On every successful authentication the filter now emits dynamic metadata under the envoy.filters.http.basic_auth namespace with key 'username'. This allows downstream RBAC filters to detect that BasicAuth succeeded, which is the missing piece needed to implement OR semantics when combining BasicAuth with other auth methods (e.g. JWT). See envoyproxy/gateway#8491 for the motivating use case. Signed-off-by: QuentinBisson --- .../http/basic_auth/v3/basic_auth.proto | 15 ++ .../http/basic_auth/basic_auth_filter.cc | 21 ++- .../http/basic_auth/basic_auth_filter.h | 7 +- .../filters/http/basic_auth/config.cc | 6 +- .../filters/http/basic_auth/filter_test.cc | 136 +++++++++++++++++- 5 files changed, 178 insertions(+), 7 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 af3c442c031a5..e3c16052be178 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 @@ -47,6 +47,21 @@ message BasicAuth { // If it is not specified, the filter loads the credential from the "Authorization" header. string authentication_header = 3 [(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME strict: false}]; + + // If set to true, requests without Basic credentials (missing Authorization header, or + // Authorization header with a non-Basic scheme such as Bearer) are allowed to pass through + // without authentication. Requests that present Basic credentials are still fully validated. + // + // This is useful when combining BasicAuth with other authentication methods (e.g. JWT) to + // achieve OR semantics: a request is accepted if any one configured auth method succeeds. + // When allow_missing is true on all auth filters, pair it with an RBAC filter that checks + // the ``envoy.filters.http.basic_auth`` dynamic metadata to ensure at least one method + // authenticated the request. + // + // On successful authentication, this filter always emits dynamic metadata under the + // ``envoy.filters.http.basic_auth`` namespace with key ``username`` set to the + // authenticated username, regardless of this field. + bool allow_missing = 4; } // Extra settings that may be added to per-route configuration for 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 581d462c19c1f..49992229b5957 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.cc +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.cc @@ -8,6 +8,7 @@ #include "source/common/http/header_utility.h" #include "source/common/http/headers.h" #include "source/common/http/utility.h" +#include "source/common/protobuf/protobuf.h" namespace Envoy { namespace Extensions { @@ -16,6 +17,8 @@ namespace BasicAuth { namespace { constexpr uint32_t MaximumUriLength = 256; +constexpr absl::string_view DynamicMetadataNamespace = "envoy.filters.http.basic_auth"; +constexpr absl::string_view DynamicMetadataUsernameKey = "username"; // Function to compute SHA1 hash std::string computeSHA1(absl::string_view password) { @@ -31,10 +34,11 @@ std::string computeSHA1(absl::string_view password) { } // namespace FilterConfig::FilterConfig(UserMap&& users, const std::string& forward_username_header, - const std::string& authentication_header, + const std::string& authentication_header, bool allow_missing, const std::string& stats_prefix, Stats::Scope& scope) : users_(std::move(users)), forward_username_header_(forward_username_header), authentication_header_(Http::LowerCaseString(authentication_header)), + allow_missing_(allow_missing), stats_(generateStats(stats_prefix + "basic_auth.", scope)) {} BasicAuthFilter::BasicAuthFilter(FilterConfigConstSharedPtr config) : config_(std::move(config)) {} @@ -55,6 +59,9 @@ Http::FilterHeadersStatus BasicAuthFilter::decodeHeaders(Http::RequestHeaderMap& } if (auth_header.empty()) { + if (config_->allowMissing()) { + return Http::FilterHeadersStatus::Continue; + } return onDenied("User authentication failed. Missing username and password.", "no_credential_for_basic_auth"); } @@ -62,6 +69,9 @@ Http::FilterHeadersStatus BasicAuthFilter::decodeHeaders(Http::RequestHeaderMap& absl::string_view auth_value = auth_header[0]->value().getStringView(); if (!absl::StartsWith(auth_value, "Basic ")) { + if (config_->allowMissing()) { + return Http::FilterHeadersStatus::Continue; + } return onDenied("User authentication failed. Expected 'Basic' authentication scheme.", "invalid_scheme_for_basic_auth"); } @@ -90,6 +100,7 @@ Http::FilterHeadersStatus BasicAuthFilter::decodeHeaders(Http::RequestHeaderMap& headers.setCopy(Http::LowerCaseString(config_->forwardUsernameHeader()), username); } + setDynamicMetadata(username); config_->stats().allowed_.inc(); return Http::FilterHeadersStatus::Continue; } @@ -104,6 +115,14 @@ bool BasicAuthFilter::validateUser(const UserMap& users, absl::string_view usern return computeSHA1(password) == user->second.hash; } +void BasicAuthFilter::setDynamicMetadata(absl::string_view username) { + ProtobufWkt::Struct metadata; + (*metadata.mutable_fields())[std::string(DynamicMetadataUsernameKey)].set_string_value( + std::string(username)); + decoder_callbacks_->streamInfo().setDynamicMetadata(std::string(DynamicMetadataNamespace), + metadata); +} + Http::FilterHeadersStatus BasicAuthFilter::onDenied(absl::string_view body, absl::string_view response_code_details) { config_->stats().denied_.inc(); 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 d257450416dfe..3a1280625acdd 100644 --- a/source/extensions/filters/http/basic_auth/basic_auth_filter.h +++ b/source/extensions/filters/http/basic_auth/basic_auth_filter.h @@ -45,12 +45,13 @@ using UserMap = absl::flat_hash_map; class FilterConfig { public: FilterConfig(UserMap&& users, const std::string& forward_username_header, - const std::string& authentication_header, const std::string& stats_prefix, - Stats::Scope& scope); + const std::string& authentication_header, bool allow_missing, + const std::string& stats_prefix, Stats::Scope& scope); const BasicAuthStats& stats() const { return stats_; } const std::string& forwardUsernameHeader() const { return forward_username_header_; } const UserMap& users() const { return users_; } const Http::LowerCaseString& authenticationHeader() const { return authentication_header_; } + bool allowMissing() const { return allow_missing_; } private: static BasicAuthStats generateStats(const std::string& prefix, Stats::Scope& scope) { @@ -60,6 +61,7 @@ class FilterConfig { const UserMap users_; const std::string forward_username_header_; const Http::LowerCaseString authentication_header_; + const bool allow_missing_; BasicAuthStats stats_; }; using FilterConfigConstSharedPtr = std::shared_ptr; @@ -92,6 +94,7 @@ class BasicAuthFilter : public Http::PassThroughDecoderFilter, private: Http::FilterHeadersStatus onDenied(absl::string_view body, absl::string_view response_code_details); + void setDynamicMetadata(absl::string_view username); // The callback function. FilterConfigConstSharedPtr config_; diff --git a/source/extensions/filters/http/basic_auth/config.cc b/source/extensions/filters/http/basic_auth/config.cc index 9ed9a0d3f87e9..50f24317fab56 100644 --- a/source/extensions/filters/http/basic_auth/config.cc +++ b/source/extensions/filters/http/basic_auth/config.cc @@ -69,7 +69,8 @@ Http::FilterFactoryCb BasicAuthFilterFactory::createFilterFactoryFromProtoTyped( std::string)); FilterConfigConstSharedPtr config = std::make_unique( std::move(users), proto_config.forward_username_header(), - proto_config.authentication_header(), stats_prefix, context.scope()); + proto_config.authentication_header(), proto_config.allow_missing(), stats_prefix, + context.scope()); return [config](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamDecoderFilter(std::make_shared(config)); }; @@ -82,7 +83,8 @@ Http::FilterFactoryCb BasicAuthFilterFactory::createFilterFactoryFromProtoWithSe Config::DataSource::read(proto_config.users(), false, context.api()), std::string)); FilterConfigConstSharedPtr config = std::make_unique( std::move(users), proto_config.forward_username_header(), - proto_config.authentication_header(), stats_prefix, context.scope()); + proto_config.authentication_header(), proto_config.allow_missing(), 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/filter_test.cc b/test/extensions/filters/http/basic_auth/filter_test.cc index aeeb1a9909046..180f2c7ef1b06 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), "x-username", "", "stats", + config_ = std::make_unique(std::move(users), "x-username", "", + /*allow_missing=*/false, "stats", *stats_.rootScope()); filter_ = std::make_shared(config_); filter_->setDecoderFilterCallbacks(decoder_filter_callbacks_); @@ -52,6 +53,26 @@ TEST_F(FilterTest, BasicAuth) { EXPECT_EQ("user2", request_headers_user2.get_("x-username")); } +TEST_F(FilterTest, BasicAuthSetsDynamicMetadataOnSuccess) { + // user1:test1 + Http::TestRequestHeaderMapImpl request_headers{{"Authorization", "Basic dXNlcjE6dGVzdDE="}}; + request_headers.setScheme("http"); + request_headers.setHost("host"); + request_headers.setPath("/"); + + ProtobufWkt::Struct captured_metadata; + EXPECT_CALL(decoder_filter_callbacks_.stream_info_, + setDynamicMetadata("envoy.filters.http.basic_auth", _)) + .WillOnce(Invoke([&](const std::string&, const ProtobufWkt::Struct& metadata) { + captured_metadata = metadata; + })); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true)); + + ASSERT_TRUE(captured_metadata.fields().contains("username")); + EXPECT_EQ("user1", captured_metadata.fields().at("username").string_value()); +} + TEST_F(FilterTest, UserNotExist) { // user3:test2 Http::TestRequestHeaderMapImpl request_headers_user1{{"Authorization", "Basic dXNlcjM6dGVzdDI="}}; @@ -277,7 +298,8 @@ TEST_F(FilterTest, OverrideAuthorizationHeaderProvided) { users.insert({"user1", {"user1", "tESsBmE/yNY3lb6a0L6vVQEZNqw="}}); // user1:test1 FilterConfigConstSharedPtr config = std::make_unique( - std::move(users), "x-username", "x-authorization-override", "stats", *stats_.rootScope()); + std::move(users), "x-username", "x-authorization-override", /*allow_missing=*/false, "stats", + *stats_.rootScope()); std::shared_ptr filter = std::make_shared(config); filter->setDecoderFilterCallbacks(decoder_filter_callbacks_); @@ -292,6 +314,116 @@ TEST_F(FilterTest, OverrideAuthorizationHeaderProvided) { EXPECT_EQ("user1", request_headers_user1.get_("x-username")); } +// Tests for allow_missing=true behavior. + +class AllowMissingFilterTest : public testing::Test { +public: + AllowMissingFilterTest() { + UserMap users; + users.insert({"user1", {"user1", "tESsBmE/yNY3lb6a0L6vVQEZNqw="}}); // user1:test1 + config_ = std::make_unique(std::move(users), "x-username", "", + /*allow_missing=*/true, "stats", + *stats_.rootScope()); + filter_ = std::make_shared(config_); + filter_->setDecoderFilterCallbacks(decoder_filter_callbacks_); + } + + NiceMock stats_; + NiceMock decoder_filter_callbacks_; + FilterConfigConstSharedPtr config_; + std::shared_ptr filter_; +}; + +TEST_F(AllowMissingFilterTest, NoAuthHeaderPassesThrough) { + Http::TestRequestHeaderMapImpl request_headers; + request_headers.setScheme("http"); + request_headers.setHost("host"); + request_headers.setPath("/"); + + // No sendLocalReply should be called. + EXPECT_CALL(decoder_filter_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true)); +} + +TEST_F(AllowMissingFilterTest, BearerTokenPassesThrough) { + // A request with a Bearer token (intended for JWT filter) should pass through BasicAuth + // when allow_missing is true, so JWT filter can handle it downstream. + Http::TestRequestHeaderMapImpl request_headers{{"Authorization", "Bearer some.jwt.token"}}; + request_headers.setScheme("http"); + request_headers.setHost("host"); + request_headers.setPath("/"); + + EXPECT_CALL(decoder_filter_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true)); +} + +TEST_F(AllowMissingFilterTest, ValidBasicCredentialsAuthenticated) { + // Valid Basic credentials should still be validated and succeed. + Http::TestRequestHeaderMapImpl request_headers{{"Authorization", "Basic dXNlcjE6dGVzdDE="}}; + request_headers.setScheme("http"); + request_headers.setHost("host"); + request_headers.setPath("/"); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true)); + EXPECT_EQ("user1", request_headers.get_("x-username")); +} + +TEST_F(AllowMissingFilterTest, InvalidBasicCredentialsRejected) { + // Invalid Basic credentials should still be rejected even when allow_missing is true. + // user1:wrongpassword + Http::TestRequestHeaderMapImpl request_headers{ + {"Authorization", "Basic dXNlcjE6d3JvbmdwYXNzd29yZA=="}}; + request_headers.setScheme("http"); + request_headers.setHost("host"); + request_headers.setPath("/"); + EXPECT_CALL(decoder_filter_callbacks_, requestHeaders()) + .WillOnce(testing::Return(makeOptRef(request_headers))); + + EXPECT_CALL(decoder_filter_callbacks_, sendLocalReply(_, _, _, _, _)) + .WillOnce(Invoke([&](Http::Code code, absl::string_view body, + std::function, + const absl::optional grpc_status, + absl::string_view details) { + EXPECT_EQ(Http::Code::Unauthorized, code); + EXPECT_EQ("User authentication failed. Invalid username/password combination.", body); + EXPECT_EQ(grpc_status, absl::nullopt); + EXPECT_EQ(details, "invalid_credential_for_basic_auth"); + })); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers, true)); +} + +TEST_F(AllowMissingFilterTest, SuccessSetsDynamicMetadata) { + // On successful Basic auth, dynamic metadata should be set regardless of allow_missing. + Http::TestRequestHeaderMapImpl request_headers{{"Authorization", "Basic dXNlcjE6dGVzdDE="}}; + request_headers.setScheme("http"); + request_headers.setHost("host"); + request_headers.setPath("/"); + + ProtobufWkt::Struct captured_metadata; + EXPECT_CALL(decoder_filter_callbacks_.stream_info_, + setDynamicMetadata("envoy.filters.http.basic_auth", _)) + .WillOnce(Invoke([&](const std::string&, const ProtobufWkt::Struct& metadata) { + captured_metadata = metadata; + })); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true)); + + ASSERT_TRUE(captured_metadata.fields().contains("username")); + EXPECT_EQ("user1", captured_metadata.fields().at("username").string_value()); +} + +TEST_F(AllowMissingFilterTest, PassThroughDoesNotSetDynamicMetadata) { + // When request passes through (no Basic creds), no metadata should be set — + // absence of metadata is how a downstream RBAC filter detects that BasicAuth did not succeed. + Http::TestRequestHeaderMapImpl request_headers{{"Authorization", "Bearer some.jwt.token"}}; + request_headers.setScheme("http"); + request_headers.setHost("host"); + request_headers.setPath("/"); + + EXPECT_CALL(decoder_filter_callbacks_.stream_info_, setDynamicMetadata(_, _)).Times(0); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true)); +} + } // namespace BasicAuth } // namespace HttpFilters } // namespace Extensions