-
Notifications
You must be signed in to change notification settings - Fork 38
Audit log #1042
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.
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
Conversation
Signed-off-by: Nikolai Rozanov <nickolay.rozanov@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very elegant work, thanks!
Giving you first feedback, not on all remaining questions, but on issues I found by first review / iteration.
A thought about the table-level access rule implementation:
This approach should be described in auth.policy where we document mostly our auth approach (I give some text snippets I believe would work in the following).
We should make it semi-official that "we don't have a general solution for table-level auth (as we haven't needed a general implementation so far), but there is a nice custom approach to it."
"A class method on the model can be added which returns an AuthModelMixin
. That would have an __acl__()
function with your rules, which the auth policy will then go on and use. The permission_required_for_context
decorator can make sure this AuthModelMixin
object is used by the policy via ctx_loader
. It can even pass in the context if that is helpful for your logic.
See the AuditLog
model class for an example, where we required authorization logic which governs if a subset of a table (e.g. all audit logs that relate to an account) are availabe to the current user."
Signed-off-by: Nikolai Rozanov <nickolay.rozanov@gmail.com>
Signed-off-by: Nikolai Rozanov <nickolay.rozanov@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it a lot. Getting close I believe!
Signed-off-by: Nikolai Rozanov <nickolay.rozanov@gmail.com>
Signed-off-by: Nikolai Rozanov <nickolay.rozanov@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good to me, aside from three small things:
- one thing I find weird about naturalized datetime. Not a deal breaker
- the missing changelog entry, please add
- can you link the active username to a mailto link to their email?
Signed-off-by: Nikolai Rozanov <nickolay.rozanov@gmail.com>
Signed-off-by: Nikolai Rozanov <nickolay.rozanov@gmail.com>
Description
Added AuditLog data model
Started adding new records to it on user creation / deletion / activating etc.
New "Audit Log" buttons on account and user page that lead to audit log page
Look & Feel
New endpoints:
How to test
Run
flexmeasures db upgrade
Create new user / delete existing one / activate or deactivate some user
Check there are records on audit log page for this user
Further Improvements
Fix bug with user not being able to view himself unless he is an admin.
Related Items