Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SIP] Proposal for Restricting access to Dashboards #17913

Closed
rajraousb opened this issue Jan 3, 2022 · 10 comments
Closed

[SIP] Proposal for Restricting access to Dashboards #17913

rajraousb opened this issue Jan 3, 2022 · 10 comments
Labels
sip Superset Improvement Proposal

Comments

@rajraousb
Copy link

rajraousb commented Jan 3, 2022

Motivation

Dashboards might have PII or sensitive data. Currently anyone who has access to Superset can look at any Dashboard by using dashboard_id directly. They might not get access to the charts or underlying data, but can still look at the Dashboard, its structure, underlying errors etc.
Currently, anyone can access any Dashboard
• Users can bookmark dashboards and access them later
• They can share Dashboard ID and/or URL directly to anyone
• Users can directly type in the dashboard id in the form superset/dashboard/
• Even if they do not know of any particular dashboard ID, they can iterate through the dashboard to find the one they are looking for.
#10408 is at role level. However it is not practical to change roles for each user for each dashboard

Once user gets the ID and hence the dashboard:
• They will be able to look at the Dashboard, layout, any headers or text on the dashboard.
• However, permission to Underlying dataset is needed to render the dashboard data and charts.
• The concern is that anyone can get to know the layout and underlying table/bucket/hive/chart details, even if they do not actually see the data/chart.

Proposed Change

Enforce that only Owners, Co-Owners or Viewers (and Admin) can view any Dashboard

  1. Admin users will be allowed to view any dashboard as per existing functionality
    a. Admin users permission will be provided only based on request
    b. Viewing Data/Content/Chart on the dashboard is as per permission for Admin (i.e. they can see the layout only unless they have permission for underlying data which is in the bucket)
    c. Irrespective of Published/not published
  2. Anonymous users ("nobody"superset user - we have turned off this feature in our deployment but just in case) will not be able to see any dashboard
  3. Logged in user should be in owner or viewer list to see dashboard, irrespective of how they land on the dashboard page.
    a. If they click on the dashboard from the Dashboards panel (possibly a stale page)
    b. From url link in a email or message or favorite or bookmark etc.
    c. Directly typing in the dashboard id (a number) – e.g. /superset/dashboard/
  4. If a user has neither Owner nor Viewer privilege as defined in the Dashboard, they will get an error message
    a. TBD. Should they get a JSON error message or some UI
  5. 3 & 4 is a breaking change, meaning, we are changing the default behavior. If some user or customer relied on this, they will now start seeing error message until the owner of the Dashboard adds them to Viewers (or Owners). We do not want to have this under a feature switch, due to Security concern.
  6. 3 & 4 will provide additional protection to Dashboard PII data (TBD: Ananth)
    New or Changed Public Interfaces
    See Image

New dependencies

No new dependency.

Migration Plan and Compatibility

No migration step involved

Rejected Alternatives

#10408 is at role level. However it is not practical to change roles for each user for each dashboard*

@rajraousb
Copy link
Author

rajraousb commented Jan 3, 2022

Attaching design proposal
SIP Restricting Access.pdf

@betodealmeida
Copy link
Member

@rajraousb do you mind formatting the post with Markdown headers, to make it easier to read? Also, instead of attaching a .docx file please include the text as Markdown directly in the description, since people might not have tools to open a proprietary Word document.

Note that Superset supports dashboard-specific access roles (#10408) via the DASHBOARD_RBAC feature flag, which seem to cover at least some of the use cases you list here.

@rajraousb
Copy link
Author

rajraousb commented Jan 3, 2022

Code change:

    @has_access
    @expose("/dashboard/<dashboard_id>/")
    def dashboard(self, dashboard_id):
        """Server side rendering for a dashboard"""

        def check_owner_or_viewer(obj):
        #See if current user has either owner or viewer permission

            if not obj:
                return False

            if g.user.is_anonymous:
                return False

            roles = [r.name for r in get_user_roles()]
            if "Admin" in roles:
                return True

            owners = []
            owners += obj.owners

            owners += obj.viewers

            owner_names = [o.username for o in owners if o]


            if g.user and hasattr(g.user, "username") and g.user.username in owner_names:
                return True



            return False


        session = db.session()
        qry = session.query(models.Dashboard)
        if dashboard_id.isdigit():
            qry = qry.filter_by(id=int(dashboard_id))
        else:
            qry = qry.filter_by(slug=dashboard_id)

        dash = qry.one_or_none()
        if not dash:
            abort(404)


        if check_owner_or_viewer( dash ) == False:
            bootstrap_data = {
                "user_id": g.user.get_id(),

                "user_name": g.user.username,
                "user.first_name": g.user.first_name,
                "user.last_name": g.user.last_name,

                "dashboard_id": dash.id,
                "dashboard_title": dash.dashboard_title,
                "error": "Need either Owner or Viewer privilege to view this dashboard",
            }


            flash(__("You have no permission to view this dashboard"), "danger")

            return json_success(json.dumps(bootstrap_data))




        datasources = set()
        for slc in dash.slices:
            datasource = slc.datasource
            if datasource:
                datasources.add(datasource)

@rajraousb
Copy link
Author

This is an extension to #17914

@amitmiran137
Copy link
Member

I must admit that just plain RBAC on its own wasn't sufficient for us so we went ahead and extended superset in a way that for every saved dashboard there would be a matching role like 'viewer_<dashboard_id>' and then on an external UI we allow owner to provide access directly to users and behind the scenes we just assign those user with the matching role according to the dashboard_id

@amitmiran137
Copy link
Member

bc superset is based on FAB and it doesn't have a proper API for controlling users and roles we need to create them as an extension to superset and use those new APi's for the requirements above

@stale
Copy link

stale bot commented Apr 17, 2022

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.

@stale stale bot added the inactive Inactive for >= 30 days label Apr 17, 2022
@rajraousb
Copy link
Author

bc superset is based on FAB and it doesn't have a proper API for controlling users and roles we need to create them as an extension to superset and use those new APi's for the requirements above

any plan for doing this for 2.x version?

@stale stale bot removed the inactive Inactive for >= 30 days label Apr 19, 2022
@rajraousb
Copy link
Author

This has been implemented in 1.4.1 version (which I checked) onwards. Thank you for all comments.

Now, any direct access to dashboard shows error:

You don't have access to this dashboard.

@rusackas
Copy link
Member

Closing this since it's been implemented! Thanks everyone!

@rusackas rusackas added the sip Superset Improvement Proposal label Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sip Superset Improvement Proposal
Projects
Status: Denied / Closed / Discarded
Development

No branches or pull requests

4 participants