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/04/17 23:41:54 UTC

[GitHub] [incubator-superset] mistercrunch opened a new issue #9576: [DISCUSS] chart and dashboard ownership

mistercrunch opened a new issue #9576: [DISCUSS] chart and dashboard ownership
URL: https://github.com/apache/incubator-superset/issues/9576
 
 
   So currently, the security model assumes that each chart and each dashboard has one or many owners.
   
   Owners are the only people who can alter their objects, or expand the ownership of objects.
   
   Now dashboards are collections of charts, and when an owner wants to extend ownership to another person, we also grant ownership of the collection of charts in the dashboard. Every time a dashboard is saved, we make sure that all charts are owned by [at least] all the owners of the dashboard. 
   
   The idea is that it's confusing and unintuitive to own a dashboard, be able to change the layout, but not be able to change the charts. 
   
   ### Related shortcomings / thoughts
   * when saving a chart and adding it to the dashboard in the "save" modal, the ownership is not extended to that dashboard's owner. Note that typically the person will go and alter the layout and save the dashboard, which solves that issue. If we change that to fit the model, people might not understand the ownership implications of "adding to dashboard"
   * when people alter a chart, they might not know that this chart is in any or multiple dashboards, and can alter things beyond of their intended scope
   
   <img width="848" alt="Screen Shot 2020-04-17 at 4 40 43 PM" src="https://user-images.githubusercontent.com/487433/79621965-38a04c80-80ca-11ea-8d7e-a7534363f083.png">
   
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] suddjian edited a comment on issue #9576: [DISCUSS] chart and dashboard ownership

Posted by GitBox <gi...@apache.org>.

suddjian edited a comment on issue #9576: [DISCUSS] chart and dashboard ownership
URL: https://github.com/apache/incubator-superset/issues/9576#issuecomment-615550879

A few thoughts/questions:

- If you become an owner of a chart after adding it to a dashboard, what exactly is the purpose of ownership? Anyone who has rights to create a dashboard has the ensuing ability to edit any chart they can view, no?
- Should ownership be part of the permissions model, or more of a label on an object indicating who is responsible for it? Ownership should probably stand firmly on one side of that spectrum, and not in the middle. If we need both "permissions" ownership and "responsibility" ownership, maybe those should be two separate things.
- What if there was a flow to easily replace someone else's chart in your dashboard with a clone that you own? Or to assign a dashboard as an "owner" of a chart? I think it's possible to find a solution that fits all user needs.
- Superset needs a user testing/feedback infrastructure to verify these kinds of ideas.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] mistercrunch commented on issue #9576: [DISCUSS] chart and dashboard ownership

Posted by GitBox <gi...@apache.org>.

mistercrunch commented on issue #9576: [DISCUSS] chart and dashboard ownership
URL: https://github.com/apache/incubator-superset/issues/9576#issuecomment-616224064

Some answers:
- Only owners of the dashboard can add charts to them. The behavior I wanted to describe: say if both you and I are the owner of a dashboard, and I create a brand new chart and add it to this dashboard, you'd become the owner of it. Currently this would only happen once I go in the dashboard and save it (it cascades ownership to charts). It's very likely I would go and position that new chart in the dash and save it, so the gap here is pretty small. Also you saving that dashboard for whatever reasons would cascade ownership too.
- Currently ownership is part of the models ifself (many-to-many) while other non-resource-specific perms like can-read, can-modify, can-delete are part of RBAC. Business logic applies all required checks on actions. Seems ok to me. Ownership and ownership checks are [mostly] consistent across object types (charts, query, dashboard) and the same model / logic applies.
- sounds tricky / complicated, users own chart seem better than dashboard owns chart
- more tests around RBAC / ownerships would be great, I think we do make sure that non-owners cannot update / delete things, but not sure how well that's covered in tests

I think the current model that is "if you own the dashboard, we make you also own all of the charts in it" is good. We need to make it clear that this is the case (at least when adding owners to a dashboard).

I have yet to hear a user say "I really want to make this user an owner of my dashboard, but only want to allow that person to modify a subset of the charts in my dashboard". Personally I don't think we need to support that for now, and that it leads to confusing situations / more complex UI to enable that.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

[GitHub] [incubator-superset] suddjian commented on issue #9576: [DISCUSS] chart and dashboard ownership

Posted by GitBox <gi...@apache.org>.

suddjian commented on issue #9576: [DISCUSS] chart and dashboard ownership
URL: https://github.com/apache/incubator-superset/issues/9576#issuecomment-615550879

A few thoughts:

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

[GitHub] [incubator-superset] etr2460 commented on issue #9576: [DISCUSS] chart and dashboard ownership

Posted by GitBox <gi...@apache.org>.

etr2460 commented on issue #9576:
URL: https://github.com/apache/incubator-superset/issues/9576#issuecomment-616627788


   Possibly a side note here, but something that I think complicates this is the many to many relationship between charts and dashboards. If when adding to a dashboard a chart got cloned so that every chart is only owned by either 0 or 1 dashboards, ownership data would no longer need to be stored on the chart once it becomes associated with a dashboard.
   
   In general, it might be worthwhile to remove the concept of ownership from a chart totally (as @suddjian mentioned). If the chart is on a dashboard, it inherits from the dashboard's owners. If the chart is not on a dashboard, it is only editable by the creator.


----------------------------------------------------------------
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] stale[bot] closed issue #9576: [DISCUSS] chart and dashboard ownership

Posted by GitBox <gi...@apache.org>.

stale[bot] closed issue #9576:
URL: https://github.com/apache/incubator-superset/issues/9576


   


----------------------------------------------------------------
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] benceorlai commented on issue #9576: [DISCUSS] chart and dashboard ownership

Posted by GitBox <gi...@apache.org>.

benceorlai commented on issue #9576:
URL: https://github.com/apache/incubator-superset/issues/9576#issuecomment-622535156


   I tend to think of this in the [bigger] context of content lifecycle management. I can think of a case where either scenario is desired, i.e. chart ownership is / is not altered by the action of the chart being added to a dashboard. I can think of scenarios where retaining the full ownership of a chart is desired. For example for a "certified" chart, that is used as the company-wide source of truth, and the risks of inadvertent edits by additional owners is high. So I think the default behavior should be that charts do not automatically become owned by the dashboard owner and there should be a separate, intentional act of taking/conferring ownership of charts in a specific dashboard - which may need to be a specific permission.


----------------------------------------------------------------
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] stale[bot] commented on issue #9576: [DISCUSS] chart and dashboard ownership

Posted by GitBox <gi...@apache.org>.

stale[bot] commented on issue #9576:
URL: https://github.com/apache/incubator-superset/issues/9576#issuecomment-652836721


   This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue `.pinned` to prevent stale bot from closing the 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.

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