diff --git a/.bazelrc b/.bazelrc index 14db9a58c5a1..d7294707b7f0 100644 --- a/.bazelrc +++ b/.bazelrc @@ -54,9 +54,8 @@ build:docs-ci --action_env=DOCS_RST_CHECK=1 --host_action_env=DOCS_RST_CHECK=1 build --incompatible_config_setting_private_default_visibility build --incompatible_enforce_config_setting_visibility -test --bes_upload_mode=nowait_for_upload_complete -test --bes_timeout=30s test --test_verbose_timeout_warnings +test --experimental_ui_max_stdouterr_bytes=3048576 #default 1048576 # Allow tags to influence execution requirements common --experimental_allow_tags_propagation @@ -511,6 +510,7 @@ build:rbe-google --config=cache-google build:rbe-google-bes --bes_backend=grpcs://buildeventservice.googleapis.com build:rbe-google-bes --bes_results_url=https://source.cloud.google.com/results/invocations/ +build:rbe-google-bes --bes_upload_mode=fully_async # RBE (Engflow mobile) build:rbe-engflow --google_default_credentials=false diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 622f9c4c0e2a..b827bb1d6d27 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -174,6 +174,11 @@ bug_fixes: change: | Added one option to disable the response body buffering for mirror request. Also introduced a 32MB cap for the response buffer, which can be changed by the runtime flag ``http.async_response_buffer_limit`` based on the product needs. +- area: ext_authz + change: | + Validate http service path_prefix + :ref:`path_prefix `, + Validate http service path_prefix configuration must start with ``/``. removed_config_or_runtime: # *Normally occurs at the end of the* :ref:`deprecation period ` @@ -204,9 +209,15 @@ removed_config_or_runtime: - area: http change: | Removed ``envoy.reloadable_features.http_allow_partial_urls_in_referer`` runtime flag and legacy code paths. +- area: oauth + change: | + Removed ``envoy.reloadable_features.oauth_make_token_cookie_httponly`` runtime flag and legacy code paths. - area: http change: | Removed ``envoy.reloadable_features.lowercase_scheme`` runtime flag and legacy code paths. +- area: oauth + change: | + Removed ``envoy.reloadable_features.hmac_base64_encoding_only`` runtime flag and legacy code paths. - area: upstream change: | Removed ``envoy.reloadable_features.convert_legacy_lb_config`` runtime flag and legacy code paths. diff --git a/contrib/generic_proxy/filters/network/source/codecs/dubbo/config.h b/contrib/generic_proxy/filters/network/source/codecs/dubbo/config.h index c9b4da229373..379784363c15 100644 --- a/contrib/generic_proxy/filters/network/source/codecs/dubbo/config.h +++ b/contrib/generic_proxy/filters/network/source/codecs/dubbo/config.h @@ -28,6 +28,16 @@ class DubboRequest : public Request { ASSERT(inner_metadata_ != nullptr); ASSERT(inner_metadata_->hasContext()); ASSERT(inner_metadata_->hasRequest()); + + uint32_t frame_flags = FrameFlags::FLAG_END_STREAM; // Dubbo message only has one frame. + if (!inner_metadata_->context().isTwoWay()) { + frame_flags |= FrameFlags::FLAG_ONE_WAY; + } + if (inner_metadata_->context().heartbeat()) { + frame_flags |= FrameFlags::FLAG_HEARTBEAT; + } + + stream_frame_flags_ = {static_cast(inner_metadata_->requestId()), frame_flags}; } // Request @@ -43,9 +53,10 @@ class DubboRequest : public Request { // StreamFrame FrameFlags frameFlags() const override { return stream_frame_flags_; } - FrameFlags stream_frame_flags_; - Common::Dubbo::MessageMetadataSharedPtr inner_metadata_; + +private: + FrameFlags stream_frame_flags_; }; class DubboResponse : public Response { @@ -56,6 +67,16 @@ class DubboResponse : public Response { ASSERT(inner_metadata_->hasContext()); ASSERT(inner_metadata_->hasResponse()); refreshStatus(); + + uint32_t frame_flags = FrameFlags::FLAG_END_STREAM; // Dubbo message only has one frame. + if (!inner_metadata_->context().isTwoWay()) { + frame_flags |= FrameFlags::FLAG_ONE_WAY; + } + if (inner_metadata_->context().heartbeat()) { + frame_flags |= FrameFlags::FLAG_HEARTBEAT; + } + + stream_frame_flags_ = {static_cast(inner_metadata_->requestId()), frame_flags}; } void refreshStatus(); @@ -67,10 +88,11 @@ class DubboResponse : public Response { // StreamFrame FrameFlags frameFlags() const override { return stream_frame_flags_; } - FrameFlags stream_frame_flags_; - StreamStatus status_; Common::Dubbo::MessageMetadataSharedPtr inner_metadata_; + +private: + FrameFlags stream_frame_flags_; }; class DubboCodecBase : public Logger::Loggable { @@ -136,12 +158,9 @@ class DubboDecoderBase : public DubboCodecBase, public CodecType { return Common::Dubbo::DecodeStatus::Success; } - auto message = std::make_unique(metadata_); - message->stream_frame_flags_ = {{static_cast(metadata_->requestId()), - !metadata_->context().isTwoWay(), false, - metadata_->context().heartbeat()}, - true}; + auto message = std::make_unique(std::move(metadata_)); metadata_.reset(); + callback_->onDecodingSuccess(std::move(message)); return Common::Dubbo::DecodeStatus::Success; diff --git a/contrib/generic_proxy/filters/network/source/codecs/http1/config.h b/contrib/generic_proxy/filters/network/source/codecs/http1/config.h index 253665c62926..f96223bcb35b 100644 --- a/contrib/generic_proxy/filters/network/source/codecs/http1/config.h +++ b/contrib/generic_proxy/filters/network/source/codecs/http1/config.h @@ -60,7 +60,7 @@ class HttpRequestFrame : public HttpHeaderFrame { HttpRequestFrame(Http::RequestHeaderMapPtr request, bool end_stream) : request_(std::move(request)) { ASSERT(request_ != nullptr); - frame_flags_ = {StreamFlags{}, end_stream}; + frame_flags_ = {0, end_stream ? FrameFlags::FLAG_END_STREAM : FrameFlags::FLAG_EMPTY}; } absl::string_view host() const override { return request_->getHostValue(); } @@ -80,7 +80,15 @@ class HttpResponseFrame : public HttpHeaderFrame { const bool drain_close = Envoy::StringUtil::caseFindToken( response_->getConnectionValue(), ",", Http::Headers::get().ConnectionValues.Close); - frame_flags_ = {StreamFlags{0, false, drain_close, false}, end_stream}; + uint32_t flags = 0; + if (end_stream) { + flags |= FrameFlags::FLAG_END_STREAM; + } + if (drain_close) { + flags |= FrameFlags::FLAG_DRAIN_CLOSE; + } + + frame_flags_ = {0, flags}; } StreamStatus status() const override { @@ -101,7 +109,7 @@ class HttpResponseFrame : public HttpHeaderFrame { class HttpRawBodyFrame : public CommonFrame { public: HttpRawBodyFrame(Envoy::Buffer::Instance& buffer, bool end_stream) - : frame_flags_({StreamFlags{}, end_stream}) { + : frame_flags_(0, end_stream ? FrameFlags::FLAG_END_STREAM : FrameFlags::FLAG_EMPTY) { buffer_.move(buffer); } FrameFlags frameFlags() const override { return frame_flags_; } diff --git a/contrib/generic_proxy/filters/network/source/codecs/kafka/config.h b/contrib/generic_proxy/filters/network/source/codecs/kafka/config.h index 1c8e72aa6d76..b4b9f9d65ff6 100644 --- a/contrib/generic_proxy/filters/network/source/codecs/kafka/config.h +++ b/contrib/generic_proxy/filters/network/source/codecs/kafka/config.h @@ -28,8 +28,7 @@ class KafkaRequestFrame : public GenericProxy::StreamRequest { if (request_ == nullptr) { return FrameFlags{}; } - return FrameFlags{ - StreamFlags{static_cast(request_->request_header_.correlation_id_)}}; + return FrameFlags{static_cast(request_->request_header_.correlation_id_)}; } absl::string_view protocol() const override { return "kafka"; } @@ -46,7 +45,7 @@ class KafkaResponseFrame : public GenericProxy::StreamResponse { if (response_ == nullptr) { return FrameFlags{}; } - return FrameFlags{StreamFlags{static_cast(response_->metadata_.correlation_id_)}}; + return FrameFlags{static_cast(response_->metadata_.correlation_id_)}; } absl::string_view protocol() const override { return "kafka"; } diff --git a/contrib/generic_proxy/filters/network/source/interface/stream.h b/contrib/generic_proxy/filters/network/source/interface/stream.h index ccc5e1d169ce..41e66ca7c865 100644 --- a/contrib/generic_proxy/filters/network/source/interface/stream.h +++ b/contrib/generic_proxy/filters/network/source/interface/stream.h @@ -17,92 +17,35 @@ namespace Extensions { namespace NetworkFilters { namespace GenericProxy { -/** - * Stream flags from request or response to control the behavior of the - * generic proxy filter. This is mainly used as part of FrameFlags. - * All these flags could be ignored for the simple ping-pong use case. - */ -class StreamFlags { -public: - StreamFlags(uint64_t stream_id = 0, bool one_way_stream = false, bool drain_close = false, - bool is_heartbeat = false) - : stream_id_(stream_id), one_way_stream_(one_way_stream), drain_close_(drain_close), - is_heartbeat_(is_heartbeat) {} - - /** - * @return the stream id of the request or response. This is used to match the - * downstream request with the upstream response. - - * NOTE: In most cases, the stream id is not needed and will be ignored completely. - * The stream id is only used when we can't match the downstream request - * with the upstream response by the active stream instance self directly. - * For example, when the multiple downstream requests are multiplexed into one - * upstream connection. - */ - uint64_t streamId() const { return stream_id_; } - - /** - * @return whether the stream is one way stream. If request is one way stream, the - * generic proxy filter will not wait for the response from the upstream. - */ - bool oneWayStream() const { return one_way_stream_; } - - /** - * @return whether the downstream/upstream connection should be drained after - * current active stream are finished. - */ - bool drainClose() const { return drain_close_; } - - /** - * @return whether the current request/response is a heartbeat request/response. - * NOTE: It would be better to handle heartbeat request/response by another L4 - * filter. Then the generic proxy filter can be used for the simple ping-pong - * use case. - */ - bool isHeartbeat() const { return is_heartbeat_; } - -private: - uint64_t stream_id_{0}; - - bool one_way_stream_{false}; - bool drain_close_{false}; - bool is_heartbeat_{false}; -}; - /** * Flags of stream frame. This is used to control the behavior of the generic proxy filter. * All these flags could be ignored for the simple ping-pong use case. */ class FrameFlags { public: + static constexpr uint32_t FLAG_EMPTY = 0x0000; + static constexpr uint32_t FLAG_END_STREAM = 0x0001; + static constexpr uint32_t FLAG_ONE_WAY = 0x0002; + static constexpr uint32_t FLAG_DRAIN_CLOSE = 0x0004; + static constexpr uint32_t FLAG_HEARTBEAT = 0x0008; + /** * Construct FrameFlags with stream flags and end stream flag. The stream flags MUST be * same for all frames of the same stream. - * @param stream_flags StreamFlags of the stream. - * @param end_stream whether the current frame is the last frame of the request or response. + * @param stream_id the stream id of the request or response. + * @param flags flags of the current frame. Only the flags that defined in FrameFlags + * could be used. Multiple flags could be combined by bitwise OR. * @param frame_tags frame tags of the current frame. The meaning of the frame tags is * application protocol specific. */ - FrameFlags(StreamFlags stream_flags = StreamFlags(), bool end_stream = true, - uint32_t frame_tags = 0) - : stream_flags_(stream_flags), frame_tags_(frame_tags), end_stream_(end_stream) {} + FrameFlags(uint64_t stream_id = 0, uint32_t flags = FLAG_END_STREAM, uint32_t frame_tags = 0) + : stream_id_(stream_id), flags_(flags), frame_tags_(frame_tags) {} /** - * Get flags of stream that the frame belongs to. The flags MUST be same for all frames of the - * same stream. Copy semantics is used because the flags are lightweight (only 16 bytes for now). - * @return StreamFlags of the stream. + * @return the stream id of the request or response. All frames of the same stream + * MUST have the same stream id. */ - StreamFlags streamFlags() const { return stream_flags_; } - - /** - * @return the stream id of the request or response. - */ - uint64_t streamId() const { return stream_flags_.streamId(); } - - /** - * @return whether the current frame is the last frame of the request or response. - */ - bool endStream() const { return end_stream_; } + uint64_t streamId() const { return stream_id_; } /** * @return frame tags of the current frame. The meaning of the frame tags is application @@ -114,12 +57,35 @@ class FrameFlags { */ uint32_t frameTags() const { return frame_tags_; } -private: - StreamFlags stream_flags_{}; + /** + * @return whether the current frame is the last frame of the request or response. + */ + bool endStream() const { return flags_ & FLAG_END_STREAM; } + /** + * @return whether the downstream/upstream connection should be drained after + * current active stream are finished. + * NOTE: Only the response header frame's drainClose() flag will be used. + */ + bool drainClose() const { return flags_ & FLAG_DRAIN_CLOSE; } + + /** + * @return whether the stream is one way stream. If request is one way stream, the + * generic proxy filter will not wait for the response from the upstream. + * NOTE: Only the request header frame's oneWayStream() flag will be used. + */ + bool oneWayStream() const { return flags_ & FLAG_ONE_WAY; } + + /** + * @return whether the current request/response is a heartbeat request/response. + * NOTE: Only the header frame's isHeartbeat() flag will be used. + */ + bool isHeartbeat() const { return flags_ & FLAG_HEARTBEAT; } + +private: + uint64_t stream_id_{}; + uint32_t flags_{}; uint32_t frame_tags_{}; - // Default to true for backward compatibility. - bool end_stream_{true}; }; /** diff --git a/contrib/generic_proxy/filters/network/source/proxy.cc b/contrib/generic_proxy/filters/network/source/proxy.cc index c898f9df5b46..310058f54aaa 100644 --- a/contrib/generic_proxy/filters/network/source/proxy.cc +++ b/contrib/generic_proxy/filters/network/source/proxy.cc @@ -144,11 +144,27 @@ bool ActiveStream::sendFrameToDownstream(const StreamFrame& frame, bool header_f return true; } -// TODO(wbpcode): add the short_response_flags support to the sendLocalReply -// method. void ActiveStream::sendLocalReply(Status status, absl::string_view data, ResponseUpdateFunction func) { - // Clear possible response frames. + // To ensure only one response header frame is sent for local reply. + // If there is a previsous response header but has not been sent to the downstream, + // we should send the latest local reply to the downstream directly without any + // filter processing. + // If the previous response header has been sent to the downstream, it is an invalid + // situation that we cannot handle. Then, we should reset the stream and return. + // In other cases, we should continue the filter chain to process the local reply. + + const bool has_prev_response = response_header_frame_ != nullptr; + const bool has_sent_response = + has_prev_response && encoder_filter_iter_header_ == encoder_filters_.end(); + + if (has_sent_response) { + ENVOY_LOG(error, "Generic proxy: invalid local reply: response is already sent."); + resetStream(DownstreamStreamResetReason::ProtocolError); + return; + } + + // Clear possible response frames anyway now. response_header_frame_ = nullptr; response_common_frames_.clear(); @@ -167,14 +183,20 @@ void ActiveStream::sendLocalReply(Status status, absl::string_view data, } response_header_frame_ = std::move(response); + parent_.stream_drain_decision_ |= response_header_frame_->frameFlags().drainClose(); local_reply_ = true; // status message will be used as response code details. stream_info_.setResponseCodeDetails(status.message()); // Set the response code to the stream info. stream_info_.setResponseCode(response_header_frame_->status().code()); - // Send the response header frame to downstream. - sendFrameToDownstream(*response_header_frame_, true); + if (has_prev_response) { + // There was a previous response. Send the new response to the downstream directly. + sendFrameToDownstream(*response_header_frame_, true); + return; + } else { + continueEncoding(); + } } void ActiveStream::processRequestHeaderFrame() { @@ -277,7 +299,7 @@ void ActiveStream::processResponseHeaderFrame() { response_header_frame_->frameFlags().endStream()); // Send the header frame to downstream. - if (!sendFrameToDownstream(*response_header_frame_, false)) { + if (!sendFrameToDownstream(*response_header_frame_, true)) { stop_encoder_filter_chain_ = true; } }; @@ -465,15 +487,25 @@ void ActiveStream::onRequestCommonFrame(RequestCommonFramePtr request_common_fra } void ActiveStream::onResponseHeaderFrame(ResponsePtr response) { - ASSERT(response_header_frame_ == nullptr); + // To ensure only one response header frame is handled for upstream response. + // If there is a previous response header frame, no matter it is come from + // the upstream or the local reply, it is an invalid situation that we cannot + // handle. Then, we should reset the stream and return. + const bool has_prev_response = response_header_frame_ != nullptr; + if (has_prev_response) { + ENVOY_LOG(error, "Generic proxy: invalid response: has previous response."); + resetStream(DownstreamStreamResetReason::ProtocolError); + return; + } + response_header_frame_ = std::move(response); - ASSERT(response_header_frame_ != nullptr); - parent_.stream_drain_decision_ = response_header_frame_->frameFlags().streamFlags().drainClose(); + parent_.stream_drain_decision_ |= response_header_frame_->frameFlags().drainClose(); // The response code details always be "via_upstream" for response from upstream. stream_info_.setResponseCodeDetails("via_upstream"); // Set the response code to the stream info. stream_info_.setResponseCode(response_header_frame_->status().code()); + continueEncoding(); } diff --git a/contrib/generic_proxy/filters/network/source/router/router.cc b/contrib/generic_proxy/filters/network/source/router/router.cc index 52f9d56e161f..8ebf60642c46 100644 --- a/contrib/generic_proxy/filters/network/source/router/router.cc +++ b/contrib/generic_proxy/filters/network/source/router/router.cc @@ -40,12 +40,13 @@ ReasonViewAndFlag resetReasonToViewAndFlag(StreamResetReason reason) { } // namespace -UpstreamRequest::UpstreamRequest(RouterFilter& parent, StreamFlags stream_flags, +UpstreamRequest::UpstreamRequest(RouterFilter& parent, FrameFlags header_frame_flags, GenericUpstreamSharedPtr generic_upstream) : parent_(parent), generic_upstream_(std::move(generic_upstream)), stream_info_(parent.time_source_, nullptr, StreamInfo::FilterState::LifeSpan::FilterChain), upstream_info_(std::make_shared()), - stream_id_(stream_flags.streamId()), expects_response_(!stream_flags.oneWayStream()) { + stream_id_(header_frame_flags.streamId()), + expects_response_(!header_frame_flags.oneWayStream()) { // Host is known at this point and set the initial upstream host. onUpstreamHostSelected(generic_upstream_->upstreamHost()); @@ -244,7 +245,7 @@ void UpstreamRequest::onDecodingSuccess(ResponseHeaderFramePtr response_header_f } if (response_header_frame->frameFlags().endStream()) { - onUpstreamResponseComplete(response_header_frame->frameFlags().streamFlags().drainClose()); + onUpstreamResponseComplete(response_header_frame->frameFlags().drainClose()); } parent_.onResponseHeaderFrame(std::move(response_header_frame)); @@ -258,7 +259,7 @@ void UpstreamRequest::onDecodingSuccess(ResponseCommonFramePtr response_common_f } if (response_common_frame->frameFlags().endStream()) { - onUpstreamResponseComplete(response_common_frame->frameFlags().streamFlags().drainClose()); + onUpstreamResponseComplete(response_common_frame->frameFlags().drainClose()); } parent_.onResponseCommonFrame(std::move(response_common_frame)); @@ -448,8 +449,8 @@ void RouterFilter::kickOffNewUpstreamRequest() { return; } - auto upstream_request = std::make_unique( - *this, request_stream_->frameFlags().streamFlags(), std::move(generic_upstream)); + auto upstream_request = std::make_unique(*this, request_stream_->frameFlags(), + std::move(generic_upstream)); auto raw_upstream_request = upstream_request.get(); LinkedList::moveIntoList(std::move(upstream_request), upstream_requests_); raw_upstream_request->startStream(); diff --git a/contrib/generic_proxy/filters/network/source/router/router.h b/contrib/generic_proxy/filters/network/source/router/router.h index fcc71457d6c5..a2effd2c1db6 100644 --- a/contrib/generic_proxy/filters/network/source/router/router.h +++ b/contrib/generic_proxy/filters/network/source/router/router.h @@ -48,7 +48,7 @@ class UpstreamRequest : public UpstreamRequestCallbacks, public EncodingContext, Logger::Loggable { public: - UpstreamRequest(RouterFilter& parent, StreamFlags stream_flags, + UpstreamRequest(RouterFilter& parent, FrameFlags header_frame_flags, GenericUpstreamSharedPtr generic_upstream); void startStream(); diff --git a/contrib/generic_proxy/filters/network/test/codecs/http1/config_test.cc b/contrib/generic_proxy/filters/network/test/codecs/http1/config_test.cc index a9e2dcbdd784..5441bd19ddfe 100644 --- a/contrib/generic_proxy/filters/network/test/codecs/http1/config_test.cc +++ b/contrib/generic_proxy/filters/network/test/codecs/http1/config_test.cc @@ -132,14 +132,14 @@ TEST(Http1MessageFrameTest, Http1MessageFrameTest) { HttpResponseFrame frame(std::move(headers), true); EXPECT_EQ(true, frame.frameFlags().endStream()); - EXPECT_EQ(false, frame.frameFlags().streamFlags().drainClose()); + EXPECT_EQ(false, frame.frameFlags().drainClose()); auto headers2 = Http::ResponseHeaderMapImpl::create(); headers2->setConnection("close"); HttpResponseFrame frame2(std::move(headers2), false); EXPECT_EQ(false, frame2.frameFlags().endStream()); - EXPECT_EQ(true, frame2.frameFlags().streamFlags().drainClose()); + EXPECT_EQ(true, frame2.frameFlags().drainClose()); } // Request FrameType. diff --git a/contrib/generic_proxy/filters/network/test/fake_codec.h b/contrib/generic_proxy/filters/network/test/fake_codec.h index e82055f5e6c0..b03e1a61f476 100644 --- a/contrib/generic_proxy/filters/network/test/fake_codec.h +++ b/contrib/generic_proxy/filters/network/test/fake_codec.h @@ -115,8 +115,15 @@ class FakeStreamCodecFactory : public CodecFactory { data.erase("one_way"); } - auto frame_flags = - FrameFlags(StreamFlags(stream_id.value_or(0), one_way_stream, false, false), end_stream); + uint32_t flags = 0; + if (end_stream) { + flags |= FrameFlags::FLAG_END_STREAM; + } + if (one_way_stream) { + flags |= FrameFlags::FLAG_ONE_WAY; + } + + FrameFlags frame_flags(stream_id.value_or(0), flags); const auto it = data.find("message_type"); @@ -208,8 +215,7 @@ class FakeStreamCodecFactory : public CodecFactory { buffer += fmt::format("stream_id:{};", response.frameFlags().streamId()); buffer += fmt::format("end_stream:{};", response.frameFlags().endStream()); - buffer += - fmt::format("close_connection:{};", response.frameFlags().streamFlags().drainClose()); + buffer += fmt::format("close_connection:{};", response.frameFlags().drainClose()); ENVOY_LOG(debug, "FakeServerCodec::encode: {}", buffer); @@ -274,8 +280,15 @@ class FakeStreamCodecFactory : public CodecFactory { data.erase("close_connection"); } - auto frame_flags = FrameFlags( - StreamFlags(stream_id.value_or(0), false, close_connection, false), end_stream); + uint32_t flags = 0; + if (end_stream) { + flags |= FrameFlags::FLAG_END_STREAM; + } + if (close_connection) { + flags |= FrameFlags::FLAG_DRAIN_CLOSE; + } + + FrameFlags frame_flags(stream_id.value_or(0), flags); const auto it = data.find("message_type"); @@ -371,7 +384,7 @@ class FakeStreamCodecFactory : public CodecFactory { buffer += fmt::format("stream_id:{};", request.frameFlags().streamId()); buffer += fmt::format("end_stream:{};", request.frameFlags().endStream()); - buffer += fmt::format("one_way:{};", request.frameFlags().streamFlags().oneWayStream()); + buffer += fmt::format("one_way:{};", request.frameFlags().oneWayStream()); ENVOY_LOG(debug, "FakeClientCodec::encode: {}", buffer); diff --git a/contrib/generic_proxy/filters/network/test/integration_test.cc b/contrib/generic_proxy/filters/network/test/integration_test.cc index 41e10d2bede6..f6c441f88f6a 100644 --- a/contrib/generic_proxy/filters/network/test/integration_test.cc +++ b/contrib/generic_proxy/filters/network/test/integration_test.cc @@ -425,235 +425,235 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, IntegrationTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); -// TEST_P(IntegrationTest, InitializeInstance) { -// FakeStreamCodecFactoryConfig codec_factory_config; -// Registry::InjectFactory registration(codec_factory_config); +TEST_P(IntegrationTest, InitializeInstance) { + FakeStreamCodecFactoryConfig codec_factory_config; + Registry::InjectFactory registration(codec_factory_config); -// initialize(defaultConfig(), std::make_unique()); -// } + initialize(defaultConfig(), std::make_unique()); +} -// TEST_P(IntegrationTest, RequestRouteNotFound) { -// FakeStreamCodecFactoryConfig codec_factory_config; -// Registry::InjectFactory registration(codec_factory_config); +TEST_P(IntegrationTest, RequestRouteNotFound) { + FakeStreamCodecFactoryConfig codec_factory_config; + Registry::InjectFactory registration(codec_factory_config); -// initialize(defaultConfig(), std::make_unique()); -// EXPECT_TRUE(makeClientConnectionForTest()); + initialize(defaultConfig(), std::make_unique()); + EXPECT_TRUE(makeClientConnectionForTest()); -// FakeStreamCodecFactory::FakeRequest request; -// request.host_ = "service_name_0"; -// request.method_ = "hello"; -// request.path_ = "/path_or_anything"; -// request.protocol_ = "fake_fake_fake"; -// request.data_ = {{"version", "v2"}}; + FakeStreamCodecFactory::FakeRequest request; + request.host_ = "service_name_0"; + request.method_ = "hello"; + request.path_ = "/path_or_anything"; + request.protocol_ = "fake_fake_fake"; + request.data_ = {{"version", "v2"}}; -// sendRequestForTest(request); + sendRequestForTest(request); -// RELEASE_ASSERT(waitDownstreamResponseForTest(TestUtility::DefaultTimeout, 0), -// "unexpected timeout"); + RELEASE_ASSERT(waitDownstreamResponseForTest(TestUtility::DefaultTimeout, 0), + "unexpected timeout"); -// EXPECT_NE(client_codec_callabcks_->responses_[0].response_, nullptr); -// EXPECT_EQ(client_codec_callabcks_->responses_[0].response_->status().code(), -// static_cast(absl::StatusCode::kNotFound)); + EXPECT_NE(client_codec_callabcks_->responses_[0].response_, nullptr); + EXPECT_EQ(client_codec_callabcks_->responses_[0].response_->status().code(), + static_cast(absl::StatusCode::kNotFound)); -// cleanup(); -// } + cleanup(); +} -// TEST_P(IntegrationTest, RequestAndResponse) { -// FakeStreamCodecFactoryConfig codec_factory_config; -// Registry::InjectFactory registration(codec_factory_config); +TEST_P(IntegrationTest, RequestAndResponse) { + FakeStreamCodecFactoryConfig codec_factory_config; + Registry::InjectFactory registration(codec_factory_config); -// initialize(defaultConfig(), std::make_unique()); + initialize(defaultConfig(), std::make_unique()); -// EXPECT_TRUE(makeClientConnectionForTest()); + EXPECT_TRUE(makeClientConnectionForTest()); -// FakeStreamCodecFactory::FakeRequest request; -// request.host_ = "service_name_0"; -// request.method_ = "hello"; -// request.path_ = "/path_or_anything"; -// request.protocol_ = "fake_fake_fake"; -// request.data_ = {{"version", "v1"}}; + FakeStreamCodecFactory::FakeRequest request; + request.host_ = "service_name_0"; + request.method_ = "hello"; + request.path_ = "/path_or_anything"; + request.protocol_ = "fake_fake_fake"; + request.data_ = {{"version", "v1"}}; -// sendRequestForTest(request); + sendRequestForTest(request); -// waitForUpstreamConnectionForTest(); -// const std::function data_validator = -// [](const std::string& data) -> bool { return data.find("v1") != std::string::npos; }; -// waitForUpstreamRequestForTest(data_validator); + waitForUpstreamConnectionForTest(); + const std::function data_validator = + [](const std::string& data) -> bool { return data.find("v1") != std::string::npos; }; + waitForUpstreamRequestForTest(data_validator); -// FakeStreamCodecFactory::FakeResponse response; -// response.protocol_ = "fake_fake_fake"; -// response.status_ = StreamStatus(); -// response.data_["zzzz"] = "xxxx"; + FakeStreamCodecFactory::FakeResponse response; + response.protocol_ = "fake_fake_fake"; + response.status_ = StreamStatus(); + response.data_["zzzz"] = "xxxx"; -// sendResponseForTest(response); + sendResponseForTest(response); -// RELEASE_ASSERT(waitDownstreamResponseForTest(TestUtility::DefaultTimeout, 0), -// "unexpected timeout"); + RELEASE_ASSERT(waitDownstreamResponseForTest(TestUtility::DefaultTimeout, 0), + "unexpected timeout"); + + EXPECT_NE(client_codec_callabcks_->responses_[0].response_, nullptr); + EXPECT_EQ(client_codec_callabcks_->responses_[0].response_->status().code(), 0); + EXPECT_EQ(client_codec_callabcks_->responses_[0].response_->get("zzzz"), "xxxx"); + + cleanup(); +} + +TEST_P(IntegrationTest, RequestTimeout) { + FakeStreamCodecFactoryConfig codec_factory_config; + Registry::InjectFactory registration(codec_factory_config); -// EXPECT_NE(client_codec_callabcks_->responses_[0].response_, nullptr); -// EXPECT_EQ(client_codec_callabcks_->responses_[0].response_->status().code(), 0); -// EXPECT_EQ(client_codec_callabcks_->responses_[0].response_->get("zzzz"), "xxxx"); + initialize(timeoutConfig(), std::make_unique()); -// cleanup(); -// } + EXPECT_TRUE(makeClientConnectionForTest()); -// TEST_P(IntegrationTest, RequestTimeout) { -// FakeStreamCodecFactoryConfig codec_factory_config; -// Registry::InjectFactory registration(codec_factory_config); + FakeStreamCodecFactory::FakeRequest request; + request.host_ = "service_name_0"; + request.method_ = "hello"; + request.path_ = "/path_or_anything"; + request.protocol_ = "fake_fake_fake"; + request.data_ = {{"version", "v1"}}; -// initialize(timeoutConfig(), std::make_unique()); + sendRequestForTest(request); -// EXPECT_TRUE(makeClientConnectionForTest()); + waitForUpstreamConnectionForTest(); + const std::function data_validator = + [](const std::string& data) -> bool { return data.find("v1") != std::string::npos; }; + waitForUpstreamRequestForTest(data_validator); -// FakeStreamCodecFactory::FakeRequest request; -// request.host_ = "service_name_0"; -// request.method_ = "hello"; -// request.path_ = "/path_or_anything"; -// request.protocol_ = "fake_fake_fake"; -// request.data_ = {{"version", "v1"}}; + // No response is sent, so the downstream should timeout. -// sendRequestForTest(request); + RELEASE_ASSERT(waitDownstreamResponseForTest(TestUtility::DefaultTimeout, 0), + "unexpected timeout"); -// waitForUpstreamConnectionForTest(); -// const std::function data_validator = -// [](const std::string& data) -> bool { return data.find("v1") != std::string::npos; }; -// waitForUpstreamRequestForTest(data_validator); + EXPECT_NE(client_codec_callabcks_->responses_[0].response_, nullptr); + EXPECT_EQ(client_codec_callabcks_->responses_[0].response_->status().code(), 4); -// // No response is sent, so the downstream should timeout. + cleanup(); +} -// RELEASE_ASSERT(waitDownstreamResponseForTest(TestUtility::DefaultTimeout, 0), -// "unexpected timeout"); +TEST_P(IntegrationTest, MultipleRequestsWithSameStreamId) { + FakeStreamCodecFactoryConfig codec_factory_config; + Registry::InjectFactory registration(codec_factory_config); -// EXPECT_NE(client_codec_callabcks_->responses_[0].response_, nullptr); -// EXPECT_EQ(client_codec_callabcks_->responses_[0].response_->status().code(), 4); + auto codec_factory = std::make_unique(); -// cleanup(); -// } + initialize(defaultConfig(true), std::move(codec_factory)); -// TEST_P(IntegrationTest, MultipleRequestsWithSameStreamId) { -// FakeStreamCodecFactoryConfig codec_factory_config; -// Registry::InjectFactory registration(codec_factory_config); + EXPECT_TRUE(makeClientConnectionForTest()); -// auto codec_factory = std::make_unique(); + FakeStreamCodecFactory::FakeRequest request_1; + request_1.host_ = "service_name_0"; + request_1.method_ = "hello"; + request_1.path_ = "/path_or_anything"; + request_1.protocol_ = "fake_fake_fake"; + request_1.data_ = {{"version", "v1"}}; + request_1.stream_frame_flags_ = FrameFlags(1); -// initialize(defaultConfig(true), std::move(codec_factory)); + sendRequestForTest(request_1); -// EXPECT_TRUE(makeClientConnectionForTest()); + waitForUpstreamConnectionForTest(); + const std::function data_validator = + [](const std::string& data) -> bool { return data.find("v1") != std::string::npos; }; + waitForUpstreamRequestForTest(data_validator); -// FakeStreamCodecFactory::FakeRequest request_1; -// request_1.host_ = "service_name_0"; -// request_1.method_ = "hello"; -// request_1.path_ = "/path_or_anything"; -// request_1.protocol_ = "fake_fake_fake"; -// request_1.data_ = {{"version", "v1"}}; -// request_1.stream_frame_flags_ = FrameFlags(StreamFlags(1)); + FakeStreamCodecFactory::FakeRequest request_2; + request_2.host_ = "service_name_0"; + request_2.method_ = "hello"; + request_2.path_ = "/path_or_anything"; + request_2.protocol_ = "fake_fake_fake"; + request_2.data_ = {{"version", "v1"}}; + request_2.stream_frame_flags_ = FrameFlags(1); -// sendRequestForTest(request_1); + // Send the second request with the same stream id and expect the connection to be closed. + sendRequestForTest(request_2); -// waitForUpstreamConnectionForTest(); -// const std::function data_validator = -// [](const std::string& data) -> bool { return data.find("v1") != std::string::npos; }; -// waitForUpstreamRequestForTest(data_validator); + // Wait for the connection to be closed. + auto result = upstream_connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); -// FakeStreamCodecFactory::FakeRequest request_2; -// request_2.host_ = "service_name_0"; -// request_2.method_ = "hello"; -// request_2.path_ = "/path_or_anything"; -// request_2.protocol_ = "fake_fake_fake"; -// request_2.data_ = {{"version", "v1"}}; -// request_2.stream_frame_flags_ = FrameFlags(StreamFlags(1)); + cleanup(); +} -// // Send the second request with the same stream id and expect the connection to be closed. -// sendRequestForTest(request_2); +TEST_P(IntegrationTest, MultipleRequests) { + FakeStreamCodecFactoryConfig codec_factory_config; + Registry::InjectFactory registration(codec_factory_config); -// // Wait for the connection to be closed. -// auto result = upstream_connection_->waitForDisconnect(); -// RELEASE_ASSERT(result, result.message()); + auto codec_factory = std::make_unique(); -// cleanup(); -// } + initialize(defaultConfig(true), std::move(codec_factory)); -// TEST_P(IntegrationTest, MultipleRequests) { -// FakeStreamCodecFactoryConfig codec_factory_config; -// Registry::InjectFactory registration(codec_factory_config); + EXPECT_TRUE(makeClientConnectionForTest()); -// auto codec_factory = std::make_unique(); + FakeStreamCodecFactory::FakeRequest request_1; + request_1.host_ = "service_name_0"; + request_1.method_ = "hello"; + request_1.path_ = "/path_or_anything"; + request_1.protocol_ = "fake_fake_fake"; + request_1.data_ = {{"version", "v1"}, {"frame", "1_header"}}; + request_1.stream_frame_flags_ = FrameFlags(1); -// initialize(defaultConfig(true), std::move(codec_factory)); + sendRequestForTest(request_1); -// EXPECT_TRUE(makeClientConnectionForTest()); + waitForUpstreamConnectionForTest(); -// FakeStreamCodecFactory::FakeRequest request_1; -// request_1.host_ = "service_name_0"; -// request_1.method_ = "hello"; -// request_1.path_ = "/path_or_anything"; -// request_1.protocol_ = "fake_fake_fake"; -// request_1.data_ = {{"version", "v1"}, {"frame", "1_header"}}; -// request_1.stream_frame_flags_ = FrameFlags(StreamFlags(1)); + const std::function data_validator_1 = + [](const std::string& data) -> bool { + return data.find("frame:1_header") != std::string::npos; + }; + waitForUpstreamRequestForTest(data_validator_1); -// sendRequestForTest(request_1); + FakeStreamCodecFactory::FakeRequest request_2; + request_2.host_ = "service_name_0"; + request_2.method_ = "hello"; + request_2.path_ = "/path_or_anything"; + request_2.protocol_ = "fake_fake_fake"; + request_2.data_ = {{"version", "v1"}, {"frame", "2_header"}}; + request_2.stream_frame_flags_ = FrameFlags(2); -// waitForUpstreamConnectionForTest(); + // Reset request encoder callback. + test_encoding_context_ = std::make_shared(); -// const std::function data_validator_1 = -// [](const std::string& data) -> bool { -// return data.find("frame:1_header") != std::string::npos; -// }; -// waitForUpstreamRequestForTest(data_validator_1); + // Send the second request with the different stream id and expect the connection to be alive. + sendRequestForTest(request_2); + const std::function data_validator_2 = + [](const std::string& data) -> bool { + return data.find("frame:2_header") != std::string::npos; + }; + waitForUpstreamRequestForTest(data_validator_2); -// FakeStreamCodecFactory::FakeRequest request_2; -// request_2.host_ = "service_name_0"; -// request_2.method_ = "hello"; -// request_2.path_ = "/path_or_anything"; -// request_2.protocol_ = "fake_fake_fake"; -// request_2.data_ = {{"version", "v1"}, {"frame", "2_header"}}; -// request_2.stream_frame_flags_ = FrameFlags(StreamFlags(2)); + FakeStreamCodecFactory::FakeResponse response_2; + response_2.protocol_ = "fake_fake_fake"; + response_2.status_ = StreamStatus(); + response_2.data_["zzzz"] = "xxxx"; + response_2.stream_frame_flags_ = FrameFlags(2); -// // Reset request encoder callback. -// test_encoding_context_ = std::make_shared(); + sendResponseForTest(response_2); -// // Send the second request with the different stream id and expect the connection to be alive. -// sendRequestForTest(request_2); -// const std::function data_validator_2 = -// [](const std::string& data) -> bool { -// return data.find("frame:2_header") != std::string::npos; -// }; -// waitForUpstreamRequestForTest(data_validator_2); + RELEASE_ASSERT(waitDownstreamResponseForTest(TestUtility::DefaultTimeout, 2), + "unexpected timeout"); -// FakeStreamCodecFactory::FakeResponse response_2; -// response_2.protocol_ = "fake_fake_fake"; -// response_2.status_ = StreamStatus(); -// response_2.data_["zzzz"] = "xxxx"; -// response_2.stream_frame_flags_ = FrameFlags(StreamFlags(2)); + EXPECT_NE(client_codec_callabcks_->responses_[2].response_, nullptr); + EXPECT_EQ(client_codec_callabcks_->responses_[2].response_->status().code(), 0); + EXPECT_EQ(client_codec_callabcks_->responses_[2].response_->get("zzzz"), "xxxx"); + EXPECT_EQ(client_codec_callabcks_->responses_[2].response_->frameFlags().streamId(), 2); -// sendResponseForTest(response_2); + FakeStreamCodecFactory::FakeResponse response_1; + response_1.protocol_ = "fake_fake_fake"; + response_1.status_ = StreamStatus(); + response_1.data_["zzzz"] = "yyyy"; + response_1.stream_frame_flags_ = FrameFlags(1); -// RELEASE_ASSERT(waitDownstreamResponseForTest(TestUtility::DefaultTimeout, 2), -// "unexpected timeout"); + sendResponseForTest(response_1); -// EXPECT_NE(client_codec_callabcks_->responses_[2].response_, nullptr); -// EXPECT_EQ(client_codec_callabcks_->responses_[2].response_->status().code(), 0); -// EXPECT_EQ(client_codec_callabcks_->responses_[2].response_->get("zzzz"), "xxxx"); -// EXPECT_EQ(client_codec_callabcks_->responses_[2].response_->frameFlags().streamId(), 2); + RELEASE_ASSERT(waitDownstreamResponseForTest(TestUtility::DefaultTimeout, 1), + "unexpected timeout"); -// FakeStreamCodecFactory::FakeResponse response_1; -// response_1.protocol_ = "fake_fake_fake"; -// response_1.status_ = StreamStatus(); -// response_1.data_["zzzz"] = "yyyy"; -// response_1.stream_frame_flags_ = FrameFlags(StreamFlags(1)); - -// sendResponseForTest(response_1); - -// RELEASE_ASSERT(waitDownstreamResponseForTest(TestUtility::DefaultTimeout, 1), -// "unexpected timeout"); + EXPECT_NE(client_codec_callabcks_->responses_[1].response_, nullptr); + EXPECT_EQ(client_codec_callabcks_->responses_[1].response_->status().code(), 0); + EXPECT_EQ(client_codec_callabcks_->responses_[1].response_->get("zzzz"), "yyyy"); + EXPECT_EQ(client_codec_callabcks_->responses_[1].response_->frameFlags().streamId(), 1); -// EXPECT_NE(client_codec_callabcks_->responses_[1].response_, nullptr); -// EXPECT_EQ(client_codec_callabcks_->responses_[1].response_->status().code(), 0); -// EXPECT_EQ(client_codec_callabcks_->responses_[1].response_->get("zzzz"), "yyyy"); -// EXPECT_EQ(client_codec_callabcks_->responses_[1].response_->frameFlags().streamId(), 1); - -// cleanup(); -// } + cleanup(); +} TEST_P(IntegrationTest, MultipleRequestsWithMultipleFrames) { FakeStreamCodecFactoryConfig codec_factory_config; @@ -671,15 +671,15 @@ TEST_P(IntegrationTest, MultipleRequestsWithMultipleFrames) { request_1.path_ = "/path_or_anything"; request_1.protocol_ = "fake_fake_fake"; request_1.data_ = {{"version", "v1"}, {"frame", "1_header"}}; - request_1.stream_frame_flags_ = FrameFlags(StreamFlags(1), false); + request_1.stream_frame_flags_ = FrameFlags(1, FrameFlags::FLAG_EMPTY); FakeStreamCodecFactory::FakeCommonFrame request_1_frame_1; request_1_frame_1.data_ = {{"frame", "1_frame_1"}}; - request_1_frame_1.stream_frame_flags_ = FrameFlags(StreamFlags(1), false); + request_1_frame_1.stream_frame_flags_ = FrameFlags(1, FrameFlags::FLAG_EMPTY); FakeStreamCodecFactory::FakeCommonFrame request_1_frame_2; request_1_frame_2.data_ = {{"frame", "1_frame_2"}}; - request_1_frame_2.stream_frame_flags_ = FrameFlags(StreamFlags(1), true); + request_1_frame_2.stream_frame_flags_ = FrameFlags(1); FakeStreamCodecFactory::FakeRequest request_2; request_2.host_ = "service_name_0"; @@ -687,15 +687,15 @@ TEST_P(IntegrationTest, MultipleRequestsWithMultipleFrames) { request_2.path_ = "/path_or_anything"; request_2.protocol_ = "fake_fake_fake"; request_2.data_ = {{"version", "v1"}, {"frame", "2_header"}}; - request_2.stream_frame_flags_ = FrameFlags(StreamFlags(2), false); + request_2.stream_frame_flags_ = FrameFlags(2, FrameFlags::FLAG_EMPTY); FakeStreamCodecFactory::FakeCommonFrame request_2_frame_1; request_2_frame_1.data_ = {{"frame", "2_frame_1"}}; - request_2_frame_1.stream_frame_flags_ = FrameFlags(StreamFlags(2), false); + request_2_frame_1.stream_frame_flags_ = FrameFlags(2, FrameFlags::FLAG_EMPTY); FakeStreamCodecFactory::FakeCommonFrame request_2_frame_2; request_2_frame_2.data_ = {{"frame", "2_frame_2"}}; - request_2_frame_2.stream_frame_flags_ = FrameFlags(StreamFlags(2), true); + request_2_frame_2.stream_frame_flags_ = FrameFlags(2); // We handle frame one by one to make sure the order is correct. @@ -753,10 +753,10 @@ TEST_P(IntegrationTest, MultipleRequestsWithMultipleFrames) { response_2.protocol_ = "fake_fake_fake"; response_2.status_ = StreamStatus(); response_2.data_["zzzz"] = "xxxx"; - response_2.stream_frame_flags_ = FrameFlags(StreamFlags(2), false); + response_2.stream_frame_flags_ = FrameFlags(2, FrameFlags::FLAG_EMPTY); FakeStreamCodecFactory::FakeCommonFrame response_2_frame_1; - response_2_frame_1.stream_frame_flags_ = FrameFlags(StreamFlags(2), true); + response_2_frame_1.stream_frame_flags_ = FrameFlags(2); sendResponseForTest(response_2); sendResponseForTest(response_2_frame_1); @@ -773,10 +773,10 @@ TEST_P(IntegrationTest, MultipleRequestsWithMultipleFrames) { response_1.protocol_ = "fake_fake_fake"; response_1.status_ = StreamStatus(); response_1.data_["zzzz"] = "yyyy"; - response_1.stream_frame_flags_ = FrameFlags(StreamFlags(1), false); + response_1.stream_frame_flags_ = FrameFlags(1, FrameFlags::FLAG_EMPTY); FakeStreamCodecFactory::FakeCommonFrame response_1_frame_1; - response_1_frame_1.stream_frame_flags_ = FrameFlags(StreamFlags(1), true); + response_1_frame_1.stream_frame_flags_ = FrameFlags(1); sendResponseForTest(response_1); sendResponseForTest(response_1_frame_1); diff --git a/contrib/generic_proxy/filters/network/test/proxy_test.cc b/contrib/generic_proxy/filters/network/test/proxy_test.cc index 701e2c26faac..75246e829e3f 100644 --- a/contrib/generic_proxy/filters/network/test/proxy_test.cc +++ b/contrib/generic_proxy/filters/network/test/proxy_test.cc @@ -199,6 +199,13 @@ class FilterTest : public FilterConfigTest { .WillOnce(Invoke( [this](ServerCodecCallbacks& callback) { server_codec_callbacks_ = &callback; })); + ON_CALL(*server_codec_, respond(_, _, _)) + .WillByDefault( + Invoke([](Status status, absl::string_view data, const RequestHeaderFrame& req) { + FakeStreamCodecFactory::FakeServerCodec codec; + return codec.respond(status, data, req); + })); + filter_ = std::make_shared(filter_config_, factory_context_); EXPECT_EQ(filter_.get(), server_codec_callbacks_); @@ -640,8 +647,6 @@ TEST_F(FilterTest, ActiveStreamSingleFrameFiltersContinueDecodingAfterSendLocalR EXPECT_CALL(*mock_stream_filter_1, decodeHeaderFrame(_)) .WillOnce(Invoke([&](const RequestHeaderFrame&) { - EXPECT_CALL(*server_codec_, respond(_, _, _)) - .WillOnce(Return(ByMove(std::make_unique()))); EXPECT_CALL(*server_codec_, encode(_, _)); active_stream->sendLocalReply(Status(StatusCode::kUnknown, "test_detail"), {}, nullptr); // Make no sense because the filter chain will be always stopped if @@ -784,10 +789,7 @@ TEST_F(FilterTest, ActiveStreamSingleFrameFiltersContinueEncodingAfterSendLocalR EXPECT_CALL(*mock_stream_filter_1, encodeHeaderFrame(_)) .WillOnce(Invoke([&](const ResponseHeaderFrame&) { - EXPECT_CALL(*server_codec_, respond(_, _, _)) - .WillOnce(Return(ByMove(std::make_unique()))); EXPECT_CALL(*server_codec_, encode(_, _)); - active_stream->sendLocalReply(Status(StatusCode::kUnknown, "test_detail"), {}, nullptr); // Make no sense because the filter chain will be always stopped if // the stream is reset or completed. @@ -824,11 +826,11 @@ TEST_F(FilterTest, ActiveStreamMultipleFrameFiltersContinueDecoding) { .WillOnce(Return(HeaderFilterStatus::StopIteration)); auto request = std::make_unique(); - request->stream_frame_flags_ = {StreamFlags(0), false}; + request->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); auto request_frame_0 = std::make_unique(); - request_frame_0->stream_frame_flags_ = {StreamFlags(0), false}; + request_frame_0->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); auto request_frame_1 = std::make_unique(); - request_frame_1->stream_frame_flags_ = {StreamFlags(0), false}; + request_frame_1->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); auto request_frame_2 = std::make_unique(); filter_->onDecodingSuccess(std::move(request)); @@ -901,11 +903,11 @@ TEST_F(FilterTest, ActiveStreamMultipleFrameFiltersContinueEncoding) { auto active_stream = filter_->activeStreamsForTest().begin()->get(); auto response = std::make_unique(); - response->stream_frame_flags_ = {StreamFlags(0), false}; + response->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); auto response_frame_0 = std::make_unique(); - response_frame_0->stream_frame_flags_ = {StreamFlags(0), false}; + response_frame_0->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); auto response_frame_1 = std::make_unique(); - response_frame_1->stream_frame_flags_ = {StreamFlags(0), false}; + response_frame_1->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); auto response_frame_2 = std::make_unique(); EXPECT_CALL(*mock_stream_filter_1, encodeHeaderFrame(_)) @@ -980,7 +982,220 @@ TEST_F(FilterTest, ActiveStreamMultipleFrameFiltersContinueEncoding) { active_stream->onResponseCommonFrame(std::move(response_frame_2)); } +TEST_F(FilterTest, UpstreamResponseAfterPreviousUpstreamResponse) { + auto mock_stream_filter_0 = std::make_shared>(); + auto mock_stream_filter_1 = std::make_shared>(); + + mock_stream_filters_ = {{"mock_0", mock_stream_filter_0}, {"mock_1", mock_stream_filter_1}}; + + // The logger is used to test the log format. + initializeFilter(false, loggerFormFormat()); + + auto request = std::make_unique(); + + filter_->onDecodingSuccess(std::move(request)); + EXPECT_EQ(1, filter_->activeStreamsForTest().size()); + + EXPECT_EQ(filter_config_->stats().downstream_rq_total_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_active_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_error_.value(), 0); + EXPECT_EQ(filter_config_->stats().downstream_rq_reset_.value(), 0); + + auto active_stream = filter_->activeStreamsForTest().begin()->get(); + + EXPECT_CALL(factory_context_.drain_manager_, drainClose()).WillOnce(Return(false)); + EXPECT_CALL(filter_callbacks_.connection_.dispatcher_, deferredDelete_(_)); + + // Response filter chain is stopped by the first filter. + EXPECT_CALL(*mock_stream_filter_1, encodeHeaderFrame(_)) + .WillOnce(Return(HeaderFilterStatus::StopIteration)); + + auto response = std::make_unique(); + active_stream->onResponseHeaderFrame(std::move(response)); + + // The repeated response will result in the stream reset. + auto response_again = std::make_unique(); + active_stream->onResponseHeaderFrame(std::move(response_again)); + + EXPECT_EQ(filter_config_->stats().downstream_rq_total_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_active_.value(), 0); + EXPECT_EQ(filter_config_->stats().downstream_rq_error_.value(), 0); + EXPECT_EQ(filter_config_->stats().downstream_rq_reset_.value(), 1); +} + +TEST_F(FilterTest, UpstreamResponseAfterPreviousLocalReply) { + auto mock_stream_filter_0 = std::make_shared>(); + auto mock_stream_filter_1 = std::make_shared>(); + + mock_stream_filters_ = {{"mock_0", mock_stream_filter_0}, {"mock_1", mock_stream_filter_1}}; + + // The logger is used to test the log format. + initializeFilter(false, loggerFormFormat()); + + auto request = std::make_unique(); + + filter_->onDecodingSuccess(std::move(request)); + EXPECT_EQ(1, filter_->activeStreamsForTest().size()); + + EXPECT_EQ(filter_config_->stats().downstream_rq_total_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_active_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_error_.value(), 0); + EXPECT_EQ(filter_config_->stats().downstream_rq_reset_.value(), 0); + + auto active_stream = filter_->activeStreamsForTest().begin()->get(); + + EXPECT_CALL(factory_context_.drain_manager_, drainClose()).WillOnce(Return(false)); + EXPECT_CALL(filter_callbacks_.connection_.dispatcher_, deferredDelete_(_)); + + // Response filter chain of local reply is stopped by the first filter. + EXPECT_CALL(*mock_stream_filter_1, encodeHeaderFrame(_)) + .WillOnce(Return(HeaderFilterStatus::StopIteration)); + + active_stream->sendLocalReply(Status(StatusCode::kUnknown, "test_detail"), {}, nullptr); + + // The repeated response will result in the stream reset. + auto response_again = std::make_unique(); + active_stream->onResponseHeaderFrame(std::move(response_again)); + + EXPECT_EQ(filter_config_->stats().downstream_rq_total_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_active_.value(), 0); + // Although the original local reply is not sent to the downstream, it will still be counted as an + // error reply. This is corner case. + EXPECT_EQ(filter_config_->stats().downstream_rq_error_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_reset_.value(), 1); +} + +TEST_F(FilterTest, SendLocalReplyAfterPreviousLocalReply) { + auto mock_stream_filter_0 = std::make_shared>(); + auto mock_stream_filter_1 = std::make_shared>(); + + mock_stream_filters_ = {{"mock_0", mock_stream_filter_0}, {"mock_1", mock_stream_filter_1}}; + + // The logger is used to test the log format. + initializeFilter(false, loggerFormFormat()); + + auto request = std::make_unique(); + + filter_->onDecodingSuccess(std::move(request)); + EXPECT_EQ(1, filter_->activeStreamsForTest().size()); + + EXPECT_EQ(filter_config_->stats().downstream_rq_total_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_active_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_error_.value(), 0); + EXPECT_EQ(filter_config_->stats().downstream_rq_reset_.value(), 0); + + auto active_stream = filter_->activeStreamsForTest().begin()->get(); + + EXPECT_CALL(factory_context_.drain_manager_, drainClose()).WillOnce(Return(false)); + EXPECT_CALL(filter_callbacks_.connection_.dispatcher_, deferredDelete_(_)); + + // Response filter chain of local reply is stopped by the first filter. + EXPECT_CALL(*mock_stream_filter_1, encodeHeaderFrame(_)) + .WillOnce(Return(HeaderFilterStatus::StopIteration)); + + active_stream->sendLocalReply(Status(StatusCode::kUnknown, "test_detail_1"), {}, nullptr); + + EXPECT_CALL(*server_codec_, encode(_, _)) + .WillOnce(Invoke([&](const StreamFrame& stream, EncodingContext&) { + auto* typed_response = dynamic_cast(&stream); + EXPECT_EQ(typed_response->status_.code(), static_cast(StatusCode::kUnknown)); + EXPECT_EQ(typed_response->message_, "test_detail_2"); + + Buffer::OwnedImpl buffer; + buffer.add("test"); + + server_codec_callbacks_->writeToConnection(buffer); + buffer.drain(buffer.length()); + + return EncodingResult{4}; + })); + EXPECT_CALL(filter_callbacks_.connection_, write(BufferStringEqual("test"), false)); + + // The latest local reply will skip the filter chain processing and be sent to the downstream + // directly. + active_stream->sendLocalReply(Status(StatusCode::kUnknown, "test_detail_2"), {}, nullptr); + + EXPECT_EQ(filter_config_->stats().downstream_rq_total_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_active_.value(), 0); + EXPECT_EQ(filter_config_->stats().downstream_rq_error_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_reset_.value(), 0); +} + +TEST_F(FilterTest, SendLocalReplyAfterPreviousUpstreamResponseHeaderIsSent) { + auto mock_stream_filter_0 = std::make_shared>(); + auto mock_stream_filter_1 = std::make_shared>(); + + mock_stream_filters_ = {{"mock_0", mock_stream_filter_0}, {"mock_1", mock_stream_filter_1}}; + + // The logger is used to test the log format. + initializeFilter(false, loggerFormFormat()); + + auto request = std::make_unique(); + + filter_->onDecodingSuccess(std::move(request)); + EXPECT_EQ(1, filter_->activeStreamsForTest().size()); + + EXPECT_EQ(filter_config_->stats().downstream_rq_total_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_active_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_error_.value(), 0); + EXPECT_EQ(filter_config_->stats().downstream_rq_reset_.value(), 0); + + auto active_stream = filter_->activeStreamsForTest().begin()->get(); + + EXPECT_CALL(factory_context_.drain_manager_, drainClose()).WillOnce(Return(false)); + EXPECT_CALL(filter_callbacks_.connection_.dispatcher_, deferredDelete_(_)); + + testing::Sequence s; + + // Filter chain is completed and the response header is sent to the downstream. + EXPECT_CALL(*mock_stream_filter_1, encodeHeaderFrame(_)) + .WillOnce(Return(HeaderFilterStatus::Continue)); + EXPECT_CALL(*mock_stream_filter_0, encodeHeaderFrame(_)) + .WillOnce(Return(HeaderFilterStatus::Continue)); + + EXPECT_CALL(*server_codec_, encode(_, _)) + .WillOnce(Invoke([&](const StreamFrame& stream, EncodingContext&) { + auto* typed_response = dynamic_cast(&stream); + EXPECT_EQ(typed_response->status_.code(), 0); + EXPECT_EQ(typed_response->message_, "anything"); + + Buffer::OwnedImpl buffer; + buffer.add("test"); + + server_codec_callbacks_->writeToConnection(buffer); + buffer.drain(buffer.length()); + + return EncodingResult{4}; + })); + EXPECT_CALL(filter_callbacks_.connection_, write(BufferStringEqual("test"), false)); + + auto response = std::make_unique(); + response->message_ = "anything"; + // The response header is not end frame to ensure the stream won't be cleared after the response + // header is sent. + response->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); + + active_stream->onResponseHeaderFrame(std::move(response)); + + // The latest local reply will result in the stream reset because the previous response header is + // sent. + active_stream->sendLocalReply(Status(StatusCode::kUnknown, "test_detail_2"), {}, nullptr); + + EXPECT_EQ(filter_config_->stats().downstream_rq_total_.value(), 1); + EXPECT_EQ(filter_config_->stats().downstream_rq_active_.value(), 0); + EXPECT_EQ(filter_config_->stats().downstream_rq_error_.value(), 0); + EXPECT_EQ(filter_config_->stats().downstream_rq_reset_.value(), 1); +} + TEST_F(FilterTest, ActiveStreamSendLocalReply) { + auto mock_stream_filter_0 = std::make_shared>(); + auto mock_stream_filter_1 = std::make_shared>(); + auto mock_stream_filter_2 = std::make_shared>(); + + mock_stream_filters_ = {{"mock_0", mock_stream_filter_0}, + {"mock_1", mock_stream_filter_1}, + {"mock_2", mock_stream_filter_2}}; + initializeFilter(false, loggerFormFormat()); auto request = std::make_unique(); @@ -999,14 +1214,6 @@ TEST_F(FilterTest, ActiveStreamSendLocalReply) { auto active_stream = filter_->activeStreamsForTest().begin()->get(); - EXPECT_CALL(*server_codec_, respond(_, _, _)) - .WillOnce(Invoke([&](Status status, absl::string_view, const Request&) -> ResponsePtr { - auto response = std::make_unique(); - response->status_ = {static_cast(status.code()), status.code() == StatusCode::kOk}; - response->message_ = status.message(); - return response; - })); - EXPECT_CALL(*factory_context_.server_factory_context_.access_log_manager_.file_, write("host-value /path-value method-value protocol-value request-value " "response-value - 2 test_detail")); @@ -1029,6 +1236,14 @@ TEST_F(FilterTest, ActiveStreamSendLocalReply) { return EncodingResult{4}; })); + testing::InSequence s; + EXPECT_CALL(*mock_stream_filter_2, encodeHeaderFrame(_)) + .WillOnce(Return(HeaderFilterStatus::Continue)); + EXPECT_CALL(*mock_stream_filter_1, encodeHeaderFrame(_)) + .WillOnce(Return(HeaderFilterStatus::Continue)); + EXPECT_CALL(*mock_stream_filter_0, encodeHeaderFrame(_)) + .WillOnce(Return(HeaderFilterStatus::Continue)); + active_stream->sendLocalReply( Status(StatusCode::kUnknown, "test_detail"), {}, [](StreamResponse& response) { response.set("response-key", "response-value"); }); @@ -1160,7 +1375,7 @@ TEST_F(FilterTest, NewStreamAndReplyNormallyWithMultipleFrames) { request->method_ = "method-value"; request->protocol_ = "protocol-value"; request->data_["request-key"] = "request-value"; - request->stream_frame_flags_ = FrameFlags(StreamFlags(), false); + request->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); // The first frame is not the end stream and we will create a frame handler for it. filter_->onDecodingSuccess(std::move(request)); @@ -1176,14 +1391,14 @@ TEST_F(FilterTest, NewStreamAndReplyNormallyWithMultipleFrames) { EXPECT_CALL(mock_stream_frame_handler, onRequestCommonFrame(_)).Times(2); auto request_frame_1 = std::make_unique(); - request_frame_1->stream_frame_flags_ = FrameFlags(StreamFlags(), false); + request_frame_1->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); filter_->onDecodingSuccess(std::move(request_frame_1)); EXPECT_EQ(1, filter_->activeStreamsForTest().size()); EXPECT_EQ(1, filter_->frameHandlersForTest().size()); // When the last frame is the end stream, we will delete the frame handler. auto request_frame_2 = std::make_unique(); - request_frame_2->stream_frame_flags_ = FrameFlags(StreamFlags(), true); + request_frame_2->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_END_STREAM); filter_->onDecodingSuccess(std::move(request_frame_2)); EXPECT_EQ(1, filter_->activeStreamsForTest().size()); EXPECT_EQ(0, filter_->frameHandlersForTest().size()); @@ -1212,13 +1427,13 @@ TEST_F(FilterTest, NewStreamAndReplyNormallyWithMultipleFrames) { auto response = std::make_unique(); response->data_["response-key"] = "response-value"; - response->stream_frame_flags_ = FrameFlags(StreamFlags(), false); + response->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); response->status_ = {123, false}; // Response non-OK. active_stream->onResponseHeaderFrame(std::move(response)); auto response_frame_1 = std::make_unique(); - response_frame_1->stream_frame_flags_ = FrameFlags(StreamFlags(), true); + response_frame_1->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_END_STREAM); active_stream->onResponseCommonFrame(std::move(response_frame_1)); @@ -1313,7 +1528,8 @@ TEST_F(FilterTest, NewStreamAndReplyNormallyWithStreamDrainClose) { EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::FlushWrite)); auto response = std::make_unique(); - response->stream_frame_flags_ = FrameFlags(StreamFlags(0, false, true, false), true); + response->stream_frame_flags_ = + FrameFlags(0, FrameFlags::FLAG_END_STREAM | FrameFlags::FLAG_DRAIN_CLOSE); active_stream->onResponseHeaderFrame(std::move(response)); } diff --git a/contrib/generic_proxy/filters/network/test/router/router_test.cc b/contrib/generic_proxy/filters/network/test/router/router_test.cc index ad2ff8cf5124..c1cbdebefefe 100644 --- a/contrib/generic_proxy/filters/network/test/router/router_test.cc +++ b/contrib/generic_proxy/filters/network/test/router/router_test.cc @@ -516,7 +516,7 @@ TEST_F(RouterFilterTest, UpstreamRequestPoolFailureConnctionTimeoutAndWithRetryW } TEST_F(RouterFilterTest, UpstreamRequestPoolReadyAndExpectNoResponse) { - setup(FrameFlags(StreamFlags(0, true, false, false), true)); + setup(FrameFlags(0, FrameFlags::FLAG_END_STREAM | FrameFlags::FLAG_ONE_WAY)); kickOffNewUpstreamRequest(true); EXPECT_CALL(mock_filter_callback_, completeDirectly()).WillOnce(Invoke([this]() -> void { @@ -700,11 +700,11 @@ TEST_F(RouterFilterTest, UpstreamRequestPoolReadyAndResponseAndTimeout) { TEST_F(RouterFilterTest, UpstreamRequestPoolReadyAndResponseWithMultipleFrames) { // There are multiple frames in the request. - setup(FrameFlags(StreamFlags(0, false, false, true), /*end_stream*/ false)); + setup(FrameFlags(0, FrameFlags::FLAG_EMPTY)); kickOffNewUpstreamRequest(true); auto frame_1 = std::make_unique(); - frame_1->stream_frame_flags_ = FrameFlags(StreamFlags(0, false, false, true), false); + frame_1->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); EXPECT_EQ(CommonFilterStatus::StopIteration, filter_->decodeCommonFrame(*frame_1)); // This only store the frame and does nothing else because the pool is not ready yet. @@ -744,12 +744,12 @@ TEST_F(RouterFilterTest, UpstreamRequestPoolReadyAndResponseWithMultipleFrames) })); auto response = std::make_unique(); - response->stream_frame_flags_ = FrameFlags(StreamFlags(0, false, false, false), false); + response->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); notifyDecodingSuccess(std::move(response)); auto response_frame_1 = std::make_unique(); - response_frame_1->stream_frame_flags_ = FrameFlags(StreamFlags(0, false, false, false), false); + response_frame_1->stream_frame_flags_ = FrameFlags(0, FrameFlags::FLAG_EMPTY); notifyDecodingSuccess(std::move(response_frame_1)); // End stream is set to true by default. @@ -777,7 +777,8 @@ TEST_F(RouterFilterTest, UpstreamRequestPoolReadyAndResponseWithDrainCloseSetInR })); auto response = std::make_unique(); - response->stream_frame_flags_ = FrameFlags(StreamFlags(0, false, true, false), true); + response->stream_frame_flags_ = + FrameFlags(0, FrameFlags::FLAG_END_STREAM | FrameFlags::FLAG_DRAIN_CLOSE); notifyDecodingSuccess(std::move(response)); // Mock downstream closing. diff --git a/contrib/generic_proxy/filters/network/test/router/upstream_test.cc b/contrib/generic_proxy/filters/network/test/router/upstream_test.cc index cabb2d639b41..856b3afb3fde 100644 --- a/contrib/generic_proxy/filters/network/test/router/upstream_test.cc +++ b/contrib/generic_proxy/filters/network/test/router/upstream_test.cc @@ -377,10 +377,10 @@ TEST_F(UpstreamTest, BoundGenericUpstreamDecodingSuccess) { EXPECT_EQ(2, generic_upstream->waitingResponseRequestsSize()); auto request_1 = std::make_unique(); - request_1->stream_frame_flags_ = FrameFlags(StreamFlags(1)); + request_1->stream_frame_flags_ = FrameFlags(1); auto request_2 = std::make_unique(); - request_2->stream_frame_flags_ = FrameFlags(StreamFlags(2)); + request_2->stream_frame_flags_ = FrameFlags(2); EXPECT_CALL(*mock_client_codec_raw_, decode(_, _)) .WillOnce(testing::Invoke([&](Buffer::Instance&, bool) { @@ -431,10 +431,10 @@ TEST_F(UpstreamTest, BoundGenericUpstreamDecodingSuccessWithMultipleFrames) { EXPECT_EQ(2, generic_upstream->waitingResponseRequestsSize()); auto request_1 = std::make_unique(); - request_1->stream_frame_flags_ = FrameFlags(StreamFlags(1), false); + request_1->stream_frame_flags_ = FrameFlags(1, FrameFlags::FLAG_EMPTY); auto request_2 = std::make_unique(); - request_2->stream_frame_flags_ = FrameFlags(StreamFlags(2), false); + request_2->stream_frame_flags_ = FrameFlags(2, FrameFlags::FLAG_EMPTY); EXPECT_CALL(*mock_client_codec_raw_, decode(_, _)) .WillOnce(testing::Invoke([&](Buffer::Instance&, bool) { @@ -453,10 +453,10 @@ TEST_F(UpstreamTest, BoundGenericUpstreamDecodingSuccessWithMultipleFrames) { EXPECT_EQ(2, generic_upstream->waitingResponseRequestsSize()); auto request_1_frame = std::make_unique(); - request_1_frame->stream_frame_flags_ = FrameFlags(StreamFlags(1), true); + request_1_frame->stream_frame_flags_ = FrameFlags(1); auto request_2_frame = std::make_unique(); - request_2_frame->stream_frame_flags_ = FrameFlags(StreamFlags(2), true); + request_2_frame->stream_frame_flags_ = FrameFlags(2); EXPECT_CALL(*mock_client_codec_raw_, decode(_, _)) .WillOnce(testing::Invoke([&](Buffer::Instance&, bool) { @@ -504,10 +504,10 @@ TEST_F(UpstreamTest, BoundGenericUpstreamDecodingFailureAndNoUpstreamConnectionC EXPECT_EQ(2, generic_upstream->waitingResponseRequestsSize()); auto request_1 = std::make_unique(); - request_1->stream_frame_flags_ = FrameFlags(StreamFlags(1)); + request_1->stream_frame_flags_ = FrameFlags(1); auto request_2 = std::make_unique(); - request_2->stream_frame_flags_ = FrameFlags(StreamFlags(2)); + request_2->stream_frame_flags_ = FrameFlags(2); EXPECT_CALL(*mock_client_codec_raw_, decode(_, _)) .WillOnce(testing::Invoke([&](Buffer::Instance&, bool) { @@ -557,10 +557,10 @@ TEST_F(UpstreamTest, BoundGenericUpstreamDecodingFailure) { EXPECT_EQ(2, generic_upstream->waitingResponseRequestsSize()); auto request_1 = std::make_unique(); - request_1->stream_frame_flags_ = FrameFlags(StreamFlags(1)); + request_1->stream_frame_flags_ = FrameFlags(1); auto request_2 = std::make_unique(); - request_2->stream_frame_flags_ = FrameFlags(StreamFlags(2)); + request_2->stream_frame_flags_ = FrameFlags(2); EXPECT_CALL(*mock_client_codec_raw_, decode(_, _)) .WillOnce(testing::Invoke([&](Buffer::Instance&, bool) { @@ -733,7 +733,7 @@ TEST_F(UpstreamTest, OwnedGenericUpstreamDecodingSuccess) { EXPECT_EQ(1, generic_upstream->waitingResponseRequestsSize()); auto request_1 = std::make_unique(); - request_1->stream_frame_flags_ = FrameFlags(StreamFlags(1)); + request_1->stream_frame_flags_ = FrameFlags(1); EXPECT_CALL(*mock_client_codec_raw_, decode(_, _)) .WillOnce(testing::Invoke([&](Buffer::Instance&, bool) { @@ -773,7 +773,7 @@ TEST_F(UpstreamTest, OwnedGenericUpstreamDecodingSuccessWithMultipleFrames) { EXPECT_EQ(1, generic_upstream->waitingResponseRequestsSize()); auto request_1 = std::make_unique(); - request_1->stream_frame_flags_ = FrameFlags(StreamFlags(1), false); + request_1->stream_frame_flags_ = FrameFlags(1, FrameFlags::FLAG_EMPTY); EXPECT_CALL(*mock_client_codec_raw_, decode(_, _)) .WillOnce(testing::Invoke([&](Buffer::Instance&, bool) { @@ -789,7 +789,7 @@ TEST_F(UpstreamTest, OwnedGenericUpstreamDecodingSuccessWithMultipleFrames) { EXPECT_EQ(1, generic_upstream->waitingResponseRequestsSize()); auto request_1_frame = std::make_unique(); - request_1_frame->stream_frame_flags_ = FrameFlags(StreamFlags(1), true); + request_1_frame->stream_frame_flags_ = FrameFlags(1); EXPECT_CALL(*mock_client_codec_raw_, decode(_, _)) .WillOnce(testing::Invoke([&](Buffer::Instance&, bool) { diff --git a/source/common/common/bit_array.h b/source/common/common/bit_array.h index c4aced97dac3..9a6dff4a2989 100644 --- a/source/common/common/bit_array.h +++ b/source/common/common/bit_array.h @@ -129,13 +129,14 @@ class BitArray { } static inline void storeUnsignedWord(void* destination, uint64_t value) { + value = htole64(value); safeMemcpyUnsafeDst(destination, &value); } static inline uint64_t loadUnsignedWord(const void* source) { uint64_t destination; safeMemcpyUnsafeSrc(&destination, source); - return destination; + return le64toh(destination); } // Backing storage for the underlying array of bits. diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index d4901982e08a..6b63e6b08081 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -49,7 +49,6 @@ RUNTIME_GUARD(envoy_reloadable_features_enable_include_histograms); RUNTIME_GUARD(envoy_reloadable_features_exclude_host_in_eds_status_draining); RUNTIME_GUARD(envoy_reloadable_features_grpc_http1_reverse_bridge_change_http_status); RUNTIME_GUARD(envoy_reloadable_features_grpc_http1_reverse_bridge_handle_empty_response); -RUNTIME_GUARD(envoy_reloadable_features_hmac_base64_encoding_only); RUNTIME_GUARD(envoy_reloadable_features_http1_balsa_delay_reset); RUNTIME_GUARD(envoy_reloadable_features_http1_connection_close_header_in_redirect); // Ignore the automated "remove this flag" issue: we should keep this for 1 year. @@ -67,7 +66,6 @@ RUNTIME_GUARD(envoy_reloadable_features_immediate_response_use_filter_mutation_r RUNTIME_GUARD(envoy_reloadable_features_no_downgrade_to_canonical_name); RUNTIME_GUARD(envoy_reloadable_features_no_extension_lookup_by_name); RUNTIME_GUARD(envoy_reloadable_features_normalize_host_for_preresolve_dfp_dns); -RUNTIME_GUARD(envoy_reloadable_features_oauth_make_token_cookie_httponly); RUNTIME_GUARD(envoy_reloadable_features_oauth_use_standard_max_age_value); RUNTIME_GUARD(envoy_reloadable_features_oauth_use_url_encoding); RUNTIME_GUARD(envoy_reloadable_features_original_dst_rely_on_idle_timeout); 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 b00e3d2cca8a..c83ea223f81a 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 @@ -98,6 +98,13 @@ struct SuccessResponse { ResponsePtr response_; }; +absl::StatusOr validatePathPrefix(absl::string_view path_prefix) { + if (!path_prefix.empty() && path_prefix[0] != '/') { + return absl::InvalidArgumentError("path_prefix should start with \"/\"."); + } + return std::string(path_prefix); +} + } // namespace // Config @@ -117,7 +124,7 @@ ClientConfig::ClientConfig(const envoy::extensions::filters::http::ext_authz::v3 config.http_service().authorization_response().allowed_upstream_headers_to_append(), context)), cluster_name_(config.http_service().server_uri().cluster()), timeout_(timeout), - path_prefix_(path_prefix), + path_prefix_(THROW_OR_RETURN_VALUE(validatePathPrefix(path_prefix), std::string)), tracing_name_(fmt::format("async {} egress", config.http_service().server_uri().cluster())), request_headers_parser_(THROW_OR_RETURN_VALUE( Router::HeaderParser::configure( diff --git a/source/extensions/filters/http/oauth2/filter.cc b/source/extensions/filters/http/oauth2/filter.cc index d5d97279cf7c..dda635f73349 100644 --- a/source/extensions/filters/http/oauth2/filter.cc +++ b/source/extensions/filters/http/oauth2/filter.cc @@ -40,7 +40,6 @@ Http::RegisterCustomInlineHeader& secret, absl::string_vi std::string encodeHmac(const std::vector& secret, absl::string_view host, absl::string_view expires, absl::string_view token = "", absl::string_view id_token = "", absl::string_view refresh_token = "") { - if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.hmac_base64_encoding_only")) { - return encodeHmacBase64(secret, host, expires, token, id_token, refresh_token); - } else { - return encodeHmacHexBase64(secret, host, expires, token, id_token, refresh_token); - } + return encodeHmacBase64(secret, host, expires, token, id_token, refresh_token); } } // namespace @@ -683,7 +678,6 @@ void OAuth2Filter::addResponseCookies(Http::ResponseHeaderMap& headers, } // We use HTTP Only cookies. - const std::string cookie_tail = fmt::format(CookieTailFormatString, max_age); const std::string cookie_tail_http_only = fmt::format(CookieTailHttpOnlyFormatString, max_age); const CookieNames& cookie_names = config_->cookieNames(); @@ -697,17 +691,13 @@ void OAuth2Filter::addResponseCookies(Http::ResponseHeaderMap& headers, // If opted-in, we also create a new Bearer cookie for the authorization token provided by the // auth server. if (config_->forwardBearerToken()) { - std::string cookie_attribute_httponly = - Runtime::runtimeFeatureEnabled("envoy.reloadable_features.oauth_make_token_cookie_httponly") - ? cookie_tail_http_only - : cookie_tail; headers.addReferenceKey( Http::Headers::get().SetCookie, - absl::StrCat(cookie_names.bearer_token_, "=", access_token_, cookie_attribute_httponly)); + absl::StrCat(cookie_names.bearer_token_, "=", access_token_, cookie_tail_http_only)); if (!id_token_.empty()) { headers.addReferenceKey( Http::Headers::get().SetCookie, - absl::StrCat(cookie_names.id_token_, "=", id_token_, cookie_attribute_httponly)); + absl::StrCat(cookie_names.id_token_, "=", id_token_, cookie_tail_http_only)); } if (!refresh_token_.empty()) { diff --git a/test/extensions/filters/common/ext_authz/ext_authz_http_impl_test.cc b/test/extensions/filters/common/ext_authz/ext_authz_http_impl_test.cc index a093cb710c44..ad5aa4d809ba 100644 --- a/test/extensions/filters/common/ext_authz/ext_authz_http_impl_test.cc +++ b/test/extensions/filters/common/ext_authz/ext_authz_http_impl_test.cc @@ -259,6 +259,17 @@ TEST_F(ExtAuthzHttpClientTest, TestDefaultAllowedClientAndUpstreamHeaders) { config_->upstreamHeaderMatchers()->matches(Http::Headers::get().ContentLength.get())); } +TEST_F(ExtAuthzHttpClientTest, PathPrefixShouldBeSanitized) { + auto empty_config = createConfig(EMPTY_STRING, 250, ""); + EXPECT_TRUE(empty_config->pathPrefix().empty()); + + auto slash_prefix_config = createConfig(EMPTY_STRING, 250, "/the_prefix"); + EXPECT_EQ(slash_prefix_config->pathPrefix(), "/the_prefix"); + + EXPECT_THROW_WITH_MESSAGE(createConfig(EMPTY_STRING, 250, "the_prefix"), EnvoyException, + "path_prefix should start with \"/\"."); +} + // Verify client response when the authorization server returns a 200 OK and path_prefix is // configured. TEST_F(ExtAuthzHttpClientTest, AuthorizationOkWithPathRewrite) { diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc index 78a49df1e710..5c889533ee3c 100644 --- a/test/extensions/filters/http/oauth2/filter_test.cc +++ b/test/extensions/filters/http/oauth2/filter_test.cc @@ -679,39 +679,101 @@ TEST_F(OAuth2Test, OAuthErrorNonOAuthHttpCallbackLegacyEncoding) { TestScopedRuntime scoped_runtime; scoped_runtime.mergeValues({ {"envoy.reloadable_features.oauth_use_url_encoding", "false"}, + {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, }); init(); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Path.get(), "/not/_oauth"}, + // First construct the initial request to the oauth filter with URI parameters. + Http::TestRequestHeaderMapImpl first_request_headers{ + {Http::Headers::get().Path.get(), "/test?name=admin&level=trace"}, {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "http"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, + {Http::Headers::get().Scheme.get(), "https"}, }; - Http::TestResponseHeaderMapImpl response_headers{ + // This is the immediate response - a redirect to the auth cluster. + Http::TestResponseHeaderMapImpl first_response_headers{ {Http::Headers::get().Status.get(), "302"}, {Http::Headers::get().Location.get(), "https://auth.example.com/oauth/" "authorize/?client_id=" + TEST_CLIENT_ID + - "&redirect_uri=http%3A%2F%2Ftraffic.example.com%2F_oauth" + "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" "&response_type=code" "&scope=" + TEST_ENCODED_AUTH_SCOPES + - "&state=http%3A%2F%2Ftraffic.example.com%2Fnot%2F_oauth" + "&state=https%3A%2F%2Ftraffic.example.com%2Ftest%3Fname%3Dadmin%26level%3Dtrace" "&resource=oauth2-resource&resource=http%3A%2F%2Fexample.com" "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%2F..%2F%2Futf8%C3%83;foo%3Dbar%" "3Fvar1%3D1%26var2%3D2"}, }; - // explicitly tell the validator to fail the validation + // Fail the validation to trigger the OAuth flow. EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); + // Check that the redirect includes the escaped parameter characters, '?', '&' and '='. + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&first_response_headers), true)); + // This represents the beginning of the OAuth filter. EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(request_headers, false)); + filter_->decodeHeaders(first_request_headers, false)); + + // This represents the callback request from the authorization server. + Http::TestRequestHeaderMapImpl second_request_headers{ + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=https%3A%2F%2Ftraffic.example.com%" + "2Ftest%3Fname%3Dadmin%26level%3Dtrace"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + }; + + // Deliberately fail the HMAC validation check. + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + + EXPECT_CALL(*oauth_client_, asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", + "https://traffic.example.com" + TEST_CALLBACK, + AuthType::UrlEncodedBody)); + + // Invoke the callback logic. As a side effect, state_ will be populated. + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, + filter_->decodeHeaders(second_request_headers, false)); + + EXPECT_EQ(1, config_->stats().oauth_unauthorized_rq_.value()); + EXPECT_EQ(config_->clusterName(), "auth.example.com"); + + // Expected response after the callback & validation is complete - verifying we kept the + // state and method of the original request, including the query string parameters. + Http::TestRequestHeaderMapImpl second_response_headers{ + {Http::Headers::get().Status.get(), "302"}, + {Http::Headers::get().SetCookie.get(), "OauthHMAC=" + "fV62OgLipChTQQC3UFgDp+l5sCiSb3zt7nCoJiVivWw=;" + "path=/;Max-Age=;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), "OauthExpires=;path=/;Max-Age=;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), "BearerToken=;path=/;Max-Age=;secure;HttpOnly"}, + {Http::Headers::get().Location.get(), + "https://traffic.example.com/test?name=admin&level=trace"}, + }; + + EXPECT_CALL(decoder_callbacks_, + encodeHeaders_(HeaderMapEqualRef(&second_response_headers), true)); + + filter_->finishGetAccessTokenFlow(); + + // Deliberately fail the HMAC validation check. + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + + EXPECT_CALL(*oauth_client_, asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", + "https://traffic.example.com" + TEST_CALLBACK, + AuthType::UrlEncodedBody)); + + // Invoke the callback logic. As a side effect, state_ will be populated. + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, + filter_->decodeHeaders(second_request_headers, false)); + + EXPECT_EQ(1, config_->stats().oauth_unauthorized_rq_.value()); + EXPECT_EQ(config_->clusterName(), "auth.example.com"); } TEST_F(OAuth2Test, OAuthErrorNonOAuthHttpCallback) { @@ -878,192 +940,86 @@ TEST_F(OAuth2Test, CookieValidatorWithCustomNames) { // Validates the behavior of the cookie validator when the combination of some fields could be same. TEST_F(OAuth2Test, CookieValidatorSame) { - { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); - auto cookie_names = - CookieNames{"BearerToken", "OauthHMAC", "OauthExpires", "IdToken", "RefreshToken"}; - const auto expires_at_s = DateUtil::nowToSeconds(test_time_.timeSystem()) + 5; - - // Host name is `traffic.example.com:101` and the expire time is 5. - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Host.get(), "traffic.example.com:101"}, - {Http::Headers::get().Path.get(), "/anypath"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Cookie.get(), - fmt::format("{}={}", cookie_names.oauth_expires_, expires_at_s)}, - {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=xyztoken")}, - {Http::Headers::get().Cookie.get(), - absl::StrCat(cookie_names.oauth_hmac_, "=MSq8mkNQGdXx2LKGlLHMwSIj8rLZRnrHE6EWvvTUFx0=")}, - }; - - auto cookie_validator = std::make_shared(test_time_, cookie_names); - EXPECT_EQ(cookie_validator->token(), ""); - cookie_validator->setParams(request_headers, "mock-secret"); - - EXPECT_TRUE(cookie_validator->hmacIsValid()); - EXPECT_TRUE(cookie_validator->timestampIsValid()); - EXPECT_TRUE(cookie_validator->isValid()); - - // If we advance time beyond 5s the timestamp should no longer be valid. - test_time_.advanceTimeWait(std::chrono::seconds(6)); - - EXPECT_FALSE(cookie_validator->timestampIsValid()); - EXPECT_FALSE(cookie_validator->isValid()); - - test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); - const auto new_expires_at_s = DateUtil::nowToSeconds(test_time_.timeSystem()) + 15; - - // Host name is `traffic.example.com:10` and the expire time is 15. - // HMAC should be different from the above one with the separator fix. - Http::TestRequestHeaderMapImpl request_headers_second{ - {Http::Headers::get().Host.get(), "traffic.example.com:10"}, - {Http::Headers::get().Path.get(), "/anypath"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Cookie.get(), - fmt::format("{}={}", cookie_names.oauth_expires_, new_expires_at_s)}, - {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=xyztoken")}, - {Http::Headers::get().Cookie.get(), - absl::StrCat(cookie_names.oauth_hmac_, "=dbl04CSr6eWF52wdNDCRt/Uw6A4y41wbpmtUWRyD2Fo=")}, - }; - - cookie_validator->setParams(request_headers_second, "mock-secret"); - - EXPECT_TRUE(cookie_validator->hmacIsValid()); - EXPECT_TRUE(cookie_validator->timestampIsValid()); - EXPECT_TRUE(cookie_validator->isValid()); - - // If we advance time beyond 15s the timestamp should no longer be valid. - test_time_.advanceTimeWait(std::chrono::seconds(16)); - - EXPECT_FALSE(cookie_validator->timestampIsValid()); - EXPECT_FALSE(cookie_validator->isValid()); - } - { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); - auto cookie_names = - CookieNames{"BearerToken", "OauthHMAC", "OauthExpires", "IdToken", "RefreshToken"}; - const auto expires_at_s = DateUtil::nowToSeconds(test_time_.timeSystem()) + 5; - - // Host name is `traffic.example.com:101` and the expire time is 5. - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Host.get(), "traffic.example.com:101"}, - {Http::Headers::get().Path.get(), "/anypath"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Cookie.get(), - fmt::format("{}={}", cookie_names.oauth_expires_, expires_at_s)}, - {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=xyztoken")}, - {Http::Headers::get().Cookie.get(), + test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); + auto cookie_names = + CookieNames{"BearerToken", "OauthHMAC", "OauthExpires", "IdToken", "RefreshToken"}; + const auto expires_at_s = DateUtil::nowToSeconds(test_time_.timeSystem()) + 5; - absl::StrCat(cookie_names.oauth_hmac_, "=" - "MzEyYWJjOWE0MzUwMTlkNWYxZDhiMjg2OTRiMWNjYzEyMjIzZj" - "JiMmQ5NDY3YWM3MTNhMTE2YmVmNGQ0MTcxZA==")}, - }; + // Host name is `traffic.example.com:101` and the expire time is 5. + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Host.get(), "traffic.example.com:101"}, + {Http::Headers::get().Path.get(), "/anypath"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Cookie.get(), + fmt::format("{}={}", cookie_names.oauth_expires_, expires_at_s)}, + {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=xyztoken")}, + {Http::Headers::get().Cookie.get(), + absl::StrCat(cookie_names.oauth_hmac_, "=MSq8mkNQGdXx2LKGlLHMwSIj8rLZRnrHE6EWvvTUFx0=")}, + }; - auto cookie_validator = std::make_shared(test_time_, cookie_names); - EXPECT_EQ(cookie_validator->token(), ""); - cookie_validator->setParams(request_headers, "mock-secret"); + auto cookie_validator = std::make_shared(test_time_, cookie_names); + EXPECT_EQ(cookie_validator->token(), ""); + cookie_validator->setParams(request_headers, "mock-secret"); - EXPECT_TRUE(cookie_validator->hmacIsValid()); - EXPECT_TRUE(cookie_validator->timestampIsValid()); - EXPECT_TRUE(cookie_validator->isValid()); + EXPECT_TRUE(cookie_validator->hmacIsValid()); + EXPECT_TRUE(cookie_validator->timestampIsValid()); + EXPECT_TRUE(cookie_validator->isValid()); - // If we advance time beyond 5s the timestamp should no longer be valid. - test_time_.advanceTimeWait(std::chrono::seconds(6)); + // If we advance time beyond 5s the timestamp should no longer be valid. + test_time_.advanceTimeWait(std::chrono::seconds(6)); - EXPECT_FALSE(cookie_validator->timestampIsValid()); - EXPECT_FALSE(cookie_validator->isValid()); + EXPECT_FALSE(cookie_validator->timestampIsValid()); + EXPECT_FALSE(cookie_validator->isValid()); - test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); - const auto new_expires_at_s = DateUtil::nowToSeconds(test_time_.timeSystem()) + 15; + test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); + const auto new_expires_at_s = DateUtil::nowToSeconds(test_time_.timeSystem()) + 15; - // Host name is `traffic.example.com:10` and the expire time is 15. - // HMAC should be different from the above one with the separator fix. - Http::TestRequestHeaderMapImpl request_headers_second{ - {Http::Headers::get().Host.get(), "traffic.example.com:10"}, - {Http::Headers::get().Path.get(), "/anypath"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Cookie.get(), - fmt::format("{}={}", cookie_names.oauth_expires_, new_expires_at_s)}, - {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=xyztoken")}, - {Http::Headers::get().Cookie.get(), - absl::StrCat(cookie_names.oauth_hmac_, "=" - "NzViOTc0ZTAyNGFiZTllNTg1ZTc2YzFkMzQzMDkxYjdmNTMwZT" - "gwZTMyZTM1YzFiYTY2YjU0NTkxYzgzZDg1YQ==")}, - }; + // Host name is `traffic.example.com:10` and the expire time is 15. + // HMAC should be different from the above one with the separator fix. + Http::TestRequestHeaderMapImpl request_headers_second{ + {Http::Headers::get().Host.get(), "traffic.example.com:10"}, + {Http::Headers::get().Path.get(), "/anypath"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Cookie.get(), + fmt::format("{}={}", cookie_names.oauth_expires_, new_expires_at_s)}, + {Http::Headers::get().Cookie.get(), absl::StrCat(cookie_names.bearer_token_, "=xyztoken")}, + {Http::Headers::get().Cookie.get(), + absl::StrCat(cookie_names.oauth_hmac_, "=dbl04CSr6eWF52wdNDCRt/Uw6A4y41wbpmtUWRyD2Fo=")}, + }; - cookie_validator->setParams(request_headers_second, "mock-secret"); + cookie_validator->setParams(request_headers_second, "mock-secret"); - EXPECT_TRUE(cookie_validator->hmacIsValid()); - EXPECT_TRUE(cookie_validator->timestampIsValid()); - EXPECT_TRUE(cookie_validator->isValid()); + EXPECT_TRUE(cookie_validator->hmacIsValid()); + EXPECT_TRUE(cookie_validator->timestampIsValid()); + EXPECT_TRUE(cookie_validator->isValid()); - // If we advance time beyond 15s the timestamp should no longer be valid. - test_time_.advanceTimeWait(std::chrono::seconds(16)); + // If we advance time beyond 15s the timestamp should no longer be valid. + test_time_.advanceTimeWait(std::chrono::seconds(16)); - EXPECT_FALSE(cookie_validator->timestampIsValid()); - EXPECT_FALSE(cookie_validator->isValid()); - } + EXPECT_FALSE(cookie_validator->timestampIsValid()); + EXPECT_FALSE(cookie_validator->isValid()); } // Validates the behavior of the cookie validator when the expires_at value is not a valid integer. TEST_F(OAuth2Test, CookieValidatorInvalidExpiresAt) { - { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Path.get(), "/anypath"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Cookie.get(), "OauthExpires=notanumber"}, - {Http::Headers::get().Cookie.get(), "BearerToken=xyztoken"}, - {Http::Headers::get().Cookie.get(), "OauthHMAC=" - "c+1qzyrMmqG8+O4dn7b28OvNNDWcb04yJfNbZCE1zYE="}, - }; - - auto cookie_validator = std::make_shared( - test_time_, - CookieNames{"BearerToken", "OauthHMAC", "OauthExpires", "IdToken", "RefreshToken"}); - cookie_validator->setParams(request_headers, "mock-secret"); - - EXPECT_TRUE(cookie_validator->hmacIsValid()); - EXPECT_FALSE(cookie_validator->timestampIsValid()); - EXPECT_FALSE(cookie_validator->isValid()); - } - { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Path.get(), "/anypath"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Cookie.get(), "OauthExpires=notanumber"}, - {Http::Headers::get().Cookie.get(), "BearerToken=xyztoken"}, - {Http::Headers::get().Cookie.get(), "OauthHMAC=" - "NzNlZDZhY2YyYWNjOWFhMWJjZjhlZTFkOWZiNmY2ZjBlYmNkMzQzNT" - "ljNmY0ZTMyMjVmMzViNjQyMTM1Y2Q4MQ=="}, - }; + Http::TestRequestHeaderMapImpl request_headers{ + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Path.get(), "/anypath"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Cookie.get(), "OauthExpires=notanumber"}, + {Http::Headers::get().Cookie.get(), "BearerToken=xyztoken"}, + {Http::Headers::get().Cookie.get(), "OauthHMAC=" + "c+1qzyrMmqG8+O4dn7b28OvNNDWcb04yJfNbZCE1zYE="}, + }; - auto cookie_validator = std::make_shared( - test_time_, - CookieNames{"BearerToken", "OauthHMAC", "OauthExpires", "IdToken", "RefreshToken"}); - cookie_validator->setParams(request_headers, "mock-secret"); + auto cookie_validator = std::make_shared( + test_time_, + CookieNames{"BearerToken", "OauthHMAC", "OauthExpires", "IdToken", "RefreshToken"}); + cookie_validator->setParams(request_headers, "mock-secret"); - EXPECT_TRUE(cookie_validator->hmacIsValid()); - EXPECT_FALSE(cookie_validator->timestampIsValid()); - EXPECT_FALSE(cookie_validator->isValid()); - } + EXPECT_TRUE(cookie_validator->hmacIsValid()); + EXPECT_FALSE(cookie_validator->timestampIsValid()); + EXPECT_FALSE(cookie_validator->isValid()); } // Validates the behavior of the cookie validator when the expires_at value is not a valid integer. @@ -1348,7 +1304,6 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParametersLegacyEncoding) { {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, {Http::Headers::get().Scheme.get(), "https"}, }; - // Deliberately fail the HMAC validation check. EXPECT_CALL(*validator_, setParams(_, _)); EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); @@ -1383,96 +1338,7 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParametersLegacyEncoding) { filter_->finishGetAccessTokenFlow(); } - { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.oauth_use_url_encoding", "false"}, - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - init(); - // First construct the initial request to the oauth filter with URI parameters. - Http::TestRequestHeaderMapImpl first_request_headers{ - {Http::Headers::get().Path.get(), "/test?name=admin&level=trace"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, - {Http::Headers::get().Scheme.get(), "https"}, - }; - - // This is the immediate response - a redirect to the auth cluster. - Http::TestResponseHeaderMapImpl first_response_headers{ - {Http::Headers::get().Status.get(), "302"}, - {Http::Headers::get().Location.get(), - "https://auth.example.com/oauth/" - "authorize/?client_id=" + - TEST_CLIENT_ID + - "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" - "&response_type=code" - "&scope=" + - TEST_ENCODED_AUTH_SCOPES + - "&state=https%3A%2F%2Ftraffic.example.com%2Ftest%3Fname%3Dadmin%26level%3Dtrace" - "&resource=oauth2-resource&resource=http%3A%2F%2Fexample.com" - "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%2F..%2F%2Futf8%C3%83;foo%3Dbar%" - "3Fvar1%3D1%26var2%3D2"}, - }; - - // Fail the validation to trigger the OAuth flow. - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - - // Check that the redirect includes the escaped parameter characters, '?', '&' and '='. - EXPECT_CALL(decoder_callbacks_, - encodeHeaders_(HeaderMapEqualRef(&first_response_headers), true)); - - // This represents the beginning of the OAuth filter. - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(first_request_headers, false)); - - // This represents the callback request from the authorization server. - Http::TestRequestHeaderMapImpl second_request_headers{ - {Http::Headers::get().Path.get(), - "/_oauth?code=123&state=https%3A%2F%2Ftraffic.example.com%" - "2Ftest%3Fname%3Dadmin%26level%3Dtrace"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - }; - - // Deliberately fail the HMAC validation check. - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - - EXPECT_CALL(*oauth_client_, - asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", - "https://traffic.example.com" + TEST_CALLBACK, - AuthType::UrlEncodedBody)); - - // Invoke the callback logic. As a side effect, state_ will be populated. - EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, - filter_->decodeHeaders(second_request_headers, false)); - - EXPECT_EQ(2, config_->stats().oauth_unauthorized_rq_.value()); - EXPECT_EQ(config_->clusterName(), "auth.example.com"); - - // Expected response after the callback & validation is complete - verifying we kept the - // state and method of the original request, including the query string parameters. - Http::TestRequestHeaderMapImpl second_response_headers{ - {Http::Headers::get().Status.get(), "302"}, - {Http::Headers::get().SetCookie.get(), - "OauthHMAC=" - "N2Q1ZWI2M2EwMmUyYTQyODUzNDEwMGI3NTA1ODAzYTdlOTc5YjAyODkyNmY3Y2VkZWU3MGE4MjYyNTYyYmQ2Yw==;" - "path=/;Max-Age=;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), "OauthExpires=;path=/;Max-Age=;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), "BearerToken=;path=/;Max-Age=;secure;HttpOnly"}, - {Http::Headers::get().Location.get(), - "https://traffic.example.com/test?name=admin&level=trace"}, - }; - - EXPECT_CALL(decoder_callbacks_, - encodeHeaders_(HeaderMapEqualRef(&second_response_headers), true)); - - filter_->finishGetAccessTokenFlow(); - } -} +} TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParametersFillRefreshAndIdToken) { // First construct the initial request to the oauth filter with URI parameters. @@ -1566,189 +1432,87 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParametersFillRefreshAndIdToken) { // This test adds %-encoded UTF-8 characters to the URL and shows that // the new decoding correctly handles that case. TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParameters) { - { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.oauth_use_url_encoding", "true"}, - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - init(); - // First construct the initial request to the oauth filter with URI parameters. - Http::TestRequestHeaderMapImpl first_request_headers{ - {Http::Headers::get().Path.get(), "/test/utf8%C3%83?name=admin&level=trace"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, - {Http::Headers::get().Scheme.get(), "https"}, - }; - - // This is the immediate response - a redirect to the auth cluster. - Http::TestResponseHeaderMapImpl first_response_headers{ - {Http::Headers::get().Status.get(), "302"}, - {Http::Headers::get().Location.get(), - "https://auth.example.com/oauth/" - "authorize/?client_id=" + - TEST_CLIENT_ID + - "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" - "&response_type=code" - "&scope=" + - TEST_ENCODED_AUTH_SCOPES + - "&state=https%3A%2F%2Ftraffic.example.com%2Ftest%2Futf8%25C3%2583%3Fname%3Dadmin%" - "26level%3Dtrace" - "&resource=oauth2-resource&resource=http%3A%2F%2Fexample.com" - "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%252F..%252F%2Futf8%C3%83%3Bfoo%" - "3Dbar%" - "3Fvar1%3D1%26var2%3D2"}, - }; - - // Fail the validation to trigger the OAuth flow. - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - - // Check that the redirect includes the escaped parameter characters using URL encoding. - EXPECT_CALL(decoder_callbacks_, - encodeHeaders_(HeaderMapEqualRef(&first_response_headers), true)); - - // This represents the beginning of the OAuth filter. - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(first_request_headers, false)); - - // This represents the callback request from the authorization server. - Http::TestRequestHeaderMapImpl second_request_headers{ - {Http::Headers::get().Path.get(), - "/_oauth?code=123&state=https%3A%2F%2Ftraffic.example.com%" - "2Ftest%2Futf8%25C3%2583%3Fname%3Dadmin%26level%3Dtrace"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - }; - - // Deliberately fail the HMAC validation check. - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - - EXPECT_CALL(*oauth_client_, - asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", - "https://traffic.example.com" + TEST_CALLBACK, - AuthType::UrlEncodedBody)); - - // Invoke the callback logic. As a side effect, state_ will be populated. - EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, - filter_->decodeHeaders(second_request_headers, false)); - - EXPECT_EQ(1, config_->stats().oauth_unauthorized_rq_.value()); - EXPECT_EQ(config_->clusterName(), "auth.example.com"); - - // Expected response after the callback & validation is complete - verifying we kept the - // state and method of the original request, including the query string parameters and UTF8 - // sequences. - Http::TestRequestHeaderMapImpl second_response_headers{ - {Http::Headers::get().Status.get(), "302"}, - {Http::Headers::get().SetCookie.get(), "OauthHMAC=" - "fV62OgLipChTQQC3UFgDp+l5sCiSb3zt7nCoJiVivWw=;" - "path=/;Max-Age=;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), "OauthExpires=;path=/;Max-Age=;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), "BearerToken=;path=/;Max-Age=;secure;HttpOnly"}, - {Http::Headers::get().Location.get(), - "https://traffic.example.com/test/utf8%C3%83?name=admin&level=trace"}, - }; - - EXPECT_CALL(decoder_callbacks_, - encodeHeaders_(HeaderMapEqualRef(&second_response_headers), true)); - - filter_->finishGetAccessTokenFlow(); - } - { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.oauth_use_url_encoding", "true"}, - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - init(); - // First construct the initial request to the oauth filter with URI parameters. - Http::TestRequestHeaderMapImpl first_request_headers{ - {Http::Headers::get().Path.get(), "/test/utf8%C3%83?name=admin&level=trace"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, - {Http::Headers::get().Scheme.get(), "https"}, - }; + init(); + // First construct the initial request to the oauth filter with URI parameters. + Http::TestRequestHeaderMapImpl first_request_headers{ + {Http::Headers::get().Path.get(), "/test/utf8%C3%83?name=admin&level=trace"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Post}, + {Http::Headers::get().Scheme.get(), "https"}, + }; - // This is the immediate response - a redirect to the auth cluster. - Http::TestResponseHeaderMapImpl first_response_headers{ - {Http::Headers::get().Status.get(), "302"}, - {Http::Headers::get().Location.get(), - "https://auth.example.com/oauth/" - "authorize/?client_id=" + - TEST_CLIENT_ID + - "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" - "&response_type=code" - "&scope=" + - TEST_ENCODED_AUTH_SCOPES + - "&state=https%3A%2F%2Ftraffic.example.com%2Ftest%2Futf8%25C3%2583%3Fname%3Dadmin%" - "26level%3Dtrace" - "&resource=oauth2-resource&resource=http%3A%2F%2Fexample.com" - "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%252F..%252F%2Futf8%C3%83%3Bfoo%" - "3Dbar%" - "3Fvar1%3D1%26var2%3D2"}, - }; + // This is the immediate response - a redirect to the auth cluster. + Http::TestResponseHeaderMapImpl first_response_headers{ + {Http::Headers::get().Status.get(), "302"}, + {Http::Headers::get().Location.get(), + "https://auth.example.com/oauth/" + "authorize/?client_id=" + + TEST_CLIENT_ID + + "&redirect_uri=https%3A%2F%2Ftraffic.example.com%2F_oauth" + "&response_type=code" + "&scope=" + + TEST_ENCODED_AUTH_SCOPES + + "&state=https%3A%2F%2Ftraffic.example.com%2Ftest%2Futf8%25C3%2583%3Fname%3Dadmin%" + "26level%3Dtrace" + "&resource=oauth2-resource&resource=http%3A%2F%2Fexample.com" + "&resource=https%3A%2F%2Fexample.com%2Fsome%2Fpath%252F..%252F%2Futf8%C3%83%3Bfoo%" + "3Dbar%" + "3Fvar1%3D1%26var2%3D2"}, + }; - // Fail the validation to trigger the OAuth flow. - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + // Fail the validation to trigger the OAuth flow. + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - // Check that the redirect includes the escaped parameter characters using URL encoding. - EXPECT_CALL(decoder_callbacks_, - encodeHeaders_(HeaderMapEqualRef(&first_response_headers), true)); + // Check that the redirect includes the escaped parameter characters using URL encoding. + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&first_response_headers), true)); - // This represents the beginning of the OAuth filter. - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(first_request_headers, false)); + // This represents the beginning of the OAuth filter. + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(first_request_headers, false)); - // This represents the callback request from the authorization server. - Http::TestRequestHeaderMapImpl second_request_headers{ - {Http::Headers::get().Path.get(), - "/_oauth?code=123&state=https%3A%2F%2Ftraffic.example.com%" - "2Ftest%2Futf8%25C3%2583%3Fname%3Dadmin%26level%3Dtrace"}, - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - {Http::Headers::get().Scheme.get(), "https"}, - }; + // This represents the callback request from the authorization server. + Http::TestRequestHeaderMapImpl second_request_headers{ + {Http::Headers::get().Path.get(), "/_oauth?code=123&state=https%3A%2F%2Ftraffic.example.com%" + "2Ftest%2Futf8%25C3%2583%3Fname%3Dadmin%26level%3Dtrace"}, + {Http::Headers::get().Host.get(), "traffic.example.com"}, + {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, + {Http::Headers::get().Scheme.get(), "https"}, + }; - // Deliberately fail the HMAC validation check. - EXPECT_CALL(*validator_, setParams(_, _)); - EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); + // Deliberately fail the HMAC validation check. + EXPECT_CALL(*validator_, setParams(_, _)); + EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false)); - EXPECT_CALL(*oauth_client_, - asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", - "https://traffic.example.com" + TEST_CALLBACK, - AuthType::UrlEncodedBody)); + EXPECT_CALL(*oauth_client_, asyncGetAccessToken("123", TEST_CLIENT_ID, "asdf_client_secret_fdsa", + "https://traffic.example.com" + TEST_CALLBACK, + AuthType::UrlEncodedBody)); - // Invoke the callback logic. As a side effect, state_ will be populated. - EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, - filter_->decodeHeaders(second_request_headers, false)); + // Invoke the callback logic. As a side effect, state_ will be populated. + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndBuffer, + filter_->decodeHeaders(second_request_headers, false)); - EXPECT_EQ(2, config_->stats().oauth_unauthorized_rq_.value()); - EXPECT_EQ(config_->clusterName(), "auth.example.com"); + EXPECT_EQ(1, config_->stats().oauth_unauthorized_rq_.value()); + EXPECT_EQ(config_->clusterName(), "auth.example.com"); - // Expected response after the callback & validation is complete - verifying we kept the - // state and method of the original request, including the query string parameters and UTF8 - // sequences. - Http::TestRequestHeaderMapImpl second_response_headers{ - {Http::Headers::get().Status.get(), "302"}, - {Http::Headers::get().SetCookie.get(), - "OauthHMAC=" - "N2Q1ZWI2M2EwMmUyYTQyODUzNDEwMGI3NTA1ODAzYTdlOTc5YjAyODkyNmY3Y2VkZWU3MGE4MjYyNTYyYmQ2Yw==;" - "path=/;Max-Age=;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), "OauthExpires=;path=/;Max-Age=;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), "BearerToken=;path=/;Max-Age=;secure;HttpOnly"}, - {Http::Headers::get().Location.get(), - "https://traffic.example.com/test/utf8%C3%83?name=admin&level=trace"}, - }; + // Expected response after the callback & validation is complete - verifying we kept the + // state and method of the original request, including the query string parameters and UTF8 + // sequences. + Http::TestRequestHeaderMapImpl second_response_headers{ + {Http::Headers::get().Status.get(), "302"}, + {Http::Headers::get().SetCookie.get(), "OauthHMAC=" + "fV62OgLipChTQQC3UFgDp+l5sCiSb3zt7nCoJiVivWw=;" + "path=/;Max-Age=;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), "OauthExpires=;path=/;Max-Age=;secure;HttpOnly"}, + {Http::Headers::get().SetCookie.get(), "BearerToken=;path=/;Max-Age=;secure;HttpOnly"}, + {Http::Headers::get().Location.get(), + "https://traffic.example.com/test/utf8%C3%83?name=admin&level=trace"}, + }; - EXPECT_CALL(decoder_callbacks_, - encodeHeaders_(HeaderMapEqualRef(&second_response_headers), true)); + EXPECT_CALL(decoder_callbacks_, + encodeHeaders_(HeaderMapEqualRef(&second_response_headers), true)); - filter_->finishGetAccessTokenFlow(); - } + filter_->finishGetAccessTokenFlow(); } /** @@ -1759,20 +1523,8 @@ TEST_F(OAuth2Test, OAuthTestFullFlowPostWithParameters) { std::string oauthHMAC; -TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokens) { - TestScopedRuntime scoped_runtime; - if (GetParam() == 1) { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - oauthHMAC = - "ZTEzMmIyYzRmNTdmMTdiY2IyYmViZDE3ODA5ZDliOTE2MTRlNzNjYjc4MjBlMTVlOWY1OTM2ZjViZjM4MzAwNA==;"; - } else { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - oauthHMAC = "4TKyxPV/F7yyvr0XgJ2bkWFOc8t4IOFen1k29b84MAQ=;"; - } +TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokens) { + oauthHMAC = "4TKyxPV/F7yyvr0XgJ2bkWFOc8t4IOFen1k29b84MAQ=;"; // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); @@ -1806,21 +1558,9 @@ TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokens) { std::chrono::seconds(600)); } -TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshToken) { +TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshToken) { init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */)); - TestScopedRuntime scoped_runtime; - if (GetParam() == 1) { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - oauthHMAC = - "ZTEzMmIyYzRmNTdmMTdiY2IyYmViZDE3ODA5ZDliOTE2MTRlNzNjYjc4MjBlMTVlOWY1OTM2ZjViZjM4MzAwNA==;"; - } else { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - oauthHMAC = "4TKyxPV/F7yyvr0XgJ2bkWFOc8t4IOFen1k29b84MAQ=;"; - } + oauthHMAC = "4TKyxPV/F7yyvr0XgJ2bkWFOc8t4IOFen1k29b84MAQ=;"; // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); @@ -1854,25 +1594,14 @@ TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshToken) { std::chrono::seconds(600)); } -TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndDefaultRefreshTokenExpiresIn) { +TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndDefaultRefreshTokenExpiresIn) { init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, 1200 /* default_refresh_token_expires_in */)); TestScopedRuntime scoped_runtime; - if (GetParam() == 1) { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - oauthHMAC = - "ZTEzMmIyYzRmNTdmMTdiY2IyYmViZDE3ODA5ZDliOTE2MTRlNzNjYjc4MjBlMTVlOWY1OTM2ZjViZjM4MzAwNA==;"; - } else { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - oauthHMAC = "4TKyxPV/F7yyvr0XgJ2bkWFOc8t4IOFen1k29b84MAQ=;"; - } + oauthHMAC = "4TKyxPV/F7yyvr0XgJ2bkWFOc8t4IOFen1k29b84MAQ=;"; // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); @@ -1913,25 +1642,13 @@ TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndDefaultRefr * refresh token. */ -TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndRefreshTokenExpiresInFromJwt) { +TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndRefreshTokenExpiresInFromJwt) { init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, 1200 /* default_refresh_token_expires_in */)); - TestScopedRuntime scoped_runtime; - if (GetParam() == 1) { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - oauthHMAC = - "MGE2YWQyNjU0YjBmMTA1ZDQzZTE0ODA0OWVlY2Y2Nzc2YjNjZWZjNjI3MDI4M2E5YzUwMGFkMTNkMmM5ZjNkMw==;"; - } else { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - oauthHMAC = "CmrSZUsPEF1D4UgEnuz2d2s878YnAoOpxQCtE9LJ89M=;"; - } + oauthHMAC = "CmrSZUsPEF1D4UgEnuz2d2s878YnAoOpxQCtE9LJ89M=;"; // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); @@ -1977,25 +1694,14 @@ TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndRefreshToke * Expected behavior: The age of the cookie with refresh token is equal to zero. */ -TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndExpiredRefreshToken) { +TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndExpiredRefreshToken) { init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, 1200 /* default_refresh_token_expires_in */)); TestScopedRuntime scoped_runtime; - if (GetParam() == 1) { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - oauthHMAC = - "ZWY3NDZlMDcwNTM3MmIxZmZiNDRmZTBkZDcyY2JlZjEwOWUxMDExOGMwZDc5NDBlYTMxNzRhMGZiY2U0ZDY5Mg==;"; - } else { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - oauthHMAC = "73RuBwU3Kx/7RP4N1yy+8QnhARjA15QOoxdKD7zk1pI=;"; - } + oauthHMAC = "73RuBwU3Kx/7RP4N1yy+8QnhARjA15QOoxdKD7zk1pI=;"; // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. test_time_.setSystemTime(SystemTime(std::chrono::seconds(2554515000))); @@ -2041,25 +1747,14 @@ TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndExpiredRefr * Expected behavior: The age of the cookie with refresh token is equal to default value. */ -TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndNoExpClaimInRefreshToken) { +TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndNoExpClaimInRefreshToken) { init(getConfig(true /* forward_bearer_token */, true /* use_refresh_token */, ::envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType:: OAuth2Config_AuthType_URL_ENCODED_BODY /* encoded_body_type */, 1200 /* default_refresh_token_expires_in */)); TestScopedRuntime scoped_runtime; - if (GetParam() == 1) { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - oauthHMAC = - "N2FlNDRlNzQwZjgyNmI4YTdmZjQ5YTBjOWQ3ZTc0N2UyYTg3YTJiOTg4NThmZmQyZjg1YjFlZmIwMGZlNTdjMg==;"; - } else { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - oauthHMAC = "euROdA+Ca4p/9JoMnX50fiqHormIWP/S+Fse+wD+V8I=;"; - } + oauthHMAC = "euROdA+Ca4p/9JoMnX50fiqHormIWP/S+Fse+wD+V8I=;"; // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); @@ -2098,26 +1793,13 @@ TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokensUseRefreshTokenAndNoExpClaimI std::chrono::seconds(600)); } -INSTANTIATE_TEST_SUITE_P(EndcodingParams, OAuth2Test, testing::Values(1, 0)); - -TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokens_oauth_use_standard_max_age_value) { +TEST_F(OAuth2Test, OAuthAccessTokenSucessWithTokens_oauth_use_standard_max_age_value) { TestScopedRuntime scoped_runtime; scoped_runtime.mergeValues({ {"envoy.reloadable_features.oauth_use_standard_max_age_value", "false"}, }); - if (GetParam() == 1) { - oauthHMAC = - "ZmMzNzFkOWVkY2ZmNzc3M2NjYjk0ZTA0NDM4YTlkOWIxMTUxNmI3NDkyMGRkYjM1Mzg4YTBiMzc4NGRmOWU4Mw==;"; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - } else { - oauthHMAC = "/Dcdntz/d3PMuU4EQ4qdmxFRa3SSDds1OIoLN4TfnoM=;"; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - } + oauthHMAC = "/Dcdntz/d3PMuU4EQ4qdmxFRa3SSDds1OIoLN4TfnoM=;"; // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. test_time_.setSystemTime(SystemTime(std::chrono::seconds(0))); @@ -2151,55 +1833,6 @@ TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokens_oauth_use_standard_max_age_v std::chrono::seconds(600)); } -TEST_P(OAuth2Test, OAuthAccessTokenSucessWithTokens_oauth_make_token_cookie_httponly) { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.oauth_make_token_cookie_httponly", "false"}, - }); - if (GetParam() == 1) { - oauthHMAC = - "ZTEzMmIyYzRmNTdmMTdiY2IyYmViZDE3ODA5ZDliOTE2MTRlNzNjYjc4MjBlMTVlOWY1OTM2ZjViZjM4MzAwNA==;"; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - } else { - oauthHMAC = "4TKyxPV/F7yyvr0XgJ2bkWFOc8t4IOFen1k29b84MAQ=;"; - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - } - - // Set SystemTime to a fixed point so we get consistent HMAC encodings between test runs. - test_time_.setSystemTime(SystemTime(std::chrono::seconds(1000))); - - // host_ must be set, which is guaranteed (ASAN). - Http::TestRequestHeaderMapImpl request_headers{ - {Http::Headers::get().Host.get(), "traffic.example.com"}, - {Http::Headers::get().Path.get(), "/_signout"}, - {Http::Headers::get().Method.get(), Http::Headers::get().MethodValues.Get}, - }; - filter_->decodeHeaders(request_headers, false); - - // Expected response after the callback is complete. - Http::TestRequestHeaderMapImpl expected_headers{ - {Http::Headers::get().Status.get(), "302"}, - {Http::Headers::get().SetCookie.get(), - "OauthHMAC=" + oauthHMAC + "path=/;Max-Age=600;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), - "OauthExpires=1600;path=/;Max-Age=600;secure;HttpOnly"}, - {Http::Headers::get().SetCookie.get(), "BearerToken=access_code;path=/;Max-Age=600;secure"}, - {Http::Headers::get().SetCookie.get(), "IdToken=some-id-token;path=/;Max-Age=600;secure"}, - {Http::Headers::get().SetCookie.get(), - "RefreshToken=some-refresh-token;path=/;Max-Age=600;secure;HttpOnly"}, - {Http::Headers::get().Location.get(), ""}, - }; - - EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&expected_headers), true)); - - filter_->onGetAccessTokenSuccess("access_code", "some-id-token", "some-refresh-token", - std::chrono::seconds(600)); -} - TEST_F(OAuth2Test, OAuthBearerTokenFlowFromHeader) { Http::TestRequestHeaderMapImpl request_headers{ {Http::Headers::get().Path.get(), "/test?role=bearer"}, @@ -2233,18 +1866,7 @@ TEST_F(OAuth2Test, OAuthBearerTokenFlowFromQueryParameters) { filter_->decodeHeaders(request_headers, false)); } -TEST_P(OAuth2Test, CookieValidatorInTransition) { - TestScopedRuntime scoped_runtime; - if (GetParam() == 0) { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "true"}, - }); - } else { - scoped_runtime.mergeValues({ - {"envoy.reloadable_features.hmac_base64_encoding_only", "false"}, - }); - } - +TEST_F(OAuth2Test, CookieValidatorInTransition) { Http::TestRequestHeaderMapImpl request_headers_base64only{ {Http::Headers::get().Host.get(), "traffic.example.com"}, {Http::Headers::get().Path.get(), "/_signout"}, @@ -2261,9 +1883,7 @@ TEST_P(OAuth2Test, CookieValidatorInTransition) { test_time_, CookieNames{"BearerToken", "OauthHMAC", "OauthExpires", "IdToken", "RefreshToken"}); cookie_validator->setParams(request_headers_base64only, "mock-secret"); - std::cout << "before first valid() " << GetParam() << std::endl; EXPECT_TRUE(cookie_validator->hmacIsValid()); - std::cout << "after first valid() " << GetParam() << std::endl; Http::TestRequestHeaderMapImpl request_headers_hexbase64{ {Http::Headers::get().Host.get(), "traffic.example.com"},