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/06/10 21:19:19 UTC
[GitHub] [incubator-superset] john-bodley opened a new pull request #10034: chore(security): Updating assert logic
john-bodley opened a new pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034
### SUMMARY
This PR updates the security manager assertion logic making it more flexible for deployments to configure their security manager. Currently there exists two types of methods:
1. `can_access_*`, i.e., `access_datasource` (which will be renamed to `can_access_datasource` in https://github.com/apache/incubator-superset/pull/10031), which returns a boolean in response to whether the user can access said resource.
2. `assert_*_permission`, i.e., `assert_datasource_permission`, which returns `None` but raises an exception if the user cannot access said resource.
The issue with this approach is that some methods like `access_datasource` have all the logic and `assert_datasource_permission` calls `access_datasource` and raises an exception if the response is `False`, whereas others like `can_access_datasource` (which will be renamed to `can_access_table` in https://github.com/apache/incubator-superset/pull/10031) have no assert equivalent.
Additionally context as to why a user cannot access said resource lost when the logic resides in the `can_access_*` and thus the preferred approach is to have the logic in the `assert_*_permission` method and thus the `can_access_*` methods can take the form,
```python
def can_access_datasource(self, datasource) -> bool:
try:
self.assert_datasource_permission(datasource)
except SupersetSecurityException:
return False
return True
```
Secondly the term `assert` seems strange as one would expect it to raise an `AssertionError` if the assertion fails, whereas these methods raise a `SupersetSecurityException`, hence I opted for the `requests` approach (which uses `raise_for_status`) to call these methods `raise_for_access` and thus it's clearer than an exception is raised. I also added a wrapper function to various classes so rather than,
```python
query_context = QueryContext(**json.loads(request.form["query_context"]))
security_manager.assert_query_context_permission(query_context)
```
the logic is now,
```python
query_context = QueryContext(**json.loads(request.form["query_context"]))
query_context.raise_for_access()
```
Finally rather than having a slew of `raise_for_*_access` which can add for confusion if a deployment needs to override these, i.e., they intrinsically need to know how these interact, I defined a single `raise_for_access` method which contains all the logic regardless of the resource type (table, datasource, visualization, etc.).
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
CI.
### 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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **increase** coverage by `0.63%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
+ Coverage 70.47% 71.11% +0.63%
==========================================
Files 583 400 -183
Lines 31019 12854 -18165
Branches 3175 3179 +4
==========================================
- Hits 21861 9141 -12720
+ Misses 9049 3604 -5445
Partials 109 109
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `53.93% <ø> (+<0.01%)` | :arrow_up: |
| #javascript | `59.49% <ø> (+0.07%)` | :arrow_up: |
| #python | `?` | |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `66.81% <0.00%> (ø)` | |
| [superset/datasets/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YXNldHMvc2NoZW1hcy5weQ==) | | |
| [superset/charts/dao.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2Rhby5weQ==) | | |
| [superset/app.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXBwLnB5) | | |
| [superset/db\_engine\_specs/kylin.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2t5bGluLnB5) | | |
| [superset/examples/sf\_population\_polygons.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvc2ZfcG9wdWxhdGlvbl9wb2x5Z29ucy5weQ==) | | |
| [superset/migrations/env.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy9lbnYucHk=) | | |
| [superset/utils/url\_map\_converters.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvdXJsX21hcF9jb252ZXJ0ZXJzLnB5) | | |
| [superset/errors.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXJyb3JzLnB5) | | |
| [superset/dashboards/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy9jcmVhdGUucHk=) | | |
| ... and [170 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...bf94a70](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] dpgaspar commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r443719624
##########
File path: superset/charts/api.py
##########
@@ -450,7 +450,7 @@ def data(self) -> Response:
except KeyError:
return self.response_400(message="Request is incorrect")
try:
- security_manager.assert_query_context_permission(query_context)
+ query_context.raise_for_access()
Review comment:
@etr2460 #9529 kind of addresses that, it's a POC for SIP-41, uses flask error handling
----------------------------------------------------------------
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] [incubator-superset] john-bodley edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-647080998
@villebro I've rebased. Note in commit https://github.com/apache/incubator-superset/pull/10034/commits/95fe0ec0c31538d1baa8c1139bc7bf35610960a7 I also extended the logic to replace the `rejected_tables` method. I think this provides clarity/simplicity in the views but possibly adds complexity to the `raise_for_access` 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] [incubator-superset] dpgaspar commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r443719624
##########
File path: superset/charts/api.py
##########
@@ -450,7 +450,7 @@ def data(self) -> Response:
except KeyError:
return self.response_400(message="Request is incorrect")
try:
- security_manager.assert_query_context_permission(query_context)
+ query_context.raise_for_access()
Review comment:
@etr2460 #9529 kind of addresses that, it's a POC for SIP-41, is uses flask error handling
----------------------------------------------------------------
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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **decrease** coverage by `1.55%`.
> The diff coverage is `92.45%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
- Coverage 70.47% 68.92% -1.56%
==========================================
Files 583 584 +1
Lines 31019 31079 +60
Branches 3175 3180 +5
==========================================
- Hits 21861 21421 -440
- Misses 9049 9548 +499
- Partials 109 110 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `53.89% <ø> (-0.03%)` | :arrow_down: |
| #javascript | `59.48% <ø> (+0.06%)` | :arrow_up: |
| #python | `67.40% <92.45%> (-2.67%)` | :arrow_down: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
| [superset/views/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYXBpLnB5) | `66.66% <50.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.39% <80.00%> (-0.89%)` | :arrow_down: |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `80.31% <100.00%> (ø)` | |
| [superset/common/query\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2NvbnRleHQucHk=) | `79.62% <100.00%> (+0.25%)` | :arrow_up: |
| [superset/connectors/base/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `86.57% <100.00%> (-2.36%)` | :arrow_down: |
| [superset/security/manager.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `91.17% <100.00%> (+2.13%)` | :arrow_up: |
| [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.11% <100.00%> (-0.07%)` | :arrow_down: |
| [superset/connectors/druid/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9kcnVpZC9tb2RlbHMucHk=) | `28.16% <0.00%> (-54.28%)` | :arrow_down: |
| ... and [14 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...7c05667](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] villebro commented on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-646979897
@john-bodley this needs a rebase
----------------------------------------------------------------
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] [incubator-superset] john-bodley commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r438417720
##########
File path: superset/security/manager.py
##########
@@ -858,38 +852,65 @@ def set_perm(
)
)
- def assert_datasource_permission(self, datasource: "BaseDatasource") -> None:
+ def raise_for_access(
+ self,
+ database: Optional["Database"] = None,
+ datasource: Optional["BaseDatasource"] = None,
+ query_context: Optional["QueryContext"] = None,
+ table: Optional["Table"] = None,
+ viz: Optional["BaseViz"] = None,
+ ) -> None:
"""
- Assert the the user has permission to access the Superset datasource.
+ Raise an exception if the user cannot access the resource.
+ :param database: The Superset database (see table)
:param datasource: The Superset datasource
- :raises SupersetSecurityException: If the user does not have permission
+ :param query_context: The query context
+ :param table: The Superset table (see database)
+ :param viz: The visualization
+ :raises SupersetSecurityException: If the user cannot access the resource
"""
- if not self.datasource_access(datasource):
- raise SupersetSecurityException(
- self.get_datasource_access_error_object(datasource),
+ from superset import db
Review comment:
I wonder if there should be any assertion logic to ensure that the right combination of parameters are defined. Note except for `database` and `table` these are all mutually exclusive. I did consider having an argument `database_and_table` which would be an `Optional[Tuple["Database, "Table']]` but I wasn't sold on the idea and thus opted for the additional docstring context.
##########
File path: superset/views/core.py
##########
@@ -2656,8 +2668,7 @@ def fetch_datasource_metadata(self) -> FlaskResponse:
if not datasource:
return json_error_response(DATASOURCE_MISSING_ERR)
- # Check permission for datasource
Review comment:
Annexed self explanatory comment.
----------------------------------------------------------------
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] [incubator-superset] etr2460 commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r438522620
##########
File path: superset/security/manager.py
##########
@@ -269,9 +269,12 @@ def datasource_access(self, datasource: "BaseDatasource") -> bool:
:returns: Whether the use can access the Superset datasource
"""
- return self.schema_access(datasource) or self.can_access(
- "datasource_access", datasource.perm
- )
+ try:
+ self.raise_for_access(datasource=datasource)
+ except SupersetSecurityException:
Review comment:
If there's a different exception thrown, then this fails open. Is that secure?
----------------------------------------------------------------
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] [incubator-superset] codecov-commenter commented on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **decrease** coverage by `10.98%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
===========================================
- Coverage 70.47% 59.49% -10.99%
===========================================
Files 583 400 -183
Lines 31019 12854 -18165
Branches 3175 3179 +4
===========================================
- Hits 21861 7647 -14214
+ Misses 9049 5029 -4020
- Partials 109 178 +69
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.49% <ø> (+0.07%)` | :arrow_up: |
| #python | `?` | |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/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/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [315 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...4eb4e21](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] etr2460 commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r440312048
##########
File path: superset/charts/api.py
##########
@@ -450,7 +450,7 @@ def data(self) -> Response:
except KeyError:
return self.response_400(message="Request is incorrect")
try:
- security_manager.assert_query_context_permission(query_context)
+ query_context.raise_for_access()
Review comment:
not changed in this PR, but this try/catch seems kinda weird, I would've expected this api to be wrapped in the `handle_api_exception` decorator that i believe automatically generates the proper json error response from an exception
----------------------------------------------------------------
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] [incubator-superset] john-bodley merged pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley merged pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034
----------------------------------------------------------------
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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **decrease** coverage by `10.98%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
===========================================
- Coverage 70.47% 59.49% -10.99%
===========================================
Files 583 400 -183
Lines 31019 12854 -18165
Branches 3175 3179 +4
===========================================
- Hits 21861 7647 -14214
+ Misses 9049 5029 -4020
- Partials 109 178 +69
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.49% <ø> (+0.07%)` | :arrow_up: |
| #python | `?` | |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/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/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [315 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...bf94a70](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **decrease** coverage by `6.34%`.
> The diff coverage is `92.45%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
- Coverage 70.47% 64.12% -6.35%
==========================================
Files 583 584 +1
Lines 31019 31079 +60
Branches 3175 3180 +5
==========================================
- Hits 21861 19930 -1931
- Misses 9049 10971 +1922
- Partials 109 178 +69
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.48% <ø> (+0.06%)` | :arrow_up: |
| #python | `67.40% <92.45%> (-2.67%)` | :arrow_down: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
| [superset/views/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYXBpLnB5) | `66.66% <50.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.39% <80.00%> (-0.89%)` | :arrow_down: |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `80.31% <100.00%> (ø)` | |
| [superset/common/query\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2NvbnRleHQucHk=) | `79.62% <100.00%> (+0.25%)` | :arrow_up: |
| [superset/connectors/base/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `86.57% <100.00%> (-2.36%)` | :arrow_down: |
| [superset/security/manager.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `91.17% <100.00%> (+2.13%)` | :arrow_up: |
| [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.11% <100.00%> (-0.07%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/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/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [157 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...7c05667](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] john-bodley edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-647594422
@villebro I agree that the `raise_for_access` method is fairly loaded (though in general I like the standardization it has provided in terms of checking for access from the perspective of a query, viz, etc.).
The mixin is an interesting idea, though I sense it mightn’t simply the logic in `raise_for_access` as one would need to do a number of `isinstance` checks to work out whether the `DatasourceMixin` is a query context, viz, etc to pull out the appropriate attributes. Also this doesn’t address the issue of the `Table` argument as there’s no mixin related to that.
----------------------------------------------------------------
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] [incubator-superset] villebro commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r443320839
##########
File path: superset/security/manager.py
##########
@@ -858,38 +852,65 @@ def set_perm(
)
)
- def assert_datasource_permission(self, datasource: "BaseDatasource") -> None:
+ def raise_for_access(
+ self,
+ database: Optional["Database"] = None,
+ datasource: Optional["BaseDatasource"] = None,
+ query_context: Optional["QueryContext"] = None,
+ table: Optional["Table"] = None,
+ viz: Optional["BaseViz"] = None,
+ ) -> None:
"""
- Assert the the user has permission to access the Superset datasource.
+ Raise an exception if the user cannot access the resource.
+ :param database: The Superset database (see table)
:param datasource: The Superset datasource
- :raises SupersetSecurityException: If the user does not have permission
+ :param query_context: The query context
+ :param table: The Superset table (see database)
+ :param viz: The visualization
+ :raises SupersetSecurityException: If the user cannot access the resource
"""
- if not self.datasource_access(datasource):
- raise SupersetSecurityException(
- self.get_datasource_access_error_object(datasource),
+ from superset import db
Review comment:
An optional approach here would be to define `DatasourceMixin` which provides a datasource, and add it to `BaseViz`, `QueryContext` and `Datasource`. in this case we could simplify the signature of this method by combining `viz`, `datasource` and `query_context` into one single `datasource: DatasourceMixin`. The same could be done to `query` and `database` (`DatabaseMixin`). This could help clarify what the diffefent arguments mean.
----------------------------------------------------------------
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] [incubator-superset] john-bodley commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r443095456
##########
File path: superset/security/manager.py
##########
@@ -318,7 +320,7 @@ def get_datasource_access_error_object(
},
)
- def get_table_access_error_msg(self, tables: Set["Table"]) -> str:
+ def get_table_access_error_msg(self, tables: Set[str]) -> str:
Review comment:
Sadly this is somewhat of a half step back in order to move forward with standardizing the access patterns. Note long term I believe all these somewhat unpleasant msg/link methods will be deprecated in favor of the client leveraging the `SupersetError` logic which @etr2460 introduced.
----------------------------------------------------------------
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] [incubator-superset] john-bodley commented on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-644240099
@dpgaspar and @villebro I was wondering whether you had any updated thoughts regarding this approach.
----------------------------------------------------------------
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] [incubator-superset] john-bodley commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r443094694
##########
File path: superset/security/manager.py
##########
@@ -232,7 +233,8 @@ def can_access_all_databases(self) -> bool:
def can_access_database(self, database: "Database") -> bool:
"""
- Return True if the user can access the Superset database, False otherwise.
+ Return True if the user can access all the tables within the Superset database,
Review comment:
I though there was merit in being _more_ explicit about what it means to be able to access a database, schema, etc. It's not overly apparent without the comment.
----------------------------------------------------------------
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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **increase** coverage by `0.15%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
+ Coverage 70.47% 70.63% +0.15%
==========================================
Files 583 400 -183
Lines 31019 12854 -18165
Branches 3175 3179 +4
==========================================
- Hits 21861 9079 -12782
+ Misses 9049 3660 -5389
- Partials 109 115 +6
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `53.15% <ø> (-0.78%)` | :arrow_down: |
| #javascript | `59.49% <ø> (+0.07%)` | :arrow_up: |
| #python | `?` | |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/SqlLab/reducers/getInitialState.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9nZXRJbml0aWFsU3RhdGUuanM=) | `33.33% <0.00%> (-16.67%)` | :arrow_down: |
| [superset-frontend/src/reduxUtils.ts](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3JlZHV4VXRpbHMudHM=) | `70.88% <0.00%> (-8.87%)` | :arrow_down: |
| [...rontend/src/SqlLab/components/TabbedSqlEditors.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RhYmJlZFNxbEVkaXRvcnMuanN4) | `75.52% <0.00%> (-6.30%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `61.13% <0.00%> (-5.68%)` | :arrow_down: |
| [...rontend/src/SqlLab/components/SqlEditorLeftBar.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIuanN4) | `44.00% <0.00%> (-4.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/reducers/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9zcWxMYWIuanM=) | `37.44% <0.00%> (-3.30%)` | :arrow_down: |
| [...end/src/SqlLab/components/TemplateParamsEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RlbXBsYXRlUGFyYW1zRWRpdG9yLmpzeA==) | `88.57% <0.00%> (-2.86%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/SqlEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvci5qc3g=) | `53.84% <0.00%> (-1.29%)` | :arrow_down: |
| [...rontend/src/SqlLab/components/AceEditorWrapper.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0FjZUVkaXRvcldyYXBwZXIudHN4) | `55.91% <0.00%> (-1.08%)` | :arrow_down: |
| [superset/tasks/schedules.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdGFza3Mvc2NoZWR1bGVzLnB5) | | |
| ... and [178 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...4eb4e21](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] john-bodley commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r443095456
##########
File path: superset/security/manager.py
##########
@@ -318,7 +320,7 @@ def get_datasource_access_error_object(
},
)
- def get_table_access_error_msg(self, tables: Set["Table"]) -> str:
+ def get_table_access_error_msg(self, tables: Set[str]) -> str:
Review comment:
Sadly this is somewhat of a half step back in order to move forward with standardizing the access patterns. Note long term I believe all these somewhat unpleasant msg/link methods will be deprecated in favor of the client leveraging the `SupersetError` logic which @etr2460 introduced.
----------------------------------------------------------------
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] [incubator-superset] etr2460 commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r438958864
##########
File path: superset/security/manager.py
##########
@@ -269,9 +269,12 @@ def datasource_access(self, datasource: "BaseDatasource") -> bool:
:returns: Whether the use can access the Superset datasource
"""
- return self.schema_access(datasource) or self.can_access(
- "datasource_access", datasource.perm
- )
+ try:
+ self.raise_for_access(datasource=datasource)
+ except SupersetSecurityException:
Review comment:
In theory, yes. I guess, what if there's an error from the db level or somewhere else and it throws a different exception? Should we do something instead like:
```python
try:
self.raise_for_access(datasource=datasource)
return True
except SupersetSecurityException:
pass
return False
```
----------------------------------------------------------------
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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **decrease** coverage by `6.39%`.
> The diff coverage is `92.45%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
- Coverage 70.47% 64.08% -6.40%
==========================================
Files 583 584 +1
Lines 31019 31079 +60
Branches 3175 3180 +5
==========================================
- Hits 21861 19917 -1944
- Misses 9049 10984 +1935
- Partials 109 178 +69
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.48% <ø> (+0.06%)` | :arrow_up: |
| #python | `67.33% <92.45%> (-2.74%)` | :arrow_down: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
| [superset/views/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYXBpLnB5) | `66.66% <50.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.18% <80.00%> (-1.10%)` | :arrow_down: |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `80.31% <100.00%> (ø)` | |
| [superset/common/query\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2NvbnRleHQucHk=) | `79.62% <100.00%> (+0.25%)` | :arrow_up: |
| [superset/connectors/base/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `86.57% <100.00%> (-2.36%)` | :arrow_down: |
| [superset/security/manager.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `91.17% <100.00%> (+2.13%)` | :arrow_up: |
| [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.11% <100.00%> (-0.07%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/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/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [159 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...7c05667](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] john-bodley edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-647594422
@villebro I agree that the `raise_for_access` method is fairly loaded, though in general I like the standardization it has provided in terms of checking for access from the perspective of a query, viz, etc. It also has reduced a significant amount of the spaghetti security logic, i.e., pervious `assert_datasource_permission `, `can_access_datasource`, `can_access_table`, `rejected_tables`, etc. whereas now there's just the `raise_for_access` and a convenience method.
The mixin is an interesting idea, though I sense it mightn’t simply the logic in `raise_for_access` as one would need to do a number of `isinstance` checks to work out whether the `DatasourceMixin` is a query context, viz, etc to pull out the appropriate attributes. Also this doesn’t address the issue of the `Table` argument as there’s no mixin related to that.
----------------------------------------------------------------
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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **increase** coverage by `0.61%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
+ Coverage 70.47% 71.09% +0.61%
==========================================
Files 583 400 -183
Lines 31019 12854 -18165
Branches 3175 3179 +4
==========================================
- Hits 21861 9138 -12723
+ Misses 9049 3607 -5442
Partials 109 109
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `53.86% <ø> (-0.07%)` | :arrow_down: |
| #javascript | `59.49% <ø> (+0.07%)` | :arrow_up: |
| #python | `?` | |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rontend/src/SqlLab/components/QueryAutoRefresh.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5QXV0b1JlZnJlc2guanN4) | `65.90% <0.00%> (-6.82%)` | :arrow_down: |
| [superset/extensions.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXh0ZW5zaW9ucy5weQ==) | | |
| [superset/db\_engine\_specs/mssql.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL21zc3FsLnB5) | | |
| [superset/dashboards/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9zY2hlbWFzLnB5) | | |
| [superset/models/annotations.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2Fubm90YXRpb25zLnB5) | | |
| [superset/views/chart/filters.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY2hhcnQvZmlsdGVycy5weQ==) | | |
| [superset/utils/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | | |
| [superset/examples/unicode\_test\_data.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvdW5pY29kZV90ZXN0X2RhdGEucHk=) | | |
| [superset/db\_engine\_specs/snowflake.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Nub3dmbGFrZS5weQ==) | | |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | | |
| ... and [170 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...4eb4e21](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9532bff48fd2a5c01f3e5e69d3bce166822951b5&el=desc) will **decrease** coverage by `4.60%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
- Coverage 64.08% 59.48% -4.61%
==========================================
Files 584 400 -184
Lines 31054 12856 -18198
Branches 3180 3180
==========================================
- Hits 19901 7647 -12254
+ Misses 10975 5031 -5944
Partials 178 178
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `59.48% <ø> (ø)` | |
| #python | `?` | |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/views/dashboard/views.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGFzaGJvYXJkL3ZpZXdzLnB5) | | |
| [superset/datasets/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvY3JlYXRlLnB5) | | |
| [superset/datasets/commands/delete.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvZGVsZXRlLnB5) | | |
| [superset/views/log/views.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvbG9nL3ZpZXdzLnB5) | | |
| [superset/views/annotations.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYW5ub3RhdGlvbnMucHk=) | | |
| [superset/dashboards/commands/bulk\_delete.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy9idWxrX2RlbGV0ZS5weQ==) | | |
| [superset/dao/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGFvL2Jhc2UucHk=) | | |
| [superset/views/database/views.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | | |
| [superset/views/filters.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZmlsdGVycy5weQ==) | | |
| [superset/sql\_validators/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvX19pbml0X18ucHk=) | | |
| ... and [166 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [9532bff...06efb7d](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9532bff48fd2a5c01f3e5e69d3bce166822951b5&el=desc) will **increase** coverage by `4.84%`.
> The diff coverage is `92.15%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
+ Coverage 64.08% 68.93% +4.84%
==========================================
Files 584 584
Lines 31054 31077 +23
Branches 3180 3180
==========================================
+ Hits 19901 21422 +1521
+ Misses 10975 9546 -1429
+ Partials 178 109 -69
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `53.92% <ø> (?)` | |
| #javascript | `59.48% <ø> (ø)` | |
| #python | `67.40% <92.15%> (+0.06%)` | :arrow_up: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
| [superset/views/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYXBpLnB5) | `66.66% <50.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.39% <75.00%> (ø)` | |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `80.31% <100.00%> (ø)` | |
| [superset/common/query\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2NvbnRleHQucHk=) | `79.62% <100.00%> (+0.25%)` | :arrow_up: |
| [superset/connectors/base/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `86.57% <100.00%> (+0.14%)` | :arrow_up: |
| [superset/security/manager.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `91.17% <100.00%> (+2.13%)` | :arrow_up: |
| [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.11% <100.00%> (+0.05%)` | :arrow_up: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.10% <0.00%> (+0.14%)` | :arrow_up: |
| [...rontend/src/SqlLab/components/AceEditorWrapper.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0FjZUVkaXRvcldyYXBwZXIudHN4) | `56.98% <0.00%> (+1.07%)` | :arrow_up: |
| ... and [147 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [9532bff...06efb7d](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] john-bodley edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-647080998
@villebro I've rebased. Note in the second commit I also extended the logic to replace the `rejected_tables` method. I think this provides clarity/simplicity in the views but possibly adds complexity to the `raise_for_access` 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] [incubator-superset] dpgaspar commented on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-647622278
@etr2460, @john-bodley
I'm aware of this change, will take a look today
----------------------------------------------------------------
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] [incubator-superset] dpgaspar commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r443736809
##########
File path: superset/security/manager.py
##########
@@ -860,45 +834,93 @@ def set_perm(
)
)
- def assert_datasource_permission(self, datasource: "BaseDatasource") -> None:
+ def raise_for_access(
+ self,
+ database: Optional["Database"] = None,
+ datasource: Optional["BaseDatasource"] = None,
+ query: Optional["Query"] = None,
+ query_context: Optional["QueryContext"] = None,
+ table: Optional["Table"] = None,
+ viz: Optional["BaseViz"] = None,
+ ) -> None:
"""
- Assert the the user has permission to access the Superset datasource.
+ Raise an exception if the user cannot access the resource.
+ :param database: The Superset database
:param datasource: The Superset datasource
- :raises SupersetSecurityException: If the user does not have permission
+ :param query: The SQL Lab query
+ :param query_context: The query context
+ :param table: The Superset table (requires database)
+ :param viz: The visualization
+ :raises SupersetSecurityException: If the user cannot access the resource
"""
- if not self.can_access_datasource(datasource):
- raise SupersetSecurityException(
- self.get_datasource_access_error_object(datasource),
- )
+ from superset import db
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.sql_parse import Table
- def assert_query_context_permission(self, query_context: "QueryContext") -> None:
- """
- Assert the the user has permission to access the query context.
+ if database and table or query:
+ if query:
+ database = query.database
- :param query_context: The query context
- :raises SupersetSecurityException: If the user does not have permission
- """
+ database = cast("Database", database)
- self.assert_datasource_permission(query_context.datasource)
+ if self.can_access_database(database):
+ return
- def assert_viz_permission(self, viz: "BaseViz") -> None:
- """
- Assert the the user has permission to access the visualization.
+ if query:
+ tables = {
+ Table(table_.table, table_.schema or query.schema)
+ for table_ in sql_parse.ParsedQuery(query.sql).tables
+ }
+ elif table:
+ tables = {table}
- :param viz: The visualization
- :raises SupersetSecurityException: If the user does not have permission
- """
+ inaccessible = set()
+
+ for table_ in tables:
+ schema_perm = self.get_schema_perm(database, schema=table_.schema)
+
+ if not (schema_perm and self.can_access("schema_access", schema_perm)):
+ datasources = SqlaTable.query_datasources_by_name(
+ db.session, database, table_.table, schema=table_.schema
Review comment:
Regarding the `from superset import db`
We could replace it by `self.get_session`
----------------------------------------------------------------
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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **decrease** coverage by `10.98%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
===========================================
- Coverage 70.47% 59.49% -10.99%
===========================================
Files 583 400 -183
Lines 31019 12854 -18165
Branches 3175 3179 +4
===========================================
- Hits 21861 7647 -14214
+ Misses 9049 5029 -4020
- Partials 109 178 +69
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.49% <ø> (+0.07%)` | :arrow_up: |
| #python | `?` | |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/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/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [315 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...94a0fc7](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **decrease** coverage by `1.73%`.
> The diff coverage is `92.45%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
- Coverage 70.47% 68.73% -1.74%
==========================================
Files 583 584 +1
Lines 31019 31079 +60
Branches 3175 3180 +5
==========================================
- Hits 21861 21363 -498
- Misses 9049 9601 +552
- Partials 109 115 +6
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `53.15% <ø> (-0.78%)` | :arrow_down: |
| #javascript | `59.48% <ø> (+0.06%)` | :arrow_up: |
| #python | `67.40% <92.45%> (-2.67%)` | :arrow_down: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
| [superset/views/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYXBpLnB5) | `66.66% <50.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.39% <80.00%> (-0.89%)` | :arrow_down: |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `80.31% <100.00%> (ø)` | |
| [superset/common/query\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2NvbnRleHQucHk=) | `79.62% <100.00%> (+0.25%)` | :arrow_up: |
| [superset/connectors/base/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `86.57% <100.00%> (-2.36%)` | :arrow_down: |
| [superset/security/manager.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `91.17% <100.00%> (+2.13%)` | :arrow_up: |
| [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.11% <100.00%> (-0.07%)` | :arrow_down: |
| [superset/connectors/druid/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9kcnVpZC9tb2RlbHMucHk=) | `28.16% <0.00%> (-54.28%)` | :arrow_down: |
| [...et-frontend/src/SqlLab/reducers/getInitialState.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9nZXRJbml0aWFsU3RhdGUuanM=) | `33.33% <0.00%> (-16.67%)` | :arrow_down: |
| ... and [22 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...7c05667](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] john-bodley commented on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642735098
@etr2460 the `can_access_datasource` (renamed to `can_access_table` in https://github.com/apache/incubator-superset/pull/10031) is used by SQL Lab which leverages this the `raise_for_access` 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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
----------------------------------------------------------------
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] [incubator-superset] john-bodley removed a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley removed a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642735098
@etr2460 the `can_access_datasource` (renamed to `can_access_table` in https://github.com/apache/incubator-superset/pull/10031) is used by SQL Lab which leverages this the `raise_for_access` 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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **decrease** coverage by `3.30%`.
> The diff coverage is `90.56%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
- Coverage 70.47% 67.17% -3.31%
==========================================
Files 583 184 -399
Lines 31019 18214 -12805
Branches 3175 0 -3175
==========================================
- Hits 21861 12235 -9626
+ Misses 9049 5979 -3070
+ Partials 109 0 -109
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `67.17% <90.56%> (-2.90%)` | :arrow_down: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
| [superset/views/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYXBpLnB5) | `66.66% <50.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.18% <80.00%> (-1.10%)` | :arrow_down: |
| [superset/security/manager.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `90.84% <97.05%> (+1.80%)` | :arrow_up: |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `80.31% <100.00%> (ø)` | |
| [superset/common/query\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2NvbnRleHQucHk=) | `79.62% <100.00%> (+0.25%)` | :arrow_up: |
| [superset/connectors/base/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `86.57% <100.00%> (-2.36%)` | :arrow_down: |
| [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.11% <100.00%> (-0.07%)` | :arrow_down: |
| [superset/connectors/druid/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9kcnVpZC9tb2RlbHMucHk=) | `28.16% <0.00%> (-54.28%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-21.06%)` | :arrow_down: |
| ... and [420 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...7c05667](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9532bff48fd2a5c01f3e5e69d3bce166822951b5&el=desc) will **decrease** coverage by `0.09%`.
> The diff coverage is `90.19%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
- Coverage 64.08% 63.98% -0.10%
==========================================
Files 584 584
Lines 31054 31068 +14
Branches 3180 3180
==========================================
- Hits 19901 19880 -21
- Misses 10975 11010 +35
Partials 178 178
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `59.48% <ø> (ø)` | |
| #python | `67.17% <90.19%> (-0.17%)` | :arrow_down: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
| [superset/views/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYXBpLnB5) | `66.66% <50.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.18% <75.00%> (-0.22%)` | :arrow_down: |
| [superset/security/manager.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `90.84% <97.05%> (+1.80%)` | :arrow_up: |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `80.31% <100.00%> (ø)` | |
| [superset/common/query\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2NvbnRleHQucHk=) | `79.62% <100.00%> (+0.25%)` | :arrow_up: |
| [superset/connectors/base/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `86.57% <100.00%> (+0.14%)` | :arrow_up: |
| [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.11% <100.00%> (+0.05%)` | :arrow_up: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-21.06%)` | :arrow_down: |
| [superset/utils/cache.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2FjaGUucHk=) | `48.00% <0.00%> (-20.00%)` | :arrow_down: |
| ... and [14 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [9532bff...06efb7d](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **increase** coverage by `0.13%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
+ Coverage 70.47% 70.60% +0.13%
==========================================
Files 583 400 -183
Lines 31019 12854 -18165
Branches 3175 3179 +4
==========================================
- Hits 21861 9076 -12785
+ Misses 9049 3663 -5386
- Partials 109 115 +6
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `53.08% <ø> (-0.85%)` | :arrow_down: |
| #javascript | `59.49% <ø> (+0.07%)` | :arrow_up: |
| #python | `?` | |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/SqlLab/reducers/getInitialState.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9nZXRJbml0aWFsU3RhdGUuanM=) | `33.33% <0.00%> (-16.67%)` | :arrow_down: |
| [superset-frontend/src/reduxUtils.ts](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3JlZHV4VXRpbHMudHM=) | `70.88% <0.00%> (-8.87%)` | :arrow_down: |
| [...rontend/src/SqlLab/components/QueryAutoRefresh.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5QXV0b1JlZnJlc2guanN4) | `65.90% <0.00%> (-6.82%)` | :arrow_down: |
| [...rontend/src/SqlLab/components/TabbedSqlEditors.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RhYmJlZFNxbEVkaXRvcnMuanN4) | `75.52% <0.00%> (-6.30%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `61.13% <0.00%> (-5.68%)` | :arrow_down: |
| [...rontend/src/SqlLab/components/SqlEditorLeftBar.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIuanN4) | `44.00% <0.00%> (-4.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/reducers/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9zcWxMYWIuanM=) | `37.44% <0.00%> (-3.30%)` | :arrow_down: |
| [...end/src/SqlLab/components/TemplateParamsEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RlbXBsYXRlUGFyYW1zRWRpdG9yLmpzeA==) | `88.57% <0.00%> (-2.86%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/SqlEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvci5qc3g=) | `53.84% <0.00%> (-1.29%)` | :arrow_down: |
| [...rontend/src/SqlLab/components/AceEditorWrapper.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0FjZUVkaXRvcldyYXBwZXIudHN4) | `55.91% <0.00%> (-1.08%)` | :arrow_down: |
| ... and [179 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...4eb4e21](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] john-bodley edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-644240099
@dpgaspar and @villebro I was wondering whether you had any updated thoughts regarding this approach.
Note I do believe that long term many constructs of the existing security manager (from a data access perspective) will need to change as the constructs of database, schema, datasource access is somewhat regimented especially within the world of row level access (exists within Apache Superset) and column level access (exists at Airbnb in the form of metric level access).
----------------------------------------------------------------
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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **decrease** coverage by `10.98%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
===========================================
- Coverage 70.47% 59.49% -10.99%
===========================================
Files 583 400 -183
Lines 31019 12854 -18165
Branches 3175 3179 +4
===========================================
- Hits 21861 7647 -14214
+ Misses 9049 5029 -4020
- Partials 109 178 +69
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.49% <ø> (+0.07%)` | :arrow_up: |
| #python | `?` | |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/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/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [315 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...4eb4e21](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e17da58a39fb5726b7a81d9bd751e11ebc33b702&el=desc) will **decrease** coverage by `6.48%`.
> The diff coverage is `90.56%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
- Coverage 70.47% 63.99% -6.49%
==========================================
Files 583 584 +1
Lines 31019 31070 +51
Branches 3175 3180 +5
==========================================
- Hits 21861 19882 -1979
- Misses 9049 11010 +1961
- Partials 109 178 +69
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.48% <ø> (+0.06%)` | :arrow_up: |
| #python | `67.17% <90.56%> (-2.90%)` | :arrow_down: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
| [superset/views/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYXBpLnB5) | `66.66% <50.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.18% <80.00%> (-1.10%)` | :arrow_down: |
| [superset/security/manager.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `90.84% <97.05%> (+1.80%)` | :arrow_up: |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `80.31% <100.00%> (ø)` | |
| [superset/common/query\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2NvbnRleHQucHk=) | `79.62% <100.00%> (+0.25%)` | :arrow_up: |
| [superset/connectors/base/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `86.57% <100.00%> (-2.36%)` | :arrow_down: |
| [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.11% <100.00%> (-0.07%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10034/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/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [170 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [e17da58...7c05667](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] john-bodley commented on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-647594422
@villebro I agree that the `raise_for_access` method is fairly loaded (though in general I like the standardization it has provided in terms of checking for access from the perspective of a query, viz, etc.).
The mixin is an interesting idea, though I sense it mightn’t simply the logic in `raise_for_status` as one would need to do a number of `isinstance` checks to work out whether the `DatasourceMixin` is a query context, viz, etc to pull out the appropriate attributes. Also this doesn’t address the issue of the `Table` argument as there’s no mixin related to that.
----------------------------------------------------------------
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] [incubator-superset] john-bodley edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-647080998
@villebro I've rebased. Note in commit https://github.com/apache/incubator-superset/pull/10034/commits/634943c629f41e2372b55d0a4256a101c52f69de I also extended the logic to replace the `rejected_tables` method. I think this provides clarity/simplicity in the views but possibly adds complexity to the `raise_for_access` 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] [incubator-superset] john-bodley commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r438539510
##########
File path: superset/security/manager.py
##########
@@ -269,9 +269,12 @@ def datasource_access(self, datasource: "BaseDatasource") -> bool:
:returns: Whether the use can access the Superset datasource
"""
- return self.schema_access(datasource) or self.can_access(
- "datasource_access", datasource.perm
- )
+ try:
+ self.raise_for_access(datasource=datasource)
+ except SupersetSecurityException:
Review comment:
@etr2460 in theory (by construction) the `raise_for_access` method should only throw a `SupersetSecurityException` exception. Generally catching scoped (as opposed to general) exceptions is preferred.
----------------------------------------------------------------
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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9532bff48fd2a5c01f3e5e69d3bce166822951b5&el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `92.15%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
==========================================
- Coverage 64.08% 64.08% -0.01%
==========================================
Files 584 584
Lines 31054 31077 +23
Branches 3180 3180
==========================================
+ Hits 19901 19915 +14
- Misses 10975 10984 +9
Partials 178 178
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `59.48% <ø> (ø)` | |
| #python | `67.32% <92.15%> (-0.01%)` | :arrow_down: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
| [superset/views/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYXBpLnB5) | `66.66% <50.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.18% <75.00%> (-0.22%)` | :arrow_down: |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `80.31% <100.00%> (ø)` | |
| [superset/common/query\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2NvbnRleHQucHk=) | `79.62% <100.00%> (+0.25%)` | :arrow_up: |
| [superset/connectors/base/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `86.57% <100.00%> (+0.14%)` | :arrow_up: |
| [superset/security/manager.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `91.17% <100.00%> (+2.13%)` | :arrow_up: |
| [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.11% <100.00%> (+0.05%)` | :arrow_up: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `78.26% <0.00%> (-13.05%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.42% <0.00%> (-0.88%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [9532bff...06efb7d](https://codecov.io/gh/apache/incubator-superset/pull/10034?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] [incubator-superset] dpgaspar commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r443734193
##########
File path: superset/security/manager.py
##########
@@ -858,38 +852,65 @@ def set_perm(
)
)
- def assert_datasource_permission(self, datasource: "BaseDatasource") -> None:
+ def raise_for_access(
+ self,
+ database: Optional["Database"] = None,
+ datasource: Optional["BaseDatasource"] = None,
+ query_context: Optional["QueryContext"] = None,
+ table: Optional["Table"] = None,
+ viz: Optional["BaseViz"] = None,
+ ) -> None:
"""
- Assert the the user has permission to access the Superset datasource.
+ Raise an exception if the user cannot access the resource.
+ :param database: The Superset database (see table)
:param datasource: The Superset datasource
- :raises SupersetSecurityException: If the user does not have permission
+ :param query_context: The query context
+ :param table: The Superset table (see database)
+ :param viz: The visualization
+ :raises SupersetSecurityException: If the user cannot access the resource
"""
- if not self.datasource_access(datasource):
- raise SupersetSecurityException(
- self.get_datasource_access_error_object(datasource),
+ from superset import db
Review comment:
It's tempting to think of something like use a `**kwargs` then use a dict to route the named parameters to smaller methods that would implement the security assertion logic. A bit crazy, but has it's upsides, cleaner signature, forced breaking logic into smaller chunks, easy to extend.
Not 100% sure if it's doable,
----------------------------------------------------------------
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] [incubator-superset] john-bodley commented on a change in pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r440314741
##########
File path: superset/charts/api.py
##########
@@ -450,7 +450,7 @@ def data(self) -> Response:
except KeyError:
return self.response_400(message="Request is incorrect")
try:
- security_manager.assert_query_context_permission(query_context)
+ query_context.raise_for_access()
Review comment:
@etr2460 I think this merely highlights the inconsistencies with how backend errors are handled. This is detailed in [SIP-41](https://github.com/apache/incubator-superset/issues/9298).
----------------------------------------------------------------
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] [incubator-superset] john-bodley commented on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-647080998
@villebro I've rebased. Note in commit https://github.com/apache/incubator-superset/pull/10034/commits/3c5f3ef41977bf3720fb3e4359209cb6f13c122c I also extended the logic to replace the `rejected_tables` method. I think this provides clarity/simplicity in the views but possibly adds complexity to the `raise_for_access` 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] [incubator-superset] john-bodley commented on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642734059
@etr2460 SQL Lab uses the `rejected_tables` method which calls `can_access_datasource` which leverages this the `raise_for_access` 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] [incubator-superset] john-bodley edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642734059
@etr2460 SQL Lab uses the `rejected_tables` method which calls `can_access_datasource` (renamed to `can_access_table` in https://github.com/apache/incubator-superset/pull/10031) which leverages this the `raise_for_access` 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] [incubator-superset] codecov-commenter edited a comment on pull request #10034: chore(security): Updating assert logic
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#issuecomment-642276348
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=h1) Report
> Merging [#10034](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9532bff48fd2a5c01f3e5e69d3bce166822951b5&el=desc) will **increase** coverage by `0.00%`.
> The diff coverage is `92.15%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10034 +/- ##
=======================================
Coverage 64.08% 64.09%
=======================================
Files 584 584
Lines 31054 31068 +14
Branches 3180 3180
=======================================
+ Hits 19901 19913 +12
- Misses 10975 10977 +2
Partials 178 178
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `59.48% <ø> (ø)` | |
| #python | `67.35% <92.15%> (+0.01%)` | :arrow_up: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10034?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
| [superset/views/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYXBpLnB5) | `66.66% <50.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.18% <75.00%> (-0.22%)` | :arrow_down: |
| [superset/charts/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `80.31% <100.00%> (ø)` | |
| [superset/common/query\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2NvbnRleHQucHk=) | `79.62% <100.00%> (+0.25%)` | :arrow_up: |
| [superset/connectors/base/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `86.57% <100.00%> (+0.14%)` | :arrow_up: |
| [superset/security/manager.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `91.17% <100.00%> (+2.13%)` | :arrow_up: |
| [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.11% <100.00%> (+0.05%)` | :arrow_up: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `78.26% <0.00%> (-13.05%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.42% <0.00%> (-0.88%)` | :arrow_down: |
| ... and [6 more](https://codecov.io/gh/apache/incubator-superset/pull/10034/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10034?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/10034?src=pr&el=footer). Last update [9532bff...06efb7d](https://codecov.io/gh/apache/incubator-superset/pull/10034?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