You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2023/08/10 17:04:14 UTC
[superset] branch master updated: fix(charts): View in SQL Lab with relevant perm (#24903)
This is an automated email from the ASF dual-hosted git repository.
villebro pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new ce65a3b9cd fix(charts): View in SQL Lab with relevant perm (#24903)
ce65a3b9cd is described below
commit ce65a3b9cd56e4d9e1966e78e577ef7ec18d6412
Author: Rob Moore <gi...@users.noreply.github.com>
AuthorDate: Thu Aug 10 18:04:07 2023 +0100
fix(charts): View in SQL Lab with relevant perm (#24903)
---
.../src/dashboard/util/permissionUtils.test.ts | 51 ++++++++++++++++------
.../src/dashboard/util/permissionUtils.ts | 17 +++++---
.../DatasourceControl/DatasourceControl.test.tsx | 2 +-
.../controls/DatasourceControl/index.jsx | 4 +-
superset-frontend/src/pages/Home/Home.test.tsx | 2 +-
superset-frontend/src/pages/Home/index.tsx | 8 ++--
6 files changed, 56 insertions(+), 28 deletions(-)
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('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', () => {
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) {
/>
)}
</Collapse.Panel>
- {canAccessSqlLab && (
+ {canReadSavedQueries && (
<Collapse.Panel header={t('Saved queries')} key="4">
{!queryData ? (
<LoadingCards cover={checked} />