diff --git a/api/envoy/config/core/v3/base.proto b/api/envoy/config/core/v3/base.proto index dcfc660dd0287..efa8ec5186f46 100644 --- a/api/envoy/config/core/v3/base.proto +++ b/api/envoy/config/core/v3/base.proto @@ -296,6 +296,15 @@ message RuntimeFeatureFlag { string runtime_key = 2 [(validate.rules).string = {min_len: 1}]; } +// Query parameter name/value pair. +message QueryParameter { + // The key of the query parameter. Case sensitive. + string key = 1 [(validate.rules).string = {min_len: 1}]; + + // The value of the query parameter. + string value = 2; +} + // Header name/value pair. message HeaderValue { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.HeaderValue"; diff --git a/api/envoy/service/auth/v3/external_auth.proto b/api/envoy/service/auth/v3/external_auth.proto index 6e97216ad43c4..11fc057da888a 100644 --- a/api/envoy/service/auth/v3/external_auth.proto +++ b/api/envoy/service/auth/v3/external_auth.proto @@ -60,7 +60,7 @@ message DeniedHttpResponse { } // HTTP attributes for an OK response. -// [#next-free-field: 7] +// [#next-free-field: 9] message OkHttpResponse { option (udpa.annotations.versioning).previous_message_type = "envoy.service.auth.v2.OkHttpResponse"; @@ -102,6 +102,15 @@ message OkHttpResponse { // to the downstream client on success. Note that the :ref:`append field in HeaderValueOption ` // defaults to false when used in this message. repeated config.core.v3.HeaderValueOption response_headers_to_add = 6; + + // This field allows the authorization service to set (and overwrite) query + // string parameters on the original request before it is sent upstream. + repeated config.core.v3.QueryParameter query_parameters_to_set = 7; + + // This field allows the authorization service to specify which query parameters + // should be removed from the original request before it is sent upstream. Each + // element in this list is a case-sensitive query parameter name to be removed. + repeated string query_parameters_to_remove = 8; } // Intended for gRPC and Network Authorization servers `only`. diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index e306c4304abe7..b072a5e19b793 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -24,6 +24,7 @@ Removed Config or Runtime New Features ------------ +* ext_authz: added :ref:`query_parameters_to_set ` and :ref:`query_parameters_to_remove ` for adding and removing query string parameters when using a gRPC authorization server. * http: added support for :ref:`retriable health check status codes `. Deprecated diff --git a/envoy/http/query_params.h b/envoy/http/query_params.h index d30ae58b1ab36..e500f93ca3c86 100644 --- a/envoy/http/query_params.h +++ b/envoy/http/query_params.h @@ -2,6 +2,7 @@ #include #include +#include namespace Envoy { namespace Http { @@ -12,6 +13,7 @@ namespace Utility { // https://github.com/apache/incubator-pagespeed-mod/blob/master/pagespeed/kernel/http/query_params.h using QueryParams = std::map; +using QueryParamsVector = std::vector>; } // namespace Utility } // namespace Http diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index ac822a9515983..6f2a55da1056a 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -463,6 +463,18 @@ std::string Utility::stripQueryString(const HeaderString& path) { query_offset != path_str.npos ? query_offset : path_str.size()); } +std::string Utility::replaceQueryString(const HeaderString& path, + const Utility::QueryParams& params) { + std::string new_path{Http::Utility::stripQueryString(path)}; + + if (!params.empty()) { + const auto new_query_string = Http::Utility::queryParamsToString(params); + absl::StrAppend(&new_path, new_query_string); + } + + return new_path; +} + std::string Utility::parseCookieValue(const HeaderMap& headers, const std::string& key) { // TODO(wbpcode): Modify the headers parameter type to 'RequestHeaderMap'. return parseCookie(headers, key, Http::Headers::get().Cookie); diff --git a/source/common/http/utility.h b/source/common/http/utility.h index 5038ebce1e693..a13922b7aee91 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -248,6 +248,20 @@ absl::string_view findQueryStringStart(const HeaderString& path); */ std::string stripQueryString(const HeaderString& path); +/** + * Replace the query string portion of a given path with a new one. + * + * e.g. replaceQueryString("/foo?key=1", {key:2}) -> "/foo?key=2" + * replaceQueryString("/bar", {hello:there}) -> "/bar?hello=there" + * + * @param path the original path that may or may not contain an existing query string + * @param params the new params whose string representation should be formatted onto + * the `path` above + * @return std::string the new path whose query string has been replaced by `params` and whose path + * portion from `path` remains unchanged. + */ +std::string replaceQueryString(const HeaderString& path, const QueryParams& params); + /** * Parse a particular value out of a cookie * @param headers supplies the headers to get the cookie from. diff --git a/source/extensions/filters/common/ext_authz/ext_authz.h b/source/extensions/filters/common/ext_authz/ext_authz.h index 96545dd83a959..861faac9b1bdc 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz.h +++ b/source/extensions/filters/common/ext_authz/ext_authz.h @@ -12,6 +12,7 @@ #include "envoy/tracing/http_tracer.h" #include "source/common/http/headers.h" +#include "source/common/http/utility.h" #include "source/common/singleton/const_singleton.h" namespace Envoy { @@ -84,6 +85,11 @@ struct Response { // A set of HTTP headers consumed by the authorization server, will be removed // from the request to the upstream server. std::vector headers_to_remove; + // A set of query string parameters to be set (possibly overwritten) on the + // request to the upstream server. + Http::Utility::QueryParamsVector query_parameters_to_set; + // A set of query string parameters to remove from the request to the upstream server. + std::vector query_parameters_to_remove; // Optional http body used only on denied response. std::string body; // Optional http status used only on denied response. diff --git a/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc b/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc index dc730cdcb6dc8..eda59d1515008 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc +++ b/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc @@ -57,7 +57,17 @@ void GrpcClientImpl::onSuccess(std::unique_ptrheaders_to_remove.push_back(Http::LowerCaseString(header)); } } - + if (response->ok_response().query_parameters_to_set_size() > 0) { + for (const auto& query_parameter : response->ok_response().query_parameters_to_set()) { + authz_response->query_parameters_to_set.push_back( + std::pair(query_parameter.key(), query_parameter.value())); + } + } + if (response->ok_response().query_parameters_to_remove_size() > 0) { + for (const auto& key : response->ok_response().query_parameters_to_remove()) { + authz_response->query_parameters_to_remove.push_back(key); + } + } // These two vectors hold header overrides of encoded response headers. if (response->ok_response().response_headers_to_add_size() > 0) { for (const auto& header : response->ok_response().response_headers_to_add()) { diff --git a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc index 3cf233d661642..6ea634396894a 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc +++ b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc @@ -38,6 +38,8 @@ const Response& errorResponse() { Http::HeaderVector{}, Http::HeaderVector{}, {{}}, + Http::Utility::QueryParamsVector{}, + {}, EMPTY_STRING, Http::Code::Forbidden, ProtobufWkt::Struct{}}); @@ -350,9 +352,17 @@ ResponsePtr RawHttpClientImpl::toResponse(Http::ResponseMessagePtr message) { config_->upstreamHeaderToAppendMatchers(), config_->clientHeaderOnSuccessMatchers(), config_->dynamicMetadataMatchers(), - Response{CheckStatus::OK, Http::HeaderVector{}, Http::HeaderVector{}, - Http::HeaderVector{}, Http::HeaderVector{}, Http::HeaderVector{}, - std::move(headers_to_remove), EMPTY_STRING, Http::Code::OK, + Response{CheckStatus::OK, + Http::HeaderVector{}, + Http::HeaderVector{}, + Http::HeaderVector{}, + Http::HeaderVector{}, + Http::HeaderVector{}, + std::move(headers_to_remove), + Http::Utility::QueryParamsVector{}, + {}, + EMPTY_STRING, + Http::Code::OK, ProtobufWkt::Struct{}}}; return std::move(ok.response_); } @@ -370,6 +380,8 @@ ResponsePtr RawHttpClientImpl::toResponse(Http::ResponseMessagePtr message) { Http::HeaderVector{}, Http::HeaderVector{}, {{}}, + Http::Utility::QueryParamsVector{}, + {}, message->bodyAsString(), static_cast(status_code), ProtobufWkt::Struct{}}}; diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index 90df75ee279dd..6722acaaaa469 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -222,12 +222,13 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { switch (response->status) { case CheckStatus::OK: { - // Any changes to request headers can affect how the request is going to be + // Any changes to request headers or query parameters can affect how the request is going to be // routed. If we are changing the headers we also need to clear the route // cache. if (config_->clearRouteCache() && (!response->headers_to_set.empty() || !response->headers_to_append.empty() || - !response->headers_to_remove.empty())) { + !response->headers_to_remove.empty() || !response->query_parameters_to_set.empty() || + !response->query_parameters_to_remove.empty())) { ENVOY_STREAM_LOG(debug, "ext_authz is clearing route cache", *decoder_callbacks_); decoder_callbacks_->clearRouteCache(); } @@ -286,6 +287,42 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { response_headers_to_set_ = std::move(response->response_headers_to_set); } + absl::optional modified_query_parameters; + if (!response->query_parameters_to_set.empty()) { + modified_query_parameters = + Http::Utility::parseQueryString(request_headers_->Path()->value().getStringView()); + ENVOY_STREAM_LOG( + trace, "ext_authz filter set query parameter(s) on the request:", *decoder_callbacks_); + for (const auto& [key, value] : response->query_parameters_to_set) { + ENVOY_STREAM_LOG(trace, "'{}={}'", *decoder_callbacks_, key, value); + (*modified_query_parameters)[key] = value; + } + } + + if (!response->query_parameters_to_remove.empty()) { + if (!modified_query_parameters) { + modified_query_parameters = + Http::Utility::parseQueryString(request_headers_->Path()->value().getStringView()); + } + ENVOY_STREAM_LOG(trace, "ext_authz filter removed query parameter(s) from the request:", + *decoder_callbacks_); + for (const auto& key : response->query_parameters_to_remove) { + ENVOY_STREAM_LOG(trace, "'{}'", *decoder_callbacks_, key); + (*modified_query_parameters).erase(key); + } + } + + // We modified the query parameters in some way, so regenerate the `path` header and set it + // here. + if (modified_query_parameters) { + const auto new_path = Http::Utility::replaceQueryString(request_headers_->Path()->value(), + modified_query_parameters.value()); + ENVOY_STREAM_LOG( + trace, "ext_authz filter modified query parameter(s), using new path for request: {}", + *decoder_callbacks_, new_path); + request_headers_->setPath(new_path); + } + if (cluster_) { config_->incCounter(cluster_->statsScope(), config_->ext_authz_ok_); } diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index ae7073574d57f..e602f2108df0c 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -91,6 +91,69 @@ TEST(HttpUtility, parseQueryString) { "bucket%7Crq_xx%7Crq_complete%7Crq_active%7Ccx_active%29%29%7C%28server.version%29")); } +TEST(HttpUtility, stripQueryString) { + EXPECT_EQ(Utility::stripQueryString(HeaderString("/")), "/"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/?")), "/"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/?x=1")), "/"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/?x=1&y=2")), "/"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo")), "/foo"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo?")), "/foo"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo?hello=there")), "/foo"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo?hello=there&good=bye")), "/foo"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo/?")), "/foo/"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo/?x=1")), "/foo/"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo/bar")), "/foo/bar"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo/bar?")), "/foo/bar"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo/bar?a=b")), "/foo/bar"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo/bar?a=b&b=c")), "/foo/bar"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo/bar/")), "/foo/bar/"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo/bar/?")), "/foo/bar/"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo/bar/?x=1")), "/foo/bar/"); + EXPECT_EQ(Utility::stripQueryString(HeaderString("/foo/bar/?x=1&y=2")), "/foo/bar/"); +} + +TEST(HttpUtility, replaceQueryString) { + // Replace with nothing + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/"), Utility::QueryParams()), "/"); + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/?"), Utility::QueryParams()), "/"); + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/?x=0"), Utility::QueryParams()), "/"); + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/a"), Utility::QueryParams()), "/a"); + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/a/"), Utility::QueryParams()), "/a/"); + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/a/?y=5"), Utility::QueryParams()), "/a/"); + // Replace with x=1 + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/"), Utility::QueryParams({{"x", "1"}})), + "/?x=1"); + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/?"), Utility::QueryParams({{"x", "1"}})), + "/?x=1"); + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/?x=0"), Utility::QueryParams({{"x", "1"}})), + "/?x=1"); + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/a?x=0"), Utility::QueryParams({{"x", "1"}})), + "/a?x=1"); + EXPECT_EQ( + Utility::replaceQueryString(HeaderString("/a/?x=0"), Utility::QueryParams({{"x", "1"}})), + "/a/?x=1"); + // More replacements + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/foo"), + Utility::QueryParams({{"x", "1"}, {"z", "3"}})), + "/foo?x=1&z=3"); + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/foo?z=2"), + Utility::QueryParams({{"x", "1"}, {"y", "5"}})), + "/foo?x=1&y=5"); + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/foo?y=9"), + Utility::QueryParams({{"x", "1"}, {"y", "5"}})), + "/foo?x=1&y=5"); + // More path components + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/foo/bar?"), + Utility::QueryParams({{"x", "1"}, {"y", "5"}})), + "/foo/bar?x=1&y=5"); + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/foo/bar?y=9&a=b"), + Utility::QueryParams({{"x", "1"}, {"y", "5"}})), + "/foo/bar?x=1&y=5"); + EXPECT_EQ(Utility::replaceQueryString(HeaderString("/foo/bar?y=11&z=7"), + Utility::QueryParams({{"a", "b"}, {"x", "1"}, {"y", "5"}})), + "/foo/bar?a=b&x=1&y=5"); +} + TEST(HttpUtility, getResponseStatus) { EXPECT_THROW(Utility::getResponseStatus(TestResponseHeaderMapImpl{}), CodecClientException); EXPECT_EQ(200U, Utility::getResponseStatus(TestResponseHeaderMapImpl{{":status", "200"}})); diff --git a/test/extensions/filters/common/ext_authz/ext_authz_grpc_impl_test.cc b/test/extensions/filters/common/ext_authz/ext_authz_grpc_impl_test.cc index ca45b70ca26b3..57a9fd6906753 100644 --- a/test/extensions/filters/common/ext_authz/ext_authz_grpc_impl_test.cc +++ b/test/extensions/filters/common/ext_authz/ext_authz_grpc_impl_test.cc @@ -315,6 +315,46 @@ TEST_F(ExtAuthzGrpcClientTest, AuthorizationOkWithDynamicMetadata) { client_->onSuccess(std::move(check_response), span_); } +// Test the client when an OK response is received with additional query string parameters. +TEST_F(ExtAuthzGrpcClientTest, AuthorizationOkWithQueryParameters) { + initialize(); + + auto check_response = std::make_unique(); + auto status = check_response->mutable_status(); + + status->set_code(Grpc::Status::WellKnownGrpcStatus::Ok); + + const Http::Utility::QueryParamsVector query_parameters_to_set{{"add-me", "yes"}}; + for (const auto& [key, value] : query_parameters_to_set) { + auto* query_parameter = check_response->mutable_ok_response()->add_query_parameters_to_set(); + query_parameter->set_key(key); + query_parameter->set_value(value); + } + + const std::vector query_parameters_to_remove{"remove-me"}; + for (const auto& key : query_parameters_to_remove) { + check_response->mutable_ok_response()->add_query_parameters_to_remove(key); + } + + // This is the expected authz response. + auto authz_response = Response{}; + authz_response.status = CheckStatus::OK; + authz_response.query_parameters_to_set = {{"add-me", "yes"}}; + authz_response.query_parameters_to_remove = {"remove-me"}; + + envoy::service::auth::v3::CheckRequest request; + expectCallSend(request); + client_->check(request_callbacks_, request, Tracing::NullSpan::instance(), stream_info_); + + Http::TestRequestHeaderMapImpl headers; + client_->onCreateInitialMetadata(headers); + + EXPECT_CALL(span_, setTag(Eq("ext_authz_status"), Eq("ext_authz_ok"))); + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzOkResponse(authz_response)))); + client_->onSuccess(std::move(check_response), span_); +} + } // namespace ExtAuthz } // namespace Common } // namespace Filters diff --git a/test/extensions/filters/common/ext_authz/test_common.cc b/test/extensions/filters/common/ext_authz/test_common.cc index 66be0140ea7c0..cdb7867a401a0 100644 --- a/test/extensions/filters/common/ext_authz/test_common.cc +++ b/test/extensions/filters/common/ext_authz/test_common.cc @@ -130,6 +130,19 @@ bool TestCommon::compareVectorOfHeaderName(const std::vector(rhs.begin(), rhs.end()); } +bool TestCommon::compareVectorOfUnorderedStrings(const std::vector& lhs, + const std::vector& rhs) { + return std::set(lhs.begin(), lhs.end()) == + std::set(rhs.begin(), rhs.end()); +} + +// TODO(esmet): This belongs in a QueryParams class +bool TestCommon::compareQueryParamsVector(const Http::Utility::QueryParamsVector& lhs, + const Http::Utility::QueryParamsVector& rhs) { + return std::set>(lhs.begin(), lhs.end()) == + std::set>(rhs.begin(), rhs.end()); +} + } // namespace ExtAuthz } // namespace Common } // namespace Filters diff --git a/test/extensions/filters/common/ext_authz/test_common.h b/test/extensions/filters/common/ext_authz/test_common.h index 5d1e72222713d..0b058b67d42d1 100644 --- a/test/extensions/filters/common/ext_authz/test_common.h +++ b/test/extensions/filters/common/ext_authz/test_common.h @@ -46,8 +46,12 @@ class TestCommon { static HeaderValueOptionVector makeHeaderValueOption(KeyValueOptionVector&& headers); static bool compareHeaderVector(const Http::HeaderVector& lhs, const Http::HeaderVector& rhs); + static bool compareQueryParamsVector(const Http::Utility::QueryParamsVector& lhs, + const Http::Utility::QueryParamsVector& rhs); static bool compareVectorOfHeaderName(const std::vector& lhs, const std::vector& rhs); + static bool compareVectorOfUnorderedStrings(const std::vector& lhs, + const std::vector& rhs); }; MATCHER_P(AuthzErrorResponse, status, "") { @@ -111,6 +115,18 @@ MATCHER_P(AuthzOkResponse, response, "") { return false; } + // Compare query_parameters_to_set. + if (!TestCommon::compareQueryParamsVector(response.query_parameters_to_set, + arg->query_parameters_to_set)) { + return false; + } + + // Compare query_parameters_to_remove. + if (!TestCommon::compareVectorOfUnorderedStrings(response.query_parameters_to_remove, + arg->query_parameters_to_remove)) { + return false; + } + return TestCommon::compareVectorOfHeaderName(response.headers_to_remove, arg->headers_to_remove); } diff --git a/test/extensions/filters/http/ext_authz/ext_authz_test.cc b/test/extensions/filters/http/ext_authz/ext_authz_test.cc index bd8affbc05893..8f3033bd225a9 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc +++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc @@ -70,6 +70,49 @@ template class HttpFilterTestBase : public T { connection_.stream_info_.downstream_connection_info_provider_->setLocalAddress(addr_); } + void queryParameterTest(const std::string& original_path, const std::string& expected_path, + const Http::Utility::QueryParamsVector& add_me, + const std::vector& remove_me) { + InSequence s; + + // Set up all the typical headers plus a path with a query string that we'll remove later. + request_headers_.addCopy(Http::Headers::get().Host, "example.com"); + request_headers_.addCopy(Http::Headers::get().Method, "GET"); + request_headers_.addCopy(Http::Headers::get().Path, original_path); + request_headers_.addCopy(Http::Headers::get().Scheme, "https"); + + prepareCheck(); + + Filters::Common::ExtAuthz::Response response{}; + response.status = Filters::Common::ExtAuthz::CheckStatus::OK; + response.query_parameters_to_set = add_me; + response.query_parameters_to_remove = remove_me; + + auto response_ptr = std::make_unique(response); + + EXPECT_CALL(*client_, check(_, _, _, _)) + .WillOnce(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks, + const envoy::service::auth::v3::CheckRequest&, Tracing::Span&, + const StreamInfo::StreamInfo&) -> void { + callbacks.onComplete(std::move(response_ptr)); + })); + EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers_)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers_)); + EXPECT_EQ(request_headers_.getPathValue(), expected_path); + + Buffer::OwnedImpl response_data{}; + Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}}; + Http::TestResponseTrailerMapImpl response_trailers{}; + Http::MetadataMap response_metadata{}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(response_headers, false)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(response_trailers)); + EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->encodeMetadata(response_metadata)); + } + NiceMock stats_store_; envoy::config::bootstrap::v3::Bootstrap bootstrap_; FilterConfigSharedPtr config_; @@ -1789,6 +1832,54 @@ TEST_P(HttpFilterTestParam, ImmediateOkResponseWithHttpAttributes) { EXPECT_EQ(response_headers.get_("should-be-overridden"), "finally-set-by-auth-server"); } +TEST_P(HttpFilterTestParam, ImmediateOkResponseWithUnmodifiedQueryParameters) { + const std::string original_path{"/users?leave-me=alone"}; + const std::string expected_path{"/users?leave-me=alone"}; + const Http::Utility::QueryParamsVector add_me{}; + const std::vector remove_me{"remove-me"}; + queryParameterTest(original_path, expected_path, add_me, remove_me); +} + +TEST_P(HttpFilterTestParam, ImmediateOkResponseWithAddedQueryParameters) { + const std::string original_path{"/users"}; + const std::string expected_path{"/users?add-me=123"}; + const Http::Utility::QueryParamsVector add_me{{"add-me", "123"}}; + const std::vector remove_me{}; + queryParameterTest(original_path, expected_path, add_me, remove_me); +} + +TEST_P(HttpFilterTestParam, ImmediateOkResponseWithAddedAndRemovedQueryParameters) { + const std::string original_path{"/users?remove-me=123"}; + const std::string expected_path{"/users?add-me=456"}; + const Http::Utility::QueryParamsVector add_me{{"add-me", "456"}}; + const std::vector remove_me{{"remove-me"}}; + queryParameterTest(original_path, expected_path, add_me, remove_me); +} + +TEST_P(HttpFilterTestParam, ImmediateOkResponseWithRemovedQueryParameters) { + const std::string original_path{"/users?remove-me=definitely"}; + const std::string expected_path{"/users"}; + const Http::Utility::QueryParamsVector add_me{}; + const std::vector remove_me{{"remove-me"}}; + queryParameterTest(original_path, expected_path, add_me, remove_me); +} + +TEST_P(HttpFilterTestParam, ImmediateOkResponseWithOverwrittenQueryParameters) { + const std::string original_path{"/users?overwrite-me=original"}; + const std::string expected_path{"/users?overwrite-me=new"}; + const Http::Utility::QueryParamsVector add_me{{"overwrite-me", "new"}}; + const std::vector remove_me{}; + queryParameterTest(original_path, expected_path, add_me, remove_me); +} + +TEST_P(HttpFilterTestParam, ImmediateOkResponseWithManyModifiedQueryParameters) { + const std::string original_path{"/users?remove-me=1&overwrite-me=2&leave-me=3"}; + const std::string expected_path{"/users?add-me=9&leave-me=3&overwrite-me=new"}; + const Http::Utility::QueryParamsVector add_me{{"add-me", "9"}, {"overwrite-me", "new"}}; + const std::vector remove_me{{"remove-me"}}; + queryParameterTest(original_path, expected_path, add_me, remove_me); +} + // Test that an synchronous denied response from the authorization service, on the call stack, // results in request not continuing. TEST_P(HttpFilterTestParam, ImmediateDeniedResponse) {