Skip to content

Commit

Permalink
Bugfix group auth (#1587)
Browse files Browse the repository at this point in the history
* fix: fix group costom policy auth action without resource type bug

* docs: v1.9.3

* feat: update BKAPP_SUBJECT_GRADE_MANAGER_LIMIT default to 500
  • Loading branch information
zhu327 authored Nov 2, 2022
1 parent ae70ca6 commit 2b95d40
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 29 deletions.
2 changes: 1 addition & 1 deletion saas/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.9.2
1.9.3
5 changes: 2 additions & 3 deletions saas/backend/service/models/policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -562,12 +562,11 @@ def calculate_pre_changed_content(self, system_id: str, old: "UniversalPolicy")

def to_resource_expression(self, system_id: str) -> str:
"""将ABAC权限翻译为后台所需表达式"""
assert len(self.expression_resource_groups) > 0
translator = ResourceExpressionTranslator()
return translator.translate(system_id, self.expression_resource_groups.dict())

def has_abac(self) -> bool:
return len(self.expression_resource_groups) > 0
return self.auth_type in (AuthTypeEnum.ABAC.value, AuthTypeEnum.ALL.value)

def has_rbac(self) -> bool:
return len(self.instances) > 0
return self.auth_type in (AuthTypeEnum.RBAC.value, AuthTypeEnum.ALL.value)
4 changes: 3 additions & 1 deletion saas/backend/service/utils/translate.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ def translate(self, system_id: str, resource_groups: List[Dict]) -> str:
]
"""
content = [self._translate_resource_group(system_id, r) for r in resource_groups]
if len(content) == 1:
if len(content) == 0:
expression: Any = content
elif len(content) == 1:
expression = content[0]
else:
expression = {"OR": {"content": content}}
Expand Down
2 changes: 1 addition & 1 deletion saas/config/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@
SUBJECT_AUTHORIZATION_LIMIT = {
# -------- 用户 ---------
# 用户能加入的分级管理员的最大数量
"subject_grade_manager_limit": env.int("BKAPP_SUBJECT_GRADE_MANAGER_LIMIT", default=100),
"subject_grade_manager_limit": env.int("BKAPP_SUBJECT_GRADE_MANAGER_LIMIT", default=500),
# -------- 用户组 ---------
# 用户组能加入同一个系统的权限模板的最大数量
"default_subject_system_template_limit": env.int("BKAPP_DEFAULT_SUBJECT_SYSTEM_TEMPLATE_LIMIT", default=10),
Expand Down
4 changes: 4 additions & 0 deletions saas/resources/version_log/V1.9.3_2022-11-2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# V1.9.3 版本更新日志

### 缺陷修复
* 用户组授权不关联资源类型的操作未生效问题
4 changes: 4 additions & 0 deletions saas/resources/version_log/V1.9.3_2022-11-2_en.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# V1.9.3 ChangeLog

### Bug Fixes
* The problem that the operation of user group authorization not associated with resource type does not take effect
36 changes: 17 additions & 19 deletions saas/tests/service/models/policy_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,44 +243,42 @@ def test_is_absolute_abac(self, resource_groups, action_auth_type, expected):
assert is_absolute_abac == expected

@pytest.mark.parametrize(
"expression_resource_groups,expected",
"auth_type,expected",
[
([], False),
([ResourceGroup(related_resource_types=[])], True),
([ResourceGroup(related_resource_types=[]), ResourceGroup(related_resource_types=[])], True),
("rbac", False),
("none", False),
("abac", True),
("all", True),
],
)
def test_has_abac(self, expression_resource_groups, expected):
def test_has_abac(self, auth_type, expected):
p = UniversalPolicy(
action_id="a",
policy_id=0,
expired_at=0,
resource_groups=ResourceGroupList(__root__=[]),
expression_resource_groups=ResourceGroupList(__root__=expression_resource_groups),
expression_resource_groups=ResourceGroupList(__root__=[]),
auth_type=auth_type,
)
assert p.has_abac() == expected

@pytest.mark.parametrize(
"instances,expected",
"auth_type,expected",
[
([], False),
([PathNode(id="id", name="name", system_id="s_id", type="rt_id")], True),
(
[
PathNode(id="id1", name="name1", system_id="s_id1", type="rt_id1"),
PathNode(id="id2", name="name2", system_id="s_id2", type="rt_id2"),
],
True,
),
("rbac", True),
("none", False),
("abac", False),
("all", True),
],
)
def test_has_rbac(self, instances, expected):
def test_has_rbac(self, auth_type, expected):
p = UniversalPolicy(
action_id="a",
policy_id=0,
expired_at=0,
resource_groups=ResourceGroupList(__root__=[]),
instances=instances,
instances=[],
auth_type=auth_type,
)
assert p.has_rbac() == expected

Expand Down Expand Up @@ -348,7 +346,7 @@ def test_parse_abac_and_rbac(self, related_resource, expected):
[],
),
# old(auth_type, abac_data, rbac_data)
(AuthTypeEnum.ABAC.value, [], []),
(AuthTypeEnum.NONE.value, [], []),
# expected(auth_type, abac_data, rbac_data)
(
AuthTypeEnum.ABAC.value,
Expand Down
36 changes: 32 additions & 4 deletions saas/tests/service/policy/common_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
import pytest
from mock import patch

from backend.service.models import Policy, UniversalPolicyChangedContent
from backend.service.constants import AbacPolicyChangeType
from backend.service.models import AbacPolicyChangeContent, Policy, UniversalPolicyChangedContent
from backend.service.policy.common import UniversalPolicyChangedContentAnalyzer


Expand All @@ -28,7 +29,17 @@ class TestUniversalPolicyChangedContentAnalyzer:
# mock_action_auth_types
{"test": "abac"},
# expected_result
[UniversalPolicyChangedContent(action_id="test", auth_type="abac", abac=None, rbac=None)],
[
UniversalPolicyChangedContent(
action_id="test",
auth_type="abac",
abac=AbacPolicyChangeContent(
change_type=AbacPolicyChangeType.CREATED.value,
resource_expression="[]",
),
rbac=None,
)
],
),
],
)
Expand All @@ -50,7 +61,14 @@ def test_cal_for_created(self, create_policies, mock_action_auth_types, expected
# mock_action_auth_types
{"test": "abac"},
# expected_result
[UniversalPolicyChangedContent(action_id="test", auth_type="none", abac=None, rbac=None)],
[
UniversalPolicyChangedContent(
action_id="test",
auth_type="none",
abac=AbacPolicyChangeContent(change_type=AbacPolicyChangeType.DELETED.value, id=0),
rbac=None,
)
],
),
],
)
Expand All @@ -75,7 +93,17 @@ def test_cal_cal_for_deleted(self, delete_policies, mock_action_auth_types, expe
# mock_action_auth_types
{"test": "abac"},
# expected_result
[UniversalPolicyChangedContent(action_id="test", auth_type="abac", abac=None, rbac=None)],
[
UniversalPolicyChangedContent(
action_id="test",
auth_type="abac",
abac=AbacPolicyChangeContent(
change_type=AbacPolicyChangeType.UPDATED.value,
resource_expression="[]",
),
rbac=None,
)
],
),
],
)
Expand Down

0 comments on commit 2b95d40

Please sign in to comment.