django-sql-dashboard icon indicating copy to clipboard operation
django-sql-dashboard copied to clipboard

Consider supporting a way to decorate the dashboard views with custom auth

Open toolness opened this issue 4 years ago • 5 comments
trafficstars

Hello! I am interested in using django-sql-dashboard to provide access to potentially sensitive information in my Django project. Because the information is particularly sensitive, I'm interested in putting the dashboard's endpoints behind additional authentication restrictions than solely requiring that the user have the execute_sql permission (see #112)--for example, I have a custom two-factor authentication solution in my project, and I'd like to ensure that the user has recently validated their session through it.

I'm basically wondering if there is some way I can easily "decorate" the dashboard's URL config/views with such custom authentication. Since the app only appears to contain two views at the moment, I think I can just hack together a solution by simply creating my own alternative URL patterns, but it would be a bit tedious to keep this up as this project changes. So I was curious if you might accept a PR to make it possible to e.g. dynamically generate a URL config that automatically wraps all the app's views in a passed-in decorator, or something equally configurable.

I might also just be overthinking this, so feel free to tell me as much and close this PR. :)

toolness avatar May 18 '21 23:05 toolness

Supporting this kind of use-case seems reasonable to me. Could you do this right now by wrapping the dashboard view function in your own view? Something like this (pseudo code):

from django_sql_dashboard.views import dashboard_index

def custom_dashboard(request):
    if not user_has_recently_validated_session(request):
        return HttpResponse("You need to validate tour session")
    return dashboard_index(request)

Then hook up /dashboard/ in your urls.py to that custom view function.

simonw avatar May 18 '21 23:05 simonw

Happy to discuss ways of refactoring the code to make this cleaner.

simonw avatar May 18 '21 23:05 simonw

Thanks @simonw! For now I've implemented this through the following kind of function:

def get_django_admin_dashboard_urls(site):
    import django_sql_dashboard.urls
    from django.urls.resolvers import URLPattern

    urlpatterns = [
        URLPattern(
            pattern=pattern.pattern,
            callback=require_enabled_dashboard(site.admin_view(pattern.callback)),
            default_args=pattern.default_args,
            name=pattern.name,
        )
        for pattern in django_sql_dashboard.urls.urlpatterns
    ]

    return (urlpatterns, "", "")

This effectively wraps all of django-sql-dashboard's views in my own auth decorators--the site object passed-in to the function is a django.contrib.admin.AdminSite instance, and require_enabled_dashboard is just a custom decorator that raises a 404 if no dashboard database URL is configured. I've added some tests to verify that everything works as expected. You can see my full solution over at https://github.com/JustFixNYC/tenants2/pull/2103 if you want.

I was thinking that it might be useful to expose a more generic version of a function like this in django-sql-dashboard itself, to make it easier for folks who have a similar use case as me, but perhaps I'm an unusual edge case, or perhaps folks in my situation who find this issue can just copy-paste the code snippet as needed. If you do think it'd be a useful addition to django-sql-dashboard, I'm happy to submit a PR, but otherwise you're also welcome to close out this issue too.

Thanks again for your help, and for making this awesome tool!

toolness avatar May 19 '21 13:05 toolness

I'd be happy to see a PR for making this kind of pattern cleaner.

simonw avatar May 25 '21 00:05 simonw

Oh, ditto for what I just mentioned in https://github.com/simonw/django-sql-dashboard/issues/114#issuecomment-894473451 -- that is, I think that moving to class-based views could help make this easier.

toolness avatar Aug 06 '21 19:08 toolness