Skip to content

fix: system portal permission error #7808

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
+1 −1
Merged

fix: system portal permission error #7808

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
2 changes: 1 addition & 1 deletion apps/portal/views/portal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class PortalView(APIView):
responses=PortalAPI.Get.get_response(),
tags=[_("Portal")], # type: ignore
)
@has_permissions(PermissionConstants.PORTAL_EDIT, RoleConstants.ADMIN)
@has_permissions(PermissionConstants.PORTAL_READ, RoleConstants.ADMIN)
def get(self, request: Request):
return result.success(PortalSerializer().one())

@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.

The diff itself looks correct and is a good security/permission fix: a read-only GET should require PORTAL_READ, not PORTAL_EDIT. No syntax or obvious functional irregularity is introduced by this change.

Things to verify:

  1. @has_permissions semantics

    • If it requires all arguments, then PORTAL_READ and ADMIN are both required. That may be intentional, but confirm it does not accidentally restrict access to admins only.
    • If it uses OR semantics, then any ADMIN bypasses PORTAL_READ. Confirm that is acceptable.
    • Mixing a permission constant and a role constant in the same decorator should be checked against the decorator implementation.

  2. PermissionConstants.PORTAL_READ exists and is registered

    • Ensure the constant is defined, included in migrations/seeding, and recognized by the permission-checking backend.

  3. Read permission is sufficient

    • PortalSerializer().one() should be truly read-only. If it has side effects such as writes, cache invalidation, audit logging, or lazy creation, then PORTAL_READ may be semantically wrong.

  4. Sensitive data exposure

    • Confirm that users with PORTAL_READ are allowed to see all fields returned by PortalSerializer.

  5. Tests
    Add/update tests for:

    • Admin access
    • User with PORTAL_READ access
    • User with only PORTAL_EDIT denied
    • Anonymous/unauthenticated denied
    • OpenAPI/schema generation still works

Optimization suggestions, if applicable:

  • If PortalSerializer().one() is called frequently and returns a singleton portal configuration, consider caching it.
  • If the serializer accesses related models, use select_related/prefetch_related to avoid N+1 queries.
  • Ensure the permission decorator preserves function metadata for schema generation, especially with @extend_schema.

Overall: the change is appropriate. The main risk is only in the exact semantics of @has_permissions and whether mixing PORTAL_READ with RoleConstants.ADMIN matches the intended access policy.

Expand Down
Loading