Skip to content

fix: workspace model scope permission error #7787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
peslyn merged 1 commit into from
Nov 10, 2026
Merged

fix: workspace model scope permission error #7787

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions apps/common/auth/constants/permission_constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ class PermissionConstants(Enum):
PermissionMeta(
role_list=[RoleConstants.WORKSPACE_MANAGE, RoleConstants.USER],
category=Category.WORKSPACE,
scope=[PermissionScopeConstants.WORKSPACE],
scope=[PermissionScopeConstants.WORKSPACE, PermissionScopeConstants.WORKSPACE_RESOURCE],
resource_permission_group_list=[ResourcePermissionConst.MODEL_MANAGE],
),
)
Expand All @@ -249,7 +249,7 @@ class PermissionConstants(Enum):
PermissionMeta(
role_list=[RoleConstants.WORKSPACE_MANAGE, RoleConstants.USER],
category=Category.WORKSPACE,
scope=[PermissionScopeConstants.WORKSPACE_RESOURCE],
scope=[PermissionScopeConstants.WORKSPACE, PermissionScopeConstants.WORKSPACE_RESOURCE],
resource_permission_group_list=[ResourcePermissionConst.MODEL_MANAGE],
),
)
Expand All @@ -259,7 +259,7 @@ class PermissionConstants(Enum):
PermissionMeta(
role_list=[RoleConstants.WORKSPACE_MANAGE, RoleConstants.USER],
category=Category.WORKSPACE,
scope=[PermissionScopeConstants.WORKSPACE_RESOURCE],
scope=[PermissionScopeConstants.WORKSPACE, PermissionScopeConstants.WORKSPACE_RESOURCE],
resource_permission_group_list=[ResourcePermissionConst.MODEL_MANAGE],
),
)
Expand All @@ -279,7 +279,7 @@ class PermissionConstants(Enum):
PermissionMeta(
role_list=[RoleConstants.WORKSPACE_MANAGE, RoleConstants.USER],
category=Category.WORKSPACE,
scope=[PermissionScopeConstants.WORKSPACE_RESOURCE],
scope=[PermissionScopeConstants.WORKSPACE, PermissionScopeConstants.WORKSPACE_RESOURCE],
resource_permission_group_list=[ResourcePermissionConst.MODEL_MANAGE],
),
)

@shaohuzhang1 shaohuzhang1 Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is an analysis of the provided code diff:

1. Irregularities & Issues

  • Redundant Scope Addition: In three out of the four hunks, PermissionScopeConstants.WORKSPACE is added to a list that already contained PermissionScopeConstants.WORKSPACE_RESOURCE. Depending on the permission logic, having both WORKSPACE and WORKSPACE_RESOURCE might be semantically redundant or overly broad. If WORKSPACE implies all resources within it, adding WORKSPACE_RESOURCE explicitly may be unnecessary. Verify if this double-scoping causes unintended permission grants.
  • Inconsistency in First Hunk: The first hunk previously had only WORKSCOPE, while the others had WORKSPACE_RESOURCE. The new state makes them identical ([WORKSPACE, WORKSPACE_RESOURCE]). Ensure this is intentional and not a copy-paste error obscuring a need for distinct scopes across these four permissions.

2. Optimization Suggestions

  • Reduce Duplication (DRY Principle): Four separate PermissionMeta entries appear to have nearly identical configurations:
    • Same roles: [WORKSPACE_MANAGE, USER]
    • Same category: WORKSPACE
    • Same scope: [WORKSPACE, WORKSPACE_RESOURCE]
    • Same resource group: [MODEL_MANAGE]
    • Suggestion: If these entries differ only by their Enum name/key, consider refactoring to avoid repeating the PermissionMeta block four times. You could iterate over a list of keys or use a helper function/method to reduce boilerplate.
  • Data Structure Efficiency: If multiple permissions share identical scope/role/category/resource settings, consider grouping them under a single permission definition with multiple aliases or a combined resource group, rather than duplicating the entire meta object.

3. Potential Logic Risks

  • Permission Escalation: Adding WORKSPACE scope to entries that previously only had WORKSPACE_RESOURCE likely broadens the permission. USER roles now potentially get broader workspace-wide access where they previously had more restricted resource-specific access. Confirm this is the desired security outcome.

Summary

The change is syntactically correct but raises security concerns due to broadened permissions and maintenance concerns due to repetitive code blocks. Review whether the expanded scope is necessary for all four permissions.

Expand Down
Loading