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 2020/01/29 14:42:14 UTC
[GitHub] [incubator-superset] dpgaspar opened a new pull request #9046:
Fix/api superset queryview list
dpgaspar opened a new pull request #9046: Fix/api superset queryview list
URL: https://github.com/apache/incubator-superset/pull/9046
### CATEGORY
- [ ] Bug Fix
- [X] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
Proposal to deprecate `can_only_access_owned_queries` in favor of the new `can_access_all_queries`
The default changes from all users can view all other user's queries to user's can access only their own queries except `Admin` users.
`can_access_all_queries` can be added to existing roles giving their corresponding user's access to everyone's queries
### ADDITIONAL INFORMATION
- [ ] 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
### REVIEWERS
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #9046:
[query] deprecate can_only_access_owned_queries
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #9046: [query] deprecate can_only_access_owned_queries
URL: https://github.com/apache/incubator-superset/pull/9046#issuecomment-579863316
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9046?src=pr&el=h1) Report
> Merging [#9046](https://codecov.io/gh/apache/incubator-superset/pull/9046?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/22699a204d35c065650094e7bdcdbf5b0a208c33?src=pr&el=desc) will **increase** coverage by `0.29%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9046?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9046 +/- ##
==========================================
+ Coverage 59.16% 59.45% +0.29%
==========================================
Files 367 369 +2
Lines 11680 11747 +67
Branches 2863 2888 +25
==========================================
+ Hits 6910 6984 +74
+ Misses 4591 4584 -7
Partials 179 179
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9046?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../assets/src/SqlLab/components/AceEditorWrapper.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29tcG9uZW50cy9BY2VFZGl0b3JXcmFwcGVyLmpzeA==) | `54.21% <0%> (-1.48%)` | :arrow_down: |
| [superset/assets/src/SqlLab/constants.js](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29uc3RhbnRzLmpz) | `100% <0%> (ø)` | :arrow_up: |
| [superset/assets/src/chart/chartAction.js](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jaGFydC9jaGFydEFjdGlvbi5qcw==) | `47.94% <0%> (ø)` | :arrow_up: |
| [...et/assets/src/components/IndeterminateCheckbox.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0luZGV0ZXJtaW5hdGVDaGVja2JveC5qc3g=) | `100% <0%> (ø)` | |
| [...rset/assets/src/components/ConfirmStatusChange.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0NvbmZpcm1TdGF0dXNDaGFuZ2UudHN4) | `100% <0%> (ø)` | |
| [superset/assets/src/components/ListView/utils.ts](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L3V0aWxzLnRz) | `98.46% <0%> (+0.31%)` | :arrow_up: |
| [...uperset/assets/src/SqlLab/components/SqlEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29tcG9uZW50cy9TcWxFZGl0b3IuanN4) | `51.94% <0%> (+0.31%)` | :arrow_up: |
| [superset/assets/src/components/TableSelector.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL1RhYmxlU2VsZWN0b3IuanN4) | `78.35% <0%> (+0.32%)` | :arrow_up: |
| [...uperset/assets/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvY29tcG9uZW50cy9IZWFkZXIuanN4) | `42.06% <0%> (+0.93%)` | :arrow_up: |
| [...perset/assets/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L0xpc3RWaWV3LnRzeA==) | `92.2% <0%> (+1.03%)` | :arrow_up: |
| ... and [1 more](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9046?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/incubator-superset/pull/9046?src=pr&el=footer). Last update [22699a2...0702289](https://codecov.io/gh/apache/incubator-superset/pull/9046?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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #9046: [query]
deprecate can_only_access_owned_queries
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9046: [query] deprecate can_only_access_owned_queries
URL: https://github.com/apache/incubator-superset/pull/9046#issuecomment-579863316
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9046?src=pr&el=h1) Report
> Merging [#9046](https://codecov.io/gh/apache/incubator-superset/pull/9046?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/22699a204d35c065650094e7bdcdbf5b0a208c33?src=pr&el=desc) will **increase** coverage by `0.27%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9046?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9046 +/- ##
==========================================
+ Coverage 59.16% 59.43% +0.27%
==========================================
Files 367 369 +2
Lines 11680 11743 +63
Branches 2863 2884 +21
==========================================
+ Hits 6910 6980 +70
+ Misses 4591 4584 -7
Partials 179 179
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9046?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../assets/src/SqlLab/components/AceEditorWrapper.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29tcG9uZW50cy9BY2VFZGl0b3JXcmFwcGVyLmpzeA==) | `54.21% <0%> (-1.48%)` | :arrow_down: |
| [superset/assets/src/SqlLab/constants.js](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29uc3RhbnRzLmpz) | `100% <0%> (ø)` | :arrow_up: |
| [superset/assets/src/chart/chartAction.js](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jaGFydC9jaGFydEFjdGlvbi5qcw==) | `47.94% <0%> (ø)` | :arrow_up: |
| [...et/assets/src/components/IndeterminateCheckbox.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0luZGV0ZXJtaW5hdGVDaGVja2JveC5qc3g=) | `100% <0%> (ø)` | |
| [...rset/assets/src/components/ConfirmStatusChange.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0NvbmZpcm1TdGF0dXNDaGFuZ2UudHN4) | `100% <0%> (ø)` | |
| [superset/assets/src/components/ListView/utils.ts](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L3V0aWxzLnRz) | `98.46% <0%> (+0.31%)` | :arrow_up: |
| [...uperset/assets/src/SqlLab/components/SqlEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29tcG9uZW50cy9TcWxFZGl0b3IuanN4) | `51.94% <0%> (+0.31%)` | :arrow_up: |
| [...perset/assets/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L0xpc3RWaWV3LnRzeA==) | `92.2% <0%> (+1.03%)` | :arrow_up: |
| [...t/assets/src/views/dashboardList/DashboardList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy92aWV3cy9kYXNoYm9hcmRMaXN0L0Rhc2hib2FyZExpc3QudHN4) | `73.39% <0%> (+14.44%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9046?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/incubator-superset/pull/9046?src=pr&el=footer). Last update [22699a2...4fa6167](https://codecov.io/gh/apache/incubator-superset/pull/9046?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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request
#9046: [query] deprecate can_only_access_owned_queries
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #9046: [query] deprecate can_only_access_owned_queries
URL: https://github.com/apache/incubator-superset/pull/9046#discussion_r373086427
##########
File path: superset/security/manager.py
##########
@@ -177,15 +177,13 @@ def can_access(self, permission_name: str, view_name: str) -> bool:
return self.is_item_public(permission_name, view_name)
return self._has_view_access(user, permission_name, view_name)
- def can_only_access_owned_queries(self) -> bool:
+ def can_access_all_queries(self) -> bool:
"""
- Return True if the user can only access owned queries, False otherwise.
+ Return True if the user can access all queries, False otherwise.
- :returns: Whether the use can only access owned queries
+ :returns: Whether the use can access all queries
"""
- return self.can_access(
- "can_only_access_owned_queries", "can_only_access_owned_queries"
- )
+ return self.can_access("can_access_all_queries", "can_access_all_queries")
Review comment:
changed
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar merged pull request #9046: [query]
deprecate can_only_access_owned_queries
Posted by GitBox <gi...@apache.org>.
dpgaspar merged pull request #9046: [query] deprecate can_only_access_owned_queries
URL: https://github.com/apache/incubator-superset/pull/9046
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] willbarrett commented on a change in pull
request #9046: [query] deprecate can_only_access_owned_queries
Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #9046: [query] deprecate can_only_access_owned_queries
URL: https://github.com/apache/incubator-superset/pull/9046#discussion_r372655710
##########
File path: superset/security/manager.py
##########
@@ -538,11 +536,10 @@ def create_custom_permissions(self) -> None:
"""
Create custom FAB permissions.
"""
-
self.add_permission_view_menu("all_datasource_access", "all_datasource_access")
self.add_permission_view_menu("all_database_access", "all_database_access")
self.add_permission_view_menu(
Review comment:
Why is creating the permission tied to the concept of a `view` and the concept of a `menu`?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on issue #9046: [query]
deprecate can_only_access_owned_queries
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on issue #9046: [query] deprecate can_only_access_owned_queries
URL: https://github.com/apache/incubator-superset/pull/9046#issuecomment-581515429
@graceguo-supercat it would be awesome if you could take a look
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] willbarrett commented on a change in pull
request #9046: [query] deprecate can_only_access_owned_queries
Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #9046: [query] deprecate can_only_access_owned_queries
URL: https://github.com/apache/incubator-superset/pull/9046#discussion_r372655476
##########
File path: superset/security/manager.py
##########
@@ -177,15 +177,13 @@ def can_access(self, permission_name: str, view_name: str) -> bool:
return self.is_item_public(permission_name, view_name)
return self._has_view_access(user, permission_name, view_name)
- def can_only_access_owned_queries(self) -> bool:
+ def can_access_all_queries(self) -> bool:
"""
- Return True if the user can only access owned queries, False otherwise.
+ Return True if the user can access all queries, False otherwise.
- :returns: Whether the use can only access owned queries
+ :returns: Whether the use can access all queries
"""
- return self.can_access(
- "can_only_access_owned_queries", "can_only_access_owned_queries"
- )
+ return self.can_access("can_access_all_queries", "can_access_all_queries")
Review comment:
The naming of this permission is inconsistent with the naming of other similar permissions. If we wished to bring this in line with `all_datasource_access` and `all_database_access` I would recommend altering this name to `all_query_access`. `can_access_` in the permission name is redundant.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request
#9046: [query] deprecate can_only_access_owned_queries
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #9046: [query] deprecate can_only_access_owned_queries
URL: https://github.com/apache/incubator-superset/pull/9046#discussion_r373086721
##########
File path: superset/views/core.py
##########
@@ -2597,7 +2597,7 @@ def search_queries(self) -> Response:
:returns: Response with list of sql query dicts
"""
query = db.session.query(Query)
- if security_manager.can_only_access_owned_queries():
+ if not security_manager.can_access_all_queries():
Review comment:
changed
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #9046:
[query] deprecate can_only_access_owned_queries
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #9046: [query] deprecate can_only_access_owned_queries
URL: https://github.com/apache/incubator-superset/pull/9046#issuecomment-579863316
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9046?src=pr&el=h1) Report
> Merging [#9046](https://codecov.io/gh/apache/incubator-superset/pull/9046?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/22699a204d35c065650094e7bdcdbf5b0a208c33?src=pr&el=desc) will **increase** coverage by `0.29%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9046?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9046 +/- ##
==========================================
+ Coverage 59.16% 59.45% +0.29%
==========================================
Files 367 369 +2
Lines 11680 11747 +67
Branches 2863 2888 +25
==========================================
+ Hits 6910 6984 +74
+ Misses 4591 4584 -7
Partials 179 179
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9046?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../assets/src/SqlLab/components/AceEditorWrapper.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29tcG9uZW50cy9BY2VFZGl0b3JXcmFwcGVyLmpzeA==) | `54.21% <0%> (-1.48%)` | :arrow_down: |
| [superset/assets/src/SqlLab/constants.js](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29uc3RhbnRzLmpz) | `100% <0%> (ø)` | :arrow_up: |
| [superset/assets/src/chart/chartAction.js](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jaGFydC9jaGFydEFjdGlvbi5qcw==) | `47.94% <0%> (ø)` | :arrow_up: |
| [...et/assets/src/components/IndeterminateCheckbox.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0luZGV0ZXJtaW5hdGVDaGVja2JveC5qc3g=) | `100% <0%> (ø)` | |
| [...rset/assets/src/components/ConfirmStatusChange.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0NvbmZpcm1TdGF0dXNDaGFuZ2UudHN4) | `100% <0%> (ø)` | |
| [superset/assets/src/components/ListView/utils.ts](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L3V0aWxzLnRz) | `98.46% <0%> (+0.31%)` | :arrow_up: |
| [...uperset/assets/src/SqlLab/components/SqlEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29tcG9uZW50cy9TcWxFZGl0b3IuanN4) | `51.94% <0%> (+0.31%)` | :arrow_up: |
| [superset/assets/src/components/TableSelector.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL1RhYmxlU2VsZWN0b3IuanN4) | `78.35% <0%> (+0.32%)` | :arrow_up: |
| [...uperset/assets/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvY29tcG9uZW50cy9IZWFkZXIuanN4) | `42.06% <0%> (+0.93%)` | :arrow_up: |
| [...perset/assets/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L0xpc3RWaWV3LnRzeA==) | `92.2% <0%> (+1.03%)` | :arrow_up: |
| ... and [1 more](https://codecov.io/gh/apache/incubator-superset/pull/9046/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9046?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/incubator-superset/pull/9046?src=pr&el=footer). Last update [22699a2...4e8f617](https://codecov.io/gh/apache/incubator-superset/pull/9046?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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] craig-rueda commented on a change in pull
request #9046: [query] deprecate can_only_access_owned_queries
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #9046: [query] deprecate can_only_access_owned_queries
URL: https://github.com/apache/incubator-superset/pull/9046#discussion_r372501142
##########
File path: superset/security/manager.py
##########
@@ -177,15 +177,13 @@ def can_access(self, permission_name: str, view_name: str) -> bool:
return self.is_item_public(permission_name, view_name)
return self._has_view_access(user, permission_name, view_name)
- def can_only_access_owned_queries(self) -> bool:
+ def can_access_all_queries(self) -> bool:
"""
- Return True if the user can only access owned queries, False otherwise.
+ Return True if the user can access all queries, False otherwise.
- :returns: Whether the use can only access owned queries
+ :returns: Whether the use can access all queries
Review comment:
nit: typo `use` -> `user`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] willbarrett commented on a change in pull
request #9046: [query] deprecate can_only_access_owned_queries
Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #9046: [query] deprecate can_only_access_owned_queries
URL: https://github.com/apache/incubator-superset/pull/9046#discussion_r372656785
##########
File path: superset/views/core.py
##########
@@ -2597,7 +2597,7 @@ def search_queries(self) -> Response:
:returns: Response with list of sql query dicts
"""
query = db.session.query(Query)
- if security_manager.can_only_access_owned_queries():
+ if not security_manager.can_access_all_queries():
Review comment:
You could avoid the negated `if` by reversing the order of the conditions:
```python
if security_manager.can_access_all_queries():
search_user_id = request.args.get("user_id")
else:
search_user_id = g.user.get_user_id()
```
Additionally, I would recommend returning a 403 if the requestor asks for a user ID other than their own but does not have permissions to see other users' queries. Otherwise it would be possible for the end user to ask for a list of another users' queries but get back a list of their own queries. This would be an incorrect result.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org