vvangelovski / django-audit-log

Audit log for your Django models
Other
232 stars 93 forks source link

Saves User as None when when type(self.request.user) is not SimpleLazyObject #28

Closed klahnen closed 8 years ago

klahnen commented 8 years ago

I am using: djangorestframework==3.2.4 djangorestframework-jwt==1.7.2 Django==1.8.2 django-audit-log==0.7.0

Basically when I make post request for saving an object using session Authentication it sets created_by user fine.

But when I use JWT, user is not encapsulated inside SimpleLazyObject and then Audit-Log saves None.

If you can give me a clue or a workaround will be great.

Output: self.assertEqual(post.created_by.username, "user01") AttributeError: 'NoneType' object has no attribute 'username'

My Test: class PostsWithJWTTestCase(APITestCase):

def test_user_can_write_posts_to_a_course(self):
    UserFactory.create(username='pdamicol', password='paolo')

    course = CourseFactory.create(subject=None,
                                  image=None,
                                  professor=None,
                                  student_group=None)

    response = self.client.post('/api-token-auth/',
                                {
                                    "username": "user01",
                                    "password": "pass"
                                },
                                format='json')

    token = response.data['token']

    self.client.credentials(HTTP_AUTHORIZATION='JWT '+token)

    self.assertEqual(response.status_code, status.HTTP_200_OK)

    self.client.post('/courses/'+str(course.id)+'/posts/',
                     {'text': "This is post 1"},
                     format='json')

    post = Post.objects.first()

    self.assertEqual(post.text, "This is post 1")

    self.assertEqual(post.created_by.username, "user01")

    self.client.logout()
klahnen commented 8 years ago

Hi,

This is my workaround, can you give me tips for making a pull request?

Modified Code:

from django.db.models import signals
from django.utils.functional import curry

from audit_log import registration
from audit_log.models import fields
from audit_log.models.managers import AuditLogManager

"""Custom code start"""
from rest_framework.request import Request
from django.utils.functional import SimpleLazyObject
from django.contrib.auth.middleware import get_user
from rest_framework_jwt.authentication import JSONWebTokenAuthentication
"""Custom code end"""

def get_user_jwt(request):
    user = get_user(request)
    if user.is_authenticated():
        return user
    try:
        user_jwt = JSONWebTokenAuthentication().authenticate(Request(request))
        if user_jwt is not None:
            return user_jwt[0]
    except:
        pass
    return user

def _disable_audit_log_managers(instance):
    for attr in dir(instance):
        try:
            if isinstance(getattr(instance, attr), AuditLogManager):
                getattr(instance, attr).disable_tracking()
        except AttributeError:
            pass

def _enable_audit_log_managers(instance):
    for attr in dir(instance):
        try:
            if isinstance(getattr(instance, attr), AuditLogManager):
                getattr(instance, attr).enable_tracking()
        except AttributeError:
            pass

class UserLoggingMiddleware(object):

    def process_request(self, request):
        if not request.method in ('GET', 'HEAD', 'OPTIONS', 'TRACE'):
            if hasattr(request, 'user') and request.user.is_authenticated():
                user = request.user

            else:
                # user = None
                # Custom Code start
                user = get_user_jwt(request)
                # Custom Code end

            session = request.session.session_key

            update_pre_save_info = curry(self._update_pre_save_info, user, session)
            update_post_save_info = curry(self._update_post_save_info, user, session)
            signals.pre_save.connect(update_pre_save_info,  dispatch_uid = (self.__class__, request,), weak = False)
            signals.post_save.connect(update_post_save_info,  dispatch_uid = (self.__class__, request,), weak = False)

    def process_response(self, request, response):
        signals.pre_save.disconnect(dispatch_uid =  (self.__class__, request,))
        signals.post_save.disconnect(dispatch_uid =  (self.__class__, request,))
        return response

    def _update_pre_save_info(self, user, session, sender, instance, **kwargs):
        registry = registration.FieldRegistry(fields.LastUserField)
        if sender in registry:
            for field in registry.get_fields(sender):
                setattr(instance, field.name, user)

        registry = registration.FieldRegistry(fields.LastSessionKeyField)
        if sender in registry:
            for field in registry.get_fields(sender):
                setattr(instance, field.name, session)

    def _update_post_save_info(self, user, session, sender, instance, created, **kwargs ):
        if created:
            registry = registration.FieldRegistry(fields.CreatingUserField)
            if sender in registry:
                for field in registry.get_fields(sender):
                    setattr(instance, field.name, user)
                    _disable_audit_log_managers(instance)
                    instance.save()
                    _enable_audit_log_managers(instance)

            registry = registration.FieldRegistry(fields.CreatingSessionKeyField)
            if sender in registry:
                for field in registry.get_fields(sender):
                    setattr(instance, field.name, session)
                    _disable_audit_log_managers(instance)
                    instance.save()
                    _enable_audit_log_managers(instance)
klahnen commented 8 years ago

I fork your project here: https://github.com/klahnen/django-audit-log/tree/master/audit_log can you help me to check if this is enough for merging to master?

Best regards.

vvangelovski commented 8 years ago

I think the best solution to this problem will be to provide a middleware to solve this case. And document that users of JWT should use this middleware. It's too much of a special case to include this checks in the core code.

vvangelovski commented 8 years ago

There's a fix for this in 47590b8 But, this is actually not a problem with django-audit-log it's a problem with djangorestframework-jwt and the fact that it doesn't provide a mechanism like this. I will issue a pull request with them to include such middleware. In the meantime or if they decide not to include it, the solution is to use the JWTAuthMiddleware in django-audit-log.