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 2023/01/09 12:04:40 UTC
[GitHub] [superset] giovannipapini-agilelab opened a new pull request, #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
giovannipapini-agilelab opened a new pull request, #22642:
URL: https://github.com/apache/superset/pull/22642
### SUMMARY
Minor bug fix on dashboard RBAC faulty behaviour
### TESTING INSTRUCTIONS
See issue #22640
### ADDITIONAL INFORMATION
- [x] Fixes #22640
- [x] Has associated issue:
- [x] Required feature flags: `'DASHBOARD_RBAC' = True`
- [ ] 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] giovannipapini-agilelab commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "giovannipapini-agilelab (via GitHub)" <gi...@apache.org>.
giovannipapini-agilelab commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1453849225
We could also fix that text box writing something like
`If no roles are defined, then the dashboard is visibile to users on the base of general datasource permissions.`
that is the most precise behaviour definition.
--
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] mattitoo commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "mattitoo (via GitHub)" <gi...@apache.org>.
mattitoo commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1438686606
It seems to work for us
--
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 closed pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho closed pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
URL: https://github.com/apache/superset/pull/22642
--
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] CeelamkotiVinod commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "CeelamkotiVinod (via GitHub)" <gi...@apache.org>.
CeelamkotiVinod commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1438440465
I tried applying your changes, but still issue persists.
--
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] giovannipapini-agilelab commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "giovannipapini-agilelab (via GitHub)" <gi...@apache.org>.
giovannipapini-agilelab commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1416279788
Hi @eschutho, yes, it is a quite basic fix but fundamental for `DASHBOARD_RBAC` feature flag working. We have already applied this patch on our prod env, where gives the desired result.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] ivan-price-acted commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "ivan-price-acted (via GitHub)" <gi...@apache.org>.
ivan-price-acted commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1511545012
Hi @villebro indeed i had missed that and indeed that appears to address our concerns !
Looking forward to release 2.1.1 , thanks for the heads up !
--
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] villebro commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1511023637
@ivan-price-acted you may have missed this PR, which I believe addresses at least part of the permission issue: #23586
--
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] sfirke commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1460465784
Good summary @giovannipapini-agilelab. FYI I was testing out a separate feature on version 2.1.0rc1 and found that the DASHBOARD_RBAC behavior is slightly different from 2.01. If a dashboard is **draft** and has no RBAC role assigned, it still _tries_ to load - its name appears in the browser - but it errors before it can load. I suppose this is the same behavior since it's trying to load, but heads up in case that complicates how this is discussed.
--
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 #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1416272621
@giovannipapini-agilelab is this PR ready for review?
--
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] mdeshmu commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "mdeshmu (via GitHub)" <gi...@apache.org>.
mdeshmu commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1453914783
@sfirke confirmed here: https://apache-superset.slack.com/archives/C016B3LG5B4/p1677865116611839?thread_ts=1677781629.874359&cid=C016B3LG5B4
`Right now if I don't add a role to a dashboard, it gives access to all roles, i.e., even the public.`
It is really a security issue then.
--
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 #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1532251607
I'm going to close this pr for https://github.com/apache/superset/pull/23586 with hopes that the latter covers this use case.
--
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] frabenetti commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by GitBox <gi...@apache.org>.
frabenetti commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1381639753
Any news about the approval of this PR?
--
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] giovannipapini-agilelab commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "giovannipapini-agilelab (via GitHub)" <gi...@apache.org>.
giovannipapini-agilelab commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1453802370
> Will it gives access to every logged in user or will it fall back to dataset permissions ?
It will fall back to user's dataset permissions. It seems to me the right behaviour, coherent with the text box under dashboard roles `If no roles are defined, then the dashboard is available to all roles.`
--
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] mdeshmu commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "mdeshmu (via GitHub)" <gi...@apache.org>.
mdeshmu commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1453827729
My understanding of this text:
`If no roles are defined, then the dashboard is available to all roles.`
is
dashboard will be available to any logged in user which is then a security issue.
--
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] giovannipapini-agilelab commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "giovannipapini-agilelab (via GitHub)" <gi...@apache.org>.
giovannipapini-agilelab commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1460452540
> @giovannipapini-agilelab With your changes in this PR, Can you please test that if dashboard is published and no role is assigned to dashboard, is it accessible by a user with Public or Gamma Role ?
Hi, we verified this case and in indeed the behaviour is this:
- If the dashboard is **draft** it is not listed in dashboard menu and only owners and admins can open it.
- If the dashboard **is published** has some rbac role assigned, then the it is listed in dashboard menu (d.m.) and visible **to and only to** those roles (with the exception of dashboard owners and admins). Other roles do not see it listed in d.m. and when they open it via permalink they get a "You cannot access this dashboard!" error page.
Imo this is the correct behaviour (and note that it is not the same as before this PR).
- If the dashboard **is published** and has no rbac role assigned, then it is visible in d.m. only to those who have dataset-level permissions, **but** it is accessible and visible *via permalink* also to everyone else (Gamma or Public roles).
This could be seen as a security issue, even more if combined with the `PUBLIC_ROLE_LIKE = "Gamma"` setting.
Probably we should fix the fallback behaviour, in this or in some other PR, what do you think @mdeshmu?
--
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] frabenetti commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by GitBox <gi...@apache.org>.
frabenetti commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1377376354
We have the same issue and I see this PR will solve the problem, I tested the modification in a docker in versione 2.0.1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1381752786
# [Codecov](https://codecov.io/gh/apache/superset/pull/22642?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 [#22642](https://codecov.io/gh/apache/superset/pull/22642?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7a4beaf) into [master](https://codecov.io/gh/apache/superset/commit/30dab3a00a2e7f8d3dbfbb742cd72e44719d1c9c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (30dab3a) will **decrease** coverage by `9.86%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #22642 +/- ##
==========================================
- Coverage 65.66% 55.79% -9.87%
==========================================
Files 1859 1859
Lines 71093 71096 +3
Branches 7777 7777
==========================================
- Hits 46680 39667 -7013
- Misses 22388 29404 +7016
Partials 2025 2025
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `52.46% <ø> (+0.13%)` | :arrow_up: |
| python | `57.89% <ø> (-20.60%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `51.43% <ø> (?)` | |
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/22642?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/security/manager.py](https://codecov.io/gh/apache/superset/pull/22642?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `62.24% <ø> (-33.51%)` | :arrow_down: |
| [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/22642?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/22642?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/22642?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/22642?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: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/22642?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/key\_value/commands/delete\_expired.py](https://codecov.io/gh/apache/superset/pull/22642?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZV9leHBpcmVkLnB5) | `0.00% <0.00%> (-84.00%)` | :arrow_down: |
| [superset/dashboards/commands/importers/v0.py](https://codecov.io/gh/apache/superset/pull/22642?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy9pbXBvcnRlcnMvdjAucHk=) | `15.62% <0.00%> (-76.25%)` | :arrow_down: |
| [superset/datasets/commands/create.py](https://codecov.io/gh/apache/superset/pull/22642?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvY3JlYXRlLnB5) | `30.61% <0.00%> (-69.39%)` | :arrow_down: |
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/22642?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `25.00% <0.00%> (-69.05%)` | :arrow_down: |
| ... and [325 more](https://codecov.io/gh/apache/superset/pull/22642?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] ivan-price-acted commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "ivan-price-acted (via GitHub)" <gi...@apache.org>.
ivan-price-acted commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1510833345
Hi there,
Just discovered this 'feature' whilst testing for our organisation: the fact that (currently) Draft dashboards provide a window for unauthorised users onto data they wouldn't normally be allowed to see, (if no RBAC roles are assigned) if this PR merges than those same unauthorised users will see the data when the dashboards are published.
I'm surprised we don't simply delete the line of code in question, i.e. be explicit in the RBAC rules that if we want a world-readable dashboard we assign a role that we know all users have ?
Why do we assume no roles == everyone cas access the data ? It is very likely users creating dashboards will forget to publish or unpublish their dashboards and not assign any RBAC rules, and will not understand the implications in terms of who can access their data between the two states.
As an administrator this presents a risk, trusting the users to do the right thing here (manage the draft / published AND the RBAC rules) is not realistic for us.
Can we make a plea for removing line 1994
```
or (dashboard.published and not dashboard.roles)
```
completely, and require the user to add an explicit role instead ?
-ivan
--
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] giovannipapini-agilelab commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "giovannipapini-agilelab (via GitHub)" <gi...@apache.org>.
giovannipapini-agilelab commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1460467905
> Are you testing out applying your patch to 2.1.0rc1?
No, atm we are using the patches applied to v2.0.1 as of this PR. For sure I can try 2.1.0rc1 with this patches and see which is the behaviour.
--
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] mdeshmu commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "mdeshmu (via GitHub)" <gi...@apache.org>.
mdeshmu commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1454010422
@giovannipapini-agilelab With your changes in this PR, Can you please test that if dashboard is published and no role is assigned to dashboard, is it accessible by a user with Public or Gamma Role ?
--
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] giovannipapini-agilelab commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "giovannipapini-agilelab (via GitHub)" <gi...@apache.org>.
giovannipapini-agilelab commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1453866376
@mdeshmu may I add those also those fixes in this pr?
--
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] mdeshmu commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "mdeshmu (via GitHub)" <gi...@apache.org>.
mdeshmu commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1453792467
@giovannipapini-agilelab How will this behave if DASHBOARD_RBAC is enabled with no roles assigned in a published dashboard's properties? Will it gives access to every logged in user or will it fall back to dataset permissions ?
--
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] mdeshmu commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "mdeshmu (via GitHub)" <gi...@apache.org>.
mdeshmu commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1453861591
IMO, correction of that text is also needed. Something like this as shown in this documentation https://superset.apache.org/docs/creating-charts-dashboards/creating-your-first-dashboard/#manage-access-to-dashboards
`If no roles are defined, then the access will fallback to Dataset permissions.`
@amitmiran137 I feel there is a fullstop (.) missing in the documentation in here ->
--
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] mdeshmu commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "mdeshmu (via GitHub)" <gi...@apache.org>.
mdeshmu commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1453880378
@giovannipapini-agilelab I would love to see clarity in the texts.
--
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] sfirke commented on pull request #22642: fix(dashboard rbac): make a dashboard with no role assigned accessible only when PUBLISHED instead of DRAFT
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #22642:
URL: https://github.com/apache/superset/pull/22642#issuecomment-1460462142
Good summary @giovannipapini-agilelab. Are you testing out applying your patch to 2.1.0rc1? I was testing out another feature on that version and found that the DASHBOARD_RBAC behavior had changed from 2.01. Now if a dashboard is _draft_ and has no RBAC role assigned, it tries to load - its name appears in the browser - but it errors before it can load. So I would make sure you're trying your changes on top of the latest.
> This could be seen as a security issue, even more if combined with the PUBLIC_ROLE_LIKE = "Gamma" setting.
Agreed.
--
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