diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 1cee041d58cd9..5f0d3e3689d57 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -472,6 +472,13 @@ def _com_google_absl(): actual = "@com_google_absl//absl/debugging:symbolize", ) + # Require abseil_time as an indirect dependency as it is needed by the + # direct dependency jwt_verify_lib. + native.bind( + name = "abseil_time", + actual = "@com_google_absl//absl/time:time", + ) + def _com_google_protobuf(): _repository_impl("com_google_protobuf") diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index bc6718e53cdf2..d7f770952b9b4 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -75,7 +75,7 @@ REPOSITORY_LOCATIONS = dict( remote = "https://github.com/google/googleapis", ), com_github_google_jwt_verify = dict( - commit = "4eb9e96485b71e00d43acc7207501caafb085b4a", # 2018-06-11 + commit = "66792a057ec54e4b75c6a2eeda4e98220bd12a9a", # 2018-08-17 remote = "https://github.com/google/jwt_verify_lib", ), com_github_nodejs_http_parser = dict( diff --git a/source/extensions/filters/http/common/BUILD b/source/extensions/filters/http/common/BUILD index ad080a666e93c..f2b17633bc88c 100644 --- a/source/extensions/filters/http/common/BUILD +++ b/source/extensions/filters/http/common/BUILD @@ -23,3 +23,17 @@ envoy_cc_library( "//include/envoy/server:filter_config_interface", ], ) + +envoy_cc_library( + name = "jwks_fetcher_lib", + srcs = ["jwks_fetcher.cc"], + hdrs = ["jwks_fetcher.h"], + external_deps = [ + "jwt_verify_lib", + ], + deps = [ + "//include/envoy/upstream:cluster_manager_interface", + "//source/common/http:utility_lib", + "@envoy_api//envoy/api/v2/core:http_uri_cc", + ], +) diff --git a/source/extensions/filters/http/common/jwks_fetcher.cc b/source/extensions/filters/http/common/jwks_fetcher.cc new file mode 100644 index 0000000000000..b46f72cacb939 --- /dev/null +++ b/source/extensions/filters/http/common/jwks_fetcher.cc @@ -0,0 +1,106 @@ +#include "extensions/filters/http/common/jwks_fetcher.h" + +#include "common/common/enum_to_int.h" +#include "common/http/headers.h" +#include "common/http/utility.h" + +#include "jwt_verify_lib/status.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace { + +class JwksFetcherImpl : public JwksFetcher, + public Logger::Loggable, + public Http::AsyncClient::Callbacks { +public: + JwksFetcherImpl(Upstream::ClusterManager& cm) : cm_(cm) { ENVOY_LOG(trace, "{}", __func__); } + + ~JwksFetcherImpl() { cancel(); } + + void cancel() { + if (request_ && !complete_) { + request_->cancel(); + ENVOY_LOG(debug, "fetch pubkey [uri = {}]: canceled", uri_->uri()); + } + reset(); + } + + void fetch(const ::envoy::api::v2::core::HttpUri& uri, JwksFetcher::JwksReceiver& receiver) { + ENVOY_LOG(trace, "{}", __func__); + ASSERT(!receiver_); + complete_ = false; + receiver_ = &receiver; + uri_ = &uri; + Http::MessagePtr message = Http::Utility::prepareHeaders(uri); + message->headers().insertMethod().value().setReference(Http::Headers::get().MethodValues.Get); + ENVOY_LOG(debug, "fetch pubkey from [uri = {}]: start", uri_->uri()); + request_ = + cm_.httpAsyncClientForCluster(uri.cluster()) + .send(std::move(message), *this, + std::chrono::milliseconds(DurationUtil::durationToMilliseconds(uri.timeout()))); + } + + // HTTP async receive methods + void onSuccess(Http::MessagePtr&& response) { + ENVOY_LOG(trace, "{}", __func__); + complete_ = true; + const uint64_t status_code = Http::Utility::getResponseStatus(response->headers()); + if (status_code == enumToInt(Http::Code::OK)) { + ENVOY_LOG(debug, "{}: fetch pubkey [uri = {}]: success", __func__, uri_->uri()); + if (response->body()) { + const auto len = response->body()->length(); + const auto body = std::string(static_cast(response->body()->linearize(len)), len); + auto jwks = + google::jwt_verify::Jwks::createFrom(body, google::jwt_verify::Jwks::Type::JWKS); + if (jwks->getStatus() == google::jwt_verify::Status::Ok) { + ENVOY_LOG(debug, "{}: fetch pubkey [uri = {}]: succeeded", __func__, uri_->uri()); + receiver_->onJwksSuccess(std::move(jwks)); + } else { + ENVOY_LOG(debug, "{}: fetch pubkey [uri = {}]: invalid jwks", __func__, uri_->uri()); + receiver_->onJwksError(JwksFetcher::JwksReceiver::Failure::InvalidJwks); + } + } else { + ENVOY_LOG(debug, "{}: fetch pubkey [uri = {}]: body is empty", __func__, uri_->uri()); + receiver_->onJwksError(JwksFetcher::JwksReceiver::Failure::Network); + } + } else { + ENVOY_LOG(debug, "{}: fetch pubkey [uri = {}]: response status code {}", __func__, + uri_->uri(), status_code); + receiver_->onJwksError(JwksFetcher::JwksReceiver::Failure::Network); + } + reset(); + } + + void onFailure(Http::AsyncClient::FailureReason reason) { + ENVOY_LOG(debug, "{}: fetch pubkey [uri = {}]: network error {}", __func__, uri_->uri(), + enumToInt(reason)); + complete_ = true; + receiver_->onJwksError(JwksFetcher::JwksReceiver::Failure::Network); + reset(); + } + +private: + Upstream::ClusterManager& cm_; + bool complete_{}; + JwksFetcher::JwksReceiver* receiver_{}; + const envoy::api::v2::core::HttpUri* uri_{}; + Http::AsyncClient::Request* request_{}; + + void reset() { + request_ = nullptr; + receiver_ = nullptr; + uri_ = nullptr; + } +}; +} // namespace + +JwksFetcherPtr JwksFetcher::create(Upstream::ClusterManager& cm) { + return std::make_unique(cm); +} +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/common/jwks_fetcher.h b/source/extensions/filters/http/common/jwks_fetcher.h new file mode 100644 index 0000000000000..c76a76c8948b6 --- /dev/null +++ b/source/extensions/filters/http/common/jwks_fetcher.h @@ -0,0 +1,75 @@ +#pragma once + +#include "envoy/api/v2/core/http_uri.pb.h" +#include "envoy/common/pure.h" +#include "envoy/upstream/cluster_manager.h" + +#include "jwt_verify_lib/jwks.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { + +class JwksFetcher; +typedef std::unique_ptr JwksFetcherPtr; +/** + * JwksFetcher interface can be used to retrieve remote JWKS + * (https://tools.ietf.org/html/rfc7517) data structures returning a concrete, + * type-safe representation. An instance of this interface is designed to + * retrieve one JWKS at a time. + */ +class JwksFetcher { +public: + class JwksReceiver { + public: + enum class Failure { + /* A network error occured causing JWKS retrieval failure. */ + Network, + /* A failure occured when trying to parse the retrieved JWKS data. */ + InvalidJwks, + }; + + virtual ~JwksReceiver(){}; + /* + * Successful retrieval callback. + * of the returned JWKS object. + * @param jwks the JWKS object retrieved. + */ + virtual void onJwksSuccess(google::jwt_verify::JwksPtr&& jwks) PURE; + /* + * Retrieval error callback. + * * @param reason the failure reason. + */ + virtual void onJwksError(Failure reason) PURE; + }; + + virtual ~JwksFetcher(){}; + + /* + * Cancel any in-flight request. + */ + virtual void cancel() PURE; + + /* + * Retrieve a JWKS resource from a remote HTTP host. + * At most one outstanding request may be in-flight, + * i.e. from the invocation of `fetch()` until either + * a callback or `cancel()` is invoked, no + * additional `fetch()` may be issued. + * @param uri the uri to retrieve the jwks from. + * @param receiver the receiver of the fetched JWKS or error. + */ + virtual void fetch(const ::envoy::api::v2::core::HttpUri& uri, JwksReceiver& receiver) PURE; + + /* + * Factory method for creating a JwksFetcher. + * @param cm the cluster manager to use during Jwks retrieval + * @return a JwksFetcher instance + */ + static JwksFetcherPtr create(Upstream::ClusterManager& cm); +}; +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/jwt_authn/BUILD b/source/extensions/filters/http/jwt_authn/BUILD index 88a9b691d99fb..d7e1b572fcfbb 100644 --- a/source/extensions/filters/http/jwt_authn/BUILD +++ b/source/extensions/filters/http/jwt_authn/BUILD @@ -46,6 +46,7 @@ envoy_cc_library( "//include/envoy/server:filter_config_interface", "//include/envoy/stats:stats_macros", "//source/common/http:message_lib", + "//source/extensions/filters/http/common:jwks_fetcher_lib", ], ) diff --git a/source/extensions/filters/http/jwt_authn/authenticator.cc b/source/extensions/filters/http/jwt_authn/authenticator.cc index 4dc5ee50e22f2..9b558de3b422c 100644 --- a/source/extensions/filters/http/jwt_authn/authenticator.cc +++ b/source/extensions/filters/http/jwt_authn/authenticator.cc @@ -24,29 +24,23 @@ namespace { */ class AuthenticatorImpl : public Logger::Loggable, public Authenticator, - public Http::AsyncClient::Callbacks { + public Common::JwksFetcher::JwksReceiver { public: - AuthenticatorImpl(FilterConfigSharedPtr config) : config_(config) {} + AuthenticatorImpl(FilterConfigSharedPtr config, CreateJwksFetcherCb createJwksFetcherCb) + : config_(config), createJwksFetcherCb_(createJwksFetcherCb) {} + // Following functions are for JwksFetcher::JwksReceiver interface + void onJwksSuccess(google::jwt_verify::JwksPtr&& jwks) override; + void onJwksError(Failure reason) override; // Following functions are for Authenticator interface void verify(Http::HeaderMap& headers, Authenticator::Callbacks* callback) override; void onDestroy() override; void sanitizePayloadHeaders(Http::HeaderMap& headers) const override; private: - // Fetch a remote public key. - void fetchRemoteJwks(); - - // Following two functions are for AyncClient::Callbacks - void onSuccess(Http::MessagePtr&& response) override; - void onFailure(Http::AsyncClient::FailureReason) override; - // Verify with a specific public key. void verifyKey(); - // Handle the public key fetch done event. - void onFetchRemoteJwksDone(const std::string& jwks_str); - // Calls the callback with status. void doneWithStatus(const Status& status); @@ -56,6 +50,12 @@ class AuthenticatorImpl : public Logger::Loggable, // The config object. FilterConfigSharedPtr config_; + // The callback used to create a JwksFetcher instance. + CreateJwksFetcherCb createJwksFetcherCb_; + + // The Jwks fetcher object + Common::JwksFetcherPtr fetcher_; + // The token data JwtLocationConstPtr token_; // The JWT object. @@ -67,11 +67,6 @@ class AuthenticatorImpl : public Logger::Loggable, Http::HeaderMap* headers_{}; // The on_done function. Authenticator::Callbacks* callback_{}; - - // The pending uri_, only used for logging. - std::string uri_; - // The pending remote request so it can be canceled. - Http::AsyncClient::Request* request_{}; }; void AuthenticatorImpl::sanitizePayloadHeaders(Http::HeaderMap& headers) const { @@ -84,6 +79,7 @@ void AuthenticatorImpl::sanitizePayloadHeaders(Http::HeaderMap& headers) const { } void AuthenticatorImpl::verify(Http::HeaderMap& headers, Authenticator::Callbacks* callback) { + ASSERT(!callback_); headers_ = &headers; callback_ = callback; @@ -115,12 +111,22 @@ void AuthenticatorImpl::verify(Http::HeaderMap& headers, Authenticator::Callback return; } + // TODO(qiwzhang): Cross-platform-wise the below unix_timestamp code is wrong as the + // epoch is not guaranteed to be defined as the unix epoch. We should use + // the abseil time functionality instead or use the jwt_verify_lib to check + // the validity of a JWT. // Check "exp" claim. const auto unix_timestamp = std::chrono::duration_cast( std::chrono::system_clock::now().time_since_epoch()) .count(); - // NOTE: Service account tokens generally don't have an expiration time (due to being long lived) - // and defaulted to 0 by google::jwt_verify library but are still valid. + // If the nbf claim does *not* appear in the JWT, then the nbf field is defaulted + // to 0. + if (jwt_.nbf_ > unix_timestamp) { + doneWithStatus(Status::JwtNotYetValid); + return; + } + // If the exp claim does *not* appear in the JWT then the exp field is defaulted + // to 0. if (jwt_.exp_ > 0 && jwt_.exp_ < unix_timestamp) { doneWithStatus(Status::JwtExpired); return; @@ -137,7 +143,15 @@ void AuthenticatorImpl::verify(Http::HeaderMap& headers, Authenticator::Callback return; } - if (jwks_data_->getJwksObj() != nullptr && !jwks_data_->isExpired()) { + auto jwks_obj = jwks_data_->getJwksObj(); + if (jwks_obj != nullptr && !jwks_data_->isExpired()) { + // TODO(qiwzhang): It would seem there's a window of error whereby if the JWT issuer + // has started signing with a new key that's not in our cache, then the + // verification will fail even though the JWT is valid. A simple fix + // would be to check the JWS kid header field; if present check we have + // the key cached, if we do proceed to verify else try a new JWKS retrieval. + // JWTs without a kid header field in the JWS we might be best to get each + // time? This all only matters for remote JWKS. verifyKey(); return; } @@ -146,65 +160,20 @@ void AuthenticatorImpl::verify(Http::HeaderMap& headers, Authenticator::Callback // If request 1 triggers a remote jwks fetching, but is not yet replied when the request 2 // of using the same jwks comes. The request 2 will trigger another remote fetching for the // jwks. This can be optimized; the same remote jwks fetching can be shared by two requrests. - fetchRemoteJwks(); -} - -void AuthenticatorImpl::fetchRemoteJwks() { - const auto& http_uri = jwks_data_->getJwtProvider().remote_jwks().http_uri(); - - Http::MessagePtr message = Http::Utility::prepareHeaders(http_uri); - message->headers().insertMethod().value().setReference(Http::Headers::get().MethodValues.Get); - - if (config_->cm().get(http_uri.cluster()) == nullptr) { - doneWithStatus(Status::JwksFetchFail); - return; - } - - uri_ = http_uri.uri(); - ENVOY_LOG(debug, "fetch pubkey from [uri = {}]: start", uri_); - request_ = config_->cm() - .httpAsyncClientForCluster(http_uri.cluster()) - .send(std::move(message), *this, - std::chrono::milliseconds( - DurationUtil::durationToMilliseconds(http_uri.timeout()))); -} - -void AuthenticatorImpl::onSuccess(Http::MessagePtr&& response) { - request_ = nullptr; - const uint64_t status_code = Http::Utility::getResponseStatus(response->headers()); - if (status_code == enumToInt(Http::Code::OK)) { - ENVOY_LOG(debug, "fetch pubkey [uri = {}]: success", uri_); - if (response->body()) { - const auto len = response->body()->length(); - const auto body = std::string(static_cast(response->body()->linearize(len)), len); - onFetchRemoteJwksDone(body); - return; - } else { - ENVOY_LOG(debug, "fetch pubkey [uri = {}]: body is empty", uri_); + if (jwks_data_->getJwtProvider().has_remote_jwks()) { + if (!fetcher_) { + fetcher_ = createJwksFetcherCb_(config_->cm()); } + fetcher_->fetch(jwks_data_->getJwtProvider().remote_jwks().http_uri(), *this); } else { - ENVOY_LOG(debug, "fetch pubkey [uri = {}]: response status code {}", uri_, status_code); - } - doneWithStatus(Status::JwksFetchFail); -} - -void AuthenticatorImpl::onFailure(Http::AsyncClient::FailureReason) { - request_ = nullptr; - ENVOY_LOG(debug, "fetch pubkey [uri = {}]: failed", uri_); - doneWithStatus(Status::JwksFetchFail); -} - -void AuthenticatorImpl::onDestroy() { - if (request_ != nullptr) { - request_->cancel(); - request_ = nullptr; - ENVOY_LOG(debug, "fetch pubkey [uri = {}]: canceled", uri_); + // No valid keys for this issuer. This may happen as a result of incorrect local + // JWKS configuration. + doneWithStatus(Status::JwksNoValidKeys); } } -// Handle the public key fetch done event. -void AuthenticatorImpl::onFetchRemoteJwksDone(const std::string& jwks_str) { - const Status status = jwks_data_->setRemoteJwks(jwks_str); +void AuthenticatorImpl::onJwksSuccess(google::jwt_verify::JwksPtr&& jwks) { + const Status status = jwks_data_->setRemoteJwks(std::move(jwks))->getStatus(); if (status != Status::Ok) { doneWithStatus(status); } else { @@ -212,6 +181,14 @@ void AuthenticatorImpl::onFetchRemoteJwksDone(const std::string& jwks_str) { } } +void AuthenticatorImpl::onJwksError(Failure) { doneWithStatus(Status::JwksFetchFail); } + +void AuthenticatorImpl::onDestroy() { + if (fetcher_) { + fetcher_->cancel(); + } +} + // Verify with a specific public key. void AuthenticatorImpl::verifyKey() { const Status status = ::google::jwt_verify::verifyJwt(jwt_, *jwks_data_->getJwksObj()); @@ -249,8 +226,9 @@ void AuthenticatorImpl::doneWithStatus(const Status& status) { } // namespace -AuthenticatorPtr Authenticator::create(FilterConfigSharedPtr config) { - return std::make_unique(config); +AuthenticatorPtr Authenticator::create(FilterConfigSharedPtr config, + CreateJwksFetcherCb createJwksFetcherCb) { + return std::make_unique(config, createJwksFetcherCb); } } // namespace JwtAuthn diff --git a/source/extensions/filters/http/jwt_authn/authenticator.h b/source/extensions/filters/http/jwt_authn/authenticator.h index 8e3e2273dc9d5..937f6c3f5aab1 100644 --- a/source/extensions/filters/http/jwt_authn/authenticator.h +++ b/source/extensions/filters/http/jwt_authn/authenticator.h @@ -1,5 +1,7 @@ #pragma once +#include +#include "extensions/filters/http/common/jwks_fetcher.h" #include "extensions/filters/http/jwt_authn/filter_config.h" #include "jwt_verify_lib/status.h" @@ -13,9 +15,13 @@ class Authenticator; typedef std::unique_ptr AuthenticatorPtr; /** - * Authenticator object to handle all JWT authentication flow. + * CreateJwksFetcherCb is a callback interface for creating a JwksFetcher instance. */ +typedef std::function CreateJwksFetcherCb; +/** + * Authenticator object to handle all JWT authentication flow. + */ class Authenticator { public: virtual ~Authenticator() {} @@ -35,7 +41,8 @@ class Authenticator { virtual void sanitizePayloadHeaders(Http::HeaderMap& headers) const PURE; // Authenticator factory function. - static AuthenticatorPtr create(FilterConfigSharedPtr config); + static AuthenticatorPtr create(FilterConfigSharedPtr config, + CreateJwksFetcherCb createJwksFetcherCb); }; } // namespace JwtAuthn diff --git a/source/extensions/filters/http/jwt_authn/filter_factory.cc b/source/extensions/filters/http/jwt_authn/filter_factory.cc index 4e7a7e622ca7e..ecec7bf035059 100644 --- a/source/extensions/filters/http/jwt_authn/filter_factory.cc +++ b/source/extensions/filters/http/jwt_authn/filter_factory.cc @@ -46,8 +46,8 @@ FilterFactory::createFilterFactoryFromProtoTyped(const JwtAuthentication& proto_ validateJwtConfig(proto_config); auto filter_config = std::make_shared(proto_config, prefix, context); return [filter_config](Http::FilterChainFactoryCallbacks& callbacks) -> void { - callbacks.addStreamDecoderFilter( - std::make_shared(filter_config->stats(), Authenticator::create(filter_config))); + callbacks.addStreamDecoderFilter(std::make_shared( + filter_config->stats(), Authenticator::create(filter_config, Common::JwksFetcher::create))); }; } diff --git a/source/extensions/filters/http/jwt_authn/jwks_cache.cc b/source/extensions/filters/http/jwt_authn/jwks_cache.cc index f26735c4f4186..3648113dbc304 100644 --- a/source/extensions/filters/http/jwt_authn/jwks_cache.cc +++ b/source/extensions/filters/http/jwt_authn/jwks_cache.cc @@ -34,12 +34,13 @@ class JwksDataImpl : public JwksCache::JwksData, public Logger::LoggablegetStatus() != Status::Ok) { ENVOY_LOG(warn, "Invalid inline jwks for issuer: {}, jwks: {}", jwt_provider_.issuer(), inline_jwks); + jwks_obj_.reset(nullptr); } } } @@ -54,8 +55,8 @@ class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable= expiration_time_; } - Status setRemoteJwks(const std::string& jwks_str) override { - return setKey(jwks_str, getRemoteJwksExpirationTime()); + const ::google::jwt_verify::Jwks* setRemoteJwks(::google::jwt_verify::JwksPtr&& jwks) override { + return setKey(std::move(jwks), getRemoteJwksExpirationTime()); } private: @@ -71,15 +72,11 @@ class JwksDataImpl : public JwksCache::JwksData, public Logger::LoggablegetStatus() != Status::Ok) { - return jwks_obj->getStatus(); - } - jwks_obj_ = std::move(jwks_obj); + const ::google::jwt_verify::Jwks* setKey(::google::jwt_verify::JwksPtr&& jwks, + std::chrono::steady_clock::time_point expire) { + jwks_obj_ = std::move(jwks); expiration_time_ = expire; - return Status::Ok; + return jwks_obj_.get(); } // The jwt provider config. diff --git a/source/extensions/filters/http/jwt_authn/jwks_cache.h b/source/extensions/filters/http/jwt_authn/jwks_cache.h index 570b9c00d3f15..46d697f03e8b4 100644 --- a/source/extensions/filters/http/jwt_authn/jwks_cache.h +++ b/source/extensions/filters/http/jwt_authn/jwks_cache.h @@ -53,8 +53,9 @@ class JwksCache { // Return true if jwks object is expired. virtual bool isExpired() const PURE; - // Set a remote Jwks string. - virtual ::google::jwt_verify::Status setRemoteJwks(const std::string& jwks_str) PURE; + // Set a remote Jwks. + virtual const ::google::jwt_verify::Jwks* + setRemoteJwks(::google::jwt_verify::JwksPtr&& jwks) PURE; }; // Lookup issuer cache map. The cache only stores Jwks specified in the config. diff --git a/test/extensions/filters/http/common/BUILD b/test/extensions/filters/http/common/BUILD new file mode 100644 index 0000000000000..58f30f2f2d607 --- /dev/null +++ b/test/extensions/filters/http/common/BUILD @@ -0,0 +1,42 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) +load( + "//test/extensions:extensions_build_system.bzl", + "envoy_cc_test_library", + "envoy_extension_cc_test", +) + +envoy_package() + +envoy_cc_test_library( + name = "mock_lib", + srcs = [ + "mock.cc", + ], + hdrs = [ + "mock.h", + ], + deps = [ + "//source/extensions/filters/http/common:jwks_fetcher_lib", + "//test/mocks/server:server_mocks", + ], +) + +envoy_extension_cc_test( + name = "jwks_fetcher_test", + srcs = [ + "jwks_fetcher_test.cc", + ], + extension_name = "envoy.filters.http.jwt_authn", + deps = [ + "//source/extensions/filters/http/common:jwks_fetcher_lib", + "//test/extensions/filters/http/common:mock_lib", + "//test/mocks/http:http_mocks", + "//test/test_common:utility_lib", + ], +) diff --git a/test/extensions/filters/http/common/jwks_fetcher_test.cc b/test/extensions/filters/http/common/jwks_fetcher_test.cc new file mode 100644 index 0000000000000..46ce0063b1c0e --- /dev/null +++ b/test/extensions/filters/http/common/jwks_fetcher_test.cc @@ -0,0 +1,149 @@ +#include +#include + +#include "common/http/message_impl.h" +#include "common/protobuf/utility.h" + +#include "extensions/filters/http/common/jwks_fetcher.h" + +#include "test/extensions/filters/http/common/mock.h" +#include "test/mocks/http/mocks.h" +#include "test/test_common/utility.h" + +using ::envoy::api::v2::core::HttpUri; +using ::google::jwt_verify::Status; + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace { + +const char publicKey[] = R"( +{ + "keys": [ + { + "kty": "RSA", + "alg": "RS256", + "use": "sig", + "kid": "62a93512c9ee4c7f8067b5a216dade2763d32a47", + "n": "up97uqrF9MWOPaPkwSaBeuAPLOr9FKcaWGdVEGzQ4f3Zq5WKVZowx9TCBxmImNJ1qmUi13pB8otwM_l5lfY1AFBMxVbQCUXntLovhDaiSvYp4wGDjFzQiYA-pUq8h6MUZBnhleYrkU7XlCBwNVyN8qNMkpLA7KFZYz-486GnV2NIJJx_4BGa3HdKwQGxi2tjuQsQvao5W4xmSVaaEWopBwMy2QmlhSFQuPUpTaywTqUcUq_6SfAHhZ4IDa_FxEd2c2z8gFGtfst9cY3lRYf-c_ZdboY3mqN9Su3-j3z5r2SHWlhB_LNAjyWlBGsvbGPlTqDziYQwZN4aGsqVKQb9Vw", + "e": "AQAB" + }, + { + "kty": "RSA", + "alg": "RS256", + "use": "sig", + "kid": "b3319a147514df7ee5e4bcdee51350cc890cc89e", + "n": "up97uqrF9MWOPaPkwSaBeuAPLOr9FKcaWGdVEGzQ4f3Zq5WKVZowx9TCBxmImNJ1qmUi13pB8otwM_l5lfY1AFBMxVbQCUXntLovhDaiSvYp4wGDjFzQiYA-pUq8h6MUZBnhleYrkU7XlCBwNVyN8qNMkpLA7KFZYz-486GnV2NIJJx_4BGa3HdKwQGxi2tjuQsQvao5W4xmSVaaEWopBwMy2QmlhSFQuPUpTaywTqUcUq_6SfAHhZ4IDa_FxEd2c2z8gFGtfst9cY3lRYf-c_ZdboY3mqN9Su3-j3z5r2SHWlhB_LNAjyWlBGsvbGPlTqDziYQwZN4aGsqVKQb9Vw", + "e": "AQAB" + } + ] +} +)"; + +const std::string JwksUri = R"( +uri: https://pubkey_server/pubkey_path +cluster: pubkey_cluster +timeout: + seconds: 5 +)"; + +class JwksFetcherTest : public ::testing::Test { +public: + void SetUp() { MessageUtil::loadFromYaml(JwksUri, uri_); } + HttpUri uri_; + testing::NiceMock mock_factory_ctx_; +}; + +// Test findByIssuer +TEST_F(JwksFetcherTest, TestGetSuccess) { + // Setup + MockUpstream mock_pubkey(mock_factory_ctx_.cluster_manager_, "200", publicKey); + MockJwksReceiver receiver; + std::unique_ptr fetcher(JwksFetcher::create(mock_factory_ctx_.cluster_manager_)); + EXPECT_TRUE(fetcher != nullptr); + EXPECT_CALL(receiver, onJwksSuccessImpl(testing::_)).Times(1); + EXPECT_CALL(receiver, onJwksError(testing::_)).Times(0); + + // Act + fetcher->fetch(uri_, receiver); +} + +TEST_F(JwksFetcherTest, TestGet400) { + // Setup + MockUpstream mock_pubkey(mock_factory_ctx_.cluster_manager_, "400", "invalid"); + MockJwksReceiver receiver; + std::unique_ptr fetcher(JwksFetcher::create(mock_factory_ctx_.cluster_manager_)); + EXPECT_TRUE(fetcher != nullptr); + EXPECT_CALL(receiver, onJwksSuccessImpl(testing::_)).Times(0); + EXPECT_CALL(receiver, onJwksError(JwksFetcher::JwksReceiver::Failure::Network)).Times(1); + + // Act + fetcher->fetch(uri_, receiver); +} + +TEST_F(JwksFetcherTest, TestGetNoBody) { + // Setup + MockUpstream mock_pubkey(mock_factory_ctx_.cluster_manager_, "200", ""); + MockJwksReceiver receiver; + std::unique_ptr fetcher(JwksFetcher::create(mock_factory_ctx_.cluster_manager_)); + EXPECT_TRUE(fetcher != nullptr); + EXPECT_CALL(receiver, onJwksSuccessImpl(testing::_)).Times(0); + EXPECT_CALL(receiver, onJwksError(JwksFetcher::JwksReceiver::Failure::Network)).Times(1); + + // Act + fetcher->fetch(uri_, receiver); +} + +TEST_F(JwksFetcherTest, TestGetInvalidJwks) { + // Setup + MockUpstream mock_pubkey(mock_factory_ctx_.cluster_manager_, "200", "invalid"); + MockJwksReceiver receiver; + std::unique_ptr fetcher(JwksFetcher::create(mock_factory_ctx_.cluster_manager_)); + EXPECT_TRUE(fetcher != nullptr); + EXPECT_CALL(receiver, onJwksSuccessImpl(testing::_)).Times(0); + EXPECT_CALL(receiver, onJwksError(JwksFetcher::JwksReceiver::Failure::InvalidJwks)).Times(1); + + // Act + fetcher->fetch(uri_, receiver); +} + +TEST_F(JwksFetcherTest, TestHttpFailure) { + // Setup + MockUpstream mock_pubkey(mock_factory_ctx_.cluster_manager_, + Http::AsyncClient::FailureReason::Reset); + MockJwksReceiver receiver; + std::unique_ptr fetcher(JwksFetcher::create(mock_factory_ctx_.cluster_manager_)); + EXPECT_TRUE(fetcher != nullptr); + EXPECT_CALL(receiver, onJwksSuccessImpl(testing::_)).Times(0); + EXPECT_CALL(receiver, onJwksError(JwksFetcher::JwksReceiver::Failure::Network)).Times(1); + + // Act + fetcher->fetch(uri_, receiver); +} + +TEST_F(JwksFetcherTest, TestCancel) { + // Setup + Http::MockAsyncClientRequest request(&(mock_factory_ctx_.cluster_manager_.async_client_)); + MockUpstream mock_pubkey(mock_factory_ctx_.cluster_manager_, &request); + MockJwksReceiver receiver; + std::unique_ptr fetcher(JwksFetcher::create(mock_factory_ctx_.cluster_manager_)); + EXPECT_TRUE(fetcher != nullptr); + EXPECT_CALL(request, cancel()).Times(1); + EXPECT_CALL(receiver, onJwksSuccessImpl(testing::_)).Times(0); + EXPECT_CALL(receiver, onJwksError(testing::_)).Times(0); + + // Act + fetcher->fetch(uri_, receiver); + // Proper cancel + fetcher->cancel(); + // Re-entrant cancel + fetcher->cancel(); +} + +} // namespace +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/http/common/mock.cc b/test/extensions/filters/http/common/mock.cc new file mode 100644 index 0000000000000..3229f39cc5076 --- /dev/null +++ b/test/extensions/filters/http/common/mock.cc @@ -0,0 +1,48 @@ +#include "test/extensions/filters/http/common/mock.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +MockUpstream::MockUpstream(Upstream::MockClusterManager& mock_cm, const std::string& status, + const std::string& response_body) + : request_(&mock_cm.async_client_), status_(status), response_body_(response_body) { + ON_CALL(mock_cm.async_client_, send_(testing::_, testing::_, testing::_)) + .WillByDefault(testing::Invoke( + [this](Http::MessagePtr&, Http::AsyncClient::Callbacks& cb, + const absl::optional&) -> Http::AsyncClient::Request* { + Http::MessagePtr response_message(new Http::ResponseMessageImpl( + Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", status_}}})); + if (response_body_.length()) { + response_message->body().reset(new Buffer::OwnedImpl(response_body_)); + } else { + response_message->body().reset(nullptr); + } + cb.onSuccess(std::move(response_message)); + return &request_; + })); +} + +MockUpstream::MockUpstream(Upstream::MockClusterManager& mock_cm, + Http::AsyncClient::FailureReason reason) + : request_(&mock_cm.async_client_) { + ON_CALL(mock_cm.async_client_, send_(testing::_, testing::_, testing::_)) + .WillByDefault(testing::Invoke( + [this, reason]( + Http::MessagePtr&, Http::AsyncClient::Callbacks& cb, + const absl::optional&) -> Http::AsyncClient::Request* { + cb.onFailure(reason); + return &request_; + })); +} + +MockUpstream::MockUpstream(Upstream::MockClusterManager& mock_cm, + Http::MockAsyncClientRequest* request) + : request_(&mock_cm.async_client_) { + ON_CALL(mock_cm.async_client_, send_(testing::_, testing::_, testing::_)) + .WillByDefault(testing::Return(request)); +} +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/test/extensions/filters/http/common/mock.h b/test/extensions/filters/http/common/mock.h new file mode 100644 index 0000000000000..4d48f1b931704 --- /dev/null +++ b/test/extensions/filters/http/common/mock.h @@ -0,0 +1,58 @@ +#include "extensions/filters/http/common/jwks_fetcher.h" + +#include "test/mocks/server/mocks.h" + +#include "gmock/gmock.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { + +class MockJwksFetcher : public JwksFetcher { +public: + MOCK_METHOD0(cancel, void()); + MOCK_METHOD2(fetch, void(const ::envoy::api::v2::core::HttpUri& uri, JwksReceiver& receiver)); +}; + +// A mock HTTP upstream. +class MockUpstream { +public: + /** + * Mock upstream which returns a given response body. + */ + MockUpstream(Upstream::MockClusterManager& mock_cm, const std::string& status, + const std::string& response_body); + /** + * Mock upstream which returns a given failure. + */ + MockUpstream(Upstream::MockClusterManager& mock_cm, Http::AsyncClient::FailureReason reason); + /** + * Mock upstream which returns the given request. + */ + MockUpstream(Upstream::MockClusterManager& mock_cm, Http::MockAsyncClientRequest* request); + +private: + Http::MockAsyncClientRequest request_; + std::string status_; + std::string response_body_; +}; + +class MockJwksReceiver : public JwksFetcher::JwksReceiver { +public: + /* GoogleMock does handle r-value references hence the below construction. + * Expectations and assertions should be made on onJwksSuccessImpl in place + * of onJwksSuccess. + */ + void onJwksSuccess(google::jwt_verify::JwksPtr&& jwks) { + ASSERT(jwks); + onJwksSuccessImpl(*jwks.get()); + } + MOCK_METHOD1(onJwksSuccessImpl, void(const google::jwt_verify::Jwks& jwks)); + MOCK_METHOD1(onJwksError, void(JwksFetcher::JwksReceiver::Failure reason)); +}; + +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/http/jwt_authn/BUILD b/test/extensions/filters/http/jwt_authn/BUILD index bfc7cd47ccf67..0e90d2c0ecc2d 100644 --- a/test/extensions/filters/http/jwt_authn/BUILD +++ b/test/extensions/filters/http/jwt_authn/BUILD @@ -55,10 +55,9 @@ envoy_extension_cc_test( ], extension_name = "envoy.filters.http.jwt_authn", deps = [ - ":test_common_lib", "//source/extensions/filters/http/jwt_authn:config", + "//test/extensions/filters/http/jwt_authn:test_common_lib", "//test/mocks/server:server_mocks", - "//test/test_common:utility_lib", ], ) @@ -69,8 +68,9 @@ envoy_extension_cc_test( ], extension_name = "envoy.filters.http.jwt_authn", deps = [ - ":test_common_lib", + "//source/extensions/filters/http/common:jwks_fetcher_lib", "//source/extensions/filters/http/jwt_authn:jwks_cache_lib", + "//test/extensions/filters/http/jwt_authn:test_common_lib", "//test/test_common:utility_lib", ], ) @@ -83,8 +83,10 @@ envoy_extension_cc_test( extension_name = "envoy.filters.http.jwt_authn", deps = [ ":mock_lib", - ":test_common_lib", + "//source/extensions/filters/http/common:jwks_fetcher_lib", "//source/extensions/filters/http/jwt_authn:authenticator_lib", + "//test/extensions/filters/http/common:mock_lib", + "//test/extensions/filters/http/jwt_authn:test_common_lib", "//test/mocks/server:server_mocks", "//test/test_common:utility_lib", ], @@ -95,9 +97,9 @@ envoy_extension_cc_test( srcs = ["filter_integration_test.cc"], extension_name = "envoy.filters.http.jwt_authn", deps = [ - ":test_common_lib", "//source/extensions/filters/http/jwt_authn:config", "//test/config:utility_lib", + "//test/extensions/filters/http/jwt_authn:test_common_lib", "//test/integration:http_protocol_integration_lib", ], ) diff --git a/test/extensions/filters/http/jwt_authn/authenticator_test.cc b/test/extensions/filters/http/jwt_authn/authenticator_test.cc index 02abd237488e6..f6c2d90bc7666 100644 --- a/test/extensions/filters/http/jwt_authn/authenticator_test.cc +++ b/test/extensions/filters/http/jwt_authn/authenticator_test.cc @@ -1,8 +1,10 @@ #include "common/http/message_impl.h" #include "common/protobuf/utility.h" +#include "extensions/filters/http/common/jwks_fetcher.h" #include "extensions/filters/http/jwt_authn/authenticator.h" +#include "test/extensions/filters/http/common/mock.h" #include "test/extensions/filters/http/jwt_authn/mock.h" #include "test/extensions/filters/http/jwt_authn/test_common.h" #include "test/mocks/server/mocks.h" @@ -12,6 +14,9 @@ #include "gtest/gtest.h" using ::envoy::config::filter::http::jwt_authn::v2alpha::JwtAuthentication; +using Envoy::Extensions::HttpFilters::Common::JwksFetcher; +using Envoy::Extensions::HttpFilters::Common::JwksFetcherPtr; +using Envoy::Extensions::HttpFilters::Common::MockJwksFetcher; using ::google::jwt_verify::Status; using ::testing::_; using ::testing::Invoke; @@ -31,46 +36,32 @@ class AuthenticatorTest : public ::testing::Test { void CreateAuthenticator() { filter_config_ = ::std::make_shared(proto_config_, "", mock_factory_ctx_); - auth_ = Authenticator::create(filter_config_); + fetcher_ = new MockJwksFetcher; + fetcherPtr_.reset(fetcher_); + auth_ = Authenticator::create( + filter_config_, [this](Upstream::ClusterManager&) { return std::move(fetcherPtr_); }); + jwks_ = ::google::jwt_verify::Jwks::createFrom(PublicKey, ::google::jwt_verify::Jwks::JWKS); + EXPECT_TRUE(jwks_->getStatus() == Status::Ok); } JwtAuthentication proto_config_; FilterConfigSharedPtr filter_config_; - std::unique_ptr auth_; + MockJwksFetcher* fetcher_; + JwksFetcherPtr fetcherPtr_; + AuthenticatorPtr auth_; + ::google::jwt_verify::JwksPtr jwks_; NiceMock mock_factory_ctx_; MockAuthenticatorCallbacks mock_cb_; }; -// A mock HTTP upstream with response body. -class MockUpstream { -public: - MockUpstream(Upstream::MockClusterManager& mock_cm, const std::string& response_body) - : request_(&mock_cm.async_client_), response_body_(response_body) { - ON_CALL(mock_cm.async_client_, send_(_, _, _)) - .WillByDefault(Invoke([this](Http::MessagePtr&, Http::AsyncClient::Callbacks& cb, - const absl::optional&) - -> Http::AsyncClient::Request* { - Http::MessagePtr response_message(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); - response_message->body().reset(new Buffer::OwnedImpl(response_body_)); - cb.onSuccess(std::move(response_message)); - called_count_++; - return &request_; - })); - } - - int called_count() const { return called_count_; } - -private: - Http::MockAsyncClientRequest request_; - std::string response_body_; - int called_count_{}; -}; - // This test validates a good JWT authentication with a remote Jwks. // It also verifies Jwks cache with 10 JWT authentications, but only one Jwks fetch. TEST_F(AuthenticatorTest, TestOkJWTandCache) { - MockUpstream mock_pubkey(mock_factory_ctx_.cluster_manager_, PublicKey); + EXPECT_CALL(*fetcher_, fetch(_, _)) + .WillOnce(Invoke( + [this](const ::envoy::api::v2::core::HttpUri&, JwksFetcher::JwksReceiver& receiver) { + receiver.onJwksSuccess(std::move(jwks_)); + })); // Test OK pubkey and its cache for (int i = 0; i < 10; i++) { @@ -87,17 +78,18 @@ TEST_F(AuthenticatorTest, TestOkJWTandCache) { // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); } - - EXPECT_EQ(mock_pubkey.called_count(), 1); } // This test verifies the Jwt is forwarded if "forward" flag is set. TEST_F(AuthenticatorTest, TestForwardJwt) { - // Confit forward_jwt flag + // Confirm forward_jwt flag (*proto_config_.mutable_providers())[std::string(ProviderName)].set_forward(true); CreateAuthenticator(); - - MockUpstream mock_pubkey(mock_factory_ctx_.cluster_manager_, PublicKey); + EXPECT_CALL(*fetcher_, fetch(_, _)) + .WillOnce(Invoke( + [this](const ::envoy::api::v2::core::HttpUri&, JwksFetcher::JwksReceiver& receiver) { + receiver.onJwksSuccess(std::move(jwks_)); + })); // Test OK pubkey and its cache auto headers = Http::TestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodToken)}}; @@ -115,8 +107,11 @@ TEST_F(AuthenticatorTest, TestForwardJwt) { // This test verifies the Jwt with non existing kid TEST_F(AuthenticatorTest, TestJwtWithNonExistKid) { - MockUpstream mock_pubkey(mock_factory_ctx_.cluster_manager_, PublicKey); - + EXPECT_CALL(*fetcher_, fetch(_, _)) + .WillOnce(Invoke( + [this](const ::envoy::api::v2::core::HttpUri&, JwksFetcher::JwksReceiver& receiver) { + receiver.onJwksSuccess(std::move(jwks_)); + })); // Test OK pubkey and its cache auto headers = Http::TestHeaderMapImpl{{"Authorization", "Bearer " + std::string(NonExistKidToken)}}; @@ -131,7 +126,8 @@ TEST_F(AuthenticatorTest, TestJwtWithNonExistKid) { // This test verifies if Jwt is missing, proper status is called. TEST_F(AuthenticatorTest, TestMissedJWT) { - EXPECT_CALL(mock_factory_ctx_.cluster_manager_, httpAsyncClientForCluster(_)).Times(0); + EXPECT_CALL(*fetcher_, fetch(_, _)).Times(0); + MockAuthenticatorCallbacks mock_cb; EXPECT_CALL(mock_cb_, onComplete(_)).WillOnce(Invoke([](const Status& status) { ASSERT_EQ(status, Status::JwtMissed); })); @@ -143,7 +139,7 @@ TEST_F(AuthenticatorTest, TestMissedJWT) { // This test verifies if Jwt is invalid, JwtBadFormat status is returned. TEST_F(AuthenticatorTest, TestInvalidJWT) { - EXPECT_CALL(mock_factory_ctx_.cluster_manager_, httpAsyncClientForCluster(_)).Times(0); + EXPECT_CALL(*fetcher_, fetch(_, _)).Times(0); EXPECT_CALL(mock_cb_, onComplete(_)).WillOnce(Invoke([](const Status& status) { ASSERT_EQ(status, Status::JwtBadFormat); })); @@ -155,7 +151,7 @@ TEST_F(AuthenticatorTest, TestInvalidJWT) { // This test verifies if Authorization header has invalid prefix, JwtMissed status is returned TEST_F(AuthenticatorTest, TestInvalidPrefix) { - EXPECT_CALL(mock_factory_ctx_.cluster_manager_, httpAsyncClientForCluster(_)).Times(0); + EXPECT_CALL(*fetcher_, fetch(_, _)).Times(0); EXPECT_CALL(mock_cb_, onComplete(_)).WillOnce(Invoke([](const Status& status) { ASSERT_EQ(status, Status::JwtMissed); })); @@ -179,7 +175,7 @@ TEST_F(AuthenticatorTest, TestNonExpiringJWT) { // This test verifies when a JWT is expired, JwtExpired status is returned. TEST_F(AuthenticatorTest, TestExpiredJWT) { - EXPECT_CALL(mock_factory_ctx_.cluster_manager_, httpAsyncClientForCluster(_)).Times(0); + EXPECT_CALL(*fetcher_, fetch(_, _)).Times(0); EXPECT_CALL(mock_cb_, onComplete(_)).WillOnce(Invoke([](const Status& status) { ASSERT_EQ(status, Status::JwtExpired); })); @@ -188,43 +184,51 @@ TEST_F(AuthenticatorTest, TestExpiredJWT) { auth_->verify(headers, &mock_cb_); } -// This test verifies when a JWT is with invalid audience, JwtAudienceNotAllowed is returned. -TEST_F(AuthenticatorTest, TestNonMatchAudJWT) { - EXPECT_CALL(mock_factory_ctx_.cluster_manager_, httpAsyncClientForCluster(_)).Times(0); - EXPECT_CALL(mock_cb_, onComplete(_)).WillOnce(Invoke([](const Status& status) { - ASSERT_EQ(status, Status::JwtAudienceNotAllowed); - })); +// This test verifies when a JWT is not yet valid, JwtNotYetValid status is returned. +TEST_F(AuthenticatorTest, TestNotYetValidJWT) { + EXPECT_CALL(*fetcher_, fetch(_, _)).Times(0); + EXPECT_CALL(mock_cb_, onComplete(Status::JwtNotYetValid)).Times(1); auto headers = - Http::TestHeaderMapImpl{{"Authorization", "Bearer " + std::string(InvalidAudToken)}}; + Http::TestHeaderMapImpl{{"Authorization", "Bearer " + std::string(NotYetValidToken)}}; auth_->verify(headers, &mock_cb_); } -// This test verifies when invalid cluster is configured for remote jwks, JwksFetchFail is returned. -TEST_F(AuthenticatorTest, TestWrongCluster) { - // Get returns nullptr - EXPECT_CALL(mock_factory_ctx_.cluster_manager_, get(_)) - .WillOnce(Invoke([](const std::string& cluster) -> Upstream::ThreadLocalCluster* { - EXPECT_EQ(cluster, "pubkey_cluster"); - return nullptr; - })); +// This test verifies when an inline JWKS is misconfigured, JwksNoValidKeys is returns +TEST_F(AuthenticatorTest, TestInvalidLocalJwks) { + auto& provider = (*proto_config_.mutable_providers())[std::string(ProviderName)]; + provider.clear_remote_jwks(); + provider.mutable_local_jwks()->set_inline_string("invalid"); + CreateAuthenticator(); - EXPECT_CALL(mock_factory_ctx_.cluster_manager_, httpAsyncClientForCluster(_)).Times(0); + EXPECT_CALL(*fetcher_, fetch(_, _)).Times(0); EXPECT_CALL(mock_cb_, onComplete(_)).WillOnce(Invoke([](const Status& status) { - ASSERT_EQ(status, Status::JwksFetchFail); + ASSERT_EQ(status, Status::JwksNoValidKeys); })); auto headers = Http::TestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodToken)}}; auth_->verify(headers, &mock_cb_); } +// This test verifies when a JWT is with invalid audience, JwtAudienceNotAllowed is returned. +TEST_F(AuthenticatorTest, TestNonMatchAudJWT) { + EXPECT_CALL(*fetcher_, fetch(_, _)).Times(0); + EXPECT_CALL(mock_cb_, onComplete(_)).WillOnce(Invoke([](const Status& status) { + ASSERT_EQ(status, Status::JwtAudienceNotAllowed); + })); + + auto headers = + Http::TestHeaderMapImpl{{"Authorization", "Bearer " + std::string(InvalidAudToken)}}; + auth_->verify(headers, &mock_cb_); +} + // This test verifies when Jwt issuer is not configured, JwtUnknownIssuer is returned. TEST_F(AuthenticatorTest, TestIssuerNotFound) { // Create a config with an other issuer. (*proto_config_.mutable_providers())[std::string(ProviderName)].set_issuer("other_issuer"); CreateAuthenticator(); - EXPECT_CALL(mock_factory_ctx_.cluster_manager_, httpAsyncClientForCluster(_)).Times(0); + EXPECT_CALL(*fetcher_, fetch(_, _)).Times(0); EXPECT_CALL(mock_cb_, onComplete(_)).WillOnce(Invoke([](const Status& status) { ASSERT_EQ(status, Status::JwtUnknownIssuer); })); @@ -235,27 +239,10 @@ TEST_F(AuthenticatorTest, TestIssuerNotFound) { // This test verifies that when Jwks fetching fails, JwksFetchFail status is returned. TEST_F(AuthenticatorTest, TestPubkeyFetchFail) { - NiceMock async_client; - EXPECT_CALL(mock_factory_ctx_.cluster_manager_, httpAsyncClientForCluster(_)) - .WillOnce(Invoke([&](const std::string& cluster) -> Http::AsyncClient& { - EXPECT_EQ(cluster, "pubkey_cluster"); - return async_client; - })); - - Http::MockAsyncClientRequest request(&async_client); - Http::AsyncClient::Callbacks* callbacks; - EXPECT_CALL(async_client, send_(_, _, _)) - .WillOnce(Invoke( - [&](Http::MessagePtr& message, Http::AsyncClient::Callbacks& cb, - const absl::optional&) -> Http::AsyncClient::Request* { - EXPECT_EQ((Http::TestHeaderMapImpl{ - {":path", "/pubkey_path"}, - {":authority", "pubkey_server"}, - {":method", "GET"}, - }), - message->headers()); - callbacks = &cb; - return &request; + EXPECT_CALL(*fetcher_, fetch(_, _)) + .WillOnce( + Invoke([](const ::envoy::api::v2::core::HttpUri&, JwksFetcher::JwksReceiver& receiver) { + receiver.onJwksError(JwksFetcher::JwksReceiver::Failure::InvalidJwks); })); EXPECT_CALL(mock_cb_, onComplete(_)).WillOnce(Invoke([](const Status& status) { @@ -267,122 +254,14 @@ TEST_F(AuthenticatorTest, TestPubkeyFetchFail) { Http::MessagePtr response_message(new Http::ResponseMessageImpl( Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "401"}}})); - callbacks->onSuccess(std::move(response_message)); -} - -// This test verifies that when Jwks fetching return empty body -TEST_F(AuthenticatorTest, TestPubkeyFetchFailWithEmptyBody) { - Http::MockAsyncClientRequest request(&mock_factory_ctx_.cluster_manager_.async_client_); - Http::AsyncClient::Callbacks* callbacks; - EXPECT_CALL(mock_factory_ctx_.cluster_manager_.async_client_, send_(_, _, _)) - .WillOnce(Invoke( - [&](Http::MessagePtr&, Http::AsyncClient::Callbacks& cb, - const absl::optional&) -> Http::AsyncClient::Request* { - callbacks = &cb; - return &request; - })); - - EXPECT_CALL(mock_cb_, onComplete(_)).WillOnce(Invoke([](const Status& status) { - ASSERT_EQ(status, Status::JwksFetchFail); - })); - - auto headers = Http::TestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodToken)}}; - auth_->verify(headers, &mock_cb_); - - Http::MessagePtr response_message(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); - callbacks->onSuccess(std::move(response_message)); -} - -// This test verifies that when Jwks fetching returned with onFailure -TEST_F(AuthenticatorTest, TestPubkeyFetchFailWithOnFailure) { - Http::MockAsyncClientRequest request(&mock_factory_ctx_.cluster_manager_.async_client_); - Http::AsyncClient::Callbacks* callbacks; - EXPECT_CALL(mock_factory_ctx_.cluster_manager_.async_client_, send_(_, _, _)) - .WillOnce(Invoke( - [&](Http::MessagePtr&, Http::AsyncClient::Callbacks& cb, - const absl::optional&) -> Http::AsyncClient::Request* { - callbacks = &cb; - return &request; - })); - - EXPECT_CALL(mock_cb_, onComplete(_)).WillOnce(Invoke([](const Status& status) { - ASSERT_EQ(status, Status::JwksFetchFail); - })); - - auto headers = Http::TestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodToken)}}; - auth_->verify(headers, &mock_cb_); - - callbacks->onFailure(Http::AsyncClient::FailureReason::Reset); -} - -// This test verifies when an invalid Jwks is fetched, JwksParseError status is returned. -TEST_F(AuthenticatorTest, TestInvalidPubkey) { - NiceMock async_client; - EXPECT_CALL(mock_factory_ctx_.cluster_manager_, httpAsyncClientForCluster(_)) - .WillOnce(Invoke([&](const std::string& cluster) -> Http::AsyncClient& { - EXPECT_EQ(cluster, "pubkey_cluster"); - return async_client; - })); - - Http::MockAsyncClientRequest request(&async_client); - Http::AsyncClient::Callbacks* callbacks; - EXPECT_CALL(async_client, send_(_, _, _)) - .WillOnce(Invoke( - [&](Http::MessagePtr& message, Http::AsyncClient::Callbacks& cb, - const absl::optional&) -> Http::AsyncClient::Request* { - EXPECT_EQ((Http::TestHeaderMapImpl{ - {":path", "/pubkey_path"}, - {":authority", "pubkey_server"}, - {":method", "GET"}, - }), - message->headers()); - callbacks = &cb; - return &request; - })); - - EXPECT_CALL(mock_cb_, onComplete(_)).WillOnce(Invoke([](const Status& status) { - ASSERT_EQ(status, Status::JwksParseError); - })); - - auto headers = Http::TestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodToken)}}; - auth_->verify(headers, &mock_cb_); - - Http::MessagePtr response_message(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); - response_message->body().reset(new Buffer::OwnedImpl("invalid publik key")); - callbacks->onSuccess(std::move(response_message)); } // This test verifies when a Jwks fetching is not completed yet, but onDestory() is called, -// onComplete() callback should not be called, but internal request->cancel() should be called. +// onComplete() callback should not be called, but internal fetcher_->close() should be called. // Most importantly, no crash. TEST_F(AuthenticatorTest, TestOnDestroy) { - NiceMock async_client; - EXPECT_CALL(mock_factory_ctx_.cluster_manager_, httpAsyncClientForCluster(_)) - .WillOnce(Invoke([&](const std::string& cluster) -> Http::AsyncClient& { - EXPECT_EQ(cluster, "pubkey_cluster"); - return async_client; - })); - - Http::MockAsyncClientRequest request(&async_client); - Http::AsyncClient::Callbacks* callbacks; - EXPECT_CALL(async_client, send_(_, _, _)) - .WillOnce(Invoke( - [&](Http::MessagePtr& message, Http::AsyncClient::Callbacks& cb, - const absl::optional&) -> Http::AsyncClient::Request* { - EXPECT_EQ((Http::TestHeaderMapImpl{ - {":path", "/pubkey_path"}, - {":authority", "pubkey_server"}, - {":method", "GET"}, - }), - message->headers()); - callbacks = &cb; - return &request; - })); - - // Cancel is called once. - EXPECT_CALL(request, cancel()).Times(1); + EXPECT_CALL(*fetcher_, fetch(_, _)).Times(1); + EXPECT_CALL(*fetcher_, cancel()).Times(1); // onComplete() should not be called. EXPECT_CALL(mock_cb_, onComplete(_)).Times(0); @@ -400,8 +279,12 @@ TEST_F(AuthenticatorTest, TestNoForwardPayloadHeader) { auto& provider0 = (*proto_config_.mutable_providers())[std::string(ProviderName)]; provider0.clear_forward_payload_header(); CreateAuthenticator(); + EXPECT_CALL(*fetcher_, fetch(_, _)) + .WillOnce(Invoke( + [this](const ::envoy::api::v2::core::HttpUri&, JwksFetcher::JwksReceiver& receiver) { + receiver.onJwksSuccess(std::move(jwks_)); + })); - MockUpstream mock_pubkey(mock_factory_ctx_.cluster_manager_, PublicKey); auto headers = Http::TestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodToken)}}; MockAuthenticatorCallbacks mock_cb; EXPECT_CALL(mock_cb, onComplete(_)).WillOnce(Invoke([](const Status& status) { diff --git a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc index 536d373f21aaf..3440c215d207f 100644 --- a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc +++ b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc @@ -59,9 +59,7 @@ TEST_P(LocalJwksIntegrationTest, WithGoodToken) { EXPECT_EQ(payload_entry->value().getStringView(), ExpectedPayloadValue); // Verify the token is removed. EXPECT_FALSE(upstream_request_->headers().Authorization()); - upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, true); - response->waitForEndStream(); ASSERT_TRUE(response->complete()); EXPECT_STREQ("200", response->headers().Status()->value().c_str()); 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 a679043628aaf..4610e5d86e8b4 100644 --- a/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc +++ b/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc @@ -22,10 +22,12 @@ class JwksCacheTest : public ::testing::Test { void SetUp() { MessageUtil::loadFromYaml(ExampleConfig, config_); cache_ = JwksCache::create(config_); + jwks_ = google::jwt_verify::Jwks::createFrom(PublicKey, google::jwt_verify::Jwks::JWKS); } JwtAuthentication config_; JwksCachePtr cache_; + google::jwt_verify::JwksPtr jwks_; }; // Test findByIssuer @@ -44,7 +46,7 @@ TEST_F(JwksCacheTest, TestSetRemoteJwks) { auto jwks = cache_->findByIssuer("https://example.com"); EXPECT_TRUE(jwks->getJwksObj() == nullptr); - EXPECT_EQ(jwks->setRemoteJwks(PublicKey), Status::Ok); + EXPECT_EQ(jwks->setRemoteJwks(std::move(jwks_))->getStatus(), Status::Ok); EXPECT_FALSE(jwks->getJwksObj() == nullptr); EXPECT_FALSE(jwks->isExpired()); @@ -63,7 +65,7 @@ TEST_F(JwksCacheTest, TestSetRemoteJwksWithDefaultCacheDuration) { auto jwks = cache_->findByIssuer("https://example.com"); EXPECT_TRUE(jwks->getJwksObj() == nullptr); - EXPECT_EQ(jwks->setRemoteJwks(PublicKey), Status::Ok); + EXPECT_EQ(jwks->setRemoteJwks(std::move(jwks_))->getStatus(), Status::Ok); EXPECT_FALSE(jwks->getJwksObj() == nullptr); EXPECT_FALSE(jwks->isExpired()); } diff --git a/test/extensions/filters/http/jwt_authn/test_common.h b/test/extensions/filters/http/jwt_authn/test_common.h index 3568d4ef68d6f..8cad51b755085 100644 --- a/test/extensions/filters/http/jwt_authn/test_common.h +++ b/test/extensions/filters/http/jwt_authn/test_common.h @@ -85,6 +85,19 @@ const char ExpiredToken[] = "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJodH "V1AdwgX_5n3SmQTacVN0WcSgk6YJRZG6VE8PjxZP9bEameBmbSB0810giKRpdTU1-" "RJtjq6aCSTD4CYXtW38T5uko4V-S4zifK3BXeituUTebkgoA"; +// An NotYetValid token +// {"iss":"https://example.com","sub":"test@example.com","aud":"example_service","nbf":9223372036854775807} +const char NotYetValidToken[] = + "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9." + "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcGxlLmNvbSIsImF1" + "ZCI6ImV4YW1wbGVfc2VydmljZSIsIm5iZiI6OTIyMzM3MjAzNjg1NDc3NTgwN30K" + ".izDa6aHNgbsbeRzucE0baXIP7SXOrgopYQ" + "ALLFAsKq_N0GvOyqpAZA9nwCAhqCkeKWcL-9gbQe3XJa0KN3FPa2NbW4ChenIjmf2" + "QYXOuOQaDu9QRTdHEY2Y4mRy6DiTZAsBHWGA71_cLX-rzTSO_8aC8eIqdHo898oJw" + "3E8ISKdryYjayb9X3wtF6KLgNomoD9_nqtOkliuLElD8grO0qHKI1xQurGZNaoeyi" + "V1AdwgX_5n3SmQTacVN0WcSgk6YJRZG6VE8PjxZP9bEameBmbSB0810giKRpdTU1-" + "RJtjq6aCSTD4CYXtW38T5uko4V-S4zifK3BXeituUTebkgoA"; + // A token with aud as invalid_service // Payload: // {"iss":"https://example.com","sub":"test@example.com","aud":"invalid_service","exp":2001001001}