diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 7515d061f709e..ac10887c35e41 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -20,6 +20,7 @@ Bug Fixes --------- *Changes expected to improve the state of the world and are unlikely to have negative effects* +* ext_authz: fix the ext_authz network filter to correctly set response flag and code details to ``UAEX`` when a connection is denied. * listener: fixed the crash when updating listeners that do not bind to port. * thrift_proxy: fix the thrift_proxy connection manager to correctly report success/error response metrics when performing :ref:`payload passthrough `. diff --git a/source/extensions/filters/common/ext_authz/ext_authz.h b/source/extensions/filters/common/ext_authz/ext_authz.h index 861faac9b1bdc..ee30b3b70fc97 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz.h +++ b/source/extensions/filters/common/ext_authz/ext_authz.h @@ -33,6 +33,17 @@ struct TracingConstantValues { using TracingConstants = ConstSingleton; +/** + * Possible constant response code details values for a check call. + */ +struct ResponseCodeDetailsValues { + // The ext_authz filter denied the downstream request/connection. + const std::string AuthzDenied = "ext_authz_denied"; + // The ext_authz filter encountered a failure, and was configured to fail-closed. + const std::string AuthzError = "ext_authz_error"; +}; +using ResponseCodeDetails = ConstSingleton; + /** * Constant auth related HTTP headers. All lower case. This group of headers can * contain prefix override headers. diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index bd6f7a14e569a..c6021c566baeb 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -13,14 +13,6 @@ namespace Extensions { namespace HttpFilters { namespace ExtAuthz { -struct RcDetailsValues { - // The ext_authz filter denied the downstream request. - const std::string AuthzDenied = "ext_authz_denied"; - // The ext_authz filter encountered a failure, and was configured to fail-closed. - const std::string AuthzError = "ext_authz_error"; -}; -using RcDetails = ConstSingleton; - void FilterConfigPerRoute::merge(const FilterConfigPerRoute& other) { // We only merge context extensions here, and leave boolean flags untouched since those flags are // not used from the merged config. @@ -91,8 +83,9 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, *decoder_callbacks_); decoder_callbacks_->streamInfo().setResponseFlag( StreamInfo::ResponseFlag::UnauthorizedExternalService); - decoder_callbacks_->sendLocalReply(config_->statusOnError(), EMPTY_STRING, nullptr, - absl::nullopt, RcDetails::get().AuthzError); + decoder_callbacks_->sendLocalReply( + config_->statusOnError(), EMPTY_STRING, nullptr, absl::nullopt, + Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzError); return Http::FilterHeadersStatus::StopIteration; } return Http::FilterHeadersStatus::Continue; @@ -371,7 +364,7 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { response_headers.addCopy(header.first, header.second); } }, - absl::nullopt, RcDetails::get().AuthzDenied); + absl::nullopt, Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzDenied); decoder_callbacks_->streamInfo().setResponseFlag( StreamInfo::ResponseFlag::UnauthorizedExternalService); break; @@ -396,8 +389,9 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { *decoder_callbacks_, enumToInt(config_->statusOnError())); decoder_callbacks_->streamInfo().setResponseFlag( StreamInfo::ResponseFlag::UnauthorizedExternalService); - decoder_callbacks_->sendLocalReply(config_->statusOnError(), EMPTY_STRING, nullptr, - absl::nullopt, RcDetails::get().AuthzError); + decoder_callbacks_->sendLocalReply( + config_->statusOnError(), EMPTY_STRING, nullptr, absl::nullopt, + Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzError); } break; } diff --git a/source/extensions/filters/network/ext_authz/ext_authz.cc b/source/extensions/filters/network/ext_authz/ext_authz.cc index b59a2bf04a4c8..4f9da9175fc15 100644 --- a/source/extensions/filters/network/ext_authz/ext_authz.cc +++ b/source/extensions/filters/network/ext_authz/ext_authz.cc @@ -94,6 +94,12 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { !config_->failureModeAllow())) { config_->stats().cx_closed_.inc(); filter_callbacks_->connection().close(Network::ConnectionCloseType::NoFlush); + filter_callbacks_->connection().streamInfo().setResponseFlag( + StreamInfo::ResponseFlag::UnauthorizedExternalService); + filter_callbacks_->connection().streamInfo().setResponseCodeDetails( + response->status == Filters::Common::ExtAuthz::CheckStatus::Denied + ? Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzDenied + : Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzError); } else { // Let the filter chain continue. filter_return_ = FilterReturn::Continue; 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 0f2190a17d974..4fa54f5d13bab 100644 --- a/test/extensions/filters/network/ext_authz/ext_authz_test.cc +++ b/test/extensions/filters/network/ext_authz/ext_authz_test.cc @@ -206,6 +206,11 @@ TEST_F(ExtAuthzFilterTest, DeniedWithOnData) { stats_store_.gauge("ext_authz.name.active", Stats::Gauge::ImportMode::Accumulate).value()); EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::NoFlush)); + EXPECT_CALL(filter_callbacks_.connection_.stream_info_, + setResponseFlag(StreamInfo::ResponseFlag::UnauthorizedExternalService)); + EXPECT_CALL( + filter_callbacks_.connection_.stream_info_, + setResponseCodeDetails(Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzDenied)); EXPECT_CALL(*client_, cancel()).Times(0); request_callbacks_->onComplete(makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus::Denied)); @@ -276,6 +281,11 @@ TEST_F(ExtAuthzFilterTest, FailClose) { EXPECT_CALL(filter_callbacks_.connection_, close(_)); EXPECT_CALL(filter_callbacks_, continueReading()).Times(0); + EXPECT_CALL(filter_callbacks_.connection_.stream_info_, + setResponseFlag(StreamInfo::ResponseFlag::UnauthorizedExternalService)); + EXPECT_CALL( + filter_callbacks_.connection_.stream_info_, + setResponseCodeDetails(Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzError)); request_callbacks_->onComplete(makeAuthzResponse(Filters::Common::ExtAuthz::CheckStatus::Error)); EXPECT_EQ(0U, stats_store_.counter("ext_authz.name.disabled").value()); @@ -429,7 +439,11 @@ TEST_F(ExtAuthzFilterTest, ImmediateNOK) { EXPECT_EQ(ns, NetworkFilterNames::get().ExtAuthorization); EXPECT_TRUE(TestUtility::protoEqual(returned_dynamic_metadata, dynamic_metadata)); })); - + EXPECT_CALL(filter_callbacks_.connection_.stream_info_, + setResponseFlag(StreamInfo::ResponseFlag::UnauthorizedExternalService)); + EXPECT_CALL( + filter_callbacks_.connection_.stream_info_, + setResponseCodeDetails(Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzDenied)); EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection()); Buffer::OwnedImpl data("hello"); EXPECT_EQ(Network::FilterStatus::StopIteration, filter_->onData(data, false));