Skip to content

Commit

Permalink
fix: Native filter dashboard RBAC aware dataset permission (#25029)
Browse files Browse the repository at this point in the history
(cherry picked from commit 60889d2)
  • Loading branch information
john-bodley authored and michael-s-molina committed Aug 22, 2023
1 parent 8cb5142 commit 1af6df3
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export const getFormData = ({
granularity_sqla,
type,
dashboardId,
id,
}: Partial<Filter> & {
dashboardId: number;
datasetId?: number;
Expand Down Expand Up @@ -94,6 +95,7 @@ export const getFormData = ({
viz_type: filterType,
type,
dashboardId,
native_filter_id: id,
};
};

Expand Down
35 changes: 29 additions & 6 deletions superset/security/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# under the License.
# pylint: disable=too-many-lines
"""A set of constants and methods to manage permissions and security"""
import json
import logging
import re
import time
Expand Down Expand Up @@ -1876,14 +1877,36 @@ def raise_for_access(
.one_or_none()
)
and dashboard_.roles
and (slice_id := form_data.get("slice_id"))
and (
slc := self.get_session.query(Slice)
.filter(Slice.id == slice_id)
.one_or_none()
(
# Native filter.
form_data.get("type") == "NATIVE_FILTER"
and (native_filter_id := form_data.get("native_filter_id"))
and dashboard_.json_metadata
and (json_metadata := json.loads(dashboard_.json_metadata))
and any(
target.get("datasetId") == datasource.id
for fltr in json_metadata.get(
"native_filter_configuration",
[],
)
for target in fltr.get("targets", [])
if native_filter_id == fltr.get("id")
)
)
or (
# Chart.
form_data.get("type") != "NATIVE_FILTER"
and (slice_id := form_data.get("slice_id"))
and (
slc := self.get_session.query(Slice)
.filter(Slice.id == slice_id)
.one_or_none()
)
and slc in dashboard_.slices
and slc.datasource == datasource
)
)
and slc in dashboard_.slices
and slc.datasource == datasource
and self.can_access_dashboard(dashboard_)
)
):
Expand Down
63 changes: 60 additions & 3 deletions tests/integration_tests/security_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# specific language governing permissions and limitations
# under the License.
# isort:skip_file
import json
import inspect
import time
import unittest
Expand Down Expand Up @@ -1718,14 +1719,28 @@ def test_raise_for_access_rbac(
world_health = self.get_dash_by_slug("world_health")
treemap = self.get_slice("Treemap", db.session, expunge_from_session=False)

births.json_metadata = json.dumps(
{
"native_filter_configuration": [
{
"id": "NATIVE_FILTER-ABCDEFGH",
"targets": [{"datasetId": birth_names.id}],
},
{
"id": "NATIVE_FILTER-IJKLMNOP",
"targets": [{"datasetId": treemap.id}],
},
]
}
)

mock_g.user = security_manager.find_user("gamma")
mock_is_owner.return_value = False
mock_can_access.return_value = False
mock_can_access_schema.return_value = False

for kwarg in ["query_context", "viz"]:
births.roles = []
db.session.flush()

# No dashboard roles.
with self.assertRaises(SupersetSecurityException):
Expand All @@ -1742,7 +1757,6 @@ def test_raise_for_access_rbac(
)

births.roles = [self.get_role("Gamma")]
db.session.flush()

# Undefined dashboard.
with self.assertRaises(SupersetSecurityException):
Expand Down Expand Up @@ -1807,7 +1821,50 @@ def test_raise_for_access_rbac(
}
)

db.session.rollback()
# Ill-defined native filter.
with self.assertRaises(SupersetSecurityException):
security_manager.raise_for_access(
**{
kwarg: Mock(
datasource=birth_names,
form_data={
"dashboardId": births.id,
"type": "NATIVE_FILTER",
},
)
}
)

# Native filter not associated with said datasource.
with self.assertRaises(SupersetSecurityException):
security_manager.raise_for_access(
**{
kwarg: Mock(
datasource=birth_names,
form_data={
"dashboardId": births.id,
"native_filter_id": "NATIVE_FILTER-IJKLMNOP",
"type": "NATIVE_FILTER",
},
)
}
)

# Native filter associated with said datasource.
security_manager.raise_for_access(
**{
kwarg: Mock(
datasource=birth_names,
form_data={
"dashboardId": births.id,
"native_filter_id": "NATIVE_FILTER-ABCDEFGH",
"type": "NATIVE_FILTER",
},
)
}
)

db.session.expunge_all()

@patch("superset.security.manager.g")
def test_get_user_roles(self, mock_g):
Expand Down

0 comments on commit 1af6df3

Please sign in to comment.