Using request dynamic metadata to pass data from JWT filter to authn filter.

include/istio/utils/BUILD

@@ -41,4 +41,4 @@ cc_library(
         "attribute_names.h",
     ],
     visibility = ["//visibility:public"],
-)
+)

src/envoy/http/authn/BUILD

@@ -46,6 +46,7 @@ envoy_cc_library(
         "//src/envoy/http/jwt_auth:jwt_lib",
         "//src/envoy/utils:utils_lib",
         "//src/istio/authn:context_proto",
+        "//src/envoy/utils:filter_names_lib",
     ],
 )
 
@@ -66,6 +67,7 @@ envoy_cc_library(
         "//src/envoy/utils:utils_lib",
         "//src/istio/authn:context_proto",
         "@envoy//source/exe:envoy_common_lib",
+        "//src/envoy/utils:filter_names_lib",
     ],
 )
 
@@ -85,6 +87,7 @@ envoy_cc_test(
     deps = [
         ":authenticator",
         ":test_utils",
+        "@envoy//test/mocks/http:http_mocks",
         "@envoy//test/test_common:utility_lib",
     ],
 )
@@ -97,6 +100,7 @@ envoy_cc_test(
         ":authenticator",
         ":test_utils",
         "@envoy//test/mocks/network:network_mocks",
+        "//src/envoy/utils:filter_names_lib",
         "@envoy//test/mocks/ssl:ssl_mocks",
         "@envoy//test/test_common:utility_lib",
     ],
@@ -158,6 +162,8 @@ envoy_cc_test(
     repository = "@envoy",
     deps = [
         ":filter_lib",
-        "@envoy//test/integration:http_integration_lib",
+        "@envoy//source/common/common:utility_lib",
+        "@envoy//test/integration:http_protocol_integration_lib",
+        "//src/envoy/utils:filter_names_lib",
     ],
 )

src/envoy/http/authn/authenticator_base.cc

@@ -15,7 +15,9 @@
 
 #include "src/envoy/http/authn/authenticator_base.h"
 #include "common/common/assert.h"
+#include "common/config/metadata.h"
 #include "src/envoy/http/authn/authn_utils.h"
+#include "src/envoy/utils/filter_names.h"
 #include "src/envoy/utils/utils.h"
 
 using istio::authn::Payload;
@@ -64,21 +66,11 @@ bool AuthenticatorBase::validateX509(const iaapi::MutualTls& mtls,
 }
 
 bool AuthenticatorBase::validateJwt(const iaapi::Jwt& jwt, Payload* payload) {
-  Envoy::Http::HeaderMap& header = *filter_context()->headers();
-
-  auto iter =
-      filter_context()->filter_config().jwt_output_payload_locations().find(
-          jwt.issuer());
-  if (iter ==
-      filter_context()->filter_config().jwt_output_payload_locations().end()) {
-    ENVOY_LOG(warn, "No JWT payload header location is found for the issuer {}",
-              jwt.issuer());
-    return false;
+  std::string jwt_payload;
+  if (filter_context()->getJwtPayload(jwt.issuer(), &jwt_payload)) {
+    return AuthnUtils::ProcessJwtPayload(jwt_payload, payload->mutable_jwt());
   }
-
-  LowerCaseString header_key(iter->second);
-  return AuthnUtils::GetJWTPayloadFromHeaders(header, header_key,
-                                              payload->mutable_jwt());
+  return false;
 }
 
 }  // namespace AuthN

src/envoy/http/authn/authenticator_base_test.cc

@@ -16,9 +16,11 @@
 #include "src/envoy/http/authn/authenticator_base.h"
 #include "common/common/base64.h"
 #include "common/protobuf/protobuf.h"
+#include "envoy/api/v2/core/base.pb.h"
 #include "envoy/config/filter/http/authn/v2alpha1/config.pb.h"
 #include "gmock/gmock.h"
 #include "src/envoy/http/authn/test_utils.h"
+#include "src/envoy/utils/filter_names.h"
 #include "test/mocks/network/mocks.h"
 #include "test/mocks/ssl/mocks.h"
 
@@ -27,6 +29,7 @@ using istio::authn::Payload;
 using istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig;
 using testing::NiceMock;
 using testing::Return;
