Add audit logs - #647
Conversation
Introduce a pluggable audit module with NullSink default and emit() API. Wire SQLAlchemy listeners for user and project lifecycle events, explicit emits for auth and sync endpoints (login, password, access grants/revocations, version push, soft/hard delete, restore). Add actor_context()/request_context() helpers, device_id capture, scope_id for workspace-scoped filtering, and target_type auto-derivation. This is groundwork for adding more events and to be extended in EE with custom sinks, query API, and retention policy. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Coverage Report for CI Build 30524900752Coverage increased (+0.3%) to 92.496%Details
Uncovered Changes
Coverage Regressions2 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
Implements structured audit event emission for all relevant user and project actions. Update AuditEvent dataclass with new target columns: project_id, workspace_id and user_id. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…events Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Also simplify test_get_projects_by_uuids to avoid the fake-workspace-id hack. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… events Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| .with_entities(User.id, User.email) | ||
| .first() | ||
| ) | ||
| return (found.email, found.id) if found else (None, None) |
There was a problem hiding this comment.
how about return ... else (login_str, None) to know at least something?
| ) | ||
| return "", 200 | ||
| else: | ||
| abort(403, "You do not have permissions") |
There was a problem hiding this comment.
we don't want to log this?
we do log login fail..
|
|
||
| if old_active and not user.active: | ||
| emit( | ||
| AuthEventType.USER_DEACTIVATED, | ||
| **actor_context(), | ||
| user_id=user.id, | ||
| target_email=user.email, | ||
| ) | ||
| elif not old_active and user.active: | ||
| emit( | ||
| AuthEventType.USER_RESTORED, | ||
| **actor_context(), | ||
| user_id=user.id, | ||
| target_email=user.email, | ||
| ) | ||
|
|
There was a problem hiding this comment.
we don't log superadmin assignment?
| db.session.add(user) | ||
| db.session.commit() | ||
|
|
||
| if old_active and not user.active: |
There was a problem hiding this comment.
should we re-querry the user?
| emit( | ||
| AuthEventType.USER_MARKED_FOR_DELETION, | ||
| **actor_context(), | ||
| user_id=user.id, | ||
| target_email=user.email, | ||
| ) |
There was a problem hiding this comment.
shall we emit after the action?
| **actor_context(), | ||
| project_id=project.id, | ||
| workspace_id=project.workspace_id, | ||
| target_email=requester.email if requester else None, |
There was a problem hiding this comment.
we should get the actor email when they are authenticated, no?
(applies also for the following emits)
| project_id=project.id, | ||
| workspace_id=project.workspace_id, | ||
| target_email=requester.email if requester else None, | ||
| role=permission, |
There was a problem hiding this comment.
the project name could be useful here
| project.delete() | ||
| db.session.info.pop("audit_skip_project_update", None) |
There was a problem hiding this comment.
it would be safer to put his in try: and finally: to avoid stale info message, but that's rather a theoretical risk
| project_id=project.id, | ||
| workspace_id=project.workspace_id, | ||
| target_email=user.email if user else None, | ||
| role=removed_role.value, |
| project_id=p.id, | ||
| workspace_id=p.workspace_id, | ||
| project_name=f"{p.workspace.name}/{p.name}", | ||
| ) |
There was a problem hiding this comment.
could we add info it was done by this celery task?
| *.crt | ||
|
|
||
| # Local Claude Code skills | ||
| .claude/commands/ |
There was a problem hiding this comment.
Share it with us :) If usefull.
|
|
||
| import json | ||
| import shutil | ||
| from typing import List |
There was a problem hiding this comment.
Do we need this typing If python is bumped?
| "disk_usage", | ||
| "latest_version", | ||
| "updated", | ||
| "storage_params", |
There was a problem hiding this comment.
I think when storage params are updated, this is like critical bug.
| # storage_params, locked_until), or covered by dedicated events with their own emit | ||
| # (removed_at/removed_by are suppressed via audit_skip_project_update instead). | ||
| # frozenset prevents accidental mutation of module-level state. | ||
| _SKIP = frozenset( |
There was a problem hiding this comment.
Do we have alternatives in other events to this columns?
rename this variable to something meaningful.
| "updated", | ||
| "storage_params", | ||
| "tags", | ||
| "locked_until", |
There was a problem hiding this comment.
Yes, we are not locking projects using flask, but in future it should be critical to have it in audits
| ) | ||
| if not project.removed_at: | ||
| abort(400, "Failed to remove: Project is still active") | ||
| emit( |
There was a problem hiding this comment.
Why is this not handled in project.delete() method after commit? I Think We have multiple cases like this - project deleted, user deleted ...
|
|
||
| user.reset_lockout() | ||
| db.session.commit() | ||
| emit( |
There was a problem hiding this comment.
Not better to add to db event? Or we do not know how to check columns in db events?
| return ctx | ||
|
|
||
|
|
||
| def emit_safe(event_type, **kwargs): |
There was a problem hiding this comment.
Why not use emit_safe in controllers? We want to break endpoints if something is wrong with audit logs write?
Introduce a pluggable audit module with NullSink default and emit() API.
Wire SQLAlchemy listeners for user and project lifecycle events, explicit emits for auth and sync endpoints (login, password, access grants/revocations, version push, soft/hard delete, restore).
Add actor_context()/request_context() helpers, device_id capture, scope_id for workspace-scoped filtering, and target_type auto-derivation.