acelyc111 commented on code in PR #1518:
URL:
https://github.com/apache/incubator-pegasus/pull/1518#discussion_r1229132213
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -27,45 +28,178 @@ bool policy_item::match(const access_type &ac_type, const
std::string &user_name
return static_cast<bool>(access_types & ac_type) && users.count(user_name)
!= 0;
}
-bool acl_policies::allowed(const access_type &ac_type, const std::string
&user_name) const
+policy_check_status acl_policies::policies_check(const access_type &ac_type,
+ const std::string &user_name,
+ const policy_check_type
&check_type) const
{
- // 1. Check if it is not allowed.
- for (const auto &deny_policy : deny_policies) {
- // 1.1. In 'deny_policies'.
- if (!deny_policy.match(ac_type, user_name)) {
+ if (check_type == policy_check_type::kAllow) {
+ return do_policies_check(
+ check_type, ac_type, user_name, allow_policies,
allow_policies_exclude);
+ }
+ CHECK(check_type == policy_check_type::kDeny, "");
+ return do_policies_check(check_type, ac_type, user_name, deny_policies,
deny_policies_exclude);
+}
+
+policy_check_status
+acl_policies::do_policies_check(const policy_check_type &check_type,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const std::vector<policy_item> &policies,
+ const std::vector<policy_item>
&exclude_policies) const
+{
+ for (const auto &policy : policies) {
+ // 1. Doesn't match an allow_policies or a deny_policies.
+ if (!policy.match(ac_type, user_name)) {
continue;
}
- bool in_deny_policies_exclude = false;
- for (const auto &deny_policy_exclude : deny_policies_exclude) {
- if (deny_policy_exclude.match(ac_type, user_name)) {
- in_deny_policies_exclude = true;
- break;
+ // 2. Matches a policy.
+ for (const auto &exclude_policy : exclude_policies) {
+ if (exclude_policy.match(ac_type, user_name)) {
+ // 2.1. Matches an allow/deny_policies_exclude.
+ return policy_check_status::kPending;
}
}
- // 1.2. Not in any 'deny_policies_exclude', it's not allowed.
- if (!in_deny_policies_exclude) {
- return false;
+ // 2.2. Doesn't match any allow/deny_exclude_policies.
+ if (check_type == policy_check_type::kAllow) {
+ return policy_check_status::kAllowed;
+ } else {
+ return policy_check_status::kDenied;
}
}
+ // 3. Doesn't match any policy.
+ return policy_check_status::kNotMatched;
+}
- // 2. Check if it is allowed.
- for (const auto &allow_policy : allow_policies) {
- // 2.1. In 'allow_policies'.
- if (!allow_policy.match(ac_type, user_name)) {
- continue;
- }
- for (const auto &allow_policy_exclude : allow_policies_exclude) {
- // 2.2. In some 'allow_policies_exclude', it's not allowed.
- if (allow_policy_exclude.match(ac_type, user_name)) {
- return false;
+access_control_result
+check_ranger_resource_policy_allowed(const std::vector<ranger_resource_policy>
&policies,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const match_database_type &md_type,
+ const std::string &database_name,
+ const std::string &default_database_name)
+{
+ // Check if it is denied by any policy in current resource.
+ auto check_res = do_check_ranger_resource_policy(policies,
+ ac_type,
+ user_name,
+ md_type,
+ database_name,
+ default_database_name,
+ policy_check_type::kDeny);
+ if (access_control_result::kDenied == check_res) {
+ return access_control_result::kDenied;
+ }
+
+ // Check if it is allowed by any policy in current resource.
+ check_res = do_check_ranger_resource_policy(policies,
+ ac_type,
+ user_name,
+ md_type,
+ database_name,
+ default_database_name,
+ policy_check_type::kAllow);
+ if (access_control_result::kAllowed == check_res) {
+ return access_control_result::kAllowed;
+ }
+
+ // The check that does not match any policy in current reosource returns
false.
+ return access_control_result::kDenied;
+}
+
+access_control_result
+do_check_ranger_resource_policy(const std::vector<ranger_resource_policy>
&policies,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const match_database_type &md_type,
+ const std::string &database_name,
+ const std::string &default_database_name,
+ const policy_check_type &check_type)
+{
+ for (const auto &policy : policies) {
+ if (match_database_type::kNeed == md_type) {
+ // Lagacy table not match any database.
+ if (database_name.empty() && policy.database_names.count("*") == 0
&&
+ policy.database_names.count(default_database_name) == 0) {
+ continue;
}
+ // New table not match any database.
+ if (!database_name.empty() && policy.database_names.count("*") ==
0 &&
+ policy.database_names.count(database_name) == 0) {
+ continue;
+ }
+ }
+ auto check_status = policy.policies.policies_check(ac_type, user_name,
check_type);
+ // When policy_check_type is 'kDeny' and in a 'deny_policies' and not
in any
+ // 'deny_policies_exclude'.
+ if (policy_check_type::kDeny == check_type &&
+ policy_check_status::kDenied == check_status) {
Review Comment:
If check_status equals to kDenied, check_type must be kDeny, right? If it
is, refact the code like:
```
if (policy_check_status::kDenied == check_status) {
CHECK_TRUE(policy_check_type::kDeny == check_type);
return access_control_result::kDenied;
}
```
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -27,45 +28,178 @@ bool policy_item::match(const access_type &ac_type, const
std::string &user_name
return static_cast<bool>(access_types & ac_type) && users.count(user_name)
!= 0;
}
-bool acl_policies::allowed(const access_type &ac_type, const std::string
&user_name) const
+policy_check_status acl_policies::policies_check(const access_type &ac_type,
+ const std::string &user_name,
+ const policy_check_type
&check_type) const
{
- // 1. Check if it is not allowed.
- for (const auto &deny_policy : deny_policies) {
- // 1.1. In 'deny_policies'.
- if (!deny_policy.match(ac_type, user_name)) {
+ if (check_type == policy_check_type::kAllow) {
+ return do_policies_check(
+ check_type, ac_type, user_name, allow_policies,
allow_policies_exclude);
+ }
+ CHECK(check_type == policy_check_type::kDeny, "");
+ return do_policies_check(check_type, ac_type, user_name, deny_policies,
deny_policies_exclude);
+}
+
+policy_check_status
+acl_policies::do_policies_check(const policy_check_type &check_type,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const std::vector<policy_item> &policies,
+ const std::vector<policy_item>
&exclude_policies) const
+{
+ for (const auto &policy : policies) {
+ // 1. Doesn't match an allow_policies or a deny_policies.
+ if (!policy.match(ac_type, user_name)) {
continue;
}
- bool in_deny_policies_exclude = false;
- for (const auto &deny_policy_exclude : deny_policies_exclude) {
- if (deny_policy_exclude.match(ac_type, user_name)) {
- in_deny_policies_exclude = true;
- break;
+ // 2. Matches a policy.
+ for (const auto &exclude_policy : exclude_policies) {
+ if (exclude_policy.match(ac_type, user_name)) {
+ // 2.1. Matches an allow/deny_policies_exclude.
+ return policy_check_status::kPending;
}
}
- // 1.2. Not in any 'deny_policies_exclude', it's not allowed.
- if (!in_deny_policies_exclude) {
- return false;
+ // 2.2. Doesn't match any allow/deny_exclude_policies.
+ if (check_type == policy_check_type::kAllow) {
+ return policy_check_status::kAllowed;
+ } else {
+ return policy_check_status::kDenied;
}
Review Comment:
This function can be templated too, make both policy_check_type and
policy_check_status as template parameters?
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -27,45 +28,178 @@ bool policy_item::match(const access_type &ac_type, const
std::string &user_name
return static_cast<bool>(access_types & ac_type) && users.count(user_name)
!= 0;
}
-bool acl_policies::allowed(const access_type &ac_type, const std::string
&user_name) const
+policy_check_status acl_policies::policies_check(const access_type &ac_type,
Review Comment:
How about using template functions:
```
// in .h file
template <policy_check_type check_type>
policy_check_status policies_check(...);
// in .cpp file
template<>
policy_check_status
acl_policies::policies_check<policy_check_type::kAllow>(...)
{
return do_policies_check(policy_check_type::kAllow, ac_type, user_name,
allow_policies, allow_policies_exclude);
}
template<>
policy_check_status
acl_policies::policies_check<policy_check_type::kDeny>(...)
{
return do_policies_check(policy_check_type::kAllow, ac_type, user_name,
deny_policies, deny_policies_exclude);
}
// calling
auto check_status = policy.policies.policies_check<check_type>(ac_type,
user_name);
```
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -27,45 +28,178 @@ bool policy_item::match(const access_type &ac_type, const
std::string &user_name
return static_cast<bool>(access_types & ac_type) && users.count(user_name)
!= 0;
}
-bool acl_policies::allowed(const access_type &ac_type, const std::string
&user_name) const
+policy_check_status acl_policies::policies_check(const access_type &ac_type,
+ const std::string &user_name,
+ const policy_check_type
&check_type) const
{
- // 1. Check if it is not allowed.
- for (const auto &deny_policy : deny_policies) {
- // 1.1. In 'deny_policies'.
- if (!deny_policy.match(ac_type, user_name)) {
+ if (check_type == policy_check_type::kAllow) {
+ return do_policies_check(
+ check_type, ac_type, user_name, allow_policies,
allow_policies_exclude);
+ }
+ CHECK(check_type == policy_check_type::kDeny, "");
+ return do_policies_check(check_type, ac_type, user_name, deny_policies,
deny_policies_exclude);
+}
+
+policy_check_status
+acl_policies::do_policies_check(const policy_check_type &check_type,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const std::vector<policy_item> &policies,
+ const std::vector<policy_item>
&exclude_policies) const
+{
+ for (const auto &policy : policies) {
+ // 1. Doesn't match an allow_policies or a deny_policies.
+ if (!policy.match(ac_type, user_name)) {
continue;
}
- bool in_deny_policies_exclude = false;
- for (const auto &deny_policy_exclude : deny_policies_exclude) {
- if (deny_policy_exclude.match(ac_type, user_name)) {
- in_deny_policies_exclude = true;
- break;
+ // 2. Matches a policy.
+ for (const auto &exclude_policy : exclude_policies) {
+ if (exclude_policy.match(ac_type, user_name)) {
+ // 2.1. Matches an allow/deny_policies_exclude.
+ return policy_check_status::kPending;
}
}
- // 1.2. Not in any 'deny_policies_exclude', it's not allowed.
- if (!in_deny_policies_exclude) {
- return false;
+ // 2.2. Doesn't match any allow/deny_exclude_policies.
+ if (check_type == policy_check_type::kAllow) {
+ return policy_check_status::kAllowed;
+ } else {
+ return policy_check_status::kDenied;
}
}
+ // 3. Doesn't match any policy.
+ return policy_check_status::kNotMatched;
+}
- // 2. Check if it is allowed.
- for (const auto &allow_policy : allow_policies) {
- // 2.1. In 'allow_policies'.
- if (!allow_policy.match(ac_type, user_name)) {
- continue;
- }
- for (const auto &allow_policy_exclude : allow_policies_exclude) {
- // 2.2. In some 'allow_policies_exclude', it's not allowed.
- if (allow_policy_exclude.match(ac_type, user_name)) {
- return false;
+access_control_result
+check_ranger_resource_policy_allowed(const std::vector<ranger_resource_policy>
&policies,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const match_database_type &md_type,
+ const std::string &database_name,
+ const std::string &default_database_name)
+{
+ // Check if it is denied by any policy in current resource.
+ auto check_res = do_check_ranger_resource_policy(policies,
+ ac_type,
+ user_name,
+ md_type,
+ database_name,
+ default_database_name,
+ policy_check_type::kDeny);
+ if (access_control_result::kDenied == check_res) {
+ return access_control_result::kDenied;
+ }
+
+ // Check if it is allowed by any policy in current resource.
+ check_res = do_check_ranger_resource_policy(policies,
+ ac_type,
+ user_name,
+ md_type,
+ database_name,
+ default_database_name,
+ policy_check_type::kAllow);
+ if (access_control_result::kAllowed == check_res) {
+ return access_control_result::kAllowed;
+ }
+
+ // The check that does not match any policy in current reosource returns
false.
+ return access_control_result::kDenied;
+}
+
+access_control_result
+do_check_ranger_resource_policy(const std::vector<ranger_resource_policy>
&policies,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const match_database_type &md_type,
+ const std::string &database_name,
+ const std::string &default_database_name,
+ const policy_check_type &check_type)
+{
+ for (const auto &policy : policies) {
+ if (match_database_type::kNeed == md_type) {
+ // Lagacy table not match any database.
+ if (database_name.empty() && policy.database_names.count("*") == 0
&&
+ policy.database_names.count(default_database_name) == 0) {
+ continue;
}
+ // New table not match any database.
+ if (!database_name.empty() && policy.database_names.count("*") ==
0 &&
+ policy.database_names.count(database_name) == 0) {
+ continue;
+ }
+ }
+ auto check_status = policy.policies.policies_check(ac_type, user_name,
check_type);
+ // When policy_check_type is 'kDeny' and in a 'deny_policies' and not
in any
+ // 'deny_policies_exclude'.
+ if (policy_check_type::kDeny == check_type &&
+ policy_check_status::kDenied == check_status) {
+ return access_control_result::kDenied;
}
- // 2.3. Not in any 'allow_policies_exclude', it's allowed.
- return true;
+ // When policy_check_type is 'kAllow' and in a 'allow_policies' and
not in any
+ // 'allow_policies_exclude'.
+ if (policy_check_type::kAllow == check_type &&
Review Comment:
Same to above.
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -27,45 +28,178 @@ bool policy_item::match(const access_type &ac_type, const
std::string &user_name
return static_cast<bool>(access_types & ac_type) && users.count(user_name)
!= 0;
}
-bool acl_policies::allowed(const access_type &ac_type, const std::string
&user_name) const
+policy_check_status acl_policies::policies_check(const access_type &ac_type,
+ const std::string &user_name,
+ const policy_check_type
&check_type) const
{
- // 1. Check if it is not allowed.
- for (const auto &deny_policy : deny_policies) {
- // 1.1. In 'deny_policies'.
- if (!deny_policy.match(ac_type, user_name)) {
+ if (check_type == policy_check_type::kAllow) {
+ return do_policies_check(
+ check_type, ac_type, user_name, allow_policies,
allow_policies_exclude);
+ }
+ CHECK(check_type == policy_check_type::kDeny, "");
+ return do_policies_check(check_type, ac_type, user_name, deny_policies,
deny_policies_exclude);
+}
+
+policy_check_status
+acl_policies::do_policies_check(const policy_check_type &check_type,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const std::vector<policy_item> &policies,
+ const std::vector<policy_item>
&exclude_policies) const
+{
+ for (const auto &policy : policies) {
+ // 1. Doesn't match an allow_policies or a deny_policies.
+ if (!policy.match(ac_type, user_name)) {
continue;
}
- bool in_deny_policies_exclude = false;
- for (const auto &deny_policy_exclude : deny_policies_exclude) {
- if (deny_policy_exclude.match(ac_type, user_name)) {
- in_deny_policies_exclude = true;
- break;
+ // 2. Matches a policy.
+ for (const auto &exclude_policy : exclude_policies) {
+ if (exclude_policy.match(ac_type, user_name)) {
+ // 2.1. Matches an allow/deny_policies_exclude.
+ return policy_check_status::kPending;
}
}
- // 1.2. Not in any 'deny_policies_exclude', it's not allowed.
- if (!in_deny_policies_exclude) {
- return false;
+ // 2.2. Doesn't match any allow/deny_exclude_policies.
+ if (check_type == policy_check_type::kAllow) {
+ return policy_check_status::kAllowed;
+ } else {
+ return policy_check_status::kDenied;
}
}
+ // 3. Doesn't match any policy.
+ return policy_check_status::kNotMatched;
+}
- // 2. Check if it is allowed.
- for (const auto &allow_policy : allow_policies) {
- // 2.1. In 'allow_policies'.
- if (!allow_policy.match(ac_type, user_name)) {
- continue;
- }
- for (const auto &allow_policy_exclude : allow_policies_exclude) {
- // 2.2. In some 'allow_policies_exclude', it's not allowed.
- if (allow_policy_exclude.match(ac_type, user_name)) {
- return false;
+access_control_result
+check_ranger_resource_policy_allowed(const std::vector<ranger_resource_policy>
&policies,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const match_database_type &md_type,
+ const std::string &database_name,
+ const std::string &default_database_name)
+{
+ // Check if it is denied by any policy in current resource.
+ auto check_res = do_check_ranger_resource_policy(policies,
+ ac_type,
+ user_name,
+ md_type,
+ database_name,
+ default_database_name,
+ policy_check_type::kDeny);
+ if (access_control_result::kDenied == check_res) {
+ return access_control_result::kDenied;
+ }
+
+ // Check if it is allowed by any policy in current resource.
+ check_res = do_check_ranger_resource_policy(policies,
+ ac_type,
+ user_name,
+ md_type,
+ database_name,
+ default_database_name,
+ policy_check_type::kAllow);
+ if (access_control_result::kAllowed == check_res) {
+ return access_control_result::kAllowed;
+ }
+
+ // The check that does not match any policy in current reosource returns
false.
+ return access_control_result::kDenied;
+}
+
+access_control_result
+do_check_ranger_resource_policy(const std::vector<ranger_resource_policy>
&policies,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const match_database_type &md_type,
+ const std::string &database_name,
+ const std::string &default_database_name,
+ const policy_check_type &check_type)
+{
+ for (const auto &policy : policies) {
+ if (match_database_type::kNeed == md_type) {
+ // Lagacy table not match any database.
+ if (database_name.empty() && policy.database_names.count("*") == 0
&&
+ policy.database_names.count(default_database_name) == 0) {
+ continue;
}
+ // New table not match any database.
+ if (!database_name.empty() && policy.database_names.count("*") ==
0 &&
+ policy.database_names.count(database_name) == 0) {
+ continue;
+ }
+ }
+ auto check_status = policy.policies.policies_check(ac_type, user_name,
check_type);
+ // When policy_check_type is 'kDeny' and in a 'deny_policies' and not
in any
+ // 'deny_policies_exclude'.
+ if (policy_check_type::kDeny == check_type &&
+ policy_check_status::kDenied == check_status) {
+ return access_control_result::kDenied;
}
- // 2.3. Not in any 'allow_policies_exclude', it's allowed.
- return true;
+ // When policy_check_type is 'kAllow' and in a 'allow_policies' and
not in any
+ // 'allow_policies_exclude'.
+ if (policy_check_type::kAllow == check_type &&
+ policy_check_status::kAllowed == check_status) {
+ return access_control_result::kAllowed;
+ }
+ // In a 'policies' and in a 'policies_exclude' or not match.
+ if (policy_check_status::kPending == check_status ||
+ policy_check_status::kNotMatched == check_status) {
+ continue;
+ }
+ }
+ return access_control_result::kPending;
+}
+
+access_control_result check_ranger_database_table_policy_allowed(
+ const std::vector<matched_database_table_policy> &policies,
+ const access_type &ac_type,
+ const std::string &user_name)
+{
+ // Check if it is denied by any DATABASE_TABLE policy.
+ auto check_res = do_check_ranger_database_table_policy(
+ policies, ac_type, user_name, policy_check_type::kDeny);
+ if (access_control_result::kDenied == check_res) {
+ return access_control_result::kDenied;
+ }
+
+ // Check if it is allowed by any DATABASE_TABLE policy.
+ check_res = do_check_ranger_database_table_policy(
+ policies, ac_type, user_name, policy_check_type::kAllow);
+ if (access_control_result::kAllowed == check_res) {
+ return access_control_result::kAllowed;
}
- // 3. Otherwise, it's not allowed.
- return false;
+ // The check that does not match any DATABASE_TABLE policy returns false.
+ return access_control_result::kDenied;
+}
+
+access_control_result
+do_check_ranger_database_table_policy(const
std::vector<matched_database_table_policy> &policies,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const policy_check_type &check_type)
+{
+ for (const auto &policy : policies) {
+ auto check_status = policy.policies.policies_check(ac_type, user_name,
check_type);
+ // When policy_check_type is 'kDeny' and in a 'deny_policies' and not
in any
+ // 'deny_policies_exclude'.
+ if (policy_check_type::kDeny == check_type &&
+ policy_check_status::kDenied == check_status) {
+ return access_control_result::kDenied;
+ }
+ // When policy_check_type is 'kAllow' and in a 'allow_policies' and
not in any
+ // 'allow_policies_exclude'.
+ if (policy_check_type::kAllow == check_type &&
+ policy_check_status::kAllowed == check_status) {
+ return access_control_result::kAllowed;
+ }
+ // In a 'policies' and in a 'policies_exclude' or not match.
+ if (policy_check_status::kPending == check_status ||
Review Comment:
Use CHECK_TRUE as above.
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -27,45 +28,178 @@ bool policy_item::match(const access_type &ac_type, const
std::string &user_name
return static_cast<bool>(access_types & ac_type) && users.count(user_name)
!= 0;
}
-bool acl_policies::allowed(const access_type &ac_type, const std::string
&user_name) const
+policy_check_status acl_policies::policies_check(const access_type &ac_type,
+ const std::string &user_name,
+ const policy_check_type
&check_type) const
{
- // 1. Check if it is not allowed.
- for (const auto &deny_policy : deny_policies) {
- // 1.1. In 'deny_policies'.
- if (!deny_policy.match(ac_type, user_name)) {
+ if (check_type == policy_check_type::kAllow) {
+ return do_policies_check(
+ check_type, ac_type, user_name, allow_policies,
allow_policies_exclude);
+ }
+ CHECK(check_type == policy_check_type::kDeny, "");
+ return do_policies_check(check_type, ac_type, user_name, deny_policies,
deny_policies_exclude);
+}
+
+policy_check_status
+acl_policies::do_policies_check(const policy_check_type &check_type,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const std::vector<policy_item> &policies,
+ const std::vector<policy_item>
&exclude_policies) const
+{
+ for (const auto &policy : policies) {
+ // 1. Doesn't match an allow_policies or a deny_policies.
+ if (!policy.match(ac_type, user_name)) {
continue;
}
- bool in_deny_policies_exclude = false;
- for (const auto &deny_policy_exclude : deny_policies_exclude) {
- if (deny_policy_exclude.match(ac_type, user_name)) {
- in_deny_policies_exclude = true;
- break;
+ // 2. Matches a policy.
+ for (const auto &exclude_policy : exclude_policies) {
+ if (exclude_policy.match(ac_type, user_name)) {
+ // 2.1. Matches an allow/deny_policies_exclude.
+ return policy_check_status::kPending;
}
}
- // 1.2. Not in any 'deny_policies_exclude', it's not allowed.
- if (!in_deny_policies_exclude) {
- return false;
+ // 2.2. Doesn't match any allow/deny_exclude_policies.
+ if (check_type == policy_check_type::kAllow) {
+ return policy_check_status::kAllowed;
+ } else {
+ return policy_check_status::kDenied;
}
}
+ // 3. Doesn't match any policy.
+ return policy_check_status::kNotMatched;
+}
- // 2. Check if it is allowed.
- for (const auto &allow_policy : allow_policies) {
- // 2.1. In 'allow_policies'.
- if (!allow_policy.match(ac_type, user_name)) {
- continue;
- }
- for (const auto &allow_policy_exclude : allow_policies_exclude) {
- // 2.2. In some 'allow_policies_exclude', it's not allowed.
- if (allow_policy_exclude.match(ac_type, user_name)) {
- return false;
+access_control_result
+check_ranger_resource_policy_allowed(const std::vector<ranger_resource_policy>
&policies,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const match_database_type &md_type,
+ const std::string &database_name,
+ const std::string &default_database_name)
+{
+ // Check if it is denied by any policy in current resource.
+ auto check_res = do_check_ranger_resource_policy(policies,
+ ac_type,
+ user_name,
+ md_type,
+ database_name,
+ default_database_name,
+ policy_check_type::kDeny);
+ if (access_control_result::kDenied == check_res) {
+ return access_control_result::kDenied;
+ }
+
+ // Check if it is allowed by any policy in current resource.
+ check_res = do_check_ranger_resource_policy(policies,
+ ac_type,
+ user_name,
+ md_type,
+ database_name,
+ default_database_name,
+ policy_check_type::kAllow);
+ if (access_control_result::kAllowed == check_res) {
+ return access_control_result::kAllowed;
+ }
+
+ // The check that does not match any policy in current reosource returns
false.
+ return access_control_result::kDenied;
+}
+
+access_control_result
+do_check_ranger_resource_policy(const std::vector<ranger_resource_policy>
&policies,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const match_database_type &md_type,
+ const std::string &database_name,
+ const std::string &default_database_name,
+ const policy_check_type &check_type)
+{
+ for (const auto &policy : policies) {
+ if (match_database_type::kNeed == md_type) {
+ // Lagacy table not match any database.
+ if (database_name.empty() && policy.database_names.count("*") == 0
&&
+ policy.database_names.count(default_database_name) == 0) {
+ continue;
}
+ // New table not match any database.
+ if (!database_name.empty() && policy.database_names.count("*") ==
0 &&
+ policy.database_names.count(database_name) == 0) {
+ continue;
+ }
+ }
+ auto check_status = policy.policies.policies_check(ac_type, user_name,
check_type);
+ // When policy_check_type is 'kDeny' and in a 'deny_policies' and not
in any
+ // 'deny_policies_exclude'.
+ if (policy_check_type::kDeny == check_type &&
+ policy_check_status::kDenied == check_status) {
+ return access_control_result::kDenied;
}
- // 2.3. Not in any 'allow_policies_exclude', it's allowed.
- return true;
+ // When policy_check_type is 'kAllow' and in a 'allow_policies' and
not in any
+ // 'allow_policies_exclude'.
+ if (policy_check_type::kAllow == check_type &&
+ policy_check_status::kAllowed == check_status) {
+ return access_control_result::kAllowed;
+ }
+ // In a 'policies' and in a 'policies_exclude' or not match.
+ if (policy_check_status::kPending == check_status ||
+ policy_check_status::kNotMatched == check_status) {
+ continue;
+ }
+ }
+ return access_control_result::kPending;
+}
+
+access_control_result check_ranger_database_table_policy_allowed(
+ const std::vector<matched_database_table_policy> &policies,
+ const access_type &ac_type,
+ const std::string &user_name)
+{
+ // Check if it is denied by any DATABASE_TABLE policy.
+ auto check_res = do_check_ranger_database_table_policy(
+ policies, ac_type, user_name, policy_check_type::kDeny);
+ if (access_control_result::kDenied == check_res) {
+ return access_control_result::kDenied;
+ }
+
+ // Check if it is allowed by any DATABASE_TABLE policy.
+ check_res = do_check_ranger_database_table_policy(
+ policies, ac_type, user_name, policy_check_type::kAllow);
+ if (access_control_result::kAllowed == check_res) {
+ return access_control_result::kAllowed;
}
- // 3. Otherwise, it's not allowed.
- return false;
+ // The check that does not match any DATABASE_TABLE policy returns false.
+ return access_control_result::kDenied;
+}
+
+access_control_result
+do_check_ranger_database_table_policy(const
std::vector<matched_database_table_policy> &policies,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const policy_check_type &check_type)
+{
+ for (const auto &policy : policies) {
+ auto check_status = policy.policies.policies_check(ac_type, user_name,
check_type);
+ // When policy_check_type is 'kDeny' and in a 'deny_policies' and not
in any
+ // 'deny_policies_exclude'.
+ if (policy_check_type::kDeny == check_type &&
+ policy_check_status::kDenied == check_status) {
+ return access_control_result::kDenied;
+ }
+ // When policy_check_type is 'kAllow' and in a 'allow_policies' and
not in any
+ // 'allow_policies_exclude'.
+ if (policy_check_type::kAllow == check_type &&
+ policy_check_status::kAllowed == check_status) {
+ return access_control_result::kAllowed;
+ }
Review Comment:
Templatize the code as above.
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -27,45 +28,178 @@ bool policy_item::match(const access_type &ac_type, const
std::string &user_name
return static_cast<bool>(access_types & ac_type) && users.count(user_name)
!= 0;
}
-bool acl_policies::allowed(const access_type &ac_type, const std::string
&user_name) const
+policy_check_status acl_policies::policies_check(const access_type &ac_type,
+ const std::string &user_name,
+ const policy_check_type
&check_type) const
{
- // 1. Check if it is not allowed.
- for (const auto &deny_policy : deny_policies) {
- // 1.1. In 'deny_policies'.
- if (!deny_policy.match(ac_type, user_name)) {
+ if (check_type == policy_check_type::kAllow) {
+ return do_policies_check(
+ check_type, ac_type, user_name, allow_policies,
allow_policies_exclude);
+ }
+ CHECK(check_type == policy_check_type::kDeny, "");
+ return do_policies_check(check_type, ac_type, user_name, deny_policies,
deny_policies_exclude);
+}
+
+policy_check_status
+acl_policies::do_policies_check(const policy_check_type &check_type,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const std::vector<policy_item> &policies,
+ const std::vector<policy_item>
&exclude_policies) const
+{
+ for (const auto &policy : policies) {
+ // 1. Doesn't match an allow_policies or a deny_policies.
+ if (!policy.match(ac_type, user_name)) {
continue;
}
- bool in_deny_policies_exclude = false;
- for (const auto &deny_policy_exclude : deny_policies_exclude) {
- if (deny_policy_exclude.match(ac_type, user_name)) {
- in_deny_policies_exclude = true;
- break;
+ // 2. Matches a policy.
+ for (const auto &exclude_policy : exclude_policies) {
+ if (exclude_policy.match(ac_type, user_name)) {
+ // 2.1. Matches an allow/deny_policies_exclude.
+ return policy_check_status::kPending;
}
}
- // 1.2. Not in any 'deny_policies_exclude', it's not allowed.
- if (!in_deny_policies_exclude) {
- return false;
+ // 2.2. Doesn't match any allow/deny_exclude_policies.
+ if (check_type == policy_check_type::kAllow) {
+ return policy_check_status::kAllowed;
+ } else {
+ return policy_check_status::kDenied;
}
}
+ // 3. Doesn't match any policy.
+ return policy_check_status::kNotMatched;
+}
- // 2. Check if it is allowed.
- for (const auto &allow_policy : allow_policies) {
- // 2.1. In 'allow_policies'.
- if (!allow_policy.match(ac_type, user_name)) {
- continue;
- }
- for (const auto &allow_policy_exclude : allow_policies_exclude) {
- // 2.2. In some 'allow_policies_exclude', it's not allowed.
- if (allow_policy_exclude.match(ac_type, user_name)) {
- return false;
+access_control_result
+check_ranger_resource_policy_allowed(const std::vector<ranger_resource_policy>
&policies,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const match_database_type &md_type,
+ const std::string &database_name,
+ const std::string &default_database_name)
+{
+ // Check if it is denied by any policy in current resource.
+ auto check_res = do_check_ranger_resource_policy(policies,
+ ac_type,
+ user_name,
+ md_type,
+ database_name,
+ default_database_name,
+ policy_check_type::kDeny);
+ if (access_control_result::kDenied == check_res) {
+ return access_control_result::kDenied;
+ }
+
+ // Check if it is allowed by any policy in current resource.
+ check_res = do_check_ranger_resource_policy(policies,
+ ac_type,
+ user_name,
+ md_type,
+ database_name,
+ default_database_name,
+ policy_check_type::kAllow);
+ if (access_control_result::kAllowed == check_res) {
+ return access_control_result::kAllowed;
+ }
+
+ // The check that does not match any policy in current reosource returns
false.
+ return access_control_result::kDenied;
+}
+
+access_control_result
+do_check_ranger_resource_policy(const std::vector<ranger_resource_policy>
&policies,
+ const access_type &ac_type,
+ const std::string &user_name,
+ const match_database_type &md_type,
+ const std::string &database_name,
+ const std::string &default_database_name,
+ const policy_check_type &check_type)
+{
+ for (const auto &policy : policies) {
+ if (match_database_type::kNeed == md_type) {
+ // Lagacy table not match any database.
+ if (database_name.empty() && policy.database_names.count("*") == 0
&&
+ policy.database_names.count(default_database_name) == 0) {
+ continue;
}
+ // New table not match any database.
+ if (!database_name.empty() && policy.database_names.count("*") ==
0 &&
+ policy.database_names.count(database_name) == 0) {
+ continue;
+ }
+ }
+ auto check_status = policy.policies.policies_check(ac_type, user_name,
check_type);
+ // When policy_check_type is 'kDeny' and in a 'deny_policies' and not
in any
+ // 'deny_policies_exclude'.
+ if (policy_check_type::kDeny == check_type &&
+ policy_check_status::kDenied == check_status) {
+ return access_control_result::kDenied;
}
- // 2.3. Not in any 'allow_policies_exclude', it's allowed.
- return true;
+ // When policy_check_type is 'kAllow' and in a 'allow_policies' and
not in any
+ // 'allow_policies_exclude'.
+ if (policy_check_type::kAllow == check_type &&
+ policy_check_status::kAllowed == check_status) {
+ return access_control_result::kAllowed;
+ }
+ // In a 'policies' and in a 'policies_exclude' or not match.
+ if (policy_check_status::kPending == check_status ||
+ policy_check_status::kNotMatched == check_status) {
+ continue;
+ }
Review Comment:
Here check_status must be kPending or kNotMatched, right? refactor as
```
CHECK_TRUE(policy_check_status::kPending == check_status ||
policy_check_status::kNotMatched == check_status);
```
##########
src/runtime/ranger/ranger_resource_policy_manager.cpp:
##########
@@ -599,34 +586,50 @@ dsn::error_code
ranger_resource_policy_manager::sync_policies_to_app_envs()
req->__set_app_name(app.app_name);
req->__set_keys(
{dsn::replication::replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES});
- bool is_policy_matched = false;
+ std::vector<matched_database_table_policy>
matched_database_table_policies;
for (const auto &policy : table_policies->second) {
// If this table does not match any database, its Ranger policies
will be cleaned up.
if (policy.database_names.count(database_name) == 0 &&
policy.database_names.count("*") == 0) {
continue;
}
+ // If this table does not match any database table, its Ranger
policies will be cleaned
+ // up.
+ if (policy.table_names.count(table_name) == 0 &&
policy.table_names.count("*") == 0) {
+ continue;
+ }
+ // This table matches a policy.
+ matched_database_table_policy database_table_policy(
+ {database_name, table_name, policy.policies});
+ // This table matches the policy whose database is "*".
+ if (policy.database_names.count(database_name) == 0) {
Review Comment:
Let's add a CHECK here, policy.database_names == "*" ?
##########
src/runtime/ranger/ranger_resource_policy_manager.cpp:
##########
@@ -599,34 +586,50 @@ dsn::error_code
ranger_resource_policy_manager::sync_policies_to_app_envs()
req->__set_app_name(app.app_name);
req->__set_keys(
{dsn::replication::replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES});
- bool is_policy_matched = false;
+ std::vector<matched_database_table_policy>
matched_database_table_policies;
for (const auto &policy : table_policies->second) {
// If this table does not match any database, its Ranger policies
will be cleaned up.
if (policy.database_names.count(database_name) == 0 &&
policy.database_names.count("*") == 0) {
continue;
}
+ // If this table does not match any database table, its Ranger
policies will be cleaned
+ // up.
+ if (policy.table_names.count(table_name) == 0 &&
policy.table_names.count("*") == 0) {
+ continue;
+ }
+ // This table matches a policy.
+ matched_database_table_policy database_table_policy(
+ {database_name, table_name, policy.policies});
+ // This table matches the policy whose database is "*".
+ if (policy.database_names.count(database_name) == 0) {
+ database_table_policy.matched_database_name = "*";
+ }
+ // This table matches the policy whose database table is "*".
+ if (policy.table_names.count(table_name) == 0) {
Review Comment:
Let's add a CHECK here, policy.table_names == "*" ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
