mirror of https://github.com/apache/superset.git
fix: Alpha should not be able to edit datasets that they don't own (#19854)
* fix api for checking owners * fix styles for disabling * fix styles for disabling * fix lint * fix lint * add owners key * plzz * remove * update test * add tooltip * add type * fix test * fix user reference * lit * fix test * work
This commit is contained in:
parent
ada1575177
commit
8b15b68979
|
@ -91,6 +91,11 @@ export const PRIMARY_COLOR = { r: 0, g: 122, b: 135, a: 1 };
|
|||
const ROW_LIMIT_OPTIONS = [10, 50, 100, 250, 500, 1000, 5000, 10000, 50000];
|
||||
const SERIES_LIMITS = [5, 10, 25, 50, 100, 500];
|
||||
|
||||
const appContainer = document.getElementById('app');
|
||||
const { user } = JSON.parse(
|
||||
appContainer?.getAttribute('data-bootstrap') || '{}',
|
||||
);
|
||||
|
||||
type Control = {
|
||||
savedMetrics?: Metric[] | null;
|
||||
default?: unknown;
|
||||
|
@ -167,6 +172,7 @@ const datasourceControl: SharedControlConfig<'DatasourceControl'> = {
|
|||
mapStateToProps: ({ datasource, form_data }) => ({
|
||||
datasource,
|
||||
form_data,
|
||||
user,
|
||||
}),
|
||||
};
|
||||
|
||||
|
|
|
@ -47,7 +47,22 @@ const datasource = {
|
|||
main_dttm_col: 'None',
|
||||
datasource_name: 'table1',
|
||||
description: 'desc',
|
||||
owners: [{ username: 'admin', userId: 1 }],
|
||||
};
|
||||
|
||||
const mockUser = {
|
||||
createdOn: '2021-04-27T18:12:38.952304',
|
||||
email: 'admin',
|
||||
firstName: 'admin',
|
||||
isActive: true,
|
||||
lastName: 'admin',
|
||||
permissions: {},
|
||||
roles: { Admin: Array(173) },
|
||||
userId: 1,
|
||||
username: 'admin',
|
||||
isAnonymous: false,
|
||||
};
|
||||
|
||||
const props: DatasourcePanelProps = {
|
||||
datasource,
|
||||
controls: {
|
||||
|
@ -57,6 +72,7 @@ const props: DatasourcePanelProps = {
|
|||
type: DatasourceControl,
|
||||
label: 'hello',
|
||||
datasource,
|
||||
user: mockUser,
|
||||
},
|
||||
},
|
||||
actions: {
|
||||
|
@ -154,6 +170,7 @@ test('should render a warning', async () => {
|
|||
datasource: {
|
||||
...props.controls.datasource,
|
||||
datasource: deprecatedDatasource,
|
||||
user: mockUser,
|
||||
},
|
||||
},
|
||||
}),
|
||||
|
|
|
@ -31,12 +31,14 @@ import { FAST_DEBOUNCE } from 'src/constants';
|
|||
import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
|
||||
import { ExploreActions } from 'src/explore/actions/exploreActions';
|
||||
import Control from 'src/explore/components/Control';
|
||||
import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
|
||||
import DatasourcePanelDragOption from './DatasourcePanelDragOption';
|
||||
import { DndItemType } from '../DndItemType';
|
||||
import { StyledColumnOption, StyledMetricOption } from '../optionRenderers';
|
||||
|
||||
interface DatasourceControl extends ControlConfig {
|
||||
datasource?: DatasourceMeta;
|
||||
user: UserWithPermissionsAndRoles;
|
||||
}
|
||||
|
||||
export interface Props {
|
||||
|
|
|
@ -41,6 +41,7 @@ const defaultProps = {
|
|||
id: 1,
|
||||
columns: [],
|
||||
metrics: [],
|
||||
owners: [{ username: 'admin', userId: 1 }],
|
||||
database: {
|
||||
backend: 'mysql',
|
||||
name: 'main',
|
||||
|
@ -51,6 +52,17 @@ const defaultProps = {
|
|||
setDatasource: sinon.spy(),
|
||||
},
|
||||
onChange: sinon.spy(),
|
||||
user: {
|
||||
createdOn: '2021-04-27T18:12:38.952304',
|
||||
email: 'admin',
|
||||
firstName: 'admin',
|
||||
isActive: true,
|
||||
lastName: 'admin',
|
||||
permissions: {},
|
||||
roles: { Admin: Array(173) },
|
||||
userId: 1,
|
||||
username: 'admin',
|
||||
},
|
||||
};
|
||||
|
||||
describe('DatasourceControl', () => {
|
||||
|
@ -107,6 +119,7 @@ describe('DatasourceControl', () => {
|
|||
id: 1,
|
||||
columns: [],
|
||||
metrics: [],
|
||||
owners: [{ username: 'admin', userId: 1 }],
|
||||
database: {
|
||||
backend: 'druid',
|
||||
name: 'main',
|
||||
|
|
|
@ -48,6 +48,17 @@ const createProps = () => ({
|
|||
name: 'datasource',
|
||||
actions: {},
|
||||
isEditable: true,
|
||||
user: {
|
||||
createdOn: '2021-04-27T18:12:38.952304',
|
||||
email: 'admin',
|
||||
firstName: 'admin',
|
||||
isActive: true,
|
||||
lastName: 'admin',
|
||||
permissions: {},
|
||||
roles: { Admin: Array(173) },
|
||||
userId: 1,
|
||||
username: 'admin',
|
||||
},
|
||||
onChange: jest.fn(),
|
||||
onDatasourceSave: jest.fn(),
|
||||
});
|
||||
|
|
|
@ -35,6 +35,7 @@ import Button from 'src/components/Button';
|
|||
import ErrorAlert from 'src/components/ErrorMessage/ErrorAlert';
|
||||
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
|
||||
import { URL_PARAMS } from 'src/constants';
|
||||
import { isUserAdmin } from 'src/dashboard/util/findPermission';
|
||||
|
||||
const propTypes = {
|
||||
actions: PropTypes.object.isRequired,
|
||||
|
@ -196,12 +197,32 @@ class DatasourceControl extends React.PureComponent {
|
|||
}
|
||||
|
||||
const isSqlSupported = datasource.type === 'table';
|
||||
const { user } = this.props;
|
||||
const allowEdit =
|
||||
datasource.owners.map(o => o.id).includes(user.userId) ||
|
||||
isUserAdmin(user);
|
||||
|
||||
const editText = t('Edit dataset');
|
||||
|
||||
const datasourceMenu = (
|
||||
<Menu onClick={this.handleMenuItemClick}>
|
||||
{this.props.isEditable && (
|
||||
<Menu.Item key={EDIT_DATASET} data-test="edit-dataset">
|
||||
{t('Edit dataset')}
|
||||
<Menu.Item
|
||||
key={EDIT_DATASET}
|
||||
data-test="edit-dataset"
|
||||
disabled={!allowEdit}
|
||||
>
|
||||
{!allowEdit ? (
|
||||
<Tooltip
|
||||
title={t(
|
||||
'You must be a dataset owner in order to edit. Please reach out to a dataset owner to request modifications or edit access.',
|
||||
)}
|
||||
>
|
||||
{editText}
|
||||
</Tooltip>
|
||||
) : (
|
||||
editText
|
||||
)}
|
||||
</Menu.Item>
|
||||
)}
|
||||
<Menu.Item key={CHANGE_DATASET}>{t('Change dataset')}</Menu.Item>
|
||||
|
|
|
@ -55,6 +55,7 @@ const mockdatasets = [...new Array(3)].map((_, i) => ({
|
|||
id: i,
|
||||
schema: `schema ${i}`,
|
||||
table_name: `coolest table ${i}`,
|
||||
owners: [{ username: 'admin', userId: 1 }],
|
||||
}));
|
||||
|
||||
const mockUser = {
|
||||
|
|
|
@ -56,7 +56,9 @@ import InfoTooltip from 'src/components/InfoTooltip';
|
|||
import ImportModelsModal from 'src/components/ImportModal/index';
|
||||
import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
|
||||
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
|
||||
import { isUserAdmin } from 'src/dashboard/util/findPermission';
|
||||
import AddDatasetModal from './AddDatasetModal';
|
||||
|
||||
import {
|
||||
PAGE_SIZE,
|
||||
SORT_BY,
|
||||
|
@ -75,6 +77,25 @@ const FlexRowContainer = styled.div`
|
|||
|
||||
const Actions = styled.div`
|
||||
color: ${({ theme }) => theme.colors.grayscale.base};
|
||||
|
||||
.disabled {
|
||||
svg,
|
||||
i {
|
||||
&:hover {
|
||||
path {
|
||||
fill: ${({ theme }) => theme.colors.grayscale.light1};
|
||||
}
|
||||
}
|
||||
}
|
||||
color: ${({ theme }) => theme.colors.grayscale.light1};
|
||||
.ant-menu-item:hover {
|
||||
color: ${({ theme }) => theme.colors.grayscale.light1};
|
||||
cursor: default;
|
||||
}
|
||||
&::after {
|
||||
color: ${({ theme }) => theme.colors.grayscale.light1};
|
||||
}
|
||||
}
|
||||
`;
|
||||
|
||||
type Dataset = {
|
||||
|
@ -344,6 +365,11 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
|
|||
},
|
||||
{
|
||||
Cell: ({ row: { original } }: any) => {
|
||||
// Verify owner or isAdmin
|
||||
const allowEdit =
|
||||
original.owners.map((o: Owner) => o.id).includes(user.userId) ||
|
||||
isUserAdmin(user);
|
||||
|
||||
const handleEdit = () => openDatasetEditModal(original);
|
||||
const handleDelete = () => openDatasetDeleteModal(original);
|
||||
const handleExport = () => handleBulkDatasetExport([original]);
|
||||
|
@ -387,14 +413,20 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
|
|||
{canEdit && (
|
||||
<Tooltip
|
||||
id="edit-action-tooltip"
|
||||
title={t('Edit')}
|
||||
placement="bottom"
|
||||
title={
|
||||
allowEdit
|
||||
? t('Edit')
|
||||
: t(
|
||||
'You must be a dataset owner in order to edit. Please reach out to a dataset owner to request modifications or edit access.',
|
||||
)
|
||||
}
|
||||
placement="bottomRight"
|
||||
>
|
||||
<span
|
||||
role="button"
|
||||
tabIndex={0}
|
||||
className="action-button"
|
||||
onClick={handleEdit}
|
||||
className={allowEdit ? 'action-button' : 'disabled'}
|
||||
onClick={allowEdit ? handleEdit : undefined}
|
||||
>
|
||||
<Icons.EditAlt />
|
||||
</span>
|
||||
|
|
|
@ -27,7 +27,7 @@ from marshmallow import ValidationError
|
|||
from sqlalchemy.exc import NoSuchTableError
|
||||
from sqlalchemy.orm.exc import NoResultFound
|
||||
|
||||
from superset import app, db, event_logger
|
||||
from superset import db, event_logger
|
||||
from superset.commands.utils import populate_owners
|
||||
from superset.connectors.connector_registry import ConnectorRegistry
|
||||
from superset.connectors.sqla.utils import get_physical_table_metadata
|
||||
|
@ -81,29 +81,15 @@ class Datasource(BaseSupersetView):
|
|||
|
||||
if "owners" in datasource_dict and orm_datasource.owner_class is not None:
|
||||
# Check ownership
|
||||
if app.config.get("OLD_API_CHECK_DATASET_OWNERSHIP"):
|
||||
# mimic the behavior of the new dataset command that
|
||||
# checks ownership and ensures that non-admins aren't locked out
|
||||
# of the object
|
||||
try:
|
||||
check_ownership(orm_datasource)
|
||||
except SupersetSecurityException as ex:
|
||||
raise DatasetForbiddenError() from ex
|
||||
user = security_manager.get_user_by_id(g.user.id)
|
||||
datasource_dict["owners"] = populate_owners(
|
||||
user, datasource_dict["owners"], default_to_user=False
|
||||
)
|
||||
else:
|
||||
# legacy behavior
|
||||
datasource_dict["owners"] = (
|
||||
db.session.query(orm_datasource.owner_class)
|
||||
.filter(
|
||||
orm_datasource.owner_class.id.in_(
|
||||
datasource_dict["owners"] or []
|
||||
)
|
||||
)
|
||||
.all()
|
||||
)
|
||||
try:
|
||||
check_ownership(orm_datasource)
|
||||
except SupersetSecurityException as ex:
|
||||
raise DatasetForbiddenError() from ex
|
||||
|
||||
user = security_manager.get_user_by_id(g.user.id)
|
||||
datasource_dict["owners"] = populate_owners(
|
||||
user, datasource_dict["owners"], default_to_user=False
|
||||
)
|
||||
|
||||
duplicates = [
|
||||
name
|
||||
|
|
|
@ -278,6 +278,7 @@ class TestDatasource(SupersetTestCase):
|
|||
|
||||
datasource_post = get_datasource_post()
|
||||
datasource_post["id"] = tbl_id
|
||||
datasource_post["owners"] = [1]
|
||||
data = dict(data=json.dumps(datasource_post))
|
||||
resp = self.get_json_resp("/datasource/save/", data)
|
||||
for k in datasource_post:
|
||||
|
@ -299,11 +300,14 @@ class TestDatasource(SupersetTestCase):
|
|||
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
|
||||
def test_change_database(self):
|
||||
self.login(username="admin")
|
||||
admin_user = self.get_user("admin")
|
||||
|
||||
tbl = self.get_table(name="birth_names")
|
||||
tbl_id = tbl.id
|
||||
db_id = tbl.database_id
|
||||
datasource_post = get_datasource_post()
|
||||
datasource_post["id"] = tbl_id
|
||||
datasource_post["owners"] = [admin_user.id]
|
||||
|
||||
new_db = self.create_fake_db()
|
||||
datasource_post["database"]["id"] = new_db.id
|
||||
|
@ -318,10 +322,12 @@ class TestDatasource(SupersetTestCase):
|
|||
|
||||
def test_save_duplicate_key(self):
|
||||
self.login(username="admin")
|
||||
admin_user = self.get_user("admin")
|
||||
tbl_id = self.get_table(name="birth_names").id
|
||||
|
||||
datasource_post = get_datasource_post()
|
||||
datasource_post["id"] = tbl_id
|
||||
datasource_post["owners"] = [admin_user.id]
|
||||
datasource_post["columns"].extend(
|
||||
[
|
||||
{
|
||||
|
@ -346,10 +352,12 @@ class TestDatasource(SupersetTestCase):
|
|||
|
||||
def test_get_datasource(self):
|
||||
self.login(username="admin")
|
||||
admin_user = self.get_user("admin")
|
||||
tbl = self.get_table(name="birth_names")
|
||||
|
||||
datasource_post = get_datasource_post()
|
||||
datasource_post["id"] = tbl.id
|
||||
datasource_post["owners"] = [admin_user.id]
|
||||
data = dict(data=json.dumps(datasource_post))
|
||||
self.get_json_resp("/datasource/save/", data)
|
||||
url = f"/datasource/get/{tbl.type}/{tbl.id}/"
|
||||
|
|
Loading…
Reference in New Issue