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

Some permissions (on main page) seem unnecessary because they are associated with essential functions #27765

Open
3 tasks
xavier-GitHub76 opened this issue Mar 28, 2024 · 3 comments

Comments

@xavier-GitHub76
Copy link
Contributor

Bug description

Hello,

I find permission management tricky because there is little documentation on the subject.
with Superset v3.1.1 (docker), I started from a role with no permission to try to understand the impacts of permissions.

I notice that certain essential functions rely on permissions. Here is the detail :

With no permission, I navigate on pages

HOME PAGE
the simple access on welcome page causes errors
image
image

The following permissions are therefore obligatory :

  • "can read on Dashboard" to list dashbords on home page
  • "can read on Chart" to list charts on home page
  • "can recent activity on Log"

The Create buttons are presents although the user has no permission. Click on it --> Error
image

PARAMETERS/PROFIL
OK, the page is displayed but why a permission "can profile on Superset" exists ? It's seems unnecessary

PARAMETERS/INFO
Error "Acces is denied"
image

The following permission is therefore obligatory :

  • "userinfoedit on UserDBModelView"

Proposed solution
if permission is imposed on everyone then it should not exist :

  • "can read on Dashboard"
  • "can read on Chart"
  • "can recent activity on Log"
  • "userinfoedit on UserDBModelView"
    could be deleted

Create buttons should be displayed only if permission (value to specify) are present
Same for Parameters/Info, it's should be displayed only if permission "userinfoedit on UserDBModelView" is present

if permission is unnecessary then it should not exist :

  • "can profile on Superset" could be deleted

Best regards

How to reproduce the bug

Create a role without permission
Create a user with this role
With this user, go on :

  • welcome page
  • Error dashbaords et charts listing

  • Error recent logs

  • Error on Create buttons (dashboards and charts)

  • profile page
  • Displayed without permission

  • info page
  • Error Access denied

Screenshots/recordings

No response

Superset version

3.1.1

Python version

3.9

Node version

16

Browser

Chrome

Additional context

Docker

Checklist

  • I have searched Superset docs and Slack and didn't find a solution to my problem.
  • I have searched the GitHub issue tracker and didn't find a similar bug report.
  • I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.
@xavier-GitHub76 xavier-GitHub76 changed the title Certain permissions are essential Some permissions seem unnecessary because they are associated with essential functions Mar 28, 2024
@goldjee
Copy link
Contributor

goldjee commented Apr 1, 2024

Yep, the whole permission system feels very clunky and too raw to manage. If a permission is necessary to be granted for a user to actually use the system, it should not be listed.

Also, it's worth mentioning that there are 176 permissions assigned to vanilla Admin role with no proper documentation. If it's the whole set of permissions, I believe they should be grouped by functionality so superusers could easily acknowledge what is assigned to any given role. It could be visualized as a table, like so:

Functionality Access Modify Delete
Dashboard [x] [ ] [ ]

I believe such a permission view could be a great relief to anyone who administers Superset or conducts security audit on the installation.

@xavier-GitHub76 xavier-GitHub76 changed the title Some permissions seem unnecessary because they are associated with essential functions Some permissions (on main page) seem unnecessary because they are associated with essential functions Jun 12, 2024
@xavier-GitHub76
Copy link
Contributor Author

I 'm OK with you.
Today, list of permissions is very difficult to use when the number of permissions is high.
For example, a dropdown list with 170 items is not ergonomic.

I think it could be more simple to use if following rules :

  1. If an action is enable for everyone, a permission should not exist (like 'can profile on Superset')
  2. If an action is not enable for everyone, a permission should exist (like 'can write on Dashbaord') and the buttons linked to theses permissions should be only displayed if permssion is present on role. This would avoid the presence of unnecessary buttons. They will inevitably cause a warning or an error
  3. Permissions should be grouped by category to have smaller and easier to understand lists. The categories could be the main elements managed on Superset :
  • Database
  • Dataset
  • Chart
  • Dashboard
  • SqlLab
  • Other

Each category could be displayed with tabs :
image

Your table will be very more pratical to use :
image

The administrator could thus define a role with simple boxes to check.
This would avoid having to interpret an permission based on a technical name.
Personally, I don't understand permissions like "can get value on KV"

@rusackas
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants