fix(charts): View in SQL Lab with relevant perm (#24903)

superset-frontend/src/dashboard/util/permissionUtils.test.ts

@@ -24,7 +24,7 @@ import {
 import { Dashboard } from 'src/types/Dashboard';
 import Owner from 'src/types/Owner';
 import {
-  canUserAccessSqlLab,
+  userHasPermission,
   canUserEditDashboard,
   canUserSaveAsDashboard,
   isUserAdmin,
@@ -65,11 +65,18 @@ const owner: Owner = {
   last_name: 'User',
 };
 
+const sqlLabMenuAccessPermission: [string, string] = ['menu_access', 'SQL Lab'];
+
+const arbitraryPermissions: [string, string][] = [
+  ['can_write', 'AnArbitraryView'],
+  sqlLabMenuAccessPermission,
+];
+
 const sqlLabUser: UserWithPermissionsAndRoles = {
   ...ownerUser,
   roles: {
     ...ownerUser.roles,
-    sql_lab: [],
+    sql_lab: [sqlLabMenuAccessPermission],
   },
 };
 
@@ -139,24 +146,40 @@ test('isUserAdmin returns false for non-admin user', () => {
   expect(isUserAdmin(ownerUser)).toEqual(false);
 });
 
-test('canUserAccessSqlLab returns true for admin user', () => {
-  expect(canUserAccessSqlLab(adminUser)).toEqual(true);
-});
-
-test('canUserAccessSqlLab returns false for undefined', () => {
-  expect(canUserAccessSqlLab(undefined)).toEqual(false);
+test('userHasPermission always returns true for admin user', () => {
+  arbitraryPermissions.forEach(permissionView => {
+    expect(
+      userHasPermission(adminUser, permissionView[1], permissionView[0]),
+    ).toEqual(true);
+  });
 });
 
-test('canUserAccessSqlLab returns false for undefined user', () => {
-  expect(canUserAccessSqlLab(undefinedUser)).toEqual(false);
+test('userHasPermission always returns false for undefined user', () => {
+  arbitraryPermissions.forEach(permissionView => {
+    expect(
+      userHasPermission(undefinedUser, permissionView[1], permissionView[0]),
+    ).toEqual(false);
+  });
 });
 
-test('canUserAccessSqlLab returns false for non-sqllab role', () => {
-  expect(canUserAccessSqlLab(ownerUser)).toEqual(false);
+test('userHasPermission returns false if user does not have permission', () => {
+  expect(
+    userHasPermission(
+      ownerUser,
+      sqlLabMenuAccessPermission[1],
+      sqlLabMenuAccessPermission[0],
+    ),
+  ).toEqual(false);
 });
 
-test('canUserAccessSqlLab returns true for sqllab role', () => {
-  expect(canUserAccessSqlLab(sqlLabUser)).toEqual(true);
+test('userHasPermission returns true if user has permission', () => {
+  expect(
+    userHasPermission(
+      sqlLabUser,
+      sqlLabMenuAccessPermission[1],
+      sqlLabMenuAccessPermission[0],
+    ),
+  ).toEqual(true);
 });
 
 describe('canUserSaveAsDashboard with RBAC feature flag disabled', () => {

superset-frontend/src/dashboard/util/permissionUtils.ts

@@ -28,7 +28,6 @@ import { findPermission } from 'src/utils/findPermission';
 // this should really be a config value,
 // but is hardcoded in backend logic already, so...
 const ADMIN_ROLE_NAME = 'admin';
-const SQL_LAB_ROLE = 'sql_lab';
 
 export const isUserAdmin = (
   user?: UserWithPermissionsAndRoles | UndefinedUser,
@@ -53,15 +52,21 @@ export const canUserEditDashboard = (
   (isUserAdmin(user) || isUserDashboardOwner(dashboard, user)) &&
   findPermission('can_write', 'Dashboard', user?.roles);
 
-export function canUserAccessSqlLab(
-  user?: UserWithPermissionsAndRoles | UndefinedUser,
+export function userHasPermission(
+  user: UserWithPermissionsAndRoles | UndefinedUser,
+  viewName: string,
+  permissionName: string,
 ) {
   return (
     isUserAdmin(user) ||
     (isUserWithPermissionsAndRoles(user) &&
-      Object.keys(user.roles || {}).some(
-        role => role.toLowerCase() === SQL_LAB_ROLE,
-      ))
+      Object.values(user.roles || {})
+        .flat()
+        .some(
+          permissionView =>
+            permissionView[0] === permissionName &&
+            permissionView[1] === viewName,
+        ))
   );
 }
 

superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx

@@ -149,7 +149,7 @@ test('Should show SQL Lab for sql_lab role', async () => {
       isActive: true,
       lastName: 'sql',
       permissions: {},
-      roles: { Gamma: [], sql_lab: [] },
+      roles: { Gamma: [], sql_lab: [['menu_access', 'SQL Lab']] },
       userId: 2,
       username: 'sql',
     },

superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx

@@ -43,7 +43,7 @@ import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
 import { URL_PARAMS } from 'src/constants';
 import { getDatasourceAsSaveableDataset } from 'src/utils/datasourceUtils';
 import {
-  canUserAccessSqlLab,
+  userHasPermission,
   isUserAdmin,
 } from 'src/dashboard/util/permissionUtils';
 import ModalTrigger from 'src/components/ModalTrigger';
@@ -283,7 +283,7 @@ class DatasourceControl extends React.PureComponent {
       datasource.owners?.map(o => o.id || o.value).includes(user.userId) ||
       isUserAdmin(user);
 
-    const canAccessSqlLab = canUserAccessSqlLab(user);
+    const canAccessSqlLab = userHasPermission(user, 'SQL Lab', 'menu_access');
 
     const editText = t('Edit dataset');
 

superset-frontend/src/pages/Home/Home.test.tsx

@@ -109,7 +109,7 @@ const mockedProps = {
     isAnonymous: false,
     permissions: {},
     roles: {
-      sql_lab: [],
+      sql_lab: [['can_read', 'SavedQuery']],
     },
   },
 };

superset-frontend/src/pages/Home/index.tsx

@@ -50,7 +50,7 @@ import { AntdSwitch } from 'src/components';
 import getBootstrapData from 'src/utils/getBootstrapData';
 import { TableTab } from 'src/views/CRUD/types';
 import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
-import { canUserAccessSqlLab } from 'src/dashboard/util/permissionUtils';
+import { userHasPermission } from 'src/dashboard/util/permissionUtils';
 import { WelcomePageLastTab } from 'src/features/home/types';
 import ActivityTable from 'src/features/home/ActivityTable';
 import ChartTable from 'src/features/home/ChartTable';
@@ -156,7 +156,7 @@ export const LoadingCards = ({ cover }: LoadingProps) => (
 );
 
 function Welcome({ user, addDangerToast }: WelcomeProps) {
-  const canAccessSqlLab = canUserAccessSqlLab(user);
+  const canReadSavedQueries = userHasPermission(user, 'SavedQuery', 'can_read');
   const userid = user.userId;
   const id = userid!.toString(); // confident that user is not a guest user
   const params = rison.encode({ page_size: 6 });
@@ -281,7 +281,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
           addDangerToast(t('There was an issue fetching your chart: %s', err));
           return Promise.resolve();
         }),
-      canAccessSqlLab
+      canReadSavedQueries
         ? getUserOwnedObjects(id, 'saved_query', ownSavedQueryFilters)
             .then(r => {
               setQueryData(r);
@@ -410,7 +410,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
                   />
                 )}
               </Collapse.Panel>
-              {canAccessSqlLab && (
+              {canReadSavedQueries && (
                 <Collapse.Panel header={t('Saved queries')} key="4">
                   {!queryData ? (
                     <LoadingCards cover={checked} />