From 8a65506d03f02143ceace076d227ed0b2032cd5b Mon Sep 17 00:00:00 2001 From: wklken Date: Mon, 25 Jul 2022 15:57:01 +0800 Subject: [PATCH 1/2] feat(evaluation): add string_contains --- src/Evaluation/CompareFunction.php | 34 ++++++++-- src/Evaluation/Eval.php | 40 ++++------- src/Evaluation/Operator.php | 2 + tests/Evaluation/ExprCellTest.php | 104 +++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 31 deletions(-) diff --git a/src/Evaluation/CompareFunction.php b/src/Evaluation/CompareFunction.php index 0f367f2..f278db3 100644 --- a/src/Evaluation/CompareFunction.php +++ b/src/Evaluation/CompareFunction.php @@ -45,22 +45,48 @@ function cmp_gte($v1, $v2): bool function cmp_starts_with($v1, $v2): bool { - return str_starts_with($v1, $v2); + if (is_null($v1)) { + return false; + } + return strpos($v1, $v2) === 0; + // only in php8 + // return str_starts_with($v1, $v2); } function cmp_not_starts_with($v1, $v2): bool { - return !str_starts_with($v1, $v2); + if (is_null($v1)) { + return false; + } + return !(strpos($v1, $v2) === 0); } function cmp_ends_with($v1, $v2): bool { - return str_ends_with($v1, $v2); + if (is_null($v1)) { + return false; + } + $length = strlen($v2); + return $length > 0 ? substr($v1, -$length) === $v2: true; + // only in php8 + // return str_ends_with($v1, $v2); } function cmp_not_ends_with($v1, $v2): bool { - return !str_ends_with($v1, $v2); + if (is_null($v1)) { + return false; + } + $length = strlen($v2); + return $length > 0 ? substr($v1, -$length) !== $v2: true; +} + +function cmp_string_contains($v1, $v2): bool +{ + if (is_null($v1)) { + return false; + } + return strpos($v1, $v2) !== false; } function cmp_in($v1, $v2): bool diff --git a/src/Evaluation/Eval.php b/src/Evaluation/Eval.php index 77147a1..009d6f1 100644 --- a/src/Evaluation/Eval.php +++ b/src/Evaluation/Eval.php @@ -35,11 +35,12 @@ function eval_binary_operator(string $op, string $field, $policy_value, ObjectSe } } + // reference: https://github.com/TencentBlueKing/bk-iam-saas/issues/1293 // do validate // 1. eq/not_eq/lt/lte/gt/gte the policy value should be a single value - // 2. important: starts_with/not_starts_with/ends_with/not_ends_with, the policy value should be a single value too! + // 2. important: starts_with/not_starts_with/ends_with/not_ends_with/string_contains, the policy value should be a single value too! // 3. in/not_in, the policy value should be an array - // 4. contains/not_contains, the object value should be an array + // 4. contains/not_contains, the object value should be an array, the policy value should be a single value switch ($op) { case Operator::ANY: @@ -54,13 +55,14 @@ function eval_binary_operator(string $op, string $field, $policy_value, ObjectSe // NOTE: starts_with and ends_with should be a single value!!!!! case Operator::STARTS_WITH: case Operator::ENDS_WITH: + case Operator::STRING_CONTAINS: if (is_array($policy_value)) { throw new Exception("wrong policy values! should not be an array"); } return evalPositive($op, $object_value, $policy_value); - // policy value is an array case Operator::IN: + // NOTE: policy value is an array if (!is_array($policy_value)) { throw new Exception("the policy value of operator `in` should be an array"); } @@ -71,8 +73,10 @@ function eval_binary_operator(string $op, string $field, $policy_value, ObjectSe if (!is_array($object_value)) { throw new Exception("the object attribute should be array of operator `contains`"); } + if (is_array($policy_value)) { + throw new Exception("wrong policy values! should not be an array"); + } return evalPositive($op, $object_value, $policy_value); - case Operator::NOT_EQ: // NOTE: not_starts_with and not_ends_with should be a single value!!!!! case Operator::NOT_STARTS_WITH: @@ -81,8 +85,8 @@ function eval_binary_operator(string $op, string $field, $policy_value, ObjectSe throw new Exception("wrong policy values! should not be an array"); } return evalNegative($op, $object_value, $policy_value); - // policy value is an array case Operator::NOT_IN: + // NOTE: policy value is an array if (!is_array($policy_value)) { throw new Exception("the policy value of operator `not_in` should be an array"); } @@ -92,6 +96,9 @@ function eval_binary_operator(string $op, string $field, $policy_value, ObjectSe if (!is_array($object_value)) { throw new Exception("the object attribute should be array of operator `contains`"); } + if (is_array($policy_value)) { + throw new Exception("wrong policy values! should not be an array"); + } return evalNegative($op, $object_value, $policy_value); default: return false; @@ -113,6 +120,7 @@ function eval_binary_operator(string $op, string $field, $policy_value, ObjectSe Operator::NOT_IN =>"IAM\Evaluation\cmp_not_in", Operator::CONTAINS => "IAM\Evaluation\cmp_contains", Operator::NOT_CONTAINS => "IAM\Evaluation\cmp_not_contains", + Operator::STRING_CONTAINS => "IAM\Evaluation\cmp_string_contains", ]; @@ -132,17 +140,6 @@ function evalPositive(string $op, $object_value, $policy_value): bool // contains object_value is array, policy is a single value or an array; if ($op == Operator::CONTAINS) { - if (is_array($policy_value)) { - foreach ($policy_value as $pv) { - // got one contains, return true; - if ($cmp_func($object_value, $pv)) { - return true; - } - } - // all not contains, return false; - return false; - } - return $cmp_func($object_value, $policy_value); } @@ -174,17 +171,6 @@ function evalNegative(string $op, $object_value, $policy_value): bool // contains object_value is array, policy is a single value or an array; if ($op == Operator::NOT_CONTAINS) { - if (is_array($policy_value)) { - foreach ($policy_value as $pv) { - // got one contains, return false; - if (!$cmp_func($object_value, $pv)) { - return false; - } - } - // all not contains, return true; - return true; - } - return $cmp_func($object_value, $policy_value); } diff --git a/src/Evaluation/Operator.php b/src/Evaluation/Operator.php index c4e87b6..832286e 100644 --- a/src/Evaluation/Operator.php +++ b/src/Evaluation/Operator.php @@ -39,6 +39,8 @@ class Operator const ENDS_WITH = "ends_with"; const NOT_ENDS_WITH = "not_ends_with"; + const STRING_CONTAINS = "string_contains"; + const LT = "lt"; const LTE = "lte"; const GT = "gt"; diff --git a/tests/Evaluation/ExprCellTest.php b/tests/Evaluation/ExprCellTest.php index 8c75d9a..a06327b 100644 --- a/tests/Evaluation/ExprCellTest.php +++ b/tests/Evaluation/ExprCellTest.php @@ -14,6 +14,7 @@ namespace IAM\Tests; +use Exception; use IAM\Evaluation\ExprCell; use IAM\Evaluation\ObjectSet; use JsonMapper; @@ -42,6 +43,15 @@ protected function getObject(string $id): ObjectSet ]); return $a; } + protected function getObjectWithArrayIDs(array $ids): ObjectSet + { + $a = new ObjectSet(); + $a->add("obj", [ + "id" => $ids, + ]); + return $a; + } + protected function getObjectWithAttribute(string $id, array $attribute): ObjectSet { $a = new ObjectSet(); @@ -65,6 +75,13 @@ public function testEqual(): void $obj200 = $this->getObject("200"); $this->assertFalse($expr->eval($obj200)); + + // array + $objArrayHit = $this->getObjectWithArrayIDs(["100", "200"]); + $this->assertTrue($expr->eval($objArrayHit)); + + $objArrayMiss= $this->getObjectWithArrayIDs(["200", "300"]); + $this->assertFalse($expr->eval($objArrayMiss)); } public function testIn(): void @@ -85,6 +102,53 @@ public function testIn(): void $obj300 = $this->getObject("300"); $this->assertFalse($expr->eval($obj300)); + + // array + $objArrayHit = $this->getObjectWithArrayIDs(["200", "300"]); + $this->assertTrue($expr->eval($objArrayHit)); + + $objArrayMiss= $this->getObjectWithArrayIDs(["300", "400"]); + $this->assertFalse($expr->eval($objArrayMiss)); + } + + public function testContains(): void + { + $policy = [ + 'op' => 'contains', + 'field' => 'obj.id', + 'value' => '100', + ]; + + $expr = $this->makeExpression($policy); + + // single value, false + // $obj100 = $this->getObject("100"); + // $this->assertFalse($expr->eval($obj100)); + // $this-> + + // array, hit + $objHit= $this->getObjectWithArrayIDs(['100', '200']); + $this->assertTrue($expr->eval($objHit)); + // array, miss + $objMiss = $this->getObjectWithArrayIDs(["200", "300"]); + $this->assertFalse($expr->eval($objMiss)); + } + + public function testContainsException(): void + { + $policy = [ + 'op' => 'contains', + 'field' => 'obj.id', + 'value' => '100', + ]; + + $expr = $this->makeExpression($policy); + + // single value, will raise exception + $obj100 = $this->getObject("100"); + $this->expectException(Exception::class); + $expr->eval($obj100); + } public function testStartsWith(): void @@ -139,6 +203,46 @@ public function testStartsWith(): void $this->assertFalse($expr->eval($obj1)); } + public function testStringContains(): void + { + $policy = [ + 'op' => 'string_contains', + 'field' => 'obj._bk_iam_path_', + 'value' => '/set,2/', + ]; + + $expr = $this->makeExpression($policy); + + // hit, ok + $obj1 = $this->getObjectWithAttribute("1", [ + '_bk_iam_path_' => ['/biz,1/set,2/host,3/'], + ]); + $this->assertTrue($expr->eval($obj1)); + + // not hit, false + $obj1 = $this->getObjectWithAttribute("1", [ + '_bk_iam_path_' => ['/biz,1/set,3/bbb,4/'], + ]); + $this->assertFalse($expr->eval($obj1)); + + // empty array, false + $obj1 = $this->getObjectWithAttribute("1", [ + '_bk_iam_path_' => [], + ]); + $this->assertFalse($expr->eval($obj1)); + + // empty string, false + $obj1 = $this->getObjectWithAttribute("1", [ + '_bk_iam_path_' => '', + ]); + $this->assertFalse($expr->eval($obj1)); + + // has no that attribute, false + $obj1 = $this->getObjectWithAttribute("1", [ + ]); + $this->assertFalse($expr->eval($obj1)); + } + public function testOR(): void { From bd078e7592555854476ae1d2ffef8dfd629031fd Mon Sep 17 00:00:00 2001 From: wklken Date: Tue, 26 Jul 2022 11:25:56 +0800 Subject: [PATCH 2/2] test(update): update unittest --- src/Evaluation/CompareFunction.php | 7 +-- tests/Evaluation/ExprCellTest.php | 79 ++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/src/Evaluation/CompareFunction.php b/src/Evaluation/CompareFunction.php index f278db3..f1d2160 100644 --- a/src/Evaluation/CompareFunction.php +++ b/src/Evaluation/CompareFunction.php @@ -56,9 +56,10 @@ function cmp_starts_with($v1, $v2): bool function cmp_not_starts_with($v1, $v2): bool { if (is_null($v1)) { - return false; + return true; } - return !(strpos($v1, $v2) === 0); + + return strpos($v1, $v2) !== 0; } function cmp_ends_with($v1, $v2): bool @@ -75,7 +76,7 @@ function cmp_ends_with($v1, $v2): bool function cmp_not_ends_with($v1, $v2): bool { if (is_null($v1)) { - return false; + return true; } $length = strlen($v2); return $length > 0 ? substr($v1, -$length) !== $v2: true; diff --git a/tests/Evaluation/ExprCellTest.php b/tests/Evaluation/ExprCellTest.php index a06327b..6741ba2 100644 --- a/tests/Evaluation/ExprCellTest.php +++ b/tests/Evaluation/ExprCellTest.php @@ -243,6 +243,85 @@ public function testStringContains(): void $this->assertFalse($expr->eval($obj1)); } + public function testEndsWith(): void + { + $policy = [ + 'op' => 'ends_with', + 'field' => 'obj._bk_iam_path_', + 'value' => '/set,2/', + ]; + + $expr = $this->makeExpression($policy); + + // hit, ok + $obj1 = $this->getObjectWithAttribute("1", [ + '_bk_iam_path_' => ['/biz,1/set,2/'], + ]); + $this->assertTrue($expr->eval($obj1)); + + // not hit, false + $obj1 = $this->getObjectWithAttribute("1", [ + '_bk_iam_path_' => ['/biz,1/set,3/'], + ]); + $this->assertFalse($expr->eval($obj1)); + + // empty array, false + $obj1 = $this->getObjectWithAttribute("1", [ + '_bk_iam_path_' => [], + ]); + $this->assertFalse($expr->eval($obj1)); + + // empty string, false + $obj1 = $this->getObjectWithAttribute("1", [ + '_bk_iam_path_' => '', + ]); + $this->assertFalse($expr->eval($obj1)); + + // has no that attribute, false + $obj1 = $this->getObjectWithAttribute("1", [ + ]); + $this->assertFalse($expr->eval($obj1)); + } + + public function testNotEndsWith(): void + { + $policy = [ + 'op' => 'not_ends_with', + 'field' => 'obj._bk_iam_path_', + 'value' => '/set,2/', + ]; + + $expr = $this->makeExpression($policy); + + // hit, ok + $obj1 = $this->getObjectWithAttribute("1", [ + '_bk_iam_path_' => ['/biz,1/set,3/'], + ]); + $this->assertTrue($expr->eval($obj1)); + + // not hit, false + $obj1 = $this->getObjectWithAttribute("1", [ + '_bk_iam_path_' => ['/biz,1/set,2/'], + ]); + $this->assertFalse($expr->eval($obj1)); + + // empty array, true + $obj1 = $this->getObjectWithAttribute("1", [ + '_bk_iam_path_' => [], + ]); + $this->assertTrue($expr->eval($obj1)); + + // empty string, true + $obj1 = $this->getObjectWithAttribute("1", [ + '_bk_iam_path_' => '', + ]); + $this->assertTrue($expr->eval($obj1)); + + // has no that attribute, false + $obj1 = $this->getObjectWithAttribute("1", [ + ]); + $this->assertTrue($expr->eval($obj1)); + } public function testOR(): void {