feat(store): audit user creation and deletion - #1954
Conversation
Record user.created and user.deleted in the same transaction as the user write, so a user cannot be created or deleted without its audit record. Users belong to no org, so the records sit on the platform org with the user as the target and their email in the target metadata. Signup runs unauthenticated; with no caller in the context the new user is recorded as their own actor instead of the system. Create now goes through createWithTx, the path CreateWithConsent already used, so both create paths write the record from one place. This also closes the transaction the old Create left open on a duplicate email.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 9 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: raystack/frontier/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: raystack/frontier/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughUser creation and deletion now write audit records in the same transaction as their database operations. New event constants identify each audit record. Repository tests verify the event, target user, and actor. ChangesUser lifecycle audit records
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Merge Risk: ⚪ Minimal · up to User creation and deletion now write audit records in the same transaction as the user change. No merge-blocking risk was identified in the supplied changes. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change improves audit consistency by saving each user change with its audit record. No new access bypass was identified, but failure behavior and access to the retained audit data still need verification. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
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 |
Coverage Report for CI Build 36535535428Coverage increased (+0.03%) to 52.732%Details
Uncovered Changes
Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
When signup records the new user as their own actor, use their title for the actor name and fall back to their email when it is empty.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/store/postgres/user_repository_test.go (1)
234-237: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd an audit-insert failure case to
TestCreate.
createWithTxinserts the user and then callsInsertAuditRecordInTx.TestCreatecurrently covers only successful audit insertion and duplicate-user failure. It does not force the audit insert to fail or assert that the user row is rolled back. A regression that commits the user before returning an audit error can therefore pass the test suite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: raystack/frontier/.coderabbit.yaml
Review profile: CHILL
Plan: Advanced
Run ID: 51b8e0c8-8b7b-44d2-a31a-fc8ea224caa8
📒 Files selected for processing (1)
internal/store/postgres/user_repository.go
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.
This branch was successfully deployed
3 participants
Summary
Creating or deleting a user now writes an audit record (
user.created,user.deleted) in the same transaction as the user write. The user and its audit record are saved together, or neither is. Service users and orgs already work this way.Changes
UserCreatedEventandUserDeletedEventaudit events.createWithTxwrites theuser.createdrecord.Createnow goes throughcreateWithTx, the pathCreateWithConsentalready used, so both create paths write the record from one place.Deletenow runs in a transaction, so the row delete and theuser.deletedrecord are saved together.Technical Details
PlatformOrgIDas the org, the same as the platform admin/member records. The user is the target, with their email in the target metadata so a deleted user can still be identified.user.consent_grantedhandles it.Createbug: on a duplicate email, the oldCreatereturnedErrConflictwithout closing its transaction. That code is gone.WithTxnwraps errors, soCreateunwrapsErrConflictandErrInvalidDetailsand returns the plain errors, as before. Other errors now start withrollback:. Callers match witherrors.Is, so they're unaffected.users / createWithTxinstead ofusers / Create.E2E Result
I ran this against a local server built from this branch. Consent is turned on there, so the signup goes through
CreateWithConsent.audit-demo-signup@example.orgsigns up through mailotp, so the request has no caller.audit-demo-created@example.orgwithFrontierService/CreateUser.FrontierService/DeleteUser.AdminService/ListAuditRecords, filtered bytarget_id, then returns exactly oneuser.createdand oneuser.deletedrecord for each user. The records below are copied from that response. The only edit is the email domain, replaced withexample.org.user.createdactor.nameis their email, the same as theuser.consent_grantedrecord from that signup.user.createdCreateUseruser.deletedDeleteUseruser.created, self signup{ "id": "01a0ebf5-fd7d-76e0-b006-a54176073f78", "actor": { "id": "4841f2be-b16e-4aac-9f0d-a5c1a34f94f4", "type": "app/user", "name": "audit-demo-signup@example.org", "metadata": {} }, "event": "user.created", "resource": { "id": "platform", "type": "platform", "name": "platform", "metadata": {} }, "target": { "id": "4841f2be-b16e-4aac-9f0d-a5c1a34f94f4", "type": "user", "name": "auditdemosignup_example_org", "metadata": { "email": "audit-demo-signup@example.org" } }, "occurred_at": "2026-09-29T06:59:22.100910Z", "org_id": "00000000-0000-0000-0000-000000000000", "metadata": {}, "created_at": "2026-09-29T06:59:22.100910Z" }user.created, a superadmin creates the user{ "id": "01a0ebf5-5996-7847-aba7-0191533fc65a", "actor": { "id": "5b640f15-c248-4765-9759-1524034fddf0", "type": "app/user", "name": "rohancsa_example_org", "title": "Rohan", "metadata": { "context": { "Browser": "curl", "IpAddress": "", "Location": { "City": "", "Country": "", "Latitude": "", "Longitude": "" }, "OperatingSystem": "Other" }, "is_super_user": true } }, "event": "user.created", "resource": { "id": "platform", "type": "platform", "name": "platform", "metadata": {} }, "target": { "id": "5c274d2f-35a7-4db4-abaf-e19077cb5c3a", "type": "user", "name": "auditdemocreated_example_org", "metadata": { "email": "audit-demo-created@example.org" } }, "occurred_at": "2026-09-29T06:58:40.126758Z", "org_id": "00000000-0000-0000-0000-000000000000", "metadata": {}, "created_at": "2026-09-29T06:58:40.126758Z" }user.deleted, a superadmin deletes the user. The record for deleting the signup user has the same shape.{ "id": "01a0ebf6-1a94-7dba-a655-5ec48ea9d807", "actor": { "id": "5b640f15-c248-4765-9759-1524034fddf0", "type": "app/user", "name": "rohancsa_example_org", "title": "Rohan", "metadata": { "context": { "Browser": "curl", "IpAddress": "", "Location": { "City": "", "Country": "", "Latitude": "", "Longitude": "" }, "OperatingSystem": "Other" }, "is_super_user": true } }, "event": "user.deleted", "resource": { "id": "platform", "type": "platform", "name": "platform", "metadata": {} }, "target": { "id": "5c274d2f-35a7-4db4-abaf-e19077cb5c3a", "type": "user", "name": "auditdemocreated_example_org", "metadata": { "email": "audit-demo-created@example.org" } }, "occurred_at": "2026-09-29T06:59:29.558951Z", "org_id": "00000000-0000-0000-0000-000000000000", "metadata": {}, "created_at": "2026-09-29T06:59:29.555934Z" }Test Plan
TestCreateandTestDeletecheck that exactly one audit record is written, with the right actor: the new user on create, system on delete.internal/store/postgrestests pass, along with thecore/user,core/deleterandcore/authenticatetests.golangci-lintreports 0 issues on the changed packages.SQL Safety (if your PR touches
*_repository.goorgoqu.*)?placeholders,goqu.Ex{}, orgoqu.Record{}— neverfmt.Sprintfor+building a query that gets executed.ToSQL()callers capture and forward params (query, params, err := stmt.ToSQL(); db.…Context(ctx, …, query, params...)). Neverquery, _, err := ….?placeholders inside single-quoted SQL literals ingoqu.L(usemake_interval(hours => ?)-style functions instead).//nolint:forbidigoor// #nosec G20xannotation has a one-line justification on the same line that a reviewer can verify.