From ac9a2637336decdcc52c24add5e8fc39edebb962 Mon Sep 17 00:00:00 2001 From: John Esmet Date: Thu, 11 Feb 2021 15:06:36 -0500 Subject: [PATCH] ext_authz: support response headers on OK authorization checks (#14514) Support adding response headers on OK authorization checks from ext_authz Commit Message: ext_authz: support response headers on OK authorization checks Additional Description: Risk Level: low (opt-in feature, does nothing by default) Testing: Added code to existing unit tests Docs Changes: API protos documented Release Notes: ext_authz: added :ref:`response_headers_to_add ` to support sending response headers to downstream clients on OK external authorization checks. Platform Specific Features: Fixes #7986 Signed-off-by: John Esmet --- .../filters/http/ext_authz/v3/ext_authz.proto | 9 ++ .../http/ext_authz/v4alpha/ext_authz.proto | 9 ++ api/envoy/service/auth/v3/external_auth.proto | 11 +- .../service/auth/v4alpha/external_auth.proto | 11 +- docs/root/version_history/current.rst | 2 + .../filters/http/ext_authz/v3/ext_authz.proto | 9 ++ .../http/ext_authz/v4alpha/ext_authz.proto | 9 ++ .../envoy/service/auth/v3/external_auth.proto | 11 +- .../service/auth/v4alpha/external_auth.proto | 11 +- .../filters/common/ext_authz/ext_authz.h | 4 + .../common/ext_authz/ext_authz_grpc_impl.cc | 6 + .../common/ext_authz/ext_authz_http_impl.cc | 33 ++++- .../common/ext_authz/ext_authz_http_impl.h | 12 +- .../filters/http/ext_authz/config.cc | 9 +- .../filters/http/ext_authz/ext_authz.cc | 131 ++++++++++++------ .../filters/http/ext_authz/ext_authz.h | 15 +- .../ext_authz/ext_authz_grpc_impl_test.cc | 23 +-- .../ext_authz/ext_authz_http_impl_test.cc | 14 +- .../filters/common/ext_authz/test_common.cc | 20 ++- .../filters/common/ext_authz/test_common.h | 19 ++- .../filters/http/ext_authz/config_test.cc | 4 +- .../ext_authz/ext_authz_integration_test.cc | 56 +++++++- .../filters/http/ext_authz/ext_authz_test.cc | 14 ++ .../common/fuzz/uber_per_readfilter.cc | 4 +- 24 files changed, 350 insertions(+), 96 deletions(-) diff --git a/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto b/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto index 8dac7cce89da..705629496d94 100644 --- a/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto +++ b/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto @@ -182,6 +182,9 @@ message BufferSettings { // additional headers metadata may be added to the original client request. See // :ref:`allowed_upstream_headers // ` +// for details. Additionally, the filter may add additional headers to the client's response. See +// :ref:`allowed_client_headers_on_success +// ` // for details. // // On other authorization response statuses, the filter will not allow traffic. Additional headers @@ -252,6 +255,12 @@ message AuthorizationResponse { // (Host)* will be in the response to the client. When a header is included in this list, *Path*, // *Status*, *Content-Length*, *WWWAuthenticate* and *Location* are automatically added. type.matcher.v3.ListStringMatcher allowed_client_headers = 2; + + // When this :ref:`list `. is set, authorization + // response headers that have a correspondent match will be added to the client's response when + // the authorization response itself is successful, i.e. not failed or denied. When this list is + // *not* set, no additional headers will be added to the client's response on success. + type.matcher.v3.ListStringMatcher allowed_client_headers_on_success = 4; } // Extra settings on a per virtualhost/route/weighted-cluster level. diff --git a/api/envoy/extensions/filters/http/ext_authz/v4alpha/ext_authz.proto b/api/envoy/extensions/filters/http/ext_authz/v4alpha/ext_authz.proto index a32a3b6087ea..014c8263e61c 100644 --- a/api/envoy/extensions/filters/http/ext_authz/v4alpha/ext_authz.proto +++ b/api/envoy/extensions/filters/http/ext_authz/v4alpha/ext_authz.proto @@ -182,6 +182,9 @@ message BufferSettings { // additional headers metadata may be added to the original client request. See // :ref:`allowed_upstream_headers // ` +// for details. Additionally, the filter may add additional headers to the client's response. See +// :ref:`allowed_client_headers_on_success +// ` // for details. // // On other authorization response statuses, the filter will not allow traffic. Additional headers @@ -252,6 +255,12 @@ message AuthorizationResponse { // (Host)* will be in the response to the client. When a header is included in this list, *Path*, // *Status*, *Content-Length*, *WWWAuthenticate* and *Location* are automatically added. type.matcher.v4alpha.ListStringMatcher allowed_client_headers = 2; + + // When this :ref:`list `. is set, authorization + // response headers that have a correspondent match will be added to the client's response when + // the authorization response itself is successful, i.e. not failed or denied. When this list is + // *not* set, no additional headers will be added to the client's response on success. + type.matcher.v4alpha.ListStringMatcher allowed_client_headers_on_success = 4; } // Extra settings on a per virtualhost/route/weighted-cluster level. diff --git a/api/envoy/service/auth/v3/external_auth.proto b/api/envoy/service/auth/v3/external_auth.proto index 9e2bf8fccd5b..4860be38c4b7 100644 --- a/api/envoy/service/auth/v3/external_auth.proto +++ b/api/envoy/service/auth/v3/external_auth.proto @@ -50,7 +50,7 @@ message DeniedHttpResponse { type.v3.HttpStatus status = 1 [(validate.rules).message = {required: true}]; // This field allows the authorization service to send HTTP response headers - // to the downstream client. Note that the `append` field in `HeaderValueOption` defaults to + // to the downstream client. Note that the :ref:`append field in HeaderValueOption ` defaults to // false when used in this message. repeated config.core.v3.HeaderValueOption headers = 2; @@ -60,14 +60,14 @@ message DeniedHttpResponse { } // HTTP attributes for an OK response. -// [#next-free-field: 6] +// [#next-free-field: 7] message OkHttpResponse { option (udpa.annotations.versioning).previous_message_type = "envoy.service.auth.v2.OkHttpResponse"; // HTTP entity headers in addition to the original request headers. This allows the authorization // service to append, to add or to override headers from the original request before - // dispatching it to the upstream. Note that the `append` field in `HeaderValueOption` defaults to + // dispatching it to the upstream. Note that the :ref:`append field in HeaderValueOption ` defaults to // false when used in this message. By setting the `append` field to `true`, // the filter will append the correspondent header value to the matched request header. // By leaving `append` as false, the filter will either add a new header, or override an existing @@ -96,6 +96,11 @@ message OkHttpResponse { // setting this field overrides :ref:`CheckResponse.dynamic_metadata // `. google.protobuf.Struct dynamic_metadata = 3 [deprecated = true]; + + // This field allows the authorization service to send HTTP response headers + // to the downstream client on success. Note that the :ref:`append field in HeaderValueOption ` + // defaults to false when used in this message. + repeated config.core.v3.HeaderValueOption response_headers_to_add = 6; } // Intended for gRPC and Network Authorization servers `only`. diff --git a/api/envoy/service/auth/v4alpha/external_auth.proto b/api/envoy/service/auth/v4alpha/external_auth.proto index 06ccecec15da..f368516c302e 100644 --- a/api/envoy/service/auth/v4alpha/external_auth.proto +++ b/api/envoy/service/auth/v4alpha/external_auth.proto @@ -50,7 +50,7 @@ message DeniedHttpResponse { type.v3.HttpStatus status = 1 [(validate.rules).message = {required: true}]; // This field allows the authorization service to send HTTP response headers - // to the downstream client. Note that the `append` field in `HeaderValueOption` defaults to + // to the downstream client. Note that the :ref:`append field in HeaderValueOption ` defaults to // false when used in this message. repeated config.core.v4alpha.HeaderValueOption headers = 2; @@ -60,7 +60,7 @@ message DeniedHttpResponse { } // HTTP attributes for an OK response. -// [#next-free-field: 6] +// [#next-free-field: 7] message OkHttpResponse { option (udpa.annotations.versioning).previous_message_type = "envoy.service.auth.v3.OkHttpResponse"; @@ -71,7 +71,7 @@ message OkHttpResponse { // HTTP entity headers in addition to the original request headers. This allows the authorization // service to append, to add or to override headers from the original request before - // dispatching it to the upstream. Note that the `append` field in `HeaderValueOption` defaults to + // dispatching it to the upstream. Note that the :ref:`append field in HeaderValueOption ` defaults to // false when used in this message. By setting the `append` field to `true`, // the filter will append the correspondent header value to the matched request header. // By leaving `append` as false, the filter will either add a new header, or override an existing @@ -94,6 +94,11 @@ message OkHttpResponse { // authorization service as a comma separated list like so: // ``x-envoy-auth-headers-to-remove: one-auth-header, another-auth-header``. repeated string headers_to_remove = 5; + + // This field allows the authorization service to send HTTP response headers + // to the downstream client on success. Note that the :ref:`append field in HeaderValueOption ` + // defaults to false when used in this message. + repeated config.core.v4alpha.HeaderValueOption response_headers_to_add = 6; } // Intended for gRPC and Network Authorization servers `only`. diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 807956ab7dca..9ab8bdcfd4d5 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -83,6 +83,8 @@ New Features * compression: add brotli :ref:`compressor ` and :ref:`decompressor `. * config: add `envoy.features.fail_on_any_deprecated_feature` runtime key, which matches the behaviour of compile-time flag `ENVOY_DISABLE_DEPRECATED_FEATURES`, i.e. use of deprecated fields will cause a crash. * dispatcher: supports a stack of `Envoy::ScopeTrackedObject` instead of a single tracked object. This will allow Envoy to dump more debug information on crash. +* ext_authz: added :ref:`response_headers_to_add ` to support sending response headers to downstream clients on OK authorization checks via gRPC. +* ext_authz: added :ref:`allowed_client_headers_on_success ` to support sending response headers to downstream clients on OK external authorization checks via HTTP. * grpc_json_transcoder: added option :ref:`strict_http_request_validation ` to reject invalid requests early. * grpc_json_transcoder: filter can now be configured on per-route/per-vhost level as well. Leaving empty list of services in the filter configuration disables transcoding on the specific route. * http: added support for `Envoy::ScopeTrackedObject` for HTTP/1 dispatching. Crashes while inside the dispatching loop should dump debug information. diff --git a/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto b/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto index b1ab3989f20e..81be512556f1 100644 --- a/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto +++ b/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto @@ -182,6 +182,9 @@ message BufferSettings { // additional headers metadata may be added to the original client request. See // :ref:`allowed_upstream_headers // ` +// for details. Additionally, the filter may add additional headers to the client's response. See +// :ref:`allowed_client_headers_on_success +// ` // for details. // // On other authorization response statuses, the filter will not allow traffic. Additional headers @@ -252,6 +255,12 @@ message AuthorizationResponse { // (Host)* will be in the response to the client. When a header is included in this list, *Path*, // *Status*, *Content-Length*, *WWWAuthenticate* and *Location* are automatically added. type.matcher.v3.ListStringMatcher allowed_client_headers = 2; + + // When this :ref:`list `. is set, authorization + // response headers that have a correspondent match will be added to the client's response when + // the authorization response itself is successful, i.e. not failed or denied. When this list is + // *not* set, no additional headers will be added to the client's response on success. + type.matcher.v3.ListStringMatcher allowed_client_headers_on_success = 4; } // Extra settings on a per virtualhost/route/weighted-cluster level. diff --git a/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v4alpha/ext_authz.proto b/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v4alpha/ext_authz.proto index a32a3b6087ea..014c8263e61c 100644 --- a/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v4alpha/ext_authz.proto +++ b/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v4alpha/ext_authz.proto @@ -182,6 +182,9 @@ message BufferSettings { // additional headers metadata may be added to the original client request. See // :ref:`allowed_upstream_headers // ` +// for details. Additionally, the filter may add additional headers to the client's response. See +// :ref:`allowed_client_headers_on_success +// ` // for details. // // On other authorization response statuses, the filter will not allow traffic. Additional headers @@ -252,6 +255,12 @@ message AuthorizationResponse { // (Host)* will be in the response to the client. When a header is included in this list, *Path*, // *Status*, *Content-Length*, *WWWAuthenticate* and *Location* are automatically added. type.matcher.v4alpha.ListStringMatcher allowed_client_headers = 2; + + // When this :ref:`list `. is set, authorization + // response headers that have a correspondent match will be added to the client's response when + // the authorization response itself is successful, i.e. not failed or denied. When this list is + // *not* set, no additional headers will be added to the client's response on success. + type.matcher.v4alpha.ListStringMatcher allowed_client_headers_on_success = 4; } // Extra settings on a per virtualhost/route/weighted-cluster level. diff --git a/generated_api_shadow/envoy/service/auth/v3/external_auth.proto b/generated_api_shadow/envoy/service/auth/v3/external_auth.proto index 9e2bf8fccd5b..4860be38c4b7 100644 --- a/generated_api_shadow/envoy/service/auth/v3/external_auth.proto +++ b/generated_api_shadow/envoy/service/auth/v3/external_auth.proto @@ -50,7 +50,7 @@ message DeniedHttpResponse { type.v3.HttpStatus status = 1 [(validate.rules).message = {required: true}]; // This field allows the authorization service to send HTTP response headers - // to the downstream client. Note that the `append` field in `HeaderValueOption` defaults to + // to the downstream client. Note that the :ref:`append field in HeaderValueOption ` defaults to // false when used in this message. repeated config.core.v3.HeaderValueOption headers = 2; @@ -60,14 +60,14 @@ message DeniedHttpResponse { } // HTTP attributes for an OK response. -// [#next-free-field: 6] +// [#next-free-field: 7] message OkHttpResponse { option (udpa.annotations.versioning).previous_message_type = "envoy.service.auth.v2.OkHttpResponse"; // HTTP entity headers in addition to the original request headers. This allows the authorization // service to append, to add or to override headers from the original request before - // dispatching it to the upstream. Note that the `append` field in `HeaderValueOption` defaults to + // dispatching it to the upstream. Note that the :ref:`append field in HeaderValueOption ` defaults to // false when used in this message. By setting the `append` field to `true`, // the filter will append the correspondent header value to the matched request header. // By leaving `append` as false, the filter will either add a new header, or override an existing @@ -96,6 +96,11 @@ message OkHttpResponse { // setting this field overrides :ref:`CheckResponse.dynamic_metadata // `. google.protobuf.Struct dynamic_metadata = 3 [deprecated = true]; + + // This field allows the authorization service to send HTTP response headers + // to the downstream client on success. Note that the :ref:`append field in HeaderValueOption ` + // defaults to false when used in this message. + repeated config.core.v3.HeaderValueOption response_headers_to_add = 6; } // Intended for gRPC and Network Authorization servers `only`. diff --git a/generated_api_shadow/envoy/service/auth/v4alpha/external_auth.proto b/generated_api_shadow/envoy/service/auth/v4alpha/external_auth.proto index dbb8dd61b301..451a54bc677b 100644 --- a/generated_api_shadow/envoy/service/auth/v4alpha/external_auth.proto +++ b/generated_api_shadow/envoy/service/auth/v4alpha/external_auth.proto @@ -50,7 +50,7 @@ message DeniedHttpResponse { type.v3.HttpStatus status = 1 [(validate.rules).message = {required: true}]; // This field allows the authorization service to send HTTP response headers - // to the downstream client. Note that the `append` field in `HeaderValueOption` defaults to + // to the downstream client. Note that the :ref:`append field in HeaderValueOption ` defaults to // false when used in this message. repeated config.core.v4alpha.HeaderValueOption headers = 2; @@ -60,14 +60,14 @@ message DeniedHttpResponse { } // HTTP attributes for an OK response. -// [#next-free-field: 6] +// [#next-free-field: 7] message OkHttpResponse { option (udpa.annotations.versioning).previous_message_type = "envoy.service.auth.v3.OkHttpResponse"; // HTTP entity headers in addition to the original request headers. This allows the authorization // service to append, to add or to override headers from the original request before - // dispatching it to the upstream. Note that the `append` field in `HeaderValueOption` defaults to + // dispatching it to the upstream. Note that the :ref:`append field in HeaderValueOption ` defaults to // false when used in this message. By setting the `append` field to `true`, // the filter will append the correspondent header value to the matched request header. // By leaving `append` as false, the filter will either add a new header, or override an existing @@ -96,6 +96,11 @@ message OkHttpResponse { // setting this field overrides :ref:`CheckResponse.dynamic_metadata // `. google.protobuf.Struct hidden_envoy_deprecated_dynamic_metadata = 3 [deprecated = true]; + + // This field allows the authorization service to send HTTP response headers + // to the downstream client on success. Note that the :ref:`append field in HeaderValueOption ` + // defaults to false when used in this message. + repeated config.core.v4alpha.HeaderValueOption response_headers_to_add = 6; } // Intended for gRPC and Network Authorization servers `only`. diff --git a/source/extensions/filters/common/ext_authz/ext_authz.h b/source/extensions/filters/common/ext_authz/ext_authz.h index 6011752fc3fe..0d89e5803161 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz.h +++ b/source/extensions/filters/common/ext_authz/ext_authz.h @@ -74,6 +74,10 @@ struct Response { // A set of HTTP headers returned by the authorization server, will be optionally added // (using "addCopy") to the request to the upstream server. Http::HeaderVector headers_to_add; + // A set of HTTP headers returned by the authorization server, will be optionally added + // (using "addCopy") to the response sent back to the downstream client on OK auth + // responses. + Http::HeaderVector response_headers_to_add; // A set of HTTP headers consumed by the authorization server, will be removed // from the request to the upstream server. std::vector headers_to_remove; diff --git a/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc b/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc index f065706a4f82..ee6027ca0ad9 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc +++ b/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc @@ -61,6 +61,12 @@ void GrpcClientImpl::onSuccess(std::unique_ptrheaders_to_remove.push_back(Http::LowerCaseString(header)); } } + if (response->ok_response().response_headers_to_add_size() > 0) { + for (const auto& header : response->ok_response().response_headers_to_add()) { + authz_response->response_headers_to_add.emplace_back( + Http::LowerCaseString(header.header().key()), header.header().value()); + } + } } } else { span.setTag(TracingConstants::get().TraceStatus, TracingConstants::get().TraceUnauthz); 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 4663563ada68..11da8bc4f7dd 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 @@ -35,6 +35,7 @@ const Response& errorResponse() { Http::HeaderVector{}, Http::HeaderVector{}, Http::HeaderVector{}, + Http::HeaderVector{}, {{}}, EMPTY_STRING, Http::Code::Forbidden, @@ -44,9 +45,10 @@ const Response& errorResponse() { // SuccessResponse used for creating either DENIED or OK authorization responses. struct SuccessResponse { SuccessResponse(const Http::HeaderMap& headers, const MatcherSharedPtr& matchers, - const MatcherSharedPtr& append_matchers, Response&& response) + const MatcherSharedPtr& append_matchers, + const MatcherSharedPtr& response_matchers, Response&& response) : headers_(headers), matchers_(matchers), append_matchers_(append_matchers), - response_(std::make_unique(response)) { + response_matchers_(response_matchers), response_(std::make_unique(response)) { headers_.iterate([this](const Http::HeaderEntry& header) -> Http::HeaderMap::Iterate { // UpstreamHeaderMatcher if (matchers_->matches(header.key().getStringView())) { @@ -64,6 +66,11 @@ struct SuccessResponse { Http::LowerCaseString{std::string(header.key().getStringView())}, std::string(header.value().getStringView())); } + if (response_matchers_->matches(header.key().getStringView())) { + response_->response_headers_to_add.emplace_back( + Http::LowerCaseString{std::string(header.key().getStringView())}, + std::string(header.value().getStringView())); + } return Http::HeaderMap::Iterate::Continue; }); } @@ -71,6 +78,7 @@ struct SuccessResponse { const Http::HeaderMap& headers_; const MatcherSharedPtr& matchers_; const MatcherSharedPtr& append_matchers_; + const MatcherSharedPtr& response_matchers_; ResponsePtr response_; }; @@ -106,6 +114,8 @@ ClientConfig::ClientConfig(const envoy::extensions::filters::http::ext_authz::v3 toRequestMatchers(config.http_service().authorization_request().allowed_headers())), client_header_matchers_(toClientMatchers( config.http_service().authorization_response().allowed_client_headers())), + client_header_on_success_matchers_(toClientMatchersOnSuccess( + config.http_service().authorization_response().allowed_client_headers_on_success())), upstream_header_matchers_(toUpstreamMatchers( config.http_service().authorization_response().allowed_upstream_headers())), upstream_header_to_append_matchers_(toUpstreamMatchers( @@ -132,6 +142,12 @@ ClientConfig::toRequestMatchers(const envoy::type::matcher::v3::ListStringMatche return std::make_shared(std::move(matchers)); } +MatcherSharedPtr +ClientConfig::toClientMatchersOnSuccess(const envoy::type::matcher::v3::ListStringMatcher& list) { + std::vector matchers(createStringMatchers(list)); + return std::make_shared(std::move(matchers)); +} + MatcherSharedPtr ClientConfig::toClientMatchers(const envoy::type::matcher::v3::ListStringMatcher& list) { std::vector matchers(createStringMatchers(list)); @@ -300,21 +316,24 @@ ResponsePtr RawHttpClientImpl::toResponse(Http::ResponseMessagePtr message) { // Create an Ok authorization response. if (status_code == enumToInt(Http::Code::OK)) { - SuccessResponse ok{message->headers(), config_->upstreamHeaderMatchers(), - config_->upstreamHeaderToAppendMatchers(), - Response{CheckStatus::OK, Http::HeaderVector{}, Http::HeaderVector{}, - Http::HeaderVector{}, std::move(headers_to_remove), EMPTY_STRING, - Http::Code::OK, ProtobufWkt::Struct{}}}; + SuccessResponse ok{ + message->headers(), config_->upstreamHeaderMatchers(), + config_->upstreamHeaderToAppendMatchers(), config_->clientHeaderOnSuccessMatchers(), + Response{CheckStatus::OK, Http::HeaderVector{}, Http::HeaderVector{}, Http::HeaderVector{}, + Http::HeaderVector{}, std::move(headers_to_remove), EMPTY_STRING, Http::Code::OK, + ProtobufWkt::Struct{}}}; return std::move(ok.response_); } // Create a Denied authorization response. SuccessResponse denied{message->headers(), config_->clientHeaderMatchers(), config_->upstreamHeaderToAppendMatchers(), + config_->clientHeaderOnSuccessMatchers(), Response{CheckStatus::Denied, Http::HeaderVector{}, Http::HeaderVector{}, Http::HeaderVector{}, + Http::HeaderVector{}, {{}}, message->bodyAsString(), static_cast(status_code), diff --git a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.h b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.h index d197a9a34268..f2e6e5726973 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.h +++ b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.h @@ -92,6 +92,14 @@ class ClientConfig { */ const MatcherSharedPtr& clientHeaderMatchers() const { return client_header_matchers_; } + /** + * Returns a list of matchers used for selecting the authorization response headers that + * should be send back to the client on a successful (i.e. non-denied) response. + */ + const MatcherSharedPtr& clientHeaderOnSuccessMatchers() const { + return client_header_on_success_matchers_; + } + /** * Returns a list of matchers used for selecting the authorization response headers that * should be send to an the upstream server. @@ -122,13 +130,15 @@ class ClientConfig { toRequestMatchers(const envoy::type::matcher::v3::ListStringMatcher& list); static MatcherSharedPtr toClientMatchers(const envoy::type::matcher::v3::ListStringMatcher& list); static MatcherSharedPtr + toClientMatchersOnSuccess(const envoy::type::matcher::v3::ListStringMatcher& list); + static MatcherSharedPtr toUpstreamMatchers(const envoy::type::matcher::v3::ListStringMatcher& list); const MatcherSharedPtr request_header_matchers_; const MatcherSharedPtr client_header_matchers_; + const MatcherSharedPtr client_header_on_success_matchers_; const MatcherSharedPtr upstream_header_matchers_; const MatcherSharedPtr upstream_header_to_append_matchers_; - const Http::LowerCaseStrPairVector authorization_headers_to_add_; const std::string cluster_name_; const std::chrono::milliseconds timeout_; const std::string path_prefix_; diff --git a/source/extensions/filters/http/ext_authz/config.cc b/source/extensions/filters/http/ext_authz/config.cc index 5446cb7a55c2..c99894009d9a 100644 --- a/source/extensions/filters/http/ext_authz/config.cc +++ b/source/extensions/filters/http/ext_authz/config.cc @@ -39,8 +39,7 @@ Http::FilterFactoryCb ExtAuthzFilterConfig::createFilterFactoryFromProtoTyped( &context](Http::FilterChainFactoryCallbacks& callbacks) { auto client = std::make_unique( context.clusterManager(), client_config); - callbacks.addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr{ - std::make_shared(filter_config, std::move(client))}); + callbacks.addStreamFilter(std::make_shared(filter_config, std::move(client))); }; } else if (proto_config.grpc_service().has_google_grpc()) { // Google gRPC client. @@ -65,8 +64,7 @@ Http::FilterFactoryCb ExtAuthzFilterConfig::createFilterFactoryFromProtoTyped( auto client = std::make_unique( async_client_cache->getAsyncClient(), std::chrono::milliseconds(timeout_ms), transport_api_version); - callbacks.addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr{ - std::make_shared(filter_config, std::move(client))}); + callbacks.addStreamFilter(std::make_shared(filter_config, std::move(client))); }; } else { // Envoy gRPC client. @@ -88,8 +86,7 @@ Http::FilterFactoryCb ExtAuthzFilterConfig::createFilterFactoryFromProtoTyped( auto client = std::make_unique( async_client_factory->create(), std::chrono::milliseconds(timeout_ms), transport_api_version); - callbacks.addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr{ - std::make_shared(filter_config, std::move(client))}); + callbacks.addStreamFilter(std::make_shared(filter_config, std::move(client))); }; } diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index 37247272d879..32226c3388d3 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -53,7 +53,8 @@ void Filter::initiateCall(const Http::RequestHeaderMap& headers, // If metadata_context_namespaces is specified, pass matching metadata to the ext_authz service. envoy::config::core::v3::Metadata metadata_context; - const auto& request_metadata = callbacks_->streamInfo().dynamicMetadata().filter_metadata(); + const auto& request_metadata = + decoder_callbacks_->streamInfo().dynamicMetadata().filter_metadata(); for (const auto& context_key : config_->metadataContextNamespaces()) { const auto& metadata_it = request_metadata.find(context_key); if (metadata_it != request_metadata.end()) { @@ -62,36 +63,38 @@ void Filter::initiateCall(const Http::RequestHeaderMap& headers, } Filters::Common::ExtAuthz::CheckRequestUtils::createHttpCheck( - callbacks_, headers, std::move(context_extensions), std::move(metadata_context), + decoder_callbacks_, headers, std::move(context_extensions), std::move(metadata_context), check_request_, config_->maxRequestBytes(), config_->packAsBytes(), config_->includePeerCertificate()); - ENVOY_STREAM_LOG(trace, "ext_authz filter calling authorization server", *callbacks_); + ENVOY_STREAM_LOG(trace, "ext_authz filter calling authorization server", *decoder_callbacks_); state_ = State::Calling; filter_return_ = FilterReturn::StopDecoding; // Don't let the filter chain continue as we are // going to invoke check call. - cluster_ = callbacks_->clusterInfo(); + cluster_ = decoder_callbacks_->clusterInfo(); initiating_call_ = true; - client_->check(*this, check_request_, callbacks_->activeSpan(), callbacks_->streamInfo()); + client_->check(*this, check_request_, decoder_callbacks_->activeSpan(), + decoder_callbacks_->streamInfo()); initiating_call_ = false; } Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, bool end_stream) { - Router::RouteConstSharedPtr route = callbacks_->route(); + Router::RouteConstSharedPtr route = decoder_callbacks_->route(); const auto per_route_flags = getPerRouteFlags(route); skip_check_ = per_route_flags.skip_check_; if (skip_check_) { return Http::FilterHeadersStatus::Continue; } - if (!config_->filterEnabled(callbacks_->streamInfo().dynamicMetadata())) { + if (!config_->filterEnabled(decoder_callbacks_->streamInfo().dynamicMetadata())) { stats_.disabled_.inc(); if (config_->denyAtDisable()) { - ENVOY_STREAM_LOG(trace, "ext_authz filter is disabled. Deny the request.", *callbacks_); - callbacks_->streamInfo().setResponseFlag( + ENVOY_STREAM_LOG(trace, "ext_authz filter is disabled. Deny the request.", + *decoder_callbacks_); + decoder_callbacks_->streamInfo().setResponseFlag( StreamInfo::ResponseFlag::UnauthorizedExternalService); - callbacks_->sendLocalReply(config_->statusOnError(), EMPTY_STRING, nullptr, absl::nullopt, - RcDetails::get().AuthzError); + decoder_callbacks_->sendLocalReply(config_->statusOnError(), EMPTY_STRING, nullptr, + absl::nullopt, RcDetails::get().AuthzError); return Http::FilterHeadersStatus::StopIteration; } return Http::FilterHeadersStatus::Continue; @@ -103,9 +106,9 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, Http::Utility::isH2UpgradeRequest(headers)); if (buffer_data_) { - ENVOY_STREAM_LOG(debug, "ext_authz filter is buffering the request", *callbacks_); + ENVOY_STREAM_LOG(debug, "ext_authz filter is buffering the request", *decoder_callbacks_); if (!config_->allowPartialMessage()) { - callbacks_->setDecoderBufferLimit(config_->maxRequestBytes()); + decoder_callbacks_->setDecoderBufferLimit(config_->maxRequestBytes()); } return Http::FilterHeadersStatus::StopIteration; } @@ -122,12 +125,12 @@ Http::FilterDataStatus Filter::decodeData(Buffer::Instance& data, bool end_strea const bool buffer_is_full = isBufferFull(); if (end_stream || buffer_is_full) { ENVOY_STREAM_LOG(debug, "ext_authz filter finished buffering the request since {}", - *callbacks_, buffer_is_full ? "buffer is full" : "stream is ended"); + *decoder_callbacks_, buffer_is_full ? "buffer is full" : "stream is ended"); if (!buffer_is_full) { // Make sure data is available in initiateCall. - callbacks_->addDecodedData(data, true); + decoder_callbacks_->addDecodedData(data, true); } - initiateCall(*request_headers_, callbacks_->route()); + initiateCall(*request_headers_, decoder_callbacks_->route()); return filter_return_ == FilterReturn::StopDecoding ? Http::FilterDataStatus::StopIterationAndWatermark : Http::FilterDataStatus::Continue; @@ -142,8 +145,9 @@ Http::FilterDataStatus Filter::decodeData(Buffer::Instance& data, bool end_strea Http::FilterTrailersStatus Filter::decodeTrailers(Http::RequestTrailerMap&) { if (buffer_data_ && !skip_check_) { if (filter_return_ != FilterReturn::StopDecoding) { - ENVOY_STREAM_LOG(debug, "ext_authz filter finished buffering the request", *callbacks_); - initiateCall(*request_headers_, callbacks_->route()); + ENVOY_STREAM_LOG(debug, "ext_authz filter finished buffering the request", + *decoder_callbacks_); + initiateCall(*request_headers_, decoder_callbacks_->route()); } return filter_return_ == FilterReturn::StopDecoding ? Http::FilterTrailersStatus::StopIteration : Http::FilterTrailersStatus::Continue; @@ -152,8 +156,44 @@ Http::FilterTrailersStatus Filter::decodeTrailers(Http::RequestTrailerMap&) { return Http::FilterTrailersStatus::Continue; } +Http::FilterHeadersStatus Filter::encode100ContinueHeaders(Http::ResponseHeaderMap&) { + return Http::FilterHeadersStatus::Continue; +} + +Http::FilterHeadersStatus Filter::encodeHeaders(Http::ResponseHeaderMap& headers, bool) { + ENVOY_STREAM_LOG(trace, + "ext_authz filter has {} response header(s) to add to the encoded response:", + *encoder_callbacks_, response_headers_to_add_.size()); + if (!response_headers_to_add_.empty()) { + ENVOY_STREAM_LOG( + trace, "ext_authz filter added header(s) to the encoded response:", *encoder_callbacks_); + for (const auto& header : response_headers_to_add_) { + ENVOY_STREAM_LOG(trace, "'{}':'{}'", *encoder_callbacks_, header.first.get(), header.second); + headers.addCopy(header.first, header.second); + } + } + + return Http::FilterHeadersStatus::Continue; +} + +Http::FilterDataStatus Filter::encodeData(Buffer::Instance&, bool) { + return Http::FilterDataStatus::Continue; +} + +Http::FilterTrailersStatus Filter::encodeTrailers(Http::ResponseTrailerMap&) { + return Http::FilterTrailersStatus::Continue; +} + +Http::FilterMetadataStatus Filter::encodeMetadata(Http::MetadataMap&) { + return Http::FilterMetadataStatus::Continue; +} + void Filter::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) { - callbacks_ = &callbacks; + decoder_callbacks_ = &callbacks; +} + +void Filter::setEncoderFilterCallbacks(Http::StreamEncoderFilterCallbacks& callbacks) { + encoder_callbacks_ = &callbacks; } void Filter::onDestroy() { @@ -176,17 +216,18 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { if (config_->clearRouteCache() && (!response->headers_to_set.empty() || !response->headers_to_append.empty() || !response->headers_to_remove.empty())) { - ENVOY_STREAM_LOG(debug, "ext_authz is clearing route cache", *callbacks_); - callbacks_->clearRouteCache(); + ENVOY_STREAM_LOG(debug, "ext_authz is clearing route cache", *decoder_callbacks_); + decoder_callbacks_->clearRouteCache(); } - ENVOY_STREAM_LOG(trace, "ext_authz filter added header(s) to the request:", *callbacks_); + ENVOY_STREAM_LOG(trace, + "ext_authz filter added header(s) to the request:", *decoder_callbacks_); for (const auto& header : response->headers_to_set) { - ENVOY_STREAM_LOG(trace, "'{}':'{}'", *callbacks_, header.first.get(), header.second); + ENVOY_STREAM_LOG(trace, "'{}':'{}'", *decoder_callbacks_, header.first.get(), header.second); request_headers_->setCopy(header.first, header.second); } for (const auto& header : response->headers_to_add) { - ENVOY_STREAM_LOG(trace, "'{}':'{}'", *callbacks_, header.first.get(), header.second); + ENVOY_STREAM_LOG(trace, "'{}':'{}'", *decoder_callbacks_, header.first.get(), header.second); request_headers_->addCopy(header.first, header.second); } for (const auto& header : response->headers_to_append) { @@ -200,7 +241,8 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { // combined headers: {{"original": "true"}, {"append": "1"}, {"append": "2"}}) to the request // to upstream server by only sets `headers_to_append`. if (!header_to_modify.empty()) { - ENVOY_STREAM_LOG(trace, "'{}':'{}'", *callbacks_, header.first.get(), header.second); + ENVOY_STREAM_LOG(trace, "'{}':'{}'", *decoder_callbacks_, header.first.get(), + header.second); // The current behavior of appending is by combining entries with the same key, into one // entry. The value of that combined entry is separated by ",". // TODO(dio): Consider to use addCopy instead. @@ -208,20 +250,27 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { } } - ENVOY_STREAM_LOG(trace, "ext_authz filter removed header(s) from the request:", *callbacks_); + ENVOY_STREAM_LOG(trace, + "ext_authz filter removed header(s) from the request:", *decoder_callbacks_); for (const auto& header : response->headers_to_remove) { // We don't allow removing any :-prefixed headers, nor Host, as removing // them would make the request malformed. if (!Http::HeaderUtility::isRemovableHeader(header.get())) { continue; } - ENVOY_STREAM_LOG(trace, "'{}'", *callbacks_, header.get()); + ENVOY_STREAM_LOG(trace, "'{}'", *decoder_callbacks_, header.get()); request_headers_->remove(header); } + if (!response->response_headers_to_add.empty()) { + ENVOY_STREAM_LOG(trace, "ext_authz filter saving {} header(s) to add to the response:", + *decoder_callbacks_, response->response_headers_to_add.size()); + response_headers_to_add_ = std::move(response->response_headers_to_add); + } + if (!response->dynamic_metadata.fields().empty()) { - callbacks_->streamInfo().setDynamicMetadata(HttpFilterNames::get().ExtAuthorization, - response->dynamic_metadata); + decoder_callbacks_->streamInfo().setDynamicMetadata(HttpFilterNames::get().ExtAuthorization, + response->dynamic_metadata); } if (cluster_) { @@ -234,7 +283,7 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { case CheckStatus::Denied: { ENVOY_STREAM_LOG(trace, "ext_authz filter rejected the request. Response status code: '{}", - *callbacks_, enumToInt(response->status_code)); + *decoder_callbacks_, enumToInt(response->status_code)); stats_.denied_.inc(); if (cluster_) { @@ -253,10 +302,10 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { config_->httpContext().codeStats().chargeResponseStat(info); } - callbacks_->sendLocalReply( + decoder_callbacks_->sendLocalReply( response->status_code, response->body, [&headers = response->headers_to_set, - &callbacks = *callbacks_](Http::HeaderMap& response_headers) -> void { + &callbacks = *decoder_callbacks_](Http::HeaderMap& response_headers) -> void { ENVOY_STREAM_LOG(trace, "ext_authz filter added header(s) to the local response:", callbacks); // Firstly, remove all headers requested by the ext_authz filter, to ensure that they will @@ -272,7 +321,8 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { } }, absl::nullopt, RcDetails::get().AuthzDenied); - callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::UnauthorizedExternalService); + decoder_callbacks_->streamInfo().setResponseFlag( + StreamInfo::ResponseFlag::UnauthorizedExternalService); break; } @@ -282,7 +332,8 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { } stats_.error_.inc(); if (config_->failureModeAllow()) { - ENVOY_STREAM_LOG(trace, "ext_authz filter allowed the request with error", *callbacks_); + ENVOY_STREAM_LOG(trace, "ext_authz filter allowed the request with error", + *decoder_callbacks_); stats_.failure_mode_allowed_.inc(); if (cluster_) { config_->incCounter(cluster_->statsScope(), config_->ext_authz_failure_mode_allowed_); @@ -291,11 +342,11 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { } else { ENVOY_STREAM_LOG( trace, "ext_authz filter rejected the request with an error. Response status code: {}", - *callbacks_, enumToInt(config_->statusOnError())); - callbacks_->streamInfo().setResponseFlag( + *decoder_callbacks_, enumToInt(config_->statusOnError())); + decoder_callbacks_->streamInfo().setResponseFlag( StreamInfo::ResponseFlag::UnauthorizedExternalService); - callbacks_->sendLocalReply(config_->statusOnError(), EMPTY_STRING, nullptr, absl::nullopt, - RcDetails::get().AuthzError); + decoder_callbacks_->sendLocalReply(config_->statusOnError(), EMPTY_STRING, nullptr, + absl::nullopt, RcDetails::get().AuthzError); } break; } @@ -307,7 +358,7 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { } bool Filter::isBufferFull() const { - const auto* buffer = callbacks_->decodingBuffer(); + const auto* buffer = decoder_callbacks_->decodingBuffer(); if (config_->allowPartialMessage() && buffer != nullptr) { return buffer->length() >= config_->maxRequestBytes(); } @@ -320,7 +371,7 @@ void Filter::continueDecoding() { filter_return_ = FilterReturn::ContinueDecoding; if (!initiating_call_) { - callbacks_->continueDecoding(); + decoder_callbacks_->continueDecoding(); } } diff --git a/source/extensions/filters/http/ext_authz/ext_authz.h b/source/extensions/filters/http/ext_authz/ext_authz.h index db93fa3d2eea..9b1aca32f97a 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.h +++ b/source/extensions/filters/http/ext_authz/ext_authz.h @@ -229,7 +229,7 @@ class FilterConfigPerRoute : public Router::RouteSpecificFilterConfig { * ext_authz service before allowing further filter iteration. */ class Filter : public Logger::Loggable, - public Http::StreamDecoderFilter, + public Http::StreamFilter, public Filters::Common::ExtAuthz::RequestCallbacks { public: Filter(const FilterConfigSharedPtr& config, Filters::Common::ExtAuthz::ClientPtr&& client) @@ -245,6 +245,15 @@ class Filter : public Logger::Loggable, Http::FilterTrailersStatus decodeTrailers(Http::RequestTrailerMap& trailers) override; void setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) override; + // Http::StreamEncoderFilter + Http::FilterHeadersStatus encode100ContinueHeaders(Http::ResponseHeaderMap& headers) override; + Http::FilterHeadersStatus encodeHeaders(Http::ResponseHeaderMap& headers, + bool end_stream) override; + Http::FilterDataStatus encodeData(Buffer::Instance& data, bool end_stream) override; + Http::FilterTrailersStatus encodeTrailers(Http::ResponseTrailerMap& trailers) override; + Http::FilterMetadataStatus encodeMetadata(Http::MetadataMap& trailers) override; + void setEncoderFilterCallbacks(Http::StreamEncoderFilterCallbacks& callbacks) override; + // ExtAuthz::RequestCallbacks void onComplete(Filters::Common::ExtAuthz::ResponsePtr&&) override; @@ -275,8 +284,10 @@ class Filter : public Logger::Loggable, Http::HeaderMapPtr getHeaderMap(const Filters::Common::ExtAuthz::ResponsePtr& response); FilterConfigSharedPtr config_; Filters::Common::ExtAuthz::ClientPtr client_; - Http::StreamDecoderFilterCallbacks* callbacks_{}; + Http::StreamDecoderFilterCallbacks* decoder_callbacks_{}; + Http::StreamEncoderFilterCallbacks* encoder_callbacks_{}; Http::RequestHeaderMap* request_headers_; + Http::HeaderVector response_headers_to_add_{}; State state_{State::NotStarted}; FilterReturn filter_return_{FilterReturn::ContinueDecoding}; Upstream::ClusterInfoConstSharedPtr cluster_; diff --git a/test/extensions/filters/common/ext_authz/ext_authz_grpc_impl_test.cc b/test/extensions/filters/common/ext_authz/ext_authz_grpc_impl_test.cc index 98bce5a24108..2601533b8d70 100644 --- a/test/extensions/filters/common/ext_authz/ext_authz_grpc_impl_test.cc +++ b/test/extensions/filters/common/ext_authz/ext_authz_grpc_impl_test.cc @@ -117,10 +117,13 @@ TEST_P(ExtAuthzGrpcClientTest, AuthorizationOkWithAllAtributes) { const std::string empty_body{}; const auto expected_headers = TestCommon::makeHeaderValueOption({{"foo", "bar", false}}); - auto check_response = TestCommon::makeCheckResponse( - Grpc::Status::WellKnownGrpcStatus::Ok, envoy::type::v3::OK, empty_body, expected_headers); - auto authz_response = - TestCommon::makeAuthzResponse(CheckStatus::OK, Http::Code::OK, empty_body, expected_headers); + const auto expected_downstream_headers = TestCommon::makeHeaderValueOption( + {{"authorized-by", "TestAuthService", false}, {"cookie", "authtoken=1234", true}}); + auto check_response = + TestCommon::makeCheckResponse(Grpc::Status::WellKnownGrpcStatus::Ok, envoy::type::v3::OK, + empty_body, expected_headers, expected_downstream_headers); + auto authz_response = TestCommon::makeAuthzResponse( + CheckStatus::OK, Http::Code::OK, empty_body, expected_headers, expected_downstream_headers); envoy::service::auth::v3::CheckRequest request; expectCallSend(request); @@ -190,11 +193,13 @@ TEST_P(ExtAuthzGrpcClientTest, AuthorizationDeniedWithAllAttributes) { const std::string expected_body{"test"}; const auto expected_headers = TestCommon::makeHeaderValueOption({{"foo", "bar", false}, {"foobar", "bar", true}}); - auto check_response = - TestCommon::makeCheckResponse(Grpc::Status::WellKnownGrpcStatus::PermissionDenied, - envoy::type::v3::Unauthorized, expected_body, expected_headers); - auto authz_response = TestCommon::makeAuthzResponse(CheckStatus::Denied, Http::Code::Unauthorized, - expected_body, expected_headers); + const auto expected_downstream_headers = TestCommon::makeHeaderValueOption({}); + auto check_response = TestCommon::makeCheckResponse( + Grpc::Status::WellKnownGrpcStatus::PermissionDenied, envoy::type::v3::Unauthorized, + expected_body, expected_headers, expected_downstream_headers); + auto authz_response = + TestCommon::makeAuthzResponse(CheckStatus::Denied, Http::Code::Unauthorized, expected_body, + expected_headers, expected_downstream_headers); envoy::service::auth::v3::CheckRequest request; expectCallSend(request); 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 5798af81725a..97e51e11a505 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 @@ -89,6 +89,10 @@ class ExtAuthzHttpClientTest : public testing::Test { ignore_case: true - prefix: "X-" ignore_case: true + allowed_client_headers_on_success: + patterns: + - prefix: "X-Downstream-" + ignore_case: true )EOF"; TestUtility::loadFromYaml(default_yaml, proto_config); } else { @@ -296,8 +300,11 @@ using HeaderValuePair = std::paircheck(request_callbacks_, request, parent_span_, stream_info_); diff --git a/test/extensions/filters/common/ext_authz/test_common.cc b/test/extensions/filters/common/ext_authz/test_common.cc index 0f31d402750f..67429f891e2a 100644 --- a/test/extensions/filters/common/ext_authz/test_common.cc +++ b/test/extensions/filters/common/ext_authz/test_common.cc @@ -17,7 +17,8 @@ namespace ExtAuthz { CheckResponsePtr TestCommon::makeCheckResponse(Grpc::Status::GrpcStatus response_status, envoy::type::v3::StatusCode http_status_code, const std::string& body, - const HeaderValueOptionVector& headers) { + const HeaderValueOptionVector& headers, + const HeaderValueOptionVector& downstream_headers) { auto response = std::make_unique(); auto status = response->mutable_status(); status->set_code(response_status); @@ -46,13 +47,22 @@ CheckResponsePtr TestCommon::makeCheckResponse(Grpc::Status::GrpcStatus response item->CopyFrom(header); } } + if (!downstream_headers.empty()) { + const auto ok_response_headers_to_add = + response->mutable_ok_response()->mutable_response_headers_to_add(); + for (const auto& header : downstream_headers) { + auto* item = ok_response_headers_to_add->Add(); + item->CopyFrom(header); + } + } } return response; } Response TestCommon::makeAuthzResponse(CheckStatus status, Http::Code status_code, const std::string& body, - const HeaderValueOptionVector& headers) { + const HeaderValueOptionVector& headers, + const HeaderValueOptionVector& downstream_headers) { auto authz_response = Response{}; authz_response.status = status; authz_response.status_code = status_code; @@ -70,6 +80,12 @@ Response TestCommon::makeAuthzResponse(CheckStatus status, Http::Code status_cod } } } + if (!downstream_headers.empty()) { + for (auto& header : downstream_headers) { + authz_response.response_headers_to_add.emplace_back( + Http::LowerCaseString(header.header().key()), header.header().value()); + } + } return authz_response; } diff --git a/test/extensions/filters/common/ext_authz/test_common.h b/test/extensions/filters/common/ext_authz/test_common.h index 116973697fca..358f3a9a5fb8 100644 --- a/test/extensions/filters/common/ext_authz/test_common.h +++ b/test/extensions/filters/common/ext_authz/test_common.h @@ -32,16 +32,17 @@ class TestCommon { static Http::ResponseMessagePtr makeMessageResponse(const HeaderValueOptionVector& headers, const std::string& body = std::string{}); - static CheckResponsePtr makeCheckResponse( - Grpc::Status::GrpcStatus response_status = Grpc::Status::WellKnownGrpcStatus::Ok, - envoy::type::v3::StatusCode http_status_code = envoy::type::v3::OK, - const std::string& body = std::string{}, - const HeaderValueOptionVector& headers = HeaderValueOptionVector{}); + static CheckResponsePtr makeCheckResponse(Grpc::Status::GrpcStatus response_status, + envoy::type::v3::StatusCode http_status_code, + const std::string& body, + const HeaderValueOptionVector& headers, + const HeaderValueOptionVector& downstream_headers); static Response makeAuthzResponse(CheckStatus status, Http::Code status_code = Http::Code::OK, const std::string& body = std::string{}, - const HeaderValueOptionVector& headers = HeaderValueOptionVector{}); + const HeaderValueOptionVector& headers = HeaderValueOptionVector{}, + const HeaderValueOptionVector& downstream_headers = HeaderValueOptionVector{}); static HeaderValueOptionVector makeHeaderValueOption(KeyValueOptionVector&& headers); @@ -105,6 +106,12 @@ MATCHER_P(AuthzOkResponse, response, "") { return false; } + // Compare response_headers_to_add. + if (!TestCommon::compareHeaderVector(response.response_headers_to_add, + arg->response_headers_to_add)) { + return false; + } + return TestCommon::compareVectorOfHeaderName(response.headers_to_remove, arg->headers_to_remove); } diff --git a/test/extensions/filters/http/ext_authz/config_test.cc b/test/extensions/filters/http/ext_authz/config_test.cc index 0d48e3b481ba..2204f4985327 100644 --- a/test/extensions/filters/http/ext_authz/config_test.cc +++ b/test/extensions/filters/http/ext_authz/config_test.cc @@ -58,7 +58,7 @@ void expectCorrectProtoGrpc(envoy::config::core::v3::ApiVersion api_version) { })); Http::FilterFactoryCb cb = factory.createFilterFactoryFromProto(*proto_config, "stats", context); Http::MockFilterChainFactoryCallbacks filter_callback; - EXPECT_CALL(filter_callback, addStreamDecoderFilter(_)); + EXPECT_CALL(filter_callback, addStreamFilter(_)); cb(filter_callback); } @@ -125,7 +125,7 @@ TEST(HttpExtAuthzConfigTest, CorrectProtoHttp) { EXPECT_CALL(context, scope()); Http::FilterFactoryCb cb = factory.createFilterFactoryFromProto(*proto_config, "stats", context); testing::StrictMock filter_callback; - EXPECT_CALL(filter_callback, addStreamDecoderFilter(_)); + EXPECT_CALL(filter_callback, addStreamFilter(_)); cb(filter_callback); } diff --git a/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc b/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc index 4336b54dd38f..2a109d5ed897 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc +++ b/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc @@ -235,7 +235,8 @@ class ExtAuthzGrpcIntegrationTest : public Grpc::VersionedGrpcClientIntegrationP void sendExtAuthzResponse(const Headers& headers_to_add, const Headers& headers_to_append, const Headers& headers_to_remove, const Http::TestRequestHeaderMapImpl& new_headers_from_upstream, - const Http::TestRequestHeaderMapImpl& headers_to_append_multiple) { + const Http::TestRequestHeaderMapImpl& headers_to_append_multiple, + const Headers& response_headers_to_add) { ext_authz_request_->startGrpcStream(); envoy::service::auth::v3::CheckResponse check_response; check_response.mutable_status()->set_code(Grpc::Status::WellKnownGrpcStatus::Ok); @@ -285,6 +286,17 @@ class ExtAuthzGrpcIntegrationTest : public Grpc::VersionedGrpcClientIntegrationP return Http::HeaderMap::Iterate::Continue; }); + for (const auto& response_header_to_add : response_headers_to_add) { + auto* entry = check_response.mutable_ok_response()->mutable_response_headers_to_add()->Add(); + const auto key = std::string(response_header_to_add.first); + const auto value = std::string(response_header_to_add.second); + + entry->mutable_append()->set_value(false); + entry->mutable_header()->set_key(key); + entry->mutable_header()->set_value(value); + ENVOY_LOG_MISC(trace, "sendExtAuthzResponse: set response_header_to_add {}={}", key, value); + } + ext_authz_request_->sendGrpcMessage(check_response); ext_authz_request_->finishGrpcStream(Grpc::Status::Ok); } @@ -354,11 +366,12 @@ class ExtAuthzGrpcIntegrationTest : public Grpc::VersionedGrpcClientIntegrationP std::make_pair(header_to_append.first, header_to_append.second + "-appended")); } sendExtAuthzResponse(updated_headers_to_add, updated_headers_to_append, headers_to_remove, - new_headers_from_upstream, headers_to_append_multiple); + new_headers_from_upstream, headers_to_append_multiple, Headers{}); waitForSuccessfulUpstreamResponse("200", updated_headers_to_add, updated_headers_to_append, headers_to_remove, new_headers_from_upstream, headers_to_append_multiple); + cleanup(); } @@ -592,7 +605,8 @@ TEST_P(ExtAuthzGrpcIntegrationTest, HTTP2DownstreamRequestWithLargeBody) { } // Verifies that the original request headers will be added and appended when the authorization -// server returns headers_to_add and headers_to_append in OkResponse message. +// server returns headers_to_add, response_headers_to_add, and headers_to_append in OkResponse +// message. TEST_P(ExtAuthzGrpcIntegrationTest, SendHeadersToAddAndToAppendToUpstream) { XDS_DEPRECATED_FEATURE_TEST_SKIP; expectCheckRequestWithBodyWithHeaders( @@ -626,6 +640,38 @@ TEST_P(ExtAuthzGrpcIntegrationTest, DenyAtDisableWithMetadata) { expectFilterDisableCheck(/*deny_at_disable=*/true, /*disable_with_metadata=*/true, "403"); } +TEST_P(ExtAuthzGrpcIntegrationTest, DownstreamHeadersOnSuccess) { + XDS_DEPRECATED_FEATURE_TEST_SKIP; + // Set up ext_authz filter. + initializeConfig(); + + // Use h1, set up the test. + setDownstreamProtocol(Http::CodecClient::Type::HTTP1); + HttpIntegrationTest::initialize(); + + // Start a client connection and request. + initiateClientConnection(0); + + // Wait for the ext_authz request as a result of the client request. + waitForExtAuthzRequest(expectedCheckRequest(Http::CodecClient::Type::HTTP1)); + + // Send back an ext_authz response with response_headers_to_add set. + sendExtAuthzResponse(Headers{}, Headers{}, Headers{}, Http::TestRequestHeaderMapImpl{}, + Http::TestRequestHeaderMapImpl{}, + Headers{{"downstream2", "downstream-should-see-me"}}); + + // Wait for the upstream response. + waitForSuccessfulUpstreamResponse("200"); + + // Verify the response is HTTP 200 with the header from `response_headers_to_add` above. + const std::string expected_body(response_size_, 'a'); + verifyResponse(std::move(response_), "200", + Http::TestResponseHeaderMapImpl{{":status", "200"}, + {"downstream2", "downstream-should-see-me"}}, + expected_body); + cleanup(); +} + INSTANTIATE_TEST_SUITE_P(IpVersions, ExtAuthzHttpIntegrationTest, ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); @@ -767,7 +813,7 @@ TEST_P(ExtAuthzGrpcIntegrationTest, GoogleAsyncClientCreation) { initiateClientConnection(4, Headers{}, Headers{}); waitForExtAuthzRequest(expectedCheckRequest(Http::CodecClient::Type::HTTP2)); sendExtAuthzResponse(Headers{}, Headers{}, Headers{}, Http::TestRequestHeaderMapImpl{}, - Http::TestRequestHeaderMapImpl{}); + Http::TestRequestHeaderMapImpl{}, Headers{}); waitForSuccessfulUpstreamResponse("200"); @@ -797,7 +843,7 @@ TEST_P(ExtAuthzGrpcIntegrationTest, GoogleAsyncClientCreation) { test_server_->counter("grpc.ext_authz.google_grpc_client_creation")->value()); } sendExtAuthzResponse(Headers{}, Headers{}, Headers{}, Http::TestRequestHeaderMapImpl{}, - Http::TestRequestHeaderMapImpl{}); + Http::TestRequestHeaderMapImpl{}, Headers{}); result = fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_); RELEASE_ASSERT(result, result.message()); diff --git a/test/extensions/filters/http/ext_authz/ext_authz_test.cc b/test/extensions/filters/http/ext_authz/ext_authz_test.cc index 677349a277ea..358349aa7973 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc +++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc @@ -63,6 +63,7 @@ template class HttpFilterTestBase : public T { client_ = new Filters::Common::ExtAuthz::MockClient(); filter_ = std::make_unique(config_, Filters::Common::ExtAuthz::ClientPtr{client_}); filter_->setDecoderFilterCallbacks(filter_callbacks_); + filter_->setEncoderFilterCallbacks(encoder_filter_callbacks_); addr_ = std::make_shared("1.2.3.4", 1111); } @@ -77,6 +78,7 @@ template class HttpFilterTestBase : public T { Filters::Common::ExtAuthz::MockClient* client_; std::unique_ptr filter_; NiceMock filter_callbacks_; + NiceMock encoder_filter_callbacks_; Filters::Common::ExtAuthz::RequestCallbacks* request_callbacks_; Http::TestRequestHeaderMapImpl request_headers_; Http::TestRequestTrailerMapImpl request_trailers_; @@ -1709,6 +1711,8 @@ TEST_P(HttpFilterTestParam, ImmediateOkResponseWithHttpAttributes) { response.headers_to_append = Http::HeaderVector{{request_header_key, "bar"}}; response.headers_to_set = Http::HeaderVector{{key_to_add, "foo"}, {key_to_override, "bar"}}; response.headers_to_remove = std::vector{key_to_remove}; + response.response_headers_to_add = + Http::HeaderVector{{Http::LowerCaseString{"cookie"}, "flavor=gingerbread"}}; auto response_ptr = std::make_unique(response); @@ -1726,6 +1730,16 @@ TEST_P(HttpFilterTestParam, ImmediateOkResponseWithHttpAttributes) { EXPECT_EQ(request_headers_.get_(key_to_add), "foo"); EXPECT_EQ(request_headers_.get_(key_to_override), "bar"); EXPECT_EQ(request_headers_.has(key_to_remove), false); + + Buffer::OwnedImpl response_data{}; + Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}}; + Http::TestResponseTrailerMapImpl response_trailers{}; + Http::MetadataMap response_metadata{}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(response_headers, false)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(response_trailers)); + EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->encodeMetadata(response_metadata)); + EXPECT_EQ(response_headers.get_("cookie"), "flavor=gingerbread"); } // Test that an synchronous denied response from the authorization service, on the call stack, diff --git a/test/extensions/filters/network/common/fuzz/uber_per_readfilter.cc b/test/extensions/filters/network/common/fuzz/uber_per_readfilter.cc index f6976aedf6b9..be8d0628743f 100644 --- a/test/extensions/filters/network/common/fuzz/uber_per_readfilter.cc +++ b/test/extensions/filters/network/common/fuzz/uber_per_readfilter.cc @@ -68,9 +68,11 @@ void UberFilterFuzzer::perFilterSetup(const std::string& filter_name) { const std::string empty_body{}; const auto expected_headers = Filters::Common::ExtAuthz::TestCommon::makeHeaderValueOption({}); + const auto expected_downstream_headers = + Filters::Common::ExtAuthz::TestCommon::makeHeaderValueOption({}); auto check_response = Filters::Common::ExtAuthz::TestCommon::makeCheckResponse( Grpc::Status::WellKnownGrpcStatus::Ok, envoy::type::v3::OK, empty_body, - expected_headers); + expected_headers, expected_downstream_headers); // Give response to the grpc_client by calling onSuccess(). grpc_client_impl->onSuccess(std::move(check_response), span_); return async_request_.get();