[PM-37944] Differentiate Send events according to domain

[harr1424]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-37944

📔 Objective

This PR introduces capability to differentiate when an email-verified Send is created and/or accessed based upon if the accessor email matches a domain claimed by a given organization.

companion clients PR

[codecov]

Codecov Report

❌ Patch coverage is 73.42193% with 80 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.87%. Comparing base (0352b1f) to head (9b85808).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...Repositories/Queries/EventReadPageBySendIdQuery.cs	0.00%	21 Missing ⚠️
...tityFramework/Dirt/Repositories/EventRepository.cs	0.00%	19 Missing ⚠️
src/Core/Dirt/Models/Data/EventTableEntity.cs	0.00%	15 Missing ⚠️
...ucture.Dapper/Dirt/Repositories/EventRepository.cs	0.00%	14 Missing ⚠️
.../Dirt/Repositories/TableStorage/EventRepository.cs	0.00%	4 Missing ⚠️
...Core/Dirt/Services/Implementations/EventService.cs	93.93%	3 Missing and 1 partial ⚠️
...t/Services/NoopImplementations/NoopEventService.cs	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7690      +/-   ##
==========================================
+ Coverage   61.34%   65.87%   +4.52%     
==========================================
  Files        2244     2247       +3     
  Lines       98682    98968     +286     
  Branches     8915     8939      +24     
==========================================
+ Hits        60537    65193    +4656     
+ Misses      36008    31540    -4468     
- Partials     2137     2235      +98     

☔ 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.

[harr1424]

@claude please review

[claude]

Claude finished @harr1424's task in 5m 10s —— View job

---

Claude Code Review

• Read PR diff and CLAUDE.md
• Review new SendEventClassifier service
• Review EventService changes
• Review controller / API changes
• Review repository + SQL/EF migrations
• Review tests

Overall this is a solid, well-tested feature. Per-org attribution is cleanly modeled, the new event row carries a SendId, and tests exercise the meaningful permutations (member / claimed-domain / external, base row vs org rows, empty context, invited members). A few concrete things worth addressing before merging — most are performance- or polish-level; none are blocking on correctness.

---

Findings

1. N+1 DB roundtrips per org access (perf) — src/Core/Tools/SendFeatures/Services/SendEventClassifier.cs:53-72

BuildAccessContextAsync iterates the owner's confirmed orgs and awaits GetByOrganizationEmailAsync once per org. For an owner in many orgs this is sequential I/O on the hot path of every authenticated Send access. Options:

• Run the per-org lookups in parallel with Task.WhenAll (cheap, no API change).
• Better: add a single batched call that resolves accessor membership across N orgs in one query (mirrors the batched GetVerifiedDomainsByOrganizationIdsAsync you already use for domains). This matches the perf pattern the rest of the file follows.

Send access is anonymous / unauthenticated abuse-prone — keeping the work per access bounded is worth doing now rather than as a follow-up.

2. Missing index on [Event].[SendId] (perf) — src/Sql/dbo/Dirt/Tables/Event.sql:31 and util/Migrator/DbScripts/2026-06-11_00_AddEventSendId.sql

Event_ReadPageBySendId (src/Sql/dbo/Dirt/Stored Procedures/Event_ReadPageBySendId.sql:12-24) filters on OrganizationId + SendId + Date. The only existing nonclustered index is IX_Event_DateOrganizationIdUserId which doesn't include SendId, so this lookup will scan the org's date window and post-filter. Send rows are a tiny minority of the Event table, so a filtered index is a near-free win:

CREATE NONCLUSTERED INDEX [IX_Event_OrganizationIdSendIdDate]
    ON [dbo].[Event] ([OrganizationId], [SendId], [Date] DESC)
    WHERE [SendId] IS NOT NULL

Add it in both src/Sql/dbo/... and a migration script, plus matching EF migrations (per .claude/rules/database-ef.md, both ORM tracks must agree).

3. Redundant work in anonymous Access path — src/Api/Tools/Controllers/SendsController.cs:132-144 and :197-207

