Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the unmatched request handling by set the grpc status in payload. #223

Merged
merged 4 commits into from
Jul 27, 2022
Merged

Fix the unmatched request handling by set the grpc status in payload. #223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b0abe5c
mst fix e2e with hardcode.
incfly Jul 26, 2022
8a7e9cb
wip organize the convert code.
incfly Jul 26, 2022
f5667b0
update example to use localhost
incfly Jul 26, 2022
d65420b
make format.
incfly Jul 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions bookinfo-example/authservice/templates/config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": "localhost",
},
"filters": [
{
"oidc":
Expand Down
1 change: 1 addition & 0 deletions src/service/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
27 changes: 27 additions & 0 deletions src/service/async_service_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
72 changes: 34 additions & 38 deletions src/service/async_service_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove?

#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"
Expand All @@ -22,6 +23,8 @@
namespace authservice {
namespace service {

::grpc::Status convertGrpcStatus(const google::rpc::Code status);

template <class RequestType, class ResponseType>
::grpc::Status Check(
const RequestType &request, ResponseType &response,
Expand Down Expand Up @@ -59,14 +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");

// Find a configured processing chain.
for (auto &chain : chains) {
if (chain->Matches(&request_v3)) {
Expand All @@ -80,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,
Expand All @@ -97,42 +91,44 @@ ::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<ResponseType,
::envoy::service::auth::v2::CheckResponse>) {
response = response_v2;
} else if (std::is_same_v<ResponseType,
::envoy::service::auth::v3::CheckResponse>) {
response = response_v3;
}

if constexpr (std::is_same_v<ResponseType,
::envoy::service::auth::v3::CheckResponse>) {
response = response_v3;
}

// No matching filter chain found. Allow request to continue.
spdlog::debug(
"{}: 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());
return default_response_code;
} catch (const std::exception &exception) {
Expand Down