Skip to content

Commit

Permalink
Merge commit '1.9.9' into mt-1.9.8
Browse files Browse the repository at this point in the history
Signed-off-by: jiangshantao <[email protected]>
  • Loading branch information
jiangshantao committed Feb 8, 2022
2 parents 454c405 + 451c785 commit d53d856
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 24 deletions.
17 changes: 17 additions & 0 deletions source/common/http/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,23 @@ const std::string& Utility::getProtocolString(const Protocol protocol) {
NOT_REACHED_GCOVR_EXCL_LINE;
}

std::string Utility::buildOriginalUri(const Http::RequestHeaderMap& request_headers,
const absl::optional<uint32_t> max_path_length) {
if (!request_headers.Path()) {
return "";
}
absl::string_view path(request_headers.EnvoyOriginalPath()
? request_headers.getEnvoyOriginalPathValue()
: request_headers.getPathValue());

if (max_path_length.has_value() && path.length() > max_path_length) {
path = path.substr(0, max_path_length.value());
}

return absl::StrCat(request_headers.getForwardedProtoValue(), "://",
request_headers.getHostValue(), path);
}

void Utility::extractHostPathFromUri(const absl::string_view& uri, absl::string_view& host,
absl::string_view& path) {
/**
Expand Down
9 changes: 9 additions & 0 deletions source/common/http/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,15 @@ bool sanitizeConnectionHeader(Http::RequestHeaderMap& headers);
*/
const std::string& getProtocolString(const Protocol p);

/**
* Constructs the original URI sent from the client from
* the request headers.
* @param request headers from the original request
* @param length to truncate the constructed URI's path
*/
std::string buildOriginalUri(const Http::RequestHeaderMap& request_headers,
absl::optional<uint32_t> max_path_length);

/**
* Extract host and path from a URI. The host may contain port.
* This function doesn't validate if the URI is valid. It only parses the URI with following
Expand Down
22 changes: 3 additions & 19 deletions source/common/tracing/http_tracer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,6 @@ static absl::string_view valueOrDefault(const Http::HeaderEntry* header,
return header ? header->value().getStringView() : default_value;
}

static std::string buildUrl(const Http::RequestHeaderMap& request_headers,
const uint32_t max_path_length) {
if (!request_headers.Path()) {
return "";
}
absl::string_view path(request_headers.EnvoyOriginalPath()
? request_headers.getEnvoyOriginalPathValue()
: request_headers.getPathValue());

if (path.length() > max_path_length) {
path = path.substr(0, max_path_length);
}

return absl::StrCat(request_headers.getForwardedProtoValue(), "://",
request_headers.getHostValue(), path);
}

const std::string HttpTracerUtility::IngressOperation = "ingress";
const std::string HttpTracerUtility::EgressOperation = "egress";

Expand Down Expand Up @@ -163,8 +146,9 @@ void HttpTracerUtility::finalizeDownstreamSpan(Span& span,
if (request_headers->RequestId()) {
span.setTag(Tracing::Tags::get().GuidXRequestId, request_headers->getRequestIdValue());
}
span.setTag(Tracing::Tags::get().HttpUrl,
buildUrl(*request_headers, tracing_config.maxPathTagLength()));
span.setTag(
Tracing::Tags::get().HttpUrl,
Http::Utility::buildOriginalUri(*request_headers, tracing_config.maxPathTagLength()));
span.setTag(Tracing::Tags::get().HttpMethod, request_headers->getMethodValue());
span.setTag(Tracing::Tags::get().DownstreamCluster,
valueOrDefault(request_headers->EnvoyDownstreamServiceCluster(), "-"));
Expand Down
15 changes: 12 additions & 3 deletions source/extensions/filters/http/jwt_authn/filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ namespace HttpFilters {
namespace JwtAuthn {

namespace {

constexpr absl::string_view InvalidTokenErrorString = ", error=\"invalid_token\"";
constexpr uint32_t MaximumUriLength = 256;
Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
access_control_request_method_handle(Http::CustomHeaders::get().AccessControlRequestMethod);
Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
Expand Down Expand Up @@ -87,6 +88,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers,
if (verifier == nullptr) {
onComplete(Status::Ok);
} else {
original_uri_ = Http::Utility::buildOriginalUri(headers, MaximumUriLength);
// Verify the JWT token, onComplete() will be called when completed.
context_ = Verifier::createContext(headers, decoder_callbacks_->activeSpan(), this);
verifier->verify(context_);
Expand Down Expand Up @@ -119,8 +121,15 @@ void Filter::onComplete(const Status& status) {
status == Status::JwtAudienceNotAllowed ? Http::Code::Forbidden : Http::Code::Unauthorized;
// return failure reason as message body
decoder_callbacks_->sendLocalReply(
code, ::google::jwt_verify::getStatusString(status), nullptr, absl::nullopt,
generateRcDetails(::google::jwt_verify::getStatusString(status)));
code, ::google::jwt_verify::getStatusString(status),
[uri = this->original_uri_, status](Http::ResponseHeaderMap& headers) {
std::string value = absl::StrCat("Bearer realm=\"", uri, "\"");
if (status != Status::JwtMissed) {
absl::StrAppend(&value, InvalidTokenErrorString);
}
headers.setCopy(Http::Headers::get().WWWAuthenticate, value);
},
absl::nullopt, generateRcDetails(::google::jwt_verify::getStatusString(status)));
return;
}
stats_.allowed_.inc();
Expand Down
2 changes: 2 additions & 0 deletions source/extensions/filters/http/jwt_authn/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class Filter : public Http::StreamDecoderFilter,
FilterConfigSharedPtr config_;
// Verify context for current request.
ContextSharedPtr context_;

std::string original_uri_;
};

} // namespace JwtAuthn
Expand Down
25 changes: 25 additions & 0 deletions test/common/http/utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,31 @@ TEST(HttpUtility, TestRejectTeHeaderTooLong) {
EXPECT_EQ(sanitized_headers, request_headers);
}

TEST(HttpUtility, TestRejectUriWithNoPath) {
Http::TestRequestHeaderMapImpl request_headers_no_path = {
{":method", "GET"}, {":authority", "example.com"}, {"x-forwarded-proto", "http"}};
EXPECT_EQ(Utility::buildOriginalUri(request_headers_no_path, {}), "");
}

TEST(HttpUtility, TestTruncateUri) {
Http::TestRequestHeaderMapImpl request_headers_truncated_path = {{":method", "GET"},
{":path", "/hello_world"},
{":authority", "example.com"},
{"x-forwarded-proto", "http"}};
EXPECT_EQ(Utility::buildOriginalUri(request_headers_truncated_path, 2), "http://example.com/h");
}

TEST(HttpUtility, TestUriUsesOriginalPath) {
Http::TestRequestHeaderMapImpl request_headers_truncated_path = {
{":method", "GET"},
{":path", "/hello_world"},
{":authority", "example.com"},
{"x-forwarded-proto", "http"},
{"x-envoy-original-path", "/goodbye_world"}};
EXPECT_EQ(Utility::buildOriginalUri(request_headers_truncated_path, {}),
"http://example.com/goodbye_world");
}

TEST(Url, ParsingFails) {
Utility::Url url;
EXPECT_FALSE(url.initialize("", false));
Expand Down
1 change: 1 addition & 0 deletions test/common/tracing/http_tracer_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,7 @@ TEST(HttpNullTracerTest, BasicFunctionality) {
ASSERT_EQ("", span_ptr->getBaggage("baggage_key"));
ASSERT_EQ(span_ptr->getTraceIdAsHex(), "");
span_ptr->injectContext(request_headers);
span_ptr->log(SystemTime(), "fake_event");

EXPECT_NE(nullptr, span_ptr->spawnChild(config, "foo", SystemTime()));
}
Expand Down
19 changes: 17 additions & 2 deletions test/extensions/filters/http/jwt_authn/filter_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ namespace JwtAuthn {
namespace {

const char HeaderToFilterStateFilterName[] = "envoy.filters.http.header_to_filter_state_for_test";

// This filter extracts a string header from "header" and
// save it into FilterState as name "state" as read-only Router::StringAccessor.
class HeaderToFilterStateFilter : public Http::PassThroughDecoderFilter {
Expand Down Expand Up @@ -140,6 +139,10 @@ TEST_P(LocalJwksIntegrationTest, ExpiredToken) {
response->waitForEndStream();
ASSERT_TRUE(response->complete());
EXPECT_EQ("401", response->headers().getStatusValue());
EXPECT_EQ(1, response->headers().get(Http::Headers::get().WWWAuthenticate).size());
EXPECT_EQ(
"Bearer realm=\"http://host/\", error=\"invalid_token\"",
response->headers().get(Http::Headers::get().WWWAuthenticate)[0]->value().getStringView());
}

TEST_P(LocalJwksIntegrationTest, MissingToken) {
Expand All @@ -158,6 +161,9 @@ TEST_P(LocalJwksIntegrationTest, MissingToken) {
response->waitForEndStream();
ASSERT_TRUE(response->complete());
EXPECT_EQ("401", response->headers().getStatusValue());
EXPECT_EQ(
"Bearer realm=\"http://host/\"",
response->headers().get(Http::Headers::get().WWWAuthenticate)[0]->value().getStringView());
}

TEST_P(LocalJwksIntegrationTest, ExpiredTokenHeadReply) {
Expand All @@ -177,6 +183,10 @@ TEST_P(LocalJwksIntegrationTest, ExpiredTokenHeadReply) {
response->waitForEndStream();
ASSERT_TRUE(response->complete());
EXPECT_EQ("401", response->headers().getStatusValue());
EXPECT_EQ(
"Bearer realm=\"http://host/\", error=\"invalid_token\"",
response->headers().get(Http::Headers::get().WWWAuthenticate)[0]->value().getStringView());

EXPECT_NE("0", response->headers().getContentLengthValue());
EXPECT_THAT(response->body(), ::testing::IsEmpty());
}
Expand Down Expand Up @@ -424,6 +434,9 @@ TEST_P(RemoteJwksIntegrationTest, FetchFailedJwks) {
response->waitForEndStream();
ASSERT_TRUE(response->complete());
EXPECT_EQ("401", response->headers().getStatusValue());
EXPECT_EQ(
"Bearer realm=\"http://host/\", error=\"invalid_token\"",
response->headers().get(Http::Headers::get().WWWAuthenticate)[0]->value().getStringView());

cleanup();
}
Expand All @@ -444,7 +457,9 @@ TEST_P(RemoteJwksIntegrationTest, FetchFailedMissingCluster) {
response->waitForEndStream();
ASSERT_TRUE(response->complete());
EXPECT_EQ("401", response->headers().getStatusValue());

EXPECT_EQ(
"Bearer realm=\"http://host/\", error=\"invalid_token\"",
response->headers().get(Http::Headers::get().WWWAuthenticate)[0]->value().getStringView());
cleanup();
}

Expand Down

0 comments on commit d53d856

Please sign in to comment.