You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "betodealmeida (via GitHub)" <gi...@apache.org> on 2023/02/25 01:01:51 UTC
[GitHub] [superset] betodealmeida opened a new pull request, #23200: fix: permission checks on import
betodealmeida opened a new pull request, #23200:
URL: https://github.com/apache/superset/pull/23200
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
When importing assets (databases, datasets, charts, dashboards), ensure that the user has the proper permissions to create the assets. Otherwise a user who can only create charts is able to import a chart and have a dataset and databases added.
@eschutho, @dpgaspar said it would be nice if we could include this in 2.1.0, not sure if there's still time.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
N/A
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
Updated and added unit tests.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Antonio-RiveroMartnez commented on a diff in pull request #23200: fix: permission checks on import
Posted by "Antonio-RiveroMartnez (via GitHub)" <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #23200:
URL: https://github.com/apache/superset/pull/23200#discussion_r1118141649
##########
superset/charts/commands/importers/v1/utils.py:
##########
@@ -21,17 +21,26 @@
from flask import g
from sqlalchemy.orm import Session
+from superset import security_manager
+from superset.commands.exceptions import ImportFailedError
from superset.models.slice import Slice
def import_chart(
session: Session, config: Dict[str, Any], overwrite: bool = False
) -> Slice:
+ can_write = security_manager.can_access("can_write", "Chart")
existing = session.query(Slice).filter_by(uuid=config["uuid"]).first()
+ print("BETO")
Review Comment:
```suggestion
```
##########
superset/charts/commands/importers/v1/utils.py:
##########
@@ -21,17 +21,26 @@
from flask import g
from sqlalchemy.orm import Session
+from superset import security_manager
+from superset.commands.exceptions import ImportFailedError
from superset.models.slice import Slice
def import_chart(
session: Session, config: Dict[str, Any], overwrite: bool = False
) -> Slice:
+ can_write = security_manager.can_access("can_write", "Chart")
existing = session.query(Slice).filter_by(uuid=config["uuid"]).first()
+ print("BETO")
+ print(can_write, existing)
Review Comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida merged pull request #23200: fix: permission checks on import
Posted by "betodealmeida (via GitHub)" <gi...@apache.org>.
betodealmeida merged PR #23200:
URL: https://github.com/apache/superset/pull/23200
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #23200: fix: permission checks on import
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #23200:
URL: https://github.com/apache/superset/pull/23200#issuecomment-1444849664
# [Codecov](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#23200](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (da3018d) into [master](https://codecov.io/gh/apache/superset/commit/a70b7ac3b09876a86c1e48da3d242302a33c50b7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a70b7ac) will **decrease** coverage by `11.31%`.
> The diff coverage is `78.94%`.
> :exclamation: Current head da3018d differs from pull request most recent head 3c44a54. Consider uploading reports for the commit 3c44a54 to get more accurate results
```diff
@@ Coverage Diff @@
## master #23200 +/- ##
===========================================
- Coverage 67.50% 56.19% -11.31%
===========================================
Files 1899 1899
Lines 73322 73331 +9
Branches 7930 7930
===========================================
- Hits 49494 41212 -8282
- Misses 21796 30087 +8291
Partials 2032 2032
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `52.65% <29.62%> (-0.01%)` | :arrow_down: |
| python | `58.86% <92.59%> (-23.40%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `52.55% <92.59%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/extensions/ssh.py](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXh0ZW5zaW9ucy9zc2gucHk=) | `65.78% <0.00%> (+1.68%)` | :arrow_up: |
| [...c/views/CRUD/data/database/DatabaseModal/index.tsx](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL2luZGV4LnRzeA==) | `45.47% <45.45%> (+1.49%)` | :arrow_up: |
| [superset/datasets/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL3V0aWxzLnB5) | `76.69% <83.33%> (-1.88%)` | :arrow_down: |
| [superset/charts/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY2hhcnRzL2NvbW1hbmRzL2ltcG9ydGVycy92MS91dGlscy5weQ==) | `92.00% <100.00%> (-8.00%)` | :arrow_down: |
| [superset/dashboards/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy9pbXBvcnRlcnMvdjEvdXRpbHMucHk=) | `80.76% <100.00%> (-4.17%)` | :arrow_down: |
| [superset/databases/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS91dGlscy5weQ==) | `85.71% <100.00%> (-14.29%)` | :arrow_down: |
| [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset/tags/core.py](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFncy9jb3JlLnB5) | `4.54% <0.00%> (-95.46%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `0.00% <0.00%> (-90.91%)` | :arrow_down: |
| [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `0.00% <0.00%> (-87.88%)` | :arrow_down: |
| ... and [298 more](https://codecov.io/gh/apache/superset/pull/23200?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a diff in pull request #23200: fix: permission checks on import
Posted by "betodealmeida (via GitHub)" <gi...@apache.org>.
betodealmeida commented on code in PR #23200:
URL: https://github.com/apache/superset/pull/23200#discussion_r1119369063
##########
superset/charts/commands/importers/v1/utils.py:
##########
@@ -21,17 +21,26 @@
from flask import g
from sqlalchemy.orm import Session
+from superset import security_manager
+from superset.commands.exceptions import ImportFailedError
from superset.models.slice import Slice
def import_chart(
session: Session, config: Dict[str, Any], overwrite: bool = False
) -> Slice:
+ can_write = security_manager.can_access("can_write", "Chart")
existing = session.query(Slice).filter_by(uuid=config["uuid"]).first()
+ print("BETO")
Review Comment:
Oops!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on pull request #23200: fix: permission checks on import
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on PR #23200:
URL: https://github.com/apache/superset/pull/23200#issuecomment-1462913109
@betodealmeida I'm packaging up 2.1 today, but if there's another RC we can try to get it in, otherwise we'll aim for 2.1.1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org