jazzband / django-auditlog

A Django app that keeps a log of changes made to an object.
MIT License
1.15k stars 414 forks source link

DjangoRestFramework TokenAuthentication + django-auditlog = AnonymousUser/None as Actor #137

Closed Jaredn closed 7 years ago

Jaredn commented 7 years ago

[QUESTION] I think this must be expected behavior, but I wanted to be sure. Django-auditlog works fine in my GUI, properly logging any model change that I register using Django's built-in SessionAuthentication. However, I have an API set up with DRF using their TokenAuthentication. When using the API, the django-auditlog entries have the Actor set to 'None' in the logs, and troubleshooting inside of django-auditlog's middleware shows user set to AnonymousUser and is_authenticated() set to False.

From my basic troubleshooting, it seems that django-auditlog's middleware runs BEFORE DRF's TokenAuthentication gets ran (which sets request.user).

I solved this, by making a simple middleware that I run just before django-auditlog's middleware. This is the code I used:

from re import sub
from rest_framework.authtoken.models import Token

class SetUserIfTokenAuth(object):
    """ django-auditlog works via middleware.  Unfortunately, middleware triggers before DjangoRestFramework's
    TokenAuthentication.  This middleware checks for a Token and sets the User prior to Auditlog running.
    """

    def process_request(self, request):
        header_token = request.META.get('HTTP_AUTHORIZATION', None)
        if header_token is not None:
            try:
                token = sub('Token ', '', request.META.get('HTTP_AUTHORIZATION', None))
                token_obj = Token.objects.get(key=token)
                request.user = token_obj.user
            except Token.DoesNotExist:
                pass
        return None

My settings.py looks like this:

MIDDLEWARE_CLASSES = [
    'django.middleware.security.SecurityMiddleware',
    'django.contrib.sessions.middleware.SessionMiddleware',
    'django.middleware.common.CommonMiddleware',
    'django.middleware.csrf.CsrfViewMiddleware',
    'django.contrib.auth.middleware.AuthenticationMiddleware',
    'django.contrib.auth.middleware.SessionAuthenticationMiddleware',
    'django.contrib.messages.middleware.MessageMiddleware',
    'django.middleware.clickjacking.XFrameOptionsMiddleware',
    '<projectname>.middleware.auth.SetUserIfTokenAuth',
    'auditlog.middleware.AuditlogMiddleware',
]

REST_FRAMEWORK = {
    'DEFAULT_FILTER_BACKENDS': (
        'rest_framework.filters.DjangoFilterBackend',
        'rest_framework_filters.backends.DjangoFilterBackend',
    ),
    'DEFAULT_PERMISSION_CLASSES': (
        'rest_framework.permissions.IsAuthenticatedOrReadOnly',
    ),
    'DEFAULT_AUTHENTICATION_CLASSES': (
        'rest_framework.authentication.TokenAuthentication',
        'rest_framework.authentication.BasicAuthentication',
        'rest_framework.authentication.SessionAuthentication',
    ),
    'TEST_REQUEST_DEFAULT_FORMAT': 'json',
}

So, did I solve this the right way? Is this expected? Did I do something hilariously wrong? Thanks in advance guys!

Oh It's worth noting that I'm running Django 1.9.8, django-auditlog 0.4.3 (i tried 0.3.3 as well), and DjangoRestFramework 3.4.0.

audiolion commented 7 years ago

Yeah so all the middleware is processed before it gets to the APIView where rest framework does its authentication. Adding middleware before the auditlog middleware to check the token seems to be a reasonable way to solve it.

The biggest piece is making sure you don't screw up the token processing down the line by DRF and you are authenticating them properly. The other issue that would be nice to avoid is duplicate queries (where you authorize in the middleware then DRF authorizes them again).

I would maybe directly call rest frameworks token auth methods to get all the error handling and security validations, and perhaps use a cache for the token

Jaredn commented 7 years ago

Good call. I've changed my middleware to use DRF's built-in authentication method:

from rest_framework.authentication import TokenAuthentication
from rest_framework.exceptions import AuthenticationFailed

class SetUserIfTokenAuth(object):
    """ django-auditlog works via middleware.  Unfortunately, middleware triggers before DjangoRestFramework's
    TokenAuthentication.  This middleware checks for a Token and sets the User prior to Auditlog running.
    """

    def process_request(self, request):
        tauth = TokenAuthentication()
        try:
            user, token = tauth.authenticate(request)
            request.user = user
        except (TypeError, AuthenticationFailed):
            pass
        return None
audiolion commented 7 years ago

That looks good to me! Sorry that there isn't a cleaner solution, I wonder how other libraries deal with token auth and DRF not providing the request.user at the middleware layer.

Gonna close this for now, but feel free to continue discussion

Jaredn commented 7 years ago

Thanks for your help!

For anyone else that stumbles upon this, here is the code I used to test that Auditlog is working through the API and setting actor correctly (There might be a few unused imports, sorry!):

import json
from django.core.urlresolvers import reverse
from django.contrib.auth import get_user_model
from rest_framework import status
from rest_framework.test import APITestCase, APIRequestFactory, APIClient
from rest_framework.authtoken.models import Token
from auditlog.models import LogEntry

from model_mommy import mommy

User = get_user_model()

class GenericSetupMixin(object):
    def setUp(self):
        # Superuser Credentials
        self.superuser_name = 'the_django_test_user'
        self.superuser = User.objects.create_superuser(self.superuser_name, 'john@snow.com', 'testpass')
        self.superuser.save()
        self.token = Token.objects.get_or_create(user=self.superuser)
        self.token = Token.objects.get(user=self.superuser)
        self.client = APIClient()
        self.client.credentials(HTTP_AUTHORIZATION='Token ' + self.token.key)

class DjangoAuditlog(GenericSetupMixin, APITestCase):
    def test_create_generates_full_auditlog_entry(self):
        url = '/some/url/here/'
        name = 'imarealboy'
        data = {
            'name': name,
        }
        response = self.client.post(url, data=data, format='json')
        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
        self.assertTrue(name in response.content.decode('utf-8'))

        # now check auditlog history
        logentry = LogEntry.objects.latest('timestamp')
        self.assertTrue(name in logentry.changes_str)
        self.assertTrue(self.superuser_name == logentry.actor.username)
ao1000 commented 7 years ago

@Jaredn Doesn't that duplicate the process of setting the auth token ? the custom middleware will run, and then drf will reset the token ?