From d275e4d594ec0850ab506ab343b8e252f127bce8 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Thu, 2 Apr 2026 20:08:48 +0800 Subject: [PATCH 01/29] per-route oauth2 configuration Signed-off-by: Huabing (Robin) Zhao format Signed-off-by: Huabing (Robin) Zhao update Signed-off-by: Huabing (Robin) Zhao update Signed-off-by: Huabing (Robin) Zhao fix test Signed-off-by: Huabing (Robin) Zhao fix test Signed-off-by: Huabing (Robin) Zhao improve the test Signed-off-by: Huabing (Robin) Zhao fix test Signed-off-by: Huabing (Robin) Zhao fix test Signed-off-by: Huabing (Robin) Zhao --- .../filters/http/oauth2/v3/oauth.proto | 12 ++- .../http/http_filters/oauth2_filter.rst | 38 ++++++++++ .../extensions/filters/http/oauth2/config.cc | 73 +++++++++++++------ .../extensions/filters/http/oauth2/config.h | 8 +- .../extensions/filters/http/oauth2/filter.cc | 68 +++++++++++++++-- .../extensions/filters/http/oauth2/filter.h | 21 ++++++ .../filters/http/oauth2/config_test.cc | 48 +++++++++++- .../filters/http/oauth2/filter_test.cc | 44 +++++++++++ 8 files changed, 276 insertions(+), 36 deletions(-) diff --git a/api/envoy/extensions/filters/http/oauth2/v3/oauth.proto b/api/envoy/extensions/filters/http/oauth2/v3/oauth.proto index 321282d0cfce4..1baea180639e0 100644 --- a/api/envoy/extensions/filters/http/oauth2/v3/oauth.proto +++ b/api/envoy/extensions/filters/http/oauth2/v3/oauth.proto @@ -294,8 +294,18 @@ message OAuth2Config { bool disable_token_encryption = 26; } +// Per-route OAuth2 config. +// +// This message supplies an OAuth2Config for the matched route. +// It overrides the filter-level config for requests matching the route. +// If neither the global config nor a per-route config is specified, OAuth2 is disabled for the route. +message OAuth2PerRoute { + // Full OAuth2 config for this route. + OAuth2Config config = 1 [(validate.rules).message = {required: true}]; +} + // Filter config. message OAuth2 { - // Leave this empty to disable OAuth2 for a specific route, using per filter config. + // Global/default OAuth2 config for this filter. OAuth2Config config = 1; } diff --git a/docs/root/configuration/http/http_filters/oauth2_filter.rst b/docs/root/configuration/http/http_filters/oauth2_filter.rst index 8eec4f9452c12..22d43ca9e1d12 100644 --- a/docs/root/configuration/http/http_filters/oauth2_filter.rst +++ b/docs/root/configuration/http/http_filters/oauth2_filter.rst @@ -7,6 +7,13 @@ OAuth2 * This filter should be configured with the type URL ``type.googleapis.com/envoy.extensions.filters.http.oauth2.v3.OAuth2``. * :ref:`v3 API reference ` +The OAuth2 filter also defines a route-level config message, +:ref:`OAuth2PerRoute `, +which may be attached to +:ref:`Route.typed_per_filter_config `. +The route-level config carries a complete :ref:`OAuth2Config ` +and is intended to replace, not merge with, the filter-level config. + The OAuth filter's flow involves: * An unauthenticated user arrives at myapp.com, and the oauth filter redirects them to the @@ -229,6 +236,37 @@ can be defined in one shared file. generic_secret: secret: +The following example shows how the route-level config message is attached: + +.. code-block:: yaml + + typed_per_filter_config: + envoy.filters.http.oauth2: + "@type": type.googleapis.com/envoy.extensions.filters.http.oauth2.v3.OAuth2PerRoute + config: + token_endpoint: + cluster: oauth + uri: oauth.com/token + timeout: 3s + authorization_endpoint: https://oauth.com/oauth/authorize/ + redirect_uri: "%REQ(x-forwarded-proto)%://%REQ(:authority)%/callback" + redirect_path_matcher: + path: + exact: /callback + signout_path: + path: + exact: /signout + credentials: + client_id: foo + token_secret: + name: token + sds_config: + path: "/etc/envoy/token-secret.yaml" + hmac_secret: + name: hmac + sds_config: + path: "/etc/envoy/hmac.yaml" + Notes ----- diff --git a/source/extensions/filters/http/oauth2/config.cc b/source/extensions/filters/http/oauth2/config.cc index 856c198085bc2..d5970deda4698 100644 --- a/source/extensions/filters/http/oauth2/config.cc +++ b/source/extensions/filters/http/oauth2/config.cc @@ -35,25 +35,15 @@ secretsProvider(const envoy::extensions::transport_sockets::tls::v3::SdsSecretCo return server_context.secretManager().findStaticGenericSecretProvider(config.name()); } } -} // namespace -absl::StatusOr OAuth2Config::createFilterFactoryFromProtoTyped( - const envoy::extensions::filters::http::oauth2::v3::OAuth2& proto, - const std::string& stats_prefix, Server::Configuration::FactoryContext& context) { - if (!proto.has_config()) { - return absl::InvalidArgumentError("config must be present for global config"); - } - - const auto& proto_config = proto.config(); +absl::StatusOr +createFilterConfig(const envoy::extensions::filters::http::oauth2::v3::OAuth2Config& proto_config, + Server::Configuration::ServerFactoryContext& server_context, + Init::Manager& init_manager, Stats::Scope& scope, + const std::string& stats_prefix) { const auto& credentials = proto_config.credentials(); - - const auto& client_secret = credentials.token_secret(); - const auto& hmac_secret = credentials.hmac_secret(); const auto auth_type = proto_config.auth_type(); - auto& server_context = context.serverFactoryContext(); - auto& cluster_manager = context.serverFactoryContext().clusterManager(); - // token_secret is required unless auth_type is TLS_CLIENT_AUTH if (auth_type != envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType_TLS_CLIENT_AUTH) { @@ -68,18 +58,20 @@ absl::StatusOr OAuth2Config::createFilterFactoryFromProto ENVOY_LOG_MISC(debug, "OAuth2 filter: token_secret is ignored when auth_type is TLS_CLIENT_AUTH"); } + Secret::GenericSecretConfigProviderSharedPtr secret_provider_client_secret = nullptr; if (credentials.has_token_secret() && auth_type != envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType_TLS_CLIENT_AUTH) { secret_provider_client_secret = - secretsProvider(client_secret, server_context, context.initManager()); + secretsProvider(credentials.token_secret(), server_context, init_manager); if (secret_provider_client_secret == nullptr) { return absl::InvalidArgumentError("invalid token secret configuration"); } } + auto secret_provider_hmac_secret = - secretsProvider(hmac_secret, server_context, context.initManager()); + secretsProvider(credentials.hmac_secret(), server_context, init_manager); if (secret_provider_hmac_secret == nullptr) { return absl::InvalidArgumentError("invalid HMAC secret configuration"); } @@ -93,21 +85,54 @@ absl::StatusOr OAuth2Config::createFilterFactoryFromProto auto secret_reader = std::make_shared( std::move(secret_provider_client_secret), std::move(secret_provider_hmac_secret), - context.serverFactoryContext().threadLocal(), context.serverFactoryContext().api()); - auto config = std::make_shared(proto_config, context.serverFactoryContext(), - secret_reader, context.scope(), stats_prefix); + server_context.threadLocal(), server_context.api()); + return std::make_shared(proto_config, server_context, secret_reader, scope, + stats_prefix); +} +} // namespace + +absl::StatusOr OAuth2Config::createFilterFactoryFromProtoTyped( + const envoy::extensions::filters::http::oauth2::v3::OAuth2& proto, + const std::string& stats_prefix, Server::Configuration::FactoryContext& context) { + auto& server_context = context.serverFactoryContext(); + auto& cluster_manager = context.serverFactoryContext().clusterManager(); + FilterConfigSharedPtr config = nullptr; + if (proto.has_config()) { + auto config_or_error = createFilterConfig(proto.config(), server_context, context.initManager(), + context.scope(), stats_prefix); + if (!config_or_error.ok()) { + return config_or_error.status(); + } + config = config_or_error.value(); + } return [&context, config, &cluster_manager](Http::FilterChainFactoryCallbacks& callbacks) -> void { - std::unique_ptr oauth_client = - std::make_unique(cluster_manager, config->oauthTokenEndpoint(), - config->retryPolicy(), config->defaultExpiresIn()); callbacks.addStreamFilter(std::make_shared( - config, std::move(oauth_client), context.serverFactoryContext().timeSource(), + config, + [&cluster_manager]( + const FilterConfigSharedPtr& active_config) -> std::unique_ptr { + return std::make_unique( + cluster_manager, active_config->oauthTokenEndpoint(), + active_config->retryPolicy(), active_config->defaultExpiresIn()); + }, + context.serverFactoryContext().timeSource(), context.serverFactoryContext().api().randomGenerator())); }; } +absl::StatusOr +OAuth2Config::createRouteSpecificFilterConfigTyped( + const envoy::extensions::filters::http::oauth2::v3::OAuth2PerRoute& proto, + Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor&) { + auto config_or_error = + createFilterConfig(proto.config(), context, context.initManager(), context.scope(), ""); + if (!config_or_error.ok()) { + return config_or_error.status(); + } + return std::make_shared(config_or_error.value()); +} + /* * Static registration for the OAuth2 filter. @see RegisterFactory. */ diff --git a/source/extensions/filters/http/oauth2/config.h b/source/extensions/filters/http/oauth2/config.h index c33e6c651cbb9..5efa8c8a81a8a 100644 --- a/source/extensions/filters/http/oauth2/config.h +++ b/source/extensions/filters/http/oauth2/config.h @@ -13,7 +13,8 @@ namespace HttpFilters { namespace Oauth2 { class OAuth2Config : public Extensions::HttpFilters::Common::ExceptionFreeFactoryBase< - envoy::extensions::filters::http::oauth2::v3::OAuth2> { + envoy::extensions::filters::http::oauth2::v3::OAuth2, + envoy::extensions::filters::http::oauth2::v3::OAuth2PerRoute> { public: OAuth2Config() : ExceptionFreeFactoryBase("envoy.filters.http.oauth2") {} @@ -21,6 +22,11 @@ class OAuth2Config : public Extensions::HttpFilters::Common::ExceptionFreeFactor createFilterFactoryFromProtoTyped(const envoy::extensions::filters::http::oauth2::v3::OAuth2&, const std::string&, Server::Configuration::FactoryContext&) override; + + absl::StatusOr + createRouteSpecificFilterConfigTyped( + const envoy::extensions::filters::http::oauth2::v3::OAuth2PerRoute&, + Server::Configuration::ServerFactoryContext&, ProtobufMessage::ValidationVisitor&) override; }; } // namespace Oauth2 diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 737287a94f6ee..c4143433aa137 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -607,20 +607,68 @@ bool OAuth2CookieValidator::timestampIsValid() const { bool OAuth2CookieValidator::isValid() const { return hmacIsValid() && timestampIsValid(); } -OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr config, - std::unique_ptr&& oauth_client, TimeSource& time_source, +OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr default_config, + OAuth2ClientFactory oauth_client_factory, TimeSource& time_source, Random::RandomGenerator& random) - : validator_(std::make_shared(time_source, config->cookieNames(), - config->cookieDomain())), - oauth_client_(std::move(oauth_client)), config_(std::move(config)), time_source_(time_source), + : default_config_(std::move(default_config)), + oauth_client_factory_(std::move(oauth_client_factory)), time_source_(time_source), random_(random) { + if (default_config_ != nullptr) { + setActiveConfig(default_config_); + } +} - oauth_client_->setCallbacks(*this); +OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr config, + std::unique_ptr&& oauth_client, TimeSource& time_source, + Random::RandomGenerator& random) + : oauth_client_(std::move(oauth_client)), default_config_(std::move(config)), + time_source_(time_source), random_(random) { + setActiveConfig(default_config_); } void OAuth2Filter::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) { PassThroughDecoderFilter::setDecoderFilterCallbacks(callbacks); - oauth_client_->setDecoderFilterCallbacks(callbacks); + if (oauth_client_ != nullptr) { + oauth_client_->setDecoderFilterCallbacks(callbacks); + } +} + +FilterConfigSharedPtr OAuth2Filter::getConfigForRequest() const { + const auto* route_specific_config = + Http::Utility::resolveMostSpecificPerFilterConfig(decoder_callbacks_); + if (route_specific_config != nullptr) { + return route_specific_config->config(); + } + + return default_config_; +} + +void OAuth2Filter::setActiveConfig(FilterConfigSharedPtr config) { + if (config == nullptr) { + config_.reset(); + validator_.reset(); + oauth_client_.reset(); + return; + } + + if (config_ == config && validator_ != nullptr && + (oauth_client_ != nullptr || !oauth_client_factory_)) { + return; + } + + config_ = std::move(config); + validator_ = std::make_shared(time_source_, config_->cookieNames(), + config_->cookieDomain()); + + if (oauth_client_factory_) { + oauth_client_ = oauth_client_factory_(config_); + } + if (oauth_client_ != nullptr) { + oauth_client_->setCallbacks(*this); + if (decoder_callbacks_ != nullptr) { + oauth_client_->setDecoderFilterCallbacks(*decoder_callbacks_); + } + } } /** @@ -632,6 +680,12 @@ void OAuth2Filter::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& * 5) user is unauthorized */ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& headers, bool) { + setActiveConfig(getConfigForRequest()); + // If no config is set, OAuth2 is disabled for this request. + if (config_ == nullptr) { + return Http::FilterHeadersStatus::Continue; + } + // Skip Filter and continue chain if a Passthrough header is matching. // Only increment counters here; do not modify request headers, as there may be // other instances of this filter configured that still need to process the request. diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index 4818c15445ae2..0b4fdc1967ad3 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -254,6 +255,16 @@ class FilterConfig : public Logger::Loggable { using FilterConfigSharedPtr = std::shared_ptr; +class FilterConfigPerRoute : public Router::RouteSpecificFilterConfig { +public: + explicit FilterConfigPerRoute(FilterConfigSharedPtr config) : config_(std::move(config)) {} + + const FilterConfigSharedPtr& config() const { return config_; } + +private: + FilterConfigSharedPtr config_; +}; + /** * An OAuth cookie validator: * 1. extracts cookies from a request @@ -320,6 +331,11 @@ class OAuth2Filter : public Http::PassThroughFilter, FilterCallbacks, Logger::Loggable { public: + using OAuth2ClientFactory = + std::function(const FilterConfigSharedPtr&)>; + + OAuth2Filter(FilterConfigSharedPtr default_config, OAuth2ClientFactory oauth_client_factory, + TimeSource& time_source, Random::RandomGenerator& random); OAuth2Filter(FilterConfigSharedPtr config, std::unique_ptr&& oauth_client, TimeSource& time_source, Random::RandomGenerator& random); @@ -369,10 +385,15 @@ class OAuth2Filter : public Http::PassThroughFilter, bool was_refresh_token_flow_{false}; std::unique_ptr oauth_client_; + FilterConfigSharedPtr default_config_; FilterConfigSharedPtr config_; + OAuth2ClientFactory oauth_client_factory_; TimeSource& time_source_; Random::RandomGenerator& random_; + FilterConfigSharedPtr getConfigForRequest() const; + void setActiveConfig(FilterConfigSharedPtr config); + // Determines whether or not the current request can skip the entire OAuth flow (HMAC is valid, // connection is mTLS, etc.) bool canSkipOAuth(Http::RequestHeaderMap& headers) const; diff --git a/test/extensions/filters/http/oauth2/config_test.cc b/test/extensions/filters/http/oauth2/config_test.cc index 99c4b3218bddd..fec68435b0424 100644 --- a/test/extensions/filters/http/oauth2/config_test.cc +++ b/test/extensions/filters/http/oauth2/config_test.cc @@ -140,7 +140,7 @@ TEST(ConfigTest, CreateFilter) { EXPECT_CALL(context.server_factory_context_, clusterManager()).Times(2); EXPECT_CALL(context, scope()); EXPECT_CALL(context.server_factory_context_, timeSource()); - EXPECT_CALL(context, initManager()).Times(2); + EXPECT_CALL(context, initManager()); Http::FilterFactoryCb cb = factory.createFilterFactoryFromProto(*proto_config, "stats", context).value(); Http::MockFilterChainFactoryCallbacks filter_callback; @@ -335,8 +335,50 @@ TEST(ConfigTest, CreateFilterMissingConfig) { NiceMock factory_context; const auto result = config.createFilterFactoryFromProtoTyped(proto_config, "whatever", factory_context); - EXPECT_FALSE(result.ok()); - EXPECT_EQ(result.status().message(), "config must be present for global config"); + // Empty config is valid, config can be provided at route level. + EXPECT_TRUE(result.ok()); +} + +TEST(ConfigTest, CreateRouteSpecificConfig) { + const std::string yaml = R"EOF( +config: + token_endpoint: + cluster: foo + uri: oauth.com/token + timeout: 3s + credentials: + client_id: "secret" + token_secret: + name: token + hmac_secret: + name: hmac + authorization_endpoint: https://oauth.com/oauth/authorize/ + redirect_uri: "%REQ(x-forwarded-proto)%://%REQ(:authority)%/callback" + redirect_path_matcher: + path: + exact: /callback + signout_path: + path: + exact: /signout + )EOF"; + + envoy::extensions::filters::http::oauth2::v3::OAuth2PerRoute route_config; + TestUtility::loadFromYaml(yaml, route_config); + + OAuth2Config factory; + NiceMock context; + context.cluster_manager_.initializeClusters({"foo"}, {}); + + NiceMock secret_manager; + ON_CALL(context, secretManager()).WillByDefault(ReturnRef(secret_manager)); + ON_CALL(secret_manager, findStaticGenericSecretProvider(_)) + .WillByDefault(Return(std::make_shared( + envoy::extensions::transport_sockets::tls::v3::GenericSecret()))); + + auto& validation_visitor = ProtobufMessage::getNullValidationVisitor(); + const auto result = + factory.createRouteSpecificFilterConfigTyped(route_config, context, validation_visitor); + EXPECT_TRUE(result.ok()); } TEST(ConfigTest, WrongCookieName) { diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 57d8e38a7f4c0..107441391e7dd 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -15,6 +15,7 @@ #include "source/extensions/filters/http/oauth2/filter.h" #include "test/mocks/http/mocks.h" +#include "test/mocks/router/mocks.h" #include "test/mocks/server/mocks.h" #include "test/mocks/upstream/mocks.h" #include "test/test_common/test_runtime.h" @@ -4551,6 +4552,49 @@ TEST_F(OAuth2Test, RequestIsUnchangedWhenPassThroughMatcherMatches) { EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_success").value(), 0); } +TEST_F(OAuth2Test, RouteSpecificConfigOverridesGlobalConfig) { + envoy::extensions::filters::http::oauth2::v3::OAuth2Config route_proto; + auto* endpoint = route_proto.mutable_token_endpoint(); + endpoint->set_cluster("auth.example.com"); + endpoint->set_uri("auth.example.com/_oauth"); + endpoint->mutable_timeout()->set_seconds(1); + route_proto.set_redirect_uri("%REQ(:scheme)%://%REQ(:authority)%" + TEST_CALLBACK); + route_proto.mutable_redirect_path_matcher()->mutable_path()->set_exact(TEST_CALLBACK); + route_proto.set_authorization_endpoint("https://auth.example.com/oauth/authorize/"); + route_proto.mutable_signout_path()->mutable_path()->set_exact("/_signout"); + auto* route_matcher = route_proto.add_pass_through_matcher(); + route_matcher->set_name(":method"); + route_matcher->mutable_string_match()->set_exact(Http::Headers::get().MethodValues.Get); + auto* credentials = route_proto.mutable_credentials(); + credentials->set_client_id(TEST_CLIENT_ID); + credentials->mutable_token_secret()->set_name("secret"); + credentials->mutable_hmac_secret()->set_name("hmac"); + + auto secret_reader = std::make_shared(); + auto route_config = std::make_shared(std::make_shared( + route_proto, factory_context_.server_factory_context_, secret_reader, scope_, "test.")); + + ON_CALL(decoder_callbacks_, mostSpecificPerFilterConfig()) + .WillByDefault(Return(route_config.get())); + + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Path.get(), "/resource"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + }; + + // The global config would process this GET request, so these expectations verify that the + // route-specific pass-through matcher took precedence and short-circuited the OAuth flow. + EXPECT_CALL(*validator_, setParams(_, _)).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).Times(0); + EXPECT_CALL(*oauth_client_, asyncGetAccessToken(_, _, _, _, _, _)).Times(0); + EXPECT_CALL(*oauth_client_, asyncRefreshAccessToken(_, _, _, _)).Times(0); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + EXPECT_EQ(scope_.counterFromString("test.oauth_passthrough").value(), 1); +} + // Verify cookie prefixes "__Secure-" and "__Host-" cause addition of the "Secure" attribute at // signout. TEST_F(OAuth2Test, SecureAttributeAddedForSecureCookiePrefixesOnSignout) { From 3bfa54c03e1e393470a51a4860eecd1dc388a6df Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 3 Apr 2026 13:23:59 +0800 Subject: [PATCH 02/29] remove the old constructor Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 8 -------- source/extensions/filters/http/oauth2/filter.h | 2 -- .../filters/http/oauth2/filter_test.cc | 16 +++++++++++++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index c4143433aa137..12e377231070a 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -618,14 +618,6 @@ OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr default_config, } } -OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr config, - std::unique_ptr&& oauth_client, TimeSource& time_source, - Random::RandomGenerator& random) - : oauth_client_(std::move(oauth_client)), default_config_(std::move(config)), - time_source_(time_source), random_(random) { - setActiveConfig(default_config_); -} - void OAuth2Filter::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) { PassThroughDecoderFilter::setDecoderFilterCallbacks(callbacks); if (oauth_client_ != nullptr) { diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index 0b4fdc1967ad3..595462b431131 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -336,8 +336,6 @@ class OAuth2Filter : public Http::PassThroughFilter, OAuth2Filter(FilterConfigSharedPtr default_config, OAuth2ClientFactory oauth_client_factory, TimeSource& time_source, Random::RandomGenerator& random); - OAuth2Filter(FilterConfigSharedPtr config, std::unique_ptr&& oauth_client, - TimeSource& time_source, Random::RandomGenerator& random); // Http::PassThroughFilter Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool) override; diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 107441391e7dd..c325d33430956 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -122,12 +122,22 @@ class OAuth2Test : public testing::TestWithParam { void init(FilterConfigSharedPtr config) { // Set up the OAuth client. oauth_client_ = new MockOAuth2Client(); - std::unique_ptr oauth_client_ptr{oauth_client_}; + auto oauth_client_holder = + std::make_shared>(std::unique_ptr{oauth_client_}); config_ = config; ON_CALL(test_random_, random()).WillByDefault(Return(123456789)); - filter_ = std::make_shared(config_, std::move(oauth_client_ptr), test_time_, - test_random_); + filter_ = std::make_shared( + config_, [this, oauth_client_holder](const FilterConfigSharedPtr&) + -> std::unique_ptr { + if (*oauth_client_holder != nullptr) { + return std::move(*oauth_client_holder); + } + + oauth_client_ = new MockOAuth2Client(); + return std::unique_ptr{oauth_client_}; + }, + test_time_, test_random_); filter_->setDecoderFilterCallbacks(decoder_callbacks_); filter_->setEncoderFilterCallbacks(encoder_callbacks_); validator_ = std::make_shared(); From 38df08f8df4ca288246f248790ae85bc59172571 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 3 Apr 2026 07:46:02 +0000 Subject: [PATCH 03/29] format Signed-off-by: Huabing (Robin) Zhao --- test/extensions/filters/http/oauth2/filter_test.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index c325d33430956..4e215775f9d82 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -122,14 +122,14 @@ class OAuth2Test : public testing::TestWithParam { void init(FilterConfigSharedPtr config) { // Set up the OAuth client. oauth_client_ = new MockOAuth2Client(); - auto oauth_client_holder = - std::make_shared>(std::unique_ptr{oauth_client_}); + auto oauth_client_holder = std::make_shared>( + std::unique_ptr{oauth_client_}); config_ = config; ON_CALL(test_random_, random()).WillByDefault(Return(123456789)); filter_ = std::make_shared( - config_, [this, oauth_client_holder](const FilterConfigSharedPtr&) - -> std::unique_ptr { + config_, + [this, oauth_client_holder](const FilterConfigSharedPtr&) -> std::unique_ptr { if (*oauth_client_holder != nullptr) { return std::move(*oauth_client_holder); } From c6f7bed12c53ced76425071aa073943a3f2ffa75 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 3 Apr 2026 15:56:41 +0800 Subject: [PATCH 04/29] add change log Signed-off-by: Huabing (Robin) Zhao --- changelogs/current.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 1674d768ddbf3..c33e75cd5ceae 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -97,6 +97,10 @@ minor_behavior_changes: Now all the dynamic module extension factories (HTTP, network, listener, UDP listener, and so on) will serialize the ``google.protobuf.Struct`` configuration message to JSON string and pass it to the dynamic module side as the configuration. +- area: oauth2 + change: | + Added :ref:`per-route configuration support ` + to the OAuth2 HTTP filter. bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* From 0647aaab4ff34ac5d1e0d29571c2db34e2d9ef78 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 3 Apr 2026 16:26:25 +0800 Subject: [PATCH 05/29] add missing registry metadata Signed-off-by: Huabing (Robin) Zhao --- source/extensions/extensions_metadata.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/source/extensions/extensions_metadata.yaml b/source/extensions/extensions_metadata.yaml index 8e177ce5bfe33..35c08037a4728 100644 --- a/source/extensions/extensions_metadata.yaml +++ b/source/extensions/extensions_metadata.yaml @@ -649,6 +649,7 @@ envoy.filters.http.oauth2: status: stable type_urls: - envoy.extensions.filters.http.oauth2.v3.OAuth2 + - envoy.extensions.filters.http.oauth2.v3.OAuth2PerRoute envoy.filters.http.on_demand: categories: - envoy.filters.http From c75b379f86936b1cdf70b6ca3359d5a420aa39de Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 3 Apr 2026 19:04:29 +0800 Subject: [PATCH 06/29] update oauth2 docs Signed-off-by: Huabing (Robin) Zhao --- .../http/http_filters/oauth2_filter.rst | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/docs/root/configuration/http/http_filters/oauth2_filter.rst b/docs/root/configuration/http/http_filters/oauth2_filter.rst index 22d43ca9e1d12..ec3d4e3bba227 100644 --- a/docs/root/configuration/http/http_filters/oauth2_filter.rst +++ b/docs/root/configuration/http/http_filters/oauth2_filter.rst @@ -14,6 +14,12 @@ which may be attached to The route-level config carries a complete :ref:`OAuth2Config ` and is intended to replace, not merge with, the filter-level config. +When using per-route configuration, keep the +:ref:`redirect_path_matcher ` +and :ref:`signout_path ` +within the same route prefix as the protected resources. Cookie paths should cover that same prefix +so that the callback and signout requests receive the OAuth cookies needed to complete the flow. + The OAuth filter's flow involves: * An unauthenticated user arrives at myapp.com, and the oauth filter redirects them to the @@ -267,6 +273,121 @@ The following example shows how the route-level config message is attached: sds_config: path: "/etc/envoy/hmac.yaml" +The following example shows two independent route prefixes, ``/foo`` and ``/bar``, each with its +own OAuth2 client settings. The callback path, signout path, and cookie paths are all scoped under +the same prefix as the protected route: + +.. code-block:: yaml + + http_filters: + - name: envoy.filters.http.oauth2 + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.oauth2.v3.OAuth2 + - name: envoy.filters.http.router + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + route_config: + virtual_hosts: + - name: service + domains: ["*"] + routes: + - match: + prefix: "/foo" + route: + cluster: local_service + typed_per_filter_config: + envoy.filters.http.oauth2: + "@type": type.googleapis.com/envoy.extensions.filters.http.oauth2.v3.OAuth2PerRoute + config: + token_endpoint: + cluster: google_oauth2 + uri: https://oauth2.googleapis.com/token + timeout: 3s + authorization_endpoint: https://accounts.google.com/o/oauth2/v2/auth + redirect_uri: "%REQ(x-forwarded-proto)%://%REQ(:authority)%/foo/callback" + redirect_path_matcher: + path: + exact: /foo/callback + signout_path: + path: + exact: /foo/signout + cookie_configs: + bearer_token_cookie_config: + path: "/foo" + oauth_hmac_cookie_config: + path: "/foo" + oauth_expires_cookie_config: + path: "/foo" + id_token_cookie_config: + path: "/foo" + refresh_token_cookie_config: + path: "/foo" + oauth_nonce_cookie_config: + path: "/foo" + code_verifier_cookie_config: + path: "/foo" + credentials: + client_id: foo + token_secret: + name: foo_client_secret + sds_config: + path: "/etc/foo-client-secret.yaml" + hmac_secret: + name: hmac + sds_config: + path: "/etc/hmac-secret.yaml" + auth_scopes: + - openid + - email + - match: + prefix: "/bar" + route: + cluster: local_service + typed_per_filter_config: + envoy.filters.http.oauth2: + "@type": type.googleapis.com/envoy.extensions.filters.http.oauth2.v3.OAuth2PerRoute + config: + token_endpoint: + cluster: google_oauth2 + uri: https://oauth2.googleapis.com/token + timeout: 3s + authorization_endpoint: https://accounts.google.com/o/oauth2/v2/auth + redirect_uri: "%REQ(x-forwarded-proto)%://%REQ(:authority)%/bar/callback" + redirect_path_matcher: + path: + exact: /bar/callback + signout_path: + path: + exact: /bar/signout + cookie_configs: + bearer_token_cookie_config: + path: "/bar" + oauth_hmac_cookie_config: + path: "/bar" + oauth_expires_cookie_config: + path: "/bar" + id_token_cookie_config: + path: "/bar" + refresh_token_cookie_config: + path: "/bar" + oauth_nonce_cookie_config: + path: "/bar" + code_verifier_cookie_config: + path: "/bar" + credentials: + client_id: bar + token_secret: + name: bar_client_secret + sds_config: + path: "/etc/bar-client-secret.yaml" + hmac_secret: + name: hmac + sds_config: + path: "/etc/hmac-secret.yaml" + auth_scopes: + - openid + - email + Notes ----- From 547e4910bfbb35a2e3f56badd5e4b9f94daafba8 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 3 Apr 2026 19:09:53 +0800 Subject: [PATCH 07/29] update oauth2 docs Signed-off-by: Huabing (Robin) Zhao --- .../http/http_filters/oauth2_filter.rst | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/docs/root/configuration/http/http_filters/oauth2_filter.rst b/docs/root/configuration/http/http_filters/oauth2_filter.rst index ec3d4e3bba227..bc192420ffe75 100644 --- a/docs/root/configuration/http/http_filters/oauth2_filter.rst +++ b/docs/root/configuration/http/http_filters/oauth2_filter.rst @@ -242,37 +242,6 @@ can be defined in one shared file. generic_secret: secret: -The following example shows how the route-level config message is attached: - -.. code-block:: yaml - - typed_per_filter_config: - envoy.filters.http.oauth2: - "@type": type.googleapis.com/envoy.extensions.filters.http.oauth2.v3.OAuth2PerRoute - config: - token_endpoint: - cluster: oauth - uri: oauth.com/token - timeout: 3s - authorization_endpoint: https://oauth.com/oauth/authorize/ - redirect_uri: "%REQ(x-forwarded-proto)%://%REQ(:authority)%/callback" - redirect_path_matcher: - path: - exact: /callback - signout_path: - path: - exact: /signout - credentials: - client_id: foo - token_secret: - name: token - sds_config: - path: "/etc/envoy/token-secret.yaml" - hmac_secret: - name: hmac - sds_config: - path: "/etc/envoy/hmac.yaml" - The following example shows two independent route prefixes, ``/foo`` and ``/bar``, each with its own OAuth2 client settings. The callback path, signout path, and cookie paths are all scoped under the same prefix as the protected route: From 4ac7d32601a60b40bd2ed76a45a2320fe7c1ab91 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 3 Apr 2026 22:07:44 +0800 Subject: [PATCH 08/29] update oauth2 docs Signed-off-by: Huabing (Robin) Zhao --- .../http/http_filters/oauth2_filter.rst | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/docs/root/configuration/http/http_filters/oauth2_filter.rst b/docs/root/configuration/http/http_filters/oauth2_filter.rst index bc192420ffe75..571ba7b2bb2ef 100644 --- a/docs/root/configuration/http/http_filters/oauth2_filter.rst +++ b/docs/root/configuration/http/http_filters/oauth2_filter.rst @@ -17,8 +17,11 @@ and is intended to replace, not merge with, the filter-level config. When using per-route configuration, keep the :ref:`redirect_path_matcher ` and :ref:`signout_path ` -within the same route prefix as the protected resources. Cookie paths should cover that same prefix -so that the callback and signout requests receive the OAuth cookies needed to complete the flow. +within the same route prefix as the protected resources. If cookie paths are customized, they +should cover that same prefix so that the callback and signout requests receive the OAuth cookies +needed to complete the flow. When multiple per-route OAuth2 configurations share the same host, +customizing :ref:`cookie_names ` +is recommended to avoid overlap between routes. The OAuth filter's flow involves: @@ -243,8 +246,9 @@ can be defined in one shared file. secret: The following example shows two independent route prefixes, ``/foo`` and ``/bar``, each with its -own OAuth2 client settings. The callback path, signout path, and cookie paths are all scoped under -the same prefix as the protected route: +own OAuth2 client settings. The callback path and signout path stay under the same prefix as the +protected route, the cookie paths are scoped to that prefix, and the cookie names are customized so +that the two configurations remain independent on the same host: .. code-block:: yaml @@ -297,6 +301,14 @@ the same prefix as the protected route: path: "/foo" credentials: client_id: foo + cookie_names: + bearer_token: FooBearerToken + oauth_hmac: FooOauthHMAC + oauth_expires: FooOauthExpires + id_token: FooIdToken + refresh_token: FooRefreshToken + oauth_nonce: FooOauthNonce + code_verifier: FooCodeVerifier token_secret: name: foo_client_secret sds_config: @@ -345,6 +357,14 @@ the same prefix as the protected route: path: "/bar" credentials: client_id: bar + cookie_names: + bearer_token: BarBearerToken + oauth_hmac: BarOauthHMAC + oauth_expires: BarOauthExpires + id_token: BarIdToken + refresh_token: BarRefreshToken + oauth_nonce: BarOauthNonce + code_verifier: BarCodeVerifier token_secret: name: bar_client_secret sds_config: From 44dbd9b458233758becd0a02e2ac4ce52df9122f Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 3 Apr 2026 22:30:11 +0800 Subject: [PATCH 09/29] add coverage Signed-off-by: Huabing (Robin) Zhao --- .../filters/http/oauth2/filter_test.cc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 4e215775f9d82..deba0e9081a53 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -4605,6 +4605,25 @@ TEST_F(OAuth2Test, RouteSpecificConfigOverridesGlobalConfig) { EXPECT_EQ(scope_.counterFromString("test.oauth_passthrough").value(), 1); } +TEST_F(OAuth2Test, EmptyRouteSpecificConfigOverridesGlobalConfig) { + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Path.get(), "/resource"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + }; + + ASSERT_NE(filter_->config_, nullptr); + ASSERT_NE(filter_->validator_, nullptr); + ASSERT_NE(filter_->oauth_client_, nullptr); + + ON_CALL(decoder_callbacks_, mostSpecificPerFilterConfig()).WillByDefault(Return(nullptr)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + EXPECT_EQ(filter_->config_, nullptr); + EXPECT_EQ(filter_->validator_, nullptr); + EXPECT_EQ(filter_->oauth_client_, nullptr); +} + // Verify cookie prefixes "__Secure-" and "__Host-" cause addition of the "Secure" attribute at // signout. TEST_F(OAuth2Test, SecureAttributeAddedForSecureCookiePrefixesOnSignout) { From 6674ed7944db6e1fbc6f3b836b4aab5297359144 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Sat, 4 Apr 2026 06:58:47 +0800 Subject: [PATCH 10/29] fix test Signed-off-by: Huabing (Robin) Zhao --- test/extensions/filters/http/oauth2/filter_test.cc | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index deba0e9081a53..fc99e833190b9 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -4613,15 +4613,11 @@ TEST_F(OAuth2Test, EmptyRouteSpecificConfigOverridesGlobalConfig) { {Http::Headers::get().Scheme.get(), "https"}, }; - ASSERT_NE(filter_->config_, nullptr); - ASSERT_NE(filter_->validator_, nullptr); - ASSERT_NE(filter_->oauth_client_, nullptr); - ON_CALL(decoder_callbacks_, mostSpecificPerFilterConfig()).WillByDefault(Return(nullptr)); + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).Times(0); + EXPECT_CALL(*oauth_client_, asyncGetAccessToken(_, _, _, _, _, _)).Times(0); + EXPECT_CALL(*oauth_client_, asyncRefreshAccessToken(_, _, _, _)).Times(0); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - EXPECT_EQ(filter_->config_, nullptr); - EXPECT_EQ(filter_->validator_, nullptr); - EXPECT_EQ(filter_->oauth_client_, nullptr); } // Verify cookie prefixes "__Secure-" and "__Host-" cause addition of the "Secure" attribute at From 4d6fd1282a4aff99db478afd0450addc3cd64f7d Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Sun, 5 Apr 2026 14:06:53 +0000 Subject: [PATCH 11/29] fix test Signed-off-by: Huabing (Robin) Zhao --- test/extensions/filters/http/oauth2/filter_test.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index fc99e833190b9..72a5393ba0462 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -141,7 +141,9 @@ class OAuth2Test : public testing::TestWithParam { filter_->setDecoderFilterCallbacks(decoder_callbacks_); filter_->setEncoderFilterCallbacks(encoder_callbacks_); validator_ = std::make_shared(); - filter_->validator_ = validator_; + if (config_ != nullptr) { + filter_->validator_ = validator_; + } filter_->flow_id_ = "00000000075bcd15"; } @@ -4606,6 +4608,8 @@ TEST_F(OAuth2Test, RouteSpecificConfigOverridesGlobalConfig) { } TEST_F(OAuth2Test, EmptyRouteSpecificConfigOverridesGlobalConfig) { + init(nullptr); + Http::TestRequestHeaderMapImpl request_headers{ {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Path.get(), "/resource"}, From f22ab98f1b4f2eab15823dd666dbb87c8863c980 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 7 Apr 2026 01:22:36 +0000 Subject: [PATCH 12/29] fix format Signed-off-by: Huabing (Robin) Zhao --- .../extensions/filters/http/oauth2/filter.cc | 2 +- .../filters/http/oauth2/filter_test.cc | 1450 +++++++++-------- 2 files changed, 729 insertions(+), 723 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index b1235b72a0914..578c90873476c 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -683,7 +683,7 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he if (config_ == nullptr) { return Http::FilterHeadersStatus::Continue; } - + // Skip Filter and continue chain if a Passthrough header is matching. // Only increment counters here; do not modify request headers, as there may be // other instances of this filter configured that still need to process the request. diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 0d0556ecc518a..8b3e3bbb63344 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -4631,145 +4631,333 @@ TEST_F(OAuth2Test, EmptyRouteSpecificConfigOverridesGlobalConfig) { EXPECT_CALL(*oauth_client_, asyncRefreshAccessToken(_, _, _, _)).Times(0); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); -/** - * Scenario: A malicious client sends x-envoy-oauth-status and x-envoy-oauth-failure-reason headers - * on a request that matches pass_through_matcher (OPTIONS method). - * - * Expected behavior: The injected headers should be sanitized even on pass-through requests, - * since sanitization happens before the pass_through_matcher check. - */ -TEST_F(OAuth2Test, SanitizeInjectedOAuthStatusHeadersOnPassThroughPath) { - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Path.get(), "/anypath"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Options}, - // Malicious client trying to inject OAuth status headers on a pass-through request - {"x-envoy-oauth-status", "injected-success"}, - {"x-envoy-oauth-failure-reason", "injected-reason"}, - }; + /** + * Scenario: A malicious client sends x-envoy-oauth-status and x-envoy-oauth-failure-reason + * headers on a request that matches pass_through_matcher (OPTIONS method). + * + * Expected behavior: The injected headers should be sanitized even on pass-through requests, + * since sanitization happens before the pass_through_matcher check. + */ + TEST_F(OAuth2Test, SanitizeInjectedOAuthStatusHeadersOnPassThroughPath) { + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Path.get(), "/anypath"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Options}, + // Malicious client trying to inject OAuth status headers on a pass-through request + {"x-envoy-oauth-status", "injected-success"}, + {"x-envoy-oauth-failure-reason", "injected-reason"}, + }; - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - // Verify the injected headers were sanitized even though the request was passed through - EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthStatus).empty()); - EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); + // Verify the injected headers were sanitized even though the request was passed through + EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthStatus).empty()); + EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_passthrough").value(), 1); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); -} - -// Verify cookie prefixes "__Secure-" and "__Host-" cause addition of the "Secure" attribute at -// signout. -TEST_F(OAuth2Test, SecureAttributeAddedForSecureCookiePrefixesOnSignout) { - auto make_config = - [&](absl::string_view prefix) -> envoy::extensions::filters::http::oauth2::v3::OAuth2Config { - envoy::extensions::filters::http::oauth2::v3::OAuth2Config p; - auto* endpoint = p.mutable_token_endpoint(); - endpoint->set_cluster("auth.example.com"); - endpoint->set_uri("auth.example.com/_oauth"); - p.set_authorization_endpoint("https://auth2.example.com/oauth/authorize/"); - p.mutable_signout_path()->mutable_path()->set_exact("/_signout"); - p.mutable_redirect_path_matcher()->mutable_path()->set_exact(TEST_CALLBACK); - auto* credentials = p.mutable_credentials(); - credentials->set_client_id(TEST_CLIENT_ID); - credentials->mutable_token_secret()->set_name("secret"); - credentials->mutable_hmac_secret()->set_name("hmac"); - auto* cookie_names = credentials->mutable_cookie_names(); - cookie_names->set_oauth_hmac(absl::StrCat(prefix, "OauthHMAC")); - cookie_names->set_bearer_token(absl::StrCat(prefix, "BearerToken")); - cookie_names->set_id_token(absl::StrCat(prefix, "IdToken")); - cookie_names->set_refresh_token(absl::StrCat(prefix, "RefreshToken")); - cookie_names->set_oauth_nonce(absl::StrCat(prefix, "OauthNonce")); - cookie_names->set_code_verifier(absl::StrCat(prefix, "CodeVerifier")); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_passthrough").value(), 1); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); + } - auto* matcher = p.add_pass_through_matcher(); - matcher->set_name(":method"); - matcher->mutable_string_match()->set_exact("OPTIONS"); + // Verify cookie prefixes "__Secure-" and "__Host-" cause addition of the "Secure" attribute at + // signout. + TEST_F(OAuth2Test, SecureAttributeAddedForSecureCookiePrefixesOnSignout) { + auto make_config = [&](absl::string_view prefix) + -> envoy::extensions::filters::http::oauth2::v3::OAuth2Config { + envoy::extensions::filters::http::oauth2::v3::OAuth2Config p; + auto* endpoint = p.mutable_token_endpoint(); + endpoint->set_cluster("auth.example.com"); + endpoint->set_uri("auth.example.com/_oauth"); + p.set_authorization_endpoint("https://auth2.example.com/oauth/authorize/"); + p.mutable_signout_path()->mutable_path()->set_exact("/_signout"); + p.mutable_redirect_path_matcher()->mutable_path()->set_exact(TEST_CALLBACK); + auto* credentials = p.mutable_credentials(); + credentials->set_client_id(TEST_CLIENT_ID); + credentials->mutable_token_secret()->set_name("secret"); + credentials->mutable_hmac_secret()->set_name("hmac"); + auto* cookie_names = credentials->mutable_cookie_names(); + cookie_names->set_oauth_hmac(absl::StrCat(prefix, "OauthHMAC")); + cookie_names->set_bearer_token(absl::StrCat(prefix, "BearerToken")); + cookie_names->set_id_token(absl::StrCat(prefix, "IdToken")); + cookie_names->set_refresh_token(absl::StrCat(prefix, "RefreshToken")); + cookie_names->set_oauth_nonce(absl::StrCat(prefix, "OauthNonce")); + cookie_names->set_code_verifier(absl::StrCat(prefix, "CodeVerifier")); + + auto* matcher = p.add_pass_through_matcher(); + matcher->set_name(":method"); + matcher->mutable_string_match()->set_exact("OPTIONS"); + + return p; + }; - return p; - }; + auto run_test_with_prefix = [&](absl::string_view prefix, bool expect_secure) { + auto p = make_config(prefix); + auto secret_reader = std::make_shared(); + init(std::make_shared(p, factory_context_.server_factory_context_, + secret_reader, scope_, "test.")); + + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) + .WillOnce(Invoke([&](Http::ResponseHeaderMap& passed_headers, bool) { + EXPECT_EQ(passed_headers.get(Http::Headers::get().SetCookie).size(), 4); + const auto& cookie_str = + passed_headers.get(Http::Headers::get().SetCookie)[0]->value().getStringView(); + if (expect_secure) { + EXPECT_THAT(cookie_str, testing::HasSubstr("; Secure")); + } else { + EXPECT_THAT(cookie_str, testing::Not(testing::HasSubstr("; Secure"))); + } + })); + auto request_headers = Http::TestRequestHeaderMapImpl{ + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Path.get(), "/_signout"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + }; + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); + }; - auto run_test_with_prefix = [&](absl::string_view prefix, bool expect_secure) { - auto p = make_config(prefix); - auto secret_reader = std::make_shared(); - init(std::make_shared(p, factory_context_.server_factory_context_, secret_reader, - scope_, "test.")); + run_test_with_prefix("__Secure-", true); + run_test_with_prefix("__Host-", true); + run_test_with_prefix("", false); + } - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) - .WillOnce(Invoke([&](Http::ResponseHeaderMap& passed_headers, bool) { - EXPECT_EQ(passed_headers.get(Http::Headers::get().SetCookie).size(), 4); - const auto& cookie_str = - passed_headers.get(Http::Headers::get().SetCookie)[0]->value().getStringView(); - if (expect_secure) { - EXPECT_THAT(cookie_str, testing::HasSubstr("; Secure")); - } else { - EXPECT_THAT(cookie_str, testing::Not(testing::HasSubstr("; Secure"))); - } - })); - auto request_headers = Http::TestRequestHeaderMapImpl{ - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Path.get(), "/_signout"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + // Test that custom cookie paths work correctly. + TEST_F(OAuth2Test, OAuthTestCustomCookiePaths) { + // Initialize with different paths: CSRF cookies on /auth/callback, session cookies on /app. + init(getConfig(true, true, + ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: + OAuth2Config_AuthType_URL_ENCODED_BODY, + 0, false, false, false, false, false, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + 0, 0, false, + "/app", // bearer_token_path + "/app", // hmac_path + "/app", // expires_path + "/app", // id_token_path + "/app", // refresh_token_path + "/auth/callback", // nonce_path (CSRF cookie) + "/auth/callback" // code_verifier_path + )); + + test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); + + // Helper lambda to extract Set-Cookie headers from response. + auto extract_cookies = [](const Http::TestResponseHeaderMapImpl& headers) { + std::vector cookies; + headers.iterate([&cookies](const Http::HeaderEntry& header) -> Http::HeaderMap::Iterate { + if (header.key().getStringView() == Http::Headers::get().SetCookie.get()) { + cookies.push_back(std::string(header.value().getStringView())); + } + return Http::HeaderMap::Iterate::Continue; + }); + return cookies; }; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(request_headers, false)); - }; - - run_test_with_prefix("__Secure-", true); - run_test_with_prefix("__Host-", true); - run_test_with_prefix("", false); -} -// Test that custom cookie paths work correctly. -TEST_F(OAuth2Test, OAuthTestCustomCookiePaths) { - // Initialize with different paths: CSRF cookies on /auth/callback, session cookies on /app. - init(getConfig(true, true, - ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: - OAuth2Config_AuthType_URL_ENCODED_BODY, - 0, false, false, false, false, false, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - 0, 0, false, - "/app", // bearer_token_path - "/app", // hmac_path - "/app", // expires_path - "/app", // id_token_path - "/app", // refresh_token_path - "/auth/callback", // nonce_path (CSRF cookie) - "/auth/callback" // code_verifier_path - )); + // Phase 1: Test redirect phase. + // We make sure that nonce and code_verifier cookies should have /auth/callback path. + { + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/test"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + }; + + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(false)); + + Http::TestResponseHeaderMapImpl response_headers; + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) + .WillOnce(Invoke([&response_headers](Http::ResponseHeaderMap& headers, bool) { + response_headers = Http::TestResponseHeaderMapImpl(headers); + })); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); + + auto cookies = extract_cookies(response_headers); + bool found_nonce = false, found_code_verifier = false; + for (const auto& cookie : cookies) { + if (cookie.find("OauthNonce.00000000075bcd15=") != std::string::npos) { + EXPECT_NE(cookie.find(";path=/auth/callback;"), std::string::npos) + << "OauthNonce should have path=/auth/callback, got: " << cookie; + found_nonce = true; + } + if (cookie.find("CodeVerifier.00000000075bcd15=") != std::string::npos) { + EXPECT_NE(cookie.find(";path=/auth/callback;"), std::string::npos) + << "CodeVerifier should have path=/auth/callback, got: " << cookie; + found_code_verifier = true; + } + } + EXPECT_TRUE(found_nonce) << "OauthNonce cookie not found."; + EXPECT_TRUE(found_code_verifier) << "CodeVerifier cookie not found."; + } - test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); + // Phase 2: Test token callback. Session cookies should have /app path. + // Reinitialize filter for the callback phase. + init(getConfig(true, true, + ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: + OAuth2Config_AuthType_URL_ENCODED_BODY, + 0, false, false, false, false, false, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + 0, 0, false, "/app", "/app", "/app", "/app", "/app", "/auth/callback", + "/auth/callback")); + { + Http::TestRequestHeaderMapImpl callback_headers{ + {Http::Headers::get().Path.get(), "/_oauth?code=auth_code&state=" + TEST_ENCODED_STATE}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, + {Http::Headers::get().Cookie.get(), + "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, + }; + + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*oauth_client_, asyncGetAccessToken(_, _, _, _, _, _)); + + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, + filter_->decodeHeaders(callback_headers, false)); + + Http::TestResponseHeaderMapImpl response_headers; + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) + .WillOnce(Invoke([&response_headers](Http::ResponseHeaderMap& headers, bool) { + response_headers = Http::TestResponseHeaderMapImpl(headers); + })); + + filter_->onGetAccessTokenSuccess("access_token", "id_token", "refresh_token", + std::chrono::seconds(10)); + + auto cookies = extract_cookies(response_headers); + bool found_hmac = false, found_bearer = false, found_expires = false; + for (const auto& cookie : cookies) { + if (cookie.find("OauthHMAC=") != std::string::npos) { + EXPECT_NE(cookie.find(";path=/app;"), std::string::npos) + << "OauthHMAC should have path=/app, got: " << cookie; + found_hmac = true; + } + if (cookie.find("BearerToken=") != std::string::npos) { + EXPECT_NE(cookie.find(";path=/app;"), std::string::npos) + << "BearerToken should have path=/app, got: " << cookie; + found_bearer = true; + } + if (cookie.find("OauthExpires=") != std::string::npos) { + EXPECT_NE(cookie.find(";path=/app;"), std::string::npos) + << "OauthExpires should have path=/app, got: " << cookie; + found_expires = true; + } + } + EXPECT_TRUE(found_hmac) << "OauthHMAC cookie not found."; + EXPECT_TRUE(found_bearer) << "BearerToken cookie not found."; + EXPECT_TRUE(found_expires) << "OauthExpires cookie not found."; + } - // Helper lambda to extract Set-Cookie headers from response. - auto extract_cookies = [](const Http::TestResponseHeaderMapImpl& headers) { - std::vector cookies; - headers.iterate([&cookies](const Http::HeaderEntry& header) -> Http::HeaderMap::Iterate { - if (header.key().getStringView() == Http::Headers::get().SetCookie.get()) { - cookies.push_back(std::string(header.value().getStringView())); + // Phase 3: Test signout. Cookie deletion should use the configured paths. + // Reinitialize filter for the signout phase. + init(getConfig(true, true, + ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: + OAuth2Config_AuthType_URL_ENCODED_BODY, + 0, false, false, false, false, false, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + 0, 0, false, "/app", "/app", "/app", "/app", "/app", "/auth/callback", + "/auth/callback")); + { + Http::TestRequestHeaderMapImpl signout_headers{ + {Http::Headers::get().Path.get(), "/_signout"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=csrf_token"}, + {Http::Headers::get().Cookie.get(), "CodeVerifier.00000000075bcd15=code_verifier"}, + }; + + Http::TestResponseHeaderMapImpl response_headers; + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) + .WillOnce(Invoke([&response_headers](Http::ResponseHeaderMap& headers, bool) { + response_headers = Http::TestResponseHeaderMapImpl(headers); + })); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(signout_headers, false)); + + auto cookies = extract_cookies(response_headers); + bool found_hmac_delete = false, found_nonce_delete = false, + found_code_verifier_delete = false; + for (const auto& cookie : cookies) { + if (cookie.find("OauthHMAC=deleted") != std::string::npos) { + EXPECT_NE(cookie.find("path=/app"), std::string::npos) + << "OauthHMAC deletion should have path=/app, got: " << cookie; + found_hmac_delete = true; + } + if (cookie.find("OauthNonce.00000000075bcd15=deleted") != std::string::npos) { + EXPECT_NE(cookie.find("path=/auth/callback"), std::string::npos) + << "OauthNonce deletion should have path=/auth/callback, got: " << cookie; + found_nonce_delete = true; + } + if (cookie.find("CodeVerifier.00000000075bcd15=deleted") != std::string::npos) { + EXPECT_NE(cookie.find("path=/auth/callback"), std::string::npos) + << "CodeVerifier deletion should have path=/auth/callback, got: " << cookie; + found_code_verifier_delete = true; + } } - return Http::HeaderMap::Iterate::Continue; - }); - return cookies; - }; + EXPECT_TRUE(found_hmac_delete) << "OauthHMAC deletion cookie not found."; + EXPECT_TRUE(found_nonce_delete) << "OauthNonce deletion cookie not found."; + EXPECT_TRUE(found_code_verifier_delete) << "CodeVerifier deletion cookie not found."; + } + } + + /** + * Scenario: Request to /allowfailed path with no OAuth cookies at all. + * + * Expected behavior: Since there are no cookies and the path matches allow_failed_matcher, + * the filter should not redirect to OAuth server and should continue to upstream as + * unauthenticated. + */ + TEST_F(OAuth2Test, AllowFailedWithNoCookies) { + init(getConfig()); - // Phase 1: Test redirect phase. - // We make sure that nonce and code_verifier cookies should have /auth/callback path. - { Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/test"}, + {Http::Headers::get().Path.get(), "/allowfailed/api"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Scheme.get(), "https"}, @@ -4777,63 +4965,45 @@ TEST_F(OAuth2Test, OAuthTestCustomCookiePaths) { EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(false)); - Http::TestResponseHeaderMapImpl response_headers; - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) - .WillOnce(Invoke([&response_headers](Http::ResponseHeaderMap& headers, bool) { - response_headers = Http::TestResponseHeaderMapImpl(headers); - })); + // Should NOT redirect or send local reply + EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).Times(0); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(request_headers, false)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - auto cookies = extract_cookies(response_headers); - bool found_nonce = false, found_code_verifier = false; - for (const auto& cookie : cookies) { - if (cookie.find("OauthNonce.00000000075bcd15=") != std::string::npos) { - EXPECT_NE(cookie.find(";path=/auth/callback;"), std::string::npos) - << "OauthNonce should have path=/auth/callback, got: " << cookie; - found_nonce = true; - } - if (cookie.find("CodeVerifier.00000000075bcd15=") != std::string::npos) { - EXPECT_NE(cookie.find(";path=/auth/callback;"), std::string::npos) - << "CodeVerifier should have path=/auth/callback, got: " << cookie; - found_code_verifier = true; - } - } - EXPECT_TRUE(found_nonce) << "OauthNonce cookie not found."; - EXPECT_TRUE(found_code_verifier) << "CodeVerifier cookie not found."; + // Verify the headers were added + EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), + "failed"); + EXPECT_FALSE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); + + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); } - // Phase 2: Test token callback. Session cookies should have /app path. - // Reinitialize filter for the callback phase. - init(getConfig(true, true, - ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: - OAuth2Config_AuthType_URL_ENCODED_BODY, - 0, false, false, false, false, false, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - 0, 0, false, "/app", "/app", "/app", "/app", "/app", "/auth/callback", - "/auth/callback")); - { - Http::TestRequestHeaderMapImpl callback_headers{ - {Http::Headers::get().Path.get(), "/_oauth?code=auth_code&state=" + TEST_ENCODED_STATE}, + /** + * Scenario: Request to /allowfailed path with invalid refresh token. Token endpoint is available + * and async refresh is attempted, but the token endpoint rejects the refresh token. + * + * Expected behavior: After the async refresh failure callback, since the path matches + * allow_failed_matcher, the filter should continue to upstream as unauthenticated. + */ + TEST_F(OAuth2Test, AllowFailedWithInvalidRefreshTokenAsyncFailure) { + init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); + + test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); + + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/allowfailed/api"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, + {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, + {Http::Headers::get().Cookie.get(), + "OauthHMAC=" + "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" + "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, + {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, {Http::Headers::get().Cookie.get(), "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, @@ -4841,613 +5011,449 @@ TEST_F(OAuth2Test, OAuthTestCustomCookiePaths) { EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*oauth_client_, asyncGetAccessToken(_, _, _, _, _, _)); + EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); - EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, - filter_->decodeHeaders(callback_headers, false)); - - Http::TestResponseHeaderMapImpl response_headers; - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) - .WillOnce(Invoke([&response_headers](Http::ResponseHeaderMap& headers, bool) { - response_headers = Http::TestResponseHeaderMapImpl(headers); - })); - - filter_->onGetAccessTokenSuccess("access_token", "id_token", "refresh_token", - std::chrono::seconds(10)); - - auto cookies = extract_cookies(response_headers); - bool found_hmac = false, found_bearer = false, found_expires = false; - for (const auto& cookie : cookies) { - if (cookie.find("OauthHMAC=") != std::string::npos) { - EXPECT_NE(cookie.find(";path=/app;"), std::string::npos) - << "OauthHMAC should have path=/app, got: " << cookie; - found_hmac = true; - } - if (cookie.find("BearerToken=") != std::string::npos) { - EXPECT_NE(cookie.find(";path=/app;"), std::string::npos) - << "BearerToken should have path=/app, got: " << cookie; - found_bearer = true; - } - if (cookie.find("OauthExpires=") != std::string::npos) { - EXPECT_NE(cookie.find(";path=/app;"), std::string::npos) - << "OauthExpires should have path=/app, got: " << cookie; - found_expires = true; - } - } - EXPECT_TRUE(found_hmac) << "OauthHMAC cookie not found."; - EXPECT_TRUE(found_bearer) << "BearerToken cookie not found."; - EXPECT_TRUE(found_expires) << "OauthExpires cookie not found."; - } + std::string legit_refresh_token{"legit_refresh_token"}; + EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); - // Phase 3: Test signout. Cookie deletion should use the configured paths. - // Reinitialize filter for the signout phase. - init(getConfig(true, true, - ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: - OAuth2Config_AuthType_URL_ENCODED_BODY, - 0, false, false, false, false, false, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - 0, 0, false, "/app", "/app", "/app", "/app", "/app", "/auth/callback", - "/auth/callback")); - { - Http::TestRequestHeaderMapImpl signout_headers{ - {Http::Headers::get().Path.get(), "/_signout"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=csrf_token"}, - {Http::Headers::get().Cookie.get(), "CodeVerifier.00000000075bcd15=code_verifier"}, - }; + // Mock asyncRefreshAccessToken to succeed (async work starts) + EXPECT_CALL(*oauth_client_, + asyncRefreshAccessToken("legit_refresh_token", TEST_CLIENT_ID, + "asdf_client_secret_fdsa", AuthType::UrlEncodedBody)); - Http::TestResponseHeaderMapImpl response_headers; - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) - .WillOnce(Invoke([&response_headers](Http::ResponseHeaderMap& headers, bool) { - response_headers = Http::TestResponseHeaderMapImpl(headers); - })); + // Filter should stop and wait for async response + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, + filter_->decodeHeaders(request_headers, false)); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(signout_headers, false)); - - auto cookies = extract_cookies(response_headers); - bool found_hmac_delete = false, found_nonce_delete = false, found_code_verifier_delete = false; - for (const auto& cookie : cookies) { - if (cookie.find("OauthHMAC=deleted") != std::string::npos) { - EXPECT_NE(cookie.find("path=/app"), std::string::npos) - << "OauthHMAC deletion should have path=/app, got: " << cookie; - found_hmac_delete = true; - } - if (cookie.find("OauthNonce.00000000075bcd15=deleted") != std::string::npos) { - EXPECT_NE(cookie.find("path=/auth/callback"), std::string::npos) - << "OauthNonce deletion should have path=/auth/callback, got: " << cookie; - found_nonce_delete = true; - } - if (cookie.find("CodeVerifier.00000000075bcd15=deleted") != std::string::npos) { - EXPECT_NE(cookie.find("path=/auth/callback"), std::string::npos) - << "CodeVerifier deletion should have path=/auth/callback, got: " << cookie; - found_code_verifier_delete = true; - } - } - EXPECT_TRUE(found_hmac_delete) << "OauthHMAC deletion cookie not found."; - EXPECT_TRUE(found_nonce_delete) << "OauthNonce deletion cookie not found."; - EXPECT_TRUE(found_code_verifier_delete) << "CodeVerifier deletion cookie not found."; + // Now simulate the async failure callback from token endpoint. + // onRefreshAccessTokenFailure() returns Continue; the caller (handleRefreshTokenFailure in + // oauth_client) is responsible for calling continueDecoding() when is_request_dispatched=true. + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->onRefreshAccessTokenFailure()); + + // Verify the headers were added + EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), + "failed"); + EXPECT_FALSE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); + + // Verify OAuth token and flow cookies were removed + auto cookies = Http::Utility::parseCookies(request_headers); + EXPECT_TRUE(cookies.find("BearerToken") == cookies.end()); + EXPECT_TRUE(cookies.find("OauthHMAC") == cookies.end()); + EXPECT_TRUE(cookies.find("OauthExpires") == cookies.end()); + EXPECT_TRUE(cookies.find("RefreshToken") == cookies.end()); + EXPECT_TRUE(cookies.find("OauthNonce.00000000075bcd15") == cookies.end()); + EXPECT_TRUE(cookies.find("CodeVerifier.00000000075bcd15") == cookies.end()); + + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_refreshtoken_failure").value(), 1); } -} - -/** - * Scenario: Request to /allowfailed path with no OAuth cookies at all. - * - * Expected behavior: Since there are no cookies and the path matches allow_failed_matcher, - * the filter should not redirect to OAuth server and should continue to upstream as - * unauthenticated. - */ -TEST_F(OAuth2Test, AllowFailedWithNoCookies) { - init(getConfig()); - - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/allowfailed/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - }; - - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - - // Should NOT redirect or send local reply - EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).Times(0); - - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - - // Verify the headers were added - EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), - "failed"); - EXPECT_FALSE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); -} + /** + * Scenario: Request to /allowfailed path with refresh token cookie, but asyncRefreshAccessToken + * fails synchronously (e.g. cluster not found). + * + * Expected behavior: Since asyncRefreshAccessToken fails synchronously (FailureContinue state) + * and the path matches allow_failed_matcher, decodeHeaders returns Continue immediately. + */ + TEST_F(OAuth2Test, AllowFailedWithRefreshTokenSyncFailure) { + init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); -/** - * Scenario: Request to /allowfailed path with invalid refresh token. Token endpoint is available - * and async refresh is attempted, but the token endpoint rejects the refresh token. - * - * Expected behavior: After the async refresh failure callback, since the path matches - * allow_failed_matcher, the filter should continue to upstream as unauthenticated. - */ -TEST_F(OAuth2Test, AllowFailedWithInvalidRefreshTokenAsyncFailure) { - init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); - - test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); - - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/allowfailed/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, - {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, - {Http::Headers::get().Cookie.get(), - "OauthHMAC=" - "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" - "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, - {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, - {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, - {Http::Headers::get().Cookie.get(), - "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, - }; - - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); - - std::string legit_refresh_token{"legit_refresh_token"}; - EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); - - // Mock asyncRefreshAccessToken to succeed (async work starts) - EXPECT_CALL(*oauth_client_, - asyncRefreshAccessToken("legit_refresh_token", TEST_CLIENT_ID, - "asdf_client_secret_fdsa", AuthType::UrlEncodedBody)); - - // Filter should stop and wait for async response - EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, - filter_->decodeHeaders(request_headers, false)); - - // Now simulate the async failure callback from token endpoint. - // onRefreshAccessTokenFailure() returns Continue; the caller (handleRefreshTokenFailure in - // oauth_client) is responsible for calling continueDecoding() when is_request_dispatched=true. - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->onRefreshAccessTokenFailure()); - - // Verify the headers were added - EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), - "failed"); - EXPECT_FALSE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); - - // Verify OAuth token and flow cookies were removed - auto cookies = Http::Utility::parseCookies(request_headers); - EXPECT_TRUE(cookies.find("BearerToken") == cookies.end()); - EXPECT_TRUE(cookies.find("OauthHMAC") == cookies.end()); - EXPECT_TRUE(cookies.find("OauthExpires") == cookies.end()); - EXPECT_TRUE(cookies.find("RefreshToken") == cookies.end()); - EXPECT_TRUE(cookies.find("OauthNonce.00000000075bcd15") == cookies.end()); - EXPECT_TRUE(cookies.find("CodeVerifier.00000000075bcd15") == cookies.end()); - - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_refreshtoken_failure").value(), 1); -} - -/** - * Scenario: Request to /allowfailed path with refresh token cookie, but asyncRefreshAccessToken - * fails synchronously (e.g. cluster not found). - * - * Expected behavior: Since asyncRefreshAccessToken fails synchronously (FailureContinue state) - * and the path matches allow_failed_matcher, decodeHeaders returns Continue immediately. - */ -TEST_F(OAuth2Test, AllowFailedWithRefreshTokenSyncFailure) { - init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); - - test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); - - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/allowfailed/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, - {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, - {Http::Headers::get().Cookie.get(), - "OauthHMAC=" - "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" - "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, - {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, - {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, - {Http::Headers::get().Cookie.get(), - "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, - }; - - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); - - std::string legit_refresh_token{"legit_refresh_token"}; - EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); + test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); - // asyncRefreshAccessToken fails synchronously; getState() returns FailureContinue - EXPECT_CALL(*oauth_client_, - asyncRefreshAccessToken("legit_refresh_token", TEST_CLIENT_ID, - "asdf_client_secret_fdsa", AuthType::UrlEncodedBody)); - EXPECT_CALL(*oauth_client_, getState()) - .WillOnce(Return(OAuth2Client::OAuthState::FailureContinue)); + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/allowfailed/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, + {Http::Headers::get().Cookie.get(), + "OauthHMAC=" + "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" + "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, + {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, + {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, + {Http::Headers::get().Cookie.get(), + "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, + }; - // Filter should continue immediately (not wait for async) - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); -} + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); -/** - * Scenario: OAuth callback where asyncGetAccessToken fails synchronously (e.g. cluster not found). - * - * Expected behavior: Since asyncGetAccessToken fails synchronously (FailureStop state), - * decodeHeaders returns StopIteration. The real error handling (e.g. sending 401) is done - * inside asyncGetAccessToken before returning. - */ -TEST_F(OAuth2Test, OAuthCallbackGetAccessTokenSyncFailure) { - init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); + std::string legit_refresh_token{"legit_refresh_token"}; + EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, - {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, - {Http::Headers::get().Cookie.get(), - "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Scheme.get(), "https"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - }; + // asyncRefreshAccessToken fails synchronously; getState() returns FailureContinue + EXPECT_CALL(*oauth_client_, + asyncRefreshAccessToken("legit_refresh_token", TEST_CLIENT_ID, + "asdf_client_secret_fdsa", AuthType::UrlEncodedBody)); + EXPECT_CALL(*oauth_client_, getState()) + .WillOnce(Return(OAuth2Client::OAuthState::FailureContinue)); - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + // Filter should continue immediately (not wait for async) + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + } - // asyncGetAccessToken fails synchronously; getState() returns FailureStop - EXPECT_CALL(*oauth_client_, asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", - "https://traffic.example.com" + TEST_CALLBACK, - TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); - EXPECT_CALL(*oauth_client_, getState()).WillOnce(Return(OAuth2Client::OAuthState::FailureStop)); + /** + * Scenario: OAuth callback where asyncGetAccessToken fails synchronously (e.g. cluster not + * found). + * + * Expected behavior: Since asyncGetAccessToken fails synchronously (FailureStop state), + * decodeHeaders returns StopIteration. The real error handling (e.g. sending 401) is done + * inside asyncGetAccessToken before returning. + */ + TEST_F(OAuth2Test, OAuthCallbackGetAccessTokenSyncFailure) { + init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(request_headers, false)); -} + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, + {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, + {Http::Headers::get().Cookie.get(), + "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Scheme.get(), "https"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + }; -/** - * Scenario: AJAX request (matching deny_redirect_matcher) to /allowfailed path without valid OAuth. - * - * Expected behavior: allow_failed_matcher takes precedence - request should continue as - * unauthenticated rather than returning 401 (deny_redirect would normally return 401). - */ -TEST_F(OAuth2Test, AllowFailedMatcherTakesPrecedenceOverDenyRedirect) { - init(getConfig()); + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/allowfailed/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, - {Http::Headers::get().Scheme.get(), "https"}, - {"X-Requested-With", "XMLHttpRequest"}, // Matches deny_redirect_matcher - }; + // asyncGetAccessToken fails synchronously; getState() returns FailureStop + EXPECT_CALL(*oauth_client_, + asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", + "https://traffic.example.com" + TEST_CALLBACK, + TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); + EXPECT_CALL(*oauth_client_, getState()).WillOnce(Return(OAuth2Client::OAuthState::FailureStop)); - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); + } - // Should NOT return 401, should continue - EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); + /** + * Scenario: AJAX request (matching deny_redirect_matcher) to /allowfailed path without valid + * OAuth. + * + * Expected behavior: allow_failed_matcher takes precedence - request should continue as + * unauthenticated rather than returning 401 (deny_redirect would normally return 401). + */ + TEST_F(OAuth2Test, AllowFailedMatcherTakesPrecedenceOverDenyRedirect) { + init(getConfig()); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/allowfailed/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, + {Http::Headers::get().Scheme.get(), "https"}, + {"X-Requested-With", "XMLHttpRequest"}, // Matches deny_redirect_matcher + }; - EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), - "failed"); + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); -} + // Should NOT return 401, should continue + EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); -/** - * Scenario: Request with injected OAuth status headers to test sanitization. - * A malicious client sends x-envoy-oauth-status and x-envoy-oauth-failure-reason headers - * in their request. - * - * Expected behavior: The filter should sanitize these headers to prevent header injection - * attacks, similar to how it sanitizes the Authorization header. - */ -TEST_F(OAuth2Test, SanitizeInjectedOAuthStatusHeaders) { - init(getConfig()); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/anypath"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - // Malicious client trying to inject OAuth status headers - {"x-envoy-oauth-status", "injected-success"}, - {"x-envoy-oauth-failure-reason", "injected-reason"}, - }; + EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), + "failed"); - // cookie-validation mocking - user is authenticated - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); + } - std::string legit_token{"legit_token"}; - EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); + /** + * Scenario: Request with injected OAuth status headers to test sanitization. + * A malicious client sends x-envoy-oauth-status and x-envoy-oauth-failure-reason headers + * in their request. + * + * Expected behavior: The filter should sanitize these headers to prevent header injection + * attacks, similar to how it sanitizes the Authorization header. + */ + TEST_F(OAuth2Test, SanitizeInjectedOAuthStatusHeaders) { + init(getConfig()); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/anypath"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + // Malicious client trying to inject OAuth status headers + {"x-envoy-oauth-status", "injected-success"}, + {"x-envoy-oauth-failure-reason", "injected-reason"}, + }; - // Verify the injected headers were sanitized (removed) - EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthStatus).empty()); - EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); + // cookie-validation mocking - user is authenticated + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_success").value(), 1); -} + std::string legit_token{"legit_token"}; + EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); -/** - * Scenario: Request with injected OAuth status headers on allow_failed path. - * Tests that sanitization occurs before the filter sets its own legitimate headers. - * - * Expected behavior: The injected headers should be removed, and only the filter's - * legitimate headers should be present in the final request. - */ -TEST_F(OAuth2Test, SanitizeInjectedOAuthStatusHeadersOnAllowFailedPath) { - init(getConfig()); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/allowfailed/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - // Malicious client trying to inject fake success status - {"x-envoy-oauth-status", "injected-success"}, - {"x-envoy-oauth-failure-reason", "injected-fake-reason"}, - }; + // Verify the injected headers were sanitized (removed) + EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthStatus).empty()); + EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_success").value(), 1); + } - EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).Times(0); + /** + * Scenario: Request with injected OAuth status headers on allow_failed path. + * Tests that sanitization occurs before the filter sets its own legitimate headers. + * + * Expected behavior: The injected headers should be removed, and only the filter's + * legitimate headers should be present in the final request. + */ + TEST_F(OAuth2Test, SanitizeInjectedOAuthStatusHeadersOnAllowFailedPath) { + init(getConfig()); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/allowfailed/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + // Malicious client trying to inject fake success status + {"x-envoy-oauth-status", "injected-success"}, + {"x-envoy-oauth-failure-reason", "injected-fake-reason"}, + }; - // Verify only the legitimate "failed" status is present (not the injected "success") - auto status_headers = request_headers.get(OAuth2Headers::get().OAuthStatus); - EXPECT_EQ(status_headers.size(), 1); - EXPECT_EQ(status_headers[0]->value().getStringView(), "failed"); + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - auto reason_headers = request_headers.get(OAuth2Headers::get().OAuthFailureReason); - EXPECT_EQ(reason_headers.size(), 1); - // Verify it's not the injected value - EXPECT_NE(reason_headers[0]->value().getStringView(), "injected-fake-reason"); + EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).Times(0); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); -} + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); -/** - * Scenario: handleOAuthFailure is called on an allow-failed path. - * - * This tests the OAuth2Filter::handleOAuthFailure code path (lines 1467-1471), which is - * only reachable via OAuth2ClientImpl callbacks (asyncGetAccessToken/asyncRefreshAccessToken - * failures). Since the client is mocked in filter tests, this function must be called directly - * after decodeHeaders has set request_headers_. - * - * Expected behavior: shouldAllowFailed returns true, continueWithFailedOAuth is called, - * filter returns Continue. - */ -TEST_F(OAuth2Test, HandleOAuthFailureOnAllowedPath) { - init(getConfig()); + // Verify only the legitimate "failed" status is present (not the injected "success") + auto status_headers = request_headers.get(OAuth2Headers::get().OAuthStatus); + EXPECT_EQ(status_headers.size(), 1); + EXPECT_EQ(status_headers[0]->value().getStringView(), "failed"); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/allowfailed/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - }; + auto reason_headers = request_headers.get(OAuth2Headers::get().OAuthFailureReason); + EXPECT_EQ(reason_headers.size(), 1); + // Verify it's not the injected value + EXPECT_NE(reason_headers[0]->value().getStringView(), "injected-fake-reason"); - // Use a valid OAuth flow to set request_headers_ without triggering allow-failed behavior. - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); - std::string legit_token{"legit_token"}; - EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); + } - // Now call handleOAuthFailure directly (the path exercised by real OAuth client callbacks). - // Path matches allow_failed_matcher → should continue as unauthenticated. - EXPECT_EQ(Http::FilterHeadersStatus::Continue, - filter_->handleOAuthFailure("Token exchange failed")); + /** + * Scenario: handleOAuthFailure is called on an allow-failed path. + * + * This tests the OAuth2Filter::handleOAuthFailure code path (lines 1467-1471), which is + * only reachable via OAuth2ClientImpl callbacks (asyncGetAccessToken/asyncRefreshAccessToken + * failures). Since the client is mocked in filter tests, this function must be called directly + * after decodeHeaders has set request_headers_. + * + * Expected behavior: shouldAllowFailed returns true, continueWithFailedOAuth is called, + * filter returns Continue. + */ + TEST_F(OAuth2Test, HandleOAuthFailureOnAllowedPath) { + init(getConfig()); - EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), - "failed"); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); -} + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/allowfailed/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + }; -/** - * Scenario: handleOAuthFailure is called on a regular path (not matching allow_failed_matcher), - * with extra_details provided. - * - * This tests the else branch of OAuth2Filter::handleOAuthFailure (lines 1472-1477) and the - * non-empty extra_details branch in the details formatting (line 1474). - * - * Expected behavior: shouldAllowFailed returns false, sendUnauthorizedResponse is called with - * "reason: extra_details" formatting, filter returns StopIteration. - */ -TEST_F(OAuth2Test, HandleOAuthFailureOnNonAllowedPathWithExtraDetails) { - init(getConfig()); + // Use a valid OAuth flow to set request_headers_ without triggering allow-failed behavior. + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); + std::string legit_token{"legit_token"}; + EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + + // Now call handleOAuthFailure directly (the path exercised by real OAuth client callbacks). + // Path matches allow_failed_matcher → should continue as unauthenticated. + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_->handleOAuthFailure("Token exchange failed")); + + EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), + "failed"); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); + } - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/regular/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - }; + /** + * Scenario: handleOAuthFailure is called on a regular path (not matching allow_failed_matcher), + * with extra_details provided. + * + * This tests the else branch of OAuth2Filter::handleOAuthFailure (lines 1472-1477) and the + * non-empty extra_details branch in the details formatting (line 1474). + * + * Expected behavior: shouldAllowFailed returns false, sendUnauthorizedResponse is called with + * "reason: extra_details" formatting, filter returns StopIteration. + */ + TEST_F(OAuth2Test, HandleOAuthFailureOnNonAllowedPathWithExtraDetails) { + init(getConfig()); - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); - std::string legit_token{"legit_token"}; - EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/regular/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + }; - // Path does not match allow_failed_matcher → should send 401 and stop. - EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Unauthorized, _, _, _, _)); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->handleOAuthFailure("Token exchange failed", "response code: 500")); -} + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); + std::string legit_token{"legit_token"}; + EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); -/** - * Scenario: shouldAllowFailed is called for the OAuth callback path (/_oauth) when the - * allow_failed_matcher is configured to match all paths (prefix "/"). - * - * This tests the redirectPathMatcher guard in shouldAllowFailed (lines 1430-1434): even when - * allow_failed_matcher matches the callback URL, shouldAllowFailed must return false because - * the callback path is "hosted" by Envoy and must never reach upstream as unauthenticated. - * - * Expected behavior: handleOAuthFailure sends 401 rather than continuing as unauthenticated. - */ -TEST_F(OAuth2Test, AllowFailedBlockedForCallbackPath) { - // Create a config where allow_failed_matcher matches all paths including /_oauth. - envoy::extensions::filters::http::oauth2::v3::OAuth2Config p; - auto* endpoint = p.mutable_token_endpoint(); - endpoint->set_cluster("auth.example.com"); - endpoint->set_uri("auth.example.com/_oauth"); - endpoint->mutable_timeout()->set_seconds(1); - p.set_redirect_uri("%REQ(:scheme)%://%REQ(:authority)%" + TEST_CALLBACK); - p.mutable_redirect_path_matcher()->mutable_path()->set_exact(TEST_CALLBACK); - p.set_authorization_endpoint("https://auth.example.com/oauth/authorize/"); - p.mutable_signout_path()->mutable_path()->set_exact("/_signout"); - p.set_forward_bearer_token(true); - p.set_stat_prefix("my_prefix"); - p.add_auth_scopes("user"); - auto* pass_through = p.add_pass_through_matcher(); - pass_through->set_name(":method"); - pass_through->mutable_string_match()->set_exact("OPTIONS"); - // Configure allow_failed_matcher to match all paths, including the callback path /_oauth. - auto* allow_failed_matcher = p.add_allow_failed_matcher(); - allow_failed_matcher->set_name(":path"); - allow_failed_matcher->mutable_string_match()->set_prefix("/"); - auto* credentials = p.mutable_credentials(); - credentials->set_client_id(TEST_CLIENT_ID); - credentials->mutable_token_secret()->set_name("secret"); - credentials->mutable_hmac_secret()->set_name("hmac"); - MessageUtil::validate(p, ProtobufMessage::getStrictValidationVisitor()); - auto secret_reader = std::make_shared(); - init(std::make_shared(p, factory_context_.server_factory_context_, secret_reader, - scope_, "test.")); + // Path does not match allow_failed_matcher → should send 401 and stop. + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Unauthorized, _, _, _, _)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->handleOAuthFailure("Token exchange failed", "response code: 500")); + } - // Make a callback request — asyncGetAccessToken is called and returns Idle (async pending). - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, - {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, - {Http::Headers::get().Cookie.get(), - "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Scheme.get(), "https"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - }; + /** + * Scenario: shouldAllowFailed is called for the OAuth callback path (/_oauth) when the + * allow_failed_matcher is configured to match all paths (prefix "/"). + * + * This tests the redirectPathMatcher guard in shouldAllowFailed (lines 1430-1434): even when + * allow_failed_matcher matches the callback URL, shouldAllowFailed must return false because + * the callback path is "hosted" by Envoy and must never reach upstream as unauthenticated. + * + * Expected behavior: handleOAuthFailure sends 401 rather than continuing as unauthenticated. + */ + TEST_F(OAuth2Test, AllowFailedBlockedForCallbackPath) { + // Create a config where allow_failed_matcher matches all paths including /_oauth. + envoy::extensions::filters::http::oauth2::v3::OAuth2Config p; + auto* endpoint = p.mutable_token_endpoint(); + endpoint->set_cluster("auth.example.com"); + endpoint->set_uri("auth.example.com/_oauth"); + endpoint->mutable_timeout()->set_seconds(1); + p.set_redirect_uri("%REQ(:scheme)%://%REQ(:authority)%" + TEST_CALLBACK); + p.mutable_redirect_path_matcher()->mutable_path()->set_exact(TEST_CALLBACK); + p.set_authorization_endpoint("https://auth.example.com/oauth/authorize/"); + p.mutable_signout_path()->mutable_path()->set_exact("/_signout"); + p.set_forward_bearer_token(true); + p.set_stat_prefix("my_prefix"); + p.add_auth_scopes("user"); + auto* pass_through = p.add_pass_through_matcher(); + pass_through->set_name(":method"); + pass_through->mutable_string_match()->set_exact("OPTIONS"); + // Configure allow_failed_matcher to match all paths, including the callback path /_oauth. + auto* allow_failed_matcher = p.add_allow_failed_matcher(); + allow_failed_matcher->set_name(":path"); + allow_failed_matcher->mutable_string_match()->set_prefix("/"); + auto* credentials = p.mutable_credentials(); + credentials->set_client_id(TEST_CLIENT_ID); + credentials->mutable_token_secret()->set_name("secret"); + credentials->mutable_hmac_secret()->set_name("hmac"); + MessageUtil::validate(p, ProtobufMessage::getStrictValidationVisitor()); + auto secret_reader = std::make_shared(); + init(std::make_shared(p, factory_context_.server_factory_context_, secret_reader, + scope_, "test.")); - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*oauth_client_, asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", - "https://traffic.example.com" + TEST_CALLBACK, - TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); - // getState returns Idle → decodeHeaders waits for async response. - EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, - filter_->decodeHeaders(request_headers, false)); + // Make a callback request — asyncGetAccessToken is called and returns Idle (async pending). + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, + {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, + {Http::Headers::get().Cookie.get(), + "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Scheme.get(), "https"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + }; - // Now simulate the real OAuth client calling back with a failure. - // Even though allow_failed_matcher matches "/", shouldAllowFailed returns false for /_oauth - // because redirectPathMatcher matches it first. - EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Unauthorized, _, _, _, _)); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->handleOAuthFailure("Token exchange failed")); + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*oauth_client_, + asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", + "https://traffic.example.com" + TEST_CALLBACK, + TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); + // getState returns Idle → decodeHeaders waits for async response. + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, + filter_->decodeHeaders(request_headers, false)); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 0); -} + // Now simulate the real OAuth client calling back with a failure. + // Even though allow_failed_matcher matches "/", shouldAllowFailed returns false for /_oauth + // because redirectPathMatcher matches it first. + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Unauthorized, _, _, _, _)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->handleOAuthFailure("Token exchange failed")); -/** - * Scenario: asyncRefreshAccessToken fails synchronously (FailureStop), meaning the token - * endpoint cluster was not found and shouldAllowFailed is false — a 401 was already sent - * synchronously inside asyncRefreshAccessToken. - * - * Expected behavior: decodeHeaders returns StopIteration. - */ -TEST_F(OAuth2Test, AllowFailedWithRefreshTokenSyncFailureStop) { - init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 0); + } - test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); + /** + * Scenario: asyncRefreshAccessToken fails synchronously (FailureStop), meaning the token + * endpoint cluster was not found and shouldAllowFailed is false — a 401 was already sent + * synchronously inside asyncRefreshAccessToken. + * + * Expected behavior: decodeHeaders returns StopIteration. + */ + TEST_F(OAuth2Test, AllowFailedWithRefreshTokenSyncFailureStop) { + init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/regular/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, - {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, - {Http::Headers::get().Cookie.get(), - "OauthHMAC=" - "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" - "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, - {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, - }; + test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); - std::string legit_refresh_token{"legit_refresh_token"}; - EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/regular/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, + {Http::Headers::get().Cookie.get(), + "OauthHMAC=" + "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" + "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, + {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, + }; - EXPECT_CALL(*oauth_client_, - asyncRefreshAccessToken("legit_refresh_token", TEST_CLIENT_ID, - "asdf_client_secret_fdsa", AuthType::UrlEncodedBody)); - // Sync failure: a 401 or redirect was already sent inside asyncRefreshAccessToken. - EXPECT_CALL(*oauth_client_, getState()).WillOnce(Return(OAuth2Client::OAuthState::FailureStop)); + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); + std::string legit_refresh_token{"legit_refresh_token"}; + EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(request_headers, false)); -} + EXPECT_CALL(*oauth_client_, + asyncRefreshAccessToken("legit_refresh_token", TEST_CLIENT_ID, + "asdf_client_secret_fdsa", AuthType::UrlEncodedBody)); + // Sync failure: a 401 or redirect was already sent inside asyncRefreshAccessToken. + EXPECT_CALL(*oauth_client_, getState()).WillOnce(Return(OAuth2Client::OAuthState::FailureStop)); -/** - * Scenario: asyncGetAccessToken fails synchronously with FailureContinue on the OAuth callback - * path — meaning the allow-failed path was triggered synchronously inside asyncGetAccessToken. - * - * Expected behavior: decodeHeaders returns Continue. - */ -TEST_F(OAuth2Test, OAuthCallbackGetAccessTokenSyncContinue) { - init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); + } - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, - {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, - {Http::Headers::get().Cookie.get(), - "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Scheme.get(), "https"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - }; + /** + * Scenario: asyncGetAccessToken fails synchronously with FailureContinue on the OAuth callback + * path — meaning the allow-failed path was triggered synchronously inside asyncGetAccessToken. + * + * Expected behavior: decodeHeaders returns Continue. + */ + TEST_F(OAuth2Test, OAuthCallbackGetAccessTokenSyncContinue) { + init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*oauth_client_, asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", - "https://traffic.example.com" + TEST_CALLBACK, - TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); - // Sync success on the allow-failed path: continueDecoding() was called inside - // asyncGetAccessToken. - EXPECT_CALL(*oauth_client_, getState()) - .WillOnce(Return(OAuth2Client::OAuthState::FailureContinue)); + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, + {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, + {Http::Headers::get().Cookie.get(), + "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Scheme.get(), "https"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + }; - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); -} + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*oauth_client_, + asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", + "https://traffic.example.com" + TEST_CALLBACK, + TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); + // Sync success on the allow-failed path: continueDecoding() was called inside + // asyncGetAccessToken. + EXPECT_CALL(*oauth_client_, getState()) + .WillOnce(Return(OAuth2Client::OAuthState::FailureContinue)); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + } } // namespace Oauth2 } // namespace HttpFilters From b364732f6bac59d4ebc4b04c1cd0e94465f31cee Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 7 Apr 2026 02:31:36 +0000 Subject: [PATCH 13/29] fix test Signed-off-by: Huabing (Robin) Zhao --- .../filters/http/oauth2/filter_test.cc | 1455 ++++++++--------- 1 file changed, 725 insertions(+), 730 deletions(-) diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 8b3e3bbb63344..615b9ab161f8f 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -141,8 +141,6 @@ class OAuth2Test : public testing::TestWithParam { }, test_time_, test_random_); EXPECT_CALL(*oauth_client_, getState()).WillRepeatedly(Return(OAuth2Client::OAuthState::Idle)); - filter_ = std::make_shared(config_, std::move(oauth_client_ptr), test_time_, - test_random_); filter_->setDecoderFilterCallbacks(decoder_callbacks_); filter_->setEncoderFilterCallbacks(encoder_callbacks_); validator_ = std::make_shared(); @@ -4630,334 +4628,147 @@ TEST_F(OAuth2Test, EmptyRouteSpecificConfigOverridesGlobalConfig) { EXPECT_CALL(*oauth_client_, asyncGetAccessToken(_, _, _, _, _, _)).Times(0); EXPECT_CALL(*oauth_client_, asyncRefreshAccessToken(_, _, _, _)).Times(0); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); +} - /** - * Scenario: A malicious client sends x-envoy-oauth-status and x-envoy-oauth-failure-reason - * headers on a request that matches pass_through_matcher (OPTIONS method). - * - * Expected behavior: The injected headers should be sanitized even on pass-through requests, - * since sanitization happens before the pass_through_matcher check. - */ - TEST_F(OAuth2Test, SanitizeInjectedOAuthStatusHeadersOnPassThroughPath) { - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Path.get(), "/anypath"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Options}, - // Malicious client trying to inject OAuth status headers on a pass-through request - {"x-envoy-oauth-status", "injected-success"}, - {"x-envoy-oauth-failure-reason", "injected-reason"}, - }; +/** + * Scenario: A malicious client sends x-envoy-oauth-status and x-envoy-oauth-failure-reason + * headers on a request that matches pass_through_matcher (OPTIONS method). + * + * Expected behavior: The injected headers should be sanitized even on pass-through requests, + * since sanitization happens before the pass_through_matcher check. + */ +TEST_F(OAuth2Test, SanitizeInjectedOAuthStatusHeadersOnPassThroughPath) { + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Path.get(), "/anypath"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Options}, + // Malicious client trying to inject OAuth status headers on a pass-through request + {"x-envoy-oauth-status", "injected-success"}, + {"x-envoy-oauth-failure-reason", "injected-reason"}, + }; - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - // Verify the injected headers were sanitized even though the request was passed through - EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthStatus).empty()); - EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); + // Verify the injected headers were sanitized even though the request was passed through + EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthStatus).empty()); + EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_passthrough").value(), 1); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); - } + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_passthrough").value(), 1); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); +} - // Verify cookie prefixes "__Secure-" and "__Host-" cause addition of the "Secure" attribute at - // signout. - TEST_F(OAuth2Test, SecureAttributeAddedForSecureCookiePrefixesOnSignout) { - auto make_config = [&](absl::string_view prefix) - -> envoy::extensions::filters::http::oauth2::v3::OAuth2Config { - envoy::extensions::filters::http::oauth2::v3::OAuth2Config p; - auto* endpoint = p.mutable_token_endpoint(); - endpoint->set_cluster("auth.example.com"); - endpoint->set_uri("auth.example.com/_oauth"); - p.set_authorization_endpoint("https://auth2.example.com/oauth/authorize/"); - p.mutable_signout_path()->mutable_path()->set_exact("/_signout"); - p.mutable_redirect_path_matcher()->mutable_path()->set_exact(TEST_CALLBACK); - auto* credentials = p.mutable_credentials(); - credentials->set_client_id(TEST_CLIENT_ID); - credentials->mutable_token_secret()->set_name("secret"); - credentials->mutable_hmac_secret()->set_name("hmac"); - auto* cookie_names = credentials->mutable_cookie_names(); - cookie_names->set_oauth_hmac(absl::StrCat(prefix, "OauthHMAC")); - cookie_names->set_bearer_token(absl::StrCat(prefix, "BearerToken")); - cookie_names->set_id_token(absl::StrCat(prefix, "IdToken")); - cookie_names->set_refresh_token(absl::StrCat(prefix, "RefreshToken")); - cookie_names->set_oauth_nonce(absl::StrCat(prefix, "OauthNonce")); - cookie_names->set_code_verifier(absl::StrCat(prefix, "CodeVerifier")); - - auto* matcher = p.add_pass_through_matcher(); - matcher->set_name(":method"); - matcher->mutable_string_match()->set_exact("OPTIONS"); - - return p; - }; +// Verify cookie prefixes "__Secure-" and "__Host-" cause addition of the "Secure" attribute at +// signout. +TEST_F(OAuth2Test, SecureAttributeAddedForSecureCookiePrefixesOnSignout) { + auto make_config = + [&](absl::string_view prefix) -> envoy::extensions::filters::http::oauth2::v3::OAuth2Config { + envoy::extensions::filters::http::oauth2::v3::OAuth2Config p; + auto* endpoint = p.mutable_token_endpoint(); + endpoint->set_cluster("auth.example.com"); + endpoint->set_uri("auth.example.com/_oauth"); + p.set_authorization_endpoint("https://auth2.example.com/oauth/authorize/"); + p.mutable_signout_path()->mutable_path()->set_exact("/_signout"); + p.mutable_redirect_path_matcher()->mutable_path()->set_exact(TEST_CALLBACK); + auto* credentials = p.mutable_credentials(); + credentials->set_client_id(TEST_CLIENT_ID); + credentials->mutable_token_secret()->set_name("secret"); + credentials->mutable_hmac_secret()->set_name("hmac"); + auto* cookie_names = credentials->mutable_cookie_names(); + cookie_names->set_oauth_hmac(absl::StrCat(prefix, "OauthHMAC")); + cookie_names->set_bearer_token(absl::StrCat(prefix, "BearerToken")); + cookie_names->set_id_token(absl::StrCat(prefix, "IdToken")); + cookie_names->set_refresh_token(absl::StrCat(prefix, "RefreshToken")); + cookie_names->set_oauth_nonce(absl::StrCat(prefix, "OauthNonce")); + cookie_names->set_code_verifier(absl::StrCat(prefix, "CodeVerifier")); - auto run_test_with_prefix = [&](absl::string_view prefix, bool expect_secure) { - auto p = make_config(prefix); - auto secret_reader = std::make_shared(); - init(std::make_shared(p, factory_context_.server_factory_context_, - secret_reader, scope_, "test.")); - - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) - .WillOnce(Invoke([&](Http::ResponseHeaderMap& passed_headers, bool) { - EXPECT_EQ(passed_headers.get(Http::Headers::get().SetCookie).size(), 4); - const auto& cookie_str = - passed_headers.get(Http::Headers::get().SetCookie)[0]->value().getStringView(); - if (expect_secure) { - EXPECT_THAT(cookie_str, testing::HasSubstr("; Secure")); - } else { - EXPECT_THAT(cookie_str, testing::Not(testing::HasSubstr("; Secure"))); - } - })); - auto request_headers = Http::TestRequestHeaderMapImpl{ - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Path.get(), "/_signout"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - }; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(request_headers, false)); - }; + auto* matcher = p.add_pass_through_matcher(); + matcher->set_name(":method"); + matcher->mutable_string_match()->set_exact("OPTIONS"); - run_test_with_prefix("__Secure-", true); - run_test_with_prefix("__Host-", true); - run_test_with_prefix("", false); - } + return p; + }; + + auto run_test_with_prefix = [&](absl::string_view prefix, bool expect_secure) { + auto p = make_config(prefix); + auto secret_reader = std::make_shared(); + init(std::make_shared(p, factory_context_.server_factory_context_, secret_reader, + scope_, "test.")); - // Test that custom cookie paths work correctly. - TEST_F(OAuth2Test, OAuthTestCustomCookiePaths) { - // Initialize with different paths: CSRF cookies on /auth/callback, session cookies on /app. - init(getConfig(true, true, - ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: - OAuth2Config_AuthType_URL_ENCODED_BODY, - 0, false, false, false, false, false, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - 0, 0, false, - "/app", // bearer_token_path - "/app", // hmac_path - "/app", // expires_path - "/app", // id_token_path - "/app", // refresh_token_path - "/auth/callback", // nonce_path (CSRF cookie) - "/auth/callback" // code_verifier_path - )); - - test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); - - // Helper lambda to extract Set-Cookie headers from response. - auto extract_cookies = [](const Http::TestResponseHeaderMapImpl& headers) { - std::vector cookies; - headers.iterate([&cookies](const Http::HeaderEntry& header) -> Http::HeaderMap::Iterate { - if (header.key().getStringView() == Http::Headers::get().SetCookie.get()) { - cookies.push_back(std::string(header.value().getStringView())); - } - return Http::HeaderMap::Iterate::Continue; - }); - return cookies; + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) + .WillOnce(Invoke([&](Http::ResponseHeaderMap& passed_headers, bool) { + EXPECT_EQ(passed_headers.get(Http::Headers::get().SetCookie).size(), 4); + const auto& cookie_str = + passed_headers.get(Http::Headers::get().SetCookie)[0]->value().getStringView(); + if (expect_secure) { + EXPECT_THAT(cookie_str, testing::HasSubstr("; Secure")); + } else { + EXPECT_THAT(cookie_str, testing::Not(testing::HasSubstr("; Secure"))); + } + })); + auto request_headers = Http::TestRequestHeaderMapImpl{ + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Path.get(), "/_signout"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, }; + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); + }; - // Phase 1: Test redirect phase. - // We make sure that nonce and code_verifier cookies should have /auth/callback path. - { - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/test"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - }; - - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(false)); - - Http::TestResponseHeaderMapImpl response_headers; - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) - .WillOnce(Invoke([&response_headers](Http::ResponseHeaderMap& headers, bool) { - response_headers = Http::TestResponseHeaderMapImpl(headers); - })); - - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(request_headers, false)); - - auto cookies = extract_cookies(response_headers); - bool found_nonce = false, found_code_verifier = false; - for (const auto& cookie : cookies) { - if (cookie.find("OauthNonce.00000000075bcd15=") != std::string::npos) { - EXPECT_NE(cookie.find(";path=/auth/callback;"), std::string::npos) - << "OauthNonce should have path=/auth/callback, got: " << cookie; - found_nonce = true; - } - if (cookie.find("CodeVerifier.00000000075bcd15=") != std::string::npos) { - EXPECT_NE(cookie.find(";path=/auth/callback;"), std::string::npos) - << "CodeVerifier should have path=/auth/callback, got: " << cookie; - found_code_verifier = true; - } - } - EXPECT_TRUE(found_nonce) << "OauthNonce cookie not found."; - EXPECT_TRUE(found_code_verifier) << "CodeVerifier cookie not found."; - } + run_test_with_prefix("__Secure-", true); + run_test_with_prefix("__Host-", true); + run_test_with_prefix("", false); +} - // Phase 2: Test token callback. Session cookies should have /app path. - // Reinitialize filter for the callback phase. - init(getConfig(true, true, - ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: - OAuth2Config_AuthType_URL_ENCODED_BODY, - 0, false, false, false, false, false, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - 0, 0, false, "/app", "/app", "/app", "/app", "/app", "/auth/callback", - "/auth/callback")); - { - Http::TestRequestHeaderMapImpl callback_headers{ - {Http::Headers::get().Path.get(), "/_oauth?code=auth_code&state=" + TEST_ENCODED_STATE}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, - {Http::Headers::get().Cookie.get(), - "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, - }; - - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*oauth_client_, asyncGetAccessToken(_, _, _, _, _, _)); - - EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, - filter_->decodeHeaders(callback_headers, false)); - - Http::TestResponseHeaderMapImpl response_headers; - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) - .WillOnce(Invoke([&response_headers](Http::ResponseHeaderMap& headers, bool) { - response_headers = Http::TestResponseHeaderMapImpl(headers); - })); - - filter_->onGetAccessTokenSuccess("access_token", "id_token", "refresh_token", - std::chrono::seconds(10)); - - auto cookies = extract_cookies(response_headers); - bool found_hmac = false, found_bearer = false, found_expires = false; - for (const auto& cookie : cookies) { - if (cookie.find("OauthHMAC=") != std::string::npos) { - EXPECT_NE(cookie.find(";path=/app;"), std::string::npos) - << "OauthHMAC should have path=/app, got: " << cookie; - found_hmac = true; - } - if (cookie.find("BearerToken=") != std::string::npos) { - EXPECT_NE(cookie.find(";path=/app;"), std::string::npos) - << "BearerToken should have path=/app, got: " << cookie; - found_bearer = true; - } - if (cookie.find("OauthExpires=") != std::string::npos) { - EXPECT_NE(cookie.find(";path=/app;"), std::string::npos) - << "OauthExpires should have path=/app, got: " << cookie; - found_expires = true; - } - } - EXPECT_TRUE(found_hmac) << "OauthHMAC cookie not found."; - EXPECT_TRUE(found_bearer) << "BearerToken cookie not found."; - EXPECT_TRUE(found_expires) << "OauthExpires cookie not found."; - } +// Test that custom cookie paths work correctly. +TEST_F(OAuth2Test, OAuthTestCustomCookiePaths) { + // Initialize with different paths: CSRF cookies on /auth/callback, session cookies on /app. + init(getConfig(true, true, + ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: + OAuth2Config_AuthType_URL_ENCODED_BODY, + 0, false, false, false, false, false, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + 0, 0, false, + "/app", // bearer_token_path + "/app", // hmac_path + "/app", // expires_path + "/app", // id_token_path + "/app", // refresh_token_path + "/auth/callback", // nonce_path (CSRF cookie) + "/auth/callback" // code_verifier_path + )); - // Phase 3: Test signout. Cookie deletion should use the configured paths. - // Reinitialize filter for the signout phase. - init(getConfig(true, true, - ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: - OAuth2Config_AuthType_URL_ENCODED_BODY, - 0, false, false, false, false, false, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: - CookieConfig_SameSite_DISABLED, - 0, 0, false, "/app", "/app", "/app", "/app", "/app", "/auth/callback", - "/auth/callback")); - { - Http::TestRequestHeaderMapImpl signout_headers{ - {Http::Headers::get().Path.get(), "/_signout"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=csrf_token"}, - {Http::Headers::get().Cookie.get(), "CodeVerifier.00000000075bcd15=code_verifier"}, - }; - - Http::TestResponseHeaderMapImpl response_headers; - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) - .WillOnce(Invoke([&response_headers](Http::ResponseHeaderMap& headers, bool) { - response_headers = Http::TestResponseHeaderMapImpl(headers); - })); - - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(signout_headers, false)); - - auto cookies = extract_cookies(response_headers); - bool found_hmac_delete = false, found_nonce_delete = false, - found_code_verifier_delete = false; - for (const auto& cookie : cookies) { - if (cookie.find("OauthHMAC=deleted") != std::string::npos) { - EXPECT_NE(cookie.find("path=/app"), std::string::npos) - << "OauthHMAC deletion should have path=/app, got: " << cookie; - found_hmac_delete = true; - } - if (cookie.find("OauthNonce.00000000075bcd15=deleted") != std::string::npos) { - EXPECT_NE(cookie.find("path=/auth/callback"), std::string::npos) - << "OauthNonce deletion should have path=/auth/callback, got: " << cookie; - found_nonce_delete = true; - } - if (cookie.find("CodeVerifier.00000000075bcd15=deleted") != std::string::npos) { - EXPECT_NE(cookie.find("path=/auth/callback"), std::string::npos) - << "CodeVerifier deletion should have path=/auth/callback, got: " << cookie; - found_code_verifier_delete = true; - } - } - EXPECT_TRUE(found_hmac_delete) << "OauthHMAC deletion cookie not found."; - EXPECT_TRUE(found_nonce_delete) << "OauthNonce deletion cookie not found."; - EXPECT_TRUE(found_code_verifier_delete) << "CodeVerifier deletion cookie not found."; - } - } + test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); - /** - * Scenario: Request to /allowfailed path with no OAuth cookies at all. - * - * Expected behavior: Since there are no cookies and the path matches allow_failed_matcher, - * the filter should not redirect to OAuth server and should continue to upstream as - * unauthenticated. - */ - TEST_F(OAuth2Test, AllowFailedWithNoCookies) { - init(getConfig()); + // Helper lambda to extract Set-Cookie headers from response. + auto extract_cookies = [](const Http::TestResponseHeaderMapImpl& headers) { + std::vector cookies; + headers.iterate([&cookies](const Http::HeaderEntry& header) -> Http::HeaderMap::Iterate { + if (header.key().getStringView() == Http::Headers::get().SetCookie.get()) { + cookies.push_back(std::string(header.value().getStringView())); + } + return Http::HeaderMap::Iterate::Continue; + }); + return cookies; + }; + // Phase 1: Test redirect phase. + // We make sure that nonce and code_verifier cookies should have /auth/callback path. + { Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/allowfailed/api"}, + {Http::Headers::get().Path.get(), "/test"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Scheme.get(), "https"}, @@ -4965,45 +4776,63 @@ TEST_F(OAuth2Test, EmptyRouteSpecificConfigOverridesGlobalConfig) { EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(false)); - // Should NOT redirect or send local reply - EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).Times(0); - - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + Http::TestResponseHeaderMapImpl response_headers; + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) + .WillOnce(Invoke([&response_headers](Http::ResponseHeaderMap& headers, bool) { + response_headers = Http::TestResponseHeaderMapImpl(headers); + })); - // Verify the headers were added - EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), - "failed"); - EXPECT_FALSE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); + auto cookies = extract_cookies(response_headers); + bool found_nonce = false, found_code_verifier = false; + for (const auto& cookie : cookies) { + if (cookie.find("OauthNonce.00000000075bcd15=") != std::string::npos) { + EXPECT_NE(cookie.find(";path=/auth/callback;"), std::string::npos) + << "OauthNonce should have path=/auth/callback, got: " << cookie; + found_nonce = true; + } + if (cookie.find("CodeVerifier.00000000075bcd15=") != std::string::npos) { + EXPECT_NE(cookie.find(";path=/auth/callback;"), std::string::npos) + << "CodeVerifier should have path=/auth/callback, got: " << cookie; + found_code_verifier = true; + } + } + EXPECT_TRUE(found_nonce) << "OauthNonce cookie not found."; + EXPECT_TRUE(found_code_verifier) << "CodeVerifier cookie not found."; } - /** - * Scenario: Request to /allowfailed path with invalid refresh token. Token endpoint is available - * and async refresh is attempted, but the token endpoint rejects the refresh token. - * - * Expected behavior: After the async refresh failure callback, since the path matches - * allow_failed_matcher, the filter should continue to upstream as unauthenticated. - */ - TEST_F(OAuth2Test, AllowFailedWithInvalidRefreshTokenAsyncFailure) { - init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); - - test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); - - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/allowfailed/api"}, + // Phase 2: Test token callback. Session cookies should have /app path. + // Reinitialize filter for the callback phase. + init(getConfig(true, true, + ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: + OAuth2Config_AuthType_URL_ENCODED_BODY, + 0, false, false, false, false, false, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + 0, 0, false, "/app", "/app", "/app", "/app", "/app", "/auth/callback", + "/auth/callback")); + { + Http::TestRequestHeaderMapImpl callback_headers{ + {Http::Headers::get().Path.get(), "/_oauth?code=auth_code&state=" + TEST_ENCODED_STATE}, {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, - {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, - {Http::Headers::get().Cookie.get(), - "OauthHMAC=" - "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" - "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, - {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, + {Http::Headers::get().Scheme.get(), "https"}, {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, {Http::Headers::get().Cookie.get(), "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, @@ -5011,449 +4840,615 @@ TEST_F(OAuth2Test, EmptyRouteSpecificConfigOverridesGlobalConfig) { EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); + EXPECT_CALL(*oauth_client_, asyncGetAccessToken(_, _, _, _, _, _)); - std::string legit_refresh_token{"legit_refresh_token"}; - EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, + filter_->decodeHeaders(callback_headers, false)); + + Http::TestResponseHeaderMapImpl response_headers; + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) + .WillOnce(Invoke([&response_headers](Http::ResponseHeaderMap& headers, bool) { + response_headers = Http::TestResponseHeaderMapImpl(headers); + })); + + filter_->onGetAccessTokenSuccess("access_token", "id_token", "refresh_token", + std::chrono::seconds(10)); + + auto cookies = extract_cookies(response_headers); + bool found_hmac = false, found_bearer = false, found_expires = false; + for (const auto& cookie : cookies) { + if (cookie.find("OauthHMAC=") != std::string::npos) { + EXPECT_NE(cookie.find(";path=/app;"), std::string::npos) + << "OauthHMAC should have path=/app, got: " << cookie; + found_hmac = true; + } + if (cookie.find("BearerToken=") != std::string::npos) { + EXPECT_NE(cookie.find(";path=/app;"), std::string::npos) + << "BearerToken should have path=/app, got: " << cookie; + found_bearer = true; + } + if (cookie.find("OauthExpires=") != std::string::npos) { + EXPECT_NE(cookie.find(";path=/app;"), std::string::npos) + << "OauthExpires should have path=/app, got: " << cookie; + found_expires = true; + } + } + EXPECT_TRUE(found_hmac) << "OauthHMAC cookie not found."; + EXPECT_TRUE(found_bearer) << "BearerToken cookie not found."; + EXPECT_TRUE(found_expires) << "OauthExpires cookie not found."; + } - // Mock asyncRefreshAccessToken to succeed (async work starts) - EXPECT_CALL(*oauth_client_, - asyncRefreshAccessToken("legit_refresh_token", TEST_CLIENT_ID, - "asdf_client_secret_fdsa", AuthType::UrlEncodedBody)); + // Phase 3: Test signout. Cookie deletion should use the configured paths. + // Reinitialize filter for the signout phase. + init(getConfig(true, true, + ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: + OAuth2Config_AuthType_URL_ENCODED_BODY, + 0, false, false, false, false, false, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + ::envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite:: + CookieConfig_SameSite_DISABLED, + 0, 0, false, "/app", "/app", "/app", "/app", "/app", "/auth/callback", + "/auth/callback")); + { + Http::TestRequestHeaderMapImpl signout_headers{ + {Http::Headers::get().Path.get(), "/_signout"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=csrf_token"}, + {Http::Headers::get().Cookie.get(), "CodeVerifier.00000000075bcd15=code_verifier"}, + }; - // Filter should stop and wait for async response - EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, - filter_->decodeHeaders(request_headers, false)); + Http::TestResponseHeaderMapImpl response_headers; + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)) + .WillOnce(Invoke([&response_headers](Http::ResponseHeaderMap& headers, bool) { + response_headers = Http::TestResponseHeaderMapImpl(headers); + })); - // Now simulate the async failure callback from token endpoint. - // onRefreshAccessTokenFailure() returns Continue; the caller (handleRefreshTokenFailure in - // oauth_client) is responsible for calling continueDecoding() when is_request_dispatched=true. - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->onRefreshAccessTokenFailure()); - - // Verify the headers were added - EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), - "failed"); - EXPECT_FALSE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); - - // Verify OAuth token and flow cookies were removed - auto cookies = Http::Utility::parseCookies(request_headers); - EXPECT_TRUE(cookies.find("BearerToken") == cookies.end()); - EXPECT_TRUE(cookies.find("OauthHMAC") == cookies.end()); - EXPECT_TRUE(cookies.find("OauthExpires") == cookies.end()); - EXPECT_TRUE(cookies.find("RefreshToken") == cookies.end()); - EXPECT_TRUE(cookies.find("OauthNonce.00000000075bcd15") == cookies.end()); - EXPECT_TRUE(cookies.find("CodeVerifier.00000000075bcd15") == cookies.end()); - - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_refreshtoken_failure").value(), 1); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(signout_headers, false)); + + auto cookies = extract_cookies(response_headers); + bool found_hmac_delete = false, found_nonce_delete = false, found_code_verifier_delete = false; + for (const auto& cookie : cookies) { + if (cookie.find("OauthHMAC=deleted") != std::string::npos) { + EXPECT_NE(cookie.find("path=/app"), std::string::npos) + << "OauthHMAC deletion should have path=/app, got: " << cookie; + found_hmac_delete = true; + } + if (cookie.find("OauthNonce.00000000075bcd15=deleted") != std::string::npos) { + EXPECT_NE(cookie.find("path=/auth/callback"), std::string::npos) + << "OauthNonce deletion should have path=/auth/callback, got: " << cookie; + found_nonce_delete = true; + } + if (cookie.find("CodeVerifier.00000000075bcd15=deleted") != std::string::npos) { + EXPECT_NE(cookie.find("path=/auth/callback"), std::string::npos) + << "CodeVerifier deletion should have path=/auth/callback, got: " << cookie; + found_code_verifier_delete = true; + } + } + EXPECT_TRUE(found_hmac_delete) << "OauthHMAC deletion cookie not found."; + EXPECT_TRUE(found_nonce_delete) << "OauthNonce deletion cookie not found."; + EXPECT_TRUE(found_code_verifier_delete) << "CodeVerifier deletion cookie not found."; } +} - /** - * Scenario: Request to /allowfailed path with refresh token cookie, but asyncRefreshAccessToken - * fails synchronously (e.g. cluster not found). - * - * Expected behavior: Since asyncRefreshAccessToken fails synchronously (FailureContinue state) - * and the path matches allow_failed_matcher, decodeHeaders returns Continue immediately. - */ - TEST_F(OAuth2Test, AllowFailedWithRefreshTokenSyncFailure) { - init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); +/** + * Scenario: Request to /allowfailed path with no OAuth cookies at all. + * + * Expected behavior: Since there are no cookies and the path matches allow_failed_matcher, + * the filter should not redirect to OAuth server and should continue to upstream as + * unauthenticated. + */ +TEST_F(OAuth2Test, AllowFailedWithNoCookies) { + init(getConfig()); - test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/allowfailed/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + }; - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/allowfailed/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, - {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, - {Http::Headers::get().Cookie.get(), - "OauthHMAC=" - "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" - "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, - {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, - {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, - {Http::Headers::get().Cookie.get(), - "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, - }; + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); + // Should NOT redirect or send local reply + EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).Times(0); - std::string legit_refresh_token{"legit_refresh_token"}; - EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - // asyncRefreshAccessToken fails synchronously; getState() returns FailureContinue - EXPECT_CALL(*oauth_client_, - asyncRefreshAccessToken("legit_refresh_token", TEST_CLIENT_ID, - "asdf_client_secret_fdsa", AuthType::UrlEncodedBody)); - EXPECT_CALL(*oauth_client_, getState()) - .WillOnce(Return(OAuth2Client::OAuthState::FailureContinue)); + // Verify the headers were added + EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), + "failed"); + EXPECT_FALSE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); - // Filter should continue immediately (not wait for async) - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - } + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); +} - /** - * Scenario: OAuth callback where asyncGetAccessToken fails synchronously (e.g. cluster not - * found). - * - * Expected behavior: Since asyncGetAccessToken fails synchronously (FailureStop state), - * decodeHeaders returns StopIteration. The real error handling (e.g. sending 401) is done - * inside asyncGetAccessToken before returning. - */ - TEST_F(OAuth2Test, OAuthCallbackGetAccessTokenSyncFailure) { - init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); +/** + * Scenario: Request to /allowfailed path with invalid refresh token. Token endpoint is available + * and async refresh is attempted, but the token endpoint rejects the refresh token. + * + * Expected behavior: After the async refresh failure callback, since the path matches + * allow_failed_matcher, the filter should continue to upstream as unauthenticated. + */ +TEST_F(OAuth2Test, AllowFailedWithInvalidRefreshTokenAsyncFailure) { + init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, - {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, - {Http::Headers::get().Cookie.get(), - "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Scheme.get(), "https"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - }; + test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/allowfailed/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, + {Http::Headers::get().Cookie.get(), + "OauthHMAC=" + "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" + "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, + {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, + {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, + {Http::Headers::get().Cookie.get(), + "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, + }; - // asyncGetAccessToken fails synchronously; getState() returns FailureStop - EXPECT_CALL(*oauth_client_, - asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", - "https://traffic.example.com" + TEST_CALLBACK, - TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); - EXPECT_CALL(*oauth_client_, getState()).WillOnce(Return(OAuth2Client::OAuthState::FailureStop)); + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(request_headers, false)); - } + std::string legit_refresh_token{"legit_refresh_token"}; + EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); - /** - * Scenario: AJAX request (matching deny_redirect_matcher) to /allowfailed path without valid - * OAuth. - * - * Expected behavior: allow_failed_matcher takes precedence - request should continue as - * unauthenticated rather than returning 401 (deny_redirect would normally return 401). - */ - TEST_F(OAuth2Test, AllowFailedMatcherTakesPrecedenceOverDenyRedirect) { - init(getConfig()); + // Mock asyncRefreshAccessToken to succeed (async work starts) + EXPECT_CALL(*oauth_client_, + asyncRefreshAccessToken("legit_refresh_token", TEST_CLIENT_ID, + "asdf_client_secret_fdsa", AuthType::UrlEncodedBody)); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/allowfailed/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, - {Http::Headers::get().Scheme.get(), "https"}, - {"X-Requested-With", "XMLHttpRequest"}, // Matches deny_redirect_matcher - }; + // Filter should stop and wait for async response + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, + filter_->decodeHeaders(request_headers, false)); - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + // Now simulate the async failure callback from token endpoint. + // onRefreshAccessTokenFailure() returns Continue; the caller (handleRefreshTokenFailure in + // oauth_client) is responsible for calling continueDecoding() when is_request_dispatched=true. + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->onRefreshAccessTokenFailure()); - // Should NOT return 401, should continue - EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); + // Verify the headers were added + EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), + "failed"); + EXPECT_FALSE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + // Verify OAuth token and flow cookies were removed + auto cookies = Http::Utility::parseCookies(request_headers); + EXPECT_TRUE(cookies.find("BearerToken") == cookies.end()); + EXPECT_TRUE(cookies.find("OauthHMAC") == cookies.end()); + EXPECT_TRUE(cookies.find("OauthExpires") == cookies.end()); + EXPECT_TRUE(cookies.find("RefreshToken") == cookies.end()); + EXPECT_TRUE(cookies.find("OauthNonce.00000000075bcd15") == cookies.end()); + EXPECT_TRUE(cookies.find("CodeVerifier.00000000075bcd15") == cookies.end()); + + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_refreshtoken_failure").value(), 1); +} - EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), - "failed"); +/** + * Scenario: Request to /allowfailed path with refresh token cookie, but asyncRefreshAccessToken + * fails synchronously (e.g. cluster not found). + * + * Expected behavior: Since asyncRefreshAccessToken fails synchronously (FailureContinue state) + * and the path matches allow_failed_matcher, decodeHeaders returns Continue immediately. + */ +TEST_F(OAuth2Test, AllowFailedWithRefreshTokenSyncFailure) { + init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); - } + test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); - /** - * Scenario: Request with injected OAuth status headers to test sanitization. - * A malicious client sends x-envoy-oauth-status and x-envoy-oauth-failure-reason headers - * in their request. - * - * Expected behavior: The filter should sanitize these headers to prevent header injection - * attacks, similar to how it sanitizes the Authorization header. - */ - TEST_F(OAuth2Test, SanitizeInjectedOAuthStatusHeaders) { - init(getConfig()); + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/allowfailed/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, + {Http::Headers::get().Cookie.get(), + "OauthHMAC=" + "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" + "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, + {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, + {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, + {Http::Headers::get().Cookie.get(), + "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, + }; - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/anypath"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - // Malicious client trying to inject OAuth status headers - {"x-envoy-oauth-status", "injected-success"}, - {"x-envoy-oauth-failure-reason", "injected-reason"}, - }; + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); - // cookie-validation mocking - user is authenticated - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); + std::string legit_refresh_token{"legit_refresh_token"}; + EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); - std::string legit_token{"legit_token"}; - EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); + // asyncRefreshAccessToken fails synchronously; getState() returns FailureContinue + EXPECT_CALL(*oauth_client_, + asyncRefreshAccessToken("legit_refresh_token", TEST_CLIENT_ID, + "asdf_client_secret_fdsa", AuthType::UrlEncodedBody)); + EXPECT_CALL(*oauth_client_, getState()) + .WillOnce(Return(OAuth2Client::OAuthState::FailureContinue)); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + // Filter should continue immediately (not wait for async) + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); +} - // Verify the injected headers were sanitized (removed) - EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthStatus).empty()); - EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); +/** + * Scenario: OAuth callback where asyncGetAccessToken fails synchronously (e.g. cluster not + * found). + * + * Expected behavior: Since asyncGetAccessToken fails synchronously (FailureStop state), + * decodeHeaders returns StopIteration. The real error handling (e.g. sending 401) is done + * inside asyncGetAccessToken before returning. + */ +TEST_F(OAuth2Test, OAuthCallbackGetAccessTokenSyncFailure) { + init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_success").value(), 1); - } + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, + {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, + {Http::Headers::get().Cookie.get(), + "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Scheme.get(), "https"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + }; - /** - * Scenario: Request with injected OAuth status headers on allow_failed path. - * Tests that sanitization occurs before the filter sets its own legitimate headers. - * - * Expected behavior: The injected headers should be removed, and only the filter's - * legitimate headers should be present in the final request. - */ - TEST_F(OAuth2Test, SanitizeInjectedOAuthStatusHeadersOnAllowFailedPath) { - init(getConfig()); + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/allowfailed/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - // Malicious client trying to inject fake success status - {"x-envoy-oauth-status", "injected-success"}, - {"x-envoy-oauth-failure-reason", "injected-fake-reason"}, - }; + // asyncGetAccessToken fails synchronously; getState() returns FailureStop + EXPECT_CALL(*oauth_client_, asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", + "https://traffic.example.com" + TEST_CALLBACK, + TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); + EXPECT_CALL(*oauth_client_, getState()).WillOnce(Return(OAuth2Client::OAuthState::FailureStop)); - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); +} + +/** + * Scenario: AJAX request (matching deny_redirect_matcher) to /allowfailed path without valid + * OAuth. + * + * Expected behavior: allow_failed_matcher takes precedence - request should continue as + * unauthenticated rather than returning 401 (deny_redirect would normally return 401). + */ +TEST_F(OAuth2Test, AllowFailedMatcherTakesPrecedenceOverDenyRedirect) { + init(getConfig()); + + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/allowfailed/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, + {Http::Headers::get().Scheme.get(), "https"}, + {"X-Requested-With", "XMLHttpRequest"}, // Matches deny_redirect_matcher + }; - EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).Times(0); + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + // Should NOT return 401, should continue + EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); - // Verify only the legitimate "failed" status is present (not the injected "success") - auto status_headers = request_headers.get(OAuth2Headers::get().OAuthStatus); - EXPECT_EQ(status_headers.size(), 1); - EXPECT_EQ(status_headers[0]->value().getStringView(), "failed"); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - auto reason_headers = request_headers.get(OAuth2Headers::get().OAuthFailureReason); - EXPECT_EQ(reason_headers.size(), 1); - // Verify it's not the injected value - EXPECT_NE(reason_headers[0]->value().getStringView(), "injected-fake-reason"); + EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), + "failed"); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); - } + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); +} - /** - * Scenario: handleOAuthFailure is called on an allow-failed path. - * - * This tests the OAuth2Filter::handleOAuthFailure code path (lines 1467-1471), which is - * only reachable via OAuth2ClientImpl callbacks (asyncGetAccessToken/asyncRefreshAccessToken - * failures). Since the client is mocked in filter tests, this function must be called directly - * after decodeHeaders has set request_headers_. - * - * Expected behavior: shouldAllowFailed returns true, continueWithFailedOAuth is called, - * filter returns Continue. - */ - TEST_F(OAuth2Test, HandleOAuthFailureOnAllowedPath) { - init(getConfig()); +/** + * Scenario: Request with injected OAuth status headers to test sanitization. + * A malicious client sends x-envoy-oauth-status and x-envoy-oauth-failure-reason headers + * in their request. + * + * Expected behavior: The filter should sanitize these headers to prevent header injection + * attacks, similar to how it sanitizes the Authorization header. + */ +TEST_F(OAuth2Test, SanitizeInjectedOAuthStatusHeaders) { + init(getConfig()); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/allowfailed/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - }; + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/anypath"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + // Malicious client trying to inject OAuth status headers + {"x-envoy-oauth-status", "injected-success"}, + {"x-envoy-oauth-failure-reason", "injected-reason"}, + }; - // Use a valid OAuth flow to set request_headers_ without triggering allow-failed behavior. - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); - std::string legit_token{"legit_token"}; - EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - - // Now call handleOAuthFailure directly (the path exercised by real OAuth client callbacks). - // Path matches allow_failed_matcher → should continue as unauthenticated. - EXPECT_EQ(Http::FilterHeadersStatus::Continue, - filter_->handleOAuthFailure("Token exchange failed")); - - EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), - "failed"); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); - } + // cookie-validation mocking - user is authenticated + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); - /** - * Scenario: handleOAuthFailure is called on a regular path (not matching allow_failed_matcher), - * with extra_details provided. - * - * This tests the else branch of OAuth2Filter::handleOAuthFailure (lines 1472-1477) and the - * non-empty extra_details branch in the details formatting (line 1474). - * - * Expected behavior: shouldAllowFailed returns false, sendUnauthorizedResponse is called with - * "reason: extra_details" formatting, filter returns StopIteration. - */ - TEST_F(OAuth2Test, HandleOAuthFailureOnNonAllowedPathWithExtraDetails) { - init(getConfig()); + std::string legit_token{"legit_token"}; + EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/regular/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - }; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); - std::string legit_token{"legit_token"}; - EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + // Verify the injected headers were sanitized (removed) + EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthStatus).empty()); + EXPECT_TRUE(request_headers.get(OAuth2Headers::get().OAuthFailureReason).empty()); - // Path does not match allow_failed_matcher → should send 401 and stop. - EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Unauthorized, _, _, _, _)); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->handleOAuthFailure("Token exchange failed", "response code: 500")); - } + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_failure").value(), 0); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_success").value(), 1); +} - /** - * Scenario: shouldAllowFailed is called for the OAuth callback path (/_oauth) when the - * allow_failed_matcher is configured to match all paths (prefix "/"). - * - * This tests the redirectPathMatcher guard in shouldAllowFailed (lines 1430-1434): even when - * allow_failed_matcher matches the callback URL, shouldAllowFailed must return false because - * the callback path is "hosted" by Envoy and must never reach upstream as unauthenticated. - * - * Expected behavior: handleOAuthFailure sends 401 rather than continuing as unauthenticated. - */ - TEST_F(OAuth2Test, AllowFailedBlockedForCallbackPath) { - // Create a config where allow_failed_matcher matches all paths including /_oauth. - envoy::extensions::filters::http::oauth2::v3::OAuth2Config p; - auto* endpoint = p.mutable_token_endpoint(); - endpoint->set_cluster("auth.example.com"); - endpoint->set_uri("auth.example.com/_oauth"); - endpoint->mutable_timeout()->set_seconds(1); - p.set_redirect_uri("%REQ(:scheme)%://%REQ(:authority)%" + TEST_CALLBACK); - p.mutable_redirect_path_matcher()->mutable_path()->set_exact(TEST_CALLBACK); - p.set_authorization_endpoint("https://auth.example.com/oauth/authorize/"); - p.mutable_signout_path()->mutable_path()->set_exact("/_signout"); - p.set_forward_bearer_token(true); - p.set_stat_prefix("my_prefix"); - p.add_auth_scopes("user"); - auto* pass_through = p.add_pass_through_matcher(); - pass_through->set_name(":method"); - pass_through->mutable_string_match()->set_exact("OPTIONS"); - // Configure allow_failed_matcher to match all paths, including the callback path /_oauth. - auto* allow_failed_matcher = p.add_allow_failed_matcher(); - allow_failed_matcher->set_name(":path"); - allow_failed_matcher->mutable_string_match()->set_prefix("/"); - auto* credentials = p.mutable_credentials(); - credentials->set_client_id(TEST_CLIENT_ID); - credentials->mutable_token_secret()->set_name("secret"); - credentials->mutable_hmac_secret()->set_name("hmac"); - MessageUtil::validate(p, ProtobufMessage::getStrictValidationVisitor()); - auto secret_reader = std::make_shared(); - init(std::make_shared(p, factory_context_.server_factory_context_, secret_reader, - scope_, "test.")); +/** + * Scenario: Request with injected OAuth status headers on allow_failed path. + * Tests that sanitization occurs before the filter sets its own legitimate headers. + * + * Expected behavior: The injected headers should be removed, and only the filter's + * legitimate headers should be present in the final request. + */ +TEST_F(OAuth2Test, SanitizeInjectedOAuthStatusHeadersOnAllowFailedPath) { + init(getConfig()); - // Make a callback request — asyncGetAccessToken is called and returns Idle (async pending). - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, - {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, - {Http::Headers::get().Cookie.get(), - "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Scheme.get(), "https"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - }; + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/allowfailed/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + // Malicious client trying to inject fake success status + {"x-envoy-oauth-status", "injected-success"}, + {"x-envoy-oauth-failure-reason", "injected-fake-reason"}, + }; - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*oauth_client_, - asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", - "https://traffic.example.com" + TEST_CALLBACK, - TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); - // getState returns Idle → decodeHeaders waits for async response. - EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, - filter_->decodeHeaders(request_headers, false)); + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - // Now simulate the real OAuth client calling back with a failure. - // Even though allow_failed_matcher matches "/", shouldAllowFailed returns false for /_oauth - // because redirectPathMatcher matches it first. - EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Unauthorized, _, _, _, _)); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->handleOAuthFailure("Token exchange failed")); + EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).Times(0); - EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 0); - } + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - /** - * Scenario: asyncRefreshAccessToken fails synchronously (FailureStop), meaning the token - * endpoint cluster was not found and shouldAllowFailed is false — a 401 was already sent - * synchronously inside asyncRefreshAccessToken. - * - * Expected behavior: decodeHeaders returns StopIteration. - */ - TEST_F(OAuth2Test, AllowFailedWithRefreshTokenSyncFailureStop) { - init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); + // Verify only the legitimate "failed" status is present (not the injected "success") + auto status_headers = request_headers.get(OAuth2Headers::get().OAuthStatus); + EXPECT_EQ(status_headers.size(), 1); + EXPECT_EQ(status_headers[0]->value().getStringView(), "failed"); - test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); + auto reason_headers = request_headers.get(OAuth2Headers::get().OAuthFailureReason); + EXPECT_EQ(reason_headers.size(), 1); + // Verify it's not the injected value + EXPECT_NE(reason_headers[0]->value().getStringView(), "injected-fake-reason"); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/regular/api"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, - {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, - {Http::Headers::get().Cookie.get(), - "OauthHMAC=" - "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" - "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, - {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, - }; + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); +} - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); - std::string legit_refresh_token{"legit_refresh_token"}; - EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); +/** + * Scenario: handleOAuthFailure is called on an allow-failed path. + * + * This tests the OAuth2Filter::handleOAuthFailure code path (lines 1467-1471), which is + * only reachable via OAuth2ClientImpl callbacks (asyncGetAccessToken/asyncRefreshAccessToken + * failures). Since the client is mocked in filter tests, this function must be called directly + * after decodeHeaders has set request_headers_. + * + * Expected behavior: shouldAllowFailed returns true, continueWithFailedOAuth is called, + * filter returns Continue. + */ +TEST_F(OAuth2Test, HandleOAuthFailureOnAllowedPath) { + init(getConfig()); - EXPECT_CALL(*oauth_client_, - asyncRefreshAccessToken("legit_refresh_token", TEST_CLIENT_ID, - "asdf_client_secret_fdsa", AuthType::UrlEncodedBody)); - // Sync failure: a 401 or redirect was already sent inside asyncRefreshAccessToken. - EXPECT_CALL(*oauth_client_, getState()).WillOnce(Return(OAuth2Client::OAuthState::FailureStop)); + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/allowfailed/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + }; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(request_headers, false)); - } + // Use a valid OAuth flow to set request_headers_ without triggering allow-failed behavior. + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); + std::string legit_token{"legit_token"}; + EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - /** - * Scenario: asyncGetAccessToken fails synchronously with FailureContinue on the OAuth callback - * path — meaning the allow-failed path was triggered synchronously inside asyncGetAccessToken. - * - * Expected behavior: decodeHeaders returns Continue. - */ - TEST_F(OAuth2Test, OAuthCallbackGetAccessTokenSyncContinue) { - init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); + // Now call handleOAuthFailure directly (the path exercised by real OAuth client callbacks). + // Path matches allow_failed_matcher → should continue as unauthenticated. + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_->handleOAuthFailure("Token exchange failed")); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, - {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, - {Http::Headers::get().Cookie.get(), - "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Scheme.get(), "https"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - }; + EXPECT_EQ(request_headers.get(OAuth2Headers::get().OAuthStatus)[0]->value().getStringView(), + "failed"); + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 1); +} - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*oauth_client_, - asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", - "https://traffic.example.com" + TEST_CALLBACK, - TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); - // Sync success on the allow-failed path: continueDecoding() was called inside - // asyncGetAccessToken. - EXPECT_CALL(*oauth_client_, getState()) - .WillOnce(Return(OAuth2Client::OAuthState::FailureContinue)); - - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - } +/** + * Scenario: handleOAuthFailure is called on a regular path (not matching allow_failed_matcher), + * with extra_details provided. + * + * This tests the else branch of OAuth2Filter::handleOAuthFailure (lines 1472-1477) and the + * non-empty extra_details branch in the details formatting (line 1474). + * + * Expected behavior: shouldAllowFailed returns false, sendUnauthorizedResponse is called with + * "reason: extra_details" formatting, filter returns StopIteration. + */ +TEST_F(OAuth2Test, HandleOAuthFailureOnNonAllowedPathWithExtraDetails) { + init(getConfig()); + + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/regular/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + }; + + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(true)); + std::string legit_token{"legit_token"}; + EXPECT_CALL(*validator_, token()).WillRepeatedly(ReturnRef(legit_token)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + + // Path does not match allow_failed_matcher → should send 401 and stop. + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Unauthorized, _, _, _, _)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->handleOAuthFailure("Token exchange failed", "response code: 500")); +} + +/** + * Scenario: shouldAllowFailed is called for the OAuth callback path (/_oauth) when the + * allow_failed_matcher is configured to match all paths (prefix "/"). + * + * This tests the redirectPathMatcher guard in shouldAllowFailed (lines 1430-1434): even when + * allow_failed_matcher matches the callback URL, shouldAllowFailed must return false because + * the callback path is "hosted" by Envoy and must never reach upstream as unauthenticated. + * + * Expected behavior: handleOAuthFailure sends 401 rather than continuing as unauthenticated. + */ +TEST_F(OAuth2Test, AllowFailedBlockedForCallbackPath) { + // Create a config where allow_failed_matcher matches all paths including /_oauth. + envoy::extensions::filters::http::oauth2::v3::OAuth2Config p; + auto* endpoint = p.mutable_token_endpoint(); + endpoint->set_cluster("auth.example.com"); + endpoint->set_uri("auth.example.com/_oauth"); + endpoint->mutable_timeout()->set_seconds(1); + p.set_redirect_uri("%REQ(:scheme)%://%REQ(:authority)%" + TEST_CALLBACK); + p.mutable_redirect_path_matcher()->mutable_path()->set_exact(TEST_CALLBACK); + p.set_authorization_endpoint("https://auth.example.com/oauth/authorize/"); + p.mutable_signout_path()->mutable_path()->set_exact("/_signout"); + p.set_forward_bearer_token(true); + p.set_stat_prefix("my_prefix"); + p.add_auth_scopes("user"); + auto* pass_through = p.add_pass_through_matcher(); + pass_through->set_name(":method"); + pass_through->mutable_string_match()->set_exact("OPTIONS"); + // Configure allow_failed_matcher to match all paths, including the callback path /_oauth. + auto* allow_failed_matcher = p.add_allow_failed_matcher(); + allow_failed_matcher->set_name(":path"); + allow_failed_matcher->mutable_string_match()->set_prefix("/"); + auto* credentials = p.mutable_credentials(); + credentials->set_client_id(TEST_CLIENT_ID); + credentials->mutable_token_secret()->set_name("secret"); + credentials->mutable_hmac_secret()->set_name("hmac"); + MessageUtil::validate(p, ProtobufMessage::getStrictValidationVisitor()); + auto secret_reader = std::make_shared(); + init(std::make_shared(p, factory_context_.server_factory_context_, secret_reader, + scope_, "test.")); + + // Make a callback request — asyncGetAccessToken is called and returns Idle (async pending). + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, + {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, + {Http::Headers::get().Cookie.get(), + "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Scheme.get(), "https"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + }; + + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*oauth_client_, asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", + "https://traffic.example.com" + TEST_CALLBACK, + TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); + // getState returns Idle → decodeHeaders waits for async response. + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, + filter_->decodeHeaders(request_headers, false)); + + // Now simulate the real OAuth client calling back with a failure. + // Even though allow_failed_matcher matches "/", shouldAllowFailed returns false for /_oauth + // because redirectPathMatcher matches it first. + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Unauthorized, _, _, _, _)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->handleOAuthFailure("Token exchange failed")); + + EXPECT_EQ(scope_.counterFromString("test.my_prefix.oauth_allow_failed_passthrough").value(), 0); +} + +/** + * Scenario: asyncRefreshAccessToken fails synchronously (FailureStop), meaning the token + * endpoint cluster was not found and shouldAllowFailed is false — a 401 was already sent + * synchronously inside asyncRefreshAccessToken. + * + * Expected behavior: decodeHeaders returns StopIteration. + */ +TEST_F(OAuth2Test, AllowFailedWithRefreshTokenSyncFailureStop) { + init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); + + test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); + + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/regular/api"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Cookie.get(), "OauthExpires=123"}, + {Http::Headers::get().Cookie.get(), "BearerToken=" + TEST_ENCRYPTED_ACCESS_TOKEN}, + {Http::Headers::get().Cookie.get(), + "OauthHMAC=" + "ZTRlMzU5N2Q4ZDIwZWE5ZTU5NTg3YTU3YTcxZTU0NDFkMzY1ZTc1NjMyODYyMj" + "RlNjMxZTJmNTZkYzRmZTM0ZQ===="}, + {Http::Headers::get().Cookie.get(), "RefreshToken=" + TEST_ENCRYPTED_REFRESH_TOKEN}, + }; + + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(true)); + std::string legit_refresh_token{"legit_refresh_token"}; + EXPECT_CALL(*validator_, refreshToken()).WillRepeatedly(ReturnRef(legit_refresh_token)); + + EXPECT_CALL(*oauth_client_, + asyncRefreshAccessToken("legit_refresh_token", TEST_CLIENT_ID, + "asdf_client_secret_fdsa", AuthType::UrlEncodedBody)); + // Sync failure: a 401 or redirect was already sent inside asyncRefreshAccessToken. + EXPECT_CALL(*oauth_client_, getState()).WillOnce(Return(OAuth2Client::OAuthState::FailureStop)); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); +} + +/** + * Scenario: asyncGetAccessToken fails synchronously with FailureContinue on the OAuth callback + * path — meaning the allow-failed path was triggered synchronously inside asyncGetAccessToken. + * + * Expected behavior: decodeHeaders returns Continue. + */ +TEST_F(OAuth2Test, OAuthCallbackGetAccessTokenSyncContinue) { + init(getConfig(false /* forward_bearer_token */, true /* use_refresh_token */)); + + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=" + TEST_ENCODED_STATE}, + {Http::Headers::get().Cookie.get(), "OauthNonce.00000000075bcd15=" + TEST_CSRF_TOKEN}, + {Http::Headers::get().Cookie.get(), + "CodeVerifier.00000000075bcd15=" + TEST_ENCRYPTED_CODE_VERIFIER}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Scheme.get(), "https"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + }; + + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*oauth_client_, asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", + "https://traffic.example.com" + TEST_CALLBACK, + TEST_CODE_VERIFIER, AuthType::UrlEncodedBody)); + // Sync success on the allow-failed path: continueDecoding() was called inside + // asyncGetAccessToken. + EXPECT_CALL(*oauth_client_, getState()) + .WillOnce(Return(OAuth2Client::OAuthState::FailureContinue)); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); +} } // namespace Oauth2 } // namespace HttpFilters From 6545c3b3bade056140514ba227ee6ab44eadd369 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 7 Apr 2026 02:38:49 +0000 Subject: [PATCH 14/29] simplify code Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 578c90873476c..97338e96a61ad 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -644,8 +644,7 @@ void OAuth2Filter::setActiveConfig(FilterConfigSharedPtr config) { return; } - if (config_ == config && validator_ != nullptr && - (oauth_client_ != nullptr || !oauth_client_factory_)) { + if (config_ == config && validator_ != nullptr && oauth_client_ != nullptr) { return; } @@ -653,14 +652,10 @@ void OAuth2Filter::setActiveConfig(FilterConfigSharedPtr config) { validator_ = std::make_shared(time_source_, config_->cookieNames(), config_->cookieDomain()); - if (oauth_client_factory_) { - oauth_client_ = oauth_client_factory_(config_); - } - if (oauth_client_ != nullptr) { - oauth_client_->setCallbacks(*this); - if (decoder_callbacks_ != nullptr) { - oauth_client_->setDecoderFilterCallbacks(*decoder_callbacks_); - } + oauth_client_ = oauth_client_factory_(config_); + oauth_client_->setCallbacks(*this); + if (decoder_callbacks_ != nullptr) { + oauth_client_->setDecoderFilterCallbacks(*decoder_callbacks_); } } From ee16c0e17503acf3695206fd3d5ed7ea29674ad3 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 7 Apr 2026 07:30:22 +0000 Subject: [PATCH 15/29] remove init manager for per route config Signed-off-by: Huabing (Robin) Zhao --- envoy/secret/secret_manager.h | 4 +- source/common/secret/secret_manager_impl.cc | 3 +- source/common/secret/secret_manager_impl.h | 2 +- .../extensions/filters/http/oauth2/config.cc | 6 +- .../extensions/filters/http/oauth2/filter.cc | 14 ++- .../extensions/filters/http/oauth2/filter.h | 5 + .../filters/http/oauth2/filter_test.cc | 98 +++++++++++++++++-- test/mocks/secret/mocks.h | 2 +- 8 files changed, 120 insertions(+), 14 deletions(-) diff --git a/envoy/secret/secret_manager.h b/envoy/secret/secret_manager.h index f90a8d7ab436b..75276e023f9fa 100644 --- a/envoy/secret/secret_manager.h +++ b/envoy/secret/secret_manager.h @@ -157,13 +157,15 @@ class SecretManager { * @param config_name a name that uniquely refers to the SDS config source. * @param secret_provider_context context that provides components for creating and initializing * secret provider. + * @param init_manager if supplied, register to the initialization sequence; otherwise, start + * immediately * @return GenericSecretConfigProviderSharedPtr the dynamic generic secret provider. */ virtual GenericSecretConfigProviderSharedPtr findOrCreateGenericSecretProvider(const envoy::config::core::v3::ConfigSource& config_source, const std::string& config_name, Server::Configuration::ServerFactoryContext& server_context, - Init::Manager& init_manager) PURE; + OptRef init_manager) PURE; }; using SecretManagerPtr = std::unique_ptr; diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index e0fd7cc309f6a..404035f326cd8 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -153,7 +153,8 @@ SecretManagerImpl::findOrCreateTlsSessionTicketKeysContextProvider( GenericSecretConfigProviderSharedPtr SecretManagerImpl::findOrCreateGenericSecretProvider( const envoy::config::core::v3::ConfigSource& sds_config_source, const std::string& config_name, - Server::Configuration::ServerFactoryContext& server_context, Init::Manager& init_manager) { + Server::Configuration::ServerFactoryContext& server_context, + OptRef init_manager) { return generic_secret_providers_.findOrCreate(sds_config_source, config_name, server_context, init_manager, true); } diff --git a/source/common/secret/secret_manager_impl.h b/source/common/secret/secret_manager_impl.h index 02ee9c6fc6f28..e30b9c2eecacb 100644 --- a/source/common/secret/secret_manager_impl.h +++ b/source/common/secret/secret_manager_impl.h @@ -71,7 +71,7 @@ class SecretManagerImpl : public SecretManager { findOrCreateGenericSecretProvider(const envoy::config::core::v3::ConfigSource& config_source, const std::string& config_name, Server::Configuration::ServerFactoryContext& server_context, - Init::Manager& init_manager) override; + OptRef init_manager) override; private: ProtobufTypes::MessagePtr dumpSecretConfigs(const Matchers::StringMatcher& name_matcher); diff --git a/source/extensions/filters/http/oauth2/config.cc b/source/extensions/filters/http/oauth2/config.cc index d5970deda4698..accab8cf222a9 100644 --- a/source/extensions/filters/http/oauth2/config.cc +++ b/source/extensions/filters/http/oauth2/config.cc @@ -27,7 +27,7 @@ namespace { Secret::GenericSecretConfigProviderSharedPtr secretsProvider(const envoy::extensions::transport_sockets::tls::v3::SdsSecretConfig& config, Server::Configuration::ServerFactoryContext& server_context, - Init::Manager& init_manager) { + OptRef init_manager) { if (config.has_sds_config()) { return server_context.secretManager().findOrCreateGenericSecretProvider( config.sds_config(), config.name(), server_context, init_manager); @@ -39,7 +39,7 @@ secretsProvider(const envoy::extensions::transport_sockets::tls::v3::SdsSecretCo absl::StatusOr createFilterConfig(const envoy::extensions::filters::http::oauth2::v3::OAuth2Config& proto_config, Server::Configuration::ServerFactoryContext& server_context, - Init::Manager& init_manager, Stats::Scope& scope, + OptRef init_manager, Stats::Scope& scope, const std::string& stats_prefix) { const auto& credentials = proto_config.credentials(); const auto auth_type = proto_config.auth_type(); @@ -126,7 +126,7 @@ OAuth2Config::createRouteSpecificFilterConfigTyped( const envoy::extensions::filters::http::oauth2::v3::OAuth2PerRoute& proto, Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor&) { auto config_or_error = - createFilterConfig(proto.config(), context, context.initManager(), context.scope(), ""); + createFilterConfig(proto.config(), context, absl::nullopt, context.scope(), ""); if (!config_or_error.ok()) { return config_or_error.status(); } diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 97338e96a61ad..2b91d8f40c8b4 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -51,7 +51,6 @@ constexpr const char* OIDCLogoutUrlFormatString = "{0}?id_token_hint={1}&client_id={2}&post_logout_redirect_uri={3}"; constexpr absl::string_view UnauthorizedBodyMessage = "OAuth flow failed."; - constexpr absl::string_view queryParamsError = "error"; constexpr absl::string_view queryParamsCode = "code"; constexpr absl::string_view queryParamsState = "state"; @@ -689,6 +688,11 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he } } + if (!config_->requiredSecretsAvailable()) { + sendSecretsNotReadyResponse("OAuth2 secrets are not ready"); + return Http::FilterHeadersStatus::StopIteration; + } + // Decrypt the OAuth tokens and update the corresponding cookies in the request headers // before forwarding the request upstream. This step must occur early to ensure that // other parts of the filter can access the decrypted tokens—for example, to calculate @@ -1464,6 +1468,14 @@ void OAuth2Filter::sendUnauthorizedResponse(const std::string& details) { absl::nullopt, details); } +void OAuth2Filter::sendSecretsNotReadyResponse(const std::string& details) { + ENVOY_STREAM_LOG(warn, "Responding with 503 Service Unavailable. Cause: {}", *decoder_callbacks_, + details); + config_->stats().oauth_failure_.inc(); + decoder_callbacks_->sendLocalReply(Http::Code::ServiceUnavailable, UnauthorizedBodyMessage, + nullptr, absl::nullopt, details); +} + bool OAuth2Filter::shouldAllowFailed(const Http::RequestHeaderMap& headers) const { // Never allow failed for OAuth callback endpoint - callback requests must always return 401 // on failure since the callback path is "hosted" by Envoy itself and shouldn't reach upstream. diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index 94e456ad57198..43ca994169a99 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -177,6 +177,10 @@ class FilterConfig : public Logger::Loggable { const Matchers::PathMatcher& signoutPath() const { return signout_path_; } std::string clientSecret() const { return secret_reader_->clientSecret(); } std::string hmacSecret() const { return secret_reader_->hmacSecret(); } + bool requiredSecretsAvailable() const { + return !secret_reader_->hmacSecret().empty() && + (auth_type_ == AuthType::TlsClientAuth || !secret_reader_->clientSecret().empty()); + } FilterStats& stats() { return stats_; } const std::string& encodedResourceQueryParams() const { return encoded_resource_query_params_; } const CookieNames& cookieNames() const { return cookie_names_; } @@ -447,6 +451,7 @@ class OAuth2Filter : public Http::PassThroughFilter, bool shouldDenyRedirect(const Http::RequestHeaderMap& headers) const; void continueWithFailedOAuth(const std::string& reason, const std::string& extra_details = ""); void sendUnauthorizedResponse(const std::string& details); + void sendSecretsNotReadyResponse(const std::string& details); }; } // namespace Oauth2 diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 615b9ab161f8f..f2f28f5b06a99 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -70,12 +70,16 @@ Http::RegisterCustomInlineHeader("asdf_client_secret_fdsa", ""); + envoy::extensions::filters::http::oauth2::v3::OAuth2Config p; + auto* endpoint = p.mutable_token_endpoint(); + endpoint->set_cluster("auth.example.com"); + endpoint->set_uri("auth.example.com/_oauth"); + endpoint->mutable_timeout()->set_seconds(1); + p.set_redirect_uri("%REQ(:scheme)%://%REQ(:authority)%" + TEST_CALLBACK); + p.mutable_redirect_path_matcher()->mutable_path()->set_exact(TEST_CALLBACK); + p.set_authorization_endpoint("https://auth.example.com/oauth/authorize/"); + p.mutable_signout_path()->mutable_path()->set_exact("/_signout"); + p.set_forward_bearer_token(true); + p.set_stat_prefix("my_prefix"); + auto* matcher = p.add_pass_through_matcher(); + matcher->set_name(":method"); + matcher->mutable_string_match()->set_exact("OPTIONS"); + auto credentials = p.mutable_credentials(); + credentials->set_client_id(TEST_CLIENT_ID); + credentials->mutable_token_secret()->set_name("secret"); + credentials->mutable_hmac_secret()->set_name("hmac"); + + MessageUtil::validate(p, ProtobufMessage::getStrictValidationVisitor()); + init(std::make_shared(p, factory_context_.server_factory_context_, secret_reader, + scope_, "test.")); + + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + }; + + EXPECT_CALL(decoder_callbacks_, + sendLocalReply(Http::Code::ServiceUnavailable, "OAuth flow failed.", _, _, + "OAuth2 secrets are not ready")); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); + EXPECT_EQ(1, config_->stats().oauth_failure_.value()); +} + +TEST_F(OAuth2Test, TlsClientAuthDoesNotRequireClientSecret) { + auto secret_reader = std::make_shared("", TEST_HMAC_SECRET); + envoy::extensions::filters::http::oauth2::v3::OAuth2Config p; + auto* endpoint = p.mutable_token_endpoint(); + endpoint->set_cluster("auth.example.com"); + endpoint->set_uri("auth.example.com/_oauth"); + endpoint->mutable_timeout()->set_seconds(1); + p.set_redirect_uri("%REQ(:scheme)%://%REQ(:authority)%" + TEST_CALLBACK); + p.mutable_redirect_path_matcher()->mutable_path()->set_exact(TEST_CALLBACK); + p.set_authorization_endpoint("https://auth.example.com/oauth/authorize/"); + p.mutable_signout_path()->mutable_path()->set_exact("/_signout"); + p.set_forward_bearer_token(true); + p.set_stat_prefix("my_prefix"); + p.set_auth_type(::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: + OAuth2Config_AuthType_TLS_CLIENT_AUTH); + auto* matcher = p.add_pass_through_matcher(); + matcher->set_name(":method"); + matcher->mutable_string_match()->set_exact("OPTIONS"); + auto credentials = p.mutable_credentials(); + credentials->set_client_id(TEST_CLIENT_ID); + credentials->mutable_hmac_secret()->set_name("hmac"); + + MessageUtil::validate(p, ProtobufMessage::getStrictValidationVisitor()); + init(std::make_shared(p, factory_context_.server_factory_context_, secret_reader, + scope_, "test.")); + + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Path.get(), "/original_path?var1=1&var2=2"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + }; + + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(false)); + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, true)); + EXPECT_CALL(decoder_callbacks_, sendLocalReply(_, _, _, _, _)).Times(0); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); +} + // Verifies that we fail constructing the filter if the authorization endpoint isn't a valid URL. TEST_F(OAuth2Test, InvalidAuthorizationEndpoint) { // Create a filter config with an invalid authorization_endpoint URL. diff --git a/test/mocks/secret/mocks.h b/test/mocks/secret/mocks.h index 85e151eb633c1..ff5bfbf61a282 100644 --- a/test/mocks/secret/mocks.h +++ b/test/mocks/secret/mocks.h @@ -56,7 +56,7 @@ class MockSecretManager : public SecretManager { Server::Configuration::ServerFactoryContext&, Init::Manager& init_manager)); MOCK_METHOD(GenericSecretConfigProviderSharedPtr, findOrCreateGenericSecretProvider, (const envoy::config::core::v3::ConfigSource&, const std::string&, - Server::Configuration::ServerFactoryContext&, Init::Manager& init_manager)); + Server::Configuration::ServerFactoryContext&, OptRef init_manager)); }; class MockSecretCallbacks : public SecretCallbacks { From 7fa327768bd7445f47c3bd890963322af557401e Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 7 Apr 2026 15:34:01 +0800 Subject: [PATCH 16/29] address comments Signed-off-by: Huabing (Robin) Zhao --- api/envoy/extensions/filters/http/oauth2/v3/oauth.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/envoy/extensions/filters/http/oauth2/v3/oauth.proto b/api/envoy/extensions/filters/http/oauth2/v3/oauth.proto index bf5c9bebc7290..28766ca518dac 100644 --- a/api/envoy/extensions/filters/http/oauth2/v3/oauth.proto +++ b/api/envoy/extensions/filters/http/oauth2/v3/oauth.proto @@ -318,6 +318,6 @@ message OAuth2PerRoute { // Filter config. message OAuth2 { - // Global/default OAuth2 config for this filter. + // The OAuth2 filter config. OAuth2Config config = 1; } From 185886f791db88f64dce8b10dbc3ed2641a025ba Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 7 Apr 2026 15:41:23 +0800 Subject: [PATCH 17/29] update Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 3 ++- test/extensions/filters/http/oauth2/filter_test.cc | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 2b91d8f40c8b4..681c211609ead 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -51,6 +51,7 @@ constexpr const char* OIDCLogoutUrlFormatString = "{0}?id_token_hint={1}&client_id={2}&post_logout_redirect_uri={3}"; constexpr absl::string_view UnauthorizedBodyMessage = "OAuth flow failed."; +constexpr absl::string_view ServiceUnavailableBodyMessage = "Service Unavailable"; constexpr absl::string_view queryParamsError = "error"; constexpr absl::string_view queryParamsCode = "code"; constexpr absl::string_view queryParamsState = "state"; @@ -1472,7 +1473,7 @@ void OAuth2Filter::sendSecretsNotReadyResponse(const std::string& details) { ENVOY_STREAM_LOG(warn, "Responding with 503 Service Unavailable. Cause: {}", *decoder_callbacks_, details); config_->stats().oauth_failure_.inc(); - decoder_callbacks_->sendLocalReply(Http::Code::ServiceUnavailable, UnauthorizedBodyMessage, + decoder_callbacks_->sendLocalReply(Http::Code::ServiceUnavailable, ServiceUnavailableBodyMessage, nullptr, absl::nullopt, details); } diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index f2f28f5b06a99..643b5cb5b612a 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -491,7 +491,7 @@ TEST_F(OAuth2Test, SecretsNotReadyReturnsServiceUnavailable) { }; EXPECT_CALL(decoder_callbacks_, - sendLocalReply(Http::Code::ServiceUnavailable, "OAuth flow failed.", _, _, + sendLocalReply(Http::Code::ServiceUnavailable, "Service Unavailable", _, _, "OAuth2 secrets are not ready")); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers, false)); From c5d153dfd3c61c6b21a4f5141db10f1f5f882099 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Tue, 7 Apr 2026 12:46:36 +0000 Subject: [PATCH 18/29] address comment Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 13 +------------ source/extensions/filters/http/oauth2/filter.h | 1 - test/extensions/filters/http/oauth2/filter_test.cc | 3 +++ 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 681c211609ead..946034a0caba9 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -613,18 +613,7 @@ OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr default_config, Random::RandomGenerator& random) : default_config_(std::move(default_config)), oauth_client_factory_(std::move(oauth_client_factory)), time_source_(time_source), - random_(random) { - if (default_config_ != nullptr) { - setActiveConfig(default_config_); - } -} - -void OAuth2Filter::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) { - PassThroughDecoderFilter::setDecoderFilterCallbacks(callbacks); - if (oauth_client_ != nullptr) { - oauth_client_->setDecoderFilterCallbacks(callbacks); - } -} + random_(random) {} FilterConfigSharedPtr OAuth2Filter::getConfigForRequest() const { const auto* route_specific_config = diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index 43ca994169a99..64ef32bb66472 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -385,7 +385,6 @@ class OAuth2Filter : public Http::PassThroughFilter, void finishRefreshAccessTokenFlow(); void updateTokens(const std::string& access_token, const std::string& id_token, const std::string& refresh_token, std::chrono::seconds expires_in); - void setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) override; private: friend class OAuth2Test; diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 643b5cb5b612a..6855d37d67271 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -147,6 +147,9 @@ class OAuth2Test : public testing::TestWithParam { EXPECT_CALL(*oauth_client_, getState()).WillRepeatedly(Return(OAuth2Client::OAuthState::Idle)); filter_->setDecoderFilterCallbacks(decoder_callbacks_); filter_->setEncoderFilterCallbacks(encoder_callbacks_); + if (config_ != nullptr) { + filter_->setActiveConfig(config_); + } validator_ = std::make_shared(); if (config_ != nullptr) { filter_->validator_ = validator_; From 56994e57f0e0a77ce9e78961d1146f15d0d63ff5 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 8 Apr 2026 01:58:01 +0000 Subject: [PATCH 19/29] add comment Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index 64ef32bb66472..32baca4c33ac1 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -177,6 +177,8 @@ class FilterConfig : public Logger::Loggable { const Matchers::PathMatcher& signoutPath() const { return signout_path_; } std::string clientSecret() const { return secret_reader_->clientSecret(); } std::string hmacSecret() const { return secret_reader_->hmacSecret(); } + // The secrets are loaded asynchronously if per-route configurations are used, so the filter needs + // to check if the secrets are available before processing the request. bool requiredSecretsAvailable() const { return !secret_reader_->hmacSecret().empty() && (auth_type_ == AuthType::TlsClientAuth || !secret_reader_->clientSecret().empty()); From b3f86996f84e8fe7fac724d043ed3c20121e90b7 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 8 Apr 2026 10:01:33 +0800 Subject: [PATCH 20/29] Update source/extensions/filters/http/oauth2/filter.cc Co-authored-by: code Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 946034a0caba9..8356b305a5ff2 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -635,6 +635,8 @@ void OAuth2Filter::setActiveConfig(FilterConfigSharedPtr config) { if (config_ == config && validator_ != nullptr && oauth_client_ != nullptr) { return; + if (config == nullptr) { + return; } config_ = std::move(config); From d6eb7e2a649fe21dcca81bd8801d62c993169989 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 8 Apr 2026 10:02:20 +0800 Subject: [PATCH 21/29] Update source/extensions/filters/http/oauth2/filter.cc Co-authored-by: code Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 8356b305a5ff2..09a5c8c3783f8 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -645,9 +645,8 @@ void OAuth2Filter::setActiveConfig(FilterConfigSharedPtr config) { oauth_client_ = oauth_client_factory_(config_); oauth_client_->setCallbacks(*this); - if (decoder_callbacks_ != nullptr) { - oauth_client_->setDecoderFilterCallbacks(*decoder_callbacks_); - } + ASSERT(decoder_callbacks_ != nullptr); + oauth_client_->setDecoderFilterCallbacks(*decoder_callbacks_); } /** From 1637ee57da52fe3520e5527eb1eff38f0a9e1743 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 8 Apr 2026 10:59:25 +0800 Subject: [PATCH 22/29] remove the FilterConfigPerRoute class Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/config.cc | 2 +- source/extensions/filters/http/oauth2/filter.cc | 4 ++-- source/extensions/filters/http/oauth2/filter.h | 14 +++----------- test/extensions/filters/http/oauth2/filter_test.cc | 5 +++-- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/source/extensions/filters/http/oauth2/config.cc b/source/extensions/filters/http/oauth2/config.cc index accab8cf222a9..97978e74c5f4e 100644 --- a/source/extensions/filters/http/oauth2/config.cc +++ b/source/extensions/filters/http/oauth2/config.cc @@ -130,7 +130,7 @@ OAuth2Config::createRouteSpecificFilterConfigTyped( if (!config_or_error.ok()) { return config_or_error.status(); } - return std::make_shared(config_or_error.value()); + return config_or_error.value(); } /* diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 09a5c8c3783f8..34bd623823ee0 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -617,9 +617,9 @@ OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr default_config, FilterConfigSharedPtr OAuth2Filter::getConfigForRequest() const { const auto* route_specific_config = - Http::Utility::resolveMostSpecificPerFilterConfig(decoder_callbacks_); + Http::Utility::resolveMostSpecificPerFilterConfig(decoder_callbacks_); if (route_specific_config != nullptr) { - return route_specific_config->config(); + return std::const_pointer_cast(route_specific_config->shared_from_this()); } return default_config_; diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index 32baca4c33ac1..f58a7d93620cf 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -147,7 +147,9 @@ struct CookieNames { * This class encapsulates all data needed for the filter to operate so that we don't pass around * raw protobufs and other arbitrary data. */ -class FilterConfig : public Logger::Loggable { +class FilterConfig : public Router::RouteSpecificFilterConfig, + public Logger::Loggable, + public std::enable_shared_from_this { public: FilterConfig(const envoy::extensions::filters::http::oauth2::v3::OAuth2Config& proto_config, Server::Configuration::CommonFactoryContext& context, @@ -279,16 +281,6 @@ class FilterConfig : public Logger::Loggable { using FilterConfigSharedPtr = std::shared_ptr; -class FilterConfigPerRoute : public Router::RouteSpecificFilterConfig { -public: - explicit FilterConfigPerRoute(FilterConfigSharedPtr config) : config_(std::move(config)) {} - - const FilterConfigSharedPtr& config() const { return config_; } - -private: - FilterConfigSharedPtr config_; -}; - /** * An OAuth cookie validator: * 1. extracts cookies from a request diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 6855d37d67271..e04b7bbe5e6be 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -4678,8 +4678,9 @@ TEST_F(OAuth2Test, RouteSpecificConfigOverridesGlobalConfig) { credentials->mutable_hmac_secret()->set_name("hmac"); auto secret_reader = std::make_shared(); - auto route_config = std::make_shared(std::make_shared( - route_proto, factory_context_.server_factory_context_, secret_reader, scope_, "test.")); + auto route_config = std::make_shared(route_proto, + factory_context_.server_factory_context_, + secret_reader, scope_, "test."); ON_CALL(decoder_callbacks_, mostSpecificPerFilterConfig()) .WillByDefault(Return(route_config.get())); From b577f231cc83e6546d9c5289834d0621f2157b76 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 8 Apr 2026 11:07:42 +0800 Subject: [PATCH 23/29] use raw pointer Signed-off-by: Huabing (Robin) Zhao --- .../extensions/filters/http/oauth2/config.cc | 7 +++---- .../extensions/filters/http/oauth2/filter.cc | 18 ++++++++---------- source/extensions/filters/http/oauth2/filter.h | 16 +++++++--------- .../filters/http/oauth2/filter_test.cc | 9 ++++----- 4 files changed, 22 insertions(+), 28 deletions(-) diff --git a/source/extensions/filters/http/oauth2/config.cc b/source/extensions/filters/http/oauth2/config.cc index 97978e74c5f4e..d22b07b0ea70f 100644 --- a/source/extensions/filters/http/oauth2/config.cc +++ b/source/extensions/filters/http/oauth2/config.cc @@ -110,11 +110,10 @@ absl::StatusOr OAuth2Config::createFilterFactoryFromProto [&context, config, &cluster_manager](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamFilter(std::make_shared( config, - [&cluster_manager]( - const FilterConfigSharedPtr& active_config) -> std::unique_ptr { + [&cluster_manager](const FilterConfig& active_config) -> std::unique_ptr { return std::make_unique( - cluster_manager, active_config->oauthTokenEndpoint(), - active_config->retryPolicy(), active_config->defaultExpiresIn()); + cluster_manager, active_config.oauthTokenEndpoint(), active_config.retryPolicy(), + active_config.defaultExpiresIn()); }, context.serverFactoryContext().timeSource(), context.serverFactoryContext().api().randomGenerator())); diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 34bd623823ee0..a118cd0379468 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -615,35 +615,33 @@ OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr default_config, oauth_client_factory_(std::move(oauth_client_factory)), time_source_(time_source), random_(random) {} -FilterConfigSharedPtr OAuth2Filter::getConfigForRequest() const { +const FilterConfig* OAuth2Filter::getConfigForRequest() const { const auto* route_specific_config = Http::Utility::resolveMostSpecificPerFilterConfig(decoder_callbacks_); if (route_specific_config != nullptr) { - return std::const_pointer_cast(route_specific_config->shared_from_this()); + return route_specific_config; } - return default_config_; + return default_config_.get(); } -void OAuth2Filter::setActiveConfig(FilterConfigSharedPtr config) { +void OAuth2Filter::setActiveConfig(const FilterConfig* config) { if (config == nullptr) { - config_.reset(); + config_ = nullptr; validator_.reset(); oauth_client_.reset(); return; } - if (config_ == config && validator_ != nullptr && oauth_client_ != nullptr) { - return; - if (config == nullptr) { + if (config_ == config) { return; } - config_ = std::move(config); + config_ = config; validator_ = std::make_shared(time_source_, config_->cookieNames(), config_->cookieDomain()); - oauth_client_ = oauth_client_factory_(config_); + oauth_client_ = oauth_client_factory_(*config_); oauth_client_->setCallbacks(*this); ASSERT(decoder_callbacks_ != nullptr); oauth_client_->setDecoderFilterCallbacks(*decoder_callbacks_); diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index f58a7d93620cf..4e1d61ca41aac 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -148,8 +148,7 @@ struct CookieNames { * raw protobufs and other arbitrary data. */ class FilterConfig : public Router::RouteSpecificFilterConfig, - public Logger::Loggable, - public std::enable_shared_from_this { + public Logger::Loggable { public: FilterConfig(const envoy::extensions::filters::http::oauth2::v3::OAuth2Config& proto_config, Server::Configuration::CommonFactoryContext& context, @@ -185,7 +184,7 @@ class FilterConfig : public Router::RouteSpecificFilterConfig, return !secret_reader_->hmacSecret().empty() && (auth_type_ == AuthType::TlsClientAuth || !secret_reader_->clientSecret().empty()); } - FilterStats& stats() { return stats_; } + FilterStats& stats() const { return stats_; } const std::string& encodedResourceQueryParams() const { return encoded_resource_query_params_; } const CookieNames& cookieNames() const { return cookie_names_; } const std::string& cookieDomain() const { return cookie_domain_; } @@ -249,7 +248,7 @@ class FilterConfig : public Router::RouteSpecificFilterConfig, const Matchers::PathMatcher redirect_matcher_; const Matchers::PathMatcher signout_path_; std::shared_ptr secret_reader_; - FilterStats stats_; + mutable FilterStats stats_; const std::string encoded_auth_scopes_; const std::string encoded_resource_query_params_; const std::vector pass_through_header_matchers_; @@ -347,8 +346,7 @@ class OAuth2Filter : public Http::PassThroughFilter, FilterCallbacks, Logger::Loggable { public: - using OAuth2ClientFactory = - std::function(const FilterConfigSharedPtr&)>; + using OAuth2ClientFactory = std::function(const FilterConfig&)>; OAuth2Filter(FilterConfigSharedPtr default_config, OAuth2ClientFactory oauth_client_factory, TimeSource& time_source, Random::RandomGenerator& random); @@ -402,13 +400,13 @@ class OAuth2Filter : public Http::PassThroughFilter, std::unique_ptr oauth_client_; FilterConfigSharedPtr default_config_; - FilterConfigSharedPtr config_; + const FilterConfig* config_{nullptr}; OAuth2ClientFactory oauth_client_factory_; TimeSource& time_source_; Random::RandomGenerator& random_; - FilterConfigSharedPtr getConfigForRequest() const; - void setActiveConfig(FilterConfigSharedPtr config); + const FilterConfig* getConfigForRequest() const; + void setActiveConfig(const FilterConfig* config); // Determines whether or not the current request can skip the entire OAuth flow (HMAC is valid, // connection is mTLS, etc.) diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index e04b7bbe5e6be..8fe225ed97283 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -135,7 +135,7 @@ class OAuth2Test : public testing::TestWithParam { ON_CALL(test_random_, random()).WillByDefault(Return(123456789)); filter_ = std::make_shared( config_, - [this, oauth_client_holder](const FilterConfigSharedPtr&) -> std::unique_ptr { + [this, oauth_client_holder](const FilterConfig&) -> std::unique_ptr { if (*oauth_client_holder != nullptr) { return std::move(*oauth_client_holder); } @@ -148,7 +148,7 @@ class OAuth2Test : public testing::TestWithParam { filter_->setDecoderFilterCallbacks(decoder_callbacks_); filter_->setEncoderFilterCallbacks(encoder_callbacks_); if (config_ != nullptr) { - filter_->setActiveConfig(config_); + filter_->setActiveConfig(config_.get()); } validator_ = std::make_shared(); if (config_ != nullptr) { @@ -4678,9 +4678,8 @@ TEST_F(OAuth2Test, RouteSpecificConfigOverridesGlobalConfig) { credentials->mutable_hmac_secret()->set_name("hmac"); auto secret_reader = std::make_shared(); - auto route_config = std::make_shared(route_proto, - factory_context_.server_factory_context_, - secret_reader, scope_, "test."); + auto route_config = std::make_shared( + route_proto, factory_context_.server_factory_context_, secret_reader, scope_, "test."); ON_CALL(decoder_callbacks_, mostSpecificPerFilterConfig()) .WillByDefault(Return(route_config.get())); From 4922e49d0e05a4c50355e5085f0333b253204dfb Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 8 Apr 2026 12:52:58 +0800 Subject: [PATCH 24/29] update setActiveConfig Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index a118cd0379468..fcae4ceb0a288 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -626,18 +626,17 @@ const FilterConfig* OAuth2Filter::getConfigForRequest() const { } void OAuth2Filter::setActiveConfig(const FilterConfig* config) { - if (config == nullptr) { - config_ = nullptr; - validator_.reset(); - oauth_client_.reset(); + if (config_ == config) { return; } - if (config_ == config) { + config_ = config; + if (config_ == nullptr) { + validator_.reset(); + oauth_client_.reset(); return; } - config_ = config; validator_ = std::make_shared(time_source_, config_->cookieNames(), config_->cookieDomain()); From 9f15e008e2bf3eb29b863b497109b9771256264b Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 8 Apr 2026 16:10:49 +0800 Subject: [PATCH 25/29] simplify code Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index fcae4ceb0a288..e203c3797d43d 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -626,17 +626,11 @@ const FilterConfig* OAuth2Filter::getConfigForRequest() const { } void OAuth2Filter::setActiveConfig(const FilterConfig* config) { - if (config_ == config) { + if (config == nullptr) { return; } config_ = config; - if (config_ == nullptr) { - validator_.reset(); - oauth_client_.reset(); - return; - } - validator_ = std::make_shared(time_source_, config_->cookieNames(), config_->cookieDomain()); From 9840fdd33a1c86a317346d2733f01580190c67e8 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 8 Apr 2026 16:23:56 +0800 Subject: [PATCH 26/29] simplify code Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 16 ++++++---------- source/extensions/filters/http/oauth2/filter.h | 3 +-- .../filters/http/oauth2/filter_test.cc | 2 +- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index e203c3797d43d..6c2009e15a029 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -615,17 +615,11 @@ OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr default_config, oauth_client_factory_(std::move(oauth_client_factory)), time_source_(time_source), random_(random) {} -const FilterConfig* OAuth2Filter::getConfigForRequest() const { +void OAuth2Filter::resolveAndSetActiveConfig() { const auto* route_specific_config = Http::Utility::resolveMostSpecificPerFilterConfig(decoder_callbacks_); - if (route_specific_config != nullptr) { - return route_specific_config; - } - - return default_config_.get(); -} - -void OAuth2Filter::setActiveConfig(const FilterConfig* config) { + const FilterConfig* config = + route_specific_config != nullptr ? route_specific_config : default_config_.get(); if (config == nullptr) { return; } @@ -654,7 +648,9 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he headers.remove(OAuth2Headers::get().OAuthStatus); headers.remove(OAuth2Headers::get().OAuthFailureReason); - setActiveConfig(getConfigForRequest()); + // Resolve the active configuration for the request. Per-route configuration can override the default filter configuration, + // so this step is necessary to determine which configuration to use for the current request. + resolveAndSetActiveConfig(); // If no config is set, OAuth2 is disabled for this request. if (config_ == nullptr) { return Http::FilterHeadersStatus::Continue; diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index 4e1d61ca41aac..70ab7211091e4 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -405,8 +405,7 @@ class OAuth2Filter : public Http::PassThroughFilter, TimeSource& time_source_; Random::RandomGenerator& random_; - const FilterConfig* getConfigForRequest() const; - void setActiveConfig(const FilterConfig* config); + void resolveAndSetActiveConfig(); // Determines whether or not the current request can skip the entire OAuth flow (HMAC is valid, // connection is mTLS, etc.) diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 8fe225ed97283..0b1b54f58e5f2 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -148,7 +148,7 @@ class OAuth2Test : public testing::TestWithParam { filter_->setDecoderFilterCallbacks(decoder_callbacks_); filter_->setEncoderFilterCallbacks(encoder_callbacks_); if (config_ != nullptr) { - filter_->setActiveConfig(config_.get()); + filter_->resolveAndSetActiveConfig(); } validator_ = std::make_shared(); if (config_ != nullptr) { From 92823f0250c4f812a9e0b924ade2e6336154e36d Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 8 Apr 2026 16:49:54 +0800 Subject: [PATCH 27/29] fix test Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 6c2009e15a029..e40ed2a3dde67 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -624,6 +624,11 @@ void OAuth2Filter::resolveAndSetActiveConfig() { return; } + // We only need this for testing purposes to avoid resetting the mock validator. + if (config_ == config) { + return; + } + config_ = config; validator_ = std::make_shared(time_source_, config_->cookieNames(), config_->cookieDomain()); From 70ca9fd2aefb8fde6896a571fa14edb6a38a81b1 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 8 Apr 2026 08:59:58 +0000 Subject: [PATCH 28/29] format Signed-off-by: Huabing (Robin) Zhao --- source/extensions/filters/http/oauth2/filter.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index e40ed2a3dde67..55037d9468e41 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -653,8 +653,9 @@ Http::FilterHeadersStatus OAuth2Filter::decodeHeaders(Http::RequestHeaderMap& he headers.remove(OAuth2Headers::get().OAuthStatus); headers.remove(OAuth2Headers::get().OAuthFailureReason); - // Resolve the active configuration for the request. Per-route configuration can override the default filter configuration, - // so this step is necessary to determine which configuration to use for the current request. + // Resolve the active configuration for the request. Per-route configuration can override the + // default filter configuration, so this step is necessary to determine which configuration to use + // for the current request. resolveAndSetActiveConfig(); // If no config is set, OAuth2 is disabled for this request. if (config_ == nullptr) { From 85a4d9fff1a273e75c7abea01b0e8d5634cd22fe Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Wed, 8 Apr 2026 18:13:23 +0800 Subject: [PATCH 29/29] refactor Signed-off-by: Huabing (Robin) Zhao --- .../extensions/filters/http/oauth2/config.cc | 9 +++++-- .../extensions/filters/http/oauth2/filter.cc | 14 +++++------ .../extensions/filters/http/oauth2/filter.h | 10 +++++--- .../filters/http/oauth2/filter_test.cc | 24 +++++-------------- 4 files changed, 27 insertions(+), 30 deletions(-) diff --git a/source/extensions/filters/http/oauth2/config.cc b/source/extensions/filters/http/oauth2/config.cc index d22b07b0ea70f..48c70a684d208 100644 --- a/source/extensions/filters/http/oauth2/config.cc +++ b/source/extensions/filters/http/oauth2/config.cc @@ -110,11 +110,16 @@ absl::StatusOr OAuth2Config::createFilterFactoryFromProto [&context, config, &cluster_manager](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamFilter(std::make_shared( config, - [&cluster_manager](const FilterConfig& active_config) -> std::unique_ptr { - return std::make_unique( + [&cluster_manager](const FilterConfig& active_config) -> std::shared_ptr { + return std::make_shared( cluster_manager, active_config.oauthTokenEndpoint(), active_config.retryPolicy(), active_config.defaultExpiresIn()); }, + [](TimeSource& time_source, + const FilterConfig& active_config) -> std::shared_ptr { + return std::make_shared( + time_source, active_config.cookieNames(), active_config.cookieDomain()); + }, context.serverFactoryContext().timeSource(), context.serverFactoryContext().api().randomGenerator())); }; diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index 55037d9468e41..484965d0c2544 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -609,29 +609,29 @@ bool OAuth2CookieValidator::timestampIsValid() const { bool OAuth2CookieValidator::isValid() const { return hmacIsValid() && timestampIsValid(); } OAuth2Filter::OAuth2Filter(FilterConfigSharedPtr default_config, - OAuth2ClientFactory oauth_client_factory, TimeSource& time_source, + OAuth2ClientFactory oauth_client_factory, + ValidatorFactory validator_factory, TimeSource& time_source, Random::RandomGenerator& random) : default_config_(std::move(default_config)), - oauth_client_factory_(std::move(oauth_client_factory)), time_source_(time_source), - random_(random) {} + oauth_client_factory_(std::move(oauth_client_factory)), + validator_factory_(std::move(validator_factory)), time_source_(time_source), random_(random) { +} void OAuth2Filter::resolveAndSetActiveConfig() { const auto* route_specific_config = Http::Utility::resolveMostSpecificPerFilterConfig(decoder_callbacks_); const FilterConfig* config = route_specific_config != nullptr ? route_specific_config : default_config_.get(); + if (config == nullptr) { return; } - - // We only need this for testing purposes to avoid resetting the mock validator. if (config_ == config) { return; } config_ = config; - validator_ = std::make_shared(time_source_, config_->cookieNames(), - config_->cookieDomain()); + validator_ = validator_factory_(time_source_, *config_); oauth_client_ = oauth_client_factory_(*config_); oauth_client_->setCallbacks(*this); diff --git a/source/extensions/filters/http/oauth2/filter.h b/source/extensions/filters/http/oauth2/filter.h index 70ab7211091e4..ad649608a7f1e 100644 --- a/source/extensions/filters/http/oauth2/filter.h +++ b/source/extensions/filters/http/oauth2/filter.h @@ -346,10 +346,13 @@ class OAuth2Filter : public Http::PassThroughFilter, FilterCallbacks, Logger::Loggable { public: - using OAuth2ClientFactory = std::function(const FilterConfig&)>; + using OAuth2ClientFactory = std::function(const FilterConfig&)>; + using ValidatorFactory = + std::function(TimeSource&, const FilterConfig&)>; OAuth2Filter(FilterConfigSharedPtr default_config, OAuth2ClientFactory oauth_client_factory, - TimeSource& time_source, Random::RandomGenerator& random); + ValidatorFactory validator_factory, TimeSource& time_source, + Random::RandomGenerator& random); // Http::PassThroughFilter Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool) override; @@ -398,10 +401,11 @@ class OAuth2Filter : public Http::PassThroughFilter, Http::RequestHeaderMap* request_headers_{nullptr}; bool was_refresh_token_flow_{false}; - std::unique_ptr oauth_client_; + std::shared_ptr oauth_client_; FilterConfigSharedPtr default_config_; const FilterConfig* config_{nullptr}; OAuth2ClientFactory oauth_client_factory_; + ValidatorFactory validator_factory_; TimeSource& time_source_; Random::RandomGenerator& random_; diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 0b1b54f58e5f2..42842d7a5cafa 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -127,33 +127,21 @@ class OAuth2Test : public testing::TestWithParam { void init(FilterConfigSharedPtr config) { // Set up the OAuth client. - oauth_client_ = new MockOAuth2Client(); - auto oauth_client_holder = std::make_shared>( - std::unique_ptr{oauth_client_}); + oauth_client_ = std::make_shared(); + validator_ = std::make_shared(); config_ = config; ON_CALL(test_random_, random()).WillByDefault(Return(123456789)); filter_ = std::make_shared( config_, - [this, oauth_client_holder](const FilterConfig&) -> std::unique_ptr { - if (*oauth_client_holder != nullptr) { - return std::move(*oauth_client_holder); - } - - oauth_client_ = new MockOAuth2Client(); - return std::unique_ptr{oauth_client_}; + [this](const FilterConfig&) -> std::shared_ptr { return oauth_client_; }, + [this](TimeSource&, const FilterConfig&) -> std::shared_ptr { + return validator_; }, test_time_, test_random_); EXPECT_CALL(*oauth_client_, getState()).WillRepeatedly(Return(OAuth2Client::OAuthState::Idle)); filter_->setDecoderFilterCallbacks(decoder_callbacks_); filter_->setEncoderFilterCallbacks(encoder_callbacks_); - if (config_ != nullptr) { - filter_->resolveAndSetActiveConfig(); - } - validator_ = std::make_shared(); - if (config_ != nullptr) { - filter_->validator_ = validator_; - } filter_->flow_id_ = "00000000075bcd15"; } @@ -372,7 +360,7 @@ class OAuth2Test : public testing::TestWithParam { NiceMock cm_; std::shared_ptr validator_; std::shared_ptr filter_; - MockOAuth2Client* oauth_client_; + std::shared_ptr oauth_client_; FilterConfigSharedPtr config_; Http::MockAsyncClientRequest request_; std::deque callbacks_;