From 0d3aef32749745d590eaa5f51a11ba0b6b343b68 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Thu, 11 Oct 2018 23:51:44 +0000 Subject: [PATCH 1/3] jwt_authn: write JWT payload to dynamic metadata Signed-off-by: Wayne Zhang --- .../http/jwt_authn/v2alpha/config.proto | 6 ++ source/common/protobuf/utility.cc | 12 +++ source/common/protobuf/utility.h | 8 ++ .../filters/http/jwt_authn/authenticator.cc | 10 ++- .../filters/http/jwt_authn/authenticator.h | 4 +- .../filters/http/jwt_authn/filter.cc | 6 ++ .../filters/http/jwt_authn/filter.h | 4 +- .../filters/http/jwt_authn/verifier.cc | 20 +++++ .../filters/http/jwt_authn/verifier.h | 8 ++ .../http/jwt_authn/authenticator_test.cc | 44 ++++++++-- .../filters/http/jwt_authn/filter_test.cc | 29 ++++++- .../http/jwt_authn/group_verifier_test.cc | 87 +++++++++++++++---- test/extensions/filters/http/jwt_authn/mock.h | 9 +- .../http/jwt_authn/provider_verifier_test.cc | 6 ++ 14 files changed, 223 insertions(+), 30 deletions(-) diff --git a/api/envoy/config/filter/http/jwt_authn/v2alpha/config.proto b/api/envoy/config/filter/http/jwt_authn/v2alpha/config.proto index 153264e4f1242..1e786b96dc491 100644 --- a/api/envoy/config/filter/http/jwt_authn/v2alpha/config.proto +++ b/api/envoy/config/filter/http/jwt_authn/v2alpha/config.proto @@ -157,6 +157,12 @@ message JwtProvider { // // If it is not specified, the payload will not be forwarded. string forward_payload_header = 8; + + // If true, successfully verified JWT payloads will be written to StreamInfo DynamicMetadata + // in the format as: *namespace* is the jwt_authn filter name as **envoy.filters.http.jwt_authn** + // The value is the *protobuf::Struct*. Its *fields* contains the JWT **issuer** as the key and + // the **jwt_payload** as string value for each verified token. + bool payload_in_metadata = 9; } // This message specifies how to fetch JWKS from remote and how to cache it. diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc index d8a3ab138f509..b76dbad525f6d 100644 --- a/source/common/protobuf/utility.cc +++ b/source/common/protobuf/utility.cc @@ -149,6 +149,18 @@ ProtobufWkt::Struct MessageUtil::keyValueStruct(const std::string& key, const st return struct_obj; } +ProtobufWkt::Struct +MessageUtil::stringPairStruct(const std::vector>& pairs) { + ProtobufWkt::Struct struct_obj; + auto fields = struct_obj.mutable_fields(); + for (const auto& pair : pairs) { + ProtobufWkt::Value val; + val.set_string_value(pair.second); + (*fields)[pair.first] = val; + } + return struct_obj; +} + bool ValueUtil::equal(const ProtobufWkt::Value& v1, const ProtobufWkt::Value& v2) { ProtobufWkt::Value::KindCase kind = v1.kind_case(); if (kind != v2.kind_case()) { diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index 7dcdddb24c48c..ed83b40b63240 100644 --- a/source/common/protobuf/utility.h +++ b/source/common/protobuf/utility.h @@ -271,6 +271,14 @@ class MessageUtil { * @param value the string value to associate with the key */ static ProtobufWkt::Struct keyValueStruct(const std::string& key, const std::string& value); + + /** + * Utility method to create a Struct containing a vector of key/value pairs. + * + * @param pairs a vector of string pairs + */ + static ProtobufWkt::Struct + stringPairStruct(const std::vector>& pairs); }; class ValueUtil { diff --git a/source/extensions/filters/http/jwt_authn/authenticator.cc b/source/extensions/filters/http/jwt_authn/authenticator.cc index 234d8279643a3..a2ece52c5ee6b 100644 --- a/source/extensions/filters/http/jwt_authn/authenticator.cc +++ b/source/extensions/filters/http/jwt_authn/authenticator.cc @@ -41,7 +41,7 @@ class AuthenticatorImpl : public Logger::Loggable, void onJwksError(Failure reason) override; // Following functions are for Authenticator interface void verify(Http::HeaderMap& headers, std::vector&& tokens, - AuthenticatorCallback callback) override; + SetPayloadCallback set_payload_cb, AuthenticatorCallback callback) override; void onDestroy() override; TimeSource& timeSource() { return time_source_; } @@ -78,6 +78,8 @@ class AuthenticatorImpl : public Logger::Loggable, // The HTTP request headers Http::HeaderMap* headers_{}; + // the callback function to set payload + SetPayloadCallback set_payload_cb_; // The on_done function. AuthenticatorCallback callback_; // check audience object. @@ -89,10 +91,11 @@ class AuthenticatorImpl : public Logger::Loggable, }; void AuthenticatorImpl::verify(Http::HeaderMap& headers, std::vector&& tokens, - AuthenticatorCallback callback) { + SetPayloadCallback set_payload_cb, AuthenticatorCallback callback) { ASSERT(!callback_); headers_ = &headers; tokens_ = std::move(tokens); + set_payload_cb_ = std::move(set_payload_cb); callback_ = std::move(callback); ENVOY_LOG(debug, "Jwt authentication starts"); @@ -224,6 +227,9 @@ void AuthenticatorImpl::verifyKey() { // Remove JWT from headers. curr_token_->removeJwt(*headers_); } + if (set_payload_cb_ && provider.payload_in_metadata()) { + set_payload_cb_(provider.issuer(), jwt_->payload_str_base64url_); + } doneWithStatus(Status::Ok); } diff --git a/source/extensions/filters/http/jwt_authn/authenticator.h b/source/extensions/filters/http/jwt_authn/authenticator.h index 76e8f96c05dec..a45350a503b05 100644 --- a/source/extensions/filters/http/jwt_authn/authenticator.h +++ b/source/extensions/filters/http/jwt_authn/authenticator.h @@ -19,6 +19,8 @@ typedef std::unique_ptr AuthenticatorPtr; typedef std::function AuthenticatorCallback; +typedef std::function SetPayloadCallback; + /** * CreateJwksFetcherCb is a callback interface for creating a JwksFetcher instance. */ @@ -34,7 +36,7 @@ class Authenticator { // Verify if headers satisfyies the JWT requirements. Can be limited to single provider with // extract_param. virtual void verify(Http::HeaderMap& headers, std::vector&& tokens, - AuthenticatorCallback callback) PURE; + SetPayloadCallback set_payload_cb, AuthenticatorCallback callback) 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 0589d8667c9dc..751c97fc6c742 100644 --- a/source/extensions/filters/http/jwt_authn/filter.cc +++ b/source/extensions/filters/http/jwt_authn/filter.cc @@ -2,6 +2,8 @@ #include "common/http/utility.h" +#include "extensions/filters/http/well_known_names.h" + using ::google::jwt_verify::Status; namespace Envoy { @@ -40,6 +42,10 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool) return Http::FilterHeadersStatus::StopIteration; } +void Filter::setPayload(const ProtobufWkt::Struct& payload) { + decoder_callbacks_->streamInfo().setDynamicMetadata(HttpFilterNames::get().JwtAuthn, payload); +} + void Filter::onComplete(const Status& status) { ENVOY_LOG(debug, "Called Filter : check complete {}", int(status)); // This stream has been reset, abort the callback. diff --git a/source/extensions/filters/http/jwt_authn/filter.h b/source/extensions/filters/http/jwt_authn/filter.h index 2366fd6990f4a..6251efacb1731 100644 --- a/source/extensions/filters/http/jwt_authn/filter.h +++ b/source/extensions/filters/http/jwt_authn/filter.h @@ -31,7 +31,9 @@ class Filter : public Http::StreamDecoderFilter, void setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) override; private: - // the function for Verifier::Callbacks interface. + // Following two functions are for Verifier::Callbacks interface. + // Pass the payload as Struct. + void setPayload(const ProtobufWkt::Struct& payload) 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 bffb7cb6683bd..451ce1037a899 100644 --- a/source/extensions/filters/http/jwt_authn/verifier.cc +++ b/source/extensions/filters/http/jwt_authn/verifier.cc @@ -48,11 +48,21 @@ class ContextImpl : public Verifier::Context { // Stores an authenticator object for this request. void storeAuth(AuthenticatorPtr&& auth) { auths_.emplace_back(std::move(auth)); } + // Add a pair of (issuer, payload), called by Authenticator + void addPaylod(const std::string& issuer, const std::string& payload) { + payload_pairs_.push_back({issuer, payload}); + } + + bool hasPayload() const { return !payload_pairs_.empty(); } + + ProtobufWkt::Struct getPayload() const { return MessageUtil::stringPairStruct(payload_pairs_); } + private: Http::HeaderMap& headers_; Verifier::Callbacks* callback_; std::unordered_map completion_states_; std::vector auths_; + std::vector> payload_pairs_; }; // base verifier for provider_name, provider_and_audiences, and allow_missing_or_failed. @@ -65,6 +75,10 @@ class BaseVerifierImpl : public Verifier { return parent_->onComplete(status, context); } + if (Status::Ok == status && context.hasPayload()) { + context.callback()->setPayload(context.getPayload()); + } + context.callback()->onComplete(status); context.cancel(); } @@ -97,6 +111,9 @@ class ProviderVerifierImpl : public BaseVerifierImpl { auto auth = auth_factory_.create(getAudienceChecker(), provider_name_, false); extractor_->sanitizePayloadHeaders(ctximpl.headers()); auth->verify(ctximpl.headers(), extractor_->extract(ctximpl.headers()), + [&ctximpl](const std::string& issuer, const std::string& payload) { + ctximpl.addPaylod(issuer, payload); + }, [this, context](const Status& status) { onComplete(status, static_cast(*context)); }); @@ -143,6 +160,9 @@ class AllowFailedVerifierImpl : public BaseVerifierImpl { auto auth = auth_factory_.create(nullptr, absl::nullopt, true); extractor_.sanitizePayloadHeaders(ctximpl.headers()); auth->verify(ctximpl.headers(), extractor_.extract(ctximpl.headers()), + [&ctximpl](const std::string& issuer, const std::string& payload) { + ctximpl.addPaylod(issuer, payload); + }, [this, context](const Status& status) { onComplete(status, static_cast(*context)); }); diff --git a/source/extensions/filters/http/jwt_authn/verifier.h b/source/extensions/filters/http/jwt_authn/verifier.h index 46c3324dc9ad6..03a64c22db153 100644 --- a/source/extensions/filters/http/jwt_authn/verifier.h +++ b/source/extensions/filters/http/jwt_authn/verifier.h @@ -24,6 +24,14 @@ class Verifier { public: virtual ~Callbacks() {} + /** + * Successfully verified JWT payload are stored in the struct with its + * *fields* containing **issuer** as keys and **payload** as string values + * This function is called before onComplete() function. + * It will not be called if no payload to write. + */ + virtual void setPayload(const ProtobufWkt::Struct& payload) 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 a2725ff32aed8..71488cd8847b0 100644 --- a/test/extensions/filters/http/jwt_authn/authenticator_test.cc +++ b/test/extensions/filters/http/jwt_authn/authenticator_test.cc @@ -14,14 +14,14 @@ #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::Jwks; using ::google::jwt_verify::Status; -using ::testing::_; using ::testing::Invoke; using ::testing::NiceMock; +using ::testing::_; +using Envoy::Extensions::HttpFilters::Common::JwksFetcher; +using Envoy::Extensions::HttpFilters::Common::JwksFetcherPtr; +using Envoy::Extensions::HttpFilters::Common::MockJwksFetcher; namespace Envoy { namespace Extensions { @@ -53,8 +53,12 @@ class AuthenticatorTest : public ::testing::Test { std::function on_complete_cb = [&expected_status](const Status& status) { ASSERT_EQ(status, expected_status); }; + auto set_payload_cb = [this](const std::string& issuer, const std::string& payload) { + out_issuer_ = issuer; + out_payload_ = payload; + }; auto tokens = filter_config_->getExtractor().extract(headers); - auth_->verify(headers, std::move(tokens), std::move(on_complete_cb)); + auth_->verify(headers, std::move(tokens), std::move(set_payload_cb), std::move(on_complete_cb)); } JwtAuthentication proto_config_; @@ -64,6 +68,8 @@ class AuthenticatorTest : public ::testing::Test { AuthenticatorPtr auth_; ::google::jwt_verify::JwksPtr jwks_; NiceMock mock_factory_ctx_; + std::string out_issuer_; + std::string out_payload_; }; // This test validates a good JWT authentication with a remote Jwks. @@ -105,6 +111,31 @@ TEST_F(AuthenticatorTest, TestForwardJwt) { // Verify the token is NOT removed. EXPECT_TRUE(headers.Authorization()); + + // Payload not set by default + EXPECT_EQ(out_issuer_, ""); + EXPECT_EQ(out_payload_, ""); +} + +// This test verifies the Jwt payload is set. +TEST_F(AuthenticatorTest, TestSetPayload) { + // Confit payload_in_metadata flag + (*proto_config_.mutable_providers())[std::string(ProviderName)].set_payload_in_metadata(true); + CreateAuthenticator(); + EXPECT_CALL(*raw_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)}}; + + expectVerifyStatus(Status::Ok, headers); + + // Payload is set + EXPECT_EQ(out_issuer_, "https://example.com"); + EXPECT_EQ(out_payload_, ExpectedPayloadValue); } // This test verifies the Jwt with non existing kid @@ -238,7 +269,8 @@ TEST_F(AuthenticatorTest, TestOnDestroy) { auto tokens = filter_config_->getExtractor().extract(headers); // callback should not be called. std::function on_complete_cb = [](const Status&) { FAIL(); }; - auth_->verify(headers, std::move(tokens), std::move(on_complete_cb)); + auto set_payload_cb = [this](const std::string&, const std::string&) {}; + auth_->verify(headers, std::move(tokens), std::move(set_payload_cb), std::move(on_complete_cb)); // Destroy the authenticating process. auth_->onDestroy(); diff --git a/test/extensions/filters/http/jwt_authn/filter_test.cc b/test/extensions/filters/http/jwt_authn/filter_test.cc index d1b1633751855..d8d148c61913e 100644 --- a/test/extensions/filters/http/jwt_authn/filter_test.cc +++ b/test/extensions/filters/http/jwt_authn/filter_test.cc @@ -1,7 +1,9 @@ #include "extensions/filters/http/jwt_authn/filter.h" +#include "extensions/filters/http/well_known_names.h" #include "test/extensions/filters/http/jwt_authn/mock.h" #include "test/mocks/server/mocks.h" +#include "test/test_common/utility.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -9,8 +11,8 @@ using ::envoy::config::filter::http::jwt_authn::v2alpha::JwtAuthentication; using ::google::jwt_verify::Status; -using testing::_; using testing::Invoke; +using testing::_; namespace Envoy { namespace Extensions { @@ -85,6 +87,31 @@ TEST_F(FilterTest, InlineOK) { EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(headers)); } +// This test verifies the setPayload call is handled correctly +TEST_F(FilterTest, TestSetPayloadCall) { + setupMockConfig(); + ProtobufWkt::Struct payload; + // A successful authentication completed inline: callback is called inside verify(). + EXPECT_CALL(*raw_mock_verifier_, verify(_)).WillOnce(Invoke([&payload](ContextSharedPtr context) { + context->callback()->setPayload(payload); + context->callback()->onComplete(Status::Ok); + })); + + EXPECT_CALL(filter_callbacks_.stream_info_, setDynamicMetadata(_, _)) + .WillOnce(Invoke([&payload](const std::string& ns, const ProtobufWkt::Struct& out_payload) { + EXPECT_EQ(ns, HttpFilterNames::get().JwtAuthn); + EXPECT_TRUE(TestUtility::protoEqual(out_payload, payload)); + })); + + auto headers = Http::TestHeaderMapImpl{}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers, false)); + EXPECT_EQ(1U, mock_config_->stats().allowed_.value()); + + Buffer::OwnedImpl data(""); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(headers)); +} + // This test verifies Verifier::Callback is called inline with a failure status. // All functions should return Continue except decodeHeaders(), it returns StopIteraton. TEST_F(FilterTest, InlineFailure) { diff --git a/test/extensions/filters/http/jwt_authn/group_verifier_test.cc b/test/extensions/filters/http/jwt_authn/group_verifier_test.cc index 44172c1ec9e25..55f8c770fdafe 100644 --- a/test/extensions/filters/http/jwt_authn/group_verifier_test.cc +++ b/test/extensions/filters/http/jwt_authn/group_verifier_test.cc @@ -81,10 +81,15 @@ class GroupVerifierTest : public ::testing::Test { void createSyncMockAuthsAndVerifier(const StatusMap& statuses) { for (const auto& it : statuses) { auto mock_auth = std::make_unique(); - EXPECT_CALL(*mock_auth.get(), doVerify(_, _, _)) - .WillOnce( - Invoke([status = it.second](Http::HeaderMap&, std::vector*, - AuthenticatorCallback callback) { callback(status); })); + EXPECT_CALL(*mock_auth.get(), doVerify(_, _, _, _)) + .WillOnce(Invoke([ issuer = it.first, status = it.second ]( + Http::HeaderMap&, std::vector*, + SetPayloadCallback set_payload_cb, AuthenticatorCallback callback) { + if (status == Status::Ok) { + set_payload_cb(issuer, issuer + "-payload"); + } + callback(status); + })); EXPECT_CALL(*mock_auth.get(), onDestroy()).Times(1); mock_auths_[it.first] = std::move(mock_auth); } @@ -96,12 +101,10 @@ class GroupVerifierTest : public ::testing::Test { std::unordered_map callbacks; for (std::size_t i = 0; i < providers.size(); ++i) { auto mock_auth = std::make_unique(); - EXPECT_CALL(*mock_auth.get(), doVerify(_, _, _)) - .WillOnce(Invoke([&callbacks, iss = providers[i]](Http::HeaderMap&, - std::vector*, - AuthenticatorCallback callback) { - callbacks[iss] = std::move(callback); - })); + EXPECT_CALL(*mock_auth.get(), doVerify(_, _, _, _)) + .WillOnce(Invoke([&callbacks, iss = providers[i] ]( + Http::HeaderMap&, std::vector*, SetPayloadCallback, + AuthenticatorCallback callback) { callbacks[iss] = std::move(callback); })); EXPECT_CALL(*mock_auth.get(), onDestroy()).Times(1); mock_auths_[providers[i]] = std::move(mock_auth); } @@ -151,6 +154,12 @@ TEST_F(GroupVerifierTest, DeeplyNestedAnys) { MessageUtil::loadFromYaml(config, proto_config_); createSyncMockAuthsAndVerifier(StatusMap{{"example_provider", Status::Ok}}); + EXPECT_CALL(mock_cb_, setPayload(_)).WillOnce(Invoke([](const ProtobufWkt::Struct& payload) { + EXPECT_TRUE(TestUtility::protoEqual( + payload, + MessageUtil::stringPairStruct({{"example_provider", "example_provider-payload"}}))); + })); + EXPECT_CALL(mock_cb_, onComplete(Status::Ok)).Times(1); auto headers = Http::TestHeaderMapImpl{ {"sec-istio-auth-userinfo", ""}, @@ -184,7 +193,7 @@ TEST_F(GroupVerifierTest, CanHandleUnexpectedEnd) { )"; MessageUtil::loadFromYaml(config, proto_config_); auto mock_auth = std::make_unique(); - EXPECT_CALL(*mock_auth.get(), doVerify(_, _, _)).Times(0); + EXPECT_CALL(*mock_auth.get(), doVerify(_, _, _, _)).Times(0); mock_auths_["example_provider"] = std::move(mock_auth); createVerifier(); @@ -200,6 +209,14 @@ TEST_F(GroupVerifierTest, TestRequiresAll) { createSyncMockAuthsAndVerifier( StatusMap{{"example_provider", Status::Ok}, {"other_provider", Status::Ok}}); + EXPECT_CALL(mock_cb_, setPayload(_)).WillOnce(Invoke([](const ProtobufWkt::Struct& payload) { + EXPECT_TRUE( + TestUtility::protoEqual(payload, MessageUtil::stringPairStruct({ + {"example_provider", "example_provider-payload"}, + {"other_provider", "other_provider-payload"}, + }))); + })); + EXPECT_CALL(mock_cb_, onComplete(Status::Ok)).Times(1); auto headers = Http::TestHeaderMapImpl{ {"example-auth-userinfo", ""}, @@ -217,6 +234,8 @@ TEST_F(GroupVerifierTest, TestRequiresAllBadFormat) { auto callbacks = createAsyncMockAuthsAndVerifier( std::vector{"example_provider", "other_provider"}); + // onComplete with failure status, not payload + EXPECT_CALL(mock_cb_, setPayload(_)).Times(0); EXPECT_CALL(mock_cb_, onComplete(Status::JwtBadFormat)).Times(1); auto headers = Http::TestHeaderMapImpl{ {"example-auth-userinfo", ""}, @@ -239,6 +258,8 @@ TEST_F(GroupVerifierTest, TestRequiresAllMissing) { auto callbacks = createAsyncMockAuthsAndVerifier( std::vector{"example_provider", "other_provider"}); + // onComplete with failure status, not payload + EXPECT_CALL(mock_cb_, setPayload(_)).Times(0); EXPECT_CALL(mock_cb_, onComplete(Status::JwtMissed)).Times(1); auto headers = Http::TestHeaderMapImpl{ {"example-auth-userinfo", ""}, @@ -261,6 +282,8 @@ TEST_F(GroupVerifierTest, TestRequiresAllBothFailed) { auto callbacks = createAsyncMockAuthsAndVerifier( std::vector{"example_provider", "other_provider"}); + // onComplete with failure status, not payload + EXPECT_CALL(mock_cb_, setPayload(_)).Times(0); EXPECT_CALL(mock_cb_, onComplete(Status::JwtUnknownIssuer)).Times(1); auto headers = Http::TestHeaderMapImpl{ {"example-auth-userinfo", ""}, @@ -279,6 +302,13 @@ TEST_F(GroupVerifierTest, TestRequiresAnyFirstAuthOK) { MessageUtil::loadFromYaml(RequiresAnyConfig, proto_config_); createSyncMockAuthsAndVerifier(StatusMap{{"example_provider", Status::Ok}}); + EXPECT_CALL(mock_cb_, setPayload(_)).WillOnce(Invoke([](const ProtobufWkt::Struct& payload) { + EXPECT_TRUE( + TestUtility::protoEqual(payload, MessageUtil::stringPairStruct({ + {"example_provider", "example_provider-payload"}, + }))); + })); + EXPECT_CALL(mock_cb_, onComplete(Status::Ok)).Times(1); auto headers = Http::TestHeaderMapImpl{ {"example-auth-userinfo", ""}, @@ -296,6 +326,12 @@ TEST_F(GroupVerifierTest, TestRequiresAnyLastAuthOk) { createSyncMockAuthsAndVerifier( StatusMap{{"example_provider", Status::JwtUnknownIssuer}, {"other_provider", Status::Ok}}); + EXPECT_CALL(mock_cb_, setPayload(_)).WillOnce(Invoke([](const ProtobufWkt::Struct& payload) { + EXPECT_TRUE(TestUtility::protoEqual(payload, MessageUtil::stringPairStruct({ + {"other_provider", "other_provider-payload"}, + }))); + })); + EXPECT_CALL(mock_cb_, onComplete(Status::Ok)).Times(1); auto headers = Http::TestHeaderMapImpl{ {"example-auth-userinfo", ""}, @@ -315,6 +351,8 @@ TEST_F(GroupVerifierTest, TestRequiresAnyAllAuthFailed) { createSyncMockAuthsAndVerifier(StatusMap{{"example_provider", Status::JwtHeaderBadKid}, {"other_provider", Status::JwtUnknownIssuer}}); + // onComplete with failure status, not payload + EXPECT_CALL(mock_cb_, setPayload(_)).Times(0); EXPECT_CALL(mock_cb_, onComplete(Status::JwtUnknownIssuer)).Times(1); auto headers = Http::TestHeaderMapImpl{ {"example-auth-userinfo", ""}, @@ -332,6 +370,13 @@ TEST_F(GroupVerifierTest, TestAnyInAllFirstAnyIsOk) { MessageUtil::loadFromYaml(AllWithAny, proto_config_); createSyncMockAuthsAndVerifier(StatusMap{{"provider_1", Status::Ok}, {"provider_3", Status::Ok}}); + EXPECT_CALL(mock_cb_, setPayload(_)).WillOnce(Invoke([](const ProtobufWkt::Struct& payload) { + EXPECT_TRUE(TestUtility::protoEqual(payload, MessageUtil::stringPairStruct({ + {"provider_1", "provider_1-payload"}, + {"provider_3", "provider_3-payload"}, + }))); + })); + EXPECT_CALL(mock_cb_, onComplete(Status::Ok)).Times(1); auto headers = Http::TestHeaderMapImpl{}; context_ = Verifier::createContext(headers, &mock_cb_); @@ -346,6 +391,13 @@ TEST_F(GroupVerifierTest, TestAnyInAllLastAnyIsOk) { {"provider_2", Status::Ok}, {"provider_3", Status::Ok}}); + EXPECT_CALL(mock_cb_, setPayload(_)).WillOnce(Invoke([](const ProtobufWkt::Struct& payload) { + EXPECT_TRUE(TestUtility::protoEqual(payload, MessageUtil::stringPairStruct({ + {"provider_2", "provider_2-payload"}, + {"provider_3", "provider_3-payload"}, + }))); + })); + EXPECT_CALL(mock_cb_, onComplete(Status::Ok)).Times(1); auto headers = Http::TestHeaderMapImpl{}; context_ = Verifier::createContext(headers, &mock_cb_); @@ -359,6 +411,8 @@ TEST_F(GroupVerifierTest, TestAnyInAllBothInRequireAnyIsOk) { auto callbacks = createAsyncMockAuthsAndVerifier( std::vector{"provider_1", "provider_2", "provider_3"}); + // AsyncMockVerifier doesn't set payload + EXPECT_CALL(mock_cb_, setPayload(_)).Times(0); EXPECT_CALL(mock_cb_, onComplete(Status::Ok)).Times(1); auto headers = Http::TestHeaderMapImpl{}; context_ = Verifier::createContext(headers, &mock_cb_); @@ -375,6 +429,7 @@ TEST_F(GroupVerifierTest, TestAnyInAllBothInRequireAnyFailed) { auto callbacks = createAsyncMockAuthsAndVerifier( std::vector{"provider_1", "provider_2", "provider_3"}); + EXPECT_CALL(mock_cb_, setPayload(_)).Times(0); EXPECT_CALL(mock_cb_, onComplete(Status::JwksFetchFail)).Times(1); auto headers = Http::TestHeaderMapImpl{}; context_ = Verifier::createContext(headers, &mock_cb_); @@ -392,6 +447,7 @@ TEST_F(GroupVerifierTest, TestAllInAnyBothRequireAllFailed) { createSyncMockAuthsAndVerifier( StatusMap{{"provider_1", Status::JwksFetchFail}, {"provider_3", Status::JwtExpired}}); + EXPECT_CALL(mock_cb_, setPayload(_)).Times(0); EXPECT_CALL(mock_cb_, onComplete(Status::JwtExpired)).Times(1); auto headers = Http::TestHeaderMapImpl{}; context_ = Verifier::createContext(headers, &mock_cb_); @@ -405,6 +461,8 @@ TEST_F(GroupVerifierTest, TestAllInAnyFirstAllIsOk) { auto callbacks = createAsyncMockAuthsAndVerifier( std::vector{"provider_1", "provider_2", "provider_3", "provider_4"}); + // AsyncMockVerifier doesn't set payload + EXPECT_CALL(mock_cb_, setPayload(_)).Times(0); EXPECT_CALL(mock_cb_, onComplete(Status::Ok)).Times(1); auto headers = Http::TestHeaderMapImpl{}; context_ = Verifier::createContext(headers, &mock_cb_); @@ -459,11 +517,10 @@ TEST_F(GroupVerifierTest, TestRequiresAnyWithAllowAll) { auto callbacks = createAsyncMockAuthsAndVerifier( std::vector{"example_provider", "other_provider"}); auto mock_auth = std::make_unique(); - EXPECT_CALL(*mock_auth.get(), doVerify(_, _, _)) + EXPECT_CALL(*mock_auth.get(), doVerify(_, _, _, _)) .WillOnce(Invoke( - [&](Http::HeaderMap&, std::vector*, AuthenticatorCallback callback) { - callbacks[allowfailed] = std::move(callback); - })); + [&](Http::HeaderMap&, std::vector*, SetPayloadCallback, + AuthenticatorCallback callback) { callbacks[allowfailed] = std::move(callback); })); EXPECT_CALL(*mock_auth.get(), onDestroy()).Times(1); mock_auths_[allowfailed] = std::move(mock_auth); EXPECT_CALL(mock_cb_, onComplete(Status::Ok)).Times(1); diff --git a/test/extensions/filters/http/jwt_authn/mock.h b/test/extensions/filters/http/jwt_authn/mock.h index ec437b721d652..1cd824fd128f3 100644 --- a/test/extensions/filters/http/jwt_authn/mock.h +++ b/test/extensions/filters/http/jwt_authn/mock.h @@ -19,12 +19,12 @@ class MockAuthFactory : public AuthFactory { class MockAuthenticator : public Authenticator { public: - MOCK_METHOD3(doVerify, void(Http::HeaderMap& headers, std::vector* tokens, - std::function callback)); + MOCK_METHOD4(doVerify, void(Http::HeaderMap& headers, std::vector* tokens, + SetPayloadCallback set_payload_cb, AuthenticatorCallback callback)); void verify(Http::HeaderMap& headers, std::vector&& tokens, - AuthenticatorCallback callback) { - doVerify(headers, &tokens, std::move(callback)); + SetPayloadCallback set_payload_cb, AuthenticatorCallback callback) { + doVerify(headers, &tokens, std::move(set_payload_cb), std::move(callback)); } MOCK_METHOD0(onDestroy, void()); @@ -32,6 +32,7 @@ class MockAuthenticator : public Authenticator { class MockVerifierCallbacks : public Verifier::Callbacks { public: + MOCK_METHOD1(setPayload, void(const ProtobufWkt::Struct& payload)); MOCK_METHOD1(onComplete, void(const Status& status)); }; diff --git a/test/extensions/filters/http/jwt_authn/provider_verifier_test.cc b/test/extensions/filters/http/jwt_authn/provider_verifier_test.cc index ddc454c547143..5243f61c95e06 100644 --- a/test/extensions/filters/http/jwt_authn/provider_verifier_test.cc +++ b/test/extensions/filters/http/jwt_authn/provider_verifier_test.cc @@ -36,9 +36,15 @@ class ProviderVerifierTest : public ::testing::Test { TEST_F(ProviderVerifierTest, TestOkJWT) { MessageUtil::loadFromYaml(ExampleConfig, proto_config_); + (*proto_config_.mutable_providers())[std::string(ProviderName)].set_payload_in_metadata(true); createVerifier(); MockUpstream mock_pubkey(mock_factory_ctx_.cluster_manager_, PublicKey); + EXPECT_CALL(mock_cb_, setPayload(_)).WillOnce(Invoke([](const ProtobufWkt::Struct& payload) { + EXPECT_TRUE(TestUtility::protoEqual( + payload, MessageUtil::keyValueStruct("https://example.com", ExpectedPayloadValue))); + })); + EXPECT_CALL(mock_cb_, onComplete(Status::Ok)).Times(1); auto headers = Http::TestHeaderMapImpl{ From 1570b7afb1fe0582216c2da35c7cef04637bee98 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Fri, 12 Oct 2018 00:56:44 +0000 Subject: [PATCH 2/3] Fix format Signed-off-by: Wayne Zhang --- .../filters/http/jwt_authn/authenticator_test.cc | 8 ++++---- .../filters/http/jwt_authn/filter_test.cc | 2 +- .../filters/http/jwt_authn/group_verifier_test.cc | 14 ++++++++------ 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/test/extensions/filters/http/jwt_authn/authenticator_test.cc b/test/extensions/filters/http/jwt_authn/authenticator_test.cc index 71488cd8847b0..f79e17d55932d 100644 --- a/test/extensions/filters/http/jwt_authn/authenticator_test.cc +++ b/test/extensions/filters/http/jwt_authn/authenticator_test.cc @@ -14,14 +14,14 @@ #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::Jwks; using ::google::jwt_verify::Status; +using ::testing::_; using ::testing::Invoke; using ::testing::NiceMock; -using ::testing::_; -using Envoy::Extensions::HttpFilters::Common::JwksFetcher; -using Envoy::Extensions::HttpFilters::Common::JwksFetcherPtr; -using Envoy::Extensions::HttpFilters::Common::MockJwksFetcher; namespace Envoy { namespace Extensions { diff --git a/test/extensions/filters/http/jwt_authn/filter_test.cc b/test/extensions/filters/http/jwt_authn/filter_test.cc index d8d148c61913e..467bff6f78f03 100644 --- a/test/extensions/filters/http/jwt_authn/filter_test.cc +++ b/test/extensions/filters/http/jwt_authn/filter_test.cc @@ -11,8 +11,8 @@ using ::envoy::config::filter::http::jwt_authn::v2alpha::JwtAuthentication; using ::google::jwt_verify::Status; -using testing::Invoke; using testing::_; +using testing::Invoke; namespace Envoy { namespace Extensions { diff --git a/test/extensions/filters/http/jwt_authn/group_verifier_test.cc b/test/extensions/filters/http/jwt_authn/group_verifier_test.cc index 55f8c770fdafe..b17ed2c8d0701 100644 --- a/test/extensions/filters/http/jwt_authn/group_verifier_test.cc +++ b/test/extensions/filters/http/jwt_authn/group_verifier_test.cc @@ -82,9 +82,9 @@ class GroupVerifierTest : public ::testing::Test { for (const auto& it : statuses) { auto mock_auth = std::make_unique(); EXPECT_CALL(*mock_auth.get(), doVerify(_, _, _, _)) - .WillOnce(Invoke([ issuer = it.first, status = it.second ]( - Http::HeaderMap&, std::vector*, - SetPayloadCallback set_payload_cb, AuthenticatorCallback callback) { + .WillOnce(Invoke([issuer = it.first, status = it.second]( + Http::HeaderMap&, std::vector*, + SetPayloadCallback set_payload_cb, AuthenticatorCallback callback) { if (status == Status::Ok) { set_payload_cb(issuer, issuer + "-payload"); } @@ -102,9 +102,11 @@ class GroupVerifierTest : public ::testing::Test { for (std::size_t i = 0; i < providers.size(); ++i) { auto mock_auth = std::make_unique(); EXPECT_CALL(*mock_auth.get(), doVerify(_, _, _, _)) - .WillOnce(Invoke([&callbacks, iss = providers[i] ]( - Http::HeaderMap&, std::vector*, SetPayloadCallback, - AuthenticatorCallback callback) { callbacks[iss] = std::move(callback); })); + .WillOnce(Invoke( + [&callbacks, iss = providers[i]](Http::HeaderMap&, std::vector*, + SetPayloadCallback, AuthenticatorCallback callback) { + callbacks[iss] = std::move(callback); + })); EXPECT_CALL(*mock_auth.get(), onDestroy()).Times(1); mock_auths_[providers[i]] = std::move(mock_auth); } From e7bbf75b0775aab9d8441d613a21257c8a747217 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Fri, 12 Oct 2018 01:36:29 +0000 Subject: [PATCH 3/3] Fix asan test failure Signed-off-by: Wayne Zhang --- test/extensions/filters/http/jwt_authn/authenticator_test.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/extensions/filters/http/jwt_authn/authenticator_test.cc b/test/extensions/filters/http/jwt_authn/authenticator_test.cc index f79e17d55932d..83874dd7d73e0 100644 --- a/test/extensions/filters/http/jwt_authn/authenticator_test.cc +++ b/test/extensions/filters/http/jwt_authn/authenticator_test.cc @@ -269,8 +269,7 @@ TEST_F(AuthenticatorTest, TestOnDestroy) { auto tokens = filter_config_->getExtractor().extract(headers); // callback should not be called. std::function on_complete_cb = [](const Status&) { FAIL(); }; - auto set_payload_cb = [this](const std::string&, const std::string&) {}; - auth_->verify(headers, std::move(tokens), std::move(set_payload_cb), std::move(on_complete_cb)); + auth_->verify(headers, std::move(tokens), nullptr, std::move(on_complete_cb)); // Destroy the authenticating process. auth_->onDestroy();