-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
[WiP][SIP-125] Proposal for Enhanced Data Access Permissions #28002
Labels
sip
Superset Improvement Proposal
Comments
Source for the diagram above - plantuml + vscode is pretty neat! @startuml Superset Data Access Models
title SIP #125 - Superset Data Access Models
' hide the spot
' hide circle
' avoid problems with angled crows feet
skinparam linetype ortho
!theme blueprint
'left to right direction
scale 4
package "FAB Security Models" #black {
entity "User" as user {
*id: number <<generated>>
--
uuid
*name : text
}
entity "Group" as group {
*id: number <<generated>>
--
uuid
*name : text
}
entity "Role" as role {
*id: number <<generated>>
--
uuid
*name : text
}
entity "Permission" as fab_permission {
*id: number <<generated>>
--
uuid
*name : text
}
entity "ViewMenu" as fab_viewmenu{
*id: number <<generated>>
--
uuid
*name : text
}
entity "PermissionView" as fab_permission_view{
*id: number <<generated>>
--
uuid
*name : text
}
}
package "New Data Access Models" #black {
entity "Verb" as verb {
*user_id : number <<generated>>
--
uuid
*name : text
}
entity "DataAccessRole" as data_access_role {
*user_id : number <<generated>>
--
uuid
*name : text
}
entity "DataPermission" as data_permission {
*data_perm_id: number <<generated>>
--
uuid
verb: str
entity: str
object_id: int
}
}
package "Database Models" #Black {
entity "DatabaseCatalog" as catalog {
*user_id : number <<generated>>
--
uuid
*name : text
}
entity "Schema" as schema {
*user_id : number <<generated>>
--
uuid
*name : text
}
entity "RLS" as rls {
*user_id : number <<generated>>
--
uuid
*name : text
}
entity "DatabaseConnection" as database_connection {
*user_id : number <<generated>>
--
uuid
*name : text
}
entity "Dataset" as dataset {
*user_id : number <<generated>>
--
uuid
*name : text
}
}
package "Other Models" #Black {
entity "Dashboard" as dashboard {
*user_id : number <<generated>>
--
uuid
*name : text
}
}
database_connection ||..|{ catalog
database_connection ||..|{ schema
catalog ||..|{ schema
schema ||..|{ dataset
dataset ||..|{ rls
data_access_role }|..|{ data_permission
verb ||..|{ data_permission
data_permission }|..|{ database_connection
data_permission }|..|{ dashboard
data_permission }|..|{ dataset
data_permission }|..|{ schema
user }|..|{ group
user }|..|{ role
group }|..|{ role
fab_permission ||..|{ fab_permission_view
fab_viewmenu ||..|{ fab_permission_view
fab_permission_view }|..|{ role
user }|..|{ data_access_role
group }|..|{ data_access_role
@enduml |
mistercrunch
changed the title
[DRAFT - WiP][SIP] Proposal for Enhanced Data Access Permissions
[WiP][SIP-125] Proposal for Enhanced Data Access Permissions
Apr 12, 2024
Closing in favor of SIP-126 - a more federated approach to RBAC/ABAC |
Should we close this as discarded if we're merging it with the other one? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
[SIP-125] Proposal for Enhanced Data Access Permissions
Motivation
Superset's current permission model relies on Flask-AppBuilder (FAB) for both app-level and data access permissions, dictating what users can do within the app and what data they can access (e.g., databases, schemas, tables, rows).
This proposal aims to decouple data access permissions from FAB, integrating them more directly into Superset's core models (Database, Schema, SqlaTables, RLS), thereby bringing the management of these permissions more natively inside Superset. To be clear this proposal is specific to data access permissions and propose to keep app-level permissions (as in what features of the app a user can use) in FAB.
micro glossary
To better justify breaking the security model apart let's lay the differences between app-level permission and data access permissions:
Current State
Currently, the creation, update, or deletion of permissible objects necessitates corresponding changes in FAB's permission model, where both a label and an ID are captured. This model has proven brittle and difficult to manage, leading to data sync issues and posing security risks.
In larger environments, the permissions tables have become unwieldy, lacking proper indexing, which complicates both individual access requests and the generation of user-specific dashboards lists.
Proposed Change
Deep end
This proposal introduces a structured, normalized database model that links objects directly through many-to-many relationships with roles and defines the hierarchical relationships between these objects. We also propose a new "DataAccessVerb" to clearly define the permissions associated with each role:
UX
NOTE: this requires more thinking and the involvement of a product designer, but lays down the key considerations.
I'm suggesting a new Role-centric CRUD view where the Role edit screen shows:
Header:
Membership
Data Access Objects
For data access objects, we want to allow users to navigate the potentially massive hierarchy, pick objects + verbs that apply and accumulate them in some capacity. A tree-like structure with checkboxes (or dropdown with action words) seems resonable.
There's a consideration around clarifying direct access VS inherited access in a hierarchy, and we should get that right. Giving access to all 3 current schemas in a database connection is NOT the same as giving access to the database connection and all future schemas it may grow in the future.
Another design consideration is the fact that we have an almost perfect hierarchy, but may deviate from that perfection int he future if and when we integrate with Column-level-permission, splitting datasets two ways between RLS and columns.
New or Changed Public Interfaces
PermissibleModelMixin
to reuse logic across all models that are permissibleNew Dependencies
This should be all baked under our roof. In many ways we're reframing the framework (FAB) under our root here.
Migration Plan and Compatibility
We will implement a comprehensive database migration to transition existing permissions to the new
model. The new model will fully support the semantics of the current system ensuring full backwards capability.
Rejected Alternatives + out-of scope
GRANTOR
verb or similar.Considerations - request for comments
entity
field for the different objects, or one table per object? Leaning towards a single table.The text was updated successfully, but these errors were encountered: