You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/02/01 18:26:53 UTC
[GitHub] [superset] amitmiran137 opened a new pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
amitmiran137 opened a new pull request #12875:
URL: https://github.com/apache/superset/pull/12875
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
go to the dashboard(http://127.0.0.01/superset/dashboard/<dashboardId>/) to which you have no access (no role nor dataset access)
dashboard should not be visible and should get forbidden 403
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12875:
URL: https://github.com/apache/superset/pull/12875#issuecomment-772848679
# [Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=h1) Report
> Merging [#12875](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=desc) (34deaf5) into [master](https://codecov.io/gh/apache/superset/commit/51195af4fa4581361fb3e61fb0ad31d9c74983a6?el=desc) (51195af) will **decrease** coverage by `21.50%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12875 +/- ##
===========================================
- Coverage 66.93% 45.42% -21.51%
===========================================
Files 1022 477 -545
Lines 50186 17218 -32968
Branches 5204 4444 -760
===========================================
- Hits 33591 7822 -25769
+ Misses 16455 9396 -7059
+ Partials 140 0 -140
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `45.42% <ø> (-4.35%)` | :arrow_down: |
| javascript | `?` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/dashboard/containers/SliceAdder.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL1NsaWNlQWRkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...frontend/src/dashboard/util/dropOverflowsParent.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2Ryb3BPdmVyZmxvd3NQYXJlbnQuanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...frontend/src/dashboard/util/newEntitiesFromDrop.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL25ld0VudGl0aWVzRnJvbURyb3AuanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rontend/src/dashboard/util/componentIsResizable.ts](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2NvbXBvbmVudElzUmVzaXphYmxlLnRz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rc/dashboard/util/getLayoutComponentFromChartId.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldExheW91dENvbXBvbmVudEZyb21DaGFydElkLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rontend/src/dashboard/components/dnd/handleDrop.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVEcm9wLmpz) | `0.00% <0.00%> (-96.16%)` | :arrow_down: |
| [...nd/src/dashboard/util/getComponentWidthFromDrop.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldENvbXBvbmVudFdpZHRoRnJvbURyb3AuanM=) | `0.00% <0.00%> (-95.66%)` | :arrow_down: |
| ... and [904 more](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=footer). Last update [51195af...34deaf5](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r570004146
##########
File path: superset/views/core.py
##########
@@ -1785,6 +1786,11 @@ def publish( # pylint: disable=no-self-use
@has_access
@expose("/dashboard/<dashboard_id_or_slug>/")
@event_logger.log_this_with_extra_payload
+ @check_dashboard_access(
+ on_error=lambda self, ex: Response(
+ utils.error_msg_from_exception(ex), status=403
+ )
+ )
Review comment:
@ktmud look again
I've passed the dashboard queried from the decorator back to the original func.
we could have a dedicated decorator just for querying the dashboard but I think that would be an overhead right now
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569715557
##########
File path: superset/models/dashboard.py
##########
@@ -407,3 +422,22 @@ def clear_dashboard_cache(
sqla.event.listen(TableColumn, "after_update", clear_dashboard_cache)
sqla.event.listen(DruidMetric, "after_update", clear_dashboard_cache)
sqla.event.listen(DruidColumn, "after_update", clear_dashboard_cache)
+
+
+def raise_for_dashboard_access(dashboard: Dashboard) -> None:
+ from superset.views.base import get_user_roles, is_user_admin
+ from superset.views.utils import is_owner
+
+ if is_feature_enabled("DASHBOARD_RBAC"):
+ has_rbac_access = any(
+ dashboard_role.id in [user_role.id for user_role in get_user_roles()]
+ for dashboard_role in dashboard.roles
+ )
+ can_access = (
+ is_user_admin()
+ or is_owner(dashboard, g.user)
+ or (dashboard.published and has_rbac_access)
+ )
+
+ if not can_access:
+ raise DashboardAccessDeniedError()
Review comment:
@ktmud decided to moved the raise method in here just bc it has no use for self so it has no real dependency on the security manager so not sure why should it be there and over-burden
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569426497
##########
File path: superset/security/manager.py
##########
@@ -417,6 +418,27 @@ def can_access_table(self, database: "Database", table: "Table") -> bool:
return True
+ def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None:
+ from superset.views.base import is_user_admin
+ from superset.views.utils import is_owner
+ from superset.models.dashboard import get_dashboard_access_error_object
+ from superset.views.base import get_user_roles
+
+ has_rbac_access = any(
+ dashboard_role.id in [user_role.id for user_role in get_user_roles()]
+ for dashboard_role in dashboard.roles
+ )
+ can_access = (
+ is_user_admin()
+ or is_owner(dashboard, g.user)
+ or (dashboard.published and has_rbac_access)
+ )
+
+ if not can_access:
+ raise SupersetSecurityException(
+ get_dashboard_access_error_object(dashboard)
+ )
Review comment:
it is being used for access as well
https://github.com/apache/superset/blob/abb726df1ba06f4c25e4ce353ea71926ba1a49ca/superset/security/manager.py#L1013
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12875:
URL: https://github.com/apache/superset/pull/12875#issuecomment-772848679
# [Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=h1) Report
> Merging [#12875](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=desc) (34deaf5) into [master](https://codecov.io/gh/apache/superset/commit/51195af4fa4581361fb3e61fb0ad31d9c74983a6?el=desc) (51195af) will **decrease** coverage by `21.87%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12875 +/- ##
===========================================
- Coverage 66.93% 45.05% -21.88%
===========================================
Files 1022 477 -545
Lines 50186 17218 -32968
Branches 5204 4444 -760
===========================================
- Hits 33591 7758 -25833
+ Misses 16455 9460 -6995
+ Partials 140 0 -140
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `45.05% <ø> (-4.72%)` | :arrow_down: |
| javascript | `?` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/dashboard/containers/SliceAdder.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL1NsaWNlQWRkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...frontend/src/dashboard/util/dropOverflowsParent.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2Ryb3BPdmVyZmxvd3NQYXJlbnQuanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...frontend/src/dashboard/util/newEntitiesFromDrop.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL25ld0VudGl0aWVzRnJvbURyb3AuanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rontend/src/dashboard/util/componentIsResizable.ts](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2NvbXBvbmVudElzUmVzaXphYmxlLnRz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rc/dashboard/util/getLayoutComponentFromChartId.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldExheW91dENvbXBvbmVudEZyb21DaGFydElkLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rontend/src/dashboard/components/dnd/handleDrop.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVEcm9wLmpz) | `0.00% <0.00%> (-96.16%)` | :arrow_down: |
| [...nd/src/dashboard/util/getComponentWidthFromDrop.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldENvbXBvbmVudFdpZHRoRnJvbURyb3AuanM=) | `0.00% <0.00%> (-95.66%)` | :arrow_down: |
| ... and [904 more](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=footer). Last update [51195af...34deaf5](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12875:
URL: https://github.com/apache/superset/pull/12875#issuecomment-772848679
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569208067
##########
File path: superset/security/manager.py
##########
@@ -417,6 +418,27 @@ def can_access_table(self, database: "Database", table: "Table") -> bool:
return True
+ def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None:
Review comment:
It's worth adding feature flag checks here, too, just in case this method was accidentally invoked outside of a feature flag enabled brach.
As a general note, unless it's refactoring that will also benefit the base case, new code for feature flags should all wrap behind the flag because it would also make cleaning up the feature flag easier (hope we don't have to do it for this case, though!).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569673638
##########
File path: tests/dashboards/security/security_rbac_tests.py
##########
@@ -129,6 +253,23 @@ def test_get_dashboards_list__user_get_only_published_permitted_dashboards(self)
for dash in published_dashboards + not_published_dashboards:
revoke_access_to_dashboard(dash, new_role)
+ def __arrange_only_published_permitted(self):
Review comment:
my bad on the double underscore.
the name you're suggesting will not indicate its meaning
the purpose of this method is to arrange a scenario matching to tests using it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569429573
##########
File path: superset/views/core.py
##########
@@ -1785,6 +1786,11 @@ def publish( # pylint: disable=no-self-use
@has_access
@expose("/dashboard/<dashboard_id_or_slug>/")
@event_logger.log_this_with_extra_payload
+ @check_permissions(
+ on_error=lambda self, ex: Response(
+ utils.error_msg_from_exception(ex), status=403
+ )
+ )
Review comment:
I agree
there would be also an effort to move all pages into a single page, maybe it could be part of this effort
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 closed pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
amitmiran137 closed pull request #12875:
URL: https://github.com/apache/superset/pull/12875
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569718016
##########
File path: superset/views/core.py
##########
@@ -1811,7 +1810,7 @@ def dashboard( # pylint: disable=too-many-locals
datasource = ConnectorRegistry.get_datasource(
datasource_type=datasource["type"],
datasource_id=datasource["id"],
- session=session,
+ session=db.session(),
Review comment:
bc the session = db.session() was moved into the `Dashboard.get method`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569812897
##########
File path: superset/views/core.py
##########
@@ -1785,6 +1786,11 @@ def publish( # pylint: disable=no-self-use
@has_access
@expose("/dashboard/<dashboard_id_or_slug>/")
@event_logger.log_this_with_extra_payload
+ @check_dashboard_access(
+ on_error=lambda self, ex: Response(
+ utils.error_msg_from_exception(ex), status=403
+ )
+ )
Review comment:
Maybe you can do away with the decorator for now to avoid calling `Dashboard.get` twice, or the decorator could actually update functional signature to pass the already found `dash` object to the wrapped function.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 closed pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
amitmiran137 closed pull request #12875:
URL: https://github.com/apache/superset/pull/12875
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569715557
##########
File path: superset/models/dashboard.py
##########
@@ -407,3 +422,22 @@ def clear_dashboard_cache(
sqla.event.listen(TableColumn, "after_update", clear_dashboard_cache)
sqla.event.listen(DruidMetric, "after_update", clear_dashboard_cache)
sqla.event.listen(DruidColumn, "after_update", clear_dashboard_cache)
+
+
+def raise_for_dashboard_access(dashboard: Dashboard) -> None:
+ from superset.views.base import get_user_roles, is_user_admin
+ from superset.views.utils import is_owner
+
+ if is_feature_enabled("DASHBOARD_RBAC"):
+ has_rbac_access = any(
+ dashboard_role.id in [user_role.id for user_role in get_user_roles()]
+ for dashboard_role in dashboard.roles
+ )
+ can_access = (
+ is_user_admin()
+ or is_owner(dashboard, g.user)
+ or (dashboard.published and has_rbac_access)
+ )
+
+ if not can_access:
+ raise DashboardAccessDeniedError()
Review comment:
@ktmud I decided to moved the raise method in here just bc it has no use for self so it has no real dependency on the security manager so not sure why should it be there and over-burden
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r570004146
##########
File path: superset/views/core.py
##########
@@ -1785,6 +1786,11 @@ def publish( # pylint: disable=no-self-use
@has_access
@expose("/dashboard/<dashboard_id_or_slug>/")
@event_logger.log_this_with_extra_payload
+ @check_dashboard_access(
+ on_error=lambda self, ex: Response(
+ utils.error_msg_from_exception(ex), status=403
+ )
+ )
Review comment:
@ktmud look again
I've passed the dashboard queried from the decorator back to the original func.
we could have a dedicated decorator just for querying the dashboard but I think that would be an overhead right now
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12875:
URL: https://github.com/apache/superset/pull/12875#issuecomment-772848679
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12875:
URL: https://github.com/apache/superset/pull/12875#issuecomment-772848679
# [Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=h1) Report
> Merging [#12875](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=desc) (34deaf5) into [master](https://codecov.io/gh/apache/superset/commit/51195af4fa4581361fb3e61fb0ad31d9c74983a6?el=desc) (51195af) will **decrease** coverage by `7.67%`.
> The diff coverage is `92.15%`.
[](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12875 +/- ##
==========================================
- Coverage 66.93% 59.25% -7.68%
==========================================
Files 1022 966 -56
Lines 50186 45927 -4259
Branches 5204 4444 -760
==========================================
- Hits 33591 27216 -6375
- Misses 16455 18711 +2256
+ Partials 140 0 -140
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `45.42% <ø> (-4.35%)` | :arrow_down: |
| javascript | `?` | |
| python | `67.55% <92.15%> (+3.47%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.90% <85.71%> (-0.52%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <86.95%> (-5.56%)` | :arrow_down: |
| [superset/dashboards/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `100.00% <100.00%> (ø)` | |
| [superset/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `77.82% <100.00%> (+2.08%)` | :arrow_up: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/dashboard/containers/SliceAdder.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL1NsaWNlQWRkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...frontend/src/dashboard/util/dropOverflowsParent.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2Ryb3BPdmVyZmxvd3NQYXJlbnQuanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...frontend/src/dashboard/util/newEntitiesFromDrop.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL25ld0VudGl0aWVzRnJvbURyb3AuanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rontend/src/dashboard/util/componentIsResizable.ts](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2NvbXBvbmVudElzUmVzaXphYmxlLnRz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [446 more](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=footer). Last update [51195af...34deaf5](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569811666
##########
File path: superset/models/dashboard.py
##########
@@ -407,3 +422,22 @@ def clear_dashboard_cache(
sqla.event.listen(TableColumn, "after_update", clear_dashboard_cache)
sqla.event.listen(DruidMetric, "after_update", clear_dashboard_cache)
sqla.event.listen(DruidColumn, "after_update", clear_dashboard_cache)
+
+
+def raise_for_dashboard_access(dashboard: Dashboard) -> None:
+ from superset.views.base import get_user_roles, is_user_admin
+ from superset.views.utils import is_owner
+
+ if is_feature_enabled("DASHBOARD_RBAC"):
+ has_rbac_access = any(
+ dashboard_role.id in [user_role.id for user_role in get_user_roles()]
+ for dashboard_role in dashboard.roles
+ )
+ can_access = (
+ is_user_admin()
+ or is_owner(dashboard, g.user)
+ or (dashboard.published and has_rbac_access)
+ )
+
+ if not can_access:
+ raise DashboardAccessDeniedError()
Review comment:
Sounds good. I'm thinking we may even introduce a new `Mixin` for all entities with role based access, but that can happen later when we do introduce RBAC to more entities.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] srinify merged pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
srinify merged pull request #12875:
URL: https://github.com/apache/superset/pull/12875
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 closed pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
amitmiran137 closed pull request #12875:
URL: https://github.com/apache/superset/pull/12875
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569655860
##########
File path: superset/views/core.py
##########
@@ -1785,6 +1786,11 @@ def publish( # pylint: disable=no-self-use
@has_access
@expose("/dashboard/<dashboard_id_or_slug>/")
@event_logger.log_this_with_extra_payload
+ @check_permissions(
+ on_error=lambda self, ex: Response(
+ utils.error_msg_from_exception(ex), status=403
+ )
+ )
Review comment:
I understand the need for consistency so feel free to keep it as is if you like but I think at some point it would worth migrating all these to each entity's respective `exceptions.py` and do away from `SupersetSecurityException`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] MM-Lehmann commented on pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
MM-Lehmann commented on pull request #12875:
URL: https://github.com/apache/superset/pull/12875#issuecomment-1062999805
Unfortunately, this feature breaks my dataset-based access. I can either use dashboard level OR dataset level. I've ran a db upgrade after setting the FF as well. What went wrong? (I am on 1.4.1)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569205111
##########
File path: superset/models/dashboard.py
##########
@@ -407,3 +408,30 @@ def clear_dashboard_cache(
sqla.event.listen(TableColumn, "after_update", clear_dashboard_cache)
sqla.event.listen(DruidMetric, "after_update", clear_dashboard_cache)
sqla.event.listen(DruidColumn, "after_update", clear_dashboard_cache)
+
+
+def get_dashboard(id_or_slug: str) -> Dashboard:
Review comment:
This is a good candidate for class method. In fact, it would be nice to add a `get_by_id_or_slug` to all entities with slug support for simplicity and consistency. Imagine the following API:
```python
Dashboard.get_by_id(111)
Dashboard.get_by_id_or_slug(xxxx)
# or simply
Dashboard.get(id_or_slug)
Chart.get(id_or_slug)
Tag.get(id_or_slug)
```
##########
File path: superset/security/manager.py
##########
@@ -417,6 +418,27 @@ def can_access_table(self, database: "Database", table: "Table") -> bool:
return True
+ def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None:
Review comment:
It's worth adding feature flag checks here, too, just in case this method was accidentally invoked outside of a feature flag enabled brach.
As a general note, unless it's refactoring that will also benefit the base case, new code for feature flags should all wrap behind the flag because it would also make cleaning up the feature flag easier (hope we don't have to do it for this case!).
##########
File path: superset/security/manager.py
##########
@@ -417,6 +418,27 @@ def can_access_table(self, database: "Database", table: "Table") -> bool:
return True
+ def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None:
+ from superset.views.base import is_user_admin
+ from superset.views.utils import is_owner
+ from superset.models.dashboard import get_dashboard_access_error_object
+ from superset.views.base import get_user_roles
+
+ has_rbac_access = any(
+ dashboard_role.id in [user_role.id for user_role in get_user_roles()]
+ for dashboard_role in dashboard.roles
+ )
+ can_access = (
+ is_user_admin()
+ or is_owner(dashboard, g.user)
+ or (dashboard.published and has_rbac_access)
+ )
+
+ if not can_access:
+ raise SupersetSecurityException(
+ get_dashboard_access_error_object(dashboard)
+ )
Review comment:
I believe `SupersetSecurityException` is reserved for when users haven't logged-in or the login is invalid, because it returns `401` status code.
##########
File path: tests/dashboards/security/security_rbac_tests.py
##########
@@ -48,6 +178,20 @@ def test_get_dashboards_list__admin_get_all_dashboards(self):
def test_get_dashboards_list__owner_get_all_owned_dashboards(self):
# arrange
+ (
+ not_owned_dashboards,
+ owned_dashboards,
+ ) = self.arrange_all_owned_dashboards_scenario()
+
+ # act
+ response = self.get_dashboards_list_response()
+
+ # assert
+ self.assert_dashboards_list_view_response(
+ response, 2, owned_dashboards, not_owned_dashboards
+ )
+
+ def arrange_all_owned_dashboards_scenario(self):
Review comment:
Ditto: `_create_sample_dashboards_with_owner_access`
##########
File path: superset/models/dashboard.py
##########
@@ -407,3 +408,30 @@ def clear_dashboard_cache(
sqla.event.listen(TableColumn, "after_update", clear_dashboard_cache)
sqla.event.listen(DruidMetric, "after_update", clear_dashboard_cache)
sqla.event.listen(DruidColumn, "after_update", clear_dashboard_cache)
+
+
+def get_dashboard(id_or_slug: str) -> Dashboard:
+ session = db.session()
+ qry = session.query(Dashboard)
+ if id_or_slug.isdigit():
+ qry = qry.filter_by(id=int(id_or_slug))
+ else:
+ qry = qry.filter_by(slug=id_or_slug)
+
+ return qry.one_or_none()
+
+
+def get_dashboard_access_error_object(
+ dashboard: Dashboard, # pylint: disable=invalid-name
+) -> SupersetError:
+ """
+ Return the error object for the denied Superset dashboard.
+ :param dashboard: The denied Superset dashboard
+ :returns: The error object
+ """
+ return SupersetError(
+ error_type=SupersetErrorType.DASHBOARD_SECURITY_ACCESS_ERROR,
+ message=f"This dashboard requires to have one of the access roles assigned it",
Review comment:
Can we add the exception to `superset.dashboard.exceptions` instead?
```python
class DashboardAccessDeniedError(ForbiddenError):
message = _("You don't have access to this dashboard.")
```
Since `dashboard_id` is not used anywhere for now, I wouldn't worry about adding it using a custom method, either.
##########
File path: superset/views/core.py
##########
@@ -1785,6 +1786,11 @@ def publish( # pylint: disable=no-self-use
@has_access
@expose("/dashboard/<dashboard_id_or_slug>/")
@event_logger.log_this_with_extra_payload
+ @check_permissions(
+ on_error=lambda self, ex: Response(
+ utils.error_msg_from_exception(ex), status=403
+ )
+ )
Review comment:
This is an HTML page. Returning a simple 403 response is probably not enough. We should have a decorator similar to `handle_api_exception` to handle exceptions on HTML pages.
Ideally all exceptions should be handled in a single final handler for consistent error message rendering.
But maybe the dashboard page is complex enough to warrant its own access denied page, too.
cc @junlincc
##########
File path: superset/views/core.py
##########
@@ -1811,7 +1810,7 @@ def dashboard( # pylint: disable=too-many-locals
datasource = ConnectorRegistry.get_datasource(
datasource_type=datasource["type"],
datasource_id=datasource["id"],
- session=session,
+ session=db.session(),
Review comment:
Could you explain why is this change necessary?
##########
File path: tests/dashboards/security/security_rbac_tests.py
##########
@@ -96,23 +232,11 @@ def test_get_dashboards_list__user_without_any_permissions_get_empty_list(self):
def test_get_dashboards_list__user_get_only_published_permitted_dashboards(self):
# arrange
- username = random_str()
- new_role = f"role_{random_str()}"
- self.create_user_with_roles(username, [new_role], should_create_roles=True)
-
- published_dashboards = [
- create_dashboard_to_db(published=True),
- create_dashboard_to_db(published=True),
- ]
- not_published_dashboards = [
- create_dashboard_to_db(published=False),
- create_dashboard_to_db(published=False),
- ]
-
- for dash in published_dashboards + not_published_dashboards:
- grant_access_to_dashboard(dash, new_role)
-
- self.login(username)
+ (
+ new_role,
+ not_published_dashboards,
Review comment:
Maybe we can call this `draft_dashboard`?
##########
File path: tests/dashboards/security/security_rbac_tests.py
##########
@@ -129,6 +253,23 @@ def test_get_dashboards_list__user_get_only_published_permitted_dashboards(self)
for dash in published_dashboards + not_published_dashboards:
revoke_access_to_dashboard(dash, new_role)
+ def __arrange_only_published_permitted(self):
Review comment:
Is double underscore a new convention for private methods? The method name is also a little unclear. How about `def _create_sample_dashboards_with_role_access`
##########
File path: superset/utils/decorators.py
##########
@@ -69,3 +74,34 @@ def wrapped(*args: Any, **kwargs: Any) -> Any:
return wrapped
return decorate
+
+
+def on_security_exception(self, ex) -> Response:
+ return self.response(403, **{"message": utils.error_msg_from_exception(ex)})
+
+
+def check_permissions(
Review comment:
The name of this decorator is too generic for a check that's only on dashboards.
##########
File path: tests/dashboards/security/security_rbac_tests.py
##########
@@ -31,6 +31,136 @@
"superset.extensions.feature_flag_manager._feature_flags", DASHBOARD_RBAC=True,
)
class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
+ def test_get_dashboard_view__admin_can_access(self):
+ # arrange
+ dashboard_to_access = create_dashboard_to_db(
+ owners=[], slices=[create_slice_to_db()], published=False
+ )
+ self.login("admin")
+
+ # act
+ response = self.get_dashboard_view_response(dashboard_to_access)
+
+ # assert
+ self.assert_dashboard_view_response(response, dashboard_to_access)
+
+ def test_get_dashboard_view__owner_can_access(self):
+ # arrange
+ username = random_str()
+ new_role = f"role_{random_str()}"
+ owner = self.create_user_with_roles(
+ username, [new_role], should_create_roles=True
+ )
+ dashboard_to_access = create_dashboard_to_db(
+ owners=[owner], slices=[create_slice_to_db()], published=False
+ )
+ self.login(username)
+
+ # act
+ response = self.get_dashboard_view_response(dashboard_to_access)
+
+ # assert
+ self.assert_dashboard_view_response(response, dashboard_to_access)
+
+ def test_get_dashboard_view__user_can_not_access_without_permission(self):
+ username = random_str()
+ new_role = f"role_{random_str()}"
+ self.create_user_with_roles(username, [new_role], should_create_roles=True)
+ dashboard_to_access = create_dashboard_to_db(published=True)
+ self.login(username)
+
+ # act
+ response = self.get_dashboard_view_response(dashboard_to_access)
+
+ # assert
+ self.assert403(response)
+
+ def test_get_dashboard_view__user_with_dashboard_permission_can_not_access_not_published(
+ self,
+ ):
+ # arrange
+ dashboard_to_access = create_dashboard_to_db(published=False)
+ username = random_str()
+ new_role = f"role_{random_str()}"
+ self.create_user_with_roles(username, [new_role], should_create_roles=True)
+ grant_access_to_dashboard(dashboard_to_access, new_role)
+ self.login(username)
+
+ # act
+ response = self.get_dashboard_view_response(dashboard_to_access)
+
+ # assert
+ self.assert403(response)
+
+ # post
+ revoke_access_to_dashboard(dashboard_to_access, new_role)
+
+ def test_get_dashboard_view__user_access_with_dashboard_permission(self):
+ # arrange
+
+ username = random_str()
+ new_role = f"role_{random_str()}"
+ self.create_user_with_roles(username, [new_role], should_create_roles=True)
+
+ dashboard_to_access = create_dashboard_to_db(
+ published=True, slices=[create_slice_to_db()]
+ )
+ self.login(username)
+ grant_access_to_dashboard(dashboard_to_access, new_role)
+
+ # act
+ response = self.get_dashboard_view_response(dashboard_to_access)
+
+ # assert
+ self.assert_dashboard_view_response(response, dashboard_to_access)
+
+ # post
+ revoke_access_to_dashboard(dashboard_to_access, new_role)
+
+ def test_get_dashboard_view__public_user_can_not_access_without_permission(self):
+ dashboard_to_access = create_dashboard_to_db(published=True)
+ self.logout()
+
+ # act
+ response = self.get_dashboard_view_response(dashboard_to_access)
+
+ # assert
+ self.assert403(response)
+
+ def test_get_dashboard_view__public_user_with_dashboard_permission_can_not_access_not_published(
Review comment:
nit: `replaceAll('not_published', 'draft')`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12875:
URL: https://github.com/apache/superset/pull/12875#issuecomment-772848679
# [Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=h1) Report
> Merging [#12875](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=desc) (34deaf5) into [master](https://codecov.io/gh/apache/superset/commit/51195af4fa4581361fb3e61fb0ad31d9c74983a6?el=desc) (51195af) will **decrease** coverage by `8.07%`.
> The diff coverage is `92.00%`.
[](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12875 +/- ##
==========================================
- Coverage 66.93% 58.85% -8.08%
==========================================
Files 1022 966 -56
Lines 50186 45899 -4287
Branches 5204 4444 -760
==========================================
- Hits 33591 27013 -6578
- Misses 16455 18886 +2431
+ Partials 140 0 -140
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `45.42% <ø> (-4.35%)` | :arrow_down: |
| javascript | `?` | |
| python | `66.91% <92.00%> (+2.83%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `71.77% <85.71%> (-3.65%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <86.95%> (-5.56%)` | :arrow_down: |
| [superset/dashboards/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `100.00% <100.00%> (ø)` | |
| [superset/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.62% <100.00%> (-0.13%)` | :arrow_down: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/dashboard/containers/SliceAdder.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL1NsaWNlQWRkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...frontend/src/dashboard/util/dropOverflowsParent.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2Ryb3BPdmVyZmxvd3NQYXJlbnQuanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...frontend/src/dashboard/util/newEntitiesFromDrop.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL25ld0VudGl0aWVzRnJvbURyb3AuanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rontend/src/dashboard/util/componentIsResizable.ts](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2NvbXBvbmVudElzUmVzaXphYmxlLnRz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [467 more](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=footer). Last update [51195af...34deaf5](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569693084
##########
File path: tests/dashboards/security/security_rbac_tests.py
##########
@@ -129,6 +253,23 @@ def test_get_dashboards_list__user_get_only_published_permitted_dashboards(self)
for dash in published_dashboards + not_published_dashboards:
revoke_access_to_dashboard(dash, new_role)
+ def __arrange_only_published_permitted(self):
Review comment:
Sure, naming is hard... I think in this case the intent is very clear, but it would be nice to add a docstring in the future if the function name is not very straightforward.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 commented on a change in pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #12875:
URL: https://github.com/apache/superset/pull/12875#discussion_r569421901
##########
File path: superset/models/dashboard.py
##########
@@ -407,3 +408,30 @@ def clear_dashboard_cache(
sqla.event.listen(TableColumn, "after_update", clear_dashboard_cache)
sqla.event.listen(DruidMetric, "after_update", clear_dashboard_cache)
sqla.event.listen(DruidColumn, "after_update", clear_dashboard_cache)
+
+
+def get_dashboard(id_or_slug: str) -> Dashboard:
Review comment:
I agree on the classmethod .
there are no other Entities that has slug
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] srinify merged pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
srinify merged pull request #12875:
URL: https://github.com/apache/superset/pull/12875
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io commented on pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12875:
URL: https://github.com/apache/superset/pull/12875#issuecomment-772848679
# [Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=h1) Report
> Merging [#12875](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=desc) (1ac6d8b) into [master](https://codecov.io/gh/apache/superset/commit/51195af4fa4581361fb3e61fb0ad31d9c74983a6?el=desc) (51195af) will **decrease** coverage by `5.09%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12875 +/- ##
==========================================
- Coverage 66.93% 61.83% -5.10%
==========================================
Files 1022 536 -486
Lines 50186 20075 -30111
Branches 5204 5241 +37
==========================================
- Hits 33591 12414 -21177
+ Misses 16455 7449 -9006
- Partials 140 212 +72
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `61.83% <ø> (-0.10%)` | :arrow_down: |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/dashboard/containers/Dashboard.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/filters/components/Select/types.ts](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvdHlwZXMudHM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/dashboard/containers/SliceAdder.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL1NsaWNlQWRkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/explore/reducers/getInitialState.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvZ2V0SW5pdGlhbFN0YXRlLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...-frontend/src/visualizations/presets/MainPreset.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL3ByZXNldHMvTWFpblByZXNldC5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...tend/src/dashboard/containers/DashboardBuilder.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [691 more](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=footer). Last update [51195af...1ac6d8b](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12875: feat(dashboard_rbac): dashboard_view access enforcement
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12875:
URL: https://github.com/apache/superset/pull/12875#issuecomment-772848679
# [Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=h1) Report
> Merging [#12875](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=desc) (34deaf5) into [master](https://codecov.io/gh/apache/superset/commit/51195af4fa4581361fb3e61fb0ad31d9c74983a6?el=desc) (51195af) will **decrease** coverage by `7.67%`.
> The diff coverage is `92.15%`.
[](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12875 +/- ##
==========================================
- Coverage 66.93% 59.25% -7.68%
==========================================
Files 1022 966 -56
Lines 50186 45927 -4259
Branches 5204 4444 -760
==========================================
- Hits 33591 27216 -6375
- Misses 16455 18711 +2256
+ Partials 140 0 -140
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `45.42% <ø> (-4.35%)` | :arrow_down: |
| javascript | `?` | |
| python | `67.55% <92.15%> (+3.47%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.90% <85.71%> (-0.52%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <86.95%> (-5.56%)` | :arrow_down: |
| [superset/dashboards/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `100.00% <100.00%> (ø)` | |
| [superset/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `77.82% <100.00%> (+2.08%)` | :arrow_up: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/dashboard/containers/SliceAdder.jsx](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL1NsaWNlQWRkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...frontend/src/dashboard/util/dropOverflowsParent.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2Ryb3BPdmVyZmxvd3NQYXJlbnQuanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...frontend/src/dashboard/util/newEntitiesFromDrop.js](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL25ld0VudGl0aWVzRnJvbURyb3AuanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rontend/src/dashboard/util/componentIsResizable.ts](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2NvbXBvbmVudElzUmVzaXphYmxlLnRz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [446 more](https://codecov.io/gh/apache/superset/pull/12875/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=footer). Last update [51195af...34deaf5](https://codecov.io/gh/apache/superset/pull/12875?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org