codeant-ai-for-open-source[bot] commented on code in PR #32995:
URL: https://github.com/apache/superset/pull/32995#discussion_r3557902602
##########
superset-frontend/src/pages/DashboardList/index.tsx:
##########
@@ -478,7 +481,8 @@ function DashboardList(props: DashboardListProps) {
id: 'changed_on_delta_humanized',
},
{
- Cell: ({ row: { original } }: any) => {
+ Cell: ({ row: { original } }: CellProps<Dashboard>) => {
+ const allowEdit = isUserEditorOrAdmin(user, original.editors);
const handleDelete = () =>
Review Comment:
**Suggestion:** This new gate calls `isUserEditorOrAdmin(user,
original.editors)`, but in this page `user` is a lightweight prop (no
guaranteed `permissions/roles/username`), so admin users can be misclassified
as non-admin and blocked from edit/delete unless they are explicit editors. Use
the full authenticated user object expected by the permission helper (or an
equivalent admin capability check) before applying this UI disable logic.
[logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Admins blocked from editing dashboards via table actions.
- ❌ Admins blocked from deleting dashboards via table actions.
- ⚠️ Dashboards list UI contradicts backend admin capabilities.
- ⚠️ Similar misuse appears in charts/datasets lists using same pattern.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In `superset-frontend/src/pages/DashboardList/index.tsx` lines 41-48,
`DashboardListProps` defines `user` as an object with only `userId`,
`firstName`, and
`lastName`, with no `roles`, `permissions`, or `username` fields.
2. In the same file at lines 178-183, the component reads the full
authenticated user from
Redux via `useSelector<any, UserWithPermissionsAndRoles>(state =>
state.user)`, which
includes `roles` containing the `'admin'` role for admin users, but this
Redux user is not
passed to `isUserEditorOrAdmin`.
3. Still in `DashboardList/index.tsx`, inside the `columns` `useMemo`, the
Actions column
is defined at lines 55-67 (in the hunk around 471-149) with `Cell: ({ row: {
original } }:
CellProps<Dashboard>) => { const allowEdit = isUserEditorOrAdmin(user,
original.editors);
... }`. This passes the lightweight `props.user` to `isUserEditorOrAdmin`,
not the Redux
user with roles.
4. In `superset-frontend/src/dashboard/util/permissionUtils.ts` lines 36-52,
`isUserEditorOrAdmin(user, editors)` computes `isUserAdmin(user)` using
`isUserWithPermissionsAndRoles(user)`, which requires `username`,
`permissions`, and
`roles` (see `isUserWithPermissionsAndRoles` in
`superset-frontend/src/types/bootstrapTypes.ts` lines 200-207). For the
lightweight `user`
prop, `isUserWithPermissionsAndRoles(user)` is always false, so
`isUserAdmin(user)` is
always false. When an admin (with `admin` role in `state.user.roles`) visits
the
Dashboards list table (ListView rendered at lines 16-46 in
`DashboardList/index.tsx`) and
is not listed in `original.editors`, `allowEdit` is false for that row,
causing the
edit/delete `IconButton`s in the Actions cell (lines 69-104 and 104-141) to
be rendered
with `disabled={!allowEdit}`. The admin is therefore prevented from editing
or deleting
dashboards through the list UI despite having backend privileges.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ffab99f8b1f3469eb30aa2747572fdf0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=ffab99f8b1f3469eb30aa2747572fdf0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/pages/DashboardList/index.tsx
**Line:** 486:486
**Comment:**
*Logic Error: This new gate calls `isUserEditorOrAdmin(user,
original.editors)`, but in this page `user` is a lightweight prop (no
guaranteed `permissions/roles/username`), so admin users can be misclassified
as non-admin and blocked from edit/delete unless they are explicit editors. Use
the full authenticated user object expected by the permission helper (or an
equivalent admin capability check) before applying this UI disable logic.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32995&comment_hash=429583f98bb612088960f96b6843d81d72acc95bea0d6c7515de1e0dd98e7eb2&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32995&comment_hash=429583f98bb612088960f96b6843d81d72acc95bea0d6c7515de1e0dd98e7eb2&reaction=dislike'>👎</a>
##########
superset-frontend/src/features/dashboards/DashboardCard.tsx:
##########
@@ -41,7 +43,7 @@ interface DashboardCardProps {
openDashboardEditModal?: (d: Dashboard) => void;
saveFavoriteStatus: (id: number, isStarred: boolean) => void;
favoriteStatus: boolean;
- userId?: string | number;
+ user?: { userId?: string | number };
Review Comment:
**Suggestion:** `isUserEditorOrAdmin` expects a full user object with
roles/permissions, but this new prop type only carries `userId`. That makes
admin detection fail (`isUserAdmin` returns false), so admins who are not
explicitly listed as editors will incorrectly see Edit/Delete disabled. Pass a
`UserWithPermissionsAndRoles`-compatible user object (or use a dedicated admin
flag) instead of narrowing the prop to just `userId`. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Admins blocked from editing dashboards via card view.
- ❌ Admins blocked from deleting dashboards via card view.
- ⚠️ UI permission state diverges from backend RBAC.
- ⚠️ Confusing experience for global dashboard administrators.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In `superset-frontend/src/dashboard/util/permissionUtils.ts` lines 36-52,
observe that
`isUserEditorOrAdmin(user, editors)` calls `isUserAdmin(user)`, which in
turn uses
`isUserWithPermissionsAndRoles(user)` to check `user.roles` for the
`'admin'` role.
2. In `superset-frontend/src/types/bootstrapTypes.ts` lines 40-62, see that
`UserWithPermissionsAndRoles` includes `username`, `permissions`, and
`roles`, and
`isUserWithPermissionsAndRoles(user)` (lines 200-207) returns true only when
those fields
exist.
3. In `superset-frontend/src/features/dashboards/DashboardCard.tsx` lines
37-50,
`DashboardCardProps` declares `user?: { userId?: string | number };`, and in
the component
(lines 52-70) it passes this `user` directly to `isUserEditorOrAdmin(user,
dashboard.editors)` even though the prop shape does not include `roles` or
`permissions`.
4. In `superset-frontend/src/pages/DashboardList/index.tsx` lines 332-350
and 354-363,
`renderCard` renders `<DashboardCard ... user={user} />`, where `user` is
the lightweight
prop defined at lines 41-48 (`userId`, `firstName`, `lastName` only). When an
administrator (with `admin` in `state.user.roles`, see lines 180-183) views
the Dashboards
list in card mode (ListView with `defaultViewMode="card"` at lines 39-42 and
`renderCard={renderCard}` at line 38 of the same file), `DashboardCard`
computes
`allowEdit = isUserEditorOrAdmin(user, dashboard.editors)` with a `user`
value that fails
`isUserWithPermissionsAndRoles`. As a result `isUserAdmin(user)` is always
false, so any
admin who is not listed in `dashboard.editors` will have `allowEdit ===
false`, causing
the Kebab menu edit/delete items (lines 79-107 and 126-151 in
`DashboardCard.tsx`) to be
disabled even though backend permissions and `state.user.roles` indicate
they are admins.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=69efa234dfde48aca5ff8c36bc406c20&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=69efa234dfde48aca5ff8c36bc406c20&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/features/dashboards/DashboardCard.tsx
**Line:** 46:46
**Comment:**
*Api Mismatch: `isUserEditorOrAdmin` expects a full user object with
roles/permissions, but this new prop type only carries `userId`. That makes
admin detection fail (`isUserAdmin` returns false), so admins who are not
explicitly listed as editors will incorrectly see Edit/Delete disabled. Pass a
`UserWithPermissionsAndRoles`-compatible user object (or use a dedicated admin
flag) instead of narrowing the prop to just `userId`.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32995&comment_hash=b939d447224207da380feb07ee64b6c38364716dc24d62b03b158f8e204da661&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F32995&comment_hash=b939d447224207da380feb07ee64b6c38364716dc24d62b03b158f8e204da661&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
