feat: 子流程功能token校验新增作用域资源 --story=127611598 #492
Conversation
# Reviewed, transaction id: 66281
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
代码审查总结
本次 PR 新增了 SCOPE 作用域权限管理功能，整体逻辑清晰。发现以下需要注意的问题：
关键问题
- 🚨 异常处理不完整：
scope_exists方法捕获异常后返回 False，但check_parent_task_id中未处理Template.DoesNotExist ⚠️ 逻辑遗漏：新增的ScopePermission.has_permission方法缺少返回值- ⚡ 性能隐患：
check_parent_task_id在 SCOPE 类型判断中直接使用.get()可能抛出异常并中断认证流程
建议
- 补充异常处理和缺失的返回值
- 考虑对 SCOPE 验证逻辑添加单元测试
| return Template.objects.filter( | ||
| space_id=self.space_id, scope_type=scope_type, scope_value=scope_value | ||
| ).exists() | ||
| except (ValueError, IndexError): |
There was a problem hiding this comment.
🔒 捕获过于宽泛的异常可能隐藏真实错误。建议只捕获预期的异常类型（如 AttributeError）。
bkflow/permission/models.py
Outdated
| if resource_type != ResourceType.TASK.value: | ||
| if resource_type == ResourceType.SCOPE.value: | ||
| scope_parts = db_token.resource_id.split("_") | ||
| template_obj = Template.objects.get(id=resource_id) |
There was a problem hiding this comment.
🚨 Template.objects.get() 可能抛出 DoesNotExist 异常，导致认证失败。建议使用 filter().first() 或添加 try-except。
|
|
||
| def has_permission(self, request, view): | ||
| if view.action in view.MOCK_ABOVE_ACTIONS: | ||
| return False |
There was a problem hiding this comment.
| EDIT_ABOVE_ACTIONS = ["update"] | ||
| MOCK_ABOVE_ACTIONS = ["preview_task_tree", "create_mock_task"] | ||
| permission_classes = [AdminPermission | SpaceSuperuserPermission | TemplatePermission | TemplateMockPermission] | ||
| MOCK_ABOVE_ACTIONS = ["create_mock_task"] |
There was a problem hiding this comment.
| def template_exists(self, template_id): | ||
| return Template.exists(template_id) | ||
|
|
||
| def scope_exists(self, scope_data): |
There was a problem hiding this comment.
如果使用了下划线来分给，那我们在scope_type的所有校验逻辑里，就要禁止用户传入“_”作为scope_type的值
bkflow/permission/models.py
Outdated
| if resource_type != ResourceType.TASK.value: | ||
| if resource_type == ResourceType.SCOPE.value: | ||
| scope_parts = db_token.resource_id.split("_") | ||
| template_obj = Template.objects.get(id=resource_id) |
There was a problem hiding this comment.
scope类型的权限，不仅仅能用于template，任务也是可以的
# Reviewed, transaction id: 66534
There was a problem hiding this comment.
审查摘要
本次新增了 SCOPE 类型的 Token 资源验证功能。发现以下需要关注的问题：
🚨 严重问题 (2个)
scope_exists方法中数组越界风险未充分防护Token.verify中 SCOPE 验证逻辑缺少空值校验
split("_")未限制分割次数，可能产生意外结果ScopePermission.has_permission缺少返回值- 缺少针对 SCOPE 资源类型的单元测试
建议: 优先修复数组访问安全问题和补充缺失的返回值，然后增加边界情况测试覆盖。
已标注 5 条行内评论，请查看具体建议。
| if len(scope_parts) < 2: | ||
| return False | ||
|
|
||
| scope_type, scope_value = scope_parts[0], scope_parts[1] |
There was a problem hiding this comment.
🚨 此行存在数组越界风险：当 scope_data='single' 时，len 检查通过但 scope_parts[1] 会抛出 IndexError。应在第 60 行改为 != 2。
| return False | ||
|
|
||
| scope_parts = scope_data.split("_") | ||
| if len(scope_parts) < 2: |
There was a problem hiding this comment.
⚡ split('') 未限制分割次数，当 scope_data='a_b_c' 时会产生 3 个元素。建议使用 split('', 1) 限制最多分割成 2 部分。
| if db_token.resource_id != str(resource_id): | ||
| if resource_type != ResourceType.TASK.value: | ||
| if resource_type == ResourceType.SCOPE.value: | ||
| scope_parts = db_token.resource_id.split("_") |
There was a problem hiding this comment.
| return "SCOPE" | ||
|
|
||
| def has_permission(self, request, view): | ||
| if view.action in view.MOCK_ABOVE_ACTIONS: |
There was a problem hiding this comment.
🚨 此方法缺少 return 语句，会隐式返回 None 导致权限校验失败。应返回明确的布尔值或调用父类方法。
| resource_data = result["data"] | ||
| return resource_data["scope_type"] == scope_type and resource_data["scope_value"] == scope_value | ||
| elif resource_type == ResourceType.TEMPLATE.value: | ||
| return False |
There was a problem hiding this comment.
* feat: 子流程功能token校验新增作用域资源 --story=127611598 # Reviewed, transaction id: 66281 * fix: 修复逻辑校验问题 --story=127611598 # Reviewed, transaction id: 66534
2 participants
Reviewed, transaction id: 66281