diff --git a/source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc b/source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc index 6f075374ba0cc..991807b0f040c 100644 --- a/source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc +++ b/source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc @@ -314,7 +314,8 @@ class GrpcStatsFilter : public Http::PassThroughFilter { } // namespace -Http::FilterFactoryCb GrpcStatsFilterConfigFactory::createFilterFactoryFromProtoTyped( +absl::StatusOr +GrpcStatsFilterConfigFactory::createFilterFactoryFromProtoTyped( const envoy::extensions::filters::http::grpc_stats::v3::FilterConfig& proto_config, const std::string&, Server::Configuration::FactoryContext& factory_context) { diff --git a/source/extensions/filters/http/grpc_stats/grpc_stats_filter.h b/source/extensions/filters/http/grpc_stats/grpc_stats_filter.h index 9121f43ffa8fe..659f694e2afee 100644 --- a/source/extensions/filters/http/grpc_stats/grpc_stats_filter.h +++ b/source/extensions/filters/http/grpc_stats/grpc_stats_filter.h @@ -30,12 +30,13 @@ struct GrpcStatsObject : public StreamInfo::FilterState::Object { }; class GrpcStatsFilterConfigFactory - : public Common::FactoryBase { + : public Common::ExceptionFreeFactoryBase< + envoy::extensions::filters::http::grpc_stats::v3::FilterConfig> { public: - GrpcStatsFilterConfigFactory() : FactoryBase("envoy.filters.http.grpc_stats") {} + GrpcStatsFilterConfigFactory() : ExceptionFreeFactoryBase("envoy.filters.http.grpc_stats") {} private: - Http::FilterFactoryCb createFilterFactoryFromProtoTyped( + absl::StatusOr createFilterFactoryFromProtoTyped( const envoy::extensions::filters::http::grpc_stats::v3::FilterConfig& proto_config, const std::string&, Server::Configuration::FactoryContext&) override; }; diff --git a/source/extensions/filters/http/grpc_web/config.cc b/source/extensions/filters/http/grpc_web/config.cc index 945d1a1afdd21..843f67a83c371 100644 --- a/source/extensions/filters/http/grpc_web/config.cc +++ b/source/extensions/filters/http/grpc_web/config.cc @@ -9,7 +9,7 @@ namespace Extensions { namespace HttpFilters { namespace GrpcWeb { -Http::FilterFactoryCb GrpcWebFilterConfig::createFilterFactoryFromProtoTyped( +absl::StatusOr GrpcWebFilterConfig::createFilterFactoryFromProtoTyped( const envoy::extensions::filters::http::grpc_web::v3::GrpcWeb&, const std::string&, Server::Configuration::FactoryContext& factory_context) { return [&factory_context](Http::FilterChainFactoryCallbacks& callbacks) { diff --git a/source/extensions/filters/http/grpc_web/config.h b/source/extensions/filters/http/grpc_web/config.h index ab76752466266..ef99400ef9d9f 100644 --- a/source/extensions/filters/http/grpc_web/config.h +++ b/source/extensions/filters/http/grpc_web/config.h @@ -10,13 +10,13 @@ namespace Extensions { namespace HttpFilters { namespace GrpcWeb { -class GrpcWebFilterConfig - : public Common::FactoryBase { +class GrpcWebFilterConfig : public Common::ExceptionFreeFactoryBase< + envoy::extensions::filters::http::grpc_web::v3::GrpcWeb> { public: - GrpcWebFilterConfig() : FactoryBase("envoy.filters.http.grpc_web") {} + GrpcWebFilterConfig() : ExceptionFreeFactoryBase("envoy.filters.http.grpc_web") {} private: - Http::FilterFactoryCb createFilterFactoryFromProtoTyped( + absl::StatusOr createFilterFactoryFromProtoTyped( const envoy::extensions::filters::http::grpc_web::v3::GrpcWeb& proto_config, const std::string& stats_prefix, Server::Configuration::FactoryContext& factory_context) override; diff --git a/source/extensions/filters/http/health_check/config.cc b/source/extensions/filters/http/health_check/config.cc index 950095531cdac..34f244931f02d 100644 --- a/source/extensions/filters/http/health_check/config.cc +++ b/source/extensions/filters/http/health_check/config.cc @@ -15,7 +15,7 @@ namespace Extensions { namespace HttpFilters { namespace HealthCheck { -Http::FilterFactoryCb HealthCheckFilterConfig::createFilterFactoryHelper( +absl::StatusOr HealthCheckFilterConfig::createFilterFactoryHelper( const envoy::extensions::filters::http::health_check::v3::HealthCheck& proto_config, const std::string& stats_prefix, Server::Configuration::ServerFactoryContext& context, Stats::Scope& scope) { @@ -30,7 +30,8 @@ Http::FilterFactoryCb HealthCheckFilterConfig::createFilterFactoryHelper( *header_match_data = Http::HeaderUtility::buildHeaderDataVector(proto_config.headers(), context); if (!pass_through_mode && cache_time_ms) { - throw EnvoyException("cache_time_ms must not be set when path_through_mode is disabled"); + return absl::InvalidArgumentError( + "cache_time_ms must not be set when path_through_mode is disabled"); } HealthCheckCacheManagerSharedPtr cache_manager; @@ -44,7 +45,7 @@ Http::FilterFactoryCb HealthCheckFilterConfig::createFilterFactoryHelper( auto cluster_to_percentage = std::make_unique(); for (const auto& item : proto_config.cluster_min_healthy_percentages()) { if (std::isnan(item.second.value())) { - throw EnvoyException(absl::StrCat( + return absl::InvalidArgumentError(absl::StrCat( "cluster_min_healthy_percentages contains a NaN value for cluster: ", item.first)); } cluster_to_percentage->emplace(std::make_pair(item.first, item.second.value())); @@ -61,7 +62,7 @@ Http::FilterFactoryCb HealthCheckFilterConfig::createFilterFactoryHelper( }; } -Http::FilterFactoryCb HealthCheckFilterConfig::createFilterFactoryFromProtoTyped( +absl::StatusOr HealthCheckFilterConfig::createFilterFactoryFromProtoTyped( const envoy::extensions::filters::http::health_check::v3::HealthCheck& proto_config, const std::string& stats_prefix, Server::Configuration::FactoryContext& context) { return createFilterFactoryHelper(proto_config, stats_prefix, context.serverFactoryContext(), diff --git a/source/extensions/filters/http/health_check/config.h b/source/extensions/filters/http/health_check/config.h index 7a6c905d35200..d89a886cb7744 100644 --- a/source/extensions/filters/http/health_check/config.h +++ b/source/extensions/filters/http/health_check/config.h @@ -11,12 +11,13 @@ namespace HttpFilters { namespace HealthCheck { class HealthCheckFilterConfig - : public Common::FactoryBase { + : public Common::ExceptionFreeFactoryBase< + envoy::extensions::filters::http::health_check::v3::HealthCheck> { public: - HealthCheckFilterConfig() : FactoryBase("envoy.filters.http.health_check") {} + HealthCheckFilterConfig() : ExceptionFreeFactoryBase("envoy.filters.http.health_check") {} private: - Http::FilterFactoryCb createFilterFactoryFromProtoTyped( + absl::StatusOr createFilterFactoryFromProtoTyped( const envoy::extensions::filters::http::health_check::v3::HealthCheck& proto_config, const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override; @@ -25,7 +26,7 @@ class HealthCheckFilterConfig const std::string& stats_prefix, Server::Configuration::ServerFactoryContext& context) override; - Http::FilterFactoryCb createFilterFactoryHelper( + absl::StatusOr createFilterFactoryHelper( const envoy::extensions::filters::http::health_check::v3::HealthCheck& proto_config, const std::string& stats_prefix, Server::Configuration::ServerFactoryContext& context, Stats::Scope& scope); diff --git a/source/extensions/filters/http/jwt_authn/filter_config.cc b/source/extensions/filters/http/jwt_authn/filter_config.cc index 6ef79f9a40ddf..1c3ef5869bc82 100644 --- a/source/extensions/filters/http/jwt_authn/filter_config.cc +++ b/source/extensions/filters/http/jwt_authn/filter_config.cc @@ -15,7 +15,8 @@ namespace JwtAuthn { FilterConfigImpl::FilterConfigImpl( envoy::extensions::filters::http::jwt_authn::v3::JwtAuthentication proto_config, - const std::string& stats_prefix, Server::Configuration::FactoryContext& context) + const std::string& stats_prefix, Server::Configuration::FactoryContext& context, + absl::Status& creation_status) : proto_config_(std::move(proto_config)), stats_(generateStats(stats_prefix, proto_config_.stat_prefix(), context.scope())), cm_(context.serverFactoryContext().clusterManager()), @@ -23,7 +24,10 @@ FilterConfigImpl::FilterConfigImpl( ENVOY_LOG(debug, "Loaded JwtAuthConfig: {}", proto_config_.DebugString()); - jwks_cache_ = JwksCache::create(proto_config_, context, Common::JwksFetcher::create, stats_); + auto jwks_cache_or = + JwksCache::create(proto_config_, context, Common::JwksFetcher::create, stats_); + SET_AND_RETURN_IF_NOT_OK(jwks_cache_or.status(), creation_status); + jwks_cache_ = std::move(jwks_cache_or.value()); // Validate provider URIs. // Note that the PGV well-known regex for URI is not implemented in C++, otherwise we could add a @@ -34,8 +38,9 @@ FilterConfigImpl::FilterConfigImpl( absl::string_view provider_uri = provider_value.remote_jwks().http_uri().uri(); Http::Utility::Url url; if (!url.initialize(provider_uri, /*is_connect=*/false)) { - throw EnvoyException(fmt::format("Provider '{}' has an invalid URI: '{}'", - std::get<0>(provider_pair), provider_uri)); + creation_status = absl::InvalidArgumentError(fmt::format( + "Provider '{}' has an invalid URI: '{}'", std::get<0>(provider_pair), provider_uri)); + return; } } } @@ -43,8 +48,9 @@ FilterConfigImpl::FilterConfigImpl( std::vector names; for (const auto& it : proto_config_.requirement_map()) { names.push_back(it.first); - name_verifiers_.emplace(it.first, - Verifier::create(it.second, proto_config_.providers(), *this)); + auto verifier_or = Verifier::create(it.second, proto_config_.providers(), *this); + SET_AND_RETURN_IF_NOT_OK(verifier_or.status(), creation_status); + name_verifiers_.emplace(it.first, std::move(verifier_or.value())); } // sort is just for unit-test since protobuf map order is not deterministic. std::sort(names.begin(), names.end()); @@ -52,32 +58,44 @@ FilterConfigImpl::FilterConfigImpl( for (const auto& rule : proto_config_.rules()) { switch (rule.requirement_type_case()) { - case RequirementRule::RequirementTypeCase::kRequires: - rule_pairs_.emplace_back( - Matcher::create(rule, context.serverFactoryContext()), - Verifier::create(rule.requires_(), proto_config_.providers(), *this)); + case RequirementRule::RequirementTypeCase::kRequires: { + auto matcher_or = Matcher::create(rule, context.serverFactoryContext()); + SET_AND_RETURN_IF_NOT_OK(matcher_or.status(), creation_status); + auto verifier_or = Verifier::create(rule.requires_(), proto_config_.providers(), *this); + SET_AND_RETURN_IF_NOT_OK(verifier_or.status(), creation_status); + rule_pairs_.emplace_back(std::move(matcher_or.value()), std::move(verifier_or.value())); break; + } case RequirementRule::RequirementTypeCase::kRequirementName: { // Use requirement_name to lookup requirement_map. auto map_it = proto_config_.requirement_map().find(rule.requirement_name()); if (map_it == proto_config_.requirement_map().end()) { - throw EnvoyException(fmt::format("Wrong requirement_name: {}. It should be one of [{}]", - rule.requirement_name(), all_requirement_names_)); + creation_status = absl::InvalidArgumentError( + fmt::format("Wrong requirement_name: {}. It should be one of [{}]", + rule.requirement_name(), all_requirement_names_)); + return; } - rule_pairs_.emplace_back(Matcher::create(rule, context.serverFactoryContext()), - Verifier::create(map_it->second, proto_config_.providers(), *this)); + auto matcher_or = Matcher::create(rule, context.serverFactoryContext()); + SET_AND_RETURN_IF_NOT_OK(matcher_or.status(), creation_status); + auto verifier_or = Verifier::create(map_it->second, proto_config_.providers(), *this); + SET_AND_RETURN_IF_NOT_OK(verifier_or.status(), creation_status); + rule_pairs_.emplace_back(std::move(matcher_or.value()), std::move(verifier_or.value())); } break; - case RequirementRule::RequirementTypeCase::REQUIREMENT_TYPE_NOT_SET: - rule_pairs_.emplace_back(Matcher::create(rule, context.serverFactoryContext()), nullptr); + case RequirementRule::RequirementTypeCase::REQUIREMENT_TYPE_NOT_SET: { + auto matcher_or = Matcher::create(rule, context.serverFactoryContext()); + SET_AND_RETURN_IF_NOT_OK(matcher_or.status(), creation_status); + rule_pairs_.emplace_back(std::move(matcher_or.value()), nullptr); break; } + } } if (proto_config_.has_filter_state_rules()) { filter_state_name_ = proto_config_.filter_state_rules().name(); for (const auto& it : proto_config_.filter_state_rules().requires_()) { - filter_state_verifiers_.emplace( - it.first, Verifier::create(it.second, proto_config_.providers(), *this)); + auto verifier_or = Verifier::create(it.second, proto_config_.providers(), *this); + SET_AND_RETURN_IF_NOT_OK(verifier_or.status(), creation_status); + filter_state_verifiers_.emplace(it.first, std::move(verifier_or.value())); } } } diff --git a/source/extensions/filters/http/jwt_authn/filter_config.h b/source/extensions/filters/http/jwt_authn/filter_config.h index d31ac5506828f..986182d79a764 100644 --- a/source/extensions/filters/http/jwt_authn/filter_config.h +++ b/source/extensions/filters/http/jwt_authn/filter_config.h @@ -66,7 +66,8 @@ class FilterConfigImpl : public Logger::Loggable, public AuthFactory { public: FilterConfigImpl(envoy::extensions::filters::http::jwt_authn::v3::JwtAuthentication proto_config, - const std::string& stats_prefix, Server::Configuration::FactoryContext& context); + const std::string& stats_prefix, Server::Configuration::FactoryContext& context, + absl::Status& creation_status); ~FilterConfigImpl() override = default; diff --git a/source/extensions/filters/http/jwt_authn/filter_factory.cc b/source/extensions/filters/http/jwt_authn/filter_factory.cc index dc06fd9fa79ff..1d9a820e1a5b7 100644 --- a/source/extensions/filters/http/jwt_authn/filter_factory.cc +++ b/source/extensions/filters/http/jwt_authn/filter_factory.cc @@ -22,29 +22,34 @@ using JwtVerify::Status; /** * Validate inline jwks, make sure they are the valid */ -void validateJwtConfig(const JwtAuthentication& proto_config, Api::Api& api) { +absl::Status validateJwtConfig(const JwtAuthentication& proto_config, Api::Api& api) { for (const auto& [name, provider] : proto_config.providers()) { - const auto inline_jwks = THROW_OR_RETURN_VALUE( - Config::DataSource::read(provider.local_jwks(), true, api), std::string); + auto inline_jwks_or = Config::DataSource::read(provider.local_jwks(), true, api); + RETURN_IF_NOT_OK_REF(inline_jwks_or.status()); + const auto& inline_jwks = inline_jwks_or.value(); if (!inline_jwks.empty()) { auto jwks_obj = Jwks::createFrom(inline_jwks, Jwks::JWKS); if (jwks_obj->getStatus() != Status::Ok) { - throw EnvoyException( + return absl::InvalidArgumentError( fmt::format("Provider '{}' in jwt_authn config has invalid local jwks: {}", name, JwtVerify::getStatusString(jwks_obj->getStatus()))); } } } + return absl::OkStatus(); } } // namespace -Http::FilterFactoryCb +absl::StatusOr FilterFactory::createFilterFactoryFromProtoTyped(const JwtAuthentication& proto_config, const std::string& prefix, Server::Configuration::FactoryContext& context) { - validateJwtConfig(proto_config, context.serverFactoryContext().api()); - auto filter_config = std::make_shared(proto_config, prefix, context); + RETURN_IF_NOT_OK(validateJwtConfig(proto_config, context.serverFactoryContext().api())); + absl::Status creation_status = absl::OkStatus(); + auto filter_config = + std::make_shared(proto_config, prefix, context, creation_status); + RETURN_IF_NOT_OK_REF(creation_status); return [filter_config](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamDecoderFilter(std::make_shared(filter_config)); }; diff --git a/source/extensions/filters/http/jwt_authn/filter_factory.h b/source/extensions/filters/http/jwt_authn/filter_factory.h index 2f9ec3ca412dd..1c75d267064fd 100644 --- a/source/extensions/filters/http/jwt_authn/filter_factory.h +++ b/source/extensions/filters/http/jwt_authn/filter_factory.h @@ -14,14 +14,14 @@ namespace JwtAuthn { /** * Config registration for jwt_authn filter. */ -class FilterFactory - : public Common::FactoryBase { +class FilterFactory : public Common::ExceptionFreeFactoryBase< + envoy::extensions::filters::http::jwt_authn::v3::JwtAuthentication, + envoy::extensions::filters::http::jwt_authn::v3::PerRouteConfig> { public: - FilterFactory() : FactoryBase("envoy.filters.http.jwt_authn") {} + FilterFactory() : ExceptionFreeFactoryBase("envoy.filters.http.jwt_authn") {} private: - Http::FilterFactoryCb createFilterFactoryFromProtoTyped( + absl::StatusOr createFilterFactoryFromProtoTyped( const envoy::extensions::filters::http::jwt_authn::v3::JwtAuthentication& proto_config, const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override; diff --git a/source/extensions/filters/http/jwt_authn/jwks_cache.cc b/source/extensions/filters/http/jwt_authn/jwks_cache.cc index 9c0ace3f54afe..6a782ddf1537f 100644 --- a/source/extensions/filters/http/jwt_authn/jwks_cache.cc +++ b/source/extensions/filters/http/jwt_authn/jwks_cache.cc @@ -35,7 +35,8 @@ using JwtVerify::Status; class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable { public: JwksDataImpl(const JwtProvider& jwt_provider, Server::Configuration::FactoryContext& context, - CreateJwksFetcherCb fetcher_cb, JwtAuthnFilterStats& stats) + CreateJwksFetcherCb fetcher_cb, JwtAuthnFilterStats& stats, + absl::Status& creation_status) : jwt_provider_(jwt_provider), time_source_(context.serverFactoryContext().timeSource()), tls_(context.serverFactoryContext().threadLocal()) { std::vector audiences; @@ -64,10 +65,10 @@ class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable(enable_jwt_cache, config, dispatcher.timeSource()); }); - const auto inline_jwks = - THROW_OR_RETURN_VALUE(Config::DataSource::read(jwt_provider_.local_jwks(), true, - context.serverFactoryContext().api()), - std::string); + auto inline_jwks_or = Config::DataSource::read(jwt_provider_.local_jwks(), true, + context.serverFactoryContext().api()); + SET_AND_RETURN_IF_NOT_OK(inline_jwks_or.status(), creation_status); + const auto& inline_jwks = inline_jwks_or.value(); if (!inline_jwks.empty()) { auto jwks = JwtVerify::Jwks::createFrom(inline_jwks, JwtVerify::Jwks::JWKS); if (jwks->getStatus() != Status::Ok) { @@ -84,8 +85,9 @@ class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable(provider, context, fetcher_fn, stats); + auto jwks_data = + std::make_unique(provider, context, fetcher_fn, stats, creation_status); + if (!creation_status.ok()) { + return; + } if (issuer_ptr_map_.find(provider.issuer()) == issuer_ptr_map_.end()) { issuer_ptr_map_.emplace(provider.issuer(), jwks_data.get()); } @@ -272,11 +291,14 @@ class JwksCacheImpl : public JwksCache { } // namespace -JwksCachePtr +absl::StatusOr JwksCache::create(const envoy::extensions::filters::http::jwt_authn::v3::JwtAuthentication& config, Server::Configuration::FactoryContext& context, CreateJwksFetcherCb fetcher_fn, JwtAuthnFilterStats& stats) { - return std::make_unique(config, context, fetcher_fn, stats); + absl::Status creation_status = absl::OkStatus(); + auto cache = std::make_unique(config, context, fetcher_fn, stats, creation_status); + RETURN_IF_NOT_OK_REF(creation_status); + return cache; } } // namespace JwtAuthn diff --git a/source/extensions/filters/http/jwt_authn/jwks_cache.h b/source/extensions/filters/http/jwt_authn/jwks_cache.h index 8e435a28c0ad3..b40d2e96648da 100644 --- a/source/extensions/filters/http/jwt_authn/jwks_cache.h +++ b/source/extensions/filters/http/jwt_authn/jwks_cache.h @@ -95,7 +95,7 @@ class JwksCache { virtual JwtAuthnFilterStats& stats() PURE; // Factory function to create an instance. - static JwksCachePtr + static absl::StatusOr create(const envoy::extensions::filters::http::jwt_authn::v3::JwtAuthentication& config, Server::Configuration::FactoryContext& context, CreateJwksFetcherCb fetcher_fn, JwtAuthnFilterStats& stats); diff --git a/source/extensions/filters/http/jwt_authn/matcher.cc b/source/extensions/filters/http/jwt_authn/matcher.cc index 1bdb41fabb9db..80b0a3621835a 100644 --- a/source/extensions/filters/http/jwt_authn/matcher.cc +++ b/source/extensions/filters/http/jwt_authn/matcher.cc @@ -192,8 +192,10 @@ class PathSeparatedPrefixMatcherImpl : public BaseMatcherImpl { class PathMatchPolicyMatcherImpl : public BaseMatcherImpl { public: PathMatchPolicyMatcherImpl(const RequirementRule& rule, - Server::Configuration::CommonFactoryContext& context) - : BaseMatcherImpl(rule, context), uri_template_matcher_(createUriTemplateMatcher(rule)) {} + Server::Configuration::CommonFactoryContext& context, + absl::Status& creation_status) + : BaseMatcherImpl(rule, context), + uri_template_matcher_(createUriTemplateMatcher(rule, creation_status)) {} bool matches(const Http::RequestHeaderMap& headers) const override { if (BaseMatcherImpl::matchRoute(headers) && @@ -209,7 +211,8 @@ class PathMatchPolicyMatcherImpl : public BaseMatcherImpl { private: const Router::PathMatcherSharedPtr uri_template_matcher_; - static Router::PathMatcherSharedPtr createUriTemplateMatcher(const RequirementRule& rule) { + static Router::PathMatcherSharedPtr createUriTemplateMatcher(const RequirementRule& rule, + absl::Status& creation_status) { auto& factory = Config::Utility::getAndCheckFactory( rule.match().path_match_policy()); ProtobufTypes::MessagePtr config = Envoy::Config::Utility::translateAnyToFactoryConfig( @@ -219,15 +222,16 @@ class PathMatchPolicyMatcherImpl : public BaseMatcherImpl { absl::StatusOr matcher = factory.createPathMatcher(*config); if (!matcher.ok()) { - throw EnvoyException(std::string(matcher.status().message())); + creation_status = matcher.status(); + return nullptr; } return matcher.value(); } }; -MatcherConstPtr Matcher::create(const RequirementRule& rule, - Server::Configuration::CommonFactoryContext& context) { +absl::StatusOr +Matcher::create(const RequirementRule& rule, Server::Configuration::CommonFactoryContext& context) { switch (rule.match().path_specifier_case()) { case RouteMatch::PathSpecifierCase::kPrefix: return std::make_unique(rule, context); @@ -240,7 +244,10 @@ MatcherConstPtr Matcher::create(const RequirementRule& rule, case RouteMatch::PathSpecifierCase::kPathSeparatedPrefix: return std::make_unique(rule, context); case RouteMatch::PathSpecifierCase::kPathMatchPolicy: { - return std::make_unique(rule, context); + absl::Status creation_status = absl::OkStatus(); + auto matcher = std::make_unique(rule, context, creation_status); + RETURN_IF_NOT_OK_REF(creation_status); + return matcher; } case RouteMatch::PathSpecifierCase::PATH_SPECIFIER_NOT_SET: break; // Fall through to PANIC. diff --git a/source/extensions/filters/http/jwt_authn/matcher.h b/source/extensions/filters/http/jwt_authn/matcher.h index e2ae8b052d351..94281f3d75c5c 100644 --- a/source/extensions/filters/http/jwt_authn/matcher.h +++ b/source/extensions/filters/http/jwt_authn/matcher.h @@ -34,7 +34,7 @@ class Matcher { * @param rule the proto rule match message. * @return the matcher instance. */ - static MatcherConstPtr + static absl::StatusOr create(const envoy::extensions::filters::http::jwt_authn::v3::RequirementRule& rule, Server::Configuration::CommonFactoryContext& context); }; diff --git a/source/extensions/filters/http/jwt_authn/verifier.cc b/source/extensions/filters/http/jwt_authn/verifier.cc index cc793bf1a795c..d984b1baf5215 100644 --- a/source/extensions/filters/http/jwt_authn/verifier.cc +++ b/source/extensions/filters/http/jwt_authn/verifier.cc @@ -224,7 +224,8 @@ class AllowMissingVerifierImpl : public BaseVerifierImpl { VerifierConstPtr innerCreate(const JwtRequirement& requirement, const Protobuf::Map& providers, - const AuthFactory& factory, const BaseVerifierImpl* parent); + const AuthFactory& factory, const BaseVerifierImpl* parent, + absl::Status& creation_status); // Base verifier for requires all or any. class BaseGroupVerifierImpl : public BaseVerifierImpl { @@ -251,7 +252,7 @@ class AnyVerifierImpl : public BaseGroupVerifierImpl { public: AnyVerifierImpl(const JwtRequirementOrList& or_list, const AuthFactory& factory, const Protobuf::Map& providers, - const BaseVerifierImpl* parent) + const BaseVerifierImpl* parent, absl::Status& creation_status) : BaseGroupVerifierImpl(parent) { for (const auto& it : or_list.requirements()) { @@ -263,7 +264,10 @@ class AnyVerifierImpl : public BaseGroupVerifierImpl { is_allow_missing_ = true; break; default: - verifiers_.emplace_back(innerCreate(it, providers, factory, this)); + verifiers_.emplace_back(innerCreate(it, providers, factory, this, creation_status)); + if (!creation_status.ok()) { + return; + } break; } } @@ -276,7 +280,7 @@ class AnyVerifierImpl : public BaseGroupVerifierImpl { } else { requirement.mutable_allow_missing(); } - verifiers_.emplace_back(innerCreate(requirement, providers, factory, this)); + verifiers_.emplace_back(innerCreate(requirement, providers, factory, this, creation_status)); } } @@ -331,10 +335,13 @@ class AllVerifierImpl : public BaseGroupVerifierImpl { AllVerifierImpl(const JwtRequirementAndList& and_list, const AuthFactory& factory, const Protobuf::Map& providers, // const Extractor& extractor_for_allow_fail, - const BaseVerifierImpl* parent) + const BaseVerifierImpl* parent, absl::Status& creation_status) : BaseGroupVerifierImpl(parent) { for (const auto& it : and_list.requirements()) { - verifiers_.emplace_back(innerCreate(it, providers, factory, this)); + verifiers_.emplace_back(innerCreate(it, providers, factory, this, creation_status)); + if (!creation_status.ok()) { + return; + } } } @@ -443,7 +450,8 @@ class ExtractOnlyWithoutValidationVerifierImpl : public BaseVerifierImpl { VerifierConstPtr innerCreate(const JwtRequirement& requirement, const Protobuf::Map& providers, - const AuthFactory& factory, const BaseVerifierImpl* parent) { + const AuthFactory& factory, const BaseVerifierImpl* parent, + absl::Status& creation_status) { std::string provider_name; std::vector audiences; switch (requirement.requires_type_case()) { @@ -457,11 +465,11 @@ VerifierConstPtr innerCreate(const JwtRequirement& requirement, provider_name = requirement.provider_and_audiences().provider_name(); break; case JwtRequirement::RequiresTypeCase::kRequiresAny: - return std::make_unique(requirement.requires_any(), factory, providers, - parent); + return std::make_unique(requirement.requires_any(), factory, providers, parent, + creation_status); case JwtRequirement::RequiresTypeCase::kRequiresAll: - return std::make_unique(requirement.requires_all(), factory, providers, - parent); + return std::make_unique(requirement.requires_all(), factory, providers, parent, + creation_status); case JwtRequirement::RequiresTypeCase::kAllowMissingOrFailed: return std::make_unique(factory, getAllProvidersAsList(providers), parent); @@ -478,7 +486,9 @@ VerifierConstPtr innerCreate(const JwtRequirement& requirement, const auto& it = providers.find(provider_name); if (it == providers.end()) { - throw EnvoyException(fmt::format("Required provider ['{}'] is not configured.", provider_name)); + creation_status = absl::InvalidArgumentError( + fmt::format("Required provider ['{}'] is not configured.", provider_name)); + return nullptr; } if (audiences.empty()) { return std::make_unique(provider_name, factory, it->second, parent); @@ -494,10 +504,14 @@ ContextSharedPtr Verifier::createContext(Http::RequestHeaderMap& headers, return std::make_shared(headers, parent_span, *callback); } -VerifierConstPtr Verifier::create(const JwtRequirement& requirement, - const Protobuf::Map& providers, - const AuthFactory& factory) { - return innerCreate(requirement, providers, factory, nullptr); +absl::StatusOr +Verifier::create(const JwtRequirement& requirement, + const Protobuf::Map& providers, + const AuthFactory& factory) { + absl::Status creation_status = absl::OkStatus(); + auto verifier = innerCreate(requirement, providers, factory, nullptr, creation_status); + RETURN_IF_NOT_OK_REF(creation_status); + return verifier; } } // namespace JwtAuthn diff --git a/source/extensions/filters/http/jwt_authn/verifier.h b/source/extensions/filters/http/jwt_authn/verifier.h index c5c24c81265d9..866c345e97bc1 100644 --- a/source/extensions/filters/http/jwt_authn/verifier.h +++ b/source/extensions/filters/http/jwt_authn/verifier.h @@ -85,7 +85,7 @@ class Verifier { virtual void verify(ContextSharedPtr context) const PURE; // Factory method for creating verifiers. - static VerifierConstPtr create( + static absl::StatusOr create( const envoy::extensions::filters::http::jwt_authn::v3::JwtRequirement& requirement, const Protobuf::Map& providers, diff --git a/source/extensions/filters/http/kill_request/kill_request_config.cc b/source/extensions/filters/http/kill_request/kill_request_config.cc index 50862ada1e3bc..8928689bc2b34 100644 --- a/source/extensions/filters/http/kill_request/kill_request_config.cc +++ b/source/extensions/filters/http/kill_request/kill_request_config.cc @@ -11,7 +11,7 @@ namespace Extensions { namespace HttpFilters { namespace KillRequest { -Http::FilterFactoryCb KillRequestFilterFactory::createFilterFactoryFromProtoTyped( +absl::StatusOr KillRequestFilterFactory::createFilterFactoryFromProtoTyped( const envoy::extensions::filters::http::kill_request::v3::KillRequest& proto_config, const std::string&, Server::Configuration::FactoryContext& context) { return [proto_config, &context](Http::FilterChainFactoryCallbacks& callbacks) -> void { diff --git a/source/extensions/filters/http/kill_request/kill_request_config.h b/source/extensions/filters/http/kill_request/kill_request_config.h index 4a9a6dd5bd2a0..02ebf135d3386 100644 --- a/source/extensions/filters/http/kill_request/kill_request_config.h +++ b/source/extensions/filters/http/kill_request/kill_request_config.h @@ -14,12 +14,13 @@ namespace KillRequest { * Config registration for KillRequestFilter. @see NamedHttpFilterConfigFactory. */ class KillRequestFilterFactory - : public Common::FactoryBase { + : public Common::ExceptionFreeFactoryBase< + envoy::extensions::filters::http::kill_request::v3::KillRequest> { public: - KillRequestFilterFactory() : FactoryBase("envoy.filters.http.kill_request") {} + KillRequestFilterFactory() : ExceptionFreeFactoryBase("envoy.filters.http.kill_request") {} private: - Http::FilterFactoryCb createFilterFactoryFromProtoTyped( + absl::StatusOr createFilterFactoryFromProtoTyped( const envoy::extensions::filters::http::kill_request::v3::KillRequest& proto_config, const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override; diff --git a/test/extensions/filters/http/health_check/BUILD b/test/extensions/filters/http/health_check/BUILD index c00b5ef98fcf3..ef00a284f8c5f 100644 --- a/test/extensions/filters/http/health_check/BUILD +++ b/test/extensions/filters/http/health_check/BUILD @@ -34,6 +34,7 @@ envoy_extension_cc_test( deps = [ "//source/extensions/filters/http/health_check:config", "//test/mocks/server:factory_context_mocks", + "//test/test_common:status_utility_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/route/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/http/health_check/v3:pkg_cc_proto", diff --git a/test/extensions/filters/http/health_check/config_test.cc b/test/extensions/filters/http/health_check/config_test.cc index bde48d69b07bc..f36d5582bff96 100644 --- a/test/extensions/filters/http/health_check/config_test.cc +++ b/test/extensions/filters/http/health_check/config_test.cc @@ -7,6 +7,7 @@ #include "source/extensions/filters/http/health_check/config.h" #include "test/mocks/server/factory_context.h" +#include "test/test_common/status_utility.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -21,6 +22,8 @@ namespace HttpFilters { namespace HealthCheck { namespace { +using StatusHelpers::HasStatus; + TEST(HealthCheckFilterConfig, HealthCheckFilter) { const std::string yaml_string = R"EOF( pass_through_mode: true @@ -71,10 +74,9 @@ TEST(HealthCheckFilterConfig, FailsWhenNotPassThroughButTimeoutSetYaml) { HealthCheckFilterConfig factory; NiceMock context; - EXPECT_THROW(factory.createFilterFactoryFromProto(proto_config, "dummy_stats_prefix", context) - .status() - .IgnoreError(), - EnvoyException); + EXPECT_THAT(factory.createFilterFactoryFromProto(proto_config, "dummy_stats_prefix", context), + HasStatus(absl::StatusCode::kInvalidArgument, + "cache_time_ms must not be set when path_through_mode is disabled")); } TEST(HealthCheckFilterConfig, NotFailingWhenNotPassThroughAndTimeoutNotSetYaml) { @@ -109,11 +111,10 @@ TEST(HealthCheckFilterConfig, FailsWhenNotPassThroughButTimeoutSetProto) { header.set_name(":path"); header.mutable_string_match()->set_exact("foo"); - EXPECT_THROW( - healthCheckFilterConfig.createFilterFactoryFromProto(config, "dummy_stats_prefix", context) - .status() - .IgnoreError(), - EnvoyException); + EXPECT_THAT( + healthCheckFilterConfig.createFilterFactoryFromProto(config, "dummy_stats_prefix", context), + HasStatus(absl::StatusCode::kInvalidArgument, + "cache_time_ms must not be set when path_through_mode is disabled")); } TEST(HealthCheckFilterConfig, NotFailingWhenNotPassThroughAndTimeoutNotSetProto) { diff --git a/test/extensions/filters/http/jwt_authn/BUILD b/test/extensions/filters/http/jwt_authn/BUILD index afd14e7703126..3749624ae3bf1 100644 --- a/test/extensions/filters/http/jwt_authn/BUILD +++ b/test/extensions/filters/http/jwt_authn/BUILD @@ -69,6 +69,7 @@ envoy_extension_cc_test( "//source/extensions/filters/http/jwt_authn:config", "//test/extensions/filters/http/jwt_authn:test_common_lib", "//test/mocks/server:factory_context_mocks", + "//test/test_common:status_utility_lib", "//test/test_common:test_runtime_lib", "@envoy_api//envoy/extensions/filters/http/jwt_authn/v3:pkg_cc_proto", ], @@ -95,6 +96,7 @@ envoy_extension_cc_test( "//source/extensions/filters/http/jwt_authn:config", "//test/extensions/filters/http/jwt_authn:test_common_lib", "//test/mocks/server:factory_context_mocks", + "//test/test_common:status_utility_lib", "@envoy_api//envoy/extensions/filters/http/jwt_authn/v3:pkg_cc_proto", ], ) @@ -220,6 +222,7 @@ envoy_extension_cc_test( "//source/extensions/filters/http/jwt_authn:filter_config_lib", "//source/extensions/filters/http/jwt_authn:matchers_lib", "//test/mocks/server:factory_context_mocks", + "//test/test_common:status_utility_lib", "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/extensions/filters/http/jwt_authn/v3:pkg_cc_proto", diff --git a/test/extensions/filters/http/jwt_authn/all_verifier_test.cc b/test/extensions/filters/http/jwt_authn/all_verifier_test.cc index 9d76e57a71c5a..78b7584986aa8 100644 --- a/test/extensions/filters/http/jwt_authn/all_verifier_test.cc +++ b/test/extensions/filters/http/jwt_authn/all_verifier_test.cc @@ -75,9 +75,15 @@ class AllVerifierTest : public testing::Test { } void createVerifier() { - filter_config_ = std::make_shared(proto_config_, "", mock_factory_ctx_); - verifier_ = Verifier::create(proto_config_.rules(0).requires_(), proto_config_.providers(), - *filter_config_); + absl::Status creation_status = absl::OkStatus(); + filter_config_ = + std::make_shared(proto_config_, "", mock_factory_ctx_, creation_status); + ASSERT_TRUE(creation_status.ok()); + + auto verifier_or = Verifier::create(proto_config_.rules(0).requires_(), + proto_config_.providers(), *filter_config_); + ASSERT_TRUE(verifier_or.ok()); + verifier_ = std::move(verifier_or).value(); } void modifyRequirement(const std::string& yaml) { diff --git a/test/extensions/filters/http/jwt_authn/authenticator_test.cc b/test/extensions/filters/http/jwt_authn/authenticator_test.cc index 0b190181934a2..460307f147865 100644 --- a/test/extensions/filters/http/jwt_authn/authenticator_test.cc +++ b/test/extensions/filters/http/jwt_authn/authenticator_test.cc @@ -47,7 +47,10 @@ class AuthenticatorTest : public testing::Test { JwtVerify::CheckAudience* check_audience = nullptr, const std::optional& provider = std::make_optional(ProviderName), bool allow_failed = false, bool allow_missing = false) { - filter_config_ = std::make_unique(proto_config_, "", mock_factory_ctx_); + absl::Status creation_status = absl::OkStatus(); + filter_config_ = + std::make_shared(proto_config_, "", mock_factory_ctx_, creation_status); + ASSERT_TRUE(creation_status.ok()) << creation_status; raw_fetcher_ = new MockJwksFetcher; fetcher_.reset(raw_fetcher_); auth_ = Authenticator::create( diff --git a/test/extensions/filters/http/jwt_authn/extractor_test.cc b/test/extensions/filters/http/jwt_authn/extractor_test.cc index 9bf2c0d2abb87..3aa4fcfe0886b 100644 --- a/test/extensions/filters/http/jwt_authn/extractor_test.cc +++ b/test/extensions/filters/http/jwt_authn/extractor_test.cc @@ -309,6 +309,17 @@ TEST_F(ExtractorTest, TestPrefixHeaderFlexibleMatch3) { EXPECT_EQ(tokens[1]->token(), "and0X3Rva2Vu\"}"); } +// Test that when the value_prefix is present but is followed by no Base64Url-legal +// characters, extractJWT returns the whole value as-is. +TEST_F(ExtractorTest, TestPrefixHeaderNoTokenAfterPrefix) { + auto headers = TestRequestHeaderMapImpl{{"prefix-header", "AAA "}}; + auto tokens = extractor_->extract(headers); + EXPECT_EQ(tokens.size(), 1); + + EXPECT_TRUE(tokens[0]->isIssuerAllowed("issuer5")); + EXPECT_EQ(tokens[0]->token(), "AAA "); +} + // Test extracting token from the custom query parameter: "token_param" TEST_F(ExtractorTest, TestCustomParamToken) { auto headers = TestRequestHeaderMapImpl{{":path", "/path?token_param=jwt_token"}}; diff --git a/test/extensions/filters/http/jwt_authn/filter_config_test.cc b/test/extensions/filters/http/jwt_authn/filter_config_test.cc index 577e300955adf..79079bb764814 100644 --- a/test/extensions/filters/http/jwt_authn/filter_config_test.cc +++ b/test/extensions/filters/http/jwt_authn/filter_config_test.cc @@ -6,6 +6,7 @@ #include "test/extensions/filters/http/jwt_authn/test_common.h" #include "test/mocks/server/factory_context.h" +#include "test/test_common/status_utility.h" #include "test/test_common/test_runtime.h" #include "gmock/gmock.h" @@ -21,6 +22,8 @@ namespace HttpFilters { namespace JwtAuthn { namespace { +using StatusHelpers::HasStatus; + TEST(HttpJwtAuthnFilterConfigTest, FindByMatch) { const char config[] = R"( providers: @@ -39,7 +42,9 @@ TEST(HttpJwtAuthnFilterConfigTest, FindByMatch) { TestUtility::loadFromYaml(config, proto_config); NiceMock context; - auto filter_conf = std::make_unique(proto_config, "", context); + absl::Status creation_status = absl::OkStatus(); + auto filter_conf = std::make_unique(proto_config, "", context, creation_status); + ASSERT_TRUE(creation_status.ok()); StreamInfo::FilterStateImpl filter_state(StreamInfo::FilterState::LifeSpan::FilterChain); EXPECT_TRUE(filter_conf->findVerifier( @@ -73,7 +78,9 @@ TEST(HttpJwtAuthnFilterConfigTest, FindByMatchDisabled) { TestUtility::loadFromYaml(config, proto_config); NiceMock context; - auto filter_conf = std::make_unique(proto_config, "", context); + absl::Status creation_status = absl::OkStatus(); + auto filter_conf = std::make_unique(proto_config, "", context, creation_status); + ASSERT_TRUE(creation_status.ok()); StreamInfo::FilterStateImpl filter_state(StreamInfo::FilterState::LifeSpan::FilterChain); EXPECT_TRUE(filter_conf->findVerifier( @@ -103,8 +110,10 @@ TEST(HttpJwtAuthnFilterConfigTest, FindByMatchWrongRequirementName) { TestUtility::loadFromYaml(config, proto_config); NiceMock context; - EXPECT_THROW_WITH_MESSAGE(FilterConfigImpl(proto_config, "", context), EnvoyException, - "Wrong requirement_name: rr. It should be one of [r1]"); + absl::Status creation_status = absl::OkStatus(); + FilterConfigImpl filter_config(proto_config, "", context, creation_status); + EXPECT_THAT(creation_status, HasStatus(absl::StatusCode::kInvalidArgument, + "Wrong requirement_name: rr. It should be one of [r1]")); } TEST(HttpJwtAuthnFilterConfigTest, FindByMatchRequirementName) { @@ -136,7 +145,9 @@ TEST(HttpJwtAuthnFilterConfigTest, FindByMatchRequirementName) { TestUtility::loadFromYaml(config, proto_config); NiceMock context; - auto filter_conf = std::make_unique(proto_config, "", context); + absl::Status creation_status = absl::OkStatus(); + auto filter_conf = std::make_unique(proto_config, "", context, creation_status); + ASSERT_TRUE(creation_status.ok()); StreamInfo::FilterStateImpl filter_state(StreamInfo::FilterState::LifeSpan::FilterChain); EXPECT_TRUE(filter_conf->findVerifier( @@ -170,7 +181,9 @@ TEST(HttpJwtAuthnFilterConfigTest, VerifyTLSLifetime) { JwtAuthentication proto_config; TestUtility::loadFromYaml(config, proto_config); - auto filter_conf = std::make_unique(proto_config, "", context); + absl::Status creation_status = absl::OkStatus(); + auto filter_conf = std::make_unique(proto_config, "", context, creation_status); + ASSERT_TRUE(creation_status.ok()); // Even though filter_conf is now de-allocated, using a reference to it might still work, as its // memory was not cleared. This leads to a false positive in this test when run normally. The @@ -209,7 +222,9 @@ TEST(HttpJwtAuthnFilterConfigTest, FindByFilterState) { TestUtility::loadFromYaml(config, proto_config); NiceMock context; - auto filter_conf = std::make_unique(proto_config, "", context); + absl::Status creation_status = absl::OkStatus(); + auto filter_conf = std::make_unique(proto_config, "", context, creation_status); + ASSERT_TRUE(creation_status.ok()); // Empty filter_state StreamInfo::FilterStateImpl filter_state1(StreamInfo::FilterState::LifeSpan::FilterChain); @@ -261,7 +276,9 @@ TEST(HttpJwtAuthnFilterConfigTest, FindByRequiremenMap) { TestUtility::loadFromYaml(config, proto_config); NiceMock context; - auto filter_conf = std::make_unique(proto_config, "", context); + absl::Status creation_status = absl::OkStatus(); + auto filter_conf = std::make_unique(proto_config, "", context, creation_status); + ASSERT_TRUE(creation_status.ok()); PerRouteConfig per_route; const Verifier* verifier; @@ -314,8 +331,10 @@ TEST(HttpJwtAuthnFilterConfigTest, RemoteJwksDurationVeryBig) { TestUtility::loadFromYaml(config, proto_config); NiceMock context; - EXPECT_THAT_THROWS_MESSAGE(FilterConfigImpl(proto_config, "", context), EnvoyException, - HasSubstr("Duration out-of-range")); + absl::Status creation_status = absl::OkStatus(); + FilterConfigImpl filter_config(proto_config, "", context, creation_status); + EXPECT_THAT(creation_status, + HasStatus(absl::StatusCode::kOutOfRange, HasSubstr("Duration out-of-range"))); } TEST(HttpJwtAuthnFilterConfigTest, RemoteJwksInvalidUri) { @@ -333,8 +352,10 @@ TEST(HttpJwtAuthnFilterConfigTest, RemoteJwksInvalidUri) { TestUtility::loadFromYaml(config, proto_config); NiceMock context; - EXPECT_THAT_THROWS_MESSAGE(FilterConfigImpl(proto_config, "", context), EnvoyException, - HasSubstr("invalid URI")); + absl::Status creation_status = absl::OkStatus(); + FilterConfigImpl filter_config(proto_config, "", context, creation_status); + EXPECT_THAT(creation_status, + HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr("invalid URI"))); } TEST(HttpJwtAuthnFilterConfigTest, RemoteJwksValidUri) { @@ -352,7 +373,9 @@ TEST(HttpJwtAuthnFilterConfigTest, RemoteJwksValidUri) { TestUtility::loadFromYaml(config, proto_config); NiceMock context; - FilterConfigImpl(proto_config, "", context); + absl::Status creation_status = absl::OkStatus(); + FilterConfigImpl filter_config(proto_config, "", context, creation_status); + EXPECT_TRUE(creation_status.ok()); } TEST(HttpJwtAuthnFilterConfigTest, RemoteJwksAsyncFetchRefetchDurationVeryBig) { @@ -372,8 +395,10 @@ TEST(HttpJwtAuthnFilterConfigTest, RemoteJwksAsyncFetchRefetchDurationVeryBig) { TestUtility::loadFromYaml(config, proto_config); NiceMock context; - EXPECT_THAT_THROWS_MESSAGE(FilterConfigImpl(proto_config, "", context), EnvoyException, - HasSubstr("Duration out-of-range")); + absl::Status creation_status = absl::OkStatus(); + FilterConfigImpl filter_config(proto_config, "", context, creation_status); + EXPECT_THAT(creation_status, + HasStatus(absl::StatusCode::kOutOfRange, HasSubstr("Duration out-of-range"))); } TEST(HttpJwtAuthnFilterConfigTest, RemoteJwksWithRetryPolicy) { @@ -397,7 +422,9 @@ TEST(HttpJwtAuthnFilterConfigTest, RemoteJwksWithRetryPolicy) { TestUtility::loadFromYaml(config, proto_config); NiceMock context; - auto filter_conf = std::make_unique(proto_config, "", context); + absl::Status creation_status = absl::OkStatus(); + auto filter_conf = std::make_unique(proto_config, "", context, creation_status); + ASSERT_TRUE(creation_status.ok()); auto* jwks_data = filter_conf->getJwksCache().findByIssuer("issuer1"); EXPECT_NE(nullptr, jwks_data); EXPECT_NE(nullptr, jwks_data->retryPolicy()); @@ -424,9 +451,12 @@ TEST(HttpJwtAuthnFilterConfigTest, RemoteJwksWithInvalidRetryPolicy) { TestUtility::loadFromYaml(config, proto_config); NiceMock context; - EXPECT_THAT_THROWS_MESSAGE( - FilterConfigImpl(proto_config, "", context), EnvoyException, - HasSubstr("max_interval must be greater than or equal to the base_interval")); + absl::Status creation_status = absl::OkStatus(); + FilterConfigImpl filter(proto_config, "", context, creation_status); + EXPECT_THAT( + creation_status, + HasStatus(absl::StatusCode::kInvalidArgument, + HasSubstr("max_interval must be greater than or equal to the base_interval"))); } } // namespace diff --git a/test/extensions/filters/http/jwt_authn/filter_factory_test.cc b/test/extensions/filters/http/jwt_authn/filter_factory_test.cc index ad02f3f5ebde9..2bbe987ffa15b 100644 --- a/test/extensions/filters/http/jwt_authn/filter_factory_test.cc +++ b/test/extensions/filters/http/jwt_authn/filter_factory_test.cc @@ -6,6 +6,7 @@ #include "test/extensions/filters/http/jwt_authn/test_common.h" #include "test/mocks/server/factory_context.h" +#include "test/test_common/status_utility.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -20,6 +21,8 @@ namespace HttpFilters { namespace JwtAuthn { namespace { +using StatusHelpers::HasStatus; + TEST(HttpJwtAuthnFilterFactoryTest, GoodRemoteJwks) { FilterFactory factory; ProtobufTypes::MessagePtr proto_config = factory.createEmptyConfigProto(); @@ -57,8 +60,9 @@ TEST(HttpJwtAuthnFilterFactoryTest, BadLocalJwks) { NiceMock context; FilterFactory factory; - EXPECT_THROW(factory.createFilterFactoryFromProto(proto_config, "stats", context).value(), - EnvoyException); + auto factory_cb_or = factory.createFilterFactoryFromProto(proto_config, "stats", context); + EXPECT_THAT(factory_cb_or, HasStatus(absl::StatusCode::kInvalidArgument, + ::testing::HasSubstr("invalid local jwks"))); } TEST(HttpJwtAuthnFilterFactoryTest, ProviderWithoutIssuer) { diff --git a/test/extensions/filters/http/jwt_authn/group_verifier_test.cc b/test/extensions/filters/http/jwt_authn/group_verifier_test.cc index 6b0e3a41f85f9..789bd7ed27531 100644 --- a/test/extensions/filters/http/jwt_authn/group_verifier_test.cc +++ b/test/extensions/filters/http/jwt_authn/group_verifier_test.cc @@ -76,8 +76,10 @@ class GroupVerifierTest : public testing::Test { const std::optional& provider, bool, bool) { return std::move(mock_auths_[provider ? provider.value() : allowfailed]); })); - verifier_ = Verifier::create(proto_config_.rules(0).requires_(), proto_config_.providers(), - mock_factory_); + auto verifier_or = Verifier::create(proto_config_.rules(0).requires_(), + proto_config_.providers(), mock_factory_); + ASSERT_TRUE(verifier_or.ok()); + verifier_ = std::move(verifier_or).value(); } void createSyncMockAuthsAndVerifier(const StatusMap& statuses) { for (const auto& it : statuses) { diff --git a/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc b/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc index aada401007472..a7048a8e924db 100644 --- a/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc +++ b/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc @@ -42,7 +42,9 @@ class JwksCacheTest : public testing::Test { void setupCache(const std::string& config_str) { TestUtility::loadFromYaml(config_str, config_); - cache_ = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_); + auto cache_or = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_); + ASSERT_TRUE(cache_or.ok()) << cache_or.status(); + cache_ = std::move(cache_or).value(); } JwtAuthentication config_; @@ -96,8 +98,9 @@ TEST_F(JwksCacheTest, TestSetRemoteJwks) { auto& provider0 = (*config_.mutable_providers())[std::string(ProviderName)]; // Set cache_duration to 1 second to test expiration provider0.mutable_remote_jwks()->mutable_cache_duration()->set_seconds(1); - cache_ = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_); - + auto cache_or = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_); + ASSERT_TRUE(cache_or.ok()) << cache_or.status(); + cache_ = std::move(cache_or.value()); auto jwks = cache_->findByIssuer("https://example.com"); EXPECT_TRUE(jwks->getJwksObj() == nullptr); @@ -115,8 +118,9 @@ TEST_F(JwksCacheTest, TestSetRemoteJwksWithDefaultCacheDuration) { auto& provider0 = (*config_.mutable_providers())[std::string(ProviderName)]; // Clear cache_duration to use default one. provider0.mutable_remote_jwks()->clear_cache_duration(); - cache_ = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_); - + auto cache_or = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_); + ASSERT_TRUE(cache_or.ok()) << cache_or.status(); + cache_ = std::move(cache_or.value()); auto jwks = cache_->findByIssuer("https://example.com"); EXPECT_TRUE(jwks->getJwksObj() == nullptr); @@ -132,8 +136,9 @@ TEST_F(JwksCacheTest, TestGoodInlineJwks) { auto local_jwks = provider0.mutable_local_jwks(); local_jwks->set_inline_string(PublicKey); - cache_ = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_); - + auto cache_or = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_); + ASSERT_TRUE(cache_or.ok()) << cache_or.status(); + cache_ = std::move(cache_or.value()); auto jwks = cache_->findByIssuer("https://example.com"); EXPECT_FALSE(jwks->getJwksObj() == nullptr); EXPECT_FALSE(jwks->isExpired()); @@ -146,8 +151,9 @@ TEST_F(JwksCacheTest, TestBadInlineJwks) { auto local_jwks = provider0.mutable_local_jwks(); local_jwks->set_inline_string("BAD-JWKS"); - cache_ = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_); - + auto cache_or = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_); + ASSERT_TRUE(cache_or.ok()) << cache_or.status(); + cache_ = std::move(cache_or.value()); auto jwks = cache_->findByIssuer("https://example.com"); EXPECT_TRUE(jwks->getJwksObj() == nullptr); } diff --git a/test/extensions/filters/http/jwt_authn/jwt_authn_fuzz_test.cc b/test/extensions/filters/http/jwt_authn/jwt_authn_fuzz_test.cc index ec1f85ded6746..9edad69bf7046 100644 --- a/test/extensions/filters/http/jwt_authn/jwt_authn_fuzz_test.cc +++ b/test/extensions/filters/http/jwt_authn/jwt_authn_fuzz_test.cc @@ -111,11 +111,15 @@ DEFINE_PROTO_FUZZER(const JwtAuthnFuzzInput& input) { } std::shared_ptr filter_config; - try { - filter_config = std::make_shared(input.config(), "", mock_factory_ctx); - } catch (const EnvoyException& e) { - ENVOY_LOG_MISC(debug, "EnvoyException during filter config construction: {}", e.what()); - return; + { + absl::Status creation_status = absl::OkStatus(); + filter_config = + std::make_shared(input.config(), "", mock_factory_ctx, creation_status); + if (!creation_status.ok()) { + ENVOY_LOG_MISC(debug, "Invalid filter config during construction: {}", + creation_status.message()); + return; + } } // Simulate multiple calls to execute jwt_cache and jwks_cache codes diff --git a/test/extensions/filters/http/jwt_authn/matcher_test.cc b/test/extensions/filters/http/jwt_authn/matcher_test.cc index 916b8e993a4cd..6721cd3850bbe 100644 --- a/test/extensions/filters/http/jwt_authn/matcher_test.cc +++ b/test/extensions/filters/http/jwt_authn/matcher_test.cc @@ -22,7 +22,9 @@ class MatcherTest : public testing::Test { MatcherConstPtr createMatcher(const char* config) { RequirementRule rule; TestUtility::loadFromYaml(config, rule); - return Matcher::create(rule, context_); + auto matcher_or = Matcher::create(rule, context_); + EXPECT_TRUE(matcher_or.ok()) << matcher_or.status(); + return std::move(matcher_or).value(); } NiceMock context_; @@ -264,6 +266,9 @@ TEST_F(MatcherTest, TestMatchPathMatchPolicy) { MatcherConstPtr matcher = createMatcher(config); auto headers = TestRequestHeaderMapImpl{{":path", "/bar/test/foo"}}; EXPECT_TRUE(matcher->matches(headers)); + + auto non_matching_headers = TestRequestHeaderMapImpl{{":path", "/bar/test/baz"}}; + EXPECT_FALSE(matcher->matches(non_matching_headers)); } TEST_F(MatcherTest, TestMatchPathMatchPolicyError) { diff --git a/test/extensions/filters/http/jwt_authn/provider_verifier_test.cc b/test/extensions/filters/http/jwt_authn/provider_verifier_test.cc index 2b94a824114b0..c8beaf898107b 100644 --- a/test/extensions/filters/http/jwt_authn/provider_verifier_test.cc +++ b/test/extensions/filters/http/jwt_authn/provider_verifier_test.cc @@ -6,6 +6,7 @@ #include "test/extensions/filters/http/jwt_authn/mock.h" #include "test/extensions/filters/http/jwt_authn/test_common.h" #include "test/mocks/server/factory_context.h" +#include "test/test_common/status_utility.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -21,6 +22,8 @@ namespace HttpFilters { namespace JwtAuthn { namespace { +using StatusHelpers::HasStatus; + using JwtVerify::Status; Protobuf::Struct getExpectedPayload(const std::string& name) { @@ -40,9 +43,14 @@ class ProviderVerifierTest : public testing::Test { } void createVerifier() { - filter_config_ = std::make_unique(proto_config_, "", mock_factory_ctx_); - verifier_ = Verifier::create(proto_config_.rules(0).requires_(), proto_config_.providers(), - *filter_config_); + absl::Status creation_status = absl::OkStatus(); + filter_config_ = + std::make_shared(proto_config_, "", mock_factory_ctx_, creation_status); + ASSERT_TRUE(creation_status.ok()); + auto verifier_or = Verifier::create(proto_config_.rules(0).requires_(), + proto_config_.providers(), *filter_config_); + ASSERT_TRUE(verifier_or.ok()); + verifier_ = std::move(verifier_or).value(); } JwtAuthentication proto_config_; @@ -250,12 +258,15 @@ TEST_F(ProviderVerifierTest, TestRequiresProviderWithAudiences) { verifier_->verify(Verifier::createContext(headers, parent_span_, &mock_cb_)); } -// This test verifies that requirement referencing nonexistent provider will throw exception +// This test verifies that requirement referencing nonexistent provider will fail config creation. TEST_F(ProviderVerifierTest, TestRequiresNonexistentProvider) { TestUtility::loadFromYaml(ExampleConfig, proto_config_); proto_config_.mutable_rules(0)->mutable_requires_()->set_provider_name("nosuchprovider"); - EXPECT_THROW(FilterConfigImpl(proto_config_, "", mock_factory_ctx_), EnvoyException); + absl::Status creation_status = absl::OkStatus(); + FilterConfigImpl filter_config(proto_config_, "", mock_factory_ctx_, creation_status); + EXPECT_THAT(creation_status, HasStatus(absl::StatusCode::kInvalidArgument, + ::testing::HasSubstr("Required provider"))); } class ProviderVerifiersJwtCacheTest : public ProviderVerifierTest, @@ -284,8 +295,9 @@ TEST_P(ProviderVerifiersJwtCacheTest, TestRequirementsWithAudiences) { auto* provider_and_audiences = require2.mutable_provider_and_audiences(); provider_and_audiences->set_provider_name("example_provider"); provider_and_audiences->add_audiences("other_service"); - VerifierConstPtr verifier2 = - Verifier::create(require2, proto_config_.providers(), *filter_config_); + auto verifier2_or = Verifier::create(require2, proto_config_.providers(), *filter_config_); + ASSERT_TRUE(verifier2_or.ok()) << verifier2_or.status(); + auto verifier2 = std::move(verifier2_or).value(); MockUpstream mock_pubkey(mock_factory_ctx_.server_factory_context_.cluster_manager_, PublicKey);