diff --git a/source/extensions/filters/common/rbac/engine.h b/source/extensions/filters/common/rbac/engine.h index 1281f6319bcf2..e966fc3e54898 100644 --- a/source/extensions/filters/common/rbac/engine.h +++ b/source/extensions/filters/common/rbac/engine.h @@ -25,9 +25,12 @@ class RoleBasedAccessControlEngine { * @param headers the headers of the incoming request used to identify the action/principal. An * empty map should be used if there are no headers available. * @param metadata the metadata with additional information about the action/principal. + * @param effective_policy_id it will be filled by the matching policy's ID, + * which is used to identity the source of the allow/deny. */ virtual bool allowed(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, - const envoy::api::v2::core::Metadata& metadata) const PURE; + const envoy::api::v2::core::Metadata& metadata, + std::string* effective_policy_id) const PURE; }; } // namespace RBAC diff --git a/source/extensions/filters/common/rbac/engine_impl.cc b/source/extensions/filters/common/rbac/engine_impl.cc index 1608476f8efff..0d3a01fdde89e 100644 --- a/source/extensions/filters/common/rbac/engine_impl.cc +++ b/source/extensions/filters/common/rbac/engine_impl.cc @@ -11,17 +11,22 @@ RoleBasedAccessControlEngineImpl::RoleBasedAccessControlEngineImpl( : allowed_if_matched_(rules.action() == envoy::config::rbac::v2alpha::RBAC_Action::RBAC_Action_ALLOW) { for (const auto& policy : rules.policies()) { - policies_.emplace_back(policy.second); + policies_.insert(std::make_pair(policy.first, policy.second)); } } -bool RoleBasedAccessControlEngineImpl::allowed( - const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, - const envoy::api::v2::core::Metadata& metadata) const { +bool RoleBasedAccessControlEngineImpl::allowed(const Network::Connection& connection, + const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata& metadata, + std::string* effective_policy_id) const { bool matched = false; - for (const auto& policy : policies_) { - if (policy.matches(connection, headers, metadata)) { + + for (auto it = policies_.begin(); it != policies_.end(); it++) { + if (it->second.matches(connection, headers, metadata)) { matched = true; + if (effective_policy_id != nullptr) { + *effective_policy_id = it->first; + } break; } } diff --git a/source/extensions/filters/common/rbac/engine_impl.h b/source/extensions/filters/common/rbac/engine_impl.h index 6be4d18798d27..e8343d737d94b 100644 --- a/source/extensions/filters/common/rbac/engine_impl.h +++ b/source/extensions/filters/common/rbac/engine_impl.h @@ -16,12 +16,13 @@ class RoleBasedAccessControlEngineImpl : public RoleBasedAccessControlEngine { RoleBasedAccessControlEngineImpl(const envoy::config::rbac::v2alpha::RBAC& rules); bool allowed(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, - const envoy::api::v2::core::Metadata& metadata) const override; + const envoy::api::v2::core::Metadata& metadata, + std::string* effective_policy_id) const override; private: const bool allowed_if_matched_; - std::vector policies_; + std::map policies_; }; } // namespace RBAC diff --git a/source/extensions/filters/http/rbac/rbac_filter.cc b/source/extensions/filters/http/rbac/rbac_filter.cc index 08a347a79aa9b..277ef15d614eb 100644 --- a/source/extensions/filters/http/rbac/rbac_filter.cc +++ b/source/extensions/filters/http/rbac/rbac_filter.cc @@ -9,6 +9,11 @@ namespace Extensions { namespace HttpFilters { namespace RBACFilter { +static const std::string resp_code_200 = "200"; +static const std::string resp_code_403 = "403"; +static const std::string shadow_policy_id_field = "shadow_effective_policyID"; +static const std::string shadow_resp_code_field = "shadow_response_code"; + RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig( const envoy::config::filter::http::rbac::v2::RBAC& proto_config, const std::string& stats_prefix, Stats::Scope& scope) @@ -77,16 +82,38 @@ Http::FilterHeadersStatus RoleBasedAccessControlFilter::decodeHeaders(Http::Head callbacks_->connection()->ssl()->subjectPeerCertificate() : "none", headers, callbacks_->requestInfo().dynamicMetadata().DebugString()); + std::string effective_policy_id; const absl::optional& shadow_engine = config_->engine(callbacks_->route(), EnforcementMode::Shadow); if (shadow_engine.has_value()) { + std::string shadow_resp_code = resp_code_200; if (shadow_engine->allowed(*callbacks_->connection(), headers, - callbacks_->requestInfo().dynamicMetadata())) { + callbacks_->requestInfo().dynamicMetadata(), &effective_policy_id)) { ENVOY_LOG(debug, "shadow allowed"); config_->stats().shadow_allowed_.inc(); } else { ENVOY_LOG(debug, "shadow denied"); config_->stats().shadow_denied_.inc(); + shadow_resp_code = resp_code_403; + } + + const auto& filter_metadata = callbacks_->requestInfo().dynamicMetadata().filter_metadata(); + const auto filter_it = filter_metadata.find(HttpFilterNames::get().Rbac); + if (filter_it != filter_metadata.end()) { + ProtobufWkt::Struct metrics; + + if (!effective_policy_id.empty()) { + ProtobufWkt::Value policy_id; + policy_id.set_string_value(effective_policy_id); + (*metrics.mutable_fields())[shadow_policy_id_field] = policy_id; + } + + ProtobufWkt::Value resp_code; + resp_code.set_string_value(shadow_resp_code); + (*metrics.mutable_fields())[shadow_resp_code_field] = resp_code; + + auto filter_meta = filter_metadata.at(HttpFilterNames::get().Rbac); + filter_meta.MergeFrom(metrics); } } @@ -94,7 +121,7 @@ Http::FilterHeadersStatus RoleBasedAccessControlFilter::decodeHeaders(Http::Head config_->engine(callbacks_->route(), EnforcementMode::Enforced); if (engine.has_value()) { if (engine->allowed(*callbacks_->connection(), headers, - callbacks_->requestInfo().dynamicMetadata())) { + callbacks_->requestInfo().dynamicMetadata(), nullptr)) { ENVOY_LOG(debug, "enforced allowed"); config_->stats().allowed_.inc(); return Http::FilterHeadersStatus::Continue; diff --git a/test/extensions/filters/common/rbac/engine_impl_test.cc b/test/extensions/filters/common/rbac/engine_impl_test.cc index b4f441f44cab1..cf848847728c9 100644 --- a/test/extensions/filters/common/rbac/engine_impl_test.cc +++ b/test/extensions/filters/common/rbac/engine_impl_test.cc @@ -20,12 +20,12 @@ namespace Common { namespace RBAC { namespace { -void checkEngine( - const RBAC::RoleBasedAccessControlEngineImpl& engine, bool expected, - const Envoy::Network::Connection& connection = Envoy::Network::MockConnection(), - const Envoy::Http::HeaderMap& headers = Envoy::Http::HeaderMapImpl(), - const envoy::api::v2::core::Metadata& metadata = envoy::api::v2::core::Metadata()) { - EXPECT_EQ(expected, engine.allowed(connection, headers, metadata)); +void checkEngine(const RBAC::RoleBasedAccessControlEngineImpl& engine, bool expected, + const Envoy::Network::Connection& connection = Envoy::Network::MockConnection(), + const Envoy::Http::HeaderMap& headers = Envoy::Http::HeaderMapImpl(), + const envoy::api::v2::core::Metadata& metadata = envoy::api::v2::core::Metadata(), + std::string* policy_id = nullptr) { + EXPECT_EQ(expected, engine.allowed(connection, headers, metadata, policy_id)); } TEST(RoleBasedAccessControlEngineImpl, Disabled) { diff --git a/test/extensions/filters/common/rbac/mocks.h b/test/extensions/filters/common/rbac/mocks.h index a289b6cea8c37..b0e0a4de82a10 100644 --- a/test/extensions/filters/common/rbac/mocks.h +++ b/test/extensions/filters/common/rbac/mocks.h @@ -15,8 +15,9 @@ class MockEngine : public RoleBasedAccessControlEngineImpl { MockEngine(const envoy::config::rbac::v2alpha::RBAC& rules) : RoleBasedAccessControlEngineImpl(rules){}; - MOCK_CONST_METHOD3(allowed, bool(const Envoy::Network::Connection&, const Envoy::Http::HeaderMap&, - const envoy::api::v2::core::Metadata&)); + MOCK_CONST_METHOD4(allowed, + bool(const Envoy::Network::Connection&, const Envoy::Http::HeaderMap&, + const envoy::api::v2::core::Metadata&, std::string* effective_policy_id)); }; } // namespace RBAC diff --git a/test/extensions/filters/http/rbac/rbac_filter_test.cc b/test/extensions/filters/http/rbac/rbac_filter_test.cc index 7e1d89ff852b3..4febdda18835d 100644 --- a/test/extensions/filters/http/rbac/rbac_filter_test.cc +++ b/test/extensions/filters/http/rbac/rbac_filter_test.cc @@ -1,3 +1,4 @@ +#include "common/config/metadata.h" #include "common/network/utility.h" #include "extensions/filters/http/rbac/rbac_filter.h" @@ -43,6 +44,7 @@ class RoleBasedAccessControlFilterTest : public testing::Test { void SetUp() { EXPECT_CALL(callbacks_, connection()).WillRepeatedly(Return(&connection_)); + EXPECT_CALL(callbacks_, requestInfo()).WillRepeatedly(ReturnRef(req_info_)); filter_.setDecoderFilterCallbacks(callbacks_); } @@ -53,9 +55,11 @@ class RoleBasedAccessControlFilterTest : public testing::Test { NiceMock callbacks_; NiceMock connection_{}; + NiceMock req_info_; Stats::IsolatedStoreImpl store_; RoleBasedAccessControlFilterConfigSharedPtr config_; + envoy::api::v2::core::Metadata metadata_; RoleBasedAccessControlFilter filter_; Network::Address::InstanceConstSharedPtr address_; Http::TestHeaderMapImpl headers_; @@ -98,7 +102,7 @@ TEST_F(RoleBasedAccessControlFilterTest, RouteLocalOverride) { NiceMock engine{route_config.rbac().rules()}; NiceMock per_route_config_{route_config}; - EXPECT_CALL(engine, allowed(_, _, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(engine, allowed(_, _, _, _)).WillRepeatedly(Return(true)); EXPECT_CALL(per_route_config_, engine()).WillRepeatedly(ReturnRef(engine)); EXPECT_CALL(callbacks_.route_->route_entry_, perFilterConfig(HttpFilterNames::get().Rbac))