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

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

@@ -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,10 +121,34 @@ 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);
 });
 
 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);
+});

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

@@ -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,
+      ))
+  );
+}

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

@@ -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(

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

@@ -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>
     );

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

@@ -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`

superset/errors.py

@@ -65,6 +65,7 @@ class SupersetErrorType(str, Enum):
     QUERY_SECURITY_ACCESS_ERROR = "QUERY_SECURITY_ACCESS_ERROR"
     MISSING_OWNERSHIP_ERROR = "MISSING_OWNERSHIP_ERROR"
     USER_ACTIVITY_SECURITY_ACCESS_ERROR = "USER_ACTIVITY_SECURITY_ACCESS_ERROR"
+    SQL_LAB_SECURITY_ACCESS_ERROR = "SQL_LAB_SECURITY_ACCESS_ERROR"
 
     # Other errors
     BACKEND_TIMEOUT_ERROR = "BACKEND_TIMEOUT_ERROR"

superset/security/manager.py

@@ -1964,6 +1964,19 @@ def raise_for_user_activity_access(user_id: int) -> None:
                 )
             )
 
+    def raise_for_sql_lab_access(self) -> None:
+        if not (
+            self.is_admin()
+            or "sql_lab" in (role.name for role in self.get_user_roles())
+        ):
+            raise SupersetSecurityException(
+                SupersetError(
+                    error_type=SupersetErrorType.SQL_LAB_SECURITY_ACCESS_ERROR,
+                    message="You do not have access to SQL Lab",
+                    level=ErrorLevel.ERROR,
+                )
+            )
+
     def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None:
         """
         Raise an exception if the user cannot access the dashboard.

superset/views/core.py

@@ -2777,6 +2777,12 @@ def _get_sqllab_tabs(user_id: Optional[int]) -> Dict[str, Any]:
     @expose("/sqllab/", methods=["GET", "POST"])
     def sqllab(self) -> FlaskResponse:
         """SQL Editor"""
+        try:
+            security_manager.raise_for_sql_lab_access()
+        except SupersetSecurityException as ex:
+            flash(ex.error.message, "danger")
+            return redirect("/")
+
         payload = {
             "defaultDbId": config["SQLLAB_DEFAULT_DBID"],
             "common": common_bootstrap_payload(g.user),
@@ -2804,6 +2810,12 @@ def sqllab(self) -> FlaskResponse:
     @expose("/sqllab/history/", methods=["GET"])
     @event_logger.log_this
     def sqllab_history(self) -> FlaskResponse:
+        try:
+            security_manager.raise_for_sql_lab_access()
+        except SupersetSecurityException as ex:
+            flash(ex.error.message, "danger")
+            return redirect("/")
+
         return super().render_app_template()
 
     @api

tests/integration_tests/sqllab_tests.py

@@ -257,6 +257,26 @@ def test_sql_json_has_access(self):
         db.session.commit()
         self.assertLess(0, len(data["data"]))
 
+    def test_sqllab_has_access(self):
+        self.create_user_with_roles("sqluser", ["Gamma", "sql_lab"])
+
+        for username in ("admin", "sqluser"):
+            self.login(username)
+            for endpoint in ("/superset/sqllab/", "/superset/sqllab/history/"):
+                resp = self.client.get(endpoint)
+                self.assertEqual(200, resp.status_code)
+
+        user = self.get_user("sqluser")
+        db.session.delete(user)
+        db.session.commit()
+
+    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