+using testing::StrictMock;
 
 namespace iaapi = istio::authentication::v1alpha1;
 
@@ -36,7 +39,6 @@ namespace Istio {
 namespace AuthN {
 namespace {
 
-const std::string kSecIstioAuthUserInfoHeaderKey = "sec-istio-auth-userinfo";
 const std::string kSecIstioAuthUserinfoHeaderValue =
     R"(
      {
@@ -60,13 +62,13 @@ class ValidateX509Test : public testing::TestWithParam<iaapi::MutualTls::Mode>,
  public:
   virtual ~ValidateX509Test() {}
 
-  Http::TestHeaderMapImpl request_headers_{};
   NiceMock<Envoy::Network::MockConnection> connection_{};
   NiceMock<Envoy::Ssl::MockConnection> ssl_{};
   FilterConfig filter_config_{};
-  FilterContext filter_context_{&request_headers_, &connection_,
-                                istio::envoy::config::filter::http::authn::
-                                    v2alpha1::FilterConfig::default_instance()};
+  FilterContext filter_context_{
+      envoy::api::v2::core::Metadata::default_instance(), &connection_,
+      istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig::
+          default_instance()};
 
   MockAuthenticatorBase authenticator_{&filter_context_};
 
@@ -162,39 +164,27 @@ class ValidateJwtTest : public testing::Test,
  public:
   virtual ~ValidateJwtTest() {}
 
-  Http::TestHeaderMapImpl request_headers_{};
+  // StrictMock<Envoy::RequestInfo::MockRequestInfo> request_info_{};
+  envoy::api::v2::core::Metadata dynamic_metadata_;
   NiceMock<Envoy::Network::MockConnection> connection_{};
-  NiceMock<Envoy::Ssl::MockConnection> ssl_{};
+  // NiceMock<Envoy::Ssl::MockConnection> ssl_{};
   FilterConfig filter_config_{};
-  FilterContext filter_context_{&request_headers_, &connection_,
-                                istio::envoy::config::filter::http::authn::
-                                    v2alpha1::FilterConfig::default_instance()};
-
+  FilterContext filter_context_{dynamic_metadata_, &connection_,
+                                filter_config_};
   MockAuthenticatorBase authenticator_{&filter_context_};
 
   void SetUp() override { payload_ = new Payload(); }
 
   void TearDown() override { delete (payload_); }
 
-  Http::TestHeaderMapImpl CreateTestHeaderMap(const std::string& header_key,
-                                              const std::string& header_value) {
-    // The base64 encoding is done through Base64::encode().
-    // If the test input has special chars, may need to use the counterpart of
-    // Base64UrlDecode().
-    std::string value_base64 =
-        Base64::encode(header_value.c_str(), header_value.size());
-    return Http::TestHeaderMapImpl{{header_key, value_base64}};
-  }
-
  protected:
   iaapi::MutualTls mtls_params_;
   iaapi::Jwt jwt_;
   Payload* payload_;
   Payload default_payload_;
 };
 
-// TODO: more tests for Jwt.
-TEST_F(ValidateJwtTest, ValidateJwtWithNoIstioAuthnConfig) {
+TEST_F(ValidateJwtTest, NoIstioAuthnConfig) {
   jwt_.set_issuer("issuer@foo.com");
   // authenticator_ has empty Istio authn config
   // When there is empty Istio authn config, validateJwt() should return
@@ -206,7 +196,6 @@ TEST_F(ValidateJwtTest, ValidateJwtWithNoIstioAuthnConfig) {
 TEST_F(ValidateJwtTest, NoIssuer) {
   // no issuer in jwt
   google::protobuf::util::JsonParseOptions options;
-  FilterConfig filter_config;
   JsonStringToMessage(
       R"({
               "jwt_output_payload_locations":
@@ -215,83 +204,70 @@ TEST_F(ValidateJwtTest, NoIssuer) {
               }
            }
         )",
-      &filter_config, options);
-  Http::TestHeaderMapImpl empty_request_headers{};
-  FilterContext filter_context{&empty_request_headers, &connection_,
-                               filter_config};
-  MockAuthenticatorBase authenticator{&filter_context};
+      &filter_config_, options);
 
   // When there is no issuer in the JWT config, validateJwt() should return
   // nullptr and failure.
   EXPECT_FALSE(authenticator_.validateJwt(jwt_, payload_));
   EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_));
 }
 
-TEST_F(ValidateJwtTest, EmptyJwtOutputPayloadLocations) {
+TEST_F(ValidateJwtTest, OutputPayloadLocationNotDefine) {
   jwt_.set_issuer("issuer@foo.com");
-  Http::TestHeaderMapImpl request_headers_with_jwt = CreateTestHeaderMap(
-      kSecIstioAuthUserInfoHeaderKey, kSecIstioAuthUserinfoHeaderValue);
   google::protobuf::util::JsonParseOptions options;
-  FilterConfig filter_config;
   JsonStringToMessage(
       R"({
               "jwt_output_payload_locations":
               {
               }
            }
         )",
-      &filter_config, options);
-  FilterContext filter_context{&request_headers_with_jwt, &connection_,
-                               filter_config};
-  MockAuthenticatorBase authenticator{&filter_context};
+      &filter_config_, options);
+
   // authenticator has empty jwt_output_payload_locations in Istio authn config
   // When there is no matching jwt_output_payload_locations for the issuer in
   // the Istio authn config, validateJwt() should return nullptr and failure.
   EXPECT_FALSE(authenticator_.validateJwt(jwt_, payload_));
   EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_));
 }
 
