search

simonw / django-sql-dashboard

Django app for building dashboards using raw SQL queries
https://django-sql-dashboard.datasette.io/
Apache License 2.0
437 stars 37 forks source link

Users without "execute_sql" can potentially view dashboards, but can't see a list of them #142

Open toolness opened 3 years ago

toolness commented 3 years ago

So I was a bit surprised when @simonw mentioned the following in https://github.com/simonw/django-sql-dashboard/issues/133#issuecomment-871895389:

This is deliberate, because the "export all" operation is potentially quite expensive - I don't want to accidentally open that up to anyone on a public dashboard, since that could potentially be used for a denial of service attack.

It hadn't actually occurred to me that dashboards could be completely public... I think this was because, as I was reading the source code, I saw that the dashboard_index page, which listed all the dashboards, required both login and execute_sql permission. My assumption was then that the whole dashboard (everything "under" it in the URL structure) would require similar permissions. But this is not the case, since e.g. /dashboard/my-funky-dashboard could have public visibility!

So it seems the dashboard index page actually serves two purposes:

  1. Showing users a list of dashboards they can view.
  2. Allowing users to dynamically execute SQL queries.

It seems like (1) doesn't really have any security restrictions, since some dashboards can even be public, while (2) clearly has lots of restrictions.

Maybe the index page should be modified so that it supports (1) better? This could mean, for instance, that it always shows a list of viewable dashboards (or tells the user they can't view any, if that's the case), and only shows the SQL interface and list of tables if the user has execute_sql permission?