diff --git a/api/envoy/extensions/filters/http/oauth2/v3/oauth.proto b/api/envoy/extensions/filters/http/oauth2/v3/oauth.proto index 8a2368f6522bc..28766ca518dac 100644 --- a/api/envoy/extensions/filters/http/oauth2/v3/oauth.proto +++ b/api/envoy/extensions/filters/http/oauth2/v3/oauth.proto @@ -306,8 +306,18 @@ message OAuth2Config { repeated config.route.v3.HeaderMatcher allow_failed_matcher = 27; } +// 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. + // The OAuth2 filter config. OAuth2Config config = 1; } diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 6068acbaa057c..d98efadc368f2 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -104,6 +104,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. - area: proto_api_scrubber change: | If :ref:`scrub_unknown_fields diff --git a/docs/root/configuration/http/http_filters/oauth2_filter.rst b/docs/root/configuration/http/http_filters/oauth2_filter.rst index 8eec4f9452c12..571ba7b2bb2ef 100644 --- a/docs/root/configuration/http/http_filters/oauth2_filter.rst +++ b/docs/root/configuration/http/http_filters/oauth2_filter.rst @@ -7,6 +7,22 @@ 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. + +When using per-route configuration, keep the +:ref:`redirect_path_matcher ` +and :ref:`signout_path ` +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: * An unauthenticated user arrives at myapp.com, and the oauth filter redirects them to the @@ -229,6 +245,138 @@ can be defined in one shared file. generic_secret: secret: +The following example shows two independent route prefixes, ``/foo`` and ``/bar``, each with its +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 + + 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 + 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: + 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 + 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: + path: "/etc/bar-client-secret.yaml" + hmac_secret: + name: hmac + sds_config: + path: "/etc/hmac-secret.yaml" + auth_scopes: + - openid + - email + Notes ----- 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/extensions_metadata.yaml b/source/extensions/extensions_metadata.yaml index ede636ef97d31..d13592a85db73 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 diff --git a/source/extensions/filters/http/oauth2/config.cc b/source/extensions/filters/http/oauth2/config.cc index 856c198085bc2..48c70a684d208 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); @@ -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, + OptRef 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,58 @@ 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 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())); }; } +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, absl::nullopt, context.scope(), ""); + if (!config_or_error.ok()) { + return config_or_error.status(); + } + return 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 88629ec79e652..484965d0c2544 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -51,7 +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"; @@ -608,20 +608,35 @@ 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, + ValidatorFactory validator_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), - random_(random) { - - oauth_client_->setCallbacks(*this); + : default_config_(std::move(default_config)), + oauth_client_factory_(std::move(oauth_client_factory)), + validator_factory_(std::move(validator_factory)), time_source_(time_source), random_(random) { } -void OAuth2Filter::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) { - PassThroughDecoderFilter::setDecoderFilterCallbacks(callbacks); - oauth_client_->setDecoderFilterCallbacks(callbacks); +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; + } + if (config_ == config) { + return; + } + + config_ = config; + validator_ = validator_factory_(time_source_, *config_); + + oauth_client_ = oauth_client_factory_(*config_); + oauth_client_->setCallbacks(*this); + ASSERT(decoder_callbacks_ != nullptr); + oauth_client_->setDecoderFilterCallbacks(*decoder_callbacks_); } /** @@ -638,6 +653,15 @@ 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. + resolveAndSetActiveConfig(); + // 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. @@ -648,6 +672,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 @@ -1423,6 +1452,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, ServiceUnavailableBodyMessage, + 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 bdbfaf2de4359..ad649608a7f1e 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 @@ -146,7 +147,8 @@ 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: FilterConfig(const envoy::extensions::filters::http::oauth2::v3::OAuth2Config& proto_config, Server::Configuration::CommonFactoryContext& context, @@ -176,7 +178,13 @@ 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(); } - FilterStats& stats() { return stats_; } + // 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()); + } + 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_; } @@ -240,7 +248,7 @@ class FilterConfig : public Logger::Loggable { 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_; @@ -338,8 +346,13 @@ class OAuth2Filter : public Http::PassThroughFilter, FilterCallbacks, Logger::Loggable { public: - OAuth2Filter(FilterConfigSharedPtr config, std::unique_ptr&& oauth_client, - TimeSource& time_source, Random::RandomGenerator& random); + using OAuth2ClientFactory = std::function(const FilterConfig&)>; + using ValidatorFactory = + std::function(TimeSource&, const FilterConfig&)>; + + OAuth2Filter(FilterConfigSharedPtr default_config, OAuth2ClientFactory oauth_client_factory, + ValidatorFactory validator_factory, TimeSource& time_source, + Random::RandomGenerator& random); // Http::PassThroughFilter Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool) override; @@ -367,7 +380,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; @@ -389,11 +401,16 @@ class OAuth2Filter : public Http::PassThroughFilter, Http::RequestHeaderMap* request_headers_{nullptr}; bool was_refresh_token_flow_{false}; - std::unique_ptr oauth_client_; - FilterConfigSharedPtr config_; + 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_; + void resolveAndSetActiveConfig(); + // 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; @@ -428,6 +445,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/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 842d2e47e3d7c..42842d7a5cafa 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" @@ -69,12 +70,16 @@ Http::RegisterCustomInlineHeader { void init(FilterConfigSharedPtr config) { // Set up the OAuth client. - oauth_client_ = new MockOAuth2Client(); - std::unique_ptr oauth_client_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](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_ = 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(); - filter_->validator_ = validator_; filter_->flow_id_ = "00000000075bcd15"; } @@ -352,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_; @@ -441,6 +449,88 @@ name: client EXPECT_EQ(secret_reader.hmacSecret(), "token_test"); } +TEST_F(OAuth2Test, SecretsNotReadyReturnsServiceUnavailable) { + auto secret_reader = std::make_shared("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, "Service Unavailable", _, _, + "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. @@ -4557,9 +4647,69 @@ 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( + 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); +} + +TEST_F(OAuth2Test, EmptyRouteSpecificConfigOverridesGlobalConfig) { + init(nullptr); + + 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"}, + }; + + 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)); +} + /** - * 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). + * 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. @@ -5020,7 +5170,8 @@ TEST_F(OAuth2Test, AllowFailedWithRefreshTokenSyncFailure) { } /** - * Scenario: OAuth callback where asyncGetAccessToken fails synchronously (e.g. cluster not found). + * 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 @@ -5053,7 +5204,8 @@ TEST_F(OAuth2Test, OAuthCallbackGetAccessTokenSyncFailure) { } /** - * Scenario: AJAX request (matching deny_redirect_matcher) to /allowfailed path without valid OAuth. + * 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). 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 {