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

Support projects that don't use the Django group permissions system #127

Open j4mie opened 3 years ago

j4mie commented 3 years ago

I just tried to install django-sql-dashboard on an internal project and immediately got an error when going to /dashboard/:

ValueError: Cannot query "<loggedinuser@example.com>": Must be "Group" instance.

After a bit of digging, I discovered that the culprit was this line in Dashboard.get_visible_to_user:

cls.objects.filter(
    models.Q(owned_by=user)
    | models.Q(view_policy__in=allowed_policies)
    | models.Q(view_policy=cls.ViewPolicies.GROUP, view_group__user=user)
)

The last bit there assumes that you have a User model which is using the PermissionsMixin (or otherwise has a groups relationship).

In my project, the User model is very simple:

class User(AbstractBaseUser):
    id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
    # other fields removed
    is_superuser = models.BooleanField(default=False)
    email = LowercaseEmailField(unique=True)

    USERNAME_FIELD = "email"

    @property
    def is_staff(self):
        return self.is_superuser

    def has_module_perms(self, app_label):
        return True

    def has_perm(self, perm, obj=None):
        return True

In other words - users can access /admin/ if their is_superuser flag is set, but otherwise are just logged in and can access everything. I'm not using the Django permissions system at all, really.

I'm not sure how to fix this in the general case - could django-sql-dashboard somehow detect whether the groups relationship exists? Or should it be a setting? Or an entirely custom auth system? Or should I just stop being difficult and install the Django permissions system like everyone else? 🙂

simonw commented 3 years ago

Are you using the Django Admin for that project? I hadn't considered the case where someone might have a user account that diverges in this way.

The "edit" mechanism is currently entirely dependent on the Django Admin, though that could change in the future.

I think the way to address this would be to switch over to more of a class-based-view approach to allow people to subclass the dashboard views and implement things like their own custom permissions.

I'm not opposed to doing that, but it's not going to be a priority for a while I imagine.

So yeah, the easiest way to get SQL Dashboard running here would be to spin up a separate app with the Django default user/permissions stuff configured and then point that at the read-only database connection.

simonw commented 3 years ago

See also #113 - feature request for custom auth.

j4mie commented 3 years ago

We do use the Django Admin, but we only expose it to internal users (ie devs). We have a single boolean flag is_superuser which distinguishes users that can log into the Admin from those that can't. We assume that any user that can access the Admin automatically has permission to do anything with any model (hence the has_module_perms and has_perm both return True in the model above).

I imagine this setup isn't all that uncommon - after all the PermissionsMixin is an optional mixin for custom User models, so it's likely that plenty of Django users do something like this.

I suppose a CBV approach would be one way of solving this. Or maybe a function-based view factory which allows you to pass in a custom function as an argument that the view uses to get a list of visible dashboards for the current user? django-sql-explorer solves this sort of thing by putting callback functions in settings, but I always thought that approach was a bit odd.