Merge remote-tracking branch 'upstream/main' into jwt_extension

changelogs/current.yaml

@@ -34,7 +34,15 @@ removed_config_or_runtime:
   change: |
     Removed the deprecated ``envoy.reloadable_features.validate_detailed_override_host_statuses``
     runtime flag and legacy code path.
+- area: grpc
+  change: |
+    Removed the deprecated ``envoy.reloadable_features.service_sanitize_non_utf8_strings``
+    runtime flag and legacy code path.
 
 new_features:
+- area: jwt
+  change: |
+    The jwt filter can now serialize non-primitive custom claims when maping claims to headers.
+    These claims will be serialized as JSON and encoded as Base64.
 
 deprecated:

docs/root/configuration/http/http_filters/jwt_authn_filter.rst

@@ -274,10 +274,13 @@ The field :ref:`claim_to_headers <envoy_v3_api_field_extensions.filters.http.jwt
         claim_name: sub
       - header_name: x-jwt-claim-nested-key
         claim_name: nested.claim.key
+      - header_name: x-jwt-tenants
+        claim_name: tenants
 
-JWT claim ("sub" and "nested.claim.key") will be added to HTTP headers as following format:
+In this example the `tenants` claim is an object, therefore the JWT claim ("sub", "nested.claim.key" and "tenants") will be added to HTTP headers as following format:
 
 .. code-block::
 
     x-jwt-claim-sub: <JWT Claim>
     x-jwt-claim-nested-key: <JWT Claim>
+    x-jwt-tenants: <Base64 encoded JSON JWT Claim>

source/common/protobuf/yaml_utility.cc

@@ -347,24 +347,9 @@ std::string utf8CoerceToStructurallyValid(absl::string_view str, const char repl
 } // namespace
 
 std::string MessageUtil::sanitizeUtf8String(absl::string_view input) {
-  if (!Runtime::runtimeFeatureEnabled(
-          "envoy.reloadable_features.service_sanitize_non_utf8_strings")) {
-    return std::string(input);
-  }
-
-  // This returns the original string if it was already valid, and returns a pointer to
-  // `result.data()` if it needed to coerce. The coerced string is always
-  // the same length as the source string.
-  //
-  // Initializing `result` to `input` ensures that `result` is correctly sized to receive the
-  // modified string, or in the case where no modification is needed it already contains the correct
-  // value, so `result` can be returned in both cases.
-  //
   // The choice of '!' is somewhat arbitrary, but we wanted to avoid any character that has
   // special semantic meaning in URLs or similar.
-  std::string result = utf8CoerceToStructurallyValid(input, '!');
-
-  return result;
+  return utf8CoerceToStructurallyValid(input, '!');
 }
 
 } // namespace Envoy

source/common/runtime/runtime_features.cc

@@ -73,7 +73,6 @@ RUNTIME_GUARD(envoy_reloadable_features_original_dst_rely_on_idle_timeout);
 RUNTIME_GUARD(envoy_reloadable_features_overload_manager_error_unknown_action);
 RUNTIME_GUARD(envoy_reloadable_features_proxy_status_upstream_request_timeout);
 RUNTIME_GUARD(envoy_reloadable_features_send_header_raw_value);
-RUNTIME_GUARD(envoy_reloadable_features_service_sanitize_non_utf8_strings);
 RUNTIME_GUARD(envoy_reloadable_features_skip_dns_lookup_for_proxied_requests);
 RUNTIME_GUARD(envoy_reloadable_features_ssl_transport_failure_reason_format);
 RUNTIME_GUARD(envoy_reloadable_features_stateful_session_encode_ttl_in_cookie);

source/extensions/filters/http/jwt_authn/authenticator.cc

@@ -306,21 +306,37 @@ void AuthenticatorImpl::addJWTClaimToHeader(const std::string& claim_name,
   const auto status = payload_getter.GetValue(claim_name, claim_value);
   std::string str_claim_value;
   if (status == StructUtils::OK) {
-    if (claim_value->kind_case() == Envoy::ProtobufWkt::Value::kStringValue) {
+    switch (claim_value->kind_case()) {
+    case Envoy::ProtobufWkt::Value::kStringValue:
       str_claim_value = claim_value->string_value();
-    } else if (claim_value->kind_case() == Envoy::ProtobufWkt::Value::kNumberValue) {
+      break;
+    case Envoy::ProtobufWkt::Value::kNumberValue:
       str_claim_value = convertClaimDoubleToString(claim_value->number_value());
-    } else if (claim_value->kind_case() == Envoy::ProtobufWkt::Value::kBoolValue) {
+      break;
+    case Envoy::ProtobufWkt::Value::kBoolValue:
       str_claim_value = claim_value->bool_value() ? "true" : "false";
-    } else {
-      ENVOY_LOG(
-          debug,
-          "--------claim : {} is not a primitive type of int, double, string, or bool -----------",
-          claim_name);
+      break;
+    case Envoy::ProtobufWkt::Value::kStructValue:
+      ABSL_FALLTHROUGH_INTENDED;
+    case Envoy::ProtobufWkt::Value::kListValue: {
+      std::string output;
+      auto status = claim_value->has_struct_value()
+                        ? ProtobufUtil::MessageToJsonString(claim_value->struct_value(), &output)
+                        : ProtobufUtil::MessageToJsonString(claim_value->list_value(), &output);
+      if (status.ok()) {
+        str_claim_value = Envoy::Base64::encode(output.data(), output.size());
+      }
+      break;
+    }
+    default:
+      ENVOY_LOG(debug, "[jwt_auth] claim : {} is of an unknown type '{}'", claim_name,
+                claim_value->kind_case());
+      break;
     }
+
     if (!str_claim_value.empty()) {
       headers_->addCopy(Http::LowerCaseString(header_name), str_claim_value);
-      ENVOY_LOG(debug, "--------claim : {} with value : {} is added to the header : {} -----------",
+      ENVOY_LOG(debug, "[jwt_auth] claim : {} with value : {} is added to the header : {}",
                 claim_name, str_claim_value, header_name);
     }
   }

source/server/config_validation/BUILD

@@ -100,6 +100,7 @@ envoy_cc_library(
         "//source/common/thread_local:thread_local_lib",
         "//source/common/version:version_lib",
         "//source/server:configuration_lib",
+        "//source/server:hot_restart_nop_lib",
         "//source/server:server_lib",
         "//source/server:utils_lib",
         "//source/server/admin:admin_lib",

source/server/config_validation/server.h

@@ -28,6 +28,7 @@
 #include "source/server/config_validation/api.h"
 #include "source/server/config_validation/cluster_manager.h"
 #include "source/server/config_validation/dns.h"
+#include "source/server/hot_restart_nop_impl.h"
 #include "source/server/server.h"
 
 #include "absl/types/optional.h"
@@ -89,7 +90,7 @@ class ValidationInstance final : Logger::Loggable<Logger::Id::main>,
   DrainManager& drainManager() override { return *drain_manager_; }
   AccessLog::AccessLogManager& accessLogManager() override { return access_log_manager_; }
   void failHealthcheck(bool) override {}
-  HotRestart& hotRestart() override { PANIC("not implemented"); }
+  HotRestart& hotRestart() override { return nop_hot_restart_; }
   Init::Manager& initManager() override { return init_manager_; }
   ServerLifecycleNotifier& lifecycleNotifier() override { return *this; }
   ListenerManager& listenerManager() override { return *listener_manager_; }
@@ -190,6 +191,7 @@ class ValidationInstance final : Logger::Loggable<Logger::Id::main>,
   Quic::QuicStatNames quic_stat_names_;
   Filter::TcpListenerFilterConfigProviderManagerImpl tcp_listener_config_provider_manager_;
   Server::DrainManagerPtr drain_manager_;
+  HotRestartNopImpl nop_hot_restart_;
 };
 
 } // namespace Server

test/common/protobuf/utility_test.cc

@@ -1195,19 +1195,6 @@ TEST_F(ProtobufUtilityTest, SanitizeUTF8) {
     EXPECT_EQ(absl::string_view("valid_prefix!!valid_middle!valid_suffix"), sanitized);
     EXPECT_EQ(sanitized.length(), original.length());
   }
-
-  {
-    TestScopedRuntime scoped_runtime;
-    scoped_runtime.mergeValues(
-        {{"envoy.reloadable_features.service_sanitize_non_utf8_strings", "false"}});
-    std::string original("valid_prefix");
-    original.append(1, char(0xc3));
-    original.append(1, char(0xc7));
-    original.append("valid_suffix");
-
-    std::string non_sanitized = MessageUtil::sanitizeUtf8String(original);
-    EXPECT_EQ(non_sanitized, original);
-  }
 }
 
 TEST_F(ProtobufUtilityTest, KeyValueStruct) {

test/common/upstream/cluster_manager_impl_test.cc

@@ -87,6 +87,57 @@ void verifyCaresDnsConfigAndUnpack(
   typed_dns_resolver_config.typed_config().UnpackTo(&cares);
 }
 
+// Helper to intercept calls to postThreadLocalClusterUpdate.
+class MockLocalClusterUpdate {
+public:
+  MOCK_METHOD(void, post,
+              (uint32_t priority, const HostVector& hosts_added, const HostVector& hosts_removed));
+};
+
+class MockLocalHostsRemoved {
+public:
+  MOCK_METHOD(void, post, (const HostVector&));
+};
+
+// Override postThreadLocalClusterUpdate so we can test that merged updates calls
+// it with the right values at the right times.
+class MockedUpdatedClusterManagerImpl : public TestClusterManagerImpl {
+public:
+  using TestClusterManagerImpl::TestClusterManagerImpl;
+
+  MockedUpdatedClusterManagerImpl(const envoy::config::bootstrap::v3::Bootstrap& bootstrap,
+                                  ClusterManagerFactory& factory, Stats::Store& stats,
+                                  ThreadLocal::Instance& tls, Runtime::Loader& runtime,
+                                  const LocalInfo::LocalInfo& local_info,
+                                  AccessLog::AccessLogManager& log_manager,
+                                  Event::Dispatcher& main_thread_dispatcher, Server::Admin& admin,
+                                  ProtobufMessage::ValidationContext& validation_context,
+                                  Api::Api& api, MockLocalClusterUpdate& local_cluster_update,
+                                  MockLocalHostsRemoved& local_hosts_removed,
+                                  Http::Context& http_context, Grpc::Context& grpc_context,
+                                  Router::Context& router_context, Server::Instance& server)
+      : TestClusterManagerImpl(bootstrap, factory, stats, tls, runtime, local_info, log_manager,
+                               main_thread_dispatcher, admin, validation_context, api, http_context,
+                               grpc_context, router_context, server),
+        local_cluster_update_(local_cluster_update), local_hosts_removed_(local_hosts_removed) {}
+
+protected:
+  void postThreadLocalClusterUpdate(ClusterManagerCluster&,
+                                    ThreadLocalClusterUpdateParams&& params) override {
+    for (const auto& per_priority : params.per_priority_update_params_) {
+      local_cluster_update_.post(per_priority.priority_, per_priority.hosts_added_,
+                                 per_priority.hosts_removed_);
+    }
+  }
+
+  void postThreadLocalRemoveHosts(const Cluster&, const HostVector& hosts_removed) override {
+    local_hosts_removed_.post(hosts_removed);
+  }
+
+  MockLocalClusterUpdate& local_cluster_update_;
+  MockLocalHostsRemoved& local_hosts_removed_;
+};
+
 // A gRPC MuxFactory that returns a MockGrpcMux instance when trying to instantiate a mux for the
 // `envoy.config_mux.grpc_mux_factory` type. This enables testing call expectations on the ADS gRPC
 // mux.

test/common/upstream/test_cluster_manager.h

@@ -159,18 +159,6 @@ class TestClusterManagerFactory : public ClusterManagerFactory {
   Server::MockOptions& options_ = server_context_.options_;
 };
 
-// Helper to intercept calls to postThreadLocalClusterUpdate.
-class MockLocalClusterUpdate {
-public:
-  MOCK_METHOD(void, post,
-              (uint32_t priority, const HostVector& hosts_added, const HostVector& hosts_removed));
-};
-
-class MockLocalHostsRemoved {
-public:
-  MOCK_METHOD(void, post, (const HostVector&));
-};
-
 // A test version of ClusterManagerImpl that provides a way to get a non-const handle to the
 // clusters, which is necessary in order to call updateHosts on the priority set.
 class TestClusterManagerImpl : public ClusterManagerImpl {
@@ -231,44 +219,5 @@ class TestClusterManagerImpl : public ClusterManagerImpl {
                            grpc_context, router_context, server) {}
 };
 
-// Override postThreadLocalClusterUpdate so we can test that merged updates calls
-// it with the right values at the right times.
-class MockedUpdatedClusterManagerImpl : public TestClusterManagerImpl {
-public:
-  using TestClusterManagerImpl::TestClusterManagerImpl;
-
-  MockedUpdatedClusterManagerImpl(const envoy::config::bootstrap::v3::Bootstrap& bootstrap,
-                                  ClusterManagerFactory& factory, Stats::Store& stats,
-                                  ThreadLocal::Instance& tls, Runtime::Loader& runtime,
-                                  const LocalInfo::LocalInfo& local_info,
-                                  AccessLog::AccessLogManager& log_manager,
-                                  Event::Dispatcher& main_thread_dispatcher, Server::Admin& admin,
-                                  ProtobufMessage::ValidationContext& validation_context,
-                                  Api::Api& api, MockLocalClusterUpdate& local_cluster_update,
-                                  MockLocalHostsRemoved& local_hosts_removed,
-                                  Http::Context& http_context, Grpc::Context& grpc_context,
-                                  Router::Context& router_context, Server::Instance& server)
-      : TestClusterManagerImpl(bootstrap, factory, stats, tls, runtime, local_info, log_manager,
-                               main_thread_dispatcher, admin, validation_context, api, http_context,
-                               grpc_context, router_context, server),
-        local_cluster_update_(local_cluster_update), local_hosts_removed_(local_hosts_removed) {}
-
-protected:
-  void postThreadLocalClusterUpdate(ClusterManagerCluster&,
-                                    ThreadLocalClusterUpdateParams&& params) override {
-    for (const auto& per_priority : params.per_priority_update_params_) {
-      local_cluster_update_.post(per_priority.priority_, per_priority.hosts_added_,
-                                 per_priority.hosts_removed_);
-    }
-  }
-
-  void postThreadLocalRemoveHosts(const Cluster&, const HostVector& hosts_removed) override {
-    local_hosts_removed_.post(hosts_removed);
-  }
-
-  MockLocalClusterUpdate& local_cluster_update_;
-  MockLocalHostsRemoved& local_hosts_removed_;
-};
-
 } // namespace Upstream
 } // namespace Envoy

test/extensions/filters/http/jwt_authn/authenticator_test.cc

@@ -1,6 +1,7 @@
 #include "envoy/config/core/v3/http_uri.pb.h"
 #include "envoy/extensions/filters/http/jwt_authn/v3/config.pb.h"
 
+#include "source/common/common/base64.h"
 #include "source/common/http/message_impl.h"
 #include "source/common/protobuf/utility.h"
 #include "source/extensions/filters/http/common/jwks_fetcher.h"
@@ -153,6 +154,13 @@ TEST_F(AuthenticatorTest, TestClaimToHeader) {
   EXPECT_EQ(headers.get_("x-jwt-claim-nested"), "value1");
   EXPECT_EQ(headers.get_("x-jwt-bool-claim"), "true");
   EXPECT_EQ(headers.get_("x-jwt-int-claim"), "9999");
+
+  // This check verifies whether the claim with non-primitive type are
+  // successfully serialized and added to headers.
+  std::string expected_json = "[\"str1\",\"str2\"]";
+
+  ASSERT_EQ(headers.get_("x-jwt-claim-object-key"),
+            Envoy::Base64::encode(expected_json.data(), expected_json.size()));
 }
 
 // This test verifies when wrong claim is passed in claim_to_headers

test/extensions/filters/http/jwt_authn/test_common.h

@@ -97,6 +97,8 @@ const char ExampleConfig[] = R"(
       claim_name: "nested.nested-2.key-3"
     - header_name: "x-jwt-int-claim"
       claim_name: "nested.nested-2.key-4"
+    - header_name: "x-jwt-claim-object-key"
+      claim_name: "nested.nested-2.key-5"
 rules:
 - match:
     path: "/"

tools/deprecate_version/deprecate_version.py

@@ -115,16 +115,23 @@ def create_issues(access_token, runtime_and_pr):
     if get_confirmation():
         print('Creating issues...')
         for title, body, login in issues:
+            issue_created = False
             try:
-                repo.create_issue(title, body=body, assignees=[login], labels=labels)
+                # for setec backports, we may not find a user, which would make
+                # create_issue crash.
+                if login:
+                    repo.create_issue(title, body=body, assignees=[login], labels=labels)
+                    issue_created = True
             except github.GithubException as e:
+                print((
+                    'unable to assign issue %s to %s. Add them to the Envoy proxy org'
+                    'and assign it their way.') % (title, login))
+
+            if not issue_created:
                 try:
                     if login:
                         body += '\ncc @' + login
                     repo.create_issue(title, body=body, labels=labels)
-                    print((
-                        'unable to assign issue %s to %s. Add them to the Envoy proxy org'
-                        'and assign it their way.') % (title, login))
                 except github.GithubException as e:
                     print('GithubException while creating issue.')
                     raise