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

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

Open
toolness opened this issue May 18, 2021 · 5 comments
Open

Comments

@toolness
Copy link
Contributor

toolness commented May 18, 2021

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. :)

@simonw
Copy link
Owner

simonw commented May 18, 2021

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

simonw commented May 18, 2021

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

@toolness
Copy link
Contributor Author

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 JustFixNYC/tenants2#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!

@simonw
Copy link
Owner

simonw commented May 25, 2021

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

@toolness
Copy link
Contributor Author

toolness commented Aug 6, 2021

Oh, ditto for what I just mentioned in #114 (comment) -- that is, I think that moving to class-based views could help make this easier.

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