Skip to content

Commit db55f63

Browse files
incflyincfly
committed
Fix the unmatched request handling by set the grpc status in payload. (istio-ecosystem#223)
* mst fix e2e with hardcode. Signed-off-by: Jianfei Hu <[email protected]> * wip organize the convert code. Signed-off-by: Jianfei Hu <[email protected]> * update example to use localhost Signed-off-by: Jianfei Hu <[email protected]> * make format. Signed-off-by: Jianfei Hu <[email protected]>
1 parent 28404a8 commit db55f63

File tree

4 files changed

+67
-38
lines changed

4 files changed

+67
-38
lines changed

Diff for: ‎bookinfo-example/authservice/templates/config.yaml

+5
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,14 @@ data:
2020
"listen_port": "10003",
2121
"log_level": "trace",
2222
"threads": 8,
23+
"allow_unmatched_requests": "false",
2324
"chains": [
2425
{
2526
"name": "idp_filter_chain",
27+
"match": {
28+
"header": ":authority",
29+
"prefix": "localhost",
30+
},
2631
"filters": [
2732
{
2833
"oidc":

Diff for: ‎src/service/BUILD

+1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ authsvc_cc_library(
3535
"@com_github_gabime_spdlog//:spdlog",
3636
"@com_github_grpc_grpc//:grpc++",
3737
"@envoy//source/common/config:version_converter_lib",
38+
# "@envoy//envoy/grpc:status",
3839
"@envoy_api//envoy/service/auth/v2:pkg_cc_grpc",
3940
"@envoy_api//envoy/service/auth/v3:pkg_cc_grpc",
4041
],

Diff for: ‎src/service/async_service_impl.cc

+27
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,33 @@ namespace {
1818
constexpr uint16_t kHealthCheckServerPort = 10004;
1919
}
2020

21+
::grpc::Status convertGrpcStatus(const google::rpc::Code status) {
22+
// See src/filters/filter.h:filter::Process for a description of how
23+
// status codes should be handled
24+
switch (status) {
25+
case google::rpc::Code::OK: // The request was successful
26+
case google::rpc::Code::UNAUTHENTICATED: // A filter indicated the
27+
// request had no
28+
// authentication but was
29+
// processed correctly.
30+
case google::rpc::Code::PERMISSION_DENIED: // A filter indicated
31+
// insufficient permissions
32+
// for the authenticated
33+
// requester but was processed
34+
// correctly.
35+
return ::grpc::Status::OK;
36+
case google::rpc::Code::INVALID_ARGUMENT: // The request was not well
37+
// formed. Indicate a
38+
// processing error to the
39+
// caller.
40+
return ::grpc::Status(::grpc::StatusCode::INVALID_ARGUMENT,
41+
"invalid request");
42+
default: // All other errors are treated as internal processing
43+
// failures.
44+
return ::grpc::Status(::grpc::StatusCode::INTERNAL, "internal error");
45+
}
46+
}
47+
2148
ProcessingStateV2::ProcessingStateV2(
2249
ProcessingStateFactory &parent,
2350
envoy::service::auth::v2::Authorization::AsyncService &service)

Diff for: ‎src/service/async_service_impl.h

+34-38
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "common/config/version_converter.h"
1313
#include "config/config.pb.h"
1414
#include "envoy/common/exception.h"
15+
// #include "envoy/grpc/status.h"
1516
#include "envoy/service/auth/v2/external_auth.grpc.pb.h"
1617
#include "envoy/service/auth/v3/external_auth.grpc.pb.h"
1718
#include "src/common/http/http.h"
@@ -22,6 +23,8 @@
2223
namespace authservice {
2324
namespace service {
2425

26+
::grpc::Status convertGrpcStatus(const google::rpc::Code status);
27+
2528
template <class RequestType, class ResponseType>
2629
::grpc::Status Check(
2730
const RequestType &request, ResponseType &response,
@@ -59,14 +62,6 @@ ::grpc::Status Check(
5962
return ::grpc::Status::OK;
6063
}
6164

62-
// TODO(incfly): Clean up trigger rule after checking the current Istio
63-
// ExtAuthz API is sufficient.
64-
const auto default_response_code =
65-
allow_unmatched_requests
66-
? grpc::Status::OK
67-
: grpc::Status(grpc::StatusCode::PERMISSION_DENIED,
68-
"permission denied");
69-
7065
// Find a configured processing chain.
7166
for (auto &chain : chains) {
7267
if (chain->Matches(&request_v3)) {
@@ -80,7 +75,6 @@ ::grpc::Status Check(
8075
// Create a new instance of a processor.
8176
auto processor = chain->New();
8277
auto status = processor->Process(&request_v3, &response_v3, ioc, yield);
83-
8478
// response v2/v3 conversion layer
8579
if constexpr (std::is_same_v<
8680
ResponseType,
@@ -97,42 +91,44 @@ ::grpc::Status Check(
9791
::envoy::service::auth::v3::CheckResponse>) {
9892
response = response_v3;
9993
}
100-
101-
// See src/filters/filter.h:filter::Process for a description of how
102-
// status codes should be handled
103-
switch (status) {
104-
case google::rpc::Code::OK: // The request was successful
105-
case google::rpc::Code::UNAUTHENTICATED: // A filter indicated the
106-
// request had no
107-
// authentication but was
108-
// processed correctly.
109-
case google::rpc::Code::PERMISSION_DENIED: // A filter indicated
110-
// insufficient permissions
111-
// for the authenticated
112-
// requester but was processed
113-
// correctly.
114-
return ::grpc::Status::OK;
115-
case google::rpc::Code::INVALID_ARGUMENT: // The request was not well
116-
// formed. Indicate a
117-
// processing error to the
118-
// caller.
119-
return ::grpc::Status(::grpc::StatusCode::INVALID_ARGUMENT,
120-
"invalid request");
121-
default: // All other errors are treated as internal processing
122-
// failures.
123-
return ::grpc::Status(::grpc::StatusCode::INTERNAL,
124-
"internal error");
125-
}
94+
return convertGrpcStatus(status);
12695
}
12796
}
97+
// No matching filter chain found.
98+
99+
// TODO(incfly): Clean up trigger rule after checking the current Istio
100+
// ExtAuthz API is sufficient.
101+
auto default_response_code =
102+
grpc::Status(grpc::StatusCode::PERMISSION_DENIED, "permission denied");
103+
google::rpc::Code code = google::rpc::Code::PERMISSION_DENIED;
104+
if (allow_unmatched_requests) {
105+
default_response_code = grpc::Status::OK;
106+
code = google::rpc::Code::OK;
107+
}
108+
envoy::service::auth::v2::CheckResponse response_v2;
109+
envoy::service::auth::v3::CheckResponse response_v3;
110+
response_v2.mutable_status()->set_code(code);
111+
response_v3.mutable_status()->set_code(code);
112+
if constexpr (std::is_same_v<ResponseType,
113+
::envoy::service::auth::v2::CheckResponse>) {
114+
response = response_v2;
115+
} else if (std::is_same_v<ResponseType,
116+
::envoy::service::auth::v3::CheckResponse>) {
117+
response = response_v3;
118+
}
119+
120+
if constexpr (std::is_same_v<ResponseType,
121+
::envoy::service::auth::v3::CheckResponse>) {
122+
response = response_v3;
123+
}
128124

129-
// No matching filter chain found. Allow request to continue.
130125
spdlog::debug(
131-
"{}: no matching filter chain for request to {}://{}{}, respond with: "
126+
"{}: no matching filter chain for request to "
127+
"{}://{}{}, allow_unmatched_requests {}, respond with: "
132128
"{}",
133129
__func__, request.attributes().request().http().scheme(),
134130
request.attributes().request().http().host(),
135-
request.attributes().request().http().path(),
131+
request.attributes().request().http().path(), allow_unmatched_requests,
136132
default_response_code.error_code());
137133
return default_response_code;
138134
} catch (const std::exception &exception) {

0 commit comments

Comments
 (0)