[SIP-73] Proposal for Improving the security stucture and move to resource based model

[bolkedebruin]

[SIP] Proposal for Improving the security stucture and move to resource based model

Motivation

I have been trying to implement centralized security for Superset. This means externalizing any security checks to a different service that resides outside Superset. For example, if a user needs access to a view or a data source this check would be done by and external service like Apache Ranger or Open Policy Agent. This is major thing when deploying Superset in an enterprise context where one would like to manage access rights on a platform level rather than a component level. This has proven difficult as the security implementation is not well structured at the moment. This means that segregation of duties is hard or impossible to implement.

The challenge is that Superset as a convoluted security model. To illustrate here are several examples.

• The "Admin" role is a special role that gets special permissions verified by the application layer rather than the security layer. For example DashboardAccessFilter asks the security layer if the is_user_admin rather than asking the security layer if the user has view access to a particular resource.
• get_datasources_accessible_by_user is unable to distinguish whether a user has use, view, edit, write access to the different datasources. Resulting in an all or nothing approach.
• ChartFilter first checks if the user has can_access_all_datasources (resource: datasource) and if so allows access, but if the user hasn't it checks if the users has access to user_view_menu_names on datasource (with ambiguous datasource_access) or to user_view_menu_names on schema (again with ambiguous schema_access). Note these are different kinds of resources (datasource, schema).

Proposed Change

My proposal is to

1. Restructure the security models so that every check will be done by the security layer. is_user_admin and alikes should never be called from the application layer.
2. Access to resources should be checked with hasPermission(user, action, resource) of course including metadata like Roles, originating IP, proxy addresses etc. Resource has ownership as metadata.
3. Actions can be (this list is probably not complete) `view (like in view dashboard), read (like in read query), execute (like in execute query), write (like in change dashboard, edit query, delete query), embed (like in include this chart in this HTML page).

New or Changed Public Interfaces

Security Manager plugins would need to be updated to follow the new resource based permission model.

New dependencies

• None

Migration Plan and Compatibility

Open to suggestions if required.

Rejected Alternatives

For discussion.

[suddjian]

I like this a lot. Especially "Access to resources should be checked with hasPermission(user, action, resource)". Are you still interested in pushing this forward @bolkedebruin?

This is likely to be considered a breaking change. I'm not sure what the timeline is for 2.0, but it might be a good candidate to be included.

[suddjian]

related: #16557

[stale]

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.

[rusackas]

Closing this issue due to inactivity, and we'll also consider the SIP/proposal process closed as well. If you want to rekindle this proposal, please re-open this Issue, and send a new [DISCUSS] thread to the dev@ mailing list. Thank you!