diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 457b0b042c0a..39b911a1684c 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -71,6 +71,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 ` diff --git a/source/extensions/filters/http/jwt_authn/extractor.cc b/source/extensions/filters/http/jwt_authn/extractor.cc index 805af897a612..da5083026b82 100644 --- a/source/extensions/filters/http/jwt_authn/extractor.cc +++ b/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( + std::string(value_str), location_spec->issuer_checker_, location_spec->header_)); } - tokens.push_back(std::make_unique( - std::string(value_str), location_spec->issuer_checker_, location_spec->header_)); } } diff --git a/test/extensions/filters/http/jwt_authn/extractor_test.cc b/test/extensions/filters/http/jwt_authn/extractor_test.cc index b639a0701619..3cad76fe54a9 100644 --- a/test/extensions/filters/http/jwt_authn/extractor_test.cc +++ b/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"