Skip to content

The dashboard admin list view links to change views that the user may not have permission to use #130

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

Closed
toolness opened this issue Jun 14, 2021 · 1 comment · Fixed by #131

Comments

@toolness
Copy link
Contributor

I noticed recently that our local installation of this project links to dashboard change views from the list view page (i.e. /admin/django_sql_dashboard/dashboard/) even when users don't have access to them. Here's an example:

image

The user here doesn't have the permission to change dashboards (though they do have permission to view them) yet the slugs in the screenshot above link to admin change views (e.g. URLs like /admin/django_sql_dashboard/dashboard/5/change/). When the user clicks on these links, the view raises a PermissionDenied error, which results in a 403 that isn't very helpful.

It would be nice if either the link wasn't hyperlinked at all, or if it took the user to a read-only version of the change page or something.

@toolness
Copy link
Contributor Author

Hmm, so one way to fix this is by changing the DashboardAdmin like so:

     def has_change_permission(self, request, obj=None):
         if obj is None:
-            return True
+            return super().has_change_permission(request)
         if request.user.is_superuser:
             return True
         return obj.user_can_edit(request.user)

This effectively removes the link to the change list view in the Django admin.

However, a problem with this is that the "edit" link on the dashboard index page actually sends the user to the dashboard's change view on the admin site, which does still work, but it has a link back to the change list view in its breadcrumb that will now 403 if clicked.

Um... I guess another question I have is, should any user with execute _sql permission also have change permission for the Dashboard model? (The user mentioned in this issue's description has only execute_sql and not change permission.)

simonw added a commit that referenced this issue Jul 1, 2021
toolness added a commit to JustFixNYC/tenants2 that referenced this issue Jul 7, 2021
This upgrades us to django-sql-dashboard 1.0.1, which includes a few contributions of my own:

* [The dashboard admin list view links to change views that the user may not have permission to use](simonw/django-sql-dashboard#130)

* [Support markdown and/or HTML in dashboard descriptions](simonw/django-sql-dashboard#115)
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

Successfully merging a pull request may close this issue.

1 participant