[PM-34222] Stamp collected events with server time

[AlexRubik]

🎟️ Tracking

PM-34222

📔 Objective

Organization event logs mixed two clocks. Server-originated events (create item, invite user) used the server's UTC clock, while client-originated events (copy password, view item, autofill, export vault) carried a timestamp generated on the user's device and were stored verbatim. On a device whose clock was a few minutes slow, the log showed events out of causal order: the reported case shows an item's password copied roughly three and a half minutes before the item itself was created.

CollectController was the only place a client-controlled timestamp entered the event pipeline. Every other IEventService caller already lets the service stamp the event, so this drops the client-supplied date at those five call sites and EventService's existing DateTime.UtcNow fallback applies per event, putting the whole log on one clock.

This is also an audit-log integrity fix

Worth calling out separately, because it is the stronger reason to ship this. Before this change there was no validation of any kind on the incoming date, so any authenticated user could choose the timestamp on their own audit entries: Cipher_ClientCopiedPassword, User_ClientExportedVault, Organization_ClientExportedVault. A user could backdate an export to bury it among older activity, or push it far enough into the future that it never appears in the event log's date-range query at all. That is log forgery by the actor the log exists to record, and it is now closed.

This matches the design intent already documented for event logging, that server-side events are preferred because they cannot be circumvented by modifying the client.

Other defects fixed

• A payload that omits date previously bound to DateTime.MinValue and was stored as a year-1 event.
• A future-dated event was stored but fell outside the event log's date-range query, so it could never be retrieved through the UI.

Trade-offs

Events queued on a client that was offline are now recorded at the time the server received them rather than the time the device claims. The upload interval is 60 seconds, so in the normal case this shifts a timestamp by under a minute, well inside the error it removes.

Ordering within a batch of cipher events is preserved, because the fallback is evaluated per event rather than per batch. Note this does not extend to a mixed batch: cipher events are accumulated and flushed after the loop while user and organization events are awaited inline, so a cipher event followed by an organization event will stamp the organization event first. The distortion is milliseconds, against the minutes it replaces.

An alternative that preserves the exact gaps between all events in a batch was considered and rejected as roughly four times the code on an audit path, with a new failure mode (ordering can invert between consecutive batches, which this approach never does).

No feature flag: carrying two timestamp semantics simultaneously would produce a log that is neither. Rollback is a revert. Existing rows written with a skewed device timestamp keep their values, as the true time is not recoverable.

EventModel.Date is kept so existing clients continue to work unchanged, and is now documented as accepted-and-ignored.

Consumers of these events through SIEM or webhook integrations will see the date field change meaning for client-originated events.

🧪 Testing

• 62 unit test cases and 24 integration test cases pass.
• The two integration tests verify end to end that the stored Date is server receipt time, by reading the event back through IEventRepository. Reverting any call site fails exactly those two tests.
• Also covered: a backdated timestamp, a future-dated timestamp, and a payload with no date field at all.

📸 Screenshots

Not applicable, no UI changes.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.71%. Comparing base (27de6da) to head (31343ee).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8296      +/-   ##
==========================================
+ Coverage   63.69%   63.71%   +0.01%     
==========================================
  Files        2470     2470              
  Lines      105798   105840      +42     
  Branches     9562     9581      +19     
==========================================
+ Hits        67389    67431      +42     
  Misses      36090    36090              
  Partials     2319     2319              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This bugfix stops CollectController from forwarding the client-supplied EventModel.Date at its five IEventService call sites, so EventService's per-event date.GetValueOrDefault(DateTime.UtcNow) fallback in LogUserEventAsync, LogOrganizationEventAsync, LogOrganizationUserEventAsync, and BuildCipherEventMessageAsync stamps every collected event with server time. Removing a client-controlled value from an audit path closes timestamp forgery on Cipher_ClientCopiedPassword and the vault-export events, and it also eliminates the year-1 rows written when a payload omitted date. Overload resolution still binds to the intended methods after the argument removal — the EventSystemUser overloads have no default for that parameter and remain inapplicable at two arguments — and eventModel.Date no longer appears anywhere in src/Events, so no call site was missed.

The two integration tests read the stored row back through IEventRepository.GetManyByUserAsync and assert the saved Date is at or after the pre-request timestamp, which fails on both the backdated and the absent-date regression. Each xUnit test instance constructs its own EventsApplicationFactory with a fresh SqliteTestDatabase and a uniquely-emailed user, and WebApplicationFactoryBase swaps in RepositoryEventWriteService, so the Assert.Single lookup is isolated and the write is durable before the assertion runs.

Code Review Details

No findings.

The offline-batch timestamp shift, the cipher-versus-organization ordering within a mixed batch, the deliberate absence of a feature flag, the date field changing meaning for SIEM and webhook consumers, and retaining EventModel.Date as an accepted-and-ignored property are all documented trade-offs in the PR description with sound rationale, so they are not raised as findings.