django-ninja icon indicating copy to clipboard operation
django-ninja copied to clipboard

Multiple authentication backends with conditional csrf

Open asaff1 opened this issue 3 years ago • 6 comments

Obviously cookie based auth needs the csrf, but how can I exclude the csrf for authorization header based auth? This is my configuration:

router = NinjaAPI(auth=(AuthBearer(), django_auth), csrf=True)

So the point is, the csrf check needs to be based on the authentication backend that is in use. Now it is always enforced. (django-rest-framework does that correctly)

asaff1 avatar Nov 15 '21 08:11 asaff1

you can set global csrf=False

and then extend django_auth and add csrf check inside your own authenticator

vitalik avatar Nov 15 '21 10:11 vitalik

Good idea, I think it should be the design by default. how do I call django's csrf check logic?

asaff1 avatar Nov 16 '21 14:11 asaff1

@asaff1 you can check here - https://github.com/vitalik/django-ninja/blob/master/ninja/utils.py#L17

vitalik avatar Nov 17 '21 07:11 vitalik

you can set global csrf=False

and then extend django_auth and add csrf check inside your own authenticator

You can't really do the CSRF check in the authenticator since you do not have access to the view function. I ended up solving this with a custom middleware that exempts views from CSRF if non-session authentication is used.

I'd be highly in favour of being able to control which authentication methods require CSRF and which don't natively in Django Ninja.

from ninja.operation import PathView


class ExemptAPIKeyAuthFromCSRFMiddleware:
    """Exempts API views from CSRF if not authenticated with
    Django session authentication.

    This allows API calls with an API  key to pass without
    having to pass a CSRF token."""

    def __init__(self, get_response):
        self.get_response = get_response

    def __call__(self, request):
        return self.get_response(request)

    def process_view(self, request, view_func, view_args, view_kwargs):
        if request.user.is_authenticated: # this check might not be enough if your other auth methods set `request.user`.
            return

        klass = getattr(view_func, '__self__', None)
        if not klass:
            return

        if not isinstance(klass, PathView):
            return

        for operation in klass.operations:
            setattr(operation.view_func, 'csrf_exempt', True)

Photonios avatar Dec 17 '21 08:12 Photonios

I can plus 1 Photonios here, although I have taken a slightly different approach in fixing it myself in the mean time. Using a class which is passed as auth and contains a variable which is used for bool calls, this can then be set in each auth provider accordingly to do CSRF dependent on the authentication provider.

class Mockcls:
    def __init__(self):
        self.do_csrf = True
    def __bool__(self):
        return self.do_csrf

instance = Mockcls()

NinjaAPI(auth=instance)

Which allows us to modify the do_csrf attribute in auth classes to toggle CSRF. It does feel very hacky and I would love a way to control this natively, possibly going as far as having the csrf parameter be on authentication classes rather then the NinjaAPI instance

Skelmis avatar Jan 24 '22 03:01 Skelmis

Thank you @Photonios for this but i've spotted an issue

class ExemptAPIKeyAuthFromCSRFMiddleware:

The view_func instances seem to be reused between multiple requests, so by setting csrf_exempt you're disabling CSRF checks for all future requests.

You could toggle it back to False on a per request basis, but I think that might run into race conditions when you have django serving requests in a concurrent fashion (async/gevent).

The CSRF middleware checks request._dont_enforce_csrf_check and will exit early if set https://github.com/django/django/blob/fac3dd7f390d372736e05974cc5c3ef1a3768fbb/django/middleware/csrf.py#L433-L438

We can use this to 'trick' it into thinking CSRF has already been checked just for specific requests. The modified version looks like this

class ExemptAPIKeyAuthFromCSRFMiddleware:
    def __init__(self, get_response):
        self.get_response = get_response

    def __call__(self, request):
        return self.get_response(request)

    def process_view(self, request, view_func, view_args, view_kwargs):
        if request.user.is_authenticated:
            return
        klass = getattr(view_func, "__self__", None)
        if not klass:
            return
        if isinstance(klass, PathView):
            request._dont_enforce_csrf_checks = True

Definitely need a mechanism to avoid having to do these hacks, API's that deal with browser based auth and some other auth where CSRF is not required seem to be fairly common.

shughes-uk avatar Feb 19 '22 20:02 shughes-uk

Is it better approach to set csrf_exempt=True if the view klass includes ninja.security.django_auth in auth callbacks or not ?

Like this:

from ninja.security import django_auth

class ExemptCSRFOtherThanDjangoAuthMiddleware:
    def __init__(self, get_response):
        self.get_response = get_response

    def __call__(self, request):
        return self.get_response(request)

    def process_view(self, request, view_func, view_args, view_kwargs):
        klass = getattr(view_func, "__self__", None)
        if not klass:
            return

        for operation in klass.operations:
            if django_auth not in operation.auth_callbacks:
                setattr(operation.view_func, 'csrf_exempt', True)

skokado avatar May 27 '23 04:05 skokado