django-easy-audit
django-easy-audit copied to clipboard
No user_id is recorded
I am using this with Django rest framework, using OAuth2, and it's not recording user_id
Are you only checking for users in session cookie?
I think that's a fairly accurate statement. Will accept PR to change that. We do support general usage of the login_user signal (e.g. manually telling Django, and I've seen that work for JWT login via DRF hooks). If there is a way for the library to more seamlessly hook into the general system, that would be good. Note: that is only for login. Once logged in, easy audit model signals will otherwise grab the user via it's Middleware.
Curious which version of django and python? If django 3.1, async or sync views/Middleware? Easy audit only uses sync Middleware (which should be fine and we have unit tests).
@hasahmad did you find a workaround? I am having the same issue?
@hasahmad @wmantly I have the same issue, did you figure it out?
@hasahmad, I posted this on behalf of one of my clients. The issue was due to him not authenticating some requests. Simply fixing some calls he was making anonymously fixed this issue(and a security hole he had)
@hasahmad, I posted this on behalf of one of my clients. The issue was due to him not authenticating some requests. Simply fixing some calls he was making anonymously fixed this issue(and a security hole he had)
What do you mean by that '... not authenticating some requests. Simply fixing some calls ...'? My project uses a lot of interconnected models and most (if not all) are exposed through authenticated ViewSets using the DRF's IsAuthenticated permission class.
Are you able to provide some detail about what he did specifically?
Which DRF authentication classes are you using in your project?
I just tested it with TokenAuthentication and it does not record the user in the request event. I think it is due to the fact that DRF does not plug their authentication mechanisms into Django's authentication system (it works with SessionAuthentication because it uses the standard Django session). I noticed recently that when doing a request with TokenAuthentication, the user is AnonymousUser (see my comment here). Not totally sure whether this is a bug in DRF or not.
Edit: It does work for CRUD events, though.
Turns out that there are already some solutions, unfortunately in DRF so everyone could benefit from them. One of them is DRF JWT: https://github.com/jpadilla/django-rest-framework-jwt/issues/45#issuecomment-467576074
Basically, DRF authenticates in the view layer. The workaround is to supply a custom middleware that does the authentication. However, it leads to authenticating twice and thus increasing the number of DB queries.
I looked at how the request is logged in this plugin (see https://github.com/soynatan/django-easy-audit/blob/master/easyaudit/signals/request_signals.py#L38). I wonder whether instead the request_finished signal could be reacted to (at that point the user should be there). It would require to fix how the user is retrieved though (see https://github.com/soynatan/django-easy-audit/blob/master/easyaudit/signals/request_signals.py#L62-L83).
What do you think @jheld?
For reference, the django docs on the request_finished signal.
I'm not sure that I fully understand whether the DRF JWT snippet in the link solved the problem, but regarding this library, I'm open to accepting some changes, like:
- pluggable application handler logic JIT in the request started
- similarly, for the request finished handler (request finished signal handler is not currently hooked, so that will have to be added)
- seeing if we can leverage additional authentication strategies/backends in the handler within the library itself -- can we? why aren't we? etc, and if we can & should, then making the change to do so.
@mschoettle how does the above sound, or did I miss something?
@jheld: Took me a while to get back to this.
I am playing around with the signals and EasyAuditMiddleware. What I found out is the following:
- the
request_startedsignal is triggered before the Django session middleware processes the request and setsrequest.user. I believe this is why easyaudit currently needs to manually check the session cookie. - looking at the request in the
request_finishedsignal provides the user viarequest.user. This is the case for a regular request via session authentication as well as for a request authenticated via Django REST framework (I usedTokenAuthenticationfor this)
So it seems that handling requests in request_finished would be more appropriate. At the same time it makes me wonder: Why not handle request events in the easy audit middleware directly? request signals don't receive the request requiring the workaround via thread locals. Whereas the request and response are available in the middleware.
It looks like this would solve a few of the issues reported.
What do you think?
Update: Did a quick test where I moved the request started signal handler code into the middleware.
def __call__(self, request: WSGIRequest):
self.process_request(request)
# gather request data
response = self.get_response(request)
# get user from request.user
audit_logger.request(...)
self.process_response(request, response)
return response
Based on preliminary results this works. I can create a PR for this for review @jheld.