From b83bad8aa8d977458664e32a01f30509c5c4fbe5 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Mon, 23 Aug 2021 18:36:11 +0900 Subject: [PATCH] fix Signed-off-by: Shikugawa --- docs/README.md | 2 + run/config/config_jwk_uri.json | 34 +++ src/common/http/http.cc | 252 ++++++++++++++--------- src/common/http/http.h | 46 +++-- src/filters/filter_chain.cc | 39 ++-- src/filters/filter_chain.h | 9 +- src/filters/oidc/jwks_storage.cc | 13 +- src/filters/oidc/jwks_storage.h | 70 +++---- src/filters/oidc/oidc_filter.cc | 20 +- src/filters/oidc/token_response.cc | 13 +- src/filters/oidc/token_response.h | 5 +- src/service/async_service_impl.cc | 4 +- src/service/async_service_impl.h | 2 +- test/common/http/mocks.h | 15 +- test/filters/filter_chain_test.cc | 24 +-- test/filters/oidc/BUILD | 9 + test/filters/oidc/jwks_storage_test.cc | 69 +++++++ test/filters/oidc/oidc_filter_test.cc | 50 ++--- test/filters/oidc/token_response_test.cc | 7 +- test/service/async_service_impl_test.cc | 13 +- 20 files changed, 438 insertions(+), 258 deletions(-) create mode 100644 run/config/config_jwk_uri.json create mode 100644 test/filters/oidc/jwks_storage_test.cc diff --git a/docs/README.md b/docs/README.md index bcf4ebf1..2b104435 100644 --- a/docs/README.md +++ b/docs/README.md @@ -85,7 +85,9 @@ The configuration of an OpenID Connect filter that can be used to retrieve ident | authorization_uri | The OIDC Provider's [authorization endpoint](https://openid.net/specs/openid-connect-core-1_0.html#AuthorizationEndpoint). Required. | string | | token_uri | The OIDC Provider's [token endpoint](https://openid.net/specs/openid-connect-core-1_0.html#TokenEndpoint). Required. | string | | callback_uri | This value will be used as the `redirect_uri` param of the authorization code grant [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). This URL must be one of the Redirection URI values for the Client pre-registered at the OIDC provider. Note: The Istio gateway's VirtualService must be prepared to ensure that this URL will get routed to the service so that the Authservice can intercept the request and handle it (see [example](https://github.com/istio-ecosystem/authservice/blob/master/bookinfo-example/config/bookinfo-gateway.yaml)). Required. | string | +| jwks_config | | oneof | | jwks | The JSON JWKS response from the OIDC provider’s `jwks_uri` URI which can be found in the OIDC provider's [configuration response](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse). Note that this JSON value must be escaped when embedded in a json configmap (see [example](https://github.com/istio-ecosystem/authservice/blob/master/bookinfo-example/config/authservice-configmap-template.yaml)). Used during token verification. Required. | string | +| jwks_uri | | string | | client_id | The OIDC client ID assigned to the filter to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). Required. | string | | client_secret | The OIDC client secret assigned to the filter to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). Required. | string | | scopes | Additional scopes passed to the OIDC Provider in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). The `openid` scope is always sent to the OIDC Provider, and does not need to be specified here. Required, but an empty array is allowed. | (slice of) string | diff --git a/run/config/config_jwk_uri.json b/run/config/config_jwk_uri.json new file mode 100644 index 00000000..105ade93 --- /dev/null +++ b/run/config/config_jwk_uri.json @@ -0,0 +1,34 @@ +{ + "listen_address": "0.0.0.0", + "listen_port": "10004", + "log_level": "trace", + "threads": 8, + "chains": [ + { + "name": "idp_filter_chain", + "filters": [ + { + "oidc": + { + "authorization_uri": "https://localhost:8443/auth/realms/master/protocol/openid-connect/auth", + "token_uri": "https://localhost:8443/auth/realms/master/protocol/openid-connect/token", + "client_id": "test-istio", + "client_secret": "01d4c175-e090-40ce-9182-a585cb19b0f5", + "callback_uri": "https://localhost:9000/oauth/callback", + "jwks_uri": "https://www.googleapis.com/oauth2/v3/certs", + "scopes": [], + "id_token": { + "preamble": "Bearer", + "header": "Authorization" + }, + "logout": { + "path": "/authservice_logout", + "redirect_uri": "https://google.com" + } + } + } + ] + } + ] + } + \ No newline at end of file diff --git a/src/common/http/http.cc b/src/common/http/http.cc index 8b07cd19..7b98035a 100644 --- a/src/common/http/http.cc +++ b/src/common/http/http.cc @@ -4,6 +4,8 @@ #include #include #include +#include +#include #include #include "absl/strings/escaping.h" @@ -369,12 +371,12 @@ PathQueryFragment::PathQueryFragment(absl::string_view path_query_fragment) { } } -response_t HttpImpl::DoRequest( +response_t HttpImpl::Post( absl::string_view uri, const std::map &headers, - absl::string_view body, absl::string_view ca_cert, beast::http::verb method, + absl::string_view body, absl::string_view ca_cert, absl::string_view proxy_uri, boost::asio::io_context &ioc, - boost::asio::yield_context yield) { + boost::asio::yield_context yield) const { spdlog::trace("{}", __func__); try { int version = 11; @@ -448,17 +450,45 @@ response_t HttpImpl::DoRequest( } stream.async_handshake(ssl::stream_base::client, yield); - - switch (method) { - case beast::http::verb::post: - return Post(version, parsed_uri, headers, body, stream, yield); - case beast::http::verb::get: - return Get(version, parsed_uri, headers, stream, yield); - default: - spdlog::info("unsupported method"); - return response_t{}; + // Set up an HTTP POST request message + beast::http::request req{ + beast::http::verb::post, parsed_uri.GetPathQueryFragment(), version}; + req.set(beast::http::field::host, parsed_uri.GetHost()); + for (auto header : headers) { + req.set(boost::beast::string_view(header.first.data()), + boost::beast::string_view(header.second.data())); + } + auto &req_body = req.body(); + req_body.reserve(body.size()); + req_body.append(body.begin(), body.end()); + req.prepare_payload(); + // Send the HTTP request to the remote host + beast::http::async_write(stream, req, yield); + + // Read response + beast::flat_buffer buffer; + response_t res(new beast::http::response); + beast::http::async_read(stream, buffer, *res, yield); + + // Gracefully close the socket. + // Receive an error code instead of throwing an exception if this fails, so + // we can ignore some expected not_connected errors. + boost::system::error_code ec; + stream.async_shutdown(yield[ec]); + + if (ec) { + // not_connected happens sometimes so don't bother reporting it. + // stream_truncated also happens sometime and we choose to ignore the + // stream_truncated error, as recommended by the github thread: + // https://github.com/boostorg/beast/issues/824 + if (ec != beast::errc::not_connected && + ec != boost::asio::ssl::error::stream_truncated) { + spdlog::info("{}: HTTP error encountered: {}", __func__, ec.message()); + return response_t(); + } } + return res; // If we get here then the connection is closed gracefully } catch (std::exception const &e) { spdlog::info("{}: unexpected exception: {}", __func__, e.what()); @@ -466,93 +496,129 @@ response_t HttpImpl::DoRequest( } } -response_t HttpImpl::Post( - int version, Uri uri, +response_t HttpImpl::Get( + absl::string_view uri, const std::map &headers, - absl::string_view body, beast::ssl_stream &stream, - boost::asio::yield_context &yield) { - // Set up an HTTP POST request message - beast::http::request req{ - beast::http::verb::post, uri.GetPathQueryFragment(), version}; - req.set(beast::http::field::host, uri.GetHost()); - for (auto header : headers) { - req.set(boost::beast::string_view(header.first.data()), - boost::beast::string_view(header.second.data())); - } - auto &req_body = req.body(); - req_body.reserve(body.size()); - req_body.append(body.begin(), body.end()); - req.prepare_payload(); - // Send the HTTP request to the remote host - beast::http::async_write(stream, req, yield); - - // Read response - beast::flat_buffer buffer; - response_t res(new beast::http::response); - beast::http::async_read(stream, buffer, *res, yield); - - // Gracefully close the socket. - // Receive an error code instead of throwing an exception if this fails, so - // we can ignore some expected not_connected errors. - boost::system::error_code ec; - stream.async_shutdown(yield[ec]); - - if (ec) { - // not_connected happens sometimes so don't bother reporting it. - // stream_truncated also happens sometime and we choose to ignore the - // stream_truncated error, as recommended by the github thread: - // https://github.com/boostorg/beast/issues/824 - if (ec != beast::errc::not_connected && - ec != boost::asio::ssl::error::stream_truncated) { - spdlog::info("{}: HTTP error encountered: {}", __func__, ec.message()); - return response_t(); + absl::string_view body, absl::string_view ca_cert, + absl::string_view proxy_uri, boost::asio::io_context &ioc, + boost::asio::yield_context yield) const { + spdlog::trace("{}", __func__); + try { + int version = 11; + + ssl::context ctx(ssl::context::tlsv12_client); + ctx.set_verify_mode(ssl::verify_peer); + ctx.set_default_verify_paths(); + + if (!ca_cert.empty()) { + spdlog::info("{}: Trusting the provided certificate authority", __func__); + beast::error_code ca_ec; + ctx.add_certificate_authority( + boost::asio::buffer(ca_cert.data(), ca_cert.size()), ca_ec); + if (ca_ec) { + throw boost::system::system_error{ca_ec}; + } } - } - return res; -} + auto parsed_uri = http::Uri(uri); -response_t HttpImpl::Get( - int version, Uri uri, - const std::map &headers, - beast::ssl_stream &stream, - boost::asio::yield_context &yield) { - // Set up an HTTP GET request message - beast::http::request req{ - beast::http::verb::get, uri.GetPathQueryFragment(), version}; - req.set(beast::http::field::host, uri.GetHost()); - for (auto header : headers) { - req.set(boost::beast::string_view(header.first.data()), - boost::beast::string_view(header.second.data())); - } - req.prepare_payload(); - // Send the HTTP request to the remote host - beast::http::async_write(stream, req, yield); - - // Read response - beast::flat_buffer buffer; - response_t res(new beast::http::response); - beast::http::async_read(stream, buffer, *res, yield); - - // Gracefully close the socket. - // Receive an error code instead of throwing an exception if this fails, so - // we can ignore some expected not_connected errors. - boost::system::error_code ec; - stream.async_shutdown(yield[ec]); - - if (ec) { - // not_connected happens sometimes so don't bother reporting it. - // stream_truncated also happens sometime and we choose to ignore the - // stream_truncated error, as recommended by the github thread: - // https://github.com/boostorg/beast/issues/824 - if (ec != beast::errc::not_connected && - ec != boost::asio::ssl::error::stream_truncated) { - spdlog::info("{}: HTTP error encountered: {}", __func__, ec.message()); - return response_t(); + tcp::resolver resolver(ioc); + beast::ssl_stream stream(ioc, ctx); + if (!SSL_set_tlsext_host_name(stream.native_handle(), + parsed_uri.GetHost().c_str())) { + throw boost::system::error_code{static_cast(::ERR_get_error()), + boost::asio::error::get_ssl_category()}; } - } - return res; + if (!proxy_uri.empty()) { + auto parsed_proxy_uri = http::Uri(proxy_uri); + + const auto results = resolver.async_resolve( + parsed_proxy_uri.GetHost(), + std::to_string(parsed_proxy_uri.GetPort()), yield); + spdlog::info( + "{}: opening connection to proxy {} for request to destination {}:{}", + __func__, proxy_uri.data(), parsed_uri.GetHost(), + parsed_uri.GetPort()); + beast::get_lowest_layer(stream).async_connect(results, yield); + + std::string target = absl::StrCat(parsed_uri.GetHost(), ":", + std::to_string(parsed_uri.GetPort())); + beast::http::request http_connect_req{ + beast::http::verb::connect, target, version}; + http_connect_req.set(beast::http::field::host, target); + + // Send the HTTP connect request to the remote host + beast::http::async_write(stream.next_layer(), http_connect_req, yield); + + // Read the response from the server + boost::beast::flat_buffer http_connect_buffer; + beast::http::response http_connect_res; + beast::http::parser p(http_connect_res); + p.skip(true); // skip reading the body of the response because there + // won't be a body + + beast::http::async_read(stream.next_layer(), http_connect_buffer, p, + yield); + if (http_connect_res.result() != beast::http::status::ok) { + throw std::runtime_error( + absl::StrCat("http connect failed with status: ", + http_connect_res.result_int())); + } + + } else { + spdlog::info("{}: opening connection to {}:{}", __func__, + parsed_uri.GetHost(), parsed_uri.GetPort()); + const auto results = resolver.async_resolve( + parsed_uri.GetHost(), std::to_string(parsed_uri.GetPort()), yield); + beast::get_lowest_layer(stream).async_connect(results, yield); + } + + stream.async_handshake(ssl::stream_base::client, yield); + // Set up an HTTP POST request message + beast::http::request req{ + beast::http::verb::get, parsed_uri.GetPathQueryFragment(), version}; + req.set(beast::http::field::host, parsed_uri.GetHost()); + for (auto header : headers) { + req.set(boost::beast::string_view(header.first.data()), + boost::beast::string_view(header.second.data())); + } + auto &req_body = req.body(); + req_body.reserve(body.size()); + req_body.append(body.begin(), body.end()); + req.prepare_payload(); + // Send the HTTP request to the remote host + beast::http::async_write(stream, req, yield); + + // Read response + beast::flat_buffer buffer; + response_t res(new beast::http::response); + beast::http::async_read(stream, buffer, *res, yield); + + // Gracefully close the socket. + // Receive an error code instead of throwing an exception if this fails, so + // we can ignore some expected not_connected errors. + boost::system::error_code ec; + stream.async_shutdown(yield[ec]); + + if (ec) { + // not_connected happens sometimes so don't bother reporting it. + // stream_truncated also happens sometime and we choose to ignore the + // stream_truncated error, as recommended by the github thread: + // https://github.com/boostorg/beast/issues/824 + if (ec != beast::errc::not_connected && + ec != boost::asio::ssl::error::stream_truncated) { + spdlog::info("{}: HTTP error encountered: {}", __func__, ec.message()); + return response_t(); + } + } + + return res; + // If we get here then the connection is closed gracefully + } catch (std::exception const &e) { + spdlog::info("{}: unexpected exception: {}", __func__, e.what()); + return response_t(); + } } } // namespace http diff --git a/src/common/http/http.h b/src/common/http/http.h index 5b85938e..eeaebe26 100644 --- a/src/common/http/http.h +++ b/src/common/http/http.h @@ -4,7 +4,6 @@ #include #include #include -#include #include #include #include @@ -178,15 +177,29 @@ class Http { * @param headers the http headers * @param body the http request body * @param ca_cert the ca cert to be trusted in the http call - * @param method the http method to use * @return http response. */ - virtual response_t DoRequest( + virtual response_t Post( absl::string_view uri, const std::map &headers, absl::string_view body, absl::string_view ca_cert, - beast::http::verb method, absl::string_view proxy_uri, - boost::asio::io_context &ioc, boost::asio::yield_context yield) = 0; + absl::string_view proxy_uri, boost::asio::io_context &ioc, + boost::asio::yield_context yield) const = 0; + + /** @brief Asynchronously send a Get http message with a certificate + * authority. To be used inside a Boost co-routine. + * @param endpoint the endpoint to call + * @param headers the http headers + * @param body the http request body + * @param ca_cert the ca cert to be trusted in the http call + * @return http response. + */ + virtual response_t Get( + absl::string_view uri, + const std::map &headers, + absl::string_view body, absl::string_view ca_cert, + absl::string_view proxy_uri, boost::asio::io_context &ioc, + boost::asio::yield_context yield) const = 0; }; /** @@ -194,24 +207,17 @@ class Http { */ class HttpImpl : public Http { public: - response_t DoRequest( - absl::string_view uri, - const std::map &headers, - absl::string_view body, absl::string_view ca_cert, - beast::http::verb method, absl::string_view proxy_uri, - boost::asio::io_context &ioc, boost::asio::yield_context yield) override; - - private: - response_t Post(int version, Uri uri, + response_t Post(absl::string_view uri, const std::map &headers, - absl::string_view body, - beast::ssl_stream &stream, - boost::asio::yield_context &yield); + absl::string_view body, absl::string_view ca_cert, + absl::string_view proxy_uri, boost::asio::io_context &ioc, + boost::asio::yield_context yield) const override; - response_t Get(int version, Uri uri, + response_t Get(absl::string_view uri, const std::map &headers, - beast::ssl_stream &stream, - boost::asio::yield_context &yield); + absl::string_view body, absl::string_view ca_cert, + absl::string_view proxy_uri, boost::asio::io_context &ioc, + boost::asio::yield_context yield) const override; }; } // namespace http diff --git a/src/filters/filter_chain.cc b/src/filters/filter_chain.cc index 77802678..fb3772a1 100644 --- a/src/filters/filter_chain.cc +++ b/src/filters/filter_chain.cc @@ -19,16 +19,30 @@ namespace authservice { namespace filters { -FilterChainImpl::FilterChainImpl(config::FilterChain config, +FilterChainImpl::FilterChainImpl(boost::asio::io_context& ioc, + config::FilterChain config, unsigned int threads) : threads_(threads), config_(std::move(config)), - oidc_session_store_(nullptr) {} + oidc_session_store_(nullptr) { + for (auto& filter : config_.filters()) { + if (filter.oidc().jwks_config_case() == config::oidc::OIDCConfig::kJwks) { + jwks_storage_map_.emplace_back( + std::make_shared( + filter.oidc().jwks(), google::jwt_verify::Jwks::Type::JWKS)); + } else { + jwks_storage_map_.emplace_back( + std::make_shared( + filter.oidc().jwks_uri(), + std::chrono::seconds(5) /* TODO(shikugawa): fixme */, ioc)); + } + } +} -const std::string &FilterChainImpl::Name() const { return config_.name(); } +const std::string& FilterChainImpl::Name() const { return config_.name(); } bool FilterChainImpl::Matches( - const ::envoy::service::auth::v3::CheckRequest *request) const { + const ::envoy::service::auth::v3::CheckRequest* request) const { spdlog::trace("{}", __func__); if (config_.has_match()) { auto matched = request->attributes().request().http().headers().find( @@ -49,11 +63,12 @@ bool FilterChainImpl::Matches( return true; } -std::unique_ptr FilterChainImpl::New(boost::asio::io_context &ioc) { +std::unique_ptr FilterChainImpl::New() { spdlog::trace("{}", __func__); std::unique_ptr result(new Pipe); int oidc_filter_count = 0; - for (auto &filter : *config_.mutable_filters()) { + for (auto i = 0; i < config_.filters_size(); ++i) { + auto& filter = *config_.mutable_filters(i); if (filter.has_oidc()) { ++oidc_filter_count; } else if (filter.has_mock()) { @@ -67,19 +82,9 @@ std::unique_ptr FilterChainImpl::New(boost::asio::io_context &ioc) { throw std::runtime_error( "only one filter of type \"oidc\" is allowed in a chain"); } - - std::unique_ptr jwks_storage; - if (filter.oidc().jwks_config_case() == config::oidc::OIDCConfig::kJwks) { - jwks_storage = std::make_unique( - filter.oidc().jwks(), google::jwt_verify::Jwks::Type::JWKS); - } else { - jwks_storage = std::make_unique( - filter.oidc().jwks_uri(), ioc); - } - auto token_response_parser = std::make_shared( - std::move(jwks_storage)); + jwks_storage_map_[i]->jwks()); auto session_string_generator = std::make_shared(); diff --git a/src/filters/filter_chain.h b/src/filters/filter_chain.h index 4e320681..68dd918c 100644 --- a/src/filters/filter_chain.h +++ b/src/filters/filter_chain.h @@ -8,6 +8,7 @@ #include "config/oidc/config.pb.h" #include "envoy/service/auth/v3/external_auth.grpc.pb.h" #include "src/filters/filter.h" +#include "src/filters/oidc/jwks_storage.h" #include "src/filters/oidc/session_store.h" namespace authservice { @@ -40,7 +41,7 @@ class FilterChain { * New creates a new filter instance that can be used to process a request. * @return a new filter instance. */ - virtual std::unique_ptr New(boost::asio::io_context &ioc) = 0; + virtual std::unique_ptr New() = 0; /** * Invoked periodically to give the filter chain a chance to clean up expired @@ -54,16 +55,18 @@ class FilterChainImpl : public FilterChain { unsigned int threads_; config::FilterChain config_; std::shared_ptr oidc_session_store_; + std::vector> jwks_storage_map_; public: - explicit FilterChainImpl(config::FilterChain config, unsigned int threads); + explicit FilterChainImpl(boost::asio::io_context &ioc, + config::FilterChain config, unsigned int threads); const std::string &Name() const override; bool Matches( const ::envoy::service::auth::v3::CheckRequest *request) const override; - std::unique_ptr New(boost::asio::io_context &ioc) override; + std::unique_ptr New() override; virtual void DoPeriodicCleanup() override; }; diff --git a/src/filters/oidc/jwks_storage.cc b/src/filters/oidc/jwks_storage.cc index 9ef2817a..f10c7839 100644 --- a/src/filters/oidc/jwks_storage.cc +++ b/src/filters/oidc/jwks_storage.cc @@ -1,29 +1,32 @@ #include "src/filters/oidc/jwks_storage.h" +#include namespace authservice { namespace filters { namespace oidc { NonPermanentJwksStorageImpl::JwksFetcher::JwksFetcher( JwksStorage* parent, common::http::ptr_t http_ptr, - const std::string& jwks_uri, boost::asio::io_context& io_context) + const std::string& jwks_uri, std::chrono::seconds duration, + boost::asio::io_context& io_context) : parent_(parent), jwks_uri_(jwks_uri), http_ptr_(http_ptr), ioc_(io_context), - timer_(ioc_, boost::posix_time::seconds(5)) {} + timer_(ioc_, duration) {} void NonPermanentJwksStorageImpl::JwksFetcher::request( const boost::system::error_code&) { boost::asio::spawn(ioc_, [this](boost::asio::yield_context yield) { - auto resp = http_ptr_->DoRequest(jwks_uri_, {}, "", "", - beast::http::verb::get, "", ioc_, yield); + std::cout << "spawn" << std::endl; + auto resp = http_ptr_->Get(jwks_uri_, {}, "", "", "", ioc_, yield); const auto& fetched_jwks = resp->body(); // TODO(shikugawa): Prevent lock with same jwks. // TODO(shikugawa): Support PEM. parent_->updateJwks(fetched_jwks, google::jwt_verify::Jwks::Type::JWKS); - timer_.expires_at(timer_.expires_at() + boost::posix_time::seconds(5)); + timer_.expires_at(std::chrono::steady_clock::now() + + std::chrono::seconds(5)); timer_.async_wait( [this](const boost::system::error_code& ec) { this->request(ec); }); }); diff --git a/src/filters/oidc/jwks_storage.h b/src/filters/oidc/jwks_storage.h index 182cac16..4f982030 100644 --- a/src/filters/oidc/jwks_storage.h +++ b/src/filters/oidc/jwks_storage.h @@ -3,7 +3,6 @@ #include -#include #include #include @@ -21,17 +20,7 @@ class JwksStorage { virtual void updateJwks(const std::string& new_jwks, google::jwt_verify::Jwks::Type type) = 0; - virtual const google::jwt_verify::JwksPtr& jwks() const = 0; -}; - -class JwksStorageImpl : public JwksStorage { - public: - virtual void updateJwks(const std::string&, - google::jwt_verify::Jwks::Type) override {} - - virtual const google::jwt_verify::JwksPtr& jwks() const override { - return jwks_; - } + virtual google::jwt_verify::JwksPtr jwks() = 0; protected: google::jwt_verify::JwksPtr parseJwks(const std::string& jwks, @@ -40,33 +29,39 @@ class JwksStorageImpl : public JwksStorage { spdlog::debug("status for jwks parsing: {}, {}", __func__, google::jwt_verify::getStatusString(jwks_keys->getStatus())); - if (jwks_keys->getStatus() == google::jwt_verify::Status::Ok) { - return jwks_keys; + if (jwks_keys->getStatus() != google::jwt_verify::Status::Ok) { + spdlog::warn("{}: failed to parse new JWKs", __func__); } - return nullptr; + return jwks_keys; } - - google::jwt_verify::JwksPtr jwks_; }; -class PermanentJwksStorageImpl : public JwksStorageImpl { +class PermanentJwksStorageImpl : public JwksStorage { public: explicit PermanentJwksStorageImpl(const std::string& jwks, - google::jwt_verify::Jwks::Type type) { - jwks_ = parseJwks(jwks, type); - } + google::jwt_verify::Jwks::Type type) + : jwks_(jwks), type_(type) {} void updateJwks(const std::string&, google::jwt_verify::Jwks::Type) override { } + + virtual google::jwt_verify::JwksPtr jwks() override { + return parseJwks(jwks_, type_); + } + + private: + std::string jwks_; + google::jwt_verify::Jwks::Type type_; }; -class NonPermanentJwksStorageImpl : public JwksStorageImpl { +class NonPermanentJwksStorageImpl : public JwksStorage { public: class JwksFetcher { public: JwksFetcher(JwksStorage* parent, common::http::ptr_t http_ptr, - const std::string& jwks_uri, boost::asio::io_context& ioc); + const std::string& jwks_uri, std::chrono::seconds duration, + boost::asio::io_context& ioc); private: void request(const boost::system::error_code&); @@ -75,33 +70,34 @@ class NonPermanentJwksStorageImpl : public JwksStorageImpl { const std::string jwks_uri_; common::http::ptr_t http_ptr_; boost::asio::io_context& ioc_; - boost::asio::deadline_timer timer_; + boost::asio::steady_timer timer_; }; explicit NonPermanentJwksStorageImpl(const std::string& jwks_uri, - boost::asio::io_context& ioc) - : jwks_fetcher_(std::make_unique( - this, common::http::ptr_t(new common::http::HttpImpl), jwks_uri, - ioc)) {} + std::chrono::seconds duration, + boost::asio::io_context& ioc) { + if (duration != std::chrono::seconds(0)) { + jwks_fetcher_ = std::make_unique( + this, common::http::ptr_t(new common::http::HttpImpl), jwks_uri, + duration, ioc); + } + } void updateJwks(const std::string& new_jwks, google::jwt_verify::Jwks::Type type) override { std::unique_lock lck(mux_); - auto new_jwks_keys = parseJwks(new_jwks, type); - - if (new_jwks_keys->getStatus() == google::jwt_verify::Status::Ok) { - jwks_.reset(new_jwks_keys.get()); - } else { - spdlog::error("failed to parse new JWK"); - } + jwks_ = new_jwks; + type_ = type; } - const google::jwt_verify::JwksPtr& jwks() const override { + google::jwt_verify::JwksPtr jwks() override { std::unique_lock lck(mux_); - return jwks_; + return parseJwks(jwks_, type_); } private: + std::string jwks_; + google::jwt_verify::Jwks::Type type_; std::unique_ptr jwks_fetcher_; mutable std::mutex mux_; }; diff --git a/src/filters/oidc/oidc_filter.cc b/src/filters/oidc/oidc_filter.cc index cb775d46..7ce18c62 100644 --- a/src/filters/oidc/oidc_filter.cc +++ b/src/filters/oidc/oidc_filter.cc @@ -537,11 +537,11 @@ std::shared_ptr OidcFilter::RefreshToken( }; spdlog::info("{}: POSTing to refresh access token", __func__); - auto retrieved_token_response = http_ptr_->DoRequest( - idp_config_.token_uri(), headers, - common::http::Http::EncodeFormData(params), - idp_config_.trusted_certificate_authority(), beast::http::verb::post, - idp_config_.proxy_uri(), ioc, yield); + auto retrieved_token_response = + http_ptr_->Post(idp_config_.token_uri(), headers, + common::http::Http::EncodeFormData(params), + idp_config_.trusted_certificate_authority(), + idp_config_.proxy_uri(), ioc, yield); if (retrieved_token_response == nullptr) { spdlog::warn( @@ -633,11 +633,11 @@ google::rpc::Code OidcFilter::RetrieveToken( {"grant_type", "authorization_code"}, }; - auto retrieve_token_response = http_ptr_->DoRequest( - idp_config_.token_uri(), headers, - common::http::Http::EncodeFormData(params), - idp_config_.trusted_certificate_authority(), beast::http::verb::post, - idp_config_.proxy_uri(), ioc, yield); + auto retrieve_token_response = + http_ptr_->Post(idp_config_.token_uri(), headers, + common::http::Http::EncodeFormData(params), + idp_config_.trusted_certificate_authority(), + idp_config_.proxy_uri(), ioc, yield); if (retrieve_token_response == nullptr) { spdlog::info("{}: HTTP error encountered: {}", __func__, "IdP connection error"); diff --git a/src/filters/oidc/token_response.cc b/src/filters/oidc/token_response.cc index f7a6707c..5ed06c4e 100644 --- a/src/filters/oidc/token_response.cc +++ b/src/filters/oidc/token_response.cc @@ -66,8 +66,8 @@ int64_t TokenResponse::GetIDTokenExpiry() const { } TokenResponseParserImpl::TokenResponseParserImpl( - std::unique_ptr jwks_storage) - : jwks_storage_(std::move(jwks_storage)) {} + google::jwt_verify::JwksPtr keys) + : keys_(std::move(keys)) {} std::shared_ptr TokenResponseParserImpl::Parse( const std::string &client_id, const std::string &nonce, @@ -238,14 +238,7 @@ bool TokenResponseParserImpl::IsIDTokenInvalid( // value. Verify the token signature & that our client_id is set as an entry // in the token's `aud` field. std::vector audiences = {client_id}; - - if (jwks_storage_->jwks() == nullptr) { - spdlog::info("{}: no JWKs for token validation.", __func__); - return true; - } - - auto jwt_status = google::jwt_verify::verifyJwt( - id_token, *jwks_storage_->jwks(), audiences); + auto jwt_status = google::jwt_verify::verifyJwt(id_token, *keys_, audiences); if (jwt_status != google::jwt_verify::Status::Ok) { spdlog::info("{}: `id_token` verification failed: {}", __func__, google::jwt_verify::getStatusString(jwt_status)); diff --git a/src/filters/oidc/token_response.h b/src/filters/oidc/token_response.h index 79bad831..8a0702b2 100644 --- a/src/filters/oidc/token_response.h +++ b/src/filters/oidc/token_response.h @@ -5,7 +5,6 @@ #include "absl/types/optional.h" #include "jwt_verify_lib/jwks.h" #include "jwt_verify_lib/jwt.h" -#include "src/filters/oidc/jwks_storage.h" namespace authservice { namespace filters { @@ -69,7 +68,7 @@ class TokenResponseParser { */ class TokenResponseParserImpl final : public TokenResponseParser { private: - std::unique_ptr jwks_storage_; + google::jwt_verify::JwksPtr keys_; absl::optional ParseAccessTokenExpiry( google::protobuf::Map &fields) @@ -85,7 +84,7 @@ class TokenResponseParserImpl final : public TokenResponseParser { google::protobuf::Map fields) const; public: - TokenResponseParserImpl(std::unique_ptr jwks_storage); + TokenResponseParserImpl(google::jwt_verify::JwksPtr keys); std::shared_ptr Parse( const std::string &client_id, const std::string &nonce, const std::string &raw_response_string) const override; diff --git a/src/service/async_service_impl.cc b/src/service/async_service_impl.cc index 2a101ae4..b00148ac 100644 --- a/src/service/async_service_impl.cc +++ b/src/service/async_service_impl.cc @@ -121,8 +121,8 @@ AsyncAuthServiceImpl::AsyncAuthServiceImpl(const config::Config &config) interval_in_seconds_(60), timer_(*io_context_, interval_in_seconds_) { for (const auto &chain_config : config_.chains()) { - auto chain = std::make_unique(chain_config, - config_.threads()); + auto chain = std::make_unique( + *io_context_, chain_config, config_.threads()); chains_.push_back(std::move(chain)); } grpc::ServerBuilder builder; diff --git a/src/service/async_service_impl.h b/src/service/async_service_impl.h index 3b9ad1d1..683c2fed 100644 --- a/src/service/async_service_impl.h +++ b/src/service/async_service_impl.h @@ -65,7 +65,7 @@ ::grpc::Status Check( envoy::service::auth::v3::CheckResponse response_v3; // Create a new instance of a processor. - auto processor = chain->New(ioc); + auto processor = chain->New(); auto status = processor->Process(&request_v3, &response_v3, ioc, yield); // response v2/v3 conversion layer diff --git a/test/common/http/mocks.h b/test/common/http/mocks.h index 182afe80..749937f6 100644 --- a/test/common/http/mocks.h +++ b/test/common/http/mocks.h @@ -9,13 +9,20 @@ namespace common { namespace http { class HttpMock : public Http { public: - MOCK_METHOD8( - DoRequest, + MOCK_CONST_METHOD7( + Post, response_t(absl::string_view uri, const std::map &headers, absl::string_view body, absl::string_view ca_cert, - beast::http::verb method, absl::string_view proxy_uri, - boost::asio::io_context &ioc, + absl::string_view proxy_uri, boost::asio::io_context &ioc, + boost::asio::yield_context yield)); + + MOCK_CONST_METHOD7( + Get, + response_t(absl::string_view uri, + const std::map &headers, + absl::string_view body, absl::string_view ca_cert, + absl::string_view proxy_uri, boost::asio::io_context &ioc, boost::asio::yield_context yield)); }; } // namespace http diff --git a/test/filters/filter_chain_test.cc b/test/filters/filter_chain_test.cc index 8c2684bf..8e4d07d9 100644 --- a/test/filters/filter_chain_test.cc +++ b/test/filters/filter_chain_test.cc @@ -8,11 +8,13 @@ namespace authservice { namespace filters { +boost::asio::io_context io_context; + TEST(FilterChainTest, Name) { auto configuration = std::unique_ptr(new config::FilterChain); configuration->set_name("expected"); - FilterChainImpl chain1(*configuration, 1); + FilterChainImpl chain1(io_context, *configuration, 1); ASSERT_EQ(chain1.Name(), "expected"); } @@ -20,7 +22,7 @@ TEST(FilterChainTest, MatchesWithoutMatchField) { auto configuration = std::unique_ptr(new config::FilterChain); ::envoy::service::auth::v3::CheckRequest request1; - FilterChainImpl chain1(*configuration, 1); + FilterChainImpl chain1(io_context, *configuration, 1); ASSERT_TRUE(chain1.Matches(&request1)); } @@ -37,7 +39,7 @@ TEST(FilterChainTest, MatchesPrefix) { ->mutable_http() ->mutable_headers(); headers1->insert({"x-prefix-header", "not-prefixed-value"}); - FilterChainImpl chain1(*configuration, 1); + FilterChainImpl chain1(io_context, *configuration, 1); ASSERT_FALSE(chain1.Matches(&request1)); // valid prefix case @@ -47,7 +49,7 @@ TEST(FilterChainTest, MatchesPrefix) { ->mutable_http() ->mutable_headers(); headers2->insert({"x-prefix-header", "prefixed-value"}); - FilterChainImpl chain2(*configuration, 1); + FilterChainImpl chain2(io_context, *configuration, 1); ASSERT_TRUE(chain2.Matches(&request2)); } @@ -64,7 +66,7 @@ TEST(FilterChainTest, MatchesEquality) { ->mutable_http() ->mutable_headers(); headers1->insert({"x-equality-header", "not-an-exact-value"}); - FilterChainImpl chain1(*configuration, 1); + FilterChainImpl chain1(io_context, *configuration, 1); ASSERT_FALSE(chain1.Matches(&request1)); // valid header value case @@ -74,30 +76,28 @@ TEST(FilterChainTest, MatchesEquality) { ->mutable_http() ->mutable_headers(); headers2->insert({"x-equality-header", "exact-value"}); - FilterChainImpl chain2(*configuration, 1); + FilterChainImpl chain2(io_context, *configuration, 1); ASSERT_TRUE(chain2.Matches(&request2)); } TEST(FilterChainTest, New) { - boost::asio::io_context ctx; auto configuration = std::unique_ptr(new config::FilterChain); auto filter_config = configuration->mutable_filters()->Add(); filter_config->mutable_oidc()->set_jwks("some-value"); - FilterChainImpl chain(*configuration, 1); - auto instance = chain.New(ctx); + FilterChainImpl chain(io_context, *configuration, 1); + auto instance = chain.New(); ASSERT_TRUE(dynamic_cast(instance.get()) != nullptr); } TEST(FilterChainTest, MockFilter) { - boost::asio::io_context ctx; auto configuration = std::make_unique(); auto filter_config = configuration->mutable_filters()->Add(); filter_config->mutable_mock()->set_allow(true); - FilterChainImpl chain(*configuration, 1); - auto instance = chain.New(ctx); + FilterChainImpl chain(io_context, *configuration, 1); + auto instance = chain.New(); ASSERT_TRUE(dynamic_cast(instance.get()) != nullptr); } diff --git a/test/filters/oidc/BUILD b/test/filters/oidc/BUILD index b1c1227d..63d9b28d 100644 --- a/test/filters/oidc/BUILD +++ b/test/filters/oidc/BUILD @@ -74,3 +74,12 @@ cc_test( ], linkstatic = select({"@boost//:osx": True, "//conditions:default": False}), # workaround for not being able to figure out how to link dynamically on MacOS ) + +cc_test( + name = "jwks_storage_test", + srcs = ["jwks_storage_test.cc"], + deps = [ + "//src/filters/oidc:jwks_storage", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/test/filters/oidc/jwks_storage_test.cc b/test/filters/oidc/jwks_storage_test.cc new file mode 100644 index 00000000..c2ec8fba --- /dev/null +++ b/test/filters/oidc/jwks_storage_test.cc @@ -0,0 +1,69 @@ +#include "src/filters/oidc/jwks_storage.h" + +#include "gtest/gtest.h" + +namespace authservice { +namespace filters { +namespace oidc { + +namespace { +// A good public key based on above private key +const char valid_jwt_signing_key_[] = 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 char *invalid_jwt_signing_key_ = + "MIIBCgKCAQEAnzyis1ZjfNB0bBgKFMSvvkTtwlvBsaJq7S5wA+kzeVOVpVWwkWdV" + "ha4s38XM/pa/yr47av7+z3VTmvDRyAHcaT92whREFpLv9cj5lTeJSibyr/Mrm/Yt" + "jCZVWgaOYIhwrXwKLqPr/11inWsAkfIytvHWTxZYEcXLgAXFuUuaS3uF9gEiNQwz" + "GTU1v0FqkqTBr4B8nW3HCN47XUu0t8Y0e+lf4s4OxQawWD79J9/5d3Ry0vbV3Am1" + "FtGJiJvOwRsIfVChDpYStTcHTCMqtvWbV6L11BWkpAGXSW4Hv43qa+GSYOD2QU68" + "Mb59oSk2OB+BtOLpJofmbGEGgvmwyCI9MwIDAQAB"; + +TEST(JwksStorageTest, TestPermanentJwksStorage) { + PermanentJwksStorageImpl storage(valid_jwt_signing_key_, + google::jwt_verify::Jwks::JWKS); + EXPECT_EQ(google::jwt_verify::Status::Ok, storage.jwks()->getStatus()); + + PermanentJwksStorageImpl storage2(invalid_jwt_signing_key_, + google::jwt_verify::Jwks::JWKS); + EXPECT_NE(google::jwt_verify::Status::Ok, storage2.jwks()->getStatus()); +} + +TEST(JwksStorageTest, TestNonPermanentJwksStorage) { + boost::asio::io_context io_context; + NonPermanentJwksStorageImpl storage("istio.io", std::chrono::seconds(0), + io_context); + storage.updateJwks(valid_jwt_signing_key_, google::jwt_verify::Jwks::JWKS); + EXPECT_EQ(google::jwt_verify::Status::Ok, storage.jwks()->getStatus()); + + // update key + storage.updateJwks(invalid_jwt_signing_key_, google::jwt_verify::Jwks::JWKS); + EXPECT_NE(google::jwt_verify::Status::Ok, storage.jwks()->getStatus()); +} + +} // namespace +} // namespace oidc +} // namespace filters +} // namespace authservice diff --git a/test/filters/oidc/oidc_filter_test.cc b/test/filters/oidc/oidc_filter_test.cc index da49f2e6..fed29de5 100644 --- a/test/filters/oidc/oidc_filter_test.cc +++ b/test/filters/oidc/oidc_filter_test.cc @@ -424,9 +424,8 @@ TEST_F( auto *pMessage = new beast::http::response(); auto raw_http_token_response_from_idp = common::http::response_t(pMessage); raw_http_token_response_from_idp->result(beast::http::status::ok); - EXPECT_CALL(*mocked_http, DoRequest(Eq(token_uri), _, _, Eq("some-ca"), - Eq(beast::http::verb::post), - Eq("http://some-proxy-uri.com"), _, _)) + EXPECT_CALL(*mocked_http, Post(Eq(token_uri), _, _, Eq("some-ca"), + Eq("http://some-proxy-uri.com"), _, _)) .WillOnce(Return(ByMove(std::move(raw_http_token_response_from_idp)))); auto jwt_status = @@ -480,9 +479,8 @@ TEST_F(OidcFilterTest, auto *pMessage = new beast::http::response(); auto raw_http_token_response_from_idp = common::http::response_t(pMessage); raw_http_token_response_from_idp->result(beast::http::status::ok); - EXPECT_CALL(*mocked_http, DoRequest(Eq(token_uri), _, _, Eq("some-ca"), - Eq(beast::http::verb::post), - Eq("http://some-proxy-uri.com"), _, _)) + EXPECT_CALL(*mocked_http, Post(Eq(token_uri), _, _, Eq("some-ca"), + Eq("http://some-proxy-uri.com"), _, _)) .WillOnce(Return(ByMove(std::move(raw_http_token_response_from_idp)))); auto jwt_status = @@ -578,9 +576,8 @@ TEST_F( auto *pMessage = new beast::http::response(); auto raw_http_token_response_from_idp = common::http::response_t(pMessage); raw_http_token_response_from_idp->result(beast::http::status::ok); - EXPECT_CALL(*mocked_http, DoRequest(Eq(token_uri), _, _, Eq("some-ca"), - Eq(beast::http::verb::post), - Eq("http://some-proxy-uri.com"), _, _)) + EXPECT_CALL(*mocked_http, Post(Eq(token_uri), _, _, Eq("some-ca"), + Eq("http://some-proxy-uri.com"), _, _)) .WillOnce(Return(ByMove(std::move(raw_http_token_response_from_idp)))); EXPECT_CALL(*parser_mock_, ParseRefreshTokenResponse(_, _)) @@ -626,9 +623,8 @@ TEST_F( SetExpiredAccessTokenResponseInSessionStore(); auto mocked_http = new common::http::HttpMock(); - EXPECT_CALL(*mocked_http, DoRequest(Eq(token_uri), _, _, Eq("some-ca"), - Eq(beast::http::verb::post), - Eq("http://some-proxy-uri.com"), _, _)) + EXPECT_CALL(*mocked_http, Post(Eq(token_uri), _, _, Eq("some-ca"), + Eq("http://some-proxy-uri.com"), _, _)) .WillOnce(Return(ByMove(nullptr))); auto old_session_id = std::string("session123"); @@ -673,9 +669,8 @@ TEST_F( auto *pMessage = new beast::http::response(); auto raw_http_token_response_from_idp = common::http::response_t(pMessage); raw_http_token_response_from_idp->result(beast::http::status::bad_request); - EXPECT_CALL(*mocked_http, DoRequest(Eq(token_uri), _, _, Eq("some-ca"), - Eq(beast::http::verb::post), - Eq("http://some-proxy-uri.com"), _, _)) + EXPECT_CALL(*mocked_http, Post(Eq(token_uri), _, _, Eq("some-ca"), + Eq("http://some-proxy-uri.com"), _, _)) .WillOnce(Return(ByMove(std::move(raw_http_token_response_from_idp)))); // we want the code to return before attempting to parse the bad response @@ -1063,9 +1058,8 @@ google::rpc::Code OidcFilterTest::MakeRequestWhichWillCauseTokenRetrieval( auto raw_http = common::http::response_t( new beast::http::response()); raw_http->result(beast::http::status::ok); - EXPECT_CALL(*mocked_http, DoRequest(Eq(token_uri), _, _, Eq("some-ca"), - Eq(beast::http::verb::post), - Eq("http://some-proxy-uri.com"), _, _)) + EXPECT_CALL(*mocked_http, Post(Eq(token_uri), _, _, Eq("some-ca"), + Eq("http://some-proxy-uri.com"), _, _)) .WillOnce(Return(ByMove(std::move(raw_http)))); OidcFilter filter(common::http::ptr_t(mocked_http), config_, parser_mock_, session_string_generator_mock_, session_store_mock_); @@ -1100,9 +1094,8 @@ TEST_F(OidcFilterTest, auto raw_http = common::http::response_t( new beast::http::response()); raw_http->result(beast::http::status::ok); - EXPECT_CALL(*mocked_http, DoRequest(Eq(token_uri), _, _, Eq("some-ca"), - Eq(beast::http::verb::post), - Eq("http://some-proxy-uri.com"), _, _)) + EXPECT_CALL(*mocked_http, Post(Eq(token_uri), _, _, Eq("some-ca"), + Eq("http://some-proxy-uri.com"), _, _)) .WillOnce(Return(ByMove(std::move(raw_http)))); ASSERT_FALSE(session_store_->GetTokenResponse(session_id)); OidcFilter filter(common::http::ptr_t(mocked_http), config_, parser_mock_, @@ -1220,9 +1213,8 @@ TEST_F(OidcFilterTest, RetrieveToken_ReturnsError_WhenBrokenPipe) { auto *mocked_http = new common::http::HttpMock(); auto raw_http = common::http::response_t(); - EXPECT_CALL(*mocked_http, DoRequest(Eq(token_uri), _, _, Eq("some-ca"), - Eq(beast::http::verb::post), - Eq("http://some-proxy-uri.com"), _, _)) + EXPECT_CALL(*mocked_http, Post(Eq(token_uri), _, _, Eq("some-ca"), + Eq("http://some-proxy-uri.com"), _, _)) .WillOnce(Return(ByMove(std::move(raw_http)))); OidcFilter filter(common::http::ptr_t(mocked_http), config_, parser_mock_, session_string_generator_mock_, session_store_); @@ -1258,9 +1250,8 @@ TEST_F(OidcFilterTest, RetrieveToken_ReturnsError_WhenInvalidResponse) { auto *mocked_http = new common::http::HttpMock(); auto raw_http = common::http::response_t( (new beast::http::response())); - EXPECT_CALL(*mocked_http, DoRequest(Eq(token_uri), _, _, Eq("some-ca"), - Eq(beast::http::verb::post), - Eq("http://some-proxy-uri.com"), _, _)) + EXPECT_CALL(*mocked_http, Post(Eq(token_uri), _, _, Eq("some-ca"), + Eq("http://some-proxy-uri.com"), _, _)) .WillOnce(Return(ByMove(std::move(raw_http)))); OidcFilter filter(common::http::ptr_t(mocked_http), config_, parser_mock_, session_string_generator_mock_, session_store_); @@ -1345,9 +1336,8 @@ void OidcFilterTest::AssertRetrieveToken(config::oidc::OIDCConfig &oidcConfig, auto raw_http = common::http::response_t( new beast::http::response()); raw_http->result(beast::http::status::ok); - EXPECT_CALL(*mocked_http, DoRequest(Eq(token_uri), _, _, Eq("some-ca"), - Eq(beast::http::verb::post), - Eq("http://some-proxy-uri.com"), _, _)) + EXPECT_CALL(*mocked_http, Post(Eq(token_uri), _, _, Eq("some-ca"), + Eq("http://some-proxy-uri.com"), _, _)) .WillOnce(Return(ByMove(std::move(raw_http)))); OidcFilter filter(common::http::ptr_t(mocked_http), oidcConfig, parser_mock_, session_string_generator_mock_, session_store_); diff --git a/test/filters/oidc/token_response_test.cc b/test/filters/oidc/token_response_test.cc index 698ca33c..f33f75fa 100644 --- a/test/filters/oidc/token_response_test.cc +++ b/test/filters/oidc/token_response_test.cc @@ -108,9 +108,7 @@ class TokenResponseParserTest : public ::testing::Test { auto jwks = google::jwt_verify::Jwks::createFrom( valid_jwt_signing_key_, google::jwt_verify::Jwks::JWKS); EXPECT_EQ(jwks->getStatus(), google::jwt_verify::Status::Ok); - parser_ = std::make_shared( - std::make_unique( - valid_jwt_signing_key_, google::jwt_verify::Jwks::JWKS)); + parser_ = std::make_shared(std::move(jwks)); } std::shared_ptr ValidTokenResponse(); @@ -166,8 +164,7 @@ TEST_F(TokenResponseParserTest, ParseInvalidJwtSignature) { auto jwks = google::jwt_verify::Jwks::createFrom( invalid_jwt_signing_key_, google::jwt_verify::Jwks::PEM); EXPECT_EQ(jwks->getStatus(), google::jwt_verify::Status::JwksPemBadBase64); - TokenResponseParserImpl parser(std::make_unique( - invalid_jwt_signing_key_, google::jwt_verify::Jwks::PEM)); + TokenResponseParserImpl parser(std::move(jwks)); auto result = parser.Parse(client_id, nonce, valid_token_response_Bearer_without_access_token); ASSERT_FALSE(result); diff --git a/test/service/async_service_impl_test.cc b/test/service/async_service_impl_test.cc index 326b7aa1..66b034ef 100644 --- a/test/service/async_service_impl_test.cc +++ b/test/service/async_service_impl_test.cc @@ -45,6 +45,7 @@ class AsyncServiceImplTest : public ::testing::Test { std::vector> chains_; google::protobuf::RepeatedPtrField trigger_rules_config_; + boost::asio::io_context ioc_; }; using test_types = @@ -75,8 +76,8 @@ TYPED_TEST(AsyncServiceImplTest, this->trigger_rules_config_ = config.trigger_rules(); for (const auto &chain_config : config.chains()) { - std::unique_ptr chain( - new filters::FilterChainImpl(chain_config, config.threads())); + std::unique_ptr chain(new filters::FilterChainImpl( + this->ioc_, chain_config, config.threads())); this->chains_.push_back(std::move(chain)); } @@ -106,8 +107,8 @@ TYPED_TEST(AsyncServiceImplTest, this->trigger_rules_config_ = config.trigger_rules(); for (const auto &chain_config : config.chains()) { - std::unique_ptr chain( - new filters::FilterChainImpl(chain_config, config.threads())); + std::unique_ptr chain(new filters::FilterChainImpl( + this->ioc_, chain_config, config.threads())); this->chains_.push_back(std::move(chain)); } @@ -135,8 +136,8 @@ TYPED_TEST(AsyncServiceImplTest, config::Config config = *config::GetConfig("test/fixtures/valid-config.json"); for (const auto &chain_config : config.chains()) { - std::unique_ptr chain( - new filters::FilterChainImpl(chain_config, config.threads())); + std::unique_ptr chain(new filters::FilterChainImpl( + this->ioc_, chain_config, config.threads())); this->chains_.push_back(std::move(chain)); }