diff --git a/api/envoy/api/v2/core/BUILD b/api/envoy/api/v2/core/BUILD index 666315758b826..1b5f9aba76297 100644 --- a/api/envoy/api/v2/core/BUILD +++ b/api/envoy/api/v2/core/BUILD @@ -80,6 +80,11 @@ api_go_proto_library( ], ) +api_go_proto_library( + name = "http_uri", + proto = ":http_uri", +) + api_proto_library( name = "http_uri", srcs = ["http_uri.proto"], @@ -88,11 +93,6 @@ api_proto_library( ], ) -api_go_proto_library( - name = "http_uri", - proto = ":http_uri", -) - api_proto_library( name = "grpc_service", srcs = ["grpc_service.proto"], diff --git a/api/envoy/config/filter/http/ext_authz/v2alpha/ext_authz.proto b/api/envoy/config/filter/http/ext_authz/v2alpha/ext_authz.proto index 9d602298ce170..13a643a6ad81c 100644 --- a/api/envoy/config/filter/http/ext_authz/v2alpha/ext_authz.proto +++ b/api/envoy/config/filter/http/ext_authz/v2alpha/ext_authz.proto @@ -6,24 +6,17 @@ option go_package = "v2alpha"; import "envoy/api/v2/core/grpc_service.proto"; import "envoy/api/v2/core/http_uri.proto"; -// [#protodoc-title: HTTP External Authorization ] -// The external authorization HTTP service configuration +// [#protodoc-title: External Authorization ] +// The external authorization service configuration // :ref:`configuration overview `. -// [#not-implemented-hide:] -// [#comment: The HttpService is under development and will be supported soon.] -message HttpService { - // Sets the HTTP server URI which the authorization requests must be sent to. - envoy.api.v2.core.HttpUri server_uri = 1; - - // Sets an optional prefix to the value of authorization request header `path`. - string path_prefix = 2; -} - -// External Authorization filter calls out to an external service over the -// gRPC Authorization API defined by -// :ref:`CheckRequest `. -// A failed check will cause this filter to close the HTTP request with 403(Forbidden). +// External Authorization filter calls out to an external service over either: +// +// 1. gRPC Authorization API defined by :ref:`CheckRequest `. +// 2. Raw HTTP Authorization server by passing the request headers to the service. +// +// A failed check will cause this filter to close the HTTP request normally with 403 (Forbidden), unless +// a different status code has been indicated in the authorization response. message ExtAuthz { oneof services { @@ -32,7 +25,7 @@ message ExtAuthz { envoy.api.v2.core.GrpcService grpc_service = 1; // The external authorization HTTP service configuration. - // [#not-implemented-hide:] + // The default timeout is set to 200ms by this filter. HttpService http_service = 3; } @@ -42,3 +35,28 @@ message ExtAuthz { // Defaults to false. bool failure_mode_allow = 2; } + +// External Authorization filter calls out to an upstream authorization server by passing the raw HTTP +// request headers to the server. This allows the authorization service to take a decision whether the +// request is authorized or not. +// +// A successful check allows the authorization service adding or overriding headers from the original +// request before dispatching it to the upstream. This is done by including the headers in the response +// sent back from the authorization service to the filter. Note that `Status`, `Method`, `Path` and +// `Content Length` response headers are automatically removed from this response by the filter. If other +// headers need be deleted, they should be specified in `response_headers_to_remove` field. +// +// A failed check will cause this filter to close the HTTP request normally with 403 (Forbidden), unless +// a different status code has been indicated by the authorization service via response headers. The HTTP +// service also allows the authorization filter to also pass data from the response body to the downstream +// client in case of a denied request. +message HttpService { + // Sets the HTTP server URI which the authorization requests must be sent to. + envoy.api.v2.core.HttpUri server_uri = 1; + + // Sets an optional prefix to the value of authorization request header `path`. + string path_prefix = 2; + + // Sets a list of headers that should be not be sent *from the authorization server* to the upstream. + repeated string response_headers_to_remove = 3; +} diff --git a/api/envoy/service/auth/v2alpha/BUILD b/api/envoy/service/auth/v2alpha/BUILD index 323a49eee7dff..4f44cafc8606c 100644 --- a/api/envoy/service/auth/v2alpha/BUILD +++ b/api/envoy/service/auth/v2alpha/BUILD @@ -20,5 +20,7 @@ api_proto_library( has_services = 1, deps = [ ":attribute_context", + "//envoy/api/v2/core:base", + "//envoy/type:http_status", ], ) diff --git a/api/envoy/service/auth/v2alpha/external_auth.proto b/api/envoy/service/auth/v2alpha/external_auth.proto index 601c4dea6c218..3bfa3a60358a7 100644 --- a/api/envoy/service/auth/v2alpha/external_auth.proto +++ b/api/envoy/service/auth/v2alpha/external_auth.proto @@ -4,6 +4,8 @@ package envoy.service.auth.v2alpha; option go_package = "v2alpha"; option java_generic_services = true; +import "envoy/api/v2/core/base.proto"; +import "envoy/type/http_status.proto"; import "envoy/service/auth/v2alpha/attribute_context.proto"; import "google/rpc/status.proto"; @@ -27,21 +29,45 @@ message CheckRequest { AttributeContext attributes = 1; } +// HTTP attributes for a denied response. +message DeniedHttpResponse { + // This field allows the authorization service to send a HTTP response status + // code to the downstream client other than 403 (Forbidden). + envoy.type.HttpStatus status = 1 [(validate.rules).message.required = true]; + + // This field allows the authorization service to send HTTP response headers + // to the the downstream client. + repeated envoy.api.v2.core.HeaderValueOption headers = 2; + + // This field allows the authorization service to send a response body data + // to the the downstream client. + string body = 3; +} + +// HTTP attributes for an ok response. +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. By setting `append` field to `true` in the `HeaderValueOption`, + // the filter will append the correspondent header value to the matched request header. Note that + // by Leaving `append` as false, the filter will either add a new header, or override an existing + // one if there is a match. + repeated envoy.api.v2.core.HeaderValueOption headers = 2; +} + +// Intended for gRPC and Network Authorization servers `only`. message CheckResponse { // Status `OK` allows the request. Any other status indicates the request should be denied. google.rpc.Status status = 1; - // An optional message that contains HTTP response attributes. This message is + // An message that contains HTTP response attributes. This message is // used when the authorization service needs to send custom responses to the // downstream client or, to modify/add request headers being dispatched to the upstream. - message HttpResponse { - // Http status code. - uint32 status_code = 1 [(validate.rules).uint32 = {gte: 100, lt: 600}]; - - // Http entity headers. - map headers = 2; - - // Http entity body. - string body = 3; + oneof http_response { + // Supplies http attributes for a denied response. + DeniedHttpResponse denied_response = 2; + + // Supplies http attributes for an ok response. + OkHttpResponse ok_response = 3; } } diff --git a/api/envoy/type/BUILD b/api/envoy/type/BUILD index 4859476efbd9d..0c193bcc1ced0 100644 --- a/api/envoy/type/BUILD +++ b/api/envoy/type/BUILD @@ -2,6 +2,17 @@ load("//bazel:api_build_system.bzl", "api_go_proto_library", "api_proto_library" licenses(["notice"]) # Apache 2 +api_proto_library( + name = "http_status", + srcs = ["http_status.proto"], + visibility = ["//visibility:public"], +) + +api_go_proto_library( + name = "http_status", + proto = ":http_status", +) + api_proto_library( name = "percent", srcs = ["percent.proto"], diff --git a/api/envoy/type/http_status.proto b/api/envoy/type/http_status.proto new file mode 100644 index 0000000000000..75769495f87f3 --- /dev/null +++ b/api/envoy/type/http_status.proto @@ -0,0 +1,83 @@ +syntax = "proto3"; + +package envoy.type; + +import "validate/validate.proto"; + +// HTTP response codes supported in Envoy. +// For more details: http://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml +enum StatusCode { + // Empty - This code not part of the HTTP status code specification, but it is needed for proto `enum` type. + Empty = 0; + + Continue = 100; + + OK = 200; + Created = 201; + Accepted = 202; + NonAuthoritativeInformation = 203; + NoContent = 204; + ResetContent = 205; + PartialContent = 206; + MultiStatus = 207; + AlreadyReported = 208; + IMUsed = 226; + + MultipleChoices = 300; + MovedPermanently = 301; + Found = 302; + SeeOther = 303; + NotModified = 304; + UseProxy = 305; + TemporaryRedirect = 307; + PermanentRedirect = 308; + + BadRequest = 400; + Unauthorized = 401; + PaymentRequired = 402; + Forbidden = 403; + NotFound = 404; + MethodNotAllowed = 405; + NotAcceptable = 406; + ProxyAuthenticationRequired = 407; + RequestTimeout = 408; + Conflict = 409; + Gone = 410; + LengthRequired = 411; + PreconditionFailed = 412; + PayloadTooLarge = 413; + URITooLong = 414; + UnsupportedMediaType = 415; + RangeNotSatisfiable = 416; + ExpectationFailed = 417; + MisdirectedRequest = 421; + UnprocessableEntity = 422; + Locked = 423; + FailedDependency = 424; + UpgradeRequired = 426; + PreconditionRequired = 428; + TooManyRequests = 429; + RequestHeaderFieldsTooLarge = 431; + + InternalServerError = 500; + NotImplemented = 501; + BadGateway = 502; + ServiceUnavailable = 503; + GatewayTimeout = 504; + HTTPVersionNotSupported = 505; + VariantAlsoNegotiates = 506; + InsufficientStorage = 507; + LoopDetected = 508; + NotExtended = 510; + NetworkAuthenticationRequired = 511; + +} + +// HTTP status. +message HttpStatus { + // Supplies HTTP response code. + StatusCode code = 1 [ + (validate.rules).enum = {not_in: [0]}, + (validate.rules).enum.defined_only = true + ]; +} diff --git a/docs/build.sh b/docs/build.sh index a5596ca0d7600..9425de5d9c546 100755 --- a/docs/build.sh +++ b/docs/build.sh @@ -108,6 +108,7 @@ PROTO_RST=" /envoy/service/accesslog/v2/als/envoy/service/accesslog/v2/als.proto.rst /envoy/service/auth/v2alpha/external_auth/envoy/service/auth/v2alpha/attribute_context.proto.rst /envoy/service/auth/v2alpha/external_auth/envoy/service/auth/v2alpha/external_auth.proto.rst + /envoy/type/http_status/envoy/type/http_status.proto.rst /envoy/type/percent/envoy/type/percent.proto.rst /envoy/type/range/envoy/type/range.proto.rst " diff --git a/docs/root/api-v2/types/types.rst b/docs/root/api-v2/types/types.rst index 116d6c3cb519c..cac37fffb59f7 100644 --- a/docs/root/api-v2/types/types.rst +++ b/docs/root/api-v2/types/types.rst @@ -5,5 +5,6 @@ Types :glob: :maxdepth: 2 + ../type/http_status.proto ../type/percent.proto ../type/range.proto diff --git a/docs/root/configuration/http_filters/ext_authz_filter.rst b/docs/root/configuration/http_filters/ext_authz_filter.rst index c9eabf8f94819..bd92156303970 100644 --- a/docs/root/configuration/http_filters/ext_authz_filter.rst +++ b/docs/root/configuration/http_filters/ext_authz_filter.rst @@ -5,9 +5,11 @@ External Authorization * External authorization :ref:`architecture overview ` * :ref:`HTTP filter v2 API reference ` -The external authorization HTTP filter calls an external gRPC service to check if the incoming +The external authorization HTTP filter calls an external gRPC or HTTP service to check if the incoming HTTP request is authorized or not. -If the request is deemed unauthorized then the request will be denied with 403 (Forbidden) response. +If the request is deemed unauthorized then the request will be denied normally with 403 (Forbidden) response. +Note that sending additional custom metadata from the authorization service to the upstream, or to the downstream is +also possible. This is explained in more details at :ref:`HTTP filter `. .. tip:: It is recommended that this filter is configured first in the filter chain so that requests are @@ -18,14 +20,14 @@ The content of the requests that are passed to an authorization service is speci .. _config_http_filters_ext_authz_http_configuration: -The HTTP filter, using a gRPC service, can be configured as follows. You can see all the +The HTTP filter, using a gRPC/HTTP service, can be configured as follows. You can see all the configuration options at :ref:`HTTP filter `. -Example -------- +Configuration Examples +----------------------------- -A sample filter configuration could be: +A sample filter configuration for a gRPC authorization server: .. code-block:: yaml @@ -36,6 +38,8 @@ A sample filter configuration could be: envoy_grpc: cluster_name: ext-authz +.. code-block:: yaml + clusters: - name: ext-authz type: static @@ -43,6 +47,30 @@ A sample filter configuration could be: hosts: - socket_address: { address: 127.0.0.1, port_value: 10003 } +A sample filter configuration for a raw HTTP authorization server: + +.. code-block:: yaml + + http_filters: + - name: envoy.ext_authz + config: + http_service: + server_uri: + uri: 127.0.0.1:10003 + cluster: ext-authz + timeout: 0.25s + failure_mode_allow: false + +.. code-block:: yaml + + clusters: + - name: ext-authz + connect_timeout: 0.25s + type: logical_dns + lb_policy: round_robin + hosts: + - socket_address: { address: 127.0.0.1, port_value: 10003 } + Statistics ---------- The HTTP filter outputs statistics in the *cluster..ext_authz.* namespace. diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 3844815f0887d..114e40173c18c 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -45,8 +45,10 @@ Version history to close tcp_proxy upstream connections when health checks fail. * cluster: added :ref:`option ` to drain connections from hosts after they are removed from service discovery, regardless of health status. -* cluster: fixed bug preventing the deletion of all endpoints in a priority. -* debug: added symbolized stack traces (where supported). +* cluster: fixed bug preventing the deletion of all endpoints in a priority +* debug: added symbolized stack traces (where supported) +* ext-authz filter: added support to raw HTTP authorization. +* ext-authz filter: added support to gRPC responses to carry HTTP attributes. * grpc: support added for the full set of :ref:`Google gRPC call credentials `. * gzip filter: added :ref:`stats ` to the filter. diff --git a/include/envoy/http/header_map.h b/include/envoy/http/header_map.h index 7bdf89436a8d8..b14193d0d2c1a 100644 --- a/include/envoy/http/header_map.h +++ b/include/envoy/http/header_map.h @@ -498,5 +498,10 @@ class HeaderMap { typedef std::unique_ptr HeaderMapPtr; +/** + * Convenient container type for storing Http::LowerCaseString and std::string key/value pairs. + */ +typedef std::vector> HeaderVector; + } // namespace Http } // namespace Envoy diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 13ab86a359b86..002c311e35bd3 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -80,6 +80,7 @@ envoy_cc_library( "//include/envoy/stats:stats_interface", "//source/common/common:enum_to_int", "//source/common/common:utility_lib", + "@envoy_api//envoy/type:http_status_cc", ], ) diff --git a/source/extensions/filters/common/ext_authz/BUILD b/source/extensions/filters/common/ext_authz/BUILD index c43ff4924e8b6..d954a8acd6b97 100644 --- a/source/extensions/filters/common/ext_authz/BUILD +++ b/source/extensions/filters/common/ext_authz/BUILD @@ -12,16 +12,18 @@ envoy_cc_library( name = "ext_authz_interface", hdrs = ["ext_authz.h"], deps = [ - "//include/envoy/tracing:http_tracer_interface", + "//include/envoy/http:codes_interface", + "//source/common/tracing:http_tracer_lib", "@envoy_api//envoy/service/auth/v2alpha:external_auth_cc", ], ) envoy_cc_library( - name = "ext_authz_lib", - srcs = ["ext_authz_impl.cc"], - hdrs = ["ext_authz_impl.h"], + name = "ext_authz_grpc_lib", + srcs = ["ext_authz_grpc_impl.cc"], + hdrs = ["ext_authz_grpc_impl.h"], deps = [ + ":check_request_utils_lib", ":ext_authz_interface", "//include/envoy/grpc:async_client_interface", "//include/envoy/grpc:async_client_manager_interface", @@ -31,7 +33,6 @@ envoy_cc_library( "//include/envoy/network:address_interface", "//include/envoy/network:connection_interface", "//include/envoy/network:filter_interface", - "//include/envoy/ssl:connection_interface", "//include/envoy/upstream:cluster_manager_interface", "//source/common/common:assert_lib", "//source/common/grpc:async_client_lib", @@ -42,3 +43,29 @@ envoy_cc_library( "//source/common/tracing:http_tracer_lib", ], ) + +envoy_cc_library( + name = "ext_authz_http_lib", + srcs = ["ext_authz_http_impl.cc"], + hdrs = ["ext_authz_http_impl.h"], + deps = [ + ":check_request_utils_lib", + ":ext_authz_interface", + "//source/common/common:minimal_logger_lib", + "//source/common/http:async_client_lib", + ], +) + +envoy_cc_library( + name = "check_request_utils_lib", + srcs = ["check_request_utils.cc"], + hdrs = ["check_request_utils.h"], + deps = [ + "//include/envoy/grpc:async_client_interface", + "//include/envoy/grpc:async_client_manager_interface", + "//include/envoy/http:filter_interface", + "//include/envoy/upstream:cluster_manager_interface", + "//source/common/grpc:async_client_lib", + "@envoy_api//envoy/service/auth/v2alpha:external_auth_cc", + ], +) diff --git a/source/extensions/filters/common/ext_authz/ext_authz_impl.cc b/source/extensions/filters/common/ext_authz/check_request_utils.cc similarity index 72% rename from source/extensions/filters/common/ext_authz/ext_authz_impl.cc rename to source/extensions/filters/common/ext_authz/check_request_utils.cc index 49fe65c1385b9..90f82d372def4 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz_impl.cc +++ b/source/extensions/filters/common/ext_authz/check_request_utils.cc @@ -1,72 +1,30 @@ -#include "extensions/filters/common/ext_authz/ext_authz_impl.h" +#include "extensions/filters/common/ext_authz/check_request_utils.h" #include #include #include #include -#include "envoy/access_log/access_log.h" #include "envoy/ssl/connection.h" +#include "common/buffer/buffer_impl.h" #include "common/common/assert.h" +#include "common/common/enum_to_int.h" #include "common/grpc/async_client_impl.h" +#include "common/http/codes.h" #include "common/http/headers.h" #include "common/http/utility.h" #include "common/network/utility.h" #include "common/protobuf/protobuf.h" +#include "absl/strings/str_cat.h" + namespace Envoy { namespace Extensions { namespace Filters { namespace Common { namespace ExtAuthz { -GrpcClientImpl::GrpcClientImpl(Grpc::AsyncClientPtr&& async_client, - const absl::optional& timeout) - : service_method_(*Protobuf::DescriptorPool::generated_pool()->FindMethodByName( - // TODO(dio): Define the following service method name as a constant value. - "envoy.service.auth.v2alpha.Authorization.Check")), - async_client_(std::move(async_client)), timeout_(timeout) {} - -GrpcClientImpl::~GrpcClientImpl() { ASSERT(!callbacks_); } - -void GrpcClientImpl::cancel() { - ASSERT(callbacks_ != nullptr); - request_->cancel(); - callbacks_ = nullptr; -} - -void GrpcClientImpl::check(RequestCallbacks& callbacks, - const envoy::service::auth::v2alpha::CheckRequest& request, - Tracing::Span& parent_span) { - ASSERT(callbacks_ == nullptr); - callbacks_ = &callbacks; - - request_ = async_client_->send(service_method_, request, *this, parent_span, timeout_); -} - -void GrpcClientImpl::onSuccess( - std::unique_ptr&& response, Tracing::Span& span) { - CheckStatus status = CheckStatus::OK; - ASSERT(response->status().code() != Grpc::Status::GrpcStatus::Unknown); - if (response->status().code() != Grpc::Status::GrpcStatus::Ok) { - status = CheckStatus::Denied; - span.setTag(Constants::get().TraceStatus, Constants::get().TraceUnauthz); - } else { - span.setTag(Constants::get().TraceStatus, Constants::get().TraceOk); - } - - callbacks_->onComplete(status); - callbacks_ = nullptr; -} - -void GrpcClientImpl::onFailure(Grpc::Status::GrpcStatus status, const std::string&, - Tracing::Span&) { - ASSERT(status != Grpc::Status::GrpcStatus::Ok); - callbacks_->onComplete(CheckStatus::Error); - callbacks_ = nullptr; -} - void CheckRequestUtils::setAttrContextPeer( envoy::service::auth::v2alpha::AttributeContext_Peer& peer, const Network::Connection& connection, const std::string& service, const bool local) { diff --git a/source/extensions/filters/common/ext_authz/ext_authz_impl.h b/source/extensions/filters/common/ext_authz/check_request_utils.h similarity index 66% rename from source/extensions/filters/common/ext_authz/ext_authz_impl.h rename to source/extensions/filters/common/ext_authz/check_request_utils.h index f3266c4875b2e..fa94c45fbc3e7 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz_impl.h +++ b/source/extensions/filters/common/ext_authz/check_request_utils.h @@ -13,59 +13,19 @@ #include "envoy/network/address.h" #include "envoy/network/connection.h" #include "envoy/network/filter.h" +#include "envoy/service/auth/v2alpha/external_auth.pb.h" #include "envoy/tracing/http_tracer.h" #include "envoy/upstream/cluster_manager.h" +#include "common/http/async_client_impl.h" #include "common/singleton/const_singleton.h" -#include "extensions/filters/common/ext_authz/ext_authz.h" - namespace Envoy { namespace Extensions { namespace Filters { namespace Common { namespace ExtAuthz { -typedef Grpc::TypedAsyncRequestCallbacks - ExtAuthzAsyncCallbacks; - -struct ConstantValues { - const std::string TraceStatus = "ext_authz_status"; - const std::string TraceUnauthz = "ext_authz_unauthorized"; - const std::string TraceOk = "ext_authz_ok"; -}; - -typedef ConstSingleton Constants; - -// NOTE: We create gRPC client for each filter stack instead of a client per thread. -// That is ok since this is unary RPC and the cost of doing this is minimal. -class GrpcClientImpl : public Client, public ExtAuthzAsyncCallbacks { -public: - GrpcClientImpl(Grpc::AsyncClientPtr&& async_client, - const absl::optional& timeout); - ~GrpcClientImpl(); - - // ExtAuthz::Client - void cancel() override; - void check(RequestCallbacks& callbacks, - const envoy::service::auth::v2alpha::CheckRequest& request, - Tracing::Span& parent_span) override; - - // Grpc::AsyncRequestCallbacks - void onCreateInitialMetadata(Http::HeaderMap&) override {} - void onSuccess(std::unique_ptr&& response, - Tracing::Span& span) override; - void onFailure(Grpc::Status::GrpcStatus status, const std::string& message, - Tracing::Span& span) override; - -private: - const Protobuf::MethodDescriptor& service_method_; - Grpc::AsyncClientPtr async_client_; - Grpc::AsyncRequest* request_{}; - absl::optional timeout_; - RequestCallbacks* callbacks_{}; -}; - /** * For creating ext_authz.proto (authorization) request. * CheckRequestUtils is used to extract attributes from the TCP/HTTP request diff --git a/source/extensions/filters/common/ext_authz/ext_authz.h b/source/extensions/filters/common/ext_authz/ext_authz.h index 53c67b749b908..0d242f622822a 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz.h +++ b/source/extensions/filters/common/ext_authz/ext_authz.h @@ -6,6 +6,7 @@ #include #include "envoy/common/pure.h" +#include "envoy/http/codes.h" #include "envoy/service/auth/v2alpha/external_auth.pb.h" #include "envoy/tracing/http_tracer.h" @@ -27,6 +28,24 @@ enum class CheckStatus { Denied }; +/** + * Authorization response object for a RequestCallback. + */ +struct Response { + // Call status. + CheckStatus status; + // Optional http headers used on either denied or ok responses. + Http::HeaderVector headers_to_append; + // Optional http headers used on either denied or ok responses. + Http::HeaderVector headers_to_add; + // Optional http body used only on denied response. + std::string body; + // Optional http status used only on denied response. + Http::Code status_code{}; +}; + +typedef std::unique_ptr ResponsePtr; + /** * Async callbacks used during check() calls. */ @@ -35,9 +54,9 @@ class RequestCallbacks { virtual ~RequestCallbacks() {} /** - * Called when a check request is complete. The resulting status is supplied. + * Called when a check request is complete. The resulting ResponsePtr is supplied. */ - virtual void onComplete(CheckStatus status) PURE; + virtual void onComplete(ResponsePtr&& response) PURE; }; class Client { 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 new file mode 100644 index 0000000000000..2dc5cd33023d0 --- /dev/null +++ b/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc @@ -0,0 +1,95 @@ +#include "extensions/filters/common/ext_authz/ext_authz_grpc_impl.h" + +#include "common/common/assert.h" +#include "common/grpc/async_client_impl.h" +#include "common/http/headers.h" +#include "common/http/utility.h" +#include "common/network/utility.h" +#include "common/protobuf/protobuf.h" + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace ExtAuthz { + +GrpcClientImpl::GrpcClientImpl(Grpc::AsyncClientPtr&& async_client, + const absl::optional& timeout) + : service_method_(*Protobuf::DescriptorPool::generated_pool()->FindMethodByName( + // TODO(dio): Define the following service method name as a constant value. + "envoy.service.auth.v2alpha.Authorization.Check")), + async_client_(std::move(async_client)), timeout_(timeout) {} + +GrpcClientImpl::~GrpcClientImpl() { ASSERT(!callbacks_); } + +void GrpcClientImpl::cancel() { + ASSERT(callbacks_ != nullptr); + request_->cancel(); + callbacks_ = nullptr; +} + +void GrpcClientImpl::check(RequestCallbacks& callbacks, + const envoy::service::auth::v2alpha::CheckRequest& request, + Tracing::Span& parent_span) { + ASSERT(callbacks_ == nullptr); + callbacks_ = &callbacks; + + request_ = async_client_->send(service_method_, request, *this, parent_span, timeout_); +} + +void GrpcClientImpl::onSuccess( + std::unique_ptr&& response, Tracing::Span& span) { + ASSERT(response->status().code() != Grpc::Status::GrpcStatus::Unknown); + ResponsePtr authz_response = std::make_unique(Response{}); + + if (response->status().code() == Grpc::Status::GrpcStatus::Ok) { + span.setTag(Constants::get().TraceStatus, Constants::get().TraceOk); + authz_response->status = CheckStatus::OK; + if (response->has_ok_response()) { + toAuthzResponseHeader(authz_response, response->ok_response().headers()); + } + } else { + span.setTag(Constants::get().TraceStatus, Constants::get().TraceUnauthz); + authz_response->status = CheckStatus::Denied; + if (response->has_denied_response()) { + toAuthzResponseHeader(authz_response, response->denied_response().headers()); + authz_response->status_code = + static_cast(response->denied_response().status().code()); + authz_response->body = response->denied_response().body(); + } else { + authz_response->status_code = Http::Code::Forbidden; + } + } + + callbacks_->onComplete(std::move(authz_response)); + callbacks_ = nullptr; +} + +void GrpcClientImpl::onFailure(Grpc::Status::GrpcStatus status, const std::string&, + Tracing::Span&) { + ASSERT(status != Grpc::Status::GrpcStatus::Ok); + ResponsePtr authz_response = std::make_unique(Response{}); + authz_response->status = CheckStatus::Error; + callbacks_->onComplete(std::move(authz_response)); + callbacks_ = nullptr; +} + +void GrpcClientImpl::toAuthzResponseHeader( + ResponsePtr& response, + const Protobuf::RepeatedPtrField& headers) { + for (const auto& header : headers) { + if (header.append().value()) { + response->headers_to_append.emplace_back(Http::LowerCaseString(header.header().key()), + header.header().value()); + } else { + response->headers_to_add.emplace_back(Http::LowerCaseString(header.header().key()), + header.header().value()); + } + } +} + +} // namespace ExtAuthz +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.h b/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.h new file mode 100644 index 0000000000000..0c44a92aebdc2 --- /dev/null +++ b/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.h @@ -0,0 +1,82 @@ +#pragma once + +#include +#include +#include +#include + +#include "envoy/grpc/async_client.h" +#include "envoy/grpc/async_client_manager.h" +#include "envoy/http/filter.h" +#include "envoy/http/header_map.h" +#include "envoy/http/protocol.h" +#include "envoy/network/address.h" +#include "envoy/network/connection.h" +#include "envoy/network/filter.h" +#include "envoy/tracing/http_tracer.h" +#include "envoy/upstream/cluster_manager.h" + +#include "common/singleton/const_singleton.h" + +#include "extensions/filters/common/ext_authz/check_request_utils.h" +#include "extensions/filters/common/ext_authz/ext_authz.h" + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace ExtAuthz { + +typedef Grpc::TypedAsyncRequestCallbacks + ExtAuthzAsyncCallbacks; + +struct ConstantValues { + const std::string TraceStatus = "ext_authz_status"; + const std::string TraceUnauthz = "ext_authz_unauthorized"; + const std::string TraceOk = "ext_authz_ok"; +}; + +typedef ConstSingleton Constants; + +/* + * This client implementation is used when the Ext_Authz filter needs to communicate with an gRPC + * authorization server. Unlike the HTTP client, the gRPC allows the server to define response + * objects which contain the HTTP attributes to be sent to the upstream or to the downstream client. + * The gRPC client does not rewrite path. NOTE: We create gRPC client for each filter stack instead + * of a client per thread. That is ok since this is unary RPC and the cost of doing this is minimal. + */ +class GrpcClientImpl : public Client, public ExtAuthzAsyncCallbacks { +public: + GrpcClientImpl(Grpc::AsyncClientPtr&& async_client, + const absl::optional& timeout); + ~GrpcClientImpl(); + + // ExtAuthz::Client + void cancel() override; + void check(RequestCallbacks& callbacks, + const envoy::service::auth::v2alpha::CheckRequest& request, + Tracing::Span& parent_span) override; + + // Grpc::AsyncRequestCallbacks + void onCreateInitialMetadata(Http::HeaderMap&) override {} + void onSuccess(std::unique_ptr&& response, + Tracing::Span& span) override; + void onFailure(Grpc::Status::GrpcStatus status, const std::string& message, + Tracing::Span& span) override; + +private: + void toAuthzResponseHeader( + ResponsePtr& response, + const Protobuf::RepeatedPtrField& headers); + const Protobuf::MethodDescriptor& service_method_; + Grpc::AsyncClientPtr async_client_; + Grpc::AsyncRequest* request_{}; + absl::optional timeout_; + RequestCallbacks* callbacks_{}; +}; + +} // namespace ExtAuthz +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy 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 new file mode 100644 index 0000000000000..705b368a6605e --- /dev/null +++ b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc @@ -0,0 +1,117 @@ +#include "extensions/filters/common/ext_authz/ext_authz_http_impl.h" + +#include "common/common/enum_to_int.h" +#include "common/http/async_client_impl.h" + +#include "absl/strings/str_cat.h" + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace ExtAuthz { + +namespace { + +const Http::HeaderMap* getZeroContentLengthHeader() { + static const Http::HeaderMap* header_map = + new Http::HeaderMapImpl{{Http::Headers::get().ContentLength, std::to_string(0)}}; + return header_map; +} +} // namespace + +RawHttpClientImpl::RawHttpClientImpl( + const std::string& cluster_name, Upstream::ClusterManager& cluster_manager, + const absl::optional& timeout, const std::string& path_prefix, + const std::vector& response_headers_to_remove) + : cluster_name_(cluster_name), path_prefix_(path_prefix), + response_headers_to_remove_(response_headers_to_remove), timeout_(timeout), + cm_(cluster_manager) {} + +RawHttpClientImpl::~RawHttpClientImpl() { ASSERT(!callbacks_); } + +void RawHttpClientImpl::cancel() { + ASSERT(callbacks_ != nullptr); + request_->cancel(); + callbacks_ = nullptr; +} + +void RawHttpClientImpl::check(RequestCallbacks& callbacks, + const envoy::service::auth::v2alpha::CheckRequest& request, + Tracing::Span&) { + ASSERT(callbacks_ == nullptr); + callbacks_ = &callbacks; + + Http::HeaderMapPtr headers = std::make_unique(*getZeroContentLengthHeader()); + for (const auto& header : request.attributes().request().http().headers()) { + + const Http::LowerCaseString key{header.first}; + if (key == Http::Headers::get().Path && !path_prefix_.empty()) { + std::string value; + absl::StrAppend(&value, path_prefix_, header.second); + headers->addCopy(key, value); + } else { + headers->addCopy(key, header.second); + } + } + + request_ = cm_.httpAsyncClientForCluster(cluster_name_) + .send(std::make_unique(std::move(headers)), *this, + timeout_); +} + +void RawHttpClientImpl::onSuccess(Http::MessagePtr&& response) { + ResponsePtr authz_response = std::make_unique(Response{}); + + uint64_t status_code; + if (StringUtil::atoul(response->headers().Status()->value().c_str(), status_code)) { + if (status_code == enumToInt(Http::Code::OK)) { + // Header that should not be sent to the upstream. + response->headers().removeStatus(); + response->headers().removeMethod(); + response->headers().removePath(); + response->headers().removeContentLength(); + + // Optional/Configurable headers the should not be sent to the upstream. + for (const auto& header_to_remove : response_headers_to_remove_) { + response->headers().remove(header_to_remove); + } + + authz_response->status = CheckStatus::OK; + authz_response->status_code = Http::Code::OK; + } else { + authz_response->status = CheckStatus::Denied; + authz_response->body = response->bodyAsString(); + authz_response->status_code = static_cast(status_code); + } + } else { + ENVOY_LOG(warn, "Authz_Ext failed to parse the HTTP response code."); + authz_response->status_code = Http::Code::Forbidden; + authz_response->status = CheckStatus::Denied; + } + + response->headers().iterate( + [](const Http::HeaderEntry& header, void* context) -> Http::HeaderMap::Iterate { + static_cast(context)->emplace_back( + Http::LowerCaseString{header.key().c_str()}, std::string{header.value().c_str()}); + return Http::HeaderMap::Iterate::Continue; + }, + &authz_response->headers_to_add); + + callbacks_->onComplete(std::move(authz_response)); + callbacks_ = nullptr; +} + +void RawHttpClientImpl::onFailure(Http::AsyncClient::FailureReason reason) { + ASSERT(reason == Http::AsyncClient::FailureReason::Reset); + Response authz_response{}; + authz_response.status = CheckStatus::Error; + callbacks_->onComplete(std::make_unique(authz_response)); + callbacks_ = nullptr; +} + +} // namespace ExtAuthz +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy 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 new file mode 100644 index 0000000000000..fc5f26fb4c57e --- /dev/null +++ b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.h @@ -0,0 +1,56 @@ +#pragma once + +#include "envoy/upstream/cluster_manager.h" + +#include "common/common/logger.h" + +#include "extensions/filters/common/ext_authz/ext_authz.h" + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace ExtAuthz { + +/** + * This client implementation is used when the Ext_Authz filter needs to communicate with an + * HTTP authorization server. Unlike the gRPC client that allows the server to define the + * response object, in the HTTP client, all headers and body provided in the response are + * dispatched to the downstream, and some headers to the upstream. The HTTP client also allows + * setting a path prefix witch is not available for gRPC. + */ +class RawHttpClientImpl : public Client, + public Http::AsyncClient::Callbacks, + Logger::Loggable { +public: + explicit RawHttpClientImpl(const std::string& cluster_name, + Upstream::ClusterManager& cluster_manager, + const absl::optional& timeout, + const std::string& path_prefix, + const std::vector& response_headers_to_remove); + ~RawHttpClientImpl(); + + // ExtAuthz::Client + void cancel() override; + void check(RequestCallbacks& callbacks, + const envoy::service::auth::v2alpha::CheckRequest& request, Tracing::Span&) override; + + // Http::AsyncClient::Callbacks + void onSuccess(Http::MessagePtr&& response) override; + void onFailure(Http::AsyncClient::FailureReason reason) override; + +private: + const std::string cluster_name_; + const std::string path_prefix_; + const std::vector response_headers_to_remove_; + absl::optional timeout_; + Upstream::ClusterManager& cm_; + Http::AsyncClient::Request* request_{}; + RequestCallbacks* callbacks_{}; +}; + +} // namespace ExtAuthz +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/ext_authz/BUILD b/source/extensions/filters/http/ext_authz/BUILD index d9d352e16e050..651c2913e4403 100644 --- a/source/extensions/filters/http/ext_authz/BUILD +++ b/source/extensions/filters/http/ext_authz/BUILD @@ -16,12 +16,14 @@ envoy_cc_library( hdrs = ["ext_authz.h"], deps = [ "//include/envoy/http:codes_interface", + "//source/common/buffer:buffer_lib", "//source/common/common:assert_lib", "//source/common/common:empty_string", "//source/common/common:enum_to_int", + "//source/common/common:minimal_logger_lib", "//source/common/http:codes_lib", "//source/common/router:config_lib", - "//source/extensions/filters/common/ext_authz:ext_authz_lib", + "//source/extensions/filters/common/ext_authz:ext_authz_grpc_lib", "@envoy_api//envoy/config/filter/http/ext_authz/v2alpha:ext_authz_cc", ], ) @@ -34,6 +36,7 @@ envoy_cc_library( ":ext_authz", "//include/envoy/registry", "//source/common/protobuf:utility_lib", + "//source/extensions/filters/common/ext_authz:ext_authz_http_lib", "//source/extensions/filters/http:well_known_names", "//source/extensions/filters/http/common:factory_base_lib", ], diff --git a/source/extensions/filters/http/ext_authz/config.cc b/source/extensions/filters/http/ext_authz/config.cc index a7585c3e22513..353a67fc872ff 100644 --- a/source/extensions/filters/http/ext_authz/config.cc +++ b/source/extensions/filters/http/ext_authz/config.cc @@ -8,7 +8,8 @@ #include "common/protobuf/utility.h" -#include "extensions/filters/common/ext_authz/ext_authz_impl.h" +#include "extensions/filters/common/ext_authz/ext_authz_grpc_impl.h" +#include "extensions/filters/common/ext_authz/ext_authz_http_impl.h" #include "extensions/filters/http/ext_authz/ext_authz.h" namespace Envoy { @@ -19,14 +20,32 @@ namespace ExtAuthz { Http::FilterFactoryCb ExtAuthzFilterConfig::createFilterFactoryFromProtoTyped( const envoy::config::filter::http::ext_authz::v2alpha::ExtAuthz& proto_config, const std::string&, Server::Configuration::FactoryContext& context) { - auto filter_config = + + const auto filter_config = std::make_shared(proto_config, context.localInfo(), context.scope(), context.runtime(), context.clusterManager()); - const uint32_t timeout_ms = PROTOBUF_GET_MS_OR_DEFAULT(proto_config.grpc_service(), timeout, 200); + + if (proto_config.has_http_service()) { + const uint32_t timeout_ms = PROTOBUF_GET_MS_OR_DEFAULT(proto_config.http_service().server_uri(), + timeout, DefaultTimeout); + return [ + filter_config, timeout_ms, cluster_name = proto_config.http_service().server_uri().cluster(), + path_prefix = proto_config.http_service().path_prefix() + ](Http::FilterChainFactoryCallbacks & callbacks) { + auto client = std::make_unique( + cluster_name, filter_config->cm(), std::chrono::milliseconds(timeout_ms), path_prefix, + filter_config->responseHeadersToRemove()); + callbacks.addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr{ + std::make_shared(filter_config, std::move(client))}); + }; + } + + const uint32_t timeout_ms = + PROTOBUF_GET_MS_OR_DEFAULT(proto_config.grpc_service(), timeout, DefaultTimeout); return [ grpc_service = proto_config.grpc_service(), &context, filter_config, timeout_ms ](Http::FilterChainFactoryCallbacks & callbacks) { - auto async_client_factory = + const auto async_client_factory = context.clusterManager().grpcAsyncClientManager().factoryForGrpcService( grpc_service, context.scope(), true); auto client = std::make_unique( @@ -34,7 +53,7 @@ Http::FilterFactoryCb ExtAuthzFilterConfig::createFilterFactoryFromProtoTyped( callbacks.addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr{ std::make_shared(filter_config, std::move(client))}); }; -} +}; /** * Static registration for the external authorization filter. @see RegisterFactory. diff --git a/source/extensions/filters/http/ext_authz/config.h b/source/extensions/filters/http/ext_authz/config.h index aa8b36d9c169c..e9ba463b40440 100644 --- a/source/extensions/filters/http/ext_authz/config.h +++ b/source/extensions/filters/http/ext_authz/config.h @@ -19,6 +19,7 @@ class ExtAuthzFilterConfig ExtAuthzFilterConfig() : FactoryBase(HttpFilterNames::get().EXT_AUTHORIZATION) {} private: + static constexpr uint64_t DefaultTimeout = 200; Http::FilterFactoryCb createFilterFactoryFromProtoTyped( const envoy::config::filter::http::ext_authz::v2alpha::ExtAuthz& proto_config, const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override; diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index d67faa1e5d25d..20d2ebbd76e58 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -1,19 +1,10 @@ #include "extensions/filters/http/ext_authz/ext_authz.h" -#include -#include - -#include "envoy/http/codes.h" - #include "common/common/assert.h" #include "common/common/enum_to_int.h" #include "common/http/codes.h" #include "common/router/config_impl.h" -#include "extensions/filters/common/ext_authz/ext_authz_impl.h" - -#include "fmt/format.h" - namespace Envoy { namespace Extensions { namespace HttpFilters { @@ -39,11 +30,13 @@ void Filter::initiateCall(const Http::HeaderMap& headers) { // Don't let the filter chain continue as we are going to invoke check call. filter_return_ = FilterReturn::StopDecoding; initiating_call_ = true; + ENVOY_STREAM_LOG(trace, "Ext_authz calling authorization server", *callbacks_); client_->check(*this, check_request_, callbacks_->activeSpan()); initiating_call_ = false; } Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool) { + request_headers_ = &headers; initiateCall(headers); return filter_return_ == FilterReturn::StopDecoding ? Http::FilterHeadersStatus::StopIteration : Http::FilterHeadersStatus::Continue; @@ -71,14 +64,13 @@ void Filter::onDestroy() { } } -void Filter::onComplete(Filters::Common::ExtAuthz::CheckStatus status) { +void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { ASSERT(cluster_); - state_ = State::Complete; using Filters::Common::ExtAuthz::CheckStatus; - switch (status) { + switch (response->status) { case CheckStatus::OK: cluster_->statsScope().counter("ext_authz.ok").inc(); break; @@ -91,7 +83,7 @@ void Filter::onComplete(Filters::Common::ExtAuthz::CheckStatus status) { Http::CodeUtility::ResponseStatInfo info{config_->scope(), cluster_->statsScope(), EMPTY_STRING, - enumToInt(Http::Code::Forbidden), + enumToInt(response->status_code), true, EMPTY_STRING, EMPTY_STRING, @@ -102,20 +94,52 @@ void Filter::onComplete(Filters::Common::ExtAuthz::CheckStatus status) { break; } + ENVOY_STREAM_LOG(trace, "Ext_authz received status code {}", *callbacks_, + enumToInt(response->status_code)); + // We fail open/fail close based of filter config // if there is an error contacting the service. - if (status == CheckStatus::Denied || - (status == CheckStatus::Error && !config_->failureModeAllow())) { - callbacks_->sendLocalReply(Http::Code::Forbidden, "", nullptr); + if (response->status == CheckStatus::Denied || + (response->status == CheckStatus::Error && !config_->failureModeAllow())) { + ENVOY_STREAM_LOG(debug, "Ext_authz rejected the request", *callbacks_); + callbacks_->sendLocalReply( + response->status_code, response->body, + [& authz_headers = response->headers_to_add, + &callbacks = *callbacks_ ](Http::HeaderMap & response_headers) + ->void { + for (const auto& header : authz_headers) { + ENVOY_STREAM_LOG(trace, "Ext_authz rejected response header '{}':'{}'", callbacks, + header.first.get(), header.second); + response_headers.addReferenceKey(header.first, header.second); + } + }); callbacks_->requestInfo().setResponseFlag( RequestInfo::ResponseFlag::UnauthorizedExternalService); } else { + ENVOY_STREAM_LOG(debug, "Ext_authz accepted the request", *callbacks_); // Let the filter chain continue. filter_return_ = FilterReturn::ContinueDecoding; - if (config_->failureModeAllow() && status == CheckStatus::Error) { + if (config_->failureModeAllow() && response->status == CheckStatus::Error) { // Status is Error and yet we are allowing the request. Click a counter. cluster_->statsScope().counter("ext_authz.failure_mode_allowed").inc(); } + // Only send headers if the response is ok. + if (response->status == CheckStatus::OK) { + for (const auto& header : response->headers_to_add) { + request_headers_->setReferenceKey(header.first, header.second); + ENVOY_STREAM_LOG(trace, "Ext_authz ok response added header '{}':'{}'", *callbacks_, + header.first.get(), header.second); + } + for (const auto& header : response->headers_to_append) { + Http::HeaderEntry* header_to_modify = request_headers_->get(header.first); + if (header_to_modify) { + Http::HeaderMapImpl::appendToHeader(header_to_modify->value(), header.second); + ENVOY_STREAM_LOG(trace, "Ext_authz ok response appended header '{}':'{}'", *callbacks_, + header.first.get(), header.second); + } + } + } + if (!initiating_call_) { // We got completion async. Let the filter chain continue. 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 8220bf4a327c7..81dd788a155bf 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.h +++ b/source/extensions/filters/http/ext_authz/ext_authz.h @@ -12,10 +12,11 @@ #include "envoy/upstream/cluster_manager.h" #include "common/common/assert.h" +#include "common/common/logger.h" #include "common/http/header_map_impl.h" #include "extensions/filters/common/ext_authz/ext_authz.h" -#include "extensions/filters/common/ext_authz/ext_authz_impl.h" +#include "extensions/filters/common/ext_authz/ext_authz_grpc_impl.h" namespace Envoy { namespace Extensions { @@ -37,6 +38,8 @@ class FilterConfig { Runtime::Loader& runtime, Upstream::ClusterManager& cm) : local_info_(local_info), scope_(scope), runtime_(runtime), cm_(cm), cluster_name_(config.grpc_service().envoy_grpc().cluster_name()), + response_headers_to_remove_(config.http_service().response_headers_to_remove().begin(), + config.http_service().response_headers_to_remove().end()), failure_mode_allow_(config.failure_mode_allow()) {} const LocalInfo::LocalInfo& localInfo() const { return local_info_; } @@ -44,6 +47,9 @@ class FilterConfig { Stats::Scope& scope() { return scope_; } std::string cluster() { return cluster_name_; } Upstream::ClusterManager& cm() { return cm_; } + const std::vector& responseHeadersToRemove() { + return response_headers_to_remove_; + } bool failureModeAllow() const { return failure_mode_allow_; } private: @@ -52,6 +58,7 @@ class FilterConfig { Runtime::Loader& runtime_; Upstream::ClusterManager& cm_; std::string cluster_name_; + std::vector response_headers_to_remove_; bool failure_mode_allow_; }; @@ -61,7 +68,8 @@ typedef std::shared_ptr FilterConfigSharedPtr; * HTTP ext_authz filter. Depending on the route configuration, this filter calls the global * ext_authz service before allowing further filter iteration. */ -class Filter : public Http::StreamDecoderFilter, +class Filter : public Logger::Loggable, + public Http::StreamDecoderFilter, public Filters::Common::ExtAuthz::RequestCallbacks { public: Filter(FilterConfigSharedPtr config, Filters::Common::ExtAuthz::ClientPtr&& client) @@ -77,9 +85,10 @@ class Filter : public Http::StreamDecoderFilter, void setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) override; // ExtAuthz::RequestCallbacks - void onComplete(Filters::Common::ExtAuthz::CheckStatus status) override; + void onComplete(Filters::Common::ExtAuthz::ResponsePtr&&) override; private: + void addResponseHeaders(Http::HeaderMap& header_map, const Http::HeaderVector& headers); // State of this filter's communication with the external authorization service. // The filter has either not started calling the external service, in the middle of calling // it or has completed. @@ -89,10 +98,11 @@ class Filter : public Http::StreamDecoderFilter, // the filter chain should stop. Otherwise the filter chain can continue to the next filter. enum class FilterReturn { ContinueDecoding, StopDecoding }; void initiateCall(const Http::HeaderMap& headers); - + Http::HeaderMapPtr getHeaderMap(const Filters::Common::ExtAuthz::ResponsePtr& reponse); FilterConfigSharedPtr config_; Filters::Common::ExtAuthz::ClientPtr client_; Http::StreamDecoderFilterCallbacks* callbacks_{}; + Http::HeaderMap* request_headers_; State state_{State::NotStarted}; FilterReturn filter_return_{FilterReturn::ContinueDecoding}; Upstream::ClusterInfoConstSharedPtr cluster_; diff --git a/source/extensions/filters/network/ext_authz/BUILD b/source/extensions/filters/network/ext_authz/BUILD index ef0737886dc2d..530109558523c 100644 --- a/source/extensions/filters/network/ext_authz/BUILD +++ b/source/extensions/filters/network/ext_authz/BUILD @@ -22,8 +22,8 @@ envoy_cc_library( "//include/envoy/upstream:cluster_manager_interface", "//source/common/common:assert_lib", "//source/common/tracing:http_tracer_lib", + "//source/extensions/filters/common/ext_authz:ext_authz_grpc_lib", "//source/extensions/filters/common/ext_authz:ext_authz_interface", - "//source/extensions/filters/common/ext_authz:ext_authz_lib", "@envoy_api//envoy/config/filter/network/ext_authz/v2:ext_authz_cc", ], ) diff --git a/source/extensions/filters/network/ext_authz/config.cc b/source/extensions/filters/network/ext_authz/config.cc index cffece5bb2b4b..54e5de374a3d8 100644 --- a/source/extensions/filters/network/ext_authz/config.cc +++ b/source/extensions/filters/network/ext_authz/config.cc @@ -10,7 +10,7 @@ #include "common/protobuf/utility.h" #include "extensions/filters/common/ext_authz/ext_authz.h" -#include "extensions/filters/common/ext_authz/ext_authz_impl.h" +#include "extensions/filters/common/ext_authz/ext_authz_grpc_impl.h" #include "extensions/filters/network/ext_authz/ext_authz.h" namespace Envoy { diff --git a/source/extensions/filters/network/ext_authz/ext_authz.cc b/source/extensions/filters/network/ext_authz/ext_authz.cc index d0d2ca335bfe0..73412ee16eb42 100644 --- a/source/extensions/filters/network/ext_authz/ext_authz.cc +++ b/source/extensions/filters/network/ext_authz/ext_authz.cc @@ -56,11 +56,11 @@ void Filter::onEvent(Network::ConnectionEvent event) { } } -void Filter::onComplete(Filters::Common::ExtAuthz::CheckStatus status) { +void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { status_ = Status::Complete; config_->stats().active_.dec(); - switch (status) { + switch (response->status) { case Filters::Common::ExtAuthz::CheckStatus::OK: config_->stats().ok_.inc(); break; @@ -73,14 +73,16 @@ void Filter::onComplete(Filters::Common::ExtAuthz::CheckStatus status) { } // Fail open only if configured to do so and if the check status was a error. - if (status == Filters::Common::ExtAuthz::CheckStatus::Denied || - (status == Filters::Common::ExtAuthz::CheckStatus::Error && !config_->failureModeAllow())) { + if (response->status == Filters::Common::ExtAuthz::CheckStatus::Denied || + (response->status == Filters::Common::ExtAuthz::CheckStatus::Error && + !config_->failureModeAllow())) { config_->stats().cx_closed_.inc(); filter_callbacks_->connection().close(Network::ConnectionCloseType::NoFlush); } else { // Let the filter chain continue. filter_return_ = FilterReturn::Continue; - if (config_->failureModeAllow() && status == Filters::Common::ExtAuthz::CheckStatus::Error) { + if (config_->failureModeAllow() && + response->status == Filters::Common::ExtAuthz::CheckStatus::Error) { // Status is Error and yet we are configured to allow traffic. Click a counter. config_->stats().failure_mode_allowed_.inc(); } diff --git a/source/extensions/filters/network/ext_authz/ext_authz.h b/source/extensions/filters/network/ext_authz/ext_authz.h index ef97b822c2b36..4fecd72b052e0 100644 --- a/source/extensions/filters/network/ext_authz/ext_authz.h +++ b/source/extensions/filters/network/ext_authz/ext_authz.h @@ -13,7 +13,7 @@ #include "envoy/upstream/cluster_manager.h" #include "extensions/filters/common/ext_authz/ext_authz.h" -#include "extensions/filters/common/ext_authz/ext_authz_impl.h" +#include "extensions/filters/common/ext_authz/ext_authz_grpc_impl.h" namespace Envoy { namespace Extensions { @@ -90,7 +90,7 @@ class Filter : public Network::ReadFilter, void onBelowWriteBufferLowWatermark() override {} // ExtAuthz::RequestCallbacks - void onComplete(Filters::Common::ExtAuthz::CheckStatus status) override; + void onComplete(Filters::Common::ExtAuthz::ResponsePtr&&) override; private: // State of this filter's communication with the external authorization service. diff --git a/test/extensions/filters/common/ext_authz/BUILD b/test/extensions/filters/common/ext_authz/BUILD index 11da871031dd7..27b273c5ee12d 100644 --- a/test/extensions/filters/common/ext_authz/BUILD +++ b/test/extensions/filters/common/ext_authz/BUILD @@ -10,15 +10,12 @@ load( envoy_package() envoy_cc_test( - name = "ext_authz_impl_test", - srcs = ["ext_authz_impl_test.cc"], + name = "check_request_utils_test", + srcs = ["check_request_utils_test.cc"], deps = [ - "//source/common/http:header_map_lib", - "//source/common/http:headers_lib", "//source/common/network:address_lib", - "//source/extensions/filters/common/ext_authz:ext_authz_lib", - "//test/mocks/grpc:grpc_mocks", - "//test/mocks/http:http_mocks", + "//source/common/protobuf", + "//source/extensions/filters/common/ext_authz:check_request_utils_lib", "//test/mocks/network:network_mocks", "//test/mocks/request_info:request_info_mocks", "//test/mocks/ssl:ssl_mocks", @@ -27,6 +24,24 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "ext_authz_grpc_impl_test", + srcs = ["ext_authz_grpc_impl_test.cc"], + deps = [ + "//source/extensions/filters/common/ext_authz:ext_authz_grpc_lib", + "//test/extensions/filters/common/ext_authz:ext_authz_test_common", + ], +) + +envoy_cc_test( + name = "ext_authz_http_impl_test", + srcs = ["ext_authz_http_impl_test.cc"], + deps = [ + "//source/extensions/filters/common/ext_authz:ext_authz_http_lib", + "//test/extensions/filters/common/ext_authz:ext_authz_test_common", + ], +) + envoy_cc_mock( name = "ext_authz_mocks", srcs = ["mocks.cc"], @@ -35,3 +50,18 @@ envoy_cc_mock( "//source/extensions/filters/common/ext_authz:ext_authz_interface", ], ) + +envoy_cc_mock( + name = "ext_authz_test_common", + srcs = ["test_common.cc"], + hdrs = ["test_common.h"], + deps = [ + "//source/common/http:headers_lib", + "//source/common/protobuf", + "//source/extensions/filters/common/ext_authz:ext_authz_grpc_lib", + "//test/extensions/filters/common/ext_authz:ext_authz_mocks", + "//test/mocks/grpc:grpc_mocks", + "//test/mocks/upstream:upstream_mocks", + "@envoy_api//envoy/api/v2/core:base_cc", + ], +) diff --git a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc new file mode 100644 index 0000000000000..d760e61188cef --- /dev/null +++ b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc @@ -0,0 +1,94 @@ +#include "common/network/address_impl.h" +#include "common/protobuf/protobuf.h" + +#include "extensions/filters/common/ext_authz/check_request_utils.h" + +#include "test/mocks/network/mocks.h" +#include "test/mocks/request_info/mocks.h" +#include "test/mocks/ssl/mocks.h" +#include "test/mocks/upstream/mocks.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using testing::Return; +using testing::ReturnPointee; +using testing::ReturnRef; + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace ExtAuthz { + +class CheckRequestUtilsTest : public testing::Test { +public: + CheckRequestUtilsTest() { + addr_ = std::make_shared("1.2.3.4", 1111); + protocol_ = Envoy::Http::Protocol::Http10; + }; + + Network::Address::InstanceConstSharedPtr addr_; + absl::optional protocol_; + CheckRequestUtils check_request_generator_; + NiceMock callbacks_; + NiceMock net_callbacks_; + NiceMock connection_; + NiceMock ssl_; + NiceMock req_info_; +}; + +// Verify that createTcpCheck's dependencies are invoked when it's called. +TEST_F(CheckRequestUtilsTest, BasicTcp) { + envoy::service::auth::v2alpha::CheckRequest request; + EXPECT_CALL(net_callbacks_, connection()).Times(2).WillRepeatedly(ReturnRef(connection_)); + EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(Const(connection_), ssl()).Times(2).WillRepeatedly(Return(&ssl_)); + + CheckRequestUtils::createTcpCheck(&net_callbacks_, request); +} + +// Verify that createHttpCheck's dependencies are invoked when it's called. +TEST_F(CheckRequestUtilsTest, BasicHttp) { + Http::HeaderMapImpl headers; + envoy::service::auth::v2alpha::CheckRequest request; + EXPECT_CALL(callbacks_, connection()).Times(2).WillRepeatedly(Return(&connection_)); + EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(Const(connection_), ssl()).Times(2).WillRepeatedly(Return(&ssl_)); + EXPECT_CALL(callbacks_, streamId()).WillOnce(Return(0)); + EXPECT_CALL(callbacks_, requestInfo()).Times(3).WillRepeatedly(ReturnRef(req_info_)); + EXPECT_CALL(req_info_, protocol()).Times(2).WillRepeatedly(ReturnPointee(&protocol_)); + + CheckRequestUtils::createHttpCheck(&callbacks_, headers, request); +} + +// Verify that createHttpCheck extract the proper attributes from the http request into CheckRequest +// proto object. +TEST_F(CheckRequestUtilsTest, CheckAttrContextPeer) { + Http::TestHeaderMapImpl request_headers{{"x-envoy-downstream-service-cluster", "foo"}, + {":path", "/bar"}}; + envoy::service::auth::v2alpha::CheckRequest request; + EXPECT_CALL(callbacks_, connection()).WillRepeatedly(Return(&connection_)); + EXPECT_CALL(connection_, remoteAddress()).WillRepeatedly(ReturnRef(addr_)); + EXPECT_CALL(connection_, localAddress()).WillRepeatedly(ReturnRef(addr_)); + EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_)); + EXPECT_CALL(callbacks_, streamId()).WillRepeatedly(Return(0)); + EXPECT_CALL(callbacks_, requestInfo()).WillRepeatedly(ReturnRef(req_info_)); + EXPECT_CALL(req_info_, protocol()).WillRepeatedly(ReturnPointee(&protocol_)); + EXPECT_CALL(ssl_, uriSanPeerCertificate()).WillOnce(Return("source")); + EXPECT_CALL(ssl_, uriSanLocalCertificate()).WillOnce(Return("destination")); + + CheckRequestUtils::createHttpCheck(&callbacks_, request_headers, request); + + EXPECT_EQ("source", request.attributes().source().principal()); + EXPECT_EQ("destination", request.attributes().destination().principal()); + EXPECT_EQ("foo", request.attributes().source().service()); +} + +} // namespace ExtAuthz +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy 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 new file mode 100644 index 0000000000000..6b6c5f8257351 --- /dev/null +++ b/test/extensions/filters/common/ext_authz/ext_authz_grpc_impl_test.cc @@ -0,0 +1,187 @@ +#include "envoy/api/v2/core/base.pb.h" + +#include "common/http/headers.h" +#include "common/protobuf/protobuf.h" + +#include "extensions/filters/common/ext_authz/ext_authz_grpc_impl.h" + +#include "test/extensions/filters/common/ext_authz/mocks.h" +#include "test/extensions/filters/common/ext_authz/test_common.h" +#include "test/mocks/grpc/mocks.h" +#include "test/mocks/upstream/mocks.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using testing::Invoke; +using testing::Ref; +using testing::Return; +using testing::ReturnPointee; +using testing::ReturnRef; +using testing::WhenDynamicCastTo; +using testing::WithArg; +using testing::_; + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace ExtAuthz { + +class ExtAuthzGrpcClientTest : public testing::Test { +public: + ExtAuthzGrpcClientTest() + : async_client_(new Grpc::MockAsyncClient()), timeout_(10), + client_(Grpc::AsyncClientPtr{async_client_}, timeout_) {} + + Grpc::MockAsyncClient* async_client_; + absl::optional timeout_; + Grpc::MockAsyncRequest async_request_; + GrpcClientImpl client_; + MockRequestCallbacks request_callbacks_; + Tracing::MockSpan span_; + + void expectCallSend(envoy::service::auth::v2alpha::CheckRequest& request) { + EXPECT_CALL(*async_client_, send(_, ProtoEq(request), Ref(client_), _, _)) + .WillOnce(Invoke( + [this]( + const Protobuf::MethodDescriptor& service_method, const Protobuf::Message&, + Grpc::AsyncRequestCallbacks&, Tracing::Span&, + const absl::optional& timeout) -> Grpc::AsyncRequest* { + // TODO(dio): Use a defined constant value. + EXPECT_EQ("envoy.service.auth.v2alpha.Authorization", + service_method.service()->full_name()); + EXPECT_EQ("Check", service_method.name()); + EXPECT_EQ(timeout_->count(), timeout->count()); + return &async_request_; + })); + } +}; + +// Test the client when an ok response is received. +TEST_F(ExtAuthzGrpcClientTest, AuthorizationOk) { + auto check_response = std::make_unique(); + auto status = check_response->mutable_status(); + status->set_code(Grpc::Status::GrpcStatus::Ok); + auto authz_response = Response{}; + authz_response.status = CheckStatus::OK; + + envoy::service::auth::v2alpha::CheckRequest request; + expectCallSend(request); + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + + Http::HeaderMapImpl headers; + client_.onCreateInitialMetadata(headers); + + EXPECT_CALL(span_, setTag("ext_authz_status", "ext_authz_ok")); + EXPECT_CALL(request_callbacks_, onComplete_(WhenDynamicCastTo( + AuthzResponseNoAttributes(authz_response)))); + client_.onSuccess(std::move(check_response), span_); +} + +// Test the client when an ok response is received. +TEST_F(ExtAuthzGrpcClientTest, AuthorizationOkWithAllAtributes) { + const std::string empty_body{}; + const auto expected_headers = TestCommon::makeHeaderValueOption({{"foo", "bar", false}}); + auto check_response = TestCommon::makeCheckResponse( + Grpc::Status::GrpcStatus::Ok, envoy::type::StatusCode::OK, empty_body, expected_headers); + auto authz_response = + TestCommon::makeAuthzResponse(CheckStatus::OK, Http::Code::OK, empty_body, expected_headers); + + envoy::service::auth::v2alpha::CheckRequest request; + expectCallSend(request); + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + + Http::HeaderMapImpl headers; + client_.onCreateInitialMetadata(headers); + + EXPECT_CALL(span_, setTag("ext_authz_status", "ext_authz_ok")); + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzOkResponse(authz_response)))); + client_.onSuccess(std::move(check_response), span_); +} + +// Test the client when a denied response is received. +TEST_F(ExtAuthzGrpcClientTest, AuthorizationDenied) { + auto check_response = std::make_unique(); + auto status = check_response->mutable_status(); + status->set_code(Grpc::Status::GrpcStatus::PermissionDenied); + auto authz_response = Response{}; + authz_response.status = CheckStatus::Denied; + + envoy::service::auth::v2alpha::CheckRequest request; + expectCallSend(request); + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + + Http::HeaderMapImpl headers; + client_.onCreateInitialMetadata(headers); + EXPECT_EQ(nullptr, headers.RequestId()); + EXPECT_CALL(span_, setTag("ext_authz_status", "ext_authz_unauthorized")); + EXPECT_CALL(request_callbacks_, onComplete_(WhenDynamicCastTo( + AuthzResponseNoAttributes(authz_response)))); + + client_.onSuccess(std::move(check_response), span_); +} + +// Test the client when a denied response with additional HTTP attributes is received. +TEST_F(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::GrpcStatus::PermissionDenied, + envoy::type::StatusCode::Unauthorized, + expected_body, expected_headers); + auto authz_response = TestCommon::makeAuthzResponse(CheckStatus::Denied, Http::Code::Unauthorized, + expected_body, expected_headers); + + envoy::service::auth::v2alpha::CheckRequest request; + expectCallSend(request); + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + + Http::HeaderMapImpl headers; + client_.onCreateInitialMetadata(headers); + EXPECT_EQ(nullptr, headers.RequestId()); + EXPECT_CALL(span_, setTag("ext_authz_status", "ext_authz_unauthorized")); + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzDeniedResponse(authz_response)))); + + client_.onSuccess(std::move(check_response), span_); +} + +// Test the client when an unknown error occurs. +TEST_F(ExtAuthzGrpcClientTest, UnknownError) { + envoy::service::auth::v2alpha::CheckRequest request; + expectCallSend(request); + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzErrorResponse(CheckStatus::Error)))); + client_.onFailure(Grpc::Status::Unknown, "", span_); +} + +// Test the client when the request is canceled. +TEST_F(ExtAuthzGrpcClientTest, CancelledAuthorizationRequest) { + envoy::service::auth::v2alpha::CheckRequest request; + EXPECT_CALL(*async_client_, send(_, _, _, _, _)).WillOnce(Return(&async_request_)); + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + + EXPECT_CALL(async_request_, cancel()); + client_.cancel(); +} + +// Test the client when the request times out. +TEST_F(ExtAuthzGrpcClientTest, AuthorizationRequestTimeout) { + envoy::service::auth::v2alpha::CheckRequest request; + expectCallSend(request); + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzErrorResponse(CheckStatus::Error)))); + client_.onFailure(Grpc::Status::DeadlineExceeded, "", span_); +} + +} // namespace ExtAuthz +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy 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 new file mode 100644 index 0000000000000..5588c168519b8 --- /dev/null +++ b/test/extensions/filters/common/ext_authz/ext_authz_http_impl_test.cc @@ -0,0 +1,184 @@ +#include "envoy/api/v2/core/base.pb.h" + +#include "common/http/headers.h" +#include "common/http/message_impl.h" +#include "common/protobuf/protobuf.h" +#include "common/tracing/http_tracer_impl.h" + +#include "extensions/filters/common/ext_authz/ext_authz_http_impl.h" + +#include "test/extensions/filters/common/ext_authz/mocks.h" +#include "test/extensions/filters/common/ext_authz/test_common.h" +#include "test/mocks/upstream/mocks.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using testing::Invoke; +using testing::Ref; +using testing::Return; +using testing::ReturnPointee; +using testing::ReturnRef; +using testing::WhenDynamicCastTo; +using testing::WithArg; +using testing::_; + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace ExtAuthz { + +typedef std::vector HeaderValueOptionVector; + +class ExtAuthzHttpClientTest : public testing::Test { +public: + ExtAuthzHttpClientTest() + : cluster_name_{"foo"}, cluster_manager_{}, timeout_{}, path_prefix_{"/bar"}, + response_headers_to_remove_{Http::LowerCaseString{"bar"}}, async_client_{}, + async_request_{&async_client_}, client_(cluster_name_, cluster_manager_, timeout_, + path_prefix_, response_headers_to_remove_) { + ON_CALL(cluster_manager_, httpAsyncClientForCluster(cluster_name_)) + .WillByDefault(ReturnRef(async_client_)); + } + + std::string cluster_name_; + NiceMock cluster_manager_; + MockRequestCallbacks request_callbacks_; + absl::optional timeout_; + std::string path_prefix_; + std::vector response_headers_to_remove_; + NiceMock async_client_; + NiceMock async_request_; + RawHttpClientImpl client_; +}; + +// Test the client when an ok response is received. +TEST_F(ExtAuthzHttpClientTest, AuthorizationOk) { + const auto expected_headers = TestCommon::makeHeaderValueOption({{":status", "200", false}}); + const auto authz_response = TestCommon::makeAuthzResponse(CheckStatus::OK); + auto check_response = TestCommon::makeMessageResponse(expected_headers); + envoy::service::auth::v2alpha::CheckRequest request; + + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzOkResponse(authz_response)))); + + client_.onSuccess(std::move(check_response)); +} + +// Test the client when an request contains path to be re-written and ok response is received. +TEST_F(ExtAuthzHttpClientTest, AuthorizationOkWithPathRewrite) { + const auto expected_headers = TestCommon::makeHeaderValueOption({{":status", "200", false}}); + const auto authz_response = TestCommon::makeAuthzResponse(CheckStatus::OK); + auto check_response = TestCommon::makeMessageResponse(expected_headers); + + envoy::service::auth::v2alpha::CheckRequest request{}; + auto mutable_headers = + request.mutable_attributes()->mutable_request()->mutable_http()->mutable_headers(); + (*mutable_headers)[std::string{":path"}] = std::string{"foo"}; + (*mutable_headers)[std::string{"foo"}] = std::string{"bar"}; + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzOkResponse(authz_response)))); + + client_.onSuccess(std::move(check_response)); +} + +// Test that the client removes certain response headers. +TEST_F(ExtAuthzHttpClientTest, AuthorizationOkWithRemovedHeader) { + const auto expected_headers = TestCommon::makeHeaderValueOption({{"foobar", "foo", false}}); + const std::string empty_body{}; + const auto authz_response = + TestCommon::makeAuthzResponse(CheckStatus::OK, Http::Code::OK, empty_body, expected_headers); + const auto check_response_headers = + TestCommon::makeHeaderValueOption({{":status", "200", false}, + {":path", "/bar", false}, + {":method", "post", false}, + {"content-length", "post", false}, + {"bar", "foo", false}, + {"foobar", "foo", false}}); + auto message_response = TestCommon::makeMessageResponse(check_response_headers); + + envoy::service::auth::v2alpha::CheckRequest request; + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzOkResponse(authz_response)))); + + client_.onSuccess(std::move(message_response)); +} + +// Test the client when a denied response is received due to an unknown status code. +TEST_F(ExtAuthzHttpClientTest, AuthorizationDeniedWithInvalidStatusCode) { + const auto expected_headers = TestCommon::makeHeaderValueOption({{":status", "error", false}}); + const auto authz_response = TestCommon::makeAuthzResponse( + CheckStatus::Denied, Http::Code::Forbidden, "", expected_headers); + Http::MessagePtr check_response(new Http::ResponseMessageImpl( + Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "error"}}})); + envoy::service::auth::v2alpha::CheckRequest request; + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzDeniedResponse(authz_response)))); + + client_.onSuccess(std::move(check_response)); +} + +// Test the client when a denied response is received. +TEST_F(ExtAuthzHttpClientTest, AuthorizationDenied) { + const auto expected_headers = TestCommon::makeHeaderValueOption({{":status", "403", false}}); + const auto authz_response = TestCommon::makeAuthzResponse( + CheckStatus::Denied, Http::Code::Forbidden, "", expected_headers); + auto check_response = TestCommon::makeMessageResponse(expected_headers); + + envoy::service::auth::v2alpha::CheckRequest request; + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzDeniedResponse(authz_response)))); + + client_.onSuccess(std::move(check_response)); +} + +// Test the client when a denied response is received and it contains additional HTTP attributes. +TEST_F(ExtAuthzHttpClientTest, AuthorizationDeniedWithAllAttributes) { + const auto expected_body = std::string{"test"}; + const auto expected_headers = TestCommon::makeHeaderValueOption({{":status", "401", false}}); + const auto authz_response = TestCommon::makeAuthzResponse( + CheckStatus::Denied, Http::Code::Unauthorized, expected_body, expected_headers); + auto check_response = TestCommon::makeMessageResponse(expected_headers, expected_body); + + envoy::service::auth::v2alpha::CheckRequest request; + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzDeniedResponse(authz_response)))); + + client_.onSuccess(std::move(check_response)); +} + +// Test the client when an unknown error occurs. +TEST_F(ExtAuthzHttpClientTest, AuthorizationRequestError) { + envoy::service::auth::v2alpha::CheckRequest request; + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzErrorResponse(CheckStatus::Error)))); + client_.onFailure(Http::AsyncClient::FailureReason::Reset); +} + +// Test the client when the request is canceled. +TEST_F(ExtAuthzHttpClientTest, CancelledAuthorizationRequest) { + envoy::service::auth::v2alpha::CheckRequest request; + EXPECT_CALL(async_client_, send_(_, _, _)).WillOnce(Return(&async_request_)); + client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); + + EXPECT_CALL(async_request_, cancel()); + client_.cancel(); +} + +} // namespace ExtAuthz +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/common/ext_authz/ext_authz_impl_test.cc b/test/extensions/filters/common/ext_authz/ext_authz_impl_test.cc deleted file mode 100644 index f2f2ded6e23c7..0000000000000 --- a/test/extensions/filters/common/ext_authz/ext_authz_impl_test.cc +++ /dev/null @@ -1,197 +0,0 @@ -#include -#include -#include - -#include "common/http/header_map_impl.h" -#include "common/http/headers.h" -#include "common/network/address_impl.h" -#include "common/tracing/http_tracer_impl.h" - -#include "extensions/filters/common/ext_authz/ext_authz_impl.h" - -#include "test/mocks/grpc/mocks.h" -#include "test/mocks/http/mocks.h" -#include "test/mocks/network/mocks.h" -#include "test/mocks/request_info/mocks.h" -#include "test/mocks/ssl/mocks.h" -#include "test/mocks/upstream/mocks.h" -#include "test/test_common/printers.h" -#include "test/test_common/utility.h" - -#include "gmock/gmock.h" -#include "gtest/gtest.h" - -using testing::AtLeast; -using testing::Invoke; -using testing::Ref; -using testing::Return; -using testing::ReturnPointee; -using testing::ReturnRef; -using testing::WithArg; -using testing::_; - -namespace Envoy { -namespace Extensions { -namespace Filters { -namespace Common { -namespace ExtAuthz { - -class MockRequestCallbacks : public RequestCallbacks { -public: - MOCK_METHOD1(onComplete, void(CheckStatus status)); -}; - -class ExtAuthzGrpcClientTest : public testing::Test { -public: - ExtAuthzGrpcClientTest() - : async_client_(new Grpc::MockAsyncClient()), - client_(Grpc::AsyncClientPtr{async_client_}, absl::optional()) {} - - Grpc::MockAsyncClient* async_client_; - Grpc::MockAsyncRequest async_request_; - GrpcClientImpl client_; - MockRequestCallbacks request_callbacks_; - Tracing::MockSpan span_; -}; - -TEST_F(ExtAuthzGrpcClientTest, BasicOK) { - envoy::service::auth::v2alpha::CheckRequest request; - std::unique_ptr response; - Http::HeaderMapImpl headers; - EXPECT_CALL(*async_client_, send(_, ProtoEq(request), _, _, _)).WillOnce(Return(&async_request_)); - - client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); - - client_.onCreateInitialMetadata(headers); - - response = std::make_unique(); - auto status = response->mutable_status(); - status->set_code(Grpc::Status::GrpcStatus::Ok); - EXPECT_CALL(span_, setTag("ext_authz_status", "ext_authz_ok")); - EXPECT_CALL(request_callbacks_, onComplete(CheckStatus::OK)); - client_.onSuccess(std::move(response), span_); -} - -TEST_F(ExtAuthzGrpcClientTest, BasicDenied) { - envoy::service::auth::v2alpha::CheckRequest request; - std::unique_ptr response; - Http::HeaderMapImpl headers; - - EXPECT_CALL(*async_client_, send(_, ProtoEq(request), Ref(client_), _, _)) - .WillOnce( - Invoke([this](const Protobuf::MethodDescriptor& service_method, const Protobuf::Message&, - Grpc::AsyncRequestCallbacks&, Tracing::Span&, - const absl::optional&) -> Grpc::AsyncRequest* { - // TODO(dio): Use a defined constant value. - EXPECT_EQ("envoy.service.auth.v2alpha.Authorization", - service_method.service()->full_name()); - EXPECT_EQ("Check", service_method.name()); - return &async_request_; - })); - - client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); - - client_.onCreateInitialMetadata(headers); - EXPECT_EQ(nullptr, headers.RequestId()); - - response = std::make_unique(); - auto status = response->mutable_status(); - status->set_code(Grpc::Status::GrpcStatus::PermissionDenied); - EXPECT_CALL(span_, setTag("ext_authz_status", "ext_authz_unauthorized")); - EXPECT_CALL(request_callbacks_, onComplete(CheckStatus::Denied)); - client_.onSuccess(std::move(response), span_); -} - -TEST_F(ExtAuthzGrpcClientTest, BasicError) { - envoy::service::auth::v2alpha::CheckRequest request; - EXPECT_CALL(*async_client_, send(_, ProtoEq(request), _, _, _)).WillOnce(Return(&async_request_)); - - client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); - - EXPECT_CALL(request_callbacks_, onComplete(CheckStatus::Error)); - client_.onFailure(Grpc::Status::Unknown, "", span_); -} - -TEST_F(ExtAuthzGrpcClientTest, Cancel) { - envoy::service::auth::v2alpha::CheckRequest request; - - EXPECT_CALL(*async_client_, send(_, _, _, _, _)).WillOnce(Return(&async_request_)); - - client_.check(request_callbacks_, request, Tracing::NullSpan::instance()); - - EXPECT_CALL(async_request_, cancel()); - client_.cancel(); -} - -class CheckRequestUtilsTest : public testing::Test { -public: - CheckRequestUtilsTest() { - addr_ = std::make_shared("1.2.3.4", 1111); - protocol_ = Envoy::Http::Protocol::Http10; - }; - - Network::Address::InstanceConstSharedPtr addr_; - absl::optional protocol_; - CheckRequestUtils check_request_generator_; - NiceMock callbacks_; - NiceMock net_callbacks_; - NiceMock connection_; - NiceMock ssl_; - NiceMock req_info_; -}; - -TEST_F(CheckRequestUtilsTest, BasicTcp) { - - envoy::service::auth::v2alpha::CheckRequest request; - - EXPECT_CALL(net_callbacks_, connection()).Times(2).WillRepeatedly(ReturnRef(connection_)); - EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); - EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); - EXPECT_CALL(Const(connection_), ssl()).Times(2).WillRepeatedly(Return(&ssl_)); - - CheckRequestUtils::createTcpCheck(&net_callbacks_, request); -} - -TEST_F(CheckRequestUtilsTest, BasicHttp) { - - Http::HeaderMapImpl headers; - envoy::service::auth::v2alpha::CheckRequest request; - - EXPECT_CALL(callbacks_, connection()).Times(2).WillRepeatedly(Return(&connection_)); - EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); - EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); - EXPECT_CALL(Const(connection_), ssl()).Times(2).WillRepeatedly(Return(&ssl_)); - EXPECT_CALL(callbacks_, streamId()).WillOnce(Return(0)); - EXPECT_CALL(callbacks_, requestInfo()).Times(3).WillRepeatedly(ReturnRef(req_info_)); - EXPECT_CALL(req_info_, protocol()).Times(2).WillRepeatedly(ReturnPointee(&protocol_)); - CheckRequestUtils::createHttpCheck(&callbacks_, headers, request); -} - -TEST_F(CheckRequestUtilsTest, CheckAttrContextPeer) { - - Http::TestHeaderMapImpl request_headers{{"x-envoy-downstream-service-cluster", "foo"}, - {":path", "/bar"}}; - envoy::service::auth::v2alpha::CheckRequest request; - - EXPECT_CALL(callbacks_, connection()).WillRepeatedly(Return(&connection_)); - EXPECT_CALL(connection_, remoteAddress()).WillRepeatedly(ReturnRef(addr_)); - EXPECT_CALL(connection_, localAddress()).WillRepeatedly(ReturnRef(addr_)); - EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_)); - EXPECT_CALL(callbacks_, streamId()).WillRepeatedly(Return(0)); - EXPECT_CALL(callbacks_, requestInfo()).WillRepeatedly(ReturnRef(req_info_)); - EXPECT_CALL(req_info_, protocol()).WillRepeatedly(ReturnPointee(&protocol_)); - - EXPECT_CALL(ssl_, uriSanPeerCertificate()).WillOnce(Return("source")); - EXPECT_CALL(ssl_, uriSanLocalCertificate()).WillOnce(Return("destination")); - CheckRequestUtils::createHttpCheck(&callbacks_, request_headers, request); - - EXPECT_EQ("source", request.attributes().source().principal()); - EXPECT_EQ("destination", request.attributes().destination().principal()); - EXPECT_EQ("foo", request.attributes().source().service()); -} - -} // namespace ExtAuthz -} // namespace Common -} // namespace Filters -} // namespace Extensions -} // namespace Envoy diff --git a/test/extensions/filters/common/ext_authz/mocks.cc b/test/extensions/filters/common/ext_authz/mocks.cc index 7416e537dcfe7..99bbd23ab0ea0 100644 --- a/test/extensions/filters/common/ext_authz/mocks.cc +++ b/test/extensions/filters/common/ext_authz/mocks.cc @@ -9,6 +9,9 @@ namespace ExtAuthz { MockClient::MockClient() {} MockClient::~MockClient() {} +MockRequestCallbacks::MockRequestCallbacks() {} +MockRequestCallbacks::~MockRequestCallbacks() {} + } // namespace ExtAuthz } // namespace Common } // namespace Filters diff --git a/test/extensions/filters/common/ext_authz/mocks.h b/test/extensions/filters/common/ext_authz/mocks.h index e70e4db8e324b..0ec15d0c86023 100644 --- a/test/extensions/filters/common/ext_authz/mocks.h +++ b/test/extensions/filters/common/ext_authz/mocks.h @@ -25,6 +25,16 @@ class MockClient : public Client { Tracing::Span& parent_span)); }; +class MockRequestCallbacks : public RequestCallbacks { +public: + MockRequestCallbacks(); + ~MockRequestCallbacks(); + + void onComplete(ResponsePtr&& response) override { onComplete_(response); } + + MOCK_METHOD1(onComplete_, void(ResponsePtr& response)); +}; + } // namespace ExtAuthz } // namespace Common } // namespace Filters diff --git a/test/extensions/filters/common/ext_authz/test_common.cc b/test/extensions/filters/common/ext_authz/test_common.cc new file mode 100644 index 0000000000000..cc7cfb8e60999 --- /dev/null +++ b/test/extensions/filters/common/ext_authz/test_common.cc @@ -0,0 +1,99 @@ +#include "test/extensions/filters/common/ext_authz/test_common.h" + +#include "test/mocks/upstream/mocks.h" + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace ExtAuthz { + +CheckResponsePtr TestCommon::makeCheckResponse(Grpc::Status::GrpcStatus response_status, + envoy::type::StatusCode http_status_code, + const std::string& body, + const HeaderValueOptionVector& headers) { + auto response = std::make_unique(); + auto status = response->mutable_status(); + status->set_code(response_status); + + if (response_status != Grpc::Status::GrpcStatus::Ok) { + const auto denied_response = response->mutable_denied_response(); + if (!body.empty()) { + denied_response->set_body(body); + } + + auto status_code = denied_response->mutable_status(); + status_code->set_code(http_status_code); + + auto denied_response_headers = denied_response->mutable_headers(); + if (!headers.empty()) { + for (const auto& header : headers) { + auto* item = denied_response_headers->Add(); + item->CopyFrom(header); + } + } + } else { + if (!headers.empty()) { + const auto ok_response_headers = response->mutable_ok_response()->mutable_headers(); + for (const auto& header : headers) { + auto* item = ok_response_headers->Add(); + item->CopyFrom(header); + } + } + } + return response; +} + +Response TestCommon::makeAuthzResponse(CheckStatus status, Http::Code status_code, + const std::string& body, + const HeaderValueOptionVector& headers) { + auto authz_response = Response{}; + authz_response.status = status; + authz_response.status_code = status_code; + if (!body.empty()) { + authz_response.body = body; + } + if (!headers.empty()) { + for (auto& header : headers) { + if (header.append().value()) { + authz_response.headers_to_append.emplace_back(Http::LowerCaseString(header.header().key()), + header.header().value()); + } else { + authz_response.headers_to_add.emplace_back(Http::LowerCaseString(header.header().key()), + header.header().value()); + } + } + } + return authz_response; +} + +HeaderValueOptionVector TestCommon::makeHeaderValueOption(KeyValueOptionVector&& headers) { + HeaderValueOptionVector header_option_vector{}; + for (auto header : headers) { + envoy::api::v2::core::HeaderValueOption header_value_option; + auto* mutable_header = header_value_option.mutable_header(); + mutable_header->set_key(header.key); + mutable_header->set_value(header.value); + header_value_option.mutable_append()->set_value(header.append); + header_option_vector.push_back(header_value_option); + } + return header_option_vector; +} + +Http::MessagePtr TestCommon::makeMessageResponse(const HeaderValueOptionVector& headers, + const std::string& body) { + Http::MessagePtr response( + new Http::ResponseMessageImpl(Http::HeaderMapPtr{new Http::TestHeaderMapImpl{}})); + for (auto& header : headers) { + response->headers().addCopy(Http::LowerCaseString(header.header().key()), + header.header().value()); + } + response->body().reset(new Buffer::OwnedImpl(body)); + return response; +}; + +} // namespace ExtAuthz +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/common/ext_authz/test_common.h b/test/extensions/filters/common/ext_authz/test_common.h new file mode 100644 index 0000000000000..268f4cb7e202b --- /dev/null +++ b/test/extensions/filters/common/ext_authz/test_common.h @@ -0,0 +1,105 @@ +#pragma once + +#include "envoy/api/v2/core/base.pb.h" + +#include "common/http/headers.h" + +#include "extensions/filters/common/ext_authz/ext_authz_grpc_impl.h" + +#include "test/extensions/filters/common/ext_authz/mocks.h" + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace ExtAuthz { + +MATCHER_P(AuthzErrorResponse, status, "") { return arg->status == status; } + +MATCHER_P(AuthzResponseNoAttributes, response, "") { + if (arg->status != response.status) { + return false; + } + return true; +} + +MATCHER_P(AuthzDeniedResponse, response, "") { + if (arg->status != response.status) { + return false; + } + if (arg->status_code != response.status_code) { + return false; + } + if (arg->body.compare(response.body)) { + return false; + } + // Compare headers_to_add. + if (!arg->headers_to_add.empty() && response.headers_to_add.empty()) { + return false; + } + if (!std::equal(arg->headers_to_add.begin(), arg->headers_to_add.end(), + response.headers_to_add.begin())) { + return false; + } + + return true; +} + +MATCHER_P(AuthzOkResponse, response, "") { + if (arg->status != response.status) { + return false; + } + // Compare headers_to_apppend. + if (!arg->headers_to_append.empty() && response.headers_to_append.empty()) { + return false; + } + if (!std::equal(arg->headers_to_append.begin(), arg->headers_to_append.end(), + response.headers_to_append.begin())) { + return false; + } + // Compare headers_to_add. + if (!arg->headers_to_add.empty() && response.headers_to_add.empty()) { + return false; + } + if (!std::equal(arg->headers_to_add.begin(), arg->headers_to_add.end(), + response.headers_to_add.begin())) { + return false; + } + + return true; +} + +struct KeyValueOption { + std::string key; + std::string value; + bool append; +}; + +typedef std::vector KeyValueOptionVector; +typedef std::vector HeaderValueOptionVector; +typedef std::unique_ptr CheckResponsePtr; + +class TestCommon { +public: + static Http::MessagePtr makeMessageResponse(const HeaderValueOptionVector& headers, + const std::string& body = std::string{}); + + static CheckResponsePtr + makeCheckResponse(Grpc::Status::GrpcStatus response_status = Grpc::Status::GrpcStatus::Ok, + envoy::type::StatusCode http_status_code = envoy::type::StatusCode::OK, + const std::string& body = std::string{}, + const HeaderValueOptionVector& headers = HeaderValueOptionVector{}); + + static Response + makeAuthzResponse(CheckStatus status, Http::Code status_code = Http::Code::OK, + const std::string& body = std::string{}, + const HeaderValueOptionVector& headers = HeaderValueOptionVector{}); + + static HeaderValueOptionVector makeHeaderValueOption(KeyValueOptionVector&& headers); +}; + +} // namespace ExtAuthz +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/http/ext_authz/BUILD b/test/extensions/filters/http/ext_authz/BUILD index 15e1e5e6d8c2b..0692b53be3c3a 100644 --- a/test/extensions/filters/http/ext_authz/BUILD +++ b/test/extensions/filters/http/ext_authz/BUILD @@ -16,6 +16,7 @@ envoy_extension_cc_test( srcs = ["ext_authz_test.cc"], extension_name = "envoy.filters.http.ext_authz", deps = [ + "//include/envoy/http:codes_interface", "//source/common/buffer:buffer_lib", "//source/common/common:empty_string", "//source/common/config:filter_json_lib", @@ -23,7 +24,7 @@ envoy_extension_cc_test( "//source/common/json:json_loader_lib", "//source/common/network:address_lib", "//source/common/protobuf:utility_lib", - "//source/extensions/filters/common/ext_authz:ext_authz_lib", + "//source/extensions/filters/common/ext_authz:ext_authz_grpc_lib", "//source/extensions/filters/http/ext_authz", "//test/extensions/filters/common/ext_authz:ext_authz_mocks", "//test/mocks/http:http_mocks", diff --git a/test/extensions/filters/http/ext_authz/config_test.cc b/test/extensions/filters/http/ext_authz/config_test.cc index 26463bbd945f5..d1b344ad45fe1 100644 --- a/test/extensions/filters/http/ext_authz/config_test.cc +++ b/test/extensions/filters/http/ext_authz/config_test.cc @@ -15,21 +15,24 @@ namespace Extensions { namespace HttpFilters { namespace ExtAuthz { -TEST(HttpExtAuthzConfigTest, ExtAuthzCorrectProto) { +TEST(HttpExtAuthzConfigTest, CorrectProtoGrpc) { std::string yaml = R"EOF( grpc_service: google_grpc: target_uri: ext_authz_server stat_prefix: google failure_mode_allow: false -)EOF"; + )EOF"; ExtAuthzFilterConfig factory; ProtobufTypes::MessagePtr proto_config = factory.createEmptyConfigProto(); MessageUtil::loadFromYaml(yaml, *proto_config); - NiceMock context; - + testing::StrictMock context; + EXPECT_CALL(context, localInfo()).Times(1); + EXPECT_CALL(context, clusterManager()).Times(2); + EXPECT_CALL(context, runtime()).Times(1); + EXPECT_CALL(context, scope()).Times(2); EXPECT_CALL(context.cluster_manager_.async_client_manager_, factoryForGrpcService(_, _, _)) .WillOnce(Invoke([](const envoy::api::v2::core::GrpcService&, Stats::Scope&, bool) { return std::make_unique>(); @@ -40,6 +43,34 @@ TEST(HttpExtAuthzConfigTest, ExtAuthzCorrectProto) { cb(filter_callback); } +TEST(HttpExtAuthzConfigTest, CorrectProtoHttp) { + std::string yaml = R"EOF( + http_service: + server_uri: + uri: "ext_authz:9000" + cluster: "ext_authz" + timeout: 0.25s + path_prefix: "/test" + response_headers_to_remove: + - foo_header_key + - baz_header_key + failure_mode_allow: true + )EOF"; + + ExtAuthzFilterConfig factory; + ProtobufTypes::MessagePtr proto_config = factory.createEmptyConfigProto(); + MessageUtil::loadFromYaml(yaml, *proto_config); + testing::StrictMock context; + EXPECT_CALL(context, localInfo()).Times(1); + EXPECT_CALL(context, clusterManager()).Times(1); + EXPECT_CALL(context, runtime()).Times(1); + EXPECT_CALL(context, scope()).Times(1); + Http::FilterFactoryCb cb = factory.createFilterFactoryFromProto(*proto_config, "stats", context); + testing::StrictMock filter_callback; + EXPECT_CALL(filter_callback, addStreamDecoderFilter(_)); + cb(filter_callback); +} + } // namespace ExtAuthz } // namespace HttpFilters } // namespace Extensions 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 972ae716b25fa..a36b89e1538ca 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc +++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc @@ -2,7 +2,9 @@ #include #include +#include "envoy/config/filter/http/ext_authz/v2alpha/ext_authz.pb.h" #include "envoy/config/filter/http/ext_authz/v2alpha/ext_authz.pb.validate.h" +#include "envoy/http/codes.h" #include "common/buffer/buffer_impl.h" #include "common/common/empty_string.h" @@ -142,6 +144,7 @@ TEST_P(HttpExtAuthzFilterParamTest, OkResponse) { ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_)); EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(*client_, check(_, _, testing::A())) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { @@ -157,7 +160,10 @@ TEST_P(HttpExtAuthzFilterParamTest, OkResponse) { EXPECT_CALL(filter_callbacks_.request_info_, setResponseFlag(Envoy::RequestInfo::ResponseFlag::UnauthorizedExternalService)) .Times(0); - request_callbacks_->onComplete(Filters::Common::ExtAuthz::CheckStatus::OK); + + Filters::Common::ExtAuthz::Response response{}; + response.status = Filters::Common::ExtAuthz::CheckStatus::OK; + request_callbacks_->onComplete(std::make_unique(response)); EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("ext_authz.ok").value()); @@ -171,10 +177,14 @@ TEST_P(HttpExtAuthzFilterParamTest, ImmediateOkResponse) { ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_)); EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + + Filters::Common::ExtAuthz::Response response{}; + response.status = Filters::Common::ExtAuthz::CheckStatus::OK; + EXPECT_CALL(*client_, check(_, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { - callbacks.onComplete(Filters::Common::ExtAuthz::CheckStatus::OK); + callbacks.onComplete(std::make_unique(response)); }))); EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); @@ -186,6 +196,82 @@ TEST_P(HttpExtAuthzFilterParamTest, ImmediateOkResponse) { cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("ext_authz.ok").value()); } +// Test that an synchronous denied response from the authorization service passing additional HTTP +// attributes to the downstream. +TEST_P(HttpExtAuthzFilterParamTest, ImmediateDeniedResponseWithHttpAttributes) { + InSequence s; + + ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_)); + EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + + Filters::Common::ExtAuthz::Response response{}; + response.status = Filters::Common::ExtAuthz::CheckStatus::Denied; + response.status_code = Http::Code::Unauthorized; + response.headers_to_add = Http::HeaderVector{{Http::LowerCaseString{"foo"}, "bar"}}; + response.body = std::string{"baz"}; + + auto response_ptr = std::make_unique(response); + + EXPECT_CALL(*client_, check(_, _, _)) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { + callbacks.onComplete(std::move(response_ptr)); + }))); + + EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_)); + EXPECT_EQ( + 1U, + cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("ext_authz.denied").value()); +} + +// Test that an synchronous ok response from the authorization service passing additional HTTP +// attributes to the upstream. +TEST_P(HttpExtAuthzFilterParamTest, ImmediateOkResponseWithHttpAttributes) { + InSequence s; + + // `bar` will be appended to this header. + const Http::LowerCaseString request_header_key{"baz"}; + request_headers_.addCopy(request_header_key, "foo"); + + // `foo` will be added to this key. + const Http::LowerCaseString key_to_add{"bar"}; + + // `foo` will be override with `bar`. + const Http::LowerCaseString key_to_override{"foobar"}; + request_headers_.addCopy("foobar", "foo"); + + ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_)); + EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + + Filters::Common::ExtAuthz::Response response{}; + response.status = Filters::Common::ExtAuthz::CheckStatus::OK; + response.headers_to_append = Http::HeaderVector{{request_header_key, "bar"}}; + response.headers_to_add = Http::HeaderVector{{key_to_add, "foo"}, {key_to_override, "bar"}}; + + auto response_ptr = std::make_unique(response); + + EXPECT_CALL(*client_, check(_, _, _)) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { + callbacks.onComplete(std::move(response_ptr)); + }))); + + EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_headers_)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_headers_)); + EXPECT_EQ(request_headers_.get_(request_header_key), "foo,bar"); + EXPECT_EQ(request_headers_.get_(key_to_add), "foo"); + EXPECT_EQ(request_headers_.get_(key_to_override), "bar"); +} + // Test that an synchronous denied response from the authorization service, on the call stack, // results in request not continuing. TEST_P(HttpExtAuthzFilterParamTest, ImmediateDeniedResponse) { @@ -194,10 +280,13 @@ TEST_P(HttpExtAuthzFilterParamTest, ImmediateDeniedResponse) { ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_)); EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + + Filters::Common::ExtAuthz::Response response{}; + response.status = Filters::Common::ExtAuthz::CheckStatus::Denied; EXPECT_CALL(*client_, check(_, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { - callbacks.onComplete(Filters::Common::ExtAuthz::CheckStatus::Denied); + callbacks.onComplete(std::make_unique(response)); }))); EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); @@ -211,8 +300,8 @@ TEST_P(HttpExtAuthzFilterParamTest, ImmediateDeniedResponse) { cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("ext_authz.denied").value()); } -// Test that a denied response results in the connection closing with a 403 response to the client. -TEST_P(HttpExtAuthzFilterParamTest, DeniedResponse) { +// Test that a denied response results in the connection closing with a 401 response to the client. +TEST_P(HttpExtAuthzFilterParamTest, DeniedResponseWith401) { InSequence s; ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_)); @@ -226,12 +315,54 @@ TEST_P(HttpExtAuthzFilterParamTest, DeniedResponse) { EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, false)); + + Http::TestHeaderMapImpl response_headers{{":status", "401"}}; + + EXPECT_CALL(filter_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); + + EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); + EXPECT_CALL(filter_callbacks_.request_info_, + setResponseFlag(Envoy::RequestInfo::ResponseFlag::UnauthorizedExternalService)); + + Filters::Common::ExtAuthz::Response response{}; + response.status = Filters::Common::ExtAuthz::CheckStatus::Denied; + response.status_code = Http::Code::Unauthorized; + request_callbacks_->onComplete(std::make_unique(response)); + + EXPECT_EQ( + 1U, + cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("ext_authz.denied").value()); + EXPECT_EQ( + 1U, + cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("upstream_rq_4xx").value()); +} + +// Test that a denied response results in the connection closing with a 403 response to the client. +TEST_P(HttpExtAuthzFilterParamTest, DeniedResponseWith403) { + InSequence s; + + ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_)); + EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + EXPECT_CALL(*client_, check(_, _, _)) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { + request_callbacks_ = &callbacks; + }))); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + Http::TestHeaderMapImpl response_headers{{":status", "403"}}; + EXPECT_CALL(filter_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); EXPECT_CALL(filter_callbacks_.request_info_, setResponseFlag(Envoy::RequestInfo::ResponseFlag::UnauthorizedExternalService)); - request_callbacks_->onComplete(Filters::Common::ExtAuthz::CheckStatus::Denied); + + Filters::Common::ExtAuthz::Response response{}; + response.status = Filters::Common::ExtAuthz::CheckStatus::Denied; + response.status_code = Http::Code::Forbidden; + request_callbacks_->onComplete(std::make_unique(response)); EXPECT_EQ( 1U, @@ -244,8 +375,8 @@ TEST_P(HttpExtAuthzFilterParamTest, DeniedResponse) { cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("upstream_rq_403").value()); } -// Test that when a connection awaiting a authorization response is canceled then the authorization -// call is closed. +// Test that when a connection awaiting a authorization response is canceled then the +// authorization call is closed. TEST_P(HttpExtAuthzFilterParamTest, ResetDuringCall) { InSequence s; @@ -300,11 +431,13 @@ TEST_F(HttpExtAuthzFilterTest, ErrorFailClose) { WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; }))); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, false)); EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); - request_callbacks_->onComplete(Filters::Common::ExtAuthz::CheckStatus::Error); + + Filters::Common::ExtAuthz::Response response{}; + response.status = Filters::Common::ExtAuthz::CheckStatus::Error; + request_callbacks_->onComplete(std::make_unique(response)); EXPECT_EQ( 1U, @@ -325,11 +458,13 @@ TEST_F(HttpExtAuthzFilterTest, ErrorOpen) { WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; }))); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, false)); EXPECT_CALL(filter_callbacks_, continueDecoding()); - request_callbacks_->onComplete(Filters::Common::ExtAuthz::CheckStatus::Error); + + Filters::Common::ExtAuthz::Response response{}; + response.status = Filters::Common::ExtAuthz::CheckStatus::Error; + request_callbacks_->onComplete(std::make_unique(response)); EXPECT_EQ( 1U, @@ -345,10 +480,13 @@ TEST_F(HttpExtAuthzFilterTest, ImmediateErrorOpen) { ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_)); EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); + + Filters::Common::ExtAuthz::Response response{}; + response.status = Filters::Common::ExtAuthz::CheckStatus::Error; EXPECT_CALL(*client_, check(_, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { - callbacks.onComplete(Filters::Common::ExtAuthz::CheckStatus::Error); + callbacks.onComplete(std::make_unique(response)); }))); EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); diff --git a/test/extensions/filters/network/ext_authz/ext_authz_test.cc b/test/extensions/filters/network/ext_authz/ext_authz_test.cc index 310a90983a3fc..9fff77c7ca5cb 100644 --- a/test/extensions/filters/network/ext_authz/ext_authz_test.cc +++ b/test/extensions/filters/network/ext_authz/ext_authz_test.cc @@ -61,6 +61,14 @@ class ExtAuthzFilterTest : public testing::Test { filter_->onBelowWriteBufferLowWatermark(); } + Filters::Common::ExtAuthz::ResponsePtr + makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus status) { + Filters::Common::ExtAuthz::ResponsePtr response = + std::make_unique(); + response->status = status; + return response; + } + ~ExtAuthzFilterTest() { for (const Stats::GaugeSharedPtr& gauge : stats_store_.gauges()) { EXPECT_EQ(0U, gauge->value()); @@ -114,7 +122,7 @@ TEST_F(ExtAuthzFilterTest, OKWithOnData) { EXPECT_EQ(1U, stats_store_.gauge("ext_authz.name.active").value()); EXPECT_CALL(filter_callbacks_, continueReading()); - request_callbacks_->onComplete(Filters::Common::ExtAuthz::CheckStatus::OK); + request_callbacks_->onComplete(makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus::OK)); EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data, false)); @@ -152,7 +160,7 @@ TEST_F(ExtAuthzFilterTest, DeniedWithOnData) { EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::NoFlush)); EXPECT_CALL(*client_, cancel()).Times(0); - request_callbacks_->onComplete(Filters::Common::ExtAuthz::CheckStatus::Denied); + request_callbacks_->onComplete(makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus::Denied)); EXPECT_EQ(Network::FilterStatus::StopIteration, filter_->onData(data, false)); @@ -182,7 +190,7 @@ TEST_F(ExtAuthzFilterTest, FailOpen) { EXPECT_CALL(filter_callbacks_.connection_, close(_)).Times(0); EXPECT_CALL(*client_, cancel()).Times(0); EXPECT_CALL(filter_callbacks_, continueReading()); - request_callbacks_->onComplete(Filters::Common::ExtAuthz::CheckStatus::Error); + request_callbacks_->onComplete(makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus::Error)); EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data, false)); @@ -213,7 +221,7 @@ TEST_F(ExtAuthzFilterTest, FailClose) { EXPECT_CALL(filter_callbacks_.connection_, close(_)).Times(1); EXPECT_CALL(filter_callbacks_, continueReading()).Times(0); - request_callbacks_->onComplete(Filters::Common::ExtAuthz::CheckStatus::Error); + request_callbacks_->onComplete(makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus::Error)); EXPECT_EQ(1U, stats_store_.counter("ext_authz.name.total").value()); EXPECT_EQ(1U, stats_store_.counter("ext_authz.name.error").value()); @@ -241,7 +249,7 @@ TEST_F(ExtAuthzFilterTest, DoNotCallCancelonRemoteClose) { EXPECT_EQ(Network::FilterStatus::StopIteration, filter_->onData(data, false)); EXPECT_CALL(filter_callbacks_, continueReading()); - request_callbacks_->onComplete(Filters::Common::ExtAuthz::CheckStatus::Error); + request_callbacks_->onComplete(makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus::Error)); EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data, false)); @@ -295,7 +303,7 @@ TEST_F(ExtAuthzFilterTest, ImmediateOK) { EXPECT_CALL(*client_, check(_, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { - callbacks.onComplete(Filters::Common::ExtAuthz::CheckStatus::OK); + callbacks.onComplete(makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus::OK)); }))); EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection()); @@ -325,7 +333,7 @@ TEST_F(ExtAuthzFilterTest, ImmediateNOK) { EXPECT_CALL(*client_, check(_, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { - callbacks.onComplete(Filters::Common::ExtAuthz::CheckStatus::Denied); + callbacks.onComplete(makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus::Denied)); }))); EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection()); @@ -351,7 +359,7 @@ TEST_F(ExtAuthzFilterTest, ImmediateErrorFailOpen) { EXPECT_CALL(*client_, check(_, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void { - callbacks.onComplete(Filters::Common::ExtAuthz::CheckStatus::Error); + callbacks.onComplete(makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus::Error)); }))); EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection());