diff --git a/BUILD b/BUILD index 538bfad2133..9f1b1a686f3 100644 --- a/BUILD +++ b/BUILD @@ -850,6 +850,7 @@ grpc_cc_library( "absl/status:statusor", "absl/strings", "absl/strings:str_format", + "absl/types:optional", ], language = "c++", deps = [ diff --git a/src/core/lib/security/authorization/grpc_authorization_policy_provider.cc b/src/core/lib/security/authorization/grpc_authorization_policy_provider.cc index a70a6c90b8e..83fe30461e1 100644 --- a/src/core/lib/security/authorization/grpc_authorization_policy_provider.cc +++ b/src/core/lib/security/authorization/grpc_authorization_policy_provider.cc @@ -16,9 +16,7 @@ #include "src/core/lib/security/authorization/grpc_authorization_policy_provider.h" -#include - -#include +#include "absl/types/optional.h" #include #include @@ -33,6 +31,8 @@ #include "src/core/lib/iomgr/error.h" #include "src/core/lib/iomgr/load_file.h" #include "src/core/lib/security/authorization/grpc_authorization_engine.h" +#include "src/core/lib/security/authorization/rbac_policy.h" +#include "src/core/lib/security/authorization/rbac_translator.h" #include "src/core/lib/slice/slice.h" #include "src/core/lib/slice/slice_internal.h" @@ -54,8 +54,10 @@ StaticDataAuthorizationPolicyProvider::StaticDataAuthorizationPolicyProvider( RbacPolicies policies) : allow_engine_(MakeRefCounted( std::move(policies.allow_policy))), - deny_engine_(MakeRefCounted( - std::move(policies.deny_policy))) {} + deny_engine_(policies.deny_policy.has_value() + ? MakeRefCounted( + std::move(*policies.deny_policy)) + : nullptr) {} namespace { @@ -159,8 +161,12 @@ absl::Status FileWatcherAuthorizationPolicyProvider::ForceUpdate() { MutexLock lock(&mu_); allow_engine_ = MakeRefCounted( std::move(rbac_policies_or->allow_policy)); - deny_engine_ = MakeRefCounted( - std::move(rbac_policies_or->deny_policy)); + if (rbac_policies_or->deny_policy.has_value()) { + deny_engine_ = MakeRefCounted( + std::move(*rbac_policies_or->deny_policy)); + } else { + deny_engine_.reset(); + } if (cb_ != nullptr) { cb_(contents_changed, absl::OkStatus()); } diff --git a/src/core/lib/security/authorization/rbac_translator.cc b/src/core/lib/security/authorization/rbac_translator.cc index 89866f78d6c..9cb9acbdfd7 100644 --- a/src/core/lib/security/authorization/rbac_translator.cc +++ b/src/core/lib/security/authorization/rbac_translator.cc @@ -111,16 +111,20 @@ absl::StatusOr ParsePrincipalsArray(const Json& json) { absl::StatusOr ParsePeer(const Json& json) { std::vector> peer; - auto it = json.object_value().find("principals"); - if (it != json.object_value().end()) { - if (it->second.type() != Json::Type::ARRAY) { - return absl::InvalidArgumentError("\"principals\" is not an array."); - } - auto principal_names_or = ParsePrincipalsArray(it->second); - if (!principal_names_or.ok()) return principal_names_or.status(); - if (!principal_names_or.value().principals.empty()) { - peer.push_back(std::make_unique( - std::move(principal_names_or.value()))); + for (const auto& object : json.object_value()) { + if (object.first == "principals") { + if (object.second.type() != Json::Type::ARRAY) { + return absl::InvalidArgumentError("\"principals\" is not an array."); + } + auto principal_names_or = ParsePrincipalsArray(object.second); + if (!principal_names_or.ok()) return principal_names_or.status(); + if (!principal_names_or.value().principals.empty()) { + peer.push_back(std::make_unique( + std::move(principal_names_or.value()))); + } + } else { + return absl::InvalidArgumentError(absl::StrFormat( + "policy contains unknown field \"%s\" in \"source\".", object.first)); } } if (peer.empty()) { @@ -154,28 +158,36 @@ absl::StatusOr ParseHeaderValues( } absl::StatusOr ParseHeaders(const Json& json) { - auto it = json.object_value().find("key"); - if (it == json.object_value().end()) { - return absl::InvalidArgumentError("\"key\" is not present."); - } - if (it->second.type() != Json::Type::STRING) { - return absl::InvalidArgumentError("\"key\" is not a string."); + absl::string_view key; + const Json* values = nullptr; + for (const auto& object : json.object_value()) { + if (object.first == "key") { + if (object.second.type() != Json::Type::STRING) { + return absl::InvalidArgumentError("\"key\" is not a string."); + } + key = object.second.string_value(); + if (absl::StartsWith(key, ":") || absl::StartsWith(key, "grpc-") || + IsUnsupportedHeader(key)) { + return absl::InvalidArgumentError( + absl::StrFormat("Unsupported \"key\" %s.", key)); + } + } else if (object.first == "values") { + if (object.second.type() != Json::Type::ARRAY) { + return absl::InvalidArgumentError("\"values\" is not an array."); + } + values = &object.second; + } else { + return absl::InvalidArgumentError(absl::StrFormat( + "policy contains unknown field \"%s\".", object.first)); + } } - absl::string_view header_name = it->second.string_value(); - if (absl::StartsWith(header_name, ":") || - absl::StartsWith(header_name, "grpc-") || - IsUnsupportedHeader(header_name)) { - return absl::InvalidArgumentError( - absl::StrFormat("Unsupported \"key\" %s.", header_name)); + if (key.empty()) { + return absl::InvalidArgumentError("\"key\" is not present."); } - it = json.object_value().find("values"); - if (it == json.object_value().end()) { + if (values == nullptr) { return absl::InvalidArgumentError("\"values\" is not present."); } - if (it->second.type() != Json::Type::ARRAY) { - return absl::InvalidArgumentError("\"values\" is not an array."); - } - return ParseHeaderValues(it->second, header_name); + return ParseHeaderValues(*values, key); } absl::StatusOr ParseHeadersArray(const Json& json) { @@ -220,28 +232,31 @@ absl::StatusOr ParsePathsArray(const Json& json) { absl::StatusOr ParseRequest(const Json& json) { std::vector> request; - auto it = json.object_value().find("paths"); - if (it != json.object_value().end()) { - if (it->second.type() != Json::Type::ARRAY) { - return absl::InvalidArgumentError("\"paths\" is not an array."); - } - auto paths_or = ParsePathsArray(it->second); - if (!paths_or.ok()) return paths_or.status(); - if (!paths_or.value().permissions.empty()) { - request.push_back( - std::make_unique(std::move(paths_or.value()))); - } - } - it = json.object_value().find("headers"); - if (it != json.object_value().end()) { - if (it->second.type() != Json::Type::ARRAY) { - return absl::InvalidArgumentError("\"headers\" is not an array."); - } - auto headers_or = ParseHeadersArray(it->second); - if (!headers_or.ok()) return headers_or.status(); - if (!headers_or.value().permissions.empty()) { - request.push_back( - std::make_unique(std::move(headers_or.value()))); + for (const auto& object : json.object_value()) { + if (object.first == "paths") { + if (object.second.type() != Json::Type::ARRAY) { + return absl::InvalidArgumentError("\"paths\" is not an array."); + } + auto paths_or = ParsePathsArray(object.second); + if (!paths_or.ok()) return paths_or.status(); + if (!paths_or.value().permissions.empty()) { + request.push_back( + std::make_unique(std::move(paths_or.value()))); + } + } else if (object.first == "headers") { + if (object.second.type() != Json::Type::ARRAY) { + return absl::InvalidArgumentError("\"headers\" is not an array."); + } + auto headers_or = ParseHeadersArray(object.second); + if (!headers_or.ok()) return headers_or.status(); + if (!headers_or.value().permissions.empty()) { + request.push_back( + std::make_unique(std::move(headers_or.value()))); + } + } else { + return absl::InvalidArgumentError(absl::StrFormat( + "policy contains unknown field \"%s\" in \"request\".", + object.first)); } } if (request.empty()) { @@ -250,36 +265,53 @@ absl::StatusOr ParseRequest(const Json& json) { return Rbac::Permission::MakeAndPermission(std::move(request)); } -absl::StatusOr ParseRules(const Json& json) { - Rbac::Principal principals; - auto it = json.object_value().find("source"); - if (it != json.object_value().end()) { - if (it->second.type() != Json::Type::OBJECT) { - return absl::InvalidArgumentError("\"source\" is not an object."); +absl::StatusOr ParseRule(const Json& json, + std::string* policy_name) { + absl::optional principals; + absl::optional permissions; + for (const auto& object : json.object_value()) { + if (object.first == "name") { + if (object.second.type() != Json::Type::STRING) { + return absl::InvalidArgumentError( + absl::StrCat("\"name\" is not a string.")); + } + *policy_name = object.second.string_value(); + } else if (object.first == "source") { + if (object.second.type() != Json::Type::OBJECT) { + return absl::InvalidArgumentError("\"source\" is not an object."); + } + auto peer_or = ParsePeer(object.second); + if (!peer_or.ok()) return peer_or.status(); + principals = std::move(*peer_or); + } else if (object.first == "request") { + if (object.second.type() != Json::Type::OBJECT) { + return absl::InvalidArgumentError("\"request\" is not an object."); + } + auto request_or = ParseRequest(object.second); + if (!request_or.ok()) return request_or.status(); + permissions = std::move(*request_or); + } else { + return absl::InvalidArgumentError(absl::StrFormat( + "policy contains unknown field \"%s\" in \"rule\".", object.first)); } - auto peer_or = ParsePeer(it->second); - if (!peer_or.ok()) return peer_or.status(); - principals = std::move(peer_or.value()); - } else { + } + if (policy_name->empty()) { + return absl::InvalidArgumentError(absl::StrCat("\"name\" is not present.")); + } + if (!principals.has_value()) { principals = Rbac::Principal::MakeAnyPrincipal(); } - Rbac::Permission permissions; - it = json.object_value().find("request"); - if (it != json.object_value().end()) { - if (it->second.type() != Json::Type::OBJECT) { - return absl::InvalidArgumentError("\"request\" is not an object."); - } - auto request_or = ParseRequest(it->second); - if (!request_or.ok()) return request_or.status(); - permissions = std::move(request_or.value()); - } else { + if (!permissions.has_value()) { permissions = Rbac::Permission::MakeAnyPermission(); } - return Rbac::Policy(std::move(permissions), std::move(principals)); + return Rbac::Policy(std::move(*permissions), std::move(*principals)); } absl::StatusOr> ParseRulesArray( const Json& json, absl::string_view name) { + if (json.array_value().empty()) { + return absl::InvalidArgumentError("rules is empty."); + } std::map policies; for (size_t i = 0; i < json.array_value().size(); ++i) { const Json& child = json.array_value().at(i); @@ -287,24 +319,15 @@ absl::StatusOr> ParseRulesArray( return absl::InvalidArgumentError( absl::StrCat("rules ", i, ": is not an object.")); } - auto it = child.object_value().find("name"); - if (it == child.object_value().end()) { - return absl::InvalidArgumentError( - absl::StrCat("rules ", i, ": \"name\" is not present.")); - } - if (it->second.type() != Json::Type::STRING) { - return absl::InvalidArgumentError( - absl::StrCat("rules ", i, ": \"name\" is not a string.")); - } - std::string policy_name = - std::string(name) + "_" + it->second.string_value(); - auto policy_or = ParseRules(child); + std::string policy_name; + auto policy_or = ParseRule(child, &policy_name); if (!policy_or.ok()) { return absl::Status( policy_or.status().code(), absl::StrCat("rules ", i, ": ", policy_or.status().message())); } - policies[policy_name] = std::move(policy_or.value()); + policies[std::string(name) + "_" + policy_name] = + std::move(policy_or.value()); } return std::move(policies); } @@ -337,45 +360,51 @@ absl::StatusOr GenerateRbacPolicies( return absl::InvalidArgumentError( "SDK authorization policy is not an object."); } - auto it = json->mutable_object()->find("name"); - if (it == json->mutable_object()->end()) { + auto it = json->object_value().find("name"); + if (it == json->object_value().end()) { return absl::InvalidArgumentError("\"name\" field is not present."); } if (it->second.type() != Json::Type::STRING) { return absl::InvalidArgumentError("\"name\" is not a string."); } absl::string_view name = it->second.string_value(); - RbacPolicies rbac_policies; - it = json->mutable_object()->find("deny_rules"); - if (it != json->mutable_object()->end()) { - if (it->second.type() != Json::Type::ARRAY) { - return absl::InvalidArgumentError("\"deny_rules\" is not an array."); - } - auto deny_policy_or = ParseDenyRulesArray(it->second, name); - if (!deny_policy_or.ok()) { - return absl::Status( - deny_policy_or.status().code(), - absl::StrCat("deny_", deny_policy_or.status().message())); + RbacPolicies rbacs; + bool has_allow_rbac = false; + for (const auto& object : json->object_value()) { + if (object.first == "name") { + continue; + } else if (object.first == "deny_rules") { + if (object.second.type() != Json::Type::ARRAY) { + return absl::InvalidArgumentError("\"deny_rules\" is not an array."); + } + auto deny_policy_or = ParseDenyRulesArray(object.second, name); + if (!deny_policy_or.ok()) { + return absl::Status( + deny_policy_or.status().code(), + absl::StrCat("deny_", deny_policy_or.status().message())); + } + rbacs.deny_policy = std::move(*deny_policy_or); + } else if (object.first == "allow_rules") { + if (object.second.type() != Json::Type::ARRAY) { + return absl::InvalidArgumentError("\"allow_rules\" is not an array."); + } + auto allow_policy_or = ParseAllowRulesArray(object.second, name); + if (!allow_policy_or.ok()) { + return absl::Status( + allow_policy_or.status().code(), + absl::StrCat("allow_", allow_policy_or.status().message())); + } + rbacs.allow_policy = std::move(*allow_policy_or); + has_allow_rbac = true; + } else { + return absl::InvalidArgumentError(absl::StrFormat( + "policy contains unknown field \"%s\".", object.first)); } - rbac_policies.deny_policy = std::move(deny_policy_or.value()); - } else { - rbac_policies.deny_policy.action = Rbac::Action::kDeny; } - it = json->mutable_object()->find("allow_rules"); - if (it == json->mutable_object()->end()) { + if (!has_allow_rbac) { return absl::InvalidArgumentError("\"allow_rules\" is not present."); } - if (it->second.type() != Json::Type::ARRAY) { - return absl::InvalidArgumentError("\"allow_rules\" is not an array."); - } - auto allow_policy_or = ParseAllowRulesArray(it->second, name); - if (!allow_policy_or.ok()) { - return absl::Status( - allow_policy_or.status().code(), - absl::StrCat("allow_", allow_policy_or.status().message())); - } - rbac_policies.allow_policy = std::move(allow_policy_or.value()); - return std::move(rbac_policies); + return std::move(rbacs); } } // namespace grpc_core diff --git a/src/core/lib/security/authorization/rbac_translator.h b/src/core/lib/security/authorization/rbac_translator.h index 28803becf26..fbe7095198e 100644 --- a/src/core/lib/security/authorization/rbac_translator.h +++ b/src/core/lib/security/authorization/rbac_translator.h @@ -19,18 +19,22 @@ #include "absl/status/statusor.h" #include "absl/strings/string_view.h" +#include "absl/types/optional.h" #include "src/core/lib/security/authorization/rbac_policy.h" namespace grpc_core { struct RbacPolicies { - Rbac deny_policy; + absl::optional deny_policy; Rbac allow_policy; }; -// Translates SDK authorization policy to Envoy RBAC policies. Returns error on -// failure. +// Translates SDK authorization policy to Envoy RBAC policies. On success, will +// return one of the following - +// 1. One allow RBAC policy or, +// 2. Two RBAC policies: one deny policy and one allow policy. +// Returns error on failure. // authz_policy: Authorization Policy string in JSON format. absl::StatusOr GenerateRbacPolicies( absl::string_view authz_policy); diff --git a/test/core/security/grpc_authorization_policy_provider_test.cc b/test/core/security/grpc_authorization_policy_provider_test.cc index fa59fc18734..af235590f2e 100644 --- a/test/core/security/grpc_authorization_policy_provider_test.cc +++ b/test/core/security/grpc_authorization_policy_provider_test.cc @@ -131,11 +131,7 @@ TEST(AuthorizationPolicyProviderTest, FileWatcherSuccessValidPolicyRefresh) { EXPECT_EQ(allow_engine->num_policies(), 2); deny_engine = dynamic_cast(engines.deny_engine.get()); - ASSERT_NE(deny_engine, nullptr); - EXPECT_EQ(deny_engine->action(), Rbac::Action::kDeny); - EXPECT_EQ(deny_engine->num_policies(), 0); - dynamic_cast(provider->get()) - ->SetCallbackForTesting(nullptr); + EXPECT_EQ(deny_engine, nullptr); } TEST(AuthorizationPolicyProviderTest, @@ -260,11 +256,7 @@ TEST(AuthorizationPolicyProviderTest, FileWatcherRecoversFromFailure) { EXPECT_EQ(allow_engine->num_policies(), 2); deny_engine = dynamic_cast(engines.deny_engine.get()); - ASSERT_NE(deny_engine, nullptr); - EXPECT_EQ(deny_engine->action(), Rbac::Action::kDeny); - EXPECT_EQ(deny_engine->num_policies(), 0); - dynamic_cast(provider->get()) - ->SetCallbackForTesting(nullptr); + EXPECT_EQ(deny_engine, nullptr); } } // namespace grpc_core diff --git a/test/core/security/rbac_translator_test.cc b/test/core/security/rbac_translator_test.cc index 07f4abf64a6..a06e6184bbf 100644 --- a/test/core/security/rbac_translator_test.cc +++ b/test/core/security/rbac_translator_test.cc @@ -17,6 +17,8 @@ #include #include +#include "test/core/util/test_config.h" + namespace grpc_core { namespace { @@ -105,8 +107,7 @@ TEST(GenerateRbacPoliciesTest, MissingDenyRules) { "}"; auto rbac_policies = GenerateRbacPolicies(authz_policy); ASSERT_TRUE(rbac_policies.ok()); - EXPECT_EQ(rbac_policies.value().deny_policy.action, Rbac::Action::kDeny); - EXPECT_TRUE(rbac_policies.value().deny_policy.policies.empty()); + EXPECT_FALSE(rbac_policies->deny_policy.has_value()); } TEST(GenerateRbacPoliciesTest, IncorrectAllowRulesType) { @@ -145,6 +146,17 @@ TEST(GenerateRbacPoliciesTest, IncorrectRuleType) { "allow_rules 0: is not an object."); } +TEST(GenerateRbacPoliciesTest, EmptyRuleArray) { + const char* authz_policy = + "{" + " \"name\": \"authz\"," + " \"allow_rules\": []" + "}"; + auto rbac_policies = GenerateRbacPolicies(authz_policy); + EXPECT_EQ(rbac_policies.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(rbac_policies.status().message(), "allow_rules is empty."); +} + TEST(GenerateRbacPoliciesTest, MissingRuleNameField) { const char* authz_policy = "{" @@ -183,11 +195,11 @@ TEST(GenerateRbacPoliciesTest, MissingSourceAndRequest) { " }" " ]" "}"; - auto rbac_policies = GenerateRbacPolicies(authz_policy); - ASSERT_TRUE(rbac_policies.ok()); - EXPECT_EQ(rbac_policies.value().allow_policy.action, Rbac::Action::kAllow); + auto rbacs = GenerateRbacPolicies(authz_policy); + ASSERT_TRUE(rbacs.ok()); + EXPECT_EQ(rbacs->allow_policy.action, Rbac::Action::kAllow); EXPECT_THAT( - rbac_policies.value().allow_policy.policies, + rbacs->allow_policy.policies, ::testing::ElementsAre(::testing::Pair( "authz_allow_policy", ::testing::AllOf( @@ -213,11 +225,11 @@ TEST(GenerateRbacPoliciesTest, EmptySourceAndRequest) { " }" " ]" "}"; - auto rbac_policies = GenerateRbacPolicies(authz_policy); - ASSERT_TRUE(rbac_policies.ok()); - EXPECT_EQ(rbac_policies.value().allow_policy.action, Rbac::Action::kAllow); + auto rbacs = GenerateRbacPolicies(authz_policy); + ASSERT_TRUE(rbacs.ok()); + EXPECT_EQ(rbacs->allow_policy.action, Rbac::Action::kAllow); EXPECT_THAT( - rbac_policies.value().allow_policy.policies, + rbacs->allow_policy.policies, ::testing::ElementsAre(::testing::Pair( "authz_allow_policy", ::testing::AllOf( @@ -298,10 +310,10 @@ TEST(GenerateRbacPoliciesTest, ParseSourceSuccess) { " }" " ]" "}"; - auto rbac_policies = GenerateRbacPolicies(authz_policy); - ASSERT_TRUE(rbac_policies.ok()); - EXPECT_EQ(rbac_policies.value().allow_policy.action, Rbac::Action::kAllow); - EXPECT_THAT(rbac_policies.value().allow_policy.policies, + auto rbacs = GenerateRbacPolicies(authz_policy); + ASSERT_TRUE(rbacs.ok()); + EXPECT_EQ(rbacs->allow_policy.action, Rbac::Action::kAllow); + EXPECT_THAT(rbacs->allow_policy.policies, ::testing::ElementsAre(::testing::Pair( "authz_allow_policy", ::testing::AllOf( @@ -336,9 +348,10 @@ TEST(GenerateRbacPoliciesTest, ParseSourceSuccess) { StringMatcher::Type::kExact, "spiffe://abc.*.com", false))))))))))))); - EXPECT_EQ(rbac_policies.value().deny_policy.action, Rbac::Action::kDeny); + ASSERT_TRUE(rbacs->deny_policy.has_value()); + EXPECT_EQ(rbacs->deny_policy->action, Rbac::Action::kDeny); EXPECT_THAT( - rbac_policies.value().deny_policy.policies, + rbacs->deny_policy->policies, ::testing::ElementsAre(::testing::Pair( "authz_deny_policy", ::testing::AllOf( @@ -430,11 +443,12 @@ TEST(GenerateRbacPoliciesTest, ParseRequestPathsSuccess) { " }" " ]" "}"; - auto rbac_policies = GenerateRbacPolicies(authz_policy); - ASSERT_TRUE(rbac_policies.ok()); - EXPECT_EQ(rbac_policies.value().deny_policy.action, Rbac::Action::kDeny); + auto rbacs = GenerateRbacPolicies(authz_policy); + ASSERT_TRUE(rbacs.ok()); + ASSERT_TRUE(rbacs->deny_policy.has_value()); + EXPECT_EQ(rbacs->deny_policy->action, Rbac::Action::kDeny); EXPECT_THAT( - rbac_policies.value().deny_policy.policies, + rbacs->deny_policy->policies, ::testing::ElementsAre(::testing::Pair( "authz_deny_policy", ::testing::AllOf( @@ -462,9 +476,9 @@ TEST(GenerateRbacPoliciesTest, ParseRequestPathsSuccess) { "path-bar", false), EqualsPath(StringMatcher::Type::kSuffix, "baz", false))))))))))))); - EXPECT_EQ(rbac_policies.value().allow_policy.action, Rbac::Action::kAllow); + EXPECT_EQ(rbacs->allow_policy.action, Rbac::Action::kAllow); EXPECT_THAT( - rbac_policies.value().allow_policy.policies, + rbacs->allow_policy.policies, ::testing::ElementsAre(::testing::Pair( "authz_allow_policy", ::testing::AllOf( @@ -511,6 +525,98 @@ TEST(GenerateRbacPoliciesTest, IncorrectHeaderType) { "deny_rules 0: \"headers\" 0: is not an object."); } +TEST(GenerateRbacPoliciesTest, MissingHeaderKey) { + const char* authz_policy = + "{" + " \"name\": \"authz\"," + " \"allow_rules\": [" + " {" + " \"name\": \"policy\"," + " \"request\": {" + " \"headers\": [" + " {}" + " ]" + " }" + " }" + " ]" + "}"; + auto rbac_policies = GenerateRbacPolicies(authz_policy); + EXPECT_EQ(rbac_policies.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(rbac_policies.status().message(), + "allow_rules 0: \"headers\" 0: \"key\" is not present."); +} + +TEST(GenerateRbacPoliciesTest, MissingHeaderValues) { + const char* authz_policy = + "{" + " \"name\": \"authz\"," + " \"allow_rules\": [" + " {" + " \"name\": \"policy\"," + " \"request\": {" + " \"headers\": [" + " {" + " \"key\": \"key-abc\"" + " }" + " ]" + " }" + " }" + " ]" + "}"; + auto rbac_policies = GenerateRbacPolicies(authz_policy); + EXPECT_EQ(rbac_policies.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(rbac_policies.status().message(), + "allow_rules 0: \"headers\" 0: \"values\" is not present."); +} + +TEST(GenerateRbacPoliciesTest, IncorrectHeaderKeyType) { + const char* authz_policy = + "{" + " \"name\": \"authz\"," + " \"allow_rules\": [" + " {" + " \"name\": \"policy\"," + " \"request\": {" + " \"headers\": [" + " {" + " \"key\": 123," + " \"values\": [\"value-a\"]" + " }" + " ]" + " }" + " }" + " ]" + "}"; + auto rbac_policies = GenerateRbacPolicies(authz_policy); + EXPECT_EQ(rbac_policies.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(rbac_policies.status().message(), + "allow_rules 0: \"headers\" 0: \"key\" is not a string."); +} + +TEST(GenerateRbacPoliciesTest, IncorrectHeaderValuesType) { + const char* authz_policy = + "{" + " \"name\": \"authz\"," + " \"allow_rules\": [" + " {" + " \"name\": \"policy\"," + " \"request\": {" + " \"headers\": [" + " {" + " \"key\": \"key-abc\"," + " \"values\": {}" + " }" + " ]" + " }" + " }" + " ]" + "}"; + auto rbac_policies = GenerateRbacPolicies(authz_policy); + EXPECT_EQ(rbac_policies.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(rbac_policies.status().message(), + "allow_rules 0: \"headers\" 0: \"values\" is not an array."); +} + TEST(GenerateRbacPoliciesTest, UnsupportedGrpcHeaders) { const char* authz_policy = "{" @@ -642,13 +748,11 @@ TEST(GenerateRbacPoliciesTest, ParseRequestHeadersSuccess) { " }" " ]" "}"; - auto rbac_policies = GenerateRbacPolicies(authz_policy); - ASSERT_TRUE(rbac_policies.ok()); - EXPECT_EQ(rbac_policies.value().deny_policy.action, Rbac::Action::kDeny); - EXPECT_TRUE(rbac_policies.value().deny_policy.policies.empty()); - EXPECT_EQ(rbac_policies.value().allow_policy.action, Rbac::Action::kAllow); + auto rbacs = GenerateRbacPolicies(authz_policy); + ASSERT_TRUE(rbacs.ok()); + EXPECT_EQ(rbacs->allow_policy.action, Rbac::Action::kAllow); EXPECT_THAT( - rbac_policies.value().allow_policy.policies, + rbacs->allow_policy.policies, ::testing::ElementsAre(::testing::Pair( "authz_allow_policy", ::testing::AllOf( @@ -729,13 +833,11 @@ TEST(GenerateRbacPoliciesTest, ParseRulesArraySuccess) { " }" " ]" "}"; - auto rbac_policies = GenerateRbacPolicies(authz_policy); - ASSERT_TRUE(rbac_policies.ok()); - EXPECT_EQ(rbac_policies.value().deny_policy.action, Rbac::Action::kDeny); - EXPECT_TRUE(rbac_policies.value().deny_policy.policies.empty()); - EXPECT_EQ(rbac_policies.value().allow_policy.action, Rbac::Action::kAllow); + auto rbacs = GenerateRbacPolicies(authz_policy); + ASSERT_TRUE(rbacs.ok()); + EXPECT_EQ(rbacs->allow_policy.action, Rbac::Action::kAllow); EXPECT_THAT( - rbac_policies.value().allow_policy.policies, + rbacs->allow_policy.policies, ::testing::ElementsAre( ::testing::Pair( "authz_allow_policy_1", @@ -787,6 +889,101 @@ TEST(GenerateRbacPoliciesTest, ParseRulesArraySuccess) { Rbac::Principal::RuleType::kAny)))))); } +TEST(GenerateRbacPoliciesTest, UnknownFieldInTopLayer) { + const char* authz_policy = + "{" + " \"name\": \"authz\"," + " \"allow_rules\": [" + " {" + " \"name\": \"allow_policy\"" + " }" + " ]," + " \"foo\": \"123\"" + "}"; + auto rbac_policies = GenerateRbacPolicies(authz_policy); + EXPECT_EQ(rbac_policies.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(rbac_policies.status().message(), + "policy contains unknown field \"foo\"."); +} + +TEST(GenerateRbacPoliciesTest, UnknownFieldInRule) { + const char* authz_policy = + "{" + " \"name\": \"authz\"," + " \"allow_rules\": [" + " {" + " \"name\": \"allow_policy\"," + " \"foo\": {}" + " }" + " ]" + "}"; + auto rbac_policies = GenerateRbacPolicies(authz_policy); + EXPECT_EQ(rbac_policies.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT( + rbac_policies.status().message(), + "allow_rules 0: policy contains unknown field \"foo\" in \"rule\"."); +} + +TEST(GenerateRbacPoliciesTest, UnknownFieldInSource) { + const char* authz_policy = + "{" + " \"name\": \"authz\"," + " \"allow_rules\": [" + " {" + " \"name\": \"allow_policy\"," + " \"source\": " + " {" + " \"principals\": [\"spiffe://foo.abc\"]," + " \"foo\": {} " + " }" + " }" + " ]" + "}"; + auto rbac_policies = GenerateRbacPolicies(authz_policy); + EXPECT_EQ(rbac_policies.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT( + rbac_policies.status().message(), + "allow_rules 0: policy contains unknown field \"foo\" in \"source\"."); +} + +TEST(GenerateRbacPoliciesTest, UnknownFieldInRequest) { + const char* authz_policy = + "{" + " \"name\": \"authz\"," + " \"allow_rules\": [" + " {" + " \"name\": \"allow_policy\"," + " \"request\": { \"foo\": {}}" + " }" + " ]" + "}"; + auto rbac_policies = GenerateRbacPolicies(authz_policy); + EXPECT_EQ(rbac_policies.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT( + rbac_policies.status().message(), + "allow_rules 0: policy contains unknown field \"foo\" in \"request\"."); +} + +TEST(GenerateRbacPoliciesTest, UnknownFieldInHeaders) { + const char* authz_policy = + "{" + " \"name\": \"authz\"," + " \"allow_rules\": [" + " {" + " \"name\": \"allow_policy\"," + " \"request\": {" + " \"headers\": [{ \"foo\": {}}]" + " }" + " }" + " ]" + "}"; + auto rbac_policies = GenerateRbacPolicies(authz_policy); + EXPECT_EQ(rbac_policies.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(rbac_policies.status().message(), + "allow_rules 0: \"headers\" 0: policy contains unknown field " + "\"foo\"."); +} + } // namespace grpc_core int main(int argc, char** argv) {