From b0abe5c38035b43eccaa2d95e119e85779a6c726 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Tue, 26 Jul 2022 00:14:35 +0000 Subject: [PATCH 1/4] mst fix e2e with hardcode. Signed-off-by: Jianfei Hu --- .../authservice/templates/config.yaml | 5 +++++ src/service/BUILD | 1 + src/service/async_service_impl.h | 16 +++++++++++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/bookinfo-example/authservice/templates/config.yaml b/bookinfo-example/authservice/templates/config.yaml index 247b6ca4..ac74bc36 100644 --- a/bookinfo-example/authservice/templates/config.yaml +++ b/bookinfo-example/authservice/templates/config.yaml @@ -20,9 +20,14 @@ data: "listen_port": "10003", "log_level": "trace", "threads": 8, + "allow_unmatched_requests": "false", "chains": [ { "name": "idp_filter_chain", + "match": { + "header": ":authority", + "prefix": "test.example.com", + }, "filters": [ { "oidc": diff --git a/src/service/BUILD b/src/service/BUILD index 058e49b5..8dd3f1a4 100644 --- a/src/service/BUILD +++ b/src/service/BUILD @@ -35,6 +35,7 @@ authsvc_cc_library( "@com_github_gabime_spdlog//:spdlog", "@com_github_grpc_grpc//:grpc++", "@envoy//source/common/config:version_converter_lib", + # "@envoy//envoy/grpc:status", "@envoy_api//envoy/service/auth/v2:pkg_cc_grpc", "@envoy_api//envoy/service/auth/v3:pkg_cc_grpc", ], diff --git a/src/service/async_service_impl.h b/src/service/async_service_impl.h index 1e4652d5..d6f88cd0 100644 --- a/src/service/async_service_impl.h +++ b/src/service/async_service_impl.h @@ -12,6 +12,7 @@ #include "common/config/version_converter.h" #include "config/config.pb.h" #include "envoy/common/exception.h" +// #include "envoy/grpc/status.h" #include "envoy/service/auth/v2/external_auth.grpc.pb.h" #include "envoy/service/auth/v3/external_auth.grpc.pb.h" #include "src/common/http/http.h" @@ -66,6 +67,9 @@ ::grpc::Status Check( ? grpc::Status::OK : grpc::Status(grpc::StatusCode::PERMISSION_DENIED, "permission denied"); + spdlog::debug("jianfeih debug allow unmatched or not: {}, foo {}, msg {}", + allow_unmatched_requests, default_response_code.ok(), + default_response_code.error_message()); // Find a configured processing chain. for (auto &chain : chains) { @@ -128,12 +132,22 @@ ::grpc::Status Check( // No matching filter chain found. Allow request to continue. spdlog::debug( - "{}: no matching filter chain for request to {}://{}{}, respond with: " + "{}: jianfeih here1, no matching filter chain for request to " + "{}://{}{}, respond with: " "{}", __func__, request.attributes().request().http().scheme(), request.attributes().request().http().host(), request.attributes().request().http().path(), default_response_code.error_code()); + // TODO: not just here all the response code needs to be modified. + if constexpr (std::is_same_v) { + spdlog::debug("jianfeih debug setting status happening."); + envoy::service::auth::v3::CheckResponse response_v3; + response_v3.mutable_status()->set_code(7); + // Grpc::Status::WellKnownGrpcStatus::PermissionDenied); + // response = response_v3; + } return default_response_code; } catch (const std::exception &exception) { spdlog::error("%s unexpected error: %s", __func__, exception.what()); From 8a7e9cb556ce4d4b86a18b1d6afa142f01145343 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Tue, 26 Jul 2022 17:48:36 +0000 Subject: [PATCH 2/4] wip organize the convert code. Signed-off-by: Jianfei Hu --- src/service/async_service_impl.cc | 27 ++++++++++ src/service/async_service_impl.h | 84 ++++++++++++------------------- 2 files changed, 60 insertions(+), 51 deletions(-) diff --git a/src/service/async_service_impl.cc b/src/service/async_service_impl.cc index 9849e8d9..4fe0f75f 100644 --- a/src/service/async_service_impl.cc +++ b/src/service/async_service_impl.cc @@ -18,6 +18,33 @@ namespace { constexpr uint16_t kHealthCheckServerPort = 10004; } +::grpc::Status convertGrpcStatus(const google::rpc::Code status) { + // See src/filters/filter.h:filter::Process for a description of how + // status codes should be handled + switch (status) { + case google::rpc::Code::OK: // The request was successful + case google::rpc::Code::UNAUTHENTICATED: // A filter indicated the + // request had no + // authentication but was + // processed correctly. + case google::rpc::Code::PERMISSION_DENIED: // A filter indicated + // insufficient permissions + // for the authenticated + // requester but was processed + // correctly. + return ::grpc::Status::OK; + case google::rpc::Code::INVALID_ARGUMENT: // The request was not well + // formed. Indicate a + // processing error to the + // caller. + return ::grpc::Status(::grpc::StatusCode::INVALID_ARGUMENT, + "invalid request"); + default: // All other errors are treated as internal processing + // failures. + return ::grpc::Status(::grpc::StatusCode::INTERNAL, "internal error"); + } +} + ProcessingStateV2::ProcessingStateV2( ProcessingStateFactory &parent, envoy::service::auth::v2::Authorization::AsyncService &service) diff --git a/src/service/async_service_impl.h b/src/service/async_service_impl.h index d6f88cd0..c0ec4ae7 100644 --- a/src/service/async_service_impl.h +++ b/src/service/async_service_impl.h @@ -23,6 +23,8 @@ namespace authservice { namespace service { +::grpc::Status convertGrpcStatus(const google::rpc::Code status); + template ::grpc::Status Check( const RequestType &request, ResponseType &response, @@ -60,17 +62,6 @@ ::grpc::Status Check( return ::grpc::Status::OK; } - // TODO(incfly): Clean up trigger rule after checking the current Istio - // ExtAuthz API is sufficient. - const auto default_response_code = - allow_unmatched_requests - ? grpc::Status::OK - : grpc::Status(grpc::StatusCode::PERMISSION_DENIED, - "permission denied"); - spdlog::debug("jianfeih debug allow unmatched or not: {}, foo {}, msg {}", - allow_unmatched_requests, default_response_code.ok(), - default_response_code.error_message()); - // Find a configured processing chain. for (auto &chain : chains) { if (chain->Matches(&request_v3)) { @@ -84,7 +75,6 @@ ::grpc::Status Check( // Create a new instance of a processor. auto processor = chain->New(); auto status = processor->Process(&request_v3, &response_v3, ioc, yield); - // response v2/v3 conversion layer if constexpr (std::is_same_v< ResponseType, @@ -101,53 +91,45 @@ ::grpc::Status Check( ::envoy::service::auth::v3::CheckResponse>) { response = response_v3; } - - // See src/filters/filter.h:filter::Process for a description of how - // status codes should be handled - switch (status) { - case google::rpc::Code::OK: // The request was successful - case google::rpc::Code::UNAUTHENTICATED: // A filter indicated the - // request had no - // authentication but was - // processed correctly. - case google::rpc::Code::PERMISSION_DENIED: // A filter indicated - // insufficient permissions - // for the authenticated - // requester but was processed - // correctly. - return ::grpc::Status::OK; - case google::rpc::Code::INVALID_ARGUMENT: // The request was not well - // formed. Indicate a - // processing error to the - // caller. - return ::grpc::Status(::grpc::StatusCode::INVALID_ARGUMENT, - "invalid request"); - default: // All other errors are treated as internal processing - // failures. - return ::grpc::Status(::grpc::StatusCode::INTERNAL, - "internal error"); - } + return convertGrpcStatus(status); } } + // No matching filter chain found. + + // TODO(incfly): Clean up trigger rule after checking the current Istio + // ExtAuthz API is sufficient. + auto default_response_code = + grpc::Status(grpc::StatusCode::PERMISSION_DENIED, "permission denied"); + google::rpc::Code code = google::rpc::Code::PERMISSION_DENIED; + if (allow_unmatched_requests) { + default_response_code = grpc::Status::OK; + code = google::rpc::Code::OK; + } + envoy::service::auth::v2::CheckResponse response_v2; + envoy::service::auth::v3::CheckResponse response_v3; + response_v2.mutable_status()->set_code(code); + response_v3.mutable_status()->set_code(code); + if constexpr (std::is_same_v) { + response = response_v2; + } else if (std::is_same_v) { + response = response_v3; + } + + if constexpr (std::is_same_v) { + response = response_v3; + } - // No matching filter chain found. Allow request to continue. spdlog::debug( - "{}: jianfeih here1, no matching filter chain for request to " - "{}://{}{}, respond with: " + "{}: no matching filter chain for request to " + "{}://{}{}, allow_unmatched_requests {}, respond with: " "{}", __func__, request.attributes().request().http().scheme(), request.attributes().request().http().host(), - request.attributes().request().http().path(), + request.attributes().request().http().path(), allow_unmatched_requests, default_response_code.error_code()); - // TODO: not just here all the response code needs to be modified. - if constexpr (std::is_same_v) { - spdlog::debug("jianfeih debug setting status happening."); - envoy::service::auth::v3::CheckResponse response_v3; - response_v3.mutable_status()->set_code(7); - // Grpc::Status::WellKnownGrpcStatus::PermissionDenied); - // response = response_v3; - } return default_response_code; } catch (const std::exception &exception) { spdlog::error("%s unexpected error: %s", __func__, exception.what()); From f5667b0a41287f0bafde64f1ffab05da7fc7d1cd Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Tue, 26 Jul 2022 18:12:24 +0000 Subject: [PATCH 3/4] update example to use localhost Signed-off-by: Jianfei Hu --- bookinfo-example/authservice/templates/config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookinfo-example/authservice/templates/config.yaml b/bookinfo-example/authservice/templates/config.yaml index ac74bc36..7bb94592 100644 --- a/bookinfo-example/authservice/templates/config.yaml +++ b/bookinfo-example/authservice/templates/config.yaml @@ -26,7 +26,7 @@ data: "name": "idp_filter_chain", "match": { "header": ":authority", - "prefix": "test.example.com", + "prefix": "localhost", }, "filters": [ { From d65420b653e64a5f8be7f866263716dff2af2f4e Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Tue, 26 Jul 2022 18:13:34 +0000 Subject: [PATCH 4/4] make format. Signed-off-by: Jianfei Hu --- src/service/async_service_impl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/service/async_service_impl.cc b/src/service/async_service_impl.cc index 4fe0f75f..561c7316 100644 --- a/src/service/async_service_impl.cc +++ b/src/service/async_service_impl.cc @@ -18,7 +18,7 @@ namespace { constexpr uint16_t kHealthCheckServerPort = 10004; } -::grpc::Status convertGrpcStatus(const google::rpc::Code status) { +::grpc::Status convertGrpcStatus(const google::rpc::Code status) { // See src/filters/filter.h:filter::Process for a description of how // status codes should be handled switch (status) {