You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by li...@apache.org on 2026/10/19 09:00:34 UTC
(rocketmq-dashboard) branch rocketmq-studio updated: fix(acl): clear stale rows, drop page-local sorting and list principals beyond the page (#6715)
This is an automated email from the ASF dual-hosted git repository.
lizhimins pushed a commit to branch rocketmq-studio
in repository https://gitbox.apache.org/repos/asf/rocketmq-dashboard.git
The following commit(s) were added to refs/heads/rocketmq-studio by this push:
new 2e211671c fix(acl): clear stale rows, drop page-local sorting and list principals beyond the page (#6715)
2e211671c is described below
commit 2e211671cf75df555984004e2af27ab5bdc6a913
Author: Loyal-Young <hj...@gmail.com>
AuthorDate: Mon Oct 19 17:00:26 2026 +0800
fix(acl): clear stale rows, drop page-local sorting and list principals beyond the page (#6715)
Batched integration of @Loyal-Young's ACL fixes. This branch carries five PRs' worth of changes; the
individual PRs are closed as folded into this one.
- #6715 — clear stale rule/user rows and totals when a filtered reload fails, so the table cannot keep
offering edit/delete against rows that are no longer there.
- #6912 — clear stale cluster examine results on a failed re-examine.
- #6913 — drop the page-local sorters from the two paged ACL tables (server-side paging makes an
in-page sort misleading).
- #6914 — offer principals beyond the current user page when composing ACL rules.
- #6717 — ignore invalid wildcard permission entries in the risk score instead of counting them as
open wildcards.
Maintainer fix on top: fold one over-width assertion in `aclRiskDiagnostics.test.ts` to match the
configured print width.
Verified locally against `rocketmq-studio`: `AclPage.test.tsx` + `aclRiskDiagnostics.test.ts` 42/42
green, eslint clean, and reverting the #6715 guard turns its regression test red.
---
web/src/pages/instance/__tests__/AclPage.test.tsx | 143 ++++++++++++++++++++++
web/src/pages/instance/acl.tsx | 76 +++++++++---
web/src/utils/aclRiskDiagnostics.test.ts | 15 +++
web/src/utils/aclRiskDiagnostics.ts | 4 +-
4 files changed, 219 insertions(+), 19 deletions(-)
diff --git a/web/src/pages/instance/__tests__/AclPage.test.tsx b/web/src/pages/instance/__tests__/AclPage.test.tsx
index 753923fb2..1b6a61fa8 100644
--- a/web/src/pages/instance/__tests__/AclPage.test.tsx
+++ b/web/src/pages/instance/__tests__/AclPage.test.tsx
@@ -89,6 +89,7 @@ describe('ACL page', () => {
beforeEach(() => {
vi.clearAllMocks();
vi.mocked(instanceService.listInstances).mockResolvedValue([]);
+ vi.mocked(aclService.listAclUsers).mockResolvedValue([]);
vi.mocked(aclService.listAclRules).mockResolvedValue({
items: [
@@ -179,6 +180,77 @@ describe('ACL page', () => {
expect(aclService.pageAclUsers).toHaveBeenCalledTimes(1);
});
+ it('does not offer page-local sorting on either server-paginated ACL table', async () => {
+ const user = userEvent.setup({ pointerEventsCheck: 0 });
+ renderWithProviders(<AclPage />);
+
+ expect(await screen.findByText('remote-user')).toBeInTheDocument();
+ expect(document.querySelectorAll('th.ant-table-column-has-sorters')).toHaveLength(0);
+
+ await user.click(screen.getByText('用户管理'));
+ expect(await screen.findByText('remote-admin')).toBeInTheDocument();
+ expect(document.querySelectorAll('th.ant-table-column-has-sorters')).toHaveLength(0);
+ });
+
+ it('offers an ACL principal outside the loaded user page and shows its admin badge', async () => {
+ const user = userEvent.setup({ pointerEventsCheck: 0 });
+ vi.mocked(aclService.listAclUsers).mockResolvedValue([
+ {
+ id: 11,
+ username: 'remote-admin',
+ accessKey: 'acce****3456',
+ secretKey: 'secr****7654',
+ admin: false,
+ clusters: ['cluster-a'],
+ },
+ {
+ id: 21,
+ username: 'directory-only-user',
+ accessKey: 'dire****1234',
+ secretKey: 'secr****4321',
+ admin: true,
+ clusters: ['cluster-a'],
+ },
+ ]);
+ vi.mocked(aclService.listAclRules).mockResolvedValue({
+ items: [{
+ id: 21,
+ principal: 'directory-only-user',
+ resource: 'orders',
+ resourceType: 'Topic',
+ resourcePattern: 'LITERAL',
+ actions: ['PUB'],
+ decision: 'ALLOW',
+ scope: 'cluster',
+ aclVersion: 2,
+ gmtCreate: '2026-07-23T00:00:00Z',
+ }],
+ total: 1,
+ page: 1,
+ size: 20,
+ });
+ renderWithProviders(<AclPage />);
+
+ await openRulesTab(user);
+ const row = await screen.findByRole('row', { name: /directory-only-user/ });
+ expect(within(row).getByText('管理员')).toBeInTheDocument();
+ await user.click(screen.getByRole('button', { name: /添加规则/ }));
+ const dialog = await screen.findByRole('dialog');
+ await user.click(within(dialog).getByLabelText('主体'));
+ await user.click(
+ await screen.findByText('directory-only-user', {
+ selector: '.ant-select-item-option-content',
+ }),
+ );
+ expect(
+ within(dialog).getByText('directory-only-user', { selector: '.ant-select-selection-item' }),
+ ).toBeInTheDocument();
+ expect(aclService.listAclUsers).toHaveBeenCalledWith({ instanceId: undefined });
+ expect(aclService.pageAclUsers).toHaveBeenCalledWith(
+ expect.objectContaining({ page: 1, pageSize: 20 }),
+ );
+ });
+
it('reloads the server rule page after deleting one ACL rule', async () => {
const user = userEvent.setup({ pointerEventsCheck: 0 });
const confirmSpy = vi.spyOn(Modal, 'confirm').mockImplementation((config) => {
@@ -354,6 +426,38 @@ describe('ACL page', () => {
expect(screen.getByText('cluster-a')).toBeInTheDocument();
});
+ it('clears stale user rows and total when a filtered user reload fails', async () => {
+ renderWithProviders(<AclPage />);
+ const panel = screen.getByRole('tabpanel', { name: '用户管理' });
+ expect(await within(panel).findByText('remote-admin')).toBeInTheDocument();
+ expect(within(panel).getByText('共 1 个用户')).toBeInTheDocument();
+
+ vi.mocked(aclService.pageAclUsers).mockRejectedValueOnce(new Error('users unavailable'));
+ fireEvent.change(within(panel).getByRole('searchbox'), { target: { value: 'missing' } });
+
+ await waitFor(() => expect(aclService.pageAclUsers).toHaveBeenCalledTimes(2));
+ await waitFor(() => expect(within(panel).queryByText('remote-admin')).not.toBeInTheDocument());
+ expect(within(panel).queryByText('共 1 个用户')).not.toBeInTheDocument();
+ });
+
+ it('clears stale rule rows and total when a filtered rule reload fails', async () => {
+ const user = userEvent.setup({ pointerEventsCheck: 0 });
+ renderWithProviders(<AclPage />);
+ await openRulesTab(user);
+ const panel = screen.getByRole('tabpanel', { name: 'ACL 规则' });
+ expect(await within(panel).findByText('remote-topic')).toBeInTheDocument();
+ expect(within(panel).getByText('共 1 条规则')).toBeInTheDocument();
+
+ vi.mocked(aclService.listAclRules).mockRejectedValueOnce(new Error('rules unavailable'));
+ fireEvent.change(within(panel).getByPlaceholderText('搜索主体'), {
+ target: { value: 'missing' },
+ });
+
+ await waitFor(() => expect(aclService.listAclRules).toHaveBeenCalledTimes(2));
+ await waitFor(() => expect(within(panel).queryByText('remote-topic')).not.toBeInTheDocument());
+ expect(within(panel).queryByText('共 1 条规则')).not.toBeInTheDocument();
+ });
+
it('opens on the user tab and lists it before the rule tab', async () => {
renderWithProviders(<AclPage />);
@@ -952,6 +1056,45 @@ describe('ACL page', () => {
expect(aclService.examineBrokerClusterAclConfig).toHaveBeenCalledTimes(1);
});
+ it('removes a prior cluster result when a different cluster examine fails', async () => {
+ const user = userEvent.setup({ pointerEventsCheck: 0 });
+ vi.mocked(aclService.examineBrokerClusterAclConfig)
+ .mockResolvedValueOnce({
+ clusterId: 'cluster-a',
+ aclEnabled: true,
+ aclVersion: 'ACL 2.0',
+ globalWhiteRemoteAddresses: [],
+ accounts: [{
+ accessKey: 'cluster-a-account',
+ admin: false,
+ defaultTopicPerm: 'DENY',
+ defaultGroupPerm: 'DENY',
+ topicPerms: [],
+ groupPerms: [],
+ }],
+ accountCount: 1,
+ })
+ .mockRejectedValueOnce(new Error('cluster-b unavailable'));
+ renderWithProviders(<AclPage />);
+
+ await user.click(await screen.findByText('集群 ACL 配置'));
+ const clusterInput = screen.getByPlaceholderText('请输入集群 ID');
+ const examineButton = screen.getByRole('button', { name: /检\s*查\s*配\s*置/ });
+ await user.type(clusterInput, 'cluster-a');
+ await user.click(examineButton);
+ expect(await screen.findByText('cluster-a-account')).toBeInTheDocument();
+ expect(screen.getByText('集群: cluster-a')).toBeInTheDocument();
+
+ await user.clear(clusterInput);
+ await user.type(clusterInput, 'cluster-b');
+ await user.click(examineButton);
+ await waitFor(() => expect(aclService.examineBrokerClusterAclConfig).toHaveBeenCalledTimes(2));
+ await waitFor(() => expect(examineButton).not.toHaveClass('ant-btn-loading'));
+
+ expect(screen.queryByText('cluster-a-account')).not.toBeInTheDocument();
+ expect(screen.queryByText('集群: cluster-a')).not.toBeInTheDocument();
+ });
+
it('keeps cluster config ownership with the latest examine request', async () => {
const firstExamine =
deferred<Awaited<ReturnType<typeof aclService.examineBrokerClusterAclConfig>>>();
diff --git a/web/src/pages/instance/acl.tsx b/web/src/pages/instance/acl.tsx
index 51f9eca91..15f5574c7 100644
--- a/web/src/pages/instance/acl.tsx
+++ b/web/src/pages/instance/acl.tsx
@@ -65,6 +65,7 @@ import {
getAclUserCredentials,
examineBrokerClusterAclConfig,
listAclRules,
+ listAclUsers,
pageAclUsers,
pushAclToBroker,
updateAclRule,
@@ -171,6 +172,11 @@ const AclPageContent = ({
const [activeTab, setActiveTab] = useState('users');
const [ruleRefreshKey, setRuleRefreshKey] = useState(0);
const [userRefreshKey, setUserRefreshKey] = useState(0);
+ const [directoryState, setDirectoryState] = useState<{
+ instanceId?: string;
+ refreshKey: number;
+ users: NormalizedAclUser[];
+ }>({ instanceId: selectedInstanceId, refreshKey: userRefreshKey, users: [] });
const [ruleTotal, setRuleTotal] = useState(0);
const [rulePage, setRulePage] = useState(1);
const [rulePageSize, setRulePageSize] = useState(20);
@@ -237,7 +243,11 @@ const AclPageContent = ({
}
})
.catch(() => {
- if (mounted) message.error(t('common.fetchDataFailed'));
+ if (mounted) {
+ setRules([]);
+ setRuleTotal(0);
+ message.error(t('common.fetchDataFailed'));
+ }
})
.finally(() => {
if (mounted) setRulesLoading(false);
@@ -259,7 +269,11 @@ const AclPageContent = ({
}
})
.catch(() => {
- if (mounted) message.error(t('common.fetchDataFailed'));
+ if (mounted) {
+ setUsers([]);
+ setUserTotal(0);
+ message.error(t('common.fetchDataFailed'));
+ }
})
.finally(() => {
if (mounted) setUsersLoading(false);
@@ -285,9 +299,38 @@ const AclPageContent = ({
userRefreshKey,
]);
+ useEffect(() => {
+ let mounted = true;
+ void listAclUsers({ instanceId: selectedInstanceId })
+ .then((allUsers) => {
+ if (mounted) {
+ setDirectoryState({
+ instanceId: selectedInstanceId,
+ refreshKey: userRefreshKey,
+ users: allUsers.map(normalizeUser),
+ });
+ }
+ })
+ .catch(() => {
+ if (mounted) message.error(t('common.fetchDataFailed'));
+ });
+ return () => {
+ mounted = false;
+ };
+ }, [selectedInstanceId, userRefreshKey, t]);
+
/* ─── Rule helpers ─── */
+ const principalDirectory =
+ directoryState.instanceId === selectedInstanceId &&
+ directoryState.refreshKey === userRefreshKey
+ ? directoryState.users
+ : [];
+ const principalUsers = [
+ ...principalDirectory,
+ ...users.filter((user) => !principalDirectory.some((entry) => entry.username === user.username)),
+ ];
const isAdmin = (principal: string) =>
- users.find((u) => u.username === principal)?.admin ?? false;
+ principalUsers.find((u) => u.username === principal)?.admin ?? false;
const actionTagColor: Record<string, string> = {
PUB: 'blue',
@@ -447,6 +490,7 @@ const AclPageContent = ({
});
const normalized = normalizeUser(updated);
setUsers((prev) => prev.map((u) => (u.id === editingUser.id ? normalized : u)));
+ setUserRefreshKey((prev) => prev + 1);
message.success(t('acl.userUpdated'));
} else {
await createAclUser({
@@ -496,6 +540,7 @@ const AclPageContent = ({
});
const normalized = normalizeUser(updated);
setUsers((prev) => prev.map((u) => (u.id === user.id ? normalized : u)));
+ setUserRefreshKey((prev) => prev + 1);
message.success(checked ? t('acl.adminSet') : t('acl.adminRemoved'));
} catch {
message.error(t('common.operationFailed'));
@@ -518,6 +563,7 @@ const AclPageContent = ({
}
const requestGeneration = examineRequestGenerationRef.current + 1;
examineRequestGenerationRef.current = requestGeneration;
+ setClusterConfig(null);
try {
setConfigLoading(true);
const config = await examineBrokerClusterAclConfig(clusterId);
@@ -705,7 +751,6 @@ const AclPageContent = ({
key: 'principal',
// 唯一可伸展列：容器比表宽时余量集中在此，其余列保持声明宽度
minWidth: 200,
- sorter: (a, b) => a.principal.localeCompare(b.principal),
render: (text: string) => (
<Space size={6}>
<User size={14} color="#8c8c8c" weight="fill" />
@@ -723,7 +768,6 @@ const AclPageContent = ({
title: t('acl.resource'),
key: 'resource',
minWidth: 240,
- sorter: (a, b) => a.resource.localeCompare(b.resource),
render: (_: unknown, record: AclRule) => (
<Space size={6}>
<span style={{ fontFamily: 'monospace', fontWeight: 500 }}>{record.resource}</span>
@@ -757,7 +801,6 @@ const AclPageContent = ({
dataIndex: 'decision',
key: 'decision',
width: 80,
- sorter: (a, b) => a.decision.localeCompare(b.decision),
render: (decision: string) => (
<Tag color={decision === 'ALLOW' ? 'green' : 'red'} style={{ fontWeight: 600 }}>
{decision === 'ALLOW' ? t('acl.allow') : t('acl.deny')}
@@ -769,7 +812,6 @@ const AclPageContent = ({
dataIndex: 'aclVersion',
key: 'aclVersion',
width: 100,
- sorter: (a, b) => String(a.aclVersion).localeCompare(String(b.aclVersion)),
render: (version: AclRule['aclVersion']) => (
<Tag color={String(version) === '2.0' ? 'geekblue' : 'default'}>{version}</Tag>
),
@@ -779,7 +821,6 @@ const AclPageContent = ({
dataIndex: 'scope',
key: 'scope',
width: 100,
- sorter: (a, b) => a.scope.localeCompare(b.scope),
render: (scope: string) => (
<span style={{ fontSize: 14 }}>
{scope === 'cluster' ? t('acl.cluster') : t('acl.namespace')}
@@ -791,7 +832,6 @@ const AclPageContent = ({
dataIndex: 'gmtCreate',
key: 'gmtCreate',
width: 160,
- sorter: (a, b) => (a.gmtCreate ?? '').localeCompare(b.gmtCreate ?? ''),
render: (iso?: string | null) => (
<span style={{ fontSize: 14, color: '#8c8c8c' }}>{formatDate(iso)}</span>
),
@@ -841,7 +881,6 @@ const AclPageContent = ({
key: 'username',
// 唯一可伸展列：容器比表宽时余量集中在此，其余列保持声明宽度
minWidth: 200,
- sorter: (a, b) => a.username.localeCompare(b.username),
render: (text: string, record: NormalizedAclUser) => (
<Space size={6}>
<User size={14} color="#8c8c8c" weight="fill" />
@@ -860,7 +899,6 @@ const AclPageContent = ({
dataIndex: 'accessKey',
key: 'accessKey',
width: 220,
- sorter: (a, b) => a.accessKey.localeCompare(b.accessKey),
render: (text: string, record: NormalizedAclUser) => {
const revealed = revealedKeys.has(record.id);
const fullAccessKey = credentialsByUser[String(record.id)]?.accessKey ?? text;
@@ -908,7 +946,6 @@ const AclPageContent = ({
dataIndex: 'admin',
key: 'admin',
width: 100,
- sorter: (a, b) => Number(a.admin) - Number(b.admin),
render: (val: boolean, record: AclUser) => (
<Switch
checked={val}
@@ -924,7 +961,6 @@ const AclPageContent = ({
dataIndex: 'clusters',
key: 'clusters',
width: 280,
- sorter: (a, b) => a.clusters.length - b.clusters.length,
render: (clusters: string[]) => (
<TagList
items={clusters.map((c) => ({ key: c, label: c, color: 'processing' }))}
@@ -938,7 +974,6 @@ const AclPageContent = ({
dataIndex: 'gmtCreate',
key: 'gmtCreate',
width: 160,
- sorter: (a, b) => (a.gmtCreate ?? '').localeCompare(b.gmtCreate ?? ''),
render: (iso?: string | null) => (
<span style={{ fontSize: 14, color: '#8c8c8c' }}>{formatDate(iso)}</span>
),
@@ -1440,6 +1475,9 @@ const AclPageContent = ({
{clusterConfig && (
<div style={{ paddingBottom: 16 }}>
+ <div style={{ marginBottom: 12, color: '#8c8c8c', fontSize: 14 }}>
+ {t('acl.examineCluster')}: {clusterConfig.clusterId}
+ </div>
{/* Summary cards */}
<div
style={{
@@ -1595,10 +1633,12 @@ const AclPageContent = ({
disabled={tencentRoleMode && !!editingRule}
showSearch
optionFilterProp="label"
- options={users.map((u) => ({
- value: u.username,
- label: u.username,
- }))}
+ options={[
+ ...principalUsers.map((u) => ({ value: u.username, label: u.username })),
+ ...(editingRule && !principalUsers.some((u) => u.username === editingRule.principal)
+ ? [{ value: editingRule.principal, label: editingRule.principal }]
+ : []),
+ ]}
/>
</Form.Item>
diff --git a/web/src/utils/aclRiskDiagnostics.test.ts b/web/src/utils/aclRiskDiagnostics.test.ts
index 4b5c1c13e..210a7f4ab 100644
--- a/web/src/utils/aclRiskDiagnostics.test.ts
+++ b/web/src/utils/aclRiskDiagnostics.test.ts
@@ -170,6 +170,21 @@ describe('ACL risk diagnostics', () => {
);
});
+ it('reports malformed wildcard entries without counting them as grants', () => {
+ const diagnostics = analyzeAclRisk(
+ config({
+ accounts: [account({ topicPerms: ['*=INVALID'], groupPerms: ['*'] })],
+ }),
+ );
+
+ expect(diagnostics.summary.wildcardPermissionAccountCount).toBe(0);
+ expect(
+ diagnostics.issues.filter((item) => item.code === 'INVALID_PERMISSION_ENTRY'),
+ ).toHaveLength(2);
+ expect(diagnostics.issues.map((item) => item.code)).not.toContain('WILDCARD_TOPIC_PERMISSION');
+ expect(diagnostics.issues.map((item) => item.code)).not.toContain('WILDCARD_GROUP_PERMISSION');
+ });
+
it('reads the comma-joined action list the server writes for a multi-action rule', () => {
const diagnostics = analyzeAclRisk(
config({
diff --git a/web/src/utils/aclRiskDiagnostics.ts b/web/src/utils/aclRiskDiagnostics.ts
index dfe61fcb0..17a47ab57 100644
--- a/web/src/utils/aclRiskDiagnostics.ts
+++ b/web/src/utils/aclRiskDiagnostics.ts
@@ -206,7 +206,9 @@ const hasDefaultAllow = (account: PlainAccessConfig): boolean =>
PERMISSION_ALLOW_RANK[normalizePermission(account.defaultGroupPerm)] > 0;
const wildcardEntries = (entries: ParsedPermissionEntry[]): ParsedPermissionEntry[] =>
- entries.filter((entry) => isWildcardResource(entry.resource) && entry.permission !== 'DENY');
+ entries.filter(
+ (entry) => isWildcardResource(entry.resource) && PERMISSION_ALLOW_RANK[entry.permission] > 0,
+ );
const hasWildcardPermission = (account: PlainAccessConfig): boolean =>
wildcardEntries(parsePermissionEntries(account.topicPerms)).length > 0 ||