From 02f7d0f455048d08d4ce57e149c10e116620d36d Mon Sep 17 00:00:00 2001 From: wklken Date: Tue, 19 Jul 2022 16:08:18 +0800 Subject: [PATCH 1/5] feat(string_contains): add string_contains support string_contains and _bk_iam_path_ support /system,type,1/ --- iam/contrib/converter/queryset.py | 7 +- iam/contrib/converter/sql.py | 9 +- iam/eval/constants.py | 7 +- iam/eval/operators.py | 134 +++++++++++++---------- iam/utils.py | 12 +- tests/contrib/converter/test_queryset.py | 9 +- tests/contrib/converter/test_sql.py | 9 +- tests/eval/test_expression.py | 7 ++ tests/eval/test_operators.py | 65 +++++++++-- tests/test_utils.py | 25 +++++ 10 files changed, 208 insertions(+), 76 deletions(-) diff --git a/iam/contrib/converter/queryset.py b/iam/contrib/converter/queryset.py index 206395b..c59eadf 100644 --- a/iam/contrib/converter/queryset.py +++ b/iam/contrib/converter/queryset.py @@ -12,12 +12,11 @@ import operator -from six.moves import reduce from django.db.models import Q - from iam.eval.constants import KEYWORD_BK_IAM_PATH_FIELD_SUFFIX, OP from iam.eval.expression import field_value_convert +from six.moves import reduce from .base import Converter @@ -84,6 +83,9 @@ def _ends_with(self, left, right): def _not_ends_with(self, left, right): return self._negative("{}__endswith", left, right) + def _string_contains(self, left, right): + return self._positive("{}__contains", left, right) + def _lt(self, left, right): return self._positive("{}__lt", left, right) @@ -139,6 +141,7 @@ def convert(self, data): OP.NOT_STARTS_WITH: self._not_starts_with, OP.ENDS_WITH: self._ends_with, OP.NOT_ENDS_WITH: self._not_ends_with, + OP.STRING_CONTAINS: self._string_contains, OP.LT: self._lt, OP.LTE: self._lte, OP.GT: self._gt, diff --git a/iam/contrib/converter/sql.py b/iam/contrib/converter/sql.py index 07b2207..1f808fe 100644 --- a/iam/contrib/converter/sql.py +++ b/iam/contrib/converter/sql.py @@ -12,7 +12,6 @@ import six - from iam.eval.constants import OP from iam.eval.expression import field_value_convert @@ -75,12 +74,12 @@ def _not_eq(self, left, right): return self._negative("{} != {}", left, right) def _in(self, left, right): - # TODO: right shuld be a list + # TODO: right should be a list right = [self._to_str_present(r, True) for r in right] return "{} IN ({})".format(left, ",".join([str(r) for r in right])) def _not_in(self, left, right): - # TODO: right shuld be a list + # TODO: right should be a list right = [self._to_str_present(r, True) for r in right] return "{} NOT IN ({})".format(left, ",".join([str(r) for r in right])) @@ -103,6 +102,9 @@ def _ends_with(self, left, right): def _not_ends_with(self, left, right): return self._negative("{} NOT LIKE '%{}'", left, right, False) + def _string_contains(self, left, right): + return self._positive("{} LIKE '%{}%'", left, right, False) + def _lt(self, left, right): return self._positive("{} < {}", left, right) @@ -145,6 +147,7 @@ def convert(self, data): OP.NOT_STARTS_WITH: self._not_starts_with, OP.ENDS_WITH: self._ends_with, OP.NOT_ENDS_WITH: self._not_ends_with, + OP.STRING_CONTAINS: self._string_contains, OP.LT: self._lt, OP.LTE: self._lte, OP.GT: self._gt, diff --git a/iam/eval/constants.py b/iam/eval/constants.py index 0fe90a2..793e38c 100644 --- a/iam/eval/constants.py +++ b/iam/eval/constants.py @@ -34,6 +34,8 @@ class OP(object): ENDS_WITH = "ends_with" NOT_ENDS_WITH = "not_ends_with" + STRING_CONTAINS = "string_contains" + LT = "lt" LTE = "lte" GT = "gt" @@ -47,12 +49,13 @@ class OP(object): NOT_EQ, IN, NOT_IN, - CONTAINS, - NOT_CONTAINS, + # CONTAINS, + # NOT_CONTAINS, STARTS_WITH, NOT_STARTS_WITH, ENDS_WITH, NOT_ENDS_WITH, + STRING_CONTAINS, ANY, ], "numberic": [EQ, NOT_EQ, IN, NOT_IN, LT, LTE, GT, GTE], diff --git a/iam/eval/operators.py b/iam/eval/operators.py index 39b377e..7c5afaf 100644 --- a/iam/eval/operators.py +++ b/iam/eval/operators.py @@ -111,7 +111,7 @@ def expr(self): def calculate(self, left, right): pass - def _eval_positive(self, attr, attr_is_array, value, value_is_array): # NOQA + def _eval_positive(self, object_attr, is_object_attr_array, policy_value, is_policy_value_array): # NOQA """ positive: - 1 hit: return True @@ -128,58 +128,52 @@ def _eval_positive(self, attr, attr_is_array, value, value_is_array): # NOQA attr = [1, 2]; value = [3, 4]; False """ if self.op == OP.ANY: - return self.calculate(attr, value) + return self.calculate(object_attr, policy_value) - # 1. IN/NOT_IN value is a list, just only check attr + # 1. IN/NOT_IN policy_value is a list(checked in eval), just only check attr if self.op in (OP.IN,): - if attr_is_array: - for a in attr: - if self.calculate(a, value): + if is_object_attr_array: + for a in object_attr: + if self.calculate(a, policy_value): return True return False - return self.calculate(attr, value) + return self.calculate(object_attr, policy_value) - # 2. CONTAINS/NOT_CONTAINS attr is a list, just check value + # 2. CONTAINS/NOT_CONTAINS attr is a list, and policy_value will be a single value(checked in eval) if self.op in (OP.CONTAINS,): - if value_is_array: - for v in value: - if self.calculate(attr, v): - return True - return False - - return self.calculate(attr, value) + return self.calculate(object_attr, policy_value) # 3. Others, check both attr and value # 3.1 both not array, the most common situation - if not (value_is_array or attr_is_array): - return self.calculate(attr, value) + if not (is_policy_value_array or is_object_attr_array): + return self.calculate(object_attr, policy_value) # 3.2 only value is array, the second common situation - if value_is_array and (not attr_is_array): - for v in value: + if is_policy_value_array and (not is_object_attr_array): + for v in policy_value: # return early if hit - if self.calculate(attr, v): + if self.calculate(object_attr, v): return True return False # 3.3 only attr value is array - if (not value_is_array) and attr_is_array: - for a in attr: + if (not is_policy_value_array) and is_object_attr_array: + for a in object_attr: # return early if hit - if self.calculate(a, value): + if self.calculate(a, policy_value): return True return False # 4. both array - for a in attr: - for v in value: + for a in object_attr: + for v in policy_value: # return early if hit if self.calculate(a, v): return True return False - def _eval_negative(self, attr, attr_is_array, value, value_is_array): # NOQA + def _eval_negative(self, object_attr, is_object_attr_array, policy_value, is_policy_value_array): # NOQA """ negative: - 1 miss: return False @@ -195,48 +189,42 @@ def _eval_negative(self, attr, attr_is_array, value, value_is_array): # NOQA attr = [1, 2]; value = [2, 3]; False """ - # 1. IN/NOT_IN value is a list, just only check attr + # 1. IN/NOT_IN policy_value is a list(checked in eval), just only check attr if self.op in (OP.NOT_IN,): - if attr_is_array: - for a in attr: - if not self.calculate(a, value): + if is_object_attr_array: + for a in object_attr: + if not self.calculate(a, policy_value): return False return True - return self.calculate(attr, value) + return self.calculate(object_attr, policy_value) - # 2. CONTAINS/NOT_CONTAINS attr is a list, just check value + # 2. CONTAINS/NOT_CONTAINS attr is a list, and policy_value will be a single value(checked in eval) if self.op in (OP.NOT_CONTAINS,): - if value_is_array: - for v in value: - if not self.calculate(attr, v): - return False - return True - - return self.calculate(attr, value) + return self.calculate(object_attr, policy_value) # 3. Others, check both attr and value # 3.1 both not array, the most common situation - if not (value_is_array or attr_is_array): - return self.calculate(attr, value) + if not (is_policy_value_array or is_object_attr_array): + return self.calculate(object_attr, policy_value) # 3.2 only value is array, the second common situation - if value_is_array and (not attr_is_array): - for v in value: - if not self.calculate(attr, v): + if is_policy_value_array and (not is_object_attr_array): + for v in policy_value: + if not self.calculate(object_attr, v): return False return True # 3.3 only attr value is array - if (not value_is_array) and attr_is_array: - for a in attr: - if not self.calculate(a, value): + if (not is_policy_value_array) and is_object_attr_array: + for a in object_attr: + if not self.calculate(a, policy_value): return False return True # 4. both array - for a in attr: - for v in value: + for a in object_attr: + for v in policy_value: # return early if hit if not self.calculate(a, v): return False @@ -251,19 +239,42 @@ def eval(self, obj_set): if one of them is array, or both array calculate each item in array """ - attr = obj_set.get(self.field) - value = self.value + object_attr = obj_set.get(self.field) + policy_value = self.value + + is_object_attr_array = isinstance(object_attr, (list, tuple)) + is_policy_value_array = isinstance(policy_value, (list, tuple)) + + # if you add new operator, please read this first: https://github.com/TencentBlueKing/bk-iam-saas/issues/1293 + # valid the attr and value first + if self.op in (OP.IN, OP.NOT_IN): + # a in b, a not_in b + # b should be an array, while a can be a single or an array + # so we should make the in expression b always be an array + if not is_policy_value_array: + return False - attr_is_array = isinstance(attr, (list, tuple)) - value_is_array = isinstance(value, (list, tuple)) + if self.op in (OP.CONTAINS, OP.NOT_CONTAINS): + # a contains b, a not_contains b + # a should be an array, b should be a single value + # so, we should make the contains expression b always be a single string, + # while a can be a single value or an array + if not is_object_attr_array or is_policy_value_array: + return False + + if self.op in (OP.STARTS_WITH, OP.ENDS_WITH, OP.NOT_STARTS_WITH, OP.NOT_ENDS_WITH, OP.STRING_CONTAINS): + # a starts_with b, a not_starts_with, a ends_with b, a not_ends_with b + # b should be a single value, while a can be a single value or an array + if is_policy_value_array: + return False # positive and negative operator # == 命中一个即返回 # != 需要全部遍历完, 确认全部不等于才返回? if self.op.startswith("not_"): - return self._eval_negative(attr, attr_is_array, value, value_is_array) + return self._eval_negative(object_attr, is_object_attr_array, policy_value, is_policy_value_array) else: - return self._eval_positive(attr, attr_is_array, value, value_is_array) + return self._eval_positive(object_attr, is_object_attr_array, policy_value, is_policy_value_array) class EqualOperator(BinaryOperator): @@ -318,7 +329,6 @@ def calculate(self, left, right): class StartsWithOperator(BinaryOperator): def __init__(self, field, value): - # TODO: value should be string? super(StartsWithOperator, self).__init__(OP.STARTS_WITH, field, value) def calculate(self, left, right): @@ -327,7 +337,6 @@ def calculate(self, left, right): class NotStartsWithOperator(BinaryOperator): def __init__(self, field, value): - # TODO: value should be string? super(NotStartsWithOperator, self).__init__(OP.NOT_STARTS_WITH, field, value) def calculate(self, left, right): @@ -336,7 +345,6 @@ def calculate(self, left, right): class EndsWithOperator(BinaryOperator): def __init__(self, field, value): - # TODO: value should be string? super(EndsWithOperator, self).__init__(OP.ENDS_WITH, field, value) def calculate(self, left, right): @@ -345,13 +353,20 @@ def calculate(self, left, right): class NotEndsWithOperator(BinaryOperator): def __init__(self, field, value): - # TODO: value should be string? super(NotEndsWithOperator, self).__init__(OP.NOT_ENDS_WITH, field, value) def calculate(self, left, right): return not left.endswith(right) +class StringContainsOperator(BinaryOperator): + def __init__(self, field, value): + super(StringContainsOperator, self).__init__(OP.STRING_CONTAINS, field, value) + + def calculate(self, left, right): + return right in left + + class LTOperator(BinaryOperator): def __init__(self, field, value): # TODO: field / value should be numberic @@ -407,6 +422,7 @@ def calculate(self, left, right): OP.NOT_STARTS_WITH: NotStartsWithOperator, OP.ENDS_WITH: EndsWithOperator, OP.NOT_ENDS_WITH: NotEndsWithOperator, + OP.STRING_CONTAINS: StringContainsOperator, OP.LT: LTOperator, OP.LTE: LTEOperator, OP.GT: GTOperator, diff --git a/iam/utils.py b/iam/utils.py index 586b60c..5dd006e 100644 --- a/iam/utils.py +++ b/iam/utils.py @@ -82,7 +82,17 @@ def gen_perms_apply_data(system, subject, action_to_resources_list): if topo_path: for part in topo_path[1:-1].split("/"): - rtype, rid = part.split(",") + node_parts = part.split(",") + rtype, rid = "", "" + if len(node_parts) == 2: + rtype, rid = node_parts[0], node_parts[1] + elif len(node_parts) == 3: + rtype, rid = node_parts[1], node_parts[2] + # NOTE: currently, keep the name of /bk_cmdb,set,1/ same as /set,1/ + part = ",".join(node_parts[1:]) + else: + raise Exception("Invalid _bk_iam_path_: %s" % topo_path) + inst_item.append( { "type": rtype, diff --git a/tests/contrib/converter/test_queryset.py b/tests/contrib/converter/test_queryset.py index 423a7a0..0aeb714 100644 --- a/tests/contrib/converter/test_queryset.py +++ b/tests/contrib/converter/test_queryset.py @@ -11,7 +11,6 @@ """ from django.db.models import Q - from iam import DjangoQuerySetConverter @@ -105,6 +104,14 @@ def test_not_ends_with(): assertQEqual(c._not_ends_with("id", ["test", 123]), (~Q(id__endswith="test") & ~Q(id__endswith=123))) +def test_string_contains(): + c = DjangoQuerySetConverter() + + assertQEqual(c._string_contains("id", "1"), Q(id__contains="1")) + assertQEqual(c._string_contains("id", 1), Q(id__contains=1)) + assertQEqual(c._string_contains("id", ["1", 1]), (Q(id__contains="1") | Q(id__contains=1))) + + def test_lt(): c = DjangoQuerySetConverter() diff --git a/tests/contrib/converter/test_sql.py b/tests/contrib/converter/test_sql.py index dbe568a..55e103d 100644 --- a/tests/contrib/converter/test_sql.py +++ b/tests/contrib/converter/test_sql.py @@ -10,7 +10,6 @@ specific language governing permissions and limitations under the License. """ import pytest - from iam import SQLConverter @@ -104,6 +103,14 @@ def test_not_ends_with(): assert c._not_ends_with("id", ["test", 123]) == "(id NOT LIKE '%test' AND id NOT LIKE '%123')" +def test_string_contains(): + c = SQLConverter() + + assert c._string_contains("id", "test") == "id LIKE '%test%'" + assert c._string_contains("id", ["test", "test1"]) == "(id LIKE '%test%' OR id LIKE '%test1%')" + assert c._string_contains("id", ["test", 123]) == "(id LIKE '%test%' OR id LIKE '%123%')" + + def test_lt(): c = SQLConverter() diff --git a/tests/eval/test_expression.py b/tests/eval/test_expression.py index 6938aca..cfaacb6 100644 --- a/tests/eval/test_expression.py +++ b/tests/eval/test_expression.py @@ -70,12 +70,19 @@ def test_parse_bk_iam_path(): assert "/biz,1/set," == _parse_bk_iam_path("/biz,1/set,*/") + assert "/bk_cmdb,biz,1/bk_cmdb,set,1/" == _parse_bk_iam_path("/bk_cmdb,biz,1/bk_cmdb,set,1/") + assert "/bk_cmdb,biz,1/bk_cmdb,set," == _parse_bk_iam_path("/bk_cmdb,biz,1/bk_cmdb,set,*/") + # tuple assert ["a", "b"] == _parse_bk_iam_path(("a", "b")) # tuple path assert ["/biz,1/set,1/", "/biz,2/module,"] == _parse_bk_iam_path(("/biz,1/set,1/", "/biz,2/module,*/")) + assert ["/bk_cmdb,biz,1/bk_cmdb,set,1/", "/bk_cmdb,biz,2/bk_cmdb,module,"] == _parse_bk_iam_path( + ("/bk_cmdb,biz,1/bk_cmdb,set,1/", "/bk_cmdb,biz,2/bk_cmdb,module,*/") + ) + # int assert 1 == _parse_bk_iam_path(1) diff --git a/tests/eval/test_operators.py b/tests/eval/test_operators.py index ee42fba..47efa03 100644 --- a/tests/eval/test_operators.py +++ b/tests/eval/test_operators.py @@ -11,6 +11,8 @@ """ import unittest +from iam.eval.operators import StringContainsOperator + try: from unittest.mock import patch except ImportError: @@ -173,6 +175,10 @@ def test_in_operator(): assert inop.eval(d1) assert not inop.eval(d2) + # IN, policy_value is not an array, always False + assert not InOperator("host.id", "a1").eval(d1) + assert not InOperator("host.id", "a1").eval(d2) + # NOT_IN notinop = NotInOperator("host.id", ["a1", "a3"]) @@ -182,6 +188,10 @@ def test_in_operator(): assert not notinop.eval(d1) assert notinop.eval(d2) + # NOT_IN, policy_value is not an array, always False + assert not NotInOperator("host.id", "a1").eval(d1) + assert not NotInOperator("host.id", "a1").eval(d2) + # attr is a list # common: a3 d3 = ObjectSet() @@ -205,6 +215,8 @@ def test_in_operator(): assert not inop.eval(d4) assert notinop.eval(d4) + # policy value is not an array + def test_contains_operator(): d1 = ObjectSet() @@ -240,20 +252,20 @@ def test_contains_operator(): assert not nc.eval(d1) assert nc.eval(d2) - # value is a list + # if policy_value is a list, always be false! c1 = ContainsOperator("host.owner", ["a1", "a2"]) nc1 = NotContainsOperator("host.owner", ["a1", "a2"]) d3 = ObjectSet() d3.add_object("host", {"owner": ["a3", "a2"]}) - assert c1.eval(d3) + assert not c1.eval(d3) assert not nc1.eval(d3) d4 = ObjectSet() d4.add_object("host", {"owner": ["b1", "b2"]}) assert not c1.eval(d4) - assert nc1.eval(d4) + assert not nc1.eval(d4) def test_text_operator(): @@ -280,6 +292,10 @@ def test_text_operator(): assert sw.eval(d1) assert not sw.eval(d2) + # STARTS_WITH, policy_value is an array, always False + assert not StartsWithOperator("person.name", ["hel"]).eval(d1) + assert not StartsWithOperator("person.name", ["hel"]).eval(d2) + # NOT_STARTS_WITH nsw = NotStartsWithOperator("person.name", "hel") @@ -289,6 +305,10 @@ def test_text_operator(): assert not nsw.eval(d1) assert nsw.eval(d2) + # NOT_STARTS_WITH, policy_value is an array, always False + assert not NotStartsWithOperator("person.name", ["hel"]).eval(d1) + assert not NotStartsWithOperator("person.name", ["hel"]).eval(d2) + # ENDS_WITH ew = EndsWithOperator("person.name", "llo") @@ -298,6 +318,10 @@ def test_text_operator(): assert ew.eval(d1) assert not ew.eval(d2) + # ENDS_WITH, policy_value is an array, always False + assert not EndsWithOperator("person.name", ["llo"]).eval(d1) + assert not EndsWithOperator("person.name", ["llo"]).eval(d2) + # NOT_ENDS_WITH new = NotEndsWithOperator("person.name", "llo") @@ -307,6 +331,23 @@ def test_text_operator(): assert not new.eval(d1) assert new.eval(d2) + # NOT_ENDS_WITH, policy_value is an array, always False + assert not NotEndsWithOperator("person.name", ["llo"]).eval(d1) + assert not NotEndsWithOperator("person.name", ["llo"]).eval(d2) + + # STRING_CONTAINS + ew = StringContainsOperator("person.name", "llo") + + assert ew.op == OP.STRING_CONTAINS + assert ew.expr() == "(person.name string_contains 'llo')" + + assert ew.eval(d1) + assert not ew.eval(d2) + + # STRING_CONTAINS, policy_value is an array, always False + assert not StringContainsOperator("person.name", ["llo"]).eval(d1) + assert not StringContainsOperator("person.name", ["llo"]).eval(d2) + def test_math_operator(): d1 = ObjectSet() @@ -371,6 +412,16 @@ def test_math_operator(): assert gte.eval(d2) assert gte.eval(d3) + # GTE, policy list is an array + gte = GTEOperator("person.age", [21, 20]) + + assert gte.op == OP.GTE + assert gte.expr() == "(person.age gte [21, 20])" + + assert not gte.eval(d1) + assert gte.eval(d2) + assert gte.eval(d3) + def test_any_operator(): a = AnyOperator("host.id", "localhost") @@ -452,8 +503,8 @@ def test_binary_operator_eval_positive(): "id": [1, 2], }, ) - # [1, 2] contains 1 of [2,4] - eq6 = ContainsOperator("host.id", [2, 4]) + # [1, 2] contains 2 + eq6 = ContainsOperator("host.id", 2) assert eq6.eval(d4) @@ -508,8 +559,8 @@ def test_binary_operator_eval_negative(): "id": [1, 2], }, ) - # [1,2] not contains all of [3,4] - eq6 = NotContainsOperator("host.id", [3, 4]) + # [1,2] not contains 3 + eq6 = NotContainsOperator("host.id", 3) assert eq6.eval(d4) diff --git a/tests/test_utils.py b/tests/test_utils.py index ee243d1..866e718 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -35,6 +35,9 @@ def test_gen_perms_apply_data(): resource4 = Resource("another_system", "r4", "r4id", {"name": "r4n"}) resource5 = Resource("another_system", "r4", "r5id", {"name": "r5n"}) resource6 = Resource("test_system", "r6", "r6id", {"name": "r6n", "_bk_iam_path_": "/biz,1/set,2/module,3/"}) + resource7 = Resource( + "test_system", "r7", "r7id", {"name": "r7n", "_bk_iam_path_": "/bk_cmdb,biz,1/bk_cmdb,set,2/bk_cmdb,module,3/"} + ) def get_system_name(system): return {"test_system": "test_system_name", "another_system": "another_system_name"}[system] @@ -56,6 +59,7 @@ def get_resource_name(system, resource): "r2": "r2_type", "r3": "r3_type", "r6": "r6_type", + "r7": "r7_type", "biz": "biz_type", "set": "set_type", "module": "module_type", @@ -81,6 +85,7 @@ def get_resource_name(system, resource): ], }, {"action": action4, "resources_list": [[resource6]]}, + {"action": action4, "resources_list": [[resource7]]}, ], ) @@ -166,5 +171,25 @@ def get_resource_name(system, resource): } ], }, + { + "id": "action4", + "name": "action4_name", + "related_resource_types": [ + { + "system_id": "test_system", + "system_name": "test_system_name", + "type": "r7", + "type_name": "r7_type", + "instances": [ + [ + {"type": "biz", "type_name": "biz_type", "id": "1", "name": "biz,1"}, + {"type": "set", "type_name": "set_type", "id": "2", "name": "set,2"}, + {"type": "module", "type_name": "module_type", "id": "3", "name": "module,3"}, + {"type": "r7", "type_name": "r7_type", "id": "r7id", "name": "r7n"}, + ] + ], + } + ], + }, ], } From 8d3503c4e165b8d25516666617f9e13cb5dafe22 Mon Sep 17 00:00:00 2001 From: wklken Date: Tue, 19 Jul 2022 16:11:20 +0800 Subject: [PATCH 2/5] ci(flake8): ignore c901 for gen_perms_apply_data --- iam/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/iam/utils.py b/iam/utils.py index 5dd006e..16a4a6f 100644 --- a/iam/utils.py +++ b/iam/utils.py @@ -16,6 +16,7 @@ from . import meta +# flake8: noqa: C901 def gen_perms_apply_data(system, subject, action_to_resources_list): """ 根据传入的参数生成无权限交互协议数据 From f11903419eed4f4abcc684c86847a607de8e4087 Mon Sep 17 00:00:00 2001 From: wklken Date: Tue, 19 Jul 2022 19:10:50 +0800 Subject: [PATCH 3/5] feat(eval/operator): make lt/lte/gt/gte more clear, policy value should be a single value --- iam/eval/operators.py | 14 +++++++++++++- tests/eval/test_operators.py | 31 +++++++++++-------------------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/iam/eval/operators.py b/iam/eval/operators.py index 7c5afaf..e03dec0 100644 --- a/iam/eval/operators.py +++ b/iam/eval/operators.py @@ -262,7 +262,19 @@ def eval(self, obj_set): if not is_object_attr_array or is_policy_value_array: return False - if self.op in (OP.STARTS_WITH, OP.ENDS_WITH, OP.NOT_STARTS_WITH, OP.NOT_ENDS_WITH, OP.STRING_CONTAINS): + if self.op in ( + OP.EQ, + OP.NOT_EQ, + OP.LT, + OP.LTE, + OP.GT, + OP.GTE, + OP.STARTS_WITH, + OP.NOT_STARTS_WITH, + OP.ENDS_WITH, + OP.NOT_ENDS_WITH, + OP.STRING_CONTAINS, + ): # a starts_with b, a not_starts_with, a ends_with b, a not_ends_with b # b should be a single value, while a can be a single value or an array if is_policy_value_array: diff --git a/tests/eval/test_operators.py b/tests/eval/test_operators.py index 47efa03..67c6344 100644 --- a/tests/eval/test_operators.py +++ b/tests/eval/test_operators.py @@ -412,15 +412,14 @@ def test_math_operator(): assert gte.eval(d2) assert gte.eval(d3) - # GTE, policy list is an array + # GTE, policy list is an array, will all get False gte = GTEOperator("person.age", [21, 20]) assert gte.op == OP.GTE assert gte.expr() == "(person.age gte [21, 20])" - assert not gte.eval(d1) - assert gte.eval(d2) - assert gte.eval(d3) + assert not gte.eval(d2) + assert not gte.eval(d3) def test_any_operator(): @@ -471,17 +470,13 @@ def test_binary_operator_eval_positive(): eq1 = EqualOperator("host.id", 1) assert eq1.eval(d1) - eq2 = EqualOperator("host.id", [1, 2]) - assert eq2.eval(d1) - eq3 = EqualOperator("host.id", 2) assert eq3.eval(d2) - eq4 = EqualOperator("host.id", [5, 1]) - assert eq4.eval(d2) - - eq5 = EqualOperator("host.id", [3, 4]) - assert not eq5.eval(d2) + # if policyValue is an array, always got false + assert not EqualOperator("host.id", [1, 2]).eval(d1) + assert not EqualOperator("host.id", [5, 1]).eval(d2) + assert not EqualOperator("host.id", [3, 4]).eval(d2) # IN d3 = ObjectSet() @@ -527,17 +522,13 @@ def test_binary_operator_eval_negative(): neq1 = NotEqualOperator("host.id", 2) assert neq1.eval(d1) - neq2 = NotEqualOperator("host.id", [2]) - assert neq2.eval(d1) - - neq3 = NotEqualOperator("host.id", [3, 4]) - assert neq3.eval(d2) - neq4 = NotEqualOperator("host.id", 3) assert neq4.eval(d2) - neq5 = NotEqualOperator("host.id", [2, 3]) - assert not neq5.eval(d2) + # not_eq policy value is an array, always False + assert not NotEqualOperator("host.id", [2]).eval(d1) + assert not NotEqualOperator("host.id", [3, 4]).eval(d2) + assert not NotEqualOperator("host.id", [2, 3]).eval(d2) # NOT_IN d3 = ObjectSet() From 88625ba36ccda69d9e2ce8d59a015a4ef004134c Mon Sep 17 00:00:00 2001 From: wklken Date: Tue, 19 Jul 2022 19:44:43 +0800 Subject: [PATCH 4/5] refactor(eval/operators): refactor eval_positive and eval_negative --- iam/eval/operators.py | 119 ++++++++++-------------------------------- 1 file changed, 29 insertions(+), 90 deletions(-) diff --git a/iam/eval/operators.py b/iam/eval/operators.py index e03dec0..2418c7f 100644 --- a/iam/eval/operators.py +++ b/iam/eval/operators.py @@ -111,7 +111,7 @@ def expr(self): def calculate(self, left, right): pass - def _eval_positive(self, object_attr, is_object_attr_array, policy_value, is_policy_value_array): # NOQA + def _eval_positive(self, object_attr, is_object_attr_array, policy_value): # NOQA """ positive: - 1 hit: return True @@ -121,59 +121,22 @@ def _eval_positive(self, object_attr, is_object_attr_array, policy_value, is_pol op = eq => one of attr equals one of value attr = 1; value = 1; True - attr = 1; value = [1, 2]; True attr = [1, 2]; value = 2; True - attr = [1, 2]; value = [5, 1]; True - - attr = [1, 2]; value = [3, 4]; False """ - if self.op == OP.ANY: - return self.calculate(object_attr, policy_value) - - # 1. IN/NOT_IN policy_value is a list(checked in eval), just only check attr - if self.op in (OP.IN,): - if is_object_attr_array: - for a in object_attr: - if self.calculate(a, policy_value): - return True - return False - - return self.calculate(object_attr, policy_value) - - # 2. CONTAINS/NOT_CONTAINS attr is a list, and policy_value will be a single value(checked in eval) - if self.op in (OP.CONTAINS,): - return self.calculate(object_attr, policy_value) + # if self.op == OP.ANY: + # return self.calculate(object_attr, policy_value) - # 3. Others, check both attr and value - # 3.1 both not array, the most common situation - if not (is_policy_value_array or is_object_attr_array): - return self.calculate(object_attr, policy_value) - - # 3.2 only value is array, the second common situation - if is_policy_value_array and (not is_object_attr_array): - for v in policy_value: - # return early if hit - if self.calculate(object_attr, v): - return True - return False - - # 3.3 only attr value is array - if (not is_policy_value_array) and is_object_attr_array: + # NOTE: here, the policyValue should not be array! + # It's single value (except: the NotIn op policyValue is an array) + if is_object_attr_array: for a in object_attr: - # return early if hit if self.calculate(a, policy_value): return True return False - # 4. both array - for a in object_attr: - for v in policy_value: - # return early if hit - if self.calculate(a, v): - return True - return False + return self.calculate(object_attr, policy_value) - def _eval_negative(self, object_attr, is_object_attr_array, policy_value, is_policy_value_array): # NOQA + def _eval_negative(self, object_attr, is_object_attr_array, policy_value): # NOQA """ negative: - 1 miss: return False @@ -183,52 +146,18 @@ def _eval_negative(self, object_attr, is_object_attr_array, policy_value, is_pol op = not_eq => all of attr should not_eq to all of the value attr = 1; value = 2; True - attr = 1; value = [2]; True - attr = [1, 2]; value = [3, 4]; True attr = [1, 2]; value = 3; True - - attr = [1, 2]; value = [2, 3]; False """ - # 1. IN/NOT_IN policy_value is a list(checked in eval), just only check attr - if self.op in (OP.NOT_IN,): - if is_object_attr_array: - for a in object_attr: - if not self.calculate(a, policy_value): - return False - return True - - return self.calculate(object_attr, policy_value) - - # 2. CONTAINS/NOT_CONTAINS attr is a list, and policy_value will be a single value(checked in eval) - if self.op in (OP.NOT_CONTAINS,): - return self.calculate(object_attr, policy_value) - # 3. Others, check both attr and value - # 3.1 both not array, the most common situation - if not (is_policy_value_array or is_object_attr_array): - return self.calculate(object_attr, policy_value) - - # 3.2 only value is array, the second common situation - if is_policy_value_array and (not is_object_attr_array): - for v in policy_value: - if not self.calculate(object_attr, v): - return False - return True - - # 3.3 only attr value is array - if (not is_policy_value_array) and is_object_attr_array: + # NOTE: here, the policyValue should not be array! + # It's single value (except: the NotIn op policyValue is an array) + if is_object_attr_array: for a in object_attr: if not self.calculate(a, policy_value): return False return True - # 4. both array - for a in object_attr: - for v in policy_value: - # return early if hit - if not self.calculate(a, v): - return False - return True + return self.calculate(object_attr, policy_value) def eval(self, obj_set): """ @@ -245,6 +174,10 @@ def eval(self, obj_set): is_object_attr_array = isinstance(object_attr, (list, tuple)) is_policy_value_array = isinstance(policy_value, (list, tuple)) + # any + if self.op == OP.ANY: + return True + # if you add new operator, please read this first: https://github.com/TencentBlueKing/bk-iam-saas/issues/1293 # valid the attr and value first if self.op in (OP.IN, OP.NOT_IN): @@ -254,6 +187,11 @@ def eval(self, obj_set): if not is_policy_value_array: return False + if self.op == OP.IN: + return self._eval_positive(object_attr, is_object_attr_array, policy_value) + else: + return self._eval_negative(object_attr, is_object_attr_array, policy_value) + if self.op in (OP.CONTAINS, OP.NOT_CONTAINS): # a contains b, a not_contains b # a should be an array, b should be a single value @@ -261,6 +199,7 @@ def eval(self, obj_set): # while a can be a single value or an array if not is_object_attr_array or is_policy_value_array: return False + return self.calculate(object_attr, policy_value) if self.op in ( OP.EQ, @@ -280,13 +219,13 @@ def eval(self, obj_set): if is_policy_value_array: return False - # positive and negative operator - # == 命中一个即返回 - # != 需要全部遍历完, 确认全部不等于才返回? - if self.op.startswith("not_"): - return self._eval_negative(object_attr, is_object_attr_array, policy_value, is_policy_value_array) - else: - return self._eval_positive(object_attr, is_object_attr_array, policy_value, is_policy_value_array) + # positive and negative operator + # == 命中一个即返回 + # != 需要全部遍历完, 确认全部不等于才返回? + if self.op.startswith("not_"): + return self._eval_negative(object_attr, is_object_attr_array, policy_value) + else: + return self._eval_positive(object_attr, is_object_attr_array, policy_value) class EqualOperator(BinaryOperator): From a15b03be4062983ceda9c506d17fc1e4a0a673c0 Mon Sep 17 00:00:00 2001 From: wklken Date: Fri, 22 Jul 2022 11:49:52 +0800 Subject: [PATCH 5/5] chore(version): 1.2.1 --- iam/__version__.py | 2 +- iam/utils.py | 2 ++ release.md | 10 ++++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/iam/__version__.py b/iam/__version__.py index 268300d..7a1faa9 100644 --- a/iam/__version__.py +++ b/iam/__version__.py @@ -1,3 +1,3 @@ # -*- coding: utf-8 -*- -__version__ = "1.2.0" +__version__ = "1.2.1" diff --git a/iam/utils.py b/iam/utils.py index 16a4a6f..7e8e45f 100644 --- a/iam/utils.py +++ b/iam/utils.py @@ -83,6 +83,8 @@ def gen_perms_apply_data(system, subject, action_to_resources_list): if topo_path: for part in topo_path[1:-1].split("/"): + # NOTE: old _bk_iam_path_ is like /set,1/host,2/ + # while the new _bk_iam_path_ is like /bk_cmdb,set,1/bk_cmdb,host,2/ node_parts = part.split(",") rtype, rid = "", "" if len(node_parts) == 2: diff --git a/release.md b/release.md index 3160696..d04a539 100644 --- a/release.md +++ b/release.md @@ -1,5 +1,15 @@ 版本日志 =============== + +# v1.2.1 + +- add: operator `string_contains` #68 +- update: 规范化所有操作符左值/右值, 并增加校验(校验失败直接False) +- refactor: expression eval + +注意: 如果使用 RBAC 接入权限中心, 必须使用这个版本的 SDK(action.auth_type="rbac") + + # v1.2.0 - support django 3.2