-TEST_F(ValidateJwtTest, NoJwtInHeader) {
+TEST_F(ValidateJwtTest, NoJwtPayloadOutput) {
   jwt_.set_issuer("issuer@foo.com");
-  google::protobuf::util::JsonParseOptions options;
-  FilterConfig filter_config;
-  JsonStringToMessage(
-      R"({
-              "jwt_output_payload_locations":
-              {
-                "issuer@foo.com": "sec-istio-auth-jwt-output"
-              }
-           }
-        )",
-      &filter_config, options);
-  Http::TestHeaderMapImpl empty_request_headers{};
-  FilterContext filter_context{&empty_request_headers, &connection_,
-                               filter_config};
-  MockAuthenticatorBase authenticator{&filter_context};
-  // When there is no JWT in the HTTP header, validateJwt() should return
-  // nullptr and failure.
+
+  // When there is no JWT in request info dynamic metadata, validateJwt() should
+  // return nullptr and failure.
   EXPECT_FALSE(authenticator_.validateJwt(jwt_, payload_));
   EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_));
 }
 
-TEST_F(ValidateJwtTest, JwtInHeader) {
+TEST_F(ValidateJwtTest, HasJwtPayloadOutputButNoDataForKey) {
   jwt_.set_issuer("issuer@foo.com");
-  Http::TestHeaderMapImpl request_headers_with_jwt = CreateTestHeaderMap(
-      "sec-istio-auth-jwt-output", kSecIstioAuthUserinfoHeaderValue);
-  google::protobuf::util::JsonParseOptions options;
-  FilterConfig filter_config;
-  JsonStringToMessage(
-      R"({
-              "jwt_output_payload_locations":
-              {
-                "issuer@foo.com": "sec-istio-auth-jwt-output"
-              }
-           }
-        )",
-      &filter_config, options);
-  FilterContext filter_context{&request_headers_with_jwt, &connection_,
-                               filter_config};
-  MockAuthenticatorBase authenticator{&filter_context};
+
+  (*dynamic_metadata_.mutable_filter_metadata())[Utils::IstioFilterName::kJwt]
+      .MergeFrom(MessageUtil::keyValueStruct("foo", "bar"));
+
+  // When there is no JWT payload for given issuer in request info dynamic
+  // metadata, validateJwt() should return nullptr and failure.
+  EXPECT_FALSE(authenticator_.validateJwt(jwt_, payload_));
+  EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_));
+}
+
+TEST_F(ValidateJwtTest, JwtPayloadAvailableWithBadData) {
+  jwt_.set_issuer("issuer@foo.com");
+  (*dynamic_metadata_.mutable_filter_metadata())[Utils::IstioFilterName::kJwt]
+      .MergeFrom(MessageUtil::keyValueStruct("issuer@foo.com", "bad-data"));
+  // EXPECT_CALL(request_info_, dynamicMetadata());
+
+  EXPECT_FALSE(authenticator_.validateJwt(jwt_, payload_));
+  EXPECT_TRUE(MessageDifferencer::Equivalent(*payload_, default_payload_));
+}
+
+TEST_F(ValidateJwtTest, JwtPayloadAvailable) {
+  jwt_.set_issuer("issuer@foo.com");
+  (*dynamic_metadata_.mutable_filter_metadata())[Utils::IstioFilterName::kJwt]
+      .MergeFrom(MessageUtil::keyValueStruct("issuer@foo.com",
+                                             kSecIstioAuthUserinfoHeaderValue));
+
   Payload expected_payload;
   JsonStringToMessage(
       R"({
@@ -309,9 +285,9 @@ TEST_F(ValidateJwtTest, JwtInHeader) {
              }
            }
         )",
