diff --git a/source/common/common/logger.h b/source/common/common/logger.h index e30eca38da861..4df977d5e97d6 100644 --- a/source/common/common/logger.h +++ b/source/common/common/logger.h @@ -26,26 +26,27 @@ namespace Logger { FUNCTION(client) \ FUNCTION(config) \ FUNCTION(connection) \ - FUNCTION(misc) \ FUNCTION(file) \ FUNCTION(filter) \ + FUNCTION(grpc) \ FUNCTION(hc) \ FUNCTION(health_checker) \ FUNCTION(http) \ FUNCTION(http2) \ FUNCTION(lua) \ FUNCTION(main) \ + FUNCTION(misc) \ FUNCTION(mongo) \ FUNCTION(pool) \ + FUNCTION(rbac) \ FUNCTION(redis) \ FUNCTION(router) \ FUNCTION(runtime) \ + FUNCTION(stats) \ FUNCTION(testing) \ + FUNCTION(thrift) \ FUNCTION(tracing) \ - FUNCTION(upstream) \ - FUNCTION(grpc) \ - FUNCTION(stats) \ - FUNCTION(thrift) + FUNCTION(upstream) enum class Id { ALL_LOGGER_IDS(GENERATE_ENUM) diff --git a/source/extensions/filters/http/rbac/rbac_filter.cc b/source/extensions/filters/http/rbac/rbac_filter.cc index 78b0db13851f9..4fe193a5da6ba 100644 --- a/source/extensions/filters/http/rbac/rbac_filter.cc +++ b/source/extensions/filters/http/rbac/rbac_filter.cc @@ -65,13 +65,27 @@ RoleBasedAccessControlRouteSpecificFilterConfig::RoleBasedAccessControlRouteSpec Http::FilterHeadersStatus RoleBasedAccessControlFilter::decodeHeaders(Http::HeaderMap& headers, bool) { + ENVOY_LOG( + debug, + "checking request: remoteAddress: {}, localAddress: {}, ssl: {}, headers: {}, " + "dynamicMetadata: {}", + callbacks_->connection()->remoteAddress()->asString(), + callbacks_->connection()->localAddress()->asString(), + callbacks_->connection()->ssl() + ? "uriSanPeerCertificate: " + callbacks_->connection()->ssl()->uriSanPeerCertificate() + + ", subjectPeerCertificate: " + + callbacks_->connection()->ssl()->subjectPeerCertificate() + : "none", + headers, callbacks_->requestInfo().dynamicMetadata().DebugString()); const absl::optional& shadow_engine = config_->engine(callbacks_->route(), EnforcementMode::Shadow); if (shadow_engine.has_value()) { if (shadow_engine->allowed(*callbacks_->connection(), headers, callbacks_->requestInfo().dynamicMetadata())) { + ENVOY_LOG(debug, "shadow allowed"); config_->stats().shadow_allowed_.inc(); } else { + ENVOY_LOG(debug, "shadow denied"); config_->stats().shadow_denied_.inc(); } } @@ -81,15 +95,18 @@ Http::FilterHeadersStatus RoleBasedAccessControlFilter::decodeHeaders(Http::Head if (engine.has_value()) { if (engine->allowed(*callbacks_->connection(), headers, callbacks_->requestInfo().dynamicMetadata())) { + ENVOY_LOG(debug, "enforced allowed"); config_->stats().allowed_.inc(); return Http::FilterHeadersStatus::Continue; } else { + ENVOY_LOG(debug, "enforced denied"); callbacks_->sendLocalReply(Http::Code::Forbidden, "RBAC: access denied", nullptr); config_->stats().denied_.inc(); return Http::FilterHeadersStatus::StopIteration; } } + ENVOY_LOG(debug, "no engine, allowed by default"); return Http::FilterHeadersStatus::Continue; } diff --git a/source/extensions/filters/http/rbac/rbac_filter.h b/source/extensions/filters/http/rbac/rbac_filter.h index 48a0c9703b5d4..72770303b6e44 100644 --- a/source/extensions/filters/http/rbac/rbac_filter.h +++ b/source/extensions/filters/http/rbac/rbac_filter.h @@ -6,6 +6,8 @@ #include "envoy/http/filter.h" #include "envoy/stats/stats_macros.h" +#include "common/common/logger.h" + #include "extensions/filters/common/rbac/engine_impl.h" namespace Envoy { @@ -80,7 +82,8 @@ typedef std::shared_ptr /** * A filter that provides role-based access control authorization for HTTP requests. */ -class RoleBasedAccessControlFilter : public Http::StreamDecoderFilter { +class RoleBasedAccessControlFilter : public Http::StreamDecoderFilter, + public Logger::Loggable { public: RoleBasedAccessControlFilter(RoleBasedAccessControlFilterConfigSharedPtr config) : config_(config) {} diff --git a/test/extensions/filters/http/rbac/rbac_filter_test.cc b/test/extensions/filters/http/rbac/rbac_filter_test.cc index 8e5cc2b6cb87a..68f39ef1ea2b8 100644 --- a/test/extensions/filters/http/rbac/rbac_filter_test.cc +++ b/test/extensions/filters/http/rbac/rbac_filter_test.cc @@ -46,13 +46,9 @@ class RoleBasedAccessControlFilterTest : public testing::Test { filter_.setDecoderFilterCallbacks(callbacks_); } - void setDestinationPort(uint16_t port, int times = 2) { + void setDestinationPort(uint16_t port) { address_ = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", port, false); - auto& expect = EXPECT_CALL(connection_, localAddress()); - if (times > 0) { - expect.Times(times); - } - expect.WillRepeatedly(ReturnRef(address_)); + ON_CALL(connection_, localAddress()).WillByDefault(ReturnRef(address_)); } NiceMock callbacks_; @@ -94,7 +90,7 @@ TEST_F(RoleBasedAccessControlFilterTest, Denied) { } TEST_F(RoleBasedAccessControlFilterTest, RouteLocalOverride) { - setDestinationPort(456, 0); + setDestinationPort(456); envoy::config::filter::http::rbac::v2::RBACPerRoute route_config; route_config.mutable_rbac()->mutable_rules()->set_action(