From f0248a4765b483a2c103a814483eff79a2c1e4ed Mon Sep 17 00:00:00 2001 From: Kuat Date: Sat, 4 Nov 2023 13:18:21 -0700 Subject: [PATCH] Jwt: implement clear route cache (#30699) Commit Message: Follow-up to #30356 to implement clear_route_cache. Risk Level: low, new field Testing: done Docs Changes: none Release Notes: none Issue: #29681 --- .../filters/http/jwt_authn/v3/config.proto | 1 - .../filters/http/jwt_authn/authenticator.cc | 35 ++++++++++++++----- .../filters/http/jwt_authn/authenticator.h | 4 ++- .../filters/http/jwt_authn/filter.cc | 2 ++ .../filters/http/jwt_authn/filter.h | 1 + .../filters/http/jwt_authn/verifier.cc | 9 +++-- .../filters/http/jwt_authn/verifier.h | 5 +++ .../http/jwt_authn/authenticator_test.cc | 35 ++++++++++++++++--- test/extensions/filters/http/jwt_authn/mock.h | 5 +-- .../filters/http/jwt_authn/test_common.h | 29 +++++++++++++++ 10 files changed, 107 insertions(+), 19 deletions(-) diff --git a/api/envoy/extensions/filters/http/jwt_authn/v3/config.proto b/api/envoy/extensions/filters/http/jwt_authn/v3/config.proto index 1bb1b111303a..2a37b9f57d05 100644 --- a/api/envoy/extensions/filters/http/jwt_authn/v3/config.proto +++ b/api/envoy/extensions/filters/http/jwt_authn/v3/config.proto @@ -320,7 +320,6 @@ message JwtProvider { // This header is only reserved for jwt claim; any other value will be overwritten. repeated JwtClaimToHeader claim_to_headers = 15; - // [#not-implemented-hide:] // Clears route cache in order to allow JWT token to correctly affect // routing decisions. Filter clears all cached routes when: // diff --git a/source/extensions/filters/http/jwt_authn/authenticator.cc b/source/extensions/filters/http/jwt_authn/authenticator.cc index 9587ad3c1435..fa1a67c9dece 100644 --- a/source/extensions/filters/http/jwt_authn/authenticator.cc +++ b/source/extensions/filters/http/jwt_authn/authenticator.cc @@ -61,8 +61,8 @@ class AuthenticatorImpl : public Logger::Loggable, // Following functions are for Authenticator interface. void verify(Http::HeaderMap& headers, Tracing::Span& parent_span, std::vector&& tokens, - SetExtractedJwtDataCallback set_extracted_jwt_data_cb, - AuthenticatorCallback callback) override; + SetExtractedJwtDataCallback set_extracted_jwt_data_cb, AuthenticatorCallback callback, + ClearRouteCacheCallback clear_route_cb) override; void onDestroy() override; TimeSource& timeSource() { return time_source_; } @@ -87,8 +87,8 @@ class AuthenticatorImpl : public Logger::Loggable, // finds one to verify with key. void startVerify(); - // Copy the JWT Claim to HTTP Header - void addJWTClaimToHeader(const std::string& claim_name, const std::string& header_name); + // Copy the JWT Claim to HTTP Header. Returns true iff header is added. + bool addJWTClaimToHeader(const std::string& claim_name, const std::string& header_name); // The jwks cache object. JwksCache& jwks_cache_; @@ -117,6 +117,10 @@ class AuthenticatorImpl : public Logger::Loggable, SetExtractedJwtDataCallback set_extracted_jwt_data_cb_; // The on_done function. AuthenticatorCallback callback_; + // Clear route cache callback function. + ClearRouteCacheCallback clear_route_cb_; + // Set to true to clear the route cache. + bool clear_route_cache_{false}; // check audience object. const CheckAudience* check_audience_; // specific provider or not when it is allow missing or failed. @@ -143,13 +147,16 @@ std::string AuthenticatorImpl::name() const { void AuthenticatorImpl::verify(Http::HeaderMap& headers, Tracing::Span& parent_span, std::vector&& tokens, SetExtractedJwtDataCallback set_extracted_jwt_data_cb, - AuthenticatorCallback callback) { + AuthenticatorCallback callback, + ClearRouteCacheCallback clear_route_cb) { ASSERT(!callback_); headers_ = &headers; parent_span_ = &parent_span; tokens_ = std::move(tokens); set_extracted_jwt_data_cb_ = std::move(set_extracted_jwt_data_cb); callback_ = std::move(callback); + clear_route_cb_ = std::move(clear_route_cb); + clear_route_cache_ = false; ENVOY_LOG(debug, "{}: JWT authentication starts (allow_failed={}), tokens size={}", name(), is_allow_failed_, tokens_.size()); @@ -303,7 +310,7 @@ void AuthenticatorImpl::verifyKey() { handleGoodJwt(/*cache_hit=*/false); } -void AuthenticatorImpl::addJWTClaimToHeader(const std::string& claim_name, +bool AuthenticatorImpl::addJWTClaimToHeader(const std::string& claim_name, const std::string& header_name) { StructUtils payload_getter(jwt_->payload_pb_); const ProtobufWkt::Value* claim_value; @@ -342,8 +349,10 @@ void AuthenticatorImpl::addJWTClaimToHeader(const std::string& claim_name, headers_->addCopy(Http::LowerCaseString(header_name), str_claim_value); ENVOY_LOG(debug, "[jwt_auth] claim : {} with value : {} is added to the header : {}", claim_name, str_claim_value, header_name); + return true; } } + return false; } void AuthenticatorImpl::handleGoodJwt(bool cache_hit) { @@ -363,8 +372,13 @@ void AuthenticatorImpl::handleGoodJwt(bool cache_hit) { } // Copy JWT claim to header + bool header_added = false; for (const auto& header_and_claim : provider.claim_to_headers()) { - addJWTClaimToHeader(header_and_claim.claim_name(), header_and_claim.header_name()); + header_added |= + addJWTClaimToHeader(header_and_claim.claim_name(), header_and_claim.header_name()); + } + if (provider.clear_route_cache() && header_added) { + clear_route_cache_ = true; } if (!provider.forward()) { @@ -453,8 +467,13 @@ void AuthenticatorImpl::doneWithStatus(const Status& status) { } else { callback_(status); } - callback_ = nullptr; + + if (clear_route_cache_ && clear_route_cb_) { + clear_route_cb_(); + } + clear_route_cb_ = nullptr; + return; } diff --git a/source/extensions/filters/http/jwt_authn/authenticator.h b/source/extensions/filters/http/jwt_authn/authenticator.h index 8e16c5044ce4..be5809aa0ba1 100644 --- a/source/extensions/filters/http/jwt_authn/authenticator.h +++ b/source/extensions/filters/http/jwt_authn/authenticator.h @@ -22,6 +22,8 @@ using AuthenticatorCallback = std::function; +using ClearRouteCacheCallback = std::function; + /** * Authenticator object to handle all JWT authentication flow. */ @@ -34,7 +36,7 @@ class Authenticator { virtual void verify(Http::HeaderMap& headers, Tracing::Span& parent_span, std::vector&& tokens, SetExtractedJwtDataCallback set_extracted_jwt_data_cb, - AuthenticatorCallback callback) PURE; + AuthenticatorCallback callback, ClearRouteCacheCallback clear_route_cb) PURE; // Called when the object is about to be destroyed. virtual void onDestroy() PURE; diff --git a/source/extensions/filters/http/jwt_authn/filter.cc b/source/extensions/filters/http/jwt_authn/filter.cc index 93fee62e2df5..8d773b88da1b 100644 --- a/source/extensions/filters/http/jwt_authn/filter.cc +++ b/source/extensions/filters/http/jwt_authn/filter.cc @@ -104,6 +104,8 @@ void Filter::setExtractedData(const ProtobufWkt::Struct& extracted_data) { extracted_data); } +void Filter::clearRouteCache() { decoder_callbacks_->downstreamCallbacks()->clearRouteCache(); } + void Filter::onComplete(const Status& status) { ENVOY_LOG(debug, "Jwt authentication completed with: {}", ::google::jwt_verify::getStatusString(status)); diff --git a/source/extensions/filters/http/jwt_authn/filter.h b/source/extensions/filters/http/jwt_authn/filter.h index 9330d07be612..461f4bc9deb7 100644 --- a/source/extensions/filters/http/jwt_authn/filter.h +++ b/source/extensions/filters/http/jwt_authn/filter.h @@ -33,6 +33,7 @@ class Filter : public Http::StreamDecoderFilter, // Following two functions are for Verifier::Callbacks interface. // Pass the extracted data from a verified JWT as an opaque ProtobufWkt::Struct. void setExtractedData(const ProtobufWkt::Struct& extracted_data) override; + void clearRouteCache() override; // It will be called when its verify() call is completed. void onComplete(const ::google::jwt_verify::Status& status) override; diff --git a/source/extensions/filters/http/jwt_authn/verifier.cc b/source/extensions/filters/http/jwt_authn/verifier.cc index a9e4c55d8d21..92d1f5182406 100644 --- a/source/extensions/filters/http/jwt_authn/verifier.cc +++ b/source/extensions/filters/http/jwt_authn/verifier.cc @@ -127,7 +127,8 @@ class ProviderVerifierImpl : public BaseVerifierImpl { [&ctximpl](const std::string& name, const ProtobufWkt::Struct& extracted_data) { ctximpl.addExtractedData(name, extracted_data); }, - [this, &ctximpl](const Status& status) { onComplete(status, ctximpl); }); + [this, &ctximpl](const Status& status) { onComplete(status, ctximpl); }, + [&ctximpl]() { ctximpl.callback()->clearRouteCache(); }); if (!ctximpl.getCompletionState(this).is_completed_) { ctximpl.storeAuth(std::move(auth)); } else { @@ -176,7 +177,8 @@ class AllowFailedVerifierImpl : public BaseVerifierImpl { [&ctximpl](const std::string& name, const ProtobufWkt::Struct& extracted_data) { ctximpl.addExtractedData(name, extracted_data); }, - [this, &ctximpl](const Status& status) { onComplete(status, ctximpl); }); + [this, &ctximpl](const Status& status) { onComplete(status, ctximpl); }, + [&ctximpl]() { ctximpl.callback()->clearRouteCache(); }); if (!ctximpl.getCompletionState(this).is_completed_) { ctximpl.storeAuth(std::move(auth)); } else { @@ -208,7 +210,8 @@ class AllowMissingVerifierImpl : public BaseVerifierImpl { [&ctximpl](const std::string& name, const ProtobufWkt::Struct& extracted_data) { ctximpl.addExtractedData(name, extracted_data); }, - [this, &ctximpl](const Status& status) { onComplete(status, ctximpl); }); + [this, &ctximpl](const Status& status) { onComplete(status, ctximpl); }, + [&ctximpl]() { ctximpl.callback()->clearRouteCache(); }); if (!ctximpl.getCompletionState(this).is_completed_) { ctximpl.storeAuth(std::move(auth)); } else { diff --git a/source/extensions/filters/http/jwt_authn/verifier.h b/source/extensions/filters/http/jwt_authn/verifier.h index 7d20e709660a..dd775caf1d6f 100644 --- a/source/extensions/filters/http/jwt_authn/verifier.h +++ b/source/extensions/filters/http/jwt_authn/verifier.h @@ -34,6 +34,11 @@ class Verifier { */ virtual void setExtractedData(const ProtobufWkt::Struct& payload) PURE; + /** + * JWT payloads added to headers may require clearing the cached route. + */ + virtual void clearRouteCache() PURE; + /** * Called on completion of request. * diff --git a/test/extensions/filters/http/jwt_authn/authenticator_test.cc b/test/extensions/filters/http/jwt_authn/authenticator_test.cc index d07c1b60d09d..b92f5daf706a 100644 --- a/test/extensions/filters/http/jwt_authn/authenticator_test.cc +++ b/test/extensions/filters/http/jwt_authn/authenticator_test.cc @@ -58,7 +58,8 @@ class AuthenticatorTest : public testing::Test { EXPECT_TRUE(jwks_->getStatus() == Status::Ok); } - void expectVerifyStatus(Status expected_status, Http::RequestHeaderMap& headers) { + void expectVerifyStatus(Status expected_status, Http::RequestHeaderMap& headers, + bool expect_clear_route = false) { std::function on_complete_cb = [&expected_status](const Status& status) { ASSERT_EQ(status, expected_status); }; @@ -68,8 +69,10 @@ class AuthenticatorTest : public testing::Test { }; initTokenExtractor(); auto tokens = extractor_->extract(headers); + bool clear_route = false; auth_->verify(headers, parent_span_, std::move(tokens), std::move(set_extracted_jwt_data_cb), - std::move(on_complete_cb)); + std::move(on_complete_cb), [&clear_route] { clear_route = true; }); + EXPECT_EQ(expect_clear_route, clear_route); } void initTokenExtractor() { @@ -163,6 +166,29 @@ TEST_F(AuthenticatorTest, TestClaimToHeader) { Envoy::Base64::encode(expected_json.data(), expected_json.size())); } +// This test verifies whether the claim is successfully added to header or not +TEST_F(AuthenticatorTest, TestClaimToHeaderWithClearRouteCache) { + TestUtility::loadFromYaml(ClaimToHeadersConfig, proto_config_); + createAuthenticator(); + EXPECT_CALL(*raw_fetcher_, fetch(_, _)) + .WillOnce(Invoke([this](Tracing::Span&, JwksFetcher::JwksReceiver& receiver) { + receiver.onJwksSuccess(std::move(jwks_)); + })); + + { + Http::TestRequestHeaderMapImpl headers{ + {"Authorization", "Bearer " + std::string(NestedGoodToken)}}; + expectVerifyStatus(Status::Ok, headers, true); + EXPECT_EQ(headers.get_("x-jwt-claim-nested"), "value1"); + } + + { + Http::TestRequestHeaderMapImpl headers{{"Authorization", "Bearer " + std::string(GoodToken)}}; + expectVerifyStatus(Status::Ok, headers, false); + EXPECT_FALSE(headers.has("x-jwt-claim-nested")); + } +} + // This test verifies when wrong claim is passed in claim_to_headers TEST_F(AuthenticatorTest, TestClaimToHeaderWithHeaderReplace) { createAuthenticator(); @@ -854,7 +880,8 @@ TEST_F(AuthenticatorTest, TestOnDestroy) { auto tokens = extractor_->extract(headers); // callback should not be called. std::function on_complete_cb = [](const Status&) { FAIL(); }; - auth_->verify(headers, parent_span_, std::move(tokens), nullptr, std::move(on_complete_cb)); + auth_->verify(headers, parent_span_, std::move(tokens), nullptr, std::move(on_complete_cb), + nullptr); // Destroy the authenticating process. auth_->onDestroy(); @@ -1013,7 +1040,7 @@ class AuthenticatorJwtCacheTest : public testing::Test { }; auto tokens = extractor_->extract(headers); auth_->verify(headers, parent_span_, std::move(tokens), set_extracted_jwt_data_cb, - on_complete_cb); + on_complete_cb, nullptr); } ::google::jwt_verify::JwksPtr jwks_; diff --git a/test/extensions/filters/http/jwt_authn/mock.h b/test/extensions/filters/http/jwt_authn/mock.h index 36263bd6dbcc..e3a792c765ba 100644 --- a/test/extensions/filters/http/jwt_authn/mock.h +++ b/test/extensions/filters/http/jwt_authn/mock.h @@ -35,8 +35,8 @@ class MockAuthenticator : public Authenticator { void verify(Http::HeaderMap& headers, Tracing::Span& parent_span, std::vector&& tokens, - SetExtractedJwtDataCallback set_extracted_jwt_data_cb, - AuthenticatorCallback callback) override { + SetExtractedJwtDataCallback set_extracted_jwt_data_cb, AuthenticatorCallback callback, + ClearRouteCacheCallback) override { doVerify(headers, parent_span, &tokens, std::move(set_extracted_jwt_data_cb), std::move(callback)); } @@ -47,6 +47,7 @@ class MockAuthenticator : public Authenticator { class MockVerifierCallbacks : public Verifier::Callbacks { public: MOCK_METHOD(void, setExtractedData, (const ProtobufWkt::Struct& payload)); + MOCK_METHOD(void, clearRouteCache, ()); MOCK_METHOD(void, onComplete, (const Status& status)); }; diff --git a/test/extensions/filters/http/jwt_authn/test_common.h b/test/extensions/filters/http/jwt_authn/test_common.h index 71364cbfff3d..2aa30f3301f4 100644 --- a/test/extensions/filters/http/jwt_authn/test_common.h +++ b/test/extensions/filters/http/jwt_authn/test_common.h @@ -107,6 +107,35 @@ const char ExampleConfig[] = R"( bypass_cors_preflight: true )"; +// Config with claim_to_headers and clear_route_cache. +const char ClaimToHeadersConfig[] = R"( +providers: + example_provider: + issuer: https://example.com + audiences: + - example_service + - http://example_service1 + - https://example_service2/ + remote_jwks: + http_uri: + uri: https://pubkey_server/pubkey_path + cluster: pubkey_cluster + timeout: + seconds: 5 + cache_duration: + seconds: 600 + claim_to_headers: + - header_name: "x-jwt-claim-nested" + claim_name: "nested.key-1" + clear_route_cache: true +rules: +- match: + path: "/" + requires: + provider_name: "example_provider" +bypass_cors_preflight: true +)"; + const char ExampleConfigWithRegEx[] = R"( providers: example_provider: