jwt: do not concatenate duplicated headers (#32248)

Duplicated headers should not be concatenated with a comma, because comma is not allowed in a JWT token, so concatenation invalidates tokens.
This PR fixes #31468.

Risk Level:
Testing: unit tests
Docs Changes: none
Release Notes:
Platform Specific Features: none

Signed-off-by: Jacek Ewertowski <jewertow@redhat.com>

changelogs/current.yaml

@@ -75,6 +75,9 @@ bug_fixes:
     Fix crash due to uncaught exception when the operating system does not support an address type (such as IPv6) that is
     received in an mTLS client cert IP SAN. These SANs will be ignored. This applies only when using formatter
     ``%DOWNSTREAM_PEER_IP_SAN%``.
+- area: jwt_authn
+  change: |
+    Fixed JWT extractor, which concatenated headers with a comma, resultig in invalid tokens.
 
 removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

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

@@ -253,20 +253,21 @@ ExtractorImpl::extract(const Http::RequestHeaderMap& headers) const {
   for (const auto& location_it : header_locations_) {
     const auto& location_spec = location_it.second;
     ENVOY_LOG(debug, "extract {}", location_it.first);
-    const auto result =
-        Http::HeaderUtility::getAllOfHeaderAsString(headers, location_spec->header_);
-    if (result.result().has_value()) {
-      auto value_str = result.result().value();
-      if (!location_spec->value_prefix_.empty()) {
-        const auto pos = value_str.find(location_spec->value_prefix_);
-        if (pos == absl::string_view::npos) {
-          // value_prefix not found anywhere in value_str, so skip
-          continue;
+    auto header_value = headers.get(location_spec->header_);
+    if (!header_value.empty()) {
+      for (size_t i = 0; i < header_value.size(); i++) {
+        auto value_str = header_value[i]->value().getStringView();
+        if (!location_spec->value_prefix_.empty()) {
+          const auto pos = value_str.find(location_spec->value_prefix_);
+          if (pos == absl::string_view::npos) {
+            // value_prefix not found anywhere in value_str, so skip
+            continue;
+          }
+          value_str = extractJWT(value_str, pos + location_spec->value_prefix_.length());
         }
-        value_str = extractJWT(value_str, pos + location_spec->value_prefix_.length());
+        tokens.push_back(std::make_unique<const JwtHeaderLocation>(
+            std::string(value_str), location_spec->issuer_checker_, location_spec->header_));
       }
-      tokens.push_back(std::make_unique<const JwtHeaderLocation>(
-          std::string(value_str), location_spec->issuer_checker_, location_spec->header_));
     }
   }
 

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

@@ -143,6 +143,30 @@ TEST_F(ExtractorTest, TestDefaultHeaderLocationWithValidJWT) {
   EXPECT_TRUE(tokens[0]->isIssuerAllowed("issuer1"));
 }
 
+TEST_F(ExtractorTest, TestDuplicatedHeadersWithDuplicatedTokenPrefixes) {
+  auto headers = TestRequestHeaderMapImpl{{"Authorization", absl::StrCat("Bearer ", GoodToken)},
+                                          {"Authorization", absl::StrCat("Bearer ", GoodToken)}};
+  auto tokens = extractor_->extract(headers);
+  EXPECT_EQ(tokens.size(), 2);
+
+  // Only the issue1 is using default header location.
+  EXPECT_EQ(tokens[0]->token(), GoodToken);
+  EXPECT_TRUE(tokens[0]->isIssuerAllowed("issuer1"));
+  EXPECT_EQ(tokens[1]->token(), GoodToken);
+  EXPECT_TRUE(tokens[1]->isIssuerAllowed("issuer1"));
+}
+
+TEST_F(ExtractorTest, TestDuplicatedHeadersWithUniqueTokenPrefixes) {
+  auto headers = TestRequestHeaderMapImpl{{"Authorization", absl::StrCat("Bearer ", GoodToken)},
+                                          {"Authorization", absl::StrCat("Basic ", "basic")}};
+  auto tokens = extractor_->extract(headers);
+  EXPECT_EQ(tokens.size(), 1);
+
+  // Only the issue1 is using default header location.
+  EXPECT_EQ(tokens[0]->token(), GoodToken);
+  EXPECT_TRUE(tokens[0]->isIssuerAllowed("issuer1"));
+}
+
 // Test extracting JWT as Bearer token from the default header location: "Authorization" -
 // using an actual (correctly-formatted) JWT but token is invalid, like: GoodToken +
 // chars_after_space expected to get all token include characters after the space:
@@ -203,12 +227,13 @@ TEST_F(ExtractorTest, TestCustomHeaderToken) {
   EXPECT_FALSE(headers.has(Http::LowerCaseString("token-header")));
 }
 
-// Make sure a double custom header concatenates the token
+// Make sure a double custom header does not concatenate the token
 TEST_F(ExtractorTest, TestDoubleCustomHeaderToken) {
   auto headers = TestRequestHeaderMapImpl{{"token-header", "jwt_token"}, {"token-header", "foo"}};
   auto tokens = extractor_->extract(headers);
-  EXPECT_EQ(tokens.size(), 1);
-  EXPECT_EQ(tokens[0]->token(), "jwt_token,foo");
+  EXPECT_EQ(tokens.size(), 2);
+  EXPECT_EQ(tokens[0]->token(), "jwt_token");
+  EXPECT_EQ(tokens[1]->token(), "foo");
 }
 
 // Test extracting token from the custom header: "prefix-header"