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

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

Open
j4mie opened this issue Jun 10, 2021 · 3 comments
Open

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

j4mie opened this issue Jun 10, 2021 · 3 comments

Comments

@j4mie
Copy link

j4mie commented Jun 10, 2021

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

ValueError: Cannot query "<[email protected]>": 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
Copy link
Owner

simonw commented Jun 10, 2021

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
Copy link
Owner

simonw commented Jun 10, 2021

See also #113 - feature request for custom auth.

@j4mie
Copy link
Author

j4mie commented Jun 11, 2021

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.

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

2 participants