[shaohuzhang1]

fix: system user group permission error

[shaohuzhang1]

Main finding

The diff changes only authorization policy, but the added RoleConstants.WORKSPACE_MANAGE is a high-risk change.

If has_permissions(...) uses OR semantics — which is common — then any user with WORKSPACE_MANAGE can now:

• create/edit system user groups
• delete system user groups
• add members to system user groups
• remove members from system user groups

That is a potential vertical privilege escalation, especially because group membership changes can grant or remove access. A workspace-scoped role should not normally grant control over system-level IAM groups unless that is explicitly intended and enforced with object-level scoping.

If has_permissions(...) uses AND semantics, adding WORKSPACE_MANAGE may instead make these endpoints inaccessible to users who have ADMIN but not WORKSPACE_MANAGE. Either way, verify the decorator semantics and the intended policy.

Recommendations

1. Confirm has_permissions semantics — OR vs AND — before merging.

2. Avoid granting WORKSPACE_MANAGE to system-level group endpoints unless explicitly intended.

   • If workspace managers must manage only workspace-scoped groups, add object-level checks and queryset filtering.
   • Prefer a dedicated permission such as SYSTEM_USER_GROUP_*_WORKSPACE rather than mixing a workspace role into system IAM endpoints.

3. Verify RoleConstants.WORKSPACE_MANAGE exists and is imported. If it does not exist, this will fail at import/runtime with AttributeError.

4. Check the existing SystemUserGroupView decorator. It lists both SYSTEM_USER_GROUP_CREATE and SYSTEM_USER_GROUP_EDIT. If the decorator is OR-based, a user with only create permission can edit, and a user with only edit permission can create. Split permissions by HTTP method if possible.

5. Add authorization tests for:

   • ADMIN
   • WORKSPACE_MANAGE
   • users with only individual SYSTEM_USER_GROUP_* permissions
   • users with no relevant permissions
   • expected 200/403 results

6. Fix line length/formatting. The new one-line decorators are likely over 120 characters:

   • Delete line is approximately 123 chars
   • AddMember line is approximately 127 chars
   • RemoveMember line is approximately 130 chars

   Use the project formatter, e.g. Black/Ruff, and keep the decorator arguments consistently multiline.

7. Consider audit logging. Since membership changes can affect access, ensure the existing @log captures actor, target group, and affected members.

Overall: the formatting changes are minor, but the authorization change needs explicit security review before merge.