fix(identity): include auth_workos_user_id in audit log details for non-primary bindings#3728
Draft
bokelley wants to merge 1 commit into
Draft
fix(identity): include auth_workos_user_id in audit log details for non-primary bindings#3728bokelley wants to merge 1 commit into
bokelley wants to merge 1 commit into
Conversation
…on-primary bindings Audit log inserts recorded req.user.id (canonical identity) as the actor, losing the credential identity for non-primary bindings. Adds auth_workos_user_id to AuditLogEntry, centralizes merging into recordAuditLog, and updates all req.user-driven audit sites across organizations.ts, accounts-billing.ts, accounts.ts, brand-enrichment.ts, admin/users.ts, billing.ts, and http.ts. Also adds auth_user_name enrichment to the admin audit log display endpoint. Refs #3717 https://claude.ai/code/session_014nNk64VpVs3hLFS7xSPKdQ
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3717
Summary
PR #3715 introduced id-swap in the auth middleware:
req.user.idis always the canonical workos_user_id, andreq.user.authWorkosUserIdholds the actual credential when a non-primary binding is signed in. Audit log inserts were recording only the canonical id, losing the forensic trail of which bound email performed each action. Phase 2b (#3722) ships today, creating the first non-singleton bindings — this fix needed to land immediately.What this PR does:
auth_workos_user_id?: stringtoAuditLogEntryinterface;recordAuditLogmerges it into thedetailsJSONB automatically (no schema migration — details is alreadyJSONB)orgDb.recordAuditLog()call sites that usereq.user.idas the actor acrossorganizations.ts(23 sites),http.ts(3),accounts-billing.ts(1),accounts.ts(1),billing.ts(2), andbrand-enrichment.ts(1 inline SQL)accounts-billing.tsto spreadauth_workos_user_idconditionally into theirJSON.stringifydetailsadmin/users.ts: theadminUserIdpassed tomergeUsersnow usesauthWorkosUserId ?? idsomerge_useraudit entries record the credential identityadmin/cleanup.ts: same credential-id fix formergeOrganizationsactorauth_user_namedisplay enrichment to the admin audit log API endpoint (resolves the credential user via the existing WorkOS cache, guarded againstworkosbeing null in dev mode)Non-breaking justification: All changes are additive to the
detailsJSONB column.workos_user_id(= canonical) is unchanged for all routes exceptmerge_user/merge_organization, where the actor column now records the credential id directly (consistent with how those two functions have always worked — the actor is passed by the caller, not extracted from the canonical id). No existing query is broken; the new field is absent from records written before this PR.Note on merge audit entries (
merge_user/merge_organization): For these two operations,workos_user_idstores the credential id directly (the caller passes it). This differs from thedetails.auth_workos_user_idpattern used everywhere else. Theauth_user_namedisplay enrichment will not fire for these entries (they don't storeauth_workos_user_idin details). A follow-up could unify the pattern by adding anauthMergedBy?: stringparameter to both merge functions, but that is out of scope here.Note on
companyDb.recordAuditLogsites (http.ts:7962, 8032): These use a different audit table schema (user_idcolumn,metadataJSONB) and are currently dead code. Not updated here; thecompanyDbaudit surface should be handled separately when those endpoints are activated.Pre-PR review:
formatWorkOSDisplayNamehelper, add test case for id-swap scenario). Nits noted; not blocking.auth_user_nameenrichment ✓); inline SQL sites acceptable due to transaction coupling.Session: https://claude.ai/code/session_014nNk64VpVs3hLFS7xSPKdQ
Generated by Claude Code