BuildAccessContextAsync(..., accessorEmail: null) is guaranteed to return the static empty dictionary, then the controller forwards it to LogSendEventAsync. You can call LogSendEventAsync with null directly and skip the classifier hop entirely for anonymous access. The behavior in EventService.LogSendEventAsync:737-742 is identical for null vs empty dictionary (both treat every org as External), but the no-op classifier call costs a DI resolution / await per anonymous access.

4. Redundant case normalization — src/Core/Tools/SendFeatures/Services/SendEventClassifier.cs:82-87

LoadClaimedDomainsByOrgAsync both calls .ToLowerInvariant() on each domain and constructs the HashSet with StringComparer.OrdinalIgnoreCase. Pick one. Since accessorDomain is already lowercased in ExtractDomain and the lookup at :66 uses Contains(accessorDomain), either approach works — but doing both reads as defensive duplication.

5. Provider rows for access events still credit the owner — src/Core/Dirt/Services/Implementations/EventService.cs:758-767

For Send access events, the provider row sets ActingUserId = sendOwnerUserId, which will surface in the provider's event log as the owner having accessed their own Send — not what happened. Two reasonable resolutions:

• Skip provider rows for access events (pass null or omit), matching the External treatment for non-context orgs.
• Apply the same External / claimed-domain attribution to providers.

If the product decision is "providers don't see Send-access detail," option 1 is cleaner.

6. Pre-existing reviewer comment looks resolved — src/Core/Tools/SendFeatures/Commands/NonAnonymousSendCommand.cs:74, 84, 101

The earlier github-code-quality warning about send.UserId.Value being dereferenced after a nullable check is no longer present — all three log helpers guard with !send.UserId.HasValue before using .Value. Worth acknowledging on the original comment so it can be marked resolved.

---

Smaller notes

• src/Core/Dirt/Services/Implementations/EventService.cs:710-711 — LogSendEventAsync is the only public method on IEventService that takes an IReadOnlyDictionary<,>. Consider whether the optional parameter default + sentinel-null is clearer than two overloads (LogSendEventAsync(ownerId, sendId, type) and LogSendEventAsync(ownerId, sendId, type, context)). Either reads fine; current shape is acceptable.
• src/Api/Tools/Controllers/SendsController.cs:132,197,314,354 — the _featureService.IsEnabled(FeatureFlagKeys.SendEventLogging) check repeats in four places. Not worth a helper unless this list grows.
• The classifier returns IReadOnlyDictionary<Guid, SendAccessEventOrgContext> — good zero-knowledge / least-privilege shape; only the domain (not the accessor's full email) ever lands in the event log, which keeps PII out of Event.DomainName. Worth calling out in the PR description.
• Migration files 2026-06-11_00_*.sql and 2026-06-11_01_*.sql follow the dated-migration convention and are idempotent (COL_LENGTH(...) IS NULL, CREATE OR ALTER PROCEDURE). 👍

---

Tests

Test coverage is the strongest part of this PR. SendEventClassifierTests exercises null/empty/no-@ accessor, owner with no orgs, member, claimed-domain, external, case-insensitivity, invited-only members, and the invited-org filter on GetVerifiedDomainsByOrganizationIdsAsync. EventServiceTests covers the base-row + per-org attribution shape (member / claimed / external) and the no-context create/edit/delete path. SendsControllerTests updates the LogUserEventAsync → LogSendEventAsync call shape and adds positive checks that the email claim is forwarded. Consider adding a test that verifies the anonymous Access path passes accessorEmail: null (currently implicit).

---

· branch tools/send-access-event-logging-domains

[mkincaid-bw]

👍I just wanted to leave a note on this. We have some self-host customers with very large Event tables. I did a test locally to simulate this and found that this index took about a minute to build. This is because the SendId is added as NULL, and this index is filtered where SendId is NOT NULL, so the CREATE INDEX essentially just needs to scan the Event table and create an empty index.

[Banrion]

✅ DIRT Files look good

⚠️ I did a local Claude review along with my DIRT file review, and it found one bug that I thought was important share

[mcamirault]

LGTM

[sonarqubecloud]

Quality Gate passed

Issues
12 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
5.8% Duplication on New Code

See analysis details on SonarQube Cloud