diff --git a/superset-frontend/src/dashboard/util/permissionUtils.test.ts b/superset-frontend/src/dashboard/util/permissionUtils.test.ts index b9b0e071b4..c6d707dbb0 100644 --- a/superset-frontend/src/dashboard/util/permissionUtils.test.ts +++ b/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('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', () => { - expect(canUserAccessSqlLab(undefined)).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 undefined user', () => { - expect(canUserAccessSqlLab(undefinedUser)).toEqual(false); +test('userHasPermission returns false if user does not have permission', () => { + expect( + userHasPermission( + ownerUser, + sqlLabMenuAccessPermission[1], + sqlLabMenuAccessPermission[0], + ), + ).toEqual(false); }); -test('canUserAccessSqlLab returns false for non-sqllab role', () => { - expect(canUserAccessSqlLab(ownerUser)).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', () => { diff --git a/superset-frontend/src/dashboard/util/permissionUtils.ts b/superset-frontend/src/dashboard/util/permissionUtils.ts index 23744bde89..4b09710d59 100644 --- a/superset-frontend/src/dashboard/util/permissionUtils.ts +++ b/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, + )) ); } diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx index 80cb84ac8d..6def65d7d2 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx +++ b/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', }, diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx index c99193dd42..bf85716206 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx +++ b/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'); diff --git a/superset-frontend/src/pages/Home/Home.test.tsx b/superset-frontend/src/pages/Home/Home.test.tsx index 9f0b4e3ca1..8de33f323e 100644 --- a/superset-frontend/src/pages/Home/Home.test.tsx +++ b/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']], }, }, }; diff --git a/superset-frontend/src/pages/Home/index.tsx b/superset-frontend/src/pages/Home/index.tsx index cfeb4cd982..c61d15e2ab 100644 --- a/superset-frontend/src/pages/Home/index.tsx +++ b/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) { /> )} - {canAccessSqlLab && ( + {canReadSavedQueries && ( {!queryData ? (