search

soynatan / django-easy-audit

Yet another Django audit log app, hopefully the simplest one.
GNU General Public License v3.0
731 stars 183 forks source link

Add additional fields LoginEvent/RequestEvent? #84

Open xiaobosheng opened 5 years ago

xiaobosheng commented 5 years ago

LoginEvent and RequestEvent logs decent amount of useful information, but could be missing useful info from headers. I'm wondering if we can also log UserAgent in a new field?

jheld commented 5 years ago

The implementation could be done such that a LoginEvent could have a FK to RequestEvent, and include the new info on RequestEvent.

As far as if all projects want user agent etc...it could be a nullable textfield, and can be specific to the app. If we think useragent is a common enough thing we want, maybe make it more as a standard field?

jheld commented 5 years ago

@soynatan thoughts?

soynatan commented 5 years ago

LoginEvent could have a FK to RequestEvent, and include the new info on RequestEvent

How would this behave when loging through API?

jheld commented 5 years ago

Great question. So is the concern about how can we relate these objects, given that their signals are different?

Seemed more appropriate to separate the concerns, but maybe that's not feasible.

Perhaps adding this field onto LoginEvent only? If the community later wants it on both, we can add it.

This is assuming I was following your train of thought!

jheld commented 5 years ago

@soynatan so how about having it on the LoginEvent for now at least, and not include the foreign key. And the headers/agent value can be empty/null by default -- leave it as a project level setting/flag if needed.

Thoughts?

soynatan commented 5 years ago

Perhaps adding this field onto LoginEvent only? If the community later wants it on both, we can add it.

Yes, having it on the LoginEvent sounds better, at least to me! Later on, we can have it on request events too if it is necessary.

So, are you thinking of having the user agent in a different field, o throw a bunch of headers info into a field?

jheld commented 5 years ago

Great question.

I'm not sure how much info people would actually want. There isn't for my knowledge a way for the library to have a real official callback data flow (which would then let them choose what data is actually saved out to a field like this).

For simplicity, we could simply store user_agent. Which could be indexed. But for future-proofing, could be a general textfield to store json dumps of select headers. But then it cannot be a real json (queryable) field at this time.

If needed, we could simply introduce a data migration if we chose to change it to json later. LoginEvent presumably is not a massive table.

My general feedback: I don't mind either way. @soynatan do you have any leanings?

morlandi commented 5 years ago

What about a new setting to filter Request Headers to be logged ? Maybe having ['User-Agent', ] as default value, and accepting the special value 'asterisk' for all

jheld commented 5 years ago

That sounds interesting @morlandi (sane default and configurability for the projects).

I think it could be confusing if the projects change it and forget that they did or didn't have certain things before/after, but not sure how big of a deal that is.

I give 1 vote for simple (just log the UserAgent), but I'm open to what @morlandi has suggested.