geido commented on code in PR #43719:
URL: https://github.com/apache/superset/pull/43719#discussion_r3915696761


##########
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:
##########
@@ -196,13 +196,18 @@ const SliceHeaderControls = (
       .get(props.slice.viz_type)
       ?.behaviors?.includes(Behavior.InteractiveChart);
   const canExplore = props.supersetCanExplore;
-  const { canDrillToDetail, canViewQuery, canViewTable } = usePermissions();
+  const { canDrillToDetail, canGetDrillInfo, canViewQuery, canViewTable } =
+    usePermissions();
 
+  // The dataset's verbose map resolves friendly Labels for both the 
drill-to-detail
+  // pane and the "View as table" results grid, and those are separate 
permissions —
+  // so fetch it for either one, as long as the drill_info endpoint itself is
+  // readable (it is gated by `can_get_drill_info` on Dataset).
   const datasetResource = useDatasetDrillInfo(
     props.slice.datasource,
     props.dashboardId,
     props.formData,
-    !canDrillToDetail,
+    !canGetDrillInfo || !(canDrillToDetail || canViewTable),

Review Comment:
   Right — `canExplore` is the gap. Fixed by collapsing the two into one 
predicate so they can't drift again:
   
   ```ts
   // Single predicate for the "View as table" entry, so the fetch that feeds 
its
   // column headers cannot drift from the set of users who can open it.
   const canViewResultsTable = canExplore || canViewTable;
   
   const datasetResource = useDatasetDrillInfo(
     props.slice.datasource,
     props.dashboardId,
     props.formData,
     !canGetDrillInfo || !(canDrillToDetail || canViewResultsTable),
   );
   ```
   
   and the menu gate at the bottom is now `if (canViewResultsTable)` rather 
than a second copy of the expression.
   
   `canGetDrillInfo` stays as the outer guard for the reason in the thread 
below — the endpoint is `@protect()`-ed on `can_get_drill_info`, so a role 
without it would only ever get a 403.
   
   Test: `Dataset drill info API call is made for an explore-only user` — a 
user holding `can_get_drill_info` and nothing else relevant, with 
`supersetCanExplore` true. Fails on the previous predicate.
   



-- 
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]

Reply via email to