Add revision history for user account and permission changes - #396
Add revision history for user account and permission changes#396level09 wants to merge 5 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Added a baseline backfill to the migration (b6ecf45) after a review pass flagged the gap. Without it, a revision has nothing to diff against until the account is edited twice. Bulletins get edited constantly so their history closes that gap immediately, but user accounts are edited rarely, which would have left the first edit of every existing account undiffable for as long as that account went untouched. The migration now inserts one snapshot per existing user, with |
|
Changed the deletion behavior (16e3826) after review: revisions now survive account deletion instead of cascading with it. This is an evidence platform, so erasing the record of what an account was allowed to do is the wrong default even when the account itself goes away. Verified through the ORM delete path against the migrated schema: the account goes, the revision stays with Also fixed in 5c08259: Follow-up worth its own ticket: soft-delete users rather than hard-delete them at all. One hazard for whoever picks it up: do not reuse the global filter in |
| revision_id = UserHistory.query.filter_by(target_user_id=user_id).one().id | ||
|
|
||
| # deletion still works, the revision does not block it | ||
| assert user.delete() is True |
Closes BYNT-1550.
Records a snapshot of a user's account and permission state on every create and update, so an admin can answer "when did this user get this permission, and who granted it". Salvaged from the closed #217 and rebuilt on current main.
The existing
activitytable only stores{"id": N, "class": "user"}as the subject, so it records that an edit happened but not what changed. It is also skipped entirely when the action is not inACTIVITIES_LIST, so it cannot serve as the permission audit trail.Changes
UserHistorymodel following the existing*Historypattern, withtarget_user_id(subject) anduser_id(who made the change).User.to_history_dict()as the snapshot serializer. Deliberately notto_dict(): that one masks names through thesecure_*properties based on the viewing user, and carriesforce_reset(the live password reset key). A stored revision must be neither viewer-dependent nor hold secrets.User.create_revision(), called fromapi_user_createandapi_user_update.GET /admin/api/userhistory/<id>, Admin only. Not gated by theview_historypermissions used for content items, since these revisions carry account and permission state.On deletion
Users are hard deleted and every user now carries at least one revision, so a plain FK would have made
api_user_deletefail permanently.target_user_idis thereforeON DELETE CASCADEanduser_idisON DELETE SET NULL, so deleting an admin does not destroy the trail of what they did to other accounts. Covered bytest_delete_cascades_revisions.Verification
flask db upgrade), resulting schema matches the model,flask doctorreports "Migrations up to date" and "Schema aligned with models".Not included
{items: [...]}shape as the other history endpoints, so it can hang off the existing history drawer.activity, where they are currently not logged at all. Worth its own small ticket.pyproject.tomlline is a ruff per-file-ignore for the model package's re-export__init__.py, needed because adding one import made the hook lint the whole pre-existing file.