Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: RBAC for can_export for any resource #17527

Merged
merged 12 commits into from
Nov 26, 2021
Merged

fix: RBAC for can_export for any resource #17527

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
101762e
set out export
hughhhh Nov 17, 2021
14e6b38
update test
hughhhh Nov 19, 2021
a7ccc00
Merge branch 'master' of https://github.com/apache/superset into upda…
hughhhh Nov 22, 2021
26b58fe
use default dataset
hughhhh Nov 22, 2021
d91f231
update test
hughhhh Nov 22, 2021
88f8c2d
these test work
hughhhh Nov 23, 2021
a97905d
fix test
hughhhh Nov 23, 2021
6d32c10
update
hughhhh Nov 23, 2021
5e97c4a
fix
hughhhh Nov 23, 2021
8f45b86
fix test
hughhhh Nov 23, 2021
270f676
make the test better
hughhhh Nov 26, 2021
ade23b2
Merge branch 'master' of https://github.com/apache/superset into upda…
hughhhh Nov 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/chart/ChartCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export default function ChartCard({
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');
const canExport =
hasPerm('can_read') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT);
hasPerm('can_export') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT);
const theme = useTheme();

const menu = (
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/chart/ChartList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ function ChartList(props: ChartListProps) {
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');
const canExport =
hasPerm('can_read') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT);
hasPerm('can_export') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT);
const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];

const handleBulkChartExport = (chartsToExport: Chart[]) => {
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/dashboard/DashboardCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ function DashboardCard({
const history = useHistory();
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');
const canExport = hasPerm('can_read');
const canExport = hasPerm('can_export');

const theme = useTheme();
const menu = (
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ function DashboardList(props: DashboardListProps) {
const canCreate = hasPerm('can_write');
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');
const canExport = hasPerm('can_read');
const canExport = hasPerm('can_export');

const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];

Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');
const canExport =
hasPerm('can_read') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT);
hasPerm('can_export') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT);

const menuData: SubMenuProps = {
activeChild: 'Databases',
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');
const canCreate = hasPerm('can_write');
const canExport = hasPerm('can_read');
const canExport = hasPerm('can_export');

const initialSort = SORT_BY;

Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ const mockImportFile = new File(
);

fetchMock.get(queriesInfoEndpoint, {
permissions: ['can_write', 'can_read'],
permissions: ['can_write', 'can_read', 'can_export'],
});
fetchMock.get(queriesEndpoint, {
result: mockqueries,
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ function SavedQueryList({
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');
const canExport =
hasPerm('can_read') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT);
hasPerm('can_export') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT);

const openNewQuery = () => {
window.open(`${window.location.origin}/superset/sqllab?new=true`);
Expand Down
1 change: 0 additions & 1 deletion superset/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ class RouteMethod: # pylint: disable=too-few-public-methods
"bulk_delete": "write",
"delete": "write",
"distinct": "read",
"export": "read",
"get": "read",
"get_list": "read",
"info": "read",
Expand Down
5 changes: 1 addition & 4 deletions tests/integration_tests/charts/api_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,7 @@ def test_info_security_chart(self):
rv = self.get_assert_metric(uri, "info")
data = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 200
assert set(data["permissions"]) == {
"can_read",
"can_write",
}
assert set(data["permissions"]) == {"can_read", "can_write", "can_export"}

def create_chart_import(self):
buf = BytesIO()
Expand Down
4 changes: 1 addition & 3 deletions tests/integration_tests/dashboards/api_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,7 @@ def test_info_security_database(self):
rv = self.get_assert_metric(uri, "info")
data = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 200
assert "can_read" in data["permissions"]
assert "can_write" in data["permissions"]
assert len(data["permissions"]) == 2
assert set(data["permissions"]) == {"can_read", "can_write", "can_export"}

@pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
def test_get_dashboard_not_found(self):
Expand Down
8 changes: 2 additions & 6 deletions tests/integration_tests/databases/api_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,9 +744,7 @@ def test_info_security_database(self):
rv = self.get_assert_metric(uri, "info")
data = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 200
assert "can_read" in data["permissions"]
assert "can_write" in data["permissions"]
assert len(data["permissions"]) == 2
assert set(data["permissions"]) == {"can_read", "can_write", "can_export"}

def test_get_invalid_database_table_metadata(self):
"""
Expand Down Expand Up @@ -1143,9 +1141,7 @@ def test_export_database_not_allowed(self):
argument = [database.id]
uri = f"api/v1/database/export/?q={prison.dumps(argument)}"
rv = self.client.get(uri)
# export only requires can_read now, but gamma need to have explicit access to
# view the database
assert rv.status_code == 404
assert rv.status_code == 401

def test_export_database_non_existing(self):
"""
Expand Down
18 changes: 9 additions & 9 deletions tests/integration_tests/datasets/api_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,9 +375,7 @@ def test_info_security_dataset(self):
rv = self.get_assert_metric(uri, "info")
data = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 200
assert "can_read" in data["permissions"]
assert "can_write" in data["permissions"]
assert len(data["permissions"]) == 2
assert set(data["permissions"]) == {"can_read", "can_write", "can_export"}

def test_create_dataset_item(self):
"""
Expand Down Expand Up @@ -1382,18 +1380,19 @@ def test_export_dataset_not_found(self):
rv = self.get_assert_metric(uri, "export")
assert rv.status_code == 404

@pytest.mark.usefixtures("create_datasets")
def test_export_dataset_gamma(self):
"""
Dataset API: Test export dataset has gamma
"""
birth_names_dataset = self.get_birth_names_dataset()
dataset = self.get_fixture_datasets()[0]

argument = [birth_names_dataset.id]
argument = [dataset.id]
uri = f"api/v1/dataset/export/?q={prison.dumps(argument)}"

self.login(username="gamma")
rv = self.client.get(uri)
assert rv.status_code == 404
assert rv.status_code == 401
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you improve this test, granting can_export to gamma and testing that it can export after that?

I think you can do something like security_manager.add_permission_role("Gamma", "dataset_can_export") here to add the permission.


@patch.dict(
"superset.extensions.feature_flag_manager._feature_flags",
Expand Down Expand Up @@ -1444,19 +1443,20 @@ def test_export_dataset_bundle_not_found(self):
{"VERSIONED_EXPORT": True},
clear=True,
)
@pytest.mark.usefixtures("create_datasets")
def test_export_dataset_bundle_gamma(self):
"""
Dataset API: Test export dataset has gamma
"""
birth_names_dataset = self.get_birth_names_dataset()
dataset = self.get_fixture_datasets()[0]

argument = [birth_names_dataset.id]
argument = [dataset.id]
uri = f"api/v1/dataset/export/?q={prison.dumps(argument)}"

self.login(username="gamma")
rv = self.client.get(uri)
# gamma users by default do not have access to this dataset
assert rv.status_code == 404
assert rv.status_code == 401

@unittest.skip("Number of related objects depend on DB")
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
Expand Down
4 changes: 1 addition & 3 deletions tests/integration_tests/queries/saved_queries/api_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,9 +434,7 @@ def test_info_security_saved_query(self):
rv = self.get_assert_metric(uri, "info")
data = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 200
assert "can_read" in data["permissions"]
assert "can_write" in data["permissions"]
assert len(data["permissions"]) == 2
assert set(data["permissions"]) == {"can_read", "can_write", "can_export"}

def test_related_saved_query(self):
"""
Expand Down