Skip to content

feat: add overview export permissions and update related components #7854

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
wxg0103 merged 1 commit into from
Nov 12, 2026
Merged

feat: add overview export permissions and update related components #7854

Show file tree
Hide file tree
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/application/views/application_stats.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,8 @@ class TokenUsageExport(APIView):
tags=[_("Application")], # type: ignore
)
@has_permissions(
PermissionConstants.APPLICATION_EXPORT.get_workspace_application_permission(),
PermissionConstants.APPLICATION_EXPORT.get_workspace_permission_workspace_manage_role(),
PermissionConstants.APPLICATION_OVERVIEW_EXPORT.get_workspace_application_permission(),
PermissionConstants.APPLICATION_OVERVIEW_EXPORT.get_workspace_permission_workspace_manage_role(),
ViewPermission(
[RoleConstants.USER.get_workspace_role()],
[PermissionConstants.APPLICATION.get_workspace_application_permission()],
Expand Down Expand Up @@ -318,8 +318,8 @@ class TopQuestionsExport(APIView):
tags=[_("Application")], # type: ignore
)
@has_permissions(
PermissionConstants.APPLICATION_EXPORT.get_workspace_application_permission(),
PermissionConstants.APPLICATION_EXPORT.get_workspace_permission_workspace_manage_role(),
PermissionConstants.APPLICATION_OVERVIEW_EXPORT.get_workspace_application_permission(),
PermissionConstants.APPLICATION_OVERVIEW_EXPORT.get_workspace_permission_workspace_manage_role(),
ViewPermission(
[RoleConstants.USER.get_workspace_role()],
[PermissionConstants.APPLICATION.get_workspace_application_permission()],
Expand Down
25 changes: 25 additions & 0 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 @@ -1882,6 +1882,16 @@ class PermissionConstants(Enum):
),
)

APPLICATION_OVERVIEW_EXPORT = (
Permission(group=Group.APPLICATION, sub_group=Group.OVERVIEW, operate=Operate.EXPORT, bit_index=43),
PermissionMeta(
role_list=[RoleConstants.WORKSPACE_MANAGE, RoleConstants.USER],
category=Category.WORKSPACE,
scope=[PermissionScopeConstants.WORKSPACE_RESOURCE, PermissionScopeConstants.WORKSPACE],
resource_permission_group_list=[ResourcePermissionConst.APPLICATION_MANAGE],
),
)

APPLICATION_OVERVIEW_BACKEND_API_ACCESS = (
Permission(
group=Group.APPLICATION,
Expand Down Expand Up @@ -3726,6 +3736,21 @@ class PermissionConstants(Enum):
),
)

RESOURCE_APPLICATION_OVERVIEW_EXPORT = (
Permission(
group=Group.SYSTEM_RES_APPLICATION,
sub_group=Group.SYSTEM_OVERVIEW,
operate=Operate.EXPORT,
bit_index=32,
),
PermissionMeta(
role_list=[RoleConstants.ADMIN],
category=Category.RESOURCE,
is_ee=is_ee,
scope=[PermissionScopeConstants.SYSTEM],
),
)

RESOURCE_APPLICATION_OVERVIEW_BACKEND_API_ACCESS = (
Permission(
group=Group.SYSTEM_RES_APPLICATION,

@shaohuzhang1 shaohuzhang1 Nov 12, 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 patch is syntactically consistent with the existing PermissionConstants enum pattern. I do not see an immediate syntax error, but the following points should be verified:

  1. bit_index uniqueness and bit width

    • Confirm bit_index=43 and bit_index=32 are not already used within the same permission group/sub-group.
    • Pay special attention to RESOURCE_APPLICATION_OVERVIEW_EXPORT with bit_index=32. If permission masks are stored as 32-bit signed integers, bit index 32 (and 43) will overflow. Ensure the mask is stored as uint64/bigint or otherwise supports these indexes.

  2. Authorization scope / least privilege

    • APPLICATION_OVERVIEW_EXPORT grants export to RoleConstants.USER. Confirm this is intended and that export does not expose sensitive workspace data to all users. If it does, consider restricting to WORKSPACE_MANAGE or a more specific role.
    • Confirm scope=[WORKSPACE_RESOURCE, WORKSPACE] is correct. If export is workspace-level only, including WORKSPACE_RESOURCE may unintentionally allow resource-scoped access.

  3. PermissionMeta consistency

    • The first entry does not set is_ee, while the second sets is_ee=is_ee. Confirm this difference is intentional and matches neighboring permissions.
    • Confirm resource_permission_group_list=[ResourcePermissionConst.APPLICATION_MANAGE] is required for the workspace export and that the constant name is correct.
    • Confirm the resource permission does not also need a resource_permission_group_list.

  4. Naming and placement

    • The names and placement before the corresponding BACKEND_API_ACCESS entries look consistent with the existing pattern.
    • No obvious optimization is needed; using a helper/factory would only be worthwhile if this enum pattern is repeated heavily.

  5. Tests / enforcement

    • Add or verify tests for:
      • unique bit_index values,
      • correct role/scope/category metadata,
      • API enforcement of the new export permissions,
      • EE-only behavior for RESOURCE_APPLICATION_OVERVIEW_EXPORT.
    • If permissions are cached or persisted elsewhere, ensure the new permissions are refreshed/migrated appropriately.

Overall: no definite bug from the diff alone, but the main risks are bit-index collision/overflow and overly broad export authorization for RoleConstants.USER.

Expand Down
71 changes: 71 additions & 0 deletions apps/models_provider/tests/test_model_workspace_uniqueness.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
from unittest.mock import MagicMock, patch

from django.test import TestCase

from common.exception.app_exception import AppApiException
from models_provider.models import Model, Status
from models_provider.serializers.model_serializer import ModelSerializer
from users.models import User

MODEL_NAME = "zws-LLM-dp001"


class ModelWorkspaceUniquenessTests(TestCase):
def setUp(self):
self.user = User.objects.create(
username="model-workspace-uniqueness-user",
nick_name="Model workspace uniqueness user",
password="password",
role="ADMIN",
)
self.provider = MagicMock()
self.provider.get_model_type_list.return_value = [{"value": "LLM"}]

def create_model(self, workspace_id):
return Model.objects.create(
name=MODEL_NAME,
status=Status.SUCCESS,
model_type="LLM",
model_name="deepseek-chat",
user=self.user,
provider="test_provider",
credential="encrypted-credential",
model_params_form=[],
workspace_id=workspace_id,
)

def serializer_data(self, workspace_id):
return {
"user_id": str(self.user.id),
"name": MODEL_NAME,
"provider": "test_provider",
"model_type": "LLM",
"model_name": "deepseek-chat",
"model_params_form": [],
"credential": {"api_key": "secret"},
"workspace_id": workspace_id,
}

def test_create_same_name_in_other_workspace_uses_insert_workspace(self):
self.create_model("workspace-a")

with (
patch(
"models_provider.serializers.model_serializer.ModelProvideConstants", {"test_provider": self.provider}
),
patch("models_provider.serializers.model_serializer.UserResourcePermissionSerializer"),
):
result = ModelSerializer.Create(data=self.serializer_data("workspace-a")).insert("workspace-b")

self.assertEqual("workspace-b", Model.objects.get(id=result["id"]).workspace_id)

def test_create_same_name_in_same_workspace_is_rejected(self):
self.create_model("workspace-a")

with (
patch(
"models_provider.serializers.model_serializer.ModelProvideConstants", {"test_provider": self.provider}
),
self.assertRaises(AppApiException),
):
ModelSerializer.Create(data=self.serializer_data("workspace-a")).insert("workspace-a")

@shaohuzhang1 shaohuzhang1 Nov 12, 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.

Overall, the tests cover the intended behavior, but they are somewhat brittle and may pass/fail for the wrong reason.

Key issues and suggestions:

  1. Inconsistent permission mocking

    • UserResourcePermissionSerializer is patched in the first test but not the second.
    • If permission validation runs before the duplicate-name check in the second test, the test may hit real permission logic and fail for an unrelated reason.
    • Patch it consistently in both tests, or move the patch into a helper/setUp context.

  2. MagicMock provider is too permissive

    • self.provider = MagicMock() will return truthy mocks for any unexpected method call, which can hide bugs.
    • Prefer MagicMock(spec=...) / create_autospec(...) and configure every provider method used by the serializer.
    • At minimum, document which provider contract the test depends on.

  3. Permission serializer mock is also too permissive

    • Patching the class without configuring is_valid, validated_data, etc. may silently pass validation.
    • Configure the mocked return values explicitly so the test verifies the intended path.

  4. Plaintext password

    • User.objects.create(password="password", ...) stores an unhashed password in tests.
    • Prefer User.objects.create_user(...) or call set_password(...) if using a custom user model.

  5. Hardcoded role string

    • role="ADMIN" is a magic string. Use the project’s role enum/constant if available.

  6. Workspace IDs may not match the field type

    • "workspace-a" / "workspace-b" are fine only if workspace_id is a text field.
    • If workspace_id is a UUID field, use valid UUID strings.

  7. Second test should assert no row was created

    • Add a count assertion after assertRaises: 
      self.assertEqual(Model.objects.filter(workspace_id="workspace-a", name=MODEL_NAME).count(), 1)
    • Also assert the exact exception message/code if possible, e.g. assertRaisesRegex(...), so it does not pass because of an unrelated AppApiException.

  8. First test should assert full side effects

    • Checking only result["id"] workspace is good, but also assert counts: 
      self.assertEqual(Model.objects.filter(workspace_id="workspace-a", name=MODEL_NAME).count(), 1)
self.assertEqual(Model.objects.filter(workspace_id="workspace-b", name=MODEL_NAME).count(), 1)

  9. Narrow the failing operation

    • In the rejection test, construct the serializer outside the assertRaises context if possible, and assert that only .insert(...) raises. This avoids passing because of an unrelated construction/validation error.
    • If construction must happen inside, assert the exception message/code.

  10. Minor optimization

    • Since the user is only read in these tests, consider creating it with setUpTestData instead of setUp to reduce repeated DB setup.
    • Keep self.provider in setUp because it is a lightweight mock.

  11. Patch target may be brittle

    • patch("models_provider.serializers.model_serializer.ModelProvideConstants", ...) is fine only if the serializer references that exact module attribute. If it imports the constant directly elsewhere, the patch may not apply. Confirm the patch target matches the lookup location.

Recommended shape:

with (
    patch("models_provider.serializers.model_serializer.ModelProvideConstants",
          {"test_provider": self.provider}),
    patch("models_provider.serializers.model_serializer.UserResourcePermissionSerializer") as permission_serializer,
):
    permission_serializer.return_value.is_valid.return_value = True
    permission_serializer.return_value.validated_data = {}
    ...

Bottom line: the test intent is good, but the mocks are too broad, permission patching is inconsistent, and the rejection test should verify that the duplicate-name rule is the actual cause of failure.

2 changes: 2 additions & 0 deletions ui/src/permission/core/data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ export const PermissionConstants = {
APPLICATION_OVERVIEW_DISPLAY: p(Group.APPLICATION, Group.OVERVIEW, Operate.DISPLAY, 26),
APPLICATION_OVERVIEW_API_KEY: p(Group.APPLICATION, Group.OVERVIEW, Operate.API_KEY, 27),
APPLICATION_OVERVIEW_PUBLIC: p(Group.APPLICATION, Group.OVERVIEW, Operate.PUBLIC_ACCESS, 28),
APPLICATION_OVERVIEW_EXPORT: p(Group.APPLICATION, Group.OVERVIEW, Operate.EXPORT, 43),
APPLICATION_OVERVIEW_BACKEND_API_ACCESS: p(Group.APPLICATION, Group.OVERVIEW, Operate.BACKEND_API_ACCESS, 38),
APPLICATION_ACCESS_READ: p(Group.APPLICATION, Group.ACCESS, Operate.READ, 29),
APPLICATION_ACCESS_EDIT: p(Group.APPLICATION, Group.ACCESS, Operate.EDIT, 30),
Expand Down Expand Up @@ -387,6 +388,7 @@ export const PermissionConstants = {
RESOURCE_APPLICATION_OVERVIEW_DISPLAY: p(Group.SYSTEM_RES_APPLICATION, Group.SYSTEM_OVERVIEW, Operate.DISPLAY, 17),
RESOURCE_APPLICATION_OVERVIEW_API_KEY: p(Group.SYSTEM_RES_APPLICATION, Group.SYSTEM_OVERVIEW, Operate.API_KEY, 18),
RESOURCE_APPLICATION_OVERVIEW_PUBLIC: p(Group.SYSTEM_RES_APPLICATION, Group.SYSTEM_OVERVIEW, Operate.PUBLIC_ACCESS, 19),
RESOURCE_APPLICATION_OVERVIEW_EXPORT: p(Group.SYSTEM_RES_APPLICATION, Group.SYSTEM_OVERVIEW, Operate.EXPORT, 32),
RESOURCE_APPLICATION_OVERVIEW_BACKEND_API_ACCESS: p(Group.SYSTEM_RES_APPLICATION, Group.SYSTEM_OVERVIEW, Operate.BACKEND_API_ACCESS, 29),
RESOURCE_APPLICATION_ACCESS_READ: p(Group.SYSTEM_RES_APPLICATION, Group.SYSTEM_ACCESS, Operate.READ, 20),
RESOURCE_APPLICATION_ACCESS_EDIT: p(Group.SYSTEM_RES_APPLICATION, Group.SYSTEM_ACCESS, Operate.EDIT, 21),
Expand Down
4 changes: 4 additions & 0 deletions ui/src/permission/modules/application/system.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ const system = {
* 系统「智能体」「概览」只读权限
*/
overviewRead: () => canSys(P.RESOURCE_APPLICATION_OVERVIEW_READ),
/**
* 系统「智能体」「概览」导出权限
*/
overviewExport: () => canSys(P.RESOURCE_APPLICATION_OVERVIEW_EXPORT),
/**
* 系统「智能体」「概览」嵌入第三方权限
*/
Expand Down
4 changes: 4 additions & 0 deletions ui/src/permission/modules/application/workspace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ const workspace = {
* 工作空间「智能体」「概览」只读权限
*/
overviewRead: (id: string) => canRes(P.APPLICATION_OVERVIEW_READ, id),
/**
* 工作空间「智能体」「概览」导出权限
*/
overviewExport: (id: string) => canRes(P.APPLICATION_OVERVIEW_EXPORT, id),
/**
* 工作空间「智能体」「概览」嵌入第三方权限
*/
Expand Down
12 changes: 10 additions & 2 deletions ui/src/views/application-detail/overview/components/ApplicationRankingDrawer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
<script setup lang="ts">
import { onMounted, ref } from 'vue'
import { computed, onMounted, ref } from 'vue'
import type { ApplicationRankingKind, HomeDateRange, HomeRankingRecord } from '@/api/types'
import perm from '@/permission'
import { isSystemResource } from '@/utils/resource-context'
import MkDateRange from '@/components/mk-date-range/index.vue'
import type { MkDateRangeValue } from '@/components/mk-date-range/types'
import { formatTokenNumber, numberFormat } from '@/utils/number'
Expand All @@ -11,6 +13,12 @@ const props = defineProps<{ applicationId: string; initialKind: ApplicationRanki
const emit = defineEmits<{ closed: [] }>()
const { resourceApi } = useApplicationDetailContext()

/* 概览排行榜导出需要独立的概览导出权限。 */
const canExport = computed(() => {
if (isSystemResource()) return Boolean(perm.application.system.overviewExport())
return Boolean(perm.application.workspace.overviewExport(props.applicationId))
})

const visible = ref(true)
const activeKind = ref(props.initialKind)
const loading = ref(false)
Expand Down Expand Up @@ -104,7 +112,7 @@ onMounted(loadRanking)
<MkSearchInput v-model="searchName" class="w-55!" @change="handleSearch" />
</div>
<!-- 导出当前筛选下的完整用户排行 -->
<el-button plain :loading="exporting" :disabled="loading" @click="handleExport">
<el-button v-if="canExport" plain :loading="exporting" :disabled="loading" @click="handleExport">
<MkIcon name="icon_export_outlined" />
<span>导出</span>
</el-button>

@shaohuzhang1 shaohuzhang1 Nov 12, 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.

Review of the diff

1. Correctness / Reactivity

  • computed will only update if its dependencies are reactive.

    • props.applicationId is reactive, so that part is fine.
    • isSystemResource() and the perm.application.*.overviewExport() methods must read from reactive state. If perm is a plain module object populated once at startup, canExport will not update when permissions change (e.g. after login, role switch, or async permission loading). Verify perm is backed by a reactive store/composable.
    • Also check what isSystemResource() does. If it internally uses inject() / getCurrentInstance() / useRoute(), calling it inside a computed getter can be unsafe, because composables that rely on inject() must be called synchronously during setup(). Safer pattern: 
      const isSystem = computed(() => isSystemResource()) // only if pure/reactive global
// or, if it is static for this component:
const isSystem = isSystemResource()
      If it uses inject, resolve it at setup top-level.

  • Guard against an empty applicationId.

    if (!props.applicationId) return false

    Otherwise workspace.overviewExport('') may return an unexpected result.

  • Verify the permission API shape. perm.application.system.overviewExport() and perm.application.workspace.overviewExport(id) are invoked as functions. If any of them are actually booleans/getters, this will throw at runtime. The inconsistent shape (system vs. workspace) also suggests a unified helper would be cleaner.

2. Security

  • Hiding the button with v-if is only a UX control, not a security boundary. Ensure the export API enforces the same permission server-side.
  • Consider also guarding the handler for defense in depth: 
    function handleExport() {
  if (!canExport.value) return
  // ...
}
    This protects against stale state or programmatic calls.

3. UX

  • v-if removes the button from the DOM, causing layout shift. If the export button is in a fixed toolbar, consider v-show or rendering it disabled with a tooltip explaining missing permission. Either is fine; just be consistent with the rest of the app.
  • The :disabled="loading" state does not include exporting, though :loading usually blocks clicks. No issue, just noting.

4. Naming / maintainability

  • canExport is very generic in this component. Prefer canExportRanking or canExportOverviewRanking.
  • The permission logic is likely needed elsewhere. Consider extracting a composable such as useOverviewExportPermission(applicationId) or a v-permission directive to avoid duplicating isSystemResource() branching.
  • Import ordering: perm and isSystemResource are mixed into the import block. If the project enforces import sorting/grouping (ESLint import/order), this may fail lint. Minor.

5. Redundant Boolean(...)

  • If the permission functions are typed as boolean, the Boolean(...) wrappers are redundant. Keep them only if they can return undefined/null/truthy non-booleans. Typing the permission methods as boolean would be clearer.

Suggested revision

const isSystem = computed(() => isSystemResource())

const canExportRanking = computed(() => {
  if (!props.applicationId) return false

  if (isSystem.value) {
    return Boolean(perm.application.system.overviewExport())
  }
  return Boolean(perm.application.workspace.overviewExport(props.applicationId))
})

function handleExport() {
  if (!canExportRanking.value) return
  // ...
}
<el-button
  v-if="canExportRanking"
  plain
  :loading="exporting"
  :disabled="loading"
  @click="handleExport"
>

Bottom line

The change is directionally correct, but verify:

  1. perm and isSystemResource() are reactive (otherwise canExport will be stale).
  2. isSystemResource() does not rely on inject() when called inside computed.
  3. The overviewExport permission entries are actually callable functions.
  4. The server enforces the export permission independently of the UI.

Expand Down
Loading