dosubot[bot] commented on issue #29777:
URL: https://github.com/apache/superset/issues/29777#issuecomment-2258496706
<!-- Answer -->
To resolve the issue where a Gamma role user can take ownership of a chart
originally owned by a Superset Admin by re-importing the chart in Apache
Superset version 4.0.2, you need to ensure that the ownership validation logic
is correctly implemented. The `UpdateChartCommand` class in
`superset/commands/chart/update.py` provides a good example of how to handle
ownership validation.
Here is an example of how you can modify the `import_chart` function to
include proper ownership checks:
```python
def import_chart(
config: dict[str, Any],
overwrite: bool = False,
ignore_permissions: bool = False,
) -> Slice:
can_write = ignore_permissions or
security_manager.can_access("can_write", "Chart")
existing = db.session.query(Slice).filter_by(uuid=config["uuid"]).first()
if existing:
if overwrite and can_write and get_user():
if not security_manager.can_access_chart(existing):
raise ImportFailedError(
"A chart already exists and user doesn't "
"have permissions to overwrite it"
)
if not overwrite or not can_write:
return existing
config["id"] = existing.id
elif not can_write:
raise ImportFailedError(
"Chart doesn't exist and user doesn't have permission to create
charts"
)
filter_chart_annotations(config)
# TODO (betodealmeida): move this logic to import_from_dict
config["params"] = json.dumps(config["params"])
# migrate old viz types to new ones
config = migrate_chart(config)
chart = Slice.import_from_dict(config, recursive=False,
allow_reparenting=True)
if chart.id is None:
db.session.flush()
if (user := get_user()) and user not in chart.owners:
try:
security_manager.raise_for_ownership(chart)
if security_manager.can_access("can_write", "Chart"):
chart.owners.append(user)
except SupersetSecurityException:
raise ImportFailedError(
"User doesn't have permission to take ownership of the chart"
)
return chart
```
In this modification, the `security_manager.raise_for_ownership(chart)`
method is used to ensure that the user has the necessary permissions to take
ownership of the chart. If the user does not have the required permissions, an
`ImportFailedError` is raised, preventing the Gamma role user from taking
ownership of the chart. This approach aligns with the ownership validation
logic found in the `UpdateChartCommand` class
<sup>[[1]](https://github.com/apache/superset/blob/master/superset/commands/chart/update.py)</sup>.
Additionally, you can refer to the integration tests in
`tests/integration_tests/import_export_tests.py` to ensure that your changes
are properly tested. For example, the
`test_import_new_dashboard_slice_reset_ownership` test case verifies that the
ownership is correctly reset when a dashboard is imported by a different user:
```python
def test_import_new_dashboard_slice_reset_ownership(self):
admin_user = security_manager.find_user(username="admin")
self.assertTrue(admin_user)
gamma_user = security_manager.find_user(username="gamma")
self.assertTrue(gamma_user)
g.user = gamma_user
dash_with_1_slice = self._create_dashboard_for_import(id_=10200)
# set another user as an owner of importing dashboard
dash_with_1_slice.created_by = admin_user
dash_with_1_slice.changed_by = admin_user
dash_with_1_slice.owners = [admin_user]
imported_dash_id = import_dashboard(dash_with_1_slice)
imported_dash = self.get_dash(imported_dash_id)
self.assertEqual(imported_dash.created_by, gamma_user)
self.assertEqual(imported_dash.changed_by, gamma_user)
self.assertEqual(imported_dash.owners, [gamma_user])
imported_slc = imported_dash.slices[0]
self.assertEqual(imported_slc.created_by, gamma_user)
self.assertEqual(imported_slc.changed_by, gamma_user)
self.assertEqual(imported_slc.owners, [gamma_user])
```
This test ensures that the ownership is correctly assigned to the importing
user, preventing unauthorized ownership changes
<sup>[[2]](https://github.com/apache/superset/blob/master/tests/integration_tests/import_export_tests.py)</sup>.
<!-- Dosu Comment Footer -->
*To continue talking to [Dosu](https://dosu.dev), mention @dosu.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