-      &expected_payload, options);
+      &expected_payload, google::protobuf::util::JsonParseOptions{});
 
-  EXPECT_TRUE(authenticator.validateJwt(jwt_, payload_));
+  EXPECT_TRUE(authenticator_.validateJwt(jwt_, payload_));
   EXPECT_TRUE(MessageDifferencer::Equals(expected_payload, *payload_));
 }
 

src/envoy/http/authn/authn_utils.cc

@@ -50,27 +50,8 @@ void ExtractJwtAudience(
 }
 };  // namespace
 
-// Retrieve the JwtPayload from the HTTP headers with the key
-bool AuthnUtils::GetJWTPayloadFromHeaders(
-    const HeaderMap& headers, const LowerCaseString& jwt_payload_key,
-    istio::authn::JwtPayload* payload) {
-  const HeaderEntry* entry = headers.get(jwt_payload_key);
-  if (!entry) {
-    ENVOY_LOG(debug, "No JWT payload {} in the header", jwt_payload_key.get());
-    return false;
-  }
-  std::string value(entry->value().c_str(), entry->value().size());
-  // JwtAuth::Base64UrlDecode() is different from Base64::decode().
-  std::string payload_str = JwtAuth::Base64UrlDecode(value);
-  // Return an empty string if Base64 decode fails.
-  if (payload_str.empty()) {
-    ENVOY_LOG(error, "Invalid {} header, invalid base64: {}",
-              jwt_payload_key.get(), value);
-    return false;
-  }
-  *payload->mutable_raw_claims() = payload_str;
-  ::google::protobuf::Map< ::std::string, ::std::string>* claims =
-      payload->mutable_claims();
+bool AuthnUtils::ProcessJwtPayload(const std::string& payload_str,
+                                   istio::authn::JwtPayload* payload) {
   Envoy::Json::ObjectSharedPtr json_obj;
   try {
     json_obj = Json::Factory::loadFromString(payload_str);
@@ -80,6 +61,10 @@ bool AuthnUtils::GetJWTPayloadFromHeaders(
     return false;
   }
 
+  *payload->mutable_raw_claims() = payload_str;
+  ::google::protobuf::Map< ::std::string, ::std::string>* claims =
+      payload->mutable_claims();
+
   // Extract claims
   json_obj->iterate(
       [payload](const std::string& key, const Json::Object& obj) -> bool {

src/envoy/http/authn/authn_utils.h

@@ -28,12 +28,11 @@ namespace AuthN {
 // AuthnUtils class provides utility functions used for authentication.
 class AuthnUtils : public Logger::Loggable<Logger::Id::filter> {
  public:
-  // Retrieve the JWT payload from the HTTP header into the output payload map
-  // Return true if parsing the header payload key succeeds.
-  // Otherwise, return false.
-  static bool GetJWTPayloadFromHeaders(const HeaderMap& headers,
-                                       const LowerCaseString& jwt_payload_key,
-                                       istio::authn::JwtPayload* payload);
+  // Parse JWT payload string (which typically is the output from jwt filter)
+  // and populate JwtPayload object. Return true if input string can be parsed
+  // successfully. Otherwise, return false.
+  static bool ProcessJwtPayload(const std::string& jwt_payload_str,
+                                istio::authn::JwtPayload* payload);
 };
 
 }  // namespace AuthN