Fix presence match in authorization. (#28269)

* Fix presence match in authorization.

* Remove header

* Add test

* fix regex to include whitespace characters
pull/28347/head
Ashitha Santhosh 3 years ago committed by GitHub
parent 0deb64d1f6
commit 4fd524cd17
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 5
      src/core/lib/security/authorization/rbac_translator.cc
  2. 8
      test/core/security/matchers_test.cc
  3. 128
      test/core/security/rbac_translator_test.cc
  4. 2
      test/cpp/end2end/sdk_authz_end2end_test.cc

@ -30,8 +30,9 @@ namespace {
absl::string_view GetMatcherType(absl::string_view value,
StringMatcher::Type* type) {
if (value == "*") {
*type = StringMatcher::Type::kPrefix;
return "";
*type = StringMatcher::Type::kSafeRegex;
// Presence match checks for non empty strings.
return ".+";
} else if (absl::StartsWith(value, "*")) {
*type = StringMatcher::Type::kSuffix;
return absl::StripPrefix(value, "*");

@ -99,6 +99,14 @@ TEST(StringMatcherTest, SafeRegexMatchCaseSensitive) {
EXPECT_FALSE(string_matcher->Match("test-regex"));
}
TEST(StringMatcherTest, PresenceMatchUsingSafeRegex) {
auto string_matcher = StringMatcher::Create(StringMatcher::Type::kSafeRegex,
/*matcher=*/".+");
ASSERT_TRUE(string_matcher.ok());
EXPECT_TRUE(string_matcher->Match("any-value"));
EXPECT_FALSE(string_matcher->Match(""));
}
TEST(StringMatcherTest, ContainsMatchCaseSensitive) {
auto string_matcher = StringMatcher::Create(StringMatcher::Type::kContains,
/*matcher=*/"contains",

@ -21,25 +21,32 @@ namespace grpc_core {
namespace {
MATCHER_P2(EqualsPrincipalName, expected_matcher_type, expected_matcher_value,
"") {
MATCHER_P3(EqualsPrincipalName, expected_matcher_type, expected_matcher_value,
is_regex, "") {
return arg->type == Rbac::Principal::RuleType::kPrincipalName &&
arg->string_matcher.type() == expected_matcher_type &&
arg->string_matcher.string_matcher() == expected_matcher_value;
arg->string_matcher.type() == expected_matcher_type && is_regex
? arg->string_matcher.regex_matcher()->pattern() ==
expected_matcher_value
: arg->string_matcher.string_matcher() == expected_matcher_value;
}
MATCHER_P2(EqualsPath, expected_matcher_type, expected_matcher_value, "") {
MATCHER_P3(EqualsPath, expected_matcher_type, expected_matcher_value, is_regex,
"") {
return arg->type == Rbac::Permission::RuleType::kPath &&
arg->string_matcher.type() == expected_matcher_type &&
arg->string_matcher.string_matcher() == expected_matcher_value;
arg->string_matcher.type() == expected_matcher_type && is_regex
? arg->string_matcher.regex_matcher()->pattern() ==
expected_matcher_value
: arg->string_matcher.string_matcher() == expected_matcher_value;
}
MATCHER_P3(EqualsHeader, expected_name, expected_matcher_type,
expected_matcher_value, "") {
MATCHER_P4(EqualsHeader, expected_name, expected_matcher_type,
expected_matcher_value, is_regex, "") {
return arg->type == Rbac::Permission::RuleType::kHeader &&
arg->header_matcher.name() == expected_name &&
arg->header_matcher.type() == expected_matcher_type &&
arg->header_matcher.string_matcher() == expected_matcher_value;
arg->header_matcher.name() == expected_name &&
arg->header_matcher.type() == expected_matcher_type && is_regex
? arg->header_matcher.regex_matcher()->pattern() ==
expected_matcher_value
: arg->header_matcher.string_matcher() == expected_matcher_value;
}
} // namespace
@ -292,40 +299,41 @@ 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,
::testing::ElementsAre(::testing::Pair(
"authz_allow_policy",
::testing::AllOf(
::testing::Field(
&Rbac::Policy::permissions,
::testing::Field(&Rbac::Permission::type,
Rbac::Permission::RuleType::kAny)),
::testing::Field(
&Rbac::Policy::principals,
EXPECT_THAT(rbac_policies.value().allow_policy.policies,
::testing::ElementsAre(::testing::Pair(
"authz_allow_policy",
::testing::AllOf(
::testing::Field(&Rbac::Principal::type,
Rbac::Principal::RuleType::kAnd),
::testing::Field(
&Rbac::Principal::principals,
::testing::ElementsAre(::testing::AllOf(
::testing::Pointee(::testing::Field(
&Rbac::Principal::type,
Rbac::Principal::RuleType::kOr)),
::testing::Pointee(::testing::Field(
&Rbac::Policy::permissions,
::testing::Field(&Rbac::Permission::type,
Rbac::Permission::RuleType::kAny)),
::testing::Field(
&Rbac::Policy::principals,
::testing::AllOf(
::testing::Field(&Rbac::Principal::type,
Rbac::Principal::RuleType::kAnd),
::testing::Field(
&Rbac::Principal::principals,
::testing::ElementsAre(
EqualsPrincipalName(
StringMatcher::Type::kExact,
"spiffe://foo.abc"),
EqualsPrincipalName(
StringMatcher::Type::kPrefix,
"spiffe://bar"),
EqualsPrincipalName(
StringMatcher::Type::kSuffix, "baz"),
EqualsPrincipalName(
StringMatcher::Type::kExact,
"spiffe://abc.*.com")))))))))))));
::testing::ElementsAre(::testing::AllOf(
::testing::Pointee(::testing::Field(
&Rbac::Principal::type,
Rbac::Principal::RuleType::kOr)),
::testing::Pointee(::testing::Field(
&Rbac::Principal::principals,
::testing::ElementsAre(
EqualsPrincipalName(
StringMatcher::Type::kExact,
"spiffe://foo.abc", false),
EqualsPrincipalName(
StringMatcher::Type::kPrefix,
"spiffe://bar", false),
EqualsPrincipalName(
StringMatcher::Type::kSuffix,
"baz", false),
EqualsPrincipalName(
StringMatcher::Type::kExact,
"spiffe://abc.*.com",
false)))))))))))));
EXPECT_EQ(rbac_policies.value().deny_policy.action, Rbac::Action::kDeny);
EXPECT_THAT(
rbac_policies.value().deny_policy.policies,
@ -350,8 +358,8 @@ TEST(GenerateRbacPoliciesTest, ParseSourceSuccess) {
::testing::Pointee(::testing::Field(
&Rbac::Principal::principals,
::testing::ElementsAre(EqualsPrincipalName(
StringMatcher::Type::kPrefix,
"")))))))))))));
StringMatcher::Type::kSafeRegex, ".+",
true)))))))))))));
}
TEST(GenerateRbacPoliciesTest, IncorrectRequestType) {
@ -447,11 +455,11 @@ TEST(GenerateRbacPoliciesTest, ParseRequestPathsSuccess) {
&Rbac::Permission::permissions,
::testing::ElementsAre(
EqualsPath(StringMatcher::Type::kExact,
"path-foo"),
"path-foo", false),
EqualsPath(StringMatcher::Type::kPrefix,
"path-bar"),
"path-bar", false),
EqualsPath(StringMatcher::Type::kSuffix,
"baz")))))))))))));
"baz", false)))))))))))));
EXPECT_EQ(rbac_policies.value().allow_policy.action, Rbac::Action::kAllow);
EXPECT_THAT(
rbac_policies.value().allow_policy.policies,
@ -475,9 +483,9 @@ TEST(GenerateRbacPoliciesTest, ParseRequestPathsSuccess) {
Rbac::Permission::RuleType::kOr)),
::testing::Pointee(::testing::Field(
&Rbac::Permission::permissions,
::testing::ElementsAre(
EqualsPath(StringMatcher::Type::kPrefix,
"")))))))))))));
::testing::ElementsAre(EqualsPath(
StringMatcher::Type::kSafeRegex, ".+",
true)))))))))))));
}
TEST(GenerateRbacPoliciesTest, IncorrectHeaderType) {
@ -670,8 +678,8 @@ TEST(GenerateRbacPoliciesTest, ParseRequestHeadersSuccess) {
EqualsHeader(
"key-1",
HeaderMatcher::Type::
kPrefix,
""))))),
kSafeRegex,
".+", true))))),
::testing::AllOf(
::testing::Pointee(::testing::Field(
&Rbac::Permission::type,
@ -682,17 +690,18 @@ TEST(GenerateRbacPoliciesTest, ParseRequestHeadersSuccess) {
EqualsHeader("key-2",
HeaderMatcher::
Type::kExact,
"foo"),
"foo", false),
EqualsHeader(
"key-2",
HeaderMatcher::Type::
kPrefix,
"bar"),
"bar", false),
EqualsHeader(
"key-2",
HeaderMatcher::Type::
kSuffix,
"baz")))))))))))))))));
"baz",
false)))))))))))))))));
}
TEST(GenerateRbacPoliciesTest, ParseRulesArraySuccess) {
@ -743,8 +752,8 @@ TEST(GenerateRbacPoliciesTest, ParseRulesArraySuccess) {
::testing::Pointee(::testing::Field(
&Rbac::Permission::permissions,
::testing::ElementsAre(EqualsPath(
StringMatcher::Type::kExact,
"foo"))))))))),
StringMatcher::Type::kExact, "foo",
false))))))))),
::testing::Field(
&Rbac::Policy::principals,
::testing::AllOf(
@ -761,7 +770,8 @@ TEST(GenerateRbacPoliciesTest, ParseRulesArraySuccess) {
::testing::ElementsAre(
EqualsPrincipalName(
StringMatcher::Type::kExact,
"spiffe://foo.abc"))))))))))),
"spiffe://foo.abc",
false))))))))))),
::testing::Pair(
"authz_allow_policy_2",
::testing::AllOf(

@ -267,7 +267,7 @@ TEST_F(SdkAuthzEnd2EndTest, StaticInitAllowsRpcRequestEmptyDenyMatchInAllow) {
" \"name\": \"allow_echo\","
" \"request\": {"
" \"paths\": ["
" \"*/Echo\""
" \"*\""
" ],"
" \"headers\": ["
" {"

Loading…
Cancel
Save