Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions e2e/admin-rbac.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ test('admin RBAC controls access, role assignment, and privacy boundaries', asyn
await page.goto('/admin/users')
await expect(page.getByRole('heading', { name: 'Admin users' })).toBeHidden()
await expect(page.getByText('Forbidden')).toBeVisible()
await page.goto('/admin/usage')
await expect(page.getByRole('heading', { name: 'Admin usage' })).toBeHidden()
await expect(page.getByText('Forbidden')).toBeVisible()
Comment on lines +32 to +34

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Missing non-admin forbidden check for /admin/usage.json.

Only the HTML page (Lines 32-34) is verified to be forbidden for non-admins; there's no assertion that /admin/usage.json also rejects non-admin requests. The stack description for this layer calls out verifying "non-admin access is forbidden ... on both the usage page and JSON API," so this coverage looks incomplete.

✅ Suggested addition near the non-admin block
 	await page.goto('/admin/usage')
 	await expect(page.getByRole('heading', { name: 'Admin usage' })).toBeHidden()
 	await expect(page.getByText('Forbidden')).toBeVisible()
+	const usageApiForbidden = await page.request.get('/admin/usage.json')
+	expect(usageApiForbidden.ok()).toBe(false)

Also applies to: 123-130

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e/admin-rbac.spec.ts` around lines 32 - 34, Add the missing non-admin
forbidden assertion for the `/admin/usage.json` endpoint in the
`admin-rbac.spec.ts` non-admin access block. Extend the existing checks around
`page.goto('/admin/usage')` so the same unauthorized user flow also requests
`/admin/usage.json` and verifies it is rejected with the expected forbidden
response, alongside the current HTML page assertion. Use the existing non-admin
test section and related `page`/`expect` calls to keep the coverage aligned with
the RBAC behavior.

await expect(
page.getByRole('link', { name: 'Admin', exact: true }),
).toHaveCount(0)
Expand Down Expand Up @@ -111,6 +114,22 @@ test('admin RBAC controls access, role assignment, and privacy boundaries', asyn
expect(JSON.stringify(memberRecord)).not.toContain('memberPrivateSecret')
expect(JSON.stringify(memberRecord)).not.toContain('super-secret-value')

await page.goto('/admin/usage')
await expect(page.getByRole('heading', { name: 'Admin usage' })).toBeVisible()
await expect(page.getByText('Unable to load admin usage.')).toHaveCount(0)
await expect(page.getByText('Current month usage')).toBeVisible()
await expect(page.getByText('memberPrivateSecret')).toHaveCount(0)
await expect(page.getByText('super-secret-value')).toHaveCount(0)
const usageApiResponse = await page.request.get(
'/admin/usage.json?pageSize=100',
)
expect(usageApiResponse.ok()).toBe(true)
const usagePayload = await usageApiResponse.json()
expect(usagePayload.ok).toBe(true)
expect(JSON.stringify(usagePayload)).not.toContain('memberPrivateSecret')
expect(JSON.stringify(usagePayload)).not.toContain('super-secret-value')

await page.goto(`/admin/users?pageSize=100&page=${lastUsersPage}`)
await page.getByRole('button', { name: memberUser.username }).click()
await expect(page.getByText('Account metadata only')).toBeVisible()

Expand Down
6 changes: 6 additions & 0 deletions packages/worker/client/routes/admin-community-reports.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,12 @@ export function AdminCommunityReportsRoute(handle: Handle) {
>
View roles
</a>
<a
href="/admin/usage"
mix={css({ ...secondaryButtonCss, textDecoration: 'none' })}
>
Usage
</a>
</>
}
/>
Expand Down
6 changes: 6 additions & 0 deletions packages/worker/client/routes/admin-invites.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,12 @@ export function AdminInvitesRoute(handle: Handle) {
>
Roles
</a>
<a
href="/admin/usage"
mix={css({ ...secondaryButtonCss, textDecoration: 'none' })}
>
Usage
</a>
</>
}
/>
Expand Down
6 changes: 6 additions & 0 deletions packages/worker/client/routes/admin-roles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ export function AdminRolesRoute(handle: Handle) {
>
View users
</a>
<a
href="/admin/usage"
mix={css({ ...secondaryButtonCss, textDecoration: 'none' })}
>
Usage
</a>
<a
href="/admin/community-reports"
mix={css({ ...secondaryButtonCss, textDecoration: 'none' })}
Expand Down
Loading
Loading