Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Audit logging #440

Merged
merged 33 commits into from
Dec 23, 2024
Merged

Audit logging #440

merged 33 commits into from
Dec 23, 2024

Conversation

nikomakela
Copy link
Contributor

@nikomakela nikomakela commented Dec 4, 2024 •
edited
Loading

Kukkuu audit logging

Audit logging is implemented with django-auditlog, but it has some extended features applied with hel_django_auditlog_extra -app.

Audit log extension

This pull request introduces hel_django_auditlog_extra, a Django application that enhances django-auditlog with the following features:

  • Fixes set_actor context manager: The original set_actor in django-auditlog is designed to extract user and IP address information from the request and log it. However, it has issues with Graphene and its authentication mechanisms. This has been corrected.
  • New context manager for request path logging: Adds a new context manager to include request.path in the logs. This information is added to the additional_data field of LogEntry.
  • Middleware for context managers: Provides middleware that utilizes both of the aforementioned context managers.
  • Graphene decorator for access logging: Includes a Graphene decorator that logs access information every time the get_node method of a Graphene object is called.
  • Admin view mixin for audit logging: Offers an admin view mixin that enables audit logging for admin view requests.
  • Optional configuration tool: Introduces an optional configuration tool that monitors and enforces audit logging for all models, either through an inclusive or exclusive approach.

This PR is splitted so that it would contain only the new hel_django_auditlog_extra -app. The Kukkuu configuration is added in another PR: #441.

@nikomakela nikomakela mentioned this pull request Dec 4, 2024
Merged
@City-of-Helsinki City-of-Helsinki deleted a comment from sonarqubecloud bot Dec 4, 2024
@nikomakela nikomakela marked this pull request as ready for review December 4, 2024 12:35
@karisal-anders karisal-anders self-requested a review December 5, 2024 17:15
karisal-anders
karisal-anders reviewed Dec 9, 2024
View reviewed changes
Copy link
Contributor

@karisal-anders karisal-anders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall good job! I left some comments, please take a look at them.

@nikomakela
Copy link
Contributor Author

NOTE: Django-auditlog provides a generic view access log mixin out of the box: https://django-auditlog.readthedocs.io/en/latest/usage.html#automatically-logging-changes. This should be usable with DRF.

@nikomakela
feat(auditlog): installed django-auditlog
1cf8940 
KK-1113 KK-1125.
@nikomakela
chore(auditlog-extra): add apps.py
76765b1 
KK-1113
@nikomakela
feat(auditlog-extra): middleware
ce24d15 
KK-1113
@nikomakela
feat(auditlog-extra): decorator
82f72c7 
KK-1113
@nikomakela
feat(auditlog-extra): admin mixin
36ec840 
KK-1113
@nikomakela
feat(auditlog-extra): add request path to additional data in LogEntry
09bfec8 
KK-1113.

Use Python context manager to set a request path to the LogEntry
instance's additional data field when it's available, the same way as
the `set_actor` of the `django-auditlog` sets the ip address and user to
the LogEntry instance.
@nikomakela
test(auditlog-extra): init unit tests
37a38ba 
KK-1113
@terovirtanen
Copy link
Contributor

KUKKUU-API branch is deployed to platta: https://kukkuu-pr440.api.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://kukkuu-pr440.api.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

KUKKUU-API branch is deployed to platta: https://kukkuu-pr440.api.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://kukkuu-pr440.api.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

KUKKUU-API branch is deployed to platta: https://kukkuu-pr440.api.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://kukkuu-pr440.api.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

KUKKUU-API branch is deployed to platta: https://kukkuu-pr440.api.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://kukkuu-pr440.api.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

KUKKUU-API branch is deployed to platta: https://kukkuu-pr440.api.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://kukkuu-pr440.api.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

KUKKUU-API branch is deployed to platta: https://kukkuu-pr440.api.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://kukkuu-pr440.api.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

KUKKUU-API branch is deployed to platta: https://kukkuu-pr440.api.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://kukkuu-pr440.api.dev.hel.ninja 😆🎉🎉🎉

@nikomakela
Copy link
Contributor Author

  • The ContextVar stuff related to get_object would be good to test that it works

Okay, I finally had time to test this and it the test result is that the contextvar works wrong: it persists through multiple requests. I need to find a proper fix for that or manipulate the request object instead.

@terovirtanen
Copy link
Contributor

KUKKUU-API branch is deployed to platta: https://kukkuu-pr440.api.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://kukkuu-pr440.api.dev.hel.ninja 😆🎉🎉🎉

@nikomakela
fix(auditlog-extra): duplicated log creation prevention in admin mixin
7fa6aa3 
KK-1113.

Replace the contextvar usage with a request object manipulation, since
the contextvar was not reinitialized for every Django request.
@terovirtanen
Copy link
Contributor

KUKKUU-API branch is deployed to platta: https://kukkuu-pr440.api.dev.hel.ninja 🚀🚀🚀

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
96.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://kukkuu-pr440.api.dev.hel.ninja 😆🎉🎉🎉

@nikomakela nikomakela merged commit a427992 into master Dec 23, 2024
24 checks passed
@nikomakela nikomakela deleted the KK-1113-audit-log branch December 23, 2024 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karisal-anders karisal-anders karisal-anders approved these changes

@charn charn Awaiting requested review from charn

@tuomas777 tuomas777 Awaiting requested review from tuomas777

@jammi jammi Awaiting requested review from jammi

@jakezu jakezu Awaiting requested review from jakezu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nikomakela @terovirtanen @karisal-anders