iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(sqllab): remove link to sqllab if missing perms (#22566)

This commit is contained in:
Ville Brofeldt 2023-01-09 14:02:36 +02:00 committed by GitHub
parent 30dab3a00a
commit 5b2ca97341
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 287 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
superset-frontend/src/dashboard/util/permissionUtils.test.ts
View File

@ -22,7 +22,11 @@ import {
} from 'src/types/bootstrapTypes';
import { Dashboard } from 'src/types/Dashboard';
import Owner from 'src/types/Owner';
import { canUserEditDashboard, isUserAdmin } from './permissionUtils';
import {
canUserAccessSqlLab,
canUserEditDashboard,
isUserAdmin,
} from './permissionUtils';
const ownerUser: UserWithPermissionsAndRoles = {
createdOn: '2021-05-12T16:56:22.116839',
@ -60,6 +64,14 @@ const owner: Owner = {
username: ownerUser.username,
};
const sqlLabUser: UserWithPermissionsAndRoles = {
...ownerUser,
roles: {
...ownerUser.roles,
sql_lab: [],
},
};
const undefinedUser: UndefinedUser = {};
describe('canUserEditDashboard', () => {
@ -109,6 +121,10 @@ test('isUserAdmin returns true for admin user', () => {
expect(isUserAdmin(adminUser)).toEqual(true);
});
test('isUserAdmin returns false for undefined', () => {
expect(isUserAdmin(undefined)).toEqual(false);
});
test('isUserAdmin returns false for undefined user', () => {
expect(isUserAdmin(undefinedUser)).toEqual(false);
});
@ -116,3 +132,23 @@ test('isUserAdmin returns false for undefined user', () => {
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('canUserAccessSqlLab returns false for undefined user', () => {
expect(canUserAccessSqlLab(undefinedUser)).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);
});

15
superset-frontend/src/dashboard/util/permissionUtils.ts
View File

@ -27,9 +27,10 @@ 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,
user?: UserWithPermissionsAndRoles | UndefinedUser,
) =>
isUserWithPermissionsAndRoles(user) &&
Object.keys(user.roles || {}).some(
@ -50,3 +51,15 @@ export const canUserEditDashboard = (
isUserWithPermissionsAndRoles(user) &&
(isUserAdmin(user) || isUserDashboardOwner(dashboard, user)) &&
findPermission('can_write', 'Dashboard', user.roles);
export function canUserAccessSqlLab(
user?: UserWithPermissionsAndRoles | UndefinedUser,
) {
return (
isUserAdmin(user) ||
(isUserWithPermissionsAndRoles(user) &&
Object.keys(user.roles || {}).some(
role => role.toLowerCase() === SQL_LAB_ROLE,
))
);
}

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

@ -20,13 +20,13 @@
import React from 'react';
import { render, screen, act, waitFor } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { SupersetClient, DatasourceType } from '@superset-ui/core';
import { DatasourceType, JsonObject, SupersetClient } from '@superset-ui/core';
import fetchMock from 'fetch-mock';
import DatasourceControl from '.';
const SupersetClientGet = jest.spyOn(SupersetClient, 'get');
const createProps = () => ({
const createProps = (overrides: JsonObject = {}) => ({
hovered: false,
type: 'DatasourceControl',
label: 'Datasource',
@ -64,6 +64,7 @@ const createProps = () => ({
},
onChange: jest.fn(),
onDatasourceSave: jest.fn(),
...overrides,
});
async function openAndSaveChanges(datasource: any) {
@ -104,6 +105,52 @@ test('Should open a menu', async () => {
expect(screen.getByText('View in SQL Lab')).toBeInTheDocument();
});
test('Should not show SQL Lab for non sql_lab role', async () => {
const props = createProps({
user: {
createdOn: '2021-04-27T18:12:38.952304',
email: 'gamma',
firstName: 'gamma',
isActive: true,
lastName: 'gamma',
permissions: {},
roles: { Gamma: [] },
userId: 2,
username: 'gamma',
},
});
render(<DatasourceControl {...props} />);
userEvent.click(screen.getByTestId('datasource-menu-trigger'));
expect(await screen.findByText('Edit dataset')).toBeInTheDocument();
expect(screen.getByText('Swap dataset')).toBeInTheDocument();
expect(screen.queryByText('View in SQL Lab')).not.toBeInTheDocument();
});
test('Should show SQL Lab for sql_lab role', async () => {
const props = createProps({
user: {
createdOn: '2021-04-27T18:12:38.952304',
email: 'sql',
firstName: 'sql',
isActive: true,
lastName: 'sql',
permissions: {},
roles: { Gamma: [], sql_lab: [] },
userId: 2,
username: 'sql',
},
});
render(<DatasourceControl {...props} />);
userEvent.click(screen.getByTestId('datasource-menu-trigger'));
expect(await screen.findByText('Edit dataset')).toBeInTheDocument();
expect(screen.getByText('Swap dataset')).toBeInTheDocument();
expect(screen.getByText('View in SQL Lab')).toBeInTheDocument();
});
test('Click on Swap dataset option', async () => {
const props = createProps();
SupersetClientGet.mockImplementation(

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

@ -42,7 +42,10 @@ import ErrorAlert from 'src/components/ErrorMessage/ErrorAlert';
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
import { URL_PARAMS } from 'src/constants';
import { getDatasourceAsSaveableDataset } from 'src/utils/datasourceUtils';
import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
import {
canUserAccessSqlLab,
isUserAdmin,
} from 'src/dashboard/util/permissionUtils';
import ModalTrigger from 'src/components/ModalTrigger';
import ViewQueryModalFooter from 'src/explore/components/controls/ViewQueryModalFooter';
import ViewQuery from 'src/explore/components/controls/ViewQuery';
@ -279,6 +282,8 @@ class DatasourceControl extends React.PureComponent {
datasource.owners?.map(o => o.id || o.value).includes(user.userId) ||
isUserAdmin(user);
const canAccessSqlLab = canUserAccessSqlLab(user);
const editText = t('Edit dataset');
const defaultDatasourceMenu = (
@ -303,7 +308,7 @@ class DatasourceControl extends React.PureComponent {
</Menu.Item>
)}
<Menu.Item key={CHANGE_DATASET}>{t('Swap dataset')}</Menu.Item>
{datasource && (
{datasource && canAccessSqlLab && (
<Menu.Item key={VIEW_IN_SQL_LAB}>{t('View in SQL Lab')}</Menu.Item>
)}
</Menu>
@ -333,7 +338,9 @@ class DatasourceControl extends React.PureComponent {
responsive
/>
</Menu.Item>
<Menu.Item key={VIEW_IN_SQL_LAB}>{t('View in SQL Lab')}</Menu.Item>
{canAccessSqlLab && (
<Menu.Item key={VIEW_IN_SQL_LAB}>{t('View in SQL Lab')}</Menu.Item>
)}
<Menu.Item key={SAVE_AS_DATASET}>{t('Save as dataset')}</Menu.Item>
</Menu>
);

5
superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx
View File

@ -50,6 +50,7 @@ import copyTextToClipboard from 'src/utils/copy';
import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
import ImportModelsModal from 'src/components/ImportModal/index';
import Icons from 'src/components/Icons';
import { BootstrapUser } from 'src/types/bootstrapTypes';
import SavedQueryPreviewModal from './SavedQueryPreviewModal';
const PAGE_SIZE = 25;
@ -69,9 +70,7 @@ const CONFIRM_OVERWRITE_MESSAGE = t(
interface SavedQueryListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
user: {
userId: string | number;
};
user: BootstrapUser;
}
const StyledTableLabel = styled.div`

4
superset-frontend/src/views/CRUD/welcome/ActivityTable.test.tsx
View File

@ -82,7 +82,7 @@ describe('ActivityTable', () => {
activityData: mockData,
setActiveChild: jest.fn(),
user: { userId: '1' },
loadedCount: 3,
isFetchingActivityData: false,
};
let wrapper: ReactWrapper;
@ -127,7 +127,7 @@ describe('ActivityTable', () => {
activityData: {},
setActiveChild: jest.fn(),
user: { userId: '1' },
loadedCount: 3,
isFetchingActivityData: false,
};
const wrapper = mount(
<Provider store={store}>

29
superset-frontend/src/views/CRUD/welcome/ActivityTable.tsx
View File

@ -74,7 +74,7 @@ interface ActivityProps {
activeChild: string;
setActiveChild: (arg0: string) => void;
activityData: ActivityData;
loadedCount: number;
isFetchingActivityData: boolean;
}
const Styles = styled.div`
@ -128,22 +128,21 @@ export default function ActivityTable({
setActiveChild,
activityData,
user,
loadedCount,
isFetchingActivityData,
}: ActivityProps) {
const [editedObjs, setEditedObjs] = useState<Array<ActivityData>>();
const [loadingState, setLoadingState] = useState(false);
const [editedCards, setEditedCards] = useState<ActivityData[]>();
const [isFetchingEditedCards, setIsFetchingEditedCards] = useState(false);
const getEditedCards = () => {
setLoadingState(true);
setIsFetchingEditedCards(true);
getEditedObjects(user.userId).then(r => {
setEditedObjs([...r.editedChart, ...r.editedDash]);
setLoadingState(false);
setEditedCards([...r.editedChart, ...r.editedDash]);
setIsFetchingEditedCards(false);
});
};
useEffect(() => {
if (activeChild === TableTab.Edited) {
setLoadingState(true);
getEditedCards();
}
}, [activeChild]);
@ -178,9 +177,9 @@ export default function ActivityTable({
});
}
const renderActivity = () =>
(activeChild !== TableTab.Edited
? activityData[activeChild]
: editedObjs
(activeChild === TableTab.Edited
? editedCards
: activityData[activeChild]
).map((entity: ActivityObject) => {
const url = getEntityUrl(entity);
const lastActionOn = getEntityLastActionOn(entity);
@ -200,18 +199,14 @@ export default function ActivityTable({
);
});
const doneFetching = loadedCount < 3;
if ((loadingState && !editedObjs) || doneFetching) {
if ((isFetchingEditedCards && !editedCards) || isFetchingActivityData) {
return <LoadingCards />;
}
return (
<Styles>
<SubMenu activeChild={activeChild} tabs={tabs} />
{activityData[activeChild]?.length > 0 ||
(activeChild === TableTab.Edited &&
editedObjs &&
editedObjs.length > 0) ? (
(activeChild === TableTab.Edited && editedCards?.length) ? (
<CardContainer className="recentCards">
{renderActivity()}
</CardContainer>

55
superset-frontend/src/views/CRUD/welcome/Welcome.test.tsx
View File

@ -106,10 +106,15 @@ const mockedProps = {
userId: 5,
email: 'alpha@alpha.com',
isActive: true,
isAnonymous: false,
permissions: {},
roles: {
sql_lab: [],
},
},
};
describe('Welcome', () => {
describe('Welcome with sql role', () => {
let wrapper: ReactWrapper;
beforeAll(async () => {
@ -122,6 +127,10 @@ describe('Welcome', () => {
});
});
afterAll(() => {
fetchMock.resetHistory();
});
it('renders', () => {
expect(wrapper).toExist();
});
@ -142,6 +151,50 @@ describe('Welcome', () => {
});
});
describe('Welcome without sql role', () => {
let wrapper: ReactWrapper;
beforeAll(async () => {
await act(async () => {
const props = {
...mockedProps,
user: {
...mockedProps.user,
roles: {},
},
};
wrapper = mount(
<Provider store={store}>
<Welcome {...props} />
</Provider>,
);
});
});
afterAll(() => {
fetchMock.resetHistory();
});
it('renders', () => {
expect(wrapper).toExist();
});
it('renders all panels on the page on page load', () => {
expect(wrapper.find('CollapsePanel')).toHaveLength(6);
});
it('calls api methods in parallel on page load', () => {
const chartCall = fetchMock.calls(/chart\/\?q/);
const savedQueryCall = fetchMock.calls(/saved_query\/\?q/);
const recentCall = fetchMock.calls(/superset\/recent_activity\/*/);
const dashboardCall = fetchMock.calls(/dashboard\/\?q/);
expect(chartCall).toHaveLength(2);
expect(recentCall).toHaveLength(1);
expect(savedQueryCall).toHaveLength(0);
expect(dashboardCall).toHaveLength(2);
});
});
async function mountAndWait(props = mockedProps) {
const wrapper = mount(
<Provider store={store}>

107
superset-frontend/src/views/CRUD/welcome/Welcome.tsx
View File

@ -47,6 +47,7 @@ import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
import { AntdSwitch } from 'src/components';
import getBootstrapData from 'src/utils/getBootstrapData';
import { TableTab } from 'src/views/CRUD/types';
import { canUserAccessSqlLab } from 'src/dashboard/util/permissionUtils';
import { WelcomePageLastTab } from './types';
import ActivityTable from './ActivityTable';
import ChartTable from './ChartTable';
@ -161,6 +162,7 @@ export const LoadingCards = ({ cover }: LoadingProps) => (
);
function Welcome({ user, addDangerToast }: WelcomeProps) {
const canAccessSqlLab = canUserAccessSqlLab(user);
const userid = user.userId;
const id = userid!.toString(); // confident that user is not a guest user
const recent = `/superset/recent_activity/${user.userId}/?limit=6`;
@ -178,7 +180,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
const [dashboardData, setDashboardData] = useState<Array<object> | null>(
null,
);
const [loadedCount, setLoadedCount] = useState(0);
const [isFetchingActivityData, setIsFetchingActivityData] = useState(true);
const collapseState = getItem(LocalStorageKeys.homepage_collapse_state, []);
const [activeState, setActiveState] = useState<Array<string>>(collapseState);
@ -260,40 +262,46 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
value: `${id}`,
},
];
getUserOwnedObjects(id, 'dashboard')
.then(r => {
setDashboardData(r);
setLoadedCount(loadedCount => loadedCount + 1);
})
.catch((err: unknown) => {
setDashboardData([]);
setLoadedCount(loadedCount => loadedCount + 1);
addDangerToast(
t('There was an issue fetching your dashboards: %s', err),
);
});
getUserOwnedObjects(id, 'chart')
.then(r => {
setChartData(r);
setLoadedCount(loadedCount => loadedCount + 1);
})
.catch((err: unknown) => {
setChartData([]);
setLoadedCount(loadedCount => loadedCount + 1);
addDangerToast(t('There was an issue fetching your chart: %s', err));
});
getUserOwnedObjects(id, 'saved_query', ownSavedQueryFilters)
.then(r => {
setQueryData(r);
setLoadedCount(loadedCount => loadedCount + 1);
})
.catch((err: unknown) => {
setQueryData([]);
setLoadedCount(loadedCount => loadedCount + 1);
addDangerToast(
t('There was an issues fetching your saved queries: %s', err),
);
});
Promise.all([
getUserOwnedObjects(id, 'dashboard')
.then(r => {
setDashboardData(r);
return Promise.resolve();
})
.catch((err: unknown) => {
setDashboardData([]);
addDangerToast(
t('There was an issue fetching your dashboards: %s', err),
);
return Promise.resolve();
}),
getUserOwnedObjects(id, 'chart')
.then(r => {
setChartData(r);
return Promise.resolve();
})
.catch((err: unknown) => {
setChartData([]);
addDangerToast(t('There was an issue fetching your chart: %s', err));
return Promise.resolve();
}),
canAccessSqlLab
? getUserOwnedObjects(id, 'saved_query', ownSavedQueryFilters)
.then(r => {
setQueryData(r);
return Promise.resolve();
})
.catch((err: unknown) => {
setQueryData([]);
addDangerToast(
t('There was an issue fetching your saved queries: %s', err),
);
return Promise.resolve();
})
: Promise.resolve(),
]).then(() => {
setIsFetchingActivityData(false);
});
}, [otherTabFilters]);
const handleToggle = () => {
@ -323,6 +331,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
const isRecentActivityLoading =
!activityData?.[TableTab.Other] && !activityData?.[TableTab.Viewed];
return (
<WelcomeContainer>
{WelcomeMessageExtension && <WelcomeMessageExtension />}
@ -356,7 +365,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
activeChild={activeChild}
setActiveChild={setActiveChild}
activityData={activityData}
loadedCount={loadedCount}
isFetchingActivityData={isFetchingActivityData}
/>
) : (
<LoadingCards />
@ -390,18 +399,20 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
/>
)}
</Collapse.Panel>
<Collapse.Panel header={t('Saved queries')} key="4">
{!queryData ? (
<LoadingCards cover={checked} />
) : (
<SavedQueries
showThumbnails={checked}
user={user}
mine={queryData}
featureFlag={isFeatureEnabled(FeatureFlag.THUMBNAILS)}
/>
)}
</Collapse.Panel>
{canAccessSqlLab && (
<Collapse.Panel header={t('Saved queries')} key="4">
{!queryData ? (
<LoadingCards cover={checked} />
) : (
<SavedQueries
showThumbnails={checked}
user={user}
mine={queryData}
featureFlag={isFeatureEnabled(FeatureFlag.THUMBNAILS)}
/>
)}
</Collapse.Panel>
)}
</Collapse>
</>
)}

34
superset/security/manager.py
View File

@ -189,7 +189,6 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
}
ADMIN_ONLY_PERMISSIONS = {
"can_sql_json", # TODO: move can_sql_json to sql_lab role
"can_override_role_permissions",
"can_sync_druid_source",
"can_override_role_permissions",
@ -223,11 +222,11 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
ACCESSIBLE_PERMS = {"can_userinfo", "resetmypassword"}
SQLLAB_PERMISSION_VIEWS = {
("can_csv", "Superset"),
SQLLAB_ONLY_PERMISSIONS = {
("can_my_queries", "SqlLab"),
("can_read", "SavedQuery"),
("can_read", "Database"),
("can_sql_json", "Superset"),
("can_sqllab_history", "Superset"),
("can_sqllab_viz", "Superset"),
("can_sqllab_table_viz", "Superset"),
("can_sqllab", "Superset"),
@ -237,6 +236,12 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
("menu_access", "Query Search"),
}
SQLLAB_EXTRA_PERMISSION_VIEWS = {
("can_csv", "Superset"),
("can_read", "Superset"),
("can_read", "Database"),
}
data_access_permissions = (
"database_access",
"schema_access",
@ -908,7 +913,9 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
"""
return not (
self._is_user_defined_permission(pvm) or self._is_admin_only(pvm)
self._is_user_defined_permission(pvm)
or self._is_admin_only(pvm)
or self._is_sql_lab_only(pvm)
) or self._is_accessible_to_all(pvm)
def _is_gamma_pvm(self, pvm: PermissionView) -> bool:
@ -924,8 +931,19 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
self._is_user_defined_permission(pvm)
or self._is_admin_only(pvm)
or self._is_alpha_only(pvm)
or self._is_sql_lab_only(pvm)
) or self._is_accessible_to_all(pvm)
def _is_sql_lab_only(self, pvm: PermissionView) -> bool:
"""
Return True if the FAB permission/view is only SQL Lab related, False
otherwise.
:param pvm: The FAB permission/view
:returns: Whether the FAB object is SQL Lab related
"""
return (pvm.permission.name, pvm.view_menu.name) in self.SQLLAB_ONLY_PERMISSIONS
def _is_sql_lab_pvm(self, pvm: PermissionView) -> bool:
"""
Return True if the FAB permission/view is SQL Lab related, False
@ -934,7 +952,11 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
:param pvm: The FAB permission/view
:returns: Whether the FAB object is SQL Lab related
"""
return (pvm.permission.name, pvm.view_menu.name) in self.SQLLAB_PERMISSION_VIEWS
return (
self._is_sql_lab_only(pvm)
or (pvm.permission.name, pvm.view_menu.name)
in self.SQLLAB_EXTRA_PERMISSION_VIEWS
)
def _is_granter_pvm( # pylint: disable=no-self-use
self, pvm: PermissionView

8
tests/integration_tests/queries/saved_queries/api_tests.py
View File

@ -98,7 +98,7 @@ class TestSavedQueryApi(SupersetTestCase):
self.insert_default_saved_query(
label=f"label{SAVED_QUERIES_FIXTURE_COUNT}",
schema=f"schema{SAVED_QUERIES_FIXTURE_COUNT}",
username="gamma",
username="gamma_sqllab",
)
)
@ -157,12 +157,12 @@ class TestSavedQueryApi(SupersetTestCase):
"""
Saved Query API: Test get list saved query
"""
gamma = self.get_user("gamma")
user = self.get_user("gamma_sqllab")
saved_queries = (
db.session.query(SavedQuery).filter(SavedQuery.created_by == gamma).all()
db.session.query(SavedQuery).filter(SavedQuery.created_by == user).all()
)
self.login(username="gamma")
self.login(username=user.username)
uri = f"api/v1/saved_query/"
rv = self.get_assert_metric(uri, "get_list")
assert rv.status_code == 200

16
tests/integration_tests/sqllab_tests.py
View File

@ -257,6 +257,22 @@ class TestSqlLab(SupersetTestCase):
db.session.commit()
self.assertLess(0, len(data["data"]))
def test_sqllab_has_access(self):
for username in ("admin", "gamma_sqllab"):
self.login(username)
for endpoint in ("/superset/sqllab/", "/superset/sqllab/history/"):
resp = self.client.get(endpoint)
self.assertEqual(200, resp.status_code)
self.logout()
def test_sqllab_no_access(self):
self.login("gamma")
for endpoint in ("/superset/sqllab/", "/superset/sqllab/history/"):
resp = self.client.get(endpoint)
# Redirects to the main page
self.assertEqual(302, resp.status_code)
def test_sql_json_schema_access(self):
examples_db = get_example_database()
db_backend = examples_db.backend